From 7e87fd0d059539abc0f9525b4347e5926783b762 Mon Sep 17 00:00:00 2001 From: Mikhail Rogachev Date: Fri, 2 Aug 2024 12:43:53 +0400 Subject: [PATCH] fix(connector)_: connector response structure to valid json (#5625) --- services/connector/api.go | 42 +++++++++++++- services/connector/commands/accounts.go | 17 ++---- services/connector/commands/accounts_test.go | 11 +--- services/connector/commands/chain_id.go | 2 +- .../connector/commands/request_accounts.go | 4 +- .../commands/request_accounts_test.go | 17 +----- .../connector/commands/request_permissions.go | 27 +++------ .../commands/request_permissions_test.go | 10 ++-- services/connector/commands/rpc_traits.go | 2 +- .../connector/commands/send_transaction.go | 2 +- .../commands/switch_ethereum_chain.go | 2 +- services/connector/connector_flows_test.go | 56 ++++++++++++++----- 12 files changed, 111 insertions(+), 81 deletions(-) diff --git a/services/connector/api.go b/services/connector/api.go index 2fee8aaca..75d7face6 100644 --- a/services/connector/api.go +++ b/services/connector/api.go @@ -1,10 +1,18 @@ package connector import ( + "encoding/json" + "errors" + "fmt" + "github.com/status-im/status-go/services/connector/commands" persistence "github.com/status-im/status-go/services/connector/database" ) +var ( + ErrInvalidResponseFromForwardedRpc = errors.New("invalid response from forwarded RPC") +) + type API struct { s *Service r *CommandRegistry @@ -47,7 +55,37 @@ func NewAPI(s *Service) *API { } } -func (api *API) CallRPC(inputJSON string) (string, error) { +func (api *API) forwardRPC(URL string, inputJSON string) (interface{}, error) { + dApp, err := persistence.SelectDAppByUrl(api.s.db, URL) + if err != nil { + return "", err + } + + if dApp == nil { + return "", commands.ErrDAppIsNotPermittedByUser + } + + var response map[string]interface{} + rawResponse := api.s.rpc.CallRaw(inputJSON) + if err := json.Unmarshal([]byte(rawResponse), &response); err != nil { + return "", err + } + + if errorField, ok := response["error"]; ok { + errorMap, _ := errorField.(map[string]interface{}) + errorCode, _ := errorMap["code"].(float64) + errorMessage, _ := errorMap["message"].(string) + return nil, fmt.Errorf("error code %v: %s", errorCode, errorMessage) + } + + if result, ok := response["result"]; ok { + return result, nil + } + + return nil, ErrInvalidResponseFromForwardedRpc +} + +func (api *API) CallRPC(inputJSON string) (interface{}, error) { request, err := commands.RPCRequestFromJSON(inputJSON) if err != nil { return "", err @@ -57,7 +95,7 @@ func (api *API) CallRPC(inputJSON string) (string, error) { return command.Execute(request) } - return api.s.rpc.CallRaw(inputJSON), nil + return api.forwardRPC(request.URL, inputJSON) } func (api *API) RecallDAppPermission(origin string) error { diff --git a/services/connector/commands/accounts.go b/services/connector/commands/accounts.go index 004982781..de2a96149 100644 --- a/services/connector/commands/accounts.go +++ b/services/connector/commands/accounts.go @@ -2,8 +2,7 @@ package commands import ( "database/sql" - "encoding/json" - "fmt" + "strings" "github.com/status-im/status-go/eth-node/types" persistence "github.com/status-im/status-go/services/connector/database" @@ -13,17 +12,11 @@ type AccountsCommand struct { Db *sql.DB } -func (c *AccountsCommand) dAppToAccountsResponse(dApp *persistence.DApp) (string, error) { - addresses := []types.Address{dApp.SharedAccount} - responseJSON, err := json.Marshal(addresses) - if err != nil { - return "", fmt.Errorf("failed to marshal response: %v", err) - } - - return string(responseJSON), nil +func FormatAccountAddressToResponse(address types.Address) []string { + return []string{strings.ToLower(address.Hex())} } -func (c *AccountsCommand) Execute(request RPCRequest) (string, error) { +func (c *AccountsCommand) Execute(request RPCRequest) (interface{}, error) { err := request.Validate() if err != nil { return "", err @@ -38,5 +31,5 @@ func (c *AccountsCommand) Execute(request RPCRequest) (string, error) { return "", ErrDAppIsNotPermittedByUser } - return c.dAppToAccountsResponse(dApp) + return FormatAccountAddressToResponse(dApp.SharedAccount), nil } diff --git a/services/connector/commands/accounts_test.go b/services/connector/commands/accounts_test.go index 214e100c6..714cdb804 100644 --- a/services/connector/commands/accounts_test.go +++ b/services/connector/commands/accounts_test.go @@ -1,7 +1,6 @@ package commands import ( - "encoding/json" "testing" "github.com/stretchr/testify/assert" @@ -52,14 +51,8 @@ func TestGetAccountForPermittedDApp(t *testing.T) { request, err := ConstructRPCRequest("eth_accounts", []interface{}{}, &testDAppData) assert.NoError(t, err) + expectedResponse := FormatAccountAddressToResponse(sharedAccount) response, err := cmd.Execute(request) assert.NoError(t, err) - - // Unmarshal the response into a slice of addresses - var result []types.Address - err = json.Unmarshal([]byte(response), &result) - - assert.NoError(t, err) - assert.Len(t, result, 1) - assert.Equal(t, sharedAccount, result[0]) + assert.Equal(t, expectedResponse, response) } diff --git a/services/connector/commands/chain_id.go b/services/connector/commands/chain_id.go index d1b9d496d..f72206b15 100644 --- a/services/connector/commands/chain_id.go +++ b/services/connector/commands/chain_id.go @@ -14,7 +14,7 @@ type ChainIDCommand struct { Db *sql.DB } -func (c *ChainIDCommand) Execute(request RPCRequest) (string, error) { +func (c *ChainIDCommand) Execute(request RPCRequest) (interface{}, error) { err := request.Validate() if err != nil { return "", err diff --git a/services/connector/commands/request_accounts.go b/services/connector/commands/request_accounts.go index 78752d843..1c1c4e995 100644 --- a/services/connector/commands/request_accounts.go +++ b/services/connector/commands/request_accounts.go @@ -25,7 +25,7 @@ type RawAccountsResponse struct { Result []accounts.Account `json:"result"` } -func (c *RequestAccountsCommand) Execute(request RPCRequest) (string, error) { +func (c *RequestAccountsCommand) Execute(request RPCRequest) (interface{}, error) { err := request.Validate() if err != nil { return "", err @@ -61,5 +61,5 @@ func (c *RequestAccountsCommand) Execute(request RPCRequest) (string, error) { } } - return c.dAppToAccountsResponse(dApp) + return FormatAccountAddressToResponse(dApp.SharedAccount), nil } diff --git a/services/connector/commands/request_accounts_test.go b/services/connector/commands/request_accounts_test.go index a943e1fe7..6e2989597 100644 --- a/services/connector/commands/request_accounts_test.go +++ b/services/connector/commands/request_accounts_test.go @@ -86,16 +86,11 @@ func TestRequestAccountsAcceptedAndRequestAgain(t *testing.T) { } })) + expectedResponse := FormatAccountAddressToResponse(accountAddress) response, err := cmd.Execute(request) assert.NoError(t, err) - - // Unmarshal the response into a slice of addresses - var result []types.Address - err = json.Unmarshal([]byte(response), &result) - assert.NoError(t, err) - assert.Len(t, result, 1) - assert.Equal(t, accountAddress, result[0]) + assert.Equal(t, expectedResponse, response) // Check dApp in the database dApp, err := persistence.SelectDAppByUrl(db, request.URL) @@ -108,12 +103,7 @@ func TestRequestAccountsAcceptedAndRequestAgain(t *testing.T) { // This should not invoke UI side response, err = cmd.Execute(request) assert.NoError(t, err) - - err = json.Unmarshal([]byte(response), &result) - - assert.NoError(t, err) - assert.Len(t, result, 1) - assert.Equal(t, accountAddress, result[0]) + assert.Equal(t, expectedResponse, response) } func TestRequestAccountsRejected(t *testing.T) { @@ -150,5 +140,4 @@ func TestRequestAccountsRejected(t *testing.T) { _, err = cmd.Execute(request) assert.Equal(t, ErrRequestAccountsRejectedByUser, err) - } diff --git a/services/connector/commands/request_permissions.go b/services/connector/commands/request_permissions.go index ff95187a6..220a04ea0 100644 --- a/services/connector/commands/request_permissions.go +++ b/services/connector/commands/request_permissions.go @@ -1,30 +1,22 @@ package commands import ( - "encoding/json" "errors" "fmt" "time" ) -type RequestPermissionsCommand struct { -} +type RequestPermissionsCommand struct{} type Permission struct { ParentCapability string `json:"parentCapability"` Date string `json:"date"` } -type PermissionsResponse struct { - JSONRPC string `json:"jsonrpc"` - ID int `json:"id"` - Result []Permission `json:"result"` -} - var ( ErrNoRequestPermissionsParamsFound = errors.New("no request permission params found") - ErrMultipleKeysFound = errors.New("Multiple methodNames found in request permissions params") - ErrInvalidParamType = errors.New("Invalid parameter type") + ErrMultipleKeysFound = errors.New("multiple methodNames found in request permissions params") + ErrInvalidParamType = errors.New("invalid parameter type") ) func (r *RPCRequest) getRequestPermissionsParam() (string, error) { @@ -48,7 +40,7 @@ func (r *RPCRequest) getRequestPermissionsParam() (string, error) { return "", ErrNoRequestPermissionsParamsFound } -func (c *RequestPermissionsCommand) getPermissionResponse(methodName string) (string, error) { +func (c *RequestPermissionsCommand) getPermissionResponse(methodName string) Permission { date := time.Now().UnixNano() / int64(time.Millisecond) response := Permission{ @@ -56,15 +48,10 @@ func (c *RequestPermissionsCommand) getPermissionResponse(methodName string) (st Date: fmt.Sprintf("%d", date), } - responseJSON, err := json.Marshal(response) - if err != nil { - return "", fmt.Errorf("failed to marshal response: %v", err) - } - - return string(responseJSON), nil + return response } -func (c *RequestPermissionsCommand) Execute(request RPCRequest) (string, error) { +func (c *RequestPermissionsCommand) Execute(request RPCRequest) (interface{}, error) { err := request.Validate() if err != nil { return "", err @@ -75,5 +62,5 @@ func (c *RequestPermissionsCommand) Execute(request RPCRequest) (string, error) return "", err } - return c.getPermissionResponse(methodName) + return c.getPermissionResponse(methodName), nil } diff --git a/services/connector/commands/request_permissions_test.go b/services/connector/commands/request_permissions_test.go index c304aa4f0..a52869044 100644 --- a/services/connector/commands/request_permissions_test.go +++ b/services/connector/commands/request_permissions_test.go @@ -1,7 +1,6 @@ package commands import ( - "encoding/json" "testing" "github.com/stretchr/testify/assert" @@ -95,10 +94,11 @@ func TestRequestPermissionsResponse(t *testing.T) { } else { assert.NoError(t, err) - var permission Permission - err = json.Unmarshal([]byte(response), &permission) - assert.NoError(t, err) - assert.Equal(t, permission.ParentCapability, tc.expectedCapability) + if permission, ok := response.(Permission); ok { + assert.Equal(t, permission.ParentCapability, tc.expectedCapability) + } else { + assert.Fail(t, "Can't parse permission from the response") + } } }) } diff --git a/services/connector/commands/rpc_traits.go b/services/connector/commands/rpc_traits.go index 873728ad4..e0d8e6e42 100644 --- a/services/connector/commands/rpc_traits.go +++ b/services/connector/commands/rpc_traits.go @@ -29,7 +29,7 @@ type RPCRequest struct { } type RPCCommand interface { - Execute(request RPCRequest) (string, error) + Execute(request RPCRequest) (interface{}, error) } type RequestAccountsAcceptedArgs struct { diff --git a/services/connector/commands/send_transaction.go b/services/connector/commands/send_transaction.go index 37429cac9..b22b475cf 100644 --- a/services/connector/commands/send_transaction.go +++ b/services/connector/commands/send_transaction.go @@ -45,7 +45,7 @@ func (r *RPCRequest) getSendTransactionParams() (*transactions.SendTxArgs, error return &sendTxArgs, nil } -func (c *SendTransactionCommand) Execute(request RPCRequest) (string, error) { +func (c *SendTransactionCommand) Execute(request RPCRequest) (interface{}, error) { err := request.Validate() if err != nil { return "", err diff --git a/services/connector/commands/switch_ethereum_chain.go b/services/connector/commands/switch_ethereum_chain.go index dee8ef7cd..0f86f6ab6 100644 --- a/services/connector/commands/switch_ethereum_chain.go +++ b/services/connector/commands/switch_ethereum_chain.go @@ -52,7 +52,7 @@ func (c *SwitchEthereumChainCommand) getSupportedChainIDs() ([]uint64, error) { return chainutils.GetSupportedChainIDs(c.NetworkManager) } -func (c *SwitchEthereumChainCommand) Execute(request RPCRequest) (string, error) { +func (c *SwitchEthereumChainCommand) Execute(request RPCRequest) (interface{}, error) { err := request.Validate() if err != nil { return "", err diff --git a/services/connector/connector_flows_test.go b/services/connector/connector_flows_test.go index 228c6fc00..fb993fcee 100644 --- a/services/connector/connector_flows_test.go +++ b/services/connector/connector_flows_test.go @@ -3,14 +3,15 @@ package connector import ( "encoding/json" "fmt" - "strings" "testing" "github.com/stretchr/testify/assert" "github.com/status-im/status-go/eth-node/types" "github.com/status-im/status-go/params" + "github.com/status-im/status-go/services/connector/chainutils" "github.com/status-im/status-go/services/connector/commands" + walletCommon "github.com/status-im/status-go/services/wallet/common" "github.com/status-im/status-go/signal" ) @@ -21,11 +22,11 @@ func TestRequestAccountsSwitchChainAndSendTransactionFlow(t *testing.T) { nm := commands.NetworkManagerMock{} nm.SetNetworks([]*params.Network{ { - ChainID: 0x1, + ChainID: walletCommon.EthereumMainnet, Layer: 1, }, { - ChainID: 0x5, + ChainID: walletCommon.OptimismMainnet, Layer: 1, }, }) @@ -77,36 +78,65 @@ func TestRequestAccountsSwitchChainAndSendTransactionFlow(t *testing.T) { // Request accounts, now for real request = "{\"method\": \"eth_requestAccounts\", \"params\": [], \"url\": \"http://testDAppURL123\", \"name\": \"testDAppName\", \"iconUrl\": \"http://testDAppIconUrl\" }" - expectedResponse := strings.ToLower(fmt.Sprintf(`["%s"]`, accountAddress.Hex())) response, err = api.CallRPC(request) assert.NoError(t, err) - assert.Equal(t, expectedResponse, response) + assert.Equal(t, commands.FormatAccountAddressToResponse(accountAddress), response) // Request to switch ethereum chain - expectedChainId := "0x5" - request = fmt.Sprintf("{\"method\": \"wallet_switchEthereumChain\", \"params\": [{\"chainId\": \"%s\"}], \"url\": \"http://testDAppURL123\", \"name\": \"testDAppName\", \"iconUrl\": \"http://testDAppIconUrl\" }", expectedChainId) - expectedResponse = expectedChainId + expectedChainID, err := chainutils.GetHexChainID(walletCommon.ChainID(walletCommon.EthereumMainnet).String()) + assert.NoError(t, err) + request = fmt.Sprintf("{\"method\": \"wallet_switchEthereumChain\", \"params\": [{\"chainId\": \"%s\"}], \"url\": \"http://testDAppURL123\", \"name\": \"testDAppName\", \"iconUrl\": \"http://testDAppIconUrl\" }", expectedChainID) response, err = api.CallRPC(request) assert.NoError(t, err) - assert.Equal(t, expectedResponse, response) + assert.Equal(t, expectedChainID, response) // Check if the chain was switched request = "{\"method\": \"eth_chainId\", \"params\": [], \"url\": \"http://testDAppURL123\", \"name\": \"testDAppName\", \"iconUrl\": \"http://testDAppIconUrl\" }" response, err = api.CallRPC(request) assert.NoError(t, err) - assert.Equal(t, expectedResponse, response) + assert.Equal(t, expectedChainID, response) // Check the account after switching chain request = "{\"method\": \"eth_accounts\", \"params\": [], \"url\": \"http://testDAppURL123\", \"name\": \"testDAppName\", \"iconUrl\": \"http://testDAppIconUrl\" }" - expectedResponse = strings.ToLower(fmt.Sprintf(`["%s"]`, accountAddress.Hex())) response, err = api.CallRPC(request) assert.NoError(t, err) - assert.Equal(t, expectedResponse, response) + assert.Equal(t, commands.FormatAccountAddressToResponse(accountAddress), response) // Send transaction request = fmt.Sprintf("{\"method\": \"eth_sendTransaction\", \"params\":[{\"from\":\"%s\",\"to\":\"0x0200000000000000000000000000000000000000\",\"value\":\"0x12345\",\"data\":\"0x307830\"}], \"url\": \"http://testDAppURL123\", \"name\": \"testDAppName\", \"iconUrl\": \"http://testDAppIconUrl\" }", accountAddress.Hex()) - expectedResponse = expectedHash.Hex() response, err = api.CallRPC(request) assert.NoError(t, err) + assert.Equal(t, expectedHash.Hex(), response) +} + +func TestForwardedRPCs(t *testing.T) { + db, close := createDB(t) + defer close() + + rpc := commands.RPCClientMock{} + service := NewService(db, &rpc, nil) + + api := NewAPI(service) + + sharedAccount := types.BytesToAddress(types.FromHex("0x3d0ab2a774b74bb1d36f97700315adf962c69fct")) + + testDAppData := signal.ConnectorDApp{ + URL: "https://app.test.org", + Name: "testDAppName", + IconURL: "https://app.test.icon.org", + } + + request := "{\"method\": \"eth_blockNumber\", \"params\":[],\"url\":\"https://app.test.org\",\"name\":\"testDAppName\",\"iconUrl\":\"http://testDAppIconUrl\"}" + _, err := api.CallRPC(request) + assert.Equal(t, commands.ErrDAppIsNotPermittedByUser, err) + + err = commands.PersistDAppData(db, testDAppData, sharedAccount, 0x123) + assert.NoError(t, err) + + expectedResponse := "0xaa37dc" + rpc.SetResponse(fmt.Sprintf(`{"jsonrpc":"2.0","id":37,"result":"%s"}`, expectedResponse)) + + response, err := api.CallRPC(request) + assert.NoError(t, err) assert.Equal(t, expectedResponse, response) }