From 3dbe549a6105ea60ffa90dc57aa045e9e88cc84d Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Mon, 30 Mar 2020 18:24:06 +0300 Subject: [PATCH] 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}, + }, }, }, },