Handling packets recv on unknown channels

[mpoke]

Ref. #274 (comment)

The discussion is about whether the CCV modules (both on provider and consumers) should handle the case of IBC packets received on other channels than the established CCV channel. Note that one of the properties guaranteed by Interchain Security is that the CCV channel is unique.

Sending MsgRecvPacket messages to the core IBC core module is permissionless. However, there are several requirements for that packet (contained in the message) to be passed to the OnRecvPacket handler of an IBC application module. The relevant requirements for this discussions are:

• the module must have claimed the capability for the channel defined by portId, channelId, where portId = msg.Packet.DestinationPort and channelId = msg.Packet.DestinationChannel, see https://github.com/cosmos/ibc-go/blob/05c4bfbbebb20d1cc078ddf1d29dadc766d9251b/modules/core/keeper/msg_server.go#L380
• the channel must be OPEN, see https://github.com/cosmos/ibc-go/blob/05c4bfbbebb20d1cc078ddf1d29dadc766d9251b/modules/core/04-channel/keeper/packet.go#L148
• EDIT: the packet must have been sent by the counter party (i.e., the sender chain)

Consumer

Channel capabilities

The consumer CCV module claims a channel's capability in OnChanOpenInit, see

interchain-security/x/ccv/consumer/module.go

Line 281 in f3a9966

if err := am.keeper.ClaimCapability(

Sending MsgChannelOpenInit messages to he core IBC core module is permissionless. Moreover, the only requirement (relevant for this discussion) for those messages to be passed to the OnChanOpenInit handler of an IBC application module is that the module claimed the port capability, see https://github.com/cosmos/ibc-go/blob/05c4bfbbebb20d1cc078ddf1d29dadc766d9251b/modules/core/keeper/msg_server.go#L170.

The consumer CCV module claims the port capability at genesis, see

interchain-security/x/ccv/consumer/keeper/genesis.go

Line 30 in f3a9966

err := k.BindPort(ctx, types.PortID)

Thus, it will claim the capability of any channel that is ordered, on the expected port and with the expected version, as long as there is no established CCV channel (aka the ProviderChannel key is not set yet), see

interchain-security/x/ccv/consumer/module.go

Line 258 in f3a9966

func (am AppModule) OnChanOpenInit(

Open channels

The consumer CCV module channel endpoint moves to OPEN in ChannelOpenAck after executing the OnChanOpenAck handler, see https://github.com/cosmos/ibc-go/blob/05c4bfbbebb20d1cc078ddf1d29dadc766d9251b/modules/core/04-channel/keeper/handshake.go#L279. Thus, as long as the CCV channel is not already established, every channel (with the corresponding params) will move to OPEN.

Provider

Channel capabilities

The provider CCV module claims a channel's capability in OnChanOpenTry , see

interchain-security/x/ccv/provider/module.go

Line 270 in f3a9966

if err := am.keeper.ClaimCapability(ctx, chanCap, host.ChannelCapabilityPath(portID, channelID)); err != nil {

Sending MsgChannelOpenTry messages to he core IBC core module is permissionless. Moreover, the only requirement (relevant for this discussion) for those messages to be passed to the OnChanOpenTry handler of an IBC application module is that the module claimed the port capability, see https://github.com/cosmos/ibc-go/blob/05c4bfbbebb20d1cc078ddf1d29dadc766d9251b/modules/core/keeper/msg_server.go#L212

The provider CCV module claims the port capability at genesis, see

interchain-security/x/ccv/provider/keeper/genesis.go

Line 19 in f3a9966

err := k.BindPort(ctx, providertypes.PortID)

Thus, it will claim the capability of any channel that is ordered, on the expected port, and with the expected counterparty version, see

interchain-security/x/ccv/provider/module.go

Line 249 in f3a9966

counterparty channeltypes.Counterparty,

Open channels

The provider CCV module channel endpoint moves to OPEN in ChannelOpenConfirm after executing the OnChanOpenConfirm handler, see https://github.com/cosmos/ibc-go/blob/05c4bfbbebb20d1cc078ddf1d29dadc766d9251b/modules/core/04-channel/keeper/handshake.go#L362.

In OnChanOpenConfirm, if there already exists an established CCV channel, the channel (for which the handshake is in progress) is closed. Otherwise, the channel is set as the established CCV channel. In other words, only the first channel that gets to this point is marked as the CCV channel and moved to OPEN. The other channels are closed, see

interchain-security/x/ccv/provider/keeper/keeper.go

Line 276 in f3a9966

if err := k.chanCloseInit(ctx, channelID); err != nil {

Conclusion

The consumer CCV module could claim the capabilities of multiple channels that could be OPEN, which means that it MUST handle packets received on other channels than the established CCV channel.

Note: All the channels for which the consumer CCV module claims capabilities must be on the expected client (set during genesis) and connection, see

interchain-security/x/ccv/consumer/module.go

Line 287 in f3a9966

return am.keeper.VerifyProviderChain(ctx, channelID, connectionHops)

The provider CCV module CANNOT claim the capabilities of multiple channels that could be OPEN, which means that it SHOULD NOT handle packets received on other channels than the established CCV channel.

[mpoke]

Probably it makes sense to move this description of handling packets received on unknown channels to an ADR. @jtremback What do you think?

[jtremback]

So, correct me if I'm wrong: anyone can try to open a CCV channel, and once the first VSC packet is received on the consumer, the channel it came on is marked the "canonical" CCV channel. Any packets coming on other channels are ignored.

So when you write "it MUST handle packets received on other channels than the established CCV channel", by "handle" you mean "ignore"?

How does this differ from the provider, where "it SHOULD NOT handle packets"?

[mpoke]

So when you write "it MUST handle packets received on other channels than the established CCV channel", by "handle" you mean "ignore"?

Yes. And for cleaning up the state, we should close the other channels so that we don't leave random open channels in the state.

How does this differ from the provider, where "it SHOULD NOT handle packets"?

The IBC core module on the provider will not even pass to the OnRecvPacket handler of the provider CCV module messages received on any other channel than the CCV channel. This is because ONLY ONE channel between the consumer and provider can have the provider endpoint set to OPEN.

[mpoke]

@jtremback According to the EDIT in the issue description, i.e.,

One of the requirements for a packet to be passed to the OnRecvPacket handler of an IBC application module is for the packet to have been been sent by the counter party chain, i.e., a commitment of the packet was stored in state by the counter party chain.

the consumer will not receive packets that were not sent by the provider (as long as the provider follows the protocol, which we assume it does, i.e., less than 1/3 of the voting power is Byzantine). Since the provider only sends VSCPackets only on the established CCV channel (see here and here), there should be no way for the consumer to receive packets on a channel different than the CCV channel.

So, this branch

interchain-security/x/ccv/consumer/keeper/relay.go

Line 24 in 84c24b3

if found && providerChannel != packet.DestinationChannel {

should never happen.

Notice though that on the consumer, multiple channels could be OPEN (i.e., after executing ChanOpenAck). Only one of this channel will be OPEN also on the provider -- the first one to reach ChanOpenConfirm. That will be the CCV channel. Thus, for garbage collection, we should close the other channels that ended up being OPEN on the consumer. This could be done once the consumer establishes the CCV channel, i.e.,

interchain-security/x/ccv/consumer/keeper/relay.go

Line 31 in 84c24b3

k.SetProviderChannel(ctx, packet.DestinationChannel)

@AdityaSripal is there a way in IBC to iterate through all the channels built on top of the same connection?

[mpoke]

Re. garbage collection, this issue is related to #293. One of the solutions should be sufficient to deal with closing the redundant channels on both sides.

[jtremback]

I would like it very much if we could delete everything in this code path, since it is impossible to receive CCV packets on the wrong channel.

interchain-security/x/ccv/consumer/keeper/relay.go

Line 24 in 84c24b3

if found && providerChannel != packet.DestinationChannel {

Such as utils.OnRecvPacketOnUnknownChannel. I don't think that automatically cleaning a few bytes of storage space is worth the mental overhead of having this added complexity in our code. There are many unused duplicate transfer channels open on most chains. It's not an issue.