Skip to content

Commit

Permalink
feat: backport reject redundant transactions from ibc-go (#10211)
Browse files Browse the repository at this point in the history
* backport refund of redundant packets from ibc-go

* update changelog

* address review comments and lint errors

* added nolint

Co-authored-by: Carlos Rodriguez <crodveg@gmail.com>
  • Loading branch information
crodriguezvega and crodriguezvega authored Sep 28, 2021
1 parent a062a5a commit 9c91f5c
Show file tree
Hide file tree
Showing 6 changed files with 118 additions and 2 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
4 changes: 4 additions & 0 deletions RELEASE_NOTES.md
Original file line number Diff line number Diff line change
@@ -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)).
Expand Down
36 changes: 36 additions & 0 deletions simapp/ante_handler.go
Original file line number Diff line number Diff line change
@@ -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),
)
}
4 changes: 2 additions & 2 deletions simapp/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
3 changes: 3 additions & 0 deletions x/ibc/core/04-channel/types/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
)
72 changes: 72 additions & 0 deletions x/ibc/core/ante/ante.go
Original file line number Diff line number Diff line change
@@ -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)
}

0 comments on commit 9c91f5c

Please sign in to comment.