Connext Amarok contest - rfa's results

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);