From 7f7bc1e2a204f1acf3696210ba0ff0b7d897dc49 Mon Sep 17 00:00:00 2001 From: Dev Ojha Date: Wed, 1 May 2024 15:23:08 -0700 Subject: [PATCH 1/3] Make the ante handler not run RecvPacket middleware --- modules/core/ante/ante.go | 4 +-- modules/core/keeper/msg_server.go | 43 +++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 2 deletions(-) diff --git a/modules/core/ante/ante.go b/modules/core/ante/ante.go index a64a5f0146a..7394ddb5485 100644 --- a/modules/core/ante/ante.go +++ b/modules/core/ante/ante.go @@ -30,11 +30,11 @@ func (rrd RedundantRelayDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simula for _, m := range tx.GetMsgs() { switch msg := m.(type) { case *channeltypes.MsgRecvPacket: - response, err := rrd.k.RecvPacket(ctx, msg) + redundant, err := rrd.k.RecvPacketCheckNonRedundant(ctx, msg) if err != nil { return ctx, err } - if response.Result == channeltypes.NOOP { + if redundant { redundancies++ } packetMsgs++ diff --git a/modules/core/keeper/msg_server.go b/modules/core/keeper/msg_server.go index a30e96d1ed6..eb8848c1bc3 100644 --- a/modules/core/keeper/msg_server.go +++ b/modules/core/keeper/msg_server.go @@ -526,6 +526,49 @@ func (k *Keeper) RecvPacket(goCtx context.Context, msg *channeltypes.MsgRecvPack return &channeltypes.MsgRecvPacketResponse{Result: channeltypes.SUCCESS}, nil } +// RecvPacketCheckNonRedundant checks that a MsgRecvPacket is: +// 1) Valid +// 2) Not a duplicate of a packet already received +// It is only intended to be ran on CheckTx. +func (k *Keeper) RecvPacketCheckNonRedundant(goCtx context.Context, msg *channeltypes.MsgRecvPacket) (bool, error) { + ctx := sdk.UnwrapSDKContext(goCtx) + + // Lookup module by channel capability + module, capability, err := k.ChannelKeeper.LookupModuleByChannel(ctx, msg.Packet.DestinationPort, msg.Packet.DestinationChannel) + if err != nil { + ctx.Logger().Error("receive packet failed", "port-id", msg.Packet.SourcePort, "channel-id", msg.Packet.SourceChannel, "error", errorsmod.Wrap(err, "could not retrieve module from port-id")) + return false, errorsmod.Wrap(err, "could not retrieve module from port-id") + } + + // Retrieve callbacks from router + _, ok := k.PortKeeper.Route(module) + if !ok { + ctx.Logger().Error("receive packet failed", "port-id", msg.Packet.SourcePort, "error", errorsmod.Wrapf(porttypes.ErrInvalidRoute, "route not found to module: %s", module)) + return false, errorsmod.Wrapf(porttypes.ErrInvalidRoute, "route not found to module: %s", module) + } + + // Perform TAO verification + // + // If the packet was already received, perform a no-op + // Use a cached context to prevent accidental state changes + cacheCtx, writeFn := ctx.CacheContext() + // TODO: Update RecvPacket to skip cryptographic checks on Recheck. + err = k.ChannelKeeper.RecvPacket(cacheCtx, capability, msg.Packet, msg.ProofCommitment, msg.ProofHeight) + + switch err { + case nil: + writeFn() + case channeltypes.ErrNoOpMsg: + // no-ops do not need event emission as they will be ignored + ctx.Logger().Debug("no-op on redundant relay", "port-id", msg.Packet.SourcePort, "channel-id", msg.Packet.SourceChannel) + return true, nil + default: + ctx.Logger().Error("receive packet failed", "port-id", msg.Packet.SourcePort, "channel-id", msg.Packet.SourceChannel, "error", errorsmod.Wrap(err, "receive packet verification failed")) + return false, errorsmod.Wrap(err, "receive packet verification failed") + } + return false, nil +} + // Timeout defines a rpc handler method for MsgTimeout. func (k *Keeper) Timeout(goCtx context.Context, msg *channeltypes.MsgTimeout) (*channeltypes.MsgTimeoutResponse, error) { ctx := sdk.UnwrapSDKContext(goCtx) From 079a3ea2333f5a40a2bcda7936d88630e1ba1cd3 Mon Sep 17 00:00:00 2001 From: Dev Ojha Date: Wed, 1 May 2024 15:24:06 -0700 Subject: [PATCH 2/3] Remove log messages --- modules/core/keeper/msg_server.go | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/modules/core/keeper/msg_server.go b/modules/core/keeper/msg_server.go index eb8848c1bc3..2c43dd11944 100644 --- a/modules/core/keeper/msg_server.go +++ b/modules/core/keeper/msg_server.go @@ -536,14 +536,12 @@ func (k *Keeper) RecvPacketCheckNonRedundant(goCtx context.Context, msg *channel // Lookup module by channel capability module, capability, err := k.ChannelKeeper.LookupModuleByChannel(ctx, msg.Packet.DestinationPort, msg.Packet.DestinationChannel) if err != nil { - ctx.Logger().Error("receive packet failed", "port-id", msg.Packet.SourcePort, "channel-id", msg.Packet.SourceChannel, "error", errorsmod.Wrap(err, "could not retrieve module from port-id")) return false, errorsmod.Wrap(err, "could not retrieve module from port-id") } // Retrieve callbacks from router _, ok := k.PortKeeper.Route(module) if !ok { - ctx.Logger().Error("receive packet failed", "port-id", msg.Packet.SourcePort, "error", errorsmod.Wrapf(porttypes.ErrInvalidRoute, "route not found to module: %s", module)) return false, errorsmod.Wrapf(porttypes.ErrInvalidRoute, "route not found to module: %s", module) } @@ -552,18 +550,15 @@ func (k *Keeper) RecvPacketCheckNonRedundant(goCtx context.Context, msg *channel // If the packet was already received, perform a no-op // Use a cached context to prevent accidental state changes cacheCtx, writeFn := ctx.CacheContext() - // TODO: Update RecvPacket to skip cryptographic checks on Recheck. + // TODO: Update RecvPacket to skip MT inclusion checks on Recheck. err = k.ChannelKeeper.RecvPacket(cacheCtx, capability, msg.Packet, msg.ProofCommitment, msg.ProofHeight) switch err { case nil: writeFn() case channeltypes.ErrNoOpMsg: - // no-ops do not need event emission as they will be ignored - ctx.Logger().Debug("no-op on redundant relay", "port-id", msg.Packet.SourcePort, "channel-id", msg.Packet.SourceChannel) return true, nil default: - ctx.Logger().Error("receive packet failed", "port-id", msg.Packet.SourcePort, "channel-id", msg.Packet.SourceChannel, "error", errorsmod.Wrap(err, "receive packet verification failed")) return false, errorsmod.Wrap(err, "receive packet verification failed") } return false, nil From 6029be0f27f8382c0bdaede9a666c99209978561 Mon Sep 17 00:00:00 2001 From: Dev Ojha Date: Wed, 1 May 2024 15:38:22 -0700 Subject: [PATCH 3/3] Update comment --- modules/core/keeper/msg_server.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/modules/core/keeper/msg_server.go b/modules/core/keeper/msg_server.go index 2c43dd11944..db761566303 100644 --- a/modules/core/keeper/msg_server.go +++ b/modules/core/keeper/msg_server.go @@ -551,6 +551,8 @@ func (k *Keeper) RecvPacketCheckNonRedundant(goCtx context.Context, msg *channel // Use a cached context to prevent accidental state changes cacheCtx, writeFn := ctx.CacheContext() // TODO: Update RecvPacket to skip MT inclusion checks on Recheck. + // Its slightly involved because right now every client is responsible for checking that the + // height exists on the client, rather than the channel keeper. err = k.ChannelKeeper.RecvPacket(cacheCtx, capability, msg.Packet, msg.ProofCommitment, msg.ProofHeight) switch err {