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

1 stars 0 forks source link

Gas Optimizations #162

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Unnecessary codehash compare

If any code exists in the calculated conduit address, the CREATE2 will throws immediately. Ref: https://github.com/ethereum/EIPs/issues/684

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

        // If derived conduit exists, as evidenced by comparing runtime code...
        if (conduit.codehash == _CONDUIT_RUNTIME_CODE_HASH) {
            // Revert with an error indicating that the conduit already exists.
            revert ConduitAlreadyExists(conduit);
        }

        // Deploy the conduit via CREATE2 using the conduit key as the salt.
        new Conduit{ salt: conduitKey }();

Optimize control flow in updateChannel to cut a MLOAD

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

        if (isOpen && !channelPreviouslyOpen) {
            // A
        } else if (!isOpen && channelPreviouslyOpen) {
            // B
        }

change to

        if (channelPreviouslyOpen) {
            if (!isOpen){
                // B
            }
        } else {
            if (isOpen){
                // A
            }
        }

Use _name() in _nameString()

e.g. https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/contracts/Seaport.sol#L52-L55

    function _nameString() internal pure override returns (string memory) {
        // Return the name of the contract.
        return "Seaport";
    }

to

    function _nameString() internal pure override returns (string memory) {
        // Return the name of the contract.
        return _name();
    }

also in https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/contracts/ConsiderationBase.sol#L111

Use uint256 instead of bool in _channels mapping

Use uint256 or enum instead of bool will save gas https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/contracts/conduit/Conduit.sol#L33-L34

    mapping(address => bool) private _channels;

to

    mapping(address => uint256) private _channels;

with the respective changes to represent isOpen as 1 and !isOpen as 0

Pack multiple bool into a bytes32

Instead of using a bool[], multiple bool can be packed into the same memory slot in a bytes32. This would limit the maximum number of order to 255, but shouldn't be a problem practically.

contracts/lib/Consideration.sol:225:        returns (bool[] memory availableOrders, Execution[] memory executions)
contracts/lib/Consideration.sol:311:        returns (bool[] memory availableOrders, Execution[] memory executions)
contracts/lib/OrderCombiner.sol:116:        returns (bool[] memory availableOrders, Execution[] memory executions)
contracts/lib/OrderCombiner.sol:452:        returns (bool[] memory availableOrders, Execution[] memory executions)
contracts/lib/OrderCombiner.sol:567:    ) internal returns (bool[] memory availableOrders) {
contracts/lib/OrderCombiner.sol:572:        availableOrders = new bool[](totalOrders);
contracts/interfaces/ConsiderationInterface.sol:172:        returns (bool[] memory availableOrders, Execution[] memory executions);
contracts/interfaces/ConsiderationInterface.sol:245:        returns (bool[] memory availableOrders, Execution[] memory executions);
contracts/interfaces/SeaportInterface.sol:171:        returns (bool[] memory availableOrders, Execution[] memory executions);
contracts/interfaces/SeaportInterface.sol:244:        returns (bool[] memory availableOrders, Execution[] memory executions);
HardlyDifficult commented 2 years ago

Unnecessary codehash compare

This may be a worthwhile optimization to make.

Optimize control flow in updateChannel to cut a MLOAD

Updating channels is not a common operation and it doesn't impact end-users. This may offer small savings but not for a critical path.

Use _name() in _nameString()

It's not clear this would save gas for real transactions.

Use uint256 instead of bool in _channels mapping

This may offer very small savings.

Pack multiple bool into a bytes32

This could provide some savings, although integrations are less straightforward so it may not be worth including.