Skip to content

Commit

Permalink
fix: removed potential sources of non-determinism in upgrades (backport
Browse files Browse the repository at this point in the history
cosmos#10189) (cosmos#10253)

* fix: removed potential sources of non-determinism in upgrades (cosmos#10189)

forced deterministic iteration order in upgrade migrations, x/upgrade and store during upgrades

Co-authored-by: Robert Zaremba <robert@zaremba.ch>
(cherry picked from commit f757c90)

# Conflicts:
#	CHANGELOG.md

* Update CHANGELOG.md

Co-authored-by: Tomas Tauber <2410580+tomtau@users.noreply.github.com>
Co-authored-by: Robert Zaremba <robert@zaremba.ch>
  • Loading branch information
3 people authored and JeancarloBarrios committed Sep 28, 2024
1 parent 1cdf7e3 commit 7147dfc
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 8 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,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

Expand Down
14 changes: 14 additions & 0 deletions store/rootmulti/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,20 @@ func (rs *Store) loadVersion(ver int64, upgrades *types.StoreUpgrades) error {
})
}

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())
Expand Down
19 changes: 12 additions & 7 deletions types/module/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,6 @@ package module
import (
"context"
"encoding/json"
"errors"
"fmt"
"maps"
"slices"
"sort"

abci "github.com/cometbft/cometbft/api/cometbft/abci/v1"
Expand Down Expand Up @@ -670,9 +666,18 @@ func (m Manager) RunMigrations(ctx context.Context, cfg Configurator, fromVM app
modules = DefaultMigrationsOrder(m.ModuleNames())
}

sdkCtx := sdk.UnwrapSDKContext(ctx)
updatedVM := appmodule.VersionMap{}
for _, moduleName := range modules {
updatedVM := make(VersionMap)
// 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 := uint64(0)
Expand Down
1 change: 0 additions & 1 deletion x/upgrade/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
"os"
"path/filepath"
"sort"
"strconv"

"github.com/hashicorp/go-metrics"

Expand Down

0 comments on commit 7147dfc

Please sign in to comment.