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

1 stars 0 forks source link

Gas Optimizations #112

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

GAS Report

For-Loops: Cache array length outside of loops

Reading an array length at each iteration of the loop takes 6 gas (3 for mload and 3 to place memory_offset) in the stack.

Caching the array length in the stack saves around 3 gas per iteration.

For example:

for (uint256 i; i < arr.length; ++i) {}

can be changed to:

uint256 len = arr.length;
for (uint256 i; i < len; ++i) {}

Consider making the following change to these lines:

contracts/lib/OrderCombiner.sol:
 247:        for (uint256 j = 0; j < offer.length; ++j) {
 291:        for (uint256 j = 0; j < consideration.length; ++j) {
 598:        for (uint256 j = 0; j < consideration.length; ++j) {
 621:        for (uint256 i = 0; i < executions.length; ) {

contracts/lib/OrderFulfiller.sol:
 217:        for (uint256 i = 0; i < orderParameters.offer.length; ) {
 306:        for (uint256 i = 0; i < orderParameters.consideration.length; ) {

Arithmetics: ++i costs less gas compared to i++ or i += 1

++i costs less gas compared to i++ or i += 1 for unsigned integers, as pre-increment is cheaper (about 5 gas per iteration). This statement is true even with the optimizer enabled.

i++ increments i and returns the initial value of i. Which means:

uint i = 1;  
i++; // == 1 but i == 2  

But ++i returns the actual incremented value:

uint i = 1;  
++i; // == 2 and i == 2 too, so no need for a temporary variable  

In the first case, the compiler has to create a temporary variable (when used) for returning 1 instead of 2, thus it costs more gas.

The same logic applies for --i and i--.

Consider using ++i instead of i++ or i += 1 in the following instances:

contracts/lib/OrderCombiner.sol:
 229:        maximumFulfilled--;
 490:        totalFilteredExecutions += 1;
 515:        totalFilteredExecutions += 1;
 768:        totalFilteredExecutions += 1;

Unnecessary initialization of variables with default values

Uninitialized variables are assigned with a default value depending on its type:

Thus, explicitly initializing a variable with its default value costs unnecesary gas. For example, the following code:

bool b = false;
address c = address(0);
uint256 a = 0;

can be changed to:

uint256 a;
bool b;
address c;

Consider declaring the following lines without explicitly setting a value:

contracts/lib/TokenTransferrerConstants.sol:
  52:        uint256 constant ERC20_transferFrom_sig_ptr = 0x0;
  64:        uint256 constant ERC1155_safeTransferFrom_sig_ptr = 0x0;
  86:        uint256 constant ERC721_transferFrom_sig_ptr = 0x0;
  96:        uint256 constant NoContract_error_sig_ptr = 0x0;
 106:        uint256 constant TokenTransferGenericFailure_error_sig_ptr = 0x0;
 127:        uint256 constant BadReturnValueFromERC20OnTransfer_error_sig_ptr = 0x0;
 163:        uint256 constant Invalid1155BatchTransferEncoding_ptr = 0x00;

contracts/lib/OrderCombiner.sol:
 470:        uint256 totalFilteredExecutions = 0;
 751:        uint256 totalFilteredExecutions = 0;

contracts/lib/ConsiderationConstants.sol:
 282:        uint256 constant NoContract_error_sig_ptr = 0x0;

contracts/lib/AmountDeriver.sol:
  44:        uint256 extraCeiling = 0;

Unnecessary definition of variables

Some variables are defined even though they are only used once in their respective functions. Not defining these variables can help to reduce gas cost and contract size.

Instances include:

contracts/conduit/ConduitController.sol:
 425:        uint256 totalChannels = _conduits[conduit].channels.length;

contracts/conduit/Conduit.sol:
  68:        ConduitTransfer calldata standardTransfer = transfers[i];
 132:        ConduitTransfer calldata standardTransfer = standardTransfers[i];

contracts/lib/Executor.sol:
 603:        bytes4 selector = ConduitInterface.execute.selector;

contracts/lib/GettersAndDerivers.sol:
 243:        address conduitController = address(_CONDUIT_CONTROLLER);
 246:        bytes32 conduitCreationCodeHash = _CONDUIT_CREATION_CODE_HASH;

contracts/lib/AmountDeriver.sol:
 114:        uint256 valueTimesNumerator = value * numerator;
HardlyDifficult commented 2 years ago

Unnecessary initialization of variables with default values

Tested and didn't find this to be helpful.

The others should provide small savings.