From 0c217c9a0c53c9c9653b3313e8b95c225c0f2305 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Wed, 15 Jan 2025 16:32:19 +0100 Subject: [PATCH] rewrite gov ante handler (ref celestiaorg/celestia-app/issues/4202) --- app/ante/ante.go | 9 ++--- app/ante/gov.go | 82 ++++++++++++++++++++++++++++++++++------------ app/app.go | 11 +++++++ test/pfm/simapp.go | 6 ++-- 4 files changed, 79 insertions(+), 29 deletions(-) diff --git a/app/ante/ante.go b/app/ante/ante.go index 889d9eeb86..082a37d87d 100644 --- a/app/ante/ante.go +++ b/app/ante/ante.go @@ -26,6 +26,7 @@ func NewAnteHandler( channelKeeper *ibckeeper.Keeper, paramKeeper paramkeeper.Keeper, msgVersioningGateKeeper *MsgVersioningGateKeeper, + forbiddenGovUpdateParams map[string][]string, ) sdk.AnteHandler { return sdk.ChainAnteDecorators( // Wraps the panic with the string format of the transaction @@ -69,14 +70,14 @@ func NewAnteHandler( blobante.NewMinGasPFBDecorator(blobKeeper, consensusKeeper), // Ensure that the tx's total blob size is <= the max blob size. // Only applies to app version == 1. - blobante.NewMaxTotalBlobSizeDecorator(blobKeeper), + blobante.NewMaxTotalBlobSizeDecorator(blobKeeper, consensusKeeper), // Ensure that the blob shares occupied by the tx <= the max shares // available to blob data in a data square. Only applies to app version // >= 2. - blobante.NewBlobShareDecorator(blobKeeper), + blobante.NewBlobShareDecorator(blobKeeper, consensusKeeper), // Ensure that tx's with a MsgSubmitProposal have at least one proposal - // message. - NewGovProposalDecorator(), + // message. Additionally ensure that the proposals do not contain any + NewGovProposalDecorator(forbiddenGovUpdateParams), // Ensure that the tx is not an IBC packet or update message that has already been processed. ibcante.NewRedundantRelayDecorator(channelKeeper), ) diff --git a/app/ante/gov.go b/app/ante/gov.go index e7de8f01cb..81b207a33d 100644 --- a/app/ante/gov.go +++ b/app/ante/gov.go @@ -5,15 +5,21 @@ import ( "cosmossdk.io/x/authz" gov "cosmossdk.io/x/gov/types" govv1 "cosmossdk.io/x/gov/types/v1" + "cosmossdk.io/x/params/types/proposal" sdk "github.com/cosmos/cosmos-sdk/types" + gogoproto "github.com/cosmos/gogoproto/proto" + gogoany "github.com/cosmos/gogoproto/types/any" ) // GovProposalDecorator ensures that a tx with a MsgSubmitProposal has at least one message in the proposal. // Additionally it replace the x/paramfilter module that existed in v3 and earlier versions. -type GovProposalDecorator struct{} +type GovProposalDecorator struct { + // forbiddenGovUpdateParams is a map of type_url to a list of parameter fiels that cannot be changed via governance. + forbiddenGovUpdateParams map[string][]string +} -func NewGovProposalDecorator() GovProposalDecorator { - return GovProposalDecorator{} +func NewGovProposalDecorator(forbiddenParams map[string][]string) GovProposalDecorator { + return GovProposalDecorator{forbiddenParams} } // AnteHandle implements the AnteHandler interface. @@ -25,12 +31,16 @@ func (d GovProposalDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bo if len(proposal.Messages) == 0 { return ctx, errors.Wrapf(gov.ErrNoProposalMsgs, "must include at least one message in proposal") } + + if err := d.checkNestedMsgs(proposal.Messages); err != nil { + return ctx, err + } } - // we need to check if a gov proposal wasn't contains in a authz message + // we need to check if a gov proposal wasn't contained in a authz message if msgExec, ok := m.(*authz.MsgExec); ok { - for _, msg := range msgExec.Msgs { - _ = msg + if err := d.checkNestedMsgs(msgExec.Msgs); err != nil { + return ctx, err } } } @@ -38,18 +48,48 @@ func (d GovProposalDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bo return next(ctx, tx, simulate) } -// TODO: To be moved to antehandler -// BlockedParams returns the params that require a hardfork to change, and -// cannot be changed via governance. -// func (app *App) BlockedParams() [][2]string { -// return [][2]string{ -// // bank.SendEnabled -// {banktypes.ModuleName, string(banktypes.KeySendEnabled)}, -// // staking.UnbondingTime -// {stakingtypes.ModuleName, string(stakingtypes.KeyUnbondingTime)}, -// // staking.BondDenom -// {stakingtypes.ModuleName, string(stakingtypes.KeyBondDenom)}, -// // consensus.validator.PubKeyTypes -// {baseapp.Paramspace, string(baseapp.ParamStoreKeyValidatorParams)}, -// } -// } +func (d GovProposalDecorator) checkNestedMsgs(msgs []*gogoany.Any) error { + for _, msg := range msgs { + if msg.TypeUrl == "/"+gogoproto.MessageName((*authz.MsgExec)(nil)) { + // unmarshal the authz.MsgExec and check nested messages + exec := &authz.MsgExec{} + // todo unmarshal + + if err := d.checkNestedMsgs(exec.Msgs); err != nil { + return err + } + } + + if msg.TypeUrl == "/"+gogoproto.MessageName((*govv1.MsgSubmitProposal)(nil)) { + // unmarshal the gov.MsgSubmitProposal and check nested messages + proposal := &govv1.MsgSubmitProposal{} + // todo unmarshal + + if len(proposal.Messages) == 0 { + return errors.Wrapf(gov.ErrNoProposalMsgs, "must include at least one message in proposal") + } + + if err := d.checkNestedMsgs(proposal.Messages); err != nil { + return err + } + } + + forbiddenParams, ok := d.forbiddenGovUpdateParams[msg.TypeUrl] + if !ok { + continue + } + + if hasForbiddenParams(msg, msg.TypeUrl, forbiddenParams) { + return errors.Wrapf(proposal.ErrSettingParameter, "cannot update %s parameters via governance", msg.TypeUrl) + } + } + + return nil +} + +func hasForbiddenParams(msg sdk.Msg, typeURL string, forbiddenParams []string) bool { + // unmarshal msg to go struct + // check if any forbidden param is present and different from the default value + + return false +} diff --git a/app/app.go b/app/app.go index 4e2325a115..97eeffafdf 100644 --- a/app/app.go +++ b/app/app.go @@ -82,6 +82,7 @@ import ( authkeeper "github.com/cosmos/cosmos-sdk/x/auth/keeper" authtx "github.com/cosmos/cosmos-sdk/x/auth/tx" authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" + gogoproto "github.com/cosmos/gogoproto/proto" icacontrollerkeeper "github.com/cosmos/ibc-go/v9/modules/apps/27-interchain-accounts/controller/keeper" icacontrollertypes "github.com/cosmos/ibc-go/v9/modules/apps/27-interchain-accounts/controller/types" ibcfeekeeper "github.com/cosmos/ibc-go/v9/modules/apps/29-fee/keeper" @@ -537,6 +538,15 @@ func New( app.SetInitChainer(app.InitChainer) app.SetBeginBlocker(app.BeginBlocker) app.SetEndBlocker(app.EndBlocker) + + // blockedParams returns the params that require a hardfork to change, and + // cannot be changed via governance. + blockedParams := map[string][]string{ + gogoproto.MessageName(&banktypes.MsgUpdateParams{}): []string{"send_enabled"}, + gogoproto.MessageName(&stakingtypes.MsgUpdateParams{}): []string{"params.bond_denom", "params.unbonding_time"}, + gogoproto.MessageName(&consensustypes.MsgUpdateParams{}): []string{"validator"}, + } + app.SetAnteHandler(ante.NewAnteHandler( app.AuthKeeper, app.AccountsKeeper, @@ -549,6 +559,7 @@ func New( app.IBCKeeper, app.ParamsKeeper, app.MsgGateKeeper, + blockedParams, )) app.SetPostHandler(posthandler.New()) diff --git a/test/pfm/simapp.go b/test/pfm/simapp.go index c27990af72..8efa4775a3 100644 --- a/test/pfm/simapp.go +++ b/test/pfm/simapp.go @@ -50,7 +50,6 @@ import ( "github.com/cosmos/cosmos-sdk/baseapp" "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/client/grpc/cmtservice" - "github.com/cosmos/cosmos-sdk/client/grpc/tmservice" "github.com/cosmos/cosmos-sdk/codec" codectestutil "github.com/cosmos/cosmos-sdk/codec/testutil" "github.com/cosmos/cosmos-sdk/codec/types" @@ -70,7 +69,6 @@ import ( vestingtypes "github.com/cosmos/cosmos-sdk/x/auth/vesting/types" "github.com/cosmos/cosmos-sdk/x/genutil" genutiltypes "github.com/cosmos/cosmos-sdk/x/genutil/types" - "github.com/cosmos/ibc-apps/middleware/packet-forward-middleware/v7/packetforward" "github.com/gorilla/mux" "github.com/rakyll/statik/fs" abci "github.com/tendermint/tendermint/abci/types" @@ -153,7 +151,7 @@ var ( ibcmock.AppModuleBasic{}, authzmodule.AppModuleBasic{}, vesting.AppModuleBasic{}, - packetforward.AppModuleBasic{}, + // packetforward.AppModuleBasic{}, ) // module account permissions @@ -709,7 +707,7 @@ func (app *SimApp) RegisterTxService(clientCtx client.Context) { // RegisterTendermintService implements the Application.RegisterTendermintService method. func (app *SimApp) RegisterTendermintService(clientCtx client.Context) { - tmservice.RegisterTendermintService( + cmtservice.RegisterTendermintService( clientCtx, app.BaseApp.GRPCQueryRouter(), app.interfaceRegistry,