Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: ScheduleUpgradeNoHeightValidation for automated upgrades w/o gov proposal (backport #11551) #11574

Merged
merged 4 commits into from
Apr 8, 2022
Merged

feat: ScheduleUpgradeNoHeightValidation for automated upgrades w/o gov proposal (backport #11551) #11574

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c415610
feat: `ScheduleUpgradeNoHeightValidation` for automated upgrades w/o …
fedekunze Apr 7, 2022
f62e2b4
fix conflicts
fedekunze Apr 8, 2022
7eb3dd5
changelog
fedekunze Apr 8, 2022
1ca151d
fix test
fedekunze Apr 8, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ Ref: https://keepachangelog.com/en/1.0.0/

## [Unreleased]

### Features

* (x/upgrade) [\#11551](https://github.com/cosmos/cosmos-sdk/pull/11551) Update `ScheduleUpgrade` for chains to schedule an automated upgrade on `BeginBlock` without having to go though governance.

## [v0.45.2](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.45.2) - 2022-04-05

### Features
Expand Down
42 changes: 20 additions & 22 deletions x/upgrade/abci_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,22 +64,22 @@ func TestRequireName(t *testing.T) {
s := setupTest(10, map[int64]bool{})

err := s.handler(s.ctx, &types.SoftwareUpgradeProposal{Title: "prop", Plan: types.Plan{}})
require.NotNil(t, err)
require.Error(t, err)
require.True(t, errors.Is(sdkerrors.ErrInvalidRequest, err), err)
}

func TestRequireFutureBlock(t *testing.T) {
s := setupTest(10, map[int64]bool{})
err := s.handler(s.ctx, &types.SoftwareUpgradeProposal{Title: "prop", Plan: types.Plan{Name: "test", Height: s.ctx.BlockHeight()}})
require.NotNil(t, err)
err := s.handler(s.ctx, &types.SoftwareUpgradeProposal{Title: "prop", Plan: types.Plan{Name: "test", Height: s.ctx.BlockHeight() - 1}})
require.Error(t, err)
require.True(t, errors.Is(sdkerrors.ErrInvalidRequest, err), err)
}

func TestDoHeightUpgrade(t *testing.T) {
s := setupTest(10, map[int64]bool{})
t.Log("Verify can schedule an upgrade")
err := s.handler(s.ctx, &types.SoftwareUpgradeProposal{Title: "prop", Plan: types.Plan{Name: "test", Height: s.ctx.BlockHeight() + 1}})
require.Nil(t, err)
require.NoError(t, err)

VerifyDoUpgrade(t)
}
Expand All @@ -88,9 +88,9 @@ func TestCanOverwriteScheduleUpgrade(t *testing.T) {
s := setupTest(10, map[int64]bool{})
t.Log("Can overwrite plan")
err := s.handler(s.ctx, &types.SoftwareUpgradeProposal{Title: "prop", Plan: types.Plan{Name: "bad_test", Height: s.ctx.BlockHeight() + 10}})
require.Nil(t, err)
require.NoError(t, err)
err = s.handler(s.ctx, &types.SoftwareUpgradeProposal{Title: "prop", Plan: types.Plan{Name: "test", Height: s.ctx.BlockHeight() + 1}})
require.Nil(t, err)
require.NoError(t, err)

VerifyDoUpgrade(t)
}
Expand Down Expand Up @@ -137,7 +137,7 @@ func TestHaltIfTooNew(t *testing.T) {
s := setupTest(10, map[int64]bool{})
t.Log("Verify that we don't panic with registered plan not in database at all")
var called int
s.keeper.SetUpgradeHandler("future", func(ctx sdk.Context, plan types.Plan, vm module.VersionMap) (module.VersionMap, error) {
s.keeper.SetUpgradeHandler("future", func(_ sdk.Context, _ types.Plan, vm module.VersionMap) (module.VersionMap, error) {
called++
return vm, nil
})
Expand Down Expand Up @@ -180,10 +180,10 @@ func TestCanClear(t *testing.T) {
s := setupTest(10, map[int64]bool{})
t.Log("Verify upgrade is scheduled")
err := s.handler(s.ctx, &types.SoftwareUpgradeProposal{Title: "prop", Plan: types.Plan{Name: "test", Height: s.ctx.BlockHeight() + 100}})
require.Nil(t, err)
require.NoError(t, err)

err = s.handler(s.ctx, &types.CancelSoftwareUpgradeProposal{Title: "cancel"})
require.Nil(t, err)
require.NoError(t, err)

VerifyCleared(t, s.ctx)
}
Expand All @@ -192,11 +192,11 @@ func TestCantApplySameUpgradeTwice(t *testing.T) {
s := setupTest(10, map[int64]bool{})
height := s.ctx.BlockHeader().Height + 1
err := s.handler(s.ctx, &types.SoftwareUpgradeProposal{Title: "prop", Plan: types.Plan{Name: "test", Height: height}})
require.Nil(t, err)
require.NoError(t, err)
VerifyDoUpgrade(t)
t.Log("Verify an executed upgrade \"test\" can't be rescheduled")
err = s.handler(s.ctx, &types.SoftwareUpgradeProposal{Title: "prop", Plan: types.Plan{Name: "test", Height: height}})
require.NotNil(t, err)
require.Error(t, err)
require.True(t, errors.Is(sdkerrors.ErrInvalidRequest, err), err)
}

Expand Down Expand Up @@ -242,9 +242,7 @@ func VerifySet(t *testing.T, skipUpgradeHeights map[int64]bool) {
}

func TestContains(t *testing.T) {
var (
skipOne int64 = 11
)
var skipOne int64 = 11
s := setupTest(10, map[int64]bool{skipOne: true})

VerifySet(t, map[int64]bool{skipOne: true})
Expand Down Expand Up @@ -303,7 +301,7 @@ func TestUpgradeSkippingOne(t *testing.T) {

req := abci.RequestBeginBlock{Header: newCtx.BlockHeader()}
err := s.handler(s.ctx, &types.SoftwareUpgradeProposal{Title: "prop", Plan: types.Plan{Name: "test", Height: skipOne}})
require.Nil(t, err)
require.NoError(t, err)

t.Log("Verify if skip upgrade flag clears upgrade plan in one case and does upgrade on another")
VerifySet(t, map[int64]bool{skipOne: true})
Expand All @@ -316,7 +314,7 @@ func TestUpgradeSkippingOne(t *testing.T) {

t.Log("Verify the second proposal is not skipped")
err = s.handler(s.ctx, &types.SoftwareUpgradeProposal{Title: "prop2", Plan: types.Plan{Name: "test2", Height: skipTwo}})
require.Nil(t, err)
require.NoError(t, err)
// Setting block height of proposal test2
newCtx = newCtx.WithBlockHeight(skipTwo)
VerifyDoUpgradeWithCtx(t, newCtx, "test2")
Expand All @@ -338,7 +336,7 @@ func TestUpgradeSkippingOnlyTwo(t *testing.T) {

req := abci.RequestBeginBlock{Header: newCtx.BlockHeader()}
err := s.handler(s.ctx, &types.SoftwareUpgradeProposal{Title: "prop", Plan: types.Plan{Name: "test", Height: skipOne}})
require.Nil(t, err)
require.NoError(t, err)

t.Log("Verify if skip upgrade flag clears upgrade plan in both cases and does third upgrade")
VerifySet(t, map[int64]bool{skipOne: true, skipTwo: true})
Expand All @@ -351,7 +349,7 @@ func TestUpgradeSkippingOnlyTwo(t *testing.T) {

// A new proposal with height in skipUpgradeHeights
err = s.handler(s.ctx, &types.SoftwareUpgradeProposal{Title: "prop2", Plan: types.Plan{Name: "test2", Height: skipTwo}})
require.Nil(t, err)
require.NoError(t, err)
// Setting block height of proposal test2
newCtx = newCtx.WithBlockHeight(skipTwo)
require.NotPanics(t, func() {
Expand All @@ -360,7 +358,7 @@ func TestUpgradeSkippingOnlyTwo(t *testing.T) {

t.Log("Verify a new proposal is not skipped")
err = s.handler(s.ctx, &types.SoftwareUpgradeProposal{Title: "prop3", Plan: types.Plan{Name: "test3", Height: skipThree}})
require.Nil(t, err)
require.NoError(t, err)
newCtx = newCtx.WithBlockHeight(skipThree)
VerifyDoUpgradeWithCtx(t, newCtx, "test3")

Expand All @@ -375,7 +373,7 @@ func TestUpgradeWithoutSkip(t *testing.T) {
newCtx := s.ctx.WithBlockHeight(s.ctx.BlockHeight() + 1).WithBlockTime(time.Now())
req := abci.RequestBeginBlock{Header: newCtx.BlockHeader()}
err := s.handler(s.ctx, &types.SoftwareUpgradeProposal{Title: "prop", Plan: types.Plan{Name: "test", Height: s.ctx.BlockHeight() + 1}})
require.Nil(t, err)
require.NoError(t, err)
t.Log("Verify if upgrade happens without skip upgrade")
require.Panics(t, func() {
s.module.BeginBlock(newCtx, req)
Expand Down Expand Up @@ -438,7 +436,7 @@ func TestBinaryVersion(t *testing.T) {
})

err := s.handler(s.ctx, &types.SoftwareUpgradeProposal{Title: "Upgrade test", Plan: types.Plan{Name: "test0", Height: s.ctx.BlockHeight() + 2}})
require.Nil(t, err)
require.NoError(t, err)

newCtx := s.ctx.WithBlockHeight(12)
s.keeper.ApplyUpgrade(newCtx, types.Plan{
Expand All @@ -455,7 +453,7 @@ func TestBinaryVersion(t *testing.T) {
"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)
require.NoError(t, err)

newCtx := s.ctx.WithBlockHeight(13)
req := abci.RequestBeginBlock{Header: newCtx.BlockHeader()}
Expand Down
6 changes: 4 additions & 2 deletions x/upgrade/keeper/keeper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,9 @@ func (k Keeper) ScheduleUpgrade(ctx sdk.Context, plan types.Plan) error {
return err
}

if plan.Height <= ctx.BlockHeight() {
// NOTE: allow for the possibility of chains to schedule upgrades in begin block of the same block
// as a strategy for emergency hard fork recoveries
if plan.Height < ctx.BlockHeight() {
Comment on lines -174 to +176
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@marbar3778 ^^

return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "upgrade cannot be scheduled in the past")
}

Expand Down Expand Up @@ -375,7 +377,7 @@ func (k Keeper) DumpUpgradeInfoWithInfoToDisk(height int64, name string, info st
return err
}

return ioutil.WriteFile(upgradeInfoFilePath, bz, 0600)
return os.WriteFile(upgradeInfoFilePath, bz, 0o600)
}

// GetUpgradeInfoPath returns the upgrade info file path
Expand Down