Feat: RedeemManager Migration - Assumes initiator equals recipient (#299

) * Assumes that the initator and the receiptent are the same * feat: update redeemManager, remove preInitiator array from test * feat: add redeemManager migration test on holesky fork * feat: add more test for RedeemManager migration, increase coverage * chore: improve formatting * chore: update redeemManager test * chore: removed initiators param from redeemManager upgrade script --------- Co-authored-by: Mischa <[email protected]>
liquid-collective · Oct 10, 2024 · 53d70ab · 53d70ab
1 parent 9913e08
commit 53d70ab
Show file tree

Hide file tree

Showing 9 changed files with 135 additions and 81 deletions.
diff --git a/contracts/src/RedeemManager.1.sol b/contracts/src/RedeemManager.1.sol
@@ -65,26 +65,23 @@ contract RedeemManagerV1 is Initializable, IRedeemManagerV1, IProtocolVersion {
         emit SetRiver(_river);
     }
 
-    function initializeRedeemManagerV1_2(address[] calldata _prevInitiators) external init(1) {
-        _redeemQueueMigrationV1_2(_prevInitiators);
+    function initializeRedeemManagerV1_2() external init(1) {
+        _redeemQueueMigrationV1_2();
     }
 
-    function _redeemQueueMigrationV1_2(address[] memory _prevInitiators) internal {
+    function _redeemQueueMigrationV1_2() internal {
         RedeemQueueV1.RedeemRequest[] memory oldQueue = RedeemQueueV1.get();
         uint256 oldQueueLen = oldQueue.length;
         RedeemQueueV2.RedeemRequest[] storage newQueue = RedeemQueueV2.get();
 
-        if (_prevInitiators.length != oldQueueLen) {
-            revert IncompatibleArrayLengths();
-        }
         // Migrate from v1 to v2
         for (uint256 i = 0; i < oldQueueLen;) {
             newQueue[i] = RedeemQueueV2.RedeemRequest({
                 amount: oldQueue[i].amount,
                 maxRedeemableEth: oldQueue[i].maxRedeemableEth,
                 recipient: oldQueue[i].recipient,
                 height: oldQueue[i].height,
-                initiator: _prevInitiators[i] // Assign the provided initiators
+                initiator: oldQueue[i].recipient
             });
 
             unchecked {

diff --git a/contracts/src/interfaces/IRedeemManager.1.sol b/contracts/src/interfaces/IRedeemManager.1.sol
@@ -112,8 +112,7 @@ interface IRedeemManagerV1 {
     /// @param _river The address of the River contract
     function initializeRedeemManagerV1(address _river) external;
 
-    /// @param _prevInitiators The address of initiators for existing requests
-    function initializeRedeemManagerV1_2(address[] calldata _prevInitiators) external;
+    function initializeRedeemManagerV1_2() external;
 
     /// @notice Retrieve River address
     /// @return The address of River

diff --git a/contracts/test/RedeemManager.1.t.sol b/contracts/test/RedeemManager.1.t.sol
@@ -2029,7 +2029,6 @@ contract MockRedeemManagerV1 is MockRedeemManagerV1Base {
 }
 
 contract InitializeRedeemManagerV1_2Test is RedeeManagerV1TestBase {
-    address[] public prevInitiators;
     address public admin = address(0x123);
     address redeemManager;
 
@@ -2064,12 +2063,7 @@ contract InitializeRedeemManagerV1_2Test is RedeeManagerV1TestBase {
         );
         redeemManager = address(proxy);
 
-        // Setup prevInitiators
-        for (uint256 i = 0; i < 30; i++) {
-            prevInitiators.push(address(uint160(i + 1)));
-        }
-
-        // Setup initial queue (RedeemQueueV1)
+        // Setup initial queue (RedeemQueueV1) -> Create 30 random redeem requests to populate the queue before the upgrade/migration
         for (uint256 i = 0; i < 30; i++) {
             address user = address(uint160(i + 100));
             _allowlistUser(user);
@@ -2087,19 +2081,19 @@ contract InitializeRedeemManagerV1_2Test is RedeeManagerV1TestBase {
     function testInitializeTwice() public {
         RedeemManagerV1 redeemQueueImplV2 = new RedeemManagerV1();
         vm.store(redeemManager, IMPLEMENTATION_SLOT, bytes32(uint256(uint160(address(redeemQueueImplV2)))));
-        RedeemManagerV1(redeemManager).initializeRedeemManagerV1_2(prevInitiators);
+        RedeemManagerV1(redeemManager).initializeRedeemManagerV1_2();
 
         vm.expectRevert(abi.encodeWithSignature("InvalidInitialization(uint256,uint256)", 1, 2));
-        RedeemManagerV1(redeemManager).initializeRedeemManagerV1_2(prevInitiators);
+        RedeemManagerV1(redeemManager).initializeRedeemManagerV1_2();
     }
 
     function testRedeemQueueMigrationV1_2() public {
         // Call the migration function
         RedeemManagerV1 redeemQueueImplV2 = new RedeemManagerV1();
         vm.store(redeemManager, IMPLEMENTATION_SLOT, bytes32(uint256(uint160(address(redeemQueueImplV2)))));
-        RedeemManagerV1(redeemManager).initializeRedeemManagerV1_2(prevInitiators);
+        RedeemManagerV1(redeemManager).initializeRedeemManagerV1_2();
 
-        // Check all existing redeemRequests are intact after the migration  (from oldQueue)
+        // Check all existing redeemRequests are intact after the migration (from oldQueue)
         for (uint256 i = 0; i < 30; i++) {
             RedeemQueueV2.RedeemRequest memory current =
                 RedeemManagerV1(redeemManager).getRedeemRequestDetails(uint32(i));
@@ -2112,21 +2106,50 @@ contract InitializeRedeemManagerV1_2Test is RedeeManagerV1TestBase {
                 uint256 prevAmount = RedeemManagerV1(redeemManager).getRedeemRequestDetails(uint32(i - 1)).amount;
                 assertEq(current.height, prevHeight + prevAmount);
             }
-            assertEq(current.initiator, prevInitiators[i]);
+            assertEq(current.initiator, current.recipient);
         }
 
         // Check total length
         assertEq(RedeemManagerV1(redeemManager).getRedeemRequestCount(), 30);
     }
 
-    function testRedeemQueueMigrationV2_IncompatibleArrayLengths() public {
-        // Test with incompatible array length
-        address[] memory invalidInitiators = new address[](6);
-
+    function testRedeemQueueV1_2PostMigrationWithNewRequests() public {
         // Call the migration function
         RedeemManagerV1 redeemQueueImplV2 = new RedeemManagerV1();
         vm.store(redeemManager, IMPLEMENTATION_SLOT, bytes32(uint256(uint160(address(redeemQueueImplV2)))));
-        vm.expectRevert(abi.encodeWithSignature("IncompatibleArrayLengths()"));
-        RedeemManagerV1(redeemManager).initializeRedeemManagerV1_2(invalidInitiators);
+        RedeemManagerV1(redeemManager).initializeRedeemManagerV1_2();
+
+        // Add new 30 random redeem requests after upgrade / migration. Note: 30 is just a random number
+        for (uint256 i = 30; i < 60; i++) {
+            address user = address(uint160(i + 100));
+            _allowlistUser(user);
+            uint128 amount = uint128((i + 1) * 1e18);
+            river.sudoDeal(user, amount);
+
+            vm.prank(user);
+            river.approve(address(redeemManager), amount);
+            assertEq(river.balanceOf(user), amount);
+            vm.prank(user);
+            RedeemManagerV1(redeemManager).requestRedeem(amount, user);
+        }
+
+        // Check all existing and new redeemRequests are intact after the migration
+        for (uint256 i = 0; i < 60; i++) {
+            RedeemQueueV2.RedeemRequest memory current =
+                RedeemManagerV1(redeemManager).getRedeemRequestDetails(uint32(i));
+            assertEq(current.amount, (i + 1) * 1e18);
+            assertEq(current.recipient, address(uint160(i + 100)));
+            if (i == 0) {
+                assertEq(current.height, 0);
+            } else {
+                uint256 prevHeight = RedeemManagerV1(redeemManager).getRedeemRequestDetails(uint32(i - 1)).height;
+                uint256 prevAmount = RedeemManagerV1(redeemManager).getRedeemRequestDetails(uint32(i - 1)).amount;
+                assertEq(current.height, prevHeight + prevAmount);
+            }
+            assertEq(current.initiator, current.recipient);
+        }
+
+        // Check total length
+        assertEq(RedeemManagerV1(redeemManager).getRedeemRequestCount(), 60);
     }
 }
diff --git a/contracts/test/fork/holesky/1.redeemQueueMigrationV1_2_1.t.sol b/contracts/test/fork/holesky/1.redeemQueueMigrationV1_2_1.t.sol
@@ -0,0 +1,84 @@
+//SPDX-License-Identifier: BUSL-1.1
+
+pragma solidity 0.8.20;
+
+import "forge-std/Test.sol";
+
+import "../../../src/TUPProxy.sol";
+import "../../../src/RedeemManager.1.sol";
+import "../../../src/state/redeemManager/RedeemQueue.1.sol";
+import "../../../src/state/redeemManager/RedeemQueue.2.sol";
+import "../../../src/state/redeemManager/WithdrawalStack.sol";
+import {ITransparentUpgradeableProxy} from
+    "openzeppelin-contracts/contracts/proxy/transparent/TransparentUpgradeableProxy.sol";
+
+interface MockIRedeemManagerV1 {
+    function getRedeemRequestDetails(uint32 _redeemRequestId)
+        external
+        view
+        returns (RedeemQueueV1.RedeemRequest memory);
+
+    function getRedeemRequestCount() external view returns (uint256);
+}
+
+contract RedeemQueueMigrationV1_2 is Test {
+    bool internal _skip = false;
+    string internal _rpcUrl;
+
+    address internal constant REDEEM_MANAGER_STAGING_ADDRESS = 0x0693875efbF04dDAd955c04332bA3324472DF980;
+    address internal constant REDEEM_MANAGER_STAGING_PROXY_ADMIN_ADDRESS = 0x80Cf8bD4abf6C078C313f72588720AB86d45c5E6;
+
+    function setUp() external {
+        try vm.envString("TENDERLY_URL") returns (string memory rpcUrl) {
+            _rpcUrl = rpcUrl;
+            vm.createSelectFork(rpcUrl, 2440752);
+            console.log("1.RedeemQueueMigrationV1_2.t.sol is active");
+        } catch {
+            _skip = true;
+        }
+    }
+
+    modifier shouldSkip() {
+        if (!_skip) {
+            _;
+        }
+    }
+
+    function test_migrate_allRedeemRequestsInOneCall() external shouldSkip {
+        // Getting the RedeemManager proxy instance
+        TUPProxy redeemManagerProxy = TUPProxy(payable(REDEEM_MANAGER_STAGING_ADDRESS));
+
+        // Getting RedeemManagerV1 instance before the upgrade
+        MockIRedeemManagerV1 RedeemManager = MockIRedeemManagerV1(REDEEM_MANAGER_STAGING_ADDRESS);
+        // Getting all redeem request details, and count before the upgrade
+        uint256 oldCount = RedeemManager.getRedeemRequestCount();
+        RedeemQueueV1.RedeemRequest[155] memory oldRequests;
+        for (uint256 i = 0; i < 155; i++) {
+            oldRequests[i] = RedeemManager.getRedeemRequestDetails(uint32(i));
+        }
+
+        // Set up the fork at a new block for making the v1_2_1 upgrade, and testing
+        vm.createSelectFork(_rpcUrl, 2440762);
+        // Upgrade the RedeemManager
+        RedeemManagerV1 newImplementation = new RedeemManagerV1();
+        vm.prank(REDEEM_MANAGER_STAGING_PROXY_ADMIN_ADDRESS);
+        ITransparentUpgradeableProxy(address(redeemManagerProxy)).upgradeToAndCall(
+            address(newImplementation), abi.encodeWithSelector(RedeemManagerV1.initializeRedeemManagerV1_2.selector)
+        );
+
+        // After upgrade: check that state before the upgrade, and state after upgrade are same.
+        RedeemManagerV1 RManager = RedeemManagerV1(REDEEM_MANAGER_STAGING_ADDRESS);
+        uint256 newCount = RManager.getRedeemRequestCount();
+        assertEq(newCount, oldCount);
+
+        for (uint32 i = 0; i < newCount; i++) {
+            RedeemQueueV2.RedeemRequest memory newRequest = RManager.getRedeemRequestDetails(i);
+
+            assertEq(newRequest.amount, oldRequests[i].amount);
+            assertEq(newRequest.maxRedeemableEth, oldRequests[i].maxRedeemableEth);
+            assertEq(newRequest.recipient, oldRequests[i].recipient);
+            assertEq(newRequest.height, oldRequests[i].height);
+            assertEq(newRequest.initiator, oldRequests[i].recipient);
+        }
+    }
+}
diff --git a/contracts/test/fork/mainnet/4.redeemQueueMigrationV1_2_1.t.sol b/contracts/test/fork/mainnet/4.redeemQueueMigrationV1_2_1.t.sol
@@ -38,19 +38,6 @@ contract RedeemQueueMigrationV1_2 is Test {
         }
     }
 
-    function _generateRandomAddress(uint256 length) internal view returns (address[] memory) {
-        // Generate a random 20-byte address
-        address[] memory randomAddresses = new address[](length);
-
-        // Populate the array with random addresses
-        for (uint256 i = 0; i < length; i++) {
-            randomAddresses[i] =
-                address(uint160(uint256(keccak256(abi.encodePacked(block.timestamp, block.prevrandao, i)))));
-        }
-
-        return randomAddresses;
-    }
-
     modifier shouldSkip() {
         if (!_skip) {
             _;
@@ -70,17 +57,13 @@ contract RedeemQueueMigrationV1_2 is Test {
             oldRequests[i] = RedeemManager.getRedeemRequestDetails(uint32(i));
         }
 
-        // Generating mock initiators
-        address[] memory mockInitiators = _generateRandomAddress(oldCount);
-
         // Set up the fork at a new block for making the v1_2_1 upgrade, and testing
         vm.createSelectFork(_rpcUrl, 20678000);
         // Upgrade the RedeemManager
         RedeemManagerV1 newImplementation = new RedeemManagerV1();
         vm.prank(REDEEM_MANAGER_MAINNET_PROXY_ADMIN_ADDRESS);
         ITransparentUpgradeableProxy(address(redeemManagerProxy)).upgradeToAndCall(
-            address(newImplementation),
-            abi.encodeWithSelector(RedeemManagerV1.initializeRedeemManagerV1_2.selector, mockInitiators)
+            address(newImplementation), abi.encodeWithSelector(RedeemManagerV1.initializeRedeemManagerV1_2.selector)
         );
 
         // After upgrade: check that state before the upgrade, and state after upgrade are same.
@@ -95,7 +78,7 @@ contract RedeemQueueMigrationV1_2 is Test {
             assertEq(newRequest.maxRedeemableEth, oldRequests[i].maxRedeemableEth);
             assertEq(newRequest.recipient, oldRequests[i].recipient);
             assertEq(newRequest.height, oldRequests[i].height);
-            assertEq(newRequest.initiator, mockInitiators[i]);
+            assertEq(newRequest.initiator, newRequest.recipient);
         }
     }
 }
diff --git a/deploy/devHolesky/09_upgrade_transactions_v1_2_1.ts b/deploy/devHolesky/09_upgrade_transactions_v1_2_1.ts
@@ -89,18 +89,8 @@ const func: DeployFunction = async function ({ deployments, getNamedAccounts, et
     "Withdraw"
   );
 
-  // initiator addresses for the initial 7 redeem requests created before initiator was introduced.
-  const prevInitiators = [
-    "0x4d1bed3a669186130daaf5859b242f3c788d736a",
-    "0xffc58b6a27f6354eba6bb8f39fe163a1625c4b5b",
-    "0xffc58b6a27f6354eba6bb8f39fe163a1625c4b5b",
-    "0xffc58b6a27f6354eba6bb8f39fe163a1625c4b5b",
-    "0xffc58b6a27f6354eba6bb8f39fe163a1625c4b5b",
-    "0xce8dad716539e764895cf30e64466e4e82f278bc",
-    "0xffc58b6a27f6354eba6bb8f39fe163a1625c4b5b",
-  ] // addresses gotten onchain
   const redeemManagerInterface = new ethers.utils.Interface(redeemManagerNewImplementationDeployment.abi);
-  const initData = redeemManagerInterface.encodeFunctionData("initializeRedeemManagerV1_2",[prevInitiators]);
+  const initData = redeemManagerInterface.encodeFunctionData("initializeRedeemManagerV1_2");
   // Call the `upgradeToAndCall` function and pass the encoded initialization data.
   await upgradeToAndCall(
     deployments,

diff --git a/natspec/RedeemManagerV1.md b/natspec/RedeemManagerV1.md
@@ -206,18 +206,13 @@ function initializeRedeemManagerV1(address _river) external nonpayable
 ### initializeRedeemManagerV1_2
 
 ```solidity
-function initializeRedeemManagerV1_2(address[] _prevInitiators) external nonpayable
+function initializeRedeemManagerV1_2() external nonpayable
 ```
 
 
 
 
 
-#### Parameters
-
-| Name | Type | Description |
-|---|---|---|
-| _prevInitiators | address[] | The address of initiators for existing requests |
 
 ### pullExceedingEth
 

diff --git a/natspec/interfaces/IRedeemManagerV1.md b/natspec/interfaces/IRedeemManagerV1.md
@@ -206,18 +206,13 @@ function initializeRedeemManagerV1(address _river) external nonpayable
 ### initializeRedeemManagerV1_2
 
 ```solidity
-function initializeRedeemManagerV1_2(address[] _prevInitiators) external nonpayable
+function initializeRedeemManagerV1_2() external nonpayable
 ```
 
 
 
 
 
-#### Parameters
-
-| Name | Type | Description |
-|---|---|---|
-| _prevInitiators | address[] | The address of initiators for existing requests |
 
 ### pullExceedingEth
 

diff --git a/natspec/state/redeemManager/RedeemQueueV1_2.md b/natspec/state/redeemManager/RedeemQueueV1_2.md