From fa7b730f5aa93cf91f21da8e27a9b9c23d20681f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Pernas=20Maradei?= Date: Tue, 12 Mar 2024 19:22:12 +0100 Subject: [PATCH] revert api split changes This reverts commit 7777f17042a35c41eb597cf61c9676e1a0ae70fc. --- contracts/core/Dispatcher.sol | 158 +++++++++------------ contracts/core/UniversalChannelHandler.sol | 65 ++++----- contracts/examples/Mars.sol | 84 ++++++----- contracts/interfaces/IbcDispatcher.sol | 22 +-- contracts/interfaces/IbcReceiver.sol | 18 ++- test/Dispatcher.base.t.sol | 73 +++------- test/Dispatcher.proof.t.sol | 19 +-- test/Dispatcher.t.sol | 60 ++++---- test/VirtualChain.sol | 73 ++++------ test/universal.channel.t.sol | 4 +- 10 files changed, 244 insertions(+), 332 deletions(-) diff --git a/contracts/core/Dispatcher.sol b/contracts/core/Dispatcher.sol index 055b89f5..9ed41c5b 100644 --- a/contracts/core/Dispatcher.sol +++ b/contracts/core/Dispatcher.sol @@ -68,35 +68,13 @@ contract Dispatcher is IbcDispatcher, IbcEventsEmitter, Ownable, Ibc { } /** - * This function is called by a 'relayer' on behalf of a dApp. The dApp should implement IbcChannelHandler's - * onChanOpenInit. If the callback succeeds, the dApp should return the selected version and the emitted event - * will be relayed to the IBC/VIBC hub chain. + * This func is called by a 'relayer' on behalf of a dApp. The dApp should be implements IbcChannelHandler. + * The dApp should implement the onOpenIbcChannel method to handle one of the first two channel handshake methods, + * ie. ChanOpenInit or ChanOpenTry. + * If callback succeeds, the dApp should return the selected version, and an emitted event will be relayed to the + * IBC/VIBC hub chain. */ - function channelOpenInit( - IbcChannelReceiver portAddress, - string calldata version, - ChannelOrder ordering, - bool feeEnabled, - string[] calldata connectionHops, - string calldata counterpartyPortId - ) external { - if (bytes(counterpartyPortId).length == 0) { - revert IBCErrors.invalidCounterPartyPortId(); - } - - string memory selectedVersion = portAddress.onChanOpenInit(version); - - emit ChannelOpenInit( - address(portAddress), selectedVersion, ordering, feeEnabled, connectionHops, counterpartyPortId - ); - } - - /** - * This function is called by a 'relayer' on behalf of a dApp. The dApp should implement IbcChannelHandler's - * onChanOpenTry. If the callback succeeds, the dApp should return the selected version and the emitted event - * will be relayed to the IBC/VIBC hub chain. - */ - function channelOpenTry( + function openIbcChannel( IbcChannelReceiver portAddress, CounterParty calldata local, ChannelOrder ordering, @@ -109,15 +87,18 @@ contract Dispatcher is IbcDispatcher, IbcEventsEmitter, Ownable, Ibc { revert IBCErrors.invalidCounterPartyPortId(); } - consensusStateManager.verifyMembership( - proof, - channelProofKey(local.portId, local.channelId), - channelProofValue(ChannelState.TRY_PENDING, ordering, local.version, connectionHops, counterparty) - ); + if (_isChannelOpenTry(counterparty)) { + consensusStateManager.verifyMembership( + proof, + channelProofKey(local.portId, local.channelId), + channelProofValue(ChannelState.TRY_PENDING, ordering, local.version, connectionHops, counterparty) + ); + } - string memory selectedVersion = portAddress.onChanOpenTry(counterparty.version); + string memory selectedVersion = + portAddress.onOpenIbcChannel(local.version, ordering, feeEnabled, connectionHops, counterparty); - emit ChannelOpenTry( + emit OpenIbcChannel( address(portAddress), selectedVersion, ordering, @@ -129,56 +110,43 @@ contract Dispatcher is IbcDispatcher, IbcEventsEmitter, Ownable, Ibc { } /** - * This func is called by a 'relayer' after the IBC/VIBC hub chain has processed the ChannelOpenTry event. - * The dApp should implement the onChannelConnect method to handle the third channel handshake method: ChanOpenAck + * This func is called by a 'relayer' after the IBC/VIBC hub chain has processed the onOpenIbcChannel event. + * The dApp should implement the onConnectIbcChannel method to handle the last two channel handshake methods, ie. + * ChanOpenAck or ChanOpenConfirm. */ - function channelOpenAck( + function connectIbcChannel( IbcChannelReceiver portAddress, CounterParty calldata local, string[] calldata connectionHops, ChannelOrder ordering, bool feeEnabled, + bool isChanConfirm, CounterParty calldata counterparty, Ics23Proof calldata proof ) external { - consensusStateManager.verifyMembership( - proof, - channelProofKey(local.portId, local.channelId), - channelProofValue(ChannelState.ACK_PENDING, ordering, local.version, connectionHops, counterparty) - ); - - _connectChannel(portAddress, local, connectionHops, ordering, feeEnabled, counterparty); - - portAddress.onChanOpenAck(local.channelId, counterparty.version); + _verifyConnectIbcChannelProof(local, connectionHops, ordering, isChanConfirm, counterparty, proof); - emit ChannelOpenAck(address(portAddress), local.channelId); - } + portAddress.onConnectIbcChannel(local.channelId, counterparty.channelId, counterparty.version); - /** - * This func is called by a 'relayer' after the IBC/VIBC hub chain has processed the ChannelOpenTry event. - * The dApp should implement the onChannelConnect method to handle the last channel handshake method: - * ChannelOpenConfirm - */ - function channelOpenConfirm( - IbcChannelReceiver portAddress, - CounterParty calldata local, - string[] calldata connectionHops, - ChannelOrder ordering, - bool feeEnabled, - CounterParty calldata counterparty, - Ics23Proof calldata proof - ) external { - consensusStateManager.verifyMembership( - proof, - channelProofKey(local.portId, local.channelId), - channelProofValue(ChannelState.CONFIRM_PENDING, ordering, local.version, connectionHops, counterparty) + // Register port and channel mapping + // TODO: check duplicated channel registration? + // TODO: The call to `Channel` constructor MUST be move to `openIbcChannel` phase + // Then `connectIbcChannel` phase can use the `version` as part of `require` condition. + portChannelMap[address(portAddress)][local.channelId] = Channel( + counterparty.version, // TODO: this should be self version instead of counterparty version + ordering, + feeEnabled, + connectionHops, + counterparty.portId, + counterparty.channelId ); - _connectChannel(portAddress, local, connectionHops, ordering, feeEnabled, counterparty); - - portAddress.onChanOpenConfirm(local.channelId, counterparty.version); + // initialize channel sequences + nextSequenceSend[address(portAddress)][local.channelId] = 1; + nextSequenceRecv[address(portAddress)][local.channelId] = 1; + nextSequenceAck[address(portAddress)][local.channelId] = 1; - emit ChannelOpenConfirm(address(portAddress), local.channelId); + emit ConnectIbcChannel(address(portAddress), local.channelId); } /** @@ -485,31 +453,25 @@ contract Dispatcher is IbcDispatcher, IbcEventsEmitter, Ownable, Ibc { emit SendPacket(sender, channelId, packet, sequence, timeoutTimestamp); } - function _connectChannel( - IbcChannelReceiver portAddress, + function _verifyConnectIbcChannelProof( CounterParty calldata local, string[] calldata connectionHops, ChannelOrder ordering, - bool feeEnabled, - CounterParty calldata counterparty + bool isChanConfirm, + CounterParty calldata counterparty, + Ics23Proof calldata proof ) internal { - // Register port and channel mapping - // TODO: check duplicated channel registration? - // TODO: The call to `Channel` constructor MUST be move to `openIbcChannel` phase - // Then `connectIbcChannel` phase can use the `version` as part of `require` condition. - portChannelMap[address(portAddress)][local.channelId] = Channel( - counterparty.version, // TODO: this should be self version instead of counterparty version - ordering, - feeEnabled, - connectionHops, - counterparty.portId, - counterparty.channelId + consensusStateManager.verifyMembership( + proof, + channelProofKey(local.portId, local.channelId), + channelProofValue( + isChanConfirm ? ChannelState.CONFIRM_PENDING : ChannelState.ACK_PENDING, + ordering, + local.version, + connectionHops, + counterparty + ) ); - - // initialize channel sequences - nextSequenceSend[address(portAddress)][local.channelId] = 1; - nextSequenceRecv[address(portAddress)][local.channelId] = 1; - nextSequenceAck[address(portAddress)][local.channelId] = 1; } // _isPacketTimeout returns true if the given packet has timed out acoording to host chain's block height and @@ -553,4 +515,18 @@ contract Dispatcher is IbcDispatcher, IbcEventsEmitter, Ownable, Ibc { addr := mload(add(addrBytes, 20)) } } + + // For XXXX => vIBC direction, SC needs to verify the proof of membership of TRY_PENDING + // For vIBC initiated channel, SC doesn't need to verify any proof, and these should be all empty + function _isChannelOpenTry(CounterParty calldata counterparty) internal pure returns (bool open) { + if (counterparty.channelId == bytes32(0) && bytes(counterparty.version).length == 0) { + open = false; + // ChanOpenInit with unknow conterparty + } else if (counterparty.channelId != bytes32(0) && bytes(counterparty.version).length != 0) { + // this is the ChanOpenTry; counterparty must not be zero-value + open = true; + } else { + revert IBCErrors.invalidCounterParty(); + } + } } diff --git a/contracts/core/UniversalChannelHandler.sol b/contracts/core/UniversalChannelHandler.sol index 2ab9b6b5..058f6cb0 100644 --- a/contracts/core/UniversalChannelHandler.sol +++ b/contracts/core/UniversalChannelHandler.sol @@ -33,6 +33,17 @@ contract UniversalChannelHandler is IbcReceiverBase, IbcUniversalChannelMW { dispatcher.closeIbcChannel(channelId); } + // IBC callback functions + function onConnectIbcChannel(bytes32 channelId, bytes32, string calldata counterpartyVersion) + external + onlyIbcDispatcher + { + if (keccak256(abi.encodePacked(counterpartyVersion)) != keccak256(abi.encodePacked(VERSION))) { + revert UnsupportedVersion(); + } + connectedChannels.push(channelId); + } + function onCloseIbcChannel(bytes32 channelId, string calldata, bytes32) external onlyIbcDispatcher { // logic to determin if the channel should be closed bool channelFound = false; @@ -138,43 +149,23 @@ contract UniversalChannelHandler is IbcReceiverBase, IbcUniversalChannelMW { mwStackAddrs[mwBitmap] = mwAddrs; } - // IBC callback functions - function onChanOpenAck(bytes32 channelId, string calldata counterpartyVersion) external onlyIbcDispatcher { - _connectChannel(channelId, counterpartyVersion); - } - - function onChanOpenConfirm(bytes32 channelId, string calldata counterpartyVersion) external onlyIbcDispatcher { - _connectChannel(channelId, counterpartyVersion); - } - - function onChanOpenInit(string calldata version) - external - view - onlyIbcDispatcher - returns (string memory selectedVersion) - { - return _openChannel(version); - } - - function onChanOpenTry(string calldata counterpartyVersion) - external - view - onlyIbcDispatcher - returns (string memory selectedVersion) - { - return _openChannel(counterpartyVersion); - } - - function _connectChannel(bytes32 channelId, string calldata version) private { - if (keccak256(abi.encodePacked(version)) != keccak256(abi.encodePacked(VERSION))) { - revert UnsupportedVersion(); - } - connectedChannels.push(channelId); - } - - function _openChannel(string calldata version) private pure returns (string memory selectedVersion) { - if (keccak256(abi.encodePacked(version)) != keccak256(abi.encodePacked(VERSION))) { - revert UnsupportedVersion(); + function onOpenIbcChannel( + string calldata version, + ChannelOrder, + bool, + string[] calldata, + CounterParty calldata counterparty + ) external view onlyIbcDispatcher returns (string memory selectedVersion) { + if (counterparty.channelId == bytes32(0)) { + // ChanOpenInit + if (keccak256(abi.encodePacked(version)) != keccak256(abi.encodePacked(VERSION))) { + revert UnsupportedVersion(); + } + } else { + // ChanOpenTry + if (keccak256(abi.encodePacked(counterparty.version)) != keccak256(abi.encodePacked(VERSION))) { + revert UnsupportedVersion(); + } } return VERSION; } diff --git a/contracts/examples/Mars.sol b/contracts/examples/Mars.sol index 7cd091cd..41e2f70b 100644 --- a/contracts/examples/Mars.sol +++ b/contracts/examples/Mars.sol @@ -34,6 +34,22 @@ contract Mars is IbcReceiverBase, IbcReceiver { timeoutPackets.push(packet); } + function onConnectIbcChannel(bytes32 channelId, bytes32, string calldata counterpartyVersion) + external + onlyIbcDispatcher + { + // ensure negotiated version is supported + bool foundVersion = false; + for (uint256 i = 0; i < supportedVersions.length; i++) { + if (keccak256(abi.encodePacked(counterpartyVersion)) == keccak256(abi.encodePacked(supportedVersions[i]))) { + foundVersion = true; + break; + } + } + if (!foundVersion) revert UnsupportedVersion(); + connectedChannels.push(channelId); + } + function onCloseIbcChannel(bytes32 channelId, string calldata, bytes32) external onlyIbcDispatcher { // logic to determin if the channel should be closed bool channelFound = false; @@ -65,49 +81,39 @@ contract Mars is IbcReceiverBase, IbcReceiver { dispatcher.sendPacket(channelId, bytes(message), timeoutTimestamp); } - function onChanOpenAck(bytes32 channelId, string calldata counterpartyVersion) external onlyIbcDispatcher { - _connectChannel(channelId, counterpartyVersion); - } - - function onChanOpenConfirm(bytes32 channelId, string calldata counterpartyVersion) external onlyIbcDispatcher { - _connectChannel(channelId, counterpartyVersion); - } - - function onChanOpenInit(string calldata version) - external - view - onlyIbcDispatcher - returns (string memory selectedVersion) - { - return _openChannel(version); - } - - function onChanOpenTry(string calldata counterpartyVersion) - external - view - onlyIbcDispatcher - returns (string memory selectedVersion) - { - return _openChannel(counterpartyVersion); - } - - function _connectChannel(bytes32 channelId, string calldata counterpartyVersion) private { - // ensure negotiated version is supported + function onOpenIbcChannel( + string calldata version, + ChannelOrder, + bool, + string[] calldata, + CounterParty calldata counterparty + ) external view onlyIbcDispatcher returns (string memory selectedVersion) { + if (bytes(counterparty.portId).length <= 8) { + revert IBCErrors.invalidCounterPartyPortId(); + } + /** + * Version selection is determined by if the callback is invoked on behalf of ChanOpenInit or ChanOpenTry. + * ChanOpenInit: self version should be provided whereas the counterparty version is empty. + * ChanOpenTry: counterparty version should be provided whereas the self version is empty. + * In both cases, the selected version should be in the supported versions list. + */ + bool foundVersion = false; + selectedVersion = + keccak256(abi.encodePacked(version)) == keccak256(abi.encodePacked("")) ? counterparty.version : version; for (uint256 i = 0; i < supportedVersions.length; i++) { - if (keccak256(abi.encodePacked(counterpartyVersion)) == keccak256(abi.encodePacked(supportedVersions[i]))) { - connectedChannels.push(channelId); - return; + if (keccak256(abi.encodePacked(selectedVersion)) == keccak256(abi.encodePacked(supportedVersions[i]))) { + foundVersion = true; + break; } } - revert UnsupportedVersion(); - } - - function _openChannel(string calldata version) private view returns (string memory selectedVersion) { - for (uint256 i = 0; i < supportedVersions.length; i++) { - if (keccak256(abi.encodePacked(version)) == keccak256(abi.encodePacked(supportedVersions[i]))) { - return version; + if (!foundVersion) revert UnsupportedVersion(); + // if counterpartyVersion is not empty, then it must be the same foundVersion + if (keccak256(abi.encodePacked(counterparty.version)) != keccak256(abi.encodePacked(""))) { + if (keccak256(abi.encodePacked(counterparty.version)) != keccak256(abi.encodePacked(selectedVersion))) { + revert VersionMismatch(); } } - revert UnsupportedVersion(); + + return selectedVersion; } } diff --git a/contracts/interfaces/IbcDispatcher.sol b/contracts/interfaces/IbcDispatcher.sol index a2e2d540..3c30210e 100644 --- a/contracts/interfaces/IbcDispatcher.sol +++ b/contracts/interfaces/IbcDispatcher.sol @@ -23,13 +23,14 @@ interface IbcPacketSender { * Other features are implemented as callback methods in the IbcReceiver interface. */ interface IbcDispatcher is IbcPacketSender { - function channelOpenInit( + function openIbcChannel( IbcChannelReceiver portAddress, - string calldata version, + CounterParty calldata self, ChannelOrder ordering, bool feeEnabled, string[] calldata connectionHops, - string calldata counterpartyPortId + CounterParty calldata counterparty, + Ics23Proof calldata proof ) external; function closeIbcChannel(bytes32 channelId) external; @@ -45,16 +46,7 @@ interface IbcEventsEmitter { // // channel events // - event ChannelOpenInit( - address indexed portAddress, - string version, - ChannelOrder ordering, - bool feeEnabled, - string[] connectionHops, - string counterpartyPortId - ); - - event ChannelOpenTry( + event OpenIbcChannel( address indexed portAddress, string version, ChannelOrder ordering, @@ -64,9 +56,7 @@ interface IbcEventsEmitter { bytes32 counterpartyChannelId ); - event ChannelOpenAck(address indexed portAddress, bytes32 channelId); - - event ChannelOpenConfirm(address indexed portAddress, bytes32 channelId); + event ConnectIbcChannel(address indexed portAddress, bytes32 channelId); event CloseIbcChannel(address indexed portAddress, bytes32 indexed channelId); diff --git a/contracts/interfaces/IbcReceiver.sol b/contracts/interfaces/IbcReceiver.sol index 1e458aef..5c655f93 100644 --- a/contracts/interfaces/IbcReceiver.sol +++ b/contracts/interfaces/IbcReceiver.sol @@ -12,13 +12,16 @@ import {ChannelOrder, CounterParty, IbcPacket, AckPacket} from "../libs/Ibc.sol" * handshake callbacks. */ interface IbcChannelReceiver { - function onChanOpenInit(string calldata version) external returns (string memory selectedVersion); - - function onChanOpenTry(string calldata counterpartyVersion) external returns (string memory selectedVersion); - - function onChanOpenAck(bytes32 channelId, string calldata counterpartyVersion) external; - - function onChanOpenConfirm(bytes32 channelId, string calldata counterpartyVersion) external; + function onOpenIbcChannel( + string calldata version, + ChannelOrder ordering, + bool feeEnabled, + string[] calldata connectionHops, + CounterParty calldata counterparty + ) external returns (string memory selectedVersion); + + function onConnectIbcChannel(bytes32 channelId, bytes32 counterpartyChannelId, string calldata counterpartyVersion) + external; function onCloseIbcChannel(bytes32 channelId, string calldata counterpartyPortId, bytes32 counterpartyChannelId) external; @@ -50,6 +53,7 @@ contract IbcReceiverBase is Ownable { error notIbcDispatcher(); error UnsupportedVersion(); + error VersionMismatch(); error ChannelNotFound(); /** diff --git a/test/Dispatcher.base.t.sol b/test/Dispatcher.base.t.sol index f2065885..9d143199 100644 --- a/test/Dispatcher.base.t.sol +++ b/test/Dispatcher.base.t.sol @@ -65,50 +65,35 @@ contract Base is IbcEventsEmitter, ProofBase { // ⬇️ IBC functions for testing /** - * @dev Step-1 of the 4-step handshake to open an IBC channel. + * @dev Step-1/2 of the 4-step handshake to open an IBC channel. * @param le Local end settings for the channel. * @param re Remote end settings for the channel. * @param s Channel handshake settings. * @param expPass Expected pass status of the operation. * If expPass is false, `vm.expectRevert` should be called before this function. */ - function channelOpenInit(LocalEnd memory le, CounterParty memory re, ChannelHandshakeSetting memory s, bool expPass) + function openChannel(LocalEnd memory le, CounterParty memory re, ChannelHandshakeSetting memory s, bool expPass) public { - if (expPass) { - vm.expectEmit(true, true, true, true); - emit ChannelOpenInit( - address(le.receiver), le.versionExpected, s.ordering, s.feeEnabled, le.connectionHops, re.portId - ); + CounterParty memory cp; + cp.portId = re.portId; + if (!s.localInitiate) { + cp.channelId = re.channelId; + cp.version = re.version; } - dispatcher.channelOpenInit(le.receiver, le.versionCall, s.ordering, s.feeEnabled, le.connectionHops, re.portId); - } - - /** - * @dev Step-2 of the 4-step handshake to open an IBC channel. - * @param le Local end settings for the channel. - * @param re Remote end settings for the channel. - * @param s Channel handshake settings. - * @param expPass Expected pass status of the operation. - * If expPass is false, `vm.expectRevert` should be called before this function. - */ - function channelOpenTry(LocalEnd memory le, CounterParty memory re, ChannelHandshakeSetting memory s, bool expPass) - public - { if (expPass) { vm.expectEmit(true, true, true, true); - emit ChannelOpenTry( + emit OpenIbcChannel( address(le.receiver), le.versionExpected, s.ordering, s.feeEnabled, le.connectionHops, - re.portId, - re.channelId + cp.portId, + cp.channelId ); } - CounterParty memory cp = CounterParty(re.portId, re.channelId, re.version); - dispatcher.channelOpenTry( + dispatcher.openIbcChannel( le.receiver, CounterParty(le.portId, le.channelId, le.versionCall), s.ordering, @@ -120,55 +105,31 @@ contract Base is IbcEventsEmitter, ProofBase { } /** - * @dev Step-3 of the 4-step handshake to open an IBC channel. - * @param le Local end settings for the channel. - * @param re Remote end settings for the channel. - * @param s Channel handshake settings. - * @param expPass Expected pass status of the operation. - * If expPass is false, `vm.expectRevert` should be called before this function. - */ - function channelOpenAck(LocalEnd memory le, CounterParty memory re, ChannelHandshakeSetting memory s, bool expPass) - public - { - if (expPass) { - vm.expectEmit(true, true, true, true); - emit ChannelOpenAck(address(le.receiver), le.channelId); - } - dispatcher.channelOpenAck( - le.receiver, - CounterParty(le.portId, le.channelId, le.versionCall), - le.connectionHops, - s.ordering, - s.feeEnabled, - re, - s.proof - ); - } - - /** - * @dev Step-4 of the 4-step handshake to open an IBC channel. + * @dev Step-3/4 of the 4-step handshake to open an IBC channel. * @param le Local end settings for the channel. * @param re Remote end settings for the channel. * @param s Channel handshake settings. * @param expPass Expected pass status of the operation. * If expPass is false, `vm.expectRevert` should be called before this function. */ - function channelOpenConfirm( + function connectChannel( LocalEnd memory le, CounterParty memory re, ChannelHandshakeSetting memory s, + bool isChannConfirm, bool expPass ) public { if (expPass) { vm.expectEmit(true, true, true, true); - emit ChannelOpenConfirm(address(le.receiver), le.channelId); + emit ConnectIbcChannel(address(le.receiver), le.channelId); } - dispatcher.channelOpenConfirm( + dispatcher.connectIbcChannel( le.receiver, CounterParty(le.portId, le.channelId, le.versionCall), le.connectionHops, s.ordering, s.feeEnabled, + isChannConfirm, re, s.proof ); diff --git a/test/Dispatcher.proof.t.sol b/test/Dispatcher.proof.t.sol index f0d3a900..b1a73ee5 100644 --- a/test/Dispatcher.proof.t.sol +++ b/test/Dispatcher.proof.t.sol @@ -31,38 +31,39 @@ contract DispatcherIbcWithRealProofs is IbcEventsEmitter, ProofBase { } function test_ibc_channel_open_init() public { - vm.expectEmit(true, true, true, true); - emit ChannelOpenInit(address(mars), "1.0", ChannelOrder.NONE, false, connectionHops1, ch1.portId); + CounterParty memory counterparty = CounterParty(ch1.portId, bytes32(0), ""); + vm.expectEmit(true, true, true, true); + emit OpenIbcChannel(address(mars), "1.0", ChannelOrder.NONE, false, connectionHops1, ch1.portId, bytes32(0)); // since this is open chann init, the proof is not used. so use an invalid one - dispatcher.channelOpenInit(mars, ch1.version, ChannelOrder.NONE, false, connectionHops1, ch1.portId); + dispatcher.openIbcChannel(mars, ch1, ChannelOrder.NONE, false, connectionHops1, counterparty, invalidProof); } function test_ibc_channel_open_try() public { Ics23Proof memory proof = load_proof("/test/payload/channel_try_pending_proof.hex"); vm.expectEmit(true, true, true, true); - emit ChannelOpenTry(address(mars), "1.0", ChannelOrder.NONE, false, connectionHops1, ch0.portId, ch0.channelId); + emit OpenIbcChannel(address(mars), "1.0", ChannelOrder.NONE, false, connectionHops1, ch0.portId, ch0.channelId); - dispatcher.channelOpenTry(mars, ch1, ChannelOrder.NONE, false, connectionHops1, ch0, proof); + dispatcher.openIbcChannel(mars, ch1, ChannelOrder.NONE, false, connectionHops1, ch0, proof); } function test_ibc_channel_ack() public { Ics23Proof memory proof = load_proof("/test/payload/channel_ack_pending_proof.hex"); vm.expectEmit(true, true, true, true); - emit ChannelOpenAck(address(mars), ch0.channelId); + emit ConnectIbcChannel(address(mars), ch0.channelId); - dispatcher.channelOpenAck(mars, ch0, connectionHops0, ChannelOrder.NONE, false, ch1, proof); + dispatcher.connectIbcChannel(mars, ch0, connectionHops0, ChannelOrder.NONE, false, false, ch1, proof); } function test_ibc_channel_confirm() public { Ics23Proof memory proof = load_proof("/test/payload/channel_confirm_pending_proof.hex"); vm.expectEmit(true, true, true, true); - emit ChannelOpenConfirm(address(mars), ch1.channelId); + emit ConnectIbcChannel(address(mars), ch1.channelId); - dispatcher.channelOpenConfirm(mars, ch1, connectionHops1, ChannelOrder.NONE, false, ch0, proof); + dispatcher.connectIbcChannel(mars, ch1, connectionHops1, ChannelOrder.NONE, false, true, ch0, proof); } function test_ack_packet() public { diff --git a/test/Dispatcher.t.sol b/test/Dispatcher.t.sol index a65cb54a..c15d5cf8 100644 --- a/test/Dispatcher.t.sol +++ b/test/Dispatcher.t.sol @@ -32,7 +32,7 @@ contract ChannelHandshakeTest is Base { le.versionCall = versions[j]; le.versionExpected = versions[j]; // remoteEnd has no channelId or version if localEnd is the initiator - channelOpenInit(le, re, settings[i], true); + openChannel(le, re, settings[i], true); } } } @@ -49,11 +49,11 @@ contract ChannelHandshakeTest is Base { le.versionCall = versions[j]; le.versionExpected = versions[j]; // remoteEnd version is used - channelOpenInit(le, re, settings[i], true); + openChannel(le, re, settings[i], true); // auto version selection le.versionCall = ""; - channelOpenTry(le, re, settings[i], true); + openChannel(le, re, settings[i], true); } } } @@ -69,15 +69,30 @@ contract ChannelHandshakeTest is Base { le.versionCall = versions[j]; le.versionExpected = versions[j]; re.version = versions[j]; - channelOpenInit(le, re, settings[i], true); - channelOpenTry(le, re, settings[i], true); - channelOpenAck(le, re, settings[i], true); - channelOpenConfirm(le, re, settings[i], true); + openChannel(le, re, settings[i], true); + connectChannel(le, re, settings[i], false, true); } } } - function test_channelOpenInit_fail_unsupportedVersion() public { + function test_openChannel_receiver_fail_versionMismatch() public { + ChannelHandshakeSetting[4] memory settings = createSettings(false, true); + string[2] memory versions = ["1.0", "2.0"]; + for (uint256 i = 0; i < settings.length; i++) { + for (uint256 j = 0; j < versions.length; j++) { + LocalEnd memory le = _local; + CounterParty memory re = _remote; + re.version = versions[j]; + // always select the wrong version + bool isVersionOne = keccak256(abi.encodePacked(versions[j])) == keccak256(abi.encodePacked("1.0")); + le.versionCall = isVersionOne ? "2.0" : "1.0"; + vm.expectRevert(IbcReceiverBase.VersionMismatch.selector); + openChannel(le, re, settings[i], false); + } + } + } + + function test_openChannel_initiator_fail_unsupportedVersion() public { ChannelHandshakeSetting[4] memory settings = createSettings(true, true); string[2] memory versions = ["", "xxxxxxx"]; for (uint256 i = 0; i < settings.length; i++) { @@ -87,13 +102,13 @@ contract ChannelHandshakeTest is Base { le.versionCall = versions[j]; le.versionExpected = versions[j]; vm.expectRevert(IbcReceiverBase.UnsupportedVersion.selector); - channelOpenInit(le, re, settings[i], false); + openChannel(le, re, settings[i], false); } } } function test_openChannel_receiver_fail_invalidProof() public { - // When localEnd initiates, no proof verification is done in channelOpenTry + // When localEnd initiates, no proof verification is done in openIbcChannel ChannelHandshakeSetting[4] memory settings = createSettings(false, false); string[1] memory versions = ["1.0"]; for (uint256 i = 0; i < settings.length; i++) { @@ -102,15 +117,14 @@ contract ChannelHandshakeTest is Base { CounterParty memory re = _remote; le.versionCall = versions[j]; le.versionExpected = versions[j]; - vm.expectRevert(DummyConsensusStateManager.InvalidDummyMembershipProof.selector); - channelOpenTry(le, re, settings[i], false); + openChannel(le, re, settings[i], false); } } } function test_connectChannel_fail_unsupportedVersion() public { - // When localEnd initiates, counterparty version is only available in channelOpenAck + // When localEnd initiates, counterparty version is only available in connectIbcChannel ChannelHandshakeSetting[4] memory settings = createSettings(true, true); string[2] memory versions = ["", "xxxxxxx"]; for (uint256 i = 0; i < settings.length; i++) { @@ -118,18 +132,16 @@ contract ChannelHandshakeTest is Base { LocalEnd memory le = _local; CounterParty memory re = _remote; // no remote version applied in openChannel - channelOpenInit(le, re, settings[i], true); - channelOpenTry(le, re, settings[i], true); + openChannel(le, re, settings[i], true); re.version = versions[j]; - vm.expectRevert(IbcReceiverBase.UnsupportedVersion.selector); - channelOpenAck(le, re, settings[i], false); + connectChannel(le, re, settings[i], false, false); } } } function test_connectChannel_fail_invalidProof() public { - // When localEnd initiates, counterparty version is only available in channelOpenAck + // When localEnd initiates, counterparty version is only available in connectIbcChannel ChannelHandshakeSetting[8] memory settings = createSettings2(true); string[1] memory versions = ["1.0"]; for (uint256 i = 0; i < settings.length; i++) { @@ -137,13 +149,11 @@ contract ChannelHandshakeTest is Base { LocalEnd memory le = _local; CounterParty memory re = _remote; // no remote version applied in openChannel - channelOpenInit(le, re, settings[i], true); - channelOpenTry(le, re, settings[i], true); + openChannel(le, re, settings[i], true); re.version = versions[j]; settings[i].proof = invalidProof; - vm.expectRevert(DummyConsensusStateManager.InvalidDummyMembershipProof.selector); - channelOpenAck(le, re, settings[i], false); + connectChannel(le, re, settings[i], false, false); } } } @@ -201,10 +211,8 @@ contract ChannelOpenTestBase is Base { _local = LocalEnd(mars, portId, channelId, connectionHops, "1.0", "1.0"); _remote = CounterParty("eth2.7E5F4552091A69125d5DfCb7b8C2659029395Bdf", "channel-2", "1.0"); - channelOpenInit(_local, _remote, setting, true); - channelOpenTry(_local, _remote, setting, true); - channelOpenAck(_local, _remote, setting, true); - channelOpenConfirm(_local, _remote, setting, true); + openChannel(_local, _remote, setting, true); + connectChannel(_local, _remote, setting, false, true); } } diff --git a/test/VirtualChain.sol b/test/VirtualChain.sol index d23e6f89..70ca30a9 100644 --- a/test/VirtualChain.sol +++ b/test/VirtualChain.sol @@ -132,10 +132,10 @@ contract VirtualChain is Test, IbcEventsEmitter { remoteChain.channelOpenTry(remoteEnd, this, localEnd, setting, true); // step-2 vm.prank(address(this)); - this.channelOpenConfirm(localEnd, remoteChain, remoteEnd, setting, true); // step-3 + this.channelOpenAckOrConfirm(localEnd, remoteChain, remoteEnd, setting, false, true); // step-3 vm.prank(address(remoteChain)); - remoteChain.channelOpenAck(remoteEnd, this, localEnd, setting, true); // step-4 + remoteChain.channelOpenAckOrConfirm(remoteEnd, this, localEnd, setting, true, true); // step-4 } function channelOpenInit( @@ -153,17 +153,25 @@ contract VirtualChain is Test, IbcEventsEmitter { if (expPass) { vm.expectEmit(true, true, true, true); - emit ChannelOpenInit( + emit OpenIbcChannel( address(localEnd), setting.version, setting.ordering, setting.feeEnabled, connectionHops, - remoteChain.portIds(address(remoteEnd)) + remoteChain.portIds(address(remoteEnd)), + bytes32(0) ); } - dispatcher.channelOpenInit( - localEnd, setting.version, setting.ordering, setting.feeEnabled, connectionHops, cpPortId + dispatcher.openIbcChannel( + localEnd, + CounterParty(setting.portId, setting.channelId, setting.version), + setting.ordering, + setting.feeEnabled, + connectionHops, + // counterparty channelId and version are not known at this point + CounterParty(cpPortId, bytes32(0), ""), + setting.proof ); } @@ -185,7 +193,7 @@ contract VirtualChain is Test, IbcEventsEmitter { if (expPass) { vm.expectEmit(true, true, true, true); - emit ChannelOpenTry( + emit OpenIbcChannel( address(localEnd), setting.version, setting.ordering, @@ -195,7 +203,7 @@ contract VirtualChain is Test, IbcEventsEmitter { cpChanId ); } - dispatcher.channelOpenTry( + dispatcher.openIbcChannel( localEnd, CounterParty(setting.portId, setting.channelId, setting.version), setting.ordering, @@ -206,71 +214,38 @@ contract VirtualChain is Test, IbcEventsEmitter { ); } - function channelOpenAck( - IbcChannelReceiver localEnd, - VirtualChain remoteChain, - IbcChannelReceiver remoteEnd, - ChannelSetting memory setting, - bool expPass - ) external { - // local channel Id must be known - bytes32 chanId = channelIds[address(localEnd)][address(remoteEnd)]; - require(chanId != bytes32(0), "channelOpenAck: channel does not exist"); - - bytes32 cpChanId = remoteChain.channelIds(address(remoteEnd), address(localEnd)); - require(cpChanId != bytes32(0), "channelOpenAck: channel does not exist"); - - string memory cpPortId = remoteChain.portIds(address(remoteEnd)); - require(bytes(cpPortId).length > 0, "channelOpenAck: counterparty portId does not exist"); - - // set dispatcher's msg.sender to this function's msg.sender - vm.prank(msg.sender); - - if (expPass) { - vm.expectEmit(true, true, true, true); - emit ChannelOpenAck(address(localEnd), chanId); - } - dispatcher.channelOpenAck( - localEnd, - CounterParty(setting.portId, chanId, setting.version), - connectionHops, - setting.ordering, - setting.feeEnabled, - CounterParty(cpPortId, cpChanId, setting.version), - setting.proof - ); - } - - function channelOpenConfirm( + function channelOpenAckOrConfirm( IbcChannelReceiver localEnd, VirtualChain remoteChain, IbcChannelReceiver remoteEnd, ChannelSetting memory setting, + bool isChanConfirm, bool expPass ) external { // local channel Id must be known bytes32 chanId = channelIds[address(localEnd)][address(remoteEnd)]; - require(chanId != bytes32(0), "channelOpenConfirm: channel does not exist"); + require(chanId != bytes32(0), "channelOpenAckOrConfirm: channel does not exist"); bytes32 cpChanId = remoteChain.channelIds(address(remoteEnd), address(localEnd)); - require(cpChanId != bytes32(0), "channelOpenConfirm: channel does not exist"); + require(cpChanId != bytes32(0), "channelOpenAckOrConfirm: channel does not exist"); string memory cpPortId = remoteChain.portIds(address(remoteEnd)); - require(bytes(cpPortId).length > 0, "channelOpenConfirm: counterparty portId does not exist"); + require(bytes(cpPortId).length > 0, "channelOpenAckOrConfirm: counterparty portId does not exist"); // set dispatcher's msg.sender to this function's msg.sender vm.prank(msg.sender); if (expPass) { vm.expectEmit(true, true, true, true); - emit ChannelOpenConfirm(address(localEnd), chanId); + emit ConnectIbcChannel(address(localEnd), chanId); } - dispatcher.channelOpenConfirm( + dispatcher.connectIbcChannel( localEnd, CounterParty(setting.portId, chanId, setting.version), connectionHops, setting.ordering, setting.feeEnabled, + isChanConfirm, CounterParty(cpPortId, cpChanId, setting.version), setting.proof ); diff --git a/test/universal.channel.t.sol b/test/universal.channel.t.sol index 61296b91..08fe64fb 100644 --- a/test/universal.channel.t.sol +++ b/test/universal.channel.t.sol @@ -77,10 +77,10 @@ contract UniversalChannelTest is Base { eth2.channelOpenTry(ucHandler2, eth1, ucHandler1, setting, true); vm.prank(unauthorized); - eth1.channelOpenAck(ucHandler1, eth2, ucHandler2, setting, true); + eth1.channelOpenAckOrConfirm(ucHandler1, eth2, ucHandler2, setting, false, true); vm.prank(unauthorized); - eth2.channelOpenConfirm(ucHandler2, eth1, ucHandler1, setting, true); + eth2.channelOpenAckOrConfirm(ucHandler2, eth1, ucHandler1, setting, true, true); } }