search

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

1 stars 0 forks source link

Gas Optimizations #144

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Summary

Gas Optimizations

Issue Instances
1 Using calldata instead of memory for read-only arguments in external functions saves gas 2
2 Avoid contract existence checks by using solidity version 0.8.10 or later 6
3 Multiple accesses of a mapping/array should use a local variable cache 42
4 _assertConduitExists() should return the storage variable it looks up 8
5 internal functions only called once can be inlined to save gas 25
6 <array>.length should not be looked up in every loop of a for-loop 22
7 ++i/i++ should be unchecked{++i}/unchecked{i++} when it is not possible for them to overflow, as is the case when used in for- and while-loops 42
8 Optimize names to save gas 6
9 Using bools for storage incurs overhead 2
10 It costs more gas to initialize variables to zero than to let the default of zero be applied 69
11 ++i costs less gas than i++, especially when it's used in for-loops (--i/i-- too) 6
12 Usage of uints/ints smaller than 32 bytes (256 bits) incurs overhead 8

Total: 238 instances over 12 issues

Gas Optimizations

1. Using calldata instead of memory for read-only arguments in external functions saves gas

When a function with a memory array is called externally, the abi.decode() step has to use a for-loop to copy each index of the calldata to the memory index. Each iteration of this for-loop costs at least 60 gas (i.e. 60 * <mem_array>.length). Using calldata directly, obliviates the need for such a loop in the contract code and runtime execution.

If the array is passed to an internal function which passes the array to another internal function where the array is modified and therefore memory is used in the external call, it's still more gass-efficient to use calldata when the external function uses modifiers, since the modifiers may prevent the internal functions from being called. Structs have the same overhead as an array of length one

There are 2 instances of this issue:

File: reference/ReferenceConsideration.sol   #1

333:          AdvancedOrder[] memory advancedOrders,

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/ReferenceConsideration.sol#L333

File: reference/ReferenceConsideration.sol   #2

447:          AdvancedOrder[] memory advancedOrders,

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/ReferenceConsideration.sol#L447

2. Avoid contract existence checks by using solidity version 0.8.10 or later

Prior to 0.8.10 the compiler inserted extra code, including EXTCODESIZE (700 gas), to check for contract existence for external calls. In more recent solidity versions, the compiler will not insert these checks if the external call has a return value

There are 6 instances of this issue:

File: reference/lib/ReferenceSignatureVerification.sol

97:               EIP1271Interface(signer).isValidSignature(digest, signature) !=

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceSignatureVerification.sol#L97

File: reference/lib/ReferenceExecutor.sol

332           ConduitInterface(_getConduit(accumulatorStruct.conduitKey)).execute(
333               accumulatorStruct.transfers
334:          );

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceExecutor.sol#L332-L334

File: reference/lib/ReferenceZoneInteraction.sol

50                    ZoneInterface(zone).isValidOrder(
51                        orderHash,
52                        msg.sender,
53                        offerer,
54                        zoneHash
55:                   ) != ZoneInterface.isValidOrder.selector

108                       ZoneInterface(zone).isValidOrder(
109                           orderHash,
110                           msg.sender,
111                           offerer,
112                           zoneHash
113:                      ) != ZoneInterface.isValidOrder.selector

119                       ZoneInterface(zone).isValidOrderIncludingExtraData(
120                           orderHash,
121                           msg.sender,
122                           advancedOrder,
123                           priorOrderHashes,
124                           criteriaResolvers
125:                      ) != ZoneInterface.isValidOrder.selector

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceZoneInteraction.sol#L50-L55

File: reference/lib/ReferenceConsiderationBase.sol

83:               tempConduitController.getConduitCodeHashes()

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceConsiderationBase.sol#L83

3. Multiple accesses of a mapping/array should use a local variable cache

The instances below point to the second+ access of a value inside a mapping/array, within a function. Caching a mapping's value in a local storage variable when the value is accessed multiple times, saves ~42 gas per access due to not having to recalculate the key's keccak256 hash (Gkeccak256 - 30 gas) and that calculation's associated stack operations. Caching an array's struct avoids recalculating the array offsets into memory

There are 42 instances of this issue:

File: contracts/conduit/ConduitController.sol

/// @audit _conduits[conduit]
97:           _conduits[conduit].key = conduitKey;

/// @audit _conduits[conduit]
246:          delete _conduits[conduit].potentialOwner;

/// @audit _conduits[conduit]
251:              _conduits[conduit].owner,

/// @audit _conduits[conduit]
256:          _conduits[conduit].owner = msg.sender;

/// @audit _conduits[conduit]
433:          channel = _conduits[conduit].channels[channelIndex];

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/contracts/conduit/ConduitController.sol#L97

File: contracts/lib/CriteriaResolution.sol

/// @audit advancedOrders[orderIndex]
77:                       advancedOrders[orderIndex].parameters

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/contracts/lib/CriteriaResolution.sol#L77

File: contracts/lib/FulfillmentApplier.sol

/// @audit advancedOrders[<etc>]
109               advancedOrders[targetComponent.orderIndex]
110:                  .parameters

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/contracts/lib/FulfillmentApplier.sol#L109-L110

File: contracts/lib/OrderValidator.sol

/// @audit _orderStatus[orderHash]
71:           _orderStatus[orderHash].isCancelled = false;

/// @audit _orderStatus[orderHash]
72:           _orderStatus[orderHash].numerator = 1;

/// @audit _orderStatus[orderHash]
73:           _orderStatus[orderHash].denominator = 1;

/// @audit _orderStatus[orderHash]
227:                  _orderStatus[orderHash].isCancelled = false;

/// @audit _orderStatus[orderHash]
228:                  _orderStatus[orderHash].numerator = uint120(

/// @audit _orderStatus[orderHash]
231:                  _orderStatus[orderHash].denominator = uint120(denominator);

/// @audit _orderStatus[orderHash]
235:              _orderStatus[orderHash].isValidated = true;

/// @audit _orderStatus[orderHash]
236:              _orderStatus[orderHash].isCancelled = false;

/// @audit _orderStatus[orderHash]
237:              _orderStatus[orderHash].numerator = uint120(numerator);

/// @audit _orderStatus[orderHash]
238:              _orderStatus[orderHash].denominator = uint120(denominator);

/// @audit _orderStatus[orderHash]
304:                  _orderStatus[orderHash].isCancelled = true;

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/contracts/lib/OrderValidator.sol#L71

File: reference/conduit/ReferenceConduitController.sol

/// @audit _conduits[conduit]
99:           _conduits[conduit].key = conduitKey;

/// @audit _conduits[conduit]
251:          delete _conduits[conduit].potentialOwner;

/// @audit _conduits[conduit]
256:              _conduits[conduit].owner,

/// @audit _conduits[conduit]
261:          _conduits[conduit].owner = msg.sender;

/// @audit _conduits[conduit]
440:          channel = _conduits[conduit].channels[channelIndex];

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/conduit/ReferenceConduitController.sol#L99

File: reference/lib/ReferenceOrderCombiner.sol

/// @audit ordersToExecute[i]
375               ReceivedItem[] memory receivedItems = ordersToExecute[i]
376:                  .receivedItems;

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceOrderCombiner.sol#L375-L376

File: reference/lib/ReferenceFulfillmentApplier.sol

/// @audit ordersToExecute[<etc>]
118               ordersToExecute[targetComponent.orderIndex]
119:                  .spentItems[targetComponent.itemIndex]

/// @audit offerComponents[startIndex]
272:          uint256 itemIndex = offerComponents[startIndex].itemIndex;

/// @audit offerComponents[i]
313:                      itemIndex = offerComponents[i].itemIndex;

/// @audit considerationComponents[startIndex]
444           potentialCandidate.itemIndex = considerationComponents[startIndex]
445:              .itemIndex;

/// @audit considerationComponents[i]
487                       potentialCandidate.itemIndex = considerationComponents[i]
488:                          .itemIndex;

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceFulfillmentApplier.sol#L118-L119

File: reference/lib/ReferenceOrderValidator.sol

/// @audit _orderStatus[orderHash]
76:           _orderStatus[orderHash].isCancelled = false;

/// @audit _orderStatus[orderHash]
77:           _orderStatus[orderHash].numerator = 1;

/// @audit _orderStatus[orderHash]
78:           _orderStatus[orderHash].denominator = 1;

/// @audit _orderStatus[orderHash]
227:              _orderStatus[orderHash].isCancelled = false;

/// @audit _orderStatus[orderHash]
228:              _orderStatus[orderHash].numerator = uint120(

/// @audit _orderStatus[orderHash]
231:              _orderStatus[orderHash].denominator = uint120(denominator);

/// @audit _orderStatus[orderHash]
234:              _orderStatus[orderHash].isValidated = true;

/// @audit _orderStatus[orderHash]
235:              _orderStatus[orderHash].isCancelled = false;

/// @audit _orderStatus[orderHash]
236:              _orderStatus[orderHash].numerator = uint120(numerator);

/// @audit _orderStatus[orderHash]
237:              _orderStatus[orderHash].denominator = uint120(denominator);

/// @audit _orderStatus[orderHash]
297:              _orderStatus[orderHash].isCancelled = true;

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceOrderValidator.sol#L76

File: reference/lib/ReferenceCriteriaResolution.sol

/// @audit ordersToExecute[orderIndex]
81                    SpentItem[] memory spentItems = ordersToExecute[orderIndex]
82:                       .spentItems;

/// @audit ordersToExecute[orderIndex]
108                   ReceivedItem[] memory receivedItems = ordersToExecute[
109                       orderIndex
110:                  ].receivedItems;

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceCriteriaResolution.sol#L81-L82

4. _assertConduitExists() should return the storage variable it looks up

Each time _assertConduitExists(), which looks up the conduit from the _conduits mapping, thereafter, the conduit is looked up again. The function should change to return the storage struct to avoid re-calculating the mapping hash value for the conduit, which would save ~42 gas per call. Examples below show the instances where this would save gas

There are 8 instances of this issue:

File: contracts/conduit/ConduitController.sol

234          _assertConduitExists(conduit);
235  
236          // If caller does not match current potential owner of the conduit...
237:         if (msg.sender != _conduits[conduit].potentialOwner) {

273          _assertConduitExists(conduit);
274  
275          // Retrieve the current owner of the conduit in question.
276:         owner = _conduits[conduit].owner;

357          _assertConduitExists(conduit);
358  
359          // Retrieve the current potential owner of the conduit in question.
360:         potentialOwner = _conduits[conduit].potentialOwner;

379          _assertConduitExists(conduit);
380  
381          // Retrieve the current channel status for the conduit in question.
382:         isOpen = _conduits[conduit].channelIndexesPlusOne[channel] != 0;

399          _assertConduitExists(conduit);
400  
401          // Retrieve the total open channel count for the conduit in question.
402:         totalChannels = _conduits[conduit].channels.length;

422          _assertConduitExists(conduit);
423  
424          // Retrieve the total open channel count for the conduit in question.
425:         uint256 totalChannels = _conduits[conduit].channels.length;

452          _assertConduitExists(conduit);
453  
454          // Retrieve all of the open channels on the conduit in question.
455:         channels = _conduits[conduit].channels;

482          _assertConduitExists(conduit);
483  
484          // If the caller does not match the current owner of the conduit...
485:         if (msg.sender != _conduits[conduit].owner) {

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/contracts/conduit/ConduitController.sol#L234-L237

5. internal functions only called once can be inlined to save gas

Not inlining costs 20 to 40 gas because of two extra JUMP instructions and additional stack operations needed for function calls.

There are 25 instances of this issue:

File: contracts/lib/CriteriaResolution.sol

241       function _verifyProof(
242           uint256 leaf,
243           uint256 root,
244:          bytes32[] memory proof

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/contracts/lib/CriteriaResolution.sol#L241-L244

File: contracts/lib/BasicOrderFulfiller.sol

325       function _prepareBasicFulfillmentFromCalldata(
326           BasicOrderParameters calldata parameters,
327           OrderType orderType,
328           ItemType receivedItemType,
329           ItemType additionalRecipientsItemType,
330           address additionalRecipientsToken,
331:          ItemType offeredItemType

936       function _transferEthAndFinalize(
937           uint256 amount,
938           address payable to,
939:          AdditionalRecipient[] calldata additionalRecipients

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/contracts/lib/BasicOrderFulfiller.sol#L325-L331

File: contracts/lib/ConsiderationBase.sol

130       function _deriveTypehashes()
131           internal
132           pure
133           returns (
134               bytes32 nameHash,
135               bytes32 versionHash,
136               bytes32 eip712DomainTypehash,
137               bytes32 offerItemTypehash,
138               bytes32 considerationItemTypehash,
139:              bytes32 orderTypehash

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/contracts/lib/ConsiderationBase.sol#L130-L139

File: contracts/lib/Executor.sol

456:      function _trigger(bytes32 conduitKey, bytes memory accumulator) internal {

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/contracts/lib/Executor.sol#L456

File: contracts/lib/OrderCombiner.sol

445       function _executeAvailableFulfillments(
446           AdvancedOrder[] memory advancedOrders,
447           FulfillmentComponent[][] memory offerFulfillments,
448           FulfillmentComponent[][] memory considerationFulfillments,
449           bytes32 fulfillerConduitKey
450       )
451           internal
452:          returns (bool[] memory availableOrders, Execution[] memory executions)

738       function _fulfillAdvancedOrders(
739           AdvancedOrder[] memory advancedOrders,
740           Fulfillment[] calldata fulfillments
741:      ) internal returns (Execution[] memory executions) {

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/contracts/lib/OrderCombiner.sol#L445-L452

File: contracts/lib/OrderValidator.sol

450       function _doesNotSupportPartialFills(OrderType orderType)
451           internal
452           pure
453:          returns (bool isFullOrder)

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/contracts/lib/OrderValidator.sol#L450-L453

File: contracts/lib/OrderFulfiller.sol

155       function _applyFractionsAndTransferEach(
156           OrderParameters memory orderParameters,
157           uint256 numerator,
158           uint256 denominator,
159           bytes32 offererConduitKey,
160:          bytes32 fulfillerConduitKey

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/contracts/lib/OrderFulfiller.sol#L155-L160

File: reference/lib/ReferenceExecutor.sol

330:      function _trigger(AccumulatorStruct memory accumulatorStruct) internal {

426       function _getConduit(bytes32 conduitKey)
427           internal
428           view
429:          returns (address conduit)

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceExecutor.sol#L330

File: reference/lib/ReferenceOrderCombiner.sol

441       function _executeAvailableFulfillments(
442           OrderToExecute[] memory ordersToExecute,
443           FulfillmentComponent[][] memory offerFulfillments,
444           FulfillmentComponent[][] memory considerationFulfillments,
445           bytes32 fulfillerConduitKey
446       )
447           internal
448:          returns (bool[] memory availableOrders, Execution[] memory executions)

735       function _fulfillAdvancedOrders(
736           OrderToExecute[] memory ordersToExecute,
737           Fulfillment[] calldata fulfillments
738:      ) internal returns (Execution[] memory executions) {

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceOrderCombiner.sol#L441-L448

File: reference/lib/ReferenceFulfillmentApplier.sol

238       function _checkMatchingOffer(
239           OrderToExecute memory orderToExecute,
240           SpentItem memory offer,
241           Execution memory execution
242:      ) internal pure returns (bool invalidFulfillment) {

375       function _aggregateConsiderationItems(
376           OrderToExecute[] memory ordersToExecute,
377           FulfillmentComponent[] memory considerationComponents,
378           uint256 nextComponentIndex,
379           bytes32 fulfillerConduitKey
380:      ) internal view returns (Execution memory execution) {

407       function _checkMatchingConsideration(
408           ReceivedItem memory consideration,
409           ReceivedItem memory receivedItem
410:      ) internal pure returns (bool invalidFulfillment) {

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceFulfillmentApplier.sol#L238-L242

File: reference/lib/ReferenceOrderValidator.sol

420       function _doesNotSupportPartialFills(OrderType orderType)
421           internal
422           pure
423:          returns (bool isFullOrder)

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceOrderValidator.sol#L420-L423

File: reference/lib/ReferenceGettersAndDerivers.sol

36        function _hashOfferItem(OfferItem memory offerItem)
37            internal
38            view
39:           returns (bytes32)

61        function _hashConsiderationItem(ConsiderationItem memory considerationItem)
62            internal
63            view
64:           returns (bytes32)

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceGettersAndDerivers.sol#L36-L39

File: reference/lib/ReferenceOrderFulfiller.sol

151       function _applyFractionsAndTransferEach(
152           OrderParameters memory orderParameters,
153           uint256 numerator,
154           uint256 denominator,
155           bytes32 offererConduitKey,
156           bytes32 fulfillerConduitKey
157:      ) internal returns (OrderToExecute memory orderToExecute) {

362       function _convertAdvancedToOrder(AdvancedOrder memory advancedOrder)
363           internal
364           pure
365:          returns (OrderToExecute memory orderToExecute)

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceOrderFulfiller.sol#L151-L157

File: reference/lib/ReferenceBasicOrderFulfiller.sol

63        function createMappings() internal {
64            // BasicOrderType to BasicOrderRouteType
65    
66            // ETH TO ERC 721
67:           _OrderToRouteType[

504       function _hashOrder(
505           BasicFulfillmentHashes memory hashes,
506           BasicOrderParameters calldata parameters,
507           FulfillmentItemTypes memory fulfillmentItemTypes
508:      ) internal view returns (bytes32 orderHash) {

548       function _prepareBasicFulfillment(
549           BasicOrderParameters calldata parameters,
550           OrderType orderType,
551           ItemType receivedItemType,
552           ItemType additionalRecipientsItemType,
553           address additionalRecipientsToken,
554:          ItemType offeredItemType

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceBasicOrderFulfiller.sol#L63-L67

File: reference/lib/ReferenceConsiderationBase.sol

143       function _deriveTypehashes()
144           internal
145           view
146           returns (
147               bytes32 nameHash,
148               bytes32 versionHash,
149               bytes32 eip712DomainTypehash,
150               bytes32 offerItemTypehash,
151               bytes32 considerationItemTypehash,
152               bytes32 orderTypehash,
153:              bytes32 domainSeparator

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceConsiderationBase.sol#L143-L153

6. <array>.length should not be looked up in every loop of a for-loop

The overheads outlined below are PER LOOP, excluding the first loop

Caching the length changes each of these to a DUP<N> (3 gas), and gets rid of the extra DUP<N> needed to store the stack offset

There are 22 instances of this issue:

File: 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; ) {

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/contracts/lib/OrderCombiner.sol#L247

File: contracts/lib/OrderFulfiller.sol

217:              for (uint256 i = 0; i < orderParameters.offer.length; ) {

306:              for (uint256 i = 0; i < orderParameters.consideration.length; ) {

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/contracts/lib/OrderFulfiller.sol#L217

File: reference/lib/ReferenceOrderCombiner.sol

253:              for (uint256 j = 0; j < offer.length; ++j) {

300:              for (uint256 j = 0; j < consideration.length; ++j) {

603:              for (uint256 j = 0; j < consideration.length; ++j) {

621:          for (uint256 i = 0; i < executions.length; ++i) {

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceOrderCombiner.sol#L253

File: reference/lib/ReferenceFulfillmentApplier.sol

308:                      i < offerComponents.length;

481:                      i < considerationComponents.length;

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceFulfillmentApplier.sol#L308

File: reference/lib/ReferenceGettersAndDerivers.sol

104:          for (uint256 i = 0; i < orderParameters.offer.length; ++i) {

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceGettersAndDerivers.sol#L104

File: reference/lib/ReferenceOrderFulfiller.sol

188:              for (uint256 i = 0; i < orderParameters.offer.length; ++i) {

245:              for (uint256 i = 0; i < orderParameters.consideration.length; ++i) {

374:          for (uint256 i = 0; i < offer.length; ++i) {

401:          for (uint256 i = 0; i < consideration.length; ++i) {

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceOrderFulfiller.sol#L188

File: reference/lib/ReferenceBasicOrderFulfiller.sol

643:                  recipientCount < parameters.additionalRecipients.length;

714:                  additionalTips < parameters.additionalRecipients.length;

832:          for (uint256 i = 0; i < parameters.additionalRecipients.length; ++i) {

903:          for (uint256 i = 0; i < parameters.additionalRecipients.length; ++i) {

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceBasicOrderFulfiller.sol#L643

File: reference/lib/ReferenceCriteriaResolution.sol

378:          for (uint256 i = 0; i < proof.length; ++i) {

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceCriteriaResolution.sol#L378

7. ++i/i++ should be unchecked{++i}/unchecked{i++} when it is not possible for them to overflow, as is the case when used in for- and while-loops

The unchecked keyword is new in solidity version 0.8.0, so this only applies to that version or higher, which these instances are. This saves 30-40 gas per loop

There are 42 instances of this issue:

File: reference/conduit/ReferenceConduit.sol

48:           for (uint256 i = 0; i < totalStandardTransfers; i++) {

69:           for (uint256 i = 0; i < totalBatchTransfers; i++) {

91:           for (uint256 i = 0; i < totalStandardTransfers; i++) {

102:          for (uint256 i = 0; i < totalBatchTransfers; i++) {

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/conduit/ReferenceConduit.sol#L48

File: reference/lib/ReferenceExecutor.sol

382:          for (uint256 i = 0; i < currentTransferLength; ++i) {

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceExecutor.sol#L382

File: reference/lib/ReferenceOrderCombiner.sol

189:          for (uint256 i = 0; i < totalOrders; ++i) {

253:              for (uint256 j = 0; j < offer.length; ++j) {

300:              for (uint256 j = 0; j < consideration.length; ++j) {

360:          for (uint256 i = 0; i < totalOrders; ++i) {

467:          for (uint256 i = 0; i < totalOfferFulfillments; ++i) {

490:          for (uint256 i = 0; i < totalConsiderationFulfillments; ++i) {

534:              for (uint256 i = 0; i < executionLength; ++i) {

582:          for (uint256 i = 0; i < totalOrders; ++i) {

603:              for (uint256 j = 0; j < consideration.length; ++j) {

621:          for (uint256 i = 0; i < executions.length; ++i) {

749:          for (uint256 i = 0; i < totalFulfillments; ++i) {

778:              for (uint256 i = 0; i < executionLength; ++i) {

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceOrderCombiner.sol#L189

File: reference/lib/ReferenceFulfillmentApplier.sol

170:          for (uint256 i = 0; i < totalFulfillmentComponents; ++i) {

309:                      ++i

482:                      ++i

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceFulfillmentApplier.sol#L170

File: reference/lib/ReferenceOrderValidator.sol

265:          for (uint256 i = 0; i < totalOrders; ++i) {

329:          for (uint256 i = 0; i < totalOrders; ++i) {

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceOrderValidator.sol#L265

File: reference/lib/ReferenceGettersAndDerivers.sol

104:          for (uint256 i = 0; i < orderParameters.offer.length; ++i) {

113:              ++i

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceGettersAndDerivers.sol#L104

File: reference/lib/ReferenceOrderFulfiller.sol

188:              for (uint256 i = 0; i < orderParameters.offer.length; ++i) {

245:              for (uint256 i = 0; i < orderParameters.consideration.length; ++i) {

345:          for (uint256 i = 0; i < totalOrders; ++i) {

374:          for (uint256 i = 0; i < offer.length; ++i) {

401:          for (uint256 i = 0; i < consideration.length; ++i) {

448:          for (uint256 i = 0; i < totalOrders; ++i) {

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceOrderFulfiller.sol#L188

File: reference/lib/ReferenceBasicOrderFulfiller.sol

644:                  recipientCount++

715:                  additionalTips++

832:          for (uint256 i = 0; i < parameters.additionalRecipients.length; ++i) {

903:          for (uint256 i = 0; i < parameters.additionalRecipients.length; ++i) {

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceBasicOrderFulfiller.sol#L644

File: reference/lib/ReferenceCriteriaResolution.sol

55:           for (uint256 i = 0; i < arraySize; ++i) {

157:          for (uint256 i = 0; i < arraySize; ++i) {

170:              for (uint256 j = 0; j < totalItems; ++j) {

183:              for (uint256 j = 0; j < totalItems; ++j) {

221:          for (uint256 i = 0; i < arraySize; ++i) {

315:          for (uint256 i = 0; i < totalItems; ++i) {

330:          for (uint256 i = 0; i < totalItems; ++i) {

378:          for (uint256 i = 0; i < proof.length; ++i) {

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceCriteriaResolution.sol#L55

8. Optimize names to save gas

public/external function names and public member variable names can be optimized to save gas. see this link for an example of how it works. Below are the interfaces/abstract contracts that can be optimized so that the most frequently-called functions use the least amount of gas possible during method lookup. Method IDs that have two leading zero bytes can save 128 gas each during deployment, and renaming functions to have lower method IDs will save 22 gas per call, per sorted position shifted

There are 6 instances of this issue:

File: contracts/interfaces/ZoneInterface.sol

10:   interface ZoneInterface {

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/contracts/interfaces/ZoneInterface.sol#L10

File: contracts/interfaces/SeaportInterface.sol

28:   interface SeaportInterface {

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/contracts/interfaces/SeaportInterface.sol#L28

File: contracts/interfaces/ConduitInterface.sol

16:   interface ConduitInterface {

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/contracts/interfaces/ConduitInterface.sol#L16

File: contracts/interfaces/ConduitControllerInterface.sol

10:   interface ConduitControllerInterface {

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/contracts/interfaces/ConduitControllerInterface.sol#L10

File: contracts/interfaces/ImmutableCreate2FactoryInterface.sol

18:   interface ImmutableCreate2FactoryInterface {

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/contracts/interfaces/ImmutableCreate2FactoryInterface.sol#L18

File: contracts/interfaces/ConsiderationInterface.sol

29:   interface ConsiderationInterface {

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/contracts/interfaces/ConsiderationInterface.sol#L29

9. Using bools for storage incurs overhead

    // Booleans are more expensive than uint256 or any type that takes up a full
    // word because each write operation emits an extra SLOAD to first read the
    // slot's contents, replace the bits taken up by the boolean, and then write
    // back. This is the compiler's defense against contract upgrades and
    // pointer aliasing, and it cannot be disabled.

https://github.com/OpenZeppelin/openzeppelin-contracts/blob/58f635312aa21f947cae5f8578638a85aa2519f5/contracts/security/ReentrancyGuard.sol#L23-L27 Use uint256(1) and uint256(2) for true/false to avoid a Gwarmaccess (100 gas), and to avoid Gsset (20000 gas) when changing from 'false' to 'true', after having been 'true' in the past

There are 2 instances of this issue:

File: contracts/conduit/Conduit.sol   #1

33:       mapping(address => bool) private _channels;

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/contracts/conduit/Conduit.sol#L33

File: reference/conduit/ReferenceConduit.sol   #2

30:       mapping(address => bool) private _channels;

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/conduit/ReferenceConduit.sol#L30

10. It costs more gas to initialize variables to zero than to let the default of zero be applied

There are 69 instances of this issue:

File: contracts/conduit/Conduit.sol

66:           for (uint256 i = 0; i < totalStandardTransfers; ) {

130:          for (uint256 i = 0; i < totalStandardTransfers; ) {

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/contracts/conduit/Conduit.sol#L66

File: contracts/lib/CriteriaResolution.sol

56:               for (uint256 i = 0; i < totalCriteriaResolvers; ++i) {

166:              for (uint256 i = 0; i < totalAdvancedOrders; ++i) {

184:                  for (uint256 j = 0; j < totalItems; ++j) {

199:                  for (uint256 j = 0; j < totalItems; ++j) {

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/contracts/lib/CriteriaResolution.sol#L56

File: contracts/lib/BasicOrderFulfiller.sol

948:          for (uint256 i = 0; i < totalAdditionalRecipients; ) {

1040:         for (uint256 i = 0; i < totalAdditionalRecipients; ) {

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/contracts/lib/BasicOrderFulfiller.sol#L948

File: contracts/lib/OrderCombiner.sol

181:              for (uint256 i = 0; i < totalOrders; ++i) {

247:                  for (uint256 j = 0; j < offer.length; ++j) {

291:                  for (uint256 j = 0; j < consideration.length; ++j) {

373:              for (uint256 i = 0; i < totalOrders; ++i) {

470:              uint256 totalFilteredExecutions = 0;

473:              for (uint256 i = 0; i < totalOfferFulfillments; ++i) {

498:              for (uint256 i = 0; i < totalConsiderationFulfillments; ++i) {

577:              for (uint256 i = 0; i < totalOrders; ++i) {

598:                  for (uint256 j = 0; j < consideration.length; ++j) {

621:          for (uint256 i = 0; i < executions.length; ) {

751:              uint256 totalFilteredExecutions = 0;

754:              for (uint256 i = 0; i < totalFulfillments; ++i) {

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/contracts/lib/OrderCombiner.sol#L181

File: contracts/lib/OrderValidator.sol

272:              for (uint256 i = 0; i < totalOrders; ) {

350:              for (uint256 i = 0; i < totalOrders; ) {

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/contracts/lib/OrderValidator.sol#L272

File: contracts/lib/AmountDeriver.sol

44:               uint256 extraCeiling = 0;

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/contracts/lib/AmountDeriver.sol#L44

File: contracts/lib/OrderFulfiller.sol

217:              for (uint256 i = 0; i < orderParameters.offer.length; ) {

306:              for (uint256 i = 0; i < orderParameters.consideration.length; ) {

471:              for (uint256 i = 0; i < totalOrders; ++i) {

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/contracts/lib/OrderFulfiller.sol#L217

File: reference/conduit/ReferenceConduit.sol

48:           for (uint256 i = 0; i < totalStandardTransfers; i++) {

69:           for (uint256 i = 0; i < totalBatchTransfers; i++) {

91:           for (uint256 i = 0; i < totalStandardTransfers; i++) {

102:          for (uint256 i = 0; i < totalBatchTransfers; i++) {

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/conduit/ReferenceConduit.sol#L48

File: reference/lib/ReferenceExecutor.sol

382:          for (uint256 i = 0; i < currentTransferLength; ++i) {

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceExecutor.sol#L382

File: reference/lib/ReferenceOrderCombiner.sol

189:          for (uint256 i = 0; i < totalOrders; ++i) {

253:              for (uint256 j = 0; j < offer.length; ++j) {

300:              for (uint256 j = 0; j < consideration.length; ++j) {

360:          for (uint256 i = 0; i < totalOrders; ++i) {

464:          uint256 totalFilteredExecutions = 0;

467:          for (uint256 i = 0; i < totalOfferFulfillments; ++i) {

490:          for (uint256 i = 0; i < totalConsiderationFulfillments; ++i) {

534:              for (uint256 i = 0; i < executionLength; ++i) {

582:          for (uint256 i = 0; i < totalOrders; ++i) {

603:              for (uint256 j = 0; j < consideration.length; ++j) {

621:          for (uint256 i = 0; i < executions.length; ++i) {

746:          uint256 totalFilteredExecutions = 0;

749:          for (uint256 i = 0; i < totalFulfillments; ++i) {

778:              for (uint256 i = 0; i < executionLength; ++i) {

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceOrderCombiner.sol#L189

File: reference/lib/ReferenceFulfillmentApplier.sol

167:          uint256 nextComponentIndex = 0;

170:          for (uint256 i = 0; i < totalFulfillmentComponents; ++i) {

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceFulfillmentApplier.sol#L167

File: reference/lib/ReferenceOrderValidator.sol

265:          for (uint256 i = 0; i < totalOrders; ++i) {

329:          for (uint256 i = 0; i < totalOrders; ++i) {

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceOrderValidator.sol#L265

File: reference/lib/ReferenceGettersAndDerivers.sol

104:          for (uint256 i = 0; i < orderParameters.offer.length; ++i) {

111:              uint256 i = 0;

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceGettersAndDerivers.sol#L104

File: reference/lib/ReferenceAmountDeriver.sol

46:               uint256 extraCeiling = 0;

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceAmountDeriver.sol#L46

File: reference/lib/ReferenceOrderFulfiller.sol

188:              for (uint256 i = 0; i < orderParameters.offer.length; ++i) {

245:              for (uint256 i = 0; i < orderParameters.consideration.length; ++i) {

345:          for (uint256 i = 0; i < totalOrders; ++i) {

374:          for (uint256 i = 0; i < offer.length; ++i) {

401:          for (uint256 i = 0; i < consideration.length; ++i) {

448:          for (uint256 i = 0; i < totalOrders; ++i) {

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceOrderFulfiller.sol#L188

File: reference/lib/ReferenceBasicOrderFulfiller.sol

642:                  uint256 recipientCount = 0;

832:          for (uint256 i = 0; i < parameters.additionalRecipients.length; ++i) {

903:          for (uint256 i = 0; i < parameters.additionalRecipients.length; ++i) {

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceBasicOrderFulfiller.sol#L642

File: reference/lib/ReferenceCriteriaResolution.sol

55:           for (uint256 i = 0; i < arraySize; ++i) {

157:          for (uint256 i = 0; i < arraySize; ++i) {

170:              for (uint256 j = 0; j < totalItems; ++j) {

183:              for (uint256 j = 0; j < totalItems; ++j) {

221:          for (uint256 i = 0; i < arraySize; ++i) {

315:          for (uint256 i = 0; i < totalItems; ++i) {

330:          for (uint256 i = 0; i < totalItems; ++i) {

378:          for (uint256 i = 0; i < proof.length; ++i) {

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceCriteriaResolution.sol#L55

11. ++i costs less gas than i++, especially when it's used in for-loops (--i/i-- too)

Saves 6 gas per loop

There are 6 instances of this issue:

File: reference/conduit/ReferenceConduit.sol

48:           for (uint256 i = 0; i < totalStandardTransfers; i++) {

69:           for (uint256 i = 0; i < totalBatchTransfers; i++) {

91:           for (uint256 i = 0; i < totalStandardTransfers; i++) {

102:          for (uint256 i = 0; i < totalBatchTransfers; i++) {

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/conduit/ReferenceConduit.sol#L48

File: reference/lib/ReferenceBasicOrderFulfiller.sol

644:                  recipientCount++

715:                  additionalTips++

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceBasicOrderFulfiller.sol#L644

12. Usage of uints/ints smaller than 32 bytes (256 bits) incurs overhead

When using elements that are smaller than 32 bytes, your contract’s gas usage may be higher. This is because the EVM operates on 32 bytes at a time. Therefore, if the element is smaller than that, the EVM must use more operations in order to reduce the size of the element from 32 bytes to the desired size.

https://docs.soliditylang.org/en/v0.8.11/internals/layout_in_storage.html Use a larger size then downcast where needed

There are 8 instances of this issue:

File: contracts/lib/SignatureVerification.sol

42:           uint8 v;

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/contracts/lib/SignatureVerification.sol#L42

File: contracts/lib/ConsiderationStructs.sol

172:      uint120 numerator;

173:      uint120 denominator;

188:      uint120 numerator;

189:      uint120 denominator;

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/contracts/lib/ConsiderationStructs.sol#L172

File: contracts/interfaces/SignatureVerificationErrors.sol

17:       error BadSignatureV(uint8 v);

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/contracts/interfaces/SignatureVerificationErrors.sol#L17

File: reference/lib/ReferenceSignatureVerification.sol

40:           uint8 v;

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceSignatureVerification.sol#L40

File: reference/lib/ReferenceConsiderationStructs.sol

61:       uint120 numerator;

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/reference/lib/ReferenceConsiderationStructs.sol#L61

liveactionllama commented 2 years ago

Warden created this issue as a placeholder, because their submission was too large for the contest form. They then emailed their md file to our team on 06/03/2022 at 16:47 UTC. I've updated this issue with their md file content.

HardlyDifficult commented 2 years ago

Using calldata instead of memory for read-only arguments in external functions saves gas Avoid contract existence checks by using solidity version 0.8.10 or later

Generally this is true, but the recommended changes are in the reference contracts which are not targeting gas optimizations.

Multiple accesses of a mapping/array should use a local variable cache

Savings like this can add up, even though the impact is small per instance a lot of instances were reported here.

_assertConduitExists() should return the storage variable it looks up

This could provide some savings, but approaches like this can hurt the code readability.

internal functions only called once can be inlined to save gas

The viaIR compiler optimization flag used by this project is meant to help with inline optimizations, so not clear doing this manually would result in significant savings.

.length should not be looked up in every loop of a for-loop ++i/i++ should be unchecked{++i}/unchecked{i++} when it is not possible for them to overflow, as is the case when used in for- and while-loops ++i costs less gas than i++, especially when it's used in for-loops (--i/i-- too)

These should offer small savings.

Optimize names to save gas

This sort of optimization dirties the external ABI, not clear it's a worthwhile tactic to use -- but may be worth considering for the most common functions.

Using bools for storage incurs overhead

This should have minimal impact during normal usage. The majority of the savings is when closing a channel which should not be common and is not a cost paid by end-users.

It costs more gas to initialize variables to zero than to let the default of zero be applied

The compiler will mostly take care of this automatically. In testing I found very minimal impact from this sort of optimization.

Usage of uints/ints smaller than 32 bytes (256 bits) incurs overhead

These reports may save some gas (assuming they don't impact packed storage). But for code readability it can be preferable to use variables sized to the logical max supported.