From 7e7ce7f19f0033a9d22b90600dcd45890bf8e69b Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Fri, 7 Feb 2020 11:53:53 +0300 Subject: [PATCH 1/3] vm: rephrase serialization errors Seeing some blockQueue: failed adding block into the blockchain {"error": "failed to store notifications: not supported", "blockHeight": 713984, "nextIndex": 713985} in logs is not very helpful. --- pkg/vm/serialization.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/vm/serialization.go b/pkg/vm/serialization.go index 245102f20..af2da3152 100644 --- a/pkg/vm/serialization.go +++ b/pkg/vm/serialization.go @@ -35,7 +35,7 @@ func EncodeBinaryStackItem(item StackItem, w *io.BinWriter) { func serializeItemTo(item StackItem, w *io.BinWriter, seen map[StackItem]bool) { if seen[item] { - w.Err = errors.New("recursive structures are not supported") + w.Err = errors.New("recursive structures can't be serialized") return } @@ -50,7 +50,7 @@ func serializeItemTo(item StackItem, w *io.BinWriter, seen map[StackItem]bool) { w.WriteBytes([]byte{byte(integerT)}) w.WriteVarBytes(IntToBytes(t.value)) case *InteropItem: - w.Err = errors.New("not supported") + w.Err = errors.New("interop item can't be serialized") case *ArrayItem, *StructItem: seen[item] = true From b805b1a71bd8e3bd62322ad625bb1699da8915b8 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Fri, 7 Feb 2020 12:16:47 +0300 Subject: [PATCH 2/3] vm: make SerializeItem/DeserializeItem public APIs They're useful as wrappers around EncodeBinaryStackItem/DecodeBinaryStackItem. --- pkg/vm/interop.go | 4 ++-- pkg/vm/serialization.go | 6 ++++-- pkg/vm/vm_test.go | 2 +- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/pkg/vm/interop.go b/pkg/vm/interop.go index 6247d5a70..b6f6b8221 100644 --- a/pkg/vm/interop.go +++ b/pkg/vm/interop.go @@ -93,7 +93,7 @@ func runtimeNotify(vm *VM) error { // RuntimeSerialize handles syscalls System.Runtime.Serialize and Neo.Runtime.Serialize. func RuntimeSerialize(vm *VM) error { item := vm.Estack().Pop() - data, err := serializeItem(item.value) + data, err := SerializeItem(item.value) if err != nil { return err } else if len(data) > MaxItemSize { @@ -109,7 +109,7 @@ func RuntimeSerialize(vm *VM) error { func RuntimeDeserialize(vm *VM) error { data := vm.Estack().Pop().Bytes() - item, err := deserializeItem(data) + item, err := DeserializeItem(data) if err != nil { return err } diff --git a/pkg/vm/serialization.go b/pkg/vm/serialization.go index af2da3152..81c2fc6b1 100644 --- a/pkg/vm/serialization.go +++ b/pkg/vm/serialization.go @@ -17,7 +17,8 @@ const ( mapT stackItemType = 0x82 ) -func serializeItem(item StackItem) ([]byte, error) { +// SerializeItem encodes given StackItem into the byte slice. +func SerializeItem(item StackItem) ([]byte, error) { w := io.NewBufBinWriter() EncodeBinaryStackItem(item, w.BinWriter) if w.Err != nil { @@ -78,7 +79,8 @@ func serializeItemTo(item StackItem, w *io.BinWriter, seen map[StackItem]bool) { } } -func deserializeItem(data []byte) (StackItem, error) { +// DeserializeItem decodes StackItem from the given byte slice. +func DeserializeItem(data []byte) (StackItem, error) { r := io.NewBinReaderFromBuf(data) item := DecodeBinaryStackItem(r) if r.Err != nil { diff --git a/pkg/vm/vm_test.go b/pkg/vm/vm_test.go index cab01864a..5e236cf0a 100644 --- a/pkg/vm/vm_test.go +++ b/pkg/vm/vm_test.go @@ -653,7 +653,7 @@ func TestDeserializeUnknown(t *testing.T) { prog := append(getSyscallProg("Neo.Runtime.Deserialize"), byte(opcode.RET)) vm := load(prog) - data, err := serializeItem(NewBigIntegerItem(123)) + data, err := SerializeItem(NewBigIntegerItem(123)) require.NoError(t, err) data[0] = 0xFF From 7ccf7974b68b18d3050fd8dc6d4e993d899ab5df Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Fri, 7 Feb 2020 12:17:39 +0300 Subject: [PATCH 3/3] core: substitute bad notifications with error messages Tesnet sync failed with: Feb 07 00:04:19 nodoka neo-go[1747]: 2020-02-07T00:04:19.838+0300 WARN blockQueue: failed adding block into the blockchain {"error": "failed to store notifications: not supported", "blockHeight": 713984, "nextIndex": 713985} because some (not so) smart contract emitted a notification with an InteropItem inside. --- pkg/core/interop_system.go | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/pkg/core/interop_system.go b/pkg/core/interop_system.go index 9d8c930a8..811f036ae 100644 --- a/pkg/core/interop_system.go +++ b/pkg/core/interop_system.go @@ -343,7 +343,17 @@ func (ic *interopContext) runtimeCheckWitness(v *vm.VM) error { func (ic *interopContext) runtimeNotify(v *vm.VM) error { // It can be just about anything. e := v.Estack().Pop() - ne := state.NotificationEvent{ScriptHash: getContextScriptHash(v, 0), Item: e.Item()} + item := e.Item() + // But it has to be serializable, otherwise we either have some broken + // (recursive) structure inside or an interop item that can't be used + // outside of the interop subsystem anyway. I'd probably fail transactions + // that emit such broken notifications, but that might break compatibility + // with testnet/mainnet, so we're replacing these with error messages. + _, err := vm.SerializeItem(item) + if err != nil { + item = vm.NewByteArrayItem([]byte(fmt.Sprintf("bad notification: %v", err))) + } + ne := state.NotificationEvent{ScriptHash: getContextScriptHash(v, 0), Item: item} ic.notifications = append(ic.notifications, ne) return nil }