From 4d00fa80b0bf859579a504dedbba9fc5e44c2893 Mon Sep 17 00:00:00 2001 From: Dmitry Shulyak Date: Sat, 7 Apr 2018 22:16:32 +0300 Subject: [PATCH] Ensure that To field is nil when contract is created --- geth/rpc/call.go | 145 ---------------------- geth/transactions/txqueue_manager.go | 86 +++++++------ geth/transactions/txqueue_manager_test.go | 39 +++++- t/e2e/transactions/transactions_test.go | 44 +++++++ 4 files changed, 123 insertions(+), 191 deletions(-) delete mode 100644 geth/rpc/call.go diff --git a/geth/rpc/call.go b/geth/rpc/call.go deleted file mode 100644 index 5aa61a9ff..000000000 --- a/geth/rpc/call.go +++ /dev/null @@ -1,145 +0,0 @@ -package rpc - -import ( - "errors" - - gethcommon "github.com/ethereum/go-ethereum/common" - "github.com/ethereum/go-ethereum/common/hexutil" -) - -// Call represents a unit of a rpc request which is to be executed. -type Call struct { - ID int64 - Method string - Params []interface{} -} - -// contains series of errors for parsing operations. -var ( - ErrInvalidFromAddress = errors.New("Failed to parse From Address") - ErrInvalidToAddress = errors.New("Failed to parse To Address") -) - -// ParseFromAddress returns the address associated with the Call. -func (c Call) ParseFromAddress() (gethcommon.Address, error) { - params, ok := c.Params[0].(map[string]interface{}) - if !ok { - return gethcommon.HexToAddress("0x"), ErrInvalidFromAddress - } - - from, ok := params["from"].(string) - if !ok { - return gethcommon.HexToAddress("0x"), ErrInvalidFromAddress - } - - return gethcommon.HexToAddress(from), nil -} - -// ParseToAddress returns the gethcommon.Address associated with the call. -func (c Call) ParseToAddress() (gethcommon.Address, error) { - params, ok := c.Params[0].(map[string]interface{}) - if !ok { - return gethcommon.HexToAddress("0x"), ErrInvalidToAddress - } - - to, ok := params["to"].(string) - if !ok { - return gethcommon.HexToAddress("0x"), ErrInvalidToAddress - } - - return gethcommon.HexToAddress(to), nil -} - -func (c Call) parseDataField(fieldName string) hexutil.Bytes { - params, ok := c.Params[0].(map[string]interface{}) - if !ok { - return hexutil.Bytes("0x") - } - - data, ok := params[fieldName].(string) - if !ok { - data = "0x" - } - - byteCode, err := hexutil.Decode(data) - if err != nil { - byteCode = hexutil.Bytes(data) - } - - return byteCode -} - -// ParseData returns the bytes associated with the call in the deprecated "data" field. -func (c Call) ParseData() hexutil.Bytes { - return c.parseDataField("data") -} - -// ParseInput returns the bytes associated with the call. -func (c Call) ParseInput() hexutil.Bytes { - return c.parseDataField("input") -} - -// ParseValue returns the hex big associated with the call. -// nolint: dupl -func (c Call) ParseValue() *hexutil.Big { - params, ok := c.Params[0].(map[string]interface{}) - if !ok { - return nil - //return (*hexutil.Big)(big.NewInt("0x0")) - } - - inputValue, ok := params["value"].(string) - if !ok { - return nil - } - - parsedValue, err := hexutil.DecodeBig(inputValue) - if err != nil { - return nil - } - - return (*hexutil.Big)(parsedValue) -} - -// ParseGas returns the hex big associated with the call. -// nolint: dupl -func (c Call) ParseGas() *hexutil.Uint64 { - params, ok := c.Params[0].(map[string]interface{}) - if !ok { - return nil - } - - inputValue, ok := params["gas"].(string) - if !ok { - return nil - } - - parsedValue, err := hexutil.DecodeUint64(inputValue) - if err != nil { - return nil - } - - v := hexutil.Uint64(parsedValue) - return &v -} - -// ParseGasPrice returns the hex big associated with the call. -// nolint: dupl -func (c Call) ParseGasPrice() *hexutil.Big { - params, ok := c.Params[0].(map[string]interface{}) - if !ok { - return nil - } - - inputValue, ok := params["gasPrice"].(string) - if !ok { - return nil - } - - parsedValue, err := hexutil.DecodeBig(inputValue) - if err != nil { - return nil - } - - return (*hexutil.Big)(parsedValue) -} diff --git a/geth/transactions/txqueue_manager.go b/geth/transactions/txqueue_manager.go index 0920b48de..e5168506b 100644 --- a/geth/transactions/txqueue_manager.go +++ b/geth/transactions/txqueue_manager.go @@ -2,12 +2,15 @@ package transactions import ( "context" + "encoding/json" + "errors" "math/big" "sync" "time" ethereum "github.com/ethereum/go-ethereum" gethcommon "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/crypto" "github.com/ethereum/go-ethereum/log" "github.com/ethereum/go-ethereum/core/types" @@ -26,6 +29,11 @@ const ( defaultTimeout = time.Minute ) +var ( + // ErrUnexpectedArgs returned when args are of unexpected length. + ErrUnexpectedArgs = errors.New("unexpected args") +) + // RPCClientProvider is an interface that provides a way // to obtain an rpc.Client. type RPCClientProvider interface { @@ -230,10 +238,6 @@ func (m *Manager) completeTransaction(selectedAccount *account.SelectedExtKey, q chainID := big.NewInt(int64(m.networkID)) value := (*big.Int)(args.Value) - toAddr := gethcommon.Address{} - if args.To != nil { - toAddr = *args.To - } var gas uint64 if args.Gas == nil { @@ -256,16 +260,27 @@ func (m *Manager) completeTransaction(selectedAccount *account.SelectedExtKey, q } else { gas = uint64(*args.Gas) } - - m.log.Info( - "preparing raw transaction", - "from", args.From.Hex(), - "to", toAddr.Hex(), - "gas", gas, - "gasPrice", gasPrice, - "value", value, - ) - tx := types.NewTransaction(nonce, toAddr, value, gas, gasPrice, args.GetInput()) + var tx *types.Transaction + if args.To != nil { + m.log.Info("New transaction", + "From", args.From, + "To", *args.To, + "Gas", gas, + "GasPrice", gasPrice, + "Value", value, + ) + tx = types.NewTransaction(nonce, *args.To, value, gas, gasPrice, args.GetInput()) + } else { + // contract creation is rare enough to log an expected address + m.log.Info("New contract", + "From", args.From, + "Gas", gas, + "GasPrice", gasPrice, + "Value", value, + "Contract address", crypto.CreateAddress(args.From, nonce), + ) + tx = types.NewContractCreation(nonce, value, gas, gasPrice, args.GetInput()) + } signedTx, err := types.SignTx(tx, types.NewEIP155Signer(chainID), selectedAccount.AccountKey.PrivateKey) if err != nil { return hash, err @@ -294,8 +309,19 @@ func (m *Manager) DiscardTransaction(id string) error { // SendTransactionRPCHandler is a handler for eth_sendTransaction method. // It accepts one param which is a slice with a map of transaction params. func (m *Manager) SendTransactionRPCHandler(ctx context.Context, args ...interface{}) (interface{}, error) { - m.log.Info("SendTransactionRPCHandler called") - tx := Create(ctx, m.rpcCalltoSendTxArgs(args...)) + m.log.Debug("SendTransactionRPCHandler called", "ARGS", args) + if len(args) != 1 { + return nil, ErrUnexpectedArgs + } + data, err := json.Marshal(args[0]) + if err != nil { + return nil, err + } + var txArgs SendTxArgs + if err := json.Unmarshal(data, &txArgs); err != nil { + return nil, err + } + tx := Create(ctx, txArgs) if err := m.QueueTransaction(tx); err != nil { return nil, err } @@ -305,31 +331,3 @@ func (m *Manager) SendTransactionRPCHandler(ctx context.Context, args ...interfa } return rst.Hash.Hex(), nil } - -func (m *Manager) rpcCalltoSendTxArgs(args ...interface{}) SendTxArgs { - var err error - var fromAddr, toAddr gethcommon.Address - - rpcCall := rpc.Call{Params: args} - fromAddr, err = rpcCall.ParseFromAddress() - if err != nil { - fromAddr = gethcommon.HexToAddress("0x0") - } - - toAddr, err = rpcCall.ParseToAddress() - if err != nil { - toAddr = gethcommon.HexToAddress("0x0") - } - - input := rpcCall.ParseInput() - data := rpcCall.ParseData() - return SendTxArgs{ - To: &toAddr, - From: fromAddr, - Value: rpcCall.ParseValue(), - Input: input, - Data: data, - Gas: rpcCall.ParseGas(), - GasPrice: rpcCall.ParseGasPrice(), - } -} diff --git a/geth/transactions/txqueue_manager_test.go b/geth/transactions/txqueue_manager_test.go index ad35a6ed1..98a2ac913 100644 --- a/geth/transactions/txqueue_manager_test.go +++ b/geth/transactions/txqueue_manager_test.go @@ -8,11 +8,15 @@ import ( "testing" "time" + "github.com/ethereum/go-ethereum/accounts/abi/bind/backends" "github.com/ethereum/go-ethereum/accounts/keystore" gethcommon "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/common/hexutil" + "github.com/ethereum/go-ethereum/contracts/ens/contract" + "github.com/ethereum/go-ethereum/core" "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/crypto" + gethparams "github.com/ethereum/go-ethereum/params" "github.com/ethereum/go-ethereum/rlp" gethrpc "github.com/ethereum/go-ethereum/rpc" "github.com/golang/mock/gomock" @@ -25,6 +29,10 @@ import ( . "github.com/status-im/status-go/t/utils" ) +func TestTxQueueTestSuite(t *testing.T) { + suite.Run(t, new(TxQueueTestSuite)) +} + type TxQueueTestSuite struct { suite.Suite rpcClientMockCtrl *gomock.Controller @@ -48,7 +56,9 @@ func (s *TxQueueTestSuite) SetupTest() { s.client = gethrpc.DialInProc(s.server) rpclient, _ := rpc.NewClient(s.client, params.UpstreamRPCConfig{}) s.rpcClientMock.EXPECT().RPCClient().Return(rpclient) - nodeConfig, err := params.NewNodeConfig("/tmp", "", params.RopstenNetworkID, true) + // expected by simulated backend + chainID := gethparams.AllEthashProtocolChanges.ChainId.Uint64() + nodeConfig, err := params.NewNodeConfig("/tmp", "", chainID, true) s.Require().NoError(err) s.nodeConfig = nodeConfig @@ -56,7 +66,7 @@ func (s *TxQueueTestSuite) SetupTest() { s.manager.DisableNotificactions() s.manager.completionTimeout = time.Second s.manager.rpcCallTimeout = time.Second - s.manager.Start(params.RopstenNetworkID) + s.manager.Start(chainID) } func (s *TxQueueTestSuite) TearDownTest() { @@ -345,3 +355,28 @@ func (s *TxQueueTestSuite) TestLocalNonce() { resultNonce, _ = s.manager.localNonce.Load(tx.Args.From) s.Equal(uint64(nonce)+1, resultNonce.(uint64)) } + +func (s *TxQueueTestSuite) TestContractCreation() { + key, _ := crypto.GenerateKey() + testaddr := crypto.PubkeyToAddress(key.PublicKey) + genesis := core.GenesisAlloc{ + testaddr: {Balance: big.NewInt(100000000000)}, + } + backend := backends.NewSimulatedBackend(genesis) + selectedAccount := &account.SelectedExtKey{ + Address: testaddr, + AccountKey: &keystore.Key{PrivateKey: key}, + } + s.manager.ethTxClient = backend + tx := Create(context.Background(), SendTxArgs{ + From: testaddr, + Input: hexutil.Bytes(gethcommon.FromHex(contract.ENSBin)), + }) + s.NoError(s.manager.QueueTransaction(tx)) + hash, err := s.manager.CompleteTransaction(tx.ID, selectedAccount) + s.NoError(err) + backend.Commit() + receipt, err := backend.TransactionReceipt(context.TODO(), hash) + s.NoError(err) + s.Equal(crypto.CreateAddress(testaddr, 0), receipt.ContractAddress) +} diff --git a/t/e2e/transactions/transactions_test.go b/t/e2e/transactions/transactions_test.go index 00a37cf85..327d28cbc 100644 --- a/t/e2e/transactions/transactions_test.go +++ b/t/e2e/transactions/transactions_test.go @@ -139,6 +139,50 @@ func (s *TransactionsTestSuite) TestCallRPCSendTransactionUpstream() { s.Equal(`{"jsonrpc":"2.0","id":1,"result":"`+txHash.String()+`"}`, result) } +func (s *TransactionsTestSuite) TestEmptyToFieldPreserved() { + s.StartTestBackend() + defer s.StopTestBackend() + + EnsureNodeSync(s.Backend.StatusNode()) + err := s.Backend.SelectAccount(TestConfig.Account1.Address, TestConfig.Account1.Password) + s.NoError(err) + + transactionCompleted := make(chan struct{}) + signal.SetDefaultNodeNotificationHandler(func(rawSignal string) { + var sg struct { + Type string + Event json.RawMessage + } + err := json.Unmarshal([]byte(rawSignal), &sg) + s.NoError(err) + if sg.Type == transactions.EventTransactionQueued { + var event transactions.SendTransactionEvent + s.NoError(json.Unmarshal(sg.Event, &event)) + s.NotNil(event.Args.From) + s.Nil(event.Args.To) + _, err := s.Backend.CompleteTransaction(event.ID, TestConfig.Account1.Password) + s.NoError(err) + close(transactionCompleted) + } + }) + + result := s.Backend.CallRPC(`{ + "jsonrpc": "2.0", + "id": 1, + "method": "eth_sendTransaction", + "params": [{ + "from": "` + TestConfig.Account1.Address + `" + }] + }`) + s.NotContains(result, "error") + + select { + case <-transactionCompleted: + case <-time.After(10 * time.Second): + s.FailNow("sending transaction timed out") + } +} + // TestSendContractCompat tries to send transaction using the legacy "Data" // field, which is supported for backward compatibility reasons. func (s *TransactionsTestSuite) TestSendContractTxCompat() {