Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Draft: Aliasing the v1 channels to v2 counterparty so that v1 channels can support v2 packets #7174

Draft
wants to merge 10 commits into
base: feat/ibc-eureka
Choose a base branch
from
Draft

Draft: Aliasing the v1 channels to v2 counterparty so that v1 channels can support v2 packets #7174

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
2a2d739
aliasing start
AdityaSripal Aug 14, 2024
016f1f7
move counterparty logic and msgs into packetserver
AdityaSripal Aug 30, 2024
ae910a8
get all tests passing with new counterparty msg/handler in new package
AdityaSripal Sep 3, 2024
faa9add
add in channel id and leave merkle path as a prefix
AdityaSripal Sep 5, 2024
5b71520
change relay logic to separate client id from channel id
AdityaSripal Sep 6, 2024
9b9beb6
Merge branch 'feat/ibc-eureka' into aditya/alias-2
crodriguezvega Sep 9, 2024
f4dc2b2
lint
crodriguezvega Sep 9, 2024
a752e41
lint redux
crodriguezvega Sep 9, 2024
a8b8569
wire GetV2Counterparty to packet server
AdityaSripal Sep 18, 2024
1c01783
fix merge
AdityaSripal Sep 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions modules/core/04-channel/keeper/keeper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
connectiontypes "github.com/cosmos/ibc-go/v9/modules/core/03-connection/types"
"github.com/cosmos/ibc-go/v9/modules/core/04-channel/types"
porttypes "github.com/cosmos/ibc-go/v9/modules/core/05-port/types"
commitmentv2types "github.com/cosmos/ibc-go/v9/modules/core/23-commitment/types/v2"
host "github.com/cosmos/ibc-go/v9/modules/core/24-host"
"github.com/cosmos/ibc-go/v9/modules/core/exported"
)
Expand Down Expand Up @@ -95,6 +96,30 @@ func (k *Keeper) SetChannel(ctx sdk.Context, portID, channelID string, channel t
store.Set(host.ChannelKey(portID, channelID), bz)
}

// GetV2Counterparty returns a version 2 counterparty for the given port and channel ID
// by converting the channel into a version 2 counterparty
func (k *Keeper) GetV2Counterparty(ctx sdk.Context, portID, channelID string) (clienttypes.Counterparty, bool) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This effectively resolves the portID and channel ID into a counterparty. So the portID and channelID are in effect the clientID here. It's just a different arbitrary string identifier.

The one annoying thing is that the portID here is both acting as part of the v2 "client-id" (though we could move away from this with a different storage format since channel ids are unique) and also selecting the application to use.

This is fine for packets that are only sending one app data.

For MULTI_PACKET_DATA, we can deal with this by leaving the top portID as is, with the plan to eventually deprecate it. And then still allowing different ports in the internal packet data list

channel, ok := k.GetChannel(ctx, portID, channelID)
if !ok {
return clienttypes.Counterparty{}, false
}
// Do not allow channel to be converted into a version 2 counterparty
// if the channel is not OPEN or if it is ORDERED
if channel.State != types.OPEN || channel.Ordering == types.ORDERED {
return clienttypes.Counterparty{}, false
}
connection, ok := k.connectionKeeper.GetConnection(ctx, channel.ConnectionHops[0])
if !ok {
return clienttypes.Counterparty{}, false
}
merklePathPrefix := commitmentv2types.NewMerklePath(connection.Counterparty.Prefix.KeyPrefix, []byte(""))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

byte slice needs channel id info?

counterparty := clienttypes.Counterparty{
ClientId: connection.ClientId,
MerklePathPrefix: &merklePathPrefix,
}
return counterparty, true
}

// GetAppVersion gets the version for the specified channel.
func (k *Keeper) GetAppVersion(ctx sdk.Context, portID, channelID string) (string, bool) {
channel, found := k.GetChannel(ctx, portID, channelID)
Expand Down
23 changes: 18 additions & 5 deletions modules/core/packet-server/keeper/keeper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func (k Keeper) SendPacket(
data []byte,
) (uint64, error) {
// Lookup counterparty associated with our source channel to retrieve the destination channel
counterparty, ok := k.ClientKeeper.GetCounterparty(ctx, sourceChannel)
counterparty, ok := k.GetCounterparty(ctx, sourcePort, sourceChannel)
if !ok {
return 0, channeltypes.ErrChannelNotFound
}
Expand Down Expand Up @@ -125,7 +125,7 @@ func (k Keeper) RecvPacket(
// Lookup counterparty associated with our channel and ensure that it was packet was indeed
// sent by our counterparty.
// Note: This can be implemented by the current keeper
counterparty, ok := k.ClientKeeper.GetCounterparty(ctx, packet.DestinationChannel)
counterparty, ok := k.GetCounterparty(ctx, packet.DestinationPort, packet.DestinationChannel)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we have a transfer V2 packet sent from an ibc-go v10 chain to another ibc-go v10 chain over an existing channel, the packet's DestinationChannel will be filled in with the client ID of the destination chain, right? But then in the OnRecvPacket callback of the transfer the hash of the denomination will not match that of the tokens received previously (since those used the destination channel ID and not a client ID). Then those vouchers would not be fungible, right?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, the DestinationChannel will continue to be the existing channel identifier (that's the purpose of this adjustment, to keep fungibility but allow existing channels to send v2 packets)

if !ok {
return "", channeltypes.ErrChannelNotFound
}
Expand Down Expand Up @@ -200,7 +200,7 @@ func (k Keeper) WriteAcknowledgement(

// Lookup counterparty associated with our channel and ensure that it was packet was indeed
// sent by our counterparty.
counterparty, ok := k.ClientKeeper.GetCounterparty(ctx, packet.DestinationChannel)
counterparty, ok := k.GetCounterparty(ctx, packet.DestinationPort, packet.DestinationChannel)
if !ok {
return channeltypes.ErrChannelNotFound
}
Expand Down Expand Up @@ -254,7 +254,7 @@ func (k Keeper) AcknowledgePacket(

// Lookup counterparty associated with our channel and ensure that it was packet was indeed
// sent by our counterparty.
counterparty, ok := k.ClientKeeper.GetCounterparty(ctx, packet.SourceChannel)
counterparty, ok := k.GetCounterparty(ctx, packet.SourcePort, packet.SourceChannel)
if !ok {
return "", channeltypes.ErrChannelNotFound
}
Expand Down Expand Up @@ -321,7 +321,7 @@ func (k Keeper) TimeoutPacket(
}
// Lookup counterparty associated with our channel and ensure that destination channel
// is the expected counterparty
counterparty, ok := k.ClientKeeper.GetCounterparty(ctx, packet.SourceChannel)
counterparty, ok := k.GetCounterparty(ctx, packet.SourcePort, packet.SourceChannel)
if !ok {
return "", channeltypes.ErrChannelNotFound
}
Expand Down Expand Up @@ -390,3 +390,16 @@ func (k Keeper) TimeoutPacket(
func sentinelChannel(clientID string) channeltypes.Channel {
return channeltypes.Channel{Ordering: channeltypes.UNORDERED, ConnectionHops: []string{clientID}}
}

// GetCounterparty returns the v2 counterparty for a given portID and channelID.
// If the portID and channel ID resolves to a v1 channel, then we create a v2 counterparty
// using the channel information
// If the channel ID resolves to client, then we can directly retrieve the v2 counterparty
func (k Keeper) GetCounterparty(ctx sdk.Context, portID, channelID string) (clienttypes.Counterparty, bool) {
counterparty, ok := k.ChannelKeeper.GetV2Counterparty(ctx, portID, channelID)
if ok {
return counterparty, true
}
// get the counterparty directly from the client keeper using the channel id as a client identifier
return k.ClientKeeper.GetCounterparty(ctx, channelID)
}
3 changes: 3 additions & 0 deletions modules/core/packet-server/types/expected_keepers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ type ChannelKeeper interface {
// SetPacketAcknowledgement writes the acknowledgement hash under the acknowledgement path
// This is a public path that is standardized by the IBC specification
SetPacketAcknowledgement(ctx sdk.Context, portID, channelID string, sequence uint64, ackHash []byte)

// GetChannel returns a version 2 counterparty for a given portID and channel ID
GetV2Counterparty(ctx sdk.Context, portID, channelID string) (clienttypes.Counterparty, bool)
}

type ClientKeeper interface {
Expand Down
Loading