Gas: `XDEFIDistribution:withdrawableOf()` optimization

[code423n4]

Handle

Dravee

Vulnerability details

Impact

Caching state variable in local variables for repeated reads saves gas by converting expensive SLOADs into much cheaper MLOADs

SLOADs cost 2100 gas for first time reads of state variables and then 100 gas for repeated reads in the context of a transaction (post Berlin fork). MLOADs cost 3 gas units. Therefore, caching state variable in local variables for repeated reads saves gas.

Proof of Concept

The following function:

    function withdrawableOf(uint256 tokenId_) public view returns (uint256 withdrawableXDEFI_) {
        Position storage position = positionOf[tokenId_];
        return _withdrawableGiven(position.units, position.depositedXDEFI, position.pointsCorrection);
    }

can be optimized to:

    function withdrawableOf(uint256 tokenId_) public view returns (uint256 withdrawableXDEFI_) {
        Position memory position = positionOf[tokenId_];
        return _withdrawableGiven(position.units, position.depositedXDEFI, position.pointsCorrection);
    }

Because there won't be any state variable update in _withdrawableGiven (the values are read in storage and passed as memory)

Tools Used

VS Code

Recommended Mitigation Steps

Cache the position variable in memory for this read-only operation

[deluca-mike]

This is actually more expensive in terms of deploy costs and runtime costs, because the compiler is already optimizing reading the struct from storage and destruction it onto the stack, since it knows what will be needed. using memory results in a copy to memory, and then a read from memory, which benefits from no such optimization. If you change both storage instances to memory and re-run hardhat tests, you'll see form the gas report that it is more expensive.

[Ivshti]

agreed with sponsor's assessment