chore: x/upgrade audit changes

CHANGELOG.md

@@ -47,10 +47,9 @@ Ref: https://keepachangelog.com/en/1.0.0/
 * (tx) [#\11533](https://github.com/cosmos/cosmos-sdk/pull/11533) Register [`EIP191`](https://eips.ethereum.org/EIPS/eip-191) as an available `SignMode` for chains to use.
 * (x/genutil) [\#11500](https://github.com/cosmos/cosmos-sdk/pull/11500) Fix GenTx validation and adjust error messages
 * [\#11430](https://github.com/cosmos/cosmos-sdk/pull/11430) Introduce a new `grpc-only` flag, such that when enabled, will start the node in a query-only mode. Note, gRPC MUST be enabled with this flag.
-* (x/upgrade) [\#11116](https://github.com/cosmos/cosmos-sdk/pull/11116) `MsgSoftwareUpgrade` and  has been added to support v1beta2 msgs-based gov proposals.
 * (x/bank) [\#11417](https://github.com/cosmos/cosmos-sdk/pull/11417) Introduce a new `SpendableBalances` gRPC query that retrieves an account's total (paginated) spendable balances.
 * [\#11441](https://github.com/cosmos/cosmos-sdk/pull/11441) Added a new method, `IsLTE`, for `types.Coin`. This method is used to check if a `types.Coin` is less than or equal to another `types.Coin`.
-* (x/upgrade) [\#11116](https://github.com/cosmos/cosmos-sdk/pull/11116) `MsgSoftwareUpgrade` and has been added to support v1beta2 msgs-based gov proposals.
+* (x/upgrade) [\#11116](https://github.com/cosmos/cosmos-sdk/pull/11116) `MsgSoftwareUpgrade` and `MsgCancelUpgrade` have been added to support v1beta2 msgs-based gov proposals.
 * [\#11308](https://github.com/cosmos/cosmos-sdk/pull/11308) Added a mandatory metadata field to Vote in x/gov v1beta2.
 * [\#10977](https://github.com/cosmos/cosmos-sdk/pull/10977) Now every cosmos message protobuf definition must be extended with a ``cosmos.msg.v1.signer`` option to signal the signer fields in a language agnostic way.
 * [\#10710](https://github.com/cosmos/cosmos-sdk/pull/10710) Chain-id shouldn't be required for creating a transaction with both --generate-only and --offline flags.
@@ -160,8 +159,9 @@ Ref: https://keepachangelog.com/en/1.0.0/
 * [\#11274](https://github.com/cosmos/cosmos-sdk/pull/11274) `types/errors.New` now is an alias for `types/errors.Register` and should only be used in initialization code.
 * (authz)[\#11060](https://github.com/cosmos/cosmos-sdk/pull/11060) `authz.NewMsgGrant` `expiration` is now a pointer. When `nil` is used then no expiration will be set (grant won't expire).
 * (x/distribution)[\#11457](https://github.com/cosmos/cosmos-sdk/pull/11457) Add amount field to `distr.MsgWithdrawDelegatorRewardResponse` and `distr.MsgWithdrawValidatorCommissionResponse`.
+* [\#11334](https://github.com/cosmos/cosmos-sdk/pull/11334) Move `x/gov/types/v1beta2` to `x/gov/types/v1`.
 * (x/auth/middleware) [#11413](https://github.com/cosmos/cosmos-sdk/pull/11413) Refactor tx middleware to be extensible on tx fee logic. Merged `MempoolFeeMiddleware` and `TxPriorityMiddleware` functionalities into `DeductFeeMiddleware`, make the logic extensible using the `TxFeeChecker` option, the current fee logic is preserved by the default `checkTxFeeWithValidatorMinGasPrices` implementation. Change `RejectExtensionOptionsMiddleware` to `NewExtensionOptionsMiddleware` which is extensible with the `ExtensionOptionChecker` option. Unpack the tx extension options `Any`s to interface `TxExtensionOptionI`.
-* (migrations) [#1156](https://github.com/cosmos/cosmos-sdk/pull/11556#issuecomment-1091385011) Remove migration code from 0.42 and below. To use previous migrations, checkout previous versions of the cosmos-sdk.
+* (migrations) [#11556](https://github.com/cosmos/cosmos-sdk/pull/11556#issuecomment-1091385011) Remove migration code from 0.42 and below. To use previous migrations, checkout previous versions of the cosmos-sdk.
 
 ### Client Breaking Changes
 
@@ -283,16 +283,17 @@ Ref: https://keepachangelog.com/en/1.0.0/
 * (x/bank) [\#9890](https://github.com/cosmos/cosmos-sdk/pull/9890) Remove duplicate denom from denom metadata key.
 * (x/upgrade) [\#10189](https://github.com/cosmos/cosmos-sdk/issues/10189) Removed potential sources of non-determinism in upgrades
 * [\#10422](https://github.com/cosmos/cosmos-sdk/pull/10422) and [\#10529](https://github.com/cosmos/cosmos-sdk/pull/10529) Add `MinCommissionRate` param to `x/staking` module.
-* [#10763](https://github.com/cosmos/cosmos-sdk/pull/10763) modify the fields in `TallyParams` to use `string` instead of `bytes`
+* (x/gov) [#10763](https://github.com/cosmos/cosmos-sdk/pull/10763) modify the fields in `TallyParams` to use `string` instead of `bytes`
 * [#10770](https://github.com/cosmos/cosmos-sdk/pull/10770) revert tx when block gas limit exceeded
-* [\#10868](https://github.com/cosmos/cosmos-sdk/pull/10868) Bump gov to v1beta2. Both v1beta1 and v1beta2 queries and Msgs are accepted.
+* (x/gov) [\#10868](https://github.com/cosmos/cosmos-sdk/pull/10868) Bump gov to v1beta2. Both v1beta1 and v1beta2 queries and Msgs are accepted.
 * [\#11011](https://github.com/cosmos/cosmos-sdk/pull/11011) Remove burning of deposits when qourum is not reached on a governance proposal and when the deposit is not fully met.
 * [\#11019](https://github.com/cosmos/cosmos-sdk/pull/11019) Add `MsgCreatePermanentLockedAccount` and CLI method for creating permanent locked account
 * (x/staking) [\#10885] (https://github.com/cosmos/cosmos-sdk/pull/10885) Add new `CancelUnbondingDelegation`
   transaction to `x/staking` module. Delegators can now cancel unbonding delegation entry and delegate back to validator.
 * (x/feegrant) [\#10830](https://github.com/cosmos/cosmos-sdk/pull/10830) Expired allowances will be pruned from state.
 * (x/authz,x/feegrant) [\#11214](https://github.com/cosmos/cosmos-sdk/pull/11214) Fix Amino JSON encoding of authz and feegrant Msgs to be consistent with other modules.
 * (authz)[\#11060](https://github.com/cosmos/cosmos-sdk/pull/11060) Support grant with no expire time.
+* (x/gov) [\#10868](https://github.com/cosmos/cosmos-sdk/pull/10868) Bump gov to v1. 
 
 ### Deprecated
 

proto/cosmos/upgrade/v1beta1/query.proto

@@ -37,6 +37,8 @@ service Query {
   }
 
   // Returns the account with authority to conduct upgrades
+  //
+  // Since: cosmos-sdk 0.46
   rpc Authority(QueryAuthorityRequest) returns (QueryAuthorityResponse) {
     option (google.api.http).get = "/cosmos/upgrade/v1beta1/authority";
   }

x/upgrade/client/cli/parse.go

@@ -4,26 +4,26 @@ import (
 	"github.com/cosmos/cosmos-sdk/x/gov/client/cli"
 	gov "github.com/cosmos/cosmos-sdk/x/gov/types/v1beta1"
 	"github.com/cosmos/cosmos-sdk/x/upgrade/types"
-	"github.com/spf13/cobra"
+	"github.com/spf13/pflag"
 )
 
-func parseArgsToContent(cmd *cobra.Command, name string) (gov.Content, error) {
-	title, err := cmd.Flags().GetString(cli.FlagTitle)
+func parseArgsToContent(fs *pflag.FlagSet, name string) (gov.Content, error) {
+	title, err := fs.GetString(cli.FlagTitle)
 	if err != nil {
 		return nil, err
 	}
 
-	description, err := cmd.Flags().GetString(cli.FlagDescription)
+	description, err := fs.GetString(cli.FlagDescription)
 	if err != nil {
 		return nil, err
 	}
 
-	height, err := cmd.Flags().GetInt64(FlagUpgradeHeight)
+	height, err := fs.GetInt64(FlagUpgradeHeight)
 	if err != nil {
 		return nil, err
 	}
 
-	info, err := cmd.Flags().GetString(FlagUpgradeInfo)
+	info, err := fs.GetString(FlagUpgradeInfo)
 	if err != nil {
 		return nil, err
 	}

x/upgrade/client/cli/parse_test.go

@@ -0,0 +1,40 @@
+package cli
+
+import (
+	"strconv"
+	"testing"
+
+	"github.com/cosmos/cosmos-sdk/x/gov/client/cli"
+	"github.com/cosmos/cosmos-sdk/x/upgrade/types"
+	"github.com/stretchr/testify/require"
+)
+
+func TestParseArgsToContent(t *testing.T) {
+	fs := NewCmdSubmitLegacyUpgradeProposal().Flags()
+
+	proposal := types.SoftwareUpgradeProposal{
+		Title:       "proposal title",
+		Description: "proposal description",
+		Plan: types.Plan{
+			Name:   "plan name",
+			Height: 123456,
+			Info:   "plan info",
+		},
+	}
+
+	fs.Set(cli.FlagTitle, proposal.Title)
+	fs.Set(cli.FlagDescription, proposal.Description)
+	fs.Set(FlagUpgradeHeight, strconv.FormatInt(proposal.Plan.Height, 10))
+	fs.Set(FlagUpgradeInfo, proposal.Plan.Info)
+
+	content, err := parseArgsToContent(fs, proposal.Plan.Name)
+	require.NoError(t, err)
+
+	p, ok := content.(*types.SoftwareUpgradeProposal)
+	require.Equal(t, ok, true)
+	require.Equal(t, p.Title, proposal.Title)
+	require.Equal(t, p.Description, proposal.Description)
+	require.Equal(t, p.Plan.Name, proposal.Plan.Name)
+	require.Equal(t, p.Plan.Height, proposal.Plan.Height)
+	require.Equal(t, p.Plan.Info, proposal.Plan.Info)
+}

x/upgrade/client/cli/tx.go

@@ -50,7 +50,7 @@ func NewCmdSubmitLegacyUpgradeProposal() *cobra.Command {
 				return err
 			}
 			name := args[0]
-			content, err := parseArgsToContent(cmd, name)
+			content, err := parseArgsToContent(cmd.Flags(), name)
 			if err != nil {
 				return err
 			}
@@ -163,7 +163,7 @@ func NewCmdSubmitLegacyCancelUpgradeProposal() *cobra.Command {
 // If a DAEMON_NAME env var is set, that is used.
 // Otherwise, the last part of the currently running executable is used.
 func getDefaultDaemonName() string {
-	// DAEMON_NAME is specifically used here to correspond with the Comsovisor setup env vars.
+	// DAEMON_NAME is specifically used here to correspond with the Cosmovisor setup env vars.
 	name := os.Getenv("DAEMON_NAME")
 	if len(name) == 0 {
 		_, name = filepath.Split(os.Args[0])

x/upgrade/keeper/grpc_query.go

@@ -71,7 +71,7 @@ func (k Keeper) ModuleVersions(c context.Context, req *types.QueryModuleVersions
 	}, nil
 }
 
-// Authority implementsthe the Query/Authority gRPC method, returning the account capable of performing upgrades
+// Authority implements the the Query/Authority gRPC method, returning the account capable of performing upgrades
 func (k Keeper) Authority(c context.Context, req *types.QueryAuthorityRequest) (*types.QueryAuthorityResponse, error) {
 	return &types.QueryAuthorityResponse{Address: k.authority}, nil
 }

x/upgrade/keeper/msg_server_test.go

@@ -57,6 +57,58 @@ func (s *KeeperTestSuite) TestSoftwareUpgrade() {
 			if tc.expectErr {
 				s.Require().Error(err)
 				s.Require().Contains(err.Error(), tc.errMsg)
+			} else {
+				s.Require().NoError(err)
+				plan, found := s.app.UpgradeKeeper.GetUpgradePlan(s.ctx)
+				s.Require().Equal(true, found)
+				s.Require().Equal(tc.req.Plan, plan)
+			}
+		})
+	}
+}
+
+func (s *KeeperTestSuite) TestCancelUpgrade() {
+	govAccAddr := s.app.GovKeeper.GetGovernanceAccount(s.ctx).GetAddress().String()
+	err := s.app.UpgradeKeeper.ScheduleUpgrade(s.ctx, types.Plan{
+		Name:   "some name",
+		Info:   "some info",
+		Height: 123450000,
+	})
+	s.Require().NoError(err)
+
+	testCases := []struct {
+		name      string
+		req       *types.MsgCancelUpgrade
+		expectErr bool
+		errMsg    string
+	}{
+		{
+			"unauthorized authority address",
+			&types.MsgCancelUpgrade{
+				Authority: s.addrs[0].String(),
+			},
+			true,
+			"expected gov account as only signer for proposal message",
+		},
+		{
+			"upgrade cancelled successfully",
+			&types.MsgCancelUpgrade{
+				Authority: govAccAddr,
+			},
+			false,
+			"",
+		},
+	}
+	for _, tc := range testCases {
+		s.Run(tc.name, func() {
+			_, err := s.msgSrvr.CancelUpgrade(s.ctx, tc.req)
+			if tc.expectErr {
+				s.Require().Error(err)
+				s.Require().Contains(err.Error(), tc.errMsg)
+			} else {
+				s.Require().NoError(err)
+				_, found := s.app.UpgradeKeeper.GetUpgradePlan(s.ctx)
+				s.Require().Equal(false, found)
 			}
 		})
 	}

x/upgrade/spec/01_concepts.md

@@ -67,7 +67,7 @@ not defined on a per-module basis. Registering this `StoreLoader` is done via
 func UpgradeStoreLoader (upgradeHeight int64, storeUpgrades *store.StoreUpgrades) baseapp.StoreLoader
 ```
 
-If there's a planned upgrade and the upgrade height is reached, the old binary writes `Plan` to the disk before panic'ing.
+If there's a planned upgrade and the upgrade height is reached, the old binary writes `Plan` to the disk before panicking.
 
 This information is critical to ensure the `StoreUpgrades` happens smoothly at correct height and
 expected upgrade. It eliminiates the chances for the new binary to execute `StoreUpgrades` multiple
@@ -82,7 +82,7 @@ This proposal prescribes to the standard governance process. If the proposal pas
 the `Plan`, which targets a specific `Handler`, is persisted and scheduled. The
 upgrade can be delayed or hastened by updating the `Plan.Height` in a new proposal.
 
-+++ https://github.com/cosmos/cosmos-sdk/blob/08ddb217c176abe31c96af9d5f6c4c6fc645c4d4/proto/cosmos/upgrade/v1beta1/tx.proto#L19-L28
++++ https://github.com/cosmos/cosmos-sdk/blob/v0.46.0-beta2/proto/cosmos/upgrade/v1beta1/tx.proto#L24-L35
 
 ### Cancelling Upgrade Proposals
 
@@ -92,6 +92,8 @@ remove the scheduled upgrade `Plan`.
 Of course this requires that the upgrade was known to be a bad idea well before the
 upgrade itself, to allow time for a vote.
 
++++ https://github.com/cosmos/cosmos-sdk/blob/v0.46.0-beta2/proto/cosmos/upgrade/v1beta1/tx.proto#L42-L50
+
 If such a possibility is desired, the upgrade height is to be
 `2 * (VotingPeriod + DepositPeriod) + (SafetyDelta)` from the beginning of the
 upgrade proposal. The `SafetyDelta` is the time available from the success of an

x/upgrade/types/msgs.go

@@ -61,7 +61,7 @@ func (m *MsgCancelUpgrade) ValidateBasic() error {
 	return nil
 }
 
-// GetSigners returns the expected signers for MsgSoftwareUpgrade.
+// GetSigners returns the expected signers for MsgCancelUpgrade.
 func (m *MsgCancelUpgrade) GetSigners() []sdk.AccAddress {
 	addr, _ := sdk.AccAddressFromBech32(m.Authority)
 	return []sdk.AccAddress{addr}