From 3dbe549a6105ea60ffa90dc57aa045e9e88cc84d Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Mon, 30 Mar 2020 18:24:06 +0300 Subject: [PATCH 1/3] smartcontract: store MapType Parameter as a slice of KV pairs Fixes #809. Basically, there are three alternative approaches to fixing it: * allowing both []byte and string for ByteArrayType value minimal invasion into existing code, but ugly as hell and will probably backfire at some point * storing string values in ByteArrayType incurs quite a number of type conversions (and associated data copying), though note that these values are not changed usually, so dynamic properties of []byte are almost irrelevant here * storing only []byte values in ByteArrayType makes it impossible to use them as map keys which can be solved in several ways: - via an interface (Marshalable) which is good, but makes testing and comparing values in general harder, because of keys mismatch - using serialized Parameter as a key (in a string) which will need some additional marshaling/unmarshaling - converting MapType from map to a slice of key-value pairs not a bad idea as we don't use this map as a map really, the type itself is all about input/output for real VM types and this approach is also a bit closer to JSON representation of the Map --- pkg/smartcontract/parameter.go | 90 +++++++++-------------------- pkg/smartcontract/parameter_test.go | 50 +++++++++++----- pkg/vm/stack_item.go | 10 ++-- pkg/vm/stack_item_test.go | 16 +++-- 4 files changed, 79 insertions(+), 87 deletions(-) diff --git a/pkg/smartcontract/parameter.go b/pkg/smartcontract/parameter.go index 0fc872785..bab39b36b 100644 --- a/pkg/smartcontract/parameter.go +++ b/pkg/smartcontract/parameter.go @@ -35,6 +35,13 @@ type Parameter struct { Value interface{} `json:"value"` } +// ParameterPair represents key-value pair, a slice of which is stored in +// MapType Parameter. +type ParameterPair struct { + Key Parameter `json:"key"` + Value Parameter `json:"value"` +} + // NewParameter returns a Parameter with proper initialized Value // of the given ParamType. func NewParameter(t ParamType) Parameter { @@ -49,16 +56,6 @@ type rawParameter struct { Value json.RawMessage `json:"value"` } -type keyValuePair struct { - Key rawParameter `json:"key"` - Value rawParameter `json:"value"` -} - -type rawKeyValuePair struct { - Key json.RawMessage `json:"key"` - Value json.RawMessage `json:"value"` -} - // MarshalJSON implements Marshaler interface. func (p *Parameter) MarshalJSON() ([]byte, error) { var ( @@ -95,28 +92,8 @@ func (p *Parameter) MarshalJSON() ([]byte, error) { } resultRawValue, resultErr = json.Marshal(value) case MapType: - var value []keyValuePair - for key, val := range p.Value.(map[Parameter]Parameter) { - rawKey, err := json.Marshal(key.Value) - if err != nil { - return nil, err - } - rawValue, err := json.Marshal(val.Value) - if err != nil { - return nil, err - } - value = append(value, keyValuePair{ - Key: rawParameter{ - Type: key.Type, - Value: rawKey, - }, - Value: rawParameter{ - Type: val.Type, - Value: rawValue, - }, - }) - } - resultRawValue, resultErr = json.Marshal(value) + ppair := p.Value.([]ParameterPair) + resultRawValue, resultErr = json.Marshal(ppair) default: resultErr = errors.Errorf("Marshaller for type %s not implemented", p.Type) } @@ -181,22 +158,11 @@ func (p *Parameter) UnmarshalJSON(data []byte) (err error) { } p.Value = rs case MapType: - var rawMap []rawKeyValuePair - if err = json.Unmarshal(r.Value, &rawMap); err != nil { + var ppair []ParameterPair + if err = json.Unmarshal(r.Value, &ppair); err != nil { return } - rs := make(map[Parameter]Parameter) - for _, p := range rawMap { - var key, value Parameter - if err = json.Unmarshal(p.Key, &key); err != nil { - return - } - if err = json.Unmarshal(p.Value, &value); err != nil { - return - } - rs[key] = value - } - p.Value = rs + p.Value = ppair case Hash160Type: var h util.Uint160 if err = json.Unmarshal(r.Value, &h); err != nil { @@ -234,13 +200,7 @@ func (p *Parameter) EncodeBinary(w *io.BinWriter) { case ArrayType: w.WriteArray(p.Value.([]Parameter)) case MapType: - m := p.Value.(map[Parameter]Parameter) - w.WriteVarUint(uint64(len(m))) - for k := range m { - v := m[k] - k.EncodeBinary(w) - v.EncodeBinary(w) - } + w.WriteArray(p.Value.([]ParameterPair)) case Hash160Type: w.WriteBytes(p.Value.(util.Uint160).BytesBE()) case Hash256Type: @@ -273,15 +233,9 @@ func (p *Parameter) DecodeBinary(r *io.BinReader) { r.ReadArray(&ps) p.Value = ps case MapType: - ln := r.ReadVarUint() - m := make(map[Parameter]Parameter, ln) - for i := uint64(0); i < ln; i++ { - var k, v Parameter - k.DecodeBinary(r) - v.DecodeBinary(r) - m[k] = v - } - p.Value = m + ps := []ParameterPair{} + r.ReadArray(&ps) + p.Value = ps case Hash160Type: var u util.Uint160 r.ReadBytes(u[:]) @@ -296,6 +250,18 @@ func (p *Parameter) DecodeBinary(r *io.BinReader) { } } +// EncodeBinary implements io.Serializable interface. +func (p *ParameterPair) EncodeBinary(w *io.BinWriter) { + p.Key.EncodeBinary(w) + p.Value.EncodeBinary(w) +} + +// DecodeBinary implements io.Serializable interface. +func (p *ParameterPair) DecodeBinary(r *io.BinReader) { + p.Key.DecodeBinary(r) + p.Value.DecodeBinary(r) +} + // Params is an array of Parameter (TODO: drop it?). type Params []Parameter diff --git a/pkg/smartcontract/parameter_test.go b/pkg/smartcontract/parameter_test.go index 4980dfd51..77fec6c21 100644 --- a/pkg/smartcontract/parameter_test.go +++ b/pkg/smartcontract/parameter_test.go @@ -72,9 +72,15 @@ var marshalJSONTestCases = []struct { { input: Parameter{ Type: MapType, - Value: map[Parameter]Parameter{ - {Type: StringType, Value: "key1"}: {Type: IntegerType, Value: int64(1)}, - {Type: StringType, Value: "key2"}: {Type: StringType, Value: "two"}, + Value: []ParameterPair{ + { + Key: Parameter{Type: StringType, Value: "key1"}, + Value: Parameter{Type: IntegerType, Value: int64(1)}, + }, + { + Key: Parameter{Type: StringType, Value: "key2"}, + Value: Parameter{Type: StringType, Value: "two"}, + }, }, }, result: `{"type":"Map","value":[{"key":{"type":"String","value":"key1"},"value":{"type":"Integer","value":1}},{"key":{"type":"String","value":"key2"},"value":{"type":"String","value":"two"}}]}`, @@ -82,11 +88,14 @@ var marshalJSONTestCases = []struct { { input: Parameter{ Type: MapType, - Value: map[Parameter]Parameter{ - {Type: StringType, Value: "key1"}: {Type: ArrayType, Value: []Parameter{ - {Type: StringType, Value: "str 1"}, - {Type: IntegerType, Value: int64(2)}, - }}, + Value: []ParameterPair{ + { + Key: Parameter{Type: StringType, Value: "key1"}, + Value: Parameter{Type: ArrayType, Value: []Parameter{ + {Type: StringType, Value: "str 1"}, + {Type: IntegerType, Value: int64(2)}, + }}, + }, }, }, result: `{"type":"Map","value":[{"key":{"type":"String","value":"key1"},"value":{"type":"Array","value":[{"type":"String","value":"str 1"},{"type":"Integer","value":2}]}}]}`, @@ -209,9 +218,15 @@ var unmarshalJSONTestCases = []struct { input: `{"type":"Map","value":[{"key":{"type":"String","value":"key1"},"value":{"type":"Integer","value":1}},{"key":{"type":"String","value":"key2"},"value":{"type":"String","value":"two"}}]}`, result: Parameter{ Type: MapType, - Value: map[Parameter]Parameter{ - {Type: StringType, Value: "key1"}: {Type: IntegerType, Value: int64(1)}, - {Type: StringType, Value: "key2"}: {Type: StringType, Value: "two"}, + Value: []ParameterPair{ + { + Key: Parameter{Type: StringType, Value: "key1"}, + Value: Parameter{Type: IntegerType, Value: int64(1)}, + }, + { + Key: Parameter{Type: StringType, Value: "key2"}, + Value: Parameter{Type: StringType, Value: "two"}, + }, }, }, }, @@ -219,11 +234,14 @@ var unmarshalJSONTestCases = []struct { input: `{"type":"Map","value":[{"key":{"type":"String","value":"key1"},"value":{"type":"Array","value":[{"type":"String","value":"str 1"},{"type":"Integer","value":2}]}}]}`, result: Parameter{ Type: MapType, - Value: map[Parameter]Parameter{ - {Type: StringType, Value: "key1"}: {Type: ArrayType, Value: []Parameter{ - {Type: StringType, Value: "str 1"}, - {Type: IntegerType, Value: int64(2)}, - }}, + Value: []ParameterPair{ + { + Key: Parameter{Type: StringType, Value: "key1"}, + Value: Parameter{Type: ArrayType, Value: []Parameter{ + {Type: StringType, Value: "str 1"}, + {Type: IntegerType, Value: int64(2)}, + }}, + }, }, }, }, diff --git a/pkg/vm/stack_item.go b/pkg/vm/stack_item.go index d4e71756a..b185e6e85 100644 --- a/pkg/vm/stack_item.go +++ b/pkg/vm/stack_item.go @@ -480,16 +480,16 @@ func (i *MapItem) Dup() StackItem { // ToContractParameter implements StackItem interface. func (i *MapItem) ToContractParameter(seen map[StackItem]bool) smartcontract.Parameter { - value := make(map[smartcontract.Parameter]smartcontract.Parameter) + value := make([]smartcontract.ParameterPair, 0) if !seen[i] { seen[i] = true for key, val := range i.value { pValue := val.ToContractParameter(seen) pKey := fromMapKey(key).ToContractParameter(seen) - if pKey.Type == smartcontract.ByteArrayType { - pKey.Value = string(pKey.Value.([]byte)) - } - value[pKey] = pValue + value = append(value, smartcontract.ParameterPair{ + Key: pKey, + Value: pValue, + }) } } return smartcontract.Parameter{ diff --git a/pkg/vm/stack_item_test.go b/pkg/vm/stack_item_test.go index 53dca3752..c67745bfd 100644 --- a/pkg/vm/stack_item_test.go +++ b/pkg/vm/stack_item_test.go @@ -421,10 +421,18 @@ var toContractParameterTestCases = []struct { }}, result: smartcontract.Parameter{ Type: smartcontract.MapType, - Value: map[smartcontract.Parameter]smartcontract.Parameter{ - {Type: smartcontract.IntegerType, Value: int64(1)}: {Type: smartcontract.BoolType, Value: true}, - {Type: smartcontract.ByteArrayType, Value: "qwerty"}: {Type: smartcontract.IntegerType, Value: int64(3)}, - {Type: smartcontract.BoolType, Value: true}: {Type: smartcontract.BoolType, Value: false}, + Value: []smartcontract.ParameterPair{ + { + Key: smartcontract.Parameter{Type: smartcontract.IntegerType, Value: int64(1)}, + Value: smartcontract.Parameter{Type: smartcontract.BoolType, Value: true}, + }, { + Key: smartcontract.Parameter{Type: smartcontract.ByteArrayType, Value: []byte("qwerty")}, + Value: smartcontract.Parameter{Type: smartcontract.IntegerType, Value: int64(3)}, + }, { + + Key: smartcontract.Parameter{Type: smartcontract.BoolType, Value: true}, + Value: smartcontract.Parameter{Type: smartcontract.BoolType, Value: false}, + }, }, }, }, From 25201d480d268a393f35a1aeaf14ebe92e23d083 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Tue, 31 Mar 2020 00:27:58 +0300 Subject: [PATCH 2/3] smartcontract: simplify Array JSON marshalling --- pkg/smartcontract/parameter.go | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/pkg/smartcontract/parameter.go b/pkg/smartcontract/parameter.go index bab39b36b..36d32f226 100644 --- a/pkg/smartcontract/parameter.go +++ b/pkg/smartcontract/parameter.go @@ -82,14 +82,7 @@ func (p *Parameter) MarshalJSON() ([]byte, error) { resultRawValue, resultErr = json.Marshal(hex.EncodeToString(p.Value.([]byte))) } case ArrayType: - var value = make([]json.RawMessage, 0) - for _, parameter := range p.Value.([]Parameter) { - rawValue, err := json.Marshal(¶meter) - if err != nil { - return nil, err - } - value = append(value, rawValue) - } + var value = p.Value.([]Parameter) resultRawValue, resultErr = json.Marshal(value) case MapType: ppair := p.Value.([]ParameterPair) From 2d0ad30fcf6648761fd79391d2644dc2332d2114 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Tue, 31 Mar 2020 13:30:38 +0300 Subject: [PATCH 3/3] vm: rework Map with internal slice representation Which makes iterating over map stable which is important for serialization and and even fixes occasional test failures. We use the same ordering here as NEO 3.0 uses, but it should also be fine for NEO 2.0 because it has no defined order. --- pkg/core/interop_neo.go | 5 ++- pkg/vm/interop.go | 12 ++---- pkg/vm/interop_iterators.go | 9 ++-- pkg/vm/serialization.go | 6 +-- pkg/vm/stack.go | 8 ++-- pkg/vm/stack_item.go | 82 ++++++++++++++++++++++--------------- pkg/vm/stack_item_test.go | 41 ++++--------------- pkg/vm/vm.go | 27 ++++++------ pkg/vm/vm_test.go | 6 ++- 9 files changed, 91 insertions(+), 105 deletions(-) diff --git a/pkg/core/interop_neo.go b/pkg/core/interop_neo.go index fcb4843ff..c4797e517 100644 --- a/pkg/core/interop_neo.go +++ b/pkg/core/interop_neo.go @@ -450,10 +450,11 @@ func (ic *interopContext) storageFind(v *vm.VM) error { return err } - filteredMap := make(map[interface{}]vm.StackItem) + filteredMap := vm.NewMapItem() for k, v := range siMap { if strings.HasPrefix(k, prefix) { - filteredMap[k] = vm.NewByteArrayItem(v.Value) + filteredMap.Add(vm.NewByteArrayItem([]byte(k)), + vm.NewByteArrayItem(v.Value)) } } diff --git a/pkg/vm/interop.go b/pkg/vm/interop.go index b6f6b8221..d1ec282ba 100644 --- a/pkg/vm/interop.go +++ b/pkg/vm/interop.go @@ -185,7 +185,7 @@ func IteratorCreate(v *VM) error { value: t.Value().([]StackItem), }) case *MapItem: - item = NewMapIterator(t.value) + item = NewMapIterator(t) default: return errors.New("non-iterable type") } @@ -195,16 +195,10 @@ func IteratorCreate(v *VM) error { } // NewMapIterator returns new interop item containing iterator over m. -func NewMapIterator(m map[interface{}]StackItem) *InteropItem { - keys := make([]interface{}, 0, len(m)) - for k := range m { - keys = append(keys, k) - } - +func NewMapIterator(m *MapItem) *InteropItem { return NewInteropItem(&mapWrapper{ index: -1, - keys: keys, - m: m, + m: m.value, }) } diff --git a/pkg/vm/interop_iterators.go b/pkg/vm/interop_iterators.go index 62a8b507e..8ecf69b83 100644 --- a/pkg/vm/interop_iterators.go +++ b/pkg/vm/interop_iterators.go @@ -25,8 +25,7 @@ type ( mapWrapper struct { index int - keys []interface{} - m map[interface{}]StackItem + m []MapElement } concatIter struct { @@ -91,7 +90,7 @@ func (i *concatIter) Key() StackItem { } func (m *mapWrapper) Next() bool { - if next := m.index + 1; next < len(m.keys) { + if next := m.index + 1; next < len(m.m) { m.index = next return true } @@ -100,11 +99,11 @@ func (m *mapWrapper) Next() bool { } func (m *mapWrapper) Value() StackItem { - return m.m[m.keys[m.index]] + return m.m[m.index].Value } func (m *mapWrapper) Key() StackItem { - return makeStackItem(m.keys[m.index]) + return m.m[m.index].Key } func (e *keysWrapper) Next() bool { diff --git a/pkg/vm/serialization.go b/pkg/vm/serialization.go index b7c7cceed..a3d30941f 100644 --- a/pkg/vm/serialization.go +++ b/pkg/vm/serialization.go @@ -73,9 +73,9 @@ func serializeItemTo(item StackItem, w *io.BinWriter, seen map[StackItem]bool) { w.WriteBytes([]byte{byte(mapT)}) w.WriteVarUint(uint64(len(t.value))) - for k, v := range t.value { - serializeItemTo(makeStackItem(k), w, seen) - serializeItemTo(v, w, seen) + for i := range t.value { + serializeItemTo(t.value[i].Key, w, seen) + serializeItemTo(t.value[i].Value, w, seen) } } } diff --git a/pkg/vm/stack.go b/pkg/vm/stack.go index 361b81854..fc41a0406 100644 --- a/pkg/vm/stack.go +++ b/pkg/vm/stack.go @@ -228,8 +228,8 @@ func (s *Stack) updateSizeAdd(item StackItem) { s.updateSizeAdd(it) } case *MapItem: - for _, v := range t.value { - s.updateSizeAdd(v) + for i := range t.value { + s.updateSizeAdd(t.value[i].Value) } } } @@ -253,8 +253,8 @@ func (s *Stack) updateSizeRemove(item StackItem) { s.updateSizeRemove(it) } case *MapItem: - for _, v := range t.value { - s.updateSizeRemove(v) + for i := range t.value { + s.updateSizeRemove(t.value[i].Value) } } } diff --git a/pkg/vm/stack_item.go b/pkg/vm/stack_item.go index b185e6e85..6cfd772f3 100644 --- a/pkg/vm/stack_item.go +++ b/pkg/vm/stack_item.go @@ -435,15 +435,25 @@ func (i *ArrayItem) ToContractParameter(seen map[StackItem]bool) smartcontract.P } } -// MapItem represents Map object. +// MapElement is a key-value pair of StackItems. +type MapElement struct { + Key StackItem + Value StackItem +} + +// MapItem represents Map object. It's ordered, so we use slice representation +// which should be fine for maps with less than 32 or so elements. Given that +// our VM has quite low limit of overall stack items, it should be good enough, +// but it can be extended with a real map for fast random access in the future +// if need be. type MapItem struct { - value map[interface{}]StackItem + value []MapElement } // NewMapItem returns new MapItem object. func NewMapItem() *MapItem { return &MapItem{ - value: make(map[interface{}]StackItem), + value: make([]MapElement, 0), } } @@ -466,10 +476,19 @@ func (i *MapItem) String() string { return "Map" } +// Index returns an index of the key in map. +func (i *MapItem) Index(key StackItem) int { + for k := range i.value { + if i.value[k].Key.Equals(key) { + return k + } + } + return -1 +} + // Has checks if map has specified key. -func (i *MapItem) Has(key StackItem) (ok bool) { - _, ok = i.value[toMapKey(key)] - return +func (i *MapItem) Has(key StackItem) bool { + return i.Index(key) >= 0 } // Dup implements StackItem interface. @@ -483,12 +502,10 @@ func (i *MapItem) ToContractParameter(seen map[StackItem]bool) smartcontract.Par value := make([]smartcontract.ParameterPair, 0) if !seen[i] { seen[i] = true - for key, val := range i.value { - pValue := val.ToContractParameter(seen) - pKey := fromMapKey(key).ToContractParameter(seen) + for k := range i.value { value = append(value, smartcontract.ParameterPair{ - Key: pKey, - Value: pValue, + Key: i.value[k].Key.ToContractParameter(seen), + Value: i.value[k].Value.ToContractParameter(seen), }) } } @@ -500,34 +517,31 @@ func (i *MapItem) ToContractParameter(seen map[StackItem]bool) smartcontract.Par // Add adds key-value pair to the map. func (i *MapItem) Add(key, value StackItem) { - i.value[toMapKey(key)] = value -} - -// toMapKey converts StackItem so that it can be used as a map key. -func toMapKey(key StackItem) interface{} { - switch t := key.(type) { - case *BoolItem: - return t.value - case *BigIntegerItem: - return t.value.Int64() - case *ByteArrayItem: - return string(t.value) - default: + if !isValidMapKey(key) { panic("wrong key type") } + index := i.Index(key) + if index >= 0 { + i.value[index].Value = value + } else { + i.value = append(i.value, MapElement{key, value}) + } } -// fromMapKey converts map key to StackItem -func fromMapKey(key interface{}) StackItem { - switch t := key.(type) { - case bool: - return &BoolItem{value: t} - case int64: - return &BigIntegerItem{value: big.NewInt(t)} - case string: - return &ByteArrayItem{value: []byte(t)} +// Drop removes given index from the map (no bounds check done here). +func (i *MapItem) Drop(index int) { + copy(i.value[index:], i.value[index+1:]) + i.value = i.value[:len(i.value)-1] +} + +// isValidMapKey checks whether it's possible to use given StackItem as a Map +// key. +func isValidMapKey(key StackItem) bool { + switch key.(type) { + case *BoolItem, *BigIntegerItem, *ByteArrayItem: + return true default: - panic("wrong key type") + return false } } diff --git a/pkg/vm/stack_item_test.go b/pkg/vm/stack_item_test.go index c67745bfd..069682878 100644 --- a/pkg/vm/stack_item_test.go +++ b/pkg/vm/stack_item_test.go @@ -282,13 +282,13 @@ var equalsTestCases = map[string][]struct { result: false, }, { - item1: &MapItem{value: map[interface{}]StackItem{"first": NewBigIntegerItem(1), true: NewByteArrayItem([]byte{2})}}, - item2: &MapItem{value: map[interface{}]StackItem{"first": NewBigIntegerItem(1), true: NewByteArrayItem([]byte{2})}}, + item1: &MapItem{value: []MapElement{{NewByteArrayItem([]byte("first")), NewBigIntegerItem(1)}, {NewBoolItem(true), NewByteArrayItem([]byte{2})}}}, + item2: &MapItem{value: []MapElement{{NewByteArrayItem([]byte("first")), NewBigIntegerItem(1)}, {NewBoolItem(true), NewByteArrayItem([]byte{2})}}}, result: false, }, { - item1: &MapItem{value: map[interface{}]StackItem{"first": NewBigIntegerItem(1), true: NewByteArrayItem([]byte{2})}}, - item2: &MapItem{value: map[interface{}]StackItem{"first": NewBigIntegerItem(1), true: NewByteArrayItem([]byte{3})}}, + item1: &MapItem{value: []MapElement{{NewByteArrayItem([]byte("first")), NewBigIntegerItem(1)}, {NewBoolItem(true), NewByteArrayItem([]byte{2})}}}, + item2: &MapItem{value: []MapElement{{NewByteArrayItem([]byte("first")), NewBigIntegerItem(1)}, {NewBoolItem(true), NewByteArrayItem([]byte{3})}}}, result: false, }, }, @@ -414,10 +414,10 @@ var toContractParameterTestCases = []struct { result: smartcontract.Parameter{Type: smartcontract.InteropInterfaceType, Value: nil}, }, { - input: &MapItem{value: map[interface{}]StackItem{ - toMapKey(NewBigIntegerItem(1)): NewBoolItem(true), - toMapKey(NewByteArrayItem([]byte("qwerty"))): NewBigIntegerItem(3), - toMapKey(NewBoolItem(true)): NewBoolItem(false), + input: &MapItem{value: []MapElement{ + {NewBigIntegerItem(1), NewBoolItem(true)}, + {NewByteArrayItem([]byte("qwerty")), NewBigIntegerItem(3)}, + {NewBoolItem(true), NewBoolItem(false)}, }}, result: smartcontract.Parameter{ Type: smartcontract.MapType, @@ -445,28 +445,3 @@ func TestToContractParameter(t *testing.T) { assert.Equal(t, res, tc.result) } } - -var fromMapKeyTestCases = []struct { - input interface{} - result StackItem -}{ - { - input: true, - result: NewBoolItem(true), - }, - { - input: int64(4), - result: NewBigIntegerItem(4), - }, - { - input: "qwerty", - result: NewByteArrayItem([]byte("qwerty")), - }, -} - -func TestFromMapKey(t *testing.T) { - for _, tc := range fromMapKeyTestCases { - res := fromMapKey(tc.input) - assert.Equal(t, res, tc.result) - } -} diff --git a/pkg/vm/vm.go b/pkg/vm/vm.go index d5d9b5886..13ac42441 100644 --- a/pkg/vm/vm.go +++ b/pkg/vm/vm.go @@ -1014,11 +1014,11 @@ func (v *VM) execute(ctx *Context, op opcode.Opcode, parameter []byte) (err erro item := arr[index].Dup() v.estack.PushVal(item) case *MapItem: - if !t.Has(key.value) { + index := t.Index(key.Item()) + if index < 0 { panic("invalid key") } - k := toMapKey(key.value) - v.estack.Push(&Element{value: t.value[k].Dup()}) + v.estack.Push(&Element{value: t.value[index].Value.Dup()}) default: arr := obj.Bytes() if index < 0 || index >= len(arr) { @@ -1091,10 +1091,12 @@ func (v *VM) execute(ctx *Context, op opcode.Opcode, parameter []byte) (err erro a = append(a[:k], a[k+1:]...) t.value = a case *MapItem: - m := t.value - k := toMapKey(key.value) - v.estack.updateSizeRemove(m[k]) - delete(m, k) + index := t.Index(key.Item()) + // NEO 2.0 doesn't error on missing key. + if index >= 0 { + v.estack.updateSizeRemove(t.value[index].Value) + t.Drop(index) + } default: panic("REMOVE: invalid type") } @@ -1106,7 +1108,7 @@ func (v *VM) execute(ctx *Context, op opcode.Opcode, parameter []byte) (err erro switch t := elem.Value().(type) { case []StackItem: v.estack.PushVal(len(t)) - case map[interface{}]StackItem: + case []MapElement: v.estack.PushVal(len(t)) default: v.estack.PushVal(len(elem.Bytes())) @@ -1253,7 +1255,7 @@ func (v *VM) execute(ctx *Context, op opcode.Opcode, parameter []byte) (err erro arr := make([]StackItem, 0, len(m.value)) for k := range m.value { - arr = append(arr, makeStackItem(k)) + arr = append(arr, m.value[k].Key.Dup()) } v.estack.PushVal(arr) @@ -1274,7 +1276,7 @@ func (v *VM) execute(ctx *Context, op opcode.Opcode, parameter []byte) (err erro case *MapItem: arr = make([]StackItem, 0, len(t.value)) for k := range t.value { - arr = append(arr, cloneIfStruct(t.value[k])) + arr = append(arr, cloneIfStruct(t.value[k].Value)) } default: panic("not a Map, Array or Struct") @@ -1298,7 +1300,7 @@ func (v *VM) execute(ctx *Context, op opcode.Opcode, parameter []byte) (err erro } v.estack.PushVal(index < int64(len(c.Array()))) case *MapItem: - v.estack.PushVal(t.Has(key.value)) + v.estack.PushVal(t.Has(key.Item())) default: panic("wrong collection type") } @@ -1552,8 +1554,7 @@ func validateMapKey(key *Element) { if key == nil { panic("no key found") } - switch key.value.(type) { - case *ArrayItem, *StructItem, *MapItem: + if !isValidMapKey(key.Item()) { panic("key can't be a collection") } } diff --git a/pkg/vm/vm_test.go b/pkg/vm/vm_test.go index d05576fb5..e0b13920b 100644 --- a/pkg/vm/vm_test.go +++ b/pkg/vm/vm_test.go @@ -1371,8 +1371,10 @@ func TestPICKITEMDupMap(t *testing.T) { runVM(t, vm) assert.Equal(t, 2, vm.estack.Len()) assert.Equal(t, int64(1), vm.estack.Pop().BigInt().Int64()) - items := vm.estack.Pop().Value().(map[interface{}]StackItem) - assert.Equal(t, big.NewInt(-1), items[string([]byte{42})].Value()) + items := vm.estack.Pop().Value().([]MapElement) + assert.Equal(t, 1, len(items)) + assert.Equal(t, []byte{42}, items[0].Key.Value()) + assert.Equal(t, big.NewInt(-1), items[0].Value.Value()) } func TestPICKITEMMap(t *testing.T) {