From 2bcb7bd06f12c6231f4435727057950cc1045e23 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Thu, 17 Nov 2022 20:46:06 +0300 Subject: [PATCH 1/3] compiler: don't use (*VM).Istack when it's not needed --- pkg/compiler/assign_test.go | 4 ++-- pkg/compiler/binary_expr_test.go | 4 ++-- pkg/compiler/convert_test.go | 4 ++-- pkg/compiler/for_test.go | 7 +++---- pkg/compiler/vm_test.go | 2 +- 5 files changed, 10 insertions(+), 11 deletions(-) diff --git a/pkg/compiler/assign_test.go b/pkg/compiler/assign_test.go index b770e8a61..41f5a9dcd 100644 --- a/pkg/compiler/assign_test.go +++ b/pkg/compiler/assign_test.go @@ -8,6 +8,7 @@ import ( "testing" "github.com/nspcc-dev/neo-go/pkg/compiler" + "github.com/nspcc-dev/neo-go/pkg/smartcontract/trigger" "github.com/nspcc-dev/neo-go/pkg/vm" "github.com/stretchr/testify/require" ) @@ -149,8 +150,7 @@ func TestAssignments(t *testing.T) { for i, tc := range assignTestCases { v := vm.New() t.Run(tc.name, func(t *testing.T) { - v.Istack().Clear() - v.Estack().Clear() + v.Reset(trigger.Application) invokeMethod(t, fmt.Sprintf("F%d", i), ne.Script, v, di) runAndCheck(t, v, tc.result) }) diff --git a/pkg/compiler/binary_expr_test.go b/pkg/compiler/binary_expr_test.go index b03a33727..725c66136 100644 --- a/pkg/compiler/binary_expr_test.go +++ b/pkg/compiler/binary_expr_test.go @@ -8,6 +8,7 @@ import ( "testing" "github.com/nspcc-dev/neo-go/pkg/compiler" + "github.com/nspcc-dev/neo-go/pkg/smartcontract/trigger" "github.com/nspcc-dev/neo-go/pkg/vm" "github.com/stretchr/testify/require" ) @@ -238,8 +239,7 @@ func TestBinaryExprs(t *testing.T) { for i, tc := range binaryExprTestCases { v := vm.New() t.Run(tc.name, func(t *testing.T) { - v.Istack().Clear() - v.Estack().Clear() + v.Reset(trigger.Application) invokeMethod(t, fmt.Sprintf("F%d", i), ne.Script, v, di) runAndCheck(t, v, tc.result) }) diff --git a/pkg/compiler/convert_test.go b/pkg/compiler/convert_test.go index 073b153de..eba50b332 100644 --- a/pkg/compiler/convert_test.go +++ b/pkg/compiler/convert_test.go @@ -8,6 +8,7 @@ import ( "testing" "github.com/nspcc-dev/neo-go/pkg/compiler" + "github.com/nspcc-dev/neo-go/pkg/smartcontract/trigger" "github.com/nspcc-dev/neo-go/pkg/vm" "github.com/stretchr/testify/require" ) @@ -72,8 +73,7 @@ func TestConvert(t *testing.T) { for i, tc := range convertTestCases { v := vm.New() t.Run(tc.argValue+getFunctionName(tc.returnType), func(t *testing.T) { - v.Istack().Clear() - v.Estack().Clear() + v.Reset(trigger.Application) invokeMethod(t, fmt.Sprintf("F%d", i), ne.Script, v, di) runAndCheck(t, v, tc.result) }) diff --git a/pkg/compiler/for_test.go b/pkg/compiler/for_test.go index bf451af1c..a07cad913 100644 --- a/pkg/compiler/for_test.go +++ b/pkg/compiler/for_test.go @@ -8,6 +8,7 @@ import ( "testing" "github.com/nspcc-dev/neo-go/pkg/compiler" + "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" "github.com/stretchr/testify/require" @@ -727,8 +728,7 @@ func TestForLoop(t *testing.T) { for i, tc := range forLoopTestCases { v := vm.New() t.Run(tc.name, func(t *testing.T) { - v.Istack().Clear() - v.Estack().Clear() + v.Reset(trigger.Application) invokeMethod(t, fmt.Sprintf("F%d", i), ne.Script, v, di) runAndCheck(t, v, tc.result) }) @@ -785,8 +785,7 @@ func TestForLoopComplexConditions(t *testing.T) { name = tc.Assign } t.Run(name, func(t *testing.T) { - v.Istack().Clear() - v.Estack().Clear() + v.Reset(trigger.Application) invokeMethod(t, fmt.Sprintf("F%d", i), ne.Script, v, di) runAndCheck(t, v, big.NewInt(tc.Result)) }) diff --git a/pkg/compiler/vm_test.go b/pkg/compiler/vm_test.go index a6826a028..58a4cda57 100644 --- a/pkg/compiler/vm_test.go +++ b/pkg/compiler/vm_test.go @@ -84,7 +84,7 @@ func evalWithArgs(t *testing.T, src string, op []byte, args []stackitem.Item, re func assertResult(t *testing.T, vm *vm.VM, result interface{}) { assert.Equal(t, result, vm.PopResult()) - assert.Equal(t, 0, vm.Istack().Len()) + assert.Nil(t, vm.Context()) } func vmAndCompile(t *testing.T, src string) *vm.VM { From cb64957af55350b30c516a5a258bd53287f5b164 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Thu, 17 Nov 2022 22:06:49 +0300 Subject: [PATCH 2/3] vm: don't use Stack for istack We don't use all of the Stack functionality for it, so drop useless methods and avoid some interface conversions. It increases single-node TPS by about 0.9%, so nothing really important there, but not a bad change either. Maybe it can be reworked again with generics though. --- pkg/core/interop/contract/call.go | 4 +-- pkg/core/interop/runtime/engine.go | 2 +- pkg/core/native/oracle.go | 2 +- pkg/vm/context.go | 48 ++------------------------- pkg/vm/invocation_tree_test.go | 2 +- pkg/vm/json_test.go | 2 +- pkg/vm/vm.go | 52 +++++++++++++++--------------- pkg/vm/vm_test.go | 2 +- 8 files changed, 35 insertions(+), 79 deletions(-) diff --git a/pkg/core/interop/contract/call.go b/pkg/core/interop/contract/call.go index aa5989c6d..93124edea 100644 --- a/pkg/core/interop/contract/call.go +++ b/pkg/core/interop/contract/call.go @@ -163,12 +163,12 @@ var ErrNativeCall = errors.New("failed native call") // CallFromNative performs synchronous call from native contract. func CallFromNative(ic *interop.Context, caller util.Uint160, cs *state.Contract, method string, args []stackitem.Item, hasReturn bool) error { - startSize := ic.VM.Istack().Len() + startSize := len(ic.VM.Istack()) if err := callExFromNative(ic, caller, cs, method, args, callflag.All, hasReturn, false, true); err != nil { return err } - for !ic.VM.HasStopped() && ic.VM.Istack().Len() > startSize { + for !ic.VM.HasStopped() && len(ic.VM.Istack()) > startSize { if err := ic.VM.Step(); err != nil { return fmt.Errorf("%w: %v", ErrNativeCall, err) } diff --git a/pkg/core/interop/runtime/engine.go b/pkg/core/interop/runtime/engine.go index 42855c10a..de9055e29 100644 --- a/pkg/core/interop/runtime/engine.go +++ b/pkg/core/interop/runtime/engine.go @@ -41,7 +41,7 @@ func GetCallingScriptHash(ic *interop.Context) error { // GetEntryScriptHash returns entry script hash. func GetEntryScriptHash(ic *interop.Context) error { - return ic.VM.PushContextScriptHash(ic.VM.Istack().Len() - 1) + return ic.VM.PushContextScriptHash(len(ic.VM.Istack()) - 1) } // GetScriptContainer returns transaction or block that contains the script diff --git a/pkg/core/native/oracle.go b/pkg/core/native/oracle.go index 330e640a7..0189f4fd3 100644 --- a/pkg/core/native/oracle.go +++ b/pkg/core/native/oracle.go @@ -276,7 +276,7 @@ func (o *Oracle) finish(ic *interop.Context, _ []stackitem.Item) stackitem.Item // FinishInternal processes an oracle response. func (o *Oracle) FinishInternal(ic *interop.Context) error { - if ic.VM.Istack().Len() != 2 { + if len(ic.VM.Istack()) != 2 { return errors.New("Oracle.finish called from non-entry script") } if ic.Invocations[o.Hash] != 1 { diff --git a/pkg/vm/context.go b/pkg/vm/context.go index cc16ac867..b85ba2f75 100644 --- a/pkg/vm/context.go +++ b/pkg/vm/context.go @@ -5,7 +5,6 @@ import ( "encoding/json" "errors" "fmt" - "math/big" "github.com/nspcc-dev/neo-go/pkg/crypto/hash" "github.com/nspcc-dev/neo-go/pkg/smartcontract/callflag" @@ -251,42 +250,6 @@ func (c *Context) NumOfReturnVals() int { return c.retCount } -// Value implements the stackitem.Item interface. -func (c *Context) Value() interface{} { - return c -} - -// Dup implements the stackitem.Item interface. -func (c *Context) Dup() stackitem.Item { - return c -} - -// TryBool implements the stackitem.Item interface. -func (c *Context) TryBool() (bool, error) { panic("can't convert Context to Bool") } - -// TryBytes implements the stackitem.Item interface. -func (c *Context) TryBytes() ([]byte, error) { - return nil, errors.New("can't convert Context to ByteArray") -} - -// TryInteger implements the stackitem.Item interface. -func (c *Context) TryInteger() (*big.Int, error) { - return nil, errors.New("can't convert Context to Integer") -} - -// Type implements the stackitem.Item interface. -func (c *Context) Type() stackitem.Type { panic("Context cannot appear on evaluation stack") } - -// Convert implements the stackitem.Item interface. -func (c *Context) Convert(_ stackitem.Type) (stackitem.Item, error) { - panic("Context cannot be converted to anything") -} - -// Equals implements the stackitem.Item interface. -func (c *Context) Equals(s stackitem.Item) bool { - return c == s -} - func (c *Context) atBreakPoint() bool { for _, n := range c.sc.breakPoints { if n == c.nextip { @@ -296,10 +259,6 @@ func (c *Context) atBreakPoint() bool { return false } -func (c *Context) String() string { - return "execution context" -} - // IsDeployed returns whether this context contains a deployed contract. func (c *Context) IsDeployed() bool { return c.sc.NEF != nil @@ -332,13 +291,10 @@ func dumpSlot(s *slot) string { // getContextScriptHash returns script hash of the invocation stack element // number n. func (v *VM) getContextScriptHash(n int) util.Uint160 { - istack := v.Istack() - if istack.Len() <= n { + if len(v.istack) <= n { return util.Uint160{} } - element := istack.Peek(n) - ctx := element.value.(*Context) - return ctx.ScriptHash() + return v.istack[len(v.istack)-1-n].ScriptHash() } // IsCalledByEntry checks parent script contexts and return true if the current one diff --git a/pkg/vm/invocation_tree_test.go b/pkg/vm/invocation_tree_test.go index b3f3896c7..b4eef159c 100644 --- a/pkg/vm/invocation_tree_test.go +++ b/pkg/vm/invocation_tree_test.go @@ -24,7 +24,7 @@ func TestInvocationTree(t *testing.T) { cnt := 0 v := newTestVM() v.SyscallHandler = func(v *VM, _ uint32) error { - if v.Istack().Len() > 4 { // top -> call -> syscall -> call -> syscall -> ... + if len(v.Istack()) > 4 { // top -> call -> syscall -> call -> syscall -> ... v.Estack().PushVal(1) return nil } diff --git a/pkg/vm/json_test.go b/pkg/vm/json_test.go index 8256a6840..ada80b499 100644 --- a/pkg/vm/json_test.go +++ b/pkg/vm/json_test.go @@ -166,7 +166,7 @@ func testFile(t *testing.T, filename string) { if len(result.InvocationStack) > 0 { for i, s := range result.InvocationStack { - ctx := vm.istack.Peek(i).Value().(*Context) + ctx := vm.istack[len(vm.istack)-1-i] if ctx.nextip < len(ctx.sc.prog) { require.Equal(t, s.InstructionPointer, ctx.nextip) op, err := opcode.FromString(s.Instruction) diff --git a/pkg/vm/vm.go b/pkg/vm/vm.go index 7cc768d0f..92f9db474 100644 --- a/pkg/vm/vm.go +++ b/pkg/vm/vm.go @@ -69,8 +69,8 @@ type VM struct { // callback to get interop price getPrice func(opcode.Opcode, []byte) int64 - istack Stack // invocation stack. - estack *Stack // execution stack. + istack []*Context // invocation stack. + estack *Stack // execution stack. uncaughtException stackitem.Item // exception being handled @@ -110,8 +110,7 @@ func NewWithTrigger(t trigger.Type) *VM { trigger: t, } - initStack(&vm.istack, "invocation", nil) - vm.istack.elems = make([]Element, 0, 8) // Most of invocations use one-two contracts, but they're likely to have internal calls. + vm.istack = make([]*Context, 0, 8) // Most of invocations use one-two contracts, but they're likely to have internal calls. vm.estack = newStack("evaluation", &vm.refs) return vm } @@ -128,7 +127,7 @@ func (v *VM) SetPriceGetter(f func(opcode.Opcode, []byte) int64) { func (v *VM) Reset(t trigger.Type) { v.state = vmstate.None v.getPrice = nil - v.istack.elems = v.istack.elems[:0] + v.istack = v.istack[:0] v.estack.elems = v.estack.elems[:0] v.uncaughtException = nil v.refs = 0 @@ -157,8 +156,8 @@ func (v *VM) Estack() *Stack { } // Istack returns the invocation stack, so interop hooks can utilize this. -func (v *VM) Istack() *Stack { - return &v.istack +func (v *VM) Istack() []*Context { + return v.istack } // PrintOps prints the opcodes of the current loaded program to stdout. @@ -284,7 +283,7 @@ func (v *VM) Load(prog []byte) { // LoadWithFlags initializes the VM with the program and flags given. func (v *VM) LoadWithFlags(prog []byte, f callflag.CallFlag) { // Clear all stacks and state, it could be a reload. - v.istack.Clear() + v.istack = v.istack[:0] v.estack.Clear() v.state = vmstate.None v.gasConsumed = 0 @@ -359,16 +358,16 @@ func (v *VM) loadScriptWithCallingHash(b []byte, exe *nef.File, caller util.Uint ctx.sc.invTree = newTree } ctx.sc.onUnload = onContextUnload - v.istack.PushItem(ctx) + v.istack = append(v.istack, ctx) } // Context returns the current executed context. Nil if there is no context, // which implies no program is loaded. func (v *VM) Context() *Context { - if v.istack.Len() == 0 { + if len(v.istack) == 0 { return nil } - return v.istack.Peek(0).value.(*Context) + return v.istack[len(v.istack)-1] } // PopResult is used to pop the first item of the evaluation stack. This allows @@ -382,7 +381,7 @@ func (v *VM) PopResult() interface{} { // DumpIStack returns json formatted representation of the invocation stack. func (v *VM) DumpIStack() string { - b, _ := json.MarshalIndent(v.istack.ToArray(), "", " ") + b, _ := json.MarshalIndent(v.istack, "", " ") return string(b) } @@ -405,7 +404,7 @@ func (v *VM) State() vmstate.State { // Ready returns true if the VM is ready to execute the loaded program. // It will return false if no program is loaded. func (v *VM) Ready() bool { - return v.istack.Len() > 0 + return len(v.istack) > 0 } // Run starts execution of the loaded program. @@ -505,8 +504,8 @@ func (v *VM) StepOut() error { v.state = vmstate.None } - expSize := v.istack.Len() - for v.state == vmstate.None && v.istack.Len() >= expSize { + expSize := len(v.istack) + for v.state == vmstate.None && len(v.istack) >= expSize { err = v.StepInto() } if v.state == vmstate.None { @@ -527,10 +526,10 @@ func (v *VM) StepOver() error { v.state = vmstate.None } - expSize := v.istack.Len() + expSize := len(v.istack) for { err = v.StepInto() - if !(v.state == vmstate.None && v.istack.Len() > expSize) { + if !(v.state == vmstate.None && len(v.istack) > expSize) { break } } @@ -1467,11 +1466,12 @@ func (v *VM) execute(ctx *Context, op opcode.Opcode, parameter []byte) (err erro } case opcode.RET: - oldCtx := v.istack.Pop().value.(*Context) + oldCtx := v.istack[len(v.istack)-1] + v.istack = v.istack[:len(v.istack)-1] oldEstack := v.estack v.unloadContext(oldCtx) - if v.istack.Len() == 0 { + if len(v.istack) == 0 { v.state = vmstate.Halt break } @@ -1701,7 +1701,7 @@ func (v *VM) call(ctx *Context, offset int) { } // New context -> new exception handlers. newCtx.tryStack.elems = ctx.tryStack.elems[len(ctx.tryStack.elems):] - v.istack.PushItem(newCtx) + v.istack = append(v.istack, newCtx) newCtx.Jump(offset) } @@ -1739,9 +1739,8 @@ func calcJumpOffset(ctx *Context, parameter []byte) (int, int, error) { } func (v *VM) handleException() { - for pop := 0; pop < v.istack.Len(); pop++ { - ictxv := v.istack.Peek(pop) - ictx := ictxv.value.(*Context) + for pop := 0; pop < len(v.istack); pop++ { + ictx := v.istack[len(v.istack)-1-pop] for j := 0; j < ictx.tryStack.Len(); j++ { e := ictx.tryStack.Peek(j) ectx := e.Value().(*exceptionHandlingContext) @@ -1751,7 +1750,8 @@ func (v *VM) handleException() { continue } for i := 0; i < pop; i++ { - ctx := v.istack.Pop().value.(*Context) + ctx := v.istack[len(v.istack)-1] + v.istack = v.istack[:len(v.istack)-1] v.unloadContext(ctx) } if ectx.State == eTry && ectx.HasCatch() { @@ -1937,7 +1937,7 @@ func validateMapKey(key Element) { } func (v *VM) checkInvocationStackSize() { - if v.istack.Len() >= MaxInvocationStackSize { + if len(v.istack) >= MaxInvocationStackSize { panic("invocation stack is too big") } } @@ -1959,7 +1959,7 @@ func (v *VM) GetCallingScriptHash() util.Uint160 { // GetEntryScriptHash implements the ScriptHashGetter interface. func (v *VM) GetEntryScriptHash() util.Uint160 { - return v.getContextScriptHash(v.istack.Len() - 1) + return v.getContextScriptHash(len(v.istack) - 1) } // GetCurrentScriptHash implements the ScriptHashGetter interface. diff --git a/pkg/vm/vm_test.go b/pkg/vm/vm_test.go index 48807040f..d78ab22c3 100644 --- a/pkg/vm/vm_test.go +++ b/pkg/vm/vm_test.go @@ -124,7 +124,7 @@ func TestPushBytes1to75(t *testing.T) { errExec := vm.execute(nil, opcode.RET, nil) require.NoError(t, errExec) - assert.Equal(t, 0, vm.istack.Len()) + assert.Nil(t, vm.Context()) buf.Reset() } } From 8e7f65be170aedf400f97363078a54d7e642437f Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Fri, 18 Nov 2022 10:51:29 +0300 Subject: [PATCH 3/3] vm: use proper estack for exception handler v.estack might be some inner invoked contract and its stack must not be used for exception handler set up by higher-order contract. --- pkg/vm/vm.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/vm/vm.go b/pkg/vm/vm.go index 92f9db474..ef8c41351 100644 --- a/pkg/vm/vm.go +++ b/pkg/vm/vm.go @@ -1754,6 +1754,7 @@ func (v *VM) handleException() { v.istack = v.istack[:len(v.istack)-1] v.unloadContext(ctx) } + v.estack = ictx.sc.estack if ectx.State == eTry && ectx.HasCatch() { ectx.State = eCatch v.estack.PushItem(v.uncaughtException)