Skip to content

Commit 85b0edf

Browse files
colin-axnermergify[bot]
authored andcommitted
perf: minimize necessary execution on recvpacket checktx (#6302)
* perf: only perform core ibc logic on recvpacket checktx * try me linter * fix: reorder if and add comment * chore: add changelog entry (cherry picked from commit 0993246) # Conflicts: # CHANGELOG.md # modules/core/ante/ante.go # modules/core/ante/ante_test.go
1 parent f6fe145 commit 85b0edf

File tree

3 files changed

+109
-2
lines changed

3 files changed

+109
-2
lines changed

CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,11 @@ Ref: https://keepachangelog.com/en/1.0.0/
6363
* (apps/27-interchain-accounts, apps/tranfer, apps/29-fee) [\#6253](https://github.com/cosmos/ibc-go/pull/6253) Allow channel handshake to succeed if fee middleware is wired up on one side, but not the other.
6464
* (apps/27-interchain-accounts) [\#6251](https://github.com/cosmos/ibc-go/pull/6251) Use `UNORDERED` as the default ordering for new ICA channels.
6565
* (apps/transfer) [\#6268](https://github.com/cosmos/ibc-go/pull/6268) Use memo strings instead of JSON keys in `AllowedPacketData` of transfer authorization.
66+
<<<<<<< HEAD
67+
=======
68+
* (core/ante) [\#6278](https://github.com/cosmos/ibc-go/pull/6278) Performance: Exclude pruning from tendermint client updates in ante handler executions.
69+
* (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.
70+
>>>>>>> 0993246f (perf: minimize necessary execution on recvpacket checktx (#6302))
6671
6772
### Features
6873

modules/core/ante/ante.go

Lines changed: 73 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,22 @@ func (rrd RedundantRelayDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simula
3030
for _, m := range tx.GetMsgs() {
3131
switch msg := m.(type) {
3232
case *channeltypes.MsgRecvPacket:
33-
response, err := rrd.k.RecvPacket(ctx, msg)
33+
var (
34+
response *channeltypes.MsgRecvPacketResponse
35+
err error
36+
)
37+
// when we are in ReCheckTx mode, ctx.IsCheckTx() will also return true
38+
// there we must start the if statement on ctx.IsReCheckTx() to correctly
39+
// determine which mode we are in
40+
if ctx.IsReCheckTx() {
41+
response, err = rrd.k.RecvPacket(ctx, msg)
42+
} else {
43+
response, err = rrd.recvPacketCheckTx(ctx, msg)
44+
}
3445
if err != nil {
3546
return ctx, err
3647
}
48+
3749
if response.Result == channeltypes.NOOP {
3850
redundancies++
3951
}
@@ -90,3 +102,63 @@ func (rrd RedundantRelayDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simula
90102
}
91103
return next(ctx, tx, simulate)
92104
}
105+
<<<<<<< HEAD
106+
=======
107+
108+
// recvPacketCheckTx runs a subset of ibc recv packet logic to be used specifically within the RedundantRelayDecorator AnteHandler.
109+
// It only performs core IBC receiving logic and skips any application logic.
110+
func (rrd RedundantRelayDecorator) recvPacketCheckTx(ctx sdk.Context, msg *channeltypes.MsgRecvPacket) (*channeltypes.MsgRecvPacketResponse, error) {
111+
// grab channel capability
112+
_, capability, err := rrd.k.ChannelKeeper.LookupModuleByChannel(ctx, msg.Packet.DestinationPort, msg.Packet.DestinationChannel)
113+
if err != nil {
114+
return nil, errorsmod.Wrap(err, "could not retrieve module from port-id")
115+
}
116+
117+
// If the packet was already received, perform a no-op
118+
// Use a cached context to prevent accidental state changes
119+
cacheCtx, writeFn := ctx.CacheContext()
120+
err = rrd.k.ChannelKeeper.RecvPacket(cacheCtx, capability, msg.Packet, msg.ProofCommitment, msg.ProofHeight)
121+
122+
switch err {
123+
case nil:
124+
writeFn()
125+
case channeltypes.ErrNoOpMsg:
126+
return &channeltypes.MsgRecvPacketResponse{Result: channeltypes.NOOP}, nil
127+
default:
128+
return nil, errorsmod.Wrap(err, "receive packet verification failed")
129+
}
130+
131+
return &channeltypes.MsgRecvPacketResponse{Result: channeltypes.SUCCESS}, nil
132+
}
133+
134+
// updateClientCheckTx runs a subset of ibc client update logic to be used specifically within the RedundantRelayDecorator AnteHandler.
135+
// The following function performs ibc client message verification for CheckTx only and state updates in both CheckTx and ReCheckTx.
136+
// Note that misbehaviour checks are omitted.
137+
func (rrd RedundantRelayDecorator) updateClientCheckTx(ctx sdk.Context, msg *clienttypes.MsgUpdateClient) error {
138+
clientMsg, err := clienttypes.UnpackClientMessage(msg.ClientMessage)
139+
if err != nil {
140+
return err
141+
}
142+
143+
if status := rrd.k.ClientKeeper.GetClientStatus(ctx, msg.ClientId); status != exported.Active {
144+
return errorsmod.Wrapf(clienttypes.ErrClientNotActive, "cannot update client (%s) with status %s", msg.ClientId, status)
145+
}
146+
147+
clientModule, found := rrd.k.ClientKeeper.Route(msg.ClientId)
148+
if !found {
149+
return errorsmod.Wrap(clienttypes.ErrRouteNotFound, msg.ClientId)
150+
}
151+
152+
if !ctx.IsReCheckTx() {
153+
if err := clientModule.VerifyClientMessage(ctx, msg.ClientId, clientMsg); err != nil {
154+
return err
155+
}
156+
}
157+
158+
heights := clientModule.UpdateState(ctx, msg.ClientId, clientMsg)
159+
160+
ctx.Logger().With("module", "x/"+exported.ModuleName).Debug("ante ibc client update", "consensusHeights", heights)
161+
162+
return nil
163+
}
164+
>>>>>>> 0993246f (perf: minimize necessary execution on recvpacket checktx (#6302))

modules/core/ante/ante_test.go

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,18 @@
11
package ante_test
22

33
import (
4+
"fmt"
45
"testing"
56

67
"github.com/stretchr/testify/require"
78
testifysuite "github.com/stretchr/testify/suite"
89

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

12+
capabilitytypes "github.com/cosmos/ibc-go/modules/capability/types"
1113
clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types"
1214
channeltypes "github.com/cosmos/ibc-go/v8/modules/core/04-channel/types"
15+
commitmenttypes "github.com/cosmos/ibc-go/v8/modules/core/23-commitment/types"
1316
host "github.com/cosmos/ibc-go/v8/modules/core/24-host"
1417
"github.com/cosmos/ibc-go/v8/modules/core/ante"
1518
"github.com/cosmos/ibc-go/v8/modules/core/exported"
@@ -46,7 +49,7 @@ func TestAnteTestSuite(t *testing.T) {
4649
}
4750

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

@@ -342,6 +345,20 @@ func (suite *AnteTestSuite) TestAnteDecorator() {
342345
},
343346
true,
344347
},
348+
{
349+
"success on app callback error, app callbacks are skipped for performance",
350+
func(suite *AnteTestSuite) []sdk.Msg {
351+
suite.chainB.GetSimApp().IBCMockModule.IBCApp.OnRecvPacket = func(
352+
ctx sdk.Context, packet channeltypes.Packet, relayer sdk.AccAddress,
353+
) exported.Acknowledgement {
354+
panic(fmt.Errorf("failed OnRecvPacket mock callback"))
355+
}
356+
357+
// the RecvPacket message has not been submitted to the chain yet, so it will succeed
358+
return []sdk.Msg{suite.createRecvPacketMessage(false)}
359+
},
360+
nil,
361+
},
345362
{
346363
"no success on one redundant RecvPacket message",
347364
func(suite *AnteTestSuite) []sdk.Msg {
@@ -425,7 +442,11 @@ func (suite *AnteTestSuite) TestAnteDecorator() {
425442
channeltypes.NewMsgRecvPacket(packet, []byte("proof"), clienttypes.NewHeight(1, 1), "signer"),
426443
}
427444
},
445+
<<<<<<< HEAD
428446
false,
447+
=======
448+
commitmenttypes.ErrInvalidProof,
449+
>>>>>>> 0993246f (perf: minimize necessary execution on recvpacket checktx (#6302))
429450
},
430451
{
431452
"no success on one new message and one redundant message in the same block",
@@ -451,6 +472,15 @@ func (suite *AnteTestSuite) TestAnteDecorator() {
451472
},
452473
false,
453474
},
475+
{
476+
"no success on recvPacket checkTx, no capability found",
477+
func(suite *AnteTestSuite) []sdk.Msg {
478+
msg := suite.createRecvPacketMessage(false)
479+
msg.Packet.DestinationPort = "invalid-port"
480+
return []sdk.Msg{msg}
481+
},
482+
capabilitytypes.ErrCapabilityNotFound,
483+
},
454484
}
455485

456486
for _, tc := range testCases {

0 commit comments

Comments
 (0)