OpenSea Seaport contest - zkhorse's results

Low/Informational

Contract name leaks free memory pointer in padding bytes

Seaport.name() includes a trailing 0x80 byte (the free memory pointer), rather than padding the returned string value with zero bytes as required in the ABI encoding specification. For example, see the output of cast call:

$ cast call --rpc-url $(rpc mainnet) 0x00000000006cee72100d161c57ada5bb2be1ca79 'name()'

0x00000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000007536561706f727400000000000000000000000000000000000000000000000080

In practice, most ABI decoders will respect the encoded length and ignore nonzero trailing bytes included as padding, but some (notably Etherscan) will display an invalid UTF-8 character at the end of the name string:

This is also the case for the default name defined in ConsiderationBase#_name. Since this client behavior is inconsistent, consider ensuring padding consists only of zero bytes.

QA

Replace magic numbers in CriteriaResolution#_verifyProof

Inline assembly in CriteriaResolution#_verifyProof uses the magic number 0x20 as an mstore offset in Merkle hash computation:

CriteriaResolution.sol#L264-L273

                // Sort and store proof element and hash.
                switch gt(computedHash, loadedData)
                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.
                }

Consider replacing 0x20 with the existing constant OneWord to improve readability.

Replace magic number in Executor#_triggerIfArmed

The check in Executor#_triggerIfArmed uses 64 as a special sentinel value indicating that the accumulator is armed:

Executor.sol#L433-L436

        // Exit if the accumulator is not "armed".
        if (accumulator.length != 64) {
            return;
        }

However, the check inside _insert instead uses an AccumulatorDisarmed constant, and writes this sentinel value using an AccumulatorArmed constant:

Executor.sol#L601-L623

        uint256 elements;
        // "Arm" and prime accumulator if it's not already armed. The sentinel
        // value is held in the length of the accumulator array.
        if (accumulator.length == AccumulatorDisarmed) {
            elements = 1;
            bytes4 selector = ConduitInterface.execute.selector;
            assembly {
                mstore(accumulator, AccumulatorArmed) // "arm" the accumulator.
                mstore(add(accumulator, Accumulator_conduitKey_ptr), conduitKey)
                mstore(add(accumulator, Accumulator_selector_ptr), selector)
                mstore(
                    add(accumulator, Accumulator_array_offset_ptr),
                    Accumulator_array_offset
                )
                mstore(add(accumulator, Accumulator_array_length_ptr), elements)
            }
        } else {
            // Otherwise, increase the number of elements by one.
            assembly {
                elements := add(
                    mload(add(accumulator, Accumulator_array_length_ptr)),
                    1
                )
                mstore(add(accumulator, Accumulator_array_length_ptr), elements)
            }
        }

Consider replacing the magic number 64 with the existing AccumulatorArmed constant for consistency and readability.

Typos

• amount in Assertions.sol#L92
• and in ConsiderationStructs.sol#L68

ERC20 Transfer Tests

Our team wrote differential fuzz tests to compare the optimized TokenTransferrer against the ReferenceTokenTransferrer using a list of 162 common ERC20 tokens based on the Uniswap token list.

Although we didn’t find anything (besides approval and storage slot edge cases in a number of weird ERC20s that Seaport handles correctly), we are including them in case they are useful. Note that they must be run in Foundry fork mode, using the --fork-url flag.

Reuse efficient hash function from GettersAndDerivers#_deriveConduit in ConduitController

ConduitController#createConduit and ConduitController#getConduit both calculate the deterministic conduit address using abi.encodePacked, keccak256, and type conversions:

ConduitController#createConduit

    function createConduit(bytes32 conduitKey, address initialOwner)
        external
        override
        returns (address conduit)
    {
        // If the first 20 bytes of the conduit key do not match the caller...
        if (address(uint160(bytes20(conduitKey))) != msg.sender) {
            // Revert with an error indicating that the creator is invalid.
            revert InvalidCreator();
        }

        // Derive address from deployer, conduit key and creation code hash.
        conduit = address(
            uint160(
                uint256(
                    keccak256(
                        abi.encodePacked(
                            bytes1(0xff),
                            address(this),
                            conduitKey,
                            _CONDUIT_CREATION_CODE_HASH
                        )
                    )
                )
            )
        );
        ...
    }

ConduitController#getConduit

    function getConduit(bytes32 conduitKey)
        external
        view
        override
        returns (address conduit, bool exists)
    {
        // Derive address from deployer, conduit key and creation code hash.
        conduit = address(
            uint160(
                uint256(
                    keccak256(
                        abi.encodePacked(
                            bytes1(0xff),
                            address(this),
                            conduitKey,
                            _CONDUIT_CREATION_CODE_HASH
                        )
                    )
                )
            )
        );

        // Determine whether conduit exists by retrieving its runtime code.
        exists = (conduit.codehash == _CONDUIT_RUNTIME_CODE_HASH);
    }

Reusing the same efficient hash implementation using scratch space from GettersAndDerivers#_deriveConduit can save gas in these calculations.

GettersAndDerivers#_deriveConduit

        // Leverage scratch space to perform an efficient hash.
        assembly {
            // Retrieve the free memory pointer; it will be replaced afterwards.
            let freeMemoryPointer := mload(FreeMemoryPointerSlot)

            // Place the control character and the conduit controller in scratch
            // space; note that eleven bytes at the beginning are left unused.
            mstore(0, or(MaskOverByteTwelve, conduitController))

            // Place the conduit key in the next region of scratch space.
            mstore(OneWord, conduitKey)

            // Place conduit creation code hash in free memory pointer location.
            mstore(TwoWords, conduitCreationCodeHash)

            // Derive conduit by hashing and applying a mask over last 20 bytes.
            conduit := and(
                // Hash the relevant region.
                keccak256(
                    // The region starts at memory pointer 11.
                    Create2AddressDerivation_ptr,
                    // The region is 85 bytes long (1 + 20 + 32 + 32).
                    Create2AddressDerivation_length
                ),
                // The address equals the last twenty bytes of the hash.
                MaskOverLastTwentyBytes
            )

            // Restore the free memory pointer.
            mstore(FreeMemoryPointerSlot, freeMemoryPointer)
        }

You'll need to make one small modification, and use address() directly to access the current address in the first mstore:

        // Read conduit creation code hash from runtime and place on the stack.
        bytes32 conduitCreationCodeHash = _CONDUIT_CREATION_CODE_HASH;

        // Derive address from deployer, conduit key and creation code hash.
        assembly {
            // Retrieve the free memory pointer; it will be replaced afterwards.
            let freeMemoryPointer := mload(FreeMemoryPointerSlot)

            // Place the control character and the deployer address in scratch
            // space; note that eleven bytes at the beginning are left unused.
            mstore(0, or(MaskOverByteTwelve, address()))

            // Place the conduit key in the next region of scratch space.
            mstore(OneWord, conduitKey)

            // Place conduit creation code hash in free memory pointer location.
            mstore(TwoWords, conduitCreationCodeHash)

            // Derive conduit by hashing and applying a mask over last 20 bytes.
            conduit := and(
                // Hash the relevant region.
                keccak256(
                    // The region starts at memory pointer 11.
                    Create2AddressDerivation_ptr,
                    // The region is 85 bytes long (1 + 20 + 32 + 32).
                    Create2AddressDerivation_length
                ),
                // The address equals the last twenty bytes of the hash.
                MaskOverLastTwentyBytes
            )

            // Restore the free memory pointer.
            mstore(FreeMemoryPointerSlot, freeMemoryPointer)
        }

Status quo gas report:

Running 1 test for test/foundry/conduit/ConduitExecute.t.sol:ConduitExecuteTest
[PASS] testExecute(((uint8,address,address,uint128,uint128,uint8)[20])) (runs: 10000, μ: 26293831, ~: 26216823)
Test result: ok. 1 passed; 0 failed; finished in 169.99s
╭────────────────────────────┬─────────────────┬────────┬────────┬────────┬─────────╮
│ ConduitController contract ┆                 ┆        ┆        ┆        ┆         │
╞════════════════════════════╪═════════════════╪════════╪════════╪════════╪═════════╡
│ Deployment Cost            ┆ Deployment Size ┆        ┆        ┆        ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ 2357335                    ┆ 11864           ┆        ┆        ┆        ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ Function Name              ┆ min             ┆ avg    ┆ median ┆ max    ┆ # calls │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ createConduit              ┆ 766687          ┆ 766687 ┆ 766687 ┆ 766687 ┆ 1       │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ getConduitCodeHashes       ┆ 241             ┆ 241    ┆ 241    ┆ 241    ┆ 1       │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ updateChannel              ┆ 70265           ┆ 81215  ┆ 81215  ┆ 92165  ┆ 2       │
╰────────────────────────────┴─────────────────┴────────┴────────┴────────┴─────────╯

Using efficient hash:

Running 1 test for test/foundry/conduit/ConduitExecute.t.sol:ConduitExecuteTest
[PASS] testExecute(((uint8,address,address,uint128,uint128,uint8)[20])) (runs: 10000, μ: 26528857, ~: 26392955)
Test result: ok. 1 passed; 0 failed; finished in 173.44s
╭────────────────────────────┬─────────────────┬────────┬────────┬────────┬─────────╮
│ ConduitController contract ┆                 ┆        ┆        ┆        ┆         │
╞════════════════════════════╪═════════════════╪════════╪════════╪════════╪═════════╡
│ Deployment Cost            ┆ Deployment Size ┆        ┆        ┆        ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ 2342314                    ┆ 11789           ┆        ┆        ┆        ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ Function Name              ┆ min             ┆ avg    ┆ median ┆ max    ┆ # calls │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ createConduit              ┆ 766597          ┆ 766597 ┆ 766597 ┆ 766597 ┆ 1       │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ getConduitCodeHashes       ┆ 241             ┆ 241    ┆ 241    ┆ 241    ┆ 1       │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ updateChannel              ┆ 70265           ┆ 81215  ┆ 81215  ┆ 92165  ┆ 2       │
╰────────────────────────────┴─────────────────┴────────┴────────┴────────┴─────────╯