From 9b566d9defa52a4f072aa7550ed08f7f8cfb02d1 Mon Sep 17 00:00:00 2001 From: Adrian Sutton Date: Mon, 13 Feb 2023 10:01:12 +1000 Subject: [PATCH 1/2] core/types,internal/ethapi: Add tests for round tripping transaction JSON Checks that Transaction.MarshalJSON and newRPCTransaction can be parsed by Transaction.UnmarshalJSON --- core/types/transaction_marshalling_test.go | 127 ++++++++++++++++++++ internal/ethapi/api_test.go | 131 +++++++++++++++++++++ 2 files changed, 258 insertions(+) create mode 100644 core/types/transaction_marshalling_test.go create mode 100644 internal/ethapi/api_test.go diff --git a/core/types/transaction_marshalling_test.go b/core/types/transaction_marshalling_test.go new file mode 100644 index 000000000000..3576152e11d5 --- /dev/null +++ b/core/types/transaction_marshalling_test.go @@ -0,0 +1,127 @@ +package types + +import ( + "math/big" + "testing" + + "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/crypto" + "github.com/ethereum/go-ethereum/params" + "github.com/stretchr/testify/require" +) + +func TestTransaction_RoundTripJSON(t *testing.T) { + addr := common.HexToAddress("0x1234") + config := params.AllEthashProtocolChanges + signer := LatestSigner(config) + key, _ := crypto.HexToECDSA("b71c71a67e1177ad4e901695e1b4b9ee17ae16c6668d313eac2f96dbcda3f291") + + tests := allTransactionTypes(addr, config) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tx, err := SignNewTx(key, signer, tt.inner) + require.NoError(t, err, "signing failed: %v", err) + data, err := tx.MarshalJSON() + require.NoError(t, err, "marshal failed: %v", err) + + got := &Transaction{} + err = got.UnmarshalJSON(data) + require.NoError(t, err, "unmarshal failed: %v", err) + + require.Equal(t, tx.Hash(), got.Hash(), "transaction changed after round trip") + }) + } +} + +func allTransactionTypes(addr common.Address, config *params.ChainConfig) []struct { + name string + inner TxData +} { + return []struct { + name string + inner TxData + }{ + { + name: "LegacyTx", + inner: &LegacyTx{ + Nonce: 5, + GasPrice: big.NewInt(6), + Gas: 7, + To: &addr, + Value: big.NewInt(8), + Data: []byte{0, 1, 2, 3, 4}, + V: big.NewInt(9), + R: big.NewInt(10), + S: big.NewInt(11), + }, + }, + { + name: "LegacyTxContractCreation", + inner: &LegacyTx{ + Nonce: 5, + GasPrice: big.NewInt(6), + Gas: 7, + To: nil, + Value: big.NewInt(8), + Data: []byte{0, 1, 2, 3, 4}, + V: big.NewInt(32), + R: big.NewInt(10), + S: big.NewInt(11), + }, + }, + { + name: "AccessListTx", + inner: &AccessListTx{ + ChainID: config.ChainID, + Nonce: 5, + GasPrice: big.NewInt(6), + Gas: 7, + To: &addr, + Value: big.NewInt(8), + Data: []byte{0, 1, 2, 3, 4}, + AccessList: AccessList{}, + }, + }, + { + name: "AccessListTxContractCreation", + inner: &AccessListTx{ + ChainID: config.ChainID, + Nonce: 5, + GasPrice: big.NewInt(6), + Gas: 7, + To: nil, + Value: big.NewInt(8), + Data: []byte{0, 1, 2, 3, 4}, + AccessList: AccessList{}, + }, + }, + { + name: "DynamicFeeTx", + inner: &DynamicFeeTx{ + ChainID: config.ChainID, + Nonce: 5, + GasTipCap: big.NewInt(6), + GasFeeCap: big.NewInt(9), + Gas: 7, + To: &addr, + Value: big.NewInt(8), + Data: []byte{0, 1, 2, 3, 4}, + AccessList: AccessList{}, + }, + }, + { + name: "DynamicFeeTxContractCreation", + inner: &DynamicFeeTx{ + ChainID: config.ChainID, + Nonce: 5, + GasTipCap: big.NewInt(6), + GasFeeCap: big.NewInt(9), + Gas: 7, + To: nil, + Value: big.NewInt(8), + Data: []byte{0, 1, 2, 3, 4}, + AccessList: AccessList{}, + }, + }, + } +} diff --git a/internal/ethapi/api_test.go b/internal/ethapi/api_test.go new file mode 100644 index 000000000000..3cf8735ecd53 --- /dev/null +++ b/internal/ethapi/api_test.go @@ -0,0 +1,131 @@ +package ethapi + +import ( + "encoding/json" + "math/big" + "testing" + + "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/core/types" + "github.com/ethereum/go-ethereum/crypto" + "github.com/ethereum/go-ethereum/params" + "github.com/stretchr/testify/require" +) + +func TestTransaction_RoundTripRpcJSON(t *testing.T) { + addr := common.HexToAddress("0x1234") + config := params.AllEthashProtocolChanges + signer := types.LatestSigner(config) + key, _ := crypto.HexToECDSA("b71c71a67e1177ad4e901695e1b4b9ee17ae16c6668d313eac2f96dbcda3f291") + + tests := allTransactionTypes(addr, config) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tx, err := types.SignNewTx(key, signer, tt.inner) + require.NoError(t, err, "signing failed: %v", err) + rpcTx := newRPCTransaction(tx, common.Hash{}, 1234, 9, big.NewInt(10), ¶ms.ChainConfig{}) + require.NoError(t, err, "newRPCTransaction failed: %v", err) + data, err := json.Marshal(rpcTx) + require.NoError(t, err, "marshal failed: %v", err) + + got := &types.Transaction{} + err = got.UnmarshalJSON(data) + require.NoError(t, err, "unmarshal failed: %v", err) + + require.Equal(t, rpcTx.Hash, got.Hash(), "transaction changed after round trip") + }) + } +} + +func allTransactionTypes(addr common.Address, config *params.ChainConfig) []struct { + name string + inner types.TxData +} { + return []struct { + name string + inner types.TxData + }{ + { + name: "LegacyTx", + inner: &types.LegacyTx{ + Nonce: 5, + GasPrice: big.NewInt(6), + Gas: 7, + To: &addr, + Value: big.NewInt(8), + Data: []byte{0, 1, 2, 3, 4}, + V: big.NewInt(9), + R: big.NewInt(10), + S: big.NewInt(11), + }, + }, + { + name: "LegacyTxContractCreation", + inner: &types.LegacyTx{ + Nonce: 5, + GasPrice: big.NewInt(6), + Gas: 7, + To: nil, + Value: big.NewInt(8), + Data: []byte{0, 1, 2, 3, 4}, + V: big.NewInt(32), + R: big.NewInt(10), + S: big.NewInt(11), + }, + }, + { + name: "AccessListTx", + inner: &types.AccessListTx{ + ChainID: config.ChainID, + Nonce: 5, + GasPrice: big.NewInt(6), + Gas: 7, + To: &addr, + Value: big.NewInt(8), + Data: []byte{0, 1, 2, 3, 4}, + AccessList: types.AccessList{}, + }, + }, + { + name: "AccessListTxContractCreation", + inner: &types.AccessListTx{ + ChainID: config.ChainID, + Nonce: 5, + GasPrice: big.NewInt(6), + Gas: 7, + To: nil, + Value: big.NewInt(8), + Data: []byte{0, 1, 2, 3, 4}, + AccessList: types.AccessList{}, + }, + }, + { + name: "DynamicFeeTx", + inner: &types.DynamicFeeTx{ + ChainID: config.ChainID, + Nonce: 5, + GasTipCap: big.NewInt(6), + GasFeeCap: big.NewInt(9), + Gas: 7, + To: &addr, + Value: big.NewInt(8), + Data: []byte{0, 1, 2, 3, 4}, + AccessList: types.AccessList{}, + }, + }, + { + name: "DynamicFeeTxContractCreation", + inner: &types.DynamicFeeTx{ + ChainID: config.ChainID, + Nonce: 5, + GasTipCap: big.NewInt(6), + GasFeeCap: big.NewInt(9), + Gas: 7, + To: nil, + Value: big.NewInt(8), + Data: []byte{0, 1, 2, 3, 4}, + AccessList: types.AccessList{}, + }, + }, + } +} From 8a89ac9c8e989692d2aad80b52d4c32c00d26542 Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Thu, 23 Feb 2023 08:56:28 +0100 Subject: [PATCH 2/2] internal/ethapi: refactor json marshalling test --- core/types/transaction_marshalling_test.go | 127 ----------- internal/ethapi/api_test.go | 232 ++++++++++++--------- 2 files changed, 130 insertions(+), 229 deletions(-) delete mode 100644 core/types/transaction_marshalling_test.go diff --git a/core/types/transaction_marshalling_test.go b/core/types/transaction_marshalling_test.go deleted file mode 100644 index 3576152e11d5..000000000000 --- a/core/types/transaction_marshalling_test.go +++ /dev/null @@ -1,127 +0,0 @@ -package types - -import ( - "math/big" - "testing" - - "github.com/ethereum/go-ethereum/common" - "github.com/ethereum/go-ethereum/crypto" - "github.com/ethereum/go-ethereum/params" - "github.com/stretchr/testify/require" -) - -func TestTransaction_RoundTripJSON(t *testing.T) { - addr := common.HexToAddress("0x1234") - config := params.AllEthashProtocolChanges - signer := LatestSigner(config) - key, _ := crypto.HexToECDSA("b71c71a67e1177ad4e901695e1b4b9ee17ae16c6668d313eac2f96dbcda3f291") - - tests := allTransactionTypes(addr, config) - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - tx, err := SignNewTx(key, signer, tt.inner) - require.NoError(t, err, "signing failed: %v", err) - data, err := tx.MarshalJSON() - require.NoError(t, err, "marshal failed: %v", err) - - got := &Transaction{} - err = got.UnmarshalJSON(data) - require.NoError(t, err, "unmarshal failed: %v", err) - - require.Equal(t, tx.Hash(), got.Hash(), "transaction changed after round trip") - }) - } -} - -func allTransactionTypes(addr common.Address, config *params.ChainConfig) []struct { - name string - inner TxData -} { - return []struct { - name string - inner TxData - }{ - { - name: "LegacyTx", - inner: &LegacyTx{ - Nonce: 5, - GasPrice: big.NewInt(6), - Gas: 7, - To: &addr, - Value: big.NewInt(8), - Data: []byte{0, 1, 2, 3, 4}, - V: big.NewInt(9), - R: big.NewInt(10), - S: big.NewInt(11), - }, - }, - { - name: "LegacyTxContractCreation", - inner: &LegacyTx{ - Nonce: 5, - GasPrice: big.NewInt(6), - Gas: 7, - To: nil, - Value: big.NewInt(8), - Data: []byte{0, 1, 2, 3, 4}, - V: big.NewInt(32), - R: big.NewInt(10), - S: big.NewInt(11), - }, - }, - { - name: "AccessListTx", - inner: &AccessListTx{ - ChainID: config.ChainID, - Nonce: 5, - GasPrice: big.NewInt(6), - Gas: 7, - To: &addr, - Value: big.NewInt(8), - Data: []byte{0, 1, 2, 3, 4}, - AccessList: AccessList{}, - }, - }, - { - name: "AccessListTxContractCreation", - inner: &AccessListTx{ - ChainID: config.ChainID, - Nonce: 5, - GasPrice: big.NewInt(6), - Gas: 7, - To: nil, - Value: big.NewInt(8), - Data: []byte{0, 1, 2, 3, 4}, - AccessList: AccessList{}, - }, - }, - { - name: "DynamicFeeTx", - inner: &DynamicFeeTx{ - ChainID: config.ChainID, - Nonce: 5, - GasTipCap: big.NewInt(6), - GasFeeCap: big.NewInt(9), - Gas: 7, - To: &addr, - Value: big.NewInt(8), - Data: []byte{0, 1, 2, 3, 4}, - AccessList: AccessList{}, - }, - }, - { - name: "DynamicFeeTxContractCreation", - inner: &DynamicFeeTx{ - ChainID: config.ChainID, - Nonce: 5, - GasTipCap: big.NewInt(6), - GasFeeCap: big.NewInt(9), - Gas: 7, - To: nil, - Value: big.NewInt(8), - Data: []byte{0, 1, 2, 3, 4}, - AccessList: AccessList{}, - }, - }, - } -} diff --git a/internal/ethapi/api_test.go b/internal/ethapi/api_test.go index 3cf8735ecd53..762dc8337d30 100644 --- a/internal/ethapi/api_test.go +++ b/internal/ethapi/api_test.go @@ -1,3 +1,19 @@ +// Copyright 2023 The go-ethereum Authors +// This file is part of the go-ethereum library. +// +// The go-ethereum library is free software: you can redistribute it and/or modify +// it under the terms of the GNU Lesser General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// The go-ethereum library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Lesser General Public License for more details. +// +// You should have received a copy of the GNU Lesser General Public License +// along with the go-ethereum library. If not, see . + package ethapi import ( @@ -9,123 +25,135 @@ import ( "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/crypto" "github.com/ethereum/go-ethereum/params" - "github.com/stretchr/testify/require" ) func TestTransaction_RoundTripRpcJSON(t *testing.T) { - addr := common.HexToAddress("0x1234") - config := params.AllEthashProtocolChanges - signer := types.LatestSigner(config) - key, _ := crypto.HexToECDSA("b71c71a67e1177ad4e901695e1b4b9ee17ae16c6668d313eac2f96dbcda3f291") - - tests := allTransactionTypes(addr, config) - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - tx, err := types.SignNewTx(key, signer, tt.inner) - require.NoError(t, err, "signing failed: %v", err) - rpcTx := newRPCTransaction(tx, common.Hash{}, 1234, 9, big.NewInt(10), ¶ms.ChainConfig{}) - require.NoError(t, err, "newRPCTransaction failed: %v", err) - data, err := json.Marshal(rpcTx) - require.NoError(t, err, "marshal failed: %v", err) - - got := &types.Transaction{} - err = got.UnmarshalJSON(data) - require.NoError(t, err, "unmarshal failed: %v", err) + var ( + config = params.AllEthashProtocolChanges + signer = types.LatestSigner(config) + key, _ = crypto.HexToECDSA("b71c71a67e1177ad4e901695e1b4b9ee17ae16c6668d313eac2f96dbcda3f291") + tests = allTransactionTypes(common.Address{0xde, 0xad}, config) + ) + t.Parallel() + for i, tt := range tests { + var tx2 types.Transaction + tx, err := types.SignNewTx(key, signer, tt) + if err != nil { + t.Fatalf("test %d: signing failed: %v", i, err) + } + // Regular transaction + if data, err := json.Marshal(tx); err != nil { + t.Fatalf("test %d: marshalling failed; %v", i, err) + } else if err = tx2.UnmarshalJSON(data); err != nil { + t.Fatalf("test %d: sunmarshal failed: %v", i, err) + } else if want, have := tx.Hash(), tx2.Hash(); want != have { + t.Fatalf("test %d: stx changed, want %x have %x", i, want, have) + } - require.Equal(t, rpcTx.Hash, got.Hash(), "transaction changed after round trip") - }) + // rpcTransaction + rpcTx := newRPCTransaction(tx, common.Hash{}, 0, 0, nil, config) + if data, err := json.Marshal(rpcTx); err != nil { + t.Fatalf("test %d: marshalling failed; %v", i, err) + } else if err = tx2.UnmarshalJSON(data); err != nil { + t.Fatalf("test %d: unmarshal failed: %v", i, err) + } else if want, have := tx.Hash(), tx2.Hash(); want != have { + t.Fatalf("test %d: tx changed, want %x have %x", i, want, have) + } } } -func allTransactionTypes(addr common.Address, config *params.ChainConfig) []struct { - name string - inner types.TxData -} { - return []struct { - name string - inner types.TxData - }{ - { - name: "LegacyTx", - inner: &types.LegacyTx{ - Nonce: 5, - GasPrice: big.NewInt(6), - Gas: 7, - To: &addr, - Value: big.NewInt(8), - Data: []byte{0, 1, 2, 3, 4}, - V: big.NewInt(9), - R: big.NewInt(10), - S: big.NewInt(11), - }, +func allTransactionTypes(addr common.Address, config *params.ChainConfig) []types.TxData { + return []types.TxData{ + &types.LegacyTx{ + Nonce: 5, + GasPrice: big.NewInt(6), + Gas: 7, + To: &addr, + Value: big.NewInt(8), + Data: []byte{0, 1, 2, 3, 4}, + V: big.NewInt(9), + R: big.NewInt(10), + S: big.NewInt(11), }, - { - name: "LegacyTxContractCreation", - inner: &types.LegacyTx{ - Nonce: 5, - GasPrice: big.NewInt(6), - Gas: 7, - To: nil, - Value: big.NewInt(8), - Data: []byte{0, 1, 2, 3, 4}, - V: big.NewInt(32), - R: big.NewInt(10), - S: big.NewInt(11), - }, + &types.LegacyTx{ + Nonce: 5, + GasPrice: big.NewInt(6), + Gas: 7, + To: nil, + Value: big.NewInt(8), + Data: []byte{0, 1, 2, 3, 4}, + V: big.NewInt(32), + R: big.NewInt(10), + S: big.NewInt(11), }, - { - name: "AccessListTx", - inner: &types.AccessListTx{ - ChainID: config.ChainID, - Nonce: 5, - GasPrice: big.NewInt(6), - Gas: 7, - To: &addr, - Value: big.NewInt(8), - Data: []byte{0, 1, 2, 3, 4}, - AccessList: types.AccessList{}, + &types.AccessListTx{ + ChainID: config.ChainID, + Nonce: 5, + GasPrice: big.NewInt(6), + Gas: 7, + To: &addr, + Value: big.NewInt(8), + Data: []byte{0, 1, 2, 3, 4}, + AccessList: types.AccessList{ + types.AccessTuple{ + Address: common.Address{0x2}, + StorageKeys: []common.Hash{types.EmptyRootHash}, + }, }, + V: big.NewInt(32), + R: big.NewInt(10), + S: big.NewInt(11), }, - { - name: "AccessListTxContractCreation", - inner: &types.AccessListTx{ - ChainID: config.ChainID, - Nonce: 5, - GasPrice: big.NewInt(6), - Gas: 7, - To: nil, - Value: big.NewInt(8), - Data: []byte{0, 1, 2, 3, 4}, - AccessList: types.AccessList{}, + &types.AccessListTx{ + ChainID: config.ChainID, + Nonce: 5, + GasPrice: big.NewInt(6), + Gas: 7, + To: nil, + Value: big.NewInt(8), + Data: []byte{0, 1, 2, 3, 4}, + AccessList: types.AccessList{ + types.AccessTuple{ + Address: common.Address{0x2}, + StorageKeys: []common.Hash{types.EmptyRootHash}, + }, }, + V: big.NewInt(32), + R: big.NewInt(10), + S: big.NewInt(11), }, - { - name: "DynamicFeeTx", - inner: &types.DynamicFeeTx{ - ChainID: config.ChainID, - Nonce: 5, - GasTipCap: big.NewInt(6), - GasFeeCap: big.NewInt(9), - Gas: 7, - To: &addr, - Value: big.NewInt(8), - Data: []byte{0, 1, 2, 3, 4}, - AccessList: types.AccessList{}, + &types.DynamicFeeTx{ + ChainID: config.ChainID, + Nonce: 5, + GasTipCap: big.NewInt(6), + GasFeeCap: big.NewInt(9), + Gas: 7, + To: &addr, + Value: big.NewInt(8), + Data: []byte{0, 1, 2, 3, 4}, + AccessList: types.AccessList{ + types.AccessTuple{ + Address: common.Address{0x2}, + StorageKeys: []common.Hash{types.EmptyRootHash}, + }, }, + V: big.NewInt(32), + R: big.NewInt(10), + S: big.NewInt(11), }, - { - name: "DynamicFeeTxContractCreation", - inner: &types.DynamicFeeTx{ - ChainID: config.ChainID, - Nonce: 5, - GasTipCap: big.NewInt(6), - GasFeeCap: big.NewInt(9), - Gas: 7, - To: nil, - Value: big.NewInt(8), - Data: []byte{0, 1, 2, 3, 4}, - AccessList: types.AccessList{}, - }, + &types.DynamicFeeTx{ + ChainID: config.ChainID, + Nonce: 5, + GasTipCap: big.NewInt(6), + GasFeeCap: big.NewInt(9), + Gas: 7, + To: nil, + Value: big.NewInt(8), + Data: []byte{0, 1, 2, 3, 4}, + AccessList: types.AccessList{}, + V: big.NewInt(32), + R: big.NewInt(10), + S: big.NewInt(11), }, } }