Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Gas Optimizations #162

Open
code423n4 opened this issue Jun 3, 2022 · 1 comment
Open

Gas Optimizations #162

code423n4 opened this issue Jun 3, 2022 · 1 comment
Labels
bug Something isn't working G (Gas Optimization)

Comments

@code423n4
Copy link
Contributor

Unnecessary codehash compare

If any code exists in the calculated conduit address, the CREATE2 will throws immediately.
Ref: ethereum/EIPs#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);
@code423n4 code423n4 added bug Something isn't working G (Gas Optimization) labels Jun 3, 2022
code423n4 added a commit that referenced this issue Jun 3, 2022
@HardlyDifficult
Copy link
Collaborator

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working G (Gas Optimization)
Projects
None yet
Development

No branches or pull requests

2 participants