From 013ad4f3a1c38b368dc59f17a412600e3829c1d1 Mon Sep 17 00:00:00 2001 From: Aditya Sripal <14364734+AdityaSripal@users.noreply.github.com> Date: Wed, 22 Jan 2025 18:18:46 +0100 Subject: [PATCH 1/2] simplify writeack api --- modules/core/04-channel/v2/keeper/keeper.go | 32 +++++++++++++++++ .../core/04-channel/v2/keeper/msg_server.go | 5 ++- modules/core/04-channel/v2/keeper/packet.go | 34 ++++++++++++++++--- .../core/04-channel/v2/keeper/packet_test.go | 4 ++- modules/core/04-channel/v2/types/keys.go | 11 ++++++ 5 files changed, 80 insertions(+), 6 deletions(-) diff --git a/modules/core/04-channel/v2/keeper/keeper.go b/modules/core/04-channel/v2/keeper/keeper.go index a63e644f83e..00076b6e903 100644 --- a/modules/core/04-channel/v2/keeper/keeper.go +++ b/modules/core/04-channel/v2/keeper/keeper.go @@ -162,3 +162,35 @@ func (k *Keeper) SetNextSequenceSend(ctx context.Context, clientID string, seque panic(err) } } + +// SetAsyncPacket writes the packet under the async path +func (k *Keeper) SetAsyncPacket(ctx context.Context, clientID string, sequence uint64, packet types.Packet) { + store := k.KVStoreService.OpenKVStore(ctx) + bz := k.cdc.MustMarshal(&packet) + if err := store.Set(types.AsyncPacketKey(clientID, sequence), bz); err != nil { + panic(err) + } +} + +// GetAsyncPacket fetches the packet from the async path +func (k *Keeper) GetAsyncPacket(ctx context.Context, clientID string, sequence uint64) (types.Packet, bool) { + store := k.KVStoreService.OpenKVStore(ctx) + bz, err := store.Get(types.AsyncPacketKey(clientID, sequence)) + if err != nil { + panic(err) + } + if len(bz) == 0 { + return types.Packet{}, false + } + var packet types.Packet + k.cdc.MustUnmarshal(bz, &packet) + return packet, true +} + +// DeleteAsyncPacket deletes the packet from the async path +func (k *Keeper) DeleteAsyncPacket(ctx context.Context, clientID string, sequence uint64) { + store := k.KVStoreService.OpenKVStore(ctx) + if err := store.Delete(types.AsyncPacketKey(clientID, sequence)); err != nil { + panic(err) + } +} diff --git a/modules/core/04-channel/v2/keeper/msg_server.go b/modules/core/04-channel/v2/keeper/msg_server.go index aaadfc8fc80..070072942e5 100644 --- a/modules/core/04-channel/v2/keeper/msg_server.go +++ b/modules/core/04-channel/v2/keeper/msg_server.go @@ -135,9 +135,12 @@ func (k *Keeper) RecvPacket(ctx context.Context, msg *types.MsgRecvPacket) (*typ // Set packet acknowledgement only if the acknowledgement is not async. // NOTE: IBC applications modules may call the WriteAcknowledgement asynchronously if the // acknowledgement is async. - if err := k.WriteAcknowledgement(ctx, msg.Packet, ack); err != nil { + if err := k.writeAcknowledgement(ctx, msg.Packet, ack); err != nil { return nil, err } + } else { + // store the packet temporarily until the application returns an acknowledgement + k.SetAsyncPacket(ctx, msg.Packet.DestinationClient, msg.Packet.Sequence, msg.Packet) } // TODO: store the packet for async applications to access if required. diff --git a/modules/core/04-channel/v2/keeper/packet.go b/modules/core/04-channel/v2/keeper/packet.go index a0254d9ca64..63130d71530 100644 --- a/modules/core/04-channel/v2/keeper/packet.go +++ b/modules/core/04-channel/v2/keeper/packet.go @@ -148,9 +148,9 @@ func (k *Keeper) recvPacket( return nil } -// WriteAcknowledgement writes the acknowledgement to the store. -// TODO: change this function to accept destPort, destChannel, sequence, ack -func (k Keeper) WriteAcknowledgement( +// writeAcknowledgement writes the acknowledgement to the store and emits the packet and acknowledgement +// for relayers to relay the acknowledgement to the counterparty chain. +func (k Keeper) writeAcknowledgement( ctx context.Context, packet types.Packet, ack types.Acknowledgement, @@ -186,7 +186,33 @@ func (k Keeper) WriteAcknowledgement( emitWriteAcknowledgementEvents(ctx, packet, ack) - // TODO: delete the packet that has been stored in ibc-core. + return nil +} + +// WriteAcknowledgement writes the acknowledgement and emits events for asynchronous acknowledgements +// this is the method to be called by external apps when they want to write an acknowledgement asyncrhonously +func (k *Keeper) WriteAcknowledgement(ctx context.Context, clientID string, sequence uint64, ack types.Acknowledgement) error { + sdkCtx := sdk.UnwrapSDKContext(ctx) + + // Validate the acknowledgement + if err := ack.Validate(); err != nil { + sdkCtx.Logger().Error("write acknowledgement failed", "error", errorsmod.Wrap(err, "invalid acknowledgement")) + return errorsmod.Wrap(err, "invalid acknowledgement") + } + + packet, ok := k.GetAsyncPacket(ctx, clientID, sequence) + if !ok { + return errorsmod.Wrapf(types.ErrInvalidAcknowledgement, "packet with clientID (%s) and sequence (%d) not found for async acknowledgement", clientID, sequence) + } + + // Write the acknowledgement to the store + if err := k.writeAcknowledgement(ctx, packet, ack); err != nil { + sdkCtx.Logger().Error("write acknowledgement failed", "error", errorsmod.Wrap(err, "write acknowledgement failed")) + return errorsmod.Wrap(err, "write acknowledgement failed") + } + + // Delete the packet from the async store + k.DeleteAsyncPacket(ctx, clientID, sequence) return nil } diff --git a/modules/core/04-channel/v2/keeper/packet_test.go b/modules/core/04-channel/v2/keeper/packet_test.go index 4c988d2ee69..23f05f1e1e7 100644 --- a/modules/core/04-channel/v2/keeper/packet_test.go +++ b/modules/core/04-channel/v2/keeper/packet_test.go @@ -301,11 +301,13 @@ func (suite *KeeperTestSuite) TestWriteAcknowledgement() { AppAcknowledgements: [][]byte{mockv2.MockRecvPacketResult.Acknowledgement}, } + // mock receive with async acknowledgement suite.chainB.App.GetIBCKeeper().ChannelKeeperV2.SetPacketReceipt(suite.chainB.GetContext(), packet.DestinationClient, packet.Sequence) + suite.chainB.App.GetIBCKeeper().ChannelKeeperV2.SetAsyncPacket(suite.chainB.GetContext(), packet.DestinationClient, packet.Sequence, packet) tc.malleate() - err := suite.chainB.App.GetIBCKeeper().ChannelKeeperV2.WriteAcknowledgement(suite.chainB.GetContext(), packet, ack) + err := suite.chainB.App.GetIBCKeeper().ChannelKeeperV2.WriteAcknowledgement(suite.chainB.GetContext(), packet.DestinationClient, packet.Sequence, ack) expPass := tc.expError == nil if expPass { diff --git a/modules/core/04-channel/v2/types/keys.go b/modules/core/04-channel/v2/types/keys.go index 98c8e520b46..68a55d0e073 100644 --- a/modules/core/04-channel/v2/types/keys.go +++ b/modules/core/04-channel/v2/types/keys.go @@ -1,6 +1,17 @@ package types +import "fmt" + const ( // SubModuleName defines the channelv2 module name. SubModuleName = "channelv2" + + // KeyAsyncPacket defines the key to store the async packet. + KeyAsyncPacket = "async_packet" ) + +// AsyncPacketKey returns the key under which the packet is stored +// if the receiving application returns an async acknowledgement. +func AsyncPacketKey(clientID string, sequence uint64) []byte { + return []byte(fmt.Sprintf("%s/%s/%d", KeyAsyncPacket, clientID, sequence)) +} From 3bd53772f54ec041298c3e6a26541f02b611c6d9 Mon Sep 17 00:00:00 2001 From: Aditya Sripal <14364734+AdityaSripal@users.noreply.github.com> Date: Thu, 23 Jan 2025 11:47:55 +0100 Subject: [PATCH 2/2] fix tests --- modules/core/04-channel/v2/keeper/packet_test.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/modules/core/04-channel/v2/keeper/packet_test.go b/modules/core/04-channel/v2/keeper/packet_test.go index 23f05f1e1e7..094cc1b85e1 100644 --- a/modules/core/04-channel/v2/keeper/packet_test.go +++ b/modules/core/04-channel/v2/keeper/packet_test.go @@ -253,6 +253,8 @@ func (suite *KeeperTestSuite) TestWriteAcknowledgement() { "failure: client not found", func() { packet.DestinationClient = ibctesting.InvalidID + suite.chainB.App.GetIBCKeeper().ChannelKeeperV2.SetPacketReceipt(suite.chainB.GetContext(), packet.DestinationClient, packet.Sequence) + suite.chainB.App.GetIBCKeeper().ChannelKeeperV2.SetAsyncPacket(suite.chainB.GetContext(), packet.DestinationClient, packet.Sequence, packet) }, clienttypes.ErrCounterpartyNotFound, }, @@ -260,6 +262,8 @@ func (suite *KeeperTestSuite) TestWriteAcknowledgement() { "failure: counterparty client identifier different than source client", func() { packet.SourceClient = unusedChannel + suite.chainB.App.GetIBCKeeper().ChannelKeeperV2.SetPacketReceipt(suite.chainB.GetContext(), packet.DestinationClient, packet.Sequence) + suite.chainB.App.GetIBCKeeper().ChannelKeeperV2.SetAsyncPacket(suite.chainB.GetContext(), packet.DestinationClient, packet.Sequence, packet) }, clienttypes.ErrInvalidCounterparty, }, @@ -275,9 +279,17 @@ func (suite *KeeperTestSuite) TestWriteAcknowledgement() { "failure: receipt not found for packet", func() { packet.Sequence = 2 + suite.chainB.App.GetIBCKeeper().ChannelKeeperV2.SetAsyncPacket(suite.chainB.GetContext(), packet.DestinationClient, packet.Sequence, packet) }, types.ErrInvalidPacket, }, + { + "failure: async packet not found", + func() { + suite.chainB.App.GetIBCKeeper().ChannelKeeperV2.DeleteAsyncPacket(suite.chainB.GetContext(), packet.DestinationClient, packet.Sequence) + }, + types.ErrInvalidAcknowledgement, + }, } for _, tc := range testCases {