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
  • Loading branch information
tkxkd0159 authored Mar 26, 2024
1 parent ca39b18 commit c462f36
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 6 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

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"

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

0 comments on commit c462f36

Please sign in to comment.