chore: x/upgrade audit changes (#11879)

This commit is contained in:
Marie Gauthier 2022-05-05 15:38:21 +02:00 committed by GitHub
parent 3e3c114af5
commit 8416394864
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 118 additions and 17 deletions

View File

@ -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,9 +283,9 @@ 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`
@ -293,6 +293,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
* (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

View File

@ -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";
}

View File

@ -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
}

View File

@ -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)
}

View File

@ -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])

View File

@ -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
}

View File

@ -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)
}
})
}

View File

@ -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

View File

@ -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}

View File

@ -582,6 +582,8 @@ type QueryClient interface {
// Since: cosmos-sdk 0.43
ModuleVersions(ctx context.Context, in *QueryModuleVersionsRequest, opts ...grpc.CallOption) (*QueryModuleVersionsResponse, error)
// Returns the account with authority to conduct upgrades
//
// Since: cosmos-sdk 0.46
Authority(ctx context.Context, in *QueryAuthorityRequest, opts ...grpc.CallOption) (*QueryAuthorityResponse, error)
}
@ -657,6 +659,8 @@ type QueryServer interface {
// Since: cosmos-sdk 0.43
ModuleVersions(context.Context, *QueryModuleVersionsRequest) (*QueryModuleVersionsResponse, error)
// Returns the account with authority to conduct upgrades
//
// Since: cosmos-sdk 0.46
Authority(context.Context, *QueryAuthorityRequest) (*QueryAuthorityResponse, error)
}