Fix audit suggestions

contracts/core/03-connection/IBCConnection.sol

@@ -43,6 +43,10 @@ abstract contract IBCConnection is IBCHost, IBCSelfStateValidator, IIBCConnectio
             }
             connection.versions.push(msg_.version);
         } else {
+            // if the version features are empty, the version identifier should also be empty
+            if (bytes(msg_.version.identifier).length > 0) {
+                revert IBCConnectionVersionIdentifierNotEmpty();
+            }
             IBCConnectionLib.setSupportedVersions(getCompatibleVersions(), connection.versions);
         }
 

contracts/core/03-connection/IIBCConnection.sol

@@ -7,40 +7,80 @@ import {Version, Counterparty} from "../../proto/Connection.sol";
 interface IIBCConnection {
     // --------------------- Data Structure --------------------- //
 
+    /**
+     * @dev MsgConnectionOpenInit defines the msg sent by an account on Chain A to initialize a connection with Chain B.
+     * @param clientId of Chain B
+     * @param counterparty is composed of clientID of Chain A on Chain B and the prefix of Chain B
+     * @param version chosen by an account on Chain A
+     * @param delayPeriod is set to the connection delay time period
+     */
     struct MsgConnectionOpenInit {
         string clientId;
         Counterparty.Data counterparty;
         Version.Data version;
         uint64 delayPeriod;
     }
 
+    /**
+     * @dev MsgConnectionOpenTry defines a msg sent by an account on Chain B to try to open a connection on Chain B.
+     * @param counterparty is composed of clientID of Chain A on Chain B and the prefix of Chain B
+     * @param delayPeriod chosen by Chain A in `connectionOpenInit`
+     * @param clientId of Chain A
+     * @param clientStateBytes clientState that Chain A has for Chain B
+     * @param counterpartyVersions supported versions of Chain A
+     * @param proofInit proof that Chain A stored connectionEnd in state (on connectionOpenInit)
+     * @param proofClient proof that Chain A stored a light client of Chain B
+     * @param proofConsensus proof that Chain A stored Chain B's consensus state at consensus height
+     * @param proofHeight height at which relayer constructs proof of A storing connectionEnd in state
+     * @param consensusHeight latest height of Chain B which Chain A has stored in its Chain B client
+     * @param hostConsensusStateProof proof data for the consensus state of Chain B
+     */
     struct MsgConnectionOpenTry {
-        Counterparty.Data counterparty; // counterpartyConnectionIdentifier, counterpartyPrefix and counterpartyClientIdentifier
+        Counterparty.Data counterparty;
         uint64 delayPeriod;
-        string clientId; // clientID of chainA
-        bytes clientStateBytes; // clientState that chainA has for chainB
-        Version.Data[] counterpartyVersions; // supported versions of chain A
-        bytes proofInit; // proof that chainA stored connectionEnd in state (on ConnOpenInit)
-        bytes proofClient; // proof that chainA stored a light client of chainB
-        bytes proofConsensus; // proof that chainA stored chainB's consensus state at consensus height
-        Height.Data proofHeight; // height at which relayer constructs proof of A storing connectionEnd in state
-        Height.Data consensusHeight; // latest height of chain B which chain A has stored in its chain B client
-        bytes hostConsensusStateProof; // optional proof data for host state machines that are unable to introspect their own consensus state
+        string clientId;
+        bytes clientStateBytes;
+        Version.Data[] counterpartyVersions;
+        bytes proofInit;
+        bytes proofClient;
+        bytes proofConsensus;
+        Height.Data proofHeight;
+        Height.Data consensusHeight;
+        bytes hostConsensusStateProof;
     }
 
+    /**
+     * @dev MsgConnectionOpenAck defines a msg sent by an account on Chain A to acknowledge the change of connection state to TRYOPEN on Chain B.
+     * @param connectionId identifier of the connection generated at `connectionOpenInit`
+     * @param clientStateBytes clientState that Chain B has for Chain A
+     * @param version chosen by Chain B in `connectionOpenTry`
+     * @param counterpartyConnectionId identifier of the connection on Chain B
+     * @param proofTry proof that Chain B stored connectionEnd in state
+     * @param proofClient proof that Chain B stored a light client of Chain A
+     * @param proofConsensus proof that Chain B stored Chain A's consensus state at consensus height
+     * @param proofHeight height at which relayer constructed proof of B storing connectionEnd in state
+     * @param consensusHeight latest height of Chain A which Chain B has stored in its Chain A client
+     * @param hostConsensusStateProof proof data for the consensus state of Chain A
+     */
     struct MsgConnectionOpenAck {
         string connectionId;
-        bytes clientStateBytes; // client state for chainA on chainB
-        Version.Data version; // version that ChainB chose in ConnOpenTry
+        bytes clientStateBytes;
+        Version.Data version;
         string counterpartyConnectionId;
-        bytes proofTry; // proof that connectionEnd was added to ChainB state in ConnOpenTry
-        bytes proofClient; // proof of client state on chainB for chainA
-        bytes proofConsensus; // proof that chainB has stored ConsensusState of chainA on its client
-        Height.Data proofHeight; // height that relayer constructed proofTry
-        Height.Data consensusHeight; // latest height of chainA that chainB has stored on its chainA client
-        bytes hostConsensusStateProof; // optional proof data for host state machines that are unable to introspect their own consensus state
+        bytes proofTry;
+        bytes proofClient;
+        bytes proofConsensus;
+        Height.Data proofHeight;
+        Height.Data consensusHeight;
+        bytes hostConsensusStateProof;
     }
 
+    /**
+     * @dev MsgConnectionOpenConfirm defines a msg sent by an account on Chain B to acknowledge the change of connection state to OPEN on Chain A.
+     * @param connectionId identifier of the connection generated at `connectionOpenTry`
+     * @param proofAck proof that Chain A stored connectionEnd in state
+     * @param proofHeight height at which relayer constructed proof of A storing connectionEnd in state
+     */
     struct MsgConnectionOpenConfirm {
         string connectionId;
         bytes proofAck;
@@ -54,26 +94,26 @@ interface IIBCConnection {
     event GeneratedConnectionIdentifier(string connectionId);
 
     /**
-     * @dev connectionOpenInit initialises a connection attempt on chain A. The generated connection identifier
+     * @dev connectionOpenInit initialises a connection attempt on Chain A. The generated connection identifier
      * is returned.
      */
     function connectionOpenInit(MsgConnectionOpenInit calldata msg_) external returns (string memory connectionId);
 
     /**
-     * @dev connectionOpenTry relays notice of a connection attempt on chain A to chain B (this
-     * code is executed on chain B).
+     * @dev connectionOpenTry relays notice of a connection attempt on Chain A to Chain B (this
+     * code is executed on Chain B).
      */
     function connectionOpenTry(MsgConnectionOpenTry calldata msg_) external returns (string memory);
 
     /**
-     * @dev connectionOpenAck relays acceptance of a connection open attempt from chain B back
-     * to chain A (this code is executed on chain A).
+     * @dev connectionOpenAck relays acceptance of a connection open attempt from Chain B back
+     * to Chain A (this code is executed on Chain A).
      */
     function connectionOpenAck(MsgConnectionOpenAck calldata msg_) external;
 
     /**
-     * @dev connectionOpenConfirm confirms opening of a connection on chain A to chain B, after
-     * which the connection is open on both chains (this code is executed on chain B).
+     * @dev connectionOpenConfirm confirms opening of a connection on Chain A to Chain B, after
+     * which the connection is open on both chains (this code is executed on Chain B).
      */
     function connectionOpenConfirm(MsgConnectionOpenConfirm calldata msg_) external;
 }

contracts/core/03-connection/IIBCConnectionErrors.sol

@@ -18,6 +18,8 @@ interface IIBCConnectionErrors {
 
     error IBCConnectionIBCVersionNotSupported();
 
+    error IBCConnectionVersionIdentifierNotEmpty();
+
     error IBCConnectionInvalidSelfClientState();
 
     error IBCConnectionInvalidHostConsensusStateProof();

contracts/core/04-channel/IBCChannelHandshake.sol

@@ -199,7 +199,7 @@ contract IBCChannelHandshake is IBCModuleManager, IIBCChannelHandshake, IIBCChan
     }
 
     /**
-     * @dev channelOpenConfirm is called by the counterparty module to close their end of the channel, since the other end has been closed.
+     * @dev channelOpenConfirm is called by the counterparty module to acknowledge the acknowledgement of the handshake-originating module on the other chain and finish the channel opening handshake.
      */
     function channelOpenConfirm(IIBCChannelHandshake.MsgChannelOpenConfirm calldata msg_) public override {
         Channel.Data storage channel = getChannelStorage()[msg_.portId][msg_.channelId].channel;

contracts/core/04-channel/IBCChannelPacketSendRecv.sol

@@ -175,7 +175,12 @@ contract IBCChannelPacketSendRecv is
             commitments[commitmentKey] = IBCChannelLib.PACKET_RECEIPT_SUCCESSFUL_KECCAK256;
         } else if (channel.ordering == Channel.Order.ORDER_ORDERED) {
             if (channelStorage.nextSequenceRecv != msg_.packet.sequence) {
-                revert IBCChannelUnexpectedNextSequenceRecv(channelStorage.nextSequenceRecv);
+                revert IBCChannelUnexpectedNextSequenceRecv(
+                    msg_.packet.destinationPort,
+                    msg_.packet.destinationChannel,
+                    msg_.packet.sequence,
+                    channelStorage.nextSequenceRecv
+                );
             }
             channelStorage.nextSequenceRecv++;
             getCommitments()[IBCCommitment.nextSequenceRecvCommitmentKeyCalldata(

contracts/core/04-channel/IBCChannelPacketTimeout.sol

@@ -302,7 +302,7 @@ contract IBCChannelPacketTimeout is IBCChannelUpgradeBase, IIBCChannelPacketTime
         lookupModuleByChannel(msg_.packet.sourcePort, msg_.packet.sourceChannel).onTimeoutPacket(
             msg_.packet, _msgSender()
         );
-        emit TimeoutPacket(msg_.packet);
+        emit TimeoutOnClose(msg_.packet);
     }
 
     /**

contracts/core/04-channel/IIBCChannel.sol

@@ -240,6 +240,8 @@ interface IIBCChannelPacketTimeout {
 
     /// @notice event emitted upon timeout of a packet
     event TimeoutPacket(Packet packet);
+    /// @notice event emitted upon timeout of a packet due to the counterparty's channel being closed
+    event TimeoutOnClose(Packet packet);
 
     // --------------------- Functions --------------------- //
 

contracts/core/04-channel/IIBCChannelErrors.sol

@@ -84,8 +84,13 @@ interface IIBCChannelErrors {
     /// @param sequence packet sequence
     error IBCChannelPacketReceiptAlreadyExists(string destinationPort, string destinationChannel, uint64 sequence);
 
+    /// @param destinationPort destination port
+    /// @param destinationChannel destination channel
     /// @param expected expected sequence
-    error IBCChannelUnexpectedNextSequenceRecv(uint64 expected);
+    /// @param sequence packet sequence
+    error IBCChannelUnexpectedNextSequenceRecv(
+        string destinationPort, string destinationChannel, uint64 sequence, uint64 expected
+    );
 
     /// @param portId port identifier
     /// @param channelId channel identifier

contracts/core/24-host/IBCHostConfigurator.sol

@@ -1,12 +1,11 @@
 // SPDX-License-Identifier: Apache-2.0
 pragma solidity ^0.8.20;
 
-import {ERC165Checker} from "@openzeppelin/contracts/utils/introspection/ERC165Checker.sol";
 import {IBCClientLib} from "../02-client/IBCClientLib.sol";
 import {ILightClient} from "../02-client/ILightClient.sol";
 import {IBCHostLib} from "./IBCHostLib.sol";
 import {IBCModuleManager} from "../26-router/IBCModuleManager.sol";
-import {IIBCModule, IIBCModuleInitializer} from "../26-router/IIBCModule.sol";
+import {IIBCModuleInitializer} from "../26-router/IIBCModule.sol";
 import {IIBCHostConfigurator} from "./IIBCHostConfigurator.sol";
 
 /**
@@ -31,28 +30,9 @@ abstract contract IBCHostConfigurator is IIBCHostConfigurator, IBCModuleManager
     }
 
     function _bindPort(string calldata portId, IIBCModuleInitializer module) internal virtual {
-        address moduleAddress = address(module);
         if (!IBCHostLib.validatePortIdentifier(bytes(portId))) {
             revert IBCHostInvalidPortIdentifier(portId);
         }
-        if (moduleAddress == address(0) || moduleAddress == address(this)) {
-            revert IBCHostInvalidModuleAddress(moduleAddress);
-        }
-        if (!ERC165Checker.supportsERC165(moduleAddress)) {
-            revert IBCHostModuleDoesNotSupportERC165();
-        }
-        if (
-            !(
-                ERC165Checker.supportsERC165InterfaceUnchecked(moduleAddress, type(IIBCModule).interfaceId)
-                    || ERC165Checker.supportsERC165InterfaceUnchecked(
-                        moduleAddress, type(IIBCModuleInitializer).interfaceId
-                    )
-            )
-        ) {
-            revert IBCHostModuleDoesNotSupportIIBCModuleInitializer(
-                moduleAddress, type(IIBCModuleInitializer).interfaceId
-            );
-        }
-        claimPortCapability(portId, moduleAddress);
+        claimPortCapability(portId, address(module));
     }
 }

contracts/core/25-handler/IBCClientConnectionChannelHandler.sol

@@ -66,6 +66,14 @@ abstract contract IBCClientConnectionChannelHandler is
         IIBCChannelUpgradeInitTryAck ibcChannelUpgradeInitTryAck_,
         IIBCChannelUpgradeConfirmOpenTimeoutCancel ibcChannelUpgradeConfirmOpenTimeoutCancel_
     ) {
+        if (
+            address(ibcClient_) == address(0) || address(ibcConnection_) == address(0)
+                || address(ibcChannelHandshake_) == address(0) || address(ibcChannelPacketSendRecv_) == address(0)
+                || address(ibcChannelPacketTimeout_) == address(0) || address(ibcChannelUpgradeInitTryAck_) == address(0)
+                || address(ibcChannelUpgradeConfirmOpenTimeoutCancel_) == address(0)
+        ) {
+            revert("invalid address");
+        }
         ibcClient = address(ibcClient_);
         ibcConnection = address(ibcConnection_);
         ibcChannelHandshake = address(ibcChannelHandshake_);

contracts/core/26-router/IBCModuleManager.sol

@@ -21,7 +21,7 @@ contract IBCModuleManager is Context, IBCHost, IIBCModuleManager {
         if (hostStorage.portCapabilities[portId] != address(0)) {
             revert IBCHostPortCapabilityAlreadyClaimed(portId);
         }
-        if (module == address(0)) {
+        if (module == address(0) || module == address(this)) {
             revert IBCHostInvalidModuleAddress(module);
         }
         if (!ERC165Checker.supportsERC165(module)) {

tests/foundry/src/ICS03Version.t.sol

@@ -12,6 +12,9 @@ contract TestICS03Version is ICS03TestHelper {
         version = Version.Data({identifier: "1", features: new string[](1)});
         version.features[0] = "ORDER_DAG";
         assertFalse(IBCConnectionLib.isSupportedVersion(versions, version));
+        version = getConnectionVersions()[0];
+        version.identifier = "";
+        assertFalse(IBCConnectionLib.isSupportedVersion(versions, version));
     }
 
     function testFindSupportedVersion() public {