diff --git a/CHANGELOG.md b/CHANGELOG.md index 1ed748930b0e..825a804d7f2d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -71,6 +71,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Bug Fixes * [\#9965](https://github.com/cosmos/cosmos-sdk/pull/9965) Fixed `simd version` command output to report the right release tag. +* (x/upgrade) [\#10189](https://github.com/cosmos/cosmos-sdk/issues/10189) Removed potential sources of non-determinism in upgrades. ### Client Breaking Changes diff --git a/store/rootmulti/store.go b/store/rootmulti/store.go index 471a24efe2cc..07dd864de1e3 100644 --- a/store/rootmulti/store.go +++ b/store/rootmulti/store.go @@ -190,7 +190,22 @@ func (rs *Store) loadVersion(ver int64, upgrades *types.StoreUpgrades) error { // load each Store (note this doesn't panic on unmounted keys now) var newStores = make(map[types.StoreKey]types.CommitKVStore) - for key, storeParams := range rs.storesParams { + storesKeys := make([]types.StoreKey, 0, len(rs.storesParams)) + + for key := range rs.storesParams { + storesKeys = append(storesKeys, key) + } + if upgrades != nil { + // deterministic iteration order for upgrades + // (as the underlying store may change and + // upgrades make store changes where the execution order may matter) + sort.Slice(storesKeys, func(i, j int) bool { + return storesKeys[i].Name() < storesKeys[j].Name() + }) + } + + for _, key := range storesKeys { + storeParams := rs.storesParams[key] commitID := rs.getCommitID(infos, key.Name()) // If it has been added, set the initial version diff --git a/types/module/module.go b/types/module/module.go index 37f1bf97f72c..57b2df544378 100644 --- a/types/module/module.go +++ b/types/module/module.go @@ -30,6 +30,7 @@ package module import ( "encoding/json" + "sort" "github.com/gorilla/mux" "github.com/grpc-ecosystem/grpc-gateway/runtime" @@ -389,7 +390,18 @@ func (m Manager) RunMigrations(ctx sdk.Context, cfg Configurator, fromVM Version } updatedVM := make(VersionMap) - for moduleName, module := range m.Modules { + // for deterministic iteration order + // (as some migrations depend on other modules + // and the order of executing migrations matters) + // TODO: make the order user-configurable? + sortedModNames := make([]string, 0, len(m.Modules)) + for key := range m.Modules { + sortedModNames = append(sortedModNames, key) + } + sort.Strings(sortedModNames) + + for _, moduleName := range sortedModNames { + module := m.Modules[moduleName] fromVersion, exists := fromVM[moduleName] toVersion := module.ConsensusVersion() diff --git a/x/upgrade/keeper/keeper.go b/x/upgrade/keeper/keeper.go index 4dbe712757bb..71e5e97dbe18 100644 --- a/x/upgrade/keeper/keeper.go +++ b/x/upgrade/keeper/keeper.go @@ -7,6 +7,7 @@ import ( "os" "path" "path/filepath" + "sort" "github.com/tendermint/tendermint/libs/log" tmos "github.com/tendermint/tendermint/libs/os" @@ -84,7 +85,18 @@ func (k Keeper) SetModuleVersionMap(ctx sdk.Context, vm module.VersionMap) { if len(vm) > 0 { store := ctx.KVStore(k.storeKey) versionStore := prefix.NewStore(store, []byte{types.VersionMapByte}) - for modName, ver := range vm { + // Even though the underlying store (cachekv) store is sorted, we still + // prefer a deterministic iteration order of the map, to avoid undesired + // surprises if we ever change stores. + sortedModNames := make([]string, 0, len(vm)) + + for key := range vm { + sortedModNames = append(sortedModNames, key) + } + sort.Strings(sortedModNames) + + for _, modName := range sortedModNames { + ver := vm[modName] nameBytes := []byte(modName) verBytes := make([]byte, 8) binary.BigEndian.PutUint64(verBytes, ver)