From d003f8841e51486d631eed252450824cfd7b64d9 Mon Sep 17 00:00:00 2001 From: Thomas Bruyelle Date: Wed, 19 Jun 2024 17:58:19 +0200 Subject: [PATCH] fix(gov): cli command for params and upgrade proposals (#42) ## Description Closes: #39 The fix just copies the 3 command handlers for - params.ParamChangeProposal - upgrade.SoftwareUpgradeProposal - upgrade.CancelSoftwareUpgradeProposal and replace the MsgSubmitProposal package with the one from govgen. As a result WrapPropposalHandler is becomes obsolete, we only need to wrap the ProposalRESTHandler. The fix has been tested with the `json-signer` test framework, see https://github.com/tbruyelle/json-signer/blob/master/testdata/govgenV1/gov-submit.txtar#L13-L50 --- ### Author Checklist *All items are required. Please add a note to the item if the item is not applicable and please add links to any relevant follow up issues.* I have... * [x] Included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title * [ ] Added `!` to the type prefix if API, client, or state breaking change (i.e., requires minor or major version bump) * [x] Targeted the correct branch (see [PR Targeting](https://github.com/atomone-hub/govgen/blob/main/CONTRIBUTING.md#pr-targeting)) * [x] Provided a link to the relevant issue or specification * [x] Followed the guidelines for [building SDK modules](https://github.com/cosmos/cosmos-sdk/blob/main/docs/docs/building-modules) * [ ] Included the necessary unit and integration [tests](https://github.com/atomone-hub/govgen/blob/main/CONTRIBUTING.md#testing) * [x] Added a changelog entry in `.changelog` (for details, see [contributing guidelines](../../CONTRIBUTING.md#changelog)) * [ ] Included comments for [documenting Go code](https://blog.golang.org/godoc) * [ ] Updated the relevant documentation or specification * [ ] Reviewed "Files changed" and left comments if necessary * [ ] Confirmed all CI checks have passed ### Reviewers Checklist *All items are required. Please add a note if the item is not applicable and please add your handle next to the items reviewed if you only reviewed selected items.* I have... * [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title * [ ] confirmed `!` in the type prefix if API or client breaking change * [ ] confirmed all author checklist items have been addressed * [ ] reviewed state machine logic * [ ] reviewed API design and naming * [ ] reviewed documentation is accurate * [ ] reviewed tests and test coverage --------- Co-authored-by: Giuseppe Natale <12249307+giunatale@users.noreply.github.com> --- CHANGELOG.md | 1 + app/gov_handlers.go | 250 +++++++++++++++++++++++++++++++ app/modules.go | 8 +- go.mod | 2 +- x/gov/client/proposal_handler.go | 19 +-- x/gov/module.go | 9 +- 6 files changed, 264 insertions(+), 25 deletions(-) create mode 100644 app/gov_handlers.go diff --git a/CHANGELOG.md b/CHANGELOG.md index c17b143..57d52c4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ ### BUG FIXES * Fix duplicate amino path in gov module ([#44](https://github.com/atomone-hub/govgen/pull/44)) +* Fix gov cli for params and upgrade proposals ([#42](https://github.com/atomone-hub/govgen/pull/42)) * Ensure correct version of `protoc-gen-gocosmos` is installed ([#41](https://github.com/atomone-hub/govgen/pull/41)). ### DEPENDENCIES diff --git a/app/gov_handlers.go b/app/gov_handlers.go new file mode 100644 index 0000000..8cfd561 --- /dev/null +++ b/app/gov_handlers.go @@ -0,0 +1,250 @@ +package govgen + +import ( + "fmt" + "strings" + + "github.com/spf13/cobra" + + "github.com/cosmos/cosmos-sdk/client" + "github.com/cosmos/cosmos-sdk/client/tx" + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/version" + paramsrest "github.com/cosmos/cosmos-sdk/x/params/client/rest" + paramscutils "github.com/cosmos/cosmos-sdk/x/params/client/utils" + paramsproposal "github.com/cosmos/cosmos-sdk/x/params/types/proposal" + upgradecli "github.com/cosmos/cosmos-sdk/x/upgrade/client/cli" + upgraderest "github.com/cosmos/cosmos-sdk/x/upgrade/client/rest" + upgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/types" + + govclient "github.com/atomone-hub/govgen/x/gov/client" + "github.com/atomone-hub/govgen/x/gov/client/cli" + govtypes "github.com/atomone-hub/govgen/x/gov/types" +) + +var ( + paramsChangeProposalHandler = govclient.NewProposalHandler( + newSubmitParamChangeProposalTxCmd, + govclient.WrapPropposalRESTHandler(paramsrest.ProposalRESTHandler), + ) + upgradeProposalHandler = govclient.NewProposalHandler( + newCmdSubmitUpgradeProposal, + govclient.WrapPropposalRESTHandler(upgraderest.ProposalRESTHandler), + ) + cancelUpgradeProposalHandler = govclient.NewProposalHandler( + newCmdSubmitCancelUpgradeProposal, + govclient.WrapPropposalRESTHandler(upgraderest.ProposalRESTHandler), + ) +) + +// newSubmitParamChangeProposalTxCmd returns a CLI command handler for creating +// a parameter change proposal governance transaction. +// +// NOTE: copy of x/params/client.newSubmitParamChangeProposalTxCmd() except +// that it creates a govgen.gov.MsgSubmitProposal instead of a +// cosmos.gov.MsgSubmitProposal. +func newSubmitParamChangeProposalTxCmd() *cobra.Command { + return &cobra.Command{ + Use: "param-change [proposal-file]", + Args: cobra.ExactArgs(1), + Short: "Submit a parameter change proposal", + Long: strings.TrimSpace( + fmt.Sprintf(`Submit a parameter proposal along with an initial deposit. +The proposal details must be supplied via a JSON file. For values that contains +objects, only non-empty fields will be updated. + +IMPORTANT: Currently parameter changes are evaluated but not validated, so it is +very important that any "value" change is valid (ie. correct type and within bounds) +for its respective parameter, eg. "MaxValidators" should be an integer and not a decimal. + +Proper vetting of a parameter change proposal should prevent this from happening +(no deposits should occur during the governance process), but it should be noted +regardless. + +Example: +$ %s tx gov submit-proposal param-change --from= + +Where proposal.json contains: + +{ + "title": "Staking Param Change", + "description": "Update max validators", + "changes": [ + { + "subspace": "staking", + "key": "MaxValidators", + "value": 105 + } + ], + "deposit": "1000stake" +} +`, + version.AppName, + ), + ), + RunE: func(cmd *cobra.Command, args []string) error { + clientCtx, err := client.GetClientTxContext(cmd) + if err != nil { + return err + } + proposal, err := paramscutils.ParseParamChangeProposalJSON(clientCtx.LegacyAmino, args[0]) + if err != nil { + return err + } + + from := clientCtx.GetFromAddress() + content := paramsproposal.NewParameterChangeProposal( + proposal.Title, proposal.Description, proposal.Changes.ToParamChanges(), + ) + + deposit, err := sdk.ParseCoinsNormalized(proposal.Deposit) + if err != nil { + return err + } + + msg, err := govtypes.NewMsgSubmitProposal(content, deposit, from) + if err != nil { + return err + } + + return tx.GenerateOrBroadcastTxCLI(clientCtx, cmd.Flags(), msg) + }, + } +} + +// newCmdSubmitUpgradeProposal implements a command handler for submitting a software upgrade proposal transaction. +// +// NOTE: copy of x/upgrade/client.NewCmdSubmitUpgradeProposal() except +// that it creates a govgen.gov.MsgSubmitProposal instead of a +// cosmos.gov.MsgSubmitProposal. +func newCmdSubmitUpgradeProposal() *cobra.Command { + cmd := &cobra.Command{ + Use: "software-upgrade [name] (--upgrade-height [height]) (--upgrade-info [info]) [flags]", + Args: cobra.ExactArgs(1), + Short: "Submit a software upgrade proposal", + Long: "Submit a software upgrade along with an initial deposit.\n" + + "Please specify a unique name and height for the upgrade to take effect.\n" + + "You may include info to reference a binary download link, in a format compatible with: https://github.com/cosmos/cosmos-sdk/tree/master/cosmovisor", + RunE: func(cmd *cobra.Command, args []string) error { + clientCtx, err := client.GetClientTxContext(cmd) + if err != nil { + return err + } + name := args[0] + content, err := parseArgsToContent(cmd, name) + if err != nil { + return err + } + + from := clientCtx.GetFromAddress() + + depositStr, err := cmd.Flags().GetString(cli.FlagDeposit) + if err != nil { + return err + } + deposit, err := sdk.ParseCoinsNormalized(depositStr) + if err != nil { + return err + } + + msg, err := govtypes.NewMsgSubmitProposal(content, deposit, from) + if err != nil { + return err + } + + return tx.GenerateOrBroadcastTxCLI(clientCtx, cmd.Flags(), msg) + }, + } + + cmd.Flags().String(cli.FlagTitle, "", "title of proposal") + cmd.Flags().String(cli.FlagDescription, "", "description of proposal") + cmd.Flags().String(cli.FlagDeposit, "", "deposit of proposal") + cmd.Flags().Int64(upgradecli.FlagUpgradeHeight, 0, "The height at which the upgrade must happen") + cmd.Flags().String(upgradecli.FlagUpgradeInfo, "", "Optional info for the planned upgrade such as commit hash, etc.") + + return cmd +} + +// newCmdSubmitCancelUpgradeProposal implements a command handler for submitting a software upgrade cancel proposal transaction. +// +// NOTE: copy of x/upgrade/client.NewCmdSubmitCancelUpgradeProposal() except +// that it creates a govgen.gov.MsgSubmitProposal instead of a +// cosmos.gov.MsgSubmitProposal. +func newCmdSubmitCancelUpgradeProposal() *cobra.Command { + cmd := &cobra.Command{ + Use: "cancel-software-upgrade [flags]", + Args: cobra.ExactArgs(0), + Short: "Cancel the current software upgrade proposal", + Long: "Cancel a software upgrade along with an initial deposit.", + RunE: func(cmd *cobra.Command, args []string) error { + clientCtx, err := client.GetClientTxContext(cmd) + if err != nil { + return err + } + from := clientCtx.GetFromAddress() + + depositStr, err := cmd.Flags().GetString(cli.FlagDeposit) + if err != nil { + return err + } + + deposit, err := sdk.ParseCoinsNormalized(depositStr) + if err != nil { + return err + } + + title, err := cmd.Flags().GetString(cli.FlagTitle) + if err != nil { + return err + } + + description, err := cmd.Flags().GetString(cli.FlagDescription) + if err != nil { + return err + } + + content := upgradetypes.NewCancelSoftwareUpgradeProposal(title, description) + + msg, err := govtypes.NewMsgSubmitProposal(content, deposit, from) + if err != nil { + return err + } + + return tx.GenerateOrBroadcastTxCLI(clientCtx, cmd.Flags(), msg) + }, + } + + cmd.Flags().String(cli.FlagTitle, "", "title of proposal") + cmd.Flags().String(cli.FlagDescription, "", "description of proposal") + cmd.Flags().String(cli.FlagDeposit, "", "deposit of proposal") + cmd.MarkFlagRequired(cli.FlagTitle) //nolint:errcheck + cmd.MarkFlagRequired(cli.FlagDescription) //nolint:errcheck + + return cmd +} + +func parseArgsToContent(cmd *cobra.Command, name string) (govtypes.Content, error) { + title, err := cmd.Flags().GetString(cli.FlagTitle) + if err != nil { + return nil, err + } + + description, err := cmd.Flags().GetString(cli.FlagDescription) + if err != nil { + return nil, err + } + + height, err := cmd.Flags().GetInt64(upgradecli.FlagUpgradeHeight) + if err != nil { + return nil, err + } + + info, err := cmd.Flags().GetString(upgradecli.FlagUpgradeInfo) + if err != nil { + return nil, err + } + + plan := upgradetypes.Plan{Name: name, Height: height, Info: info} + content := upgradetypes.NewSoftwareUpgradeProposal(title, description, plan) + return content, nil +} diff --git a/app/modules.go b/app/modules.go index 23993a9..c5ee5bf 100644 --- a/app/modules.go +++ b/app/modules.go @@ -26,14 +26,12 @@ import ( "github.com/cosmos/cosmos-sdk/x/mint" minttypes "github.com/cosmos/cosmos-sdk/x/mint/types" "github.com/cosmos/cosmos-sdk/x/params" - paramsclient "github.com/cosmos/cosmos-sdk/x/params/client" paramstypes "github.com/cosmos/cosmos-sdk/x/params/types" "github.com/cosmos/cosmos-sdk/x/slashing" slashingtypes "github.com/cosmos/cosmos-sdk/x/slashing/types" "github.com/cosmos/cosmos-sdk/x/staking" stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" "github.com/cosmos/cosmos-sdk/x/upgrade" - upgradeclient "github.com/cosmos/cosmos-sdk/x/upgrade/client" upgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/types" govgenappparams "github.com/atomone-hub/govgen/app/params" @@ -62,9 +60,9 @@ var ModuleBasics = module.NewBasicManager( mint.AppModuleBasic{}, distr.AppModuleBasic{}, gov.NewAppModuleBasic( - paramsclient.ProposalHandler, - upgradeclient.ProposalHandler, - upgradeclient.CancelProposalHandler, + paramsChangeProposalHandler, + upgradeProposalHandler, + cancelUpgradeProposalHandler, ), params.AppModuleBasic{}, crisis.AppModuleBasic{}, diff --git a/go.mod b/go.mod index 5d7584f..66322a6 100644 --- a/go.mod +++ b/go.mod @@ -19,6 +19,7 @@ require ( github.com/stretchr/testify v1.8.4 github.com/tendermint/tendermint v0.34.27 github.com/tendermint/tm-db v0.6.7 + golang.org/x/exp v0.0.0-20230905200255-921286631fa9 google.golang.org/genproto/googleapis/api v0.0.0-20230913181813-007df8e322eb google.golang.org/grpc v1.58.2 google.golang.org/protobuf v1.31.0 @@ -137,7 +138,6 @@ require ( go.uber.org/atomic v1.10.0 // indirect go.uber.org/multierr v1.9.0 // indirect golang.org/x/crypto v0.14.0 // indirect - golang.org/x/exp v0.0.0-20230905200255-921286631fa9 // indirect golang.org/x/net v0.17.0 // indirect golang.org/x/sys v0.13.0 // indirect golang.org/x/term v0.13.0 // indirect diff --git a/x/gov/client/proposal_handler.go b/x/gov/client/proposal_handler.go index 1d5410f..71b7246 100644 --- a/x/gov/client/proposal_handler.go +++ b/x/gov/client/proposal_handler.go @@ -29,17 +29,12 @@ func NewProposalHandler(cliHandler CLIHandlerFn, restHandler RESTHandlerFn) Prop } } -func WrapPropposalHandler(h legacyclient.ProposalHandler) ProposalHandler { - return ProposalHandler{ - CLIHandler: func() *cobra.Command { - return h.CLIHandler() - }, - RESTHandler: func(ctx client.Context) rest.ProposalRESTHandler { - handler := h.RESTHandler(ctx) - return rest.ProposalRESTHandler{ - SubRoute: handler.SubRoute, - Handler: handler.Handler, - } - }, +func WrapPropposalRESTHandler(h legacyclient.RESTHandlerFn) RESTHandlerFn { + return func(ctx client.Context) rest.ProposalRESTHandler { + handler := h(ctx) + return rest.ProposalRESTHandler{ + SubRoute: handler.SubRoute, + Handler: handler.Handler, + } } } diff --git a/x/gov/module.go b/x/gov/module.go index a0e2ee0..88669bc 100644 --- a/x/gov/module.go +++ b/x/gov/module.go @@ -19,7 +19,6 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/types/module" simtypes "github.com/cosmos/cosmos-sdk/types/simulation" - sdkgovclient "github.com/cosmos/cosmos-sdk/x/gov/client" govclient "github.com/atomone-hub/govgen/x/gov/client" "github.com/atomone-hub/govgen/x/gov/client/cli" @@ -42,13 +41,9 @@ type AppModuleBasic struct { } // NewAppModuleBasic creates a new AppModuleBasic object -func NewAppModuleBasic(proposalHandlers ...sdkgovclient.ProposalHandler) AppModuleBasic { - var phs []govclient.ProposalHandler - for _, p := range proposalHandlers { - phs = append(phs, govclient.WrapPropposalHandler(p)) - } +func NewAppModuleBasic(proposalHandlers ...govclient.ProposalHandler) AppModuleBasic { return AppModuleBasic{ - proposalHandlers: phs, + proposalHandlers: proposalHandlers, } }