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 c462f36) # Conflicts: # CHANGELOG.md * Update CHANGELOG.md --------- Co-authored-by: Jayden Lee <[email protected]> Co-authored-by: Youngtaek Yoon <[email protected]>
Finschia · Mar 29, 2024 · 8e8a284 · 8e8a284
1 parent 971875b
commit 8e8a284
Show file tree

Hide file tree

Showing 4 changed files with 33 additions and 6 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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))
 	}