Bug Fixes for Router, remove axelar state variables from Router (#244)

* Removed axelar state variables from Router, fixed init, scripts, tasks

* Started modernizing router tests

* remove unecessary state calls

* Tests

* Removed unused imports

* Fix config error in Router tests (#254)

* Fixed issue where Router would pass other chain token address

* Test fixed for refund fallback flows

* Finished modernizing all tests, fixes to Router as a result of better test findings

* Lint

* Fix eggregious merge typo

---------

Co-authored-by: Nenad Misic <nenad@angelprotocol.io>

contracts/axelar/AxelarExecutable.sol

@@ -6,29 +6,16 @@ pragma solidity >=0.8.8;
 import {IVault} from "../core/vault/interfaces/IVault.sol";
 import {IAxelarGateway} from "@axelar-network/axelar-gmp-sdk-solidity/contracts/interfaces/IAxelarGateway.sol";
 import {IAxelarExecutable} from "@axelar-network/axelar-gmp-sdk-solidity/contracts/interfaces/IAxelarExecutable.sol";
-import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
-
-contract AxelarExecutable is IAxelarExecutable, Initializable {
-  IAxelarGateway public gateway;
-
-  // We want this to be intializeable by an OZ upgradable pattern so move the constructor logic to _init_ methods
-  function __AxelarExecutable_init(address gateway_) internal onlyInitializing {
-    __AxelarExecutable_init_unchained(gateway_);
-  }
-
-  function __AxelarExecutable_init_unchained(address gateway_) internal onlyInitializing {
-    if (gateway_ == address(0)) revert InvalidAddress();
-    gateway = IAxelarGateway(gateway_);
-  }
 
+abstract contract AxelarExecutable is IAxelarExecutable {
   function execute(
     bytes32 commandId,
     string calldata sourceChain,
     string calldata sourceAddress,
     bytes calldata payload
   ) public virtual override {
     bytes32 payloadHash = keccak256(payload);
-    if (!gateway.validateContractCall(commandId, sourceChain, sourceAddress, payloadHash))
+    if (!_gateway().validateContractCall(commandId, sourceChain, sourceAddress, payloadHash))
       revert NotApprovedByGateway();
     _execute(sourceChain, sourceAddress, payload);
   }
@@ -43,7 +30,7 @@ contract AxelarExecutable is IAxelarExecutable, Initializable {
   ) public virtual override {
     bytes32 payloadHash = keccak256(payload);
     if (
-      !gateway.validateContractCallAndMint(
+      !_gateway().validateContractCallAndMint(
         commandId,
         sourceChain,
         sourceAddress,
@@ -69,4 +56,10 @@ contract AxelarExecutable is IAxelarExecutable, Initializable {
     string calldata tokenSymbol,
     uint256 amount
   ) internal virtual returns (IVault.VaultActionData memory) {}
+
+  function gateway() external view returns (IAxelarGateway) {
+    return _gateway();
+  }
+
+  function _gateway() internal view virtual returns (IAxelarGateway);
 }

contracts/core/accounts/facets/AccountsStrategy.sol

@@ -16,7 +16,7 @@ import {ReentrancyGuardFacet} from "./ReentrancyGuardFacet.sol";
 import {IAccountsEvents} from "../interfaces/IAccountsEvents.sol";
 import {IVault} from "../../vault/interfaces/IVault.sol";
 import {IAccountsStrategy} from "../interfaces/IAccountsStrategy.sol";
-import {AxelarExecutableAccounts} from "../lib//AxelarExecutableAccounts.sol";
+import {AxelarExecutableAccounts} from "../lib/AxelarExecutableAccounts.sol";
 import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
 import {SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
 import {IGasFwd} from "../../gasFwd/IGasFwd.sol";

contracts/core/router/Router.sol

@@ -3,6 +3,7 @@
 pragma solidity >=0.8.8;
 
 import {LibAccounts} from "../accounts/lib/LibAccounts.sol";
+import {IAccountsStrategy} from "../accounts/interfaces/IAccountsStrategy.sol";
 import {IRouter} from "./IRouter.sol";
 import {RouterLib} from "./RouterLib.sol";
 import {IVault} from "../vault/interfaces/IVault.sol";
@@ -15,29 +16,21 @@ import {AxelarExecutable} from "../../axelar/AxelarExecutable.sol";
 import {IAxelarGateway} from "@axelar-network/axelar-gmp-sdk-solidity/contracts/interfaces/IAxelarGateway.sol";
 import {IAxelarGasService} from "@axelar-network/axelar-gmp-sdk-solidity/contracts/interfaces/IAxelarGasService.sol";
 import {IAxelarExecutable} from "@axelar-network/axelar-gmp-sdk-solidity/contracts/interfaces/IAxelarExecutable.sol";
+import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
 
-contract Router is IRouter, AxelarExecutable {
+contract Router is IRouter, Initializable, AxelarExecutable {
   using SafeERC20 for IERC20Metadata;
   string public chain;
   ILocalRegistrar public registrar;
-  IAxelarGasService public gasReceiver;
-
   uint256 constant PRECISION = 10 ** 6;
 
   /*///////////////////////////////////////////////
                         PROXY INIT
     */ ///////////////////////////////////////////////
 
-  function initialize(
-    string calldata _chain,
-    address _gateway,
-    address _gasReceiver,
-    address _registrar
-  ) public initializer {
+  function initialize(string calldata _chain, address _registrar) public initializer {
     chain = _chain;
     registrar = ILocalRegistrar(_registrar);
-    gasReceiver = IAxelarGasService(_gasReceiver);
-    __AxelarExecutable_init_unchained(_gateway);
   }
 
   /*///////////////////////////////////////////////
@@ -68,7 +61,7 @@ contract Router is IRouter, AxelarExecutable {
     // check that at least one vault is expected to receive a deposit
     require(action.lockAmt > 0 || action.liqAmt > 0, "No vault deposit specified");
     // check that token is accepted by angel protocol
-    address tokenAddress = gateway.tokenAddresses(tokenSymbol);
+    address tokenAddress = _gateway().tokenAddresses(tokenSymbol);
     require(registrar.isTokenAccepted(tokenAddress), "Token not accepted");
     // Get parameters from registrar if approved
     require(
@@ -156,7 +149,7 @@ contract Router is IRouter, AxelarExecutable {
       _action.accountIds[0],
       _action.lockAmt
     );
-    IERC20Metadata(_action.token).safeTransferFrom(
+    IERC20Metadata(_redemptionLock.token).safeTransferFrom(
       _params.Locked.vaultAddr,
       address(this),
       _redemptionLock.amount
@@ -166,19 +159,20 @@ contract Router is IRouter, AxelarExecutable {
       _action.accountIds[0],
       _action.liqAmt
     );
-
-    IERC20Metadata(_action.token).safeTransferFrom(
+    IERC20Metadata(_redemptionLiquid.token).safeTransferFrom(
       _params.Liquid.vaultAddr,
       address(this),
       _redemptionLiquid.amount
     );
 
+    // Update _action with this chain's token address.
+    // Liquid token should ALWAYS == Locked token
+    _action.token = _redemptionLiquid.token;
+
     // Pack and send the tokens back to Accounts contract
     uint256 _redeemedAmt = _redemptionLock.amount + _redemptionLiquid.amount;
     _action.lockAmt = _redemptionLock.amount;
     _action.liqAmt = _redemptionLiquid.amount;
-    _action = _prepareToSendTokens(_action, _redeemedAmt);
-    emit Redeem(_action, _redeemedAmt);
     if (
       (_redemptionLock.status == IVault.VaultActionStatus.POSITION_EXITED) &&
       (_redemptionLiquid.status == IVault.VaultActionStatus.POSITION_EXITED)
@@ -187,6 +181,8 @@ contract Router is IRouter, AxelarExecutable {
     } else {
       _action.status = IVault.VaultActionStatus.SUCCESS;
     }
+    _action = _prepareToSendTokens(_action, _redeemedAmt);
+    emit Redeem(_action, _redeemedAmt);
     return _action;
   }
 
@@ -221,9 +217,9 @@ contract Router is IRouter, AxelarExecutable {
 
     // Pack and send the tokens back
     uint256 _redeemedAmt = lockResponse.amount + liqResponse.amount;
+    _action.status = IVault.VaultActionStatus.POSITION_EXITED;
     _action = _prepareToSendTokens(_action, _redeemedAmt);
     emit Redeem(_action, _redeemedAmt);
-    _action.status = IVault.VaultActionStatus.POSITION_EXITED;
     return _action;
   }
 
@@ -241,7 +237,7 @@ contract Router is IRouter, AxelarExecutable {
   }
 
   /*////////////////////////////////////////////////
-                        AXELAR IMPL.
+                      AXELAR IMPL.
   */ ////////////////////////////////////////////////
 
   function executeLocal(
@@ -364,15 +360,15 @@ contract Router is IRouter, AxelarExecutable {
     address gasToken,
     uint256 gasFeeAmt
   ) public onlySelf {
-    address tokenAddress = gateway.tokenAddresses(symbol);
-    IERC20Metadata(tokenAddress).safeApprove(address(gateway), amount);
-    IERC20Metadata(gasToken).safeApprove(address(gasReceiver), gasFeeAmt);
+    address tokenAddress = _gateway().tokenAddresses(symbol);
+    IERC20Metadata(tokenAddress).safeApprove(address(_gateway()), amount);
+    IERC20Metadata(gasToken).safeApprove(address(_gasReceiver()), gasFeeAmt);
 
     LibAccounts.FeeSetting memory feeSetting = registrar.getFeeSettingsByFeeType(
       LibAccounts.FeeTypes.Default
     );
 
-    gasReceiver.payGasForContractCallWithToken(
+    _gasReceiver().payGasForContractCallWithToken(
       address(this),
       destinationChain,
       destinationAddress,
@@ -384,7 +380,7 @@ contract Router is IRouter, AxelarExecutable {
       feeSetting.payoutAddress
     );
 
-    gateway.callContractWithToken(destinationChain, destinationAddress, payload, symbol, amount);
+    _gateway().callContractWithToken(destinationChain, destinationAddress, payload, symbol, amount);
   }
 
   function _executeWithToken(
@@ -407,8 +403,14 @@ contract Router is IRouter, AxelarExecutable {
     IVault.VaultActionData memory action = RouterLib.unpackCalldata(payload);
 
     // grab tokens sent cross-chain
-    address tokenAddress = gateway.tokenAddresses(tokenSymbol);
-    IERC20Metadata(tokenAddress).safeTransferFrom(address(gateway), address(this), amount);
+    address tokenAddress = _gateway().tokenAddresses(tokenSymbol);
+    IERC20Metadata(tokenAddress).safeTransferFrom(address(_gateway()), address(this), amount);
+
+    // update action.token address to reflect this chain's token address
+    action.token = tokenAddress;
+
+    // update action.destinationChain to source chain in case of failure
+    action.destinationChain = sourceChain;
 
     // Leverage this.call() to enable try/catch logic
     try this.deposit(action, tokenSymbol, amount) {
@@ -443,7 +445,20 @@ contract Router is IRouter, AxelarExecutable {
       action.strategyId
     );
 
+    // update action.destinationChain to source chain for token redemptions
+    action.destinationChain = sourceChain;
+
     // Switch for calling appropriate vault/method
     return _callSwitch(params, action);
   }
+
+  function _gateway() internal view override returns (IAxelarGateway) {
+    IAccountsStrategy.NetworkInfo memory network = registrar.queryNetworkConnection(chain);
+    return IAxelarGateway(network.axelarGateway);
+  }
+
+  function _gasReceiver() internal view returns (IAxelarGasService) {
+    IAccountsStrategy.NetworkInfo memory network = registrar.queryNetworkConnection(chain);
+    return IAxelarGasService(network.gasReceiver);
+  }
 }

contracts/core/router/scripts/deploy.ts

@@ -11,8 +11,6 @@ import {
 } from "utils";
 
 export async function deployRouter(
-  axelarGateway = "",
-  gasReceiver = "",
   registrar = "",
   hre: HardhatRuntimeEnvironment
 ): Promise<Deployment | undefined> {
@@ -21,8 +19,6 @@ export async function deployRouter(
   const {proxyAdmin} = await getSigners(hre);
 
   try {
-    validateAddress(axelarGateway, "axelarGateway");
-    validateAddress(gasReceiver, "gasReceiver");
     validateAddress(registrar, "registrar");
 
     // deploy implementation
@@ -35,12 +31,7 @@ export async function deployRouter(
     // deploy proxy
     logger.out("Deploying proxy...");
     const network = await hre.ethers.provider.getNetwork();
-    const initData = router.interface.encodeFunctionData("initialize", [
-      network.name,
-      axelarGateway,
-      gasReceiver,
-      registrar,
-    ]);
+    const initData = router.interface.encodeFunctionData("initialize", [network.name, registrar]);
     const constructorArguments: ContractFunctionParams<ProxyContract__factory["deploy"]> = [
       router.address,
       proxyAdmin.address,

contracts/core/vault/APVault_V1.sol

@@ -102,7 +102,12 @@ contract APVault_V1 is IVault, ERC4626AP {
       // redeemAll if less
       return redeemAll(accountId);
     } else if (amt == 0) {
-      return RedemptionResponse({amount: 0, status: VaultActionStatus.UNPROCESSED});
+      return
+        RedemptionResponse({
+          token: vaultConfig.baseToken,
+          amount: 0,
+          status: VaultActionStatus.UNPROCESSED
+        });
     } else {
       // redeem shares for yieldToken -> approve strategy -> strategy withdraw -> base token
       uint256 yieldTokenAmt = super.redeemERC4626(amt, vaultConfig.strategy, accountId);
@@ -124,15 +129,25 @@ contract APVault_V1 is IVault, ERC4626AP {
         revert ApproveFailed();
       }
       // generate and return redemption response
-      return RedemptionResponse({amount: returnAmt, status: VaultActionStatus.SUCCESS});
+      return
+        RedemptionResponse({
+          token: vaultConfig.baseToken,
+          amount: returnAmt,
+          status: VaultActionStatus.SUCCESS
+        });
     }
   }
 
   function redeemAll(
     uint32 accountId
   ) public payable virtual override notPaused onlyApproved returns (RedemptionResponse memory) {
     if (balanceOf(accountId) == 0) {
-      return RedemptionResponse({amount: 0, status: VaultActionStatus.POSITION_EXITED});
+      return
+        RedemptionResponse({
+          token: vaultConfig.baseToken,
+          amount: 0,
+          status: VaultActionStatus.POSITION_EXITED
+        });
     }
     uint256 balance = balanceOf(accountId);
     // redeem shares for yieldToken -> approve strategy
@@ -159,6 +174,7 @@ contract APVault_V1 is IVault, ERC4626AP {
     }
     // generate redemption response
     RedemptionResponse memory response = RedemptionResponse({
+      token: vaultConfig.baseToken,
       amount: returnAmt,
       status: VaultActionStatus.POSITION_EXITED
     });

contracts/core/vault/interfaces/IVault.sol

@@ -66,6 +66,7 @@ abstract contract IVault {
   }
 
   struct RedemptionResponse {
+    address token;
     uint256 amount;
     VaultActionStatus status;
   }

contracts/multisigs/MultiSigGeneric.sol

@@ -206,7 +206,7 @@ contract MultiSigGeneric is
     confirmations[transactionId].confirmationsByOwner[msg.sender] = true;
     confirmations[transactionId].count += 1;
     emitTransactionConfirmed(msg.sender, transactionId);
-    // if execution is not required and confirmation count is met, execute 
+    // if execution is not required and confirmation count is met, execute
     if (!requireExecution && confirmations[transactionId].count >= approvalsRequired) {
       executeTransaction(transactionId);
     }

contracts/test/DummyVault.sol

@@ -38,13 +38,23 @@ contract DummyVault is IVault {
   ) public payable override returns (RedemptionResponse memory) {
     IERC20(vaultConfig.baseToken).approve(msg.sender, amt);
     emit Redeem(accountId, vaultConfig.vaultType, vaultConfig.baseToken, amt);
-    return RedemptionResponse({amount: amt, status: VaultActionStatus.SUCCESS});
+    return
+      RedemptionResponse({
+        token: vaultConfig.baseToken,
+        amount: amt,
+        status: VaultActionStatus.SUCCESS
+      });
   }
 
   function redeemAll(uint32 accountId) public payable override returns (RedemptionResponse memory) {
     IERC20(vaultConfig.baseToken).approve(msg.sender, dummyAmt);
     emit Redeem(accountId, vaultConfig.vaultType, address(this), dummyAmt);
-    return RedemptionResponse({amount: dummyAmt, status: VaultActionStatus.POSITION_EXITED});
+    return
+      RedemptionResponse({
+        token: vaultConfig.baseToken,
+        amount: dummyAmt,
+        status: VaultActionStatus.POSITION_EXITED
+      });
   }
 
   function harvest(uint32[] calldata accountIds) public override {

tasks/deploy/deployAngelProtocol.ts

@@ -60,12 +60,7 @@ task("deploy:AngelProtocol", "Will deploy complete Angel Protocol")
       );
 
       // Router deployment will require updating Registrar config's "router" address
-      const router = await deployRouter(
-        thirdPartyAddresses.axelarGateway.address,
-        thirdPartyAddresses.axelarGasService.address,
-        registrar?.address,
-        hre
-      );
+      const router = await deployRouter(registrar?.address, hre);
 
       const accounts = await deployAccountsDiamond(
         apTeamMultisig?.address,

tasks/deploy/deployLocalRegistrar.ts

@@ -41,12 +41,7 @@ task("deploy:LocalRegistrarAndRouter", "Will deploy the Local Registrar contract
         return;
       }
 
-      const routerDeployment = await deployRouter(
-        addresses.axelar.gateway,
-        addresses.axelar.gasService,
-        localRegistrarDeployment.address,
-        hre
-      );
+      const routerDeployment = await deployRouter(localRegistrarDeployment.address, hre);
 
       if (!routerDeployment) {
         return;