imp(contracts): simplified migrateClient (#533)

Author: srdtrk

abi/ICS26Router.json

@@ -522,14 +522,31 @@
     "name": "migrateClient",
     "inputs": [
       {
-        "name": "subjectClientId",
+        "name": "clientId",
         "type": "string",
         "internalType": "string"
       },
       {
-        "name": "substituteClientId",
-        "type": "string",
-        "internalType": "string"
+        "name": "counterpartyInfo",
+        "type": "tuple",
+        "internalType": "struct IICS02ClientMsgs.CounterpartyInfo",
+        "components": [
+          {
+            "name": "clientId",
+            "type": "string",
+            "internalType": "string"
+          },
+          {
+            "name": "merklePrefix",
+            "type": "bytes[]",
+            "internalType": "bytes[]"
+          }
+        ]
+      },
+      {
+        "name": "client",
+        "type": "address",
+        "internalType": "address"
       }
     ],
     "outputs": [],
@@ -1107,6 +1124,12 @@
             "internalType": "bytes[]"
           }
         ]
+      },
+      {
+        "name": "client",
+        "type": "address",
+        "indexed": false,
+        "internalType": "address"
       }
     ],
     "anonymous": false
@@ -1116,16 +1139,34 @@
     "name": "ICS02ClientMigrated",
     "inputs": [
       {
-        "name": "subjectClientId",
+        "name": "clientId",
         "type": "string",
         "indexed": false,
         "internalType": "string"
       },
       {
-        "name": "substituteClientId",
-        "type": "string",
+        "name": "counterpartyInfo",
+        "type": "tuple",
         "indexed": false,
-        "internalType": "string"
+        "internalType": "struct IICS02ClientMsgs.CounterpartyInfo",
+        "components": [
+          {
+            "name": "clientId",
+            "type": "string",
+            "internalType": "string"
+          },
+          {
+            "name": "merklePrefix",
+            "type": "bytes[]",
+            "internalType": "bytes[]"
+          }
+        ]
+      },
+      {
+        "name": "client",
+        "type": "address",
+        "indexed": false,
+        "internalType": "address"
       }
     ],
     "anonymous": false

contracts/interfaces/IICS02Client.sol

@@ -74,11 +74,17 @@ interface IICS02Client {
         external
         returns (ILightClientMsgs.UpdateResult);
 
-    /// @notice Migrate the underlying client of the subject client to the substitute client.
-    /// @dev This is a privilaged operation, only the owner of ICS02Client can call this function.
-    /// @param subjectClientId The client identifier of the subject client
-    /// @param substituteClientId The client identifier of the substitute client
-    function migrateClient(string calldata subjectClientId, string calldata substituteClientId) external;
+    /// @notice Migrate a client by replacing the existing counterparty information and contract address.
+    /// @dev This is a privilaged operation, only one with `getLightClientMigratorRole(clientId)` can call this.
+    /// @param clientId The client identifier of the client to migrate
+    /// @param counterpartyInfo The new counterparty client information
+    /// @param client The address of the new client contract
+    function migrateClient(
+        string memory clientId,
+        IICS02ClientMsgs.CounterpartyInfo calldata counterpartyInfo,
+        address client
+    )
+        external;
 
     /// @notice Submits misbehaviour to the client with the given client identifier.
     /// @param clientId The client identifier
@@ -94,13 +100,15 @@ interface IICS02Client {
 
     /// @notice Emitted when a new client is added to the client router.
     /// @param clientId The newly created client identifier
-    /// @param counterpartyInfo The counterparty client information, if provided
-    event ICS02ClientAdded(string clientId, IICS02ClientMsgs.CounterpartyInfo counterpartyInfo);
+    /// @param counterpartyInfo The counterparty client information
+    /// @param client The address of the client contract
+    event ICS02ClientAdded(string clientId, IICS02ClientMsgs.CounterpartyInfo counterpartyInfo, address client);
 
     /// @notice Emitted when a client is migrated to a new client.
-    /// @param subjectClientId The client identifier of the existing client
-    /// @param substituteClientId The client identifier of the new client migrated to
-    event ICS02ClientMigrated(string subjectClientId, string substituteClientId);
+    /// @param clientId The client identifier of the migrated client
+    /// @param counterpartyInfo The new counterparty client information
+    /// @param client The address of the new client contract
+    event ICS02ClientMigrated(string clientId, IICS02ClientMsgs.CounterpartyInfo counterpartyInfo, address client);
 
     /// @notice Emitted when a client is updated.
     /// @param clientId The client identifier of the updated ILightClientMsgs

contracts/utils/ICS02ClientUpgradeable.sol

@@ -94,7 +94,7 @@ abstract contract ICS02ClientUpgradeable is IICS02Client, IICS02ClientErrors, Ac
 
     /// @inheritdoc IICS02Client
     function addClient(
-        string memory clientId,
+        string calldata clientId,
         IICS02ClientMsgs.CounterpartyInfo calldata counterpartyInfo,
         address client
     )
@@ -126,7 +126,7 @@ abstract contract ICS02ClientUpgradeable is IICS02Client, IICS02ClientErrors, Ac
         $.clients[clientId] = ILightClient(client);
         $.counterpartyInfos[clientId] = counterpartyInfo;
 
-        emit ICS02ClientAdded(clientId, counterpartyInfo);
+        emit ICS02ClientAdded(clientId, counterpartyInfo, client);
 
         bytes32 role = getLightClientMigratorRole(clientId);
         require(_grantRole(role, _msgSender()), Unreachable());
@@ -148,24 +148,20 @@ abstract contract ICS02ClientUpgradeable is IICS02Client, IICS02ClientErrors, Ac
 
     /// @inheritdoc IICS02Client
     function migrateClient(
-        string calldata subjectClientId,
-        string calldata substituteClientId
+        string calldata clientId,
+        IICS02ClientMsgs.CounterpartyInfo calldata counterpartyInfo,
+        address client
     )
         external
-        onlyRole(getLightClientMigratorRole(subjectClientId))
+        onlyRole(getLightClientMigratorRole(clientId))
     {
-        ICS02ClientStorage storage $ = _getICS02ClientStorage();
+        getClient(clientId); // Ensure subject client exists
 
-        getClient(subjectClientId); // Ensure subject client exists
-        ILightClient substituteClient = getClient(substituteClientId);
-
-        getCounterparty(subjectClientId); // Ensure subject client's counterparty exists
-        IICS02ClientMsgs.CounterpartyInfo memory substituteCounterpartyInfo = getCounterparty(substituteClientId);
-
-        $.counterpartyInfos[subjectClientId] = substituteCounterpartyInfo;
-        $.clients[subjectClientId] = substituteClient;
+        ICS02ClientStorage storage $ = _getICS02ClientStorage();
+        $.counterpartyInfos[clientId] = counterpartyInfo;
+        $.clients[clientId] = ILightClient(client);
 
-        emit ICS02ClientMigrated(subjectClientId, substituteClientId);
+        emit ICS02ClientMigrated(clientId, counterpartyInfo, client);
     }
 
     /// @inheritdoc IICS02Client

packages/go-abigen/ics26router/contract.go

@@ -16,6 +16,7 @@ import { IICS02ClientErrors } from "../../contracts/errors/IICS02ClientErrors.so
 import { ICS02ClientUpgradeable } from "../../contracts/utils/ICS02ClientUpgradeable.sol";
 import { ERC1967Proxy } from "@openzeppelin-contracts/proxy/ERC1967/ERC1967Proxy.sol";
 import { ICS26Router } from "../../contracts/ICS26Router.sol";
+import { TestHelper } from "./utils/TestHelper.sol";
 
 contract ICS02ClientTest is Test {
     ICS02ClientUpgradeable public ics02Client;
@@ -27,6 +28,7 @@ contract ICS02ClientTest is Test {
     string public clientIdentifier;
 
     address public clientOwner = makeAddr("clientOwner");
+    TestHelper public th = new TestHelper();
 
     function setUp() public {
         ICS26Router ics26RouterLogic = new ICS26Router();
@@ -43,7 +45,7 @@ contract ICS02ClientTest is Test {
         IICS02ClientMsgs.CounterpartyInfo memory counterpartyInfo =
             IICS02ClientMsgs.CounterpartyInfo(counterpartyId, merklePrefix);
         vm.expectEmit();
-        emit IICS02Client.ICS02ClientAdded("client-0", counterpartyInfo);
+        emit IICS02Client.ICS02ClientAdded(th.FIRST_CLIENT_ID(), counterpartyInfo, lightClient);
         clientIdentifier = ics02Client.addClient(counterpartyInfo, lightClient);
         vm.stopPrank();
 
@@ -87,10 +89,6 @@ contract ICS02ClientTest is Test {
         address newLightClient = makeAddr("newLightClient");
         IICS02ClientMsgs.CounterpartyInfo memory counterpartyInfo =
             IICS02ClientMsgs.CounterpartyInfo(counterpartyId, randomPrefix);
-        vm.expectEmit();
-        emit IICS02Client.ICS02ClientAdded("client-1", counterpartyInfo);
-        string memory substituteIdentifier = ics02Client.addClient(counterpartyInfo, newLightClient);
-        assertEq(2, ics02Client.getNextClientSeq());
 
         vm.expectRevert(
             abi.encodeWithSelector(
@@ -99,18 +97,19 @@ contract ICS02ClientTest is Test {
                 ics02Client.getLightClientMigratorRole(clientIdentifier)
             )
         );
-        ics02Client.migrateClient(clientIdentifier, substituteIdentifier);
+        ics02Client.migrateClient(clientIdentifier, counterpartyInfo, newLightClient);
         vm.stopPrank();
 
         vm.startPrank(clientOwner);
-        ics02Client.migrateClient(clientIdentifier, substituteIdentifier);
+        ics02Client.migrateClient(clientIdentifier, counterpartyInfo, newLightClient);
         ILightClient fetchedLightClient = ics02Client.getClient(clientIdentifier);
         assertEq(address(fetchedLightClient), newLightClient, "client not migrated");
         vm.stopPrank();
 
         IICS02ClientMsgs.CounterpartyInfo memory fetchedCounterparty = ics02Client.getCounterparty(clientIdentifier);
         assertEq(fetchedCounterparty.clientId, counterpartyId, "counterparty not migrated");
         assertEq(fetchedCounterparty.merklePrefix, randomPrefix, "counterparty not migrated");
+        assertEq(ics02Client.getNextClientSeq(), 1, "client seq not incremented");
     }
 
     function test_RenounceRole() public {