Merge branch 'develop' of github.com:status-im/status-go into develop

geth/api/backend_txqueue_test.go

@@ -195,6 +195,59 @@ func (s *BackendTestSuite) TestSendEtherTx() {
 	s.Zero(s.backend.TxQueueManager().TransactionQueue().Count(), "tx queue must be empty at this point")
 }
 
+func (s *BackendTestSuite) TestSendEtherTxUpstream() {
+	s.StartTestBackend(params.RopstenNetworkID, WithUpstream("https://ropsten.infura.io/z6GCTmjdP3FETEJmMBI4"))
+	defer s.StopTestBackend()
+
+	time.Sleep(TestConfig.Node.SyncSeconds * time.Second) // allow to sync
+
+	err := s.backend.AccountManager().SelectAccount(TestConfig.Account1.Address, TestConfig.Account1.Password)
+	s.NoError(err)
+
+	completeQueuedTransaction := make(chan struct{})
+
+	// replace transaction notification handler
+	var txHash = gethcommon.Hash{}
+	node.SetDefaultNodeNotificationHandler(func(jsonEvent string) { // nolint: dupl
+		var envelope node.SignalEnvelope
+		err := json.Unmarshal([]byte(jsonEvent), &envelope)
+		s.NoError(err, "cannot unmarshal JSON: %s", jsonEvent)
+
+		if envelope.Type == node.EventTransactionQueued {
+			event := envelope.Event.(map[string]interface{})
+			log.Info("transaction queued (will be completed shortly)", "id", event["id"].(string))
+
+			txHash, err = s.backend.CompleteTransaction(
+				common.QueuedTxID(event["id"].(string)),
+				TestConfig.Account1.Password,
+			)
+			s.NoError(err, "cannot complete queued transaction[%v]", event["id"])
+
+			log.Info("contract transaction complete", "URL", "https://ropsten.etherscan.io/tx/"+txHash.Hex())
+			close(completeQueuedTransaction)
+		}
+	})
+
+	// This call blocks, up until Complete Transaction is called.
+	// Explicitly not setting Gas to get it estimated.
+	txHashCheck, err := s.backend.SendTransaction(nil, common.SendTxArgs{
+		From:     common.FromAddress(TestConfig.Account1.Address),
+		To:       common.ToAddress(TestConfig.Account2.Address),
+		GasPrice: (*hexutil.Big)(big.NewInt(28000000000)),
+		Value:    (*hexutil.Big)(big.NewInt(1000000000000)),
+	})
+	s.NoError(err, "cannot send transaction")
+
+	select {
+	case <-completeQueuedTransaction:
+	case <-time.After(1 * time.Minute):
+		s.FailNow("completing transaction timed out")
+	}
+
+	s.Equal(txHash.Hex(), txHashCheck.Hex(), "transaction hash returned from SendTransaction is invalid")
+	s.Zero(s.backend.TxQueueManager().TransactionQueue().Count(), "tx queue must be empty at this point")
+}
+
 func (s *BackendTestSuite) TestDoubleCompleteQueuedTransactions() {
 	require := s.Require()
 	require.NotNil(s.backend)

geth/node/txqueue_manager.go

@@ -227,12 +227,16 @@ func (m *TxQueueManager) completeRemoteTransaction(queuedTx *common.QueuedTx, pa
 
 	chainID := big.NewInt(int64(config.NetworkID))
 	nonce := uint64(txCount)
-	gas := (*big.Int)(queuedTx.Args.Gas)
 	gasPrice := (*big.Int)(queuedTx.Args.GasPrice)
 	dataVal := []byte(queuedTx.Args.Data)
 	priceVal := (*big.Int)(queuedTx.Args.Value)
 
-	tx := types.NewTransaction(nonce, *queuedTx.Args.To, priceVal, gas, gasPrice, dataVal)
+	gas, err := m.estimateGas(queuedTx.Args)
+	if err != nil {
+		return emptyHash, err
+	}
+
+	tx := types.NewTransaction(nonce, *queuedTx.Args.To, priceVal, (*big.Int)(gas), gasPrice, dataVal)
 	signedTx, err := types.SignTx(tx, types.NewEIP155Signer(chainID), selectedAcct.AccountKey.PrivateKey)
 	if err != nil {
 		return emptyHash, err
@@ -253,6 +257,54 @@ func (m *TxQueueManager) completeRemoteTransaction(queuedTx *common.QueuedTx, pa
 	return signedTx.Hash(), nil
 }
 
+func (m *TxQueueManager) estimateGas(args common.SendTxArgs) (*hexutil.Big, error) {
+	if args.Gas != nil {
+		return args.Gas, nil
+	}
+
+	client := m.nodeManager.RPCClient()
+	ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
+	defer cancel()
+
+	var gasPrice hexutil.Big
+	if args.GasPrice != nil {
+		gasPrice = (hexutil.Big)(*args.GasPrice)
+	}
+
+	var value hexutil.Big
+	if args.Value != nil {
+		value = (hexutil.Big)(*args.Value)
+	}
+
+	params := struct {
+		From     gethcommon.Address  `json:"from"`
+		To       *gethcommon.Address `json:"to"`
+		Gas      hexutil.Big         `json:"gas"`
+		GasPrice hexutil.Big         `json:"gasPrice"`
+		Value    hexutil.Big         `json:"value"`
+		Data     hexutil.Bytes       `json:"data"`
+	}{
+		From:     args.From,
+		To:       args.To,
+		GasPrice: gasPrice,
+		Value:    value,
+		Data:     []byte(args.Data),
+	}
+
+	var estimatedGas hexutil.Big
+	if err := client.CallContext(
+		ctx,
+		&estimatedGas,
+		"eth_estimateGas",
+		params,
+	); err != nil {
+		log.Warn("failed to estimate gas", "err", err)
+		return nil, err
+	}
+
+	return &estimatedGas, nil
+}
+
 // CompleteTransactions instructs backend to complete sending of multiple transactions
 func (m *TxQueueManager) CompleteTransactions(ids []common.QueuedTxID, password string) map[common.QueuedTxID]common.RawCompleteTransactionResult {
 	results := make(map[common.QueuedTxID]common.RawCompleteTransactionResult)

geth/rpc/client.go

@@ -72,9 +72,8 @@ func (c *Client) Call(result interface{}, method string, args ...interface{}) er
 //
 // It uses custom routing scheme for calls.
 func (c *Client) CallContext(ctx context.Context, result interface{}, method string, args ...interface{}) error {
-	if c.router.routeLocally(method) {
-		return c.local.CallContext(ctx, result, method, args...)
-	}
-
+	if c.router.routeRemote(method) {
 		return c.upstream.CallContext(ctx, result, method, args...)
+	}
+	return c.local.CallContext(ctx, result, method, args...)
 }

geth/rpc/route.go

@@ -15,60 +15,73 @@ func newRouter(upstreamEnabled bool) *router {
 		upstreamEnabled: upstreamEnabled,
 	}
 
-	for _, m := range localMethods {
+	for _, m := range remoteMethods {
 		r.methods[m] = true
 	}
 
 	return r
 }
 
-// routeLocally returns true if given method should be routed to
-// the local node
-func (r *router) routeLocally(method string) bool {
-	// if upstream is disabled, always route to
-	// the local node
+// routeRemote returns true if given method should be routed to the remote node
+func (r *router) routeRemote(method string) bool {
 	if !r.upstreamEnabled {
-		return true
+		return false
 	}
 
 	// else check route using the methods list
 	return r.methods[method]
 }
 
-// localMethods contains methods that should be routed to
-// the local node; the rest is considered to be routed to
-// the upstream node.
-// TODO: in the future, default option may be "local node",
-// so it'd be convinient to keep track of "remoteMethods" here.
-var localMethods = [...]string{
-	//Whisper commands
-	"shh_post",
-	"shh_version",
-	"shh_newIdentity",
-	"shh_hasIdentity",
-	"shh_newGroup",
-	"shh_addToGroup",
-	"shh_newFilter",
-	"shh_uninstallFilter",
-	"shh_getFilterChanges",
-	"shh_getMessages",
-
-	// DB commands
-	"db_putString",
-	"db_getString",
-	"db_putHex",
-	"db_getHex",
-
-	// Other commands
-	"net_version",
-	"net_peerCount",
-	"net_listening",
-
-	// blockchain commands
-	"eth_sign",
-	"eth_accounts",
-	"eth_getCompilers",
-	"eth_compileLLL",
-	"eth_compileSolidity",
-	"eth_compileSerpent",
+// remoteMethods contains methods that should be routed to
+// the upstream node; the rest is considered to be routed to
+// the local node.
+// TODO(tiabc): Write a test on each of these methods to ensure they're all routed to the proper node and ensure they really work.
+// Although it's tempting to only list methods coming to the local node as there're fewer of them
+// but it's deceptive: we want to ensure that only known requests leave our zone of responsibility.
+// Also, we want new requests in newer Geth versions not to be accidentally routed to the upstream.
+// The list of methods: https://github.com/ethereum/wiki/wiki/JSON-RPC
+var remoteMethods = [...]string{
+	"eth_protocolVersion",
+	"eth_syncing",
+	"eth_coinbase",
+	"eth_mining",
+	"eth_hashrate",
+	"eth_gasPrice",
+	//"eth_accounts", // goes to the local because we handle sub-accounts
+	"eth_blockNumber",
+	"eth_getBalance",
+	"eth_getStorageAt",
+	"eth_getTransactionCount",
+	"eth_getBlockTransactionCountByHash",
+	"eth_getBlockTransactionCountByNumber",
+	"eth_getUncleCountByBlockHash",
+	"eth_getUncleCountByBlockNumber",
+	"eth_getCode",
+	//"eth_sign", // goes to the local because only the local node has an injected account to sign the payload with
+	"eth_sendTransaction",
+	"eth_sendRawTransaction",
+	"eth_call",
+	"eth_estimateGas",
+	"eth_getBlockByHash",
+	"eth_getBlockByNumber",
+	"eth_getTransactionByHash",
+	"eth_getTransactionByBlockHashAndIndex",
+	"eth_getTransactionByBlockNumberAndIndex",
+	"eth_getTransactionReceipt",
+	"eth_getUncleByBlockHashAndIndex",
+	"eth_getUncleByBlockNumberAndIndex",
+	//"eth_getCompilers",    // goes to the local because there's no need to send it anywhere
+	//"eth_compileLLL",      // goes to the local because there's no need to send it anywhere
+	//"eth_compileSolidity", // goes to the local because there's no need to send it anywhere
+	//"eth_compileSerpent",  // goes to the local because there's no need to send it anywhere
+	"eth_newFilter",
+	"eth_newBlockFilter",
+	"eth_newPendingTransactionFilter",
+	"eth_uninstallFilter",
+	"eth_getFilterChanges",
+	"eth_getFilterLogs",
+	"eth_getLogs",
+	"eth_getWork",
+	"eth_submitWork",
+	"eth_submitHashrate",
 }

geth/rpc/route_test.go

@@ -6,28 +6,30 @@ import (
 )
 
 // some of the upstream examples
-var upstreamMethods = []string{"some_weirdo_method", "eth_syncing", "eth_getBalance", "eth_call", "eth_getTransactionReceipt"}
+var localMethods = []string{"some_weirdo_method", "shh_newMessageFilter", "net_version"}
 
 func TestRouteWithUpstream(t *testing.T) {
 	router := newRouter(true)
 
-	for _, method := range localMethods {
-		require.True(t, router.routeLocally(method))
+	for _, method := range remoteMethods {
+		require.True(t, router.routeRemote(method))
 	}
 
-	for _, method := range upstreamMethods {
-		require.False(t, router.routeLocally(method))
+	for _, method := range localMethods {
+		t.Run(method, func(t *testing.T) {
+			require.False(t, router.routeRemote(method))
+		})
 	}
 }
 
 func TestRouteWithoutUpstream(t *testing.T) {
 	router := newRouter(false)
 
-	for _, method := range localMethods {
-		require.True(t, router.routeLocally(method))
+	for _, method := range remoteMethods {
+		require.True(t, router.routeRemote(method))
 	}
 
-	for _, method := range upstreamMethods {
-		require.True(t, router.routeLocally(method))
+	for _, method := range localMethods {
+		require.True(t, router.routeRemote(method))
 	}
 }