diff --git a/.changelog/unreleased/improvements/2381-remove-governance-whitelisting.md b/.changelog/unreleased/improvements/2381-remove-governance-whitelisting.md new file mode 100644 index 0000000000..c75ccb407f --- /dev/null +++ b/.changelog/unreleased/improvements/2381-remove-governance-whitelisting.md @@ -0,0 +1,2 @@ +- `[x/democracy/governance]` Removal of consumer governance whitelisting functionality + ([\#2381](https://github.com/cosmos/interchain-security/pull/2381)) \ No newline at end of file diff --git a/.changelog/unreleased/state-breaking/2381-remove-governance-whitelisting.md b/.changelog/unreleased/state-breaking/2381-remove-governance-whitelisting.md new file mode 100644 index 0000000000..c75ccb407f --- /dev/null +++ b/.changelog/unreleased/state-breaking/2381-remove-governance-whitelisting.md @@ -0,0 +1,2 @@ +- `[x/democracy/governance]` Removal of consumer governance whitelisting functionality + ([\#2381](https://github.com/cosmos/interchain-security/pull/2381)) \ No newline at end of file diff --git a/app/consumer-democracy/ante/forbidden_proposals_ante.go b/app/consumer-democracy/ante/forbidden_proposals_ante.go deleted file mode 100644 index 2856aa4e5b..0000000000 --- a/app/consumer-democracy/ante/forbidden_proposals_ante.go +++ /dev/null @@ -1,57 +0,0 @@ -package ante - -import ( - "fmt" - - sdk "github.com/cosmos/cosmos-sdk/types" - govv1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1" - govv1beta1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1beta1" -) - -type ForbiddenProposalsDecorator struct { - isLegacyProposalWhitelisted func(govv1beta1.Content) bool - isModuleWhiteList func(string) bool -} - -func NewForbiddenProposalsDecorator( - whiteListFn func(govv1beta1.Content) bool, - isModuleWhiteList func(string) bool, -) ForbiddenProposalsDecorator { - return ForbiddenProposalsDecorator{ - isLegacyProposalWhitelisted: whiteListFn, - isModuleWhiteList: isModuleWhiteList, - } -} - -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() { - // if the message is MsgSubmitProposal, check if proposal is whitelisted - submitProposalMgs, ok := msg.(*govv1.MsgSubmitProposal) - if !ok { - continue - } - - messages := submitProposalMgs.GetMessages() - for _, message := range messages { - if sdkMsg, isLegacyProposal := message.GetCachedValue().(*govv1.MsgExecLegacyContent); isLegacyProposal { - // legacy gov proposal content - content, err := govv1.LegacyContentFromMessage(sdkMsg) - if err != nil { - return ctx, fmt.Errorf("tx contains invalid LegacyContent") - } - if !decorator.isLegacyProposalWhitelisted(content) { - return ctx, fmt.Errorf("tx contains unsupported proposal message types at height %d", currHeight) - } - continue - } - // not legacy gov proposal content and not whitelisted - if !decorator.isModuleWhiteList(message.TypeUrl) { - return ctx, fmt.Errorf("tx contains unsupported proposal message types at height %d", currHeight) - } - } - } - - return next(ctx, tx, simulate) -} diff --git a/app/consumer-democracy/ante/forbidden_proposals_ante_test.go b/app/consumer-democracy/ante/forbidden_proposals_ante_test.go deleted file mode 100644 index 58b8284b78..0000000000 --- a/app/consumer-democracy/ante/forbidden_proposals_ante_test.go +++ /dev/null @@ -1,169 +0,0 @@ -package ante_test - -import ( - "testing" - - ibctransfertypes "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types" - "github.com/stretchr/testify/require" - - sdk "github.com/cosmos/cosmos-sdk/types" - authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" - govtypes "github.com/cosmos/cosmos-sdk/x/gov/types" - govv1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1" - minttypes "github.com/cosmos/cosmos-sdk/x/mint/types" - "github.com/cosmos/cosmos-sdk/x/params/types/proposal" - - app "github.com/cosmos/interchain-security/v6/app/consumer-democracy" - "github.com/cosmos/interchain-security/v6/app/consumer-democracy/ante" -) - -// in SDKv47 parameter updates full params object is required -// either all params can be updated or none can be updated -func TestForbiddenProposalsDecorator(t *testing.T) { - txCfg := app.MakeTestEncodingConfig().TxConfig - - // here we try to set whatever params exist to their default values - // the actual parameter setting is not important, what's being tested is the ante handle filter - // Note: mint params CAN be changed according to WhiteListModule in proposals_whitelisting.go - updateMintParams := &minttypes.MsgUpdateParams{ - Authority: authtypes.NewModuleAddress(govtypes.ModuleName).String(), - Params: minttypes.DefaultParams(), - } - - // Note: auth params CANNOT be changed according to WhiteListModule in proposals_whitelisting.go - updateAuthParams := &authtypes.MsgUpdateParams{ - Authority: authtypes.NewModuleAddress(govtypes.ModuleName).String(), - Params: authtypes.DefaultParams(), - } - - testCases := []struct { - name string - ctx sdk.Context - msgs []sdk.Msg - expectErr bool - }{ - { - name: "Allowed param change - mint module", - ctx: sdk.Context{}, - msgs: []sdk.Msg{ - newParamChangeProposalMsg([]sdk.Msg{updateMintParams}), - }, - expectErr: false, - }, - { - name: "Forbidden param change - auth module", - ctx: sdk.Context{}, - msgs: []sdk.Msg{ - newParamChangeProposalMsg([]sdk.Msg{updateAuthParams}), - }, - expectErr: true, - }, - { - name: "Allowed and forbidden param changes in the same msg", - ctx: sdk.Context{}, - msgs: []sdk.Msg{ - newParamChangeProposalMsg([]sdk.Msg{updateMintParams, updateAuthParams}), - }, - expectErr: true, - }, - { - name: "Allowed and forbidden param changes in different msg", - ctx: sdk.Context{}, - msgs: []sdk.Msg{ - newParamChangeProposalMsg([]sdk.Msg{updateMintParams}), - newParamChangeProposalMsg([]sdk.Msg{updateAuthParams}), - }, - expectErr: true, - }, - } - - for _, tc := range testCases { - tc := tc - - t.Run(tc.name, func(t *testing.T) { - handler := ante.NewForbiddenProposalsDecorator(app.IsProposalWhitelisted, app.IsModuleWhiteList) - - 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) - } - }) - } -} - -// Legacy parameter proposals are not supported in cosmos-sdk v0.50 -// since modules parameters were moved to their respective modules -// this test is to ensure that legacy parameter proposals are not allowed -func TestForbiddenLegacyProposalsDecorator(t *testing.T) { - txCfg := app.MakeTestEncodingConfig().TxConfig - - testCases := []struct { - name string - ctx sdk.Context - msgs []sdk.Msg - expectErr bool - }{ - { - name: "Forbidden param change", - ctx: sdk.Context{}, - msgs: []sdk.Msg{ - newLegacyParamChangeProposalMsg([]proposal.ParamChange{ - {Subspace: authtypes.ModuleName, Key: "MaxMemoCharacters", Value: ""}, - }), - }, - expectErr: true, - }, - { - name: "Multiple forbidden param changes in the same msg", - ctx: sdk.Context{}, - msgs: []sdk.Msg{ - newLegacyParamChangeProposalMsg([]proposal.ParamChange{ - {Subspace: ibctransfertypes.ModuleName, Key: "SendEnabled", Value: "true"}, - {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, app.IsModuleWhiteList) - - 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) - } - }) - } -} - -// Use ParamChangeProposal -func newLegacyParamChangeProposalMsg(changes []proposal.ParamChange) *govv1.MsgSubmitProposal { - paramChange := proposal.ParameterChangeProposal{Changes: changes} - msgContent, err := govv1.NewLegacyContent(¶mChange, authtypes.NewModuleAddress(govtypes.ModuleName).String()) - if err != nil { - return nil - } - msg, _ := govv1.NewMsgSubmitProposal([]sdk.Msg{msgContent}, sdk.NewCoins(), sdk.AccAddress{}.String(), "", "", "", false) - return msg -} - -func newParamChangeProposalMsg(msgs []sdk.Msg) *govv1.MsgSubmitProposal { - msg, _ := govv1.NewMsgSubmitProposal(msgs, sdk.NewCoins(), sdk.AccAddress{}.String(), "", "", "", false) - return msg -} diff --git a/app/consumer-democracy/ante_handler.go b/app/consumer-democracy/ante_handler.go index 42016627b7..4d7b788603 100644 --- a/app/consumer-democracy/ante_handler.go +++ b/app/consumer-democracy/ante_handler.go @@ -10,7 +10,6 @@ import ( sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" "github.com/cosmos/cosmos-sdk/x/auth/ante" - democracyante "github.com/cosmos/interchain-security/v6/app/consumer-democracy/ante" consumerante "github.com/cosmos/interchain-security/v6/app/consumer/ante" ibcconsumerkeeper "github.com/cosmos/interchain-security/v6/x/ccv/consumer/keeper" ) @@ -45,7 +44,6 @@ func NewAnteHandler(options HandlerOptions) (sdk.AnteHandler, error) { ante.NewExtensionOptionsDecorator(nil), consumerante.NewMsgFilterDecorator(options.ConsumerKeeper), consumerante.NewDisabledModulesDecorator("/cosmos.evidence", "/cosmos.slashing"), - democracyante.NewForbiddenProposalsDecorator(IsProposalWhitelisted, IsModuleWhiteList), ante.NewValidateBasicDecorator(), ante.NewTxTimeoutHeightDecorator(), ante.NewValidateMemoDecorator(options.AccountKeeper), diff --git a/app/consumer-democracy/app.go b/app/consumer-democracy/app.go index cef5aa8f9c..646babd205 100644 --- a/app/consumer-democracy/app.go +++ b/app/consumer-democracy/app.go @@ -38,6 +38,7 @@ import ( "cosmossdk.io/x/feegrant" feegrantkeeper "cosmossdk.io/x/feegrant/keeper" feegrantmodule "cosmossdk.io/x/feegrant/module" + // add mint "cosmossdk.io/x/upgrade" upgradekeeper "cosmossdk.io/x/upgrade/keeper" @@ -114,7 +115,6 @@ import ( consumerkeeper "github.com/cosmos/interchain-security/v6/x/ccv/consumer/keeper" consumertypes "github.com/cosmos/interchain-security/v6/x/ccv/consumer/types" ccvdistr "github.com/cosmos/interchain-security/v6/x/ccv/democracy/distribution" - ccvgov "github.com/cosmos/interchain-security/v6/x/ccv/democracy/governance" ccvstaking "github.com/cosmos/interchain-security/v6/x/ccv/democracy/staking" ) @@ -549,7 +549,7 @@ func New( capability.NewAppModule(appCodec, *app.CapabilityKeeper, false), crisis.NewAppModule(&app.CrisisKeeper, skipGenesisInvariants, app.GetSubspace(crisistypes.ModuleName)), feegrantmodule.NewAppModule(appCodec, app.AccountKeeper, app.BankKeeper, app.FeeGrantKeeper, app.interfaceRegistry), - ccvgov.NewAppModule(appCodec, app.GovKeeper, app.AccountKeeper, app.BankKeeper, IsProposalWhitelisted, app.GetSubspace(govtypes.ModuleName), IsModuleWhiteList), + gov.NewAppModule(appCodec, &app.GovKeeper, app.AccountKeeper, app.BankKeeper, app.GetSubspace(govtypes.ModuleName)), mint.NewAppModule(appCodec, app.MintKeeper, app.AccountKeeper, nil, app.GetSubspace(minttypes.ModuleName)), slashing.NewAppModule(appCodec, app.SlashingKeeper, app.AccountKeeper, app.BankKeeper, app.ConsumerKeeper, app.GetSubspace(slashingtypes.ModuleName), app.interfaceRegistry), ccvdistr.NewAppModule(appCodec, app.DistrKeeper, app.AccountKeeper, app.BankKeeper, *app.StakingKeeper, authtypes.FeeCollectorName, app.GetSubspace(distrtypes.ModuleName)), diff --git a/app/consumer-democracy/proposals_whitelisting.go b/app/consumer-democracy/proposals_whitelisting.go deleted file mode 100644 index 975c9dbbad..0000000000 --- a/app/consumer-democracy/proposals_whitelisting.go +++ /dev/null @@ -1,55 +0,0 @@ -package app - -import ( - banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" - "github.com/cosmos/cosmos-sdk/x/gov/types/v1beta1" - "github.com/cosmos/cosmos-sdk/x/params/types/proposal" -) - -func IsProposalWhitelisted(content v1beta1.Content) bool { - switch c := content.(type) { - case *proposal.ParameterChangeProposal: - return isLegacyParamChangeWhitelisted(c.Changes) - default: - return false - } -} - -func isLegacyParamChangeWhitelisted(paramChanges []proposal.ParamChange) bool { - for _, paramChange := range paramChanges { - _, found := LegacyWhitelistedParams[legacyParamChangeKey{Subspace: paramChange.Subspace, Key: paramChange.Key}] - if !found { - return false - } - } - return true -} - -type legacyParamChangeKey struct { - Subspace, Key string -} - -// these parameters don't exist in the consumer app -- keeping them as an example -var LegacyWhitelistedParams = map[legacyParamChangeKey]struct{}{ - // add whitelisted legacy parameters here [cosmos-sdk <= 0.47] - // commented parameters are just an example - most params have been moved to their respective modules - // and they cannot be changed through legacy governance proposals - {Subspace: banktypes.ModuleName, Key: "SendEnabled"}: {}, -} - -// add whitelisted module param update messages [cosmos-sdk >= 0.47] -var WhiteListModule = map[string]struct{}{ - "/cosmos.gov.v1.MsgUpdateParams": {}, - "/cosmos.bank.v1beta1.MsgUpdateParams": {}, - "/cosmos.staking.v1beta1.MsgUpdateParams": {}, - "/cosmos.distribution.v1beta1.MsgUpdateParams": {}, - "/cosmos.mint.v1beta1.MsgUpdateParams": {}, - "/cosmos.gov.v1beta1.TextProposal": {}, - "/ibc.applications.transfer.v1.MsgUpdateParams": {}, - "/interchain_security.ccv.consumer.v1.MsgUpdateParams": {}, -} - -func IsModuleWhiteList(typeUrl string) bool { - _, found := WhiteListModule[typeUrl] - return found -} diff --git a/docs/docs/build/modules/04-democracy.md b/docs/docs/build/modules/04-democracy.md index 7ade73bd3c..b2632cf91c 100644 --- a/docs/docs/build/modules/04-democracy.md +++ b/docs/docs/build/modules/04-democracy.md @@ -4,7 +4,7 @@ sidebar_position: 4 # x/ccv/democracy -The democracy modules comprise `x/ccv/democracy/staking`, `x/ccv/democracy/distribution` and `x/ccv/democracy/governance` with overrides and extensions required for normal operation when participating in ICS. +The democracy modules comprise `x/ccv/democracy/staking` and `x/ccv/democracy/distribution` with overrides and extensions required for normal operation when participating in ICS. The modules are plug-and-play and only require small wiring changes to be enabled. @@ -216,225 +216,6 @@ func NewApp(...) { } ``` -## Governance - -The `x/ccv/democracy/governance` module extends the `x/governance` module with the functionality to filter proposals. -The module uses `AnteHandler` to limit the types of proposals that can be executed. -As a result, consumer chains can limit the types of governance proposals that can be executed on chain to avoid inadvertent changes to the ICS protocol that could affect security properties. - -### Integration - -Add new `AnteHandler` to your `app`. - -```go - -// app/ante/forbidden_proposals.go -package ante - -import ( - "fmt" - - sdk "github.com/cosmos/cosmos-sdk/types" - govv1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1" - govv1beta1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1beta1" - ibctransfertypes "github.com/cosmos/ibc-go/v7/modules/apps/transfer/types" - - "github.com/cosmos/cosmos-sdk/x/gov/types/v1beta1" - "github.com/cosmos/cosmos-sdk/x/params/types/proposal" -) - -type ForbiddenProposalsDecorator struct { - isLegacyProposalWhitelisted func(govv1beta1.Content) bool - isModuleWhiteList func(string) bool -} - -func NewForbiddenProposalsDecorator( - whiteListFn func(govv1beta1.Content) bool, - isModuleWhiteList func(string) bool, -) ForbiddenProposalsDecorator { - return ForbiddenProposalsDecorator{ - isLegacyProposalWhitelisted: whiteListFn, - isModuleWhiteList: isModuleWhiteList, - } -} - -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() { - // if the message is MsgSubmitProposal, check if proposal is whitelisted - submitProposalMgs, ok := msg.(*govv1.MsgSubmitProposal) - if !ok { - continue - } - - messages := submitProposalMgs.GetMessages() - for _, message := range messages { - if sdkMsg, isLegacyProposal := message.GetCachedValue().(*govv1.MsgExecLegacyContent); isLegacyProposal { - // legacy gov proposal content - content, err := govv1.LegacyContentFromMessage(sdkMsg) - if err != nil { - return ctx, fmt.Errorf("tx contains invalid LegacyContent") - } - if !decorator.isLegacyProposalWhitelisted(content) { - return ctx, fmt.Errorf("tx contains unsupported proposal message types at height %d", currHeight) - } - continue - } - // not legacy gov proposal content and not whitelisted - if !decorator.isModuleWhiteList(message.TypeUrl) { - return ctx, fmt.Errorf("tx contains unsupported proposal message types at height %d", currHeight) - } - } - } - - return next(ctx, tx, simulate) -} - -func IsProposalWhitelisted(content v1beta1.Content) bool { - switch c := content.(type) { - case *proposal.ParameterChangeProposal: - return isLegacyParamChangeWhitelisted(c.Changes) - - default: - return false - } -} - -func isLegacyParamChangeWhitelisted(paramChanges []proposal.ParamChange) bool { - for _, paramChange := range paramChanges { - _, found := LegacyWhitelistedParams[legacyParamChangeKey{Subspace: paramChange.Subspace, Key: paramChange.Key}] - if !found { - return false - } - } - return true -} - -type legacyParamChangeKey struct { - Subspace, Key string -} - -// Legacy params can be whitelisted -var LegacyWhitelistedParams = map[legacyParamChangeKey]struct{}{ - {Subspace: ibctransfertypes.ModuleName, Key: "SendEnabled"}: {}, - {Subspace: ibctransfertypes.ModuleName, Key: "ReceiveEnabled"}: {}, -} - -// New proposal types can be whitelisted -var WhiteListModule = map[string]struct{}{ - "/cosmos.gov.v1.MsgUpdateParams": {}, - "/cosmos.bank.v1beta1.MsgUpdateParams": {}, - "/cosmos.staking.v1beta1.MsgUpdateParams": {}, - "/cosmos.distribution.v1beta1.MsgUpdateParams": {}, - "/cosmos.mint.v1beta1.MsgUpdateParams": {}, -} - -func IsModuleWhiteList(typeUrl string) bool { - _, found := WhiteListModule[typeUrl] - return found -} -``` - -Add the `AnteHandler` to the list of supported antehandlers: - -```diff -// app/ante_handler.go -package app - -import ( - ... - -+ democracyante "github.com/cosmos/interchain-security/v4/app/consumer-democracy/ante" -+ consumerante "github.com/cosmos/interchain-security/v4/app/consumer/ante" -+ icsconsumerkeeper "github.com/cosmos/interchain-security/v4/x/ccv/consumer/keeper" -) - -type HandlerOptions struct { - ante.HandlerOptions - - IBCKeeper *ibckeeper.Keeper -+ ConsumerKeeper ibcconsumerkeeper.Keeper -} - -func NewAnteHandler(options HandlerOptions) (sdk.AnteHandler, error) { - .... - - anteDecorators := []sdk.AnteDecorator{ - ... -+ consumerante.NewMsgFilterDecorator(options.ConsumerKeeper), -+ consumerante.NewDisabledModulesDecorator("/cosmos.evidence", "/cosmos.slashing"), -+ democracyante.NewForbiddenProposalsDecorator(IsProposalWhitelisted, IsModuleWhiteList), - ... - } - - return sdk.ChainAnteDecorators(anteDecorators...), nil -} -``` - -Wire the module in `app.go`. - -```diff -// app/app.go -package app -import ( - ... - sdkgov "github.com/cosmos/cosmos-sdk/x/gov" - govkeeper "github.com/cosmos/cosmos-sdk/x/gov/keeper" - govtypes "github.com/cosmos/cosmos-sdk/x/gov/types" - govv1beta1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1beta1" - -+ ccvgov "github.com/cosmos/interchain-security/v4/x/ccv/democracy/governance" -) - -var ( - - // use sdk governance module - ModuleBasics = module.NewBasicManager( - ... - sdkgov.NewAppModuleBasic( - []govclient.ProposalHandler{ - paramsclient.ProposalHandler, - upgradeclient.LegacyProposalHandler, - upgradeclient.LegacyCancelProposalHandler, - }, - ), - ) -) - -func NewApp(...) { - // retain sdk gov router and keeper registrations - sdkgovRouter := govv1beta1.NewRouter() - sdkgovRouter. - AddRoute(govtypes.RouterKey, govv1beta1.ProposalHandler). - AddRoute(paramproposal.RouterKey, params.NewParamChangeProposalHandler(app.ParamsKeeper)). - AddRoute(upgradetypes.RouterKey, upgrade.NewSoftwareUpgradeProposalHandler(&app.UpgradeKeeper)) - govConfig := govtypes.DefaultConfig() - - app.GovKeeper = *govkeeper.NewKeeper( - appCodec, - keys[govtypes.StoreKey], - app.AccountKeeper, - app.BankKeeper, - app.StakingKeeper, - app.MsgServiceRouter(), - govConfig, - authtypes.NewModuleAddress(govtypes.ModuleName).String(), - ) - - app.GovKeeper.SetLegacyRouter(sdkgovRouter) - - - // register the module with module manager - // replace the x/gov module - app.MM = module.NewManager( -- sdkgov.NewAppModule(appCodec, app.GovKeeper, app.AccountKeeper, app.BankKeeper, IsProposalWhitelisted, app.GetSubspace(govtypes.ModuleName), IsModuleWhiteList), -+ ccvgov.NewAppModule(appCodec, app.GovKeeper, app.AccountKeeper, app.BankKeeper, IsProposalWhitelisted, app.GetSubspace(govtypes.ModuleName), IsModuleWhiteList), - ... - ) -} -``` - ## Distribution The `x/ccv/democracy/distribution` module allows the consumer chain to send rewards to the provider chain while retaining the logic of the `x/distribution` module for internal reward distribution to governators and their delegators. diff --git a/docs/docs/consumer-development/consumer-chain-governance.md b/docs/docs/consumer-development/consumer-chain-governance.md index 86b56f8418..58a35541a8 100644 --- a/docs/docs/consumer-development/consumer-chain-governance.md +++ b/docs/docs/consumer-development/consumer-chain-governance.md @@ -4,7 +4,7 @@ sidebar_position: 3 # Consumer Chain Governance -Different consumer chains can do governance in different ways. However, no matter what the governance method is, there are a few settings specifically related to consensus that consumer chain governance cannot change. We'll cover what these are in the "Whitelist" section below. +Different consumer chains can do governance in different ways. ## Democracy module @@ -19,7 +19,3 @@ For an example, see the [Democracy Consumer](https://github.com/cosmos/interchai There are several great DAO and governance frameworks written as CosmWasm contracts. These can be used as the main governance system for a consumer chain. Actions triggered by the CosmWasm governance contracts are able to affect parameters and trigger actions on the consumer chain. For an example, see [Neutron](https://github.com/neutron-org/neutron/). - -## The Whitelist - -Not everything on a consumer chain can be changed by the consumer's governance. Some settings having to do with consensus etc. can only be changed by the provider chain. Consumer chains include a whitelist of parameters that are allowed to be changed by the consumer chain governance. For an example, see [Neutron's](https://github.com/neutron-org/neutron/blob/main/app/proposals_allowlisting.go) whitelist. diff --git a/docs/upgrades_reference/democracy.md b/docs/upgrades_reference/democracy.md index 4c89199627..e03c043bdf 100644 --- a/docs/upgrades_reference/democracy.md +++ b/docs/upgrades_reference/democracy.md @@ -67,11 +67,5 @@ The inner workings of `EndBlock` were refactored to allow using the cosmos-sdk g ## App wiring & tests -Whitelisted proposal list was changed because `param-change` proposals were deprecated for most modules (they cannot be submitted). - -Added to whitelists: -* `/cosmos.gov.v1beta1.TextProposal` - - e2e tests were refactored to send the `TextProposal` instead of a `param-change` because there are no modules that can process `param-change` so we cannot use those proposals any longer. diff --git a/docs/upgrades_reference/imports_only.md b/docs/upgrades_reference/imports_only.md index f8f0ab08b5..88b221cee7 100644 --- a/docs/upgrades_reference/imports_only.md +++ b/docs/upgrades_reference/imports_only.md @@ -1,8 +1,4 @@ # import change only [file list] -* app/consumer-democracy/ante_handler.go -* app/consumer-democracy/proposals_whitelisting.go -* app/consumer-democracy/proposals_whitelisting_test.go -* app/consumer/ante/disabled_modules_ante_test.go * app/consumer/ante/msg_filter_ante_test.go * app/consumer/ante_handler.go * app/provider/ante_handler.go diff --git a/scripts/test_doc/test_documentation.md b/scripts/test_doc/test_documentation.md index 1bdbaee73d..5e8b1eb6b2 100644 --- a/scripts/test_doc/test_documentation.md +++ b/scripts/test_doc/test_documentation.md @@ -13,9 +13,8 @@ | Function | Short Description | |----------|-------------------| - [TestDemocracyRewardsDistribution](../../tests/integration/democracy.go#L78) | TestDemocracyRewardsDistribution checks that rewards to democracy representatives, community pool, and provider redistribution account are done correctly.
Details* Set up a democracy consumer chain.
* Create a new block.
* Check that rewards to democracy representatives, community pool, and provider redistribution account are distributed in the right proportions.
| - [TestDemocracyGovernanceWhitelisting](../../tests/integration/democracy.go#L194) | TestDemocracyGovernanceWhitelisting checks that only whitelisted governance proposals can be executed on democracy consumer chains.
DetailsFor context, see the whitelist for proposals in app/consumer-democracy/proposals_whitelisting.go.
* Set up a democracy consumer chain.
* Submit a proposal containing changes to the auth and mint module parameters.
* Check that the proposal is not executed, since the change to the auth module is not whitelisted.
* Submit a proposal containing changes *only* to the mint module parameters.
* Check that the proposal is executed, since the change to the mint module is whitelisted.
* Submit a proposal containing changes *only* to the auth module parameters.
* Check that again, the proposal is not executed, since the change to the auth module is not whitelisted.
| - [TestDemocracyMsgUpdateParams](../../tests/integration/democracy.go#L294) | TestDemocracyMsgUpdateParams checks that the consumer parameters can be updated through a governance proposal.
Details* Set up a democracy consumer chain.
* Submit a proposal containing changes to the consumer module parameters.
* Check that the proposal is executed, and the parameters are updated.
| + [TestDemocracyRewardsDistribution](../../tests/integration/democracy.go#L77) | TestDemocracyRewardsDistribution checks that rewards to democracy representatives, community pool, and provider redistribution account are done correctly.
Details* Set up a democracy consumer chain.
* Create a new block.
* Check that rewards to democracy representatives, community pool, and provider redistribution account are distributed in the right proportions.
| + [TestDemocracyMsgUpdateParams](../../tests/integration/democracy.go#L187) | TestDemocracyMsgUpdateParams checks that the consumer parameters can be updated through a governance proposal.
Details* Set up a democracy consumer chain.
* Submit a proposal containing changes to the consumer module parameters.
* Check that the proposal is executed, and the parameters are updated.
| # [distribution.go](../../tests/integration/distribution.go) diff --git a/tests/e2e/steps_democracy.go b/tests/e2e/steps_democracy.go index f8b34f1001..ccf2cf465f 100644 --- a/tests/e2e/steps_democracy.go +++ b/tests/e2e/steps_democracy.go @@ -73,7 +73,6 @@ func stepsDemocracy(consumerName string, expectRegisteredRewardDistribution bool }, }, { - // whitelisted legacy proposal can only handle ibctransfer.SendEnabled/ReceiveEnabled Action: SubmitEnableTransfersProposalAction{ Chain: ChainID(consumerName), From: ValidatorID("alice"), diff --git a/tests/integration/democracy.go b/tests/integration/democracy.go index 1271bce12e..645cdb7805 100644 --- a/tests/integration/democracy.go +++ b/tests/integration/democracy.go @@ -14,7 +14,6 @@ import ( govkeeper "github.com/cosmos/cosmos-sdk/x/gov/keeper" govtypes "github.com/cosmos/cosmos-sdk/x/gov/types" govv1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1" - minttypes "github.com/cosmos/cosmos-sdk/x/mint/types" icstestingutils "github.com/cosmos/interchain-security/v6/testutil/ibc_testing" testutil "github.com/cosmos/interchain-security/v6/testutil/integration" @@ -180,112 +179,6 @@ func (s *ConsumerDemocracyTestSuite) TestDemocracyRewardsDistribution() { } } -// TestDemocracyGovernanceWhitelisting checks that only whitelisted governance proposals -// can be executed on democracy consumer chains. -// @Long Description@ -// For context, see the whitelist for proposals in app/consumer-democracy/proposals_whitelisting.go. -// * Set up a democracy consumer chain. -// * Submit a proposal containing changes to the auth and mint module parameters. -// * Check that the proposal is not executed, since the change to the auth module is not whitelisted. -// * Submit a proposal containing changes *only* to the mint module parameters. -// * Check that the proposal is executed, since the change to the mint module is whitelisted. -// * Submit a proposal containing changes *only* to the auth module parameters. -// * Check that again, the proposal is not executed, since the change to the auth module is not whitelisted. -func (s *ConsumerDemocracyTestSuite) TestDemocracyGovernanceWhitelisting() { - govKeeper := s.consumerApp.GetTestGovKeeper() - params, err := govKeeper.Params.Get(s.consumerCtx()) - s.Require().NoError(err) - - stakingKeeper := s.consumerApp.GetTestStakingKeeper() - bankKeeper := s.consumerApp.GetTestBankKeeper() - accountKeeper := s.consumerApp.GetTestAccountKeeper() - mintKeeper := s.consumerApp.GetTestMintKeeper() - newAuthParamValue := uint64(128) - newMintParamValue := math.LegacyNewDecWithPrec(1, 1) // "0.100000000000000000" - votingAccounts := s.consumerChain.SenderAccounts - bondDenom, err := stakingKeeper.BondDenom(s.consumerCtx()) - s.Require().NoError(err) - depositAmount := params.MinDeposit - duration := (3 * time.Second) - params.VotingPeriod = &duration - err = govKeeper.Params.Set(s.consumerCtx(), params) - s.Assert().NoError(err) - proposer := s.consumerChain.SenderAccount - s.consumerChain.NextBlock() - votersOldBalances := getAccountsBalances(s.consumerCtx(), bankKeeper, bondDenom, votingAccounts) - - // submit proposal with forbidden and allowed changes - mintParams, err := mintKeeper.Params.Get(s.consumerCtx()) - s.Require().NoError(err) - mintParams.InflationMax = newMintParamValue - msg_1 := &minttypes.MsgUpdateParams{ - Authority: authtypes.NewModuleAddress(govtypes.ModuleName).String(), - Params: mintParams, - } - authParams := accountKeeper.GetParams(s.consumerCtx()) - authParams.MaxMemoCharacters = newAuthParamValue - msg_2 := &authtypes.MsgUpdateParams{ - Authority: authtypes.NewModuleAddress(govtypes.ModuleName).String(), - Params: authParams, - } - err = submitProposalWithDepositAndVote(govKeeper, s.consumerCtx(), []sdk.Msg{msg_1, msg_2}, votingAccounts, proposer.GetAddress(), depositAmount) - s.Assert().NoError(err) - // set current header time to be equal or later than voting end time in order to process proposal from active queue, - // once the proposal is added to the chain - s.consumerChain.CurrentHeader.Time = s.consumerChain.CurrentHeader.Time.Add(*params.VotingPeriod) - // at this moment, proposal is added, but not yet executed. we are saving old param values for comparison - oldAuthParamValue := accountKeeper.GetParams(s.consumerCtx()).MaxMemoCharacters - oldMintParams, err := mintKeeper.Params.Get(s.consumerCtx()) - s.Require().NoError(err) - oldMintParamValue := oldMintParams.InflationMax - s.consumerChain.NextBlock() - // at this moment, proposal is executed or deleted if forbidden - currentAuthParamValue := accountKeeper.GetParams(s.consumerCtx()).MaxMemoCharacters - currentMintParam, err := mintKeeper.Params.Get(s.consumerCtx()) - s.Require().NoError(err) - currentMintParamValue := currentMintParam.InflationMax - // check that parameters are not changed, since the proposal contained both forbidden and allowed changes - s.Assert().Equal(oldAuthParamValue, currentAuthParamValue) - s.Assert().NotEqual(newAuthParamValue, currentAuthParamValue) - s.Assert().Equal(oldMintParamValue, currentMintParamValue) - s.Assert().NotEqual(newMintParamValue, currentMintParamValue) - // deposit is refunded - s.Assert().Equal(votersOldBalances, getAccountsBalances(s.consumerCtx(), bankKeeper, bondDenom, votingAccounts)) - - // submit proposal with allowed changes - err = submitProposalWithDepositAndVote(govKeeper, s.consumerCtx(), []sdk.Msg{msg_1}, votingAccounts, proposer.GetAddress(), depositAmount) - s.Assert().NoError(err) - s.consumerChain.CurrentHeader.Time = s.consumerChain.CurrentHeader.Time.Add(*params.VotingPeriod) - oldMintParam, err := mintKeeper.Params.Get(s.consumerCtx()) - s.Require().NoError(err) - oldMintParamValue = oldMintParam.InflationMax - s.consumerChain.NextBlock() - currentMintParam, err = mintKeeper.Params.Get(s.consumerCtx()) - s.Require().NoError(err) - - currentMintParamValue = currentMintParam.InflationMax - // check that parameters are changed, since the proposal contained only allowed changes - s.Assert().Equal(newMintParamValue, currentMintParamValue) - s.Assert().NotEqual(oldMintParamValue, currentMintParamValue) - // deposit is refunded - s.Assert().Equal(votersOldBalances, getAccountsBalances(s.consumerCtx(), bankKeeper, bondDenom, votingAccounts)) - - // submit proposal with forbidden changes - - err = submitProposalWithDepositAndVote(govKeeper, s.consumerCtx(), []sdk.Msg{msg_2}, votingAccounts, proposer.GetAddress(), depositAmount) - s.Assert().NoError(err) - s.consumerChain.CurrentHeader.Time = s.consumerChain.CurrentHeader.Time.Add(*params.VotingPeriod) - s.consumerChain.NextBlock() - oldAuthParamValue = accountKeeper.GetParams(s.consumerCtx()).MaxMemoCharacters - s.consumerChain.NextBlock() - currentAuthParamValue = accountKeeper.GetParams(s.consumerCtx()).MaxMemoCharacters - // check that parameters are not changed, since the proposal contained forbidden changes - s.Assert().Equal(oldAuthParamValue, currentAuthParamValue) - s.Assert().NotEqual(newAuthParamValue, currentAuthParamValue) - // deposit is refunded - s.Assert().Equal(votersOldBalances, getAccountsBalances(s.consumerCtx(), bankKeeper, bondDenom, votingAccounts)) -} - // TestDemocracyMsgUpdateParams checks that the consumer parameters can be updated through a governance proposal. // @Long Description@ // * Set up a democracy consumer chain. diff --git a/testutil/integration/debug_test.go b/testutil/integration/debug_test.go index 1f422b2a91..6092298a15 100644 --- a/testutil/integration/debug_test.go +++ b/testutil/integration/debug_test.go @@ -57,10 +57,6 @@ func TestDemocracyRewardsDistribution(t *testing.T) { runConsumerDemocracyTestByName(t, "TestDemocracyRewardsDistribution") } -func TestDemocracyGovernanceWhitelisting(t *testing.T) { - runConsumerDemocracyTestByName(t, "TestDemocracyGovernanceWhitelisting") -} - func TestDemocracyMsgUpdateParams(t *testing.T) { runConsumerDemocracyTestByName(t, "TestDemocracyMsgUpdateParams") } diff --git a/x/ccv/democracy/governance/doc.go b/x/ccv/democracy/governance/doc.go deleted file mode 100644 index 3b920e95e6..0000000000 --- a/x/ccv/democracy/governance/doc.go +++ /dev/null @@ -1,11 +0,0 @@ -/* -Package governance defines a "wrapper" module around the Cosmos SDK's native -x/governance module. In other words, it provides the exact same functionality as -the native module in that it simply embeds the native module. However, it -overrides `EndBlock` core method to remove forbidden governance proposals from the -storage(s) before the original `EndBlock` method from the native module is called. - -The consumer chain should utilize the x/ccv/democracy/governance module to perform democratic -actions such as participating and voting within the chain's governance system. -*/ -package governance diff --git a/x/ccv/democracy/governance/module.go b/x/ccv/democracy/governance/module.go deleted file mode 100644 index 1ac1da886a..0000000000 --- a/x/ccv/democracy/governance/module.go +++ /dev/null @@ -1,165 +0,0 @@ -package governance - -import ( - "context" - "fmt" - "time" - - "cosmossdk.io/collections" - "cosmossdk.io/core/appmodule" - - "github.com/cosmos/cosmos-sdk/codec" - sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/cosmos/cosmos-sdk/types/module" - gov "github.com/cosmos/cosmos-sdk/x/gov" - govkeeper "github.com/cosmos/cosmos-sdk/x/gov/keeper" - govtypes "github.com/cosmos/cosmos-sdk/x/gov/types" - govv1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1" - govv1beta1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1beta1" -) - -const ( - AttributeValueProposalForbidden = "proposal_forbidden" -) - -var ( - _ module.AppModule = AppModule{} - _ module.AppModuleSimulation = AppModule{} - - _ appmodule.HasEndBlocker = AppModule{} -) - -// AppModule embeds the Cosmos SDK's x/governance AppModule -type AppModule struct { - // embed the Cosmos SDK's x/governance AppModule - gov.AppModule - - keeper govkeeper.Keeper - isLegacyProposalWhitelisted func(govv1beta1.Content) bool - isModuleWhiteList func(string) bool -} - -type ParamChangeKey struct { - MsgType, Key string -} - -// NewAppModule creates a new AppModule object using the native x/governance module AppModule constructor. -func NewAppModule(cdc codec.Codec, - keeper govkeeper.Keeper, - ak govtypes.AccountKeeper, - bk govtypes.BankKeeper, - isProposalWhitelisted func(govv1beta1.Content) bool, - ss govtypes.ParamSubspace, - isModuleWhiteList func(string) bool, -) AppModule { - govAppModule := gov.NewAppModule(cdc, &keeper, ak, bk, ss) - return AppModule{ - AppModule: govAppModule, - keeper: keeper, - isLegacyProposalWhitelisted: isProposalWhitelisted, - isModuleWhiteList: isModuleWhiteList, - } -} - -func (am AppModule) EndBlock(c context.Context) error { - ctx := sdk.UnwrapSDKContext(c) - rng := collections.NewPrefixUntilPairRange[time.Time, uint64](ctx.BlockTime()) - keeper := am.keeper - // if there are forbidden proposals in active proposals queue, refund deposit, delete votes for that proposal - // and delete proposal from all storages - err := am.keeper.ActiveProposalsQueue.Walk(ctx, rng, func(key collections.Pair[time.Time, uint64], _ uint64) (bool, error) { - proposal, err := keeper.Proposals.Get(ctx, key.K2()) - if err != nil { - return false, err - } - deleteForbiddenProposal(ctx, am, proposal) - return false, nil - }) - if err != nil { - return err - } - return am.AppModule.EndBlock(ctx) -} - -// isProposalWhitelisted checks whether a proposal is whitelisted -func isProposalWhitelisted(ctx sdk.Context, am AppModule, proposal govv1.Proposal) bool { - messages := proposal.GetMessages() - - // iterate over all the proposal messages - for _, message := range messages { - sdkMsg, isLegacyProposal := message.GetCachedValue().(*govv1.MsgExecLegacyContent) - if isLegacyProposal { - // legacy gov proposal content - content, err := govv1.LegacyContentFromMessage(sdkMsg) - if err != nil { - continue - } - if !am.isLegacyProposalWhitelisted(content) { - // not whitelisted - return false - } - // not legacy gov proposal content - } else if !am.isModuleWhiteList(message.TypeUrl) { - // not whitelisted - return false - } - } - return true -} - -// deleteForbiddenProposal removes proposals that are not whitelisted -func deleteForbiddenProposal(ctx sdk.Context, am AppModule, proposal govv1.Proposal) { - if isProposalWhitelisted(ctx, am, proposal) { - return - } - - logger := am.keeper.Logger(ctx) - - // delete the votes related to the proposal calling Tally - // Tally's return result won't be used in decision if the tokens will be burned or refunded (they are always refunded), but - // this function needs to be called to delete the votes related to the given proposal, since the deleteVote function is - // private and cannot be called directly from the overridden app module - _, _, _, err := am.keeper.Tally(ctx, proposal) - if err != nil { - logger.Warn( - "failed to tally disallowed proposal", - "proposal", proposal.Id, - "title", proposal.GetTitle(), - "total_deposit", proposal.TotalDeposit) - return - } - - err = am.keeper.DeleteProposal(ctx, proposal.Id) - if err != nil { - logger.Warn( - "failed to delete disallowed proposal", - "proposal", proposal.Id, - "title", proposal.GetTitle(), - "total_deposit", proposal.TotalDeposit) - return - } - - err = am.keeper.RefundAndDeleteDeposits(ctx, proposal.Id) - if err != nil { - logger.Warn( - "failed to refund deposits for disallowed proposal", - "proposal", proposal.Id, - "title", proposal.GetTitle(), - "total_deposit", proposal.TotalDeposit) - return - } - - ctx.EventManager().EmitEvent( - sdk.NewEvent( - govtypes.EventTypeActiveProposal, - sdk.NewAttribute(govtypes.AttributeKeyProposalID, fmt.Sprintf("%d", proposal.Id)), - sdk.NewAttribute(govtypes.AttributeKeyProposalResult, AttributeValueProposalForbidden), - ), - ) - - logger.Info( - "proposal is not allowed; deleted", - "proposal", proposal.Id, - "title", proposal.GetTitle(), - "total_deposit", proposal.TotalDeposit) -}