From 72726d46d36cb30a759b4185032ea7d755fc8b90 Mon Sep 17 00:00:00 2001 From: Anna Shaleva Date: Fri, 24 Sep 2021 17:22:45 +0300 Subject: [PATCH] core: refactor callers of MemCachedStore.Seek MemCachedStore.Seek now sorts results, so its callers may omit sorting. --- internal/fakechain/fakechain.go | 2 +- pkg/core/blockchain.go | 2 +- pkg/core/blockchainer/blockchainer.go | 2 +- pkg/core/dao/dao.go | 23 ++++++++++-------- pkg/core/interop_system.go | 21 +++++++---------- pkg/core/native/designate.go | 21 ++++++++++------- pkg/core/native/management.go | 6 ++--- pkg/core/native/native_neo.go | 34 ++++++++++++--------------- pkg/core/native/oracle.go | 12 +++++----- pkg/core/native/policy.go | 12 +++++----- pkg/core/state/storage_item.go | 6 +++++ 11 files changed, 73 insertions(+), 68 deletions(-) diff --git a/internal/fakechain/fakechain.go b/internal/fakechain/fakechain.go index 0054d246b..ae08e4ad3 100644 --- a/internal/fakechain/fakechain.go +++ b/internal/fakechain/fakechain.go @@ -332,7 +332,7 @@ func (chain *FakeChain) GetTestVM(t trigger.Type, tx *transaction.Transaction, b } // GetStorageItems implements Blockchainer interface. -func (chain *FakeChain) GetStorageItems(id int32) (map[string]state.StorageItem, error) { +func (chain *FakeChain) GetStorageItems(id int32) ([]state.StorageItemWithKey, error) { panic("TODO") } diff --git a/pkg/core/blockchain.go b/pkg/core/blockchain.go index a5d9aeecd..4d1217b89 100644 --- a/pkg/core/blockchain.go +++ b/pkg/core/blockchain.go @@ -1494,7 +1494,7 @@ func (bc *Blockchain) GetStorageItem(id int32, key []byte) state.StorageItem { } // GetStorageItems returns all storage items for a given contract id. -func (bc *Blockchain) GetStorageItems(id int32) (map[string]state.StorageItem, error) { +func (bc *Blockchain) GetStorageItems(id int32) ([]state.StorageItemWithKey, error) { return bc.dao.GetStorageItems(id) } diff --git a/pkg/core/blockchainer/blockchainer.go b/pkg/core/blockchainer/blockchainer.go index 97bd64611..e6619843c 100644 --- a/pkg/core/blockchainer/blockchainer.go +++ b/pkg/core/blockchainer/blockchainer.go @@ -58,7 +58,7 @@ type Blockchainer interface { GetStateModule() StateRoot GetStateSyncModule() StateSync GetStorageItem(id int32, key []byte) state.StorageItem - GetStorageItems(id int32) (map[string]state.StorageItem, error) + GetStorageItems(id int32) ([]state.StorageItemWithKey, error) GetTestVM(t trigger.Type, tx *transaction.Transaction, b *block.Block) *vm.VM GetTransaction(util.Uint256) (*transaction.Transaction, uint32, error) SetOracle(service services.Oracle) diff --git a/pkg/core/dao/dao.go b/pkg/core/dao/dao.go index 2a509344f..885db02cc 100644 --- a/pkg/core/dao/dao.go +++ b/pkg/core/dao/dao.go @@ -48,8 +48,8 @@ type DAO interface { GetStateSyncPoint() (uint32, error) GetStateSyncCurrentBlockHeight() (uint32, error) GetStorageItem(id int32, key []byte) state.StorageItem - GetStorageItems(id int32) (map[string]state.StorageItem, error) - GetStorageItemsWithPrefix(id int32, prefix []byte) (map[string]state.StorageItem, error) + GetStorageItems(id int32) ([]state.StorageItemWithKey, error) + GetStorageItemsWithPrefix(id int32, prefix []byte) ([]state.StorageItemWithKey, error) GetTransaction(hash util.Uint256) (*transaction.Transaction, uint32, error) GetVersion() (string, error) GetWrapped() DAO @@ -313,28 +313,31 @@ func (dao *Simple) DeleteStorageItem(id int32, key []byte) error { } // GetStorageItems returns all storage items for a given id. -func (dao *Simple) GetStorageItems(id int32) (map[string]state.StorageItem, error) { +func (dao *Simple) GetStorageItems(id int32) ([]state.StorageItemWithKey, error) { return dao.GetStorageItemsWithPrefix(id, nil) } // GetStorageItemsWithPrefix returns all storage items with given id for a // given scripthash. -func (dao *Simple) GetStorageItemsWithPrefix(id int32, prefix []byte) (map[string]state.StorageItem, error) { - var siMap = make(map[string]state.StorageItem) +func (dao *Simple) GetStorageItemsWithPrefix(id int32, prefix []byte) ([]state.StorageItemWithKey, error) { + var siArr []state.StorageItemWithKey - saveToMap := func(k, v []byte) { + saveToArr := func(k, v []byte) { // Cut prefix and hash. // Must copy here, #1468. key := slice.Copy(k) val := slice.Copy(v) - siMap[string(key)] = state.StorageItem(val) + siArr = append(siArr, state.StorageItemWithKey{ + Key: key, + Item: state.StorageItem(val), + }) } - dao.Seek(id, prefix, saveToMap) - return siMap, nil + dao.Seek(id, prefix, saveToArr) + return siArr, nil } // Seek executes f for all items with a given prefix. -// If key is to be used outside of f, they must be copied. +// If key is to be used outside of f, they may not be copied. func (dao *Simple) Seek(id int32, prefix []byte, f func(k, v []byte)) { lookupKey := makeStorageItemKey(id, nil) if prefix != nil { diff --git a/pkg/core/interop_system.go b/pkg/core/interop_system.go index a56ef18cd..f3d9f1a87 100644 --- a/pkg/core/interop_system.go +++ b/pkg/core/interop_system.go @@ -1,13 +1,11 @@ package core import ( - "bytes" "crypto/elliptic" "errors" "fmt" "math" "math/big" - "sort" "github.com/nspcc-dev/neo-go/pkg/core/block" "github.com/nspcc-dev/neo-go/pkg/core/interop" @@ -188,26 +186,23 @@ func storageFind(ic *interop.Context) error { if opts&istorage.FindDeserialize == 0 && (opts&istorage.FindPick0 != 0 || opts&istorage.FindPick1 != 0) { return fmt.Errorf("%w: PickN is specified without Deserialize", errFindInvalidOptions) } - siMap, err := ic.DAO.GetStorageItemsWithPrefix(stc.ID, prefix) + siArr, err := ic.DAO.GetStorageItemsWithPrefix(stc.ID, prefix) if err != nil { return err } - arr := make([]stackitem.MapElement, 0, len(siMap)) - for k, v := range siMap { - keycopy := make([]byte, len(k)+len(prefix)) + arr := make([]stackitem.MapElement, 0, len(siArr)) + for _, kv := range siArr { + keycopy := make([]byte, len(kv.Key)+len(prefix)) copy(keycopy, prefix) - copy(keycopy[len(prefix):], k) + copy(keycopy[len(prefix):], kv.Key) arr = append(arr, stackitem.MapElement{ Key: stackitem.NewByteArray(keycopy), - Value: stackitem.NewByteArray(v), + Value: stackitem.NewByteArray(kv.Item), }) } - sort.Slice(arr, func(i, j int) bool { - k1 := arr[i].Key.Value().([]byte) - k2 := arr[j].Key.Value().([]byte) - return bytes.Compare(k1, k2) == -1 - }) + // Items in arr should be sorted by key, but GetStorageItemsWithPrefix returns + // sorted items, so no need to sort them one more time. filteredMap := stackitem.NewMapWithValue(arr) item := istorage.NewIterator(filteredMap, len(prefix), opts) diff --git a/pkg/core/native/designate.go b/pkg/core/native/designate.go index 844690a3d..ccf5767b8 100644 --- a/pkg/core/native/designate.go +++ b/pkg/core/native/designate.go @@ -257,17 +257,22 @@ func (s *Designate) GetDesignatedByRole(d dao.DAO, r noderoles.Role, index uint3 if err != nil { return nil, 0, err } - var ns NodeList - var bestIndex uint32 - var resSi state.StorageItem - for k, si := range kvs { - if len(k) < 4 { + var ( + ns NodeList + bestIndex uint32 + resSi state.StorageItem + ) + // kvs are sorted by key (BE index bytes) in ascending way, so iterate backwards to get the latest designated. + for i := len(kvs) - 1; i >= 0; i-- { + kv := kvs[i] + if len(kv.Key) < 4 { continue } - siInd := binary.BigEndian.Uint32([]byte(k)) - if (resSi == nil || siInd > bestIndex) && siInd <= index { + siInd := binary.BigEndian.Uint32(kv.Key) + if siInd <= index { bestIndex = siInd - resSi = si + resSi = kv.Item + break } } if resSi != nil { diff --git a/pkg/core/native/management.go b/pkg/core/native/management.go index c66e9dc8d..7403559e7 100644 --- a/pkg/core/native/management.go +++ b/pkg/core/native/management.go @@ -391,12 +391,12 @@ func (m *Management) Destroy(d dao.DAO, hash util.Uint160) error { if err != nil { return err } - siMap, err := d.GetStorageItems(contract.ID) + siArr, err := d.GetStorageItems(contract.ID) if err != nil { return err } - for k := range siMap { - err := d.DeleteStorageItem(contract.ID, []byte(k)) + for _, kv := range siArr { + err := d.DeleteStorageItem(contract.ID, []byte(kv.Key)) if err != nil { return err } diff --git a/pkg/core/native/native_neo.go b/pkg/core/native/native_neo.go index 9df6e468b..53604bca3 100644 --- a/pkg/core/native/native_neo.go +++ b/pkg/core/native/native_neo.go @@ -471,24 +471,20 @@ func (n *NEO) getGASPerBlock(ic *interop.Context, _ []stackitem.Item) stackitem. } func (n *NEO) getSortedGASRecordFromDAO(d dao.DAO) (gasRecord, error) { - grMap, err := d.GetStorageItemsWithPrefix(n.ID, []byte{prefixGASPerBlock}) + grArr, err := d.GetStorageItemsWithPrefix(n.ID, []byte{prefixGASPerBlock}) if err != nil { return gasRecord{}, fmt.Errorf("failed to get gas records from storage: %w", err) } - var ( - i int - gr = make(gasRecord, len(grMap)) - ) - for indexBytes, gasValue := range grMap { + var gr = make(gasRecord, len(grArr)) + for i, kv := range grArr { + indexBytes, gasValue := kv.Key, kv.Item gr[i] = gasIndexPair{ Index: binary.BigEndian.Uint32([]byte(indexBytes)), GASPerBlock: *bigint.FromBytes(gasValue), } - i++ } - sort.Slice(gr, func(i, j int) bool { - return gr[i].Index < gr[j].Index - }) + // GAS records should be sorted by index, but GetStorageItemsWithPrefix returns + // values sorted by BE bytes of index, so we're OK with that. return gr, nil } @@ -836,21 +832,21 @@ func (n *NEO) ModifyAccountVotes(acc *state.NEOBalance, d dao.DAO, value *big.In } func (n *NEO) getCandidates(d dao.DAO, sortByKey bool) ([]keyWithVotes, error) { - siMap, err := d.GetStorageItemsWithPrefix(n.ID, []byte{prefixCandidate}) + siArr, err := d.GetStorageItemsWithPrefix(n.ID, []byte{prefixCandidate}) if err != nil { return nil, err } - arr := make([]keyWithVotes, 0, len(siMap)) - for key, si := range siMap { - c := new(candidate).FromBytes(si) + arr := make([]keyWithVotes, 0, len(siArr)) + for _, kv := range siArr { + c := new(candidate).FromBytes(kv.Item) if c.Registered { - arr = append(arr, keyWithVotes{Key: key, Votes: &c.Votes}) + arr = append(arr, keyWithVotes{Key: string(kv.Key), Votes: &c.Votes}) } } - if sortByKey { - // Sort by serialized key bytes (that's the way keys are stored and retrieved from the storage by default). - sort.Slice(arr, func(i, j int) bool { return strings.Compare(arr[i].Key, arr[j].Key) == -1 }) - } else { + if !sortByKey { + // sortByKey assumes to sort by serialized key bytes (that's the way keys + // are stored and retrieved from the storage by default). Otherwise, need + // to sort using big.Int comparator. sort.Slice(arr, func(i, j int) bool { // The most-voted validators should end up in the front of the list. cmp := arr[i].Votes.Cmp(arr[j].Votes) diff --git a/pkg/core/native/oracle.go b/pkg/core/native/oracle.go index c060a9739..393c0d979 100644 --- a/pkg/core/native/oracle.go +++ b/pkg/core/native/oracle.go @@ -483,21 +483,21 @@ func (o *Oracle) getOriginalTxID(d dao.DAO, tx *transaction.Transaction) util.Ui // getRequests returns all requests which have not been finished yet. func (o *Oracle) getRequests(d dao.DAO) (map[uint64]*state.OracleRequest, error) { - m, err := d.GetStorageItemsWithPrefix(o.ID, prefixRequest) + arr, err := d.GetStorageItemsWithPrefix(o.ID, prefixRequest) if err != nil { return nil, err } - reqs := make(map[uint64]*state.OracleRequest, len(m)) - for k, si := range m { - if len(k) != 8 { + reqs := make(map[uint64]*state.OracleRequest, len(arr)) + for _, kv := range arr { + if len(kv.Key) != 8 { return nil, errors.New("invalid request ID") } req := new(state.OracleRequest) - err = stackitem.DeserializeConvertible(si, req) + err = stackitem.DeserializeConvertible(kv.Item, req) if err != nil { return nil, err } - id := binary.BigEndian.Uint64([]byte(k)) + id := binary.BigEndian.Uint64([]byte(kv.Key)) reqs[id] = req } return reqs, nil diff --git a/pkg/core/native/policy.go b/pkg/core/native/policy.go index e377f0447..dffe1371c 100644 --- a/pkg/core/native/policy.go +++ b/pkg/core/native/policy.go @@ -162,20 +162,20 @@ func (p *Policy) PostPersist(ic *interop.Context) error { p.storagePrice = uint32(getIntWithKey(p.ID, ic.DAO, storagePriceKey)) p.blockedAccounts = make([]util.Uint160, 0) - siMap, err := ic.DAO.GetStorageItemsWithPrefix(p.ID, []byte{blockedAccountPrefix}) + siArr, err := ic.DAO.GetStorageItemsWithPrefix(p.ID, []byte{blockedAccountPrefix}) if err != nil { return fmt.Errorf("failed to get blocked accounts from storage: %w", err) } - for key := range siMap { - hash, err := util.Uint160DecodeBytesBE([]byte(key)) + for _, kv := range siArr { + hash, err := util.Uint160DecodeBytesBE([]byte(kv.Key)) if err != nil { return fmt.Errorf("failed to decode blocked account hash: %w", err) } p.blockedAccounts = append(p.blockedAccounts, hash) } - sort.Slice(p.blockedAccounts, func(i, j int) bool { - return p.blockedAccounts[i].Less(p.blockedAccounts[j]) - }) + // blockedAccounts should be sorted by account BE bytes, but GetStorageItemsWithPrefix + // returns values sorted by key (which is account's BE bytes), so don't need to sort + // one more time. p.isValid = true return nil diff --git a/pkg/core/state/storage_item.go b/pkg/core/state/storage_item.go index ba9777cf8..0a6eebb23 100644 --- a/pkg/core/state/storage_item.go +++ b/pkg/core/state/storage_item.go @@ -2,3 +2,9 @@ package state // StorageItem is the value to be stored with read-only flag. type StorageItem []byte + +// StorageItemWithKey is a storage item with corresponding key. +type StorageItemWithKey struct { + Key []byte + Item StorageItem +}