QA Report

[code423n4]

LOW

vulnerable solidity compiler version

Seaport uses 0.8.13 while there are known bugs discovered in that version affecting arrays of dynamic types & Data Location, so compiler version should be updated to atleast 0.8.14 on both reference and optimized.

Implementation does not match comment

Location: https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/lib/Executor.sol#L541-L544

Comment is wrong stating "Ensure result was extracted and matches EIP-1271 magic value." while it should match value ConduitInterface.execute.selector

NON-CRITICAL

• _performERC20Transfer does zero code size check after executing the call, while _performERC721Transfer and _performERC1155Transfer does at vert begining of execution, so it should be normalized depending upon business logic.

GAS OPTIMIZATIONS

• https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/lib/BasicOrderFulfiller.sol#L86 , instead of div bit shift >> 2 can be used to reduce runtime gas by 1 unit
• Make constructors payable to reduce addition checks and hence removing some opcodes

[HardlyDifficult]

Merging with https://github.com/code-423n4/2022-05-opensea-seaport-findings/issues/206

[HardlyDifficult]

vulnerable solidity compiler version

Seaport 1.1 was deployed with 0.8.14

Implementation does not match comment

The comment could be fixed / improved here.

_performERC20Transfer does zero code size check after executing the call

I believe the differences here were intentional. When data is returned (as is typically the case with ERC20) then checking the code size is redundant.

div bit shift >> 2

This could be included for a very small savings.

Make constructors payable

This is poor style IMO. And optimizing the constructor doesn't help end-users so this is not an important path to optimize.

EIP1271 signature will fail when length is 65

See comments in https://github.com/code-423n4/2022-05-opensea-seaport-findings/issues/206