Skip to content

Commit

Permalink
fix: remove map iteration non-determinism with keys (#1302)
Browse files Browse the repository at this point in the history
* remove map iteration non-determinism with keys

* add CHANGELOG

(cherry picked from commit c462f36)

# Conflicts:
#	CHANGELOG.md
  • Loading branch information
tkxkd0159 authored and mergify[bot] committed Mar 26, 2024
1 parent 971875b commit b384d08
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 6 deletions.
12 changes: 12 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,23 @@ Ref: https://keepachangelog.com/en/1.0.0/
### Improvements

### Bug Fixes
<<<<<<< HEAD
* (x/auth) [#1281](https://github.com/Finschia/finschia-sdk/pull/1281) `ModuleAccount.Validate` now reports a nil `.BaseAccount` instead of panicking. (backport #1274)
* (x/foundation) [\#1283](https://github.com/Finschia/finschia-sdk/pull/1283) add init logic of foundation module accounts to InitGenesis in order to eliminate potential panic (backport #1277)
* (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
=======
* chore(deps) [\#1141](https://github.com/Finschia/finschia-sdk/pull/1141) Bump github.com/cosmos/ledger-cosmos-go from 0.12.2 to 0.13.2 to fix ledger signing issue
* (x/auth, x/slashing) [\#1179](https://github.com/Finschia/finschia-sdk/pull/1179) modify missing changes of converting to tendermint
* (x/auth) [#1274](https://github.com/Finschia/finschia-sdk/pull/1274) `ModuleAccount.Validate` now reports a nil `.BaseAccount` instead of panicking.
* (x/collection) [\#1276](https://github.com/Finschia/finschia-sdk/pull/1276) eliminates potential risk for Insufficient Sanity Check of tokenID in Genesis
* (x/foundation) [\#1277](https://github.com/Finschia/finschia-sdk/pull/1277) add init logic of foundation module accounts to InitGenesis in order to eliminate potential panic
* (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
>>>>>>> c462f3686 (fix: remove map iteration non-determinism with keys (#1302))
### Removed

Expand Down
7 changes: 6 additions & 1 deletion baseapp/baseapp.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"errors"
"fmt"
"reflect"
"sort"
"strings"
"sync"

Expand All @@ -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"
Expand Down Expand Up @@ -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)
}
}
Expand Down
25 changes: 21 additions & 4 deletions store/rootmulti/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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})
Expand Down Expand Up @@ -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{
Expand Down
6 changes: 5 additions & 1 deletion types/module/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)

Check warning on line 364 in types/module/module.go

View check run for this annotation

Codecov / codecov/patch

types/module/module.go#L364

Added line #L364 was not covered by tests
panic(fmt.Sprintf(
"%s: all modules must be defined when setting %s, missing: %v", setOrderFnName, setOrderFnName, missing))
}
Expand Down

0 comments on commit b384d08

Please sign in to comment.