Platform: Code4rena
Start Date: 20/05/2022
Pot Size: $1,000,000 USDC
Total HM: 4
Participants: 59
Period: 14 days
Judge: leastwood
Id: 128
League: ETH
Rank: 11/59
Findings: 2
Award: $6,888.62
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: Spearbit
Also found by: 0xsanson, Chom, IllIllI, OriDabush, Saw-mon_and_Natalie, broccoli, cccz, cmichel, csanuragjain, foobar, hack3r-0m, hickuphh3, hubble, hyh, ilan, kebabsec, mayo, oyc_109, peritoflores, rfa, scaraven, sces60107, shung, sorrynotsorry, tintin, twojoy, zkhorse, zzzitron
5598.4222 USDC - $5,598.42
Vulnerability details:
| Issue | Instances | |
|---|---|---|
| 1 | Use of transferFrom() rather than safeTransferFrom() for NFTs in the reference code will lead to the loss of NFTs | 1 |
| 2 | Holdings not verified | 1 |
| 3 | Lack of escrow will lead to a lot of spoofing | 1 |
| 4 | Incorrect NatSpec | 1 |
| 5 | Reference code signatures can't work with optimized code | 1 |
| 6 | Misleading comments | 1 |
| 7 | Incomplete code cleanup | 1 |
| 8 | The unchecked keyword is for mathematical impossibilities, not assumptions | 1 |
| 9 | 'ERC20' tokens with incorrect return types that are over-sized, are interpreted as returning bool | 1 |
| 10 | Missing fill-or-kill behavior | 1 |
| 11 | abi.encodePacked() should not be used with dynamic types when passing the result to a hash function such as keccak256() | 7 |
Total: 17 instances over 11 issues
| Issue | Instances | |
|---|---|---|
| 1 | Unused file | 1 |
| 2 | Adding a return statement when the function defines a named return variable, is redundant | 9 |
| 3 | constants should be defined rather than using magic numbers | 41 |
| 4 | Use a more recent version of solidity | 6 |
| 5 | Inconsistent spacing in comments | 1 |
| 6 | Inconsistent method of specifying a floating pragma | 22 |
| 7 | Non-library/interface files should use fixed compiler versions, not floating ones | 20 |
| 8 | Inconsistent code formatting | 6 |
| 9 | Inconsistent use of named arguments within interfaces in same file | 2 |
| 10 | Duplicated code | 1 |
| 11 | Anyone should be able to cancel an expired order | 1 |
| 12 | Typos | 7 |
| 13 | File is missing NatSpec | 8 |
| 14 | NatSpec is incomplete | 13 |
| 15 | Event is missing indexed fields | 6 |
| 16 | Not using the named return variables anywhere in the function is confusing | 46 |
Total: 190 instances over 16 issues
transferFrom() rather than safeTransferFrom() for NFTs in the reference code will lead to the loss of NFTsSee the Medium issue I filed for the non-reference, optimized code for details
There is 1 instance of this issue:
File: reference/lib/ReferenceTokenTransferrer.sol #1 82: ERC721Interface(token).transferFrom(from, to, identifier);
During offer creation, there is no code that verifies that the user actually holds the items they're offering (e.g. balanceOf(). The code should make some sort of attempt to verify holdings before accepting an order, to cut down on the amount of spoofing possible
There is 1 instance of this issue:
File: contracts/lib/Consideration.sol #1 174 * of items for offer and consideration. Any order that is not 175 * currently active, has already been fully filled, or has been 176: * cancelled will be omitted. Remaining offer and consideration
The average user will not be able to simulate whether an order will be fulfillable or not, and even if it is at that exact moment, without an escrow, the seller can transfer funds, cancel, or front-run at any time. There should be an escrow option so that users have a way to know that offers are real. This was found to be of Medium risk in a recent contest
There is 1 instance of this issue:
File: contracts/lib/Consideration.sol #1 174 * of items for offer and consideration. Any order that is not 175 * currently active, has already been fully filled, or has been 176: * cancelled will be omitted. Remaining offer and consideration
The qualification in parentheses should have been added to the amount parameter, not the identifier parameter
There is 1 instance of this issue:
File: contracts/lib/Executor.sol #1 301 * @param identifier The tokenId to transfer (must be 1 for ERC721). 302: * @param amount The amount to transfer.
The reference code uses "rc.1" in its construction of the domain separator, whereas the actual code uses "1"
There is 1 instance of this issue:
File: reference/lib/ReferenceConsiderationBase.sol #1 30: string internal constant _VERSION = "rc.1";
The code checks the first twenty bytes, not the last
There is 1 instance of this issue:
File: contracts/interfaces/ConduitControllerInterface.sol #1 111 * the last twenty bytes of the conduit key must match 112: * the caller of this contract.
It seems as though the plan originally was to use this interface for doing create2, but later things switched to using a salt with new, and this interface wasn't moved to be test-only. If the above is right, you should see if anything during that transition was also missed
There is 1 instance of this issue:
File: contracts/interfaces/ImmutableCreate2FactoryInterface.sol #1 18: interface ImmutableCreate2FactoryInterface {
unchecked keyword is for mathematical impossibilities, not assumptionsIt's an assumption that the nonce cannot be incremented that far, and therfore the use of unchecked is invalid
There is 1 instance of this issue:
File: contracts/lib/NonceManager.sol #1 32 // No need to check for overflow; nonce cannot be incremented that far. 33 unchecked { 34 // Increment current nonce for the supplied offerer. 35 newNonce = ++_nonces[msg.sender]; 36: }
boolIf there's a token that has the wrong return type, e.g. returns (bool,bool), the code below will mload and check the value of the first bool, and will ignore the fact that there is extra return data. Th code should change to be eq(returndatasize(), 32). The reference code has the same issue
There is 1 instance of this issue:
File: contracts/lib/TokenTransferrer.sol #1 41 // Make call & copy up to 32 bytes of return data to scratch space. 42 let callStatus := call( 43 gas(), 44 token, 45 0, 46 ERC20_transferFrom_sig_ptr, 47 ERC20_transferFrom_length, 48 0, 49 OneWord 50 ) 51 52 // Determine whether transfer was successful using status & result. 53 let success := and( 54 // Set success to whether the call reverted, if not check it 55 // either returned exactly 1 (can't just be non-zero data), or 56 // had no return data. 57 or( 58: and(eq(mload(0), 1), gt(returndatasize(), 31)),
A useful order type for traders is the fill-or-kill order type, and by requiring that the ending timestamp be greater than the current timestamp, you are unnecessarily preventing this order type
There is 1 instance of this issue:
File: contracts/lib/Verifiers.sol #1 37 function _verifyTime( 38 uint256 startTime, 39 uint256 endTime, 40 bool revertOnInvalid 41 ) internal view returns (bool valid) { 42 // Revert if order's timespan hasn't started yet or has already ended. 43: if (startTime > block.timestamp || endTime <= block.timestamp) {
abi.encodePacked() should not be used with dynamic types when passing the result to a hash function such as keccak256()Use abi.encode() instead which will pad items to 32 bytes, which will prevent hash collisions (e.g. abi.encodePacked(0x123,0x456) => 0x123456 => abi.encodePacked(0x1,0x23456), but abi.encode(0x123,0x456) => 0x0...1230...456). "Unless there is a compelling reason, abi.encode should be preferred". If there is only one argument to abi.encodePacked() it can often be cast to bytes() or bytes32() instead.
If all arguments are strings and or bytes, bytes.concat() should be used instead
There are 7 instances of this issue:
File: contracts/lib/ConsiderationBase.sol 193 eip712DomainTypehash = keccak256( 194 abi.encodePacked( 195 "EIP712Domain(", 196 "string name,", 197 "string version,", 198 "uint256 chainId,", 199 "address verifyingContract", 200 ")" 201 ) 202: ); 211 orderTypehash = keccak256( 212 abi.encodePacked( 213 orderComponentsPartialTypeString, 214 considerationItemTypeString, 215 offerItemTypeString 216 ) 217: );
File: reference/lib/ReferenceGettersAndDerivers.sol 129: keccak256(abi.encodePacked(offerHashes)), 130: keccak256(abi.encodePacked(considerationHashes)),
File: reference/lib/ReferenceBasicOrderFulfiller.sol 769 hashes.offerItemsHash = keccak256( 770 abi.encodePacked(offerItemHashes) 771: );
File: reference/lib/ReferenceConsiderationBase.sol 207 eip712DomainTypehash = keccak256( 208 abi.encodePacked( 209 "EIP712Domain(", 210 "string name,", 211 "string version,", 212 "uint256 chainId,", 213 "address verifyingContract", 214 ")" 215 ) 216: ); 225 orderTypehash = keccak256( 226 abi.encodePacked( 227 orderComponentsPartialTypeString, 228 considerationItemTypeString, 229 offerItemTypeString 230 ) 231: );
The file is never imported by any other file. After deleting the file, I was still able to run the hardhat tests, so it seems as though this file is no longer needed.
There is 1 instance of this issue:
File: reference/shim/Shim.sol #1 0: // SPDX-License-Identifier: MIT
return statement when the function defines a named return variable, is redundantThere are 9 instances of this issue:
File: contracts/lib/FulfillmentApplier.sol 120: return execution; // Execution(considerationItem, offerer, conduitKey);
File: contracts/lib/OrderFulfiller.sol 478: return advancedOrders;
File: reference/lib/ReferenceOrderCombiner.sol 655: return availableOrders; 789: return executions;
File: reference/lib/ReferenceFulfillmentApplier.sol 128: return execution; // Execution(considerationItem, offerer, conduitKey);
File: reference/lib/ReferenceOrderFulfiller.sol 299: return orderToExecute; 351: return advancedOrders; 427: return orderToExecute; 454: return ordersToExecute;
constants should be defined rather than using magic numbersNormally I wouldn't have flagged hex values, but it looks like in other places you've created constants for them, so I've flagged all missing places
There are 41 instances of this issue:
File: contracts/conduit/ConduitController.sol /// @audit 0xff 74: bytes1(0xff), /// @audit 0xff 326: bytes1(0xff),
File: contracts/lib/CriteriaResolution.sol /// @audit 0x20 268: mstore(0x20, loadedData) // Place new hash next. /// @audit 0x20 272: mstore(0x20, computedHash) // Place existing hash next.
File: contracts/lib/SignatureVerification.sol /// @audit 64 45: if (signature.length == 64) { /// @audit 65 63: } else if (signature.length == 65) { /// @audit 27 78: if (v != 27 && v != 28) { /// @audit 28 78: if (v != 27 && v != 28) {
File: contracts/lib/Executor.sol /// @audit 3 387: uint256(3), /// @audit 64 433: if (accumulator.length != 64) {
File: contracts/lib/GettersAndDerivers.sol /// @audit 0xf8 322: mstore(add(version, OneWord), shl(0xf8, Version))
File: contracts/Seaport.sol /// @audit 0x20 40: mstore(0, 0x20) /// @audit 0x27 41: mstore(0x27, 0x07536561706f7274) /// @audit 0x07536561706f7274 41: mstore(0x27, 0x07536561706f7274) /// @audit 0x60 42: return(0, 0x60)
File: reference/conduit/ReferenceConduitController.sol /// @audit 0xff 76: bytes1(0xff), /// @audit 0xff 332: bytes1(0xff),
File: reference/lib/ReferenceSignatureVerification.sol /// @audit 64 43: if (signature.length == 64) { /// @audit 255 51: v = uint8(uint256(vs >> 255)) + 27; /// @audit 27 51: v = uint8(uint256(vs >> 255)) + 27; /// @audit 65 52: } else if (signature.length == 65) { /// @audit 64 54: v = uint8(signature[64]); /// @audit 27 57: if (v != 27 && v != 28) { /// @audit 28 57: if (v != 27 && v != 28) {
File: reference/lib/ReferenceExecutor.sol /// @audit 3 269: uint256(3),
File: reference/lib/ReferenceGettersAndDerivers.sol /// @audit 0x1901 157: abi.encodePacked(uint16(0x1901), domainSeparator, orderHash) /// @audit 0xff 184: bytes1(0xff),
File: reference/lib/ReferenceCriteriaResolution.sol /// @audit 3 358: withCriteria = uint256(itemType) > 3;
File: reference/lib/ReferenceTokenTransferrer.sol /// @audit 32 50: if (data.length != 0 && data.length >= 32) {
File: reference/lib/ReferenceAssertions.sol /// @audit 4 112: bool validOffsets = (abi.decode(msg.data[4:36], (uint256)) == 32 && /// @audit 36 112: bool validOffsets = (abi.decode(msg.data[4:36], (uint256)) == 32 && /// @audit 32 112: bool validOffsets = (abi.decode(msg.data[4:36], (uint256)) == 32 && /// @audit 548 113: abi.decode(msg.data[548:580], (uint256)) == 576 && /// @audit 580 113: abi.decode(msg.data[548:580], (uint256)) == 576 && /// @audit 576 113: abi.decode(msg.data[548:580], (uint256)) == 576 && /// @audit 580 114: abi.decode(msg.data[580:612], (uint256)) == /// @audit 612 114: abi.decode(msg.data[580:612], (uint256)) == /// @audit 608 115: 608 + 64 * abi.decode(msg.data[612:644], (uint256))); /// @audit 64 115: 608 + 64 * abi.decode(msg.data[612:644], (uint256))); /// @audit 612 115: 608 + 64 * abi.decode(msg.data[612:644], (uint256))); /// @audit 644 115: 608 + 64 * abi.decode(msg.data[612:644], (uint256)));
Use a solidity version of at least 0.8.12 to get string.concat() to be used instead of abi.encodePacked(<str>,<str>)
There are 6 instances of this issue:
File: contracts/conduit/ConduitController.sol 2: pragma solidity >=0.8.7;
File: reference/conduit/ReferenceConduitController.sol 2: pragma solidity ^0.8.7;
File: reference/lib/ReferenceGettersAndDerivers.sol 2: pragma solidity ^0.8.7;
File: reference/lib/ReferenceBasicOrderFulfiller.sol 2: pragma solidity ^0.8.7;
File: reference/lib/ReferenceCriteriaResolution.sol 2: pragma solidity ^0.8.7;
File: reference/lib/ReferenceConsiderationBase.sol 2: pragma solidity ^0.8.7;
Some lines use // x and some use //x. The instances below point out the usages that don't follow the majority, within each file
There is 1 instance of this issue:
File: reference/lib/ReferenceBasicOrderFulfiller.sol #1 763: offerItem.amount //Assembly uses OfferItem instead of SpentItem.
Some files use >=, some use ^. The instances below are examples of the method that has the fewest instances for a specific version. Note that using >= without also specifying <= will lead to failures to compile when the major version changes and there are breaking-changes, so ^ should be preferred regardless of the instance counts
There are 22 instances of this issue:
File: reference/conduit/ReferenceConduitController.sol 2: pragma solidity ^0.8.7;
File: reference/conduit/ReferenceConduit.sol 2: pragma solidity ^0.8.7;
File: reference/lib/ReferenceSignatureVerification.sol 2: pragma solidity ^0.8.7;
File: reference/lib/ReferenceExecutor.sol 2: pragma solidity ^0.8.7;
File: reference/lib/ReferenceOrderCombiner.sol 2: pragma solidity ^0.8.7;
File: reference/lib/ReferenceFulfillmentApplier.sol 2: pragma solidity ^0.8.7;
File: reference/lib/ReferenceOrderValidator.sol 2: pragma solidity ^0.8.7;
File: reference/lib/ReferenceZoneInteraction.sol 2: pragma solidity ^0.8.7;
File: reference/lib/ReferenceGettersAndDerivers.sol 2: pragma solidity ^0.8.7;
File: reference/lib/ReferenceAmountDeriver.sol 2: pragma solidity ^0.8.7;
File: reference/lib/ReferenceOrderFulfiller.sol 2: pragma solidity ^0.8.7;
File: reference/lib/ReferenceBasicOrderFulfiller.sol 2: pragma solidity ^0.8.7;
File: reference/lib/ReferenceNonceManager.sol 2: pragma solidity ^0.8.7;
File: reference/lib/ReferenceCriteriaResolution.sol 2: pragma solidity ^0.8.7;
File: reference/lib/ReferenceReentrancyGuard.sol 2: pragma solidity ^0.8.7;
File: reference/lib/ReferenceVerifiers.sol 2: pragma solidity ^0.8.7;
File: reference/lib/ReferenceConsiderationStructs.sol 2: pragma solidity ^0.8.7;
File: reference/lib/ReferenceTokenTransferrer.sol 2: pragma solidity ^0.8.7;
File: reference/lib/ReferenceConsiderationBase.sol 2: pragma solidity ^0.8.7;
File: reference/lib/ReferenceAssertions.sol 2: pragma solidity ^0.8.7;
File: reference/shim/Shim.sol 2: pragma solidity ^0.8.7;
File: reference/ReferenceConsideration.sol 2: pragma solidity ^0.8.7;
There are 20 instances of this issue:
File: reference/conduit/ReferenceConduitController.sol 2: pragma solidity ^0.8.7;
File: reference/conduit/ReferenceConduit.sol 2: pragma solidity ^0.8.7;
File: reference/lib/ReferenceSignatureVerification.sol 2: pragma solidity ^0.8.7;
File: reference/lib/ReferenceExecutor.sol 2: pragma solidity ^0.8.7;
File: reference/lib/ReferenceOrderCombiner.sol 2: pragma solidity ^0.8.7;
File: reference/lib/ReferenceFulfillmentApplier.sol 2: pragma solidity ^0.8.7;
File: reference/lib/ReferenceOrderValidator.sol 2: pragma solidity ^0.8.7;
File: reference/lib/ReferenceZoneInteraction.sol 2: pragma solidity ^0.8.7;
File: reference/lib/ReferenceGettersAndDerivers.sol 2: pragma solidity ^0.8.7;
File: reference/lib/ReferenceAmountDeriver.sol 2: pragma solidity ^0.8.7;
File: reference/lib/ReferenceOrderFulfiller.sol 2: pragma solidity ^0.8.7;
File: reference/lib/ReferenceBasicOrderFulfiller.sol 2: pragma solidity ^0.8.7;
File: reference/lib/ReferenceNonceManager.sol 2: pragma solidity ^0.8.7;
File: reference/lib/ReferenceCriteriaResolution.sol 2: pragma solidity ^0.8.7;
File: reference/lib/ReferenceReentrancyGuard.sol 2: pragma solidity ^0.8.7;
File: reference/lib/ReferenceVerifiers.sol 2: pragma solidity ^0.8.7;
File: reference/lib/ReferenceTokenTransferrer.sol 2: pragma solidity ^0.8.7;
File: reference/lib/ReferenceConsiderationBase.sol 2: pragma solidity ^0.8.7;
File: reference/lib/ReferenceAssertions.sol 2: pragma solidity ^0.8.7;
File: reference/ReferenceConsideration.sol 2: pragma solidity ^0.8.7;
In most places throughout the code, external, payable, and returns (<x>) each appear on their respective own lines when the full function signature doesn't fit within 80 characters. The examples below don't follow this pattern
There are 6 instances of this issue:
File: contracts/interfaces/ConsiderationInterface.sol 118: ) external payable returns (bool fulfilled); 273: ) external payable returns (Execution[] memory executions); 315: ) external payable returns (Execution[] memory executions);
File: contracts/interfaces/SeaportInterface.sol 117: ) external payable returns (bool fulfilled); 272: ) external payable returns (Execution[] memory executions); 314: ) external payable returns (Execution[] memory executions);
The examples below have functions where there are no named arguments. The other interfaces defined in this file are pretty similar, but have named arguments, as is the pattern throughout the rest of the interfaces in the rest of the files
There are 2 instances of this issue:
File: contracts/interfaces/AbridgedTokenInterfaces.sol #1 5 function transferFrom( 6 address, 7 address, 8 uint256 9: ) external returns (bool);
File: contracts/interfaces/AbridgedTokenInterfaces.sol #2 13 function transferFrom( 14 address, 15 address, 16 uint256 17: ) external;
The code block below is copy-pasted from here with only minor modifications. The code should be refactored to a common function that takes in an array and does the transfers. The non-reference version of the file has the same issue, but I'm assuming that you'd want that one to stay as-is to avoid the extra JUMP associated with a function call
There is 1 instance of this issue:
File: reference/conduit/ReferenceConduit.sol #1 88 uint256 totalStandardTransfers = standardTransfers.length; 89 90 // Iterate over each standard transfer. 91 for (uint256 i = 0; i < totalStandardTransfers; i++) { 92 // Retrieve the transfer in question. 93 ConduitTransfer calldata standardTransfer = standardTransfers[i]; 94 95 // Perform the transfer. 96 _transfer(standardTransfer); 97: }
Having orders live around forever makes off-chain tracking via events, more difficult. By allowing anyone to cancel orders whose end time has passed, will allow some orders to be cleaned up
There is 1 instance of this issue:
File: contracts/lib/Consideration.sol #1 413 * @notice Cancel an arbitrary number of orders. Note that only the offerer 414 * or the zone of a given order may cancel it. Callers should ensure 415 * that the intended order was cancelled by calling `getOrderStatus` 416 * and confirming that `isCancelled` returns `true`. 417 * 418 * @param orders The orders to cancel. 419 * 420 * @return cancelled A boolean indicating whether the supplied orders have 421 * been successfully cancelled. 422 */ 423: function cancel(OrderComponents[] calldata orders)
There are 7 instances of this issue:
File: contracts/lib/FulfillmentApplier.sol /// @audit receipient 180: // Set the offerer as the receipient if execution amount is nonzero.
File: contracts/lib/Assertions.sol /// @audit amont 92: // Revert if the supplied amont is equal to zero.
File: contracts/interfaces/SeaportInterface.sol /// @audit toreceive 287: * order toreceive ERC1155 tokens. Also note that
File: contracts/interfaces/ZoneInteractionErrors.sol /// @audit offerrer 13: * either the offerrer or the order's zone or approved as valid by the
File: contracts/interfaces/TokenTransferrerErrors.sol /// @audit falsey 56: * @dev Revert with an error when an ERC20 token transfer returns a falsey
File: contracts/interfaces/ConsiderationInterface.sol /// @audit toreceive 288: * order toreceive ERC1155 tokens. Also note that
File: reference/lib/ReferenceExecutor.sol /// @audit invalud 436: // Revert with an error indicating an invalud conduit.
There are 8 instances of this issue:
File: contracts/conduit/lib/ConduitEnums.sol
File: contracts/conduit/lib/ConduitStructs.sol
File: contracts/lib/ConsiderationConstants.sol
File: contracts/lib/ConsiderationEnums.sol
File: contracts/lib/TokenTransferrerConstants.sol
File: contracts/interfaces/ZoneInterface.sol
File: contracts/interfaces/AbridgedTokenInterfaces.sol
File: contracts/interfaces/EIP1271Interface.sol
There are 13 instances of this issue:
File: contracts/conduit/ConduitController.sol /// @audit Missing: '@param newPotentialOwner' 181 /** 182 * @notice Initiate conduit ownership transfer by assigning a new potential 183 * owner for the given conduit. Once set, the new potential owner 184 * may call `acceptOwnership` to claim ownership of the conduit. 185 * Only the owner of the conduit in question may call this function. 186 * 187 * @param conduit The conduit for which to initiate ownership transfer. 188 */ 189: function transferOwnership(address conduit, address newPotentialOwner)
File: contracts/lib/BasicOrderFulfiller.sol /// @audit Missing: '@param accumulator' 1001 /** 1002 * @dev Internal function to transfer ERC20 tokens to a given recipient as 1003 * part of basic order fulfillment. 1004 * 1005 * @param from The originator of the ERC20 token transfer. 1006 * @param to The recipient of the ERC20 token transfer. 1007 * @param erc20Token The ERC20 token to transfer. 1008 * @param amount The amount of ERC20 tokens to transfer. 1009 * @param additionalRecipients The additional recipients of the order. 1010 * @param fromOfferer A boolean indicating whether to decrement 1011 * amount from the offered amount. 1012 */ 1013 function _transferERC20AndFinalize( 1014 address from, 1015 address to, 1016 address erc20Token, 1017 uint256 amount, 1018 AdditionalRecipient[] calldata additionalRecipients, 1019 bool fromOfferer, 1020: bytes memory accumulator
File: contracts/lib/AmountDeriver.sol /// @audit Missing: '@param roundUp' 123 /** 124 * @dev Internal pure function to apply a fraction to a consideration 125 * or offer item. 126 * 127 * @param startAmount The starting amount of the item. 128 * @param endAmount The ending amount of the item. 129 * @param numerator A value indicating the portion of the order that 130 * should be filled. 131 * @param denominator A value indicating the total size of the order. 132 * @param elapsed The time elapsed since the order's start time. 133 * @param remaining The time left until the order's end time. 134 * @param duration The total duration of the order. 135 * 136 * @return amount The received item to transfer with the final amount. 137 */ 138 function _applyFraction( 139 uint256 startAmount, 140 uint256 endAmount, 141 uint256 numerator, 142 uint256 denominator, 143 uint256 elapsed, 144 uint256 remaining, 145 uint256 duration, 146 bool roundUp 147: ) internal pure returns (uint256 amount) {
File: contracts/interfaces/ConduitControllerInterface.sol /// @audit Missing: '@param newPotentialOwner' 139 /** 140 * @notice Initiate conduit ownership transfer by assigning a new potential 141 * owner for the given conduit. Once set, the new potential owner 142 * may call `acceptOwnership` to claim ownership of the conduit. 143 * Only the owner of the conduit in question may call this function. 144 * 145 * @param conduit The conduit for which to initiate ownership transfer. 146 */ 147: function transferOwnership(address conduit, address newPotentialOwner)
File: reference/conduit/ReferenceConduitController.sol /// @audit Missing: '@param newPotentialOwner' 184 /** 185 * @dev External function for initiating conduit ownership transfer by 186 * assigning a new potential owner for the given conduit. Once set, the 187 * new potential owner may call `acceptOwnership` to claim ownership of 188 * the conduit. Only the owner of the conduit in question may call this 189 * function. 190 * 191 * @param conduit The conduit for which to initiate ownership transfer. 192 */ 193: function transferOwnership(address conduit, address newPotentialOwner)
File: reference/lib/ReferenceAmountDeriver.sol /// @audit Missing: '@param roundUp' 103 /** 104 * @dev Internal pure function to apply a fraction to a consideration 105 * or offer item. 106 * 107 * @param startAmount The starting amount of the item. 108 * @param endAmount The ending amount of the item. 109 * @param fractionData A struct containing the data used to apply a 110 * fraction to an order. 111 * @return amount The received item to transfer with the final amount. 112 */ 113 function _applyFraction( 114 uint256 startAmount, 115 uint256 endAmount, 116 FractionData memory fractionData, 117 bool roundUp 118: ) internal pure returns (uint256 amount) {
File: reference/lib/ReferenceBasicOrderFulfiller.sol /// @audit Missing: '@return' 502 * @param fulfillmentItemTypes The fulfillment's item type. 503 */ 504 function _hashOrder( 505 BasicFulfillmentHashes memory hashes, 506 BasicOrderParameters calldata parameters, 507 FulfillmentItemTypes memory fulfillmentItemTypes 508: ) internal view returns (bytes32 orderHash) {
File: reference/lib/ReferenceConsiderationBase.sol /// @audit Missing: '@param _eip712DomainTypeHash' 87 /** 88 * @dev Internal view function to derive the initial EIP-712 domain 89 * separator. 90 * 91 * @return The derived domain separator. 92 */ 93 function _deriveInitialDomainSeparator( 94 bytes32 _eip712DomainTypeHash, 95 bytes32 _nameHash, 96 bytes32 _versionHash 97: ) internal view virtual returns (bytes32) { /// @audit Missing: '@param _nameHash' 87 /** 88 * @dev Internal view function to derive the initial EIP-712 domain 89 * separator. 90 * 91 * @return The derived domain separator. 92 */ 93 function _deriveInitialDomainSeparator( 94 bytes32 _eip712DomainTypeHash, 95 bytes32 _nameHash, 96 bytes32 _versionHash 97: ) internal view virtual returns (bytes32) { /// @audit Missing: '@param _versionHash' 87 /** 88 * @dev Internal view function to derive the initial EIP-712 domain 89 * separator. 90 * 91 * @return The derived domain separator. 92 */ 93 function _deriveInitialDomainSeparator( 94 bytes32 _eip712DomainTypeHash, 95 bytes32 _nameHash, 96 bytes32 _versionHash 97: ) internal view virtual returns (bytes32) { /// @audit Missing: '@param _eip712DomainTypeHash' 106 /** 107 * @dev Internal view function to derive the EIP-712 domain separator. 108 * 109 * @return The derived domain separator. 110 */ 111 function _deriveDomainSeparator( 112 bytes32 _eip712DomainTypeHash, 113 bytes32 _nameHash, 114 bytes32 _versionHash 115: ) internal view virtual returns (bytes32) { /// @audit Missing: '@param _nameHash' 106 /** 107 * @dev Internal view function to derive the EIP-712 domain separator. 108 * 109 * @return The derived domain separator. 110 */ 111 function _deriveDomainSeparator( 112 bytes32 _eip712DomainTypeHash, 113 bytes32 _nameHash, 114 bytes32 _versionHash 115: ) internal view virtual returns (bytes32) { /// @audit Missing: '@param _versionHash' 106 /** 107 * @dev Internal view function to derive the EIP-712 domain separator. 108 * 109 * @return The derived domain separator. 110 */ 111 function _deriveDomainSeparator( 112 bytes32 _eip712DomainTypeHash, 113 bytes32 _nameHash, 114 bytes32 _versionHash 115: ) internal view virtual returns (bytes32) {
indexed fieldsEach event should use three indexed fields if there are three or more fields
There are 6 instances of this issue:
File: contracts/interfaces/ConsiderationEventsAndErrors.sol 25 event OrderFulfilled( 26 bytes32 orderHash, 27 address indexed offerer, 28 address indexed zone, 29 address fulfiller, 30 SpentItem[] offer, 31 ReceivedItem[] consideration 32: ); 41 event OrderCancelled( 42 bytes32 orderHash, 43 address indexed offerer, 44 address indexed zone 45: ); 56 event OrderValidated( 57 bytes32 orderHash, 58 address indexed offerer, 59 address indexed zone 60: ); 68: event NonceIncremented(uint256 newNonce, address indexed offerer);
File: contracts/interfaces/ConduitInterface.sol 48: event ChannelUpdated(address channel, bool open);
File: contracts/interfaces/ConduitControllerInterface.sol 29: event NewConduit(address conduit, bytes32 conduitKey);
Consider changing the variable to be an unnamed one
There are 46 instances of this issue:
File: contracts/lib/OrderCombiner.sol /// @audit executions 704 function _matchAdvancedOrders( 705 AdvancedOrder[] memory advancedOrders, 706 CriteriaResolver[] memory criteriaResolvers, 707 Fulfillment[] calldata fulfillments 708: ) internal returns (Execution[] memory executions) {
File: contracts/lib/OrderValidator.sol /// @audit newNumerator 104 function _validateOrderAndUpdateStatus( 105 AdvancedOrder memory advancedOrder, 106 CriteriaResolver[] memory criteriaResolvers, 107 bool revertOnInvalid, 108 bytes32[] memory priorOrderHashes 109 ) 110 internal 111 returns ( 112 bytes32 orderHash, 113 uint256 newNumerator, 114: uint256 newDenominator /// @audit newDenominator 104 function _validateOrderAndUpdateStatus( 105 AdvancedOrder memory advancedOrder, 106 CriteriaResolver[] memory criteriaResolvers, 107 bool revertOnInvalid, 108 bytes32[] memory priorOrderHashes 109 ) 110 internal 111 returns ( 112 bytes32 orderHash, 113 uint256 newNumerator, 114: uint256 newDenominator /// @audit isValidated 418 function _getOrderStatus(bytes32 orderHash) 419 internal 420 view 421 returns ( 422 bool isValidated, 423 bool isCancelled, 424 uint256 totalFilled, 425: uint256 totalSize /// @audit isCancelled 418 function _getOrderStatus(bytes32 orderHash) 419 internal 420 view 421 returns ( 422 bool isValidated, 423 bool isCancelled, 424 uint256 totalFilled, 425: uint256 totalSize /// @audit totalFilled 418 function _getOrderStatus(bytes32 orderHash) 419 internal 420 view 421 returns ( 422 bool isValidated, 423 bool isCancelled, 424 uint256 totalFilled, 425: uint256 totalSize /// @audit totalSize 418 function _getOrderStatus(bytes32 orderHash) 419 internal 420 view 421 returns ( 422 bool isValidated, 423 bool isCancelled, 424 uint256 totalFilled, 425: uint256 totalSize
File: contracts/lib/Consideration.sol /// @audit availableOrders 215 function fulfillAvailableOrders( 216 Order[] calldata orders, 217 FulfillmentComponent[][] calldata offerFulfillments, 218 FulfillmentComponent[][] calldata considerationFulfillments, 219 bytes32 fulfillerConduitKey, 220 uint256 maximumFulfilled 221 ) 222 external 223 payable 224 override 225: returns (bool[] memory availableOrders, Execution[] memory executions) /// @audit executions 215 function fulfillAvailableOrders( 216 Order[] calldata orders, 217 FulfillmentComponent[][] calldata offerFulfillments, 218 FulfillmentComponent[][] calldata considerationFulfillments, 219 bytes32 fulfillerConduitKey, 220 uint256 maximumFulfilled 221 ) 222 external 223 payable 224 override 225: returns (bool[] memory availableOrders, Execution[] memory executions) /// @audit availableOrders 300 function fulfillAvailableAdvancedOrders( 301 AdvancedOrder[] memory advancedOrders, 302 CriteriaResolver[] calldata criteriaResolvers, 303 FulfillmentComponent[][] calldata offerFulfillments, 304 FulfillmentComponent[][] calldata considerationFulfillments, 305 bytes32 fulfillerConduitKey, 306 uint256 maximumFulfilled 307 ) 308 external 309 payable 310 override 311: returns (bool[] memory availableOrders, Execution[] memory executions) /// @audit executions 300 function fulfillAvailableAdvancedOrders( 301 AdvancedOrder[] memory advancedOrders, 302 CriteriaResolver[] calldata criteriaResolvers, 303 FulfillmentComponent[][] calldata offerFulfillments, 304 FulfillmentComponent[][] calldata considerationFulfillments, 305 bytes32 fulfillerConduitKey, 306 uint256 maximumFulfilled 307 ) 308 external 309 payable 310 override 311: returns (bool[] memory availableOrders, Execution[] memory executions) /// @audit executions 349 function matchOrders( 350 Order[] calldata orders, 351 Fulfillment[] calldata fulfillments 352: ) external payable override returns (Execution[] memory executions) { /// @audit executions 398 function matchAdvancedOrders( 399 AdvancedOrder[] memory advancedOrders, 400 CriteriaResolver[] calldata criteriaResolvers, 401 Fulfillment[] calldata fulfillments 402: ) external payable override returns (Execution[] memory executions) { /// @audit isValidated 517 function getOrderStatus(bytes32 orderHash) 518 external 519 view 520 override 521 returns ( 522 bool isValidated, 523 bool isCancelled, 524 uint256 totalFilled, 525: uint256 totalSize /// @audit isCancelled 517 function getOrderStatus(bytes32 orderHash) 518 external 519 view 520 override 521 returns ( 522 bool isValidated, 523 bool isCancelled, 524 uint256 totalFilled, 525: uint256 totalSize /// @audit totalFilled 517 function getOrderStatus(bytes32 orderHash) 518 external 519 view 520 override 521 returns ( 522 bool isValidated, 523 bool isCancelled, 524 uint256 totalFilled, 525: uint256 totalSize /// @audit totalSize 517 function getOrderStatus(bytes32 orderHash) 518 external 519 view 520 override 521 returns ( 522 bool isValidated, 523 bool isCancelled, 524 uint256 totalFilled, 525: uint256 totalSize /// @audit version 556 function information() 557 external 558 view 559 override 560 returns ( 561 string memory version, 562 bytes32 domainSeparator, 563: address conduitController /// @audit domainSeparator 556 function information() 557 external 558 view 559 override 560 returns ( 561 string memory version, 562 bytes32 domainSeparator, 563: address conduitController /// @audit conduitController 556 function information() 557 external 558 view 559 override 560 returns ( 561 string memory version, 562 bytes32 domainSeparator, 563: address conduitController
File: reference/conduit/ReferenceConduit.sol /// @audit magicValue 36 function execute(ConduitTransfer[] calldata transfers) 37 external 38 override 39: returns (bytes4 magicValue) /// @audit magicValue 59 function executeBatch1155( 60 ConduitBatch1155Transfer[] calldata batchTransfers 61: ) external override returns (bytes4 magicValue) { /// @audit magicValue 80 function executeWithBatch1155( 81 ConduitTransfer[] calldata standardTransfers, 82 ConduitBatch1155Transfer[] calldata batchTransfers 83: ) external override returns (bytes4 magicValue) {
File: reference/lib/ReferenceOrderCombiner.sol /// @audit executions 694 function _matchAdvancedOrders( 695 AdvancedOrder[] memory advancedOrders, 696 CriteriaResolver[] memory criteriaResolvers, 697 Fulfillment[] calldata fulfillments 698: ) internal returns (Execution[] memory executions) {
File: reference/lib/ReferenceFulfillmentApplier.sol /// @audit invalidFulfillment 238 function _checkMatchingOffer( 239 OrderToExecute memory orderToExecute, 240 SpentItem memory offer, 241 Execution memory execution 242: ) internal pure returns (bool invalidFulfillment) { /// @audit invalidFulfillment 407 function _checkMatchingConsideration( 408 ReceivedItem memory consideration, 409 ReceivedItem memory receivedItem 410: ) internal pure returns (bool invalidFulfillment) {
File: reference/lib/ReferenceOrderValidator.sol /// @audit newNumerator 109 function _validateOrderAndUpdateStatus( 110 AdvancedOrder memory advancedOrder, 111 CriteriaResolver[] memory criteriaResolvers, 112 bool revertOnInvalid, 113 bytes32[] memory priorOrderHashes 114 ) 115 internal 116 returns ( 117 bytes32 orderHash, 118 uint256 newNumerator, 119: uint256 newDenominator /// @audit newDenominator 109 function _validateOrderAndUpdateStatus( 110 AdvancedOrder memory advancedOrder, 111 CriteriaResolver[] memory criteriaResolvers, 112 bool revertOnInvalid, 113 bytes32[] memory priorOrderHashes 114 ) 115 internal 116 returns ( 117 bytes32 orderHash, 118 uint256 newNumerator, 119: uint256 newDenominator /// @audit isValidated 388 function _getOrderStatus(bytes32 orderHash) 389 internal 390 view 391 returns ( 392 bool isValidated, 393 bool isCancelled, 394 uint256 totalFilled, 395: uint256 totalSize /// @audit isCancelled 388 function _getOrderStatus(bytes32 orderHash) 389 internal 390 view 391 returns ( 392 bool isValidated, 393 bool isCancelled, 394 uint256 totalFilled, 395: uint256 totalSize /// @audit totalFilled 388 function _getOrderStatus(bytes32 orderHash) 389 internal 390 view 391 returns ( 392 bool isValidated, 393 bool isCancelled, 394 uint256 totalFilled, 395: uint256 totalSize /// @audit totalSize 388 function _getOrderStatus(bytes32 orderHash) 389 internal 390 view 391 returns ( 392 bool isValidated, 393 bool isCancelled, 394 uint256 totalFilled, 395: uint256 totalSize
File: reference/lib/ReferenceGettersAndDerivers.sol /// @audit orderHash 91 function _deriveOrderHash( 92 OrderParameters memory orderParameters, 93 uint256 nonce 94: ) internal view returns (bytes32 orderHash) {
File: reference/ReferenceConsideration.sol /// @audit availableOrders 234 function fulfillAvailableOrders( 235 Order[] calldata orders, 236 FulfillmentComponent[][] calldata offerFulfillments, 237 FulfillmentComponent[][] calldata considerationFulfillments, 238 bytes32 fulfillerConduitKey, 239 uint256 maximumFulfilled 240 ) 241 external 242 payable 243 override 244 notEntered 245 nonReentrant 246: returns (bool[] memory availableOrders, Execution[] memory executions) /// @audit executions 234 function fulfillAvailableOrders( 235 Order[] calldata orders, 236 FulfillmentComponent[][] calldata offerFulfillments, 237 FulfillmentComponent[][] calldata considerationFulfillments, 238 bytes32 fulfillerConduitKey, 239 uint256 maximumFulfilled 240 ) 241 external 242 payable 243 override 244 notEntered 245 nonReentrant 246: returns (bool[] memory availableOrders, Execution[] memory executions) /// @audit availableOrders 332 function fulfillAvailableAdvancedOrders( 333 AdvancedOrder[] memory advancedOrders, 334 CriteriaResolver[] calldata criteriaResolvers, 335 FulfillmentComponent[][] calldata offerFulfillments, 336 FulfillmentComponent[][] calldata considerationFulfillments, 337 bytes32 fulfillerConduitKey, 338 uint256 maximumFulfilled 339 ) 340 external 341 payable 342 override 343 notEntered 344 nonReentrant 345: returns (bool[] memory availableOrders, Execution[] memory executions) /// @audit executions 332 function fulfillAvailableAdvancedOrders( 333 AdvancedOrder[] memory advancedOrders, 334 CriteriaResolver[] calldata criteriaResolvers, 335 FulfillmentComponent[][] calldata offerFulfillments, 336 FulfillmentComponent[][] calldata considerationFulfillments, 337 bytes32 fulfillerConduitKey, 338 uint256 maximumFulfilled 339 ) 340 external 341 payable 342 override 343 notEntered 344 nonReentrant 345: returns (bool[] memory availableOrders, Execution[] memory executions) /// @audit executions 390 function matchOrders( 391 Order[] calldata orders, 392 Fulfillment[] calldata fulfillments 393 ) 394 external 395 payable 396 override 397 notEntered 398 nonReentrant 399: returns (Execution[] memory executions) /// @audit executions 446 function matchAdvancedOrders( 447 AdvancedOrder[] memory advancedOrders, 448 CriteriaResolver[] calldata criteriaResolvers, 449 Fulfillment[] calldata fulfillments 450 ) 451 external 452 payable 453 override 454 notEntered 455 nonReentrant 456: returns (Execution[] memory executions) /// @audit isValidated 574 function getOrderStatus(bytes32 orderHash) 575 external 576 view 577 override 578 returns ( 579 bool isValidated, 580 bool isCancelled, 581 uint256 totalFilled, 582: uint256 totalSize /// @audit isCancelled 574 function getOrderStatus(bytes32 orderHash) 575 external 576 view 577 override 578 returns ( 579 bool isValidated, 580 bool isCancelled, 581 uint256 totalFilled, 582: uint256 totalSize /// @audit totalFilled 574 function getOrderStatus(bytes32 orderHash) 575 external 576 view 577 override 578 returns ( 579 bool isValidated, 580 bool isCancelled, 581 uint256 totalFilled, 582: uint256 totalSize /// @audit totalSize 574 function getOrderStatus(bytes32 orderHash) 575 external 576 view 577 override 578 returns ( 579 bool isValidated, 580 bool isCancelled, 581 uint256 totalFilled, 582: uint256 totalSize /// @audit version 613 function information() 614 external 615 view 616 override 617 returns ( 618 string memory version, 619 bytes32 domainSeparator, 620: address conduitController /// @audit domainSeparator 613 function information() 614 external 615 view 616 override 617 returns ( 618 string memory version, 619 bytes32 domainSeparator, 620: address conduitController /// @audit conduitController 613 function information() 614 external 615 view 616 override 617 returns ( 618 string memory version, 619 bytes32 domainSeparator, 620: address conduitController
#0 - liveactionllama
2022-06-03T17:31:33Z
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.
#1 - HardlyDifficult
2022-06-20T16:05:19Z
🌟 Selected for report: Dravee
Also found by: 0x1f8b, 0x29A, 0xalpharush, Chom, Czar102, Hawkeye, IllIllI, MaratCerby, MiloTruck, NoamYakov, OriDabush, RoiEvenHaim, Spearbit, Tadashi, TerrierLover, TomJ, asutorufos, cccz, cmichel, csanuragjain, defsec, delfin454000, djxploit, ellahi, foobar, gzeon, hake, hickuphh3, ignacio, ilan, joestakey, kaden, mayo, ming, oyc_109, peritoflores, rfa, sach1r0, sashik_eth, shung, sirhashalot, twojoy, zer0dot, zkhorse
1290.2048 USDC - $1,290.20
| 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
calldata instead of memory for read-only arguments in external functions saves gasWhen 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,
File: reference/ReferenceConsideration.sol #2 447: AdvancedOrder[] memory advancedOrders,
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) !=
File: reference/lib/ReferenceExecutor.sol 332 ConduitInterface(_getConduit(accumulatorStruct.conduitKey)).execute( 333 accumulatorStruct.transfers 334: );
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
File: reference/lib/ReferenceConsiderationBase.sol 83: tempConduitController.getConduitCodeHashes()
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];
File: contracts/lib/CriteriaResolution.sol /// @audit advancedOrders[orderIndex] 77: advancedOrders[orderIndex].parameters
File: contracts/lib/FulfillmentApplier.sol /// @audit advancedOrders[<etc>] 109 advancedOrders[targetComponent.orderIndex] 110: .parameters
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;
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];
File: reference/lib/ReferenceOrderCombiner.sol /// @audit ordersToExecute[i] 375 ReceivedItem[] memory receivedItems = ordersToExecute[i] 376: .receivedItems;
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;
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;
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;
_assertConduitExists() should return the storage variable it looks upEach 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) {
internal functions only called once can be inlined to save gasNot 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
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
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
File: contracts/lib/Executor.sol 456: function _trigger(bytes32 conduitKey, bytes memory accumulator) internal {
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) {
File: contracts/lib/OrderValidator.sol 450 function _doesNotSupportPartialFills(OrderType orderType) 451 internal 452 pure 453: returns (bool isFullOrder)
File: contracts/lib/OrderFulfiller.sol 155 function _applyFractionsAndTransferEach( 156 OrderParameters memory orderParameters, 157 uint256 numerator, 158 uint256 denominator, 159 bytes32 offererConduitKey, 160: bytes32 fulfillerConduitKey
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)
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) {
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) {
File: reference/lib/ReferenceOrderValidator.sol 420 function _doesNotSupportPartialFills(OrderType orderType) 421 internal 422 pure 423: returns (bool isFullOrder)
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)
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)
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
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
<array>.length should not be looked up in every loop of a for-loopThe overheads outlined below are PER LOOP, excluding the first loop
MLOAD (3 gas)CALLDATALOAD (3 gas)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; ) {
File: contracts/lib/OrderFulfiller.sol 217: for (uint256 i = 0; i < orderParameters.offer.length; ) { 306: for (uint256 i = 0; i < orderParameters.consideration.length; ) {
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) {
File: reference/lib/ReferenceFulfillmentApplier.sol 308: i < offerComponents.length; 481: i < considerationComponents.length;
File: reference/lib/ReferenceGettersAndDerivers.sol 104: for (uint256 i = 0; i < orderParameters.offer.length; ++i) {
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) {
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) {
File: reference/lib/ReferenceCriteriaResolution.sol 378: for (uint256 i = 0; i < proof.length; ++i) {
++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-loopsThe 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++) {
File: reference/lib/ReferenceExecutor.sol 382: for (uint256 i = 0; i < currentTransferLength; ++i) {
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) {
File: reference/lib/ReferenceFulfillmentApplier.sol 170: for (uint256 i = 0; i < totalFulfillmentComponents; ++i) { 309: ++i 482: ++i
File: reference/lib/ReferenceOrderValidator.sol 265: for (uint256 i = 0; i < totalOrders; ++i) { 329: for (uint256 i = 0; i < totalOrders; ++i) {
File: reference/lib/ReferenceGettersAndDerivers.sol 104: for (uint256 i = 0; i < orderParameters.offer.length; ++i) { 113: ++i
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) {
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) {
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) {
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 {
File: contracts/interfaces/SeaportInterface.sol 28: interface SeaportInterface {
File: contracts/interfaces/ConduitInterface.sol 16: interface ConduitInterface {
File: contracts/interfaces/ConduitControllerInterface.sol 10: interface ConduitControllerInterface {
File: contracts/interfaces/ImmutableCreate2FactoryInterface.sol 18: interface ImmutableCreate2FactoryInterface {
File: contracts/interfaces/ConsiderationInterface.sol 29: interface ConsiderationInterface {
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;
File: reference/conduit/ReferenceConduit.sol #2 30: mapping(address => bool) private _channels;
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; ) {
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) {
File: contracts/lib/BasicOrderFulfiller.sol 948: for (uint256 i = 0; i < totalAdditionalRecipients; ) { 1040: for (uint256 i = 0; i < totalAdditionalRecipients; ) {
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) {
File: contracts/lib/OrderValidator.sol 272: for (uint256 i = 0; i < totalOrders; ) { 350: for (uint256 i = 0; i < totalOrders; ) {
File: contracts/lib/AmountDeriver.sol 44: uint256 extraCeiling = 0;
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) {
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++) {
File: reference/lib/ReferenceExecutor.sol 382: for (uint256 i = 0; i < currentTransferLength; ++i) {
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) {
File: reference/lib/ReferenceFulfillmentApplier.sol 167: uint256 nextComponentIndex = 0; 170: for (uint256 i = 0; i < totalFulfillmentComponents; ++i) {
File: reference/lib/ReferenceOrderValidator.sol 265: for (uint256 i = 0; i < totalOrders; ++i) { 329: for (uint256 i = 0; i < totalOrders; ++i) {
File: reference/lib/ReferenceGettersAndDerivers.sol 104: for (uint256 i = 0; i < orderParameters.offer.length; ++i) { 111: uint256 i = 0;
File: reference/lib/ReferenceAmountDeriver.sol 46: uint256 extraCeiling = 0;
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) {
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) {
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) {
++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++) {
File: reference/lib/ReferenceBasicOrderFulfiller.sol 644: recipientCount++ 715: additionalTips++
uints/ints smaller than 32 bytes (256 bits) incurs overheadWhen 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;
File: contracts/lib/ConsiderationStructs.sol 172: uint120 numerator; 173: uint120 denominator; 188: uint120 numerator; 189: uint120 denominator;
File: contracts/interfaces/SignatureVerificationErrors.sol 17: error BadSignatureV(uint8 v);
File: reference/lib/ReferenceSignatureVerification.sol 40: uint8 v;
File: reference/lib/ReferenceConsiderationStructs.sol 61: uint120 numerator;
#0 - liveactionllama
2022-06-03T17:28:57Z
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.
#1 - HardlyDifficult
2022-06-26T15:16:57Z
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.
<array>.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.