Merge pull request #234 from hyperledger-labs/refactor-handler

Refactoring to merge handler into each feature implementation

Signed-off-by: Jun Kimura <jun.kimura@datachain.jp>

.gas-snapshot

@@ -1,41 +1,41 @@
-IBCTest:testBenchmarkCreateMockClient() (gas: 216645)
-IBCTest:testBenchmarkRecvPacket() (gas: 153912)
-IBCTest:testBenchmarkSendPacket() (gas: 89919)
-IBCTest:testBenchmarkUpdateMockClient() (gas: 129946)
-IBCTest:testToUint128((uint64,uint64)) (runs: 256, μ: 1013, ~: 1013)
-TestICS02:testCreateClient() (gas: 20419568)
-TestICS02:testInvalidCreateClient() (gas: 20267951)
-TestICS02:testInvalidUpdateClient() (gas: 20272833)
-TestICS02:testRegisterClient() (gas: 19902791)
-TestICS02:testRegisterClientDuplicatedClientType() (gas: 19884771)
-TestICS02:testRegisterClientInvalidClientType() (gas: 19873570)
-TestICS02:testUpdateClient() (gas: 20415107)
-TestICS03Handshake:testConnOpenAck() (gas: 1642938)
-TestICS03Handshake:testConnOpenConfirm() (gas: 1781624)
-TestICS03Handshake:testConnOpenInit() (gas: 1319897)
-TestICS03Handshake:testConnOpenTry() (gas: 2148987)
-TestICS03Handshake:testInvalidConnOpenAck() (gas: 2031709)
-TestICS03Handshake:testInvalidConnOpenConfirm() (gas: 2037872)
-TestICS03Handshake:testInvalidConnOpenInit() (gas: 693724)
-TestICS03Handshake:testInvalidConnOpenTry() (gas: 2156100)
-TestICS03Version:testCopyVersions() (gas: 560316)
-TestICS03Version:testFindSupportedVersion() (gas: 19986)
-TestICS03Version:testIsSupportedVersion() (gas: 8317)
-TestICS03Version:testPickVersion() (gas: 27093)
-TestICS03Version:testVerifyProposedVersion() (gas: 12368)
-TestICS03Version:testVerifySupportedFeature() (gas: 4441)
-TestICS04Handshake:testChanOpenAck() (gas: 3081103)
-TestICS04Handshake:testChanOpenConfirm() (gas: 3272966)
-TestICS04Handshake:testChanOpenInit() (gas: 2337459)
-TestICS04Handshake:testChanOpenTry() (gas: 2840835)
-TestICS04Handshake:testInvalidChanOpenAck() (gas: 2175906)
-TestICS04Handshake:testInvalidChanOpenConfirm() (gas: 2234633)
-TestICS04Handshake:testInvalidChanOpenInit() (gas: 1333705)
-TestICS04Handshake:testInvalidChanOpenTry() (gas: 1411715)
-TestICS04Packet:testInvalidSendPacket() (gas: 2301656)
-TestICS04Packet:testSendPacket() (gas: 2486762)
-TestICS20:testAddressToHex(address) (runs: 256, μ: 23716, ~: 23858)
-TestICS20:testHexToAddress(string) (runs: 256, μ: 4817, ~: 4829)
-TestICS20:testIsEscapedString() (gas: 49914)
-TestICS20:testMarshaling() (gas: 150062)
-TestICS20:testParseAmount(uint256) (runs: 256, μ: 27191, ~: 21814)
+IBCTest:testBenchmarkCreateMockClient() (gas: 211711)
+IBCTest:testBenchmarkRecvPacket() (gas: 140015)
+IBCTest:testBenchmarkSendPacket() (gas: 86064)
+IBCTest:testBenchmarkUpdateMockClient() (gas: 126678)
+IBCTest:testToUint128((uint64,uint64)) (runs: 256, μ: 917, ~: 917)
+TestICS02:testCreateClient() (gas: 23422491)
+TestICS02:testInvalidCreateClient() (gas: 23270044)
+TestICS02:testInvalidUpdateClient() (gas: 23273913)
+TestICS02:testRegisterClient() (gas: 22894201)
+TestICS02:testRegisterClientDuplicatedClientType() (gas: 22876251)
+TestICS02:testRegisterClientInvalidClientType() (gas: 22861044)
+TestICS02:testUpdateClient() (gas: 23417285)
+TestICS03Handshake:testConnOpenAck() (gas: 1594072)
+TestICS03Handshake:testConnOpenConfirm() (gas: 1728943)
+TestICS03Handshake:testConnOpenInit() (gas: 1287196)
+TestICS03Handshake:testConnOpenTry() (gas: 2088127)
+TestICS03Handshake:testInvalidConnOpenAck() (gas: 1953639)
+TestICS03Handshake:testInvalidConnOpenConfirm() (gas: 1974862)
+TestICS03Handshake:testInvalidConnOpenInit() (gas: 672171)
+TestICS03Handshake:testInvalidConnOpenTry() (gas: 2093785)
+TestICS03Version:testCopyVersions() (gas: 560134)
+TestICS03Version:testFindSupportedVersion() (gas: 19740)
+TestICS03Version:testIsSupportedVersion() (gas: 8272)
+TestICS03Version:testPickVersion() (gas: 26823)
+TestICS03Version:testVerifyProposedVersion() (gas: 12185)
+TestICS03Version:testVerifySupportedFeature() (gas: 4357)
+TestICS04Handshake:testChanOpenAck() (gas: 2989540)
+TestICS04Handshake:testChanOpenConfirm() (gas: 3172516)
+TestICS04Handshake:testChanOpenInit() (gas: 2276564)
+TestICS04Handshake:testChanOpenTry() (gas: 2760444)
+TestICS04Handshake:testInvalidChanOpenAck() (gas: 2117646)
+TestICS04Handshake:testInvalidChanOpenConfirm() (gas: 2176812)
+TestICS04Handshake:testInvalidChanOpenInit() (gas: 1281923)
+TestICS04Handshake:testInvalidChanOpenTry() (gas: 1356309)
+TestICS04Packet:testInvalidSendPacket() (gas: 2319493)
+TestICS04Packet:testSendPacket() (gas: 2377298)
+TestICS20:testAddressToHex(address) (runs: 256, μ: 22708, ~: 22824)
+TestICS20:testHexToAddress(string) (runs: 256, μ: 4776, ~: 4734)
+TestICS20:testIsEscapedString() (gas: 48979)
+TestICS20:testMarshaling() (gas: 148145)
+TestICS20:testParseAmount(uint256) (runs: 256, μ: 27065, ~: 21653)

contracts/apps/20-transfer/ICS20Transfer.sol

@@ -2,8 +2,7 @@
 pragma solidity ^0.8.9;
 
 import "../commons/IBCAppBase.sol";
-import "../../core/05-port/IIBCModule.sol";
-import "../../core/25-handler/IBCHandler.sol";
+import "../../core/26-router/IIBCModule.sol";
 import "../../proto/Channel.sol";
 import "./ICS20Lib.sol";
 import "solidity-bytes-utils/contracts/BytesLib.sol";

contracts/apps/20-transfer/ICS20TransferBank.sol

@@ -3,16 +3,16 @@ pragma solidity ^0.8.9;
 
 import "./ICS20Transfer.sol";
 import "./IICS20Bank.sol";
-import "../../core/25-handler/IBCHandler.sol";
+import "../../core/25-handler/IIBCHandler.sol";
 import "solidity-bytes-utils/contracts/BytesLib.sol";
 
 contract ICS20TransferBank is ICS20Transfer {
     using BytesLib for bytes;
 
-    IBCHandler private immutable ibcHandler;
+    IIBCHandler private immutable ibcHandler;
     IICS20Bank private immutable bank;
 
-    constructor(IBCHandler ibcHandler_, IICS20Bank bank_) {
+    constructor(IIBCHandler ibcHandler_, IICS20Bank bank_) {
         ibcHandler = ibcHandler_;
         bank = bank_;
     }
@@ -43,7 +43,7 @@ contract ICS20TransferBank is ICS20Transfer {
             require(_burn(_msgSender(), denom, amount));
         }
         bytes memory packetData = ICS20Lib.marshalJSON(denom, amount, _encodeSender(_msgSender()), receiver);
-        IBCHandler(ibcAddress()).sendPacket(
+        IIBCHandler(ibcAddress()).sendPacket(
             sourcePort, sourceChannel, Height.Data({revision_number: 0, revision_height: timeoutHeight}), 0, packetData
         );
     }

contracts/apps/commons/IBCAppBase.sol

@@ -2,7 +2,7 @@
 pragma solidity ^0.8.9;
 
 import "@openzeppelin/contracts/utils/Context.sol";
-import "../../core/05-port/IIBCModule.sol";
+import "../../core/26-router/IIBCModule.sol";
 
 /**
  * @dev Base contract of the IBC App protocol

contracts/apps/mock/IBCMockApp.sol

@@ -1,17 +1,19 @@
 // SPDX-License-Identifier: Apache-2.0
 pragma solidity ^0.8.9;
 
-import "../commons/IBCAppBase.sol";
-import "../../core/05-port/IIBCModule.sol";
-import "../../core/25-handler/IBCHandler.sol";
-import "./IBCMockLib.sol";
+import {Height} from "../../proto/Client.sol";
+import {Packet} from "../../proto/Channel.sol";
+import {IBCAppBase} from "../commons/IBCAppBase.sol";
+import {IIBCModule} from "../../core/26-router/IIBCModule.sol";
+import {IIBCHandler} from "../../core/25-handler/IIBCHandler.sol";
+import {IBCMockLib} from "./IBCMockLib.sol";
 
 contract IBCMockApp is IBCAppBase {
     string public constant MOCKAPP_VERSION = "mockapp-1";
 
-    IBCHandler immutable ibcHandler;
+    IIBCHandler immutable ibcHandler;
 
-    constructor(IBCHandler ibcHandler_) {
+    constructor(IIBCHandler ibcHandler_) {
         ibcHandler = ibcHandler_;
     }
 

contracts/clients/IBFT2Client.sol

@@ -3,7 +3,7 @@ pragma solidity ^0.8.9;
 
 import "../core/02-client/ILightClient.sol";
 import "../core/02-client/IBCHeight.sol";
-import "../core/25-handler/IBCHandler.sol";
+import "../core/25-handler/IIBCHandler.sol";
 import "../proto/Client.sol";
 import {
     IbcLightclientsIbft2V1ClientState as ClientState,
@@ -421,7 +421,7 @@ contract IBFT2Client is ILightClient {
             return false;
         }
 
-        return keccak256(IBCHandler(ibcHandler).getCommitmentPrefix()) == keccak256(prefix);
+        return keccak256(IIBCHandler(ibcHandler).getCommitmentPrefix()) == keccak256(prefix);
     }
 
     function verifyMembership(bytes calldata proof, bytes32 root, bytes32 slot, bytes32 expectedValue)

contracts/clients/MockClient.sol

@@ -3,7 +3,7 @@ pragma solidity ^0.8.9;
 
 import "../core/02-client/ILightClient.sol";
 import "../core/02-client/IBCHeight.sol";
-import "../core/25-handler/IBCHandler.sol";
+import "../core/25-handler/IIBCHandler.sol";
 import "../proto/Client.sol";
 import {
     IbcLightclientsMockV1ClientState as ClientState,
@@ -167,7 +167,7 @@ contract MockClient is ILightClient {
         bytes calldata value
     ) external view virtual override returns (bool) {
         require(consensusStates[clientId][height.toUint128()].timestamp != 0, "consensus state not found");
-        require(keccak256(IBCHandler(ibcHandler).getCommitmentPrefix()) == keccak256(prefix), "invalid prefix");
+        require(keccak256(IIBCHandler(ibcHandler).getCommitmentPrefix()) == keccak256(prefix), "invalid prefix");
         return sha256(value) == proof.toBytes32(0);
     }
 
@@ -185,7 +185,7 @@ contract MockClient is ILightClient {
         bytes memory
     ) external view virtual override returns (bool) {
         require(consensusStates[clientId][height.toUint128()].timestamp != 0, "consensus state not found");
-        require(keccak256(IBCHandler(ibcHandler).getCommitmentPrefix()) == keccak256(prefix), "invalid prefix");
+        require(keccak256(IIBCHandler(ibcHandler).getCommitmentPrefix()) == keccak256(prefix), "invalid prefix");
         return proof.length == 0;
     }
 

contracts/core/02-client/IBCClient.sol

@@ -1,32 +1,20 @@
 // SPDX-License-Identifier: Apache-2.0
 pragma solidity ^0.8.9;
 
-import "@openzeppelin/contracts/utils/Address.sol";
-import "@openzeppelin/contracts/utils/Strings.sol";
-import "./ILightClient.sol";
-import "../25-handler/IBCMsgs.sol";
-import "../24-host/IBCStore.sol";
-import "../24-host/IBCCommitment.sol";
-import "../02-client/IIBCClient.sol";
+import {Strings} from "@openzeppelin/contracts/utils/Strings.sol";
+import {ILightClient, ConsensusStateUpdate} from "./ILightClient.sol";
+import {IBCHost} from "../24-host/IBCHost.sol";
+import {IBCCommitment} from "../24-host/IBCCommitment.sol";
+import {IIBCClient} from "../02-client/IIBCClient.sol";
 
 /**
  * @dev IBCClient is a contract that implements [ICS-2](https://github.com/cosmos/ibc/tree/main/spec/core/ics-002-client-semantics).
  */
-contract IBCClient is IBCStore, IIBCClient {
-    /**
-     * @dev registerClient registers a new client type into the client registry
-     */
-    function registerClient(string calldata clientType, ILightClient client) external override {
-        require(validateClientType(bytes(clientType)), "invalid clientType");
-        require(address(clientRegistry[clientType]) == address(0), "clientType already exists");
-        require(address(client) != address(this) && Address.isContract(address(client)), "invalid client address");
-        clientRegistry[clientType] = address(client);
-    }
-
+contract IBCClient is IBCHost, IIBCClient {
     /**
      * @dev createClient creates a new client state and populates it with a given consensus state
      */
-    function createClient(IBCMsgs.MsgCreateClient calldata msg_) external override returns (string memory clientId) {
+    function createClient(MsgCreateClient calldata msg_) external override returns (string memory clientId) {
         address clientImpl = clientRegistry[msg_.clientType];
         require(clientImpl != address(0), "unregistered client type");
         clientId = generateClientIdentifier(msg_.clientType);
@@ -41,14 +29,14 @@ contract IBCClient is IBCStore, IIBCClient {
         commitments[IBCCommitment.consensusStateCommitmentKey(
             clientId, update.height.revision_number, update.height.revision_height
         )] = update.consensusStateCommitment;
-
+        emit GeneratedClientIdentifier(clientId);
         return clientId;
     }
 
     /**
      * @dev updateClient updates the consensus state and the state root from a provided header
      */
-    function updateClient(IBCMsgs.MsgUpdateClient calldata msg_) external override {
+    function updateClient(MsgUpdateClient calldata msg_) external override {
         require(commitments[IBCCommitment.clientStateCommitmentKey(msg_.clientId)] != bytes32(0));
         (bytes32 clientStateCommitment, ConsensusStateUpdate[] memory updates, bool ok) =
             checkAndGetClient(msg_.clientId).updateClient(msg_.clientId, msg_.clientMessage);
@@ -73,36 +61,4 @@ contract IBCClient is IBCStore, IIBCClient {
         nextClientSequence++;
         return identifier;
     }
-
-    /**
-     * @dev validateClientType validates the client type
-     *   - clientType must be non-empty
-     *   - clientType must be in the form of `^[a-z][a-z0-9-]*[a-z0-9]$`
-     */
-    function validateClientType(bytes memory clientTypeBytes) internal pure returns (bool) {
-        if (clientTypeBytes.length == 0) {
-            return false;
-        }
-        unchecked {
-            for (uint256 i = 0; i < clientTypeBytes.length; i++) {
-                uint256 c = uint256(uint8(clientTypeBytes[i]));
-                if (0x61 <= c && c <= 0x7a) {
-                    // a-z
-                    continue;
-                } else if (c == 0x2d) {
-                    // hyphen cannot be the first or last character
-                    if (i == 0 || i == clientTypeBytes.length - 1) {
-                        return false;
-                    }
-                    continue;
-                } else if (0x30 <= c && c <= 0x39) {
-                    // 0-9
-                    continue;
-                } else {
-                    return false;
-                }
-            }
-        }
-        return true;
-    }
 }

contracts/core/02-client/IBCClientLib.sol

@@ -0,0 +1,36 @@
+// SPDX-License-Identifier: Apache-2.0
+pragma solidity ^0.8.9;
+
+library IBCClientLib {
+    /**
+     * @dev validateClientType validates the client type
+     *   - clientType must be non-empty
+     *   - clientType must be in the form of `^[a-z][a-z0-9-]*[a-z0-9]$`
+     */
+    function validateClientType(bytes memory clientTypeBytes) internal pure returns (bool) {
+        if (clientTypeBytes.length == 0) {
+            return false;
+        }
+        unchecked {
+            for (uint256 i = 0; i < clientTypeBytes.length; i++) {
+                uint256 c = uint256(uint8(clientTypeBytes[i]));
+                if (0x61 <= c && c <= 0x7a) {
+                    // a-z
+                    continue;
+                } else if (c == 0x2d) {
+                    // hyphen cannot be the first or last character
+                    if (i == 0 || i == clientTypeBytes.length - 1) {
+                        return false;
+                    }
+                    continue;
+                } else if (0x30 <= c && c <= 0x39) {
+                    // 0-9
+                    continue;
+                } else {
+                    return false;
+                }
+            }
+        }
+        return true;
+    }
+}

contracts/core/02-client/IBCHeight.sol

@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: Apache-2.0
 pragma solidity ^0.8.9;
 
-import "../../proto/Client.sol";
+import {Height} from "../../proto/Client.sol";
 
 library IBCHeight {
     function toUint128(Height.Data memory self) internal pure returns (uint128) {

contracts/core/02-client/IIBCClient.sol

@@ -1,22 +1,27 @@
 // SPDX-License-Identifier: Apache-2.0
 pragma solidity ^0.8.9;
 
-import "./ILightClient.sol";
-import "../25-handler/IBCMsgs.sol";
-
 interface IIBCClient {
-    /**
-     * @dev registerClient registers a new client type into the client registry
-     */
-    function registerClient(string calldata clientType, ILightClient client) external;
+    struct MsgCreateClient {
+        string clientType;
+        bytes clientStateBytes;
+        bytes consensusStateBytes;
+    }
+
+    struct MsgUpdateClient {
+        string clientId;
+        bytes clientMessage;
+    }
+
+    event GeneratedClientIdentifier(string);
 
     /**
      * @dev createClient creates a new client state and populates it with a given consensus state
      */
-    function createClient(IBCMsgs.MsgCreateClient calldata msg_) external returns (string memory clientId);
+    function createClient(MsgCreateClient calldata msg_) external returns (string memory clientId);
 
     /**
      * @dev updateClient updates the consensus state and the state root from a provided header
      */
-    function updateClient(IBCMsgs.MsgUpdateClient calldata msg_) external;
+    function updateClient(MsgUpdateClient calldata msg_) external;
 }

contracts/core/02-client/ILightClient.sol

@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: Apache-2.0
 pragma solidity ^0.8.9;
 
-import "../../proto/Client.sol";
+import {Height} from "../../proto/Client.sol";
 
 /**
  * @dev This defines an interface for Light Client contract can be integrated with ibc-solidity.