diff --git a/geth/common/rpccall.go b/geth/common/rpccall.go index f42ca27cf..467fac834 100644 --- a/geth/common/rpccall.go +++ b/geth/common/rpccall.go @@ -50,14 +50,13 @@ func (r RPCCall) ParseToAddress() (gethcommon.Address, error) { return gethcommon.HexToAddress(to), nil } -// ParseData returns the bytes associated with the call. -func (r RPCCall) ParseData() hexutil.Bytes { +func (r RPCCall) parseDataField(fieldName string) hexutil.Bytes { params, ok := r.Params[0].(map[string]interface{}) if !ok { return hexutil.Bytes("0x") } - data, ok := params["data"].(string) + data, ok := params[fieldName].(string) if !ok { data = "0x" } @@ -70,6 +69,16 @@ func (r RPCCall) ParseData() hexutil.Bytes { return byteCode } +// ParseData returns the bytes associated with the call in the deprecated "data" field. +func (r RPCCall) ParseData() hexutil.Bytes { + return r.parseDataField("data") +} + +// ParseInput returns the bytes associated with the call. +func (r RPCCall) ParseInput() hexutil.Bytes { + return r.parseDataField("input") +} + // ParseValue returns the hex big associated with the call. // nolint: dupl func (r RPCCall) ParseValue() *hexutil.Big { @@ -150,12 +159,14 @@ func (r RPCCall) ToSendTxArgs() SendTxArgs { toAddr = gethcommon.HexToAddress("0x0") } - input := r.ParseData() + input := r.ParseInput() + data := r.ParseData() return SendTxArgs{ To: &toAddr, From: fromAddr, Value: r.ParseValue(), Input: input, + Data: data, Gas: r.ParseGas(), GasPrice: r.ParseGasPrice(), } diff --git a/geth/common/types.go b/geth/common/types.go index 0d2117ec2..10737dd1c 100644 --- a/geth/common/types.go +++ b/geth/common/types.go @@ -24,7 +24,8 @@ import ( // errors var ( - ErrDeprecatedMethod = errors.New("Method is depricated and will be removed in future release") + ErrDeprecatedMethod = errors.New("Method is depricated and will be removed in future release") + ErrInvalidSendTxArgs = errors.New("Transaction arguments are invalid (are both 'input' and 'data' fields used?)") ) // NodeManager defines expected methods for managing Status node @@ -105,7 +106,35 @@ type SendTxArgs struct { GasPrice *hexutil.Big `json:"gasPrice"` Value *hexutil.Big `json:"value"` Nonce *hexutil.Uint64 `json:"nonce"` - Input hexutil.Bytes `json:"input"` + // We keep both "input" and "data" for backward compatibility. + // "input" is a preferred field. + // see `vendor/github.com/ethereum/go-ethereum/internal/ethapi/api.go:1107` + Input hexutil.Bytes `json:"input"` + Data hexutil.Bytes `json:"data"` +} + +// Valid checks whether this structure is filled in correctly. +func (args SendTxArgs) Valid() bool { + // if at least one of the fields is empty, it is a valid struct + if isNilOrEmpty(args.Input) || isNilOrEmpty(args.Data) { + return true + } + + // we only allow both fields to present if they have the same data + return bytes.Equal(args.Input, args.Data) +} + +// GetInput returns either Input or Data field's value dependent on what is filled. +func (args SendTxArgs) GetInput() hexutil.Bytes { + if !isNilOrEmpty(args.Input) { + return args.Input + } + + return args.Data +} + +func isNilOrEmpty(bytes hexutil.Bytes) bool { + return bytes == nil || len(bytes) == 0 } // APIResponse generic response from API diff --git a/geth/common/types_test.go b/geth/common/types_test.go new file mode 100644 index 000000000..c34512fcc --- /dev/null +++ b/geth/common/types_test.go @@ -0,0 +1,33 @@ +package common + +import ( + "testing" + + "github.com/ethereum/go-ethereum/common/hexutil" + "github.com/stretchr/testify/assert" +) + +func TestSendTxArgsValidity(t *testing.T) { + // 1. If only data fields is set, valid and return data + + bytes1 := hexutil.Bytes([]byte{0xAA, 0xBB, 0xCC, 0xDD}) + bytes2 := hexutil.Bytes([]byte{0x00, 0x01, 0x02}) + + bytesEmpty := hexutil.Bytes([]byte{}) + + doSendTxValidityTest(t, SendTxArgs{}, true, nil) + doSendTxValidityTest(t, SendTxArgs{Input: bytes1}, true, bytes1) + doSendTxValidityTest(t, SendTxArgs{Data: bytes1}, true, bytes1) + doSendTxValidityTest(t, SendTxArgs{Input: bytes1, Data: bytes1}, true, bytes1) + doSendTxValidityTest(t, SendTxArgs{Input: bytes1, Data: bytes2}, false, nil) + doSendTxValidityTest(t, SendTxArgs{Input: bytes1, Data: bytesEmpty}, true, bytes1) + doSendTxValidityTest(t, SendTxArgs{Input: bytesEmpty, Data: bytes1}, true, bytes1) + doSendTxValidityTest(t, SendTxArgs{Input: bytesEmpty, Data: bytesEmpty}, true, bytesEmpty) +} + +func doSendTxValidityTest(t *testing.T, args SendTxArgs, expectValid bool, expectValue hexutil.Bytes) { + assert.Equal(t, expectValid, args.Valid(), "Valid() returned unexpected value") + if expectValid { + assert.Equal(t, expectValue, args.GetInput(), "GetInput() returned unexpected value") + } +} diff --git a/geth/transactions/txqueue_manager.go b/geth/transactions/txqueue_manager.go index 661fcf01a..8acf74e78 100644 --- a/geth/transactions/txqueue_manager.go +++ b/geth/transactions/txqueue_manager.go @@ -89,6 +89,9 @@ func (m *Manager) TransactionQueue() *queue.TxQueue { // QueueTransaction puts a transaction into the queue. func (m *Manager) QueueTransaction(tx *common.QueuedTx) error { + if !tx.Args.Valid() { + return common.ErrInvalidSendTxArgs + } to := "" if tx.Args.To != nil { to = tx.Args.To.Hex() @@ -213,6 +216,9 @@ func (m *Manager) completeTransaction(selectedAccount *account.SelectedExtKey, q nonce = localNonce } args := queuedTx.Args + if !args.Valid() { + return hash, common.ErrInvalidSendTxArgs + } gasPrice := (*big.Int)(args.GasPrice) if args.GasPrice == nil { ctx, cancel = context.WithTimeout(context.Background(), m.rpcCallTimeout) @@ -239,7 +245,7 @@ func (m *Manager) completeTransaction(selectedAccount *account.SelectedExtKey, q To: args.To, GasPrice: gasPrice, Value: value, - Data: args.Input, + Data: args.GetInput(), }) if err != nil { return hash, err @@ -260,7 +266,7 @@ func (m *Manager) completeTransaction(selectedAccount *account.SelectedExtKey, q "gasPrice", gasPrice, "value", value, ) - tx := types.NewTransaction(nonce, toAddr, value, gas, gasPrice, args.Input) + tx := types.NewTransaction(nonce, toAddr, value, gas, gasPrice, args.GetInput()) signedTx, err := types.SignTx(tx, types.NewEIP155Signer(chainID), selectedAccount.AccountKey.PrivateKey) if err != nil { return hash, err diff --git a/t/e2e/transactions/transactions_test.go b/t/e2e/transactions/transactions_test.go index c5bbd53b1..b35a35acd 100644 --- a/t/e2e/transactions/transactions_test.go +++ b/t/e2e/transactions/transactions_test.go @@ -25,6 +25,8 @@ import ( "github.com/stretchr/testify/suite" ) +type initFunc func([]byte, *common.SendTxArgs) + func TestTransactionsTestSuite(t *testing.T) { suite.Run(t, new(TransactionsTestSuite)) } @@ -139,8 +141,51 @@ func (s *TransactionsTestSuite) TestCallRPCSendTransactionUpstream() { s.Equal(`{"jsonrpc":"2.0","id":1,"result":"`+txHash.String()+`"}`, result) } -// FIXME(tiabc): Sometimes it fails due to "no suitable peers found". +// TestSendContractCompat tries to send transaction using the legacy "Data" +// field, which is supported for backward compatibility reasons. +func (s *TransactionsTestSuite) TestSendContractTxCompat() { + initFunc := func(byteCode []byte, args *common.SendTxArgs) { + args.Data = (hexutil.Bytes)(byteCode) + } + s.testSendContractTx(initFunc, nil, "") +} + +// TestSendContractCompat tries to send transaction using both the legacy +// "Data" and "Input" fields. Also makes sure that the error is returned if +// they have different values. +func (s *TransactionsTestSuite) TestSendContractTxCollision() { + // Scenario 1: Both fields are filled and have the same value, expect success + initFunc := func(byteCode []byte, args *common.SendTxArgs) { + args.Input = (hexutil.Bytes)(byteCode) + args.Data = (hexutil.Bytes)(byteCode) + } + s.testSendContractTx(initFunc, nil, "") + + // Scenario 2: Both fields are filled with different values, expect an error + inverted := func(source []byte) []byte { + inverse := make([]byte, len(source)) + copy(inverse, source) + for i, b := range inverse { + inverse[i] = b ^ 0xFF + } + return inverse + } + + initFunc2 := func(byteCode []byte, args *common.SendTxArgs) { + args.Input = (hexutil.Bytes)(byteCode) + args.Data = (hexutil.Bytes)(inverted(byteCode)) + } + s.testSendContractTx(initFunc2, common.ErrInvalidSendTxArgs, "expected error when invalid tx args are sent") +} + func (s *TransactionsTestSuite) TestSendContractTx() { + initFunc := func(byteCode []byte, args *common.SendTxArgs) { + args.Input = (hexutil.Bytes)(byteCode) + } + s.testSendContractTx(initFunc, nil, "") +} + +func (s *TransactionsTestSuite) testSendContractTx(setInputAndDataValue initFunc, expectedError error, expectedErrorDescription string) { s.StartTestBackend() defer s.StopTestBackend() @@ -208,13 +253,20 @@ func (s *TransactionsTestSuite) TestSendContractTx() { s.NoError(err) gas := uint64(params.DefaultGas) - txHashCheck, err := s.Backend.SendTransaction(context.TODO(), common.SendTxArgs{ + args := common.SendTxArgs{ From: account.FromAddress(TestConfig.Account1.Address), To: nil, // marker, contract creation is expected //Value: (*hexutil.Big)(new(big.Int).Mul(big.NewInt(1), gethcommon.Ether)), - Gas: (*hexutil.Uint64)(&gas), - Input: (hexutil.Bytes)(byteCode), - }) + Gas: (*hexutil.Uint64)(&gas), + } + + setInputAndDataValue(byteCode, &args) + txHashCheck, err := s.Backend.SendTransaction(context.TODO(), args) + + if expectedError != nil { + s.Equal(expectedError, err, expectedErrorDescription) + return + } s.NoError(err, "cannot send transaction") select {