From 8e8a284b91b9773fa751327b39bd99c1f8e5fc4e Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Fri, 29 Mar 2024 02:19:33 +0000 Subject: [PATCH] fix: remove map iteration non-determinism with keys (backport #1302) (#1305) * fix: remove map iteration non-determinism with keys (#1302) * remove map iteration non-determinism with keys * add CHANGELOG (cherry picked from commit c462f368628c092dafe48c254114143dbf596b35) # Conflicts: # CHANGELOG.md * Update CHANGELOG.md --------- Co-authored-by: Jayden Lee <41176085+tkxkd0159@users.noreply.github.com> Co-authored-by: Youngtaek Yoon --- CHANGELOG.md | 1 + baseapp/baseapp.go | 7 ++++++- store/rootmulti/store.go | 25 +++++++++++++++++++++---- types/module/module.go | 6 +++++- 4 files changed, 33 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index eb197ca3e7..77f7aef24b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -48,6 +48,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (x/collection) [\#1282](https://github.com/Finschia/finschia-sdk/pull/1282) eliminates potential risk for Insufficient Sanity Check of tokenID in Genesis (backport #1276) * (x/collection) [\#1290](https://github.com/Finschia/finschia-sdk/pull/1290) export x/collection params into genesis (backport #1268) * (x/foundation) [\#1295](https://github.com/Finschia/finschia-sdk/pull/1295) add missing error handling for migration +* (sec) [\#1302](https://github.com/Finschia/finschia-sdk/pull/1302) remove map iteration non-determinism with keys + sorting ### Removed diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index cd0798dcc7..422b63db43 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -4,6 +4,7 @@ import ( "errors" "fmt" "reflect" + "sort" "strings" "sync" @@ -16,6 +17,7 @@ import ( "github.com/Finschia/ostracon/crypto/tmhash" "github.com/Finschia/ostracon/libs/log" dbm "github.com/tendermint/tm-db" + "golang.org/x/exp/maps" "github.com/Finschia/finschia-sdk/codec/types" "github.com/Finschia/finschia-sdk/server/config" @@ -272,7 +274,10 @@ func (app *BaseApp) MountKVStores(keys map[string]*sdk.KVStoreKey) { // MountMemoryStores mounts all in-memory KVStores with the BaseApp's internal // commit multi-store. func (app *BaseApp) MountMemoryStores(keys map[string]*sdk.MemoryStoreKey) { - for _, memKey := range keys { + skeys := maps.Keys(keys) + sort.Strings(skeys) + for _, key := range skeys { + memKey := keys[key] app.MountStore(memKey, sdk.StoreTypeMemory) } } diff --git a/store/rootmulti/store.go b/store/rootmulti/store.go index 8165debd66..703cc1b6e5 100644 --- a/store/rootmulti/store.go +++ b/store/rootmulti/store.go @@ -41,6 +41,19 @@ const ( const iavlDisablefastNodeDefault = true +// keysFromStoreKeyMap returns a slice of keys for the provided map lexically sorted by StoreKey.Name() +func keysFromStoreKeyMap[V any](m map[types.StoreKey]V) []types.StoreKey { + keys := make([]types.StoreKey, 0, len(m)) + for key := range m { + keys = append(keys, key) + } + sort.Slice(keys, func(i, j int) bool { + ki, kj := keys[i], keys[j] + return ki.Name() < kj.Name() + }) + return keys +} + // Store is composed of many CommitStores. Name contrasts with // cacheMultiStore which is used for branching other MultiStores. It implements // the CommitMultiStore interface. @@ -691,8 +704,9 @@ func (rs *Store) Snapshot(height uint64, protoWriter protoio.Writer) error { *iavl.Store name string } - stores := []namedStore{} - for key := range rs.stores { + stores := make([]namedStore, 0) + keys := keysFromStoreKeyMap(rs.stores) + for _, key := range keys { switch store := rs.GetCommitKVStore(key).(type) { case *iavl.Store: stores = append(stores, namedStore{name: key.Name(), Store: store}) @@ -900,9 +914,12 @@ func (rs *Store) loadCommitStoreFromParams(key types.StoreKey, id types.CommitID } func (rs *Store) buildCommitInfo(version int64) *types.CommitInfo { + keys := keysFromStoreKeyMap(rs.stores) storeInfos := []types.StoreInfo{} - for key, store := range rs.stores { - if store.GetStoreType() == types.StoreTypeTransient { + for _, key := range keys { + store := rs.stores[key] + storeType := store.GetStoreType() + if storeType == types.StoreTypeTransient || storeType == types.StoreTypeMemory { continue } storeInfos = append(storeInfos, types.StoreInfo{ diff --git a/types/module/module.go b/types/module/module.go index 527d3bcd02..8d932455f9 100644 --- a/types/module/module.go +++ b/types/module/module.go @@ -38,6 +38,7 @@ import ( ocabci "github.com/Finschia/ostracon/abci/types" abci "github.com/tendermint/tendermint/abci/types" + "golang.org/x/exp/maps" "github.com/Finschia/finschia-sdk/client" "github.com/Finschia/finschia-sdk/codec" @@ -351,13 +352,16 @@ func (m *Manager) assertNoForgottenModules(setOrderFnName string, moduleNames [] for _, m := range moduleNames { ms[m] = true } + + allKeys := maps.Keys(m.Modules) var missing []string - for m := range m.Modules { + for _, m := range allKeys { if !ms[m] { missing = append(missing, m) } } if len(missing) != 0 { + sort.Strings(missing) panic(fmt.Sprintf( "%s: all modules must be defined when setting %s, missing: %v", setOrderFnName, setOrderFnName, missing)) }