Use struct type as callback parameters of channel handshake

.gas-snapshot

@@ -1,5 +1,5 @@
 IBCTest:testBenchmarkCreateMockClient() (gas: 214230)
-IBCTest:testBenchmarkRecvPacket() (gas: 157447)
+IBCTest:testBenchmarkRecvPacket() (gas: 157491)
 IBCTest:testBenchmarkSendPacket() (gas: 86050)
 IBCTest:testBenchmarkUpdateMockClient() (gas: 129918)
 IBCTest:testConnectionOpenInit() (gas: 496526)

contracts/apps/20-transfer/ICS20Transfer.sol

@@ -72,45 +72,38 @@ abstract contract ICS20Transfer is IBCAppBase {
         }
     }
 
-    function onChanOpenInit(
-        Channel.Order order,
-        string[] calldata,
-        string calldata,
-        string calldata channelId,
-        ChannelCounterparty.Data calldata,
-        string calldata version
-    ) external virtual override onlyIBC returns (string memory) {
-        require(order == Channel.Order.ORDER_UNORDERED, "must be unordered");
-        bytes memory versionBytes = bytes(version);
+    function onChanOpenInit(IIBCModule.MsgOnChanOpenInit calldata msg_)
+        external
+        virtual
+        override
+        onlyIBC
+        returns (string memory)
+    {
+        require(msg_.order == Channel.Order.ORDER_UNORDERED, "must be unordered");
+        bytes memory versionBytes = bytes(msg_.version);
         require(versionBytes.length == 0 || keccak256(versionBytes) == keccak256(bytes(ICS20_VERSION)));
-        channelEscrowAddresses[channelId] = address(this);
+        channelEscrowAddresses[msg_.channelId] = address(this);
         return ICS20_VERSION;
     }
 
-    function onChanOpenTry(
-        Channel.Order order,
-        string[] calldata,
-        string calldata,
-        string calldata channelId,
-        ChannelCounterparty.Data calldata,
-        string calldata counterpartyVersion
-    ) external virtual override onlyIBC returns (string memory) {
-        require(order == Channel.Order.ORDER_UNORDERED, "must be unordered");
-        require(keccak256(bytes(counterpartyVersion)) == keccak256(bytes(ICS20_VERSION)));
-        channelEscrowAddresses[channelId] = address(this);
-        return ICS20_VERSION;
-    }
-
-    function onChanOpenAck(string calldata, string calldata, string calldata counterpartyVersion)
+    function onChanOpenTry(IIBCModule.MsgOnChanOpenTry calldata msg_)
         external
         virtual
         override
         onlyIBC
+        returns (string memory)
     {
-        require(keccak256(bytes(counterpartyVersion)) == keccak256(bytes(ICS20_VERSION)));
+        require(msg_.order == Channel.Order.ORDER_UNORDERED, "must be unordered");
+        require(keccak256(bytes(msg_.counterpartyVersion)) == keccak256(bytes(ICS20_VERSION)));
+        channelEscrowAddresses[msg_.channelId] = address(this);
+        return ICS20_VERSION;
+    }
+
+    function onChanOpenAck(IIBCModule.MsgOnChanOpenAck calldata msg_) external virtual override onlyIBC {
+        require(keccak256(bytes(msg_.counterpartyVersion)) == keccak256(bytes(ICS20_VERSION)));
     }
 
-    function onChanCloseInit(string calldata, string calldata) external virtual override onlyIBC {
+    function onChanCloseInit(IIBCModule.MsgOnChanCloseInit calldata) external virtual override onlyIBC {
         revert("not allowed");
     }
 

contracts/apps/commons/IBCAppBase.sol

@@ -33,61 +33,54 @@ abstract contract IBCAppBase is Context, IIBCModule {
      *
      * NOTE: You should apply an `onlyIBC` modifier to the function if a derived contract overrides it.
      */
-    function onChanOpenInit(
-        Channel.Order,
-        string[] calldata,
-        string calldata,
-        string calldata channelId,
-        ChannelCounterparty.Data calldata,
-        string calldata
-    ) external virtual override onlyIBC returns (string memory) {}
+    function onChanOpenInit(IIBCModule.MsgOnChanOpenInit calldata)
+        external
+        virtual
+        override
+        onlyIBC
+        returns (string memory)
+    {}
 
     /**
      * @dev See IIBCModule-onChanOpenTry
      *
      * NOTE: You should apply an `onlyIBC` modifier to the function if a derived contract overrides it.
      */
-    function onChanOpenTry(
-        Channel.Order,
-        string[] calldata,
-        string calldata,
-        string calldata channelId,
-        ChannelCounterparty.Data calldata,
-        string calldata
-    ) external virtual override onlyIBC returns (string memory) {}
+    function onChanOpenTry(IIBCModule.MsgOnChanOpenTry calldata)
+        external
+        virtual
+        override
+        onlyIBC
+        returns (string memory)
+    {}
 
     /**
      * @dev See IIBCModule-onChanOpenAck
      *
      * NOTE: You should apply an `onlyIBC` modifier to the function if a derived contract overrides it.
      */
-    function onChanOpenAck(string calldata portId, string calldata channelId, string calldata counterpartyVersion)
-        external
-        virtual
-        override
-        onlyIBC
-    {}
+    function onChanOpenAck(IIBCModule.MsgOnChanOpenAck calldata) external virtual override onlyIBC {}
 
     /**
      * @dev See IIBCModule-onChanOpenConfirm
      *
      * NOTE: You should apply an `onlyIBC` modifier to the function if a derived contract overrides it.
      */
-    function onChanOpenConfirm(string calldata portId, string calldata channelId) external virtual override onlyIBC {}
+    function onChanOpenConfirm(IIBCModule.MsgOnChanOpenConfirm calldata) external virtual override onlyIBC {}
 
     /**
      * @dev See IIBCModule-onChanCloseInit
      *
      * NOTE: You should apply an `onlyIBC` modifier to the function if a derived contract overrides it.
      */
-    function onChanCloseInit(string calldata portId, string calldata channelId) external virtual override onlyIBC {}
+    function onChanCloseInit(IIBCModule.MsgOnChanCloseInit calldata) external virtual override onlyIBC {}
 
     /**
      * @dev See IIBCModule-onChanCloseConfirm
      *
      * NOTE: You should apply an `onlyIBC` modifier to the function if a derived contract overrides it.
      */
-    function onChanCloseConfirm(string calldata portId, string calldata channelId) external virtual override onlyIBC {}
+    function onChanCloseConfirm(IIBCModule.MsgOnChanCloseConfirm calldata) external virtual override onlyIBC {}
 
     /**
      * @dev See IIBCModule-onRecvPacket

contracts/core/05-port/IIBCModule.sol

@@ -6,62 +6,109 @@ import "../../proto/Channel.sol";
 // IIBCModule defines an interface that implements all the callbacks
 // that modules must define as specified in ICS-26
 interface IIBCModule {
-    // onChanOpenInit will verify that the relayer-chosen parameters
-    // are valid and perform any custom INIT logic.
-    // It may return an error if the chosen parameters are invalid
-    // in which case the handshake is aborted.
-    // If the provided version string is non-empty, OnChanOpenInit should return
-    // the version string if valid or an error if the provided version is invalid.
-    // If the version string is empty, OnChanOpenInit is expected to
-    // return a default version string representing the version(s) it supports.
-    // If there is no default version string for the application,
-    // it should return an error if provided version is empty string.
-    function onChanOpenInit(
-        Channel.Order,
-        string[] calldata connectionHops,
-        string calldata portId,
-        string calldata channelId,
-        ChannelCounterparty.Data calldata counterparty,
-        string calldata version
-    ) external returns (string memory);
-
-    // OnChanOpenTry will verify the relayer-chosen parameters along with the
-    // counterparty-chosen version string and perform custom TRY logic.
-    // If the relayer-chosen parameters are invalid, the callback must return
-    // an error to abort the handshake. If the counterparty-chosen version is not
-    // compatible with this modules supported versions, the callback must return
-    // an error to abort the handshake. If the versions are compatible, the try callback
-    // must select the final version string and return it to core IBC.
-    // OnChanOpenTry may also perform custom initialization logic
-    function onChanOpenTry(
-        Channel.Order,
-        string[] calldata connectionHops,
-        string calldata portId,
-        string calldata channelId,
-        ChannelCounterparty.Data calldata counterparty,
-        string calldata counterpartyVersion
-    ) external returns (string memory);
-
-    // OnChanOpenAck will error if the counterparty selected version string
-    // is invalid to abort the handshake. It may also perform custom ACK logic.
-    function onChanOpenAck(string calldata portId, string calldata channelId, string calldata counterpartyVersion)
-        external;
-
-    // OnChanOpenConfirm will perform custom CONFIRM logic and may error to abort the handshake.
-    function onChanOpenConfirm(string calldata portId, string calldata channelId) external;
-
-    function onChanCloseInit(string calldata portId, string calldata channelId) external;
-
-    function onChanCloseConfirm(string calldata portId, string calldata channelId) external;
-
-    // OnRecvPacket must return an acknowledgement that implements the Acknowledgement interface.
-    // In the case of an asynchronous acknowledgement, nil should be returned.
-    // If the acknowledgement returned is successful, the state changes on callback are written,
-    // otherwise the application state changes are discarded. In either case the packet is received
-    // and the acknowledgement is written (in synchronous cases).
+    struct MsgOnChanOpenInit {
+        Channel.Order order;
+        string[] connectionHops;
+        string portId;
+        string channelId;
+        ChannelCounterparty.Data counterparty;
+        string version;
+    }
+
+    struct MsgOnChanOpenTry {
+        Channel.Order order;
+        string[] connectionHops;
+        string portId;
+        string channelId;
+        ChannelCounterparty.Data counterparty;
+        string counterpartyVersion;
+    }
+
+    struct MsgOnChanOpenAck {
+        string portId;
+        string channelId;
+        string counterpartyVersion;
+    }
+
+    struct MsgOnChanOpenConfirm {
+        string portId;
+        string channelId;
+    }
+
+    struct MsgOnChanCloseInit {
+        string portId;
+        string channelId;
+    }
+
+    struct MsgOnChanCloseConfirm {
+        string portId;
+        string channelId;
+    }
+
+    /**
+     * @dev onChanOpenInit will verify that the relayer-chosen parameters
+     * are valid and perform any custom INIT logic.
+     * It may return an error if the chosen parameters are invalid
+     * in which case the handshake is aborted.
+     * If the provided version string is non-empty, OnChanOpenInit should return
+     * the version string if valid or an error if the provided version is invalid.
+     * If the version string is empty, OnChanOpenInit is expected to
+     * return a default version string representing the version(s) it supports.
+     * If there is no default version string for the application,
+     * it should return an error if provided version is empty string.
+     */
+    function onChanOpenInit(MsgOnChanOpenInit calldata msg_) external returns (string memory);
+
+    /**
+     * @dev onChanOpenTry will verify the relayer-chosen parameters along with the
+     * counterparty-chosen version string and perform custom TRY logic.
+     * If the relayer-chosen parameters are invalid, the callback must return
+     * an error to abort the handshake. If the counterparty-chosen version is not
+     * compatible with this modules supported versions, the callback must return
+     * an error to abort the handshake. If the versions are compatible, the try callback
+     * must select the final version string and return it to core IBC.
+     * OnChanOpenTry may also perform custom initialization logic
+     */
+    function onChanOpenTry(MsgOnChanOpenTry calldata msg_) external returns (string memory);
+
+    /**
+     * @dev OnChanOpenAck will error if the counterparty selected version string
+     * is invalid to abort the handshake. It may also perform custom ACK logic.
+     */
+    function onChanOpenAck(MsgOnChanOpenAck calldata msd_) external;
+
+    /**
+     * @dev OnChanOpenConfirm will perform custom CONFIRM logic and may error to abort the handshake.
+     */
+    function onChanOpenConfirm(MsgOnChanOpenConfirm calldata msg_) external;
+
+    /**
+     * @dev OnChanCloseInit will perform custom CLOSE_INIT logic and may error to abort the handshake.
+     * NOTE: If the application does not allow the channel to be closed, this function must revert.
+     */
+    function onChanCloseInit(MsgOnChanCloseInit calldata msg_) external;
+
+    /**
+     * @dev OnChanCloseConfirm will perform custom CLOSE_CONFIRM logic and may error to abort the handshake.
+     */
+    function onChanCloseConfirm(MsgOnChanCloseConfirm calldata msg_) external;
+
+    /**
+     * @dev OnRecvPacket must return an acknowledgement that implements the Acknowledgement interface.
+     * In the case of an asynchronous acknowledgement, nil should be returned.
+     * If the acknowledgement returned is successful, the state changes on callback are written,
+     * otherwise the application state changes are discarded. In either case the packet is received
+     * and the acknowledgement is written (in synchronous cases).
+     */
     function onRecvPacket(Packet.Data calldata, address relayer) external returns (bytes memory);
 
+    /**
+     * @dev onAcknowledgementPacket is called when a packet sent by this module has been acknowledged.
+     */
     function onAcknowledgementPacket(Packet.Data calldata, bytes calldata acknowledgement, address relayer) external;
 
+    /**
+     * @dev onTimeoutPacket is called when a packet sent by this module has timed-out (such that it will not be received on the destination chain).
+     */
     function onTimeoutPacket(Packet.Data calldata, address relayer) external;
 }

contracts/core/25-handler/IBCChannelHandler.sol

@@ -34,12 +34,14 @@ abstract contract IBCChannelHandler is ModuleManager {
 
         IIBCModule module = lookupModuleByPort(msg_.portId);
         version = module.onChanOpenInit(
-            msg_.channel.ordering,
-            msg_.channel.connection_hops,
-            msg_.portId,
-            channelId,
-            msg_.channel.counterparty,
-            msg_.channel.version
+            IIBCModule.MsgOnChanOpenInit({
+                order: msg_.channel.ordering,
+                connectionHops: msg_.channel.connection_hops,
+                portId: msg_.portId,
+                channelId: channelId,
+                counterparty: msg_.channel.counterparty,
+                version: msg_.channel.version
+            })
         );
         claimCapability(channelCapabilityPath(msg_.portId, channelId), address(module));
         emit GeneratedChannelIdentifier(channelId);
@@ -55,12 +57,14 @@ abstract contract IBCChannelHandler is ModuleManager {
         channelId = abi.decode(res, (string));
         IIBCModule module = lookupModuleByPort(msg_.portId);
         version = module.onChanOpenTry(
-            msg_.channel.ordering,
-            msg_.channel.connection_hops,
-            msg_.portId,
-            channelId,
-            msg_.channel.counterparty,
-            msg_.counterpartyVersion
+            IIBCModule.MsgOnChanOpenTry({
+                order: msg_.channel.ordering,
+                connectionHops: msg_.channel.connection_hops,
+                portId: msg_.portId,
+                channelId: channelId,
+                counterparty: msg_.channel.counterparty,
+                counterpartyVersion: msg_.counterpartyVersion
+            })
         );
         claimCapability(channelCapabilityPath(msg_.portId, channelId), address(module));
         emit GeneratedChannelIdentifier(channelId);
@@ -69,21 +73,33 @@ abstract contract IBCChannelHandler is ModuleManager {
 
     function channelOpenAck(IBCMsgs.MsgChannelOpenAck calldata msg_) external {
         ibcChannel.functionDelegateCall(abi.encodeWithSelector(IIBCChannelHandshake.channelOpenAck.selector, msg_));
-        lookupModuleByPort(msg_.portId).onChanOpenAck(msg_.portId, msg_.channelId, msg_.counterpartyVersion);
+        lookupModuleByPort(msg_.portId).onChanOpenAck(
+            IIBCModule.MsgOnChanOpenAck({
+                portId: msg_.portId,
+                channelId: msg_.channelId,
+                counterpartyVersion: msg_.counterpartyVersion
+            })
+        );
     }
 
     function channelOpenConfirm(IBCMsgs.MsgChannelOpenConfirm calldata msg_) external {
         ibcChannel.functionDelegateCall(abi.encodeWithSelector(IIBCChannelHandshake.channelOpenConfirm.selector, msg_));
-        lookupModuleByPort(msg_.portId).onChanOpenConfirm(msg_.portId, msg_.channelId);
+        lookupModuleByPort(msg_.portId).onChanOpenConfirm(
+            IIBCModule.MsgOnChanOpenConfirm({portId: msg_.portId, channelId: msg_.channelId})
+        );
     }
 
     function channelCloseInit(IBCMsgs.MsgChannelCloseInit calldata msg_) external {
         ibcChannel.functionDelegateCall(abi.encodeWithSelector(IIBCChannelHandshake.channelCloseInit.selector, msg_));
-        lookupModuleByPort(msg_.portId).onChanCloseInit(msg_.portId, msg_.channelId);
+        lookupModuleByPort(msg_.portId).onChanCloseInit(
+            IIBCModule.MsgOnChanCloseInit({portId: msg_.portId, channelId: msg_.channelId})
+        );
     }
 
     function channelCloseConfirm(IBCMsgs.MsgChannelCloseConfirm calldata msg_) external {
         ibcChannel.functionDelegateCall(abi.encodeWithSelector(IIBCChannelHandshake.channelCloseConfirm.selector, msg_));
-        lookupModuleByPort(msg_.portId).onChanCloseConfirm(msg_.portId, msg_.channelId);
+        lookupModuleByPort(msg_.portId).onChanCloseConfirm(
+            IIBCModule.MsgOnChanCloseConfirm({portId: msg_.portId, channelId: msg_.channelId})
+        );
     }
 }