From 4ab18d084a15bf4da9537ea8f4031d3bc1e50d11 Mon Sep 17 00:00:00 2001 From: Evgeniy Stratonikov Date: Wed, 20 Oct 2021 14:32:01 +0300 Subject: [PATCH 1/7] rpc/request: add unmarshal benchmark Signed-off-by: Evgeniy Stratonikov --- pkg/rpc/request/types_test.go | 49 ++++++++++++++++++++++++++++ pkg/rpc/server/server_helper_test.go | 6 ++-- pkg/rpc/server/server_test.go | 45 +++++++++++++++++++++++++ 3 files changed, 97 insertions(+), 3 deletions(-) create mode 100644 pkg/rpc/request/types_test.go diff --git a/pkg/rpc/request/types_test.go b/pkg/rpc/request/types_test.go new file mode 100644 index 000000000..aabbb27fe --- /dev/null +++ b/pkg/rpc/request/types_test.go @@ -0,0 +1,49 @@ +package request + +import ( + "bytes" + "encoding/json" + "io" + "testing" +) + +type readCloser struct { + io.Reader +} + +func (readCloser) Close() error { return nil } + +func BenchmarkUnmarshal(b *testing.B) { + req := []byte(`{"jsonrpc":"2.0", "method":"invokefunction","params":["0x50befd26fdf6e4d957c11e078b24ebce6291456f", "someMethod", [{"type": "String", "value": "50befd26fdf6e4d957c11e078b24ebce6291456f"}, {"type": "Integer", "value": "42"}, {"type": "Boolean", "value": false}]]}`) + b.Run("single", func(b *testing.B) { + b.ReportAllocs() + b.Run("unmarshal", func(b *testing.B) { + b.ReportAllocs() + b.ResetTimer() + for i := 0; i < b.N; i++ { + b.StopTimer() + in := new(In) + b.StartTimer() + err := json.Unmarshal(req, in) + if err != nil { + b.FailNow() + } + } + }) + b.Run("decode data", func(b *testing.B) { + b.ReportAllocs() + b.ResetTimer() + for i := 0; i < b.N; i++ { + b.StopTimer() + r := NewRequest() + r.In = new(In) + rd := bytes.NewReader(req) + b.StartTimer() + err := r.DecodeData(readCloser{rd}) + if err != nil { + b.FailNow() + } + } + }) + }) +} diff --git a/pkg/rpc/server/server_helper_test.go b/pkg/rpc/server/server_helper_test.go index 24c18da73..6dbb23608 100644 --- a/pkg/rpc/server/server_helper_test.go +++ b/pkg/rpc/server/server_helper_test.go @@ -28,7 +28,7 @@ const ( notaryPass = "one" ) -func getUnitTestChain(t *testing.T, enableOracle bool, enableNotary bool) (*core.Blockchain, *oracle.Oracle, config.Config, *zap.Logger) { +func getUnitTestChain(t testing.TB, enableOracle bool, enableNotary bool) (*core.Blockchain, *oracle.Oracle, config.Config, *zap.Logger) { net := netmode.UnitTestNet configPath := "../../../config" cfg, err := config.Load(configPath, net) @@ -97,7 +97,7 @@ func getTestBlocks(t *testing.T) []*block.Block { return blocks } -func initClearServerWithServices(t *testing.T, needOracle bool, needNotary bool) (*core.Blockchain, *Server, *httptest.Server) { +func initClearServerWithServices(t testing.TB, needOracle bool, needNotary bool) (*core.Blockchain, *Server, *httptest.Server) { chain, orc, cfg, logger := getUnitTestChain(t, needOracle, needNotary) serverConfig := network.NewServerConfig(cfg) @@ -113,7 +113,7 @@ func initClearServerWithServices(t *testing.T, needOracle bool, needNotary bool) return chain, &rpcServer, srv } -func initClearServerWithInMemoryChain(t *testing.T) (*core.Blockchain, *Server, *httptest.Server) { +func initClearServerWithInMemoryChain(t testing.TB) (*core.Blockchain, *Server, *httptest.Server) { return initClearServerWithServices(t, false, false) } diff --git a/pkg/rpc/server/server_test.go b/pkg/rpc/server/server_test.go index c0fca9a7b..cd640ca35 100644 --- a/pkg/rpc/server/server_test.go +++ b/pkg/rpc/server/server_test.go @@ -27,7 +27,9 @@ import ( "github.com/nspcc-dev/neo-go/pkg/crypto/keys" "github.com/nspcc-dev/neo-go/pkg/encoding/address" "github.com/nspcc-dev/neo-go/pkg/io" + "github.com/nspcc-dev/neo-go/pkg/network" "github.com/nspcc-dev/neo-go/pkg/network/payload" + "github.com/nspcc-dev/neo-go/pkg/rpc/request" "github.com/nspcc-dev/neo-go/pkg/rpc/response" "github.com/nspcc-dev/neo-go/pkg/rpc/response/result" rpc2 "github.com/nspcc-dev/neo-go/pkg/services/oracle/broadcaster" @@ -39,6 +41,7 @@ import ( "github.com/nspcc-dev/neo-go/pkg/wallet" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "go.uber.org/zap/zapcore" ) type executor struct { @@ -2160,3 +2163,45 @@ func checkNep17TransfersAux(t *testing.T, e *executor, acc interface{}, sent, rc } require.Equal(t, arr, res.Received) } + +func BenchmarkHandleIn(b *testing.B) { + chain, orc, cfg, logger := getUnitTestChain(b, false, false) + + serverConfig := network.NewServerConfig(cfg) + serverConfig.LogLevel = zapcore.FatalLevel + server, err := network.NewServer(serverConfig, chain, logger) + require.NoError(b, err) + rpcServer := New(chain, cfg.ApplicationConfiguration.RPC, server, orc, logger) + defer chain.Close() + + do := func(b *testing.B, req []byte) { + b.ReportAllocs() + b.ResetTimer() + for i := 0; i < b.N; i++ { + b.StopTimer() + in := new(request.In) + b.StartTimer() + err := json.Unmarshal(req, in) + if err != nil { + b.FailNow() + } + + res := rpcServer.handleIn(in, nil) + if res.Error != nil { + b.FailNow() + } + } + b.StopTimer() + } + + b.Run("no extra params", func(b *testing.B) { + do(b, []byte(`{"jsonrpc":"2.0", "method":"validateaddress","params":["Nbb1qkwcwNSBs9pAnrVVrnFbWnbWBk91U2"]}`)) + }) + + b.Run("with extra params", func(b *testing.B) { + do(b, []byte(`{"jsonrpc":"2.0", "method":"validateaddress","params":["Nbb1qkwcwNSBs9pAnrVVrnFbWnbWBk91U2", +"set", "of", "different", "parameters", "to", "see", "the", "difference", "between", "unmarshalling", "algorithms", 1234, 5678, 1234567, 765432, true, false, null, +"0x50befd26fdf6e4d957c11e078b24ebce6291456f", "someMethod", [{"type": "String", "value": "50befd26fdf6e4d957c11e078b24ebce6291456f"}, +{"type": "Integer", "value": "42"}, {"type": "Boolean", "value": false}]]}`)) + }) +} From 1a32fcb2dcc2a84cc3cc762e69cbac416a8edc9b Mon Sep 17 00:00:00 2001 From: Anna Shaleva Date: Thu, 28 Oct 2021 17:41:35 +0300 Subject: [PATCH 2/7] core: specify `method not found` call error It's useful for debugging and external users. --- pkg/core/interop/contract/call.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/core/interop/contract/call.go b/pkg/core/interop/contract/call.go index c6f7c59c2..8389e4557 100644 --- a/pkg/core/interop/contract/call.go +++ b/pkg/core/interop/contract/call.go @@ -66,7 +66,7 @@ func Call(ic *interop.Context) error { } md := cs.Manifest.ABI.GetMethod(method, len(args)) if md == nil { - return errors.New("method not found") + return fmt.Errorf("method not found: %s/%d", method, len(args)) } hasReturn := md.ReturnType != smartcontract.VoidType if !hasReturn { From 2e8bbf2a87708517e56aa73a5d31f44a9b6e6ad1 Mon Sep 17 00:00:00 2001 From: Anna Shaleva Date: Thu, 28 Oct 2021 14:10:18 +0300 Subject: [PATCH 3/7] rpc: *In parameters marshalling optimisation Parse request parameters on-demand. --- pkg/rpc/client/rpc_test.go | 4 +- pkg/rpc/client/wsclient.go | 6 +- pkg/rpc/client/wsclient_test.go | 37 +-- pkg/rpc/request/param.go | 265 +++++++++++---------- pkg/rpc/request/param_test.go | 357 +++++++++++++++-------------- pkg/rpc/request/params.go | 9 - pkg/rpc/request/txBuilder.go | 30 +-- pkg/rpc/request/tx_builder_test.go | 82 +++---- pkg/rpc/request/types.go | 15 +- pkg/rpc/server/server.go | 117 ++++------ pkg/rpc/server/server_test.go | 4 - 11 files changed, 444 insertions(+), 482 deletions(-) diff --git a/pkg/rpc/client/rpc_test.go b/pkg/rpc/client/rpc_test.go index bafb99d4e..c14ddc130 100644 --- a/pkg/rpc/client/rpc_test.go +++ b/pkg/rpc/client/rpc_test.go @@ -1649,8 +1649,8 @@ func wrapInitResponse(r *request.In, resp string) string { case "getversion": response = `{"id":1,"jsonrpc":"2.0","result":{"network":42,"tcpport":20332,"wsport":20342,"nonce":2153672787,"useragent":"/NEO-GO:0.73.1-pre-273-ge381358/"}}` case "getcontractstate": - p, _ := r.Params() - name, _ := p.ValueWithType(0, request.StringT).GetString() + p := request.Params(r.RawParams) + name, _ := p.Value(0).GetString() switch name { case "NeoToken": response = `{"id":1,"jsonrpc":"2.0","result":{"id":-1,"script":"DANORU9Ba2d4Cw==","manifest":{"name":"NEO","abi":{"hash":"0xde5f57d430d3dece511cf975a8d37848cb9e0525","methods":[{"name":"name","offset":0,"parameters":null,"returntype":"String"},{"name":"symbol","offset":0,"parameters":null,"returntype":"String"},{"name":"decimals","offset":0,"parameters":null,"returntype":"Integer"},{"name":"totalSupply","offset":0,"parameters":null,"returntype":"Integer"},{"name":"balanceOf","offset":0,"parameters":[{"name":"account","type":"Hash160"}],"returntype":"Integer"},{"name":"transfer","offset":0,"parameters":[{"name":"from","type":"Hash160"},{"name":"to","type":"Hash160"},{"name":"amount","type":"Integer"}],"returntype":"Boolean"},{"name":"onPersist","offset":0,"parameters":null,"returntype":"Void"},{"name":"postPersist","offset":0,"parameters":null,"returntype":"Void"},{"name":"unclaimedGas","offset":0,"parameters":[{"name":"account","type":"Hash160"},{"name":"end","type":"Integer"}],"returntype":"Integer"},{"name":"registerCandidate","offset":0,"parameters":[{"name":"pubkey","type":"PublicKey"}],"returntype":"Boolean"},{"name":"unregisterCandidate","offset":0,"parameters":[{"name":"pubkey","type":"PublicKey"}],"returntype":"Boolean"},{"name":"vote","offset":0,"parameters":[{"name":"account","type":"Hash160"},{"name":"pubkey","type":"PublicKey"}],"returntype":"Boolean"},{"name":"getCandidates","offset":0,"parameters":null,"returntype":"Array"},{"name":"getŠ”ommittee","offset":0,"parameters":null,"returntype":"Array"},{"name":"getNextBlockValidators","offset":0,"parameters":null,"returntype":"Array"},{"name":"getGasPerBlock","offset":0,"parameters":null,"returntype":"Integer"},{"name":"setGasPerBlock","offset":0,"parameters":[{"name":"gasPerBlock","type":"Integer"}],"returntype":"Boolean"}],"events":[{"name":"Transfer","parameters":null}]},"groups":[],"permissions":[{"contract":"*","methods":"*"}],"supportedstandards":["NEP-5"],"trusts":[],"safemethods":["name","symbol","decimals","totalSupply","balanceOf","unclaimedGas","getCandidates","getŠ”ommittee","getNextBlockValidators"],"extra":null},"hash":"0xde5f57d430d3dece511cf975a8d37848cb9e0525"}}` diff --git a/pkg/rpc/client/wsclient.go b/pkg/rpc/client/wsclient.go index 8c52ec345..f2ed6aed8 100644 --- a/pkg/rpc/client/wsclient.go +++ b/pkg/rpc/client/wsclient.go @@ -134,9 +134,7 @@ readloop: // Bad event received. break } - var slice []json.RawMessage - err = json.Unmarshal(rr.RawParams, &slice) - if err != nil || (event != response.MissedEventID && len(slice) != 1) { + if event != response.MissedEventID && len(rr.RawParams) != 1 { // Bad event received. break } @@ -159,7 +157,7 @@ readloop: break readloop } if event != response.MissedEventID { - err = json.Unmarshal(slice[0], val) + err = json.Unmarshal(rr.RawParams[0].RawMessage, val) if err != nil { // Bad event received. break diff --git a/pkg/rpc/client/wsclient_test.go b/pkg/rpc/client/wsclient_test.go index 6ba2d4199..2f43e77b6 100644 --- a/pkg/rpc/client/wsclient_test.go +++ b/pkg/rpc/client/wsclient_test.go @@ -187,10 +187,8 @@ func TestWSFilteredSubscriptions(t *testing.T) { }, func(t *testing.T, p *request.Params) { param := p.Value(1) - raw, ok := param.Value.(json.RawMessage) - require.True(t, ok) filt := new(request.BlockFilter) - require.NoError(t, json.Unmarshal(raw, filt)) + require.NoError(t, json.Unmarshal(param.RawMessage, filt)) require.Equal(t, 3, filt.Primary) }, }, @@ -202,10 +200,8 @@ func TestWSFilteredSubscriptions(t *testing.T) { }, func(t *testing.T, p *request.Params) { param := p.Value(1) - raw, ok := param.Value.(json.RawMessage) - require.True(t, ok) filt := new(request.TxFilter) - require.NoError(t, json.Unmarshal(raw, filt)) + require.NoError(t, json.Unmarshal(param.RawMessage, filt)) require.Equal(t, util.Uint160{1, 2, 3, 4, 5}, *filt.Sender) require.Nil(t, filt.Signer) }, @@ -218,10 +214,8 @@ func TestWSFilteredSubscriptions(t *testing.T) { }, func(t *testing.T, p *request.Params) { param := p.Value(1) - raw, ok := param.Value.(json.RawMessage) - require.True(t, ok) filt := new(request.TxFilter) - require.NoError(t, json.Unmarshal(raw, filt)) + require.NoError(t, json.Unmarshal(param.RawMessage, filt)) require.Nil(t, filt.Sender) require.Equal(t, util.Uint160{0, 42}, *filt.Signer) }, @@ -235,10 +229,8 @@ func TestWSFilteredSubscriptions(t *testing.T) { }, func(t *testing.T, p *request.Params) { param := p.Value(1) - raw, ok := param.Value.(json.RawMessage) - require.True(t, ok) filt := new(request.TxFilter) - require.NoError(t, json.Unmarshal(raw, filt)) + require.NoError(t, json.Unmarshal(param.RawMessage, filt)) require.Equal(t, util.Uint160{1, 2, 3, 4, 5}, *filt.Sender) require.Equal(t, util.Uint160{0, 42}, *filt.Signer) }, @@ -251,10 +243,8 @@ func TestWSFilteredSubscriptions(t *testing.T) { }, func(t *testing.T, p *request.Params) { param := p.Value(1) - raw, ok := param.Value.(json.RawMessage) - require.True(t, ok) filt := new(request.NotificationFilter) - require.NoError(t, json.Unmarshal(raw, filt)) + require.NoError(t, json.Unmarshal(param.RawMessage, filt)) require.Equal(t, util.Uint160{1, 2, 3, 4, 5}, *filt.Contract) require.Nil(t, filt.Name) }, @@ -267,10 +257,8 @@ func TestWSFilteredSubscriptions(t *testing.T) { }, func(t *testing.T, p *request.Params) { param := p.Value(1) - raw, ok := param.Value.(json.RawMessage) - require.True(t, ok) filt := new(request.NotificationFilter) - require.NoError(t, json.Unmarshal(raw, filt)) + require.NoError(t, json.Unmarshal(param.RawMessage, filt)) require.Equal(t, "my_pretty_notification", *filt.Name) require.Nil(t, filt.Contract) }, @@ -284,10 +272,8 @@ func TestWSFilteredSubscriptions(t *testing.T) { }, func(t *testing.T, p *request.Params) { param := p.Value(1) - raw, ok := param.Value.(json.RawMessage) - require.True(t, ok) filt := new(request.NotificationFilter) - require.NoError(t, json.Unmarshal(raw, filt)) + require.NoError(t, json.Unmarshal(param.RawMessage, filt)) require.Equal(t, util.Uint160{1, 2, 3, 4, 5}, *filt.Contract) require.Equal(t, "my_pretty_notification", *filt.Name) }, @@ -300,10 +286,8 @@ func TestWSFilteredSubscriptions(t *testing.T) { }, func(t *testing.T, p *request.Params) { param := p.Value(1) - raw, ok := param.Value.(json.RawMessage) - require.True(t, ok) filt := new(request.ExecutionFilter) - require.NoError(t, json.Unmarshal(raw, filt)) + require.NoError(t, json.Unmarshal(param.RawMessage, filt)) require.Equal(t, "FAULT", filt.State) }, }, @@ -320,9 +304,8 @@ func TestWSFilteredSubscriptions(t *testing.T) { req := request.In{} err = ws.ReadJSON(&req) require.NoError(t, err) - params, err := req.Params() - require.NoError(t, err) - c.serverCode(t, params) + params := request.Params(req.RawParams) + c.serverCode(t, ¶ms) err = ws.SetWriteDeadline(time.Now().Add(2 * time.Second)) require.NoError(t, err) err = ws.WriteMessage(1, []byte(`{"jsonrpc": "2.0", "id": 1, "result": "0"}`)) diff --git a/pkg/rpc/request/param.go b/pkg/rpc/request/param.go index 03834d050..eacc6ea27 100644 --- a/pkg/rpc/request/param.go +++ b/pkg/rpc/request/param.go @@ -22,11 +22,9 @@ type ( // the server or to send to a server using // the client. Param struct { - Type paramType - Value interface{} + json.RawMessage } - paramType int // FuncParam represents a function argument parameter used in the // invokefunction RPC method. FuncParam struct { @@ -64,68 +62,151 @@ type ( } ) -// These are parameter types accepted by RPC server. -const ( - defaultT paramType = iota - StringT - NumberT - BooleanT - ArrayT - FuncParamT - BlockFilterT - TxFilterT - NotificationFilterT - ExecutionFilterT - SignerWithWitnessT +var ( + jsonNullBytes = []byte("null") + jsonFalseBytes = []byte("false") + jsonTrueBytes = []byte("true") + errMissingParameter = errors.New("parameter is missing") + errNotAString = errors.New("not a string") + errNotAnInt = errors.New("not an integer") + errNotABool = errors.New("not a boolean") + errNotAnArray = errors.New("not an array") ) -var errMissingParameter = errors.New("parameter is missing") - func (p Param) String() string { - return fmt.Sprintf("%v", p.Value) + str, _ := p.GetString() + return str } -// GetString returns string value of the parameter. +// GetStringStrict returns string value of the parameter. +func (p *Param) GetStringStrict() (string, error) { + if p == nil { + return "", errMissingParameter + } + if p.IsNull() { + return "", errNotAString + } + var s string + err := json.Unmarshal(p.RawMessage, &s) + if err != nil { + return "", errNotAString + } + return s, nil +} + +// GetString returns string value of the parameter or tries to cast parameter to a string value. func (p *Param) GetString() (string, error) { if p == nil { return "", errMissingParameter } - str, ok := p.Value.(string) - if !ok { - return "", errors.New("not a string") + if p.IsNull() { + return "", errNotAString } - return str, nil + var s string + err := json.Unmarshal(p.RawMessage, &s) + if err == nil { + return s, nil + } + var i int + err = json.Unmarshal(p.RawMessage, &i) + if err == nil { + return strconv.Itoa(i), nil + } + var b bool + err = json.Unmarshal(p.RawMessage, &b) + if err == nil { + if b { + return "true", nil + } + return "false", nil + } + return "", errNotAString } -// GetBoolean returns boolean value of the parameter. -func (p *Param) GetBoolean() bool { +// GetBooleanStrict returns boolean value of the parameter. +func (p *Param) GetBooleanStrict() (bool, error) { if p == nil { - return false + return false, errMissingParameter } - switch p.Type { - case NumberT: - return p.Value != 0 - case StringT: - return p.Value != "" - case BooleanT: - return p.Value == true - default: - return true + if bytes.Equal(p.RawMessage, jsonTrueBytes) { + return true, nil } + if bytes.Equal(p.RawMessage, jsonFalseBytes) { + return false, nil + } + return false, errNotABool } -// GetInt returns int value of te parameter. +// GetBoolean returns boolean value of the parameter or tries to cast parameter to a bool value. +func (p *Param) GetBoolean() (bool, error) { + if p == nil { + return false, errMissingParameter + } + if p.IsNull() { + return false, errNotABool + } + var b bool + err := json.Unmarshal(p.RawMessage, &b) + if err == nil { + return b, nil + } + var s string + err = json.Unmarshal(p.RawMessage, &s) + if err == nil { + return s != "", nil + } + var i int + err = json.Unmarshal(p.RawMessage, &i) + if err == nil { + return i != 0, nil + } + return false, errNotABool +} + +// GetIntStrict returns int value of the parameter if the parameter is integer. +func (p *Param) GetIntStrict() (int, error) { + if p == nil { + return 0, errMissingParameter + } + if p.IsNull() { + return 0, errNotAnInt + } + var i int + err := json.Unmarshal(p.RawMessage, &i) + if err != nil { + return i, errNotAnInt + } + return i, nil +} + +// GetInt returns int value of the parameter or tries to cast parameter to an int value. func (p *Param) GetInt() (int, error) { if p == nil { return 0, errMissingParameter } - i, ok := p.Value.(int) - if ok { + if p.IsNull() { + return 0, errNotAnInt + } + var i int + err := json.Unmarshal(p.RawMessage, &i) + if err == nil { return i, nil - } else if s, ok := p.Value.(string); ok { + } + var s string + err = json.Unmarshal(p.RawMessage, &s) + if err == nil { return strconv.Atoi(s) } - return 0, errors.New("not an integer") + var b bool + err = json.Unmarshal(p.RawMessage, &b) + if err == nil { + i = 0 + if b { + i = 1 + } + return i, nil + } + return 0, errNotAnInt } // GetArray returns a slice of Params stored in the parameter. @@ -133,21 +214,14 @@ func (p *Param) GetArray() ([]Param, error) { if p == nil { return nil, errMissingParameter } - a, ok := p.Value.([]Param) - if ok { - return a, nil + if p.IsNull() { + return nil, errNotAnArray } - raw, ok := p.Value.(json.RawMessage) - if !ok { - return nil, errors.New("not an array") - } - - a = []Param{} - err := json.Unmarshal(raw, &a) + a := []Param{} + err := json.Unmarshal(p.RawMessage, &a) if err != nil { - return nil, errors.New("not an array") + return nil, errNotAnArray } - p.Value = a return a, nil } @@ -200,20 +274,9 @@ func (p *Param) GetFuncParam() (FuncParam, error) { if p == nil { return FuncParam{}, errMissingParameter } - fp, ok := p.Value.(FuncParam) - if ok { - return fp, nil - } - raw, ok := p.Value.(json.RawMessage) - if !ok { - return FuncParam{}, errors.New("not a function parameter") - } - err := json.Unmarshal(raw, &fp) - if err != nil { - return fp, err - } - p.Value = fp - return fp, nil + fp := FuncParam{} + err := json.Unmarshal(p.RawMessage, &fp) + return fp, err } // GetBytesHex returns []byte value of the parameter if @@ -240,25 +303,19 @@ func (p *Param) GetBytesBase64() ([]byte, error) { // GetSignerWithWitness returns SignerWithWitness value of the parameter. func (p *Param) GetSignerWithWitness() (SignerWithWitness, error) { - c, ok := p.Value.(SignerWithWitness) - if ok { - return c, nil - } - raw, ok := p.Value.(json.RawMessage) - if !ok { - return SignerWithWitness{}, errors.New("not a signer") - } aux := new(signerWithWitnessAux) - err := json.Unmarshal(raw, aux) + err := json.Unmarshal(p.RawMessage, aux) if err != nil { - return SignerWithWitness{}, errors.New("not a signer") + return SignerWithWitness{}, fmt.Errorf("not a signer: %w", err) } - accParam := Param{StringT, aux.Account} - acc, err := accParam.GetUint160FromAddressOrHex() + acc, err := util.Uint160DecodeStringLE(strings.TrimPrefix(aux.Account, "0x")) if err != nil { - return SignerWithWitness{}, errors.New("not a signer") + acc, err = address.StringToUint160(aux.Account) } - c = SignerWithWitness{ + if err != nil { + return SignerWithWitness{}, fmt.Errorf("not a signer: %w", err) + } + c := SignerWithWitness{ Signer: transaction.Signer{ Account: acc, Scopes: aux.Scopes, @@ -270,7 +327,6 @@ func (p *Param) GetSignerWithWitness() (SignerWithWitness, error) { VerificationScript: aux.VerificationScript, }, } - p.Value = c return c, nil } @@ -309,48 +365,9 @@ func (p Param) GetSignersWithWitnesses() ([]transaction.Signer, []transaction.Wi return signers, witnesses, nil } -// UnmarshalJSON implements json.Unmarshaler interface. -func (p *Param) UnmarshalJSON(data []byte) error { - r := bytes.NewReader(data) - jd := json.NewDecoder(r) - jd.UseNumber() - tok, err := jd.Token() - if err != nil { - return err - } - switch t := tok.(type) { - case json.Delim: - if t == json.Delim('[') { - var arr []Param - err := json.Unmarshal(data, &arr) - if err != nil { - return err - } - p.Type = ArrayT - p.Value = arr - } else { - p.Type = defaultT - p.Value = json.RawMessage(data) - } - case bool: - p.Type = BooleanT - p.Value = t - case float64: // unexpected because of `UseNumber`. - panic("unexpected") - case json.Number: - value, err := strconv.Atoi(string(t)) - if err != nil { - return err - } - p.Type = NumberT - p.Value = value - case string: - p.Type = StringT - p.Value = t - default: // null - p.Type = defaultT - } - return nil +// IsNull returns whether parameter represents JSON nil value. +func (p *Param) IsNull() bool { + return bytes.Equal(p.RawMessage, jsonNullBytes) } // signerWithWitnessAux is an auxiluary struct for JSON marshalling. We need it because of diff --git a/pkg/rpc/request/param_test.go b/pkg/rpc/request/param_test.go index 7303006c9..1b5900700 100644 --- a/pkg/rpc/request/param_test.go +++ b/pkg/rpc/request/param_test.go @@ -4,6 +4,7 @@ import ( "encoding/base64" "encoding/hex" "encoding/json" + "fmt" "testing" "github.com/nspcc-dev/neo-go/pkg/core/transaction" @@ -15,66 +16,178 @@ import ( ) func TestParam_UnmarshalJSON(t *testing.T) { - msg := `["str1", 123, null, ["str2", 3], [{"type": "String", "value": "jajaja"}], - {"account": "0xcadb3dc2faa3ef14a13b619c9a43124755aa2569"}, - {"account": "NYxb4fSZVKAz8YsgaPK2WkT3KcAE9b3Vag", "scopes": "Global"}, - [{"account": "0xcadb3dc2faa3ef14a13b619c9a43124755aa2569", "scopes": "Global"}]]` - expected := Params{ + type testCase struct { + check func(t *testing.T, p *Param) + expectedRawMessage []byte + } + msg := `["123", 123, null, ["str2", 3], [{"type": "String", "value": "jajaja"}], + {"account": "0xcadb3dc2faa3ef14a13b619c9a43124755aa2569"}, + {"account": "NYxb4fSZVKAz8YsgaPK2WkT3KcAE9b3Vag", "scopes": "Global"}, + [{"account": "0xcadb3dc2faa3ef14a13b619c9a43124755aa2569", "scopes": "Global"}]]` + expected := []testCase{ { - Type: StringT, - Value: "str1", - }, - { - Type: NumberT, - Value: 123, - }, - { - Type: defaultT, - }, - { - Type: ArrayT, - Value: []Param{ - { - Type: StringT, - Value: "str2", - }, - { - Type: NumberT, - Value: 3, - }, + check: func(t *testing.T, p *Param) { + expectedS := "123" + actualS, err := p.GetStringStrict() + require.NoError(t, err) + require.Equal(t, expectedS, actualS) + actualS, err = p.GetString() + require.NoError(t, err) + require.Equal(t, expectedS, actualS) + + expectedI := 123 + _, err = p.GetIntStrict() + require.Error(t, err) + actualI, err := p.GetInt() + require.NoError(t, err) + require.Equal(t, expectedI, actualI) + + expectedB := true + _, err = p.GetBooleanStrict() + require.Error(t, err) + actualB, err := p.GetBoolean() + require.NoError(t, err) + require.Equal(t, expectedB, actualB) + + _, err = p.GetArray() + require.Error(t, err) }, + expectedRawMessage: []byte(`"123"`), }, { - Type: ArrayT, - Value: []Param{ - { - Type: defaultT, - Value: json.RawMessage(`{"type": "String", "value": "jajaja"}`), - }, + check: func(t *testing.T, p *Param) { + expectedS := "123" + _, err := p.GetStringStrict() + require.Error(t, err) + actualS, err := p.GetString() + require.NoError(t, err) + require.Equal(t, expectedS, actualS) + + expectedI := 123 + actualI, err := p.GetIntStrict() + require.NoError(t, err) + require.Equal(t, expectedI, actualI) + actualI, err = p.GetInt() + require.NoError(t, err) + require.Equal(t, expectedI, actualI) + + expectedB := true + _, err = p.GetBooleanStrict() + require.Error(t, err) + actualB, err := p.GetBoolean() + require.NoError(t, err) + require.Equal(t, expectedB, actualB) + + _, err = p.GetArray() + require.Error(t, err) }, + expectedRawMessage: []byte(`123`), }, { - Type: defaultT, - Value: json.RawMessage(`{"account": "0xcadb3dc2faa3ef14a13b619c9a43124755aa2569"}`), - }, - { - Type: defaultT, - Value: json.RawMessage(`{"account": "NYxb4fSZVKAz8YsgaPK2WkT3KcAE9b3Vag", "scopes": "Global"}`), - }, - { - Type: ArrayT, - Value: []Param{ - { - Type: defaultT, - Value: json.RawMessage(`{"account": "0xcadb3dc2faa3ef14a13b619c9a43124755aa2569", "scopes": "Global"}`), - }, + check: func(t *testing.T, p *Param) { + _, err := p.GetStringStrict() + require.Error(t, err) + _, err = p.GetString() + require.Error(t, err) + + _, err = p.GetIntStrict() + require.Error(t, err) + _, err = p.GetInt() + require.Error(t, err) + + _, err = p.GetBooleanStrict() + require.Error(t, err) + _, err = p.GetBoolean() + require.Error(t, err) + + _, err = p.GetArray() + require.Error(t, err) }, + expectedRawMessage: []byte(`null`), + }, + { + check: func(t *testing.T, p *Param) { + _, err := p.GetStringStrict() + require.Error(t, err) + _, err = p.GetString() + require.Error(t, err) + + _, err = p.GetIntStrict() + require.Error(t, err) + _, err = p.GetInt() + require.Error(t, err) + + _, err = p.GetBooleanStrict() + require.Error(t, err) + _, err = p.GetBoolean() + require.Error(t, err) + + a, err := p.GetArray() + require.NoError(t, err) + require.Equal(t, []Param{ + {RawMessage: json.RawMessage(`"str2"`)}, + {RawMessage: json.RawMessage(`3`)}, + }, a) + }, + expectedRawMessage: []byte(`["str2", 3]`), + }, + { + check: func(t *testing.T, p *Param) { + a, err := p.GetArray() + require.NoError(t, err) + require.Equal(t, 1, len(a)) + fp, err := a[0].GetFuncParam() + require.NoError(t, err) + require.Equal(t, smartcontract.StringType, fp.Type) + strVal, err := fp.Value.GetStringStrict() + require.NoError(t, err) + require.Equal(t, "jajaja", strVal) + }, + expectedRawMessage: []byte(`[{"type": "String", "value": "jajaja"}]`), + }, + { + check: func(t *testing.T, p *Param) { + actual, err := p.GetSignerWithWitness() + require.NoError(t, err) + expectedAcc, err := util.Uint160DecodeStringLE("cadb3dc2faa3ef14a13b619c9a43124755aa2569") + require.NoError(t, err) + require.Equal(t, SignerWithWitness{Signer: transaction.Signer{Account: expectedAcc}}, actual) + }, + expectedRawMessage: []byte(`{"account": "0xcadb3dc2faa3ef14a13b619c9a43124755aa2569"}`), + }, + { + check: func(t *testing.T, p *Param) { + actual, err := p.GetSignerWithWitness() + require.NoError(t, err) + expectedAcc, err := address.StringToUint160("NYxb4fSZVKAz8YsgaPK2WkT3KcAE9b3Vag") + require.NoError(t, err) + require.Equal(t, SignerWithWitness{Signer: transaction.Signer{Account: expectedAcc, Scopes: transaction.Global}}, actual) + }, + expectedRawMessage: []byte(`{"account": "NYxb4fSZVKAz8YsgaPK2WkT3KcAE9b3Vag", "scopes": "Global"}`), + }, + { + check: func(t *testing.T, p *Param) { + actualSigs, actualWtns, err := p.GetSignersWithWitnesses() + require.NoError(t, err) + require.Equal(t, 1, len(actualSigs)) + require.Equal(t, 1, len(actualWtns)) + expectedAcc, err := util.Uint160DecodeStringLE("cadb3dc2faa3ef14a13b619c9a43124755aa2569") + require.NoError(t, err) + require.Equal(t, transaction.Signer{Account: expectedAcc, Scopes: transaction.Global}, actualSigs[0]) + require.Equal(t, transaction.Witness{}, actualWtns[0]) + }, + expectedRawMessage: []byte(`[{"account": "0xcadb3dc2faa3ef14a13b619c9a43124755aa2569", "scopes": "Global"}]`), }, } var ps Params require.NoError(t, json.Unmarshal([]byte(msg), &ps)) - require.Equal(t, expected, ps) + require.Equal(t, len(expected), len(ps)) + for i, tc := range expected { + require.NotNil(t, ps[i]) + require.Equal(t, json.RawMessage(tc.expectedRawMessage), ps[i].RawMessage, i) + tc.check(t, &ps[i]) + } } func TestGetWitness(t *testing.T) { @@ -108,11 +221,10 @@ func TestGetWitness(t *testing.T) { } for _, tc := range testCases { - p := Param{Value: json.RawMessage(tc.raw)} + p := Param{RawMessage: json.RawMessage(tc.raw)} actual, err := p.GetSignerWithWitness() require.NoError(t, err) require.Equal(t, tc.expected, actual) - require.Equal(t, tc.expected, p.Value) actual, err = p.GetSignerWithWitness() // valid second invocation. require.NoError(t, err) @@ -120,57 +232,24 @@ func TestGetWitness(t *testing.T) { } } -func TestParamGetString(t *testing.T) { - p := Param{StringT, "jajaja"} - str, err := p.GetString() - assert.Equal(t, "jajaja", str) - require.Nil(t, err) - - p = Param{StringT, int(100500)} - _, err = p.GetString() - require.NotNil(t, err) -} - -func TestParamGetInt(t *testing.T) { - p := Param{NumberT, int(100500)} - i, err := p.GetInt() - assert.Equal(t, 100500, i) - require.Nil(t, err) - - p = Param{NumberT, "jajaja"} - _, err = p.GetInt() - require.NotNil(t, err) -} - -func TestParamGetArray(t *testing.T) { - p := Param{ArrayT, []Param{{NumberT, 42}}} - a, err := p.GetArray() - assert.Equal(t, []Param{{NumberT, 42}}, a) - require.Nil(t, err) - - p = Param{ArrayT, 42} - _, err = p.GetArray() - require.NotNil(t, err) -} - func TestParamGetUint256(t *testing.T) { gas := "602c79718b16e442de58778e148d0b1084e3b2dffd5de6b7b16cee7969282de7" u256, _ := util.Uint256DecodeStringLE(gas) - p := Param{StringT, gas} + p := Param{RawMessage: []byte(fmt.Sprintf(`"%s"`, gas))} u, err := p.GetUint256() assert.Equal(t, u256, u) require.Nil(t, err) - p = Param{StringT, "0x" + gas} + p = Param{RawMessage: []byte(fmt.Sprintf(`"0x%s"`, gas))} u, err = p.GetUint256() require.NoError(t, err) assert.Equal(t, u256, u) - p = Param{StringT, 42} + p = Param{RawMessage: []byte(`42`)} _, err = p.GetUint256() require.NotNil(t, err) - p = Param{StringT, "qq2c79718b16e442de58778e148d0b1084e3b2dffd5de6b7b16cee7969282de7"} + p = Param{RawMessage: []byte(`"qq2c79718b16e442de58778e148d0b1084e3b2dffd5de6b7b16cee7969282de7"`)} _, err = p.GetUint256() require.NotNil(t, err) } @@ -178,16 +257,16 @@ func TestParamGetUint256(t *testing.T) { func TestParamGetUint160FromHex(t *testing.T) { in := "50befd26fdf6e4d957c11e078b24ebce6291456f" u160, _ := util.Uint160DecodeStringLE(in) - p := Param{StringT, in} + p := Param{RawMessage: []byte(fmt.Sprintf(`"%s"`, in))} u, err := p.GetUint160FromHex() assert.Equal(t, u160, u) require.Nil(t, err) - p = Param{StringT, 42} + p = Param{RawMessage: []byte(`42`)} _, err = p.GetUint160FromHex() require.NotNil(t, err) - p = Param{StringT, "wwbefd26fdf6e4d957c11e078b24ebce6291456f"} + p = Param{RawMessage: []byte(`"wwbefd26fdf6e4d957c11e078b24ebce6291456f"`)} _, err = p.GetUint160FromHex() require.NotNil(t, err) } @@ -195,16 +274,16 @@ func TestParamGetUint160FromHex(t *testing.T) { func TestParamGetUint160FromAddress(t *testing.T) { in := "NPAsqZkx9WhNd4P72uhZxBhLinSuNkxfB8" u160, _ := address.StringToUint160(in) - p := Param{StringT, in} + p := Param{RawMessage: []byte(fmt.Sprintf(`"%s"`, in))} u, err := p.GetUint160FromAddress() assert.Equal(t, u160, u) require.Nil(t, err) - p = Param{StringT, 42} + p = Param{RawMessage: []byte(`42`)} _, err = p.GetUint160FromAddress() require.NotNil(t, err) - p = Param{StringT, "QK2nJJpJr6o664CWJKi1QRXjqeic2zRp8y"} + p = Param{RawMessage: []byte(`"QK2nJJpJr6o664CWJKi1QRXjqeic2zRp8y"`)} _, err = p.GetUint160FromAddress() require.NotNil(t, err) } @@ -214,14 +293,14 @@ func TestParam_GetUint160FromAddressOrHex(t *testing.T) { inHex, _ := address.StringToUint160(in) t.Run("Address", func(t *testing.T) { - p := Param{StringT, in} + p := Param{RawMessage: []byte(fmt.Sprintf(`"%s"`, in))} u, err := p.GetUint160FromAddressOrHex() require.NoError(t, err) require.Equal(t, inHex, u) }) t.Run("Hex", func(t *testing.T) { - p := Param{StringT, inHex.StringLE()} + p := Param{RawMessage: []byte(fmt.Sprintf(`"%s"`, inHex.StringLE()))} u, err := p.GetUint160FromAddressOrHex() require.NoError(t, err) require.Equal(t, inHex, u) @@ -230,21 +309,15 @@ func TestParam_GetUint160FromAddressOrHex(t *testing.T) { func TestParamGetFuncParam(t *testing.T) { fp := FuncParam{ - Type: smartcontract.StringType, - Value: Param{ - Type: StringT, - Value: "jajaja", - }, - } - p := Param{ - Type: FuncParamT, - Value: fp, + Type: smartcontract.StringType, + Value: Param{RawMessage: []byte(`"jajaja"`)}, } + p := Param{RawMessage: []byte(`{"type": "String", "value": "jajaja"}`)} newfp, err := p.GetFuncParam() assert.Equal(t, fp, newfp) require.Nil(t, err) - p = Param{FuncParamT, 42} + p = Param{RawMessage: []byte(`42`)} _, err = p.GetFuncParam() require.NotNil(t, err) } @@ -252,16 +325,17 @@ func TestParamGetFuncParam(t *testing.T) { func TestParamGetBytesHex(t *testing.T) { in := "602c79718b16e442de58778e148d0b1084e3b2dffd5de6b7b16cee7969282de7" inb, _ := hex.DecodeString(in) - p := Param{StringT, in} + p := Param{RawMessage: []byte(fmt.Sprintf(`"%s"`, in))} bh, err := p.GetBytesHex() assert.Equal(t, inb, bh) require.Nil(t, err) - p = Param{StringT, 42} - _, err = p.GetBytesHex() - require.NotNil(t, err) + p = Param{RawMessage: []byte(`42`)} + h, err := p.GetBytesHex() + assert.Equal(t, []byte{0x42}, h) // that's the way how C# works: 42 -> "42" -> []byte{0x42} + require.Nil(t, err) - p = Param{StringT, "qq2c79718b16e442de58778e148d0b1084e3b2dffd5de6b7b16cee7969282de7"} + p = Param{RawMessage: []byte(`"qq2c79718b16e442de58778e148d0b1084e3b2dffd5de6b7b16cee7969282de7"`)} _, err = p.GetBytesHex() require.NotNil(t, err) } @@ -270,16 +344,16 @@ func TestParamGetBytesBase64(t *testing.T) { in := "Aj4A8DoW6HB84EXrQu6A05JFFUHuUQ3BjhyL77rFTXQm" inb, err := base64.StdEncoding.DecodeString(in) require.NoError(t, err) - p := Param{StringT, in} + p := Param{RawMessage: []byte(fmt.Sprintf(`"%s"`, in))} bh, err := p.GetBytesBase64() assert.Equal(t, inb, bh) require.Nil(t, err) - p = Param{StringT, 42} + p = Param{RawMessage: []byte(`42`)} _, err = p.GetBytesBase64() require.NotNil(t, err) - p = Param{StringT, "@j4A8DoW6HB84EXrQu6A05JFFUHuUQ3BjhyL77rFTXQm"} + p = Param{RawMessage: []byte("@j4A8DoW6HB84EXrQu6A05JFFUHuUQ3BjhyL77rFTXQm")} _, err = p.GetBytesBase64() require.NotNil(t, err) } @@ -296,12 +370,12 @@ func TestParamGetSigner(t *testing.T) { VerificationScript: []byte{1, 2, 3}, }, } - p := Param{Type: SignerWithWitnessT, Value: c} + p := Param{RawMessage: []byte(`{"account": "0x0000000000000000000000000000000004030201", "scopes": "Global", "invocation": "AQID", "verification": "AQID"}`)} actual, err := p.GetSignerWithWitness() require.NoError(t, err) require.Equal(t, c, actual) - p = Param{Type: SignerWithWitnessT, Value: `{"account": "0xcadb3dc2faa3ef14a13b619c9a43124755aa2569", "scopes": 0}`} + p = Param{RawMessage: []byte(`"not a signer"`)} _, err = p.GetSignerWithWitness() require.Error(t, err) } @@ -310,10 +384,7 @@ func TestParamGetSigners(t *testing.T) { u1 := util.Uint160{1, 2, 3, 4} u2 := util.Uint160{5, 6, 7, 8} t.Run("from hashes", func(t *testing.T) { - p := Param{ArrayT, []Param{ - {Type: StringT, Value: u1.StringLE()}, - {Type: StringT, Value: u2.StringLE()}, - }} + p := Param{RawMessage: []byte(fmt.Sprintf(`["%s", "%s"]`, u1.StringLE(), u2.StringLE()))} actual, _, err := p.GetSignersWithWitnesses() require.NoError(t, err) require.Equal(t, 2, len(actual)) @@ -321,56 +392,8 @@ func TestParamGetSigners(t *testing.T) { require.True(t, u2.Equals(actual[1].Account)) }) - t.Run("from signers", func(t *testing.T) { - c1 := SignerWithWitness{ - Signer: transaction.Signer{ - Account: u1, - Scopes: transaction.Global, - }, - Witness: transaction.Witness{ - InvocationScript: []byte{1, 2, 3}, - VerificationScript: []byte{1, 2, 3}, - }, - } - c2 := SignerWithWitness{ - Signer: transaction.Signer{ - Account: u2, - Scopes: transaction.CustomContracts, - AllowedContracts: []util.Uint160{ - {1, 2, 3}, - {4, 5, 6}, - }, - }, - } - p := Param{ArrayT, []Param{ - {Type: SignerWithWitnessT, Value: c1}, - {Type: SignerWithWitnessT, Value: c2}, - }} - actualS, actualW, err := p.GetSignersWithWitnesses() - require.NoError(t, err) - require.Equal(t, 2, len(actualS)) - require.Equal(t, transaction.Signer{ - Account: c1.Account, - Scopes: c1.Scopes, - }, actualS[0]) - require.Equal(t, transaction.Signer{ - Account: c2.Account, - Scopes: c2.Scopes, - AllowedContracts: c2.AllowedContracts, - }, actualS[1]) - require.EqualValues(t, 2, len(actualW)) - require.EqualValues(t, transaction.Witness{ - InvocationScript: c1.InvocationScript, - VerificationScript: c1.VerificationScript, - }, actualW[0]) - require.Equal(t, transaction.Witness{}, actualW[1]) - }) - t.Run("bad format", func(t *testing.T) { - p := Param{ArrayT, []Param{ - {Type: StringT, Value: u1.StringLE()}, - {Type: StringT, Value: "bla"}, - }} + p := Param{RawMessage: []byte(`"not a signer"`)} _, _, err := p.GetSignersWithWitnesses() require.Error(t, err) }) diff --git a/pkg/rpc/request/params.go b/pkg/rpc/request/params.go index c35e13b43..9ba8fb159 100644 --- a/pkg/rpc/request/params.go +++ b/pkg/rpc/request/params.go @@ -17,15 +17,6 @@ func (p Params) Value(index int) *Param { return nil } -// ValueWithType returns the param struct at the given index if it -// exists and matches the given type. -func (p Params) ValueWithType(index int, valType paramType) *Param { - if val := p.Value(index); val != nil && val.Type == valType { - return val - } - return nil -} - func (p Params) String() string { return fmt.Sprintf("%v", []Param(p)) } diff --git a/pkg/rpc/request/txBuilder.go b/pkg/rpc/request/txBuilder.go index f8a64b1de..f0dbc6747 100644 --- a/pkg/rpc/request/txBuilder.go +++ b/pkg/rpc/request/txBuilder.go @@ -70,8 +70,8 @@ func ExpandArrayIntoScript(script *io.BinWriter, slice []Param) error { } emit.Int(script, int64(val)) case smartcontract.BoolType: - val, ok := fp.Value.Value.(bool) - if !ok { + val, err := fp.Value.GetBoolean() // not GetBooleanStrict(), because that's the way C# code works + if err != nil { return errors.New("not a bool") } if val { @@ -102,29 +102,21 @@ func ExpandArrayIntoScript(script *io.BinWriter, slice []Param) error { func CreateFunctionInvocationScript(contract util.Uint160, method string, params Params) ([]byte, error) { script := io.NewBufBinWriter() for i := len(params) - 1; i >= 0; i-- { - switch params[i].Type { - case StringT: - emit.String(script.BinWriter, params[i].String()) - case NumberT: - num, err := params[i].GetInt() - if err != nil { - return nil, err - } - emit.String(script.BinWriter, strconv.Itoa(num)) - case BooleanT: - val := params[i].GetBoolean() - emit.Bool(script.BinWriter, val) - case ArrayT: - slice, err := params[i].GetArray() - if err != nil { - return nil, err - } + if slice, err := params[i].GetArray(); err == nil { err = ExpandArrayIntoScript(script.BinWriter, slice) if err != nil { return nil, err } emit.Int(script.BinWriter, int64(len(slice))) emit.Opcodes(script.BinWriter, opcode.PACK) + } else if s, err := params[i].GetStringStrict(); err == nil { + emit.String(script.BinWriter, s) + } else if n, err := params[i].GetIntStrict(); err == nil { + emit.String(script.BinWriter, strconv.Itoa(n)) + } else if b, err := params[i].GetBooleanStrict(); err == nil { + emit.Bool(script.BinWriter, b) + } else { + return nil, fmt.Errorf("failed to convert parmeter %s to script parameter", params[i]) } } if len(params) == 0 { diff --git a/pkg/rpc/request/tx_builder_test.go b/pkg/rpc/request/tx_builder_test.go index 9bc6a8214..52a36627b 100644 --- a/pkg/rpc/request/tx_builder_test.go +++ b/pkg/rpc/request/tx_builder_test.go @@ -2,10 +2,10 @@ package request import ( "encoding/hex" + "fmt" "testing" "github.com/nspcc-dev/neo-go/pkg/io" - "github.com/nspcc-dev/neo-go/pkg/smartcontract" "github.com/nspcc-dev/neo-go/pkg/util" "github.com/nspcc-dev/neo-go/pkg/vm/opcode" "github.com/stretchr/testify/assert" @@ -13,7 +13,7 @@ import ( ) func TestInvocationScriptCreationGood(t *testing.T) { - p := Param{Type: StringT, Value: "50befd26fdf6e4d957c11e078b24ebce6291456f"} + p := Param{RawMessage: []byte(`"50befd26fdf6e4d957c11e078b24ebce6291456f"`)} contract, err := p.GetUint160FromHex() require.Nil(t, err) @@ -21,49 +21,60 @@ func TestInvocationScriptCreationGood(t *testing.T) { ps Params script string }{{ - ps: Params{{Type: StringT, Value: "transfer"}}, + ps: Params{{RawMessage: []byte(`"transfer"`)}}, script: "c21f0c087472616e736665720c146f459162ceeb248b071ec157d9e4f6fd26fdbe5041627d5b52", }, { - ps: Params{{Type: NumberT, Value: 42}}, + ps: Params{{RawMessage: []byte(`42`)}}, script: "c21f0c0234320c146f459162ceeb248b071ec157d9e4f6fd26fdbe5041627d5b52", }, { - ps: Params{{Type: StringT, Value: "m"}, {Type: BooleanT, Value: true}}, + ps: Params{{RawMessage: []byte(`"m"`)}, {RawMessage: []byte(`true`)}}, script: "11db201f0c016d0c146f459162ceeb248b071ec157d9e4f6fd26fdbe5041627d5b52", }, { - ps: Params{{Type: StringT, Value: "a"}, {Type: ArrayT, Value: []Param{}}}, + ps: Params{{RawMessage: []byte(`"a"`)}, {RawMessage: []byte(`[]`)}}, script: "10c01f0c01610c146f459162ceeb248b071ec157d9e4f6fd26fdbe5041627d5b52", }, { - ps: Params{{Type: StringT, Value: "a"}, {Type: ArrayT, Value: []Param{{Type: FuncParamT, Value: FuncParam{Type: smartcontract.ByteArrayType, Value: Param{Type: StringT, Value: "AwEtR+diEK7HO+Oas9GG4KQP6Nhr+j1Pq/2le6E7iPlq"}}}}}}, + ps: Params{{RawMessage: []byte(`"a"`)}, {RawMessage: []byte(`[{"type": "ByteString", "value": "AwEtR+diEK7HO+Oas9GG4KQP6Nhr+j1Pq/2le6E7iPlq"}]`)}}, script: "0c2103012d47e76210aec73be39ab3d186e0a40fe8d86bfa3d4fabfda57ba13b88f96a11c01f0c01610c146f459162ceeb248b071ec157d9e4f6fd26fdbe5041627d5b52", }, { - ps: Params{{Type: StringT, Value: "a"}, {Type: ArrayT, Value: []Param{{Type: FuncParamT, Value: FuncParam{Type: smartcontract.SignatureType, Value: Param{Type: StringT, Value: "4edf5005771de04619235d5a4c7a9a11bb78e008541f1da7725f654c33380a3c87e2959a025da706d7255cb3a3fa07ebe9c6559d0d9e6213c68049168eb1056f"}}}}}}, + ps: Params{{RawMessage: []byte(`"a"`)}, {RawMessage: []byte(`[{"type": "Signature", "value": "4edf5005771de04619235d5a4c7a9a11bb78e008541f1da7725f654c33380a3c87e2959a025da706d7255cb3a3fa07ebe9c6559d0d9e6213c68049168eb1056f"}]`)}}, script: "0c404edf5005771de04619235d5a4c7a9a11bb78e008541f1da7725f654c33380a3c87e2959a025da706d7255cb3a3fa07ebe9c6559d0d9e6213c68049168eb1056f11c01f0c01610c146f459162ceeb248b071ec157d9e4f6fd26fdbe5041627d5b52", }, { - ps: Params{{Type: StringT, Value: "a"}, {Type: ArrayT, Value: []Param{{Type: FuncParamT, Value: FuncParam{Type: smartcontract.StringType, Value: Param{Type: StringT, Value: "50befd26fdf6e4d957c11e078b24ebce6291456f"}}}}}}, + ps: Params{{RawMessage: []byte(`"a"`)}, {RawMessage: []byte(`[{"type": "String", "value": "50befd26fdf6e4d957c11e078b24ebce6291456f"}]`)}}, script: "0c283530626566643236666466366534643935376331316530373862323465626365363239313435366611c01f0c01610c146f459162ceeb248b071ec157d9e4f6fd26fdbe5041627d5b52", }, { - ps: Params{{Type: StringT, Value: "a"}, {Type: ArrayT, Value: []Param{{Type: FuncParamT, Value: FuncParam{Type: smartcontract.Hash160Type, Value: Param{Type: StringT, Value: "50befd26fdf6e4d957c11e078b24ebce6291456f"}}}}}}, + ps: Params{{RawMessage: []byte(`"a"`)}, {RawMessage: []byte(`[{"type": "Hash160", "value": "50befd26fdf6e4d957c11e078b24ebce6291456f"}]`)}}, script: "0c146f459162ceeb248b071ec157d9e4f6fd26fdbe5011c01f0c01610c146f459162ceeb248b071ec157d9e4f6fd26fdbe5041627d5b52", }, { - ps: Params{{Type: StringT, Value: "a"}, {Type: ArrayT, Value: []Param{{Type: FuncParamT, Value: FuncParam{Type: smartcontract.Hash256Type, Value: Param{Type: StringT, Value: "602c79718b16e442de58778e148d0b1084e3b2dffd5de6b7b16cee7969282de7"}}}}}}, + ps: Params{{RawMessage: []byte(`"a"`)}, {RawMessage: []byte(`[{"type": "Hash256", "value": "602c79718b16e442de58778e148d0b1084e3b2dffd5de6b7b16cee7969282de7"}]`)}}, script: "0c20e72d286979ee6cb1b7e65dfddfb2e384100b8d148e7758de42e4168b71792c6011c01f0c01610c146f459162ceeb248b071ec157d9e4f6fd26fdbe5041627d5b52", }, { - ps: Params{{Type: StringT, Value: "a"}, {Type: ArrayT, Value: []Param{{Type: FuncParamT, Value: FuncParam{Type: smartcontract.PublicKeyType, Value: Param{Type: StringT, Value: "03c089d7122b840a4935234e82e26ae5efd0c2acb627239dc9f207311337b6f2c1"}}}}}}, + ps: Params{{RawMessage: []byte(`"a"`)}, {RawMessage: []byte(`[{"type": "PublicKey", "value": "03c089d7122b840a4935234e82e26ae5efd0c2acb627239dc9f207311337b6f2c1"}]`)}}, script: "0c2103c089d7122b840a4935234e82e26ae5efd0c2acb627239dc9f207311337b6f2c111c01f0c01610c146f459162ceeb248b071ec157d9e4f6fd26fdbe5041627d5b52", }, { - ps: Params{{Type: StringT, Value: "a"}, {Type: ArrayT, Value: []Param{{Type: FuncParamT, Value: FuncParam{Type: smartcontract.IntegerType, Value: Param{Type: NumberT, Value: 42}}}}}}, + ps: Params{{RawMessage: []byte(`"a"`)}, {RawMessage: []byte(`[{"type": "Integer", "value": 42}]`)}}, script: "002a11c01f0c01610c146f459162ceeb248b071ec157d9e4f6fd26fdbe5041627d5b52", }, { - ps: Params{{Type: StringT, Value: "a"}, {Type: ArrayT, Value: []Param{{Type: FuncParamT, Value: FuncParam{Type: smartcontract.BoolType, Value: Param{Type: BooleanT, Value: true}}}}}}, + ps: Params{{RawMessage: []byte(`"a"`)}, {RawMessage: []byte(`[{"type": "Integer", "value": "42"}]`)}}, // C# code doesn't use strict type assertions for JSON-ised params + script: "002a11c01f0c01610c146f459162ceeb248b071ec157d9e4f6fd26fdbe5041627d5b52", + }, { + ps: Params{{RawMessage: []byte(`"a"`)}, {RawMessage: []byte(`[{"type": "Integer", "value": true}]`)}}, // C# code doesn't use strict type assertions for JSON-ised params script: "1111c01f0c01610c146f459162ceeb248b071ec157d9e4f6fd26fdbe5041627d5b52", }, { - ps: Params{{Type: StringT, Value: "a"}, {Type: ArrayT, Value: []Param{{Type: FuncParamT, Value: FuncParam{Type: smartcontract.BoolType, Value: Param{Type: BooleanT, Value: false}}}}}}, + ps: Params{{RawMessage: []byte(`"a"`)}, {RawMessage: []byte(`[{"type": "Boolean", "value": true}]`)}}, + script: "1111c01f0c01610c146f459162ceeb248b071ec157d9e4f6fd26fdbe5041627d5b52", + }, { + ps: Params{{RawMessage: []byte(`"a"`)}, {RawMessage: []byte(`[{"type": "Boolean", "value": false}]`)}}, script: "1011c01f0c01610c146f459162ceeb248b071ec157d9e4f6fd26fdbe5041627d5b52", + }, { + ps: Params{{RawMessage: []byte(`"a"`)}, {RawMessage: []byte(`[{"type": "Boolean", "value": "blah"}]`)}}, // C# code doesn't use strict type assertions for JSON-ised params + script: "1111c01f0c01610c146f459162ceeb248b071ec157d9e4f6fd26fdbe5041627d5b52", }} - for _, ps := range paramScripts { - script, err := CreateFunctionInvocationScript(contract, ps.ps[0].String(), ps.ps[1:]) + for i, ps := range paramScripts { + method, err := ps.ps[0].GetString() + require.NoError(t, err, fmt.Sprintf("testcase #%d", i)) + script, err := CreateFunctionInvocationScript(contract, method, ps.ps[1:]) assert.Nil(t, err) - assert.Equal(t, ps.script, hex.EncodeToString(script)) + assert.Equal(t, ps.script, hex.EncodeToString(script), fmt.Sprintf("testcase #%d", i)) } } @@ -71,25 +82,18 @@ func TestInvocationScriptCreationBad(t *testing.T) { contract := util.Uint160{} var testParams = []Params{ - {{Type: NumberT, Value: "qwerty"}}, - {{Type: ArrayT, Value: 42}}, - {{Type: ArrayT, Value: []Param{{Type: NumberT, Value: 42}}}}, - {{Type: ArrayT, Value: []Param{{Type: FuncParamT, Value: FuncParam{Type: smartcontract.ByteArrayType, Value: Param{Type: StringT, Value: "qwerty"}}}}}}, - {{Type: ArrayT, Value: []Param{{Type: FuncParamT, Value: FuncParam{Type: smartcontract.SignatureType, Value: Param{Type: StringT, Value: "qwerty"}}}}}}, - {{Type: ArrayT, Value: []Param{{Type: FuncParamT, Value: FuncParam{Type: smartcontract.StringType, Value: Param{Type: NumberT, Value: 42}}}}}}, - {{Type: ArrayT, Value: []Param{{Type: FuncParamT, Value: FuncParam{Type: smartcontract.Hash160Type, Value: Param{Type: StringT, Value: "qwerty"}}}}}}, - {{Type: ArrayT, Value: []Param{{Type: FuncParamT, Value: FuncParam{Type: smartcontract.Hash256Type, Value: Param{Type: StringT, Value: "qwerty"}}}}}}, - {{Type: ArrayT, Value: []Param{{Type: FuncParamT, Value: FuncParam{Type: smartcontract.PublicKeyType, Value: Param{Type: NumberT, Value: 42}}}}}}, - {{Type: ArrayT, Value: []Param{{Type: FuncParamT, Value: FuncParam{Type: smartcontract.PublicKeyType, Value: Param{Type: StringT, Value: "qwerty"}}}}}}, - {{Type: ArrayT, Value: []Param{{Type: FuncParamT, Value: FuncParam{Type: smartcontract.IntegerType, Value: Param{Type: StringT, Value: "qwerty"}}}}}}, - {{Type: ArrayT, Value: []Param{{Type: FuncParamT, Value: FuncParam{Type: smartcontract.IntegerType, Value: Param{Type: BooleanT, Value: true}}}}}}, - {{Type: ArrayT, Value: []Param{{Type: FuncParamT, Value: FuncParam{Type: smartcontract.BoolType, Value: Param{Type: NumberT, Value: 42}}}}}}, - {{Type: ArrayT, Value: []Param{{Type: FuncParamT, Value: FuncParam{Type: smartcontract.BoolType, Value: Param{Type: StringT, Value: "qwerty"}}}}}}, - {{Type: ArrayT, Value: []Param{{Type: FuncParamT, Value: FuncParam{Type: smartcontract.UnknownType, Value: Param{}}}}}}, + {{RawMessage: []byte(`[{"type": "ByteArray", "value": "qwerty"}]`)}}, + {{RawMessage: []byte(`[{"type": "Signature", "value": "qwerty"}]`)}}, + {{RawMessage: []byte(`[{"type": "Hash160", "value": "qwerty"}]`)}}, + {{RawMessage: []byte(`[{"type": "Hash256", "value": "qwerty"}]`)}}, + {{RawMessage: []byte(`[{"type": "PublicKey", "value": 42}]`)}}, + {{RawMessage: []byte(`[{"type": "PublicKey", "value": "qwerty"}]`)}}, + {{RawMessage: []byte(`[{"type": "Integer", "value": "123q"}]`)}}, + {{RawMessage: []byte(`[{"type": "Unknown"}]`)}}, } - for _, ps := range testParams { + for i, ps := range testParams { _, err := CreateFunctionInvocationScript(contract, "", ps) - assert.NotNil(t, err) + assert.NotNil(t, err, fmt.Sprintf("testcase #%d", i)) } } @@ -99,11 +103,11 @@ func TestExpandArrayIntoScript(t *testing.T) { Expected []byte }{ { - Input: []Param{{Type: FuncParamT, Value: FuncParam{Type: smartcontract.StringType, Value: Param{Value: "a"}}}}, + Input: []Param{{RawMessage: []byte(`{"type": "String", "value": "a"}`)}}, Expected: []byte{byte(opcode.PUSHDATA1), 1, byte('a')}, }, { - Input: []Param{{Type: FuncParamT, Value: FuncParam{Type: smartcontract.ArrayType, Value: Param{Value: []Param{{Type: FuncParamT, Value: FuncParam{Type: smartcontract.StringType, Value: Param{Value: "a"}}}}}}}}, + Input: []Param{{RawMessage: []byte(`{"type": "Array", "value": [{"type": "String", "value": "a"}]}`)}}, Expected: []byte{byte(opcode.PUSHDATA1), 1, byte('a'), byte(opcode.PUSH1), byte(opcode.PACK)}, }, } @@ -115,10 +119,10 @@ func TestExpandArrayIntoScript(t *testing.T) { } errorCases := [][]Param{ { - {Type: FuncParamT, Value: FuncParam{Type: smartcontract.ArrayType, Value: Param{Value: "a"}}}, + {RawMessage: []byte(`{"type": "Array", "value": "a"}`)}, }, { - {Type: FuncParamT, Value: FuncParam{Type: smartcontract.ArrayType, Value: Param{Value: []Param{{Type: FuncParamT, Value: nil}}}}}, + {RawMessage: []byte(`{"type": "Array", "value": null}`)}, }, } for _, c := range errorCases { diff --git a/pkg/rpc/request/types.go b/pkg/rpc/request/types.go index 450815489..654771060 100644 --- a/pkg/rpc/request/types.go +++ b/pkg/rpc/request/types.go @@ -53,7 +53,7 @@ type Request struct { type In struct { JSONRPC string `json:"jsonrpc"` Method string `json:"method"` - RawParams json.RawMessage `json:"params,omitempty"` + RawParams []Param `json:"params,omitempty"` RawID json.RawMessage `json:"id,omitempty"` } @@ -134,16 +134,3 @@ func NewIn() *In { JSONRPC: JSONRPCVersion, } } - -// Params takes a slice of any type and attempts to bind -// the params to it. -func (r *In) Params() (*Params, error) { - params := Params{} - - err := json.Unmarshal(r.RawParams, ¶ms) - if err != nil { - return nil, fmt.Errorf("error parsing params: %w", err) - } - - return ¶ms, nil -} diff --git a/pkg/rpc/server/server.go b/pkg/rpc/server/server.go index 051dda712..a1ee03739 100644 --- a/pkg/rpc/server/server.go +++ b/pkg/rpc/server/server.go @@ -335,10 +335,7 @@ func (s *Server) handleIn(req *request.In, sub *subscriber) response.Abstract { return s.packResponse(req, nil, response.NewInvalidParamsError("Problem parsing JSON", fmt.Errorf("invalid version, expected 2.0 got: '%s'", req.JSONRPC))) } - reqParams, err := req.Params() - if err != nil { - return s.packResponse(req, nil, response.NewInvalidParamsError("Problem parsing request parameters", err)) - } + reqParams := request.Params(req.RawParams) s.log.Debug("processing rpc request", zap.String("method", req.Method), @@ -349,11 +346,11 @@ func (s *Server) handleIn(req *request.In, sub *subscriber) response.Abstract { resErr = response.NewMethodNotFoundError(fmt.Sprintf("Method '%s' not supported", req.Method), nil) handler, ok := rpcHandlers[req.Method] if ok { - res, resErr = handler(s, *reqParams) + res, resErr = handler(s, reqParams) } else if sub != nil { handler, ok := rpcWsHandlers[req.Method] if ok { - res, resErr = handler(s, *reqParams, sub) + res, resErr = handler(s, reqParams, sub) } } return s.packResponse(req, res, resErr) @@ -468,21 +465,18 @@ func (s *Server) blockHashFromParam(param *request.Param) (util.Uint256, *respon return hash, response.ErrInvalidParams } - switch param.Type { - case request.StringT: + if _, err := param.GetStringStrict(); err == nil { var err error hash, err = param.GetUint256() if err != nil { return hash, response.ErrInvalidParams } - case request.NumberT: + } else { num, err := s.blockHeightFromParam(param) if err != nil { return hash, response.ErrInvalidParams } hash = s.chain.GetHeaderHash(num) - default: - return hash, response.ErrInvalidParams } return hash, nil } @@ -499,7 +493,7 @@ func (s *Server) getBlock(reqParams request.Params) (interface{}, *response.Erro return nil, response.NewInternalServerError(fmt.Sprintf("Problem locating block with hash: %s", hash), err) } - if reqParams.Value(1).GetBoolean() { + if v, _ := reqParams.Value(1).GetBoolean(); v { return result.NewBlock(block, s.chain), nil } writer := io.NewBufBinWriter() @@ -508,11 +502,7 @@ func (s *Server) getBlock(reqParams request.Params) (interface{}, *response.Erro } func (s *Server) getBlockHash(reqParams request.Params) (interface{}, *response.Error) { - param := reqParams.ValueWithType(0, request.NumberT) - if param == nil { - return nil, response.ErrInvalidParams - } - num, err := s.blockHeightFromParam(param) + num, err := s.blockHeightFromParam(reqParams.Value(0)) if err != nil { return nil, response.ErrInvalidParams } @@ -557,7 +547,7 @@ func (s *Server) getPeers(_ request.Params) (interface{}, *response.Error) { } func (s *Server) getRawMempool(reqParams request.Params) (interface{}, *response.Error) { - verbose := reqParams.Value(0).GetBoolean() + verbose, _ := reqParams.Value(0).GetBoolean() mp := s.chain.GetMemPool() hashList := make([]util.Uint256, 0) for _, item := range mp.GetVerifiedTransactions() { @@ -574,11 +564,15 @@ func (s *Server) getRawMempool(reqParams request.Params) (interface{}, *response } func (s *Server) validateAddress(reqParams request.Params) (interface{}, *response.Error) { - param := reqParams.Value(0) - if param == nil { + param, err := reqParams.Value(0).GetString() + if err != nil { return nil, response.ErrInvalidParams } - return validateAddress(param.Value), nil + + return result.ValidateAddress{ + Address: reqParams.Value(0), + IsValid: validateAddress(param), + }, nil } // calculateNetworkFee calculates network fee for the transaction. @@ -669,11 +663,11 @@ func (s *Server) getApplicationLog(reqParams request.Params) (interface{}, *resp trig := trigger.All if len(reqParams) > 1 { - trigString := reqParams.ValueWithType(1, request.StringT) - if trigString == nil { + trigString, err := reqParams.Value(1).GetString() + if err != nil { return nil, response.ErrInvalidParams } - trig, err = trigger.FromString(trigString.String()) + trig, err = trigger.FromString(trigString) if err != nil { return nil, response.ErrInvalidParams } @@ -902,19 +896,13 @@ func (s *Server) contractIDFromParam(param *request.Param) (int32, *response.Err if param == nil { return 0, response.ErrInvalidParams } - switch param.Type { - case request.StringT: - var err error - scriptHash, err := param.GetUint160FromHex() - if err != nil { - return 0, response.ErrInvalidParams - } + if scriptHash, err := param.GetUint160FromHex(); err == nil { cs := s.chain.GetContractState(scriptHash) if cs == nil { return 0, response.ErrUnknown } result = cs.ID - case request.NumberT: + } else { id, err := param.GetInt() if err != nil { return 0, response.ErrInvalidParams @@ -923,8 +911,6 @@ func (s *Server) contractIDFromParam(param *request.Param) (int32, *response.Err return 0, response.WrapErrorWithData(response.ErrInvalidParams, err) } result = int32(id) - default: - return 0, response.ErrInvalidParams } return result, nil } @@ -935,8 +921,7 @@ func (s *Server) contractScriptHashFromParam(param *request.Param) (util.Uint160 if param == nil { return result, response.ErrInvalidParams } - switch param.Type { - case request.StringT: + if _, err := param.GetStringStrict(); err == nil { var err error result, err = param.GetUint160FromAddressOrHex() if err == nil { @@ -950,7 +935,7 @@ func (s *Server) contractScriptHashFromParam(param *request.Param) (util.Uint160 if err != nil { return result, response.NewRPCError("Unknown contract: querying by name is supported for native contracts only", "", nil) } - case request.NumberT: + } else { id, err := param.GetInt() if err != nil { return result, response.ErrInvalidParams @@ -962,8 +947,6 @@ func (s *Server) contractScriptHashFromParam(param *request.Param) (util.Uint160 if err != nil { return result, response.NewRPCError("Unknown contract", "", err) } - default: - return result, response.ErrInvalidParams } return result, nil } @@ -1244,7 +1227,7 @@ func (s *Server) getrawtransaction(reqParams request.Params) (interface{}, *resp err = fmt.Errorf("invalid transaction %s: %w", txHash, err) return nil, response.NewRPCError("Unknown transaction", err.Error(), err) } - if reqParams.Value(1).GetBoolean() { + if v, _ := reqParams.Value(1).GetBoolean(); v { if height == math.MaxUint32 { return result.NewTransactionOutputRaw(tx, nil, nil, s.chain), nil } @@ -1299,12 +1282,7 @@ func (s *Server) getNativeContracts(_ request.Params) (interface{}, *response.Er // getBlockSysFee returns the system fees of the block, based on the specified index. func (s *Server) getBlockSysFee(reqParams request.Params) (interface{}, *response.Error) { - param := reqParams.ValueWithType(0, request.NumberT) - if param == nil { - return 0, response.ErrInvalidParams - } - - num, err := s.blockHeightFromParam(param) + num, err := s.blockHeightFromParam(reqParams.Value(0)) if err != nil { return 0, response.NewRPCError("Invalid height", "", nil) } @@ -1331,7 +1309,7 @@ func (s *Server) getBlockHeader(reqParams request.Params) (interface{}, *respons return nil, respErr } - verbose := reqParams.Value(1).GetBoolean() + verbose, _ := reqParams.Value(1).GetBoolean() h, err := s.chain.GetHeader(hash) if err != nil { return nil, response.NewRPCError("unknown block", "", nil) @@ -1351,7 +1329,7 @@ func (s *Server) getBlockHeader(reqParams request.Params) (interface{}, *respons // getUnclaimedGas returns unclaimed GAS amount of the specified address. func (s *Server) getUnclaimedGas(ps request.Params) (interface{}, *response.Error) { - u, err := ps.ValueWithType(0, request.StringT).GetUint160FromAddressOrHex() + u, err := ps.Value(0).GetUint160FromAddressOrHex() if err != nil { return nil, response.ErrInvalidParams } @@ -1423,7 +1401,11 @@ func (s *Server) invokeFunction(reqParams request.Params) (interface{}, *respons if len(tx.Signers) == 0 { tx.Signers = []transaction.Signer{{Account: util.Uint160{}, Scopes: transaction.None}} } - script, err := request.CreateFunctionInvocationScript(scriptHash, reqParams[1].String(), reqParams[2:checkWitnessHashesIndex]) + method, err := reqParams[1].GetString() + if err != nil { + return nil, response.ErrInvalidParams + } + script, err := request.CreateFunctionInvocationScript(scriptHash, method, reqParams[2:checkWitnessHashesIndex]) if err != nil { return nil, response.NewInternalServerError("can't create invocation script", err) } @@ -1545,7 +1527,7 @@ func (s *Server) runScriptInVM(t trigger.Type, script []byte, contractScriptHash // submitBlock broadcasts a raw block over the NEO network. func (s *Server) submitBlock(reqParams request.Params) (interface{}, *response.Error) { - blockBytes, err := reqParams.ValueWithType(0, request.StringT).GetBytesBase64() + blockBytes, err := reqParams.Value(0).GetBytesBase64() if err != nil { return nil, response.NewInvalidParamsError("missing parameter or not base64", err) } @@ -1670,34 +1652,27 @@ func (s *Server) subscribe(reqParams request.Params, sub *subscriber) (interface // Optional filter. var filter interface{} if p := reqParams.Value(1); p != nil { - param, ok := p.Value.(json.RawMessage) - if !ok { - return nil, response.ErrInvalidParams - } - jd := json.NewDecoder(bytes.NewReader(param)) + param := *p + jd := json.NewDecoder(bytes.NewReader(param.RawMessage)) jd.DisallowUnknownFields() switch event { case response.BlockEventID: flt := new(request.BlockFilter) err = jd.Decode(flt) - p.Type = request.BlockFilterT - p.Value = *flt + filter = *flt case response.TransactionEventID, response.NotaryRequestEventID: flt := new(request.TxFilter) err = jd.Decode(flt) - p.Type = request.TxFilterT - p.Value = *flt + filter = *flt case response.NotificationEventID: flt := new(request.NotificationFilter) err = jd.Decode(flt) - p.Type = request.NotificationFilterT - p.Value = *flt + filter = *flt case response.ExecutionEventID: flt := new(request.ExecutionFilter) err = jd.Decode(flt) if err == nil && (flt.State == "HALT" || flt.State == "FAULT") { - p.Type = request.ExecutionFilterT - p.Value = *flt + filter = *flt } else if err == nil { err = errors.New("invalid state") } @@ -1705,7 +1680,6 @@ func (s *Server) subscribe(reqParams request.Params, sub *subscriber) (interface if err != nil { return nil, response.ErrInvalidParams } - filter = p.Value } s.subsLock.Lock() @@ -1937,7 +1911,7 @@ drainloop: func (s *Server) blockHeightFromParam(param *request.Param) (int, *response.Error) { num, err := param.GetInt() if err != nil { - return 0, nil + return 0, response.ErrInvalidParams } if num < 0 || num > int(s.chain.BlockHeight()) { @@ -1971,10 +1945,8 @@ func (s *Server) logRequestError(r *request.Request, jsonErr *response.Error) { if r.In != nil { logFields = append(logFields, zap.String("method", r.In.Method)) - params, err := r.In.Params() - if err == nil { - logFields = append(logFields, zap.Any("params", params)) - } + params := request.Params(r.In.RawParams) + logFields = append(logFields, zap.Any("params", params)) } s.log.Error("Error encountered with rpc request", logFields...) @@ -2021,11 +1993,10 @@ func (s *Server) writeHTTPServerResponse(r *request.Request, w http.ResponseWrit // validateAddress verifies that the address is a correct NEO address // see https://docs.neo.org/en-us/node/cli/2.9.4/api/validateaddress.html -func validateAddress(addr interface{}) result.ValidateAddress { - resp := result.ValidateAddress{Address: addr} +func validateAddress(addr interface{}) bool { if addr, ok := addr.(string); ok { _, err := address.StringToUint160(addr) - resp.IsValid = (err == nil) + return err == nil } - return resp + return false } diff --git a/pkg/rpc/server/server_test.go b/pkg/rpc/server/server_test.go index cd640ca35..af7c194f2 100644 --- a/pkg/rpc/server/server_test.go +++ b/pkg/rpc/server/server_test.go @@ -455,10 +455,6 @@ var rpcTestCases = map[string][]rpcTestCase{ return &v }, }, - { - params: "1", - fail: true, - }, }, "getblock": { { From 2fd04fbb35bf0a7d1bc2cc5a556494630ca45e9a Mon Sep 17 00:00:00 2001 From: Anna Shaleva Date: Tue, 9 Nov 2021 10:37:22 +0300 Subject: [PATCH 4/7] rpc: allow to pass null parameter to invoke* calls --- pkg/rpc/request/txBuilder.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkg/rpc/request/txBuilder.go b/pkg/rpc/request/txBuilder.go index f0dbc6747..c61a8f4c7 100644 --- a/pkg/rpc/request/txBuilder.go +++ b/pkg/rpc/request/txBuilder.go @@ -90,6 +90,10 @@ func ExpandArrayIntoScript(script *io.BinWriter, slice []Param) error { } emit.Int(script, int64(len(val))) emit.Opcodes(script, opcode.PACK) + case smartcontract.AnyType: + if fp.Value.IsNull() { + emit.Opcodes(script, opcode.PUSHNULL) + } default: return fmt.Errorf("parameter type %v is not supported", fp.Type) } From 867bb708fca470028ca277be8af77fac6e8fa55d Mon Sep 17 00:00:00 2001 From: Anna Shaleva Date: Thu, 28 Oct 2021 17:24:38 +0300 Subject: [PATCH 5/7] rpc: add cache to basic parameters Need to cache values of string, bool, int and array because they can be reused multiple times by RPC handlers. Other values don't need to be cached. --- pkg/rpc/request/param.go | 187 ++++++++++++++++++++++++++------------- 1 file changed, 127 insertions(+), 60 deletions(-) diff --git a/pkg/rpc/request/param.go b/pkg/rpc/request/param.go index eacc6ea27..78fc8b08a 100644 --- a/pkg/rpc/request/param.go +++ b/pkg/rpc/request/param.go @@ -23,6 +23,7 @@ type ( // the client. Param struct { json.RawMessage + cache interface{} } // FuncParam represents a function argument parameter used in the @@ -86,12 +87,18 @@ func (p *Param) GetStringStrict() (string, error) { if p.IsNull() { return "", errNotAString } - var s string - err := json.Unmarshal(p.RawMessage, &s) - if err != nil { - return "", errNotAString + if p.cache == nil { + var s string + err := json.Unmarshal(p.RawMessage, &s) + if err != nil { + return "", errNotAString + } + p.cache = s } - return s, nil + if s, ok := p.cache.(string); ok { + return s, nil + } + return "", errNotAString } // GetString returns string value of the parameter or tries to cast parameter to a string value. @@ -102,25 +109,40 @@ func (p *Param) GetString() (string, error) { if p.IsNull() { return "", errNotAString } - var s string - err := json.Unmarshal(p.RawMessage, &s) - if err == nil { - return s, nil + if p.cache == nil { + var s string + err := json.Unmarshal(p.RawMessage, &s) + if err == nil { + p.cache = s + } else { + var i int + err = json.Unmarshal(p.RawMessage, &i) + if err == nil { + p.cache = i + } else { + var b bool + err = json.Unmarshal(p.RawMessage, &b) + if err == nil { + p.cache = b + } else { + return "", errNotAString + } + } + } } - var i int - err = json.Unmarshal(p.RawMessage, &i) - if err == nil { - return strconv.Itoa(i), nil - } - var b bool - err = json.Unmarshal(p.RawMessage, &b) - if err == nil { - if b { + switch t := p.cache.(type) { + case string: + return t, nil + case int: + return strconv.Itoa(t), nil + case bool: + if t { return "true", nil } return "false", nil + default: + return "", errNotAString } - return "", errNotAString } // GetBooleanStrict returns boolean value of the parameter. @@ -129,9 +151,11 @@ func (p *Param) GetBooleanStrict() (bool, error) { return false, errMissingParameter } if bytes.Equal(p.RawMessage, jsonTrueBytes) { + p.cache = true return true, nil } if bytes.Equal(p.RawMessage, jsonFalseBytes) { + p.cache = false return false, nil } return false, errNotABool @@ -146,21 +170,36 @@ func (p *Param) GetBoolean() (bool, error) { return false, errNotABool } var b bool - err := json.Unmarshal(p.RawMessage, &b) - if err == nil { - return b, nil + if p.cache == nil { + err := json.Unmarshal(p.RawMessage, &b) + if err == nil { + p.cache = b + } else { + var s string + err = json.Unmarshal(p.RawMessage, &s) + if err == nil { + p.cache = s + } else { + var i int + err = json.Unmarshal(p.RawMessage, &i) + if err == nil { + p.cache = i + } else { + return false, errNotABool + } + } + } } - var s string - err = json.Unmarshal(p.RawMessage, &s) - if err == nil { - return s != "", nil + switch t := p.cache.(type) { + case bool: + return t, nil + case string: + return t != "", nil + case int: + return t != 0, nil + default: + return false, errNotABool } - var i int - err = json.Unmarshal(p.RawMessage, &i) - if err == nil { - return i != 0, nil - } - return false, errNotABool } // GetIntStrict returns int value of the parameter if the parameter is integer. @@ -171,12 +210,18 @@ func (p *Param) GetIntStrict() (int, error) { if p.IsNull() { return 0, errNotAnInt } - var i int - err := json.Unmarshal(p.RawMessage, &i) - if err != nil { - return i, errNotAnInt + if p.cache == nil { + var i int + err := json.Unmarshal(p.RawMessage, &i) + if err != nil { + return i, errNotAnInt + } + p.cache = i } - return i, nil + if i, ok := p.cache.(int); ok { + return i, nil + } + return 0, errNotAnInt } // GetInt returns int value of the parameter or tries to cast parameter to an int value. @@ -187,26 +232,40 @@ func (p *Param) GetInt() (int, error) { if p.IsNull() { return 0, errNotAnInt } - var i int - err := json.Unmarshal(p.RawMessage, &i) - if err == nil { - return i, nil - } - var s string - err = json.Unmarshal(p.RawMessage, &s) - if err == nil { - return strconv.Atoi(s) - } - var b bool - err = json.Unmarshal(p.RawMessage, &b) - if err == nil { - i = 0 - if b { - i = 1 + if p.cache == nil { + var i int + err := json.Unmarshal(p.RawMessage, &i) + if err == nil { + p.cache = i + } else { + var s string + err = json.Unmarshal(p.RawMessage, &s) + if err == nil { + p.cache = s + } else { + var b bool + err = json.Unmarshal(p.RawMessage, &b) + if err == nil { + p.cache = b + } else { + return 0, errNotAnInt + } + } } - return i, nil } - return 0, errNotAnInt + switch t := p.cache.(type) { + case int: + return t, nil + case string: + return strconv.Atoi(t) + case bool: + if t { + return 1, nil + } + return 0, nil + default: + return 0, errNotAnInt + } } // GetArray returns a slice of Params stored in the parameter. @@ -217,12 +276,18 @@ func (p *Param) GetArray() ([]Param, error) { if p.IsNull() { return nil, errNotAnArray } - a := []Param{} - err := json.Unmarshal(p.RawMessage, &a) - if err != nil { - return nil, errNotAnArray + if p.cache == nil { + a := []Param{} + err := json.Unmarshal(p.RawMessage, &a) + if err != nil { + return nil, errNotAnArray + } + p.cache = a } - return a, nil + if a, ok := p.cache.([]Param); ok { + return a, nil + } + return nil, errNotAnArray } // GetUint256 returns Uint256 value of the parameter. @@ -274,6 +339,7 @@ func (p *Param) GetFuncParam() (FuncParam, error) { if p == nil { return FuncParam{}, errMissingParameter } + // This one doesn't need to be cached, it's used only once. fp := FuncParam{} err := json.Unmarshal(p.RawMessage, &fp) return fp, err @@ -303,6 +369,7 @@ func (p *Param) GetBytesBase64() ([]byte, error) { // GetSignerWithWitness returns SignerWithWitness value of the parameter. func (p *Param) GetSignerWithWitness() (SignerWithWitness, error) { + // This one doesn't need to be cached, it's used only once. aux := new(signerWithWitnessAux) err := json.Unmarshal(p.RawMessage, aux) if err != nil { From 4072c2fa90387287d4587272c18838bfe0e776c4 Mon Sep 17 00:00:00 2001 From: Anna Shaleva Date: Fri, 29 Oct 2021 17:06:45 +0300 Subject: [PATCH 6/7] rpc: handlers parameters audit Make them compatible with C#. --- docs/rpc.md | 9 ++++- pkg/rpc/server/server.go | 75 +++++++++++++++++----------------------- 2 files changed, 40 insertions(+), 44 deletions(-) diff --git a/docs/rpc.md b/docs/rpc.md index af0c14b98..901bd06ca 100644 --- a/docs/rpc.md +++ b/docs/rpc.md @@ -95,10 +95,17 @@ it only works for native contracts. VM state is included to verbose response along with other transaction fields if the transaction is already on chain. +##### `getstateroot` + +This method is able to accept state root hash instead of index, unlike the C# node +where only index is accepted. + ##### `getstorage` This method doesn't work for the Ledger contract, you can get data via regular -`getblock` and `getrawtransaction` calls. +`getblock` and `getrawtransaction` calls. This method is able to get storage of +the native contract by its name (case-insensitive), unlike the C# node where +it only possible for index or hash. #### `getnep17balances` diff --git a/pkg/rpc/server/server.go b/pkg/rpc/server/server.go index a1ee03739..c8e23a9ce 100644 --- a/pkg/rpc/server/server.go +++ b/pkg/rpc/server/server.go @@ -459,22 +459,18 @@ func (s *Server) getConnectionCount(_ request.Params) (interface{}, *response.Er } func (s *Server) blockHashFromParam(param *request.Param) (util.Uint256, *response.Error) { - var hash util.Uint256 - + var ( + hash util.Uint256 + err error + ) if param == nil { return hash, response.ErrInvalidParams } - if _, err := param.GetStringStrict(); err == nil { - var err error - hash, err = param.GetUint256() - if err != nil { - return hash, response.ErrInvalidParams - } - } else { - num, err := s.blockHeightFromParam(param) - if err != nil { - return hash, response.ErrInvalidParams + if hash, err = param.GetUint256(); err != nil { + num, respErr := s.blockHeightFromParam(param) + if respErr != nil { + return hash, respErr } hash = s.chain.GetHeaderHash(num) } @@ -921,32 +917,28 @@ func (s *Server) contractScriptHashFromParam(param *request.Param) (util.Uint160 if param == nil { return result, response.ErrInvalidParams } - if _, err := param.GetStringStrict(); err == nil { - var err error - result, err = param.GetUint160FromAddressOrHex() - if err == nil { - return result, nil - } - name, err := param.GetString() - if err != nil { - return result, response.ErrInvalidParams - } - result, err = s.chain.GetNativeContractScriptHash(name) - if err != nil { - return result, response.NewRPCError("Unknown contract: querying by name is supported for native contracts only", "", nil) - } - } else { - id, err := param.GetInt() - if err != nil { - return result, response.ErrInvalidParams - } - if err := checkInt32(id); err != nil { - return result, response.WrapErrorWithData(response.ErrInvalidParams, err) - } - result, err = s.chain.GetContractScriptHash(int32(id)) - if err != nil { - return result, response.NewRPCError("Unknown contract", "", err) - } + nameOrHashOrIndex, err := param.GetString() + if err != nil { + return result, response.ErrInvalidParams + } + result, err = param.GetUint160FromAddressOrHex() + if err == nil { + return result, nil + } + result, err = s.chain.GetNativeContractScriptHash(nameOrHashOrIndex) + if err == nil { + return result, nil + } + id, err := strconv.Atoi(nameOrHashOrIndex) + if err != nil { + return result, response.NewRPCError("Unknown contract", "", err) + } + if err := checkInt32(id); err != nil { + return result, response.WrapErrorWithData(response.ErrInvalidParams, err) + } + result, err = s.chain.GetContractScriptHash(int32(id)) + if err != nil { + return result, response.NewRPCError("Unknown contract", "", err) } return result, nil } @@ -1176,7 +1168,7 @@ func (s *Server) getStateRoot(ps request.Params) (interface{}, *response.Error) } var rt *state.MPTRoot var h util.Uint256 - height, err := p.GetInt() + height, err := p.GetIntStrict() if err == nil { if err := checkUint32(height); err != nil { return nil, response.WrapErrorWithData(response.ErrInvalidParams, err) @@ -1557,10 +1549,7 @@ func (s *Server) submitNotaryRequest(ps request.Params) (interface{}, *response. return nil, response.NewInternalServerError("P2PNotaryRequest was received, but P2PSignatureExtensions are disabled", nil) } - if len(ps) < 1 { - return nil, response.NewInvalidParamsError("not enough parameters", nil) - } - bytePayload, err := ps[0].GetBytesBase64() + bytePayload, err := ps.Value(0).GetBytesBase64() if err != nil { return nil, response.NewInvalidParamsError("not base64", err) } From 3c13c8b7a58cd38b1d301df33e4f0cce65be9c01 Mon Sep 17 00:00:00 2001 From: Anna Shaleva Date: Wed, 10 Nov 2021 14:31:13 +0300 Subject: [PATCH 7/7] rpc: refactor GetUint160FromHex helper We can trim prefix using `strings` library like it is done for uint256. --- pkg/rpc/request/param.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/pkg/rpc/request/param.go b/pkg/rpc/request/param.go index 78fc8b08a..77bef2e85 100644 --- a/pkg/rpc/request/param.go +++ b/pkg/rpc/request/param.go @@ -306,11 +306,8 @@ func (p *Param) GetUint160FromHex() (util.Uint160, error) { if err != nil { return util.Uint160{}, err } - if len(s) == 2*util.Uint160Size+2 && s[0] == '0' && s[1] == 'x' { - s = s[2:] - } - return util.Uint160DecodeStringLE(s) + return util.Uint160DecodeStringLE(strings.TrimPrefix(s, "0x")) } // GetUint160FromAddress returns Uint160 value of the parameter that was