From 78ed8771627b6dd9c55272e0ba857c80545d6bc5 Mon Sep 17 00:00:00 2001 From: Dev Ojha <ValarDragon@users.noreply.github.com> Date: Wed, 25 May 2022 02:47:47 +0200 Subject: [PATCH] Add msg_filter_ante_handler to block IBCTimeoutOnClose (#1571) Closes: #XXX ## What is the purpose of the change Add a msg_filter_ante for v9, and block MsgTimeoutOnClose, due to edge cases that can happen if Terra re-enables the previously closed IBC channels. (This is because Closed channels were never meant to be re-opened) ## Brief Changelog Adds a ante handler filter for IBC Timeout close messages. This is a port of Bez' old work. ## Testing and Verifying Covered by provided tests/ ## Documentation and Release Note - Does this pull request introduce a new feature or user-facing behavior changes? yes - Is a relevant changelog entry added to the `Unreleased` section in `CHANGELOG.md`? no - How is the feature or change documented? TODO --- app/ante.go | 4 +- app/upgrades/v9/msg_filter_ante.go | 35 +++++++++++++ app/upgrades/v9/msg_filter_ante_test.go | 70 +++++++++++++++++++++++++ 3 files changed, 107 insertions(+), 2 deletions(-) create mode 100644 app/upgrades/v9/msg_filter_ante.go create mode 100644 app/upgrades/v9/msg_filter_ante_test.go diff --git a/app/ante.go b/app/ante.go index 0705af4d3b2..ba999fc802a 100644 --- a/app/ante.go +++ b/app/ante.go @@ -12,8 +12,8 @@ import ( "github.com/cosmos/cosmos-sdk/x/auth/signing" osmoante "github.com/osmosis-labs/osmosis/v7/ante" + v9 "github.com/osmosis-labs/osmosis/v7/app/upgrades/v9" - v8 "github.com/osmosis-labs/osmosis/v7/app/upgrades/v8" txfeeskeeper "github.com/osmosis-labs/osmosis/v7/x/txfees/keeper" txfeestypes "github.com/osmosis-labs/osmosis/v7/x/txfees/types" ) @@ -42,7 +42,7 @@ func NewAnteHandler( wasmkeeper.NewLimitSimulationGasDecorator(wasmConfig.SimulationGasLimit), wasmkeeper.NewCountTXDecorator(txCounterStoreKey), ante.NewRejectExtensionOptionsDecorator(), - v8.MsgFilterDecorator{}, + v9.MsgFilterDecorator{}, // Use Mempool Fee Decorator from our txfees module instead of default one from auth // https://github.com/cosmos/cosmos-sdk/blob/master/x/auth/middleware/fee.go#L34 mempoolFeeDecorator, diff --git a/app/upgrades/v9/msg_filter_ante.go b/app/upgrades/v9/msg_filter_ante.go new file mode 100644 index 00000000000..b6035b35976 --- /dev/null +++ b/app/upgrades/v9/msg_filter_ante.go @@ -0,0 +1,35 @@ +package v9 + +import ( + "fmt" + + sdk "github.com/cosmos/cosmos-sdk/types" + ibcchanneltypes "github.com/cosmos/ibc-go/v3/modules/core/04-channel/types" +) + +// MsgFilterDecorator defines an AnteHandler decorator for the v9 upgrade that +// provide height-gated message filtering acceptance. +type MsgFilterDecorator struct{} + +// AnteHandle performs an AnteHandler check that returns an error if the tx contains a message +// that is blocked. +// Right now, we block MsgTimeoutOnClose due to incorrect behavior that could occur if a packet is re-enabled. +func (mfd MsgFilterDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (newCtx sdk.Context, err error) { + if hasInvalidMsgs(tx.GetMsgs()) { + currHeight := ctx.BlockHeight() + return ctx, fmt.Errorf("tx contains unsupported message types at height %d", currHeight) + } + + return next(ctx, tx, simulate) +} + +func hasInvalidMsgs(msgs []sdk.Msg) bool { + for _, msg := range msgs { + switch msg.(type) { + case *ibcchanneltypes.MsgTimeoutOnClose: + return true + } + } + + return false +} diff --git a/app/upgrades/v9/msg_filter_ante_test.go b/app/upgrades/v9/msg_filter_ante_test.go new file mode 100644 index 00000000000..0a3d9b5fee5 --- /dev/null +++ b/app/upgrades/v9/msg_filter_ante_test.go @@ -0,0 +1,70 @@ +package v9_test + +import ( + "testing" + + sdk "github.com/cosmos/cosmos-sdk/types" + banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" + "github.com/stretchr/testify/require" + + ibcchanneltypes "github.com/cosmos/ibc-go/v3/modules/core/04-channel/types" + + "github.com/osmosis-labs/osmosis/v7/app" + v8 "github.com/osmosis-labs/osmosis/v7/app/upgrades/v8" + v9 "github.com/osmosis-labs/osmosis/v7/app/upgrades/v9" +) + +func noOpAnteDecorator() sdk.AnteHandler { + return func(ctx sdk.Context, _ sdk.Tx, _ bool) (sdk.Context, error) { + return ctx, nil + } +} + +func TestMsgFilterDecorator(t *testing.T) { + handler := v9.MsgFilterDecorator{} + txCfg := app.MakeEncodingConfig().TxConfig + + addr1 := sdk.AccAddress([]byte("addr1_______________")) + addr2 := sdk.AccAddress([]byte("addr2_______________")) + + testCases := []struct { + name string + ctx sdk.Context + msgs []sdk.Msg + expectErr bool + }{ + { + name: "valid tx", + ctx: sdk.Context{}.WithBlockHeight(v8.UpgradeHeight - 1), + msgs: []sdk.Msg{ + banktypes.NewMsgSend(addr1, addr2, sdk.NewCoins(sdk.NewInt64Coin("foo", 5))), + }, + expectErr: false, + }, + { + name: "invalid tx", + ctx: sdk.Context{}.WithBlockHeight(v8.UpgradeHeight), + msgs: []sdk.Msg{ + banktypes.NewMsgSend(addr1, addr2, sdk.NewCoins(sdk.NewInt64Coin("foo", 5))), + &ibcchanneltypes.MsgTimeoutOnClose{}, + }, + expectErr: true, + }, + } + + for _, tc := range testCases { + tc := tc + + t.Run(tc.name, func(t *testing.T) { + txBuilder := txCfg.NewTxBuilder() + require.NoError(t, txBuilder.SetMsgs(tc.msgs...)) + + _, err := handler.AnteHandle(tc.ctx, txBuilder.GetTx(), false, noOpAnteDecorator()) + if tc.expectErr { + require.Error(t, err) + } else { + require.NoError(t, err) + } + }) + } +}