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: add gov spam prevention antehandler #2251

Closed
wants to merge 9 commits into from
Closed
Show file tree
Hide file tree
Changes from 8 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
10 changes: 9 additions & 1 deletion ante/ante.go
Original file line number Diff line number Diff line change
@@ -1,20 +1,25 @@
package ante

import (
"github.com/cosmos/cosmos-sdk/codec"
sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
"github.com/cosmos/cosmos-sdk/x/auth/ante"
govkeeper "github.com/cosmos/cosmos-sdk/x/gov/keeper"
paramtypes "github.com/cosmos/cosmos-sdk/x/params/types"
ibcante "github.com/cosmos/ibc-go/v4/modules/core/ante"
ibckeeper "github.com/cosmos/ibc-go/v4/modules/core/keeper"

gaiafeeante "github.com/cosmos/gaia/v9/x/globalfee/ante"
gaiagovante "github.com/cosmos/gaia/v9/x/gov/ante"
Copy link
Contributor

Choose a reason for hiding this comment

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

This shouldn't be under x/ as it's not a module.

Copy link
Author

Choose a reason for hiding this comment

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

Where should it live sir?

Copy link
Contributor

Choose a reason for hiding this comment

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

you can move it to gaia/ante/

)

// HandlerOptions extend the SDK's AnteHandler options by requiring the IBC
// channel keeper.
type HandlerOptions struct {
ante.HandlerOptions
Codec codec.BinaryCodec
GovKeeper *govkeeper.Keeper
IBCkeeper *ibckeeper.Keeper
BypassMinFeeMsgTypes []string
GlobalFeeSubspace paramtypes.Subspace
Expand All @@ -34,6 +39,9 @@ func NewAnteHandler(opts HandlerOptions) (sdk.AnteHandler, error) {
if opts.IBCkeeper == nil {
return nil, sdkerrors.Wrap(sdkerrors.ErrLogic, "IBC keeper is required for AnteHandler")
}
if opts.GovKeeper == nil {
return nil, sdkerrors.Wrap(sdkerrors.ErrLogic, "gov keeper is required for AnteHandler")
}
if opts.GlobalFeeSubspace.Name() == "" {
return nil, sdkerrors.Wrap(sdkerrors.ErrNotFound, "globalfee param store is required for AnteHandler")
}
Expand All @@ -60,7 +68,7 @@ func NewAnteHandler(opts HandlerOptions) (sdk.AnteHandler, error) {
ante.NewValidateMemoDecorator(opts.AccountKeeper),
ante.NewConsumeGasForTxSizeDecorator(opts.AccountKeeper),
gaiafeeante.NewFeeDecorator(opts.BypassMinFeeMsgTypes, opts.GlobalFeeSubspace, opts.StakingSubspace, maxBypassMinFeeMsgGasUsage),

gaiagovante.NewGovPreventSpamDecorator(opts.Codec, opts.GovKeeper),
ante.NewDeductFeeDecorator(opts.AccountKeeper, opts.BankKeeper, opts.FeegrantKeeper),
ante.NewSetPubKeyDecorator(opts.AccountKeeper), // SetPubKeyDecorator must be called before all signature verification decorators
ante.NewValidateSigCountDecorator(opts.AccountKeeper),
Expand Down
2 changes: 2 additions & 0 deletions app/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,9 @@ func NewGaiaApp(
SignModeHandler: encodingConfig.TxConfig.SignModeHandler(),
SigGasConsumer: ante.DefaultSigVerificationGasConsumer,
},
Codec: app.appCodec,
IBCkeeper: app.IBCKeeper,
GovKeeper: &app.GovKeeper,
BypassMinFeeMsgTypes: bypassMinFeeMsgTypes,
GlobalFeeSubspace: app.GetSubspace(globalfee.ModuleName),
StakingSubspace: app.GetSubspace(stakingtypes.ModuleName),
Expand Down
2 changes: 1 addition & 1 deletion tests/e2e/e2e_setup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ var (
stakingAmountCoin = sdk.NewCoin(uatomDenom, stakingAmount)
tokenAmount = sdk.NewCoin(uatomDenom, sdk.NewInt(3300000000)) // 3,300uatom
standardFees = sdk.NewCoin(uatomDenom, sdk.NewInt(330000)) // 0.33uatom
depositAmount = sdk.NewCoin(uatomDenom, sdk.NewInt(10000000)) // 10uatom
depositAmount = sdk.NewCoin(uatomDenom, sdk.NewInt(3300000000)) // 3,300uatom
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you give more context on this change and how it tests the new ante handler?

Copy link
Contributor

Choose a reason for hiding this comment

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

unit tests and e2e tests need to be added:

  • gov proposal with deposit less than MiniumInitialDepositRate
  • gov proposal with deposit more than/equal toMiniumInitialDepositRate
  • authz.msg contains gov proposal with deposit less than MiniumInitialDepositRate
  • authz.msg contains gov proposal with deposit more than/equal toMiniumInitialDepositRate

Copy link
Contributor

Choose a reason for hiding this comment

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

the depositAmount = 3,300uatom, is this enough ? depositAmount need to be >= 2,000,000uatom due to
MiniumInitialDepositRate = sdk.NewDecWithPrec(10, 2), gas = 200,000

distModuleAddress = authtypes.NewModuleAddress(distrtypes.ModuleName).String()
proposalCounter = 0
)
Expand Down
90 changes: 90 additions & 0 deletions x/gov/ante/prevent_spam.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
package ante

import (
"github.com/cosmos/cosmos-sdk/codec"
sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
"github.com/cosmos/cosmos-sdk/x/authz"
govkeeper "github.com/cosmos/cosmos-sdk/x/gov/keeper"
govtypes "github.com/cosmos/cosmos-sdk/x/gov/types"
)

var MiniumInitialDepositRate = sdk.NewDecWithPrec(10, 2)

type GovPreventSpamDecorator struct {
govKeeper *govkeeper.Keeper
cdc codec.BinaryCodec
}

func NewGovPreventSpamDecorator(cdc codec.BinaryCodec, govKeeper *govkeeper.Keeper) GovPreventSpamDecorator {
return GovPreventSpamDecorator{
govKeeper: govKeeper,
cdc: cdc,
}
}

func (gpsd GovPreventSpamDecorator) AnteHandle(
ctx sdk.Context, tx sdk.Tx,
simulate bool, next sdk.AnteHandler,
) (newCtx sdk.Context, err error) {
msgs := tx.GetMsgs()
if err = gpsd.checkSpamSubmitProposalMsg(ctx, msgs); err != nil {
return ctx, err
}

return next(ctx, tx, simulate)
}

func (gpsd GovPreventSpamDecorator) checkSpamSubmitProposalMsg(ctx sdk.Context, msgs []sdk.Msg) error {
validMsg := func(m sdk.Msg) error {
if msg, ok := m.(*govtypes.MsgSubmitProposal); ok {
// prevent spam gov msg
depositParams := gpsd.govKeeper.GetDepositParams(ctx)
miniumInitialDeposit := gpsd.calcMiniumInitialDeposit(depositParams.MinDeposit)
if msg.InitialDeposit.IsAllLT(miniumInitialDeposit) {
return sdkerrors.Wrapf(sdkerrors.ErrInsufficientFunds, "not enough initial deposit. required: %v", miniumInitialDeposit)
}
}

return nil
}

validAuthz := func(execMsg *authz.MsgExec) error {
for _, v := range execMsg.Msgs {
var innerMsg sdk.Msg
if err := gpsd.cdc.UnpackAny(v, &innerMsg); err != nil {
return sdkerrors.Wrapf(sdkerrors.ErrUnauthorized, "cannot unmarshal authz exec msgs")
}

if err := validMsg(innerMsg); err != nil {
return err
}
}

return nil
}

for _, m := range msgs {
if msg, ok := m.(*authz.MsgExec); ok {
if err := validAuthz(msg); err != nil {
return err
}
continue
}

// validate normal msgs
if err := validMsg(m); err != nil {
return err
}
}
return nil
}

func (gpsd GovPreventSpamDecorator) calcMiniumInitialDeposit(minDeposit sdk.Coins) (miniumInitialDeposit sdk.Coins) {
for _, coin := range minDeposit {
miniumInitialCoin := MiniumInitialDepositRate.MulInt(coin.Amount).RoundInt()
miniumInitialDeposit = miniumInitialDeposit.Add(sdk.NewCoin(coin.Denom, miniumInitialCoin))
}

return
}