Upgradable strategies

.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

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
+@yield-protocol/utils-v2=lib/yield-utils-v2
+openzeppelin/=lib/openzeppelin-contracts/

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,

src/StrategyMigrator.sol

@@ -11,17 +11,16 @@ 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;
 
     /// 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`.

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_));

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));
 

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();