code-423n4 / 2021-09-yaxis-findings

0 stars 0 forks source link

`tokens[i]` can be memorized #143

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Handle

0xsanson

Vulnerability details

Impact

In StablesConverter.convert there are multiple storage reads of tokens[i] that add up gas. Consider saving the variable in memory.

Tools Used

editor

Recommended Mitigation Steps

Rewrite

IERC20 _token;    // add this
for (uint8 i = 0; i < 3; i++) {
    _token = tokens[i];  // add this
    //if (_output == address(tokens[i])) {
    if (_output == address(_token)) {
        //uint256 _before = tokens[i].balanceOf(address(this));
        uint256 _before = _token.balanceOf(address(this));
        stableSwap3Pool.remove_liquidity_one_coin(
            _inputAmount,
            i,
            _estimatedOutput
        );
        //uint256 _after = tokens[i].balanceOf(address(this));
        uint256 _after = _token.balanceOf(address(this));
        _outputAmount = _after.sub(_before);
        //tokens[i].safeTransfer(msg.sender, _outputAmount);
        _token.safeTransfer(msg.sender, _outputAmount);
        return _outputAmount;
    }
}
uN2RVw5q commented 2 years ago

Consider saving the variable in memory.

It should be stack instead of memory. But the suggestion is still correct.

uN2RVw5q commented 2 years ago

Implemented in https://github.com/code-423n4/2021-09-yaxis/pull/31

GalloDaSballo commented 2 years ago

Sponsor mitigated

Quoting from #56 Warm SLOADs cost 100 gas and CALLs cost 2600 gas after Berlin upgrade. MLOADs cost only 3 gas units.