Gas Optimizations

[code423n4]

Define function pointers in constructor

Impact

The function _applyFractionsAndTransferEach() contains two function pointer assignments, which are difficult to read. They could also be set in the constructor and stored immutable, which will make the code more readable and save some gas.

Proof of Concept

Context: OrderFulfiller.sol#L199-L214, OrderFulfiller.sol#L289-L303

contract OrderFulfiller is ...
    function _applyFractionsAndTransferEach(...) ... {
        ...
        function(OfferItem memory, address, bytes32, bytes memory) internal _transferOfferItem;
        function(ReceivedItem memory, address, bytes32, bytes memory) internal _transferReceivedItem = _transfer;
        assembly {  _transferOfferItem := _transferReceivedItem  }
        ...
        function(ConsiderationItem memory, address, bytes32, bytes memory) internal _transferConsiderationItem;
        function(ReceivedItem memory, address, bytes32, bytes memory) internal _transferReceivedItem = _transfer;
        assembly {   _transferConsiderationItem := _transferReceivedItem  }
    }
}

Tools Used

Manual review

Recommended Mitigation Steps

Consider setting the function pointers in the constructor, as shown in the following example:

//SPDX-License-Identifier: MIT
pragma solidity 0.8.14;
import "hardhat/console.sol"; 

contract TestFunctionPointers{
    struct OfferItem    { uint256 startAmount; }
    struct ReceivedItem { uint256 identifier;  }
    function _transfer(OfferItem memory oi) public {
        console.log("In _transfer",oi.startAmount);
    }
    function(ReceivedItem memory) internal immutable _transferReceivedItem;
    constructor() {
        function(OfferItem memory)    internal fpa = _transfer;
        function(ReceivedItem memory) internal fpb;
        assembly { fpb := fpa }
        _transferReceivedItem = fpb;
        ReceivedItem memory ri;
        ri.identifier = 1234567;
        _transferReceivedItem(ri);
    }  
}

Gas saving in _verifyProof()

Impact

The function _verifyProof() of contract CriteriaResolution uses a switch statement. This can be optimized to run withouth a jump to save some gas. As this is within a loop, it might be worth the effort.

Proof of Concept

Context: CriteriaResolution.sol#L265-L273

This is the current code:

function _verifyProof( ... ) ... {
    ...
    assembly {
        ...
        for { ... } { ... }
            ...
            switch gt(computedHash, loadedData)  // here is a jump
                case 0 {
                    mstore(0, computedHash) // Place existing hash first.
                    mstore(0x20, loadedData) // Place new hash next.
                }
                default {
                    mstore(0, loadedData) // Place new hash first.
                    mstore(0x20, computedHash) // Place existing hash next.
                }
           ...
    }
}

Tools Used

Manual review

Recommended Mitigation Steps

Consider using the following:

assembly {
    let g := mul( gt(computedHash, loadedData), 0x20)
    mstore(g, computedHash )
    mstore(sub(0x20,g), loadedData )
}

Here is some test code to see the difference:

//SPDX-License-Identifier: MIT
pragma solidity 0.8.14;
import "hardhat/console.sol"; 

contract TestStore{
    function min(uint a,uint b) internal pure returns(uint) {
        if (a<b) return a; else return b;
    }

    function t1(uint computedHash , uint loadedData) internal {
        assembly {
            switch gt(computedHash, loadedData)
                case 0 {
                    mstore(0, computedHash) 
                    mstore(0x20, loadedData) 
                }
                default {
                    mstore(0, loadedData) 
                    mstore(0x20, computedHash) 
                }
        }
    }

    function t2(uint computedHash , uint loadedData) internal {
        assembly {
            let g := mul( gt(computedHash, loadedData), 0x20)
            mstore(g, computedHash )
            mstore(sub(0x20,g), loadedData )
        }
    }

    constructor()  {
        uint n=11;uint g1;uint g2;uint g=type(uint).max;uint i;
        g=type(uint).max;g1=gasleft();for(i=0;i<n;i++) { t1(i,4); } g2=gasleft();g=min(g,g1-g2);console.log(g/n); // 284
        g=type(uint).max;g1=gasleft();for(i=0;i<n;i++) { t2(i,4); } g2=gasleft();g=min(g,g1-g2);console.log(g/n); // 268
    }
}

Gas saving by replacing the if condition with || by a branchless expression

Impact

  if (numerator > denominator || numerator == 0) {
    ...
  }

The condition can be replaced by or(iszero(numerator), gt(numerator, denominator)) which saves one jumpi.

Proof of Concept

Context: Seaport.sol#L137

Tools Used

Manual review

Recommended Mitigation Steps

Replace the if statement by the equivalent code in assembly (mentioned above).

[HardlyDifficult]

These are interesting suggestions to consider. They seem to have small impact, but go deep into the code in order to make non-obvious recommendations.