From 23d9f671e93ed40ab68d4ff749446c19b3bab1f9 Mon Sep 17 00:00:00 2001 From: MSalopek <35486649+MSalopek@users.noreply.github.com> Date: Thu, 9 Mar 2023 16:34:50 +0100 Subject: [PATCH] add spam prevention antehandler (#2262) * add spam prevention antehandler * sort imports * run make format * fix misleading var names * update wrong test cases * fix failing gov tests * fix linter errors * uncomment e2e gov steps logs --- ante/ante.go | 8 ++ ante/gov_ante.go | 95 ++++++++++++++++++++++++ ante/gov_ante_test.go | 92 +++++++++++++++++++++++ app/app.go | 2 + tests/e2e/e2e_globalfee_proposal_test.go | 2 +- tests/e2e/e2e_gov_test.go | 22 +++--- tests/e2e/e2e_setup_test.go | 29 ++++++-- 7 files changed, 235 insertions(+), 15 deletions(-) create mode 100644 ante/gov_ante.go create mode 100644 ante/gov_ante_test.go diff --git a/ante/ante.go b/ante/ante.go index f1e1ce391da..db1b2397200 100644 --- a/ante/ante.go +++ b/ante/ante.go @@ -1,9 +1,11 @@ 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" @@ -15,6 +17,8 @@ import ( // channel keeper. type HandlerOptions struct { ante.HandlerOptions + Codec codec.BinaryCodec + GovKeeper *govkeeper.Keeper IBCkeeper *ibckeeper.Keeper BypassMinFeeMsgTypes []string GlobalFeeSubspace paramtypes.Subspace @@ -40,6 +44,9 @@ func NewAnteHandler(opts HandlerOptions) (sdk.AnteHandler, error) { if opts.StakingSubspace.Name() == "" { return nil, sdkerrors.Wrap(sdkerrors.ErrNotFound, "staking param store is required for AnteHandler") } + if opts.GovKeeper == nil { + return nil, sdkerrors.Wrap(sdkerrors.ErrLogic, "gov keeper is required for AnteHandler") + } sigGasConsumer := opts.SigGasConsumer if sigGasConsumer == nil { @@ -59,6 +66,7 @@ func NewAnteHandler(opts HandlerOptions) (sdk.AnteHandler, error) { ante.NewTxTimeoutHeightDecorator(), ante.NewValidateMemoDecorator(opts.AccountKeeper), ante.NewConsumeGasForTxSizeDecorator(opts.AccountKeeper), + NewGovPreventSpamDecorator(opts.Codec, opts.GovKeeper), gaiafeeante.NewFeeDecorator(opts.BypassMinFeeMsgTypes, opts.GlobalFeeSubspace, opts.StakingSubspace, maxBypassMinFeeMsgGasUsage), ante.NewDeductFeeDecorator(opts.AccountKeeper, opts.BankKeeper, opts.FeegrantKeeper), diff --git a/ante/gov_ante.go b/ante/gov_ante.go new file mode 100644 index 00000000000..faca639b771 --- /dev/null +++ b/ante/gov_ante.go @@ -0,0 +1,95 @@ +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" +) + +// initial deposit must be greater than or equal to 10% of the minimum deposit +var minInitialDepositFraction = 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 (g GovPreventSpamDecorator) AnteHandle( + ctx sdk.Context, tx sdk.Tx, + simulate bool, next sdk.AnteHandler, +) (newCtx sdk.Context, err error) { + // run checks only on CheckTx or simulate + if !ctx.IsCheckTx() || simulate { + return next(ctx, tx, simulate) + } + + msgs := tx.GetMsgs() + if err = g.ValidateGovMsgs(ctx, msgs); err != nil { + return ctx, err + } + + return next(ctx, tx, simulate) +} + +// validateGovMsgs checks if the InitialDeposit amounts are greater than the minimum initial deposit amount +func (g GovPreventSpamDecorator) ValidateGovMsgs(ctx sdk.Context, msgs []sdk.Msg) error { + validMsg := func(m sdk.Msg) error { + if msg, ok := m.(*govtypes.MsgSubmitProposal); ok { + // prevent messages with insufficient initial deposit amount + depositParams := g.govKeeper.GetDepositParams(ctx) + minInitialDeposit := g.calcMinInitialDeposit(depositParams.MinDeposit) + if msg.InitialDeposit.IsAllLT(minInitialDeposit) { + return sdkerrors.Wrapf(sdkerrors.ErrInsufficientFunds, "insufficient initial deposit amount - required: %v", minInitialDeposit) + } + } + + return nil + } + + validAuthz := func(execMsg *authz.MsgExec) error { + for _, v := range execMsg.Msgs { + var innerMsg sdk.Msg + if err := g.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 (g GovPreventSpamDecorator) calcMinInitialDeposit(minDeposit sdk.Coins) (minInitialDeposit sdk.Coins) { + for _, coin := range minDeposit { + minInitialCoins := minInitialDepositFraction.MulInt(coin.Amount).RoundInt() + minInitialDeposit = minInitialDeposit.Add(sdk.NewCoin(coin.Denom, minInitialCoins)) + } + return +} diff --git a/ante/gov_ante_test.go b/ante/gov_ante_test.go new file mode 100644 index 00000000000..14c86948dc2 --- /dev/null +++ b/ante/gov_ante_test.go @@ -0,0 +1,92 @@ +package ante_test + +import ( + "fmt" + "testing" + + sdk "github.com/cosmos/cosmos-sdk/types" + govtypes "github.com/cosmos/cosmos-sdk/x/gov/types" + "github.com/stretchr/testify/suite" + + "github.com/cosmos/cosmos-sdk/client" + "github.com/cosmos/cosmos-sdk/testutil/testdata" + "github.com/cosmos/gaia/v9/ante" + gaiahelpers "github.com/cosmos/gaia/v9/app/helpers" + tmrand "github.com/tendermint/tendermint/libs/rand" + tmproto "github.com/tendermint/tendermint/proto/tendermint/types" + + gaiaapp "github.com/cosmos/gaia/v9/app" +) + +var ( + insufficientCoins = sdk.NewCoins(sdk.NewInt64Coin(sdk.DefaultBondDenom, 100)) + minCoins = sdk.NewCoins(sdk.NewInt64Coin(sdk.DefaultBondDenom, 1000000)) + moreThanMinCoins = sdk.NewCoins(sdk.NewInt64Coin(sdk.DefaultBondDenom, 2500000)) + testAddr = sdk.AccAddress("test1") +) + +type GovAnteHandlerTestSuite struct { + suite.Suite + + app *gaiaapp.GaiaApp + ctx sdk.Context + clientCtx client.Context +} + +func (s *GovAnteHandlerTestSuite) SetupTest() { + app := gaiahelpers.Setup(s.T()) + ctx := app.BaseApp.NewContext(false, tmproto.Header{ + ChainID: fmt.Sprintf("test-chain-%s", tmrand.Str(4)), + Height: 1, + }) + + encodingConfig := gaiaapp.MakeTestEncodingConfig() + encodingConfig.Amino.RegisterConcrete(&testdata.TestMsg{}, "testdata.TestMsg", nil) + testdata.RegisterInterfaces(encodingConfig.InterfaceRegistry) + + s.app = app + s.ctx = ctx + s.clientCtx = client.Context{}.WithTxConfig(encodingConfig.TxConfig) +} + +func TestGovSpamPreventionSuite(t *testing.T) { + suite.Run(t, new(GovAnteHandlerTestSuite)) +} + +func (s *GovAnteHandlerTestSuite) TestGlobalFeeMinimumGasFeeAnteHandler() { + // setup test + s.SetupTest() + tests := []struct { + title, description string + proposalType string + proposerAddr sdk.AccAddress + initialDeposit sdk.Coins + expectPass bool + }{ + {"Passing proposal 1", "the purpose of this proposal is to pass", govtypes.ProposalTypeText, testAddr, minCoins, true}, + {"Passing proposal 2", "the purpose of this proposal is to pass with more coins than minimum", govtypes.ProposalTypeText, testAddr, moreThanMinCoins, true}, + {"Failing proposal", "the purpose of this proposal is to fail", govtypes.ProposalTypeText, testAddr, insufficientCoins, false}, + } + + decorator := ante.NewGovPreventSpamDecorator(s.app.AppCodec(), &s.app.GovKeeper) + + for _, tc := range tests { + content := govtypes.ContentFromProposalType(tc.title, tc.description, tc.proposalType) + s.Require().NotNil(content) + + msg, err := govtypes.NewMsgSubmitProposal( + content, + tc.initialDeposit, + tc.proposerAddr, + ) + + s.Require().NoError(err) + + err = decorator.ValidateGovMsgs(s.ctx, []sdk.Msg{msg}) + if tc.expectPass { + s.Require().NoError(err, "expected %v to pass", tc.title) + } else { + s.Require().Error(err, "expected %v to fail", tc.title) + } + } +} diff --git a/app/app.go b/app/app.go index 78455444069..62b5e28bc49 100644 --- a/app/app.go +++ b/app/app.go @@ -202,7 +202,9 @@ func NewGaiaApp( SignModeHandler: encodingConfig.TxConfig.SignModeHandler(), SigGasConsumer: ante.DefaultSigVerificationGasConsumer, }, + Codec: appCodec, IBCkeeper: app.IBCKeeper, + GovKeeper: &app.GovKeeper, BypassMinFeeMsgTypes: bypassMinFeeMsgTypes, GlobalFeeSubspace: app.GetSubspace(globalfee.ModuleName), StakingSubspace: app.GetSubspace(stakingtypes.ModuleName), diff --git a/tests/e2e/e2e_globalfee_proposal_test.go b/tests/e2e/e2e_globalfee_proposal_test.go index de780b4af68..3f48007af6d 100644 --- a/tests/e2e/e2e_globalfee_proposal_test.go +++ b/tests/e2e/e2e_globalfee_proposal_test.go @@ -20,7 +20,7 @@ func (s *IntegrationTestSuite) govProposeNewGlobalfee(newGlobalfee sdk.DecCoins, // gov proposing new fees s.T().Logf("Proposal number: %d", proposalCounter) s.T().Logf("Submitting, deposit and vote legacy Gov Proposal: change global fee to %s", newGlobalfee.String()) - s.runGovProcess(chainAAPIEndpoint, submitter, proposalCounter, paramtypes.ProposalTypeChange, submitGovFlags, depositGovFlags, voteGovFlags, "vote") + s.runGovProcess(chainAAPIEndpoint, submitter, proposalCounter, paramtypes.ProposalTypeChange, submitGovFlags, depositGovFlags, voteGovFlags, "vote", false) // query the proposal status and new fee s.Require().Eventually( diff --git a/tests/e2e/e2e_gov_test.go b/tests/e2e/e2e_gov_test.go index 1971194035b..fb47d009200 100644 --- a/tests/e2e/e2e_gov_test.go +++ b/tests/e2e/e2e_gov_test.go @@ -35,7 +35,7 @@ func (s *IntegrationTestSuite) GovSoftwareUpgrade() { submitGovFlags := []string{"software-upgrade", "Upgrade-0", "--title='Upgrade V1'", "--description='Software Upgrade'", fmt.Sprintf("--upgrade-height=%d", proposalHeight)} depositGovFlags := []string{strconv.Itoa(proposalCounter), depositAmount.String()} voteGovFlags := []string{strconv.Itoa(proposalCounter), "yes=0.8,no=0.1,abstain=0.05,no_with_veto=0.05"} - s.runGovProcess(chainAAPIEndpoint, sender, proposalCounter, upgradetypes.ProposalTypeSoftwareUpgrade, submitGovFlags, depositGovFlags, voteGovFlags, "weighted-vote") + s.runGovProcess(chainAAPIEndpoint, sender, proposalCounter, upgradetypes.ProposalTypeSoftwareUpgrade, submitGovFlags, depositGovFlags, voteGovFlags, "weighted-vote", true) s.verifyChainHaltedAtUpgradeHeight(s.chainA, 0, proposalHeight) s.T().Logf("Successfully halted chain at height %d", proposalHeight) @@ -76,13 +76,13 @@ func (s *IntegrationTestSuite) GovCancelSoftwareUpgrade() { submitGovFlags := []string{"software-upgrade", "Upgrade-1", "--title='Upgrade V1'", "--description='Software Upgrade'", fmt.Sprintf("--upgrade-height=%d", proposalHeight)} depositGovFlags := []string{strconv.Itoa(proposalCounter), depositAmount.String()} voteGovFlags := []string{strconv.Itoa(proposalCounter), "yes"} - s.runGovProcess(chainAAPIEndpoint, sender, proposalCounter, upgradetypes.ProposalTypeSoftwareUpgrade, submitGovFlags, depositGovFlags, voteGovFlags, "vote") + s.runGovProcess(chainAAPIEndpoint, sender, proposalCounter, upgradetypes.ProposalTypeSoftwareUpgrade, submitGovFlags, depositGovFlags, voteGovFlags, "vote", true) proposalCounter++ submitGovFlags = []string{"cancel-software-upgrade", "--title='Upgrade V1'", "--description='Software Upgrade'"} depositGovFlags = []string{strconv.Itoa(proposalCounter), depositAmount.String()} voteGovFlags = []string{strconv.Itoa(proposalCounter), "yes"} - s.runGovProcess(chainAAPIEndpoint, sender, proposalCounter, upgradetypes.ProposalTypeCancelSoftwareUpgrade, submitGovFlags, depositGovFlags, voteGovFlags, "vote") + s.runGovProcess(chainAAPIEndpoint, sender, proposalCounter, upgradetypes.ProposalTypeCancelSoftwareUpgrade, submitGovFlags, depositGovFlags, voteGovFlags, "vote", true) s.verifyChainPassesUpgradeHeight(s.chainA, 0, proposalHeight) s.T().Logf("Successfully canceled upgrade at height %d", proposalHeight) @@ -113,7 +113,7 @@ func (s *IntegrationTestSuite) GovCommunityPoolSpend() { submitGovFlags := []string{"community-pool-spend", configFile(proposalCommunitySpendFilename)} depositGovFlags := []string{strconv.Itoa(proposalCounter), depositAmount.String()} voteGovFlags := []string{strconv.Itoa(proposalCounter), "yes"} - s.runGovProcess(chainAAPIEndpoint, sender, proposalCounter, distrtypes.ProposalTypeCommunityPoolSpend, submitGovFlags, depositGovFlags, voteGovFlags, "vote") + s.runGovProcess(chainAAPIEndpoint, sender, proposalCounter, distrtypes.ProposalTypeCommunityPoolSpend, submitGovFlags, depositGovFlags, voteGovFlags, "vote", false) s.Require().Eventually( func() bool { @@ -149,7 +149,7 @@ func (s *IntegrationTestSuite) AddRemoveConsumerChain() { submitGovFlags := []string{"consumer-addition", configFile(proposalAddConsumerChainFilename)} depositGovFlags := []string{strconv.Itoa(proposalCounter), depositAmount.String()} voteGovFlags := []string{strconv.Itoa(proposalCounter), "yes"} - s.runGovProcess(chainAAPIEndpoint, sender, proposalCounter, ccvtypes.ProposalTypeConsumerAddition, submitGovFlags, depositGovFlags, voteGovFlags, "vote") + s.runGovProcess(chainAAPIEndpoint, sender, proposalCounter, ccvtypes.ProposalTypeConsumerAddition, submitGovFlags, depositGovFlags, voteGovFlags, "vote", false) // Query and assert consumer has been added s.execQueryConsumerChains(s.chainA, 0, gaiaHomePath, validateConsumerAddition, consumerChainID) @@ -159,8 +159,7 @@ func (s *IntegrationTestSuite) AddRemoveConsumerChain() { submitGovFlags = []string{"consumer-removal", configFile(proposalRemoveConsumerChainFilename)} depositGovFlags = []string{strconv.Itoa(proposalCounter), depositAmount.String()} voteGovFlags = []string{strconv.Itoa(proposalCounter), "yes"} - s.runGovProcess(chainAAPIEndpoint, sender, proposalCounter, ccvtypes.ProposalTypeConsumerRemoval, submitGovFlags, depositGovFlags, voteGovFlags, "vote") - + s.runGovProcess(chainAAPIEndpoint, sender, proposalCounter, ccvtypes.ProposalTypeConsumerRemoval, submitGovFlags, depositGovFlags, voteGovFlags, "vote", false) // Query and assert consumer has been removed s.execQueryConsumerChains(s.chainA, 0, gaiaHomePath, validateConsumerRemoval, consumerChainID) } @@ -186,9 +185,14 @@ func validateConsumerRemoval(res ccvtypes.QueryConsumerChainsResponse, consumerC return true } -func (s *IntegrationTestSuite) runGovProcess(chainAAPIEndpoint, sender string, proposalID int, proposalType string, submitFlags []string, depositFlags []string, voteFlags []string, voteCommand string) { +func (s *IntegrationTestSuite) runGovProcess(chainAAPIEndpoint, sender string, proposalID int, proposalType string, submitFlags []string, depositFlags []string, voteFlags []string, voteCommand string, withDeposit bool) { s.T().Logf("Submitting Gov Proposal: %s", proposalType) - s.submitGovCommand(chainAAPIEndpoint, sender, proposalID, "submit-proposal", submitFlags, govtypes.StatusDepositPeriod) + // min deposit of 1000uatom is required in e2e tests, otherwise the gov antehandler causes the proposal to be dropped + sflags := submitFlags + if withDeposit { + sflags = append(sflags, "--deposit=1000uatom") + } + s.submitGovCommand(chainAAPIEndpoint, sender, proposalID, "submit-proposal", sflags, govtypes.StatusDepositPeriod) s.T().Logf("Depositing Gov Proposal: %s", proposalType) s.submitGovCommand(chainAAPIEndpoint, sender, proposalID, "deposit", depositFlags, govtypes.StatusVotingPeriod) s.T().Logf("Voting Gov Proposal: %s", proposalType) diff --git a/tests/e2e/e2e_setup_test.go b/tests/e2e/e2e_setup_test.go index f790a50b65f..83e35e83c0d 100644 --- a/tests/e2e/e2e_setup_test.go +++ b/tests/e2e/e2e_setup_test.go @@ -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(330000000)) // 3,300uatom distModuleAddress = authtypes.NewModuleAddress(distrtypes.ModuleName).String() proposalCounter = 0 ) @@ -627,7 +627,7 @@ func (s *IntegrationTestSuite) writeGovParamChangeProposalGlobalFees(c *chain, c Value: coins, }, }, - Deposit: "", + Deposit: "1000uatom", }, "", " ") s.Require().NoError(err) @@ -641,7 +641,7 @@ func (s *IntegrationTestSuite) writeGovCommunitySpendProposal(c *chain, amount s Description: "Fund Team!", Recipient: recipient, Amount: amount, - Deposit: "100uatom", + Deposit: "1000uatom", } commSpendBody, err := json.MarshalIndent(proposalCommSpend, "", " ") s.Require().NoError(err) @@ -650,6 +650,16 @@ func (s *IntegrationTestSuite) writeGovCommunitySpendProposal(c *chain, amount s s.Require().NoError(err) } +type ConsumerAdditionProposalWithDeposit struct { + ccvprovider.ConsumerAdditionProposal + Deposit string `json:"deposit"` +} + +type ConsumerRemovalProposalWithDeposit struct { + ccvprovider.ConsumerRemovalProposal + Deposit string `json:"deposit"` +} + func (s *IntegrationTestSuite) writeAddRemoveConsumerProposals(c *chain, consumerChainID string) { hash, _ := json.Marshal("Z2VuX2hhc2g=") addProp := &ccvprovider.ConsumerAdditionProposal{ @@ -669,6 +679,10 @@ func (s *IntegrationTestSuite) writeAddRemoveConsumerProposals(c *chain, consume BlocksPerDistributionTransmission: 10, HistoricalEntries: 10000, } + addPropWithDeposit := ConsumerAdditionProposalWithDeposit{ + ConsumerAdditionProposal: *addProp, + Deposit: "1000uatom", + } removeProp := &ccvprovider.ConsumerRemovalProposal{ Title: "Remove consumer chain", @@ -677,10 +691,15 @@ func (s *IntegrationTestSuite) writeAddRemoveConsumerProposals(c *chain, consume StopTime: time.Now(), } - consumerAddBody, err := json.MarshalIndent(addProp, "", " ") + removePropWithDeposit := ConsumerRemovalProposalWithDeposit{ + ConsumerRemovalProposal: *removeProp, + Deposit: "1000uatom", + } + + consumerAddBody, err := json.MarshalIndent(addPropWithDeposit, "", " ") s.Require().NoError(err) - consumerRemoveBody, err := json.MarshalIndent(removeProp, "", " ") + consumerRemoveBody, err := json.MarshalIndent(removePropWithDeposit, "", " ") s.Require().NoError(err) err = writeFile(filepath.Join(c.validators[0].configDir(), "config", proposalAddConsumerChainFilename), consumerAddBody)