From 5da82e8cf0d51e73ea804f9208225bce479462fd Mon Sep 17 00:00:00 2001 From: Evgenii Stratonikov Date: Wed, 11 Mar 2020 16:04:28 +0300 Subject: [PATCH 1/2] vm: add TryBytes() to StackItem interface Conversion should be done in a StackItem, not in an Element. --- pkg/vm/context.go | 5 +++++ pkg/vm/stack.go | 18 ++++------------- pkg/vm/stack_item.go | 48 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 57 insertions(+), 14 deletions(-) diff --git a/pkg/vm/context.go b/pkg/vm/context.go index 43ea9c6ae..b934b2ba3 100644 --- a/pkg/vm/context.go +++ b/pkg/vm/context.go @@ -170,6 +170,11 @@ func (c *Context) Dup() StackItem { return c } +// TryBytes implements StackItem interface. +func (c *Context) TryBytes() ([]byte, error) { + return nil, errors.New("can't convert Context to ByteArray") +} + // ToContractParameter implements StackItem interface. func (c *Context) ToContractParameter(map[StackItem]bool) smartcontract.Parameter { return smartcontract.Parameter{ diff --git a/pkg/vm/stack.go b/pkg/vm/stack.go index b9b417212..361b81854 100644 --- a/pkg/vm/stack.go +++ b/pkg/vm/stack.go @@ -125,21 +125,11 @@ func (e *Element) Bool() bool { // Bytes attempts to get the underlying value of the element as a byte array. // Will panic if the assertion failed which will be caught by the VM. func (e *Element) Bytes() []byte { - switch t := e.value.(type) { - case *ByteArrayItem: - return t.value - case *BigIntegerItem: - return t.Bytes() // neoVM returns in LE - case *BoolItem: - if t.value { - return []byte{1} - } - // return []byte{0} - // FIXME revert when NEO 3.0 https://github.com/nspcc-dev/neo-go/issues/477 - return []byte{} - default: - panic("can't convert to []byte: " + t.String()) + bs, err := e.value.TryBytes() + if err != nil { + panic(err) } + return bs } // Array attempts to get the underlying value of the element as an array of diff --git a/pkg/vm/stack_item.go b/pkg/vm/stack_item.go index 9ab19f835..188b11de6 100644 --- a/pkg/vm/stack_item.go +++ b/pkg/vm/stack_item.go @@ -4,6 +4,7 @@ import ( "encoding/binary" "encoding/hex" "encoding/json" + "errors" "fmt" "math/big" "reflect" @@ -18,6 +19,8 @@ type StackItem interface { Value() interface{} // Dup duplicates current StackItem. Dup() StackItem + // TryBytes converts StackItem to a byte slice. + TryBytes() ([]byte, error) // ToContractParameter converts StackItem to smartcontract.Parameter ToContractParameter(map[StackItem]bool) smartcontract.Parameter } @@ -118,6 +121,11 @@ func (i *StructItem) Dup() StackItem { return i } +// TryBytes implements StackItem interface. +func (i *StructItem) TryBytes() ([]byte, error) { + return nil, errors.New("can't convert Struct to ByteArray") +} + // ToContractParameter implements StackItem interface. func (i *StructItem) ToContractParameter(seen map[StackItem]bool) smartcontract.Parameter { var value []smartcontract.Parameter @@ -167,6 +175,11 @@ func (i *BigIntegerItem) Bytes() []byte { return emit.IntToBytes(i.value) } +// TryBytes implements StackItem interface. +func (i *BigIntegerItem) TryBytes() ([]byte, error) { + return i.Bytes(), nil +} + // Value implements StackItem interface. func (i *BigIntegerItem) Value() interface{} { return i.value @@ -226,6 +239,21 @@ func (i *BoolItem) Dup() StackItem { return &BoolItem{i.value} } +// Bytes converts BoolItem to bytes. +func (i *BoolItem) Bytes() []byte { + if i.value { + return []byte{1} + } + // return []byte{0} + // FIXME revert when NEO 3.0 https://github.com/nspcc-dev/neo-go/issues/477 + return []byte{} +} + +// TryBytes implements StackItem interface. +func (i *BoolItem) TryBytes() ([]byte, error) { + return i.Bytes(), nil +} + // ToContractParameter implements StackItem interface. func (i *BoolItem) ToContractParameter(map[StackItem]bool) smartcontract.Parameter { return smartcontract.Parameter{ @@ -260,6 +288,11 @@ func (i *ByteArrayItem) String() string { return "ByteArray" } +// TryBytes implements StackItem interface. +func (i *ByteArrayItem) TryBytes() ([]byte, error) { + return i.value, nil +} + // Dup implements StackItem interface. func (i *ByteArrayItem) Dup() StackItem { a := make([]byte, len(i.value)) @@ -301,6 +334,11 @@ func (i *ArrayItem) String() string { return "Array" } +// TryBytes implements StackItem interface. +func (i *ArrayItem) TryBytes() ([]byte, error) { + return nil, errors.New("can't convert Array to ByteArray") +} + // Dup implements StackItem interface. func (i *ArrayItem) Dup() StackItem { // reference type @@ -341,6 +379,11 @@ func (i *MapItem) Value() interface{} { return i.value } +// TryBytes implements StackItem interface. +func (i *MapItem) TryBytes() ([]byte, error) { + return nil, errors.New("can't convert Map to ByteArray") +} + func (i *MapItem) String() string { return "Map" } @@ -438,6 +481,11 @@ func (i *InteropItem) Dup() StackItem { return i } +// TryBytes implements StackItem interface. +func (i *InteropItem) TryBytes() ([]byte, error) { + return nil, errors.New("can't convert Interop to ByteArray") +} + // ToContractParameter implements StackItem interface. func (i *InteropItem) ToContractParameter(map[StackItem]bool) smartcontract.Parameter { return smartcontract.Parameter{ From dfc59129c7909b31e3bfda28ec9d5e5b37f0cc2a Mon Sep 17 00:00:00 2001 From: Evgenii Stratonikov Date: Wed, 11 Mar 2020 16:44:10 +0300 Subject: [PATCH 2/2] vm: implement EQUAL opcode properly When comparing elements of different types, conversions should be performed. This commit implement custom equality predicate for each stack item type. --- pkg/vm/context.go | 5 +++ pkg/vm/stack_item.go | 84 ++++++++++++++++++++++++++++++++++++++++++++ pkg/vm/vm.go | 14 +------- pkg/vm/vm_test.go | 10 ++++++ 4 files changed, 100 insertions(+), 13 deletions(-) diff --git a/pkg/vm/context.go b/pkg/vm/context.go index b934b2ba3..64817e4b4 100644 --- a/pkg/vm/context.go +++ b/pkg/vm/context.go @@ -175,6 +175,11 @@ func (c *Context) TryBytes() ([]byte, error) { return nil, errors.New("can't convert Context to ByteArray") } +// Equals implements StackItem interface. +func (c *Context) Equals(s StackItem) bool { + return c == s +} + // ToContractParameter implements StackItem interface. func (c *Context) ToContractParameter(map[StackItem]bool) smartcontract.Parameter { return smartcontract.Parameter{ diff --git a/pkg/vm/stack_item.go b/pkg/vm/stack_item.go index 188b11de6..0a03fe20f 100644 --- a/pkg/vm/stack_item.go +++ b/pkg/vm/stack_item.go @@ -1,6 +1,7 @@ package vm import ( + "bytes" "encoding/binary" "encoding/hex" "encoding/json" @@ -21,6 +22,8 @@ type StackItem interface { Dup() StackItem // TryBytes converts StackItem to a byte slice. TryBytes() ([]byte, error) + // Equals checks if 2 StackItems are equal. + Equals(s StackItem) bool // ToContractParameter converts StackItem to smartcontract.Parameter ToContractParameter(map[StackItem]bool) smartcontract.Parameter } @@ -126,6 +129,25 @@ func (i *StructItem) TryBytes() ([]byte, error) { return nil, errors.New("can't convert Struct to ByteArray") } +// Equals implements StackItem interface. +func (i *StructItem) Equals(s StackItem) bool { + if i == s { + return true + } else if s == nil { + return false + } + val, ok := s.(*StructItem) + if !ok || len(i.value) != len(val.value) { + return false + } + for j := range i.value { + if !i.value[j].Equals(val.value[j]) { + return false + } + } + return true +} + // ToContractParameter implements StackItem interface. func (i *StructItem) ToContractParameter(seen map[StackItem]bool) smartcontract.Parameter { var value []smartcontract.Parameter @@ -180,6 +202,21 @@ func (i *BigIntegerItem) TryBytes() ([]byte, error) { return i.Bytes(), nil } +// Equals implements StackItem interface. +func (i *BigIntegerItem) Equals(s StackItem) bool { + if i == s { + return true + } else if s == nil { + return false + } + val, ok := s.(*BigIntegerItem) + if ok { + return i.value.Cmp(val.value) == 0 + } + bs, err := s.TryBytes() + return err == nil && bytes.Equal(i.Bytes(), bs) +} + // Value implements StackItem interface. func (i *BigIntegerItem) Value() interface{} { return i.value @@ -254,6 +291,21 @@ func (i *BoolItem) TryBytes() ([]byte, error) { return i.Bytes(), nil } +// Equals implements StackItem interface. +func (i *BoolItem) Equals(s StackItem) bool { + if i == s { + return true + } else if s == nil { + return false + } + val, ok := s.(*BoolItem) + if ok { + return i.value == val.value + } + bs, err := s.TryBytes() + return err == nil && bytes.Equal(i.Bytes(), bs) +} + // ToContractParameter implements StackItem interface. func (i *BoolItem) ToContractParameter(map[StackItem]bool) smartcontract.Parameter { return smartcontract.Parameter{ @@ -293,6 +345,17 @@ func (i *ByteArrayItem) TryBytes() ([]byte, error) { return i.value, nil } +// Equals implements StackItem interface. +func (i *ByteArrayItem) Equals(s StackItem) bool { + if i == s { + return true + } else if s == nil { + return false + } + bs, err := s.TryBytes() + return err == nil && bytes.Equal(i.value, bs) +} + // Dup implements StackItem interface. func (i *ByteArrayItem) Dup() StackItem { a := make([]byte, len(i.value)) @@ -339,6 +402,11 @@ func (i *ArrayItem) TryBytes() ([]byte, error) { return nil, errors.New("can't convert Array to ByteArray") } +// Equals implements StackItem interface. +func (i *ArrayItem) Equals(s StackItem) bool { + return i == s +} + // Dup implements StackItem interface. func (i *ArrayItem) Dup() StackItem { // reference type @@ -384,6 +452,11 @@ func (i *MapItem) TryBytes() ([]byte, error) { return nil, errors.New("can't convert Map to ByteArray") } +// Equals implements StackItem interface. +func (i *MapItem) Equals(s StackItem) bool { + return i == s +} + func (i *MapItem) String() string { return "Map" } @@ -486,6 +559,17 @@ func (i *InteropItem) TryBytes() ([]byte, error) { return nil, errors.New("can't convert Interop to ByteArray") } +// Equals implements StackItem interface. +func (i *InteropItem) Equals(s StackItem) bool { + if i == s { + return true + } else if s == nil { + return false + } + val, ok := s.(*InteropItem) + return ok && i.value == val.value +} + // ToContractParameter implements StackItem interface. func (i *InteropItem) ToContractParameter(map[StackItem]bool) smartcontract.Parameter { return smartcontract.Parameter{ diff --git a/pkg/vm/vm.go b/pkg/vm/vm.go index 4ec5b603f..9bb0ba675 100644 --- a/pkg/vm/vm.go +++ b/pkg/vm/vm.go @@ -8,7 +8,6 @@ import ( "io/ioutil" "math/big" "os" - "reflect" "text/tabwriter" "unicode/utf8" @@ -703,18 +702,7 @@ func (v *VM) execute(ctx *Context, op opcode.Opcode, parameter []byte) (err erro if a == nil { panic("no second-to-the-top element found") } - if ta, ok := a.value.(*ArrayItem); ok { - if tb, ok := b.value.(*ArrayItem); ok { - v.estack.PushVal(ta == tb) - break - } - } else if ma, ok := a.value.(*MapItem); ok { - if mb, ok := b.value.(*MapItem); ok { - v.estack.PushVal(ma == mb) - break - } - } - v.estack.PushVal(reflect.DeepEqual(a, b)) + v.estack.PushVal(a.value.Equals(b.value)) // Bit operations. case opcode.INVERT: diff --git a/pkg/vm/vm_test.go b/pkg/vm/vm_test.go index c8488d189..bbc654a9a 100644 --- a/pkg/vm/vm_test.go +++ b/pkg/vm/vm_test.go @@ -1006,6 +1006,16 @@ func TestEQUALGoodInteger(t *testing.T) { assert.Equal(t, &BoolItem{true}, vm.estack.Pop().value) } +func TestEQUALIntegerByteArray(t *testing.T) { + prog := makeProgram(opcode.EQUAL) + vm := load(prog) + vm.estack.PushVal([]byte{16}) + vm.estack.PushVal(16) + runVM(t, vm) + assert.Equal(t, 1, vm.estack.Len()) + assert.Equal(t, &BoolItem{true}, vm.estack.Pop().value) +} + func TestEQUALArrayTrue(t *testing.T) { prog := makeProgram(opcode.DUP, opcode.EQUAL) vm := load(prog)