diff --git a/.solhint.json b/.solhint.json index 1599261..1adf0b2 100644 --- a/.solhint.json +++ b/.solhint.json @@ -10,6 +10,7 @@ "func-visibility": ["warn", { "ignoreConstructors": true }], "not-rely-on-time": "off", "max-states-count": "off", + "gas-custom-errors": "error", "immutable-vars-naming": "off" }, "plugins": ["prettier"] diff --git a/contracts/AaveV3InvestStrategy.sol b/contracts/AaveV3InvestStrategy.sol index c881ce2..b67899d 100644 --- a/contracts/AaveV3InvestStrategy.sol +++ b/contracts/AaveV3InvestStrategy.sol @@ -24,8 +24,6 @@ contract AaveV3InvestStrategy is IInvestStrategy { IPool internal immutable _aave; IERC20 internal immutable _asset; - // From OZ v5 - error AccessControlUnauthorizedAccount(address account, bytes32 neededRole); error CanBeCalledOnlyThroughDelegateCall(); error CannotDisconnectWithAssets(); error NoExtraDataAllowed(); diff --git a/contracts/AccessManagedMSV.sol b/contracts/AccessManagedMSV.sol index fc4ed69..a3ce387 100644 --- a/contracts/AccessManagedMSV.sol +++ b/contracts/AccessManagedMSV.sol @@ -3,11 +3,13 @@ pragma solidity ^0.8.0; import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; import {IERC4626} from "@openzeppelin/contracts/interfaces/IERC4626.sol"; +import {IAccessManager} from "@openzeppelin/contracts/access/manager/IAccessManager.sol"; import {ERC4626Upgradeable} from "@openzeppelin/contracts-upgradeable/token/ERC20/extensions/ERC4626Upgradeable.sol"; import {UUPSUpgradeable} from "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol"; import {Math} from "@openzeppelin/contracts/utils/math/Math.sol"; import {MSVBase} from "./MSVBase.sol"; import {IInvestStrategy} from "./interfaces/IInvestStrategy.sol"; +import {AccessManagedProxy} from "./AccessManagedProxy.sol"; /** * @title AccessManagedMSV @@ -122,4 +124,32 @@ contract AccessManagedMSV is MSVBase, UUPSUpgradeable, ERC4626Upgradeable { super._deposit(caller, receiver, assets, shares); _depositToStrategies(assets); } + + /** + * @dev Returns the selector used to define the role required to call forwardToStrategy on a given strategy and + * method + * + * @param strategyIndex The index of the strategy in the _strategies array + * @param method Id of the method to call. Is recommended that the strategy defines an enum with the methods that + * can be called externally and validates this value. + * @return selector The bytes4 selector required to execute the call (will be used with target=address(this)) + */ + function getForwardToStrategySelector(uint8 strategyIndex, uint8 method) public view returns (bytes4 selector) { + // I assemble a fake selector combining the address of the strategy, the index, and the method called + address strategy = address(_strategies[strategyIndex]); + return (bytes4(bytes1(strategyIndex)) >> 8) ^ (bytes4(bytes1(method))) ^ bytes4(bytes20(strategy)); + } + + /// @inheritdoc MSVBase + function _checkForwardToStrategy(uint8 strategyIndex, uint8 method, bytes memory) internal view override { + // To call forwardToStrategy, besides the access the method, we will check with the ACCESS_MANAGER we + // canCall() + IAccessManager acMgr = AccessManagedProxy(payable(address(this))).ACCESS_MANAGER(); + (bool immediate, ) = acMgr.canCall(msg.sender, address(this), getForwardToStrategySelector(strategyIndex, method)); + // This only works when immediate == true, so timelocks can't be applied on this extra permission, + // only on the forwardToStrategy call. + // In the future we might use consumeScheduledOp flow to implement specific delays for specific + // forward calls + if (!immediate) revert AccessManagedProxy.AccessManagedUnauthorized(msg.sender); + } } diff --git a/contracts/CompoundV3InvestStrategy.sol b/contracts/CompoundV3InvestStrategy.sol index ab7a480..cd60ca7 100644 --- a/contracts/CompoundV3InvestStrategy.sol +++ b/contracts/CompoundV3InvestStrategy.sol @@ -4,7 +4,6 @@ pragma solidity ^0.8.0; import {ICompoundV3} from "./interfaces/ICompoundV3.sol"; import {ICometRewards} from "./interfaces/ICometRewards.sol"; import {IERC20} from "@openzeppelin/contracts/interfaces/IERC20.sol"; -import {IAccessControl} from "@openzeppelin/contracts/access/IAccessControl.sol"; import {SwapLibrary} from "@ensuro/swaplibrary/contracts/SwapLibrary.sol"; import {IInvestStrategy} from "./interfaces/IInvestStrategy.sol"; import {StorageSlot} from "@openzeppelin/contracts/utils/StorageSlot.sol"; @@ -36,8 +35,6 @@ contract CompoundV3InvestStrategy is IInvestStrategy { event SwapConfigChanged(SwapLibrary.SwapConfig oldConfig, SwapLibrary.SwapConfig newConfig); - // From OZ v5 - error AccessControlUnauthorizedAccount(address account, bytes32 neededRole); error CanBeCalledOnlyThroughDelegateCall(); error CannotDisconnectWithAssets(); error NoExtraDataAllowed(); @@ -52,12 +49,6 @@ contract CompoundV3InvestStrategy is IInvestStrategy { _; } - modifier onlyRole(bytes32 role) { - if (!IAccessControl(address(this)).hasRole(role, msg.sender)) - revert AccessControlUnauthorizedAccount(msg.sender, role); - _; - } - constructor(ICompoundV3 cToken_, ICometRewards rewardsManager_) { _cToken = cToken_; _rewardsManager = rewardsManager_; @@ -65,7 +56,7 @@ contract CompoundV3InvestStrategy is IInvestStrategy { } function connect(bytes memory initData) external virtual override onlyDelegCall { - _setSwapConfigNoCheck(SwapLibrary.SwapConfig(SwapLibrary.SwapProtocol.undefined, 0, bytes("")), initData); + _setSwapConfig(SwapLibrary.SwapConfig(SwapLibrary.SwapProtocol.undefined, 0, bytes("")), initData); } function disconnect(bool force) external virtual override onlyDelegCall { @@ -103,7 +94,7 @@ contract CompoundV3InvestStrategy is IInvestStrategy { _cToken.supply(_baseToken, assets); } - function _harvestRewards(uint256 price) internal onlyRole(HARVEST_ROLE) { + function _harvestRewards(uint256 price) internal { (address reward, , ) = _rewardsManager.rewardConfig(address(_cToken)); if (reward == address(0)) return; _rewardsManager.claim(address(_cToken), address(this), true); @@ -119,14 +110,7 @@ contract CompoundV3InvestStrategy is IInvestStrategy { emit RewardsClaimed(reward, earned, reinvestAmount); } - function _setSwapConfig(bytes memory newSwapConfigAsBytes) internal onlyRole(SWAP_ADMIN_ROLE) { - _setSwapConfigNoCheck(_getSwapConfig(address(this)), newSwapConfigAsBytes); - } - - function _setSwapConfigNoCheck( - SwapLibrary.SwapConfig memory oldSwapConfig, - bytes memory newSwapConfigAsBytes - ) internal { + function _setSwapConfig(SwapLibrary.SwapConfig memory oldSwapConfig, bytes memory newSwapConfigAsBytes) internal { SwapLibrary.SwapConfig memory swapConfig = abi.decode(newSwapConfigAsBytes, (SwapLibrary.SwapConfig)); swapConfig.validate(); if (abi.encode(swapConfig).length != newSwapConfigAsBytes.length) revert NoExtraDataAllowed(); @@ -140,7 +124,7 @@ contract CompoundV3InvestStrategy is IInvestStrategy { uint256 price = abi.decode(params, (uint256)); _harvestRewards(price); } else if (checkedMethod == ForwardMethods.setSwapConfig) { - _setSwapConfig(params); + _setSwapConfig(_getSwapConfig(address(this)), params); } // Show never reach to this revert, since method should be one of the enum values but leave it in case // we add new values in the enum and we forgot to add them here diff --git a/contracts/MSVBase.sol b/contracts/MSVBase.sol index 1a06702..e3d1ffa 100644 --- a/contracts/MSVBase.sol +++ b/contracts/MSVBase.sol @@ -148,6 +148,16 @@ abstract contract MSVBase is IExposeStorage { revert OnlyStrategyStorageExposed(); } + /** + * @dev Checks the caller can execute this forwardToStrategy call, otherwise reverts + * + * @param strategyIndex The index of the strategy in the _strategies array + * @param method Id of the method to call. Is recommended that the strategy defines an enum with the methods that + * can be called externally and validates this value. + * @param extraData Additional parameters sent to the method. + */ + function _checkForwardToStrategy(uint8 strategyIndex, uint8 method, bytes memory extraData) internal view virtual; + /** * @dev Used to call specific methods on the strategies. Anyone can call this method, is responsability of the * IInvestStrategy to check access permissions when needed. @@ -162,6 +172,7 @@ abstract contract MSVBase is IExposeStorage { uint8 method, bytes memory extraData ) external virtual returns (bytes memory) { + _checkForwardToStrategy(strategyIndex, method, extraData); IInvestStrategy strategy = _strategies[strategyIndex]; if (address(strategy) == address(0)) revert InvalidStrategy(); return _strategies[strategyIndex].dcForward(method, extraData); diff --git a/contracts/MultiStrategyERC4626.sol b/contracts/MultiStrategyERC4626.sol index 5f1e7d8..bdcab09 100644 --- a/contracts/MultiStrategyERC4626.sol +++ b/contracts/MultiStrategyERC4626.sol @@ -26,6 +26,7 @@ contract MultiStrategyERC4626 is MSVBase, PermissionedERC4626 { bytes32 public constant STRATEGY_ADMIN_ROLE = keccak256("STRATEGY_ADMIN_ROLE"); bytes32 public constant QUEUE_ADMIN_ROLE = keccak256("QUEUE_ADMIN_ROLE"); bytes32 public constant REBALANCER_ROLE = keccak256("REBALANCER_ROLE"); + bytes32 public constant FORWARD_TO_STRATEGY_ROLE = keccak256("FORWARD_TO_STRATEGY_ROLE"); /// @custom:oz-upgrades-unsafe-allow constructor constructor() { @@ -177,4 +178,35 @@ contract MultiStrategyERC4626 is MSVBase, PermissionedERC4626 { ) public override onlyRole(REBALANCER_ROLE) returns (uint256) { return super.rebalance(strategyFromIdx, strategyToIdx, amount); } + + /** + * @dev Returns the AccessControl role required to call forwardToStrategy on a given strategy and method + * + * @param strategyIndex The index of the strategy in the _strategies array + * @param method Id of the method to call. Is recommended that the strategy defines an enum with the methods that + * can be called externally and validates this value. + * @return role The bytes32 role required to execute the call + */ + function getForwardToStrategyRole(uint8 strategyIndex, uint8 method) public view returns (bytes32 role) { + address strategy = address(_strategies[strategyIndex]); + return + bytes32(bytes20(strategy)) ^ + (bytes32(bytes1(method)) >> 160) ^ + (bytes32(bytes1(strategyIndex)) >> 168) ^ + FORWARD_TO_STRATEGY_ROLE; + } + + /// @inheritdoc MSVBase + // solhint-disable no-empty-blocks + function _checkForwardToStrategy( + uint8 strategyIndex, + uint8 method, + bytes memory + ) + internal + view + override + onlyRole(FORWARD_TO_STRATEGY_ROLE) + onlyRole(getForwardToStrategyRole(strategyIndex, method)) + {} } diff --git a/contracts/SwapStableInvestStrategy.sol b/contracts/SwapStableInvestStrategy.sol index 5712488..5d69722 100644 --- a/contracts/SwapStableInvestStrategy.sol +++ b/contracts/SwapStableInvestStrategy.sol @@ -2,7 +2,6 @@ pragma solidity ^0.8.0; import {IERC20Metadata} from "@openzeppelin/contracts/interfaces/IERC20Metadata.sol"; -import {IAccessControl} from "@openzeppelin/contracts/access/IAccessControl.sol"; import {SwapLibrary} from "@ensuro/swaplibrary/contracts/SwapLibrary.sol"; import {IInvestStrategy} from "./interfaces/IInvestStrategy.sol"; import {StorageSlot} from "@openzeppelin/contracts/utils/StorageSlot.sol"; @@ -33,8 +32,6 @@ contract SwapStableInvestStrategy is IInvestStrategy { event SwapConfigChanged(SwapLibrary.SwapConfig oldConfig, SwapLibrary.SwapConfig newConfig); - // From OZ v5 - error AccessControlUnauthorizedAccount(address account, bytes32 neededRole); error CanBeCalledOnlyThroughDelegateCall(); error CannotDisconnectWithAssets(); error NoExtraDataAllowed(); @@ -48,12 +45,6 @@ contract SwapStableInvestStrategy is IInvestStrategy { _; } - modifier onlyRole(bytes32 role) { - if (!IAccessControl(address(this)).hasRole(role, msg.sender)) - revert AccessControlUnauthorizedAccount(msg.sender, role); - _; - } - /** * @dev Constructor of the strategy * @@ -72,7 +63,7 @@ contract SwapStableInvestStrategy is IInvestStrategy { } function connect(bytes memory initData) external virtual override onlyDelegCall { - _setSwapConfigNoCheck(SwapLibrary.SwapConfig(SwapLibrary.SwapProtocol.undefined, 0, bytes("")), initData); + _setSwapConfig(SwapLibrary.SwapConfig(SwapLibrary.SwapProtocol.undefined, 0, bytes("")), initData); } function disconnect(bool force) external virtual override onlyDelegCall { @@ -135,14 +126,7 @@ contract SwapStableInvestStrategy is IInvestStrategy { swapConfig.exactInput(address(_asset), address(_investAsset), assets, _price); } - function _setSwapConfig(bytes memory newSwapConfigAsBytes) internal onlyRole(SWAP_ADMIN_ROLE) { - _setSwapConfigNoCheck(_getSwapConfig(address(this)), newSwapConfigAsBytes); - } - - function _setSwapConfigNoCheck( - SwapLibrary.SwapConfig memory oldSwapConfig, - bytes memory newSwapConfigAsBytes - ) internal { + function _setSwapConfig(SwapLibrary.SwapConfig memory oldSwapConfig, bytes memory newSwapConfigAsBytes) internal { SwapLibrary.SwapConfig memory swapConfig = abi.decode(newSwapConfigAsBytes, (SwapLibrary.SwapConfig)); swapConfig.validate(); if (abi.encode(swapConfig).length != newSwapConfigAsBytes.length) revert NoExtraDataAllowed(); @@ -153,7 +137,7 @@ contract SwapStableInvestStrategy is IInvestStrategy { function forwardEntryPoint(uint8 method, bytes memory params) external onlyDelegCall returns (bytes memory) { ForwardMethods checkedMethod = ForwardMethods(method); if (checkedMethod == ForwardMethods.setSwapConfig) { - _setSwapConfig(params); + _setSwapConfig(_getSwapConfig(address(this)), params); } // Should never reach to this revert, since method should be one of the enum values but leave it in case // we add new values in the enum and we forgot to add them here diff --git a/contracts/SingleStrategyERC4626.sol b/contracts/mock/SingleStrategyERC4626.sol similarity index 94% rename from contracts/SingleStrategyERC4626.sol rename to contracts/mock/SingleStrategyERC4626.sol index abd70c8..c92b001 100644 --- a/contracts/SingleStrategyERC4626.sol +++ b/contracts/mock/SingleStrategyERC4626.sol @@ -7,10 +7,10 @@ import {Address} from "@openzeppelin/contracts/utils/Address.sol"; import {StorageSlot} from "@openzeppelin/contracts/utils/StorageSlot.sol"; import {SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; import {Math} from "@openzeppelin/contracts/utils/math/Math.sol"; -import {PermissionedERC4626} from "./PermissionedERC4626.sol"; -import {IInvestStrategy} from "./interfaces/IInvestStrategy.sol"; -import {IExposeStorage} from "./interfaces/IExposeStorage.sol"; -import {InvestStrategyClient} from "./InvestStrategyClient.sol"; +import {PermissionedERC4626} from "../PermissionedERC4626.sol"; +import {IInvestStrategy} from "../interfaces/IInvestStrategy.sol"; +import {IExposeStorage} from "../interfaces/IExposeStorage.sol"; +import {InvestStrategyClient} from "../InvestStrategyClient.sol"; /** * @title SingleStrategyERC4626 @@ -22,6 +22,9 @@ import {InvestStrategyClient} from "./InvestStrategyClient.sol"; * The code of the IInvestStrategy is called using delegatecall, so it has full control over the assets and * storage of this contract, so you must be very careful the kind of IInvestStrategy is plugged. * + * WARNING: this contract isn't fully tested and has known errors (lack of access validation on forwardToStrategy), + * so this is not intended to be used in production. Is just for the purpose of testing strategies. + * * @custom:security-contact security@ensuro.co * @author Ensuro */ diff --git a/hardhat.config.js b/hardhat.config.js index 25760af..3b54ce2 100644 --- a/hardhat.config.js +++ b/hardhat.config.js @@ -15,6 +15,7 @@ module.exports = { enabled: true, runs: 200, }, + evmVersion: "cancun", }, }, networks: { diff --git a/package-lock.json b/package-lock.json index e05d4a1..39f9d16 100644 --- a/package-lock.json +++ b/package-lock.json @@ -25,7 +25,7 @@ "hardhat": "^2.22.17", "hardhat-contract-sizer": "^2.10.0", "prettier": "^3.4.2", - "solhint": "^5.0.3", + "solhint": "^5.0.4", "solhint-plugin-prettier": "0.1.0" } }, @@ -7194,11 +7194,12 @@ } }, "node_modules/solhint": { - "version": "5.0.3", + "version": "5.0.4", + "resolved": "https://registry.npmjs.org/solhint/-/solhint-5.0.4.tgz", + "integrity": "sha512-GzKBjJ8S2utRsEOCJXhY2H35gwHGmoQY2rQXcPGT4XdDlzwgGYz0Ovo9Xm7cmWYgSo121uF+5tiwrqQT2b+Rtw==", "dev": true, - "license": "MIT", "dependencies": { - "@solidity-parser/parser": "^0.18.0", + "@solidity-parser/parser": "^0.19.0", "ajv": "^6.12.6", "antlr4": "^4.13.1-patch-1", "ast-parents": "^0.0.1", @@ -7238,9 +7239,10 @@ } }, "node_modules/solhint/node_modules/@solidity-parser/parser": { - "version": "0.18.0", - "dev": true, - "license": "MIT" + "version": "0.19.0", + "resolved": "https://registry.npmjs.org/@solidity-parser/parser/-/parser-0.19.0.tgz", + "integrity": "sha512-RV16k/qIxW/wWc+mLzV3ARyKUaMUTBy9tOLMzFhtNSKYeTAanQ3a5MudJKf/8arIFnA2L27SNjarQKmFg0w/jA==", + "dev": true }, "node_modules/solhint/node_modules/ansi-styles": { "version": "4.3.0", diff --git a/package.json b/package.json index ce58fc4..8b590db 100644 --- a/package.json +++ b/package.json @@ -14,7 +14,7 @@ "hardhat": "^2.22.17", "hardhat-contract-sizer": "^2.10.0", "prettier": "^3.4.2", - "solhint": "^5.0.3", + "solhint": "^5.0.4", "solhint-plugin-prettier": "0.1.0" }, "dependencies": { diff --git a/test/test-compound-v3-vault.js b/test/test-compound-v3-vault.js index 6783b06..33aae9b 100644 --- a/test/test-compound-v3-vault.js +++ b/test/test-compound-v3-vault.js @@ -2,10 +2,18 @@ const { expect } = require("chai"); const { amountFunction, _W, getRole, grantRole, getTransactionEvent } = require("@ensuro/utils/js/utils"); const { initForkCurrency, setupChain } = require("@ensuro/utils/js/test-utils"); const { buildUniswapConfig } = require("@ensuro/swaplibrary/js/utils"); -const { encodeSwapConfig, encodeDummyStorage, tagit } = require("./utils"); +const { + encodeSwapConfig, + encodeDummyStorage, + tagit, + makeAllViewsPublic, + setupAMRole, + mergeFragments, +} = require("./utils"); const { anyUint } = require("@nomicfoundation/hardhat-chai-matchers/withArgs"); const hre = require("hardhat"); const helpers = require("@nomicfoundation/hardhat-network-helpers"); +const { deploy: ozUpgradesDeploy } = require("@openzeppelin/hardhat-upgrades/dist/utils"); const { ethers } = hre; const { MaxUint256, ZeroAddress } = hre.ethers; @@ -188,6 +196,13 @@ const variants = [ expect(action).to.be.revertedWithACError(contract, user, role), getSwapConfig: async (vault) => vault.getSwapConfig(), setSwapConfig: async (vault, swapConfig) => vault.setSwapConfig(swapConfig), + grantAccess: async (hre, operation, vault, admin, user) => + grantRole( + hre, + vault.connect(admin), + operation == CompoundV3StrategyMethods.harvestRewards ? "HARVEST_ROLE" : "SWAP_ADMIN_ROLE", + user + ), }, { name: "CompoundV3Strategy", @@ -201,10 +216,19 @@ const variants = [ }, }); const strategy = await CompoundV3InvestStrategy.deploy(ADDRESSES.cUSDCv3, ADDRESSES.REWARDS); - const SingleStrategyERC4626 = await ethers.getContractFactory("SingleStrategyERC4626"); + const MultiStrategyERC4626 = await ethers.getContractFactory("MultiStrategyERC4626"); const vault = await hre.upgrades.deployProxy( - SingleStrategyERC4626, - [NAME, SYMB, adminAddr, ADDRESSES.USDC, await ethers.resolveAddress(strategy), encodeSwapConfig(swapConfig)], + MultiStrategyERC4626, + [ + NAME, + SYMB, + adminAddr, + ADDRESSES.USDC, + [await ethers.resolveAddress(strategy)], + [encodeSwapConfig(swapConfig)], + [0], + [0], + ], { kind: "uups", unsafeAllow: ["delegatecall"], @@ -217,7 +241,7 @@ const variants = [ return { currency, - SingleStrategyERC4626, + MultiStrategyERC4626, CompoundV3InvestStrategy, swapConfig, vault, @@ -233,14 +257,24 @@ const variants = [ }, harvestRewards: async (vault, amount) => vault.forwardToStrategy( + 0, CompoundV3StrategyMethods.harvestRewards, ethers.AbiCoder.defaultAbiCoder().encode(["uint256"], [amount]) ), - accessControlCheck: async (action, user, role, contract) => - expect(action).to.be.revertedWithACError(contract, user, role), + accessControlCheck: async (action, user, role, contract) => { + if (role === "SWAP_ADMIN_ROLE") { + role = await contract.getForwardToStrategyRole(0, CompoundV3StrategyMethods.setSwapConfig); + } + return expect(action).to.be.revertedWithACError(contract, user, role); + }, getSwapConfig: async (vault, strategy) => strategy.getSwapConfig(vault), setSwapConfig: async (vault, swapConfig) => - vault.forwardToStrategy(CompoundV3StrategyMethods.setSwapConfig, encodeSwapConfig(swapConfig)), + vault.forwardToStrategy(0, CompoundV3StrategyMethods.setSwapConfig, encodeSwapConfig(swapConfig)), + grantAccess: async (hre, operation, vault, admin, user) => { + await grantRole(hre, vault.connect(admin), "FORWARD_TO_STRATEGY_ROLE", user); + const specificRole = await vault.getForwardToStrategyRole(0, operation); + await grantRole(hre, vault.connect(admin), specificRole, user); + }, }, { name: "AAVEV3Strategy", @@ -251,10 +285,19 @@ const variants = [ const { currency, adminAddr, swapConfig, admin, lp, lp2, guardian, anon, swapLibrary } = await setUp(); const AaveV3InvestStrategy = await ethers.getContractFactory("AaveV3InvestStrategy"); const strategy = await AaveV3InvestStrategy.deploy(ADDRESSES.USDC, ADDRESSES.AAVEv3); - const SingleStrategyERC4626 = await ethers.getContractFactory("SingleStrategyERC4626"); + const MultiStrategyERC4626 = await ethers.getContractFactory("MultiStrategyERC4626"); const vault = await hre.upgrades.deployProxy( - SingleStrategyERC4626, - [NAME, SYMB, adminAddr, ADDRESSES.USDC, await ethers.resolveAddress(strategy), ethers.toUtf8Bytes("")], + MultiStrategyERC4626, + [ + NAME, + SYMB, + adminAddr, + ADDRESSES.USDC, + [await ethers.resolveAddress(strategy)], + [ethers.toUtf8Bytes("")], + [0], + [0], + ], { kind: "uups", unsafeAllow: ["delegatecall"], @@ -267,7 +310,7 @@ const variants = [ return { currency, - SingleStrategyERC4626, + MultiStrategyERC4626, AaveV3InvestStrategy, swapConfig, vault, @@ -287,6 +330,124 @@ const variants = [ getSwapConfig: null, setSwapConfig: null, }, + { + name: "CompoundV3Strategy+AccessManaged", + tagit: tagit, + cToken: ADDRESSES.cUSDCv3, + supplyToken: ADDRESSES.USDC, + fixture: async () => { + const { currency, adminAddr, swapConfig, admin, lp, lp2, guardian, anon, swapLibrary } = await setUp(); + const CompoundV3InvestStrategy = await ethers.getContractFactory("CompoundV3InvestStrategy", { + libraries: { + SwapLibrary: await ethers.resolveAddress(swapLibrary), + }, + }); + const strategy = await CompoundV3InvestStrategy.deploy(ADDRESSES.cUSDCv3, ADDRESSES.REWARDS); + const AccessManager = await ethers.getContractFactory("AccessManager"); + const AccessManagedMSV = await ethers.getContractFactory("AccessManagedMSV"); + const AccessManagedProxy = await ethers.getContractFactory("AccessManagedProxy"); + + const acMgr = await AccessManager.deploy(admin); + + const roles = { + LP_ROLE: 1, + LOM_ADMIN: 2, + REBALANCER_ROLE: 3, + STRATEGY_ADMIN_ROLE: 4, + QUEUE_ADMIN_ROLE: 5, + FORWARD_TO_STRATEGY_ROLE: 6, + }; + + const vault = await hre.upgrades.deployProxy( + AccessManagedMSV, + [NAME, SYMB, ADDRESSES.USDC, [await ethers.resolveAddress(strategy)], [encodeSwapConfig(swapConfig)], [0], [0]], + { + kind: "uups", + unsafeAllow: ["delegatecall"], + proxyFactory: AccessManagedProxy, + deployFunction: async (hre, opts, factory, ...args) => ozUpgradesDeploy(hre, opts, factory, ...args, acMgr), + } + ); + await currency.connect(lp).approve(vault, MaxUint256); + await currency.connect(lp2).approve(vault, MaxUint256); + + await makeAllViewsPublic(acMgr.connect(admin), vault); + + await setupAMRole(acMgr.connect(admin), vault, roles, "LP_ROLE", [ + "withdraw", + "deposit", + "mint", + "redeem", + "transfer", + ]); + await setupAMRole(acMgr.connect(admin), vault, roles, "STRATEGY_ADMIN_ROLE", [ + "addStrategy", + "replaceStrategy", + "removeStrategy", + ]); + + await setupAMRole(acMgr.connect(admin), vault, roles, "QUEUE_ADMIN_ROLE", [ + "changeDepositQueue", + "changeWithdrawQueue", + ]); + + await setupAMRole(acMgr.connect(admin), vault, roles, "REBALANCER_ROLE", ["rebalance"]); + + await setupAMRole(acMgr.connect(admin), vault, roles, "FORWARD_TO_STRATEGY_ROLE", ["forwardToStrategy"]); + + await acMgr.connect(admin).grantRole(roles.LP_ROLE, lp, 0); + await acMgr.connect(admin).grantRole(roles.LP_ROLE, lp2, 0); + + const combinedABI = mergeFragments(AccessManagedMSV.interface.fragments, AccessManagedProxy.interface.fragments); + const deploymentTransaction = vault.deploymentTransaction(); + const vaultWithCombinedABI = await ethers.getContractAt(combinedABI, await ethers.resolveAddress(vault)); + vaultWithCombinedABI.deploymentTransaction = () => deploymentTransaction; + + return { + currency, + AccessManagedProxy, + CompoundV3InvestStrategy, + swapConfig, + vault: vaultWithCombinedABI, + strategy, + adminAddr, + lp, + lp2, + anon, + guardian, + admin, + swapLibrary, + acMgr, + roles, + }; + }, + harvestRewards: async (vault, amount) => + vault.forwardToStrategy( + 0, + CompoundV3StrategyMethods.harvestRewards, + ethers.AbiCoder.defaultAbiCoder().encode(["uint256"], [amount]) + ), + accessControlCheck: async (action, user, _, contract) => expect(action).to.be.revertedWithAMError(contract, user), + getSwapConfig: async (vault, strategy) => strategy.getSwapConfig(vault), + setSwapConfig: async (vault, swapConfig) => + vault.forwardToStrategy(0, CompoundV3StrategyMethods.setSwapConfig, encodeSwapConfig(swapConfig)), + grantAccess: async (hre, operation, vault, admin, user, acMgr) => { + const roles = { + LP_ROLE: 1, + LOM_ADMIN: 2, + REBALANCER_ROLE: 3, + STRATEGY_ADMIN_ROLE: 4, + QUEUE_ADMIN_ROLE: 5, + FORWARD_TO_STRATEGY_ROLE: 6, + }; + + await acMgr.connect(admin).grantRole(roles.FORWARD_TO_STRATEGY_ROLE, user, 0); + const specificSelector = await vault.getForwardToStrategySelector(0, operation); + await acMgr.connect(admin).setTargetFunctionRole(vault, [specificSelector], specificSelector); + await acMgr.connect(admin).grantRole(specificSelector, user, 0); + }, + accessManaged: true, + }, { name: "SwapStableAAVEV3Strategy", tagit: tagit, @@ -305,10 +466,19 @@ const variants = [ ADDRESSES.AAVEv3 ); - const SingleStrategyERC4626 = await ethers.getContractFactory("SingleStrategyERC4626"); + const MultiStrategyERC4626 = await ethers.getContractFactory("MultiStrategyERC4626"); const vault = await hre.upgrades.deployProxy( - SingleStrategyERC4626, - [NAME, SYMB, adminAddr, ADDRESSES.USDC, await ethers.resolveAddress(strategy), encodeSwapConfig(swapConfig)], + MultiStrategyERC4626, + [ + NAME, + SYMB, + adminAddr, + ADDRESSES.USDC, + [await ethers.resolveAddress(strategy)], + [encodeSwapConfig(swapConfig)], + [0], + [0], + ], { kind: "uups", unsafeAllow: ["delegatecall"], @@ -322,7 +492,7 @@ const variants = [ return { currency, - SingleStrategyERC4626, + MultiStrategyERC4626, SwapStableAaveV3InvestStrategy, swapConfig, vault, @@ -341,7 +511,12 @@ const variants = [ expect(action).to.be.revertedWithACError(contract, user, role), getSwapConfig: async (vault, strategy) => strategy.getSwapConfig(vault), setSwapConfig: async (vault, swapConfig) => - vault.forwardToStrategy(SwapStableAaveV3InvestStrategyMethods.setSwapConfig, encodeSwapConfig(swapConfig)), + vault.forwardToStrategy(0, SwapStableAaveV3InvestStrategyMethods.setSwapConfig, encodeSwapConfig(swapConfig)), + grantAccess: async (hre, operation, vault, admin, user) => { + await grantRole(hre, vault.connect(admin), "FORWARD_TO_STRATEGY_ROLE", user); + const specificRole = await vault.getForwardToStrategyRole(0, operation); + await grantRole(hre, vault.connect(admin), specificRole, user); + }, }, ]; @@ -358,8 +533,10 @@ variants.forEach((variant) => { expect(await vault.symbol()).to.equal(SYMB); expect(await vault.asset()).to.equal(currency); expect(await vault.totalAssets()).to.equal(0); - expect(await vault.hasRole(getRole("DEFAULT_ADMIN_ROLE"), admin)).to.equal(true); - expect(await vault.hasRole(getRole("DEFAULT_ADMIN_ROLE"), anon)).to.equal(false); + if (!variant.accessManaged) { + expect(await vault.hasRole(getRole("DEFAULT_ADMIN_ROLE"), admin)).to.equal(true); + expect(await vault.hasRole(getRole("DEFAULT_ADMIN_ROLE"), anon)).to.equal(false); + } }); variant.tagit("Checks vault constructs with disabled initializer [CompoundV3ERC4626]", async () => { @@ -372,25 +549,10 @@ variants.forEach((variant) => { ); }); - variant.tagit("Checks vault constructs with disabled initializer [!CompoundV3ERC4626]", async () => { - const { SingleStrategyERC4626, adminAddr, swapConfig, strategy } = await helpers.loadFixture(variant.fixture); - const newVault = await SingleStrategyERC4626.deploy(); - await expect(newVault.deploymentTransaction()).to.emit(newVault, "Initialized"); - await expect( - newVault.initialize( - "foo", - "bar", - adminAddr, - ADDRESSES.USDC, - await ethers.resolveAddress(strategy), - encodeSwapConfig(swapConfig) - ) - ).to.be.revertedWithCustomError(SingleStrategyERC4626, "InvalidInitialization"); - }); - variant.tagit("Checks reverts if extraData is sent on initialization [!CompoundV3ERC4626]", async () => { + if (variant.accessManaged) return; // tagit doens't support double neg const { - SingleStrategyERC4626, + MultiStrategyERC4626, adminAddr, swapConfig, strategy, @@ -403,8 +565,8 @@ variants.forEach((variant) => { variant.name !== "AAVEV3Strategy" ? encodeSwapConfig(swapConfig) + "f".repeat(64) : `0x${"f".repeat(64)}`; await expect( hre.upgrades.deployProxy( - SingleStrategyERC4626, - [NAME, SYMB, adminAddr, ADDRESSES.USDC, await ethers.resolveAddress(strategy), initData], + MultiStrategyERC4626, + [NAME, SYMB, adminAddr, ADDRESSES.USDC, [await ethers.resolveAddress(strategy)], [initData], [0], [0]], { kind: "uups", unsafeAllow: ["delegatecall"], @@ -415,15 +577,22 @@ variants.forEach((variant) => { variant.tagit("Checks entering the vault is permissioned, exit isn't [!SwapStableAAVEV3Strategy]", async () => { const { currency, vault, anon, lp } = await helpers.loadFixture(variant.fixture); - await expect(vault.connect(anon).deposit(_A(100), anon)).to.be.revertedWithCustomError( - vault, - "ERC4626ExceededMaxDeposit" - ); - await expect(vault.connect(anon).mint(_A(100), anon)).to.be.revertedWithCustomError( - vault, - "ERC4626ExceededMaxMint" - ); + if (variant.accessManaged) { + await expect(vault.connect(anon).deposit(_A(100), anon)).to.be.revertedWithAMError(vault, anon); + + await expect(vault.connect(anon).mint(_A(100), anon)).to.be.revertedWithAMError(vault, anon); + } else { + await expect(vault.connect(anon).deposit(_A(100), anon)).to.be.revertedWithCustomError( + vault, + "ERC4626ExceededMaxDeposit" + ); + + await expect(vault.connect(anon).mint(_A(100), anon)).to.be.revertedWithCustomError( + vault, + "ERC4626ExceededMaxMint" + ); + } await expect(vault.connect(lp).deposit(_A(100), lp)) .to.emit(vault, "Deposit") @@ -437,6 +606,8 @@ variants.forEach((variant) => { expect(await currency.balanceOf(vault)).to.equal(0); expect(await vault.totalAssets()).to.closeTo(_A(100), MCENT); + if (variant.accessManaged) return; // In access managed both enter and exit require authorization + await expect(vault.connect(anon).withdraw(_A(100), anon, anon)).to.be.revertedWithCustomError( vault, "ERC4626ExceededMaxWithdraw" @@ -488,20 +659,55 @@ variants.forEach((variant) => { }); variant.tagit("Checks rewards can be harvested [!AAVEV3Strategy] [!SwapStableAAVEV3Strategy]", async () => { - const { currency, vault, admin, anon, lp, lp2, strategy } = await helpers.loadFixture(variant.fixture); + const { currency, vault, admin, anon, lp, lp2, strategy, acMgr, roles } = await helpers.loadFixture( + variant.fixture + ); await expect(vault.connect(lp).mint(_A(1000), lp)).not.to.be.reverted; await expect(vault.connect(lp2).mint(_A(2000), lp2)).not.to.be.reverted; expect(await vault.totalAssets()).to.be.closeTo(_A(3000), MCENT); - await variant.accessControlCheck( - variant.harvestRewards(vault.connect(anon), _A(100)), - anon, - "HARVEST_ROLE", - strategy - ); - - await grantRole(hre, vault.connect(admin), "HARVEST_ROLE", anon); + if (variant.name === "CompoundV3ERC4626") { + await variant.accessControlCheck( + variant.harvestRewards(vault.connect(anon), _A(100)), + anon, + "HARVEST_ROLE", + vault + ); + await grantRole(hre, vault.connect(admin), "HARVEST_ROLE", anon); + } else if (variant.accessManaged) { + // Using AccessManagedMSV + await variant.accessControlCheck( + variant.harvestRewards(vault.connect(anon), _A(100)), + anon, + "FORWARD_TO_STRATEGY_ROLE", + vault + ); + await acMgr.connect(admin).grantRole(roles.FORWARD_TO_STRATEGY_ROLE, anon, 0); + // Still fails because other role is missing + const specificSelector = await vault.getForwardToStrategySelector(0, CompoundV3StrategyMethods.harvestRewards); + await variant.accessControlCheck(variant.harvestRewards(vault.connect(anon), _A(100)), anon, null, vault); + await acMgr.connect(admin).setTargetFunctionRole(vault, [specificSelector], specificSelector); + await acMgr.connect(admin).grantRole(specificSelector, anon, 0); + } else { + // Using MultiStrategyERC4626 + await variant.accessControlCheck( + variant.harvestRewards(vault.connect(anon), _A(100)), + anon, + "FORWARD_TO_STRATEGY_ROLE", + vault + ); + await grantRole(hre, vault.connect(admin), "FORWARD_TO_STRATEGY_ROLE", anon); + // Still fails because other role is missing + const specificRole = await vault.getForwardToStrategyRole(0, CompoundV3StrategyMethods.harvestRewards); + await variant.accessControlCheck( + variant.harvestRewards(vault.connect(anon), _A(100)), + anon, + specificRole, + vault + ); + await grantRole(hre, vault.connect(admin), specificRole, anon); + } await expect(variant.harvestRewards(vault.connect(anon), _A(100))).to.be.revertedWith("AS"); @@ -533,14 +739,16 @@ variants.forEach((variant) => { }); variant.tagit("Checks only authorized user can change swap config [!AAVEV3Strategy]", async () => { - const { currency, vault, admin, anon, lp, swapConfig, strategy, swapLibrary } = await helpers.loadFixture( + const { currency, vault, admin, anon, lp, swapConfig, strategy, swapLibrary, acMgr } = await helpers.loadFixture( variant.fixture ); expect(await variant.getSwapConfig(vault, strategy)).to.deep.equal(swapConfig); await expect(vault.connect(lp).mint(_A(3000), lp)).not.to.be.reverted; - await grantRole(hre, vault.connect(admin), "HARVEST_ROLE", anon); + if (variant.name !== "SwapStableAAVEV3Strategy") { + await variant.grantAccess(hre, CompoundV3StrategyMethods.harvestRewards, vault, admin, anon, acMgr); + } await helpers.time.increase(MONTH); const assets = await vault.totalAssets(); @@ -553,14 +761,35 @@ variants.forEach((variant) => { ); } - await variant.accessControlCheck( - variant.setSwapConfig(vault.connect(anon), swapConfig), - anon, - "SWAP_ADMIN_ROLE", - strategy - ); - - await grantRole(hre, vault.connect(admin), "SWAP_ADMIN_ROLE", anon); + if (variant.name !== "SwapStableAAVEV3Strategy") { + await variant.accessControlCheck( + variant.setSwapConfig(vault.connect(anon), swapConfig), + anon, + "SWAP_ADMIN_ROLE", + vault + ); + await variant.grantAccess(hre, CompoundV3StrategyMethods.setSwapConfig, vault, admin, anon, acMgr); + } else { + await variant.accessControlCheck( + variant.setSwapConfig(vault.connect(anon), swapConfig), + anon, + "FORWARD_TO_STRATEGY_ROLE", + vault + ); + await grantRole(hre, vault.connect(admin), "FORWARD_TO_STRATEGY_ROLE", anon); + // Still fails because other role is missing + const specificRole = await vault.getForwardToStrategyRole( + 0, + SwapStableAaveV3InvestStrategyMethods.setSwapConfig + ); + await variant.accessControlCheck( + variant.setSwapConfig(vault.connect(anon), swapConfig), + anon, + specificRole, + vault + ); + await grantRole(hre, vault.connect(admin), specificRole, anon); + } // Check validates new config await expect( @@ -731,9 +960,9 @@ variants.forEach((variant) => { await helpers.loadFixture(variant.fixture); // Just to increase coverage, I check forwardEntryPoint reverts with wrong input - await expect(vault.forwardToStrategy(123, ethers.toUtf8Bytes(""))).to.be.reverted; + await expect(vault.forwardToStrategy(0, 123, ethers.toUtf8Bytes(""))).to.be.reverted; - expect(await vault.strategy()).to.equal(strategy); + expect((await vault.strategies())[0]).to.equal(strategy); await expect(vault.connect(lp).mint(_A(3000), lp)).not.to.be.reverted; expect(await vault.totalAssets()).to.closeTo(_A(3000), MCENT); @@ -741,11 +970,12 @@ variants.forEach((variant) => { expect(await vault.maxWithdraw(lp)).to.closeTo(_A(3000), MCENT); await expect( - vault.connect(anon).setStrategy(ZeroAddress, encodeSwapConfig(swapConfig), false) - ).to.be.revertedWithACError(strategy, anon, "SET_STRATEGY_ROLE"); - await grantRole(hre, vault.connect(admin), "SET_STRATEGY_ROLE", anon); + vault.connect(anon).replaceStrategy(0, ZeroAddress, encodeSwapConfig(swapConfig), false) + ).to.be.revertedWithACError(vault, anon, "STRATEGY_ADMIN_ROLE"); + await grantRole(hre, vault.connect(admin), "STRATEGY_ADMIN_ROLE", anon); - await expect(vault.connect(anon).setStrategy(ZeroAddress, encodeSwapConfig(swapConfig), false)).to.be.reverted; + await expect(vault.connect(anon).replaceStrategy(0, ZeroAddress, encodeSwapConfig(swapConfig), false)).to.be + .reverted; // If I pause withdraw, it can't withdraw and setStrategy fails await helpers.impersonateAccount(ADDRESSES.cUSDCv3_GUARDIAN); @@ -759,7 +989,7 @@ variants.forEach((variant) => { expect(await vault.maxWithdraw(lp)).to.equal(0); await expect( - vault.connect(anon).setStrategy(strategy, encodeSwapConfig(swapConfig), false) + vault.connect(anon).replaceStrategy(0, strategy, encodeSwapConfig(swapConfig), false) ).to.be.revertedWithCustomError(cUSDCv3, "Paused"); const DummyInvestStrategy = await ethers.getContractFactory("DummyInvestStrategy"); @@ -767,7 +997,7 @@ variants.forEach((variant) => { const dummyStrategy = await DummyInvestStrategy.deploy(ADDRESSES.USDC); // But if I force, it works - await expect(vault.connect(anon).setStrategy(otherStrategy, encodeSwapConfig(swapConfig), true)) + await expect(vault.connect(anon).replaceStrategy(0, otherStrategy, encodeSwapConfig(swapConfig), true)) .to.emit(vault, "StrategyChanged") .withArgs(strategy, otherStrategy) .to.emit(vault, "WithdrawFailed") @@ -776,7 +1006,7 @@ variants.forEach((variant) => { expect(await vault.totalAssets()).to.closeTo(_A(3000), CENT); // Setting a dummyStrategy returns totalAssets == 0 because can't see the assets in Compound - let tx = await vault.connect(anon).setStrategy(dummyStrategy, encodeDummyStorage({}), true); + let tx = await vault.connect(anon).replaceStrategy(0, dummyStrategy, encodeDummyStorage({}), true); await expect(tx) .to.emit(vault, "StrategyChanged") .withArgs(otherStrategy, dummyStrategy) @@ -791,7 +1021,7 @@ variants.forEach((variant) => { expect(await dummyStrategy.getFail(vault)).to.deep.equal([false, false, false, false]); // Setting again `strategy` works fine - await expect(vault.connect(anon).setStrategy(strategy, encodeSwapConfig(swapConfig), false)) + await expect(vault.connect(anon).replaceStrategy(0, strategy, encodeSwapConfig(swapConfig), false)) .to.emit(vault, "StrategyChanged") .withArgs(dummyStrategy, strategy); expect(await vault.totalAssets()).to.closeTo(_A(3000), CENT); @@ -801,7 +1031,7 @@ variants.forEach((variant) => { await cUSDCv3.connect(compGuardian).pause(false, false, false, false, false); // Setting a dummyStrategy sends the assets to the vault - tx = await vault.connect(anon).setStrategy(dummyStrategy, encodeDummyStorage({}), false); + tx = await vault.connect(anon).replaceStrategy(0, dummyStrategy, encodeDummyStorage({}), false); await expect(tx) .to.emit(vault, "StrategyChanged") .withArgs(strategy, dummyStrategy) @@ -830,9 +1060,9 @@ variants.forEach((variant) => { } = await helpers.loadFixture(variant.fixture); // Just to increase coverage, I check forwardEntryPoint reverts - await expect(vault.forwardToStrategy(123, ethers.toUtf8Bytes(""))).to.be.reverted; + await expect(vault.forwardToStrategy(0, 123, ethers.toUtf8Bytes(""))).to.be.reverted; - expect(await vault.strategy()).to.equal(strategy); + expect((await vault.strategies())[0]).to.equal(strategy); await expect(vault.connect(lp).mint(_A(3000), lp)).not.to.be.reverted; expect(await vault.totalAssets()).to.closeTo(_A(3000), _A(5)); @@ -840,11 +1070,11 @@ variants.forEach((variant) => { expect(await vault.maxWithdraw(lp)).to.closeTo(_A(3000), _A(5)); await expect( - vault.connect(anon).setStrategy(ZeroAddress, ethers.toUtf8Bytes(""), false) - ).to.be.revertedWithACError(strategy, anon, "SET_STRATEGY_ROLE"); - await grantRole(hre, vault.connect(admin), "SET_STRATEGY_ROLE", anon); + vault.connect(anon).replaceStrategy(0, ZeroAddress, ethers.toUtf8Bytes(""), false) + ).to.be.revertedWithACError(vault, anon, "STRATEGY_ADMIN_ROLE"); + await grantRole(hre, vault.connect(admin), "STRATEGY_ADMIN_ROLE", anon); - await expect(vault.connect(anon).setStrategy(ZeroAddress, ethers.toUtf8Bytes(""), false)).to.be.reverted; + await expect(vault.connect(anon).replaceStrategy(0, ZeroAddress, ethers.toUtf8Bytes(""), false)).to.be.reverted; // If I pause withdraw, it can't withdraw and setStrategy fails await helpers.impersonateAccount(ADDRESSES.AAVEPoolAdmin); @@ -858,7 +1088,7 @@ variants.forEach((variant) => { expect(await vault.maxRedeem(lp)).to.equal(0); expect(await vault.maxWithdraw(lp)).to.equal(0); - await expect(vault.connect(anon).setStrategy(strategy, ethers.toUtf8Bytes(""), false)).to.be.revertedWith( + await expect(vault.connect(anon).replaceStrategy(0, strategy, ethers.toUtf8Bytes(""), false)).to.be.revertedWith( "29" // RESERVE_PAUSED ); @@ -881,7 +1111,7 @@ variants.forEach((variant) => { // But if I force, it works - await expect(vault.connect(anon).setStrategy(otherStrategy, initConfig, true)) + await expect(vault.connect(anon).replaceStrategy(0, otherStrategy, initConfig, true)) .to.emit(vault, "StrategyChanged") .withArgs(strategy, otherStrategy) .to.emit(vault, "WithdrawFailed"); @@ -889,7 +1119,7 @@ variants.forEach((variant) => { expect(await vault.totalAssets()).to.closeTo(_A(3000), _A(5)); // Setting a dummyStrategy returns totalAssets == 0 because can't see the assets in Compound - let tx = await vault.connect(anon).setStrategy(dummyStrategy, encodeDummyStorage({}), true); + let tx = await vault.connect(anon).replaceStrategy(0, dummyStrategy, encodeDummyStorage({}), true); await expect(tx) .to.emit(vault, "StrategyChanged") .withArgs(otherStrategy, dummyStrategy) @@ -903,7 +1133,7 @@ variants.forEach((variant) => { expect(await dummyStrategy.getFail(vault)).to.deep.equal([false, false, false, false]); // Setting again `strategy` works fine - await expect(vault.connect(anon).setStrategy(strategy, initConfig, false)) + await expect(vault.connect(anon).replaceStrategy(0, strategy, initConfig, false)) .to.emit(vault, "StrategyChanged") .withArgs(dummyStrategy, strategy); expect(await vault.totalAssets()).to.closeTo(_A(3000), _A(5)); @@ -912,7 +1142,7 @@ variants.forEach((variant) => { await aaveConfig.connect(aaveAdmin).setReservePause(variant.supplyToken, false); // Setting a dummyStrategy sends the assets to the vault - tx = await vault.connect(anon).setStrategy(dummyStrategy, encodeDummyStorage({}), false); + tx = await vault.connect(anon).replaceStrategy(0, dummyStrategy, encodeDummyStorage({}), false); await expect(tx) .to.emit(vault, "StrategyChanged") .withArgs(strategy, dummyStrategy) diff --git a/test/test-integration-fork.js b/test/test-integration-fork.js index 0f63b7b..083b674 100644 --- a/test/test-integration-fork.js +++ b/test/test-integration-fork.js @@ -152,6 +152,10 @@ describe("MultiStrategy Integration fork tests", function () { // Take the price from the oracle (8 decimals) and add 10 more to convert it to wad const compUSD = (await COMPPrice.latestRoundData())[1] * 10n ** 10n; + await vault.connect(admin).grantRole(getRole("FORWARD_TO_STRATEGY_ROLE"), admin); + const specificRole = await vault.getForwardToStrategyRole(1, CompoundV3StrategyMethods.harvestRewards); + await vault.connect(admin).grantRole(specificRole, admin); + await vault .connect(admin) .forwardToStrategy( diff --git a/test/test-multi-strategy-erc4626.js b/test/test-multi-strategy-erc4626.js index 230c6bd..8e70ac9 100644 --- a/test/test-multi-strategy-erc4626.js +++ b/test/test-multi-strategy-erc4626.js @@ -1,7 +1,7 @@ const { expect } = require("chai"); const { _A, getRole } = require("@ensuro/utils/js/utils"); const { initCurrency } = require("@ensuro/utils/js/test-utils"); -const { encodeDummyStorage, dummyStorage, tagit, makeAllViewsPublic, mergeFragments } = require("./utils"); +const { encodeDummyStorage, dummyStorage, tagit, makeAllViewsPublic, mergeFragments, setupAMRole } = require("./utils"); const hre = require("hardhat"); const helpers = require("@nomicfoundation/hardhat-network-helpers"); @@ -90,66 +90,16 @@ const variants = [ await vault.connect(admin).grantRole(getRole(role), user); } - return { - deployVault, - grantRole, - ...ret, - }; - }, - }, - { - name: "LOM-MultiStrategyERC4626", - tagit: tagit, - accessError: "revertedWithACError", - fixture: async () => { - const ret = await setUp(); - const { strategies, MultiStrategyERC4626, adminAddr, currency, admin } = ret; - const LimitOutflowModifier = await ethers.getContractFactory("LimitOutflowModifier"); - const ERC1967Proxy = await ethers.getContractFactory("ERC1967Proxy"); - const msv = await MultiStrategyERC4626.deploy(); - const lom = await LimitOutflowModifier.deploy(msv); - - async function deployVault(strategies_, initStrategyDatas, depositQueue, withdrawQueue) { - if (strategies_ === undefined) { - strategies_ = strategies; - } else if (typeof strategies_ == "number") { - strategies_ = strategies.slice(0, strategies_); - } - if (initStrategyDatas === undefined) { - initStrategyDatas = strategies_.map(() => encodeDummyStorage({})); - } - if (depositQueue === undefined) { - depositQueue = strategies_.map((_, i) => i); - } - if (withdrawQueue === undefined) { - withdrawQueue = strategies_.map((_, i) => i); - } - const initializeData = msv.interface.encodeFunctionData("initialize", [ - NAME, - SYMB, - adminAddr, - await ethers.resolveAddress(currency), - await Promise.all(strategies_.map(ethers.resolveAddress)), - initStrategyDatas, - depositQueue, - withdrawQueue, - ]); - const proxy = await ERC1967Proxy.deploy(lom, initializeData); - const deploymentTransaction = proxy.deploymentTransaction(); - const vault = msv.attach(await ethers.resolveAddress(proxy)); - vault.deploymentTransaction = () => deploymentTransaction; - const vaultAsLOM = LimitOutflowModifier.attach(proxy); - await vaultAsLOM.LOM__setLimit(3600 * 24, _A(1)); - return vault; - } - - async function grantRole(vault, role, user) { - await vault.connect(admin).grantRole(getRole(role), user); + async function grantForwardToStrategy(vault, strategyIndex, method, user) { + await vault.connect(admin).grantRole(getRole("FORWARD_TO_STRATEGY_ROLE"), user); + const specificRole = await vault.getForwardToStrategyRole(strategyIndex, method); + await vault.connect(admin).grantRole(specificRole, user); } return { deployVault, grantRole, + grantForwardToStrategy, ...ret, }; }, @@ -179,6 +129,7 @@ const variants = [ REBALANCER_ROLE: 3, STRATEGY_ADMIN_ROLE: 4, QUEUE_ADMIN_ROLE: 5, + FORWARD_TO_STRATEGY_ROLE: 6, }; async function deployVault(strategies_, initStrategyDatas, depositQueue, withdrawQueue) { @@ -211,46 +162,27 @@ const variants = [ vault.deploymentTransaction = () => deploymentTransaction; await makeAllViewsPublic(acMgr.connect(admin), vault); - // Also make forwardToStrategy public - await acMgr - .connect(admin) - .setTargetFunctionRole( - vault, - [vault.interface.getFunction("forwardToStrategy").selector], - await acMgr.PUBLIC_ROLE() - ); - - // Setup LP role - await acMgr.connect(admin).labelRole(roles.LP_ROLE, "LP_ROLE"); - for (const method of ["withdraw", "deposit", "mint", "redeem", "transfer"]) { - await acMgr - .connect(admin) - .setTargetFunctionRole(vault, [vault.interface.getFunction(method).selector], roles.LP_ROLE); - } + await setupAMRole(acMgr.connect(admin), vault, roles, "LP_ROLE", [ + "withdraw", + "deposit", + "mint", + "redeem", + "transfer", + ]); + await setupAMRole(acMgr.connect(admin), vault, roles, "STRATEGY_ADMIN_ROLE", [ + "addStrategy", + "replaceStrategy", + "removeStrategy", + ]); - // Setup STRATEGY_ADMIN_ROLE role - await acMgr.connect(admin).labelRole(roles.STRATEGY_ADMIN_ROLE, "STRATEGY_ADMIN_ROLE"); - for (const method of ["addStrategy", "replaceStrategy", "removeStrategy"]) { - await acMgr - .connect(admin) - .setTargetFunctionRole(vault, [vault.interface.getFunction(method).selector], roles.STRATEGY_ADMIN_ROLE); - } + await setupAMRole(acMgr.connect(admin), vault, roles, "QUEUE_ADMIN_ROLE", [ + "changeDepositQueue", + "changeWithdrawQueue", + ]); - // Setup QUEUE_ADMIN_ROLE role - await acMgr.connect(admin).labelRole(roles.STRATEGY_ADMIN_ROLE, "QUEUE_ADMIN_ROLE"); - for (const method of ["changeDepositQueue", "changeWithdrawQueue"]) { - await acMgr - .connect(admin) - .setTargetFunctionRole(vault, [vault.interface.getFunction(method).selector], roles.QUEUE_ADMIN_ROLE); - } + await setupAMRole(acMgr.connect(admin), vault, roles, "REBALANCER_ROLE", ["rebalance"]); - // Setup REBALANCER_ROLE role - await acMgr.connect(admin).labelRole(roles.REBALANCER_ROLE, "REBALANCER_ROLE"); - for (const method of ["rebalance"]) { - await acMgr - .connect(admin) - .setTargetFunctionRole(vault, [vault.interface.getFunction(method).selector], roles.REBALANCER_ROLE); - } + await setupAMRole(acMgr.connect(admin), vault, roles, "FORWARD_TO_STRATEGY_ROLE", ["forwardToStrategy"]); await vault.connect(admin).LOM__setLimit(3600 * 24, _A(1)); @@ -258,14 +190,24 @@ const variants = [ } async function grantRole(_, role, user) { - const roleId = roles[role]; + const roleId = role.startsWith("0x") ? role : roles[role]; if (roleId === undefined) throw new Error(`Unknown role ${role}`); await acMgr.connect(admin).grantRole(roleId, user, 0); } + async function grantForwardToStrategy(vault, strategyIndex, method, user) { + await acMgr.connect(admin).grantRole(roles.FORWARD_TO_STRATEGY_ROLE, user, 0); + const specificSelector = await vault.getForwardToStrategySelector(strategyIndex, method); + await acMgr.connect(admin).setTargetFunctionRole(vault, [specificSelector], specificSelector); + await acMgr.connect(admin).grantRole(specificSelector, user, 0); + } + return { deployVault, grantRole, + grantForwardToStrategy, + acMgr, + AccessManagedMSV, ...ret, }; }, @@ -299,6 +241,41 @@ async function invariantChecks(vault) { variants.forEach((variant) => { describe(`${variant.name} contract tests`, function () { + variant.tagit("Checks vault constructs with disabled initializer [MultiStrategyERC4626]", async () => { + const { MultiStrategyERC4626, adminAddr, currency, strategies } = await helpers.loadFixture(variant.fixture); + const newVault = await MultiStrategyERC4626.deploy(); + await expect(newVault.deploymentTransaction()).to.emit(newVault, "Initialized"); + await expect( + newVault.initialize( + "foo", + "bar", + adminAddr, + await ethers.resolveAddress(currency), + [strategies[0]], + [encodeDummyStorage({})], + [0], + [0] + ) + ).to.be.revertedWithCustomError(MultiStrategyERC4626, "InvalidInitialization"); + }); + + variant.tagit("Checks vault constructs with disabled initializer [!MultiStrategyERC4626]", async () => { + const { AccessManagedMSV, currency, strategies } = await helpers.loadFixture(variant.fixture); + const newVault = await AccessManagedMSV.deploy(); + await expect(newVault.deploymentTransaction()).to.emit(newVault, "Initialized"); + await expect( + newVault.initialize( + "foo", + "bar", + await ethers.resolveAddress(currency), + [strategies[0]], + [encodeDummyStorage({})], + [0], + [0] + ) + ).to.be.revertedWithCustomError(AccessManagedMSV, "InvalidInitialization"); + }); + variant.tagit("Initializes the vault correctly", async () => { const { deployVault, currency, strategies } = await helpers.loadFixture(variant.fixture); const vault = await deployVault(1); @@ -329,9 +306,126 @@ variants.forEach((variant) => { } }); - variant.tagit("It sets and reads the right value from strategy storage", async () => { - const { deployVault, strategies } = await helpers.loadFixture(variant.fixture); + variant.tagit("It checks calls to forwardToStrategy require permission [MultiStrategyERC4626]", async () => { + const { deployVault, strategies, anon, grantRole } = await helpers.loadFixture(variant.fixture); + const vault = await deployVault(3); + await expect(vault.connect(anon).forwardToStrategy(4, 0, encodeDummyStorage({}))).to.be.revertedWithACError( + vault, + anon, + "FORWARD_TO_STRATEGY_ROLE" + ); + + await grantRole(vault, "FORWARD_TO_STRATEGY_ROLE", anon); + let specificRole = await vault.getForwardToStrategyRole(4, 0); + await expect(vault.connect(anon).forwardToStrategy(4, 0, encodeDummyStorage({}))).to.be.revertedWithACError( + vault, + anon, + specificRole + ); + await grantRole(vault, specificRole, anon); + + await expect(vault.connect(anon).forwardToStrategy(4, 0, encodeDummyStorage({}))).to.be.revertedWithCustomError( + vault, + "InvalidStrategy" + ); + + for (let i = 0; i < 3; i++) { + let strategy = strategies[i]; + let failConfig = {}; + expect(await strategy.getFail(vault)).to.be.deep.equal(dummyStorage(failConfig)); + + failConfig = { failDisconnect: true }; + specificRole = await vault.getForwardToStrategyRole(i, 0); + await expect( + vault.connect(anon).forwardToStrategy(i, 0, encodeDummyStorage(failConfig)) + ).to.be.revertedWithACError(vault, anon, specificRole); + await grantRole(vault, specificRole, anon); + await expect(vault.connect(anon).forwardToStrategy(i, 0, encodeDummyStorage(failConfig))).not.to.be.reverted; + expect(await strategy.getFail(vault)).to.be.deep.equal(dummyStorage(failConfig)); + + failConfig = { failConnect: true }; + await expect(vault.connect(anon).forwardToStrategy(i, 0, encodeDummyStorage(failConfig))).not.to.be.reverted; + expect(await strategy.getFail(vault)).to.be.deep.equal(dummyStorage(failConfig)); + + await expect(vault.connect(anon).forwardToStrategy(i, 0, encodeDummyStorage({}))).not.to.be.reverted; + expect(await strategy.getFail(vault)).to.be.deep.equal(dummyStorage({})); + + failConfig = { failWithdraw: true }; + await expect(vault.connect(anon).forwardToStrategy(i, 0, encodeDummyStorage(failConfig))).not.to.be.reverted; + expect(await strategy.getFail(vault)).to.be.deep.equal(dummyStorage(failConfig)); + + expect(await vault.getBytesSlot(await strategy.storageSlot())).to.be.equal(encodeDummyStorage(failConfig)); + await expect(vault.getBytesSlot(ethers.zeroPadValue(ethers.toQuantity(123), 32))).to.be.revertedWithCustomError( + vault, + "OnlyStrategyStorageExposed" + ); + } + }); + + variant.tagit("It checks calls to forwardToStrategy require permission [!MultiStrategyERC4626]", async () => { + const { deployVault, strategies, anon, grantRole, acMgr, admin } = await helpers.loadFixture(variant.fixture); const vault = await deployVault(3); + await expect(vault.connect(anon).forwardToStrategy(4, 0, encodeDummyStorage({}))).to.be.revertedWithAMError( + vault, + anon + ); + + await grantRole(vault, "FORWARD_TO_STRATEGY_ROLE", anon); + let specificSelector = await vault.getForwardToStrategySelector(4, 0); + // Still fails because missing the specific permission + await expect(vault.connect(anon).forwardToStrategy(4, 0, encodeDummyStorage({}))).to.be.revertedWithAMError( + vault, + anon + ); + + // Grant the specific permission + await acMgr.connect(admin).setTargetFunctionRole(vault, [specificSelector], specificSelector); + await grantRole(vault, specificSelector, anon); + + await expect(vault.connect(anon).forwardToStrategy(4, 0, encodeDummyStorage({}))).to.be.revertedWithCustomError( + vault, + "InvalidStrategy" + ); + + for (let i = 0; i < 3; i++) { + let strategy = strategies[i]; + let failConfig = {}; + expect(await strategy.getFail(vault)).to.be.deep.equal(dummyStorage(failConfig)); + + failConfig = { failDisconnect: true }; + specificSelector = await vault.getForwardToStrategySelector(i, 0); + await expect( + vault.connect(anon).forwardToStrategy(i, 0, encodeDummyStorage(failConfig)) + ).to.be.revertedWithAMError(vault, anon); + + await acMgr.connect(admin).setTargetFunctionRole(vault, [specificSelector], specificSelector); + await grantRole(vault, specificSelector, anon); + await expect(vault.connect(anon).forwardToStrategy(i, 0, encodeDummyStorage(failConfig))).not.to.be.reverted; + expect(await strategy.getFail(vault)).to.be.deep.equal(dummyStorage(failConfig)); + + failConfig = { failConnect: true }; + await expect(vault.connect(anon).forwardToStrategy(i, 0, encodeDummyStorage(failConfig))).not.to.be.reverted; + expect(await strategy.getFail(vault)).to.be.deep.equal(dummyStorage(failConfig)); + + await expect(vault.connect(anon).forwardToStrategy(i, 0, encodeDummyStorage({}))).not.to.be.reverted; + expect(await strategy.getFail(vault)).to.be.deep.equal(dummyStorage({})); + + failConfig = { failWithdraw: true }; + await expect(vault.connect(anon).forwardToStrategy(i, 0, encodeDummyStorage(failConfig))).not.to.be.reverted; + expect(await strategy.getFail(vault)).to.be.deep.equal(dummyStorage(failConfig)); + + expect(await vault.getBytesSlot(await strategy.storageSlot())).to.be.equal(encodeDummyStorage(failConfig)); + await expect(vault.getBytesSlot(ethers.zeroPadValue(ethers.toQuantity(123), 32))).to.be.revertedWithCustomError( + vault, + "OnlyStrategyStorageExposed" + ); + } + }); + + variant.tagit("It sets and reads the right value from strategy storage", async () => { + const { deployVault, strategies, grantForwardToStrategy, anon } = await helpers.loadFixture(variant.fixture); + const vault = (await deployVault(3)).connect(anon); + await grantForwardToStrategy(vault, 4, 0, anon); await expect(vault.forwardToStrategy(4, 0, encodeDummyStorage({}))).to.be.revertedWithCustomError( vault, "InvalidStrategy" @@ -343,6 +437,7 @@ variants.forEach((variant) => { expect(await strategy.getFail(vault)).to.be.deep.equal(dummyStorage(failConfig)); failConfig = { failDisconnect: true }; + await grantForwardToStrategy(vault, i, 0, anon); await expect(vault.forwardToStrategy(i, 0, encodeDummyStorage(failConfig))).not.to.be.reverted; expect(await strategy.getFail(vault)).to.be.deep.equal(dummyStorage(failConfig)); @@ -426,19 +521,14 @@ variants.forEach((variant) => { }); variant.tagit("It respects the order of deposit and withdrawal queues", async () => { - const { deployVault, lp, lp2, currency, grantRole, strategies } = await helpers.loadFixture(variant.fixture); + const { deployVault, lp, lp2, currency, grantRole, grantForwardToStrategy, strategies } = + await helpers.loadFixture(variant.fixture); const vault = await deployVault(4, undefined, [3, 2, 1, 0], [2, 0, 3, 1]); await currency.connect(lp).approve(vault, MaxUint256); if (variant.accessManaged) { - await expect(vault.connect(lp).deposit(_A(100), lp)).to.be.revertedWithCustomError( - vault, - "AccessManagedUnauthorized" - ); - await expect(vault.connect(lp).mint(_A(100), lp)).to.be.revertedWithCustomError( - vault, - "AccessManagedUnauthorized" - ); + await expect(vault.connect(lp).deposit(_A(100), lp)).to.be.revertedWithAMError(vault, lp); + await expect(vault.connect(lp).mint(_A(100), lp)).to.be.revertedWithAMError(vault, lp); } else { await expect(vault.connect(lp).deposit(_A(100), lp)).to.be.revertedWithCustomError( vault, @@ -457,29 +547,32 @@ variants.forEach((variant) => { // Check money went to strategy[3] expect(await currency.balanceOf(await strategies[3].other())).to.be.equal(_A(100)); + await grantForwardToStrategy(vault, 0, 0, lp); + await grantForwardToStrategy(vault, 2, 0, lp); + await grantForwardToStrategy(vault, 3, 0, lp); // Then disable deposits on 3 - await vault.forwardToStrategy(3, 0, encodeDummyStorage({ failDeposit: true })); - await vault.forwardToStrategy(2, 0, encodeDummyStorage({ failDeposit: true })); + await vault.connect(lp).forwardToStrategy(3, 0, encodeDummyStorage({ failDeposit: true })); + await vault.connect(lp).forwardToStrategy(2, 0, encodeDummyStorage({ failDeposit: true })); expect(await vault.maxWithdraw(lp)).to.equal(_A(100)); expect(await vault.maxRedeem(lp)).to.equal(_A(100)); expect(await vault.maxWithdraw(lp2)).to.equal(_A(0)); expect(await vault.maxRedeem(lp2)).to.equal(_A(0)); - await vault.forwardToStrategy(3, 0, encodeDummyStorage({ failDeposit: true, failWithdraw: true })); + await vault.connect(lp).forwardToStrategy(3, 0, encodeDummyStorage({ failDeposit: true, failWithdraw: true })); expect(await vault.maxWithdraw(lp)).to.equal(_A(0)); expect(await vault.maxRedeem(lp)).to.equal(_A(0)); - await vault.forwardToStrategy(0, 0, encodeDummyStorage({ failDeposit: true })); + await vault.connect(lp).forwardToStrategy(0, 0, encodeDummyStorage({ failDeposit: true })); expect(await vault.maxDeposit(lp)).to.equal(MaxUint256); expect(await vault.maxMint(lp)).to.equal(MaxUint256); - await vault.forwardToStrategy(0, 0, encodeDummyStorage({})); + await vault.connect(lp).forwardToStrategy(0, 0, encodeDummyStorage({})); await expect(vault.connect(lp).deposit(_A(200), lp)).not.to.be.reverted; expect(await currency.balanceOf(await strategies[1].other())).to.be.equal(_A(200)); expect(await vault.totalAssets()).to.be.equal(_A(300)); - await vault.forwardToStrategy(3, 0, encodeDummyStorage({ failDeposit: true })); + await vault.connect(lp).forwardToStrategy(3, 0, encodeDummyStorage({ failDeposit: true })); await expect(vault.connect(lp).transfer(lp2, _A(150))).not.to.be.reverted; if (variant.accessManaged) { @@ -496,7 +589,8 @@ variants.forEach((variant) => { }); variant.tagit("It respects the order of deposit and authorized user can rebalance", async () => { - const { deployVault, lp, lp2, currency, grantRole, strategies } = await helpers.loadFixture(variant.fixture); + const { deployVault, lp, lp2, currency, grantRole, grantForwardToStrategy, strategies } = + await helpers.loadFixture(variant.fixture); const vault = await deployVault(4, undefined, [3, 2, 1, 0], [2, 0, 3, 1]); await currency.connect(lp).approve(vault, MaxUint256); await grantRole(vault, "LP_ROLE", lp); @@ -522,7 +616,8 @@ variants.forEach((variant) => { .to.be.revertedWithCustomError(vault, "RebalanceAmountExceedsMaxWithdraw") .withArgs(_A(100)); - await vault.forwardToStrategy(2, 0, encodeDummyStorage({ failDeposit: true })); + await grantForwardToStrategy(vault, 2, 0, lp); + await vault.connect(lp).forwardToStrategy(2, 0, encodeDummyStorage({ failDeposit: true })); await expect(vault.connect(lp2).rebalance(3, 2, _A(20))) .to.be.revertedWithCustomError(vault, "RebalanceAmountExceedsMaxDeposit") .withArgs(_A(0)); @@ -621,7 +716,8 @@ variants.forEach((variant) => { }); variant.tagit("It can removeStrategy only if doesn't have funds", async () => { - const { deployVault, lp, lp2, currency, grantRole, strategies } = await helpers.loadFixture(variant.fixture); + const { deployVault, lp, lp2, currency, grantRole, grantForwardToStrategy, strategies } = + await helpers.loadFixture(variant.fixture); const vault = await deployVault(3, undefined, [1, 0, 2], [2, 0, 1]); await currency.connect(lp).approve(vault, MaxUint256); await grantRole(vault, "LP_ROLE", lp); @@ -659,7 +755,9 @@ variants.forEach((variant) => { expect(await vault.depositQueue()).to.deep.equal([1, 2].concat(Array(MAX_STRATEGIES - 2).fill(0))); expect(await vault.withdrawQueue()).to.deep.equal([2, 1].concat(Array(MAX_STRATEGIES - 2).fill(0))); - await expect(vault.forwardToStrategy(1, 0, encodeDummyStorage({ failDisconnect: true }))).not.to.be.reverted; + await grantForwardToStrategy(vault, 1, 0, lp); + await expect(vault.connect(lp).forwardToStrategy(1, 0, encodeDummyStorage({ failDisconnect: true }))).not.to.be + .reverted; await expect(vault.connect(lp2).removeStrategy(1, false)).to.be.revertedWithCustomError(strategies[2], "Fail"); await invariantChecks(vault); @@ -791,522 +889,8 @@ variants.forEach((variant) => { }); variant.tagit("It can replaceStrategy if authorized", async () => { - const { deployVault, lp, lp2, currency, grantRole, strategies } = await helpers.loadFixture(variant.fixture); - const vault = await deployVault(3, undefined, [1, 0, 2], [2, 0, 1]); - - await expect(vault.connect(lp2).replaceStrategy(0, strategies[5], encodeDummyStorage({}), false)).to.be[ - variant.accessError - ](vault, lp2, "STRATEGY_ADMIN_ROLE"); - - await grantRole(vault, "STRATEGY_ADMIN_ROLE", lp2); - - await expect(vault.connect(lp2).replaceStrategy(33, strategies[5], encodeDummyStorage({}), false)).to.be.reverted; - - await expect( - vault.connect(lp2).replaceStrategy(4, strategies[5], encodeDummyStorage({}), false) - ).to.be.revertedWithCustomError(vault, "InvalidStrategy"); - - // Deposit some funds to make it more interesting - await currency.connect(lp).approve(vault, MaxUint256); - await grantRole(vault, "LP_ROLE", lp); - await expect(vault.connect(lp).deposit(_A(100), lp)).not.to.be.reverted; - expect(await vault.totalAssets()).to.equal(_A(100)); - await invariantChecks(vault); - - await vault.forwardToStrategy(1, 0, encodeDummyStorage({ failWithdraw: true })); - await expect( - vault.connect(lp2).replaceStrategy(1, strategies[5], encodeDummyStorage({}), false) - ).to.be.revertedWithCustomError(strategies[1], "Fail"); - - await expect(vault.connect(lp2).replaceStrategy(1, strategies[5], encodeDummyStorage({}), true)) - .to.emit(vault, "StrategyChanged") - .withArgs(strategies[1], strategies[5]) - .to.emit(vault, "WithdrawFailed"); - expect(await vault.totalAssets()).to.equal(_A(0)); // Funds lost in the disconnected strategy - - await expect(vault.connect(lp2).replaceStrategy(1, strategies[1], encodeDummyStorage({}), true)) - .to.emit(vault, "StrategyChanged") - .withArgs(strategies[5], strategies[1]); - expect(await vault.totalAssets()).to.equal(_A(100)); // Funds recovered - - await invariantChecks(vault); - - await expect( - vault.connect(lp2).replaceStrategy(1, strategies[5], encodeDummyStorage({ failConnect: true }), true) - ).to.revertedWithCustomError(strategies[5], "Fail"); - - await expect( - vault.connect(lp2).replaceStrategy(1, strategies[5], encodeDummyStorage({ failDeposit: true }), false) - ).to.revertedWithCustomError(strategies[5], "Fail"); - - await expect( - vault.connect(lp2).replaceStrategy(1, strategies[5], encodeDummyStorage({ failDeposit: true }), true) - ) - .to.emit(vault, "StrategyChanged") - .withArgs(strategies[1], strategies[5]) - .to.emit(vault, "DepositFailed"); - - expect(await vault.totalAssets()).to.equal(_A(0)); // Funds were not deposited to the strategy - - await expect(vault.connect(lp2).replaceStrategy(1, strategies[6], encodeDummyStorage({}), false)) - .to.emit(vault, "StrategyChanged") - .withArgs(strategies[5], strategies[6]); - - expect(await vault.totalAssets()).to.equal(_A(100)); // replaceStrategy recovers the funds in the contract - - // Can't replace with an strategy that's present already - await expect(vault.connect(lp2).replaceStrategy(1, strategies[0], encodeDummyStorage({}), false)) - .to.be.revertedWithCustomError(vault, "DuplicatedStrategy") - .withArgs(strategies[0]); - - // But can replace with the same strategy (might be necessary in some case) - await expect(vault.connect(lp2).replaceStrategy(1, strategies[6], encodeDummyStorage({}), false)) - .to.emit(vault, "StrategyChanged") - .withArgs(strategies[6], strategies[6]); - }); - - variant.tagit("Initialization fails if any strategy and vault have different assets", async () => { - const { MultiStrategyERC4626, DummyInvestStrategy, adminAddr, currency, admin, deployVault, strategies } = + const { deployVault, lp, lp2, currency, grantRole, grantForwardToStrategy, strategies } = await helpers.loadFixture(variant.fixture); - - const differentCurrency = await initCurrency( - { name: "Different USDC", symbol: "DUSDC", decimals: 6, initial_supply: _A(50000), extraArgs: [admin] }, - [] - ); - - const differentStrategy = await DummyInvestStrategy.deploy(differentCurrency); - await expect( - hre.upgrades.deployProxy( - MultiStrategyERC4626, - [ - NAME, - SYMB, - adminAddr, - await ethers.resolveAddress(currency), - [await ethers.resolveAddress(differentStrategy)], - [encodeDummyStorage({})], - [0], - [0], - ], - { - kind: "uups", - unsafeAllow: ["delegatecall"], - } - ) - ).to.be.revertedWithCustomError(MultiStrategyERC4626, "InvalidStrategyAsset"); - - // Sending different length arrays fail - await expect(deployVault(1, Array(2).fill(encodeDummyStorage({})))).to.be.revertedWithCustomError( - MultiStrategyERC4626, - "InvalidStrategiesLength" - ); - await expect(deployVault(2, undefined, [0])).to.be.revertedWithCustomError( - MultiStrategyERC4626, - "InvalidStrategiesLength" - ); - await expect(deployVault(3, undefined, undefined, [1, 0])).to.be.revertedWithCustomError( - MultiStrategyERC4626, - "InvalidStrategiesLength" - ); - await expect(deployVault([ZeroAddress])).to.be.revertedWithCustomError(MultiStrategyERC4626, "InvalidStrategy"); - await expect(deployVault([strategies[0], strategies[1], strategies[0]])).to.be.revertedWithCustomError( - MultiStrategyERC4626, - "DuplicatedStrategy" - ); - await expect(deployVault(2, undefined, [3, 2])).to.be.revertedWithCustomError( - MultiStrategyERC4626, - "InvalidStrategyInDepositQueue" - ); - await expect(deployVault(2, undefined, undefined, [3, 2])).to.be.revertedWithCustomError( - MultiStrategyERC4626, - "InvalidStrategyInWithdrawQueue" - ); - await expect(deployVault(2, undefined, [1, 1])).to.be.revertedWithCustomError( - MultiStrategyERC4626, - "InvalidStrategyInDepositQueue" - ); - await expect(deployVault(2, undefined, undefined, [1, 1])).to.be.revertedWithCustomError( - MultiStrategyERC4626, - "InvalidStrategyInWithdrawQueue" - ); - // Successful initialization emits DepositQueueChanged, WithdrawQueueChanged - const vault = await deployVault(3, undefined, [2, 1, 0], [1, 0, 2]); - expect(vault.deploymentTransaction()) - .to.emit(vault, "StrategyAdded") - .withArgs(strategies[0], 0) - .to.emit(vault, "StrategyAdded") - .withArgs(strategies[1], 1) - .to.emit(vault, "StrategyAdded") - .withArgs(strategies[2], 2) - .to.emit(vault, "DepositQueueChanged") - .withArgs([2, 1, 0]) - .to.emit(vault, "WithdrawQueueChanged") - .withArgs([1, 0, 2]); - await invariantChecks(vault); - }); - - variant.tagit("It respects the order of deposit and withdrawal queues", async () => { - const { deployVault, lp, lp2, currency, grantRole, strategies } = await helpers.loadFixture(variant.fixture); - const vault = await deployVault(4, undefined, [3, 2, 1, 0], [2, 0, 3, 1]); - await currency.connect(lp).approve(vault, MaxUint256); - - if (variant.accessManaged) { - await expect(vault.connect(lp).deposit(_A(100), lp)).to.be.revertedWithAMError(vault, lp); - await expect(vault.connect(lp).mint(_A(100), lp)).to.be.revertedWithAMError(vault, lp); - } else { - await expect(vault.connect(lp).deposit(_A(100), lp)).to.be.revertedWithCustomError( - vault, - "ERC4626ExceededMaxDeposit" - ); - await expect(vault.connect(lp).mint(_A(100), lp)).to.be.revertedWithCustomError( - vault, - "ERC4626ExceededMaxMint" - ); - } - - await grantRole(vault, "LP_ROLE", lp); - await expect(vault.connect(lp).deposit(_A(100), lp)).not.to.be.reverted; - - expect(await vault.totalAssets()).to.be.equal(_A(100)); - // Check money went to strategy[3] - expect(await currency.balanceOf(await strategies[3].other())).to.be.equal(_A(100)); - - // Then disable deposits on 3 - await vault.forwardToStrategy(3, 0, encodeDummyStorage({ failDeposit: true })); - await vault.forwardToStrategy(2, 0, encodeDummyStorage({ failDeposit: true })); - - expect(await vault.maxWithdraw(lp)).to.equal(_A(100)); - expect(await vault.maxRedeem(lp)).to.equal(_A(100)); - expect(await vault.maxWithdraw(lp2)).to.equal(_A(0)); - expect(await vault.maxRedeem(lp2)).to.equal(_A(0)); - - await vault.forwardToStrategy(3, 0, encodeDummyStorage({ failDeposit: true, failWithdraw: true })); - expect(await vault.maxWithdraw(lp)).to.equal(_A(0)); - expect(await vault.maxRedeem(lp)).to.equal(_A(0)); - - await vault.forwardToStrategy(0, 0, encodeDummyStorage({ failDeposit: true })); - expect(await vault.maxDeposit(lp)).to.equal(MaxUint256); - expect(await vault.maxMint(lp)).to.equal(MaxUint256); - await vault.forwardToStrategy(0, 0, encodeDummyStorage({})); - - await expect(vault.connect(lp).deposit(_A(200), lp)).not.to.be.reverted; - expect(await currency.balanceOf(await strategies[1].other())).to.be.equal(_A(200)); - expect(await vault.totalAssets()).to.be.equal(_A(300)); - - await vault.forwardToStrategy(3, 0, encodeDummyStorage({ failDeposit: true })); - - await expect(vault.connect(lp).transfer(lp2, _A(150))).not.to.be.reverted; - - if (variant.accessManaged) { - await grantRole(vault, "LP_ROLE", lp2); - } - await expect(vault.connect(lp2).redeem(_A(150), lp2, lp2)).not.to.be.reverted; - expect(await currency.balanceOf(await strategies[3].other())).to.be.equal(_A(0)); - expect(await currency.balanceOf(await strategies[1].other())).to.be.equal(_A(150)); - - await expect(vault.connect(lp).redeem(_A(150), lp, lp)).not.to.be.reverted; - expect(await currency.balanceOf(await strategies[3].other())).to.be.equal(_A(0)); - expect(await currency.balanceOf(await strategies[1].other())).to.be.equal(_A(0)); - expect(await vault.totalAssets()).to.be.equal(_A(0)); - }); - - variant.tagit("It respects the order of deposit and authorized user can rebalance", async () => { - const { deployVault, lp, lp2, currency, grantRole, strategies } = await helpers.loadFixture(variant.fixture); - const vault = await deployVault(4, undefined, [3, 2, 1, 0], [2, 0, 3, 1]); - await currency.connect(lp).approve(vault, MaxUint256); - await grantRole(vault, "LP_ROLE", lp); - await expect(vault.connect(lp).deposit(_A(100), lp)).not.to.be.reverted; - - expect(await vault.totalAssets()).to.be.equal(_A(100)); - // Check money went to strategy[3] - expect(await currency.balanceOf(await strategies[3].other())).to.be.equal(_A(100)); - - await expect(vault.connect(lp2).rebalance(3, 1, _A(50))).to.be[variant.accessError]( - vault, - lp2, - "REBALANCER_ROLE" - ); - - await grantRole(vault, "REBALANCER_ROLE", lp2); - - await expect(vault.connect(lp2).rebalance(33, 1, _A(50))).to.be.revertedWithCustomError(vault, "InvalidStrategy"); - await expect(vault.connect(lp2).rebalance(1, 33, _A(50))).to.be.revertedWithCustomError(vault, "InvalidStrategy"); - await expect(vault.connect(lp2).rebalance(5, 1, _A(50))).to.be.revertedWithCustomError(vault, "InvalidStrategy"); - await expect(vault.connect(lp2).rebalance(1, 5, _A(50))).to.be.revertedWithCustomError(vault, "InvalidStrategy"); - await expect(vault.connect(lp2).rebalance(3, 1, _A(200))) - .to.be.revertedWithCustomError(vault, "RebalanceAmountExceedsMaxWithdraw") - .withArgs(_A(100)); - - await vault.forwardToStrategy(2, 0, encodeDummyStorage({ failDeposit: true })); - await expect(vault.connect(lp2).rebalance(3, 2, _A(20))) - .to.be.revertedWithCustomError(vault, "RebalanceAmountExceedsMaxDeposit") - .withArgs(_A(0)); - - await expect(vault.connect(lp2).rebalance(3, 1, _A(40))) - .to.emit(vault, "Rebalance") - .withArgs(strategies[3], strategies[1], _A(40)); - - expect(await currency.balanceOf(await strategies[3].other())).to.be.equal(_A(60)); - expect(await currency.balanceOf(await strategies[1].other())).to.be.equal(_A(40)); - - await expect(vault.connect(lp2).rebalance(3, 0, MaxUint256)) - .to.emit(vault, "Rebalance") - .withArgs(strategies[3], strategies[0], _A(60)); - - expect(await currency.balanceOf(await strategies[3].other())).to.be.equal(_A(0)); - expect(await currency.balanceOf(await strategies[0].other())).to.be.equal(_A(60)); - expect(await currency.balanceOf(await strategies[1].other())).to.be.equal(_A(40)); - - await expect(vault.connect(lp2).rebalance(3, 0, MaxUint256)).not.to.emit(vault, "Rebalance"); - }); - - variant.tagit("It can addStrategy and is added at the bottom of the queues", async () => { - const { deployVault, lp, lp2, currency, grantRole, strategies } = await helpers.loadFixture(variant.fixture); - const vault = await deployVault(3, undefined, [1, 0, 2], [2, 0, 1]); - await currency.connect(lp).approve(vault, MaxUint256); - await grantRole(vault, "LP_ROLE", lp); - await expect(vault.connect(lp).deposit(_A(100), lp)).not.to.be.reverted; - await invariantChecks(vault); - - expect(await vault.totalAssets()).to.be.equal(_A(100)); - // Check money went to strategy[1] - expect(await currency.balanceOf(await strategies[1].other())).to.be.equal(_A(100)); - - expect(await vault.depositQueue()).to.deep.equal([2, 1, 3].concat(Array(MAX_STRATEGIES - 3).fill(0))); - expect(await vault.withdrawQueue()).to.deep.equal([3, 1, 2].concat(Array(MAX_STRATEGIES - 3).fill(0))); - - await expect(vault.connect(lp2).addStrategy(strategies[5], encodeDummyStorage({}))).to.be[variant.accessError]( - vault, - lp2, - "STRATEGY_ADMIN_ROLE" - ); - - await grantRole(vault, "STRATEGY_ADMIN_ROLE", lp2); - - await expect(vault.connect(lp2).addStrategy(ZeroAddress, encodeDummyStorage({}))).to.be.revertedWithCustomError( - vault, - "InvalidStrategy" - ); - - await expect(vault.connect(lp2).addStrategy(strategies[1], encodeDummyStorage({}))).to.be.revertedWithCustomError( - vault, - "DuplicatedStrategy" - ); - - await expect( - vault.connect(lp2).addStrategy(strategies[5], encodeDummyStorage({ failConnect: true })) - ).to.be.revertedWithCustomError(strategies[5], "Fail"); - - await expect(vault.connect(lp2).addStrategy(strategies[5], encodeDummyStorage({}))) - .to.emit(vault, "StrategyAdded") - .withArgs(strategies[5], 3); - expect(await vault.depositQueue()).to.deep.equal([2, 1, 3, 4].concat(Array(MAX_STRATEGIES - 4).fill(0))); - expect(await vault.withdrawQueue()).to.deep.equal([3, 1, 2, 4].concat(Array(MAX_STRATEGIES - 4).fill(0))); - await invariantChecks(vault); - }); - - variant.tagit("It can add up to 32 strategies", async () => { - const { deployVault, lp2, DummyInvestStrategy, currency, grantRole, strategies } = await helpers.loadFixture( - variant.fixture - ); - const vault = await deployVault(30); - await invariantChecks(vault); - await grantRole(vault, "STRATEGY_ADMIN_ROLE", lp2); - - // Add 31 works fine - await expect(vault.connect(lp2).addStrategy(strategies[30], encodeDummyStorage({}))) - .to.emit(vault, "StrategyAdded") - .withArgs(strategies[30], 30); - await invariantChecks(vault); - - // Add 32 works fine - await expect(vault.connect(lp2).addStrategy(strategies[31], encodeDummyStorage({}))) - .to.emit(vault, "StrategyAdded") - .withArgs(strategies[31], 31); - await invariantChecks(vault); - - const strategy33 = await DummyInvestStrategy.deploy(currency); - - // Another one fails - await expect(vault.connect(lp2).addStrategy(strategy33, encodeDummyStorage({}))).to.be.revertedWithCustomError( - vault, - "InvalidStrategiesLength" - ); - await invariantChecks(vault); - }); - - variant.tagit("It can removeStrategy only if doesn't have funds", async () => { - const { deployVault, lp, lp2, currency, grantRole, strategies } = await helpers.loadFixture(variant.fixture); - const vault = await deployVault(3, undefined, [1, 0, 2], [2, 0, 1]); - await currency.connect(lp).approve(vault, MaxUint256); - await grantRole(vault, "LP_ROLE", lp); - await expect(vault.connect(lp).mint(_A(100), lp)).not.to.be.reverted; - await invariantChecks(vault); - - expect(await vault.totalAssets()).to.be.equal(_A(100)); - // Check money went to strategy[3] - expect(await currency.balanceOf(await strategies[1].other())).to.be.equal(_A(100)); - - await expect(vault.connect(lp2).removeStrategy(0, false)).to.be[variant.accessError]( - vault, - lp2, - "STRATEGY_ADMIN_ROLE" - ); - - await grantRole(vault, "STRATEGY_ADMIN_ROLE", lp2); - - await expect(vault.connect(lp2).removeStrategy(33, false)).to.be.revertedWithCustomError( - vault, - "InvalidStrategy" - ); - await expect(vault.connect(lp2).removeStrategy(5, false)).to.be.revertedWithCustomError(vault, "InvalidStrategy"); - await expect(vault.connect(lp2).removeStrategy(1, false)).to.be.revertedWithCustomError( - vault, - "CannotRemoveStrategyWithAssets" - ); - - await expect(vault.connect(lp2).removeStrategy(0, false)) - .to.emit(vault, "StrategyRemoved") - .withArgs(strategies[0], 0); - await invariantChecks(vault); - - // Indexes changed but kept in the same order - expect(await vault.depositQueue()).to.deep.equal([1, 2].concat(Array(MAX_STRATEGIES - 2).fill(0))); - expect(await vault.withdrawQueue()).to.deep.equal([2, 1].concat(Array(MAX_STRATEGIES - 2).fill(0))); - - await expect(vault.forwardToStrategy(1, 0, encodeDummyStorage({ failDisconnect: true }))).not.to.be.reverted; - - await expect(vault.connect(lp2).removeStrategy(1, false)).to.be.revertedWithCustomError(strategies[2], "Fail"); - await invariantChecks(vault); - await expect(vault.connect(lp2).removeStrategy(1, true)) - .to.emit(vault, "StrategyRemoved") - .withArgs(strategies[2], 1); - await invariantChecks(vault); - - expect(await vault.depositQueue()).to.deep.equal([1].concat(Array(MAX_STRATEGIES - 1).fill(0))); - expect(await vault.withdrawQueue()).to.deep.equal([1].concat(Array(MAX_STRATEGIES - 1).fill(0))); - - await expect(vault.connect(lp).redeem(_A(100), lp, lp)).not.to.be.reverted; - - await expect(vault.connect(lp2).removeStrategy(0, false)).to.be.revertedWithCustomError( - vault, - "InvalidStrategiesLength" - ); - }); - - variant.tagit("It can removeStrategy in different order", async () => { - const { deployVault, lp2, grantRole, strategies } = await helpers.loadFixture(variant.fixture); - const vault = await deployVault(3, undefined, [1, 0, 2], [2, 0, 1]); - - await grantRole(vault, "STRATEGY_ADMIN_ROLE", lp2); - - await expect(vault.connect(lp2).removeStrategy(1, false)) - .to.emit(vault, "StrategyRemoved") - .withArgs(strategies[1], 1); - await invariantChecks(vault); - - // Indexes changed but kept in the same order - expect(await vault.depositQueue()).to.deep.equal([1, 2].concat(Array(MAX_STRATEGIES - 2).fill(0))); - expect(await vault.withdrawQueue()).to.deep.equal([2, 1].concat(Array(MAX_STRATEGIES - 2).fill(0))); - - await expect(vault.connect(lp2).removeStrategy(1, false)) - .to.emit(vault, "StrategyRemoved") - .withArgs(strategies[2], 1); - await invariantChecks(vault); - - expect(await vault.depositQueue()).to.deep.equal([1].concat(Array(MAX_STRATEGIES - 1).fill(0))); - expect(await vault.withdrawQueue()).to.deep.equal([1].concat(Array(MAX_STRATEGIES - 1).fill(0))); - - await expect(vault.connect(lp2).removeStrategy(0, false)).to.be.revertedWithCustomError( - vault, - "InvalidStrategiesLength" - ); - }); - - variant.tagit("It can change the depositQueue if authorized", async () => { - const { deployVault, lp2, grantRole } = await helpers.loadFixture(variant.fixture); - const vault = await deployVault(3, undefined, [1, 0, 2], [2, 0, 1]); - expect(await vault.depositQueue()).to.deep.equal([2, 1, 3].concat(Array(MAX_STRATEGIES - 3).fill(0))); - - await expect(vault.connect(lp2).changeDepositQueue([0, 1, 2])).to.be[variant.accessError]( - vault, - lp2, - "QUEUE_ADMIN_ROLE" - ); - await grantRole(vault, "QUEUE_ADMIN_ROLE", lp2); - - await expect(vault.connect(lp2).changeDepositQueue([1, 1, 2])) - .to.be.revertedWithCustomError(vault, "InvalidQueueIndexDuplicated") - .withArgs(1); - await expect(vault.connect(lp2).changeDepositQueue([0, 1, 3])).to.be.revertedWithCustomError( - vault, - "InvalidQueue" - ); - await expect(vault.connect(lp2).changeDepositQueue([0, 32, 2])).to.be.revertedWithCustomError( - vault, - "InvalidQueue" - ); - await expect(vault.connect(lp2).changeDepositQueue([0, 1])).to.be.revertedWithCustomError( - vault, - "InvalidQueueLength" - ); - - await expect(vault.connect(lp2).changeDepositQueue([2, 1, 0])) - .to.emit(vault, "DepositQueueChanged") - .withArgs([2, 1, 0]); - await invariantChecks(vault); - - const vault32 = await deployVault(32); - await grantRole(vault32, "QUEUE_ADMIN_ROLE", lp2); - await expect(vault.connect(lp2).changeDepositQueue([...Array(33).keys()])).to.be.revertedWithCustomError( - vault, - "InvalidQueue" - ); - }); - - variant.tagit("It can change the withdrawQueue if authorized", async () => { - const { deployVault, lp2, grantRole } = await helpers.loadFixture(variant.fixture); - const vault = await deployVault(3, undefined, [1, 0, 2], [2, 0, 1]); - expect(await vault.withdrawQueue()).to.deep.equal([3, 1, 2].concat(Array(MAX_STRATEGIES - 3).fill(0))); - - await expect(vault.connect(lp2).changeWithdrawQueue([0, 1, 2])).to.be[variant.accessError]( - vault, - lp2, - "QUEUE_ADMIN_ROLE" - ); - await grantRole(vault, "QUEUE_ADMIN_ROLE", lp2); - - await expect(vault.connect(lp2).changeWithdrawQueue([1, 1, 2])) - .to.be.revertedWithCustomError(vault, "InvalidQueueIndexDuplicated") - .withArgs(1); - await expect(vault.connect(lp2).changeWithdrawQueue([0, 1, 3])).to.be.revertedWithCustomError( - vault, - "InvalidQueue" - ); - await expect(vault.connect(lp2).changeWithdrawQueue([0, 32, 2])).to.be.revertedWithCustomError( - vault, - "InvalidQueue" - ); - await expect(vault.connect(lp2).changeWithdrawQueue([0, 1])).to.be.revertedWithCustomError( - vault, - "InvalidQueueLength" - ); - - await expect(vault.connect(lp2).changeWithdrawQueue([2, 1, 0])) - .to.emit(vault, "WithdrawQueueChanged") - .withArgs([2, 1, 0]); - await invariantChecks(vault); - - const vault32 = await deployVault(32); - await grantRole(vault32, "QUEUE_ADMIN_ROLE", lp2); - await expect(vault.connect(lp2).changeWithdrawQueue([...Array(33).keys()])).to.be.revertedWithCustomError( - vault, - "InvalidQueue" - ); - }); - - variant.tagit("It can replaceStrategy if authorized", async () => { - const { deployVault, lp, lp2, currency, grantRole, strategies } = await helpers.loadFixture(variant.fixture); const vault = await deployVault(3, undefined, [1, 0, 2], [2, 0, 1]); await expect(vault.connect(lp2).replaceStrategy(0, strategies[5], encodeDummyStorage({}), false)).to.be[ @@ -1328,7 +912,8 @@ variants.forEach((variant) => { expect(await vault.totalAssets()).to.equal(_A(100)); await invariantChecks(vault); - await vault.forwardToStrategy(1, 0, encodeDummyStorage({ failWithdraw: true })); + await grantForwardToStrategy(vault, 1, 0, lp); + await vault.connect(lp).forwardToStrategy(1, 0, encodeDummyStorage({ failWithdraw: true })); await expect( vault.connect(lp2).replaceStrategy(1, strategies[5], encodeDummyStorage({}), false) ).to.be.revertedWithCustomError(strategies[1], "Fail"); diff --git a/test/test-storage-gaps.js b/test/test-storage-gaps.js index 53cbe75..9958e79 100644 --- a/test/test-storage-gaps.js +++ b/test/test-storage-gaps.js @@ -11,7 +11,7 @@ describe("Storage Gaps", () => { it(`${contract} has a proper storage gap`, async () => { const { storage, types } = await getStorageLayout( hre, - `contracts/${contract}.sol`, + contract === "SingleStrategyERC4626" ? `contracts/mock/${contract}.sol` : `contracts/${contract}.sol`, contract.split("/").slice(-1)[0] ); diff --git a/test/test-swap-stable-invest-strategy.js b/test/test-swap-stable-invest-strategy.js index 94f8dbf..1d2e02a 100644 --- a/test/test-swap-stable-invest-strategy.js +++ b/test/test-swap-stable-invest-strategy.js @@ -317,11 +317,13 @@ variants.forEach((variant) => { [modifiedSwapConfig] ); - await expect( - vault.connect(anon).forwardToStrategy(SwapStableInvestStrategyMethods.setSwapConfig, newSwapConfigAsBytes) - ).to.be.revertedWithACError(strategy, anon, "SWAP_ADMIN_ROLE"); - - await vault.connect(admin).grantRole(await getRole("SWAP_ADMIN_ROLE"), anon); + /// Access validations no longer implemented in the strategy they should be implemented in the vault + /// contract + /// await expect( + /// vault.connect(anon).forwardToStrategy(SwapStableInvestStrategyMethods.setSwapConfig, newSwapConfigAsBytes) + /// ).to.be.revertedWithACError(strategy, anon, "SWAP_ADMIN_ROLE"); + /// + /// await vault.connect(admin).grantRole(await getRole("SWAP_ADMIN_ROLE"), anon); let tx = await vault .connect(anon) diff --git a/test/utils.js b/test/utils.js index 46fcb5f..f94e872 100644 --- a/test/utils.js +++ b/test/utils.js @@ -60,6 +60,13 @@ Assertion.addMethod("revertedWithAMError", function (contract, user) { return new Assertion(this._obj).to.be.revertedWithCustomError(contract, "AccessManagedUnauthorized").withArgs(user); }); +async function setupAMRole(acMgr, vault, roles, role, methods) { + await acMgr.labelRole(roles[role], role); + for (const method of methods) { + await acMgr.setTargetFunctionRole(vault, [vault.interface.getFunction(method).selector], roles[role]); + } +} + module.exports = { encodeDummyStorage, encodeSwapConfig, @@ -67,4 +74,5 @@ module.exports = { tagit, makeAllViewsPublic, mergeFragments, + setupAMRole, };