diff --git a/modules/core/04-channel/v2/keeper/events.go b/modules/core/04-channel/v2/keeper/events.go index 9d90ff5b36e..2d1927a809e 100644 --- a/modules/core/04-channel/v2/keeper/events.go +++ b/modules/core/04-channel/v2/keeper/events.go @@ -25,3 +25,8 @@ func EmitAcknowledgePacketEvents(ctx context.Context, packet channeltypesv2.Pack func EmitTimeoutPacketEvents(ctx context.Context, packet channeltypesv2.Packet) { // TODO: https://github.com/cosmos/ibc-go/issues/7386 } + +// EmitWriteAcknowledgementEvents emits events for WriteAcknowledgement. +func EmitWriteAcknowledgementEvents(ctx context.Context, packet channeltypesv2.Packet, ack channeltypesv2.Acknowledgement) { + // TODO: https://github.com/cosmos/ibc-go/issues/7386 +} diff --git a/modules/core/04-channel/v2/keeper/export_test.go b/modules/core/04-channel/v2/keeper/export_test.go new file mode 100644 index 00000000000..cef873b66d3 --- /dev/null +++ b/modules/core/04-channel/v2/keeper/export_test.go @@ -0,0 +1,17 @@ +package keeper + +import ( + "context" + + hostv2 "github.com/cosmos/ibc-go/v9/modules/core/24-host/v2" +) + +// GetPacketAcknowledgement fetches the packet acknowledgement from the store. +func (k *Keeper) GetPacketAcknowledgement(ctx context.Context, sourceID string, sequence uint64) []byte { + store := k.storeService.OpenKVStore(ctx) + bz, err := store.Get(hostv2.PacketAcknowledgementKey(sourceID, sequence)) + if err != nil { + panic(err) + } + return bz +} diff --git a/modules/core/04-channel/v2/keeper/msg_server.go b/modules/core/04-channel/v2/keeper/msg_server.go index 740ebb02daa..b3323a44fd2 100644 --- a/modules/core/04-channel/v2/keeper/msg_server.go +++ b/modules/core/04-channel/v2/keeper/msg_server.go @@ -2,6 +2,7 @@ package keeper import ( "context" + "slices" errorsmod "cosmossdk.io/errors" @@ -9,6 +10,8 @@ import ( channeltypesv1 "github.com/cosmos/ibc-go/v9/modules/core/04-channel/types" channeltypesv2 "github.com/cosmos/ibc-go/v9/modules/core/04-channel/v2/types" + telemetryv2 "github.com/cosmos/ibc-go/v9/modules/core/internal/v2/telemetry" + coretypes "github.com/cosmos/ibc-go/v9/modules/core/types" ) var _ channeltypesv2.MsgServer = &Keeper{} @@ -80,11 +83,6 @@ func (k *Keeper) Acknowledgement(ctx context.Context, msg *channeltypesv2.MsgAck // RecvPacket implements the PacketMsgServer RecvPacket method. func (k *Keeper) RecvPacket(ctx context.Context, msg *channeltypesv2.MsgRecvPacket) (*channeltypesv2.MsgRecvPacketResponse, error) { sdkCtx := sdk.UnwrapSDKContext(ctx) - err := k.recvPacket(ctx, msg.Packet, msg.ProofCommitment, msg.ProofHeight) - if err != nil { - sdkCtx.Logger().Error("receive packet failed", "source-channel", msg.Packet.SourceChannel, "dest-channel", msg.Packet.DestinationChannel, "error", errorsmod.Wrap(err, "send packet failed")) - return nil, errorsmod.Wrapf(err, "receive packet failed for source id: %s and destination id: %s", msg.Packet.SourceChannel, msg.Packet.DestinationChannel) - } signer, err := sdk.AccAddressFromBech32(msg.Signer) if err != nil { @@ -92,18 +90,73 @@ func (k *Keeper) RecvPacket(ctx context.Context, msg *channeltypesv2.MsgRecvPack return nil, errorsmod.Wrap(err, "invalid address for msg Signer") } - _ = signer + // Perform TAO verification + // + // If the packet was already received, perform a no-op + // Use a cached context to prevent accidental state changes + cacheCtx, writeFn := sdkCtx.CacheContext() + err = k.recvPacket(cacheCtx, msg.Packet, msg.ProofCommitment, msg.ProofHeight) - // TODO: implement once app router is wired up. - // https://github.com/cosmos/ibc-go/issues/7384 - // for _, pd := range packet.PacketData { - // cbs := k.PortKeeper.AppRouter.Route(pd.SourcePort) - // err := cbs.OnRecvPacket(ctx, packet, msg.ProofCommitment, msg.ProofHeight, signer) - // if err != nil { - // return nil, err - // } - // } + switch err { + case nil: + writeFn() + case channeltypesv1.ErrNoOpMsg: + // no-ops do not need event emission as they will be ignored + sdkCtx.Logger().Debug("no-op on redundant relay", "source-channel", msg.Packet.SourceChannel) + return &channeltypesv2.MsgRecvPacketResponse{Result: channeltypesv1.NOOP}, nil + default: + sdkCtx.Logger().Error("receive packet failed", "source-channel", msg.Packet.SourceChannel, "error", errorsmod.Wrap(err, "receive packet verification failed")) + return nil, errorsmod.Wrap(err, "receive packet verification failed") + } + + // build up the recv results for each application callback. + ack := channeltypesv2.Acknowledgement{ + AcknowledgementResults: []channeltypesv2.AcknowledgementResult{}, + } + for _, pd := range msg.Packet.Data { + // Cache context so that we may discard state changes from callback if the acknowledgement is unsuccessful. + cacheCtx, writeFn = sdkCtx.CacheContext() + cb := k.Router.Route(pd.DestinationPort) + res := cb.OnRecvPacket(cacheCtx, msg.Packet.SourceChannel, msg.Packet.DestinationChannel, pd, signer) + + if res.Status != channeltypesv2.PacketStatus_Failure { + // write application state changes for asynchronous and successful acknowledgements + writeFn() + } else { + // Modify events in cached context to reflect unsuccessful acknowledgement + sdkCtx.EventManager().EmitEvents(convertToErrorEvents(cacheCtx.EventManager().Events())) + } + + ack.AcknowledgementResults = append(ack.AcknowledgementResults, channeltypesv2.AcknowledgementResult{ + AppName: pd.DestinationPort, + RecvPacketResult: res, + }) + } + + // note this should never happen as the packet data would have had to be empty. + if len(ack.AcknowledgementResults) == 0 { + sdkCtx.Logger().Error("receive packet failed", "source-channel", msg.Packet.SourceChannel, "error", errorsmod.Wrap(err, "invalid acknowledgement results")) + return &channeltypesv2.MsgRecvPacketResponse{Result: channeltypesv1.FAILURE}, nil + } + + // NOTE: TBD how we will handle async acknowledgements with more than one packet data. + isAsync := slices.ContainsFunc(ack.AcknowledgementResults, func(ackResult channeltypesv2.AcknowledgementResult) bool { + return ackResult.RecvPacketResult.Status == channeltypesv2.PacketStatus_Async + }) + + if !isAsync { + // 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 { + return nil, err + } + } + + defer telemetryv2.ReportRecvPacket(msg.Packet) + + sdkCtx.Logger().Info("receive packet callback succeeded", "source-channel", msg.Packet.SourceChannel, "dest-channel", msg.Packet.DestinationChannel, "result", channeltypesv1.SUCCESS.String()) return &channeltypesv2.MsgRecvPacketResponse{Result: channeltypesv1.SUCCESS}, nil } @@ -135,3 +188,24 @@ func (k *Keeper) Timeout(ctx context.Context, timeout *channeltypesv2.MsgTimeout return &channeltypesv2.MsgTimeoutResponse{Result: channeltypesv1.SUCCESS}, nil } + +// convertToErrorEvents converts all events to error events by appending the +// error attribute prefix to each event's attribute key. +// TODO: https://github.com/cosmos/ibc-go/issues/7436 +func convertToErrorEvents(events sdk.Events) sdk.Events { + if events == nil { + return nil + } + + newEvents := make(sdk.Events, len(events)) + for i, event := range events { + newAttributes := make([]sdk.Attribute, len(event.Attributes)) + for j, attribute := range event.Attributes { + newAttributes[j] = sdk.NewAttribute(coretypes.ErrorAttributeKeyPrefix+attribute.Key, attribute.Value) + } + + newEvents[i] = sdk.NewEvent(coretypes.ErrorAttributeKeyPrefix+event.Type, newAttributes...) + } + + return newEvents +} diff --git a/modules/core/04-channel/v2/keeper/msg_server_test.go b/modules/core/04-channel/v2/keeper/msg_server_test.go index 1f4aabd1c86..c28aed97ae4 100644 --- a/modules/core/04-channel/v2/keeper/msg_server_test.go +++ b/modules/core/04-channel/v2/keeper/msg_server_test.go @@ -9,6 +9,8 @@ import ( clienttypes "github.com/cosmos/ibc-go/v9/modules/core/02-client/types" channeltypesv1 "github.com/cosmos/ibc-go/v9/modules/core/04-channel/types" channeltypesv2 "github.com/cosmos/ibc-go/v9/modules/core/04-channel/v2/types" + commitmenttypes "github.com/cosmos/ibc-go/v9/modules/core/23-commitment/types" + hostv2 "github.com/cosmos/ibc-go/v9/modules/core/24-host/v2" ibctesting "github.com/cosmos/ibc-go/v9/testing" "github.com/cosmos/ibc-go/v9/testing/mock" mockv2 "github.com/cosmos/ibc-go/v9/testing/mock/v2" @@ -111,3 +113,157 @@ func (suite *KeeperTestSuite) TestMsgSendPacket() { }) } } + +func (suite *KeeperTestSuite) TestMsgRecvPacket() { + var ( + path *ibctesting.Path + msg *channeltypesv2.MsgRecvPacket + recvPacket channeltypesv2.Packet + expectedAck channeltypesv2.Acknowledgement + ) + + testCases := []struct { + name string + malleate func() + expError error + }{ + { + name: "success", + malleate: func() {}, + expError: nil, + }, + { + name: "success: failed recv result", + malleate: func() { + failedRecvResult := channeltypesv2.RecvPacketResult{ + Status: channeltypesv2.PacketStatus_Failure, + Acknowledgement: mock.MockFailPacketData, + } + + // a failed ack should be returned by the application. + expectedAck.AcknowledgementResults[0].RecvPacketResult = failedRecvResult + + path.EndpointB.Chain.GetSimApp().MockModuleV2B.IBCApp.OnRecvPacket = func(ctx context.Context, sourceChannel string, destinationChannel string, data channeltypesv2.PacketData, relayer sdk.AccAddress) channeltypesv2.RecvPacketResult { + return failedRecvResult + } + }, + }, + { + name: "success: async recv result", + malleate: func() { + asyncResult := channeltypesv2.RecvPacketResult{ + Status: channeltypesv2.PacketStatus_Async, + Acknowledgement: nil, + } + + // an async ack should be returned by the application. + expectedAck.AcknowledgementResults[0].RecvPacketResult = asyncResult + + path.EndpointB.Chain.GetSimApp().MockModuleV2B.IBCApp.OnRecvPacket = func(ctx context.Context, sourceChannel string, destinationChannel string, data channeltypesv2.PacketData, relayer sdk.AccAddress) channeltypesv2.RecvPacketResult { + return asyncResult + } + }, + }, + { + name: "success: NoOp", + malleate: func() { + suite.chainB.App.GetIBCKeeper().ChannelKeeperV2.SetPacketReceipt(suite.chainB.GetContext(), recvPacket.SourceChannel, recvPacket.Sequence) + expectedAck = channeltypesv2.Acknowledgement{} + }, + }, + { + name: "failure: counterparty not found", + malleate: func() { + // change the destination id to a non-existent channel. + recvPacket.DestinationChannel = "not-existent-channel" + }, + expError: channeltypesv2.ErrChannelNotFound, + }, + { + name: "failure: invalid proof", + malleate: func() { + // proof verification fails because the packet commitment is different due to a different sequence. + recvPacket.Sequence = 10 + }, + expError: commitmenttypes.ErrInvalidProof, + }, + } + + for _, tc := range testCases { + tc := tc + + suite.Run(tc.name, func() { + suite.SetupTest() // reset + + path = ibctesting.NewPath(suite.chainA, suite.chainB) + path.SetupV2() + + timeoutTimestamp := suite.chainA.GetTimeoutTimestamp() + msgSendPacket := channeltypesv2.NewMsgSendPacket(path.EndpointA.ChannelID, timeoutTimestamp, suite.chainA.SenderAccount.GetAddress().String(), mockv2.NewMockPacketData(mockv2.ModuleNameA, mockv2.ModuleNameB)) + + res, err := path.EndpointA.Chain.SendMsgs(msgSendPacket) + suite.Require().NoError(err) + suite.Require().NotNil(res) + + suite.Require().NoError(path.EndpointB.UpdateClient()) + + recvPacket = channeltypesv2.NewPacket(1, path.EndpointA.ChannelID, path.EndpointB.ChannelID, timeoutTimestamp, mockv2.NewMockPacketData(mockv2.ModuleNameA, mockv2.ModuleNameB)) + + // default expected ack is a single successful recv result for moduleB. + expectedAck = channeltypesv2.Acknowledgement{ + AcknowledgementResults: []channeltypesv2.AcknowledgementResult{ + { + AppName: mockv2.ModuleNameB, + RecvPacketResult: channeltypesv2.RecvPacketResult{ + Status: channeltypesv2.PacketStatus_Success, + Acknowledgement: mock.MockPacketData, + }, + }, + }, + } + + tc.malleate() + + // get proof of packet commitment from chainA + packetKey := hostv2.PacketCommitmentKey(recvPacket.SourceChannel, recvPacket.Sequence) + proof, proofHeight := path.EndpointA.QueryProof(packetKey) + + msg = channeltypesv2.NewMsgRecvPacket(recvPacket, proof, proofHeight, suite.chainB.SenderAccount.GetAddress().String()) + + res, err = path.EndpointB.Chain.SendMsgs(msg) + suite.Require().NoError(path.EndpointA.UpdateClient()) + + ck := path.EndpointB.Chain.GetSimApp().IBCKeeper.ChannelKeeperV2 + + expPass := tc.expError == nil + if expPass { + suite.Require().NoError(err) + suite.Require().NotNil(res) + + // packet receipt should be written + _, ok := ck.GetPacketReceipt(path.EndpointB.Chain.GetContext(), recvPacket.SourceChannel, recvPacket.Sequence) + suite.Require().True(ok) + + ackWritten := ck.HasPacketAcknowledgement(path.EndpointB.Chain.GetContext(), recvPacket.DestinationChannel, recvPacket.Sequence) + + if len(expectedAck.AcknowledgementResults) == 0 || expectedAck.AcknowledgementResults[0].RecvPacketResult.Status == channeltypesv2.PacketStatus_Async { + // ack should not be written for async app or if the packet receipt was already present. + suite.Require().False(ackWritten) + } else { // successful or failed acknowledgement + // ack should be written for synchronous app (default mock application behaviour). + suite.Require().True(ackWritten) + ackBz := path.EndpointB.Chain.Codec.MustMarshal(&expectedAck) + expectedBz := channeltypesv1.CommitAcknowledgement(ackBz) + + actualAckBz := ck.GetPacketAcknowledgement(path.EndpointB.Chain.GetContext(), recvPacket.DestinationChannel, recvPacket.Sequence) + suite.Require().Equal(expectedBz, actualAckBz) + } + + } else { + ibctesting.RequireErrorIsOrContains(suite.T(), err, tc.expError) + _, ok := ck.GetPacketReceipt(path.EndpointB.Chain.GetContext(), recvPacket.SourceChannel, recvPacket.Sequence) + suite.Require().False(ok) + } + }) + } +} diff --git a/modules/core/04-channel/v2/keeper/packet.go b/modules/core/04-channel/v2/keeper/packet.go index a3ac7e3401e..be9b06051a7 100644 --- a/modules/core/04-channel/v2/keeper/packet.go +++ b/modules/core/04-channel/v2/keeper/packet.go @@ -297,3 +297,50 @@ func (k *Keeper) timeoutPacket( return nil } + +// WriteAcknowledgement writes the acknowledgement to the store. +func (k Keeper) WriteAcknowledgement( + ctx context.Context, + packet channeltypesv2.Packet, + ack channeltypesv2.Acknowledgement, +) error { + // Lookup channel associated with destination channel ID and ensure + // that the packet was indeed sent by our counterparty by verifying + // packet sender is our channel's counterparty channel id. + channel, ok := k.GetChannel(ctx, packet.DestinationChannel) + if !ok { + // TODO: figure out how aliasing will work when more than one packet data is sent. + channel, ok = k.convertV1Channel(ctx, packet.Data[0].DestinationPort, packet.DestinationChannel) + if !ok { + return errorsmod.Wrap(channeltypes.ErrChannelNotFound, packet.DestinationChannel) + } + } + + if channel.CounterpartyChannelId != packet.SourceChannel { + return channeltypes.ErrInvalidChannelIdentifier + } + + // NOTE: IBC app modules might have written the acknowledgement synchronously on + // the OnRecvPacket callback so we need to check if the acknowledgement is already + // set on the store and return an error if so. + if k.HasPacketAcknowledgement(ctx, packet.DestinationChannel, packet.Sequence) { + return channeltypes.ErrAcknowledgementExists + } + + if _, found := k.GetPacketReceipt(ctx, packet.DestinationChannel, packet.Sequence); !found { + return errorsmod.Wrap(channeltypes.ErrInvalidPacket, "receipt not found for packet") + } + + ackBz := k.cdc.MustMarshal(&ack) + // set the acknowledgement so that it can be verified on the other side + k.SetPacketAcknowledgement( + ctx, packet.DestinationChannel, packet.GetSequence(), + channeltypes.CommitAcknowledgement(ackBz), + ) + + k.Logger(ctx).Info("acknowledgement written", "sequence", strconv.FormatUint(packet.Sequence, 10), "dest-channel", packet.DestinationChannel) + + EmitWriteAcknowledgementEvents(ctx, packet, ack) + + return nil +} diff --git a/modules/core/04-channel/v2/types/msgs.go b/modules/core/04-channel/v2/types/msgs.go index f2b52aaf802..85cfb134a21 100644 --- a/modules/core/04-channel/v2/types/msgs.go +++ b/modules/core/04-channel/v2/types/msgs.go @@ -1,5 +1,9 @@ package types +import ( + clienttypes "github.com/cosmos/ibc-go/v9/modules/core/02-client/types" +) + // NewMsgSendPacket creates a new MsgSendPacket instance. func NewMsgSendPacket(sourceChannel string, timeoutTimestamp uint64, signer string, packetData ...PacketData) *MsgSendPacket { return &MsgSendPacket{ @@ -9,3 +13,13 @@ func NewMsgSendPacket(sourceChannel string, timeoutTimestamp uint64, signer stri Signer: signer, } } + +// NewMsgRecvPacket creates a new MsgRecvPacket instance. +func NewMsgRecvPacket(packet Packet, proofCommitment []byte, proofHeight clienttypes.Height, signer string) *MsgRecvPacket { + return &MsgRecvPacket{ + Packet: packet, + ProofCommitment: proofCommitment, + ProofHeight: proofHeight, + Signer: signer, + } +} diff --git a/modules/core/internal/v2/telemetry/packet.go b/modules/core/internal/v2/telemetry/packet.go new file mode 100644 index 00000000000..7c5c8588247 --- /dev/null +++ b/modules/core/internal/v2/telemetry/packet.go @@ -0,0 +1,14 @@ +package telemetry + +import ( + channeltypesv2 "github.com/cosmos/ibc-go/v9/modules/core/04-channel/v2/types" +) + +// ReportRecvPacket TODO: https://github.com/cosmos/ibc-go/issues/7437 +func ReportRecvPacket(packet channeltypesv2.Packet) {} + +// ReportTimeoutPacket TODO: https://github.com/cosmos/ibc-go/issues/7437 +func ReportTimeoutPacket(packet channeltypesv2.Packet, timeoutType string) {} + +// ReportAcknowledgePacket TODO: https://github.com/cosmos/ibc-go/issues/7437 +func ReportAcknowledgePacket(packet channeltypesv2.Packet) {} diff --git a/testing/mock/v2/ibc_app.go b/testing/mock/v2/ibc_app.go index ddf4c67f115..6d6c72a8eb7 100644 --- a/testing/mock/v2/ibc_app.go +++ b/testing/mock/v2/ibc_app.go @@ -9,7 +9,7 @@ import ( ) type IBCApp struct { - OnSendPacket func(ctx context.Context, sourceID string, destinationID string, sequence uint64, data channeltypesv2.PacketData, signer sdk.AccAddress) error - OnRecvPacket func(ctx context.Context, sourceID string, destinationID string, data channeltypesv2.PacketData, relayer sdk.AccAddress) channeltypesv2.RecvPacketResult - OnTimeoutPacket func(ctx context.Context, sourceID string, destinationID string, data channeltypesv2.PacketData, relayer sdk.AccAddress) error + OnSendPacket func(ctx context.Context, sourceChannel string, destinationChannel string, sequence uint64, data channeltypesv2.PacketData, signer sdk.AccAddress) error + OnRecvPacket func(ctx context.Context, sourceChannel string, destinationChannel string, data channeltypesv2.PacketData, relayer sdk.AccAddress) channeltypesv2.RecvPacketResult + OnTimeoutPacket func(ctx context.Context, sourceChannel string, destinationChannel string, data channeltypesv2.PacketData, relayer sdk.AccAddress) error } diff --git a/testing/utils.go b/testing/utils.go index 53d84ab559a..861bb54e1a6 100644 --- a/testing/utils.go +++ b/testing/utils.go @@ -87,6 +87,7 @@ func UnmarshalMsgResponses(cdc codec.Codec, data []byte, msgs ...codec.ProtoMars // RequireErrorIsOrContains verifies that the passed error is either a target error or contains its error message. func RequireErrorIsOrContains(t *testing.T, err, targetError error, msgAndArgs ...interface{}) { t.Helper() + require.Error(t, err) require.True( t, errors.Is(err, targetError) ||