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

add spam prevention antehandler (backport #2262) #2276

Merged
merged 1 commit into from
Mar 9, 2023
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
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