QA Report

[code423n4]

FINDINGS

Open todos left in the code File: BridgeFacet.sol line 492

      // TODO: do we want to store a mapping of custodied token balances here?

File: BridgeFacet.sol line 579

      // TODO: do we need to keep this

File: BridgeFacet.sol line 594

    // FIXME: routers can repay any-amount out-of-band using the `repayAavePortal` method

File: BridgeFacet.sol line 1027

    // TODO: Should we call approve(0) and approve(totalRepayAmount) instead? or with a try catch to not affect gas on all cases?

Commented code should be deleted

File:DiamondLoupeFacet.sol line 20-23

  // struct Facet {
  //     address facetAddress;
  //     bytes4[] functionSelectors;
  // }

TYPO

File:ProposedOwnableFacet.sol line 163

    // Contract as sournce of truth

sournce instead of source

File:ProposedOwnableFacet.sol line 176

    // Contract as sournce of truth

sournce instead of source

File: ProposedOwnableUpgradeable.sol line 170, see also line 198, line 212

    // Contract as sournce of truth

File: RelayerFacet.sol line 35

   * @notice Emitted when a rlayer is added or removed from whitelists

rlayer instead of relayer

Get rid of safemath as the over/underflow checks are in built in solidity 0.8+

We can save on some gas by getting rid of safemath library and using unchecked blocks for arithmetic operations that can never overflow/underflow SafeMath is no longer needed starting with Solidity 0.8. The compiler now has built in overflow checking.

If we get rid of safemath we can optimize some functions to save on gas. Since the compiler will check for under/overflows by default, we can disable this checks for arithmetic operations that are guaranteed to never over/underflow. This would include the following

File:AmplificationUtils.sol line 15

  using SafeMath for uint256;

File:SwapUtils.sol line 20

[0xleastwood]

This submissions is really just typos and TODOs. Last suggestion is a gas optimisation.