code-423n4 / 2022-02-badger-citadel-findings

0 stars 0 forks source link

Gas Optimizations #73

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Avoid calling staticcall instruction and power calculation every time someone buys tokens (save 5000 gas)

Every time a users calls the function buy() it will execute a

    tokenOutAmount_ = getAmountOut(_tokenInAmount);                  LOC 175

The function is:

function getAmountOut(uint256 _tokenInAmount) public view returns (uint256 tokenOutAmount_) { tokenOutAmount_ = (_tokenInAmount * 10**tokenOut.decimals()) / tokenOutPrice; }

The tokenOut.decimals() is a contract call, more specifically a staticcall OPCODE that will cost you 700 gas according to https://eips.ethereum.org/EIPS/eip-1380. Apart from that power calculation is also expensive. I have code a PoC contract and mi results is that you would be saving 5000 gas if you use a precomputed variable

The Poc

I have coded these two simple contracts in a single file that you can test easily in remix


 pragma solidity ^0.8.0;

import "https://github.com/OpenZeppelin/openzeppelin-contracts-upgradeable/blob/master/contracts/token/ERC20/ERC20Upgradeable.sol";

contract MyToken is ERC20Upgradeable{

function decimals() public view override returns(uint8){`
    return 8;`
}

}

contract StaticCallGasPoC{
//Document about  GasCost of StaticCall  https://eips.ethereum.org/EIPS/eip-1380

    ERC20Upgradeable public tokenOut;
    uint256 public tokenOutPrice = 40000;
    uint private exchangeRate = 2500;  // 10**tokenOut.decimals() / tokenOutprice;

constructor (ERC20Upgradeable _tokenOut) {
    tokenOut=_tokenOut;

}
function setTokenOutPrice(uint _tokenOutPrice) public{
    tokenOutPrice = _tokenOutPrice;
}

function expensivePower(uint _tokenInAmount) public view returns(uint){
    return (_tokenInAmount * 10**tokenOut.decimals())/ tokenOutPrice;  
    //cost of static call = 700  
}

function cheapPower(uint _tokenInAmount) public view returns(uint){
    return _tokenInAmount*exchangeRate;
}
}

Simulation in remix shows that expensivePower is 5000 gas more expensive that cheapPower

Proposed solution

I would suggest you to use a precomputed variable stored at the time you create your constract because buy() is a function that is highly used and you would be saving a few dollars per transaction

exchangeRate = 10**tokenOut.decimals() / tokenOutPrice

or at least

otherConstant = 10**tokenOut.decimals()

Just to mention that if you use the first option you need to set again exchangeRate when you setTokenOutPrice al L303.

GalloDaSballo commented 2 years ago

I think there's merits to the optimization

That said: STATICCALL is 100 gas

As for the difference in gas cost I believe it's mostly due to how many cold SLOAD the current version would do vs the one you propose. The issue with the proposed solution is that we're doing a division early, meaning we're loosing a lot of precision, this can be fixed by adding a constant multiplier.

I think the finding is valid