Skip to content

Commit

Permalink
Add msg_filter_ante_handler to block IBCTimeoutOnClose (#1571)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
ValarDragon authored May 25, 2022
1 parent 2f3c7b1 commit 78ed877
Show file tree
Hide file tree
Showing 3 changed files with 107 additions and 2 deletions.
4 changes: 2 additions & 2 deletions app/ante.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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,
Expand Down
35 changes: 35 additions & 0 deletions app/upgrades/v9/msg_filter_ante.go
Original file line number Diff line number Diff line change
@@ -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
}
70 changes: 70 additions & 0 deletions app/upgrades/v9/msg_filter_ante_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
})
}
}

0 comments on commit 78ed877

Please sign in to comment.