Skip to content

Commit

Permalink
add spam prevention antehandler (#2262)
Browse files Browse the repository at this point in the history
* 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

(cherry picked from commit 23d9f67)
  • Loading branch information
MSalopek authored and mergify[bot] committed Mar 9, 2023
1 parent 682770f commit 9382c96
Show file tree
Hide file tree
Showing 7 changed files with 235 additions and 15 deletions.
8 changes: 8 additions & 0 deletions ante/ante.go
Original file line number Diff line number Diff line change
@@ -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"
Expand All @@ -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
Expand All @@ -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 {
Expand All @@ -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),
Expand Down
95 changes: 95 additions & 0 deletions ante/gov_ante.go
Original file line number Diff line number Diff line change
@@ -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
}
92 changes: 92 additions & 0 deletions ante/gov_ante_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
}
}
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: 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_globalfee_proposal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
22 changes: 13 additions & 9 deletions tests/e2e/e2e_gov_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand All @@ -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)
}
Expand All @@ -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)
Expand Down
29 changes: 24 additions & 5 deletions 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(330000000)) // 3,300uatom
distModuleAddress = authtypes.NewModuleAddress(distrtypes.ModuleName).String()
proposalCounter = 0
)
Expand Down Expand Up @@ -627,7 +627,7 @@ func (s *IntegrationTestSuite) writeGovParamChangeProposalGlobalFees(c *chain, c
Value: coins,
},
},
Deposit: "",
Deposit: "1000uatom",
}, "", " ")
s.Require().NoError(err)

Expand All @@ -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)
Expand All @@ -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{
Expand All @@ -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",
Expand All @@ -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)
Expand Down

0 comments on commit 9382c96

Please sign in to comment.