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) #6335

Merged
merged 3 commits into from
May 21, 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 @@ -43,6 +43,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
### State Machine Breaking

### Improvements
* (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
42 changes: 41 additions & 1 deletion modules/core/ante/ante.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package ante

import (
errorsmod "cosmossdk.io/errors"

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

clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types"
Expand Down Expand Up @@ -30,10 +32,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(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(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 +104,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, errorsmod.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, errorsmod.Wrap(err, "receive packet verification failed")
}

return &channeltypes.MsgRecvPacketResponse{Result: channeltypes.SUCCESS}, nil
}
26 changes: 25 additions & 1 deletion modules/core/ante/ante_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package ante_test

import (
"fmt"
"testing"

"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -46,7 +47,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 @@ -342,6 +343,20 @@ func (suite *AnteTestSuite) TestAnteDecorator() {
},
true,
},
{
"success on app callback error, app callbacks are skipped for performance",
func(suite *AnteTestSuite) []sdk.Msg {
suite.chainB.GetSimApp().IBCMockModule.IBCApp.OnRecvPacket = func(
ctx sdk.Context, packet channeltypes.Packet, relayer sdk.AccAddress,
) exported.Acknowledgement {
panic(fmt.Errorf("failed OnRecvPacket mock callback"))
}

// the RecvPacket message has not been submitted to the chain yet, so it will succeed
return []sdk.Msg{suite.createRecvPacketMessage(false)}
},
true,
},
{
"no success on one redundant RecvPacket message",
func(suite *AnteTestSuite) []sdk.Msg {
Expand Down Expand Up @@ -451,6 +466,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