OpenSea Seaport 1.2 contest - 0xSmartContract's results

Summary

Low Risk Issues List

Total 2 issues

Non-Critical Issues List

Total 14 issues

[L-01] Missing Event for initiazings

Context:

15 results - 15 files

contracts/Seaport.sol:
  93:     constructor(address conduitController) Consideration(conduitController) {}

contracts/conduit/Conduit.sol:
  73:     constructor() {

contracts/conduit/ConduitController.sol:
  31:     constructor() {

contracts/lib/Assertions.sol:
  35:     constructor(

contracts/lib/BasicOrderFulfiller.sol:
  41:     constructor(address conduitController) OrderValidator(conduitController) {}

contracts/lib/Consideration.sol:
  50:     constructor(address conduitController) OrderCombiner(conduitController) {}

contracts/lib/ConsiderationBase.sol:
  56:     constructor(address conduitController) {

contracts/lib/Executor.sol:
  35:     constructor(address conduitController) Verifiers(conduitController) {}

contracts/lib/GettersAndDerivers.sol:
  25:     constructor(

contracts/lib/OrderCombiner.sol:
  41:     constructor(address conduitController) OrderFulfiller(conduitController) {}

contracts/lib/OrderFulfiller.sol:
  45:     constructor(

contracts/lib/OrderValidator.sol:
  55:     constructor(address conduitController) Executor(conduitController) {}

contracts/lib/ReentrancyGuard.sol:
  23:     constructor() {

contracts/lib/TypehashDirectory.sol:
  30:     constructor() {

contracts/lib/Verifiers.sol:
  26:     constructor(address conduitController) Assertions(conduitController) {}

Description: Events help non-contract tools to track changes Many utilities and analysis frameworks make use of event-emits and are monitored at program startups.

Recommendation: Add Event-Emit

[L-02] Use Create3 for deploy to a new EVM chain

Context:

1 result - 1 file

README.md:
  171  
  172: Rinkeby
  173: Goerli
  174: Kovan
  175: Sepolia
  176: Polygon
  177: Mumbai
  178: Optimism
  179: Optimistic Kovan
  180: Arbitrum
  181: Arbitrum Nova
  182: Arbitrum Rinkeby
  183: Avalanche Fuji
  184: Avalanche C-Chain
  185: Gnosis Chain
  186: BSC
  188: 
  189: To be deployed on other EVM chains, such as:
  190: 
  191: - Klaytn
  192: - Baobab
  193: - Skale
  194: - Celo
  195: - Fantom
  196: - RSK
  197: 
  198: To deploy to a new EVM chain, follow the [steps outlined here](docs/Deployment.md).

Seaport and the ConduitController can be deployed to the same address on all EVM chains using the CREATE2 Factory.

https://github.com/ProjectOpenSea/seaport/blob/5de7302bc773d9821ba4759e47fc981680911ea0/docs/Deployment.md

Description: Deploying a contract to multiple chains with the same address is annoying. One usually would create a new Ethereum account, seed it with enough tokens to pay for gas on every chain, and then deploy the contract naively

This relies on the fact that the new account's nonce is synced on all the chains, therefore resulting in the same contract address. However, deployment is often a complex process that involves several transactions (e.g. for initialization), which means it's easy for nonces to fall out of sync and make it forever impossible to deploy the contract at the desired address.

One could use a CREATE2 factory that deterministically deploys contracts to an address that's unrelated to the deployer's nonce, but the address is still related to the hash of the contract's creation code.

This means if you wanted to use different constructor parameters on different chains, the deployed contracts will have different addresses.

A CREATE3 factory offers the best solution: the address of the deployed contract is determined by only the deployer address and the salt. This makes it far easier to deploy contracts to multiple chains at the same addresses

https://github.com/ZeframLou/create3-factory

[N-01]  Use "memory-safe" keyword for assembly

Without the use of inline assembly, the compiler can rely on memory to remain in a well-defined state at all times. This is especially relevant for the new code generation pipeline via Yul IR: this code generation path can move local variables from stack to memory to avoid stack-too-deep errors and perform additional memory optimizations, if it can rely on certain assumptions about memory use

While we recommend to always respect Solidity’s memory model, inline assembly allows you to use memory in an incompatible way. Therefore, moving stack variables to memory and additional memory optimizations are, by default, disabled in the presence of any inline assembly block that contains a memory operation or assigns to Solidity variables in memory.

However, you can specifically annotate an assembly block to indicate that it in fact respects Solidity’s memory model as follows:

assembly ("memory-safe") {

https://docs.soliditylang.org/en/v0.8.16/assembly.html#memory-safety

[N-02] For modern and more readable code; update import usages

Context:

28 results - 23 files

contracts/conduit/Conduit.sol:
  14  
  15: import "./lib/ConduitConstants.sol";
  16  

contracts/lib/AmountDeriver.sol:
  7  
  8: import "./ConsiderationConstants.sol";
  9  

contracts/lib/Assertions.sol:
  13  
  14: import "./ConsiderationErrors.sol";
  15  

contracts/lib/BasicOrderFulfiller.sol:
  22  
  23: import "./ConsiderationErrors.sol";
  24  

contracts/lib/Consideration.sol:
  22  
  23: import "../helpers/PointerLibraries.sol";
  24  
  25: import "./ConsiderationConstants.sol";
  26  

contracts/lib/ConsiderationBase.sol:
  11  
  12: import "./ConsiderationConstants.sol";
  13  

contracts/lib/ConsiderationDecoder.sol:
  19  
  20: import "./ConsiderationConstants.sol";
  21  
  22: import "../helpers/PointerLibraries.sol";
  23  

contracts/lib/ConsiderationEncoder.sol:
   3  
   4: import "./ConsiderationConstants.sol";
   5  

  19  
  20: import "../helpers/PointerLibraries.sol";
  21  

contracts/lib/ConsiderationErrors.sol:
  5  
  6: import "./ConsiderationConstants.sol";
  7  

contracts/lib/CounterManager.sol:
   9  
  10: import "./ConsiderationConstants.sol";
  11  

contracts/lib/CriteriaResolution.sol:
  14  
  15: import "./ConsiderationErrors.sol";
  16  
  17: import "../helpers/PointerLibraries.sol";
  18  

contracts/lib/Executor.sol:
  15  
  16: import "./ConsiderationConstants.sol";
  17  
  18: import "./ConsiderationErrors.sol";
  19  

contracts/lib/FulfillmentApplier.sol:
  15  
  16: import "./ConsiderationErrors.sol";
  17  

contracts/lib/GettersAndDerivers.sol:
  7  
  8: import "./ConsiderationConstants.sol";
  9  

contracts/lib/LowLevelHelpers.sol:
  3  
  4: import "./ConsiderationConstants.sol";
  5  

contracts/lib/OrderCombiner.sol:
  22  
  23: import "./ConsiderationErrors.sol";
  24  

contracts/lib/OrderFulfiller.sol:
  22  
  23: import "./ConsiderationErrors.sol";
  24  

contracts/lib/OrderValidator.sol:
  18  
  19: import "./ConsiderationErrors.sol";
  20  

contracts/lib/ReentrancyGuard.sol:
  7  
  8: import "./ConsiderationErrors.sol";
  9  

contracts/lib/SignatureVerification.sol:
  11  
  12: import "./ConsiderationErrors.sol";
  13  

contracts/lib/TokenTransferrer.sol:
  3  
  4: import "./TokenTransferrerConstants.sol";
  5  

contracts/lib/Verifiers.sol:
   9  
  10: import "./ConsiderationErrors.sol";
  11  

contracts/lib/ZoneInteraction.sol:
  15  
  16: import "./ConsiderationEncoder.sol";

Description: Solidity code is also cleaner in another way that might not be noticeable: the struct Point. We were importing it previously with global import but not using it. The Point struct polluted the source code with an unnecessary object we were not using because we did not need it. This was breaking the rule of modularity and modular programming: only import what you need Specific imports with curly braces allow us to apply this rule better.

Recommendation: import {contract1 , contract2} from "filename.sol";

A good example from the ArtGobblers project;

import {Owned} from "solmate/auth/Owned.sol";
import {ERC721} from "solmate/tokens/ERC721.sol";
import {LibString} from "solmate/utils/LibString.sol";
import {MerkleProofLib} from "solmate/utils/MerkleProofLib.sol";
import {FixedPointMathLib} from "solmate/utils/FixedPointMathLib.sol";
import {ERC1155, ERC1155TokenReceiver} from "solmate/tokens/ERC1155.sol";
import {toWadUnsafe, toDaysWadUnsafe} from "solmate/utils/SignedWadMath.sol";

[N-03] Include return parameters in NatSpec comments

Context: All Contracts

Description: It is recommended that Solidity contracts are fully annotated using NatSpec for all public interfaces (everything in the ABI). It is clearly stated in the Solidity official documentation. In complex projects such as Defi, the interpretation of all functions and their arguments and returns is important for code readability and auditability.

https://docs.soliditylang.org/en/v0.8.15/natspec-format.html

Recommendation: Include return parameters in NatSpec comments

Recommendation Code Style: (from Uniswap3)

    /// @notice Adds liquidity for the given recipient/tickLower/tickUpper position
    /// @dev The caller of this method receives a callback in the form of IUniswapV3MintCallback#uniswapV3MintCallback
    /// in which they must pay any token0 or token1 owed for the liquidity. The amount of token0/token1 due depends
    /// on tickLower, tickUpper, the amount of liquidity, and the current price.
    /// @param recipient The address for which the liquidity will be created
    /// @param tickLower The lower tick of the position in which to add liquidity
    /// @param tickUpper The upper tick of the position in which to add liquidity
    /// @param amount The amount of liquidity to mint
    /// @param data Any data that should be passed through to the callback
    /// @return amount0 The amount of token0 that was paid to mint the given amount of liquidity. Matches the value in the callback
    /// @return amount1 The amount of token1 that was paid to mint the given amount of liquidity. Matches the value in the callback
    function mint(
        address recipient,
        int24 tickLower,
        int24 tickUpper,
        uint128 amount,
        bytes calldata data
    ) external returns (uint256 amount0, uint256 amount1);

[N-04] Use of bytes.concat() instead of abi.encodePacked()

contracts/lib/TypehashDirectory.sol:
  176          return
  177:             abi.encodePacked(
  178:                 considerationItemTypeString,
  179:                 offerItemTypeString,
  180:                 orderComponentsPartialTypeString
  181:             );

Rather than using abi.encodePacked for appending bytes, since version 0.8.4, bytes.concat() is enabled

Since version 0.8.4 for appending bytes, bytes.concat() can be used instead of abi.encodePacked(,)

[N-05] For functions, follow Solidity standard naming conventions (internal function style rule)

Context:

340 results - 3 files


Some of the findings to be given as examples;

contracts/helpers/PointerLibraries.sol:
  2785:     function readInt8(MemoryPointer mPtr) internal pure returns (int8 value) {
  2792:     function readInt16(MemoryPointer mPtr) internal pure returns (int16 value) {
  2799:     function readInt24(MemoryPointer mPtr) internal pure returns (int24 value) {
  2806:     function readInt32(MemoryPointer mPtr) internal pure returns (int32 value) {
  2813:     function readInt40(MemoryPointer mPtr) internal pure returns (int40 value) {
  2820:     function readInt48(MemoryPointer mPtr) internal pure returns (int48 value) {
  2827:     function readInt56(MemoryPointer mPtr) internal pure returns (int56 value) {
  2834:     function readInt64(MemoryPointer mPtr) internal pure returns (int64 value) {
  2841:     function readInt72(MemoryPointer mPtr) internal pure returns (int72 value) {
  2848:     function readInt80(MemoryPointer mPtr) internal pure returns (int80 value) {
  2855:     function readInt88(MemoryPointer mPtr) internal pure returns (int88 value) {
  2862:     function readInt96(MemoryPointer mPtr) internal pure returns (int96 value) {
  3051:     function write(MemoryPointer mPtr, MemoryPointer valuePtr) internal pure {
  3058:     function write(MemoryPointer mPtr, bool value) internal pure {
  3065:     function write(MemoryPointer mPtr, address value) internal pure {
  3073:     function writeBytes32(MemoryPointer mPtr, bytes32 value) internal pure {
  3080:     function write(MemoryPointer mPtr, uint256 value) internal pure {
  3088:     function writeInt(MemoryPointer mPtr, int256 value) internal pure {


contracts/lib/TypehashDirectory.sol:
   14:     bytes3 internal constant twoSubstring = 0x5B325D;
   15:     uint256 internal constant twoSubstringLength = 3;
   18:     uint256 internal constant MaxTreeHeight = 24;
   20:     uint256 internal constant InvalidOpcode = 0xfe;
   21:     uint256 internal constant OneWord = 0x20;
   22:     uint256 internal constant OneWordShift = 5;
   23:     uint256 internal constant AlmostOneWord = 0x1f;
   24:     uint256 internal constant FreeMemoryPointerSlot = 0x40;
  131:     function getTreeSubTypes() internal pure returns (bytes memory) {

Description: The above codes don't follow Solidity's standard naming convention,

internal and private functions : the mixedCase format starting with an underscore (_mixedCase starting with an underscore)

[N-06] Pragma version^0.8.17 version too recent to be trusted.

https://github.com/ethereum/solidity/blob/develop/Changelog.md 0.8.17 (2022-09-08) 0.8.16 (2022-08-08) 0.8.15 (2022-06-15) 0.8.10 (2021-11-09)

Unexpected bugs can be reported in recent versions; Risks related to recent releases Risks of complex code generation changes Risks of new language features Risks of known bugs

Use a non-legacy and more battle-tested version Use 0.8.10

[N-07] Tokens accidentally sent to the contract cannot be recovered

It can't be recovered if the tokens accidentally arrive at the contract address, which has happened to many popular projects, so I recommend adding a recovery code to your critical contracts.

Recommended Mitigation Steps

Add this code:

 /**
  * @notice Sends ERC20 tokens trapped in contract to external address
  * @dev Onlyowner is allowed to make this function call
  * @param account is the receiving address
  * @param externalToken is the token being sent
  * @param amount is the quantity being sent
  * @return boolean value indicating whether the operation succeeded.
  *
 */
  function rescueERC20(address account, address externalToken, uint256 amount) public onlyOwner returns (bool) {
    IERC20(externalToken).transfer(account, amount);
    return true;
  }
}

[N-08] Project Upgrade and Stop Scenario should be

At the start of the project, the system may need to be stopped or upgraded, I suggest you have a script beforehand and add it to the documentation. This can also be called an " EMERGENCY STOP (CIRCUIT BREAKER) PATTERN ".

https://github.com/maxwoe/solidity_patterns/blob/master/security/EmergencyStop.sol

[N-09] Use descriptive names for Contracts and Libraries

This codebase will be difficult to navigate, as there are no descriptive naming conventions that specify which files should contain meaningful logic.

Prefixes should be added like this by filing:

Interface I_ absctract contracts Abs_ Libraries Lib_

We recommend that you implement this or a similar agreement.

[N-10] Use a single file for all system-wide constants

There are many addresses and constants used in the system. It is recommended to put the most used ones in one file (for example constants.sol, use inheritance to access these values)

This will help with readability and easier maintenance for future changes. This also helps with any issues, as some of these hard-coded values are admin addresses.

constants.sol Use and import this file in contracts that require access to these values. This is just a suggestion, in some use cases this may result in higher gas usage in the distribution.

538 results - 19 files

[N-11]  Add NatSpec Mapping comment

Description: Add NatSpec comments describing mapping keys and values

Context:

6 results - 5 files

contracts/conduit/Conduit.sol:
  34:     mapping(address => bool) private _channels;

contracts/conduit/ConduitController.sol:
  21:     mapping(address => ConduitProperties) internal _conduits;

contracts/interfaces/ConduitControllerInterface.sol:
  20:         mapping(address => uint256) channelIndexesPlusOne;

contracts/lib/CounterManager.sol:
  20:     mapping(address => uint256) private _counters;

contracts/lib/OrderValidator.sol:
  42:     mapping(bytes32 => OrderStatus) private _orderStatus;
  45:     mapping(address => uint256) internal _contractNonces;

Recommendation:

 /// @dev    address(user) -> address(ERC0 Contract Address) -> uint256(allowance amount from user)
    mapping(address => mapping(address => uint256)) public allowance;

[N-12]  Initializing state variables with their default values is confusing

Description: It's not best practice to initialize state variables with their default values, so it won't be confusing to replace the default value with no value

Context:

9 results - 3 files

contracts/conduit/lib/ConduitConstants.sol:
   8: uint256 constant ChannelClosed_error_ptr = 0x00;
  16: uint256 constant ChannelKey_channel_ptr = 0x00;

contracts/lib/ConsiderationConstants.sol:
   44: uint256 constant information_version_offset = 0;
  108: uint256 constant OrderParameters_offerer_offset = 0x00;
  457: uint256 constant IsValidOrder_sig_ptr = 0x0;
  998: uint256 constant ratifyOrder_offer_head_offset = 0x00;


contracts/lib/TokenTransferrerConstants.sol:
  58: uint256 constant ERC20_transferFrom_sig_ptr = 0x0;
  70: uint256 constant ERC1155_safeTransferFrom_sig_ptr = 0x0;
  92: uint256 constant ERC721_transferFrom_sig_ptr = 0x0;

[N-13] Add to Event in receive() function

The receiver() function is important in terms of tracking the amount of Ether sent to the contract (if address sender is added as a parameter, address-based tracking and analysis can be provided)

So it is recommended to add event-emit to the function

Context:

contracts/lib/Consideration.sol:
  65       */
  66:     receive() external payable {
  67:         // Ensure the reentrancy guard is currently set to accept native tokens.
  68:         _assertAcceptingNativeTokens();
  69:     }

Description: Events help non-contract tools to track changes Many utilities and analysis frameworks make use of event-emits and are monitored at program startups.

Recommendation: Add Event-Emit

[N-14 ] Floating pragma

Description: Contracts should be deployed with the same compiler version and flags that they have been tested with thoroughly. Locking the pragma helps to ensure that contracts do not accidentally get deployed using, for example, an outdated compiler version that might introduce bugs that affect the contract system negatively. https://swcregistry.io/docs/SWC-103

Floating Pragma List: pragma ^0.8.13 - ^0.8.17 (all contracts)

Recommendation: Lock the pragma version and also consider known bugs (https://github.com/ethereum/solidity/releases) for the compiler version that is chosen.

Total 6 issues

[G-01] Structs can be packed into fewer storage slots

One slot can be saved in the OrderComponents, BasicOrderParameters, OrderParameters and ZoneParameters structures in the ConsiderationStructs.sol contract.

4 results 1 file: The OrderComponents struct can be packed into one slot less slot as suggested below.

contracts\lib\ConsiderationStructs.sol:
  26: struct OrderComponents {
  27:     address offerer;
  28:     address zone;
  29:     OfferItem[] offer;
  30:     ConsiderationItem[] consideration;
  31:     OrderType orderType;
- 32:     uint256 startTime;
+ 32:     uint128 startTime;
- 33:     uint256 endTime;
+ 33:     uint128 endTime;
  34:     bytes32 zoneHash;
  35:     uint256 salt;
  36:     bytes32 conduitKey;
  37:     uint256 counter;
  38: }

https://github.com/ProjectOpenSea/seaport/blob/5de7302bc773d9821ba4759e47fc981680911ea0/contracts/lib/ConsiderationStructs.sol#L26-L38

The BasicOrderParameters struct can be packed into one slot less slot as suggested below.

contracts\lib\ConsiderationStructs.sol:
  106: struct BasicOrderParameters {
  107:     // calldata offset
  108:     address considerationToken; // 0x24
  109:     uint256 considerationIdentifier; // 0x44 
  110:     uint256 considerationAmount; // 0x64  
  111:     address payable offerer; // 0x84
  112:     address zone; // 0xa4
  113:     address offerToken; // 0xc4	
  114:     uint256 offerIdentifier; // 0xe4	
  115:     uint256 offerAmount; // 0x104	
  116:     BasicOrderType basicOrderType; // 0x124 	
- 117:     uint256 startTime; // 0x144 	
+ 117:     uint128 startTime; // 0x144
- 118:     uint256 endTime; // 0x164
+ 118:     uint128 endTime; // 0x144
  119:     bytes32 zoneHash; // 0x164 
  120:     uint256 salt; // 0x134	
  121:     bytes32 offererConduitKey; // 0x1a4
  122:     bytes32 fulfillerConduitKey; // 0x1c4 
  123:     uint256 totalOriginalAdditionalRecipients; // 0x1e4
  124:     AdditionalRecipient[] additionalRecipients; // 0x204 
  125:     bytes signature; // 0x224	
  126:     // Total length, excluding dynamic array data: 0x244 0x264 (548) 	
  127: }

https://github.com/ProjectOpenSea/seaport/blob/5de7302bc773d9821ba4759e47fc981680911ea0/contracts/lib/ConsiderationStructs.sol#L104-L125

The OrderParameters struct can be packed into one slot less slot as suggested below.

contracts\lib\ConsiderationStructs.sol:
  149: struct OrderParameters {
  150:     address offerer; // 0x00
  151:     address zone; // 0x20
  152:     OfferItem[] offer; // 0x40
  153:     ConsiderationItem[] consideration; // 0x60
  154:     OrderType orderType; // 0x80
- 155:     uint256 startTime; // 0xa0
+ 155:     uint128 startTime; // 0xa0
- 156:     uint256 endTime; // 0xc0
+ 156:     uint128 endTime; // 0xa0
  157:     bytes32 zoneHash; // 0xc0
  158:     uint256 salt; // 0xe0
  159:     bytes32 conduitKey; // 0x100
  160:     uint256 totalOriginalConsiderationItems; // 0x120
  161:     // offer.length                          // 0x140
  162: }

https://github.com/ProjectOpenSea/seaport/blob/5de7302bc773d9821ba4759e47fc981680911ea0/contracts/lib/ConsiderationStructs.sol#L143-L156

The ZoneParameters struct can be packed into one slot less slot as suggested below.

contracts\lib\ConsiderationStructs.sol:
  262: struct ZoneParameters {
  263:     bytes32 orderHash;
  264:     address fulfiller;
  265:     address offerer;
  266:     SpentItem[] offer;
  267:     ReceivedItem[] consideration;
  268:     bytes extraData;
  269:     bytes32[] orderHashes;
- 270:     uint256 startTime;
+ 270:     uint128 startTime;
- 271:     uint256 endTime;
+ 271:     uint128 endTime;
  272:     bytes32 zoneHash;
  273: }

https://github.com/ProjectOpenSea/seaport/blob/5de7302bc773d9821ba4759e47fc981680911ea0/contracts/lib/ConsiderationStructs.sol#L256-L267

A total of 4 slots have been won.

[G-02] Using delete instead of setting struct0 saves gas (~70 gas)

2 results - 1 file:

contracts\lib\OrderCombiner.sol:
  239:    advancedOrder.numerator = 0;

  258:    advancedOrder.numerator = 0;

https://github.com/ProjectOpenSea/seaport/blob/5de7302bc773d9821ba4759e47fc981680911ea0/contracts/lib/OrderCombiner.sol#L239 https://github.com/ProjectOpenSea/seaport/blob/5de7302bc773d9821ba4759e47fc981680911ea0/contracts/lib/OrderCombiner.sol#L258

Recommendation code:

contracts\lib\OrderCombiner.sol:
- 239:    advancedOrder.numerator = 0;
+ 239:    delete advancedOrder.numerator;

[G-03] Using delete instead of setting address(0) saves gas (~70 gas)

4 results - 1 file:

contracts\lib\FulfillmentApplier.sol:
   78:    considerationExecution.offerer = address(0);

   79:    considerationExecution.item.recipient = payable(0);

  212:    execution.offerer = address(0);

  213:    execution.item.recipient = payable(0);

https://github.com/ProjectOpenSea/seaport/blob/5de7302bc773d9821ba4759e47fc981680911ea0/contracts/lib/FulfillmentApplier.sol#L78-L79 https://github.com/ProjectOpenSea/seaport/blob/5de7302bc773d9821ba4759e47fc981680911ea0/contracts/lib/FulfillmentApplier.sol#L212-L213

Proof Of Concepts:

contract Example {
    struct Execution {
        address payable offerer;
    }

    Execution execution;

    function setValueStorage() public {
        execution = Execution(payable(0xCA35b7d915458EF540aDe6068dFe2F44E8fa733c));
    }

    //  use for empty value Slot: 23456 gas
    //	use for non empty value Slot: 21456 gas
    function reset() public {
        delete execution.offerer;
    }

    // use for empty value Slot: 23525 gas
    // use for non empty value Slot: 21525 gas 
    function setToZero() public  {
        execution.offerer = payable(0);
    }

    function get() public view returns (address) {
        return execution.offerer;
    }
}

[G-04] Setting the constructor to payable (13 gas)

You can cut out 10 opcodes in the creation-time EVM bytecode if you declare a constructor payable. Making the constructor payable eliminates the need for an initial check of msg.value == 0 and saves 13 gas on deployment with no security risks.

14 results - 14 files

contracts\Seaport.sol:
  93:    constructor(address conduitController) Consideration(conduitController) {}

https://github.com/ProjectOpenSea/seaport/blob/5de7302bc773d9821ba4759e47fc981680911ea0/contracts/Seaport.sol#L93

contracts\conduit\Conduit.sol:
  73:    constructor() {

https://github.com/ProjectOpenSea/seaport/blob/5de7302bc773d9821ba4759e47fc981680911ea0/contracts/conduit/Conduit.sol#L73

contracts\lib\Assertions.sol:
  35:    constructor(
  36         address conduitController
  37     ) GettersAndDerivers(conduitController) {}

https://github.com/ProjectOpenSea/seaport/blob/5de7302bc773d9821ba4759e47fc981680911ea0/contracts/lib/Assertions.sol#L35-L37

contracts\lib\BasicOrderFulfiller.sol:
  41:    constructor(address conduitController) OrderValidator(conduitController) {}

https://github.com/ProjectOpenSea/seaport/blob/5de7302bc773d9821ba4759e47fc981680911ea0/contracts/lib/BasicOrderFulfiller.sol#L41

contracts\lib\Consideration.sol:
  50:    constructor(address conduitController) OrderCombiner(conduitController) {}

https://github.com/ProjectOpenSea/seaport/blob/5de7302bc773d9821ba4759e47fc981680911ea0/contracts/lib/Consideration.sol#L50

contracts\lib\ConsiderationBase.sol:
  56:    constructor(address conduitController) {

https://github.com/ProjectOpenSea/seaport/blob/5de7302bc773d9821ba4759e47fc981680911ea0/contracts/lib/ConsiderationBase.sol#L56

contracts\lib\Executor.sol:
  35:    constructor(address conduitController) Verifiers(conduitController) {}

https://github.com/ProjectOpenSea/seaport/blob/5de7302bc773d9821ba4759e47fc981680911ea0/contracts/lib/Executor.sol#L35

contracts\lib\GettersAndDerivers.sol:
  25:    constructor(
  26         address conduitController
  27     ) ConsiderationBase(conduitController) {}

https://github.com/ProjectOpenSea/seaport/blob/5de7302bc773d9821ba4759e47fc981680911ea0/contracts/lib/GettersAndDerivers.sol#L25-L27

contracts\lib\OrderCombiner.sol:
  41:    constructor(address conduitController) OrderFulfiller(conduitController) {}

https://github.com/ProjectOpenSea/seaport/blob/5de7302bc773d9821ba4759e47fc981680911ea0/contracts/lib/OrderCombiner.sol#L41

contracts\lib\OrderFulfiller.sol:
  45:     constructor(
  46         address conduitController
  47     ) BasicOrderFulfiller(conduitController) {}

https://github.com/ProjectOpenSea/seaport/blob/5de7302bc773d9821ba4759e47fc981680911ea0/contracts/lib/OrderFulfiller.sol#L45-L47

contracts\lib\OrderValidator.sol:
  55:     constructor(address conduitController) Executor(conduitController) {}

https://github.com/ProjectOpenSea/seaport/blob/5de7302bc773d9821ba4759e47fc981680911ea0/contracts/lib/OrderValidator.sol#L55

contracts\lib\ReentrancyGuard.sol:
  23:     constructor() {

https://github.com/ProjectOpenSea/seaport/blob/5de7302bc773d9821ba4759e47fc981680911ea0/contracts/lib/ReentrancyGuard.sol#L23

contracts\lib\TypehashDirectory.sol:
  30:     constructor() {

https://github.com/ProjectOpenSea/seaport/blob/5de7302bc773d9821ba4759e47fc981680911ea0/contracts/lib/TypehashDirectory.sol#L30

contracts\lib\Verifiers.sol:
  26:     constructor(address conduitController) Assertions(conduitController) {}

https://github.com/ProjectOpenSea/seaport/blob/5de7302bc773d9821ba4759e47fc981680911ea0/contracts/lib/Verifiers.sol#L26

Recommendation: Set the constructor to payable

[G-05] Optimize names to save gas (22 gas)

Contracts most called functions could simply save gas by function ordering via Method ID. Calling a function at runtime will be cheaper if the function is positioned earlier in the order (has a relatively lower Method ID) because 22 gas are added to the cost of a function for every position that came before it. The caller can save on gas if you prioritize most called functions. 

Context:  All Contracts

Recommendation:  Find a lower method ID name for the most called functions for example Call() vs. Call1() is cheaper by 22 gas For example, the function IDs in the Executor.sol contract will be the most used; A lower method ID may be given.

Proof of Consept: https://medium.com/joyso/solidity-how-does-function-name-affect-gas-consumption-in-smart-contract-47d270d8ac92

Executor.sol function names can be named and sorted according to METHOD ID

Sighash   |   Function Signature
========================
40142041  =>  _transferNativeTokens(address,uint256)
47991382  =>  _trigger(bytes32,bytes)
91353327  =>  _transferERC1155(address,address,address,uint256,uint256,bytes32,bytes)
26bb4f83  =>  _transfer(ReceivedItem,address,bytes32,bytes)
4d0abe39  =>  _transferIndividual721Or1155Item(ItemType,address,address,address,uint256,uint256,bytes32)
1eaee91a  =>  _transferERC20(address,address,address,uint256,bytes32,bytes)
69c575d9  =>  _transferERC721(address,address,address,uint256,uint256,bytes32,bytes)
d2e316d3  =>  _triggerIfArmedAndNotAccumulatable(bytes,bytes32)
0ac8a5bc  =>  _triggerIfArmed(bytes)
0b476195  =>  _callConduitUsingOffsets(bytes32,uint256,uint256)
4c12903a  =>  _getAccumulatorConduitKey(bytes)
376ad5b8  =>  _insert(bytes32,bytes,ConduitItemType,address,address,address,uint256,uint256)

[G-06] Functions guaranteed to revert_ when callled by normal users can be marked payable (21 gas)

If a function modifier or require such as onlyOwner-admin is used, the function will revert if a normal user tries to pay the function. Marking the function as payable will lower the gas cost for legitimate callers because the compiler will not include checks for whether a payment was provided. The extra opcodes avoided are CALLVALUE(2), DUP1(3), ISZERO(3), PUSH2(3), JUMPI(10), PUSH1(3), DUP1(3), REVERT(0), JUMPDEST(1), POP(2) which costs an average of about 21 gas per call to the function, in addition to the extra deployment cost.

3 results - 1 file

contracts\conduit\Conduit.sol:
  92     function execute(
  93         ConduitTransfer[] calldata transfers
  94:     ) external override onlyOpenChannel returns (bytes4 magicValue) {

  127     function executeBatch1155(
  128         ConduitBatch1155Transfer[] calldata batchTransfers
  129     ) external override onlyOpenChannel returns (bytes4 magicValue) {

  154     function executeWithBatch1155(
  155         ConduitTransfer[] calldata standardTransfers,
  156         ConduitBatch1155Transfer[] calldata batchTransfers
  157:     ) external override onlyOpenChannel returns (bytes4 magicValue) {

https://github.com/ProjectOpenSea/seaport/blob/5de7302bc773d9821ba4759e47fc981680911ea0/contracts/conduit/Conduit.sol#L94 https://github.com/ProjectOpenSea/seaport/blob/5de7302bc773d9821ba4759e47fc981680911ea0/contracts/conduit/Conduit.sol#L129 https://github.com/ProjectOpenSea/seaport/blob/5de7302bc773d9821ba4759e47fc981680911ea0/contracts/conduit/Conduit.sol#L157

Recommendation: Functions guaranteed to revert when called by normal users can be marked payable  (for only onlyOpenChannelonly functions)