diff --git a/contracts/core/03-connection/IBCConnection.sol b/contracts/core/03-connection/IBCConnection.sol index 372a6215..cc2ce6d1 100644 --- a/contracts/core/03-connection/IBCConnection.sol +++ b/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); } diff --git a/contracts/core/03-connection/IIBCConnection.sol b/contracts/core/03-connection/IIBCConnection.sol index 1c5c45e3..de8fbe5e 100644 --- a/contracts/core/03-connection/IIBCConnection.sol +++ b/contracts/core/03-connection/IIBCConnection.sol @@ -7,6 +7,13 @@ 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; @@ -14,33 +21,66 @@ interface IIBCConnection { 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; } diff --git a/contracts/core/03-connection/IIBCConnectionErrors.sol b/contracts/core/03-connection/IIBCConnectionErrors.sol index 2cd91356..b0191fc2 100644 --- a/contracts/core/03-connection/IIBCConnectionErrors.sol +++ b/contracts/core/03-connection/IIBCConnectionErrors.sol @@ -18,6 +18,8 @@ interface IIBCConnectionErrors { error IBCConnectionIBCVersionNotSupported(); + error IBCConnectionVersionIdentifierNotEmpty(); + error IBCConnectionInvalidSelfClientState(); error IBCConnectionInvalidHostConsensusStateProof(); diff --git a/contracts/core/04-channel/IBCChannelHandshake.sol b/contracts/core/04-channel/IBCChannelHandshake.sol index 10208a78..4cc64e96 100644 --- a/contracts/core/04-channel/IBCChannelHandshake.sol +++ b/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; diff --git a/contracts/core/04-channel/IBCChannelPacketSendRecv.sol b/contracts/core/04-channel/IBCChannelPacketSendRecv.sol index 7a17d207..acba3faa 100644 --- a/contracts/core/04-channel/IBCChannelPacketSendRecv.sol +++ b/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( diff --git a/contracts/core/04-channel/IBCChannelPacketTimeout.sol b/contracts/core/04-channel/IBCChannelPacketTimeout.sol index 677ed578..01cd9814 100644 --- a/contracts/core/04-channel/IBCChannelPacketTimeout.sol +++ b/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); } /** diff --git a/contracts/core/04-channel/IIBCChannel.sol b/contracts/core/04-channel/IIBCChannel.sol index 7d00a1b2..09ad1aaa 100644 --- a/contracts/core/04-channel/IIBCChannel.sol +++ b/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 --------------------- // diff --git a/contracts/core/04-channel/IIBCChannelErrors.sol b/contracts/core/04-channel/IIBCChannelErrors.sol index 54df1e6c..aea595c1 100644 --- a/contracts/core/04-channel/IIBCChannelErrors.sol +++ b/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 diff --git a/contracts/core/24-host/IBCHostConfigurator.sol b/contracts/core/24-host/IBCHostConfigurator.sol index 7c5ab986..41dc1260 100644 --- a/contracts/core/24-host/IBCHostConfigurator.sol +++ b/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)); } } diff --git a/contracts/core/25-handler/IBCClientConnectionChannelHandler.sol b/contracts/core/25-handler/IBCClientConnectionChannelHandler.sol index 7702f8aa..822d4fc1 100644 --- a/contracts/core/25-handler/IBCClientConnectionChannelHandler.sol +++ b/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_); diff --git a/contracts/core/26-router/IBCModuleManager.sol b/contracts/core/26-router/IBCModuleManager.sol index 4f19817c..35d4f139 100644 --- a/contracts/core/26-router/IBCModuleManager.sol +++ b/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)) { diff --git a/tests/foundry/src/ICS03Version.t.sol b/tests/foundry/src/ICS03Version.t.sol index 7fda95bf..de8c09ff 100644 --- a/tests/foundry/src/ICS03Version.t.sol +++ b/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 {