feat!: add protection against accidental downgrades (#10407)
## Description Closes: #10318 --- ### 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... - [ ] 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 or client breaking change - [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting)) - [ ] provided a link to the relevant issue or specification - [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules) - [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing) - [ ] added a changelog entry to `CHANGELOG.md` - [ ] 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 - [ ] manually tested (if applicable)
This commit is contained in:
parent
95e65fe4cf
commit
5622115865
|
@ -57,6 +57,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
|
||||||
* [\#10311](https://github.com/cosmos/cosmos-sdk/pull/10311) Adds cli to use tips transactions. It adds an `--aux` flag to all CLI tx commands to generate the aux signer data (with optional tip), and a new `tx aux-to-fee` subcommand to let the fee payer gather aux signer data and broadcast the tx
|
* [\#10311](https://github.com/cosmos/cosmos-sdk/pull/10311) Adds cli to use tips transactions. It adds an `--aux` flag to all CLI tx commands to generate the aux signer data (with optional tip), and a new `tx aux-to-fee` subcommand to let the fee payer gather aux signer data and broadcast the tx
|
||||||
* [\#10430](https://github.com/cosmos/cosmos-sdk/pull/10430) ADR-040: Add store/v2 `MultiStore` implementation
|
* [\#10430](https://github.com/cosmos/cosmos-sdk/pull/10430) ADR-040: Add store/v2 `MultiStore` implementation
|
||||||
* [\#10947](https://github.com/cosmos/cosmos-sdk/pull/10947) Add `AllowancesByGranter` query to the feegrant module
|
* [\#10947](https://github.com/cosmos/cosmos-sdk/pull/10947) Add `AllowancesByGranter` query to the feegrant module
|
||||||
|
* [\#10407](https://github.com/cosmos/cosmos-sdk/pull/10407) Add validation to `x/upgrade` module's `BeginBlock` to check accidental binary downgrades
|
||||||
|
|
||||||
### API Breaking Changes
|
### API Breaking Changes
|
||||||
|
|
||||||
|
|
|
@ -22,7 +22,24 @@ import (
|
||||||
// skipUpgradeHeightArray is a set of block heights for which the upgrade must be skipped
|
// skipUpgradeHeightArray is a set of block heights for which the upgrade must be skipped
|
||||||
func BeginBlocker(k keeper.Keeper, ctx sdk.Context, _ abci.RequestBeginBlock) {
|
func BeginBlocker(k keeper.Keeper, ctx sdk.Context, _ abci.RequestBeginBlock) {
|
||||||
defer telemetry.ModuleMeasureSince(types.ModuleName, time.Now(), telemetry.MetricKeyBeginBlocker)
|
defer telemetry.ModuleMeasureSince(types.ModuleName, time.Now(), telemetry.MetricKeyBeginBlocker)
|
||||||
|
|
||||||
plan, found := k.GetUpgradePlan(ctx)
|
plan, found := k.GetUpgradePlan(ctx)
|
||||||
|
|
||||||
|
if !k.DowngradeVerified() {
|
||||||
|
k.SetDowngradeVerified(true)
|
||||||
|
lastAppliedPlan, _ := k.GetLastCompletedUpgrade(ctx)
|
||||||
|
// This check will make sure that we are using a valid binary.
|
||||||
|
// It'll panic in these cases if there is no upgrade handler registered for the last applied upgrade.
|
||||||
|
// 1. If there is no scheduled upgrade.
|
||||||
|
// 2. If the plan is not ready.
|
||||||
|
// 3. If the plan is ready and skip upgrade height is set for current height.
|
||||||
|
if !found || !plan.ShouldExecute(ctx) || (plan.ShouldExecute(ctx) && k.IsSkipHeight(ctx.BlockHeight())) {
|
||||||
|
if lastAppliedPlan != "" && !k.HasHandler(lastAppliedPlan) {
|
||||||
|
panic(fmt.Sprintf("Wrong app version %d, upgrade handler is missing for %s upgrade plan", ctx.ConsensusParams().Version.AppVersion, lastAppliedPlan))
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
if !found {
|
if !found {
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
|
@ -410,3 +410,70 @@ func TestDumpUpgradeInfoToFile(t *testing.T) {
|
||||||
err = os.Remove(upgradeInfoFilePath)
|
err = os.Remove(upgradeInfoFilePath)
|
||||||
require.Nil(err)
|
require.Nil(err)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TODO: add testcase to for `no upgrade handler is present for last applied upgrade`.
|
||||||
|
func TestBinaryVersion(t *testing.T) {
|
||||||
|
var skipHeight int64 = 15
|
||||||
|
s := setupTest(t, 10, map[int64]bool{skipHeight: true})
|
||||||
|
|
||||||
|
testCases := []struct {
|
||||||
|
name string
|
||||||
|
preRun func() (sdk.Context, abci.RequestBeginBlock)
|
||||||
|
expectPanic bool
|
||||||
|
}{
|
||||||
|
{
|
||||||
|
"test not panic: no scheduled upgrade or applied upgrade is present",
|
||||||
|
func() (sdk.Context, abci.RequestBeginBlock) {
|
||||||
|
req := abci.RequestBeginBlock{Header: s.ctx.BlockHeader()}
|
||||||
|
return s.ctx, req
|
||||||
|
},
|
||||||
|
false,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"test not panic: upgrade handler is present for last applied upgrade",
|
||||||
|
func() (sdk.Context, abci.RequestBeginBlock) {
|
||||||
|
s.keeper.SetUpgradeHandler("test0", func(_ sdk.Context, _ types.Plan, vm module.VersionMap) (module.VersionMap, error) {
|
||||||
|
return vm, nil
|
||||||
|
})
|
||||||
|
|
||||||
|
err := s.handler(s.ctx, &types.SoftwareUpgradeProposal{Title: "Upgrade test", Plan: types.Plan{Name: "test0", Height: s.ctx.BlockHeight() + 2}})
|
||||||
|
require.Nil(t, err)
|
||||||
|
|
||||||
|
newCtx := s.ctx.WithBlockHeight(12)
|
||||||
|
s.keeper.ApplyUpgrade(newCtx, types.Plan{
|
||||||
|
Name: "test0",
|
||||||
|
Height: 12,
|
||||||
|
})
|
||||||
|
|
||||||
|
req := abci.RequestBeginBlock{Header: newCtx.BlockHeader()}
|
||||||
|
return newCtx, req
|
||||||
|
},
|
||||||
|
false,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"test panic: upgrade needed",
|
||||||
|
func() (sdk.Context, abci.RequestBeginBlock) {
|
||||||
|
err := s.handler(s.ctx, &types.SoftwareUpgradeProposal{Title: "Upgrade test", Plan: types.Plan{Name: "test2", Height: 13}})
|
||||||
|
require.Nil(t, err)
|
||||||
|
|
||||||
|
newCtx := s.ctx.WithBlockHeight(13)
|
||||||
|
req := abci.RequestBeginBlock{Header: newCtx.BlockHeader()}
|
||||||
|
return newCtx, req
|
||||||
|
},
|
||||||
|
true,
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, tc := range testCases {
|
||||||
|
ctx, req := tc.preRun()
|
||||||
|
if tc.expectPanic {
|
||||||
|
require.Panics(t, func() {
|
||||||
|
s.module.BeginBlock(ctx, req)
|
||||||
|
})
|
||||||
|
} else {
|
||||||
|
require.NotPanics(t, func() {
|
||||||
|
s.module.BeginBlock(ctx, req)
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
|
@ -17,6 +17,7 @@ import (
|
||||||
"github.com/cosmos/cosmos-sdk/store/prefix"
|
"github.com/cosmos/cosmos-sdk/store/prefix"
|
||||||
sdk "github.com/cosmos/cosmos-sdk/types"
|
sdk "github.com/cosmos/cosmos-sdk/types"
|
||||||
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
|
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
|
||||||
|
"github.com/cosmos/cosmos-sdk/types/kv"
|
||||||
"github.com/cosmos/cosmos-sdk/types/module"
|
"github.com/cosmos/cosmos-sdk/types/module"
|
||||||
xp "github.com/cosmos/cosmos-sdk/x/upgrade/exported"
|
xp "github.com/cosmos/cosmos-sdk/x/upgrade/exported"
|
||||||
"github.com/cosmos/cosmos-sdk/x/upgrade/types"
|
"github.com/cosmos/cosmos-sdk/x/upgrade/types"
|
||||||
|
@ -33,6 +34,7 @@ type Keeper struct {
|
||||||
cdc codec.BinaryCodec // App-wide binary codec
|
cdc codec.BinaryCodec // App-wide binary codec
|
||||||
upgradeHandlers map[string]types.UpgradeHandler // map of plan name to upgrade handler
|
upgradeHandlers map[string]types.UpgradeHandler // map of plan name to upgrade handler
|
||||||
versionSetter xp.ProtocolVersionSetter // implements setting the protocol version field on BaseApp
|
versionSetter xp.ProtocolVersionSetter // implements setting the protocol version field on BaseApp
|
||||||
|
downgradeVerified bool // tells if we've already sanity checked that this binary version isn't being used against an old state.
|
||||||
}
|
}
|
||||||
|
|
||||||
// NewKeeper constructs an upgrade Keeper which requires the following arguments:
|
// NewKeeper constructs an upgrade Keeper which requires the following arguments:
|
||||||
|
@ -228,6 +230,23 @@ func (k Keeper) GetUpgradedConsensusState(ctx sdk.Context, lastHeight int64) ([]
|
||||||
return bz, true
|
return bz, true
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// GetLastCompletedUpgrade returns the last applied upgrade name and height.
|
||||||
|
func (k Keeper) GetLastCompletedUpgrade(ctx sdk.Context) (string, int64) {
|
||||||
|
iter := sdk.KVStoreReversePrefixIterator(ctx.KVStore(k.storeKey), []byte{types.DoneByte})
|
||||||
|
defer iter.Close()
|
||||||
|
if iter.Valid() {
|
||||||
|
return parseDoneKey(iter.Key()), int64(binary.BigEndian.Uint64(iter.Value()))
|
||||||
|
}
|
||||||
|
|
||||||
|
return "", 0
|
||||||
|
}
|
||||||
|
|
||||||
|
// parseDoneKey - split upgrade name from the done key
|
||||||
|
func parseDoneKey(key []byte) string {
|
||||||
|
kv.AssertKeyAtLeastLength(key, 2)
|
||||||
|
return string(key[1:])
|
||||||
|
}
|
||||||
|
|
||||||
// GetDoneHeight returns the height at which the given upgrade was executed
|
// GetDoneHeight returns the height at which the given upgrade was executed
|
||||||
func (k Keeper) GetDoneHeight(ctx sdk.Context, name string) int64 {
|
func (k Keeper) GetDoneHeight(ctx sdk.Context, name string) int64 {
|
||||||
store := prefix.NewStore(ctx.KVStore(k.storeKey), []byte{types.DoneByte})
|
store := prefix.NewStore(ctx.KVStore(k.storeKey), []byte{types.DoneByte})
|
||||||
|
@ -389,3 +408,13 @@ func (k Keeper) ReadUpgradeInfoFromDisk() (types.Plan, error) {
|
||||||
|
|
||||||
return upgradeInfo, nil
|
return upgradeInfo, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// SetDowngradeVerified updates downgradeVerified.
|
||||||
|
func (k *Keeper) SetDowngradeVerified(v bool) {
|
||||||
|
k.downgradeVerified = v
|
||||||
|
}
|
||||||
|
|
||||||
|
// DowngradeVerified returns downgradeVerified.
|
||||||
|
func (k Keeper) DowngradeVerified() bool {
|
||||||
|
return k.downgradeVerified
|
||||||
|
}
|
||||||
|
|
|
@ -232,6 +232,45 @@ func (s *KeeperTestSuite) TestMigrations() {
|
||||||
s.Require().Equal(vmBefore["bank"]+1, vm["bank"])
|
s.Require().Equal(vmBefore["bank"]+1, vm["bank"])
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func (s *KeeperTestSuite) TestLastCompletedUpgrade() {
|
||||||
|
keeper := s.app.UpgradeKeeper
|
||||||
|
require := s.Require()
|
||||||
|
|
||||||
|
s.T().Log("verify empty name if applied upgrades are empty")
|
||||||
|
name, height := keeper.GetLastCompletedUpgrade(s.ctx)
|
||||||
|
require.Equal("", name)
|
||||||
|
require.Equal(int64(0), height)
|
||||||
|
|
||||||
|
keeper.SetUpgradeHandler("test0", func(_ sdk.Context, _ types.Plan, vm module.VersionMap) (module.VersionMap, error) {
|
||||||
|
return vm, nil
|
||||||
|
})
|
||||||
|
|
||||||
|
keeper.ApplyUpgrade(s.ctx, types.Plan{
|
||||||
|
Name: "test0",
|
||||||
|
Height: 10,
|
||||||
|
})
|
||||||
|
|
||||||
|
s.T().Log("verify valid upgrade name and height")
|
||||||
|
name, height = keeper.GetLastCompletedUpgrade(s.ctx)
|
||||||
|
require.Equal("test0", name)
|
||||||
|
require.Equal(int64(10), height)
|
||||||
|
|
||||||
|
keeper.SetUpgradeHandler("test1", func(_ sdk.Context, _ types.Plan, vm module.VersionMap) (module.VersionMap, error) {
|
||||||
|
return vm, nil
|
||||||
|
})
|
||||||
|
|
||||||
|
newCtx := s.ctx.WithBlockHeight(15)
|
||||||
|
keeper.ApplyUpgrade(newCtx, types.Plan{
|
||||||
|
Name: "test1",
|
||||||
|
Height: 15,
|
||||||
|
})
|
||||||
|
|
||||||
|
s.T().Log("verify valid upgrade name and height with multiple upgrades")
|
||||||
|
name, height = keeper.GetLastCompletedUpgrade(newCtx)
|
||||||
|
require.Equal("test1", name)
|
||||||
|
require.Equal(int64(15), height)
|
||||||
|
}
|
||||||
|
|
||||||
func TestKeeperTestSuite(t *testing.T) {
|
func TestKeeperTestSuite(t *testing.T) {
|
||||||
suite.Run(t, new(KeeperTestSuite))
|
suite.Run(t, new(KeeperTestSuite))
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in New Issue