Connext Amarok contest - ElKu's results

1. Usage of safeApprove is not recommended. OpenZeppelin have deprecated this function here

SafeERC20.safeApprove(IERC20(_assetIn), address(pool), _amountIn);

2. abi.encodepacked() should not be used with dynamic types when passing the result to a hash function such as keccak256(). Use abi.encode() instead which will pad items to 32 bytes, which will prevent hash. “Unless there is a compelling reason, abi.encode should be preferred”.

return keccak256(abi.encodePacked(bytes(_name).length, _name, bytes(_symbol).length, _symbol, _decimals));

1. Inside the for loop, the loop increment can be done within an unchecked block. As its unlikely we will reach the limit of uint256 for the loop index. Like below: for (uint256 i; i < i_max; ) { //some operation unchecked { i++; } } Also there is no need to initialize i to 0 as its initialized to 0 by default. The length of array used in for loops should be cached to save gas. For example: pooledTokens.length, amounts.length, balances.length etc in the following references:

a. for (uint256 i; i < numFacets; i++) b. for (uint8 i = 0; i < _pooledTokens.length; i++) (the i here cannot be more than 32(see line 408). So we can uncheck the i++ even though I is uint8) c. for (uint256 i = 0; i < tokenAddresses.length; i++) d. for (uint256 i = 0; i < calls.length; i++) e. for (uint8 i = 0; i < _pooledTokens.length; i++) f. for (uint8 i = 0; i < 10; i += 1) g. for (uint256 i = 0; i < xp.length; i++) h. for (uint256 i = 0; i < numTokens; i++) i. for (uint256 i = 0; i < MAX_LOOP_LIMIT; i++) j. for (uint256 i = 0; i < numTokens; i++) k. for (uint256 i = 0; i < MAX_LOOP_LIMIT; i++) l. for (uint256 j = 0; j < numTokens; j++) m. for (uint256 i = 0; i < numTokens; i++) n. for (uint256 i = 0; i < numTokens; i++) o. for (uint256 i = 0; i < MAX_LOOP_LIMIT; i++) p. for (uint256 i = 0; i < balances.length; i++) q. for (uint256 i = 0; i < balances.length; i++) r. for (uint256 i = 0; i < pooledTokens.length; i++) s. for (uint256 i = 0; i < pooledTokens.length; i++) t. for (uint256 i = 0; i < amounts.length; i++) u. for (uint256 i = 0; i < pooledTokens.length; i++) v. for (uint256 i = 0; i < pooledTokens.length; i++) w. for (uint256 i = 0; i < pooledTokens.length; i++) x. for (uint256 i = 0; i < pooledTokens.length; i++)

2. Since the current solidity version takes care of overflow problems, there is no need to use SafeMath library. And many of the operations can be put under unchecked block, because of their unique conditions. This saves gas. Situations below: a. https://github.com/code-423n4/2022-06-connext/blob/main/contracts/contracts/core/connext/libraries/AmplificationUtils.sol#L61 and https://github.com/code-423n4/2022-06-connext/blob/main/contracts/contracts/core/connext/libraries/AmplificationUtils.sol#L64

a1-a0 or a0-a1(line 64) will never overflow. Similarly t1-t0 will never overflow. So the equation can be simplified to: unchecked{ return a0 + (a1-a0) * (block.timestamp-t0) / (t1-t0); } b. https://github.com/code-423n4/2022-06-connext/blob/main/contracts/contracts/core/connext/libraries/AmplificationUtils.sol#L84-L85

3. Instead of require statement we can use custom error messages. Also require statements with an && can be split into two statements. For example: require(a > 0 && a < MAX_A, "a must be > 0 and < MAX_A");

can be rewritten as:

if(a == 0) a_must_be_gt0(); if(a >= MAX_A) a_must_notbe_gtMAX();

References below: a. require(block.timestamp >= self.initialATime.add(1 days), "Wait 1 day before starting ramp"); require(futureTime_ >= block.timestamp.add(MIN_RAMP_TIME), "Insufficient ramp time"); b. require(futureAPrecise.mul(MAX_A_CHANGE) >= initialAPrecise, "futureA_ is too small"); c. require(futureAPrecise <= initialAPrecise.mul(MAX_A_CHANGE), "futureA_ is too large"); d. require(self.futureATime > block.timestamp, "Ramp is already stopped"); e. require(isValidAction(_action), "!action"); f. require(msg.sender == diamondStorage().contractOwner, "LibDiamond: Must be contract owner"); g. require(_functionSelectors.length > 0, "LibDiamondCut: No selectors in facet to cut"); h. require(_facetAddress != address(0), "LibDiamondCut: Add facet can't be address(0)"); i. require(oldFacetAddress == address(0), "LibDiamondCut: Can't add function that already exists"); j. require(_functionSelectors.length > 0, "LibDiamondCut: No selectors in facet to cut"); k. require(_facetAddress != address(0), "LibDiamondCut: Add facet can't be address(0)"); l. require(oldFacetAddress != _facetAddress, "LibDiamondCut: Can't replace function with same function"); m. require(_functionSelectors.length > 0, "LibDiamondCut: No selectors in facet to cut"); n. require(_facetAddress == address(0), "LibDiamondCut: Remove facet address must be address(0)"); o. require(_facetAddress != address(0), "LibDiamondCut: Can't remove function that doesn't exist"); p. require(_facetAddress != address(this), "LibDiamondCut: Can't remove immutable function"); q. require(_calldata.length == 0, "LibDiamondCut: _init is address(0) but_calldata is not empty"); r. require(_calldata.length > 0, "LibDiamondCut: _calldata is empty but _init is not address(0)"); s. require(tokenIndex < xp.length, "Token index out of range"); t. require(tokenAmount <= xp[tokenIndex], "Withdraw exceeds available"); u. require(tokenIndex < numTokens, "Token not found"); v. require(numTokens == precisionMultipliers.length, "Balances must match multipliers"); w. require(tokenIndexFrom != tokenIndexTo, "Can't compare token to itself"); require(tokenIndexFrom < numTokens && tokenIndexTo < numTokens, "Tokens must be in pool"); x. require(tokenIndexFrom < xp.length && tokenIndexTo < xp.length, "Token index out of range"); y. require(tokenIndexFrom < xp.length && tokenIndexTo < xp.length, "Token index out of range"); z. require(amount <= totalSupply, "Cannot exceed total supply"); aa. require(index < self.pooledTokens.length, "Token index out of range"); bb. require(dx <= tokenFrom.balanceOf(msg.sender), "Cannot swap more than you own"); cc. require(dy >= minDy, "Swap didn't result in min tokens"); dd. require(dy <= self.balances[tokenIndexTo], "Cannot get more than pool balance"); ee. require(dx <= maxDx, "Swap needs more than max tokens"); ff. require(dx <= tokenFrom.balanceOf(msg.sender), "Cannot swap more than you own"); gg. require(dx == tokenFrom.balanceOf(address(this)).sub(beforeBalance), "not support fee token"); hh. require(dx <= tokenFrom.balanceOf(msg.sender), "Cannot swap more than you own"); ii. require(dy >= minDy, "Swap didn't result in min tokens"); jj. require(dy <= self.balances[tokenIndexTo], "Cannot get more than pool balance"); kk. require(dx <= maxDx, "Swap didn't result in min tokens"); ll. require(amounts.length == pooledTokens.length, "Amounts must match pooled tokens"); mm. require(v.totalSupply != 0 || amounts[i] > 0, "Must supply all tokens in pool"); nn. require(v.d1 > v.d0, "D should increase"); oo. require(toMint >= minToMint, "Couldn't mint min requested"); pp. require(amount <= lpToken.balanceOf(msg.sender), ">LP.balanceOf"); require(minAmounts.length == pooledTokens.length, "minAmounts must match poolTokens"); qq. require(amounts[i] >= minAmounts[i], "amounts[i] < minAmounts[i]"); rr. require(tokenAmount <= lpToken.balanceOf(msg.sender), ">LP.balanceOf"); require(tokenIndex < pooledTokens.length, "Token not found"); ss. require(dy >= minAmount, "dy < minAmount"); tt. require(amounts.length == pooledTokens.length, "Amounts should match pool tokens"); uu. require(maxBurnAmount <= v.lpToken.balanceOf(msg.sender) && maxBurnAmount != 0, ">LP.balanceOf"); vv. require(tokenAmount != 0, "Burnt amount cannot be zero"); ww. require(tokenAmount <= maxBurnAmount, "tokenAmount > maxBurnAmount"); xx. require(newAdminFee <= MAX_ADMIN_FEE, "Fee is too high"); yy. require(newSwapFee <= MAX_SWAP_FEE, "Fee is too high"); zz. require(_remote != bytes32(0), "!remote"); aaa. require(msg.sender == connext, "#OC:027"); bbb. require(msg.sender == admin, "caller is not the admin"); ccc. require(baseTokenPrice > 0, "invalid base token"); ddd. require(amount != 0, "LPToken: cannot mint 0"); eee. require(to != address(this), "LPToken: cannot send to itself"); fff. require(_tokenId.domain != 0, "!repr"); ggg. require(_token != address(0), "!token"); hhh. require(_pooledTokens.length > 1, "_pooledTokens.length <= 1"); require(_pooledTokens.length <= 32, "_pooledTokens.length > 32"); require(_pooledTokens.length == decimals.length, "_pooledTokens decimals mismatch"); iii. require(address(_pooledTokens[i]) != address(0), "The 0 address isn't an ERC-20"); require(decimals[i] <= SwapUtils.POOL_PRECISION_DECIMALS, "Token decimals exceeds max"); jjj. require(_a < AmplificationUtils.MAX_A, "_a exceeds maximum"); require(_fee < SwapUtils.MAX_SWAP_FEE, "_fee exceeds maximum"); require(_adminFee < SwapUtils.MAX_ADMIN_FEE, "_adminFee exceeds maximum"); kkk. require(lpToken.initialize(lpTokenName, lpTokenSymbol), "could not init lpToken clone"); lll. require(block.timestamp <= deadline, "Deadline not met"); mmm. require(index < swapStorage.pooledTokens.length, "Out of range"); nnn. require(address(getToken(index)) == tokenAddress, "Token does not exist"); ooo. require(index < swapStorage.pooledTokens.length, "Index out of range");