Connext Amarok contest - obtarian's results

1) Description of AssetLogic._swapAsset lacks one input argument

AssetLogic._swapAsset() description has no mention of the _slippageTol argument:

  /**
   * @notice Swaps assetIn t assetOut using the stored stable swap or internal swap pool
   * @dev Will not swap if the asset passed in is the adopted asset
   * @param _canonicalId - The canonical token id
   * @param _assetIn - The address of the from asset
   * @param _assetOut - The address of the to asset
   * @param _amount - The amount of the local asset to swap
   * @return The amount of assetOut
   * @return The address of assetOut
   */
  function _swapAsset(
    bytes32 _canonicalId,
    address _assetIn,
    address _assetOut,
    uint256 _amount,
    uint256 _slippageTol
  ) internal returns (uint256, address) {

Recommended Mitigation Steps

As an example:

  /**
   * @param _amount      - The amount of the local asset to swap
   * @param _slippageTol - Slippage tolerance
   * @return The amount of assetOut
   */

2) Description of AssetLogic._swapAssetOut lacks one output argument

The success return value is omitted:

  /**
   * @notice Swaps assetIn t assetOut using the stored stable swap or internal swap pool
   * @dev Will not swap if the asset passed in is the adopted asset
   * @param _canonicalId - The canonical token id
   * @param _assetIn - The address of the from asset
   * @param _assetOut - The address of the to asset
   * @param _amountOut - The amount of the _assetOut to swap
   * @return The amount of assetIn
   * @return The address of assetOut
   */
  function _swapAssetOut(

Recommended Mitigation Steps

As an example:

* @return Success value
* @return The amount of assetIn

3) Open TODOs in BridgeFacet

Open TODO comments that require some decisions to be made and then code completion:

    // TODO: Should we call approve(0) and approve(totalRepayAmount) instead? or with a try catch to not affect gas on all cases?
    // Example: https://github.com/aave/aave-v3-periphery/blob/ca184e5278bcbc10d28c3dbbc604041d7cfac50b/contracts/adapters/paraswap/ParaSwapRepayAdapter.sol#L138-L140
    SafeERC20.safeIncreaseAllowance(IERC20(adopted), s.aavePool, totalRepayAmount);

Also:

      // TODO: do we need to keep this
      bytes32 details = action.detailsHash();
      IBridgeToken(token).setDetailsHash(details);

Recommended Mitigation Steps

Consider removing all TODO comments with the corresponding code amendments (i.e. fix, won't fix, etc) before release as they are relevant to the development process only.