From 9b4fd99fbc0dd040c03eeef4cb7fcad01ee47b9a Mon Sep 17 00:00:00 2001 From: Roman Khimov <roman@nspcc.ru> Date: Tue, 3 Mar 2020 12:52:56 +0300 Subject: [PATCH 1/3] vm: break circular references when recursing into ToContractParameters Reference types can have circular pointers to each other, thus we need to control recursion. --- pkg/rpc/response/result/application_log.go | 4 +- pkg/vm/context.go | 2 +- pkg/vm/stack_item.go | 51 +++++++++++++--------- pkg/vm/stack_item_test.go | 3 +- 4 files changed, 37 insertions(+), 23 deletions(-) diff --git a/pkg/rpc/response/result/application_log.go b/pkg/rpc/response/result/application_log.go index 1788d87cd..9e4f7fcc8 100644 --- a/pkg/rpc/response/result/application_log.go +++ b/pkg/rpc/response/result/application_log.go @@ -6,6 +6,7 @@ import ( "github.com/CityOfZion/neo-go/pkg/core/state" "github.com/CityOfZion/neo-go/pkg/smartcontract" "github.com/CityOfZion/neo-go/pkg/util" + "github.com/CityOfZion/neo-go/pkg/vm" ) // ApplicationLog wrapper used for the representation of the @@ -35,7 +36,8 @@ type NotificationEvent struct { func NewApplicationLog(appExecRes *state.AppExecResult, scriptHash util.Uint160) ApplicationLog { events := make([]NotificationEvent, 0, len(appExecRes.Events)) for _, e := range appExecRes.Events { - item := e.Item.ToContractParameter() + seen := make(map[vm.StackItem]bool) + item := e.Item.ToContractParameter(seen) events = append(events, NotificationEvent{ Contract: e.ScriptHash, Item: item, diff --git a/pkg/vm/context.go b/pkg/vm/context.go index bfabd0a34..462531711 100644 --- a/pkg/vm/context.go +++ b/pkg/vm/context.go @@ -171,7 +171,7 @@ func (c *Context) Dup() StackItem { } // ToContractParameter implements StackItem interface. -func (c *Context) ToContractParameter() smartcontract.Parameter { +func (c *Context) ToContractParameter(map[StackItem]bool) smartcontract.Parameter { panic("Not implemented") } diff --git a/pkg/vm/stack_item.go b/pkg/vm/stack_item.go index 7c19e0b33..d7184abdf 100644 --- a/pkg/vm/stack_item.go +++ b/pkg/vm/stack_item.go @@ -19,7 +19,7 @@ type StackItem interface { // Dup duplicates current StackItem. Dup() StackItem // ToContractParameter converts StackItem to smartcontract.Parameter - ToContractParameter() smartcontract.Parameter + ToContractParameter(map[StackItem]bool) smartcontract.Parameter } func makeStackItem(v interface{}) StackItem { @@ -119,11 +119,15 @@ func (i *StructItem) Dup() StackItem { } // ToContractParameter implements StackItem interface. -func (i *StructItem) ToContractParameter() smartcontract.Parameter { +func (i *StructItem) ToContractParameter(seen map[StackItem]bool) smartcontract.Parameter { var value []smartcontract.Parameter - for _, stackItem := range i.value { - parameter := stackItem.ToContractParameter() - value = append(value, parameter) + + if !seen[i] { + seen[i] = true + for _, stackItem := range i.value { + parameter := stackItem.ToContractParameter(seen) + value = append(value, parameter) + } } return smartcontract.Parameter{ Type: smartcontract.ArrayType, @@ -179,7 +183,7 @@ func (i *BigIntegerItem) Dup() StackItem { } // ToContractParameter implements StackItem interface. -func (i *BigIntegerItem) ToContractParameter() smartcontract.Parameter { +func (i *BigIntegerItem) ToContractParameter(map[StackItem]bool) smartcontract.Parameter { return smartcontract.Parameter{ Type: smartcontract.IntegerType, Value: i.value.Int64(), @@ -223,7 +227,7 @@ func (i *BoolItem) Dup() StackItem { } // ToContractParameter implements StackItem interface. -func (i *BoolItem) ToContractParameter() smartcontract.Parameter { +func (i *BoolItem) ToContractParameter(map[StackItem]bool) smartcontract.Parameter { return smartcontract.Parameter{ Type: smartcontract.BoolType, Value: i.value, @@ -264,7 +268,7 @@ func (i *ByteArrayItem) Dup() StackItem { } // ToContractParameter implements StackItem interface. -func (i *ByteArrayItem) ToContractParameter() smartcontract.Parameter { +func (i *ByteArrayItem) ToContractParameter(map[StackItem]bool) smartcontract.Parameter { return smartcontract.Parameter{ Type: smartcontract.ByteArrayType, Value: i.value, @@ -304,11 +308,15 @@ func (i *ArrayItem) Dup() StackItem { } // ToContractParameter implements StackItem interface. -func (i *ArrayItem) ToContractParameter() smartcontract.Parameter { +func (i *ArrayItem) ToContractParameter(seen map[StackItem]bool) smartcontract.Parameter { var value []smartcontract.Parameter - for _, stackItem := range i.value { - parameter := stackItem.ToContractParameter() - value = append(value, parameter) + + if !seen[i] { + seen[i] = true + for _, stackItem := range i.value { + parameter := stackItem.ToContractParameter(seen) + value = append(value, parameter) + } } return smartcontract.Parameter{ Type: smartcontract.ArrayType, @@ -350,15 +358,18 @@ func (i *MapItem) Dup() StackItem { } // ToContractParameter implements StackItem interface. -func (i *MapItem) ToContractParameter() smartcontract.Parameter { +func (i *MapItem) ToContractParameter(seen map[StackItem]bool) smartcontract.Parameter { value := make(map[smartcontract.Parameter]smartcontract.Parameter) - for key, val := range i.value { - pValue := val.ToContractParameter() - pKey := fromMapKey(key).ToContractParameter() - if pKey.Type == smartcontract.ByteArrayType { - pKey.Value = string(pKey.Value.([]byte)) + if !seen[i] { + seen[i] = true + for key, val := range i.value { + pValue := val.ToContractParameter(seen) + pKey := fromMapKey(key).ToContractParameter(seen) + if pKey.Type == smartcontract.ByteArrayType { + pKey.Value = string(pKey.Value.([]byte)) + } + value[pKey] = pValue } - value[pKey] = pValue } return smartcontract.Parameter{ Type: smartcontract.MapType, @@ -428,7 +439,7 @@ func (i *InteropItem) Dup() StackItem { } // ToContractParameter implements StackItem interface. -func (i *InteropItem) ToContractParameter() smartcontract.Parameter { +func (i *InteropItem) ToContractParameter(map[StackItem]bool) smartcontract.Parameter { return smartcontract.Parameter{ Type: smartcontract.InteropInterfaceType, Value: nil, diff --git a/pkg/vm/stack_item_test.go b/pkg/vm/stack_item_test.go index af54b4110..24ca5f2f9 100644 --- a/pkg/vm/stack_item_test.go +++ b/pkg/vm/stack_item_test.go @@ -60,7 +60,8 @@ var toContractParameterTestCases = []struct { func TestToContractParameter(t *testing.T) { for _, tc := range toContractParameterTestCases { - res := tc.input.ToContractParameter() + seen := make(map[StackItem]bool) + res := tc.input.ToContractParameter(seen) assert.Equal(t, res, tc.result) } } From 56e37ad6bad4af49a93e7be8d3aee620206fd130 Mon Sep 17 00:00:00 2001 From: Roman Khimov <roman@nspcc.ru> Date: Tue, 3 Mar 2020 13:05:57 +0300 Subject: [PATCH 2/3] vm: drop duplicating stackItem structure, build JSON from Parameters smartcontract.Parameter has everything needed now. --- pkg/vm/context.go | 5 ++++- pkg/vm/output.go | 47 ----------------------------------------------- pkg/vm/stack.go | 14 +++++++++++++- pkg/vm/vm.go | 4 +++- 4 files changed, 20 insertions(+), 50 deletions(-) delete mode 100644 pkg/vm/output.go diff --git a/pkg/vm/context.go b/pkg/vm/context.go index 462531711..cb540cea2 100644 --- a/pkg/vm/context.go +++ b/pkg/vm/context.go @@ -172,7 +172,10 @@ func (c *Context) Dup() StackItem { // ToContractParameter implements StackItem interface. func (c *Context) ToContractParameter(map[StackItem]bool) smartcontract.Parameter { - panic("Not implemented") + return smartcontract.Parameter{ + Type: smartcontract.StringType, + Value: c.String(), + } } func (c *Context) atBreakPoint() bool { diff --git a/pkg/vm/output.go b/pkg/vm/output.go deleted file mode 100644 index 46c27d3f9..000000000 --- a/pkg/vm/output.go +++ /dev/null @@ -1,47 +0,0 @@ -package vm - -import "encoding/json" - -// StackOutput holds information about the stack, used for pretty printing -// the stack. -type stackItem struct { - Value interface{} `json:"value"` - Type string `json:"type"` -} - -func appendToItems(items *[]stackItem, val StackItem, seen map[StackItem]bool) { - if arr, ok := val.Value().([]StackItem); ok { - if seen[val] { - return - } - seen[val] = true - intItems := make([]stackItem, 0, len(arr)) - for _, v := range arr { - appendToItems(&intItems, v, seen) - } - *items = append(*items, stackItem{ - Value: intItems, - Type: val.String(), - }) - - } else { - *items = append(*items, stackItem{ - Value: val, - Type: val.String(), - }) - } -} - -func stackToArray(s *Stack) []stackItem { - items := make([]stackItem, 0, s.Len()) - seen := make(map[StackItem]bool) - s.IterBack(func(e *Element) { - appendToItems(&items, e.value, seen) - }) - return items -} - -func buildStackOutput(s *Stack) string { - b, _ := json.MarshalIndent(stackToArray(s), "", " ") - return string(b) -} diff --git a/pkg/vm/stack.go b/pkg/vm/stack.go index b968d9cc2..bc3142363 100644 --- a/pkg/vm/stack.go +++ b/pkg/vm/stack.go @@ -6,6 +6,7 @@ import ( "fmt" "math/big" + "github.com/CityOfZion/neo-go/pkg/smartcontract" "github.com/CityOfZion/neo-go/pkg/vm/emit" ) @@ -469,7 +470,18 @@ func (s *Stack) popSigElements() ([][]byte, error) { return elems, nil } +// ToContractParameters converts Stack to slice of smartcontract.Parameter. +func (s *Stack) ToContractParameters() []smartcontract.Parameter { + items := make([]smartcontract.Parameter, 0, s.Len()) + s.IterBack(func(e *Element) { + // Each item is independent. + seen := make(map[StackItem]bool) + items = append(items, e.value.ToContractParameter(seen)) + }) + return items +} + // MarshalJSON implements JSON marshalling interface. func (s *Stack) MarshalJSON() ([]byte, error) { - return json.Marshal(stackToArray(s)) + return json.Marshal(s.ToContractParameters()) } diff --git a/pkg/vm/vm.go b/pkg/vm/vm.go index b08795ef7..e5300377b 100644 --- a/pkg/vm/vm.go +++ b/pkg/vm/vm.go @@ -3,6 +3,7 @@ package vm import ( "crypto/sha1" "encoding/binary" + "encoding/json" "fmt" "io/ioutil" "math/big" @@ -301,7 +302,8 @@ func (v *VM) Stack(n string) string { if n == "estack" { s = v.estack } - return buildStackOutput(s) + b, _ := json.MarshalIndent(s.ToContractParameters(), "", " ") + return string(b) } // State returns string representation of the state for the VM. From 4c8d327353c05d1ee83b499d293aa55a0793a35c Mon Sep 17 00:00:00 2001 From: Roman Khimov <roman@nspcc.ru> Date: Tue, 3 Mar 2020 13:08:34 +0300 Subject: [PATCH 3/3] rpc: drop duplicating Invoke* structures And use smartcontract.Parameter instead of vm.StackItem where appropriate. Closes #689. --- cli/smartcontract/smart_contract.go | 4 ++-- pkg/rpc/client/rpc.go | 13 ++++++------- pkg/rpc/response/result/invoke.go | 4 ++-- pkg/rpc/response/types.go | 11 ----------- pkg/rpc/server/server.go | 2 +- pkg/rpc/server/server_helper_test.go | 10 ---------- pkg/rpc/server/server_test.go | 12 ++++++------ 7 files changed, 17 insertions(+), 39 deletions(-) diff --git a/cli/smartcontract/smart_contract.go b/cli/smartcontract/smart_contract.go index db59a2844..aec4f901f 100644 --- a/cli/smartcontract/smart_contract.go +++ b/cli/smartcontract/smart_contract.go @@ -16,7 +16,7 @@ import ( "github.com/CityOfZion/neo-go/pkg/crypto/keys" "github.com/CityOfZion/neo-go/pkg/rpc/client" "github.com/CityOfZion/neo-go/pkg/rpc/request" - "github.com/CityOfZion/neo-go/pkg/rpc/response" + "github.com/CityOfZion/neo-go/pkg/rpc/response/result" "github.com/CityOfZion/neo-go/pkg/smartcontract" "github.com/CityOfZion/neo-go/pkg/util" "github.com/CityOfZion/neo-go/pkg/vm" @@ -364,7 +364,7 @@ func invokeInternal(ctx *cli.Context, withMethod bool, signAndPush bool) error { operation string params = make([]smartcontract.Parameter, 0) paramsStart = 1 - resp *response.InvokeResult + resp *result.Invoke wif *keys.WIF ) diff --git a/pkg/rpc/client/rpc.go b/pkg/rpc/client/rpc.go index 5cb38dbeb..0fdf06ab7 100644 --- a/pkg/rpc/client/rpc.go +++ b/pkg/rpc/client/rpc.go @@ -7,7 +7,6 @@ import ( "github.com/CityOfZion/neo-go/pkg/core/transaction" "github.com/CityOfZion/neo-go/pkg/crypto/keys" "github.com/CityOfZion/neo-go/pkg/rpc/request" - "github.com/CityOfZion/neo-go/pkg/rpc/response" "github.com/CityOfZion/neo-go/pkg/rpc/response/result" "github.com/CityOfZion/neo-go/pkg/smartcontract" "github.com/CityOfZion/neo-go/pkg/util" @@ -68,10 +67,10 @@ func (c *Client) GetUnspents(address string) (*result.Unspents, error) { // InvokeScript returns the result of the given script after running it true the VM. // NOTE: This is a test invoke and will not affect the blockchain. -func (c *Client) InvokeScript(script string) (*response.InvokeResult, error) { +func (c *Client) InvokeScript(script string) (*result.Invoke, error) { var ( params = request.NewRawParams(script) - resp = &response.InvokeResult{} + resp = &result.Invoke{} ) if err := c.performRequest("invokescript", params, resp); err != nil { return nil, err @@ -82,10 +81,10 @@ func (c *Client) InvokeScript(script string) (*response.InvokeResult, error) { // InvokeFunction returns the results after calling the smart contract scripthash // with the given operation and parameters. // NOTE: this is test invoke and will not affect the blockchain. -func (c *Client) InvokeFunction(script, operation string, params []smartcontract.Parameter) (*response.InvokeResult, error) { +func (c *Client) InvokeFunction(script, operation string, params []smartcontract.Parameter) (*result.Invoke, error) { var ( p = request.NewRawParams(script, operation, params) - resp = &response.InvokeResult{} + resp = &result.Invoke{} ) if err := c.performRequest("invokefunction", p, resp); err != nil { return nil, err @@ -95,10 +94,10 @@ func (c *Client) InvokeFunction(script, operation string, params []smartcontract // Invoke returns the results after calling the smart contract scripthash // with the given parameters. -func (c *Client) Invoke(script string, params []smartcontract.Parameter) (*response.InvokeResult, error) { +func (c *Client) Invoke(script string, params []smartcontract.Parameter) (*result.Invoke, error) { var ( p = request.NewRawParams(script, params) - resp = &response.InvokeResult{} + resp = &result.Invoke{} ) if err := c.performRequest("invoke", p, resp); err != nil { return nil, err diff --git a/pkg/rpc/response/result/invoke.go b/pkg/rpc/response/result/invoke.go index f79d4d365..171fd1b65 100644 --- a/pkg/rpc/response/result/invoke.go +++ b/pkg/rpc/response/result/invoke.go @@ -1,7 +1,7 @@ package result import ( - "github.com/CityOfZion/neo-go/pkg/vm" + "github.com/CityOfZion/neo-go/pkg/smartcontract" ) // Invoke represents code invocation result and is used by several RPC calls @@ -10,5 +10,5 @@ type Invoke struct { State string `json:"state"` GasConsumed string `json:"gas_consumed"` Script string `json:"script"` - Stack *vm.Stack + Stack []smartcontract.Parameter } diff --git a/pkg/rpc/response/types.go b/pkg/rpc/response/types.go index a655d6bab..7c53040fc 100644 --- a/pkg/rpc/response/types.go +++ b/pkg/rpc/response/types.go @@ -4,19 +4,8 @@ import ( "encoding/json" "github.com/CityOfZion/neo-go/pkg/rpc/response/result" - "github.com/CityOfZion/neo-go/pkg/smartcontract" - "github.com/CityOfZion/neo-go/pkg/vm" ) -// InvokeResult represents the outcome of a script that is -// executed by the NEO VM. -type InvokeResult struct { - State vm.State `json:"state"` - GasConsumed string `json:"gas_consumed"` - Script string `json:"script"` - Stack []smartcontract.Parameter -} - // Header is a generic JSON-RPC 2.0 response header (ID and JSON-RPC version). type Header struct { ID json.RawMessage `json:"id"` diff --git a/pkg/rpc/server/server.go b/pkg/rpc/server/server.go index 119786933..9b5967554 100644 --- a/pkg/rpc/server/server.go +++ b/pkg/rpc/server/server.go @@ -624,7 +624,7 @@ func (s *Server) runScriptInVM(script []byte) *result.Invoke { State: vm.State(), GasConsumed: vm.GasConsumed().String(), Script: hex.EncodeToString(script), - Stack: vm.Estack(), + Stack: vm.Estack().ToContractParameters(), } return result } diff --git a/pkg/rpc/server/server_helper_test.go b/pkg/rpc/server/server_helper_test.go index 39198c317..6bfeaba4c 100644 --- a/pkg/rpc/server/server_helper_test.go +++ b/pkg/rpc/server/server_helper_test.go @@ -12,21 +12,11 @@ import ( "github.com/CityOfZion/neo-go/pkg/core/transaction" "github.com/CityOfZion/neo-go/pkg/io" "github.com/CityOfZion/neo-go/pkg/network" - "github.com/CityOfZion/neo-go/pkg/rpc/request" "github.com/CityOfZion/neo-go/pkg/util" "github.com/stretchr/testify/require" "go.uber.org/zap/zaptest" ) -// InvokeFunctionResult struct for testing. -type InvokeFunctionResult struct { - Script string `json:"script"` - State string `json:"state"` - GasConsumed string `json:"gas_consumed"` - Stack []request.FuncParam `json:"stack"` - TX string `json:"tx,omitempty"` -} - func initServerWithInMemoryChain(t *testing.T) (*core.Blockchain, http.HandlerFunc) { var nBlocks uint32 diff --git a/pkg/rpc/server/server_test.go b/pkg/rpc/server/server_test.go index 4686efabb..52f0ea008 100644 --- a/pkg/rpc/server/server_test.go +++ b/pkg/rpc/server/server_test.go @@ -474,9 +474,9 @@ var rpcTestCases = map[string][]rpcTestCase{ { name: "positive", params: `["50befd26fdf6e4d957c11e078b24ebce6291456f", [{"type": "String", "value": "qwerty"}]]`, - result: func(e *executor) interface{} { return &InvokeFunctionResult{} }, + result: func(e *executor) interface{} { return &result.Invoke{} }, check: func(t *testing.T, e *executor, inv interface{}) { - res, ok := inv.(*InvokeFunctionResult) + res, ok := inv.(*result.Invoke) require.True(t, ok) assert.Equal(t, "06717765727479676f459162ceeb248b071ec157d9e4f6fd26fdbe50", res.Script) assert.NotEqual(t, "", res.State) @@ -513,9 +513,9 @@ var rpcTestCases = map[string][]rpcTestCase{ { name: "positive", params: `["50befd26fdf6e4d957c11e078b24ebce6291456f", "test", []]`, - result: func(e *executor) interface{} { return &InvokeFunctionResult{} }, + result: func(e *executor) interface{} { return &result.Invoke{} }, check: func(t *testing.T, e *executor, inv interface{}) { - res, ok := inv.(*InvokeFunctionResult) + res, ok := inv.(*result.Invoke) require.True(t, ok) assert.NotEqual(t, "", res.Script) assert.NotEqual(t, "", res.State) @@ -547,9 +547,9 @@ var rpcTestCases = map[string][]rpcTestCase{ { name: "positive", params: `["51c56b0d48656c6c6f2c20776f726c6421680f4e656f2e52756e74696d652e4c6f67616c7566"]`, - result: func(e *executor) interface{} { return &InvokeFunctionResult{} }, + result: func(e *executor) interface{} { return &result.Invoke{} }, check: func(t *testing.T, e *executor, inv interface{}) { - res, ok := inv.(*InvokeFunctionResult) + res, ok := inv.(*result.Invoke) require.True(t, ok) assert.NotEqual(t, "", res.Script) assert.NotEqual(t, "", res.State)