From c462f368628c092dafe48c254114143dbf596b35 Mon Sep 17 00:00:00 2001
From: Jayden Lee <41176085+tkxkd0159@users.noreply.github.com>
Date: Tue, 26 Mar 2024 17:33:47 +0900
Subject: [PATCH] fix: remove map iteration non-determinism with keys (#1302)

* remove map iteration non-determinism with keys

* add CHANGELOG
---
 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 2f53c61cd4..60d73d53b4 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -60,6 +60,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 * (x/collection, x/token) [\#1288](https://github.com/Finschia/finschia-sdk/pull/1288) use accAddress to compare in validatebasic function in collection & token modules
 * (x/collection) [\#1268](https://github.com/Finschia/finschia-sdk/pull/1268) export x/collection params into genesis
 * (x/collection) [\#1294](https://github.com/Finschia/finschia-sdk/pull/1294) reject NFT coins on FT APIs
+* (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 493eb08fa4..da36d40b24 100644
--- a/baseapp/baseapp.go
+++ b/baseapp/baseapp.go
@@ -4,6 +4,7 @@ import (
 	"errors"
 	"fmt"
 	"reflect"
+	"sort"
 	"strings"
 
 	"github.com/gogo/protobuf/proto"
@@ -12,6 +13,7 @@ import (
 	"github.com/tendermint/tendermint/libs/log"
 	tmproto "github.com/tendermint/tendermint/proto/tendermint/types"
 	dbm "github.com/tendermint/tm-db"
+	"golang.org/x/exp/maps"
 
 	"github.com/Finschia/finschia-sdk/codec/types"
 	"github.com/Finschia/finschia-sdk/snapshots"
@@ -273,7 +275,10 @@ func (app *BaseApp) MountTransientStores(keys map[string]*sdk.TransientStoreKey)
 // 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 7f8d20fa10..3aef50bcca 100644
--- a/store/rootmulti/store.go
+++ b/store/rootmulti/store.go
@@ -40,6 +40,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.
@@ -690,8 +703,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})
@@ -899,9 +913,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 2a3367fbc9..390dfca5e4 100644
--- a/types/module/module.go
+++ b/types/module/module.go
@@ -36,6 +36,7 @@ import (
 	"github.com/grpc-ecosystem/grpc-gateway/runtime"
 	"github.com/spf13/cobra"
 	abci "github.com/tendermint/tendermint/abci/types"
+	"golang.org/x/exp/maps"
 
 	"github.com/Finschia/finschia-sdk/client"
 	"github.com/Finschia/finschia-sdk/codec"
@@ -349,13 +350,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))
 	}