code-423n4 / 2022-06-notional-coop-findings

1 stars 1 forks source link

Gas Optimizations #223

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

_isWrappedFCash involves reasonably large gas usage and is called for each position twice. It is enough to call it once and save the results in memory allocated array for reuse.

Proof of Concept

_isWrappedFCash is called 2 times more often than it's needed:

https://github.com/code-423n4/2022-06-notional-coop/blob/6f8c325f604e2576e2fe257b6b57892ca181509a/index-coop-notional-trade-module/contracts/protocol/modules/v1/NotionalTradeModule.sol#L596-L626

    function _getFCashPositions(ISetToken _setToken)
    internal
    view
    returns(address[] memory fCashPositions)
    {
        ISetToken.Position[] memory positions = _setToken.getPositions();
        uint positionsLength = positions.length;
        uint numFCashPositions;

        for(uint256 i = 0; i < positionsLength; i++) {
            // Check that the given position is an equity position
            if(positions[i].unit > 0) {
                address component = positions[i].component;
                if(_isWrappedFCash(component)) {
                    numFCashPositions++;
                }
            }
        }

        fCashPositions = new address[](numFCashPositions);

        uint j;
        for(uint256 i = 0; i < positionsLength; i++) {
            if(positions[i].unit > 0) {
                address component = positions[i].component;
                if(_isWrappedFCash(component)) {
                    fCashPositions[j] = component;
                    j++;
                }
            }
        }

Recommended Mitigation Steps

Consider saving the flags to memory array and use it instead of calling _isWrappedFCash in the second cycle.