From 763452fe33bcff8a818bdb0ea08b584a2969ce41 Mon Sep 17 00:00:00 2001 From: Evgeniy Kulikov Date: Wed, 13 Feb 2019 21:01:52 +0300 Subject: [PATCH] Fix #130: Wrong answer for RPC server method getaccountstate (#131) * Fix #130: Wrong answer for RPC server method getaccountstate - fixed RPC Server response - fixed RPC Server tests - remove unused package from go.mod * add index and type checker * fix review comments (thx @aprasolova) --- go.mod | 1 - pkg/rpc/errors.go | 8 +++++++ pkg/rpc/params.go | 17 ++++++++++++++ pkg/rpc/server.go | 9 +++---- pkg/rpc/server_test.go | 31 +++++++++++++++++------- pkg/rpc/wrappers/account_state.go | 39 ++++++++++++++++++++++--------- 6 files changed, 78 insertions(+), 27 deletions(-) diff --git a/go.mod b/go.mod index 06a63a4f2..0e44bbbc4 100644 --- a/go.mod +++ b/go.mod @@ -19,5 +19,4 @@ require ( golang.org/x/tools v0.0.0-20180318012157-96caea41033d gopkg.in/airbrake/gobrake.v2 v2.0.9 // indirect gopkg.in/gemnasium/logrus-airbrake-hook.v2 v2.1.2 // indirect - gopkg.in/h2non/gock.v1 v1.0.12 // indirect ) diff --git a/pkg/rpc/errors.go b/pkg/rpc/errors.go index 662b24c6b..1127d0786 100644 --- a/pkg/rpc/errors.go +++ b/pkg/rpc/errors.go @@ -17,6 +17,14 @@ type ( } ) +var ( + // ErrEmptyParams is CSharp error caused by index out of range + ErrEmptyParams = newError(-2146233086, http.StatusOK, "Index was out of range. Must be non-negative and less than the size of the collection.\nParameter name: index", "", nil) + + // ErrTypeMismatch is CSharp error caused by type mismatch + ErrTypeMismatch = newError(-2146233033, http.StatusOK, "One of the identified items was in an invalid format.", "", nil) +) + func newError(code int64, httpCode int, message string, data string, cause error) *Error { return &Error{ Code: code, diff --git a/pkg/rpc/params.go b/pkg/rpc/params.go index 0995c734a..689070617 100644 --- a/pkg/rpc/params.go +++ b/pkg/rpc/params.go @@ -60,3 +60,20 @@ func (p Params) ValueAtAndType(index int, valueType string) (*Param, bool) { return nil, false } + +func (p Params) Value(index int) (*Param, *Error) { + if len(p) <= index { + return nil, ErrEmptyParams + } + return &p[index], nil +} + +func (p Params) ValueWithType(index int, valType string) (*Param, *Error) { + val, err := p.Value(index) + if err != nil { + return nil, err + } else if val.Type != valType { + return nil, ErrTypeMismatch + } + return &p[index], nil +} diff --git a/pkg/rpc/server.go b/pkg/rpc/server.go index 7bcdb548c..2dd5fcf82 100644 --- a/pkg/rpc/server.go +++ b/pkg/rpc/server.go @@ -208,12 +208,9 @@ Methods: } case "getaccountstate": - var err error - - param, exists := reqParams.ValueAtAndType(0, "string") - if !exists { - err = errors.New("expected param at index 0 to be a valid string account address parameter") - resultsErr = NewInvalidParamsError(err.Error(), err) + param, err := reqParams.ValueWithType(0, "string") + if err != nil { + resultsErr = err } else if scriptHash, err := crypto.Uint160DecodeAddress(param.StringVal); err != nil { err = errors.Wrapf(err, "unable to decode %s to Uint160", param.StringVal) resultsErr = NewInvalidParamsError(err.Error(), err) diff --git a/pkg/rpc/server_test.go b/pkg/rpc/server_test.go index ea73bd7fe..02d2e12f5 100644 --- a/pkg/rpc/server_test.go +++ b/pkg/rpc/server_test.go @@ -90,19 +90,32 @@ func TestHandler(t *testing.T) { "getpeers", `{"jsonrpc":"2.0","result":{"unconnected":[],"connected":[],"bad":[]},"id":1}`}, - {`{ "jsonrpc": "2.0", "id": 1, "method": "getaccountstate", "params": ["AK2nJJpJr6o664CWJKi1QRXjqeic2zRp8y"] }`, - "getaccountstate_1", - `{"jsonrpc":"2.0","result":{"version":0,"address":"AK2nJJpJr6o664CWJKi1QRXjqeic2zRp8y","script_hash":"0xe9eed8dc39332032dc22e5d6e86332c50327ba23","frozen":false,"votes":[],"balances":{"602c79718b16e442de58778e148d0b1084e3b2dffd5de6b7b16cee7969282de7":"72099.99960000","c56f33fc6ecfcd0c225c4ab356fee59390af8560be0e930faebe74a6daff7c9b":"99989900"}},"id":1}`, + // Good case, valid address + { + rpcCall: `{ "jsonrpc": "2.0", "id": 1, "method": "getaccountstate", "params": ["AK2nJJpJr6o664CWJKi1QRXjqeic2zRp8y"] }`, + method: "getaccountstate_1", + expectedResult: `{"jsonrpc":"2.0","result":{"version":0,"script_hash":"0xe9eed8dc39332032dc22e5d6e86332c50327ba23","frozen":false,"votes":[],"balances":[{"asset":"0x602c79718b16e442de58778e148d0b1084e3b2dffd5de6b7b16cee7969282de7","value":"72099.99960000"},{"asset":"0xc56f33fc6ecfcd0c225c4ab356fee59390af8560be0e930faebe74a6daff7c9b","value":"99989900"}]},"id":1}`, }, - {`{ "jsonrpc": "2.0", "id": 1, "method": "getaccountstate", "params": ["AK2nJJpJr6o664CWJKi1QRXjqeic2zR"] }`, - "getaccountstate_2", - `{"jsonrpc":"2.0","error":{"code":-32602,"message":"Invalid Params","data":"unable to decode AK2nJJpJr6o664CWJKi1QRXjqeic2zR to Uint160: invalid base-58 check string: invalid checksum."},"id":1}`, + // Bad case, invalid address + { + rpcCall: `{ "jsonrpc": "2.0", "id": 1, "method": "getaccountstate", "params": ["AK2nJJpJr6o664CWJKi1QRXjqeic2zR"] }`, + method: "getaccountstate_2", + expectedResult: `{"jsonrpc":"2.0","error":{"code":-32602,"message":"Invalid Params","data":"unable to decode AK2nJJpJr6o664CWJKi1QRXjqeic2zR to Uint160: invalid base-58 check string: invalid checksum."},"id":1}`, }, - {`{ "jsonrpc": "2.0", "id": 1, "method": "getaccountstate", "params": [123] }`, - "getaccountstate_3", - `{"jsonrpc":"2.0","error":{"code":-32602,"message":"Invalid Params","data":"expected param at index 0 to be a valid string account address parameter"},"id":1}`, + // Bad case, not string + { + rpcCall: `{ "jsonrpc": "2.0", "id": 1, "method": "getaccountstate", "params": [123] }`, + method: "getaccountstate_3", + expectedResult: `{"jsonrpc":"2.0","error":{"code":-2146233033,"message":"One of the identified items was in an invalid format."},"id":1}`, + }, + + // Bad case, empty params + { + rpcCall: `{ "jsonrpc": "2.0", "id": 1, "method": "getaccountstate", "params": [] }`, + method: "getaccountstate_4", + expectedResult: `{"jsonrpc":"2.0","error":{"code":-2146233086,"message":"Index was out of range. Must be non-negative and less than the size of the collection.\nParameter name: index"},"id":1}`, }, } diff --git a/pkg/rpc/wrappers/account_state.go b/pkg/rpc/wrappers/account_state.go index 98f7ebda3..8d6ab755e 100644 --- a/pkg/rpc/wrappers/account_state.go +++ b/pkg/rpc/wrappers/account_state.go @@ -1,6 +1,9 @@ package wrappers import ( + "bytes" + "sort" + "github.com/CityOfZion/neo-go/pkg/core" "github.com/CityOfZion/neo-go/pkg/crypto" "github.com/CityOfZion/neo-go/pkg/util" @@ -9,23 +12,38 @@ import ( // AccountState wrapper used for the representation of // core.AccountState on the RPC Server. type AccountState struct { - Version uint8 `json:"version"` - Address string `json:"address"` - ScriptHash util.Uint160 `json:"script_hash"` - IsFrozen bool `json:"frozen"` - Votes []*crypto.PublicKey `json:"votes"` - Balances map[string]util.Fixed8 `json:"balances"` + Version uint8 `json:"version"` + ScriptHash util.Uint160 `json:"script_hash"` + IsFrozen bool `json:"frozen"` + Votes []*crypto.PublicKey `json:"votes"` + Balances []Balance `json:"balances"` +} + +// Balances type for sorting balances in rpc response +type Balances []Balance + +func (b Balances) Len() int { return len(b) } +func (b Balances) Less(i, j int) bool { return bytes.Compare(b[i].Asset[:], b[j].Asset[:]) != -1 } +func (b Balances) Swap(i, j int) { b[i], b[j] = b[j], b[i] } + +// Balance response wrapper +type Balance struct { + Asset util.Uint256 `json:"asset"` + Value util.Fixed8 `json:"value"` } // NewAccountState creates a new AccountState wrapper. func NewAccountState(a *core.AccountState) AccountState { - balances := make(map[string]util.Fixed8) - address := crypto.AddressFromUint160(a.ScriptHash) - + balances := make(Balances, 0, len(a.Balances)) for k, v := range a.Balances { - balances[k.String()] = v + balances = append(balances, Balance{ + Asset: k, + Value: v, + }) } + sort.Sort(balances) + // reverse scriptHash to be consistent with other client scriptHash, err := util.Uint160DecodeBytes(a.ScriptHash.BytesReverse()) if err != nil { @@ -38,6 +56,5 @@ func NewAccountState(a *core.AccountState) AccountState { IsFrozen: a.IsFrozen, Votes: a.Votes, Balances: balances, - Address: address, } }