Connext Amarok contest - 0xf15ers's results

1. Add address(0) check to protect from unintended consequences.

• https://github.com/code-423n4/2022-06-connext/blob/b4532655071566b33c41eac46e75be29b4a381ed/contracts/contracts/core/connext/facets/PortalFacet.sol#L57-L59

2. Unused local variable

• https://github.com/code-423n4/2022-06-connext/blob/b4532655071566b33c41eac46e75be29b4a381ed/contracts/contracts/core/connext/facets/PortalFacet.sol#L98
• Variable adopted of repayAavePortal() is not used anywhere.

(bool success, uint256 amountIn, address adopted) = AssetLogic.swapFromLocalAssetIfNeededForExactOut(
      _local,
      totalAmount,
      _maxIn
    );

3. Redundant calculation of sponsoredFee in reimburseRelayerFees()

• https://github.com/code-423n4/2022-06-connext/blob/b4532655071566b33c41eac46e75be29b4a381ed/contracts/contracts/core/connext/helpers/SponsorVault.sol#L246
• https://github.com/code-423n4/2022-06-connext/blob/b4532655071566b33c41eac46e75be29b4a381ed/contracts/contracts/core/connext/helpers/SponsorVault.sol#L253
• sponsoredFee in L246 will revert anyway when den is zero.

function reimburseRelayerFees(......) {
    if (address(gasTokenOracle) != address(0)) {
      (num, den) = gasTokenOracle.getRate(_originDomain);
      // note: this will revert when den == 0
      sponsoredFee = (_originRelayerFee * num) / den;
    } else {
      num = rates[_originDomain].num;
      den = rates[_originDomain].den;
    }

    if (den != 0) {
      sponsoredFee = (_originRelayerFee * num) / den;     
  }

4. Unused function parameters

   --> contracts/core/connext/facets/BridgeFacet.sol:391:5:
    |
391 |     uint32 _nonce,
    |     ^^^^^^^^^^^^^

   --> contracts/core/connext/facets/BridgeFacet.sol:987:5:
    |
987 |     address _router,
    |     ^^^^^^^^^^^^^^^

   --> contracts/core/connext/facets/PortalFacet.sol:128:5:
    |
128 |     address _router, // @qa unused function parameter
    |     ^^^^^^^^^^^^^^^

1. Cache array length in loops instead of computing length in every iteration

While looping through array, the array length can be cached to save gas instead of computing length in each array iteration.

for eg.

uint256 len = modules.length;
for(uint256 i = 0; i < len; i++) {
  ...
}

> grep -rn './core' -e 'for.*[.]length'
./core/connext/facets/StableSwapFacet.sol:415:    for (uint8 i = 0; i < _pooledTokens.length; i++) {
./core/connext/facets/RelayerFacet.sol:140:    for (uint256 i; i < _transferIds.length; ) {
./core/connext/facets/RelayerFacet.sol:164:    for (uint256 i; i < _transferIds.length; ) {
./core/connext/libraries/LibDiamond.sol:104:    for (uint256 facetIndex; facetIndex < _diamondCut.length; facetIndex++) {
./core/connext/libraries/LibDiamond.sol:129:    for (uint256 selectorIndex; selectorIndex < _functionSelectors.length; selectorIndex++) {
./core/connext/libraries/LibDiamond.sol:147:    for (uint256 selectorIndex; selectorIndex < _functionSelectors.length; selectorIndex++) {
./core/connext/libraries/LibDiamond.sol:162:    for (uint256 selectorIndex; selectorIndex < _functionSelectors.length; selectorIndex++) {
./core/connext/libraries/SwapUtils.sol:205:    for (uint256 i = 0; i < xp.length; i++) {
./core/connext/libraries/SwapUtils.sol:558:    for (uint256 i = 0; i < balances.length; i++) {
./core/connext/libraries/SwapUtils.sol:591:    for (uint256 i = 0; i < balances.length; i++) {
./core/connext/libraries/SwapUtils.sol:844:    for (uint256 i = 0; i < pooledTokens.length; i++) {
./core/connext/libraries/SwapUtils.sol:869:      for (uint256 i = 0; i < pooledTokens.length; i++) {
./core/connext/libraries/SwapUtils.sol:924:    for (uint256 i = 0; i < amounts.length; i++) {
./core/connext/libraries/SwapUtils.sol:1014:      for (uint256 i = 0; i < pooledTokens.length; i++) {
./core/connext/libraries/SwapUtils.sol:1019:      for (uint256 i = 0; i < pooledTokens.length; i++) {
./core/connext/libraries/SwapUtils.sol:1039:    for (uint256 i = 0; i < pooledTokens.length; i++) {
./core/connext/libraries/SwapUtils.sol:1055:    for (uint256 i = 0; i < pooledTokens.length; i++) {
./core/connext/helpers/ConnextPriceOracle.sol:176:    for (uint256 i = 0; i < tokenAddresses.length; i++) {
./core/connext/helpers/StableSwap.sol:81:    for (uint8 i = 0; i < _pooledTokens.length; i++) {

2. Prefix increment (++i) is cheaper than postfix increment (i++)

Using prefix increment is saves small amount of gas than postfix increment because it returns the updated value hence doesn't requires to store intermediate value. This can be more significant in loops where this operation is done multiple times.

for eg.

// before
for(uint256 i = 0; i < len; i++) {
  ...
}

// Replace with
for(uint256 i = 0; i < len; ++i) {
  ...
}

./core/connext/libraries/SwapUtils.sol:425:    for (uint256 i = 0; i < MAX_LOOP_LIMIT; i++) {
./core/connext/libraries/SwapUtils.sol:558:    for (uint256 i = 0; i < balances.length; i++) {
./core/connext/libraries/SwapUtils.sol:591:    for (uint256 i = 0; i < balances.length; i++) {
./core/connext/libraries/SwapUtils.sol:844:    for (uint256 i = 0; i < pooledTokens.length; i++) {
./core/connext/libraries/SwapUtils.sol:869:      for (uint256 i = 0; i < pooledTokens.length; i++) {
./core/connext/libraries/SwapUtils.sol:924:    for (uint256 i = 0; i < amounts.length; i++) {
./core/connext/libraries/SwapUtils.sol:1014:      for (uint256 i = 0; i < pooledTokens.length; i++) {
./core/connext/libraries/SwapUtils.sol:1019:      for (uint256 i = 0; i < pooledTokens.length; i++) {
./core/connext/libraries/SwapUtils.sol:1039:    for (uint256 i = 0; i < pooledTokens.length; i++) {
./core/connext/libraries/SwapUtils.sol:1055:    for (uint256 i = 0; i < pooledTokens.length; i++) {
./core/connext/helpers/ConnextPriceOracle.sol:176:    for (uint256 i = 0; i < tokenAddresses.length; i++) {
./core/connext/helpers/StableSwap.sol:81:    for (uint8 i = 0; i < _pooledTokens.length; i++) {
...

3.Cache compute of address(this).balance in ternary operator to save gas instead of evaluating it twice

• https://github.com/code-423n4/2022-06-connext/blob/b4532655071566b33c41eac46e75be29b4a381ed/contracts/contracts/core/connext/helpers/SponsorVault.sol#L258

sponsoredFee = sponsoredFee > address(this).balance ? address(this).balance : sponsoredFee;