Cyfrin / 2023-07-foundry-defi-stablecoin

37 stars 32 forks source link

Can return from the `getUsdValue()` method if amount is 0 #1135

Closed codehawks-bot closed 1 year ago

codehawks-bot commented 1 year ago

Can return from the getUsdValue() method if amount is 0

Severity

Gas Optimization / Informational

Relevant GitHub Links

https://github.com/Cyfrin/2023-07-foundry-defi-stablecoin/tree/main/src/DecentralizedStableCoin.sol

Summary

Can return from the getUsdValue() method if amount is 0

Vulnerability Details

If the amount passed is 0, then the USD value will be 0. There is no point in calling the oracle and performing bunch of operations just to multiply the final answer with 0.

File: src/DSCEngine.sol

361:    function getUsdValue(address token, uint256 amount) public view returns (uint256) {
            AggregatorV3Interface priceFeed = AggregatorV3Interface(s_priceFeeds[token]);
            (, int256 price,,,) = priceFeed.staleCheckLatestRoundData();
            // 1 ETH = $1000
            // The returned value from CL will be 1000 * 1e8
            return ((uint256(price) * ADDITIONAL_FEED_PRECISION) * amount) / PRECISION;
        }

Link to code - https://github.com/Cyfrin/2023-07-foundry-defi-stablecoin/blob/main/src/DSCEngine.sol#L361

Tools Used

Code Review using VSCode

Recommendations

Add amount == 0 check and return early.

Code change:

File: src/DSCEngine.sol

361:    function getUsdValue(address token, uint256 amount) public view returns (uint256) {
+           if (amount == 0) {
+              return 0;
+           }
            AggregatorV3Interface priceFeed = AggregatorV3Interface(s_priceFeeds[token]);
            ...
            ...
            ...
        }
PatrickAlphaC commented 1 year ago

This is an edge case, and I think more often than not, it will not be called with 0 tokens, so the extra check is actually a waste of gas.