Skip to content

Commit

Permalink
rewrite gov ante handler (ref celestiaorg/issues/4202)
Browse files Browse the repository at this point in the history
  • Loading branch information
julienrbrt committed Jan 15, 2025
1 parent 3cc1a76 commit 0c217c9
Show file tree
Hide file tree
Showing 4 changed files with 79 additions and 29 deletions.
9 changes: 5 additions & 4 deletions app/ante/ante.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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),
)
Expand Down
82 changes: 61 additions & 21 deletions app/ante/gov.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -25,31 +31,65 @@ 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
}
}
}

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
}
11 changes: 11 additions & 0 deletions app/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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,
Expand All @@ -549,6 +559,7 @@ func New(
app.IBCKeeper,
app.ParamsKeeper,
app.MsgGateKeeper,
blockedParams,
))
app.SetPostHandler(posthandler.New())

Expand Down
6 changes: 2 additions & 4 deletions test/pfm/simapp.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
Expand Down Expand Up @@ -153,7 +151,7 @@ var (
ibcmock.AppModuleBasic{},
authzmodule.AppModuleBasic{},
vesting.AppModuleBasic{},
packetforward.AppModuleBasic{},
// packetforward.AppModuleBasic{},
)

// module account permissions
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit 0c217c9

Please sign in to comment.