Merge pull request #2525 from nspcc-dev/immutable-items

vm: implement immutable stackitems

cli/nep11_test.go

@@ -319,7 +319,7 @@ func TestNEP11_ND_OwnerOf_BalanceOf_Transfer(t *testing.T) {
 		ScriptHash: verifyH,
 		Name:       "OnNEP11Payment",
 		Item: stackitem.NewArray([]stackitem.Item{
-			stackitem.NewBuffer(nftOwnerHash.BytesBE()),
+			stackitem.NewByteArray(nftOwnerHash.BytesBE()),
 			stackitem.NewBigInteger(big.NewInt(1)),
 			stackitem.NewByteArray(tokenID1),
 			stackitem.NewByteArray([]byte("some_data")),

pkg/core/interop/runtime/engine.go

@@ -68,7 +68,7 @@ func Notify(ic *interop.Context) error {
 	if len(bytes) > MaxNotificationSize {
 		return fmt.Errorf("notification size shouldn't exceed %d", MaxNotificationSize)
 	}
-	ic.AddNotification(ic.VM.GetCurrentScriptHash(), name, stackitem.DeepCopy(stackitem.NewArray(args)).(*stackitem.Array))
+	ic.AddNotification(ic.VM.GetCurrentScriptHash(), name, stackitem.DeepCopy(stackitem.NewArray(args), true).(*stackitem.Array))
 	return nil
 }
 

pkg/core/interop/runtime/engine_test.go

@@ -161,6 +161,7 @@ func TestNotify(t *testing.T) {
 		require.NoError(t, Notify(ic))
 		require.Equal(t, 1, len(ic.Notifications))
 
+		arr.MarkAsReadOnly() // tiny hack for test to be able to compare object references.
 		ev := ic.Notifications[0]
 		require.Equal(t, "good event", ev.Name)
 		require.Equal(t, h, ev.ScriptHash)

pkg/core/interop/runtime/util.go

@@ -54,7 +54,7 @@ func GetNotifications(ic *interop.Context) error {
 		ev := stackitem.NewArray([]stackitem.Item{
 			stackitem.NewByteArray(notifications[i].ScriptHash.BytesBE()),
 			stackitem.Make(notifications[i].Name),
-			stackitem.DeepCopy(notifications[i].Item).(*stackitem.Array),
+			notifications[i].Item,
 		})
 		arr.Append(ev)
 	}

pkg/core/interop/runtime/util_test.go

@@ -54,6 +54,7 @@ func TestRuntimeGetNotifications(t *testing.T) {
 			name, err := stackitem.ToString(elem[1])
 			require.NoError(t, err)
 			require.Equal(t, ic.Notifications[i].Name, name)
+			ic.Notifications[i].Item.MarkAsReadOnly() // tiny hack for test to be able to compare object references.
 			require.Equal(t, ic.Notifications[i].Item, elem[2])
 		}
 	})

pkg/core/interop_system_core_test.go

@@ -111,6 +111,7 @@ func TestRuntimeGetNotifications(t *testing.T) {
 			name, err := stackitem.ToString(elem[1])
 			require.NoError(t, err)
 			require.Equal(t, ic.Notifications[i].Name, name)
+			ic.Notifications[i].Item.MarkAsReadOnly() // tiny hack for test to be able to compare object references.
 			require.Equal(t, ic.Notifications[i].Item, elem[2])
 		}
 	})

pkg/vm/opcodebench_test.go

@@ -65,7 +65,7 @@ func opParamSlotsPushVM(op opcode.Opcode, param []byte, sslot int, slotloc int,
 		for i := range items {
 			item, ok := items[i].(stackitem.Item)
 			if ok {
-				item = stackitem.DeepCopy(item)
+				item = stackitem.DeepCopy(item, true)
 			} else {
 				item = stackitem.Make(items[i])
 			}

pkg/vm/stackitem/immutable.go

@@ -0,0 +1,21 @@
+package stackitem
+
+type ro struct {
+	isReadOnly bool
+}
+
+// IsReadOnly implements Immutable interface.
+func (r *ro) IsReadOnly() bool {
+	return r.isReadOnly
+}
+
+// MarkAsReadOnly implements immutable interface.
+func (r *ro) MarkAsReadOnly() {
+	r.isReadOnly = true
+}
+
+// Immutable is an interface supported by compound types (Array, Map, Struct).
+type Immutable interface {
+	IsReadOnly() bool
+	MarkAsReadOnly()
+}

pkg/vm/stackitem/item.go

@@ -70,6 +70,8 @@ var (
 	// can also be returned by serialization functions if the resulting
 	// value exceeds MaxSize.
 	ErrTooBig = errors.New("too big")
+	// ErrReadOnly is returned on attempt to modify immutable stack item.
+	ErrReadOnly = errors.New("item is read-only")
 
 	errTooBigComparable = fmt.Errorf("%w: uncomparable", ErrTooBig)
 	errTooBigInteger    = fmt.Errorf("%w: integer", ErrTooBig)
@@ -185,6 +187,7 @@ func convertPrimitive(item Item, typ Type) (Item, error) {
 type Struct struct {
 	value []Item
 	rc
+	ro
 }
 
 // NewStruct returns a new Struct object.
@@ -202,16 +205,25 @@ func (i *Struct) Value() interface{} {
 // Remove removes the element at `pos` index from the Struct value.
 // It will panic if a bad index given.
 func (i *Struct) Remove(pos int) {
+	if i.IsReadOnly() {
+		panic(ErrReadOnly)
+	}
 	i.value = append(i.value[:pos], i.value[pos+1:]...)
 }
 
 // Append adds an Item to the end of the Struct value.
 func (i *Struct) Append(item Item) {
+	if i.IsReadOnly() {
+		panic(ErrReadOnly)
+	}
 	i.value = append(i.value, item)
 }
 
 // Clear removes all elements from the Struct item value.
 func (i *Struct) Clear() {
+	if i.IsReadOnly() {
+		panic(ErrReadOnly)
+	}
 	i.value = i.value[:0]
 }
 
@@ -662,6 +674,7 @@ func (i *ByteArray) Convert(typ Type) (Item, error) {
 type Array struct {
 	value []Item
 	rc
+	ro
 }
 
 // NewArray returns a new Array object.
@@ -679,16 +692,25 @@ func (i *Array) Value() interface{} {
 // Remove removes the element at `pos` index from Array value.
 // It will panics on bad index.
 func (i *Array) Remove(pos int) {
+	if i.IsReadOnly() {
+		panic(ErrReadOnly)
+	}
 	i.value = append(i.value[:pos], i.value[pos+1:]...)
 }
 
 // Append adds an Item to the end of the Array value.
 func (i *Array) Append(item Item) {
+	if i.IsReadOnly() {
+		panic(ErrReadOnly)
+	}
 	i.value = append(i.value, item)
 }
 
 // Clear removes all elements from the Array item value.
 func (i *Array) Clear() {
+	if i.IsReadOnly() {
+		panic(ErrReadOnly)
+	}
 	i.value = i.value[:0]
 }
 
@@ -763,6 +785,7 @@ type MapElement struct {
 type Map struct {
 	value []MapElement
 	rc
+	ro
 }
 
 // NewMap returns a new Map object.
@@ -789,6 +812,9 @@ func (i *Map) Value() interface{} {
 
 // Clear removes all elements from the Map item value.
 func (i *Map) Clear() {
+	if i.IsReadOnly() {
+		panic(ErrReadOnly)
+	}
 	i.value = i.value[:0]
 }
 
@@ -860,6 +886,9 @@ func (i *Map) Add(key, value Item) {
 	if err := IsValidMapKey(key); err != nil {
 		panic(err)
 	}
+	if i.IsReadOnly() {
+		panic(ErrReadOnly)
+	}
 	index := i.Index(key)
 	if index >= 0 {
 		i.value[index].Value = value
@@ -870,6 +899,9 @@ func (i *Map) Add(key, value Item) {
 
 // Drop removes the given index from the map (no bounds check done here).
 func (i *Map) Drop(index int) {
+	if i.IsReadOnly() {
+		panic(ErrReadOnly)
+	}
 	copy(i.value[index:], i.value[index+1:])
 	i.value = i.value[:len(i.value)-1]
 }
@@ -1139,12 +1171,12 @@ func (i *Buffer) Len() int {
 // DeepCopy returns a new deep copy of the provided item.
 // Values of Interop items are not deeply copied.
 // It does preserve duplicates only for non-primitive types.
-func DeepCopy(item Item) Item {
+func DeepCopy(item Item, asImmutable bool) Item {
 	seen := make(map[Item]Item, typicalNumOfItems)
-	return deepCopy(item, seen)
+	return deepCopy(item, seen, asImmutable)
 }
 
-func deepCopy(item Item, seen map[Item]Item) Item {
+func deepCopy(item Item, seen map[Item]Item, asImmutable bool) Item {
 	if it := seen[item]; it != nil {
 		return it
 	}
@@ -1155,24 +1187,28 @@ func deepCopy(item Item, seen map[Item]Item) Item {
 		arr := NewArray(make([]Item, len(it.value)))
 		seen[item] = arr
 		for i := range it.value {
-			arr.value[i] = deepCopy(it.value[i], seen)
+			arr.value[i] = deepCopy(it.value[i], seen, asImmutable)
 		}
+		arr.MarkAsReadOnly()
 		return arr
 	case *Struct:
 		arr := NewStruct(make([]Item, len(it.value)))
 		seen[item] = arr
 		for i := range it.value {
-			arr.value[i] = deepCopy(it.value[i], seen)
+			arr.value[i] = deepCopy(it.value[i], seen, asImmutable)
 		}
+		arr.MarkAsReadOnly()
 		return arr
 	case *Map:
 		m := NewMap()
 		seen[item] = m
 		for i := range it.value {
-			key := deepCopy(it.value[i].Key, seen)
+			key := deepCopy(it.value[i].Key, seen,
-			value := deepCopy(it.value[i].Value, seen)
+				false) // Key is always primitive and not a Buffer.
+			value := deepCopy(it.value[i].Value, seen, asImmutable)
 			m.Add(key, value)
 		}
+		m.MarkAsReadOnly()
 		return m
 	case *BigInteger:
 		bi := new(big.Int).Set(it.Big())
@@ -1180,6 +1216,9 @@ func deepCopy(item Item, seen map[Item]Item) Item {
 	case *ByteArray:
 		return NewByteArray(slice.Copy(*it))
 	case *Buffer:
+		if asImmutable {
+			return NewByteArray(slice.Copy(*it))
+		}
 		return NewBuffer(slice.Copy(*it))
 	case Bool:
 		return it

pkg/vm/stackitem/item_test.go

@@ -530,7 +530,10 @@ func TestDeepCopy(t *testing.T) {
 	}
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			actual := DeepCopy(tc.item)
+			actual := DeepCopy(tc.item, false)
+			if immut, ok := tc.item.(Immutable); ok {
+				immut.MarkAsReadOnly() // tiny hack for test to be able to compare object references.
+			}
 			require.Equal(t, tc.item, actual)
 			if tc.item.Type() != BooleanT {
 				require.False(t, actual == tc.item)
@@ -539,7 +542,7 @@ func TestDeepCopy(t *testing.T) {
 	}
 
 	t.Run("Null", func(t *testing.T) {
-		require.Equal(t, Null{}, DeepCopy(Null{}))
+		require.Equal(t, Null{}, DeepCopy(Null{}, false))
 	})
 
 	t.Run("Array", func(t *testing.T) {
@@ -547,7 +550,8 @@ func TestDeepCopy(t *testing.T) {
 		arr.value[0] = NewBool(true)
 		arr.value[1] = arr
 
-		actual := DeepCopy(arr)
+		actual := DeepCopy(arr, false)
+		arr.isReadOnly = true // tiny hack for test to be able to compare object references.
 		require.Equal(t, arr, actual)
 		require.False(t, arr == actual)
 		require.True(t, actual == actual.(*Array).value[1])
@@ -558,7 +562,8 @@ func TestDeepCopy(t *testing.T) {
 		arr.value[0] = NewBool(true)
 		arr.value[1] = arr
 
-		actual := DeepCopy(arr)
+		actual := DeepCopy(arr, false)
+		arr.isReadOnly = true // tiny hack for test to be able to compare object references.
 		require.Equal(t, arr, actual)
 		require.False(t, arr == actual)
 		require.True(t, actual == actual.(*Struct).value[1])
@@ -569,7 +574,8 @@ func TestDeepCopy(t *testing.T) {
 		m.value[0] = MapElement{Key: NewBool(true), Value: m}
 		m.value[1] = MapElement{Key: NewBigInteger(big.NewInt(1)), Value: NewByteArray([]byte{1, 2, 3})}
 
-		actual := DeepCopy(m)
+		actual := DeepCopy(m, false)
+		m.isReadOnly = true // tiny hack for test to be able to compare object references.
 		require.Equal(t, m, actual)
 		require.False(t, m == actual)
 		require.True(t, actual == actual.(*Map).value[0].Value)

pkg/vm/vm.go

@@ -1245,10 +1245,16 @@ func (v *VM) execute(ctx *Context, op opcode.Opcode, parameter []byte) (err erro
 				v.throw(stackitem.NewByteArray([]byte(msg)))
 				return
 			}
+			if t.(stackitem.Immutable).IsReadOnly() {
+				panic(stackitem.ErrReadOnly)
+			}
 			v.refs.Remove(arr[index])
 			arr[index] = item
 			v.refs.Add(arr[index])
 		case *stackitem.Map:
+			if t.IsReadOnly() {
+				panic(stackitem.ErrReadOnly)
+			}
 			if i := t.Index(key.value); i >= 0 {
 				v.refs.Remove(t.Value().([]stackitem.MapElement)[i].Value)
 			} else {
@@ -1279,6 +1285,9 @@ func (v *VM) execute(ctx *Context, op opcode.Opcode, parameter []byte) (err erro
 		item := v.estack.Pop()
 		switch t := item.value.(type) {
 		case *stackitem.Array, *stackitem.Struct:
+			if t.(stackitem.Immutable).IsReadOnly() {
+				panic(stackitem.ErrReadOnly)
+			}
 			a := t.Value().([]stackitem.Item)
 			for i, j := 0, len(a)-1; i < j; i, j = i+1, j-1 {
 				a[i], a[j] = a[j], a[i]
@@ -1301,24 +1310,28 @@ func (v *VM) execute(ctx *Context, op opcode.Opcode, parameter []byte) (err erro
 			if k < 0 || k >= len(a) {
 				panic("REMOVE: invalid index")
 			}
-			v.refs.Remove(a[k])
+			toRemove := a[k]
 			t.Remove(k)
+			v.refs.Remove(toRemove)
 		case *stackitem.Struct:
 			a := t.Value().([]stackitem.Item)
 			k := toInt(key.BigInt())
 			if k < 0 || k >= len(a) {
 				panic("REMOVE: invalid index")
 			}
-			v.refs.Remove(a[k])
+			toRemove := a[k]
 			t.Remove(k)
+			v.refs.Remove(toRemove)
 		case *stackitem.Map:
 			index := t.Index(key.Item())
 			// NEO 2.0 doesn't error on missing key.
 			if index >= 0 {
 				elems := t.Value().([]stackitem.MapElement)
-				v.refs.Remove(elems[index].Key)
+				key := elems[index].Key
-				v.refs.Remove(elems[index].Value)
+				val := elems[index].Value
 				t.Drop(index)
+				v.refs.Remove(key)
+				v.refs.Remove(val)
 			}
 		default:
 			panic("REMOVE: invalid type")
@@ -1328,16 +1341,25 @@ func (v *VM) execute(ctx *Context, op opcode.Opcode, parameter []byte) (err erro
 		elem := v.estack.Pop()
 		switch t := elem.value.(type) {
 		case *stackitem.Array:
+			if t.IsReadOnly() {
+				panic(stackitem.ErrReadOnly)
+			}
 			for _, item := range t.Value().([]stackitem.Item) {
 				v.refs.Remove(item)
 			}
 			t.Clear()
 		case *stackitem.Struct:
+			if t.IsReadOnly() {
+				panic(stackitem.ErrReadOnly)
+			}
 			for _, item := range t.Value().([]stackitem.Item) {
 				v.refs.Remove(item)
 			}
 			t.Clear()
 		case *stackitem.Map:
+			if t.IsReadOnly() {
+				panic(stackitem.ErrReadOnly)
+			}
 			elems := t.Value().([]stackitem.MapElement)
 			for i := range elems {
 				v.refs.Remove(elems[i].Key)
@@ -1353,6 +1375,7 @@ func (v *VM) execute(ctx *Context, op opcode.Opcode, parameter []byte) (err erro
 		elems := arr.Value().([]stackitem.Item)
 		index := len(elems) - 1
 		elem := elems[index]
+		v.estack.PushItem(elem) // push item on stack firstly, to match the reference behaviour.
 		switch item := arr.(type) {
 		case *stackitem.Array:
 			item.Remove(index)
@@ -1360,7 +1383,6 @@ func (v *VM) execute(ctx *Context, op opcode.Opcode, parameter []byte) (err erro
 			item.Remove(index)
 		}
 		v.refs.Remove(elem)
-		v.estack.PushItem(elem)
 
 	case opcode.SIZE:
 		elem := v.estack.Pop()