From a24f4048e3de42df40e046acf39b569e493f8c92 Mon Sep 17 00:00:00 2001 From: Domen Grabec Date: Mon, 13 Nov 2023 15:16:06 +0100 Subject: [PATCH 1/4] make VaultAdmin not fail when withdrawing from all of the strategis when withdrawAll from a strategy fails --- ...lancerComposablePoolBrokenTestStrategy.sol | 33 +++++++++ .../balancer/BalancerMetaPoolStrategy.sol | 8 ++- contracts/contracts/vault/VaultAdmin.sol | 4 +- contracts/contracts/vault/VaultStorage.sol | 1 + contracts/test/fixture/_custom-deploys.js | 2 +- contracts/test/fixture/_fixture.js | 31 ++++++-- contracts/test/fixture/_hot-deploy.js | 71 ++++++++++++++----- .../BalancerComposableStablePool.fork-test.js | 36 +++++++++- .../balancerPoolReentrancy.fork-test.js | 2 +- 9 files changed, 160 insertions(+), 28 deletions(-) create mode 100644 contracts/contracts/mocks/BalancerComposablePoolBrokenTestStrategy.sol diff --git a/contracts/contracts/mocks/BalancerComposablePoolBrokenTestStrategy.sol b/contracts/contracts/mocks/BalancerComposablePoolBrokenTestStrategy.sol new file mode 100644 index 0000000000..79fe6467c0 --- /dev/null +++ b/contracts/contracts/mocks/BalancerComposablePoolBrokenTestStrategy.sol @@ -0,0 +1,33 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.0; + +/** + * @title OETH Balancer BalancerComposablePoolTest Strategy + * This one is required to expose internal functions to our test suite. In fixtures + * we replace the mainnet byte code with the one from this contract. + * + * @author Origin Protocol Inc + */ +import { BalancerComposablePoolStrategy } from "../strategies/balancer/BalancerComposablePoolStrategy.sol"; + +contract BalancerComposablePoolBrokenTestStrategy is + BalancerComposablePoolStrategy +{ + constructor( + BaseStrategyConfig memory _stratConfig, + BaseBalancerConfig memory _balancerConfig, + address _auraRewardPoolAddress, + uint256 _bptTokenPoolPosition + ) + BalancerComposablePoolStrategy( + _stratConfig, + _balancerConfig, + _auraRewardPoolAddress, + _bptTokenPoolPosition + ) + {} + + function withdrawAll() external override onlyVaultOrGovernor nonReentrant { + require(false, "Simulating a failed call"); + } +} diff --git a/contracts/contracts/strategies/balancer/BalancerMetaPoolStrategy.sol b/contracts/contracts/strategies/balancer/BalancerMetaPoolStrategy.sol index b5cb1fa4d7..c9b0583fed 100644 --- a/contracts/contracts/strategies/balancer/BalancerMetaPoolStrategy.sol +++ b/contracts/contracts/strategies/balancer/BalancerMetaPoolStrategy.sol @@ -448,7 +448,13 @@ contract BalancerMetaPoolStrategy is BaseAuraStrategy { * * Is only executable by the OToken's Vault or the Governor. */ - function withdrawAll() external override onlyVaultOrGovernor nonReentrant { + function withdrawAll() + external + virtual + override + onlyVaultOrGovernor + nonReentrant + { _withdrawAll(false); } diff --git a/contracts/contracts/vault/VaultAdmin.sol b/contracts/contracts/vault/VaultAdmin.sol index 5d4adcf2e1..454367831f 100644 --- a/contracts/contracts/vault/VaultAdmin.sol +++ b/contracts/contracts/vault/VaultAdmin.sol @@ -618,7 +618,9 @@ contract VaultAdmin is VaultStorage { function withdrawAllFromStrategies() external onlyGovernorOrStrategist { uint256 stratCount = allStrategies.length; for (uint256 i = 0; i < stratCount; ++i) { - IStrategy(allStrategies[i]).withdrawAll(); + try IStrategy(allStrategies[i]).withdrawAll() {} catch { + emit WithdrawFromStrategyFailed(allStrategies[i]); + } } } diff --git a/contracts/contracts/vault/VaultStorage.sol b/contracts/contracts/vault/VaultStorage.sol index 066754e38f..b47f073ae2 100644 --- a/contracts/contracts/vault/VaultStorage.sol +++ b/contracts/contracts/vault/VaultStorage.sol @@ -52,6 +52,7 @@ contract VaultStorage is Initializable, Governable { uint256 _fromAssetAmount, uint256 _toAssetAmount ); + event WithdrawFromStrategyFailed(address _strategyAddress); // Assets supported by the Vault, i.e. Stablecoins enum UnitConversion { diff --git a/contracts/test/fixture/_custom-deploys.js b/contracts/test/fixture/_custom-deploys.js index 98c1dc9ed3..a1e96a87d3 100644 --- a/contracts/test/fixture/_custom-deploys.js +++ b/contracts/test/fixture/_custom-deploys.js @@ -32,7 +32,7 @@ const deployBalancerFrxEethRethWstEThStrategyMissConfigured = async () => { }); const strategy = await ethers.getContract("BalancerComposablePoolStrategy"); - log("Strategy deployed, calling initialize") + log("Strategy deployed, calling initialize"); // prettier-ignore await strategy .connect(sTimelock)["initialize(address[],address[],address[])"]( diff --git a/contracts/test/fixture/_fixture.js b/contracts/test/fixture/_fixture.js index 901bae7c39..96d36ac344 100644 --- a/contracts/test/fixture/_fixture.js +++ b/contracts/test/fixture/_fixture.js @@ -1034,12 +1034,8 @@ async function balancerFrxETHwstETHeETHFixture( * replace the byte code with the one that exposes internal functions */ async function balancerRethWETHExposeFunctionFixture() { - const fixture = await balancerREthFixture(); await hotDeployOption(fixture, "balancerRethWETHExposeFunctionFixture"); - - const { balancerREthStrategy, josh } = fixture; - return fixture; } @@ -1056,7 +1052,31 @@ async function balancerSfrxETHRETHWstETHMissConfiguredStrategy() { */ async function balancerSfrxETHRETHWstETHExposeFunctionFixture() { const fixture = await balancerFrxETHwstETHeETHFixture(); - await hotDeployOption(fixture, "balancerSfrxETHRETHWstETHExposeFunctionFixture"); + await hotDeployOption( + fixture, + "balancerSfrxETHRETHWstETHExposeFunctionFixture" + ); + return fixture; +} + +/** + * Configure a Vault with the Balancer strategy for frxEth/Reth/wstEth pool and + * replace the byte code with the one that fails on a withdrawAll call + */ +async function balancerSfrxETHRETHWstETHBrokenWithdrawalFixture() { + const fixture = await balancerFrxETHwstETHeETHFixture(); + await hotDeployOption( + fixture, + "balancerSfrxETHRETHWstETHBrokenWithdrawalFixture", + { + isOethFixture: true, + /* force deploy strategy (+ vaultAdmin in this case) for test suite + * to be able to run a VaultAdmin test that hasn't been deployed yet. + */ + forceDeployStrategy: true, + } + ); + return fixture; } @@ -2083,6 +2103,7 @@ module.exports = { balancerRethWETHExposeFunctionFixture, balancerSfrxETHRETHWstETHExposeFunctionFixture, balancerSfrxETHRETHWstETHMissConfiguredStrategy, + balancerSfrxETHRETHWstETHBrokenWithdrawalFixture, fluxStrategyFixture, buybackFixture, nodeSnapshot, diff --git a/contracts/test/fixture/_hot-deploy.js b/contracts/test/fixture/_hot-deploy.js index f29f3468c0..da257c7bc4 100644 --- a/contracts/test/fixture/_hot-deploy.js +++ b/contracts/test/fixture/_hot-deploy.js @@ -39,8 +39,12 @@ async function constructNewContract(fixture, implContractName) { ], addresses.mainnet.rETH_WETH_AuraRewards, // Address of the Aura rewards contract ]; - } - else if (implContractName === "BalancerComposablePoolTestStrategy") { + } else if ( + [ + "BalancerComposablePoolBrokenTestStrategy", + "BalancerComposablePoolTestStrategy", + ].includes(implContractName) + ) { return [ [ addresses.mainnet.wstETH_sfrxETH_rETH_BPT, @@ -102,15 +106,16 @@ async function constructNewContract(fixture, implContractName) { async function hotDeployOption( fixture, fixtureName, - config = { isOethFixture: false } + config = { isOethFixture: false, forceDeployStrategy: false } ) { if (!isFork) return; const hotDeployOptions = (process.env.HOT_DEPLOY || "") .split(",") .map((item) => item.trim()); - const { isOethFixture } = config; - const deployStrat = hotDeployOptions.includes("strategy"); + const { isOethFixture, forceDeployStrategy } = config; + const deployStrat = + hotDeployOptions.includes("strategy") || forceDeployStrategy; const deployVaultCore = hotDeployOptions.includes("vaultCore"); const deployVaultAdmin = hotDeployOptions.includes("vaultAdmin"); const deployHarvester = hotDeployOptions.includes("harvester"); @@ -127,10 +132,13 @@ async function hotDeployOption( ); // IMPORTANT: remove once rETH/WETH is redeployed with the new code base - await fixture.balancerREthStrategy.connect(fixture.josh).cachePoolAssets(); + await fixture.balancerREthStrategy + .connect(fixture.josh) + .cachePoolAssets(); // IMPORTANT also remove this one - await fixture.balancerREthStrategy.connect(fixture.josh).cacheRateProviders(); - + await fixture.balancerREthStrategy + .connect(fixture.josh) + .cacheRateProviders(); } else if (fixtureName === "morphoCompoundFixture") { await hotDeployFixture( fixture, // fixture @@ -155,19 +163,48 @@ async function hotDeployOption( "balancerREthStrategy", // fixtureStrategyVarName "BalancerMetaPoolTestStrategy" // implContractName ); - } else if (fixtureName === "balancerRethWETHExposeFunctionFixture") { - await hotDeployFixture( - fixture, // fixture - "balancerREthStrategy", // fixtureStrategyVarName - "BalancerMetaPoolTestStrategy" // implContractName - ); - } else if (fixtureName === "balancerSfrxETHRETHWstETHExposeFunctionFixture") { + } else if ( + fixtureName === "balancerSfrxETHRETHWstETHBrokenWithdrawalFixture" + ) { await hotDeployFixture( fixture, // fixture "balancerSfrxWstRETHStrategy", // fixtureStrategyVarName - "BalancerComposablePoolTestStrategy" // implContractName + "BalancerComposablePoolBrokenTestStrategy" // implContractName ); - } else if (fixtureName === "deployBalancerFrxEethRethWstEThStrategyMissConfigured") { + /* + * Delete this piece of code once the new VaultAdmin implementation is deployed. + */ + const oethVaultAdminImplAddress = + "0x" + + ( + await ethers.provider.send("eth_getStorageAt", [ + fixture.oethVault.address, + "0xa2bd3d3cf188a41358c8b401076eb59066b09dec5775650c0de4c55187d17bd9", // Vault admin implementation position + "latest", // block + ]) + ).substring(26); + + if ( + oethVaultAdminImplAddress !== + "0x31a91336414d3b955e494e7d485a6b06b55fc8fb" + ) { + throw Error( + "OETHVaultAdmin has been re-deployed. Hot-deploy shouldn't be re-deploying it anymore." + ); + } + + await hotDeployVaultAdmin( + fixture, + true, // deploy VaultAdmin + false, // deploy VaultCore + true // isOethFixture + ); + } else if ( + [ + "balancerSfrxETHRETHWstETHExposeFunctionFixture", + "deployBalancerFrxEethRethWstEThStrategyMissConfigured", + ].includes(fixtureName) + ) { await hotDeployFixture( fixture, // fixture "balancerSfrxWstRETHStrategy", // fixtureStrategyVarName diff --git a/contracts/test/strategies/BalancerComposableStablePool.fork-test.js b/contracts/test/strategies/BalancerComposableStablePool.fork-test.js index aef6deaaf4..eade3e261a 100644 --- a/contracts/test/strategies/BalancerComposableStablePool.fork-test.js +++ b/contracts/test/strategies/BalancerComposableStablePool.fork-test.js @@ -12,6 +12,7 @@ const { createFixtureLoader, balancerSfrxETHRETHWstETHExposeFunctionFixture, balancerSfrxETHRETHWstETHMissConfiguredStrategy, + balancerSfrxETHRETHWstETHBrokenWithdrawalFixture, } = require("../fixture/_fixture"); const { tiltPool, unTiltPool } = require("../fixture/_pool_tilt"); @@ -289,13 +290,18 @@ describe("ForkTest: Balancer ComposableStablePool sfrxETH/wstETH/rETH Strategy", "checkBalance()" ](); - expect(rethInVaultBefore.sub(rethInVaultAfter)).to.approxEqualTolerance(rethUnits, 0.5); + expect(rethInVaultBefore.sub(rethInVaultAfter)).to.approxEqualTolerance( + rethUnits, + 0.5 + ); // stETH has rounding issues expect(stETHInVaultBefore.sub(stETHInVaultAfter)).to.approxEqualTolerance( stethUnits, 0.01 ); - expect(frxETHInVaultBefore.sub(frxETHInVaultAfter)).to.approxEqualTolerance(frxethUnits, 0.5); + expect( + frxETHInVaultBefore.sub(frxETHInVaultAfter) + ).to.approxEqualTolerance(frxethUnits, 0.5); expect( strategyValueAfter.sub(strategyValueBefore) ).to.approxEqualTolerance( @@ -428,6 +434,32 @@ describe("ForkTest: Balancer ComposableStablePool sfrxETH/wstETH/rETH Strategy", }); } + it("Should be able to call withdrawAllFromStrategies even when one errors out", async function () { + const fixture = await balancerSfrxETHRETHWstETHBrokenWithdrawalFixture(); + const { timelock, oethVault, balancerSfrxWstRETHStrategy } = fixture; + const strategyAddress = balancerSfrxWstRETHStrategy.address.toLowerCase(); + + // Withdraw all from strategies + const tx = await oethVault.connect(timelock).withdrawAllFromStrategies(); + const events = (await tx.wait()).events || []; + /* + * If OethVaultAdmin has triggered the "WithdrawFromStrategyFailed" with the + * balancerSfrxWstRETHStrategy address the below variable will equal that address, + * otherwise it will be `undefined` + */ + const balancerStrategyAddressOption = events + // 0xbf15... -> "WithdrawFromStrategyFailed" event + .filter( + (e) => + e.topics[0] === + "0xbf15aee350c3b4fb830dccbe5255de03252ad7eea9c1e5e721c2280704ef5b37" + ) + .map((e) => "0x" + e.data.substring(26).toLowerCase()) + .find((evtStrategyAddress) => evtStrategyAddress === strategyAddress); + + expect(balancerStrategyAddressOption).to.not.be.undefined; + }); + it("Should be able to withdraw all of pool liquidity", async function () { const { oethVault, stETH, frxETH, reth, balancerSfrxWstRETHStrategy } = fixture; diff --git a/contracts/test/strategies/balancerPoolReentrancy.fork-test.js b/contracts/test/strategies/balancerPoolReentrancy.fork-test.js index 2ded8411d2..fed0bdca92 100644 --- a/contracts/test/strategies/balancerPoolReentrancy.fork-test.js +++ b/contracts/test/strategies/balancerPoolReentrancy.fork-test.js @@ -10,7 +10,7 @@ const { deployWithConfirmation } = require("../../utils/deploy"); const addresses = require("../../utils/addresses"); const { setERC20TokenBalance } = require("../_fund"); -describe.only("ForkTest: Balancer MetaStablePool - Read-only Reentrancy", function () { +describe("ForkTest: Balancer MetaStablePool - Read-only Reentrancy", function () { this.timeout(0); // Retry up to 3 times on CI this.retries(isCI ? 3 : 0); From 0c12fc37805cc6c26e300ea780ec2f2c2d871ccc Mon Sep 17 00:00:00 2001 From: Domen Grabec Date: Mon, 13 Nov 2023 17:12:05 +0100 Subject: [PATCH 2/4] add a separate withdrawal function --- contracts/contracts/interfaces/IVault.sol | 2 ++ contracts/contracts/vault/VaultAdmin.sol | 14 ++++++++++++++ contracts/test/abi/IVault.json | 7 +++++++ .../BalancerComposableStablePool.fork-test.js | 9 +++++++-- 4 files changed, 30 insertions(+), 2 deletions(-) diff --git a/contracts/contracts/interfaces/IVault.sol b/contracts/contracts/interfaces/IVault.sol index e6b1668ca9..8d6af1b124 100644 --- a/contracts/contracts/interfaces/IVault.sol +++ b/contracts/contracts/interfaces/IVault.sol @@ -124,6 +124,8 @@ interface IVault { function withdrawAllFromStrategies() external; + function faultTolerantWithdrawAllFromStrategies() external; + function withdrawFromStrategy( address _strategyFromAddress, address[] calldata _assets, diff --git a/contracts/contracts/vault/VaultAdmin.sol b/contracts/contracts/vault/VaultAdmin.sol index 454367831f..cb2be335d3 100644 --- a/contracts/contracts/vault/VaultAdmin.sol +++ b/contracts/contracts/vault/VaultAdmin.sol @@ -616,6 +616,20 @@ contract VaultAdmin is VaultStorage { * @notice Withdraws all assets from all the strategies and sends assets to the Vault. */ function withdrawAllFromStrategies() external onlyGovernorOrStrategist { + uint256 stratCount = allStrategies.length; + for (uint256 i = 0; i < stratCount; ++i) { + IStrategy(allStrategies[i]).withdrawAll(); + } + } + + /** + * @notice Withdraws all assets from all the strategies and sends assets to the Vault. + * and doesn't revert event when `withdrawAll` of a strategy reverts. + */ + function faultTolerantWithdrawAllFromStrategies() + external + onlyGovernorOrStrategist + { uint256 stratCount = allStrategies.length; for (uint256 i = 0; i < stratCount; ++i) { try IStrategy(allStrategies[i]).withdrawAll() {} catch { diff --git a/contracts/test/abi/IVault.json b/contracts/test/abi/IVault.json index d27c3f8fdb..e11a44dff5 100644 --- a/contracts/test/abi/IVault.json +++ b/contracts/test/abi/IVault.json @@ -991,6 +991,13 @@ "stateMutability": "nonpayable", "type": "function" }, + { + "inputs": [], + "name": "faultTolerantWithdrawAllFromStrategies", + "outputs": [], + "stateMutability": "nonpayable", + "type": "function" + }, { "inputs": [ { diff --git a/contracts/test/strategies/BalancerComposableStablePool.fork-test.js b/contracts/test/strategies/BalancerComposableStablePool.fork-test.js index eade3e261a..4f6dcc168a 100644 --- a/contracts/test/strategies/BalancerComposableStablePool.fork-test.js +++ b/contracts/test/strategies/BalancerComposableStablePool.fork-test.js @@ -434,13 +434,15 @@ describe("ForkTest: Balancer ComposableStablePool sfrxETH/wstETH/rETH Strategy", }); } - it("Should be able to call withdrawAllFromStrategies even when one errors out", async function () { + it("Should be able to call faultTolerantWithdrawAllFromStrategies even when one errors out", async function () { const fixture = await balancerSfrxETHRETHWstETHBrokenWithdrawalFixture(); const { timelock, oethVault, balancerSfrxWstRETHStrategy } = fixture; const strategyAddress = balancerSfrxWstRETHStrategy.address.toLowerCase(); // Withdraw all from strategies - const tx = await oethVault.connect(timelock).withdrawAllFromStrategies(); + const tx = await oethVault + .connect(timelock) + .faultTolerantWithdrawAllFromStrategies(); const events = (await tx.wait()).events || []; /* * If OethVaultAdmin has triggered the "WithdrawFromStrategyFailed" with the @@ -458,6 +460,9 @@ describe("ForkTest: Balancer ComposableStablePool sfrxETH/wstETH/rETH Strategy", .find((evtStrategyAddress) => evtStrategyAddress === strategyAddress); expect(balancerStrategyAddressOption).to.not.be.undefined; + + await expect(oethVault.connect(timelock).withdrawAllFromStrategies()).to + .be.reverted; }); it("Should be able to withdraw all of pool liquidity", async function () { From b420197ec3fb3f4c3078acdce87490c8128f1b06 Mon Sep 17 00:00:00 2001 From: Domen Grabec Date: Mon, 13 Nov 2023 18:21:59 +0100 Subject: [PATCH 3/4] remove only --- .../test/strategies/BalancerComposableStablePool.fork-test.js | 2 +- contracts/test/strategies/balancerMetaStablePool.fork-test.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/contracts/test/strategies/BalancerComposableStablePool.fork-test.js b/contracts/test/strategies/BalancerComposableStablePool.fork-test.js index 440616188a..4f6dcc168a 100644 --- a/contracts/test/strategies/BalancerComposableStablePool.fork-test.js +++ b/contracts/test/strategies/BalancerComposableStablePool.fork-test.js @@ -30,7 +30,7 @@ const loadBalancerFrxWstrETHFixture = createFixtureLoader( } ); -describe.only("ForkTest: Balancer ComposableStablePool sfrxETH/wstETH/rETH Strategy", function () { +describe("ForkTest: Balancer ComposableStablePool sfrxETH/wstETH/rETH Strategy", function () { this.timeout(0); this.retries(isCI ? 3 : 0); diff --git a/contracts/test/strategies/balancerMetaStablePool.fork-test.js b/contracts/test/strategies/balancerMetaStablePool.fork-test.js index d946652c55..c1a98f7fa1 100644 --- a/contracts/test/strategies/balancerMetaStablePool.fork-test.js +++ b/contracts/test/strategies/balancerMetaStablePool.fork-test.js @@ -37,7 +37,7 @@ const loadBalancerREthFixtureNotDefault = createFixtureLoader( const loadBalancerWstEthFixture = createFixtureLoader(balancerWstEthFixture); -describe.only("ForkTest: Balancer MetaStablePool rETH/WETH Strategy", function () { +describe("ForkTest: Balancer MetaStablePool rETH/WETH Strategy", function () { this.timeout(0); this.retries(isCI ? 3 : 0); From cef7b5bb418d889d6aea163b3e00a9b579c526da Mon Sep 17 00:00:00 2001 From: Domen Grabec Date: Wed, 15 Nov 2023 13:14:27 +0100 Subject: [PATCH 4/4] fix fork tests --- .../balancer/BalancerMetaPoolStrategy.sol | 2 +- contracts/test/fixture/_fixture.js | 14 +++++--- contracts/test/fixture/_hot-deploy.js | 34 ++++++++++++++----- .../balancerMetaStablePool.fork-test.js | 5 +++ 4 files changed, 41 insertions(+), 14 deletions(-) diff --git a/contracts/contracts/strategies/balancer/BalancerMetaPoolStrategy.sol b/contracts/contracts/strategies/balancer/BalancerMetaPoolStrategy.sol index cc648a85b4..3f8deaef29 100644 --- a/contracts/contracts/strategies/balancer/BalancerMetaPoolStrategy.sol +++ b/contracts/contracts/strategies/balancer/BalancerMetaPoolStrategy.sol @@ -160,7 +160,7 @@ contract BalancerMetaPoolStrategy is BaseAuraStrategy { /* This check is triggered when the _deposit is called with * a duplicate asset in the _strategyAssets array * - * A duplicate asset supplied with 0 amount or an amount close to + * A duplicate asset supplied with 0 amount or an amount close to * 0 that wraps to a 0 amount will still pass this check. */ require( diff --git a/contracts/test/fixture/_fixture.js b/contracts/test/fixture/_fixture.js index da6bc8c8eb..3c246e0b86 100644 --- a/contracts/test/fixture/_fixture.js +++ b/contracts/test/fixture/_fixture.js @@ -320,12 +320,12 @@ const defaultFixture = deployments.createFixture(async () => { fraxEthStrategyProxy.address ); - const balancerRethStrategyProxy = await ethers.getContract( + const balancerREthStrategyProxy = await ethers.getContract( "OETHBalancerMetaPoolrEthStrategyProxy" ); balancerREthStrategy = await ethers.getContractAt( "BalancerMetaPoolStrategy", - balancerRethStrategyProxy.address + balancerREthStrategyProxy.address ); const balancerSfrxWstRETHStrategyProxy = await ethers.getContract( @@ -905,6 +905,7 @@ async function convexVaultFixture() { */ async function balancerREthFixture(config = { defaultStrategy: true }) { const fixture = await defaultFixture(); + await hotDeployOption(fixture, "balancerREthFixture", { isOethFixture: true, }); @@ -1035,7 +1036,9 @@ async function balancerFrxETHwstETHeETHFixture( */ async function balancerRethWETHExposeFunctionFixture() { const fixture = await balancerREthFixture(); - await hotDeployOption(fixture, "balancerRethWETHExposeFunctionFixture"); + await hotDeployOption(fixture, "balancerRethWETHExposeFunctionFixture", { + forceDeployStrategy: true, + }); return fixture; } @@ -1054,7 +1057,10 @@ async function balancerSfrxETHRETHWstETHExposeFunctionFixture() { const fixture = await balancerFrxETHwstETHeETHFixture(); await hotDeployOption( fixture, - "balancerSfrxETHRETHWstETHExposeFunctionFixture" + "balancerSfrxETHRETHWstETHExposeFunctionFixture", + { + forceDeployStrategy: true, + } ); return fixture; } diff --git a/contracts/test/fixture/_hot-deploy.js b/contracts/test/fixture/_hot-deploy.js index b3f755f531..1e0c34ba92 100644 --- a/contracts/test/fixture/_hot-deploy.js +++ b/contracts/test/fixture/_hot-deploy.js @@ -120,9 +120,30 @@ async function hotDeployOption( const deployVaultAdmin = hotDeployOptions.includes("vaultAdmin"); const deployHarvester = hotDeployOptions.includes("harvester"); - log(`Running fixture hot deployment w/ config; isOethFixture:${isOethFixture} strategy:${!!deployStrat} + log(`Running fixture [${fixtureName}] hot deployment w/ config; isOethFixture:${isOethFixture} strategy:${!!deployStrat} vaultCore:${!!deployVaultCore} vaultAdmin:${!!deployVaultAdmin} harvester:${!!deployHarvester}`); + const cacheAssetsAndProviders = async (strategyVarName) => { + const proxy = await ethers.getContractAt( + "InitializeGovernedUpgradeabilityProxy", + fixture[strategyVarName].address + ); + + const implAddress = await proxy.implementation(); + log(`Current impl address: ${implAddress}`); + // new version with cached rate providers / assets has not been deployed yet + if (implAddress === "0xAaA1d497fdff9a88048743Db31d3173a2E442A3D") { + log(`Caching rate providers and pool assets`); + await fixture[strategyVarName].connect(fixture.josh).cachePoolAssets(); + + await fixture[strategyVarName].connect(fixture.josh).cacheRateProviders(); + } else { + throw new Error( + `New ${strategyVarName} deployed, removed manual caching!` + ); + } + }; + if (deployStrat) { if (fixtureName === "balancerREthFixture") { await hotDeployFixture( @@ -131,14 +152,7 @@ async function hotDeployOption( "BalancerMetaPoolStrategy" // implContractName ); - // IMPORTANT: remove once rETH/WETH is redeployed with the new code base - await fixture.balancerREthStrategy - .connect(fixture.josh) - .cachePoolAssets(); - // IMPORTANT also remove this one - await fixture.balancerREthStrategy - .connect(fixture.josh) - .cacheRateProviders(); + await cacheAssetsAndProviders("balancerREthStrategy"); } else if (fixtureName === "morphoCompoundFixture") { await hotDeployFixture( fixture, // fixture @@ -163,6 +177,8 @@ async function hotDeployOption( "balancerREthStrategy", // fixtureStrategyVarName "BalancerMetaPoolTestStrategy" // implContractName ); + + await cacheAssetsAndProviders("balancerREthStrategy"); } else if ( fixtureName === "balancerSfrxETHRETHWstETHBrokenWithdrawalFixture" ) { diff --git a/contracts/test/strategies/balancerMetaStablePool.fork-test.js b/contracts/test/strategies/balancerMetaStablePool.fork-test.js index f4f851548d..100b70553b 100644 --- a/contracts/test/strategies/balancerMetaStablePool.fork-test.js +++ b/contracts/test/strategies/balancerMetaStablePool.fork-test.js @@ -534,6 +534,11 @@ describe("ForkTest: Balancer MetaStablePool rETH/WETH Strategy", function () { }); it(`Should fail when duplicating an asset in withdrawal call`, async function () { + /* using expose function so that the code is force-deployed - unit needed, + * that is until we deploy updated implementation to the mainnet + */ + fixture = await balancerRethWETHExposeFunctionFixture(); + const { balancerREthStrategy, oethVault, weth } = fixture; const oethVaultSigner = await impersonateAndFund(oethVault.address);