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.

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

Implement v2 RecvPacket rpc handler #7421

Open
wants to merge 15 commits into
base: feat/ibc-eureka
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions modules/core/04-channel/v2/keeper/events.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
17 changes: 17 additions & 0 deletions modules/core/04-channel/v2/keeper/export_test.go
Original file line number Diff line number Diff line change
@@ -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
}
104 changes: 89 additions & 15 deletions modules/core/04-channel/v2/keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,16 @@ package keeper

import (
"context"
"slices"

errorsmod "cosmossdk.io/errors"

sdk "github.com/cosmos/cosmos-sdk/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"
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{}
Expand Down Expand Up @@ -80,30 +83,80 @@ 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 {
sdkCtx.Logger().Error("receive packet failed", "error", errorsmod.Wrap(err, "invalid address for msg Signer"))
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:
Copy link
Contributor

Choose a reason for hiding this comment

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

Just noticed one thing: this case can never be true, since we check for err != nil above. I believe this is supposed to be a different err (the one at line 92? But in that case we still don't need this case)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

really nice catch! Also highlights the missing NoOp case, I'll push a fix for this

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
}

Expand Down Expand Up @@ -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...)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can avoid having the newAttributes slice if we do

	newEvents := make(sdk.Events, len(events))
	for i, event := range events {
		newEvents[i] = sdk.NewEvent(coretypes.ErrorAttributeKeyPrefix+event.Type)
		for _, attribute := range event.Attributes {
                        newEvents[i].AppendAttributes(sdk.NewAttribute(coretypes.ErrorAttributeKeyPrefix+attribute.Key, attribute.Value))
		}

	}

WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this was just copy pasted from the existing fn in v1 msg_server.go, I'll create another issue to remove the duplication, and maybe we can make these changes here? I'll link this comment in the issue.

Copy link
Contributor

Choose a reason for hiding this comment

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

ah sure no big deal!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

}

return newEvents
}
156 changes: 156 additions & 0 deletions modules/core/04-channel/v2/keeper/msg_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
}
})
}
}
47 changes: 47 additions & 0 deletions modules/core/04-channel/v2/keeper/packet.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Loading
Loading