fix(priceConverter): Correct multiplication factor in getPrice function - Githubissues

PatrickAlphaC / hardhat-fund-me-fcc

82 stars 184 forks source link

fix(priceConverter): Correct multiplication factor in getPrice function #188

Closed skarthik05 closed 6 months ago

skarthik05 commented 6 months ago

Description: This pull request corrects a potential error in the getPrice function within the PriceConverter library. The function is responsible for converting the answer from the price feed to a uint256 value representing the ETH/USD rate.

Changes:

Updated the multiplication factor from 10000000000 to 10**18 to ensure that the result is in 18 decimal places.
This change aligns the function with standard practices for handling decimal values in Ethereum smart contracts.

Impact:

This change ensures the accuracy of the price conversion in contracts utilizing the PriceConverter library.
It resolves potential discrepancies that could arise due to incorrect multiplication factors.

Reviewer: @PatrickAlphaC

PatrickAlphaC commented 6 months ago

Hi, thanks for the PR! However, this doesn't make sense.

The ETH/USD price feed has 8 decimal places, see here.

Screenshot 2024-03-30 at 4 10 18 PM

This means, that when we get a price back, let's say the price of ETH is $2,000. We'd get 2000 * (10 8). Because of this, we need to multiply this answer again by (10 10) to get 18 decimal places.

This PR would make the answer (10 * 8) (10 ** 18) which is WAY too big!