From f429bbe1850e9c161a8d0b461573818c6ebbb79a Mon Sep 17 00:00:00 2001 From: alcueca Date: Tue, 23 May 2023 12:21:44 +0100 Subject: [PATCH 1/2] forge install: openzeppelin-contracts v4.8.3 --- .gitmodules | 3 +++ lib/openzeppelin-contracts | 1 + 2 files changed, 4 insertions(+) create mode 160000 lib/openzeppelin-contracts diff --git a/.gitmodules b/.gitmodules index b2bfb7a..5775448 100644 --- a/.gitmodules +++ b/.gitmodules @@ -11,3 +11,6 @@ [submodule "lib/yield-utils-v2"] path = lib/yield-utils-v2 url = https://github.com/yieldprotocol/yield-utils-v2 +[submodule "lib/openzeppelin-contracts"] + path = lib/openzeppelin-contracts + url = https://github.com/openzeppelin/openzeppelin-contracts diff --git a/lib/openzeppelin-contracts b/lib/openzeppelin-contracts new file mode 160000 index 0000000..0a25c19 --- /dev/null +++ b/lib/openzeppelin-contracts @@ -0,0 +1 @@ +Subproject commit 0a25c1940ca220686588c4af3ec526f725fe2582 From 30443e961f67d7f123583812c7654da87ff0fea1 Mon Sep 17 00:00:00 2001 From: alcueca Date: Tue, 23 May 2023 13:33:50 +0100 Subject: [PATCH 2/2] feat: Upgradable Strategies --- remappings.txt | 3 ++- src/Strategy.sol | 31 ++++++++++++++++--------- src/StrategyMigrator.sol | 5 ++-- src/draft/StrategyV3.sol | 5 +--- test/StrategyMigrator.t.sol | 13 ++++++++++- test/StrategyTest.t.sol | 46 ++++++++++++++++++++++++++++++++++++- 6 files changed, 82 insertions(+), 21 deletions(-) diff --git a/remappings.txt b/remappings.txt index 262bc8f..a9b6a69 100644 --- a/remappings.txt +++ b/remappings.txt @@ -1,4 +1,5 @@ forge-std/=lib/forge-std/src/ @yield-protocol/vault-v2=lib/vault-v2 @yield-protocol/yieldspace-tv=lib/yieldspace-tv -@yield-protocol/utils-v2=lib/yield-utils-v2 \ No newline at end of file +@yield-protocol/utils-v2=lib/yield-utils-v2 +openzeppelin/=lib/openzeppelin-contracts/ \ No newline at end of file diff --git a/src/Strategy.sol b/src/Strategy.sol index e4ec68e..a0c38fe 100644 --- a/src/Strategy.sol +++ b/src/Strategy.sol @@ -12,6 +12,7 @@ import { IERC20 } from "@yield-protocol/utils-v2/src/token/IERC20.sol"; import { ERC20Rewards } from "@yield-protocol/utils-v2/src/token/ERC20Rewards.sol"; import { IFYToken } from "@yield-protocol/vault-v2/src/interfaces/IFYToken.sol"; import { IPool } from "@yield-protocol/yieldspace-tv/src/interfaces/IPool.sol"; +import { UUPSUpgradeable } from "openzeppelin/contracts/proxy/utils/UUPSUpgradeable.sol"; /// @dev The Strategy contract allows liquidity providers to provide liquidity in yieldspace /// pool tokens and receive strategy tokens that represent a stake in a YieldSpace pool contract. @@ -21,7 +22,7 @@ import { IPool } from "@yield-protocol/yieldspace-tv/src/interfaces/IPool.sol"; /// The strategy can also `eject` from a Pool before maturity. Any fyToken obtained will be available /// to be bought by anyone at face value. If the pool tokens can't be burned, they will be ejected /// and the strategy can be recapitalized. -contract Strategy is AccessControl, ERC20Rewards, StrategyMigrator { // TODO: I'd like to import IStrategy +contract Strategy is UUPSUpgradeable, AccessControl, ERC20Rewards, StrategyMigrator { // TODO: I'd like to import IStrategy enum State {DEPLOYED, DIVESTED, INVESTED, EJECTED, DRAINED} using MinimalTransferHelper for IERC20; using MinimalTransferHelper for IFYToken; @@ -34,6 +35,7 @@ contract Strategy is AccessControl, ERC20Rewards, StrategyMigrator { // TODO: I' event SoldFYToken(uint256 soldFYToken, uint256 returnedBase); State public state; // The state determines which functions are available + bool public initialized; // Lock for the implementation and upgrade contracts // IERC20 public immutable base; // Base token for this strategy (inherited from StrategyMigrator) // IFYToken public override fyToken; // Current fyToken for this strategy (inherited from StrategyMigrator) @@ -43,21 +45,28 @@ contract Strategy is AccessControl, ERC20Rewards, StrategyMigrator { // TODO: I' uint256 public poolCached; // Pool tokens held by the strategy uint256 public fyTokenCached; // In emergencies, the strategy can keep fyToken - constructor(string memory name_, string memory symbol_, IFYToken fyToken_) - ERC20Rewards(name_, symbol_, SafeERC20Namer.tokenDecimals(address(fyToken_))) - StrategyMigrator( - IERC20(fyToken_.underlying()), - fyToken_) + constructor(string memory name_, string memory symbol_, IERC20 base_) + StrategyMigrator(base_) + ERC20Rewards(name_, symbol_, SafeERC20Namer.tokenDecimals(address(base_))) { - // Deploy with a seriesId_ matching the migrating strategy if using the migration feature - // Deploy with any series matching the desired base in any other case - fyToken = fyToken_; + initialized = true; // Lock the implementation contract + } - base = IERC20(fyToken_.underlying()); - _grantRole(Strategy.init.selector, address(this)); // Enable the `mint` -> `init` hook. + /// @dev Give the ROOT role and create a LOCK role with itself as the admin role and no members. + /// Calling setRoleAdmin(msg.sig, LOCK) means no one can grant that msg.sig role anymore. + function initialize (address root_, IFYToken fyToken_) public { + require(!initialized, "Already initialized"); + require (base == IERC20(fyToken_.underlying()), "Base and fyToken mismatch"); + fyToken = fyToken_; + initialized = true; // On an uninitialized contract, no governance functions can be executed, because no one has permission to do so + _grantRole(ROOT, root_); // Grant ROOT + _setRoleAdmin(LOCK, LOCK); // Create the LOCK role by setting itself as its own admin, creating an independent role tree } + /// @dev Allow to set a new implementation + function _authorizeUpgrade(address newImplementation) internal override auth {} + modifier isState(State target) { require ( target == state, diff --git a/src/StrategyMigrator.sol b/src/StrategyMigrator.sol index c0412d2..12042a7 100644 --- a/src/StrategyMigrator.sol +++ b/src/StrategyMigrator.sol @@ -11,7 +11,7 @@ import {IERC20} from "@yield-protocol/utils-v2/src/token/IERC20.sol"; abstract contract StrategyMigrator is IStrategyMigrator { /// Mock pool base - Must match that of the calling strategy - IERC20 public base; + IERC20 public immutable base; /// Mock pool fyToken - Can be any address IFYToken public fyToken; @@ -19,9 +19,8 @@ abstract contract StrategyMigrator is IStrategyMigrator { /// Mock pool maturity - Can be set to a value far in the future to avoid `divest` calls uint32 public maturity; - constructor(IERC20 base_, IFYToken fyToken_) { + constructor(IERC20 base_) { base = base_; - fyToken = fyToken_; } /// @dev Mock pool init. Called within `invest`. diff --git a/src/draft/StrategyV3.sol b/src/draft/StrategyV3.sol index adb4b8b..c5af788 100644 --- a/src/draft/StrategyV3.sol +++ b/src/draft/StrategyV3.sol @@ -78,9 +78,7 @@ contract StrategyV3 is AccessControl, ERC20Rewards, StrategyMigrator { // TODO: constructor(string memory name, string memory symbol, ILadle ladle_, IFYToken fyToken_) ERC20Rewards(name, symbol, SafeERC20Namer.tokenDecimals(address(fyToken_))) - StrategyMigrator( - IERC20(fyToken_.underlying()), - fyToken_) + StrategyMigrator(IERC20(fyToken_.underlying())) { ladle = ladle_; cauldron = ladle_.cauldron(); @@ -89,7 +87,6 @@ contract StrategyV3 is AccessControl, ERC20Rewards, StrategyMigrator { // TODO: // Deploy with any series matching the desired base in any other case fyToken = fyToken_; - base = IERC20(fyToken_.underlying()); bytes6 baseId_; baseId = baseId_ = fyToken_.underlyingId(); baseJoin = address(ladle_.joins(baseId_)); diff --git a/test/StrategyMigrator.t.sol b/test/StrategyMigrator.t.sol index eddbf45..8f29ca0 100644 --- a/test/StrategyMigrator.t.sol +++ b/test/StrategyMigrator.t.sol @@ -6,6 +6,7 @@ import "../src/Strategy.sol"; import "@yield-protocol/yieldspace-tv/src/interfaces/IPool.sol"; import "@yield-protocol/vault-v2/src/interfaces/IFYToken.sol"; import { TestConstants } from "./utils/TestConstants.sol"; +import { ERC1967Proxy } from "openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol"; abstract contract ZeroState is Test, TestConstants { @@ -44,7 +45,17 @@ abstract contract ZeroState is Test, TestConstants { baseToken = srcStrategy.base(); - dstStrategy = new Strategy("", "", fyToken); + dstStrategy = new Strategy("", "", baseToken); + + ERC1967Proxy dstStrategyProxy = new ERC1967Proxy( + address(dstStrategy), + abi.encodeWithSignature( + "initialize(address,address)", + address(this), + address(fyToken) + ) + ); + dstStrategy = Strategy(address(dstStrategyProxy)); dstStrategy.grantRole(StrategyMigrator.init.selector, address(srcStrategy)); diff --git a/test/StrategyTest.t.sol b/test/StrategyTest.t.sol index 90e0d03..5cae33b 100644 --- a/test/StrategyTest.t.sol +++ b/test/StrategyTest.t.sol @@ -12,6 +12,7 @@ import {IERC20} from "@yield-protocol/utils-v2/src/token/IERC20.sol"; import {IERC20Metadata} from "@yield-protocol/utils-v2/src/token/IERC20Metadata.sol"; import { TestConstants } from "./utils/TestConstants.sol"; import { TestExtensions } from "./utils/TestExtensions.sol"; +import { ERC1967Proxy } from "openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol"; import "@yield-protocol/vault-v2/src/interfaces/DataTypes.sol"; @@ -51,7 +52,17 @@ abstract contract DeployedState is Test, TestConstants, TestExtensions { sharesToken = pool.sharesToken(); // Strategy V2 - strategy = new Strategy("StrategyTest.t.sol", "test", fyToken); + strategy = new Strategy("StrategyTest.t.sol", "test", baseToken); + + ERC1967Proxy strategyProxy = new ERC1967Proxy( + address(strategy), + abi.encodeWithSignature( + "initialize(address,address)", + address(this), + address(fyToken) + ) + ); + strategy = Strategy(address(strategyProxy)); // The strategy needs to be given permission to initalize the pool vm.prank(timelock); @@ -75,7 +86,40 @@ abstract contract DeployedState is Test, TestConstants, TestExtensions { } } + contract DeployedStateTest is DeployedState { + + // Test that the storage is initialized + function testStorageInitialized() public { + assertTrue(strategy.initialized()); + } + + // Test that the storage can't be initialized again + function testInitializeRevertsIfInitialized() public { + strategy.grantRole(Strategy.initialize.selector, address(this)); + + vm.expectRevert("Already initialized"); + strategy.initialize(address(this), IFYToken(address(0))); + } + + // Test that only authorized addresses can upgrade + function testUpgradeToRevertsIfNotAuthed() public { + vm.expectRevert("Access denied"); + strategy.upgradeTo(address(0)); + } + + // Test that the upgrade works + function testUpgradeTo() public { + Strategy strategyV2 = new Strategy("StrategyTest.t.sol", "test", IERC20(address(0))); + + strategy.grantRole(0x3659cfe6, address(this)); // upgradeTo(address) + strategy.upgradeTo(address(strategyV2)); + + assertEq(address(strategy.base()), address(0)); + assertTrue(strategy.hasRole(strategy.ROOT(), address(this))); + assertTrue(strategy.initialized()); + } + function testInit() public { console2.log("strategy.init()"); uint256 initAmount = 10 ** baseToken.decimals();