Skip to content

Commit

Permalink
Democracy governance whitelisting #284 (#364)
Browse files Browse the repository at this point in the history
* Democracy governance whitelisting #284

* Democracy gov whitelist - added logs and event

* Gov whitelisting -  delete only active forbidden prop.

* Democracy gov whitelisting - test moved

Co-authored-by: Daniel T <[email protected]>
Co-authored-by: Jehan <[email protected]>
  • Loading branch information
3 people authored Oct 5, 2022
1 parent 149b42e commit fbd6c40
Show file tree
Hide file tree
Showing 9 changed files with 433 additions and 4 deletions.
32 changes: 32 additions & 0 deletions app/consumer-democracy/ante/forbidden_proposals_ante.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
package ante

import (
"fmt"

sdk "github.com/cosmos/cosmos-sdk/types"
govtypes "github.com/cosmos/cosmos-sdk/x/gov/types"
)

type ForbiddenProposalsDecorator struct {
IsProposalWhitelisted func(govtypes.Content) bool
}

func NewForbiddenProposalsDecorator(whiteListFn func(govtypes.Content) bool) ForbiddenProposalsDecorator {
return ForbiddenProposalsDecorator{IsProposalWhitelisted: whiteListFn}
}

func (decorator ForbiddenProposalsDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (newCtx sdk.Context, err error) {
currHeight := ctx.BlockHeight()

for _, msg := range tx.GetMsgs() {
submitProposalMgs, ok := msg.(*govtypes.MsgSubmitProposal)
//if the message is MsgSubmitProposal, check if proposal is whitelisted
if ok {
if !decorator.IsProposalWhitelisted(submitProposalMgs.GetContent()) {
return ctx, fmt.Errorf("tx contains unsupported proposal message types at height %d", currHeight)
}
}
}

return next(ctx, tx, simulate)
}
97 changes: 97 additions & 0 deletions app/consumer-democracy/ante/forbidden_proposals_ante_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
package ante_test

import (
"testing"

sdk "github.com/cosmos/cosmos-sdk/types"
authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
govtypes "github.com/cosmos/cosmos-sdk/x/gov/types"
"github.com/cosmos/cosmos-sdk/x/params/types/proposal"
app "github.com/cosmos/interchain-security/app/consumer-democracy"
"github.com/cosmos/interchain-security/app/consumer-democracy/ante"
"github.com/stretchr/testify/require"
"github.com/tendermint/spm/cosmoscmd"
)

func TestForbiddenProposalsDecorator(t *testing.T) {
txCfg := cosmoscmd.MakeEncodingConfig(app.ModuleBasics).TxConfig

testCases := []struct {
name string
ctx sdk.Context
msgs []sdk.Msg
expectErr bool
}{
{
name: "Allowed param change",
ctx: sdk.Context{},
msgs: []sdk.Msg{
newParamChangeProposalMsg([]proposal.ParamChange{
//only subspace and key are relevant for testing
{Subspace: banktypes.ModuleName, Key: "SendEnabled", Value: ""},
}),
},
expectErr: false,
},
{
name: "Forbidden param change",
ctx: sdk.Context{},
msgs: []sdk.Msg{
newParamChangeProposalMsg([]proposal.ParamChange{
{Subspace: authtypes.ModuleName, Key: "MaxMemoCharacters", Value: ""},
}),
},
expectErr: true,
},
{
name: "Allowed and forbidden param changes in the same msg",
ctx: sdk.Context{},
msgs: []sdk.Msg{
newParamChangeProposalMsg([]proposal.ParamChange{
{Subspace: banktypes.ModuleName, Key: "SendEnabled", Value: ""},
{Subspace: authtypes.ModuleName, Key: "MaxMemoCharacters", Value: ""},
}),
},
expectErr: true,
},
{
name: "Allowed and forbidden param changes in different msg",
ctx: sdk.Context{},
msgs: []sdk.Msg{
newParamChangeProposalMsg([]proposal.ParamChange{
{Subspace: banktypes.ModuleName, Key: "SendEnabled", Value: ""},
}),
newParamChangeProposalMsg([]proposal.ParamChange{
{Subspace: authtypes.ModuleName, Key: "MaxMemoCharacters", Value: ""},
}),
},
expectErr: true,
},
}

for _, tc := range testCases {
tc := tc

t.Run(tc.name, func(t *testing.T) {
handler := ante.NewForbiddenProposalsDecorator(app.IsProposalWhitelisted)

txBuilder := txCfg.NewTxBuilder()
require.NoError(t, txBuilder.SetMsgs(tc.msgs...))

_, err := handler.AnteHandle(tc.ctx, txBuilder.GetTx(), false,
func(ctx sdk.Context, _ sdk.Tx, _ bool) (sdk.Context, error) { return ctx, nil })
if tc.expectErr {
require.Error(t, err)
} else {
require.NoError(t, err)
}
})
}
}

func newParamChangeProposalMsg(changes []proposal.ParamChange) *govtypes.MsgSubmitProposal {
paramChange := proposal.ParameterChangeProposal{Changes: changes}
msg, _ := govtypes.NewMsgSubmitProposal(&paramChange, sdk.NewCoins(), sdk.AccAddress{})
return msg
}
2 changes: 2 additions & 0 deletions app/consumer-democracy/ante_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"github.com/cosmos/cosmos-sdk/x/auth/ante"
ibcante "github.com/cosmos/ibc-go/v3/modules/core/ante"
ibckeeper "github.com/cosmos/ibc-go/v3/modules/core/keeper"
democracyante "github.com/cosmos/interchain-security/app/consumer-democracy/ante"
consumerante "github.com/cosmos/interchain-security/app/consumer/ante"
ibcconsumerkeeper "github.com/cosmos/interchain-security/x/ccv/consumer/keeper"
)
Expand Down Expand Up @@ -40,6 +41,7 @@ func NewAnteHandler(options HandlerOptions) (sdk.AnteHandler, error) {
ante.NewRejectExtensionOptionsDecorator(),
consumerante.NewMsgFilterDecorator(options.ConsumerKeeper),
consumerante.NewDisabledModulesDecorator("/cosmos.evidence", "/cosmos.slashing"),
democracyante.NewForbiddenProposalsDecorator(IsProposalWhitelisted),
ante.NewMempoolFeeDecorator(),
ante.NewValidateBasicDecorator(),
ante.NewTxTimeoutHeightDecorator(),
Expand Down
9 changes: 5 additions & 4 deletions app/consumer-democracy/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,10 @@ import (
ccvstakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"
ccvstaking "github.com/cosmos/interchain-security/x/ccv/democracy/staking"

ccvgov "github.com/cosmos/cosmos-sdk/x/gov"
gov "github.com/cosmos/cosmos-sdk/x/gov"
ccvgovkeeper "github.com/cosmos/cosmos-sdk/x/gov/keeper"
ccvgovtypes "github.com/cosmos/cosmos-sdk/x/gov/types"
ccvgov "github.com/cosmos/interchain-security/x/ccv/democracy/governance"

// add mint
ccvmint "github.com/cosmos/cosmos-sdk/x/mint"
Expand Down Expand Up @@ -129,7 +130,7 @@ var (
ccvstaking.AppModuleBasic{},
ccvmint.AppModuleBasic{},
ccvdistr.AppModuleBasic{},
ccvgov.NewAppModuleBasic(
gov.NewAppModuleBasic(
// TODO: eventually remove upgrade proposal handler and cancel proposal handler
paramsclient.ProposalHandler, ccvdistrclient.ProposalHandler, upgradeclient.ProposalHandler, upgradeclient.CancelProposalHandler,
),
Expand Down Expand Up @@ -477,7 +478,7 @@ func New(
capability.NewAppModule(appCodec, *app.CapabilityKeeper),
crisis.NewAppModule(&app.CrisisKeeper, skipGenesisInvariants),
feegrantmodule.NewAppModule(appCodec, app.AccountKeeper, app.BankKeeper, app.FeeGrantKeeper, app.interfaceRegistry),
ccvgov.NewAppModule(appCodec, app.GovKeeper, app.AccountKeeper, app.BankKeeper),
ccvgov.NewAppModule(appCodec, app.GovKeeper, app.AccountKeeper, app.BankKeeper, IsProposalWhitelisted),
ccvmint.NewAppModule(appCodec, app.MintKeeper, app.AccountKeeper),
slashing.NewAppModule(appCodec, app.SlashingKeeper, app.AccountKeeper, app.BankKeeper, app.ConsumerKeeper),
ccvdistr.NewAppModule(appCodec, app.DistrKeeper, app.AccountKeeper, app.BankKeeper, app.StakingKeeper, authtypes.FeeCollectorName),
Expand Down Expand Up @@ -580,7 +581,7 @@ func New(
bank.NewAppModule(appCodec, app.BankKeeper, app.AccountKeeper),
capability.NewAppModule(appCodec, *app.CapabilityKeeper),
feegrantmodule.NewAppModule(appCodec, app.AccountKeeper, app.BankKeeper, app.FeeGrantKeeper, app.interfaceRegistry),
ccvgov.NewAppModule(appCodec, app.GovKeeper, app.AccountKeeper, app.BankKeeper),
ccvgov.NewAppModule(appCodec, app.GovKeeper, app.AccountKeeper, app.BankKeeper, IsProposalWhitelisted),
ccvmint.NewAppModule(appCodec, app.MintKeeper, app.AccountKeeper),
ccvstaking.NewAppModule(appCodec, app.StakingKeeper, app.AccountKeeper, app.BankKeeper),
ccvdistr.NewAppModule(appCodec, app.DistrKeeper, app.AccountKeeper, app.BankKeeper, app.StakingKeeper, authtypes.FeeCollectorName),
Expand Down
68 changes: 68 additions & 0 deletions app/consumer-democracy/proposals_whitelisting.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
package app

import (
banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
distrtypes "github.com/cosmos/cosmos-sdk/x/distribution/types"
govtypes "github.com/cosmos/cosmos-sdk/x/gov/types"
minttypes "github.com/cosmos/cosmos-sdk/x/mint/types"
"github.com/cosmos/cosmos-sdk/x/params/types/proposal"
stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"
ibctransfertypes "github.com/cosmos/ibc-go/v3/modules/apps/transfer/types"
)

func IsProposalWhitelisted(content govtypes.Content) bool {

switch c := content.(type) {
case *proposal.ParameterChangeProposal:
return isParamChangeWhitelisted(c.Changes)

default:
return false
}

}

func isParamChangeWhitelisted(paramChanges []proposal.ParamChange) bool {
for _, paramChange := range paramChanges {
_, found := WhitelistedParams[paramChangeKey{Subspace: paramChange.Subspace, Key: paramChange.Key}]
if !found {
return false
}
}
return true
}

type paramChangeKey struct {
Subspace, Key string
}

var WhitelistedParams = map[paramChangeKey]struct{}{
//bank
{Subspace: banktypes.ModuleName, Key: "SendEnabled"}: {},
//governance
{Subspace: govtypes.ModuleName, Key: "depositparams"}: {}, //min_deposit, max_deposit_period
{Subspace: govtypes.ModuleName, Key: "votingparams"}: {}, //voting_period
{Subspace: govtypes.ModuleName, Key: "tallyparams"}: {}, //quorum,threshold,veto_threshold
//staking
{Subspace: stakingtypes.ModuleName, Key: "UnbondingTime"}: {},
{Subspace: stakingtypes.ModuleName, Key: "MaxValidators"}: {},
{Subspace: stakingtypes.ModuleName, Key: "MaxEntries"}: {},
{Subspace: stakingtypes.ModuleName, Key: "HistoricalEntries"}: {},
{Subspace: stakingtypes.ModuleName, Key: "BondDenom"}: {},
//distribution
{Subspace: distrtypes.ModuleName, Key: "communitytax"}: {},
{Subspace: distrtypes.ModuleName, Key: "baseproposerreward"}: {},
{Subspace: distrtypes.ModuleName, Key: "bonusproposerreward"}: {},
{Subspace: distrtypes.ModuleName, Key: "withdrawaddrenabled"}: {},
//mint
{Subspace: minttypes.ModuleName, Key: "MintDenom"}: {},
{Subspace: minttypes.ModuleName, Key: "InflationRateChange"}: {},
{Subspace: minttypes.ModuleName, Key: "InflationMax"}: {},
{Subspace: minttypes.ModuleName, Key: "InflationMin"}: {},
{Subspace: minttypes.ModuleName, Key: "GoalBonded"}: {},
{Subspace: minttypes.ModuleName, Key: "BlocksPerYear"}: {},
//ibc transfer
{Subspace: ibctransfertypes.ModuleName, Key: "SendEnabled"}: {},
{Subspace: ibctransfertypes.ModuleName, Key: "ReceiveEnabled"}: {},
//add interchain account params(HostEnabled, AllowedMessages) once the module is added to the consumer app
}
21 changes: 21 additions & 0 deletions app/consumer-democracy/proposals_whitelisting_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package app_test

import (
"testing"

ibctesting "github.com/cosmos/ibc-go/v3/testing"
appConsumer "github.com/cosmos/interchain-security/app/consumer-democracy"
simapp "github.com/cosmos/interchain-security/testutil/simapp"
"github.com/stretchr/testify/require"
)

func TestDemocracyGovernanceWhitelistingKeys(t *testing.T) {
chain := ibctesting.NewTestChain(t, simapp.NewBasicCoordinator(t), simapp.SetupTestingAppConsumerDemocracy, "test")
paramKeeper := chain.App.(*appConsumer.App).ParamsKeeper
for paramKey := range appConsumer.WhitelistedParams {
ss, ok := paramKeeper.GetSubspace(paramKey.Subspace)
require.True(t, ok, "Unknown subspace %s", paramKey.Subspace)
hasKey := ss.Has(chain.GetContext(), []byte(paramKey.Key))
require.True(t, hasKey, "Invalid key %s for subspace %s", paramKey.Key, paramKey.Subspace)
}
}
Loading

0 comments on commit fbd6c40

Please sign in to comment.