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 9 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
13 changes: 5 additions & 8 deletions app/ante/handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,13 +45,12 @@ 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
// reject MsgEthereumTxs and disable the Msg types that cannot be included on an authz.MsgExec msgs field
NewRejectMessagesDecorator().WithPredicate(lightclientante.BlockMsg).WithPredicate(BlockTypeUrls(
sdk.MsgTypeURL(&evmtypes.MsgEthereumTx{}),
sdk.MsgTypeURL(&vestingtypes.MsgCreateVestingAccount{}),
sdk.MsgTypeURL(&vestingtypes.MsgCreatePeriodicVestingAccount{}),
sdk.MsgTypeURL(&vestingtypes.MsgCreatePermanentLockedAccount{}),
),
sdk.MsgTypeURL(&vestingtypes.MsgCreatePermanentLockedAccount{}))),

ante.NewSetUpContextDecorator(),
ante.NewValidateBasicDecorator(),
Expand Down Expand Up @@ -83,13 +82,11 @@ 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
NewRejectMessagesDecorator().WithPredicate(lightclientante.BlockMsg).WithPredicate(BlockTypeUrls(
sdk.MsgTypeURL(&evmtypes.MsgEthereumTx{}),
sdk.MsgTypeURL(&vestingtypes.MsgCreateVestingAccount{}),
sdk.MsgTypeURL(&vestingtypes.MsgCreatePeriodicVestingAccount{}),
sdk.MsgTypeURL(&vestingtypes.MsgCreatePermanentLockedAccount{}),
),
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
109 changes: 57 additions & 52 deletions app/ante/reject_msgs.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,28 +9,43 @@ 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{}{}
func BlockTypeUrls(typeUrls ...string) Predicate {
block := make(map[string]struct{})
for _, url := range typeUrls {
block[url] = struct{}{}
}
return func(url string, _ int) bool {
_, ok := block[url]
return ok
}
}

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 +61,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 +86,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()
default:
danwt marked this conversation as resolved.
Show resolved Hide resolved
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:
}

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
}
50 changes: 44 additions & 6 deletions app/ante/reject_msgs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,49 @@ 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"
lightclientante "github.com/dymensionxyz/dymension/v3/x/lightclient/ante"
"github.com/stretchr/testify/require"

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

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

decorator := ante.NewRejectMessagesDecorator().WithPredicate(lightclientante.BlockMsg)

{

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 +60,7 @@ func (suite *AnteTestSuite) TestRejectMessagesDecorator() {
sdk.MsgTypeURL(&types.MsgDelegate{}),
}

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

testCases := []struct {
name string
Expand All @@ -40,7 +78,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 +105,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 +138,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 +163,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
13 changes: 13 additions & 0 deletions x/lightclient/ante/ibc_msg_update_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,19 @@ import (
"github.com/dymensionxyz/gerr-cosmos/gerrc"
)

var (
blockUrlUpdate = sdk.MsgTypeURL(&ibcclienttypes.MsgUpdateClient{})
blockUrlMisbehavior = sdk.MsgTypeURL(&ibcclienttypes.MsgSubmitMisbehaviour{})
)

// Depth is the nesting depth of the message with authz and gov proposal. depth 0 is a top level message.
// 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.
func BlockMsg(typeURL string, depth int) bool {
danwt marked this conversation as resolved.
Show resolved Hide resolved
return (typeURL == blockUrlUpdate || typeURL == blockUrlMisbehavior) && 0 < depth
}

func (i IBCMessagesDecorator) HandleMsgUpdateClient(ctx sdk.Context, msg *ibcclienttypes.MsgUpdateClient) error {
if !i.k.Enabled() {
return nil
Expand Down
Loading