diff --git a/CHANGELOG.md b/CHANGELOG.md index aafa5ef067df..385c6e454466 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,6 +40,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (store) [\#10026](https://github.com/cosmos/cosmos-sdk/pull/10026) Improve CacheKVStore datastructures / algorithms, to no longer take O(N^2) time when interleaving iterators and insertions. * (store) [\#10040](https://github.com/cosmos/cosmos-sdk/pull/10040) Bump IAVL to v0.17.1 which includes performance improvements on a batch load. +* [\#10211](https://github.com/cosmos/cosmos-sdk/pull/10211) Backport of the mechanism to reject redundant IBC transactions from [ibc-go \#235](https://github.com/cosmos/ibc-go/pull/235). ### Bug Fixes diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 35ff8ae10d80..98bd917f85f3 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -1,3 +1,7 @@ +# Cosmos SDK v0.42.10 "Stargate" Release Notes + +This release includes a new `AnteHandler` that rejects redundant IBC transactions to save relayers fees. This is a backport of [ibc-go \#235](https://github.com/cosmos/ibc-go/pull/235). + # Cosmos SDK v0.42.9 "Stargate" Release Notes This release includes an important `x/capabiliy` module bug fix for 0.42.7 and 0.42.8 which prohibits IBC to create new channels (issuse [\#9800](https://github.com/cosmos/cosmos-sdk/issues/9800)). diff --git a/simapp/ante_handler.go b/simapp/ante_handler.go new file mode 100644 index 000000000000..954a3b7c55b6 --- /dev/null +++ b/simapp/ante_handler.go @@ -0,0 +1,36 @@ +package simapp + +import ( + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/x/auth/ante" + "github.com/cosmos/cosmos-sdk/x/auth/signing" + "github.com/cosmos/cosmos-sdk/x/gov/types" + channelkeeper "github.com/cosmos/cosmos-sdk/x/ibc/core/04-channel/keeper" + ibcante "github.com/cosmos/cosmos-sdk/x/ibc/core/ante" +) + +func NewAnteHandler( + ak ante.AccountKeeper, + bankKeeper types.BankKeeper, //nolint:interfacer + sigGasConsumer ante.SignatureVerificationGasConsumer, + signModeHandler signing.SignModeHandler, + channelKeeper channelkeeper.Keeper, +) sdk.AnteHandler { + return sdk.ChainAnteDecorators( + ante.NewSetUpContextDecorator(), // outermost AnteDecorator. SetUpContext must be called first + ante.NewRejectExtensionOptionsDecorator(), + ante.NewMempoolFeeDecorator(), + ante.NewValidateBasicDecorator(), + ante.TxTimeoutHeightDecorator{}, + ante.NewValidateMemoDecorator(ak), + ante.NewConsumeGasForTxSizeDecorator(ak), + ante.NewRejectFeeGranterDecorator(), + ante.NewSetPubKeyDecorator(ak), // SetPubKeyDecorator must be called before all signature verification decorators + ante.NewValidateSigCountDecorator(ak), + ante.NewDeductFeeDecorator(ak, bankKeeper), + ante.NewSigGasConsumeDecorator(ak, sigGasConsumer), + ante.NewSigVerificationDecorator(ak, signModeHandler), + ante.NewIncrementSequenceDecorator(ak), + ibcante.NewAnteDecorator(channelKeeper), + ) +} diff --git a/simapp/app.go b/simapp/app.go index 18221250a7f7..b60693f7a949 100644 --- a/simapp/app.go +++ b/simapp/app.go @@ -403,9 +403,9 @@ func NewSimApp( app.SetInitChainer(app.InitChainer) app.SetBeginBlocker(app.BeginBlocker) app.SetAnteHandler( - ante.NewAnteHandler( + NewAnteHandler( app.AccountKeeper, app.BankKeeper, ante.DefaultSigVerificationGasConsumer, - encodingConfig.TxConfig.SignModeHandler(), + encodingConfig.TxConfig.SignModeHandler(), app.IBCKeeper.ChannelKeeper, ), ) app.SetEndBlocker(app.EndBlocker) diff --git a/x/ibc/core/04-channel/types/errors.go b/x/ibc/core/04-channel/types/errors.go index 82cf773057d7..b1d11e1fddd5 100644 --- a/x/ibc/core/04-channel/types/errors.go +++ b/x/ibc/core/04-channel/types/errors.go @@ -25,4 +25,7 @@ var ( ErrPacketReceived = sdkerrors.Register(SubModuleName, 18, "packet already received") ErrAcknowledgementExists = sdkerrors.Register(SubModuleName, 19, "acknowledgement for packet already exists") ErrInvalidChannelIdentifier = sdkerrors.Register(SubModuleName, 20, "invalid channel identifier") + + // Antehandler error + ErrRedundantTx = sdkerrors.Register(SubModuleName, 22, "packet messages are redundant") ) diff --git a/x/ibc/core/ante/ante.go b/x/ibc/core/ante/ante.go new file mode 100644 index 000000000000..b3cae30245e5 --- /dev/null +++ b/x/ibc/core/ante/ante.go @@ -0,0 +1,72 @@ +package ante + +import ( + sdk "github.com/cosmos/cosmos-sdk/types" + clienttypes "github.com/cosmos/cosmos-sdk/x/ibc/core/02-client/types" + channelkeeper "github.com/cosmos/cosmos-sdk/x/ibc/core/04-channel/keeper" + channeltypes "github.com/cosmos/cosmos-sdk/x/ibc/core/04-channel/types" +) + +type Decorator struct { + k channelkeeper.Keeper +} + +func NewAnteDecorator(k channelkeeper.Keeper) Decorator { + return Decorator{k: k} +} + +// AnteDecorator returns an error if a multiMsg tx only contains packet messages (Recv, Ack, Timeout) and additional update messages and all packet messages +// are redundant. If the transaction is just a single UpdateClient message, or the multimsg transaction contains some other message type, then the antedecorator returns no error +// and continues processing to ensure these transactions are included. +// This will ensure that relayers do not waste fees on multiMsg transactions when another relayer has already submitted all packets, by rejecting the tx at the mempool layer. +func (ad Decorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (sdk.Context, error) { + // do not run redundancy check on DeliverTx or simulate + if (ctx.IsCheckTx() || ctx.IsReCheckTx()) && !simulate { + // keep track of total packet messages and number of redundancies across `RecvPacket`, `AcknowledgePacket`, and `TimeoutPacket/OnClose` + redundancies := 0 + packetMsgs := 0 + for _, m := range tx.GetMsgs() { + switch msg := m.(type) { + case *channeltypes.MsgRecvPacket: + if _, found := ad.k.GetPacketReceipt(ctx, msg.Packet.GetDestPort(), msg.Packet.GetDestChannel(), msg.Packet.GetSequence()); found { + redundancies++ + } + packetMsgs++ + + case *channeltypes.MsgAcknowledgement: + if commitment := ad.k.GetPacketCommitment(ctx, msg.Packet.GetSourcePort(), msg.Packet.GetSourceChannel(), msg.Packet.GetSequence()); len(commitment) == 0 { + redundancies++ + } + packetMsgs++ + + case *channeltypes.MsgTimeout: + if commitment := ad.k.GetPacketCommitment(ctx, msg.Packet.GetSourcePort(), msg.Packet.GetSourceChannel(), msg.Packet.GetSequence()); len(commitment) == 0 { + redundancies++ + } + packetMsgs++ + + case *channeltypes.MsgTimeoutOnClose: + if commitment := ad.k.GetPacketCommitment(ctx, msg.Packet.GetSourcePort(), msg.Packet.GetSourceChannel(), msg.Packet.GetSequence()); len(commitment) == 0 { + redundancies++ + } + packetMsgs++ + + case *clienttypes.MsgUpdateClient: + // do nothing here, as we want to avoid updating clients if it is batched with only redundant messages + + default: + // if the multiMsg tx has a msg that is not a packet msg or update msg, then we will not return error + // regardless of if all packet messages are redundant. This ensures that non-packet messages get processed + // even if they get batched with redundant packet messages. + return next(ctx, tx, simulate) + } + + } + + // only return error if all packet messages are redundant + if redundancies == packetMsgs && packetMsgs > 0 { + return ctx, channeltypes.ErrRedundantTx + } + } + return next(ctx, tx, simulate) +}