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

perf: minimize necessary execution on recvpacket checktx (backport #6302) #6333

Merged
merged 2 commits into from
May 20, 2024
Merged
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
### Improvements

* (apps/27-interchain-accounts) [\#6144](https://github.com/cosmos/ibc-go/pull/6144) Emit an event signalling that the host submodule is disabled.
* (core/ante) [\#6302](https://github.com/cosmos/ibc-go/pull/6302) Performance: Skip app callbacks during RecvPacket execution in checkTx within the redundant relay ante handler.

### Features

Expand Down
41 changes: 40 additions & 1 deletion modules/core/ante/ante.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package ante

import (
sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"

clienttypes "github.com/cosmos/ibc-go/v7/modules/core/02-client/types"
channeltypes "github.com/cosmos/ibc-go/v7/modules/core/04-channel/types"
Expand Down Expand Up @@ -30,10 +31,22 @@ 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(sdk.WrapSDKContext(ctx), msg)
var (
response *channeltypes.MsgRecvPacketResponse
err error
)
// when we are in ReCheckTx mode, ctx.IsCheckTx() will also return true
// there we must start the if statement on ctx.IsReCheckTx() to correctly
// determine which mode we are in
if ctx.IsReCheckTx() {
response, err = rrd.k.RecvPacket(sdk.WrapSDKContext(ctx), msg)
} else {
response, err = rrd.recvPacketCheckTx(ctx, msg)
}
if err != nil {
return ctx, err
}

if response.Result == channeltypes.NOOP {
redundancies++
}
Expand Down Expand Up @@ -90,3 +103,29 @@ func (rrd RedundantRelayDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simula
}
return next(ctx, tx, simulate)
}

// recvPacketCheckTx runs a subset of ibc recv packet logic to be used specifically within the RedundantRelayDecorator AnteHandler.
// It only performs core IBC receiving logic and skips any application logic.
func (rrd RedundantRelayDecorator) recvPacketCheckTx(ctx sdk.Context, msg *channeltypes.MsgRecvPacket) (*channeltypes.MsgRecvPacketResponse, error) {
// grab channel capability
_, capability, err := rrd.k.ChannelKeeper.LookupModuleByChannel(ctx, msg.Packet.DestinationPort, msg.Packet.DestinationChannel)
if err != nil {
return nil, sdkerrors.Wrap(err, "could not retrieve module from port-id")
}

// If the packet was already received, perform a no-op
// Use a cached context to prevent accidental state changes
cacheCtx, writeFn := ctx.CacheContext()
err = rrd.k.ChannelKeeper.RecvPacket(cacheCtx, capability, msg.Packet, msg.ProofCommitment, msg.ProofHeight)

switch err {
case nil:
writeFn()
case channeltypes.ErrNoOpMsg:
return &channeltypes.MsgRecvPacketResponse{Result: channeltypes.NOOP}, nil
default:
return nil, sdkerrors.Wrap(err, "receive packet verification failed")
}

return &channeltypes.MsgRecvPacketResponse{Result: channeltypes.SUCCESS}, nil
}
14 changes: 12 additions & 2 deletions modules/core/ante/ante_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,11 @@ package ante_test
import (
"testing"

sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/stretchr/testify/require"
"github.com/stretchr/testify/suite"

sdk "github.com/cosmos/cosmos-sdk/types"

clienttypes "github.com/cosmos/ibc-go/v7/modules/core/02-client/types"
channeltypes "github.com/cosmos/ibc-go/v7/modules/core/04-channel/types"
host "github.com/cosmos/ibc-go/v7/modules/core/24-host"
Expand Down Expand Up @@ -45,7 +46,7 @@ func TestAnteTestSuite(t *testing.T) {
}

// createRecvPacketMessage creates a RecvPacket message for a packet sent from chain A to chain B.
func (suite *AnteTestSuite) createRecvPacketMessage(isRedundant bool) sdk.Msg {
func (suite *AnteTestSuite) createRecvPacketMessage(isRedundant bool) *channeltypes.MsgRecvPacket {
sequence, err := suite.path.EndpointA.SendPacket(clienttypes.NewHeight(2, 0), 0, ibctesting.MockPacketData)
suite.Require().NoError(err)

Expand Down Expand Up @@ -450,6 +451,15 @@ func (suite *AnteTestSuite) TestAnteDecorator() {
},
false,
},
{
"no success on recvPacket checkTx, no capability found",
func(suite *AnteTestSuite) []sdk.Msg {
msg := suite.createRecvPacketMessage(false)
msg.Packet.DestinationPort = "invalid-port"
return []sdk.Msg{msg}
},
false,
},
}

for _, tc := range testCases {
Expand Down
Loading