From 776bd85ded8194bd668bc9e20b6176b62f754b96 Mon Sep 17 00:00:00 2001 From: Evgenii Stratonikov Date: Wed, 27 May 2020 09:27:43 +0300 Subject: [PATCH 1/3] vm,dao: return storage iterator from DAO in Storage.Find interop Reproduce behavior of the reference realization: - if item was Put in cache after it was encountered during Storage.Find, it must appear twice - checking if item is in cache must be performed in real-time during `Iterator.Next()` --- pkg/core/dao/cacheddao.go | 46 +++++++++++++++++++++++++++++++++++++++ pkg/core/interop_neo.go | 17 +++------------ pkg/core/storage_find.go | 44 +++++++++++++++++++++++++++++++++++++ 3 files changed, 93 insertions(+), 14 deletions(-) create mode 100644 pkg/core/storage_find.go diff --git a/pkg/core/dao/cacheddao.go b/pkg/core/dao/cacheddao.go index 327501157..f35627712 100644 --- a/pkg/core/dao/cacheddao.go +++ b/pkg/core/dao/cacheddao.go @@ -315,6 +315,52 @@ func (cd *Cached) DeleteStorageItem(scripthash util.Uint160, key []byte) error { return nil } +// StorageIteratorFunc is a function returning key-value pair or error. +type StorageIteratorFunc func() ([]byte, []byte, error) + +// GetStorageItemsIterator returns iterator over all storage items. +// Function returned can be called until first error. +func (cd *Cached) GetStorageItemsIterator(hash util.Uint160, prefix []byte) (StorageIteratorFunc, error) { + items, err := cd.DAO.GetStorageItems(hash) + if err != nil { + return nil, err + } + + sort.Slice(items, func(i, j int) bool { return bytes.Compare(items[i].Key, items[j].Key) == -1 }) + + cache := cd.storage.getItems(hash) + + var getItemFromCache StorageIteratorFunc + keyIndex := -1 + getItemFromCache = func() ([]byte, []byte, error) { + keyIndex++ + for ; keyIndex < len(cd.storage.keys[hash]); keyIndex++ { + k := cd.storage.keys[hash][keyIndex] + v := cache[k] + if v.State != delOp && bytes.HasPrefix([]byte(k), prefix) { + val := make([]byte, len(v.StorageItem.Value)) + copy(val, v.StorageItem.Value) + return []byte(k), val, nil + } + } + return nil, nil, errors.New("no more items") + } + + var f func() ([]byte, []byte, error) + index := -1 + f = func() ([]byte, []byte, error) { + index++ + for ; index < len(items); index++ { + _, ok := cache[string(items[index].Key)] + if !ok && bytes.HasPrefix(items[index].Key, prefix) { + return items[index].Key, items[index].Value, nil + } + } + return getItemFromCache() + } + return f, nil +} + // GetStorageItems returns all storage items for a given scripthash. func (cd *Cached) GetStorageItems(hash util.Uint160) ([]StorageItemWithKey, error) { items, err := cd.DAO.GetStorageItems(hash) diff --git a/pkg/core/interop_neo.go b/pkg/core/interop_neo.go index 64cb44614..ccddb26b6 100644 --- a/pkg/core/interop_neo.go +++ b/pkg/core/interop_neo.go @@ -1,7 +1,6 @@ package core import ( - "bytes" "errors" "fmt" "math" @@ -463,22 +462,12 @@ func (ic *interopContext) storageFind(v *vm.VM) error { return err } pref := v.Estack().Pop().Bytes() - siMap, err := ic.dao.GetStorageItems(stc.ScriptHash) + next, err := ic.dao.GetStorageItemsIterator(stc.ScriptHash, pref) if err != nil { return err } - - filteredMap := vm.NewMapItem() - for i := range siMap { - k := siMap[i].Key - if bytes.HasPrefix(k, pref) { - filteredMap.Add(vm.NewByteArrayItem(siMap[i].Key), - vm.NewByteArrayItem(siMap[i].Value)) - } - } - - item := vm.NewMapIterator(filteredMap) - v.Estack().PushVal(item) + item := newStorageIterator(next) + v.Estack().PushVal(vm.NewInteropItem(item)) return nil } diff --git a/pkg/core/storage_find.go b/pkg/core/storage_find.go new file mode 100644 index 000000000..14231896d --- /dev/null +++ b/pkg/core/storage_find.go @@ -0,0 +1,44 @@ +package core + +import ( + "github.com/nspcc-dev/neo-go/pkg/core/dao" + "github.com/nspcc-dev/neo-go/pkg/vm" +) + +type storageWrapper struct { + next dao.StorageIteratorFunc + key, value vm.StackItem + finished bool +} + +// newStorageIterator returns new storage iterator from the `next()` callback. +func newStorageIterator(next dao.StorageIteratorFunc) *storageWrapper { + return &storageWrapper{ + next: next, + } +} + +// Next implements iterator interface. +func (s *storageWrapper) Next() bool { + if s.finished { + return false + } + key, value, err := s.next() + if err != nil { + s.finished = true + return false + } + s.key = vm.NewByteArrayItem(key) + s.value = vm.NewByteArrayItem(value) + return true +} + +// Value implements iterator interface. +func (s *storageWrapper) Value() vm.StackItem { + return s.value +} + +// Key implements iterator interface. +func (s *storageWrapper) Key() vm.StackItem { + return s.key +} From 503442a60d837df62e3715cf9d9f7e16f89a5b6f Mon Sep 17 00:00:00 2001 From: Evgenii Stratonikov Date: Wed, 27 May 2020 10:56:47 +0300 Subject: [PATCH 2/3] dao: restrict GetStorageItems by prefix All storage items can still be retrived via zero-length prefix. --- pkg/core/blockchain.go | 2 +- pkg/core/dao/cacheddao.go | 8 ++++---- pkg/core/dao/dao.go | 6 +++--- pkg/core/interop_neo.go | 2 +- pkg/core/interop_system.go | 2 +- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/pkg/core/blockchain.go b/pkg/core/blockchain.go index a507db63f..aa452bf86 100644 --- a/pkg/core/blockchain.go +++ b/pkg/core/blockchain.go @@ -1147,7 +1147,7 @@ func (bc *Blockchain) GetStorageItem(scripthash util.Uint160, key []byte) *state // GetStorageItems returns all storage items for a given scripthash. func (bc *Blockchain) GetStorageItems(hash util.Uint160) (map[string]*state.StorageItem, error) { - siMap, err := bc.dao.GetStorageItems(hash) + siMap, err := bc.dao.GetStorageItems(hash, nil) if err != nil { return nil, err } diff --git a/pkg/core/dao/cacheddao.go b/pkg/core/dao/cacheddao.go index f35627712..39e65cc22 100644 --- a/pkg/core/dao/cacheddao.go +++ b/pkg/core/dao/cacheddao.go @@ -321,7 +321,7 @@ type StorageIteratorFunc func() ([]byte, []byte, error) // GetStorageItemsIterator returns iterator over all storage items. // Function returned can be called until first error. func (cd *Cached) GetStorageItemsIterator(hash util.Uint160, prefix []byte) (StorageIteratorFunc, error) { - items, err := cd.DAO.GetStorageItems(hash) + items, err := cd.DAO.GetStorageItems(hash, prefix) if err != nil { return nil, err } @@ -352,7 +352,7 @@ func (cd *Cached) GetStorageItemsIterator(hash util.Uint160, prefix []byte) (Sto index++ for ; index < len(items); index++ { _, ok := cache[string(items[index].Key)] - if !ok && bytes.HasPrefix(items[index].Key, prefix) { + if !ok { return items[index].Key, items[index].Value, nil } } @@ -362,8 +362,8 @@ func (cd *Cached) GetStorageItemsIterator(hash util.Uint160, prefix []byte) (Sto } // GetStorageItems returns all storage items for a given scripthash. -func (cd *Cached) GetStorageItems(hash util.Uint160) ([]StorageItemWithKey, error) { - items, err := cd.DAO.GetStorageItems(hash) +func (cd *Cached) GetStorageItems(hash util.Uint160, prefix []byte) ([]StorageItemWithKey, error) { + items, err := cd.DAO.GetStorageItems(hash, prefix) if err != nil { return nil, err } diff --git a/pkg/core/dao/dao.go b/pkg/core/dao/dao.go index 41a892058..7126969ac 100644 --- a/pkg/core/dao/dao.go +++ b/pkg/core/dao/dao.go @@ -35,7 +35,7 @@ type DAO interface { GetNEP5Balances(acc util.Uint160) (*state.NEP5Balances, error) GetNEP5TransferLog(acc util.Uint160, index uint32) (*state.NEP5TransferLog, error) GetStorageItem(scripthash util.Uint160, key []byte) *state.StorageItem - GetStorageItems(hash util.Uint160) ([]StorageItemWithKey, error) + GetStorageItems(hash util.Uint160, prefix []byte) ([]StorageItemWithKey, error) GetTransaction(hash util.Uint256) (*transaction.Transaction, uint32, error) GetUnspentCoinState(hash util.Uint256) (*state.UnspentCoin, error) GetValidatorState(publicKey *keys.PublicKey) (*state.Validator, error) @@ -442,7 +442,7 @@ type StorageItemWithKey struct { } // GetStorageItems returns all storage items for a given scripthash. -func (dao *Simple) GetStorageItems(hash util.Uint160) ([]StorageItemWithKey, error) { +func (dao *Simple) GetStorageItems(hash util.Uint160, prefix []byte) ([]StorageItemWithKey, error) { var res []StorageItemWithKey var err error @@ -462,7 +462,7 @@ func (dao *Simple) GetStorageItems(hash util.Uint160) ([]StorageItemWithKey, err s.Key = k[21:] res = append(res, s) } - dao.Store.Seek(storage.AppendPrefix(storage.STStorage, hash.BytesLE()), saveToMap) + dao.Store.Seek(makeStorageItemKey(hash, prefix), saveToMap) if err != nil { return nil, err } diff --git a/pkg/core/interop_neo.go b/pkg/core/interop_neo.go index ccddb26b6..cd3c0d8fd 100644 --- a/pkg/core/interop_neo.go +++ b/pkg/core/interop_neo.go @@ -582,7 +582,7 @@ func (ic *interopContext) contractMigrate(v *vm.VM) error { } if contract.HasStorage() { hash := getContextScriptHash(v, 0) - siMap, err := ic.dao.GetStorageItems(hash) + siMap, err := ic.dao.GetStorageItems(hash, nil) if err != nil { return err } diff --git a/pkg/core/interop_system.go b/pkg/core/interop_system.go index bfa669bda..84855717f 100644 --- a/pkg/core/interop_system.go +++ b/pkg/core/interop_system.go @@ -561,7 +561,7 @@ func (ic *interopContext) contractDestroy(v *vm.VM) error { return err } if cs.HasStorage() { - siMap, err := ic.dao.GetStorageItems(hash) + siMap, err := ic.dao.GetStorageItems(hash, nil) if err != nil { return err } From b0d07c30311d7199b359d2ef1c6d77eb016b97f5 Mon Sep 17 00:00:00 2001 From: Evgenii Stratonikov Date: Wed, 27 May 2020 11:09:23 +0300 Subject: [PATCH 3/3] vm: make Iterator interface public There is nothing wrong with iterators being implemented in other parts of code (e.g. Storage.Find). In this case type assertions can prevent bugs at compile-time. --- pkg/core/storage_find.go | 8 +++++--- pkg/vm/interop.go | 10 +++++----- pkg/vm/interop_iterators.go | 11 ++++++----- 3 files changed, 16 insertions(+), 13 deletions(-) diff --git a/pkg/core/storage_find.go b/pkg/core/storage_find.go index 14231896d..740a9edd5 100644 --- a/pkg/core/storage_find.go +++ b/pkg/core/storage_find.go @@ -11,6 +11,8 @@ type storageWrapper struct { finished bool } +var _ vm.Iterator = (*storageWrapper)(nil) + // newStorageIterator returns new storage iterator from the `next()` callback. func newStorageIterator(next dao.StorageIteratorFunc) *storageWrapper { return &storageWrapper{ @@ -18,7 +20,7 @@ func newStorageIterator(next dao.StorageIteratorFunc) *storageWrapper { } } -// Next implements iterator interface. +// Next implements vm.Iterator interface. func (s *storageWrapper) Next() bool { if s.finished { return false @@ -33,12 +35,12 @@ func (s *storageWrapper) Next() bool { return true } -// Value implements iterator interface. +// Value implements vm.Iterator interface. func (s *storageWrapper) Value() vm.StackItem { return s.value } -// Key implements iterator interface. +// Key implements vm.Iterator interface. func (s *storageWrapper) Key() vm.StackItem { return s.key } diff --git a/pkg/vm/interop.go b/pkg/vm/interop.go index d1ec282ba..b8a2cbf21 100644 --- a/pkg/vm/interop.go +++ b/pkg/vm/interop.go @@ -205,9 +205,9 @@ func NewMapIterator(m *MapItem) *InteropItem { // IteratorConcat handles syscall Neo.Iterator.Concat. func IteratorConcat(v *VM) error { iop1 := v.Estack().Pop().Interop() - iter1 := iop1.value.(iterator) + iter1 := iop1.value.(Iterator) iop2 := v.Estack().Pop().Interop() - iter2 := iop2.value.(iterator) + iter2 := iop2.value.(Iterator) v.Estack().Push(&Element{value: NewInteropItem( &concatIter{ @@ -222,7 +222,7 @@ func IteratorConcat(v *VM) error { // IteratorKey handles syscall Neo.Iterator.Key. func IteratorKey(v *VM) error { iop := v.estack.Pop().Interop() - iter := iop.value.(iterator) + iter := iop.value.(Iterator) v.Estack().Push(&Element{value: iter.Key()}) return nil @@ -231,7 +231,7 @@ func IteratorKey(v *VM) error { // IteratorKeys handles syscall Neo.Iterator.Keys. func IteratorKeys(v *VM) error { iop := v.estack.Pop().Interop() - iter := iop.value.(iterator) + iter := iop.value.(Iterator) v.Estack().Push(&Element{value: NewInteropItem( &keysWrapper{iter}, )}) @@ -242,7 +242,7 @@ func IteratorKeys(v *VM) error { // IteratorValues handles syscall Neo.Iterator.Values. func IteratorValues(v *VM) error { iop := v.estack.Pop().Interop() - iter := iop.value.(iterator) + iter := iop.value.(Iterator) v.Estack().Push(&Element{value: NewInteropItem( &valuesWrapper{iter}, )}) diff --git a/pkg/vm/interop_iterators.go b/pkg/vm/interop_iterators.go index 8ecf69b83..eb415de35 100644 --- a/pkg/vm/interop_iterators.go +++ b/pkg/vm/interop_iterators.go @@ -18,7 +18,8 @@ type ( ) type ( - iterator interface { + // Iterator defined public interface for VM's iterator type. + Iterator interface { enumerator Key() StackItem } @@ -29,16 +30,16 @@ type ( } concatIter struct { - current iterator - second iterator + current Iterator + second Iterator } keysWrapper struct { - iter iterator + iter Iterator } valuesWrapper struct { - iter iterator + iter Iterator } )