From 28a5a0c51dc37d62e87ef3eb550ac1d1e16e30a3 Mon Sep 17 00:00:00 2001 From: katzman Date: Wed, 13 Sep 2023 22:41:30 +0000 Subject: [PATCH] Updated Router tests and fixed utils --- contracts/core/router/Router.sol | 11 +- test/core/router/Router.ts | 174 +++++++++--------------- test/utils/helpers/IVaultHelpers.ts | 4 +- test/utils/helpers/RouterHelpers.ts | 4 +- test/utils/helpers/accounts/defaults.ts | 8 +- 5 files changed, 82 insertions(+), 119 deletions(-) diff --git a/contracts/core/router/Router.sol b/contracts/core/router/Router.sol index 7cecc4b34..aadab1844 100644 --- a/contracts/core/router/Router.sol +++ b/contracts/core/router/Router.sol @@ -66,14 +66,13 @@ contract Router is IRouter, Initializable, AxelarExecutable { modifier operatorOnly { require( - registrar.getVaultOperatorApproved(msg.sender) + registrar.getVaultOperatorApproved(msg.sender), "Operator only" ); _; } modifier validateDeposit( IVault.VaultActionData memory action, - string calldata tokenSymbol, uint256 amount ) { // deposit only @@ -83,8 +82,7 @@ contract Router is IRouter, Initializable, 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); - require(registrar.isTokenAccepted(tokenAddress), "Token not accepted"); + require(registrar.isTokenAccepted(action.token), "Token not accepted"); // Get parameters from registrar if approved require( registrar.getStrategyApprovalState(action.strategyId) == @@ -132,9 +130,8 @@ contract Router is IRouter, Initializable, AxelarExecutable { /// @dev onlySelf restricted public method to enable try/catch in caller function deposit( IVault.VaultActionData memory action, - string calldata tokenSymbol, uint256 amount - ) public onlySelf validateDeposit(action, tokenSymbol, amount) { + ) public onlySelf validateDeposit(action, amount) { LocalRegistrarLib.StrategyParams memory params = registrar.getStrategyParamsById( action.strategyId ); @@ -466,7 +463,7 @@ contract Router is IRouter, Initializable, AxelarExecutable { action.destinationChain = sourceChain; // Leverage this.call() to enable try/catch logic - try this.deposit(action, tokenSymbol, amount) { + try this.deposit(action, amount) { emit Deposit(action); action.status = IVault.VaultActionStatus.SUCCESS; return action; diff --git a/test/core/router/Router.ts b/test/core/router/Router.ts index 89e292d53..d051e7841 100644 --- a/test/core/router/Router.ts +++ b/test/core/router/Router.ts @@ -5,6 +5,7 @@ import {Wallet} from "ethers"; import hre from "hardhat"; import { DEFAULT_ACTION_DATA, + DEFAULT_HARVEST_REQUEST, DEFAULT_NETWORK_INFO, DEFAULT_STRATEGY_ID, packActionData, @@ -207,7 +208,7 @@ describe("Router", function () { const TOTAL_AMT = LOCK_AMT + LIQ_AMT; const getDefaultActionData = () => ({ ...DEFAULT_ACTION_DATA, - accountIds: [1], + accountId: 1, lockAmt: LOCK_AMT, liqAmt: LIQ_AMT, }); @@ -246,28 +247,6 @@ describe("Router", function () { }); describe("and the refund call is successful back through axelar", function () { - it("when more than one account is specified", async function () { - let actionData = getDefaultActionData(); - actionData.selector = liquidVault.interface.getSighash("deposit"); - actionData.token = token.address; - actionData.accountIds = [1, 2, 3]; - let packedData = await packActionData(actionData, hre); - await expect( - router.executeWithToken( - ethers.utils.formatBytes32String("true"), - originatingChain, - accountsContract, - packedData, - "TKN", - TOTAL_AMT - ) - ) - .to.emit(router, "ErrorLogged") - .withArgs(Array, "Only one account allowed"); - expect(token.approve).to.have.been.calledWith(gateway.address, TOTAL_AMT - GAS_COST); - expect(token.approve).to.have.been.calledWith(gasService.address, GAS_COST); - }); - it("when an action other than deposit is called", async function () { let actionData = getDefaultActionData(); actionData.token = token.address; @@ -403,7 +382,7 @@ describe("Router", function () { const TOTAL_AMT = LOCK_AMT + LIQ_AMT; const getDefaultActionData = () => ({ ...DEFAULT_ACTION_DATA, - accountIds: [1], + accountId: 1, lockAmt: LOCK_AMT, liqAmt: LIQ_AMT, }); @@ -443,27 +422,6 @@ describe("Router", function () { router = await deployRouterAsProxy(registrar.address); }); - it("when more than one account is specified", async function () { - let actionData = getDefaultActionData(); - actionData.selector = liquidVault.interface.getSighash("deposit"); - actionData.token = token.address; - actionData.accountIds = [1, 2, 3]; - let packedData = await packActionData(actionData, hre); - await expect( - router.executeWithToken( - ethers.utils.formatBytes32String("true"), - originatingChain, - accountsContract, - packedData, - await token.symbol(), - TOTAL_AMT - ) - ) - .to.emit(router, "ErrorLogged") - .withArgs(Array, "Only one account allowed"); - expect(token.transfer).to.have.been.calledWith(collector.address, TOTAL_AMT); - }); - it("when an action other than deposit is called", async function () { let actionData = getDefaultActionData(); actionData.token = token.address; @@ -597,7 +555,7 @@ describe("Router", function () { const TOTAL_AMT = LOCK_AMT + LIQ_AMT; const getDefaultActionData = () => ({ ...DEFAULT_ACTION_DATA, - accountIds: [1], + accountId: 1, lockAmt: LOCK_AMT, liqAmt: LIQ_AMT, }); @@ -649,7 +607,7 @@ describe("Router", function () { let actionData = getDefaultActionData(); actionData.selector = liquidVault.interface.getSighash("deposit"); actionData.token = token.address; - let packedData = await packActionData(actionData, hre); + let packedData = packActionData(actionData, hre); expect( await router.executeWithToken( ethers.utils.formatBytes32String("true"), @@ -717,23 +675,6 @@ describe("Router", function () { expect(lockedVault.redeemAll).to.have.been.called; expect(liquidVault.redeemAll).to.have.been.called; }); - - it("correctly calls harvest via execute", async function () { - let actionData = getDefaultActionData(); - actionData.selector = liquidVault.interface.getSighash("harvest"); - actionData.token = token.address; - let packedData = await packActionData(actionData, hre); - expect( - await router.execute( - ethers.utils.formatBytes32String("true"), - originatingChain, - accountsContract, - packedData - ) - ); - expect(lockedVault.harvest).to.have.been.called; - expect(liquidVault.harvest).to.have.been.called; - }); }); describe("Deposit", function () { @@ -750,7 +691,7 @@ describe("Router", function () { const TOTAL_AMT = LOCK_AMT + LIQ_AMT; const getDefaultActionData = () => ({ ...DEFAULT_ACTION_DATA, - accountIds: [1], + accountId: 1, lockAmt: LOCK_AMT, liqAmt: LIQ_AMT, }); @@ -828,7 +769,7 @@ describe("Router", function () { const TOTAL_AMT = LOCK_AMT + LIQ_AMT; const getDefaultActionData = () => ({ ...DEFAULT_ACTION_DATA, - accountIds: [1], + accountId: 1, lockAmt: LOCK_AMT, liqAmt: LIQ_AMT, }); @@ -908,7 +849,7 @@ describe("Router", function () { destinationChain: originatingChain, strategyId: DEFAULT_STRATEGY_ID, selector: liquidVault.interface.getSighash("redeem"), - accountIds: [1], + accountId: 1, token: token.address, lockAmt: LOCK_AMT - 2, // less weighted gas liqAmt: LIQ_AMT - 3, // less weighted gas @@ -977,7 +918,7 @@ describe("Router", function () { const TOTAL_AMT = LOCK_AMT + LIQ_AMT; const getDefaultActionData = () => ({ ...DEFAULT_ACTION_DATA, - accountIds: [1], + accountId: 1, lockAmt: LOCK_AMT, liqAmt: LIQ_AMT, }); @@ -1057,7 +998,7 @@ describe("Router", function () { destinationChain: originatingChain, strategyId: DEFAULT_STRATEGY_ID, selector: liquidVault.interface.getSighash("redeemAll"), - accountIds: [1], + accountId: 1, token: token.address, lockAmt: LOCK_AMT - 2, // less weighted gas liqAmt: LIQ_AMT - 3, // less weighted gas @@ -1086,6 +1027,7 @@ describe("Router", function () { }); it("Reverts if the redemption amount is less than the gas fee", async function () { + registrar.getGasByToken.whenCalledWith(token.address).returns(TOTAL_AMT + 1); let actionData = getDefaultActionData(); actionData.token = token.address; actionData.selector = liquidVault.interface.getSighash("redeemAll"); @@ -1095,12 +1037,11 @@ describe("Router", function () { amount: LOCK_AMT, status: VaultActionStatus.POSITION_EXITED, }); - liquidVault.redeem.returns({ + liquidVault.redeemAll.returns({ token: token.address, amount: LIQ_AMT, status: VaultActionStatus.POSITION_EXITED, }); - registrar.getGasByToken.whenCalledWith(token.address).returns(TOTAL_AMT + 1); await expect( router.execute( ethers.utils.formatBytes32String("true"), @@ -1122,11 +1063,9 @@ describe("Router", function () { const LOCK_AMT = 111; const LIQ_AMT = 222; const TOTAL_AMT = LOCK_AMT + LIQ_AMT; - const getDefaultActionData = () => ({ - ...DEFAULT_ACTION_DATA, - accountIds: [1], - lockAmt: LOCK_AMT, - liqAmt: LIQ_AMT, + const getDefaultHarvestRequest = () => ({ + ...DEFAULT_HARVEST_REQUEST, + accountIds: [1] }); beforeEach(async function () { @@ -1159,6 +1098,7 @@ describe("Router", function () { registrar.getAccountsContractAddressByChain.whenCalledWith(localChain).returns(owner.address); registrar.getStrategyApprovalState.returns(StrategyApprovalState.APPROVED); registrar.getStrategyParamsById.returns(stratParams); + registrar.getVaultOperatorApproved.whenCalledWith(owner.address).returns(true); registrar.getFeeSettingsByFeeType.returns({payoutAddress: collector.address, bps: 1}); registrar.thisChain.returns(localChain); token.transfer.returns(true); @@ -1170,22 +1110,22 @@ describe("Router", function () { }); it("Harvests the targeted accounts and forwards tokens cross-chain", async function () { - let actionData = getDefaultActionData(); - actionData.token = token.address; - actionData.selector = liquidVault.interface.getSighash("harvest"); - let packedData = packActionData(actionData, hre); - lockedVault.harvest.returns(LOCK_AMT); - liquidVault.harvest.returns(LIQ_AMT); + let requestData = getDefaultHarvestRequest(); + lockedVault.harvest.returns({ + token: token.address, + amount: LOCK_AMT, + status: VaultActionStatus.SUCCESS, + }); + liquidVault.harvest.returns({ + token: token.address, + amount: LIQ_AMT, + status: VaultActionStatus.SUCCESS, + }); expect( - await router.execute( - ethers.utils.formatBytes32String("true"), - originatingChain, - accountsContract, - packedData - ) - ).to.emit(router, "RewardsHarvested"); - expect(lockedVault.harvest).to.have.been.calledWith(actionData.accountIds); - expect(liquidVault.harvest).to.have.been.calledWith(actionData.accountIds); + await router.harvest(requestData) + ).to.not.be.reverted; + expect(lockedVault.harvest).to.have.been.calledWith(requestData.accountIds); + expect(liquidVault.harvest).to.have.been.calledWith(requestData.accountIds); expect(token.transfer).to.have.been.calledWith(router.address, LOCK_AMT); expect(token.transfer).to.have.been.calledWith(router.address, LIQ_AMT); expect(token.approve).to.have.been.calledWith(gateway.address, TOTAL_AMT); @@ -1196,17 +1136,14 @@ describe("Router", function () { let lockedVault: FakeContract; let liquidVault: FakeContract; let registrar: FakeContract; - let gateway: FakeContract; let token: FakeContract; let router: Router; const LOCK_AMT = 111; const LIQ_AMT = 222; const TOTAL_AMT = LOCK_AMT + LIQ_AMT; - const getDefaultActionData = () => ({ - ...DEFAULT_ACTION_DATA, - accountIds: [1], - lockAmt: LOCK_AMT, - liqAmt: LIQ_AMT, + const getDefaultHarvestRequest = () => ({ + ...DEFAULT_HARVEST_REQUEST, + accountIds: [1] }); beforeEach(async function () { @@ -1232,6 +1169,7 @@ describe("Router", function () { .returns(owner.address); registrar.getStrategyApprovalState.returns(StrategyApprovalState.APPROVED); registrar.getStrategyParamsById.returns(stratParams); + registrar.getVaultOperatorApproved.whenCalledWith(owner.address).returns(true); registrar.getFeeSettingsByFeeType.returns({payoutAddress: collector.address, bps: 1}); registrar.thisChain.returns(originatingChain); token.transfer.returns(true); @@ -1242,18 +1180,40 @@ describe("Router", function () { router = await deployRouterAsProxy(registrar.address); }); + it("Reverts if the caller is not a valid operator", async function () { + let requestData = getDefaultHarvestRequest(); + lockedVault.harvest.returns({ + token: token.address, + amount: LOCK_AMT, + status: VaultActionStatus.SUCCESS, + }); + liquidVault.harvest.returns({ + token: token.address, + amount: LIQ_AMT, + status: VaultActionStatus.SUCCESS, + }); + await expect( + router.connect(user).harvest(requestData) + ).to.be.revertedWith("Operator only"); + }) + it("Harvests the targeted account and forwards tokens to the local collector", async function () { - let actionData = getDefaultActionData(); - actionData.token = token.address; - actionData.selector = liquidVault.interface.getSighash("harvest"); - let packedData = packActionData(actionData, hre); - lockedVault.harvest.returns(LOCK_AMT); - liquidVault.harvest.returns(LIQ_AMT); + let requestData = getDefaultHarvestRequest(); + lockedVault.harvest.returns({ + token: token.address, + amount: LOCK_AMT, + status: VaultActionStatus.SUCCESS, + }); + liquidVault.harvest.returns({ + token: token.address, + amount: LIQ_AMT, + status: VaultActionStatus.SUCCESS, + }); expect( - await router.connect(owner).executeLocal(originatingChain, owner.address, packedData) - ).to.emit(router, "RewardsHarvested"); - expect(lockedVault.harvest).to.have.been.calledWith(actionData.accountIds); - expect(liquidVault.harvest).to.have.been.calledWith(actionData.accountIds); + await router.harvest(requestData) + ).to.not.be.reverted; + expect(lockedVault.harvest).to.have.been.calledWith(requestData.accountIds); + expect(liquidVault.harvest).to.have.been.calledWith(requestData.accountIds); expect(token.transfer).to.have.been.calledWith(router.address, LOCK_AMT); expect(token.transfer).to.have.been.calledWith(router.address, LIQ_AMT); expect(token.transfer).to.have.been.calledWith(collector.address, TOTAL_AMT); diff --git a/test/utils/helpers/IVaultHelpers.ts b/test/utils/helpers/IVaultHelpers.ts index 33c3b9ce5..a43823cdb 100644 --- a/test/utils/helpers/IVaultHelpers.ts +++ b/test/utils/helpers/IVaultHelpers.ts @@ -5,7 +5,7 @@ export function convertVaultActionStructToArray(actionData: IVault.VaultActionDa actionData.destinationChain, actionData.strategyId, actionData.selector, - actionData.accountIds, + actionData.accountId, actionData.token, actionData.lockAmt, actionData.liqAmt, @@ -18,7 +18,7 @@ export function convertArrayToVaultActionStruct(decodedData: any) { destinationChain: decodedData[0], strategyId: decodedData[1], selector: decodedData[2], - accountIds: decodedData[3], + accountId: decodedData[3], token: decodedData[4], lockAmt: decodedData[5], liqAmt: decodedData[6], diff --git a/test/utils/helpers/RouterHelpers.ts b/test/utils/helpers/RouterHelpers.ts index 566b1610e..c419573b3 100644 --- a/test/utils/helpers/RouterHelpers.ts +++ b/test/utils/helpers/RouterHelpers.ts @@ -6,7 +6,7 @@ export function packActionData( _actionData: IVault.VaultActionDataStruct, {ethers}: HardhatRuntimeEnvironment ): string { - const TypeList = ["string", "bytes4", "bytes4", "uint[]", "address", "uint", "uint", "uint"]; + const TypeList = ["string", "bytes4", "bytes4", "uint", "address", "uint", "uint", "uint"]; return ethers.utils.defaultAbiCoder.encode( TypeList, convertVaultActionStructToArray(_actionData) @@ -17,7 +17,7 @@ export function unpackActionData( _encodedActionData: string, {ethers}: HardhatRuntimeEnvironment ): IVault.VaultActionDataStruct { - const TypeList = ["string", "string", "string", "uint[]", "string", "uint", "uint", "uint"]; + const TypeList = ["string", "string", "string", "uint", "string", "uint", "uint", "uint"]; let decoded = ethers.utils.defaultAbiCoder.decode(TypeList, _encodedActionData); return convertArrayToVaultActionStruct(decoded); } diff --git a/test/utils/helpers/accounts/defaults.ts b/test/utils/helpers/accounts/defaults.ts index c56548205..d46b15fd0 100644 --- a/test/utils/helpers/accounts/defaults.ts +++ b/test/utils/helpers/accounts/defaults.ts @@ -7,6 +7,7 @@ import {IVault} from "typechain-types/contracts/core/accounts/facets/AccountsStr import {LocalRegistrarLib} from "typechain-types/contracts/core/registrar/LocalRegistrar"; import {ADDRESS_ZERO} from "utils"; import {DEFAULT_NETWORK} from "../../constants"; +import { IRouter } from "typechain-types"; export const DEFAULT_PERMISSIONS_STRUCT: LibAccounts.SettingsPermissionStruct = { locked: false, @@ -144,9 +145,14 @@ export const DEFAULT_ACTION_DATA: IVault.VaultActionDataStruct = { destinationChain: "", strategyId: DEFAULT_STRATEGY_ID, selector: "", - accountIds: [], + accountId: 0, token: "", lockAmt: 0, liqAmt: 0, status: 0, }; + +export const DEFAULT_HARVEST_REQUEST: IRouter.HarvestRequestStruct = { + strategyId: DEFAULT_STRATEGY_ID, + accountIds: [], +}