Skip to content

Commit

Permalink
channel handshake: split open-ack and open-confirm
Browse files Browse the repository at this point in the history
  • Loading branch information
nicopernas committed Mar 4, 2024
1 parent 88264cb commit a66a3f7
Show file tree
Hide file tree
Showing 10 changed files with 150 additions and 79 deletions.
91 changes: 54 additions & 37 deletions contracts/core/Dispatcher.sol
Original file line number Diff line number Diff line change
Expand Up @@ -129,43 +129,52 @@ 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
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);

emit ConnectIbcChannel(address(portAddress), local.channelId);
emit ChannelOpenConfirm(address(portAddress), local.channelId);
}

/**
Expand Down Expand Up @@ -472,25 +481,33 @@ 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
)
portAddress.onChannelConnect(local.channelId, counterparty.version);

// 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
Expand Down
5 changes: 1 addition & 4 deletions contracts/core/UniversalChannelHandler.sol
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,7 @@ contract UniversalChannelHandler is IbcReceiverBase, IbcUniversalChannelMW {
dispatcher.closeIbcChannel(channelId);
}

function onConnectIbcChannel(bytes32 channelId, bytes32, string calldata counterpartyVersion)
external
onlyIbcDispatcher
{
function onChannelConnect(bytes32 channelId, string calldata counterpartyVersion) external onlyIbcDispatcher {
if (keccak256(abi.encodePacked(counterpartyVersion)) != keccak256(abi.encodePacked(VERSION))) {
revert UnsupportedVersion();
}
Expand Down
13 changes: 4 additions & 9 deletions contracts/examples/Mars.sol
Original file line number Diff line number Diff line change
Expand Up @@ -34,20 +34,15 @@ contract Mars is IbcReceiverBase, IbcReceiver {
timeoutPackets.push(packet);
}

function onConnectIbcChannel(bytes32 channelId, bytes32, string calldata counterpartyVersion)
external
onlyIbcDispatcher
{
function onChannelConnect(bytes32 channelId, 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;
connectedChannels.push(channelId);
return;
}
}
if (!foundVersion) revert UnsupportedVersion();
connectedChannels.push(channelId);
revert UnsupportedVersion();
}

function onCloseIbcChannel(bytes32 channelId, string calldata, bytes32) external onlyIbcDispatcher {
Expand Down
4 changes: 3 additions & 1 deletion contracts/interfaces/IbcDispatcher.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
3 changes: 1 addition & 2 deletions contracts/interfaces/IbcReceiver.sol
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,7 @@ interface IbcChannelReceiver {

function onChannelOpenTry(string calldata counterpartyVersion) external returns (string memory selectedVersion);

function onConnectIbcChannel(bytes32 channelId, bytes32 counterpartyChannelId, string calldata counterpartyVersion)
external;
function onChannelConnect(bytes32 channelId, string calldata counterpartyVersion) external;

function onCloseIbcChannel(bytes32 channelId, string calldata counterpartyPortId, bytes32 counterpartyChannelId)
external;
Expand Down
36 changes: 30 additions & 6 deletions test/Dispatcher.base.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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
);
Expand Down
8 changes: 4 additions & 4 deletions test/Dispatcher.proof.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
12 changes: 8 additions & 4 deletions test/Dispatcher.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
}
Expand Down Expand Up @@ -120,8 +121,9 @@ 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);
}
}
}
Expand All @@ -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);
}
}
}
Expand Down Expand Up @@ -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);
}
}

Expand Down
Loading

0 comments on commit a66a3f7

Please sign in to comment.