Connext Amarok contest - TomJ's results

[L-01] Immutable addresses should 0-Check

I recommend adding check of 0-address for immutable addresses. Not doing so might lead to non-functional contract when it is updated to 0-address accidentally.

Issue found at

Executor.sol
47:  constructor(address _connext) {
48:    connext = _connext;
49:  }

[L-02] abi.encodePacked should not be used with dynamic types

This is because using abi.encodePacked with dynamic types will cause a hash collisions. link: https://docs.soliditylang.org/en/v0.8.13/abi-spec.html#non-standard-packed-mode I recommend using abi.encode instead.

./connext/libraries/ConnextMessage.sol:178:    return keccak256(abi.encodePacked(bytes(_name).length, _name, bytes(_symbol).length, _symbol, _decimals));

[L-03] safeApprove is Deprecated

safeApprove is deprecated. Please use safeIncreaseAllowance and safeDecreaseAllowance instead. link: https://github.com/OpenZeppelin/openzeppelin-contracts/blob/bfff03c0d2a59bcd8e2ead1da9aed9edf0080d05/contracts/token/ERC20/utils/SafeERC20.sol#L38-L45

./connext/libraries/AssetLogic.sol:347:        SafeERC20.safeApprove(IERC20(_assetIn), address(pool), _amountIn);

[N-01] Event is missing indexed fields

Each event should have 3 indexed fields if there are 3 or more fields.

Issue found at

./relayer-fee/libraries/RelayerFeeRouter.sol:50:  event Send(uint32 domain, address recipient, bytes32[] transferIds, bytes32 remote, bytes message);
./relayer-fee/RelayerFeeRouter.sol:50:  event Send(uint32 domain, address recipient, bytes32[] transferIds, bytes32 remote, bytes message);
./promise/PromiseRouter.sol:78-86:  event Send(uint32 domain, bytes32 remote, bytes32 transferId, address callbackAddress, bool success, bytes data, bytes message);
./promise/PromiseRouter.sol:99-107:  event Receive(uint64 indexed originAndNonce, uint32 indexed origin, bytes32 transferId, address callbackAddress, bool success, bytes data, bytes message);
./promise/PromiseRouter.sol:116:  event CallbackFeeAdded(bytes32 indexed transferId, uint256 addedFee, uint256 totalFee, address caller);
./promise/PromiseRouter.sol:123:  event CallbackExecuted(bytes32 indexed transferId, address relayer);
./connext/facets/BridgeFacet.sol:75-82:  event XCalled(bytes32 indexed transferId, XCallArgs xcallArgs, XCalledEventArgs args, uint256 nonce, bytes message, address caller);
./connext/facets/BridgeFacet.sol:93-100:  event Reconciled(bytes32 indexed transferId, uint32 indexed origin, address[] routers, address asset, uint256 amount, address caller);
./connext/facets/BridgeFacet.sol:114-121:  event Executed(bytes32 indexed transferId, address indexed to, ExecuteArgs args, address transactingAsset, uint256 transactingAmount, address caller);
./connext/facets/BridgeFacet.sol:129:  event TransferRelayerFeesUpdated(bytes32 indexed transferId, uint256 relayerFee, address caller);
./connext/facets/BridgeFacet.sol:139-144:  event ForcedReceiveLocal(bytes32 indexed transferId, bytes32 indexed canonicalId, uint32 canonicalDomain, uint256 amount);
./connext/facets/BridgeFacet.sol:153:  event AavePortalMintUnbacked(bytes32 indexed transferId, address indexed router, address asset, uint256 amount);
./connext/facets/BridgeFacet.sol:162:  event AavePortalRepayment(bytes32 indexed transferId, address asset, uint256 amount, uint256 fee);
./connext/facets/BridgeFacet.sol:171:  event AavePortalRepaymentDebt(bytes32 indexed transferId, address asset, uint256 amount, uint256 fee);
./connext/facets/BridgeFacet.sol:179:  event SponsorVaultUpdated(address oldSponsorVault, address newSponsorVault, address caller);
./connext/facets/BridgeFacet.sol:187:  event PromiseRouterUpdated(address oldRouter, address newRouter, address caller);
./connext/facets/BridgeFacet.sol:195:  event ExecutorUpdated(address oldExecutor, address newExecutor, address caller);
./connext/facets/PortalFacet.sol:30:  event AavePortalRouterRepayment(address indexed router, address asset, uint256 amount, uint256 fee);
./connext/facets/RoutersFacet.sol:65:  event RouterAdded(address indexed router, address caller);
./connext/facets/RoutersFacet.sol:72:  event RouterRemoved(address indexed router, address caller);
./connext/facets/RoutersFacet.sol:103:  event MaxRoutersPerTransferUpdated(uint256 maxRoutersPerTransfer, address caller);
./connext/facets/RoutersFacet.sol:110:  event LiquidityFeeNumeratorUpdated(uint256 liquidityFeeNumerator, address caller);
./connext/facets/RoutersFacet.sol:117:  event RouterApprovedForPortal(address router, address caller);
./connext/facets/RoutersFacet.sol:124:  event RouterUnapprovedForPortal(address router, address caller);
./connext/facets/RoutersFacet.sol:133-139:  event RouterLiquidityAdded(address indexed router, address local, bytes32 canonicalId, uint256 amount, address caller);
./connext/facets/RoutersFacet.sol:149:  event RouterLiquidityRemoved(address indexed router, address to, address local, uint256 amount, address caller);
./connext/facets/RelayerFacet.sol:25:  event RelayerFeeRouterUpdated(address oldRouter, address newRouter, address caller);
./connext/facets/RelayerFacet.sol:32:  event RelayerAdded(address relayer, address caller);
./connext/facets/RelayerFacet.sol:39:  event RelayerRemoved(address relayer, address caller);
./connext/facets/RelayerFacet.sol:48:  event InitiatedClaim(uint32 indexed domain, address indexed recipient, address caller, bytes32[] transferIds);
./connext/facets/RelayerFacet.sol:56:  event Claimed(address indexed recipient, uint256 total, bytes32[] transferIds);
./connext/facets/AssetFacet.sol:27:  event WrapperUpdated(address oldWrapper, address newWrapper, address caller);
./connext/facets/AssetFacet.sol:35:  event TokenRegistryUpdated(address oldTokenRegistry, address newTokenRegistry, address caller);
./connext/facets/AssetFacet.sol:44:  event StableSwapAdded(bytes32 canonicalId, uint32 domain, address swapPool, address caller);
./connext/facets/AssetFacet.sol:55:  event AssetAdded(bytes32 canonicalId, uint32 domain, address adoptedAsset, address supportedAsset, address caller);
./connext/facets/AssetFacet.sol:62:  event AssetRemoved(bytes32 canonicalId, address caller);
./connext/facets/ProposedOwnableFacet.sol:52:  event RouterOwnershipRenunciationProposed(uint256 timestamp);
./connext/facets/ProposedOwnableFacet.sol:54:  event RouterOwnershipRenounced(bool renounced);
./connext/facets/ProposedOwnableFacet.sol:56:  event AssetOwnershipRenunciationProposed(uint256 timestamp);
./connext/facets/ProposedOwnableFacet.sol:58:  event AssetOwnershipRenounced(bool renounced);
./connext/libraries/AmplificationUtils.sol:17:  event RampA(uint256 oldA, uint256 newA, uint256 initialTime, uint256 futureTime);
./connext/libraries/AmplificationUtils.sol:18:  event StopRampA(uint256 currentA, uint256 time);
./connext/libraries/LibDiamond.sol:69:  event DiamondCutProposed(IDiamondCut.FacetCut[] _diamondCut, address _init, bytes _calldata, uint256 deadline);
./connext/libraries/LibDiamond.sol:81:  event DiamondCutRescinded(IDiamondCut.FacetCut[] _diamondCut, address _init, bytes _calldata);
./connext/libraries/LibDiamond.sol:92:  event DiamondCut(IDiamondCut.FacetCut[] _diamondCut, address _init, bytes _calldata);
./connext/libraries/SwapUtils.sol:25:  event TokenSwap(address indexed buyer, uint256 tokensSold, uint256 tokensBought, uint128 soldId, uint128 boughtId);
./connext/libraries/SwapUtils.sol:26-32:  event AddLiquidity(address indexed provider, uint256[] tokenAmounts, uint256[] fees, uint256 invariant, uint256 lpTokenSupply);
./connext/libraries/SwapUtils.sol:33:  event RemoveLiquidity(address indexed provider, uint256[] tokenAmounts, uint256 lpTokenSupply);
./connext/libraries/SwapUtils.sol:34-40:  event RemoveLiquidityOne(address indexed provider, uint256 lpTokenAmount, uint256 lpTokenSupply, uint256 boughtId, uint256 tokensBought);
./connext/libraries/SwapUtils.sol:41-47:  event RemoveLiquidityImbalance(address indexed provider, uint256[] tokenAmounts, uint256[] fees, uint256 invariant, uint256 lpTokenSupply);
./connext/libraries/SwapUtils.sol:48:  event NewAdminFee(uint256 newAdminFee);
./connext/libraries/SwapUtils.sol:49:  event NewSwapFee(uint256 newSwapFee);
./connext/helpers/ProposedOwnableUpgradeable.sol:62:  event RouterOwnershipRenunciationProposed(uint256 timestamp);
./connext/helpers/ProposedOwnableUpgradeable.sol:64:  event RouterOwnershipRenounced(bool renounced);
./connext/helpers/ProposedOwnableUpgradeable.sol:66:  event AssetOwnershipRenunciationProposed(uint256 timestamp);
./connext/helpers/ProposedOwnableUpgradeable.sol:68:  event AssetOwnershipRenounced(bool renounced);
./connext/helpers/ConnextPriceOracle.sol:65:  event NewAdmin(address oldAdmin, address newAdmin);
./connext/helpers/ConnextPriceOracle.sol:66:  event PriceRecordUpdated(address token, address baseToken, address lpToken, bool _active);
./connext/helpers/ConnextPriceOracle.sol:67:  event DirectPriceUpdated(address token, uint256 oldPrice, uint256 newPrice);
./connext/helpers/ConnextPriceOracle.sol:68:  event AggregatorUpdated(address tokenAddress, address source);
./connext/helpers/ConnextPriceOracle.sol:69:  event V1PriceOracleUpdated(address oldAddress, address newAddress);
./connext/helpers/SponsorVault.sol:73:  event ConnextUpdated(address oldConnext, address newConnext, address caller);
./connext/helpers/SponsorVault.sol:78:  event RateUpdated(uint32 originDomain, Rate oldRate, Rate newRate, address caller);
./connext/helpers/SponsorVault.sol:83:  event RelayerFeeCapUpdated(uint256 oldRelayerFeeCap, uint256 newRelayerFeeCap, address caller);
./connext/helpers/SponsorVault.sol:88:  event GasTokenOracleUpdated(address oldOracle, address newOracle, address caller);
./connext/helpers/SponsorVault.sol:93:  event TokenExchangeUpdated(address token, address oldTokenExchange, address newTokenExchange, address caller);
./connext/helpers/SponsorVault.sol:98:  event ReimburseLiquidityFees(address token, uint256 amount, address receiver);
./connext/helpers/SponsorVault.sol:103:  event ReimburseRelayerFees(uint256 amount, address receiver);
./connext/helpers/SponsorVault.sol:108:  event Deposit(address token, uint256 amount, address caller);
./connext/helpers/SponsorVault.sol:113:  event Withdraw(address token, address receiver, uint256 amount, address caller);

[N-02] Unnecessary use of named returns

Several function adds return statement even thought named returns variable are used. Remove unnecessary named returns variable to improve code readability. Also keeping the use of named returns or return statement consistent through out the whole project if possible is recommended.

Issue found at StableSwap.sol (remove returns variable availableTokenAmount)

291:   function calculateRemoveLiquidityOneToken(uint256 tokenAmount, uint8 tokenIndex)
292:     external
293:     view
293:     override
294:     returns (uint256 availableTokenAmount)
295:   {
296:     return swapStorage.calculateWithdrawOneToken(tokenAmount, tokenIndex);
297:   }

[N-03] nonReentrant modifier should occur before all other modifiers

This is best practice to protect against reentrancy in other modifiers.

./connext/facets/BridgeFacet.sol:279:  function xcall(XCallArgs calldata _args) external payable whenNotPaused nonReentrant returns (bytes32) {
./connext/facets/BridgeFacet.sol:411:  function execute(ExecuteArgs calldata _args) external whenNotPaused nonReentrant returns (bytes32) {

[N-04] Should Resolve TODOs before Deployment

Questions/Issues in the code should be resolved before the deployment.

Issue found at

./connext/facets/BridgeFacet.sol:492:      // TODO: do we want to store a mapping of custodied token balances here?
./connext/facets/BridgeFacet.sol:579:      // TODO: do we need to keep this
./connext/facets/BridgeFacet.sol:1027:    // TODO: Should we call approve(0) and approve(totalRepayAmount) instead? or with a try catch to not affect gas on all cases?
./connext/libraries/LibConnextStorage.sol:303:  // BridgeFacet (cont.) TODO: can we move this
./connext/helpers/Executor.sol:7:// TODO: see note in below file re: npm

[G-01] Unnecessary Default Value Initialization

When variable is not initialized, it will have its default values. Example: 0 for uint, false for bool and address(0) for address

I suggest removing default value initialization for following variables.

./connext/facets/BridgeFacet.sol:68:  uint16 public constant AAVE_REFERRAL_CODE = 0;
./connext/facets/StableSwapFacet.sol:415:    for (uint8 i = 0; i < _pooledTokens.length; i++) {
./connext/facets/VersionFacet.sol:16:  uint8 internal immutable _version = 0;
./connext/libraries/SwapUtils.sol:205:    for (uint256 i = 0; i < xp.length; i++) {
./connext/libraries/SwapUtils.sol:254:    for (uint256 i = 0; i < numTokens; i++) {
./connext/libraries/SwapUtils.sol:268:    for (uint256 i = 0; i < MAX_LOOP_LIMIT; i++) {
./connext/libraries/SwapUtils.sol:289:    for (uint256 i = 0; i < numTokens; i++) {
./connext/libraries/SwapUtils.sol:300:    for (uint256 i = 0; i < MAX_LOOP_LIMIT; i++) {
./connext/libraries/SwapUtils.sol:302:      for (uint256 j = 0; j < numTokens; j++) {
./connext/libraries/SwapUtils.sol:344:    for (uint256 i = 0; i < numTokens; i++) {
./connext/libraries/SwapUtils.sol:405:    for (uint256 i = 0; i < numTokens; i++) {
./connext/libraries/SwapUtils.sol:425:    for (uint256 i = 0; i < MAX_LOOP_LIMIT; i++) {
./connext/libraries/SwapUtils.sol:558:    for (uint256 i = 0; i < balances.length; i++) {
./connext/libraries/SwapUtils.sol:591:    for (uint256 i = 0; i < balances.length; i++) {
./connext/libraries/SwapUtils.sol:844:    for (uint256 i = 0; i < pooledTokens.length; i++) {
./connext/libraries/SwapUtils.sol:869:      for (uint256 i = 0; i < pooledTokens.length; i++) {
./connext/libraries/SwapUtils.sol:924:    for (uint256 i = 0; i < amounts.length; i++) {
./connext/libraries/SwapUtils.sol:1014:      for (uint256 i = 0; i < pooledTokens.length; i++) {
./connext/libraries/SwapUtils.sol:1019:      for (uint256 i = 0; i < pooledTokens.length; i++) {
./connext/libraries/SwapUtils.sol:1039:    for (uint256 i = 0; i < pooledTokens.length; i++) {
./connext/libraries/SwapUtils.sol:1055:    for (uint256 i = 0; i < pooledTokens.length; i++) {
./connext/helpers/ConnextPriceOracle.sol:176:    for (uint256 i = 0; i < tokenAddresses.length; i++) {
./connext/helpers/StableSwap.sol:81:    for (uint8 i = 0; i < _pooledTokens.length; i++) {

For example these can change to:

• uint16 public constant AAVE_REFERRAL_CODE;
• for (uint8 i; i < _pooledTokens.length; i++) {

[G-02] Save Gas in For-Loops by Storing Array's Length as a Variable

3 gas per iteration can be saved by storing an array's length as a variable before the for-loop.

Issue found at:

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

For example, I suggest changing it to:

pooledTokensLength = _pooledTokens.length
for (uint i; i < pooledTokensLength; i++) {

[G-03] ++i costs less gas than i++

It is better to use ++i than i++ when possible since it costs less gas.

Issue found at:

./connext/facets/BridgeFacet.sol:613:          i++;
./connext/facets/BridgeFacet.sol:684:          i++;
./connext/facets/BridgeFacet.sol:799:            i++;
./connext/facets/StableSwapFacet.sol:415:    for (uint8 i = 0; i < _pooledTokens.length; i++) {
./connext/facets/DiamondLoupeFacet.sol:31:    for (uint256 i; i < numFacets; i++) {
./connext/facets/RelayerFacet.sol:144:        i++;
./connext/facets/RelayerFacet.sol:168:        i++;
./connext/libraries/LibDiamond.sol:104:    for (uint256 facetIndex; facetIndex < _diamondCut.length; facetIndex++) {
./connext/libraries/LibDiamond.sol:129:    for (uint256 selectorIndex; selectorIndex < _functionSelectors.length; selectorIndex++) {
./connext/libraries/LibDiamond.sol:134:      selectorPosition++;
./connext/libraries/LibDiamond.sol:147:    for (uint256 selectorIndex; selectorIndex < _functionSelectors.length; selectorIndex++) {
./connext/libraries/LibDiamond.sol:153:      selectorPosition++;
./connext/libraries/LibDiamond.sol:162:    for (uint256 selectorIndex; selectorIndex < _functionSelectors.length; selectorIndex++) {
./connext/libraries/SwapUtils.sol:205:    for (uint256 i = 0; i < xp.length; i++) {
./connext/libraries/SwapUtils.sol:254:    for (uint256 i = 0; i < numTokens; i++) {
./connext/libraries/SwapUtils.sol:268:    for (uint256 i = 0; i < MAX_LOOP_LIMIT; i++) {
./connext/libraries/SwapUtils.sol:289:    for (uint256 i = 0; i < numTokens; i++) {
./connext/libraries/SwapUtils.sol:300:    for (uint256 i = 0; i < MAX_LOOP_LIMIT; i++) {
./connext/libraries/SwapUtils.sol:302:      for (uint256 j = 0; j < numTokens; j++) {
./connext/libraries/SwapUtils.sol:344:    for (uint256 i = 0; i < numTokens; i++) {
./connext/libraries/SwapUtils.sol:405:    for (uint256 i = 0; i < numTokens; i++) {
./connext/libraries/SwapUtils.sol:425:    for (uint256 i = 0; i < MAX_LOOP_LIMIT; i++) {
./connext/libraries/SwapUtils.sol:558:    for (uint256 i = 0; i < balances.length; i++) {
./connext/libraries/SwapUtils.sol:591:    for (uint256 i = 0; i < balances.length; i++) {
./connext/libraries/SwapUtils.sol:844:    for (uint256 i = 0; i < pooledTokens.length; i++) {
./connext/libraries/SwapUtils.sol:869:      for (uint256 i = 0; i < pooledTokens.length; i++) {
./connext/libraries/SwapUtils.sol:924:    for (uint256 i = 0; i < amounts.length; i++) {
./connext/libraries/SwapUtils.sol:1014:      for (uint256 i = 0; i < pooledTokens.length; i++) {
./connext/libraries/SwapUtils.sol:1019:      for (uint256 i = 0; i < pooledTokens.length; i++) {
./connext/libraries/SwapUtils.sol:1039:    for (uint256 i = 0; i < pooledTokens.length; i++) {
./connext/libraries/SwapUtils.sol:1055:    for (uint256 i = 0; i < pooledTokens.length; i++) {
./connext/helpers/ConnextPriceOracle.sol:176:    for (uint256 i = 0; i < tokenAddresses.length; i++) {
./connext/helpers/StableSwap.sol:81:    for (uint8 i = 0; i < _pooledTokens.length; i++) {

[G-04] Defined Variables Used Only Once

Certain variables is defined even though they are used only once. Remove these unnecessary variables to save gas. For cases where it will reduce the readability, one can use comments to help describe what the code is doing.

Issue found at

1. DiamondLoupeFacet.sol

• Remove ds variable of facetFunctionSelectors function

47:     LibDiamond.DiamondStorage storage ds = LibDiamond.diamondStorage();
48:     facetFunctionSelectors_ = ds.facetFunctionSelectors[_facet].functionSelectors;

• Remove ds variable of facetAddress function

63:     LibDiamond.DiamondStorage storage ds = LibDiamond.diamondStorage();
64:     facetAddress_ = ds.selectorToFacetAndPosition[_functionSelector].facetAddress;

• Remove ds variable of supportsInterface function

69:     LibDiamond.DiamondStorage storage ds = LibDiamond.diamondStorage();
70:     return ds.supportedInterfaces[_interfaceId];

2. PortalFacet.sol

• Remove totalAmount and routerBalance variable of repayAavePortal function

 87:    uint256 totalAmount = _backingAmount + _feeAmount; // in adopted
100:      totalAmount,

88:    uint256 routerBalance = s.routerBalances[msg.sender][_local]; // in local   
91:    if (routerBalance < _maxIn) revert PortalFacet__repayAavePortal_insufficientFunds();

3. ConnextPriceOracle.sol

• Remove rawTokenAmount, tokenDecimalDelta, tokenAmount, rawBaseTokenAmount, baseTokenDecimalDelta, baseTokenAmount, baseTokenPrice and tokenPrice variable of getPriceFromDex function

102:      uint256 rawTokenAmount = IERC20Extended(priceInfo.token).balanceOf(priceInfo.lpToken);
103:      uint256 tokenDecimalDelta = 18 - uint256(IERC20Extended(priceInfo.token).decimals());
104:      uint256 tokenAmount = rawTokenAmount.mul(10**tokenDecimalDelta);
105:      uint256 rawBaseTokenAmount = IERC20Extended(priceInfo.baseToken).balanceOf(priceInfo.lpToken);
106:      uint256 baseTokenDecimalDelta = 18 - uint256(IERC20Extended(priceInfo.baseToken).decimals());
107:      uint256 baseTokenAmount = rawBaseTokenAmount.mul(10**baseTokenDecimalDelta);
108:      uint256 baseTokenPrice = getTokenPrice(priceInfo.baseToken);
109:      uint256 tokenPrice = baseTokenPrice.mul(baseTokenAmount).div(tokenAmount);
111:      return tokenPrice;

• Remove chainLinkPrice variable of getPriceFromOracle function

118:    uint256 chainLinkPrice = getPriceFromChainlink(_tokenAddress);
119:    return chainLinkPrice;

• Remove retVal and price variable of getPriceFromChainlink function

133:      uint256 retVal = uint256(answer);
134:      uint256 price = retVal.mul(10**(18 - uint256(aggregator.decimals())));
136:      return price;

• Remove baseTokenPrice variable of setDexPriceInfo function

149:    uint256 baseTokenPrice = getTokenPrice(_baseToken);
150:    require(baseTokenPrice > 0, "invalid base token");

4. StableSwap.sol

• Remove totalIndexFrom and totalIndexTo variable of calculateSwapOutFromAddress function

234:    uint8 tokenIndexFrom = getTokenIndex(assetIn);
235:    uint8 tokenIndexTo = getTokenIndex(assetOut);
236:    return swapStorage.calculateSwapInv(tokenIndexFrom, tokenIndexTo, amountOut);

• Remove totalIndexFrom and totalIndexTo variable of swapExact function

342:    uint8 tokenIndexFrom = getTokenIndex(assetIn);
343:    uint8 tokenIndexTo = getTokenIndex(assetOut);
344:    return swapStorage.swap(tokenIndexFrom, tokenIndexTo, amountIn, minAmountOut);

• Remove totalIndexFrom and totalIndexTo variable of swapExactOut function

360:    uint8 tokenIndexFrom = getTokenIndex(assetIn);
361:    uint8 tokenIndexTo = getTokenIndex(assetOut);
362:    return swapStorage.swapOut(tokenIndexFrom, tokenIndexTo, amountOut, maxAmountIn);

5. Executor.sol

• Remove _parsed variable of originSender function

79:    bytes29 _parsed = LibCrossDomainProperty.parseDomainAndSenderBytes(properties);
80:    return LibCrossDomainProperty.sender(_parsed);

• Remove _parsed variable of origin function

90:    bytes29 _parsed = LibCrossDomainProperty.parseDomainAndSenderBytes(properties);
91:    return LibCrossDomainProperty.domain(_parsed);

6. AssetLogic.sol

• Remove starting variable of transferAssetToContract function

109:    uint256 starting = IERC20(_assetId).balanceOf(address(this));
113:    return IERC20(_assetId).balanceOf(address(this)) - starting;

7. LibCrossDomainProperty.sol

• Remove _len variable of isValidPropertyLength function

60:    uint256 _len = _view.len();
61:    return _len == PROPERTY_LEN;

8. ConnextMessage.sol

• Remove _len variable of isValidMessageLength function

89:    uint256 _len = _view.len();
90:    return _len == TOKEN_ID_LEN + TRANSFER_LEN;

• Remove _actionLen and _type variable of action function

219:    uint256 _actionLen = _message.len() - TOKEN_ID_LEN;
220:    uint40 _type = uint40(msgType(_message));
221:    return _message.slice(TOKEN_ID_LEN, _actionLen, _type);

For mitigation, simply don't define variable that is used only once. Below is mitigation example of above 1.

facetFunctionSelectors_ = LibDiamond.diamondStorage().facetFunctionSelectors[_facet].functionSelectors;

[G-05] Redundant Use of Variable

Update admin / _owner valiable after emiting the event so we do not have to define oldAdmin / oldOwner variable to save gas.

Issue found in

1. ConnextPriceOracle.sol

168:  function setAdmin(address newAdmin) external onlyAdmin {
169:    address oldAdmin = admin;
170:    admin = newAdmin;
172:    emit NewAdmin(oldAdmin, newAdmin);
173:  }

Change it to

168:  function setAdmin(address newAdmin) external onlyAdmin {
169:    emit NewAdmin(admin, newAdmin);
170:    admin = newAdmin;
171:  }

2. ProposedOwnableUpgradeable.sol

313:  function _setOwner(address newOwner) private {
314:    address oldOwner = _owner;
315:    _owner = newOwner;
316:    _proposedOwnershipTimestamp = 0;
317:    emit OwnershipTransferred(oldOwner, newOwner);

Change it to

313:  function _setOwner(address newOwner) private {
314:    emit OwnershipTransferred(_owner, newOwner);
315:    _owner = newOwner;
316:    _proposedOwnershipTimestamp = 0;

[G-06] != 0 costs less gass then > 0

!= 0 costs less gas when optimizer is enabled and is used for unsigned integers in "require" statement. I suggest changing > 0 to != 0

Issue found at:

./connext/libraries/AmplificationUtils.sol:86:    require(futureA_ > 0 && futureA_ < MAX_A, "futureA_ must be > 0 and < MAX_A");
./connext/libraries/LibDiamond.sol:121:    require(_functionSelectors.length > 0, "LibDiamondCut: No selectors in facet to cut");
./connext/libraries/LibDiamond.sol:139:    require(_functionSelectors.length > 0, "LibDiamondCut: No selectors in facet to cut");
./connext/libraries/LibDiamond.sol:158:    require(_functionSelectors.length > 0, "LibDiamondCut: No selectors in facet to cut");
./connext/libraries/LibDiamond.sol:226:      require(_calldata.length > 0, "LibDiamondCut: _calldata is empty but _init is not address(0)");
./connext/libraries/LibDiamond.sol:247:    require(contractSize > 0, _errorMessage);
./connext/libraries/SwapUtils.sol:845:      require(v.totalSupply != 0 || amounts[i] > 0, "Must supply all tokens in pool");
./connext/helpers/ConnextPriceOracle.sol:150:    require(baseTokenPrice > 0, "invalid base token");

[G-07] Reduce the Long Revert Strings of Error Messages

By keeping the revert strings within 32 bytes will save you gas since each slot is 32 bytes.

Following are revert strings that are more than 32 bytes.

./connext/libraries/LibDiamond.sol:66:    require(msg.sender == diamondStorage().contractOwner, "LibDiamond: Must be contract owner");
./connext/libraries/LibDiamond.sol:121:    require(_functionSelectors.length > 0, "LibDiamondCut: No selectors in facet to cut");
./connext/libraries/LibDiamond.sol:123:    require(_facetAddress != address(0), "LibDiamondCut: Add facet can't be address(0)");
./connext/libraries/LibDiamond.sol:132:      require(oldFacetAddress == address(0), "LibDiamondCut: Can't add function that already exists");
./connext/libraries/LibDiamond.sol:139:    require(_functionSelectors.length > 0, "LibDiamondCut: No selectors in facet to cut");
./connext/libraries/LibDiamond.sol:141:    require(_facetAddress != address(0), "LibDiamondCut: Add facet can't be address(0)");
./connext/libraries/LibDiamond.sol:150:      require(oldFacetAddress != _facetAddress, "LibDiamondCut: Can't replace function with same function");
./connext/libraries/LibDiamond.sol:158:    require(_functionSelectors.length > 0, "LibDiamondCut: No selectors in facet to cut");
./connext/libraries/LibDiamond.sol:161:    require(_facetAddress == address(0), "LibDiamondCut: Remove facet address must be address(0)");
./connext/libraries/LibDiamond.sol:191:    require(_facetAddress != address(0), "LibDiamondCut: Can't remove function that doesn't exist");
./connext/libraries/LibDiamond.sol:193:    require(_facetAddress != address(this), "LibDiamondCut: Can't remove immutable function");
./connext/libraries/LibDiamond.sol:224:      require(_calldata.length == 0, "LibDiamondCut: _init is address(0) but_calldata is not empty");
./connext/libraries/LibDiamond.sol:226:      require(_calldata.length > 0, "LibDiamondCut: _calldata is empty but _init is not address(0)");
./connext/libraries/SwapUtils.sol:697:    require(dy <= self.balances[tokenIndexTo], "Cannot get more than pool balance");
./connext/libraries/SwapUtils.sol:784:    require(dy <= self.balances[tokenIndexTo], "Cannot get more than pool balance");

[G-08] Both named returns and return statement are used Removing unused named returns variable in below code can save gas and improve code readability.

Issue found at StableSwap.sol (remove returns variable availableTokenAmount)

291:   function calculateRemoveLiquidityOneToken(uint256 tokenAmount, uint8 tokenIndex)
292:     external
293:     view
293:     override
294:     returns (uint256 availableTokenAmount)
295:   {
296:     return swapStorage.calculateWithdrawOneToken(tokenAmount, tokenIndex);
297:   }

[G-09] Use require instead of && When there are multiple conditions in require statement, break down the require statement into multiple require statements instead of using && can save gas.

Issue found at

./connext/libraries/AmplificationUtils.sol:86:    require(futureA_ > 0 && futureA_ < MAX_A, "futureA_ must be > 0 and < MAX_A");
./connext/libraries/SwapUtils.sol:397:    require(tokenIndexFrom < numTokens && tokenIndexTo < numTokens, "Tokens must be in pool");
./connext/libraries/SwapUtils.sol:493:    require(tokenIndexFrom < xp.length && tokenIndexTo < xp.length, "Token index out of range");
./connext/libraries/SwapUtils.sol:524:    require(tokenIndexFrom < xp.length && tokenIndexTo < xp.length, "Token index out of range");
./connext/libraries/SwapUtils.sol:1007:    require(maxBurnAmount <= v.lpToken.balanceOf(msg.sender) && maxBurnAmount != 0, ">LP.balanceOf");
./connext/helpers/StableSwap.sol:85:          tokenIndexes[address(_pooledTokens[i])] == 0 && _pooledTokens[0] != _pooledTokens[i],

For example these can be changed to

require(futureA_ > 0);
require(futureA_ < MAX_A, "futureA_ must be > 0 and < MAX_A");

[G-10] Use Calldata instead of Memory for Read Only Function Parameters

It is cheaper gas to use calldata than memory if the function parameter is read only. Calldata is a non-modifiable, non-persistent area where function arguments are stored, and behaves mostly like memory.

I recommend changing following memory to calldata

BridgeFacet.sol
./connext/facets/BridgeFacet.sol:541:  function _reconcile(uint32 _origin, bytes memory _message) internal {
./connext/facets/BridgeFacet.sol:706:  function _getTransferId(XCallArgs calldata _args, ConnextMessage.TokenId memory _canonical)
./connext/facets/BridgeFacet.sol:917:  function _reconcileProcessMessage(bytes memory _message)

StableSwapFacet.sol (initializeSwap function)
./connext/facets/StableSwapFacet.sol:395:    IERC20[] memory _pooledTokens,
./connext/facets/StableSwapFacet.sol:396:    uint8[] memory decimals,
./connext/facets/StableSwapFacet.sol:397:    string memory lpTokenName,
./connext/facets/StableSwapFacet.sol:398:    string memory lpTokenSymbol,

LibDiamond.sol (proposeDiamondCut function)
./connext/libraries/LibDiamond.sol:72:    IDiamondCut.FacetCut[] memory _diamondCut,
./connext/libraries/LibDiamond.sol:74:    bytes memory _calldata
LibDiamond.sol (rescindDiamondCut function)
./connext/libraries/LibDiamond.sol:84:    IDiamondCut.FacetCut[] memory _diamondCut,
./connext/libraries/LibDiamond.sol:86:    bytes memory _calldata
LibDiamond.sol (diamondCut function)
./connext/libraries/LibDiamond.sol:96:    IDiamondCut.FacetCut[] memory _diamondCut,
./connext/libraries/LibDiamond.sol:98:    bytes memory _calldata
LibDiamond.sol 
./connext/libraries/LibDiamond.sol:120:  function addFunctions(address _facetAddress, bytes4[] memory _functionSelectors) internal {
./connext/libraries/LibDiamond.sol:138:  function replaceFunctions(address _facetAddress, bytes4[] memory _functionSelectors) internal {
./connext/libraries/LibDiamond.sol:157:  function removeFunctions(address _facetAddress, bytes4[] memory _functionSelectors) internal {
./connext/libraries/LibDiamond.sol:222:  function initializeDiamondCut(address _init, bytes memory _calldata) internal {
./connext/libraries/LibDiamond.sol:242:  function enforceHasContractCode(address _contract, string memory _errorMessage) internal view {

ConnextMessage.sol
./connext/libraries/ConnextMessage.sol:146:  function formatTokenId(TokenId memory _tokenId) internal pure returns (bytes29) {
ConnextMessage.sol (formatDetailsHash function)
./connext/libraries/ConnextMessage.sol:174:    string memory _name,
./connext/libraries/ConnextMessage.sol:175:    string memory _symbol,

AssetLogic.sol (swapToLocalAssetIfNeeded function)
./connext/libraries/AssetLogic.sol:160:    ConnextMessage.TokenId memory _canonical,

LibCrossDomainProperty.sol
./connext/libraries/LibCrossDomainProperty.sol:157:  function parseDomainAndSenderBytes(bytes memory _property) internal pure returns (bytes29) {

SwapUtils.sol (getYD function)
./connext/libraries/SwapUtils.sol:244:    uint256[] memory xp,
SwapUtils.sol 
./connext/libraries/SwapUtils.sol:286:  function getD(uint256[] memory xp, uint256 a) internal pure returns (uint256) {
./connext/libraries/SwapUtils.sol:336:  function _xp(uint256[] memory balances, uint256[] memory precisionMultipliers)
SwapUtils.sol (getY function)
./connext/libraries/SwapUtils.sol:393:    uint256[] memory xp
SwapUtils.sol (_calculateSwap function)
./connext/libraries/SwapUtils.sol:489:    uint256[] memory balances
SwapUtils.sol (_calculateSwapInv function)
./connext/libraries/SwapUtils.sol:520:    uint256[] memory balances
SwapUtils.sol ( _calculateRemoveLiquidity function)
./connext/libraries/SwapUtils.sol:550:    uint256[] memory balances,
SwapUtils.sol (addLiquidity function)
./connext/libraries/SwapUtils.sol:819:    uint256[] memory amounts,
SwapUtils.sol (removeLiquidityImbalance function)
./connext/libraries/SwapUtils.sol:988:    uint256[] memory amounts,

LPToken.sol
./connext/helpers/LPToken.sol:21:  function initialize(string memory name, string memory symbol) external initializer returns (bool) {

StableSwap.sol (initialize function)
./connext/helpers/StableSwap.sol:62:    IERC20[] memory _pooledTokens,
./connext/helpers/StableSwap.sol:63:    uint8[] memory decimals,
./connext/helpers/StableSwap.sol:64:    string memory lpTokenName,
./connext/helpers/StableSwap.sol:65:    string memory lpTokenSymbol,

Executor.sol
./connext/helpers/Executor.sol:113:  function execute(ExecutorArgs memory _args) external payable override onlyConnext returns (bool, bytes memory) {