code-423n4 / 2022-05-opensea-seaport-findings

1 stars 0 forks source link

Gas Optimizations #160

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

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

........
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

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; ) {   
HardlyDifficult commented 2 years ago

keccak256 hash computation can be precomputed to save deployment gas

Given that this only impacts the constructor and therefore will not impact users, it seems reasonable to leave the code as-is for improved readability. If the precomputed form was used one would need to run the hash manually in order to validate it is as expected.

The other two findings should provide small savings.