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

@@ -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
 )

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,

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
+}

pkg/rpc/server.go

@@ -208,12 +208,9 @@ Methods:
 		}
 
 	case "getaccountstate":
-		var err error
+		param, err := reqParams.ValueWithType(0, "string")
-
+		if err != nil {
-		param, exists := reqParams.ValueAtAndType(0, "string")
+			resultsErr = err
-		if !exists {
-			err = errors.New("expected param at index 0 to be a valid string account address parameter")
-			resultsErr = NewInvalidParamsError(err.Error(), 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)

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"] }`,
+		// Good case, valid address
-			"getaccountstate_1",
+		{
-			`{"jsonrpc":"2.0","result":{"version":0,"address":"AK2nJJpJr6o664CWJKi1QRXjqeic2zRp8y","script_hash":"0xe9eed8dc39332032dc22e5d6e86332c50327ba23","frozen":false,"votes":[],"balances":{"602c79718b16e442de58778e148d0b1084e3b2dffd5de6b7b16cee7969282de7":"72099.99960000","c56f33fc6ecfcd0c225c4ab356fee59390af8560be0e930faebe74a6daff7c9b":"99989900"}},"id":1}`,
+			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"] }`,
+		// Bad case, invalid address
-			"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}`,
+			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] }`,
+		// Bad case, not string
-			"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}`,
+			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}`,
 		},
 	}
 

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"
@@ -10,22 +13,37 @@ import (
 // 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"`
+	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)
+	balances := make(Balances, 0, len(a.Balances))
-	address := crypto.AddressFromUint160(a.ScriptHash)
-
 	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,
 	}
 }