Merge pull request #1631 from nspcc-dev/fix/appexec

vm: allow to serialize interop items

pkg/core/blockchain_test.go

@@ -1539,3 +1539,17 @@ func TestRemoveUntraceable(t *testing.T) {
 	require.NoError(t, err)
 	require.Len(t, b.Transactions, 0)
 }
+
+func TestInvalidNotification(t *testing.T) {
+	bc := newTestChain(t)
+	defer bc.Close()
+
+	cs, _ := getTestContractState(bc)
+	require.NoError(t, bc.contracts.Management.PutContractState(bc.dao, cs))
+
+	aer, err := invokeContractMethod(bc, 1_00000000, cs.Hash, "invalidStack")
+	require.NoError(t, err)
+	require.Equal(t, 2, len(aer.Stack))
+	require.Nil(t, aer.Stack[0])
+	require.Equal(t, stackitem.InteropT, aer.Stack[1].Type())
+}

pkg/core/interop_system_test.go

@@ -506,6 +506,10 @@ func getTestContractState(bc *Blockchain) (*state.Contract, *state.Contract) {
 	emit.String(w.BinWriter, "destroy")
 	emit.AppCall(w.BinWriter, mgmtHash)
 	emit.Opcodes(w.BinWriter, opcode.RET)
+	invalidStackOff := w.Len()
+	emit.Opcodes(w.BinWriter, opcode.NEWARRAY0, opcode.DUP, opcode.DUP, opcode.APPEND, opcode.NEWMAP)
+	emit.Syscall(w.BinWriter, interopnames.SystemIteratorCreate)
+	emit.Opcodes(w.BinWriter, opcode.RET)
 
 	script := w.Bytes()
 	h := hash.Hash160(script)
@@ -604,6 +608,11 @@ func getTestContractState(bc *Blockchain) (*state.Contract, *state.Contract) {
 			Offset:     destroyOff,
 			ReturnType: smartcontract.VoidType,
 		},
+		{
+			Name:       "invalidStack",
+			Offset:     invalidStackOff,
+			ReturnType: smartcontract.VoidType,
+		},
 	}
 	cs := &state.Contract{
 		Script:   script,

pkg/core/state/notification_event.go

@@ -57,7 +57,11 @@ func (aer *AppExecResult) EncodeBinary(w *io.BinWriter) {
 	w.WriteB(byte(aer.Trigger))
 	w.WriteB(byte(aer.VMState))
 	w.WriteU64LE(uint64(aer.GasConsumed))
-	stackitem.EncodeBinaryStackItem(stackitem.NewArray(aer.Stack), w)
+	// Stack items are expected to be marshaled one by one.
+	w.WriteVarUint(uint64(len(aer.Stack)))
+	for _, it := range aer.Stack {
+		stackitem.EncodeBinaryStackItemAppExec(it, w)
+	}
 	w.WriteArray(aer.Events)
 	w.WriteVarBytes([]byte(aer.FaultException))
 }
@@ -68,15 +72,21 @@ func (aer *AppExecResult) DecodeBinary(r *io.BinReader) {
 	aer.Trigger = trigger.Type(r.ReadB())
 	aer.VMState = vm.State(r.ReadB())
 	aer.GasConsumed = int64(r.ReadU64LE())
-	item := stackitem.DecodeBinaryStackItem(r)
+	sz := r.ReadVarUint()
-	if r.Err == nil {
+	if stackitem.MaxArraySize < sz && r.Err == nil {
-		arr, ok := item.Value().([]stackitem.Item)
+		r.Err = errors.New("invalid format")
-		if !ok {
+	}
-			r.Err = errors.New("array expected")
+	if r.Err != nil {
+		return
+	}
+	arr := make([]stackitem.Item, sz)
+	for i := 0; i < int(sz); i++ {
+		arr[i] = stackitem.DecodeBinaryStackItemAppExec(r)
+		if r.Err != nil {
 			return
 		}
-		aer.Stack = arr
 	}
+	aer.Stack = arr
 	r.ReadArray(&aer.Events)
 	aer.FaultException = r.ReadString()
 }
@@ -182,23 +192,22 @@ type executionAux struct {
 
 // MarshalJSON implements implements json.Marshaler interface.
 func (e Execution) MarshalJSON() ([]byte, error) {
-	var st json.RawMessage
+	var errRecursive = []byte(`"error: recursive reference"`)
 	arr := make([]json.RawMessage, len(e.Stack))
 	for i := range arr {
+		if e.Stack[i] == nil {
+			arr[i] = errRecursive
+			continue
+		}
 		data, err := stackitem.ToJSONWithTypes(e.Stack[i])
 		if err != nil {
-			st = []byte(`"error: recursive reference"`)
+			data = errRecursive
-			break
 		}
 		arr[i] = data
 	}
-	var err error
+	st, err := json.Marshal(arr)
-	if st == nil {
+	if err != nil {
-		st, err = json.Marshal(arr)
+		return nil, err
-		if err != nil {
-			return nil, err
-		}
-
 	}
 	return json.Marshal(&executionAux{
 		Trigger:        e.Trigger.String(),
@@ -222,7 +231,11 @@ func (e *Execution) UnmarshalJSON(data []byte) error {
 		for i := range arr {
 			st[i], err = stackitem.FromJSONWithTypes(arr[i])
 			if err != nil {
-				break
+				var s string
+				if json.Unmarshal(arr[i], &s) != nil {
+					break
+				}
+				err = nil
 			}
 		}
 		if err == nil {

pkg/core/state/notification_event_test.go

@@ -6,6 +6,7 @@ import (
 
 	"github.com/nspcc-dev/neo-go/internal/random"
 	"github.com/nspcc-dev/neo-go/internal/testserdes"
+	"github.com/nspcc-dev/neo-go/pkg/io"
 	"github.com/nspcc-dev/neo-go/pkg/smartcontract/trigger"
 	"github.com/nspcc-dev/neo-go/pkg/vm"
 	"github.com/nspcc-dev/neo-go/pkg/vm/stackitem"
@@ -23,35 +24,63 @@ func TestEncodeDecodeNotificationEvent(t *testing.T) {
 }
 
 func TestEncodeDecodeAppExecResult(t *testing.T) {
-	t.Run("halt", func(t *testing.T) {
+	newAer := func() *AppExecResult {
-		appExecResult := &AppExecResult{
+		return &AppExecResult{
 			Container: random.Uint256(),
 			Execution: Execution{
 				Trigger:     1,
 				VMState:     vm.HaltState,
 				GasConsumed: 10,
-				Stack:       []stackitem.Item{},
+				Stack:       []stackitem.Item{stackitem.NewBool(true)},
 				Events:      []NotificationEvent{},
 			},
 		}
-
+	}
+	t.Run("halt", func(t *testing.T) {
+		appExecResult := newAer()
+		appExecResult.VMState = vm.HaltState
 		testserdes.EncodeDecodeBinary(t, appExecResult, new(AppExecResult))
 	})
 	t.Run("fault", func(t *testing.T) {
-		appExecResult := &AppExecResult{
+		appExecResult := newAer()
-			Container: random.Uint256(),
+		appExecResult.VMState = vm.FaultState
-			Execution: Execution{
-				Trigger:        1,
-				VMState:        vm.FaultState,
-				GasConsumed:    10,
-				Stack:          []stackitem.Item{},
-				Events:         []NotificationEvent{},
-				FaultException: "unhandled error",
-			},
-		}
-
 		testserdes.EncodeDecodeBinary(t, appExecResult, new(AppExecResult))
 	})
+	t.Run("with interop", func(t *testing.T) {
+		appExecResult := newAer()
+		appExecResult.Stack = []stackitem.Item{stackitem.NewInterop(nil)}
+		testserdes.EncodeDecodeBinary(t, appExecResult, new(AppExecResult))
+	})
+	t.Run("recursive reference", func(t *testing.T) {
+		var arr = stackitem.NewArray(nil)
+		arr.Append(arr)
+		appExecResult := newAer()
+		appExecResult.Stack = []stackitem.Item{arr, stackitem.NewBool(true), stackitem.NewInterop(123)}
+
+		bs, err := testserdes.EncodeBinary(appExecResult)
+		require.NoError(t, err)
+		actual := new(AppExecResult)
+		require.NoError(t, testserdes.DecodeBinary(bs, actual))
+		require.Equal(t, 3, len(actual.Stack))
+		require.Nil(t, actual.Stack[0])
+		require.Equal(t, true, actual.Stack[1].Value())
+		require.Equal(t, stackitem.InteropT, actual.Stack[2].Type())
+
+		bs1, err := testserdes.EncodeBinary(actual)
+		require.NoError(t, err)
+		require.Equal(t, bs, bs1)
+	})
+	t.Run("invalid item type", func(t *testing.T) {
+		aer := newAer()
+		w := io.NewBufBinWriter()
+		w.WriteBytes(aer.Container[:])
+		w.WriteB(byte(aer.Trigger))
+		w.WriteB(byte(aer.VMState))
+		w.WriteU64LE(uint64(aer.GasConsumed))
+		stackitem.EncodeBinaryStackItem(stackitem.NewBool(true), w.BinWriter)
+		require.NoError(t, w.Err)
+		require.Error(t, testserdes.DecodeBinary(w.Bytes(), new(AppExecResult)))
+	})
 }
 
 func TestMarshalUnmarshalJSONNotificationEvent(t *testing.T) {
@@ -127,7 +156,7 @@ func TestMarshalUnmarshalJSONAppExecResult(t *testing.T) {
 				Trigger:        trigger.Application,
 				VMState:        vm.FaultState,
 				GasConsumed:    10,
-				Stack:          []stackitem.Item{},
+				Stack:          []stackitem.Item{stackitem.NewBool(true)},
 				Events:         []NotificationEvent{},
 				FaultException: "unhandled exception",
 			},
@@ -149,20 +178,28 @@ func TestMarshalUnmarshalJSONAppExecResult(t *testing.T) {
 	})
 
 	t.Run("MarshalJSON recursive reference", func(t *testing.T) {
-		i := make([]stackitem.Item, 1)
+		arr := stackitem.NewArray(nil)
-		recursive := stackitem.NewArray(i)
+		arr.Append(arr)
-		i[0] = recursive
+		errAer := &AppExecResult{
-		errorCases := []*AppExecResult{
+			Execution: Execution{
-			{
+				Trigger: trigger.Application,
-				Execution: Execution{
+				Stack:   []stackitem.Item{arr, stackitem.NewBool(true), stackitem.NewInterop(123)},
-					Stack: i,
-				},
 			},
 		}
-		for _, errCase := range errorCases {
+
-			_, err := json.Marshal(errCase)
+		bs, err := json.Marshal(errAer)
-			require.NoError(t, err)
+		require.NoError(t, err)
-		}
+
+		actual := new(AppExecResult)
+		require.NoError(t, json.Unmarshal(bs, actual))
+		require.Equal(t, 3, len(actual.Stack))
+		require.Nil(t, actual.Stack[0])
+		require.Equal(t, true, actual.Stack[1].Value())
+		require.Equal(t, stackitem.InteropT, actual.Stack[2].Type())
+
+		bs1, err := json.Marshal(actual)
+		require.NoError(t, err)
+		require.Equal(t, bs, bs1)
 	})
 
 	t.Run("UnmarshalJSON error", func(t *testing.T) {

pkg/vm/stackitem/serialization.go

@@ -23,14 +23,30 @@ func SerializeItem(item Item) ([]byte, error) {
 // similar to io.Serializable's EncodeBinary, but works with Item
 // interface.
 func EncodeBinaryStackItem(item Item, w *io.BinWriter) {
-	serializeItemTo(item, w, make(map[Item]bool))
+	serializeItemTo(item, w, false, make(map[Item]bool))
 }
 
-func serializeItemTo(item Item, w *io.BinWriter, seen map[Item]bool) {
+// EncodeBinaryStackItemAppExec encodes given Item into the given BinWriter. It's
+// similar to EncodeBinaryStackItem but allows to encode interop (only type, value is lost).
+func EncodeBinaryStackItemAppExec(item Item, w *io.BinWriter) {
+	bw := io.NewBufBinWriter()
+	serializeItemTo(item, bw.BinWriter, true, make(map[Item]bool))
+	if bw.Err != nil {
+		w.WriteBytes([]byte{byte(InvalidT)})
+		return
+	}
+	w.WriteBytes(bw.Bytes())
+}
+
+func serializeItemTo(item Item, w *io.BinWriter, allowInvalid bool, seen map[Item]bool) {
 	if seen[item] {
 		w.Err = errors.New("recursive structures can't be serialized")
 		return
 	}
+	if item == nil && allowInvalid {
+		w.WriteBytes([]byte{byte(InvalidT)})
+		return
+	}
 
 	switch t := item.(type) {
 	case *ByteArray:
@@ -46,6 +62,10 @@ func serializeItemTo(item Item, w *io.BinWriter, seen map[Item]bool) {
 		w.WriteBytes([]byte{byte(IntegerT)})
 		w.WriteVarBytes(bigint.ToBytes(t.Value().(*big.Int)))
 	case *Interop:
+		if allowInvalid {
+			w.WriteBytes([]byte{byte(InteropT)})
+			return
+		}
 		w.Err = errors.New("interop item can't be serialized")
 	case *Array, *Struct:
 		seen[item] = true
@@ -60,7 +80,7 @@ func serializeItemTo(item Item, w *io.BinWriter, seen map[Item]bool) {
 		arr := t.Value().([]Item)
 		w.WriteVarUint(uint64(len(arr)))
 		for i := range arr {
-			serializeItemTo(arr[i], w, seen)
+			serializeItemTo(arr[i], w, allowInvalid, seen)
 		}
 	case *Map:
 		seen[item] = true
@@ -68,8 +88,8 @@ func serializeItemTo(item Item, w *io.BinWriter, seen map[Item]bool) {
 		w.WriteBytes([]byte{byte(MapT)})
 		w.WriteVarUint(uint64(len(t.Value().([]MapElement))))
 		for i := range t.Value().([]MapElement) {
-			serializeItemTo(t.Value().([]MapElement)[i].Key, w, seen)
+			serializeItemTo(t.Value().([]MapElement)[i].Key, w, allowInvalid, seen)
-			serializeItemTo(t.Value().([]MapElement)[i].Value, w, seen)
+			serializeItemTo(t.Value().([]MapElement)[i].Value, w, allowInvalid, seen)
 		}
 	case Null:
 		w.WriteB(byte(AnyT))
@@ -91,6 +111,16 @@ func DeserializeItem(data []byte) (Item, error) {
 // as a function because Item itself is an interface. Caveat: always check
 // reader's error value before using the returned Item.
 func DecodeBinaryStackItem(r *io.BinReader) Item {
+	return decodeBinaryStackItem(r, false)
+}
+
+// DecodeBinaryStackItemAppExec is similar to DecodeBinaryStackItem
+// but allows Interop values to be present.
+func DecodeBinaryStackItemAppExec(r *io.BinReader) Item {
+	return decodeBinaryStackItem(r, true)
+}
+
+func decodeBinaryStackItem(r *io.BinReader, allowInvalid bool) Item {
 	var t = Type(r.ReadB())
 	if r.Err != nil {
 		return nil
@@ -132,7 +162,15 @@ func DecodeBinaryStackItem(r *io.BinReader) Item {
 		return m
 	case AnyT:
 		return Null{}
+	case InteropT:
+		if allowInvalid {
+			return NewInterop(nil)
+		}
+		fallthrough
 	default:
+		if t == InvalidT && allowInvalid {
+			return nil
+		}
 		r.Err = fmt.Errorf("unknown type: %v", t)
 		return nil
 	}

pkg/vm/stackitem/type.go

@@ -17,6 +17,7 @@ const (
 	StructT    Type = 0x41
 	MapT       Type = 0x48
 	InteropT   Type = 0x60
+	InvalidT   Type = 0xFF
 )
 
 // String implements fmt.Stringer interface.