HardlyDifficult
commented
2 years ago 1) Checking _assertCallerIsConduitOwner may be an intentional redundancy in order to surface a more useful error message in that scenario. This shouldn't be a significant savings and may not be desirable.
2) The optimizer should now handle this case automatically -- I tested this to confirm it's not helpful, some got a tiny bit better other functions a bit worse.
Closing as invalid since there are no clear wins here.
1)Remove _assertConduitExists() in Function _assertCallerIsConduitOwner() to Save Gas
Impact
Function _assertCallerIsConduitOwner() First Check Existing of Conduit and then Only Check for Its Owner.
Actually if Owner was found, then the Conduit is exist. The First Check for Existing of Conduit is not necessary.
Proof of Concept
https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/conduit/ConduitController.sol#L482
Recommended Mitigation Steps
Remove Line482: _assertConduitExists(conduit);
2) Variable Default Value is 0 and thus Initialized to 0 is Waste of Gas
Impact
The local variable need not be initialized to 0 because the default value is 0. Avoiding this anti-pattern can save a few opcodes and therefore a tiny bit of gas.
Proof of Concept
https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/conduit/Conduit.sol#L66 https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/conduit/Conduit.sol#L130 https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/lib/BasicOrderFulfiller.sol#L948 https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/lib/BasicOrderFulfiller.sol#L1040 More...
Recommended Mitigation Steps
Remove explicit 0 initialization
Before: for (uint256 i = 0; i < totalStandardTransfers; )
After: for (uint256 i; i < totalStandardTransfers; )