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 #258

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

Gas Optimizations #258

code423n4 opened this issue Jun 19, 2022 · 1 comment
Labels
bug Something isn't working G (Gas Optimization) resolved Finding has been patched by sponsor (sponsor pls link to PR containing fix)

Comments

@code423n4
Copy link
Contributor

Gas

Title: Packing of variable in struct in LibConnextStorage.sol

Occurrences:
https://github.com/code-423n4/2022-06-connext/blob/main/contracts/contracts/core/connext/libraries/LibConnextStorage.sol#L38-L51
https://github.com/code-423n4/2022-06-connext/blob/main/contracts/contracts/core/connext/libraries/LibConnextStorage.sol#L109-L310

Bool has 1 byte size, and address has 20 byte size if we put bool type var next to address, we can save storage slot (1 slot has max size 32 bytes)

Change to:

struct CallParams {
  address to;
  bytes callData;
  uint32 originDomain;
  uint32 destinationDomain;
  address agent;
  bool forceSlow; // @audit-info: Move bool here near address
  bool receiveLocal;
  address recovery;
  address callback;
  uint256 callbackFee;
  uint256 relayerFee;
  uint256 slippageTol;
}

Title: Better way to do increment

https://github.com/code-423n4/2022-06-connext/blob/main/contracts/contracts/core/connext/facets/BridgeFacet.sol#L332

Using ++ var is more efficient than doing += 1 increment
Change to:

	++s.nonce;

It can save 17 gas execution fee

Title: Avoid SLOAD to emit XCalled

https://github.com/code-423n4/2022-06-connext/blob/main/contracts/contracts/core/connext/facets/BridgeFacet.sol#L371

We can increase s.nonce by 1 and dong MSTORE at the same line:

	uint _sNonce = s.nonce++ //@audit-info: caching s.nonce and do increment

	emit XCalled(transferId, _args, eventArgs, _sNonce, message, msg.sender); // MLOAD instead SLOAD

By doing this way we can save 162 gas execution

Title: Using unchecked and prefix increment for i

https://github.com/code-423n4/2022-06-connext/blob/main/contracts/contracts/core/connext/facets/DiamondLoupeFacet.sol#L31

Change to:

    for (uint256 i; i < numFacets;) {
      address facetAddress_ = ds.facetAddresses[i];
      facets_[i].facetAddress = facetAddress_;
      facets_[i].functionSelectors = ds.facetFunctionSelectors[facetAddress_].functionSelectors;
      unchecked{++i;}
    }

Title: Using storage for struct

https://github.com/code-423n4/2022-06-connext/blob/main/contracts/contracts/core/connext/facets/PortalFacet.sol#L134

Instead of caching s.adoptedToCanonical[adopted] into memory, using storage is more gas efficient. It's just called once in the function

    ConnextMessage.TokenId storage canonical = s.adoptedToCanonical[adopted];

It saves 2176 gas execution gas fee

Title: Emitting block.timestamp is better

https://github.com/code-423n4/2022-06-connext/blob/main/contracts/contracts/core/connext/facets/ProposedOwnableFacet.sol#L267

Instead of doin SLOAD to emit RouterOwnershipRenunciationProposed, we can use block.timestamp

    emit RouterOwnershipRenunciationProposed(block.timestamp);

Title: Using delete statement to set to default value

https://github.com/code-423n4/2022-06-connext/blob/main/contracts/contracts/core/connext/facets/ProposedOwnableFacet.sol#L272
https://github.com/code-423n4/2022-06-connext/blob/main/contracts/contracts/core/connext/facets/ProposedOwnableFacet.sol#L283
https://github.com/code-423n4/2022-06-connext/blob/main/contracts/contracts/core/connext/facets/ProposedOwnableFacet.sol#L288
https://github.com/code-423n4/2022-06-connext/blob/main/contracts/contracts/core/connext/facets/RelayerFacet.sol#L166
https://github.com/code-423n4/2022-06-connext/blob/main/contracts/contracts/core/connext/facets/RoutersFacet.sol#L323-L324
https://github.com/code-423n4/2022-06-connext/blob/main/contracts/contracts/core/connext/facets/RoutersFacet.sol#L379
https://github.com/code-423n4/2022-06-connext/blob/main/contracts/contracts/core/connext/facets/RoutersFacet.sol#L445-L447

Using delete statement can save 4 gas on setting value to default value

delete s._routerOwnershipTimestamp;

Title: Avoid SLOAD while emitting OwnershipProposed

https://github.com/code-423n4/2022-06-connext/blob/main/contracts/contracts/core/connext/facets/ProposedOwnableFacet.sol#L295

By using newlyProposed var we can save gas by avoiding SLOAD

    emit OwnershipProposed(newlyProposed);
@code423n4 code423n4 added bug Something isn't working G (Gas Optimization) labels Jun 19, 2022
code423n4 added a commit that referenced this issue Jun 19, 2022
@liu-zhipeng
Copy link
Collaborator

liu-zhipeng commented Jul 1, 2022

  1. Resolved
  2. Resolved
  3. Resolved
  4. Duplicated
  5. Resolved
  6. Resolved
  7. Resolved
  8. Duplicated

@liu-zhipeng liu-zhipeng added the resolved Finding has been patched by sponsor (sponsor pls link to PR containing fix) label Jul 1, 2022
@itsmetechjay itsmetechjay reopened this Jul 1, 2022
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) resolved Finding has been patched by sponsor (sponsor pls link to PR containing fix)
Projects
None yet
Development

No branches or pull requests

3 participants