diff --git a/contracts/core/Dispatcher.sol b/contracts/core/Dispatcher.sol index 37d7a0c3..055b89f5 100644 --- a/contracts/core/Dispatcher.sol +++ b/contracts/core/Dispatcher.sol @@ -129,43 +129,56 @@ contract Dispatcher is IbcDispatcher, IbcEventsEmitter, Ownable, Ibc { } /** - * 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. + * 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 */ - function connectIbcChannel( + function channelOpenAck( IbcChannelReceiver portAddress, CounterParty calldata local, string[] calldata connectionHops, ChannelOrder ordering, bool feeEnabled, - bool isChanConfirm, CounterParty calldata counterparty, Ics23Proof calldata proof ) external { - _verifyConnectIbcChannelProof(local, connectionHops, ordering, isChanConfirm, counterparty, proof); + consensusStateManager.verifyMembership( + proof, + channelProofKey(local.portId, local.channelId), + channelProofValue(ChannelState.ACK_PENDING, ordering, local.version, connectionHops, counterparty) + ); - portAddress.onConnectIbcChannel(local.channelId, counterparty.channelId, counterparty.version); + _connectChannel(portAddress, local, connectionHops, ordering, feeEnabled, 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 + portAddress.onChanOpenAck(local.channelId, counterparty.version); + + emit ChannelOpenAck(address(portAddress), local.channelId); + } + + /** + * 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) ); - // initialize channel sequences - nextSequenceSend[address(portAddress)][local.channelId] = 1; - nextSequenceRecv[address(portAddress)][local.channelId] = 1; - nextSequenceAck[address(portAddress)][local.channelId] = 1; + _connectChannel(portAddress, local, connectionHops, ordering, feeEnabled, counterparty); + + portAddress.onChanOpenConfirm(local.channelId, counterparty.version); - emit ConnectIbcChannel(address(portAddress), local.channelId); + emit ChannelOpenConfirm(address(portAddress), local.channelId); } /** @@ -472,25 +485,31 @@ contract Dispatcher is IbcDispatcher, IbcEventsEmitter, Ownable, Ibc { emit SendPacket(sender, channelId, packet, sequence, timeoutTimestamp); } - function _verifyConnectIbcChannelProof( + function _connectChannel( + IbcChannelReceiver portAddress, CounterParty calldata local, string[] calldata connectionHops, ChannelOrder ordering, - bool isChanConfirm, - CounterParty calldata counterparty, - Ics23Proof calldata proof + bool feeEnabled, + CounterParty calldata counterparty ) internal { - consensusStateManager.verifyMembership( - proof, - channelProofKey(local.portId, local.channelId), - channelProofValue( - isChanConfirm ? ChannelState.CONFIRM_PENDING : ChannelState.ACK_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 ); + + // 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 diff --git a/contracts/core/UniversalChannelHandler.sol b/contracts/core/UniversalChannelHandler.sol index 25e7f39b..2ab9b6b5 100644 --- a/contracts/core/UniversalChannelHandler.sol +++ b/contracts/core/UniversalChannelHandler.sol @@ -33,16 +33,6 @@ contract UniversalChannelHandler is IbcReceiverBase, IbcUniversalChannelMW { dispatcher.closeIbcChannel(channelId); } - 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; @@ -149,6 +139,14 @@ contract UniversalChannelHandler is IbcReceiverBase, IbcUniversalChannelMW { } // 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 @@ -167,6 +165,13 @@ contract UniversalChannelHandler is IbcReceiverBase, IbcUniversalChannelMW { 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(); diff --git a/contracts/examples/Mars.sol b/contracts/examples/Mars.sol index 771aa48d..7cd091cd 100644 --- a/contracts/examples/Mars.sol +++ b/contracts/examples/Mars.sol @@ -34,22 +34,6 @@ 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; @@ -81,6 +65,14 @@ 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 @@ -99,6 +91,17 @@ contract Mars is IbcReceiverBase, IbcReceiver { return _openChannel(counterpartyVersion); } + function _connectChannel(bytes32 channelId, string calldata counterpartyVersion) private { + // ensure negotiated version is supported + for (uint256 i = 0; i < supportedVersions.length; i++) { + if (keccak256(abi.encodePacked(counterpartyVersion)) == keccak256(abi.encodePacked(supportedVersions[i]))) { + connectedChannels.push(channelId); + return; + } + } + 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]))) { diff --git a/contracts/interfaces/IbcDispatcher.sol b/contracts/interfaces/IbcDispatcher.sol index a13fcd6a..a2e2d540 100644 --- a/contracts/interfaces/IbcDispatcher.sol +++ b/contracts/interfaces/IbcDispatcher.sol @@ -64,7 +64,9 @@ interface IbcEventsEmitter { bytes32 counterpartyChannelId ); - event ConnectIbcChannel(address indexed portAddress, bytes32 channelId); + event ChannelOpenAck(address indexed portAddress, bytes32 channelId); + + event ChannelOpenConfirm(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 3191ae1f..1e458aef 100644 --- a/contracts/interfaces/IbcReceiver.sol +++ b/contracts/interfaces/IbcReceiver.sol @@ -16,8 +16,9 @@ interface IbcChannelReceiver { function onChanOpenTry(string calldata counterpartyVersion) external returns (string memory selectedVersion); - function onConnectIbcChannel(bytes32 channelId, bytes32 counterpartyChannelId, string calldata counterpartyVersion) - external; + function onChanOpenAck(bytes32 channelId, string calldata counterpartyVersion) external; + + function onChanOpenConfirm(bytes32 channelId, string calldata counterpartyVersion) external; function onCloseIbcChannel(bytes32 channelId, string calldata counterpartyPortId, bytes32 counterpartyChannelId) external; diff --git a/test/Dispatcher.base.t.sol b/test/Dispatcher.base.t.sol index 9a6c5ea4..f2065885 100644 --- a/test/Dispatcher.base.t.sol +++ b/test/Dispatcher.base.t.sol @@ -120,31 +120,55 @@ contract Base is IbcEventsEmitter, ProofBase { } /** - * @dev Step-3/4 of the 4-step handshake to open an IBC channel. + * @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 connectChannel( + 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. + * @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( LocalEnd memory le, CounterParty memory re, ChannelHandshakeSetting memory s, - bool isChannConfirm, bool expPass ) public { if (expPass) { vm.expectEmit(true, true, true, true); - emit ConnectIbcChannel(address(le.receiver), le.channelId); + emit ChannelOpenConfirm(address(le.receiver), le.channelId); } - dispatcher.connectIbcChannel( + dispatcher.channelOpenConfirm( 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 84b6a89a..f0d3a900 100644 --- a/test/Dispatcher.proof.t.sol +++ b/test/Dispatcher.proof.t.sol @@ -51,18 +51,18 @@ contract DispatcherIbcWithRealProofs is IbcEventsEmitter, ProofBase { Ics23Proof memory proof = load_proof("/test/payload/channel_ack_pending_proof.hex"); vm.expectEmit(true, true, true, true); - emit ConnectIbcChannel(address(mars), ch0.channelId); + emit ChannelOpenAck(address(mars), ch0.channelId); - dispatcher.connectIbcChannel(mars, ch0, connectionHops0, ChannelOrder.NONE, false, false, ch1, proof); + dispatcher.channelOpenAck(mars, ch0, connectionHops0, ChannelOrder.NONE, 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 ConnectIbcChannel(address(mars), ch1.channelId); + emit ChannelOpenConfirm(address(mars), ch1.channelId); - dispatcher.connectIbcChannel(mars, ch1, connectionHops1, ChannelOrder.NONE, false, true, ch0, proof); + dispatcher.channelOpenConfirm(mars, ch1, connectionHops1, ChannelOrder.NONE, false, ch0, proof); } function test_ack_packet() public { diff --git a/test/Dispatcher.t.sol b/test/Dispatcher.t.sol index 8fbcaa2c..a65cb54a 100644 --- a/test/Dispatcher.t.sol +++ b/test/Dispatcher.t.sol @@ -71,7 +71,8 @@ contract ChannelHandshakeTest is Base { re.version = versions[j]; channelOpenInit(le, re, settings[i], true); channelOpenTry(le, re, settings[i], true); - connectChannel(le, re, settings[i], false, true); + channelOpenAck(le, re, settings[i], true); + channelOpenConfirm(le, re, settings[i], true); } } } @@ -109,7 +110,7 @@ contract ChannelHandshakeTest is Base { } function test_connectChannel_fail_unsupportedVersion() public { - // When localEnd initiates, counterparty version is only available in connectIbcChannel + // When localEnd initiates, counterparty version is only available in channelOpenAck ChannelHandshakeSetting[4] memory settings = createSettings(true, true); string[2] memory versions = ["", "xxxxxxx"]; for (uint256 i = 0; i < settings.length; i++) { @@ -120,14 +121,15 @@ contract ChannelHandshakeTest is Base { channelOpenInit(le, re, settings[i], true); channelOpenTry(le, re, settings[i], true); re.version = versions[j]; + vm.expectRevert(IbcReceiverBase.UnsupportedVersion.selector); - connectChannel(le, re, settings[i], false, false); + channelOpenAck(le, re, settings[i], false); } } } function test_connectChannel_fail_invalidProof() public { - // When localEnd initiates, counterparty version is only available in connectIbcChannel + // When localEnd initiates, counterparty version is only available in channelOpenAck ChannelHandshakeSetting[8] memory settings = createSettings2(true); string[1] memory versions = ["1.0"]; for (uint256 i = 0; i < settings.length; i++) { @@ -139,8 +141,9 @@ contract ChannelHandshakeTest is Base { channelOpenTry(le, re, settings[i], true); re.version = versions[j]; settings[i].proof = invalidProof; + vm.expectRevert(DummyConsensusStateManager.InvalidDummyMembershipProof.selector); - connectChannel(le, re, settings[i], false, false); + channelOpenAck(le, re, settings[i], false); } } } @@ -200,7 +203,8 @@ contract ChannelOpenTestBase is Base { channelOpenInit(_local, _remote, setting, true); channelOpenTry(_local, _remote, setting, true); - connectChannel(_local, _remote, setting, false, true); + channelOpenAck(_local, _remote, setting, true); + channelOpenConfirm(_local, _remote, setting, true); } } diff --git a/test/VirtualChain.sol b/test/VirtualChain.sol index c022412f..d23e6f89 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.channelOpenAckOrConfirm(localEnd, remoteChain, remoteEnd, setting, false, true); // step-3 + this.channelOpenConfirm(localEnd, remoteChain, remoteEnd, setting, true); // step-3 vm.prank(address(remoteChain)); - remoteChain.channelOpenAckOrConfirm(remoteEnd, this, localEnd, setting, true, true); // step-4 + remoteChain.channelOpenAck(remoteEnd, this, localEnd, setting, true); // step-4 } function channelOpenInit( @@ -206,38 +206,71 @@ contract VirtualChain is Test, IbcEventsEmitter { ); } - function channelOpenAckOrConfirm( + function channelOpenAck( 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), "channelOpenAckOrConfirm: channel does not exist"); + require(chanId != bytes32(0), "channelOpenAck: channel does not exist"); bytes32 cpChanId = remoteChain.channelIds(address(remoteEnd), address(localEnd)); - require(cpChanId != bytes32(0), "channelOpenAckOrConfirm: channel does not exist"); + require(cpChanId != bytes32(0), "channelOpenAck: channel does not exist"); string memory cpPortId = remoteChain.portIds(address(remoteEnd)); - require(bytes(cpPortId).length > 0, "channelOpenAckOrConfirm: counterparty portId does not exist"); + 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 ConnectIbcChannel(address(localEnd), chanId); + emit ChannelOpenAck(address(localEnd), chanId); } - dispatcher.connectIbcChannel( + dispatcher.channelOpenAck( + localEnd, + CounterParty(setting.portId, chanId, setting.version), + connectionHops, + setting.ordering, + setting.feeEnabled, + CounterParty(cpPortId, cpChanId, setting.version), + setting.proof + ); + } + + function channelOpenConfirm( + 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), "channelOpenConfirm: channel does not exist"); + + bytes32 cpChanId = remoteChain.channelIds(address(remoteEnd), address(localEnd)); + require(cpChanId != bytes32(0), "channelOpenConfirm: channel does not exist"); + + string memory cpPortId = remoteChain.portIds(address(remoteEnd)); + require(bytes(cpPortId).length > 0, "channelOpenConfirm: 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); + } + dispatcher.channelOpenConfirm( 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 08fe64fb..61296b91 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.channelOpenAckOrConfirm(ucHandler1, eth2, ucHandler2, setting, false, true); + eth1.channelOpenAck(ucHandler1, eth2, ucHandler2, setting, true); vm.prank(unauthorized); - eth2.channelOpenAckOrConfirm(ucHandler2, eth1, ucHandler1, setting, true, true); + eth2.channelOpenConfirm(ucHandler2, eth1, ucHandler1, setting, true); } }