Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(ante): allow rejection based on depth #1443

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 31 additions & 14 deletions app/ante/handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package ante
import (
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/x/auth/ante"
ibcclienttypes "github.com/cosmos/ibc-go/v7/modules/core/02-client/types"
ibcante "github.com/cosmos/ibc-go/v7/modules/core/ante"
ethante "github.com/evmos/ethermint/app/ante"
txfeesante "github.com/osmosis-labs/osmosis/v15/x/txfees/ante"
Expand Down Expand Up @@ -45,13 +46,21 @@ func newLegacyCosmosAnteHandlerEip712(options HandlerOptions) sdk.AnteHandler {
See https://jumpcrypto.com/writing/bypassing-ethermint-ante-handlers/
for an explanation of these message blocking decorators
*/
NewRejectMessagesDecorator(
// reject MsgEthereumTxs and disable the Msg types that cannot be included on an authz.MsgExec msgs field
sdk.MsgTypeURL(&evmtypes.MsgEthereumTx{}),
sdk.MsgTypeURL(&vestingtypes.MsgCreateVestingAccount{}),
sdk.MsgTypeURL(&vestingtypes.MsgCreatePeriodicVestingAccount{}),
sdk.MsgTypeURL(&vestingtypes.MsgCreatePermanentLockedAccount{}),
),
// reject MsgEthereumTxs and disable the Msg types that cannot be included on an authz.MsgExec msgs field
NewRejectMessagesDecorator().WithPredicate(
BlockTypeUrls(
1,
// Only blanket rejects depth greater than zero because we have our own custom logic for depth 0
// Note that there is never a genuine reason to pass both ibc update client and misbehaviour submission through gov or auth,
// it's always done by relayers directly.
sdk.MsgTypeURL(&ibcclienttypes.MsgUpdateClient{}),
sdk.MsgTypeURL(&ibcclienttypes.MsgSubmitMisbehaviour{}))).
WithPredicate(BlockTypeUrls(
0,
sdk.MsgTypeURL(&evmtypes.MsgEthereumTx{}),
sdk.MsgTypeURL(&vestingtypes.MsgCreateVestingAccount{}),
sdk.MsgTypeURL(&vestingtypes.MsgCreatePeriodicVestingAccount{}),
sdk.MsgTypeURL(&vestingtypes.MsgCreatePermanentLockedAccount{}))),

ante.NewSetUpContextDecorator(),
ante.NewValidateBasicDecorator(),
Expand Down Expand Up @@ -83,13 +92,21 @@ func newCosmosAnteHandler(options HandlerOptions) sdk.AnteHandler {
deductFeeDecorator := txfeesante.NewDeductFeeDecorator(*options.TxFeesKeeper, options.AccountKeeper, options.BankKeeper, options.FeegrantKeeper)

return sdk.ChainAnteDecorators(
NewRejectMessagesDecorator(
// reject MsgEthereumTxs and disable the Msg types that cannot be included on an authz.MsgExec msgs field
sdk.MsgTypeURL(&evmtypes.MsgEthereumTx{}),
sdk.MsgTypeURL(&vestingtypes.MsgCreateVestingAccount{}),
sdk.MsgTypeURL(&vestingtypes.MsgCreatePeriodicVestingAccount{}),
sdk.MsgTypeURL(&vestingtypes.MsgCreatePermanentLockedAccount{}),
),
// reject MsgEthereumTxs and disable the Msg types that cannot be included on an authz.MsgExec msgs field
NewRejectMessagesDecorator().WithPredicate(
BlockTypeUrls(
1,
// Only blanket rejects depth greater than zero because we have our own custom logic for depth 0
// Note that there is never a genuine reason to pass both ibc update client and misbehaviour submission through gov or auth,
// it's always done by relayers directly.
sdk.MsgTypeURL(&ibcclienttypes.MsgUpdateClient{}),
sdk.MsgTypeURL(&ibcclienttypes.MsgSubmitMisbehaviour{}))).
WithPredicate(BlockTypeUrls(
0,
sdk.MsgTypeURL(&evmtypes.MsgEthereumTx{}),
sdk.MsgTypeURL(&vestingtypes.MsgCreateVestingAccount{}),
sdk.MsgTypeURL(&vestingtypes.MsgCreatePeriodicVestingAccount{}),
sdk.MsgTypeURL(&vestingtypes.MsgCreatePermanentLockedAccount{}))),
ante.NewSetUpContextDecorator(),
ante.NewExtensionOptionsDecorator(options.ExtensionOptionChecker),
// Use Mempool Fee TransferEnabledDecorator from our txfees module instead of default one from auth
Expand Down
110 changes: 59 additions & 51 deletions app/ante/reject_msgs.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,28 +9,46 @@ import (
"github.com/cosmos/cosmos-sdk/x/authz"
govtypesv1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1"
"github.com/cosmos/cosmos-sdk/x/group"
"github.com/dymensionxyz/gerr-cosmos/gerrc"
evmtypes "github.com/evmos/ethermint/x/evm/types"
)

// RejectMessagesDecorator prevents invalid msg types from being executed
type RejectMessagesDecorator struct {
disabledMsgTypeURLs map[string]struct{}
// message is rejected if any Predicate returns true
predicates []Predicate
}

var _ sdk.AnteDecorator = RejectMessagesDecorator{}
// Predicate should return true if message is not allowed
type Predicate = func(typeURL string, depth int) bool
danwt marked this conversation as resolved.
Show resolved Hide resolved

// NewRejectMessagesDecorator creates a decorator to block provided messages from reaching the mempool
func NewRejectMessagesDecorator(disabledMsgTypeURLs ...string) RejectMessagesDecorator {
disabledMsgsMap := make(map[string]struct{})
for _, url := range disabledMsgTypeURLs {
disabledMsgsMap[url] = struct{}{}
// Blocks any message with depth of depthMax OR MORE
// Depth 0 is top level message
// Depth 1 or more is wrapped in something
func BlockTypeUrls(depthMax int, typeUrls ...string) Predicate {
block := make(map[string]struct{})
for _, url := range typeUrls {
block[url] = struct{}{}
}
return func(url string, depth int) bool {
danwt marked this conversation as resolved.
Show resolved Hide resolved
_, ok := block[url]
return ok && depthMax <= depth
}
}

var _ sdk.AnteDecorator = RejectMessagesDecorator{}

return RejectMessagesDecorator{
disabledMsgTypeURLs: disabledMsgsMap,
func NewRejectMessagesDecorator() *RejectMessagesDecorator {
return &RejectMessagesDecorator{
predicates: []Predicate{},
}
}

func (rmd *RejectMessagesDecorator) WithPredicate(p Predicate) *RejectMessagesDecorator {
danwt marked this conversation as resolved.
Show resolved Hide resolved
rmd.predicates = append(rmd.predicates, p)
return rmd
}

// AnteHandle recursively rejects messages such as those that requires ethereum-specific authentication.
// For example `MsgEthereumTx` requires fee to be deducted in the ante handler in
// order to perform the refund.
Expand All @@ -46,21 +64,22 @@ func (rmd RejectMessagesDecorator) AnteHandle(
return next(ctx, tx, simulate)
}

const maxNestedMsgs = 6
const maxDepth = 6

func (rmd RejectMessagesDecorator) checkMsgs(ctx sdk.Context, msgs []sdk.Msg, nestedMsgs int) error {
// depth=0 means top level message
func (rmd RejectMessagesDecorator) checkMsgs(ctx sdk.Context, msgs []sdk.Msg, depth int) error {
for _, msg := range msgs {
if err := rmd.checkMsg(ctx, msg, nestedMsgs); err != nil {
if err := rmd.checkMsg(ctx, msg, depth); err != nil {
return err
}
}
return nil
}

func (rmd RejectMessagesDecorator) checkMsg(ctx sdk.Context, msg sdk.Msg, nestedMsgs int) error {
typeURL := sdk.MsgTypeURL(msg)
if _, ok := rmd.disabledMsgTypeURLs[typeURL]; ok {
return fmt.Errorf("found disabled msg type: %s", typeURL)
// depth=0 means top level message
func (rmd RejectMessagesDecorator) checkMsg(ctx sdk.Context, msg sdk.Msg, depth int) error {
if depth >= maxDepth {
return fmt.Errorf("found more nested msgs than permitted. Limit is : %d", maxDepth)
}

if _, ok := msg.(*evmtypes.MsgEthereumTx); ok {
Expand All @@ -70,51 +89,40 @@ func (rmd RejectMessagesDecorator) checkMsg(ctx sdk.Context, msg sdk.Msg, nested
)
}

if nestedMsgs >= maxNestedMsgs {
return fmt.Errorf("found more nested msgs than permitted. Limit is : %d", maxNestedMsgs)
typeURL := sdk.MsgTypeURL(msg)
for _, pred := range rmd.predicates {
if pred(typeURL, depth) {
return gerrc.ErrInvalidArgument.Wrapf("disabled: %s", typeURL)
}
danwt marked this conversation as resolved.
Show resolved Hide resolved
}

innerMsgs, err := extractMsgs(msg)
if err != nil {
return err
}
switch concreteMsg := msg.(type) {
var err error
var inner []sdk.Msg

switch m := msg.(type) {
case *authz.MsgExec:
nestedMsgs++
if err := rmd.checkMsgs(ctx, innerMsgs, nestedMsgs); err != nil {
return err
}
inner, err = m.GetMessages()
case *govtypesv1.MsgSubmitProposal:
inner, err = m.GetMsgs()
case *group.MsgSubmitProposal:
inner, err = m.GetMsgs()
case *authz.MsgGrant:
authorization, err := concreteMsg.GetAuthorization()
authorization, err := m.GetAuthorization()
if err != nil {
return err
}
url := authorization.MsgTypeURL()
if _, ok := rmd.disabledMsgTypeURLs[url]; ok {
return fmt.Errorf("granting disabled msg type: %s is not allowed", url)
}
case *govtypesv1.MsgSubmitProposal:
nestedMsgs++
if err := rmd.checkMsgs(ctx, innerMsgs, nestedMsgs); err != nil {
return err
}
case *group.MsgSubmitProposal:
nestedMsgs++
if err := rmd.checkMsgs(ctx, innerMsgs, nestedMsgs); err != nil {
return err
typeURL = authorization.MsgTypeURL()
for _, pred := range rmd.predicates {
if pred(typeURL, depth) {
return gerrc.ErrInvalidArgument.Wrapf("disabled grant: %s", typeURL)
}
}
default:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure why no return nil here
why u need to go through return rmd.checkMsgs(ctx, inner, depth+1) with empty inner to stop the recursion

}

return nil
}

func extractMsgs(msg any) ([]sdk.Msg, error) {
if msgWithMsgs, ok := msg.(interface{ GetMsgs() ([]sdk.Msg, error) }); ok {
return msgWithMsgs.GetMsgs()
}
if msgWithMessages, ok := msg.(interface{ GetMessages() ([]sdk.Msg, error) }); ok {
return msgWithMessages.GetMessages()
if err != nil {
return err
}
return nil, nil

return rmd.checkMsgs(ctx, inner, depth+1)
danwt marked this conversation as resolved.
Show resolved Hide resolved
}
49 changes: 43 additions & 6 deletions app/ante/reject_msgs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,48 @@ import (
banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
govtypesv1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1"
"github.com/cosmos/cosmos-sdk/x/staking/types"
ibcclienttypes "github.com/cosmos/ibc-go/v7/modules/core/02-client/types"
"github.com/stretchr/testify/require"

"github.com/dymensionxyz/dymension/v3/app/ante"
)

func (suite *AnteTestSuite) TestRejectMessagesDecoratorCustom() {
suite.SetupTestCheckTx(false)

decorator := ante.NewRejectMessagesDecorator().WithPredicate(ante.BlockTypeUrls(1, sdk.MsgTypeURL(&ibcclienttypes.MsgUpdateClient{})))

{

m := []sdk.Msg{
// nest = 0 is OK
&ibcclienttypes.MsgUpdateClient{},
}
tx := &mockTx{msgs: m}

ctx := suite.ctx.WithBlockHeight(1)
_, err := decorator.AnteHandle(ctx, tx, false, func(sdk.Context, sdk.Tx, bool) (sdk.Context, error) { return ctx, nil })

suite.NoError(err)
}
{
m := []sdk.Msg{
&authz.MsgExec{
Grantee: "cosmos1...",
Msgs: []*codectypes.Any{
packMsg(suite.T(), &ibcclienttypes.MsgUpdateClient{}),
},
},
}
tx := &mockTx{msgs: m}

ctx := suite.ctx.WithBlockHeight(1)
_, err := decorator.AnteHandle(ctx, tx, false, func(sdk.Context, sdk.Tx, bool) (sdk.Context, error) { return ctx, nil })

suite.Error(err)
}
}

func (suite *AnteTestSuite) TestRejectMessagesDecorator() {
suite.SetupTestCheckTx(false)

Expand All @@ -22,7 +59,7 @@ func (suite *AnteTestSuite) TestRejectMessagesDecorator() {
sdk.MsgTypeURL(&types.MsgDelegate{}),
}

decorator := ante.NewRejectMessagesDecorator(disabledMsgTypes...)
decorator := ante.NewRejectMessagesDecorator().WithPredicate(ante.BlockTypeUrls(0, disabledMsgTypes...))

testCases := []struct {
name string
Expand All @@ -40,7 +77,7 @@ func (suite *AnteTestSuite) TestRejectMessagesDecorator() {
},
},
expectPass: false,
expectedError: "found disabled msg type: /cosmos.bank.v1beta1.MsgSend",
expectedError: "/cosmos.bank.v1beta1.MsgSend",
},
{
name: "Transaction with allowed message (MsgMultiSend)",
Expand All @@ -67,7 +104,7 @@ func (suite *AnteTestSuite) TestRejectMessagesDecorator() {
},
},
expectPass: false,
expectedError: "found disabled msg type: /cosmos.bank.v1beta1.MsgSend",
expectedError: "/cosmos.bank.v1beta1.MsgSend",
},
{
name: "Transaction with allowed message nested in MsgExec",
Expand Down Expand Up @@ -100,12 +137,12 @@ func (suite *AnteTestSuite) TestRejectMessagesDecorator() {
},
},
expectPass: false,
expectedError: "found disabled msg type: /cosmos.bank.v1beta1.MsgSend",
expectedError: "/cosmos.bank.v1beta1.MsgSend",
},
{
name: "Transaction exceeding max nested messages",
msgs: []sdk.Msg{
generateDeeplyNestedMsgExec(suite.T(), 7), // exceeds maxNestedMsgs (6)
generateDeeplyNestedMsgExec(suite.T(), 7), // exceeds maxDepth (6)
},
expectPass: false,
expectedError: "found more nested msgs than permitted. Limit is : 6",
Expand All @@ -125,7 +162,7 @@ func (suite *AnteTestSuite) TestRejectMessagesDecorator() {
},
},
expectPass: false,
expectedError: "granting disabled msg type: /cosmos.bank.v1beta1.MsgSend is not allowed",
expectedError: "/cosmos.bank.v1beta1.MsgSend",
},
{
name: "Transaction with authz.MsgGrant granting allowed message",
Expand Down
Loading