OpenSea Seaport contest - twojoy's results

1. Missing zero address check of initialOwner in createConduit of ConduitController.sol

• Consider adding zero address checks while setting initialOwner.

function createConduit(bytes32 conduitKey, address initialOwner)
        external
        override
        returns (address conduit)
    {
       ......
       //recommendation
       if(initialOwner == address(0){
        revert ZeroAddressError();
        .....
     }

• ConduitController.sol#L57

2. Use specific memory offset variables instead of generic offset in inline assembly

In contracts/lib/GettersAndDerivers.sol#L68

While writing assembly, use the specific memory offset variable for specific structs instead of using generic offsets such as OneWords TwoWords. This make the code easy to read/understand and reduces chances of error on change.

  // Current

  // Get the pointer to the offers array.
  let offerArrPtr := mload(add(orderParameters, TwoWords))

  
  // Recommended Change:

  // Get the pointer to the offers array.
  let offerArrPtr := mload(add(orderParameters, OrderParameters_offer_head_offset))

1. keccak256 hash computation can be precomputed to save deployment gas

• In _deriveTypehashes() of ConsiderationBase.sol the keccak256 hash of the encoded values can be precomputed.

........
eip712DomainTypehash = keccak256(
            abi.encodePacked(
                "EIP712Domain(",
                    "string name,",
                    "string version,",
                    "uint256 chainId,",
                    "address verifyingContract",
                ")"
            )
....


// recommendation: precompute

eip712DomainTypehash = 0x8b73c3c69bb8fe3d512ecc4cf759cc79239f7b179b0ffacaa9a75d522b39400f 

2. Use prefix increment/decrement operation whenever possible

• Use of prefix increment/decrement operations save gas compared to postfix operation. This can be used to save gas.
• contracts/lib/OrderCombiner.sol#L229
• contracts/lib/OrderCombiner.sol#L490
• contracts/lib/OrderCombiner.sol#L515

3. array length in loops can be cached instead of calculating in every iteration

The loop bounds are calculated with array.length which are calculated in every loop iterations which can result in high gas. The array length can be cached instead of calculating in every loop iteration to save gas.

// Current
for (uint i = 0; i < offer.length; ++i) {
}

// Recommendation
uint len = offer.length;
for (uint i = 0; i < len; ++i) {
}

The instances where this pattern can be applied is found as follows

❯ grep -rn './contracts/' -e 'for.*[.]length'
./contracts/lib/OrderFulfiller.sol:217:            for (uint256 i = 0; i < orderParameters.offer.length; ) {
./contracts/lib/OrderFulfiller.sol:306:            for (uint256 i = 0; i < orderParameters.consideration.length; ) {
./contracts/lib/OrderCombiner.sol:247:                for (uint256 j = 0; j < offer.length; ++j) {
./contracts/lib/OrderCombiner.sol:291:                for (uint256 j = 0; j < consideration.length; ++j) {
./contracts/lib/OrderCombiner.sol:598:                for (uint256 j = 0; j < consideration.length; ++j) {
./contracts/lib/OrderCombiner.sol:621:        for (uint256 i = 0; i < executions.length; ) {