From 754fe37d79ee3e8b939e1b7e0c926b3e4831fdc4 Mon Sep 17 00:00:00 2001 From: Flavien Binet Date: Wed, 20 Apr 2022 17:56:08 +0200 Subject: [PATCH 1/7] tests: add testcase showing invalid return value from GetLastCompletedUpgrade --- x/upgrade/keeper/keeper_test.go | 36 +++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/x/upgrade/keeper/keeper_test.go b/x/upgrade/keeper/keeper_test.go index a91b0b748d21..0bd8fac77223 100644 --- a/x/upgrade/keeper/keeper_test.go +++ b/x/upgrade/keeper/keeper_test.go @@ -278,6 +278,42 @@ func (s *KeeperTestSuite) TestLastCompletedUpgrade() { require.Equal(int64(15), height) } +func (s *KeeperTestSuite) TestLastCompletedUpgradeOrdering() { + keeper := s.app.UpgradeKeeper + require := s.Require() + + // apply first upgrade + + keeper.SetUpgradeHandler("test-v0.9", func(_ sdk.Context, _ types.Plan, vm module.VersionMap) (module.VersionMap, error) { + return vm, nil + }) + + keeper.ApplyUpgrade(s.ctx, types.Plan{ + Name: "test-v0.9", + Height: 10, + }) + + name, height := keeper.GetLastCompletedUpgrade(s.ctx) + require.Equal("test-v0.9", name) + require.Equal(int64(10), height) + + // apply second upgrade + + keeper.SetUpgradeHandler("test-v0.10", 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: "test-v0.10", + Height: 15, + }) + + name, height = keeper.GetLastCompletedUpgrade(newCtx) + require.Equal("test-v0.10", name) + require.Equal(int64(15), height) +} + func TestKeeperTestSuite(t *testing.T) { suite.Run(t, new(KeeperTestSuite)) } From 20ded9273b5abc5de2760c14fc9374413b86c13c Mon Sep 17 00:00:00 2001 From: Flavien Binet Date: Mon, 25 Apr 2022 17:56:42 +0200 Subject: [PATCH 2/7] fix: sort by block height in GetLastCompletedUpgrade Integrating solution from https://github.com/cosmos/cosmos-sdk/issues/11707#issuecomment-1106624708 --- x/upgrade/keeper/keeper.go | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/x/upgrade/keeper/keeper.go b/x/upgrade/keeper/keeper.go index a17530d091c7..760721680161 100644 --- a/x/upgrade/keeper/keeper.go +++ b/x/upgrade/keeper/keeper.go @@ -239,8 +239,27 @@ func (k Keeper) GetUpgradedConsensusState(ctx sdk.Context, lastHeight int64) ([] 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())) + + upgrades := []struct { + Name string + BlockHeight int64 + }{} + + for iter.Valid() { + name := parseDoneKey(iter.Key()) + value := int64(binary.BigEndian.Uint64(iter.Value())) + upgrades = append(upgrades, struct { + Name string + BlockHeight int64 + }{Name: name, BlockHeight: value}) + iter.Next() + } + sort.SliceStable(upgrades, func(i, j int) bool { + return upgrades[i].BlockHeight > upgrades[j].BlockHeight + }) + + if len(upgrades) > 0 { + return upgrades[0].Name, upgrades[0].BlockHeight } return "", 0 From d62bb272e6e6b5e2e8769987d7eaf97f17ddb7dd Mon Sep 17 00:00:00 2001 From: Flavien Binet Date: Mon, 25 Apr 2022 18:24:31 +0200 Subject: [PATCH 3/7] fix: style --- x/upgrade/keeper/keeper.go | 11 ++++++----- x/upgrade/keeper/keeper_test.go | 2 -- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/x/upgrade/keeper/keeper.go b/x/upgrade/keeper/keeper.go index 760721680161..e7ca2fc1e228 100644 --- a/x/upgrade/keeper/keeper.go +++ b/x/upgrade/keeper/keeper.go @@ -235,16 +235,17 @@ func (k Keeper) GetUpgradedConsensusState(ctx sdk.Context, lastHeight int64) ([] return bz, true } +type upgrade struct { + Name string + BlockHeight int64 +} + // 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() - upgrades := []struct { - Name string - BlockHeight int64 - }{} - + upgrades := []upgrade{} for iter.Valid() { name := parseDoneKey(iter.Key()) value := int64(binary.BigEndian.Uint64(iter.Value())) diff --git a/x/upgrade/keeper/keeper_test.go b/x/upgrade/keeper/keeper_test.go index 0bd8fac77223..705f1f14f737 100644 --- a/x/upgrade/keeper/keeper_test.go +++ b/x/upgrade/keeper/keeper_test.go @@ -283,7 +283,6 @@ func (s *KeeperTestSuite) TestLastCompletedUpgradeOrdering() { require := s.Require() // apply first upgrade - keeper.SetUpgradeHandler("test-v0.9", func(_ sdk.Context, _ types.Plan, vm module.VersionMap) (module.VersionMap, error) { return vm, nil }) @@ -298,7 +297,6 @@ func (s *KeeperTestSuite) TestLastCompletedUpgradeOrdering() { require.Equal(int64(10), height) // apply second upgrade - keeper.SetUpgradeHandler("test-v0.10", func(_ sdk.Context, _ types.Plan, vm module.VersionMap) (module.VersionMap, error) { return vm, nil }) From 4ab2c62c62c86b2c7664ccf13b0c3775aaa39a2a Mon Sep 17 00:00:00 2001 From: Flavien Binet Date: Mon, 25 Apr 2022 18:35:58 +0200 Subject: [PATCH 4/7] fix: use upgrade struct --- x/upgrade/keeper/keeper.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/x/upgrade/keeper/keeper.go b/x/upgrade/keeper/keeper.go index e7ca2fc1e228..9f2778e38005 100644 --- a/x/upgrade/keeper/keeper.go +++ b/x/upgrade/keeper/keeper.go @@ -249,10 +249,7 @@ func (k Keeper) GetLastCompletedUpgrade(ctx sdk.Context) (string, int64) { for iter.Valid() { name := parseDoneKey(iter.Key()) value := int64(binary.BigEndian.Uint64(iter.Value())) - upgrades = append(upgrades, struct { - Name string - BlockHeight int64 - }{Name: name, BlockHeight: value}) + upgrades = append(upgrades, upgrade{Name: name, BlockHeight: value}) iter.Next() } sort.SliceStable(upgrades, func(i, j int) bool { From 1741e7ef51e1f8da40140f50fedfe5568cfa593b Mon Sep 17 00:00:00 2001 From: daeMOn Date: Mon, 25 Apr 2022 18:46:51 +0200 Subject: [PATCH 5/7] Update x/upgrade/keeper/keeper.go Co-authored-by: Aleksandr Bezobchuk --- x/upgrade/keeper/keeper.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x/upgrade/keeper/keeper.go b/x/upgrade/keeper/keeper.go index 9f2778e38005..0656e705fcce 100644 --- a/x/upgrade/keeper/keeper.go +++ b/x/upgrade/keeper/keeper.go @@ -246,11 +246,11 @@ func (k Keeper) GetLastCompletedUpgrade(ctx sdk.Context) (string, int64) { defer iter.Close() upgrades := []upgrade{} - for iter.Valid() { + for ; iter.Valid(); iter.Next() { name := parseDoneKey(iter.Key()) value := int64(binary.BigEndian.Uint64(iter.Value())) + upgrades = append(upgrades, upgrade{Name: name, BlockHeight: value}) - iter.Next() } sort.SliceStable(upgrades, func(i, j int) bool { return upgrades[i].BlockHeight > upgrades[j].BlockHeight From e921e726a3e0006f0c137cabb522f485607550f6 Mon Sep 17 00:00:00 2001 From: Flavien Binet Date: Mon, 25 Apr 2022 18:54:23 +0200 Subject: [PATCH 6/7] docs: add changelog entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e9b898a2769e..15ee27bd0707 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,6 +39,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Features +* (x/upgrade) [\#11705](https://github.com/cosmos/cosmos-sdk/pull/11705) Fix `GetLastCompleteUpgrade` to properly return the latest upgrade. * (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. * (cli) [\#11548](https://github.com/cosmos/cosmos-sdk/pull/11548) Add Tendermint's `inspect` command to the `tendermint` sub-command. * (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. From d7dff66cc212e7d258df4fd60ab80256faa180da Mon Sep 17 00:00:00 2001 From: Flavien Binet Date: Mon, 25 Apr 2022 19:04:00 +0200 Subject: [PATCH 7/7] docs: move changelog entry to bugfix section --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 15ee27bd0707..7d17ade32ebb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,7 +39,6 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Features -* (x/upgrade) [\#11705](https://github.com/cosmos/cosmos-sdk/pull/11705) Fix `GetLastCompleteUpgrade` to properly return the latest upgrade. * (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. * (cli) [\#11548](https://github.com/cosmos/cosmos-sdk/pull/11548) Add Tendermint's `inspect` command to the `tendermint` sub-command. * (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. @@ -212,6 +211,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Bug Fixes +* (x/upgrade) [\#11705](https://github.com/cosmos/cosmos-sdk/pull/11705) Fix `GetLastCompleteUpgrade` to properly return the latest upgrade. * [\#11724](https://github.com/cosmos/cosmos-sdk/pull/11724) Fix data race issues with api.Server * [\#11693](https://github.com/cosmos/cosmos-sdk/pull/11693) Add validation for gentx cmd. * [\#11645](https://github.com/cosmos/cosmos-sdk/pull/11645) Fix `--home` flag ignored when running help.