Back to Metatron's contests

Connext Amarok contest - Metatron's results

The interoperability protocol of L2 Ethereum.

General Information

Platform: Code4rena

Start Date: 08/06/2022

Pot Size: $115,000 USDC

Total HM: 26

Participants: 72

Period: 11 days

Judge: leastwood

Total Solo HM: 14

Id: 132

League: ETH

Connext

Findings Distribution

Researcher Performance

Rank: 64/72

Findings: 1

Award: $119.85

🌟 Selected for report: 0

🚀 Solo Findings: 0

External Links

Code4rena Contest PageGithub Contest RepositoryGithub Findings RepositoryConnext

Findings Information

🌟 Selected for report: IllIllI

Also found by: 0x1f8b, 0x29A, 0xKitsune, 0xNazgul, 0xf15ers, 0xkatana, 0xmint, BowTiedWardens, ElKu, Fitraldys, Funen, Kaiziron, Lambda, Metatron, MiloTruck, Randyyy, Ruhum, SmartSek, TomJ, Tomio, UnusualTurtle, Waze, _Adam, apostle0x01, asutorufos, c3phas, catchup, csanuragjain, defsec, fatherOfBlocks, hansfriese, hyh, ignacio, joestakey, k, kaden, nahnah, oyc_109, rfa, robee, sach1r0, simon135, slywaters

Awards

119.8548 USDC - $119.85

Labels

bug
duplicate
G (Gas Optimization)

External Links

Link to GitHub issue

Note: results in G1-5 are filtered to only the 34 (.sol) file mentioned in the scope (and ignoring the .t.sol counters)

[G-01] ++i costs less gas compared to i++ ++i costs about 5 gas less per iteration compared to i++ for unsigned integer. This statement is true even with the optimizer enabled. Summerized my results where i is used in a loop, is unsigned integer, and you safly can be changed to ++i without changing any behavior,

I've found 34 locations in 9 files:

contracts/contracts/core/connext/facets/BridgeFacet.sol:
  612          unchecked {
  613:           i++;
  614          }

  683          unchecked {
  684:           i++;
  685          }

  798            unchecked {
  799:             i++;
  800            }

contracts/contracts/core/connext/facets/DiamondLoupeFacet.sol:
  30      facets_ = new Facet[](numFacets);
  31:     for (uint256 i; i < numFacets; i++) {
  32        address facetAddress_ = ds.facetAddresses[i];

contracts/contracts/core/connext/facets/RelayerFacet.sol:
  143        unchecked {
  144:         i++;
  145        }

  167        unchecked {
  168:         i++;
  169        }

contracts/contracts/core/connext/facets/StableSwapFacet.sol:
  414  
  415:     for (uint8 i = 0; i < _pooledTokens.length; i++) {
  416        if (i > 0) {

contracts/contracts/core/connext/helpers/ConnextPriceOracle.sol:
  175    function setAggregators(address[] calldata tokenAddresses, address[] calldata sources) external onlyAdmin {
  176:     for (uint256 i = 0; i < tokenAddresses.length; i++) {
  177        aggregators[tokenAddresses[i]] = AggregatorV3Interface(sources[i]);

contracts/contracts/core/connext/helpers/StableSwap.sol:
  80  
  81:     for (uint8 i = 0; i < _pooledTokens.length; i++) {
  82        if (i > 0) {

contracts/contracts/core/connext/libraries/LibDiamond.sol:
  103      );
  104:     for (uint256 facetIndex; facetIndex < _diamondCut.length; facetIndex++) {
  105        IDiamondCut.FacetCutAction action = _diamondCut[facetIndex].action;

  128      }
  129:     for (uint256 selectorIndex; selectorIndex < _functionSelectors.length; selectorIndex++) {
  130        bytes4 selector = _functionSelectors[selectorIndex];

  133        addFunction(ds, selector, selectorPosition, _facetAddress);
  134:       selectorPosition++;
  135      }

  146      }
  147:     for (uint256 selectorIndex; selectorIndex < _functionSelectors.length; selectorIndex++) {
  148        bytes4 selector = _functionSelectors[selectorIndex];

  152        addFunction(ds, selector, selectorPosition, _facetAddress);
  153:       selectorPosition++;
  154      }

  161      require(_facetAddress == address(0), "LibDiamondCut: Remove facet address must be address(0)");
  162:     for (uint256 selectorIndex; selectorIndex < _functionSelectors.length; selectorIndex++) {
  163        bytes4 selector = _functionSelectors[selectorIndex];

contracts/contracts/core/connext/libraries/SwapUtils.sol:
   204      v.feePerToken = _feePerToken(self.swapFee, xp.length);
   205:     for (uint256 i = 0; i < xp.length; i++) {
   206        uint256 xpi = xp[i];

   253  
   254:     for (uint256 i = 0; i < numTokens; i++) {
   255        if (i != tokenIndex) {

   267      uint256 y = d;
   268:     for (uint256 i = 0; i < MAX_LOOP_LIMIT; i++) {
   269        yPrev = y;

   288      uint256 s;
   289:     for (uint256 i = 0; i < numTokens; i++) {
   290        s = s.add(xp[i]);

   299  
   300:     for (uint256 i = 0; i < MAX_LOOP_LIMIT; i++) {
   301        uint256 dP = d;
   302:       for (uint256 j = 0; j < numTokens; j++) {
   303          dP = dP.mul(d).div(xp[j].mul(numTokens));

   343      uint256[] memory xp = new uint256[](numTokens);
   344:     for (uint256 i = 0; i < numTokens; i++) {
   345        xp[i] = balances[i].mul(precisionMultipliers[i]);

   404      uint256 _x;
   405:     for (uint256 i = 0; i < numTokens; i++) {
   406        if (i == tokenIndexFrom) {

   424      // iterative approximation
   425:     for (uint256 i = 0; i < MAX_LOOP_LIMIT; i++) {
   426        yPrev = y;

   557  
   558:     for (uint256 i = 0; i < balances.length; i++) {
   559        amounts[i] = balances[i].mul(amount).div(totalSupply);

   590      uint256 d0 = getD(_xp(balances, multipliers), a);
   591:     for (uint256 i = 0; i < balances.length; i++) {
   592        if (deposit) {

   843  
   844:     for (uint256 i = 0; i < pooledTokens.length; i++) {
   845        require(v.totalSupply != 0 || amounts[i] > 0, "Must supply all tokens in pool");

   868        uint256 feePerToken = _feePerToken(self.swapFee, pooledTokens.length);
   869:       for (uint256 i = 0; i < pooledTokens.length; i++) {
   870          uint256 idealBalance = v.d1.mul(v.balances[i]).div(v.d0);

   923  
   924:     for (uint256 i = 0; i < amounts.length; i++) {
   925        require(amounts[i] >= minAmounts[i], "amounts[i] < minAmounts[i]");

  1013        v.d0 = getD(_xp(v.balances, v.multipliers), v.preciseA);
  1014:       for (uint256 i = 0; i < pooledTokens.length; i++) {
  1015          balances1[i] = v.balances[i].sub(amounts[i], "Cannot withdraw more than available");

  1018  
  1019:       for (uint256 i = 0; i < pooledTokens.length; i++) {
  1020          uint256 idealBalance = v.d1.mul(v.balances[i]).div(v.d0);

  1038  
  1039:     for (uint256 i = 0; i < pooledTokens.length; i++) {
  1040        pooledTokens[i].safeTransfer(msg.sender, amounts[i]);

  1054      IERC20[] memory pooledTokens = self.pooledTokens;
  1055:     for (uint256 i = 0; i < pooledTokens.length; i++) {
  1056        IERC20 token = pooledTokens[i];

contracts/contracts/core/relayer-fee/libraries/RelayerFeeMessage.sol:
  84        unchecked {
  85:         i++;
  86        }

[G-02] An array’s length should be cached to save gas in for-loops Reading array length at each iteration of the loop takes 6 gas (3 for mload and 3 to place memory_offset). Caching the array length in the stack saves around 3 gas per iteration. I've found 19 locations in 6 different files:

contracts/contracts/core/connext/facets/RelayerFacet.sol:
  139      // Ensure the relayer can claim all transfers specified.
  140:     for (uint256 i; i < _transferIds.length; ) {
  141        if (s.transferRelayer[_transferIds[i]] != msg.sender)

  163      uint256 total;
  164:     for (uint256 i; i < _transferIds.length; ) {
  165        total += s.relayerFees[_transferIds[i]];

contracts/contracts/core/connext/facets/StableSwapFacet.sol:
  414  
  415:     for (uint8 i = 0; i < _pooledTokens.length; i++) {
  416        if (i > 0) {

contracts/contracts/core/connext/helpers/ConnextPriceOracle.sol:
  175    function setAggregators(address[] calldata tokenAddresses, address[] calldata sources) external onlyAdmin {
  176:     for (uint256 i = 0; i < tokenAddresses.length; i++) {
  177        aggregators[tokenAddresses[i]] = AggregatorV3Interface(sources[i]);

contracts/contracts/core/connext/helpers/StableSwap.sol:
  80  
  81:     for (uint8 i = 0; i < _pooledTokens.length; i++) {
  82        if (i > 0) {

contracts/contracts/core/connext/libraries/LibDiamond.sol:
  103      );
  104:     for (uint256 facetIndex; facetIndex < _diamondCut.length; facetIndex++) {
  105        IDiamondCut.FacetCutAction action = _diamondCut[facetIndex].action;

  128      }
  129:     for (uint256 selectorIndex; selectorIndex < _functionSelectors.length; selectorIndex++) {
  130        bytes4 selector = _functionSelectors[selectorIndex];

  146      }
  147:     for (uint256 selectorIndex; selectorIndex < _functionSelectors.length; selectorIndex++) {
  148        bytes4 selector = _functionSelectors[selectorIndex];

  161      require(_facetAddress == address(0), "LibDiamondCut: Remove facet address must be address(0)");
  162:     for (uint256 selectorIndex; selectorIndex < _functionSelectors.length; selectorIndex++) {
  163        bytes4 selector = _functionSelectors[selectorIndex];

contracts/contracts/core/connext/libraries/SwapUtils.sol:
   204      v.feePerToken = _feePerToken(self.swapFee, xp.length);
   205:     for (uint256 i = 0; i < xp.length; i++) {
   206        uint256 xpi = xp[i];

   557  
   558:     for (uint256 i = 0; i < balances.length; i++) {
   559        amounts[i] = balances[i].mul(amount).div(totalSupply);

   590      uint256 d0 = getD(_xp(balances, multipliers), a);
   591:     for (uint256 i = 0; i < balances.length; i++) {
   592        if (deposit) {

   843  
   844:     for (uint256 i = 0; i < pooledTokens.length; i++) {
   845        require(v.totalSupply != 0 || amounts[i] > 0, "Must supply all tokens in pool");

   868        uint256 feePerToken = _feePerToken(self.swapFee, pooledTokens.length);
   869:       for (uint256 i = 0; i < pooledTokens.length; i++) {
   870          uint256 idealBalance = v.d1.mul(v.balances[i]).div(v.d0);

   923  
   924:     for (uint256 i = 0; i < amounts.length; i++) {
   925        require(amounts[i] >= minAmounts[i], "amounts[i] < minAmounts[i]");

  1013        v.d0 = getD(_xp(v.balances, v.multipliers), v.preciseA);
  1014:       for (uint256 i = 0; i < pooledTokens.length; i++) {
  1015          balances1[i] = v.balances[i].sub(amounts[i], "Cannot withdraw more than available");

  1018  
  1019:       for (uint256 i = 0; i < pooledTokens.length; i++) {
  1020          uint256 idealBalance = v.d1.mul(v.balances[i]).div(v.d0);

  1038  
  1039:     for (uint256 i = 0; i < pooledTokens.length; i++) {
  1040        pooledTokens[i].safeTransfer(msg.sender, amounts[i]);

  1054      IERC20[] memory pooledTokens = self.pooledTokens;
  1055:     for (uint256 i = 0; i < pooledTokens.length; i++) {
  1056        IERC20 token = pooledTokens[i];

[G-03] If a variable is not set/initialized, it is assumed to have the default value 0 for uint Explicitly initializing it with its default value is an anti-pattern and wastes gas. For example: uint8 i = 0; should be replaced with uint8 i;

I've found 22 locations in 5 different files:

contracts/contracts/core/connext/facets/StableSwapFacet.sol:
  414  
  415:     for (uint8 i = 0; i < _pooledTokens.length; i++) {
  416        if (i > 0) {

contracts/contracts/core/connext/helpers/ConnextPriceOracle.sol:
  175    function setAggregators(address[] calldata tokenAddresses, address[] calldata sources) external onlyAdmin {
  176:     for (uint256 i = 0; i < tokenAddresses.length; i++) {
  177        aggregators[tokenAddresses[i]] = AggregatorV3Interface(sources[i]);

contracts/contracts/core/connext/helpers/StableSwap.sol:
  80  
  81:     for (uint8 i = 0; i < _pooledTokens.length; i++) {
  82        if (i > 0) {

contracts/contracts/core/connext/libraries/SwapUtils.sol:
   204      v.feePerToken = _feePerToken(self.swapFee, xp.length);
   205:     for (uint256 i = 0; i < xp.length; i++) {
   206        uint256 xpi = xp[i];

   253  
   254:     for (uint256 i = 0; i < numTokens; i++) {
   255        if (i != tokenIndex) {

   267      uint256 y = d;
   268:     for (uint256 i = 0; i < MAX_LOOP_LIMIT; i++) {
   269        yPrev = y;

   288      uint256 s;
   289:     for (uint256 i = 0; i < numTokens; i++) {
   290        s = s.add(xp[i]);

   299  
   300:     for (uint256 i = 0; i < MAX_LOOP_LIMIT; i++) {
   301        uint256 dP = d;
   302:       for (uint256 j = 0; j < numTokens; j++) {
   303          dP = dP.mul(d).div(xp[j].mul(numTokens));

   343      uint256[] memory xp = new uint256[](numTokens);
   344:     for (uint256 i = 0; i < numTokens; i++) {
   345        xp[i] = balances[i].mul(precisionMultipliers[i]);

   404      uint256 _x;
   405:     for (uint256 i = 0; i < numTokens; i++) {
   406        if (i == tokenIndexFrom) {

   424      // iterative approximation
   425:     for (uint256 i = 0; i < MAX_LOOP_LIMIT; i++) {
   426        yPrev = y;

   557  
   558:     for (uint256 i = 0; i < balances.length; i++) {
   559        amounts[i] = balances[i].mul(amount).div(totalSupply);

   590      uint256 d0 = getD(_xp(balances, multipliers), a);
   591:     for (uint256 i = 0; i < balances.length; i++) {
   592        if (deposit) {

   843  
   844:     for (uint256 i = 0; i < pooledTokens.length; i++) {
   845        require(v.totalSupply != 0 || amounts[i] > 0, "Must supply all tokens in pool");

   868        uint256 feePerToken = _feePerToken(self.swapFee, pooledTokens.length);
   869:       for (uint256 i = 0; i < pooledTokens.length; i++) {
   870          uint256 idealBalance = v.d1.mul(v.balances[i]).div(v.d0);

   923  
   924:     for (uint256 i = 0; i < amounts.length; i++) {
   925        require(amounts[i] >= minAmounts[i], "amounts[i] < minAmounts[i]");

  1013        v.d0 = getD(_xp(v.balances, v.multipliers), v.preciseA);
  1014:       for (uint256 i = 0; i < pooledTokens.length; i++) {
  1015          balances1[i] = v.balances[i].sub(amounts[i], "Cannot withdraw more than available");

  1018  
  1019:       for (uint256 i = 0; i < pooledTokens.length; i++) {
  1020          uint256 idealBalance = v.d1.mul(v.balances[i]).div(v.d0);

  1038  
  1039:     for (uint256 i = 0; i < pooledTokens.length; i++) {
  1040        pooledTokens[i].safeTransfer(msg.sender, amounts[i]);

  1054      IERC20[] memory pooledTokens = self.pooledTokens;
  1055:     for (uint256 i = 0; i < pooledTokens.length; i++) {
  1056        IERC20 token = pooledTokens[i];

contracts/contracts/core/relayer-fee/libraries/RelayerFeeMessage.sol:
  80      bytes32[] memory ids = new bytes32[](length);
  81:     for (uint256 i = 0; i < length; ) {
  82        ids[i] = _view.index(TRANSFER_IDS_START + i * TRANSFER_ID_LEN, TRANSFER_ID_LEN);

[G-05] Inrements can be uncheck From Solidity 0.8.0^, there’s a default overflow check on unsigned integers. It’s possible to uncheck this in for-loops and save some gas at each iteration, but at the cost of some code readability, as this uncheck cannot be made inline. Source: https://github.com/ethereum/solidity/issues/10695

The code would go from:

for (uint256 i; i < numFacets; i++) {
 // ...  
}

to:

for (uint256 i; i < numFacets;) { 
 // ...  
 unchecked { ++i; }  
}

The risk of overflow is infeasible for a uint256 here.

I've found 24 locations in 4 different files:

contracts/contracts/core/connext/facets/DiamondLoupeFacet.sol:
  30      facets_ = new Facet[](numFacets);
  31:     for (uint256 i; i < numFacets; i++) {
  32        address facetAddress_ = ds.facetAddresses[i];

contracts/contracts/core/connext/helpers/ConnextPriceOracle.sol:
  175    function setAggregators(address[] calldata tokenAddresses, address[] calldata sources) external onlyAdmin {
  176:     for (uint256 i = 0; i < tokenAddresses.length; i++) {
  177        aggregators[tokenAddresses[i]] = AggregatorV3Interface(sources[i]);

contracts/contracts/core/connext/libraries/LibDiamond.sol:
  103      );
  104:     for (uint256 facetIndex; facetIndex < _diamondCut.length; facetIndex++) {
  105        IDiamondCut.FacetCutAction action = _diamondCut[facetIndex].action;

  128      }
  129:     for (uint256 selectorIndex; selectorIndex < _functionSelectors.length; selectorIndex++) {
  130        bytes4 selector = _functionSelectors[selectorIndex];

  146      }
  147:     for (uint256 selectorIndex; selectorIndex < _functionSelectors.length; selectorIndex++) {
  148        bytes4 selector = _functionSelectors[selectorIndex];

  161      require(_facetAddress == address(0), "LibDiamondCut: Remove facet address must be address(0)");
  162:     for (uint256 selectorIndex; selectorIndex < _functionSelectors.length; selectorIndex++) {
  163        bytes4 selector = _functionSelectors[selectorIndex];

contracts/contracts/core/connext/libraries/SwapUtils.sol:
   204      v.feePerToken = _feePerToken(self.swapFee, xp.length);
   205:     for (uint256 i = 0; i < xp.length; i++) {
   206        uint256 xpi = xp[i];

   253  
   254:     for (uint256 i = 0; i < numTokens; i++) {
   255        if (i != tokenIndex) {

   267      uint256 y = d;
   268:     for (uint256 i = 0; i < MAX_LOOP_LIMIT; i++) {
   269        yPrev = y;

   288      uint256 s;
   289:     for (uint256 i = 0; i < numTokens; i++) {
   290        s = s.add(xp[i]);

   299  
   300:     for (uint256 i = 0; i < MAX_LOOP_LIMIT; i++) {
   301        uint256 dP = d;
   302:       for (uint256 j = 0; j < numTokens; j++) {
   303          dP = dP.mul(d).div(xp[j].mul(numTokens));

   343      uint256[] memory xp = new uint256[](numTokens);
   344:     for (uint256 i = 0; i < numTokens; i++) {
   345        xp[i] = balances[i].mul(precisionMultipliers[i]);

   404      uint256 _x;
   405:     for (uint256 i = 0; i < numTokens; i++) {
   406        if (i == tokenIndexFrom) {

   424      // iterative approximation
   425:     for (uint256 i = 0; i < MAX_LOOP_LIMIT; i++) {
   426        yPrev = y;

   557  
   558:     for (uint256 i = 0; i < balances.length; i++) {
   559        amounts[i] = balances[i].mul(amount).div(totalSupply);

   590      uint256 d0 = getD(_xp(balances, multipliers), a);
   591:     for (uint256 i = 0; i < balances.length; i++) {
   592        if (deposit) {

   843  
   844:     for (uint256 i = 0; i < pooledTokens.length; i++) {
   845        require(v.totalSupply != 0 || amounts[i] > 0, "Must supply all tokens in pool");

   868        uint256 feePerToken = _feePerToken(self.swapFee, pooledTokens.length);
   869:       for (uint256 i = 0; i < pooledTokens.length; i++) {
   870          uint256 idealBalance = v.d1.mul(v.balances[i]).div(v.d0);

   923  
   924:     for (uint256 i = 0; i < amounts.length; i++) {
   925        require(amounts[i] >= minAmounts[i], "amounts[i] < minAmounts[i]");

  1013        v.d0 = getD(_xp(v.balances, v.multipliers), v.preciseA);
  1014:       for (uint256 i = 0; i < pooledTokens.length; i++) {
  1015          balances1[i] = v.balances[i].sub(amounts[i], "Cannot withdraw more than available");

  1018  
  1019:       for (uint256 i = 0; i < pooledTokens.length; i++) {
  1020          uint256 idealBalance = v.d1.mul(v.balances[i]).div(v.d0);

  1038  
  1039:     for (uint256 i = 0; i < pooledTokens.length; i++) {
  1040        pooledTokens[i].safeTransfer(msg.sender, amounts[i]);

  1054      IERC20[] memory pooledTokens = self.pooledTokens;
  1055:     for (uint256 i = 0; i < pooledTokens.length; i++) {
  1056        IERC20 token = pooledTokens[i];

[G-06] Upgrade pragma to 0.8.15 to save gas Across the whole solution, the declared pragma is 0.8.14 Acording to the release note of 0.8.15: https://blog.soliditylang.org/2022/06/15/solidity-0.8.15-release-announcement/ The benchmark shows saving of 2.5-10% Bytecode size, Saving 2-8% Deployment gas, And saving up to 6.2% Runtime gas.

[G-07] From solidity 0.8.4 and above, Custom errors from are cheaper than revert strings (cheaper deployment cost and runtime cost when the revert condition is met) Source: https://blog.soliditylang.org/2021/04/21/custom-errors/: Starting from Solidity v0.8.4, there is a convenient and gas-efficient way to explain to users why an operation failed through the use of custom errors. Until now, you could already use strings to give more information about failures (e.g., revert("Insufficient funds.");), but they are rather expensive, especially when it comes to deploy cost, and it is difficult to use dynamic information in them.

AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax © 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter