tests: make transaction tests run again, fix #19033 (#19529)

* tests: make transaction tests run again, fix #19033

* tests: refactor transaction tests

tests/gen_tttransaction.go

@@ -1,95 +0,0 @@
-// Code generated by github.com/fjl/gencodec. DO NOT EDIT.
-
-package tests
-
-import (
-	"encoding/json"
-	"errors"
-	"math/big"
-
-	"github.com/ethereum/go-ethereum/common"
-	"github.com/ethereum/go-ethereum/common/hexutil"
-	"github.com/ethereum/go-ethereum/common/math"
-)
-
-var _ = (*ttTransactionMarshaling)(nil)
-
-func (t ttTransaction) MarshalJSON() ([]byte, error) {
-	type ttTransaction struct {
-		Data     hexutil.Bytes         `gencodec:"required"`
-		GasLimit math.HexOrDecimal64   `gencodec:"required"`
-		GasPrice *math.HexOrDecimal256 `gencodec:"required"`
-		Nonce    math.HexOrDecimal64   `gencodec:"required"`
-		Value    *math.HexOrDecimal256 `gencodec:"required"`
-		R        *math.HexOrDecimal256 `gencodec:"required"`
-		S        *math.HexOrDecimal256 `gencodec:"required"`
-		V        *math.HexOrDecimal256 `gencodec:"required"`
-		To       common.Address        `gencodec:"required"`
-	}
-	var enc ttTransaction
-	enc.Data = t.Data
-	enc.GasLimit = math.HexOrDecimal64(t.GasLimit)
-	enc.GasPrice = (*math.HexOrDecimal256)(t.GasPrice)
-	enc.Nonce = math.HexOrDecimal64(t.Nonce)
-	enc.Value = (*math.HexOrDecimal256)(t.Value)
-	enc.R = (*math.HexOrDecimal256)(t.R)
-	enc.S = (*math.HexOrDecimal256)(t.S)
-	enc.V = (*math.HexOrDecimal256)(t.V)
-	enc.To = t.To
-	return json.Marshal(&enc)
-}
-
-func (t *ttTransaction) UnmarshalJSON(input []byte) error {
-	type ttTransaction struct {
-		Data     *hexutil.Bytes        `gencodec:"required"`
-		GasLimit *math.HexOrDecimal64  `gencodec:"required"`
-		GasPrice *math.HexOrDecimal256 `gencodec:"required"`
-		Nonce    *math.HexOrDecimal64  `gencodec:"required"`
-		Value    *math.HexOrDecimal256 `gencodec:"required"`
-		R        *math.HexOrDecimal256 `gencodec:"required"`
-		S        *math.HexOrDecimal256 `gencodec:"required"`
-		V        *math.HexOrDecimal256 `gencodec:"required"`
-		To       *common.Address       `gencodec:"required"`
-	}
-	var dec ttTransaction
-	if err := json.Unmarshal(input, &dec); err != nil {
-		return err
-	}
-	if dec.Data == nil {
-		return errors.New("missing required field 'data' for ttTransaction")
-	}
-	t.Data = *dec.Data
-	if dec.GasLimit == nil {
-		return errors.New("missing required field 'gasLimit' for ttTransaction")
-	}
-	t.GasLimit = uint64(*dec.GasLimit)
-	if dec.GasPrice == nil {
-		return errors.New("missing required field 'gasPrice' for ttTransaction")
-	}
-	t.GasPrice = (*big.Int)(dec.GasPrice)
-	if dec.Nonce == nil {
-		return errors.New("missing required field 'nonce' for ttTransaction")
-	}
-	t.Nonce = uint64(*dec.Nonce)
-	if dec.Value == nil {
-		return errors.New("missing required field 'value' for ttTransaction")
-	}
-	t.Value = (*big.Int)(dec.Value)
-	if dec.R == nil {
-		return errors.New("missing required field 'r' for ttTransaction")
-	}
-	t.R = (*big.Int)(dec.R)
-	if dec.S == nil {
-		return errors.New("missing required field 's' for ttTransaction")
-	}
-	t.S = (*big.Int)(dec.S)
-	if dec.V == nil {
-		return errors.New("missing required field 'v' for ttTransaction")
-	}
-	t.V = (*big.Int)(dec.V)
-	if dec.To == nil {
-		return errors.New("missing required field 'to' for ttTransaction")
-	}
-	t.To = *dec.To
-	return nil
-}

tests/transaction_test.go

@@ -17,7 +17,6 @@
 package tests
 
 import (
-	"math/big"
 	"testing"
 
 	"github.com/ethereum/go-ethereum/params"
@@ -27,26 +26,27 @@ func TestTransaction(t *testing.T) {
 	t.Parallel()
 
 	txt := new(testMatcher)
-	txt.config(`^Homestead/`, params.ChainConfig{
+	// These can't be parsed, invalid hex in RLP
-		HomesteadBlock: big.NewInt(0),
+	txt.skipLoad("^ttWrongRLP/.*")
-	})
+	// We don't allow more than uint64 in gas amount
-	txt.config(`^EIP155/`, params.ChainConfig{
+	// This is a pseudo-consensus vulnerability, but not in practice
-		HomesteadBlock: big.NewInt(0),
+	// because of the gas limit
-		EIP150Block:    big.NewInt(0),
+	txt.skipLoad("^ttGasLimit/TransactionWithGasLimitxPriceOverflow.json")
-		EIP155Block:    big.NewInt(0),
+	// We _do_ allow more than uint64 in gas price, as opposed to the tests
-		EIP158Block:    big.NewInt(0),
+	// This is also not a concern, as long as tx.Cost() uses big.Int for
-		ChainID:        big.NewInt(1),
+	// calculating the final cozt
-	})
+	txt.skipLoad(".*TransactionWithGasPriceOverflow.*")
-	txt.config(`^Byzantium/`, params.ChainConfig{
-		HomesteadBlock: big.NewInt(0),
-		EIP150Block:    big.NewInt(0),
-		EIP155Block:    big.NewInt(0),
-		EIP158Block:    big.NewInt(0),
-		ByzantiumBlock: big.NewInt(0),
-	})
 
+	// The nonce is too large for uint64. Not a concern, it means geth won't
+	// accept transactions at a certain point in the distant future
+	txt.skipLoad("^ttNonce/TransactionWithHighNonce256.json")
+
+	// The value is larger than uint64, which according to the test is invalid.
+	// Geth accepts it, which is not a consensus issue since we use big.Int's
+	// internally to calculate the cost
+	txt.skipLoad("^ttValue/TransactionWithHighValueOverflow.json")
 	txt.walk(t, transactionTestDir, func(t *testing.T, name string, test *TransactionTest) {
-		cfg := txt.findConfig(name)
+		cfg := params.MainnetChainConfig
 		if err := txt.checkFailure(t, name, test.Run(cfg)); err != nil {
 			t.Error(err)
 		}

tests/transaction_test_util.go

@@ -17,14 +17,11 @@
 package tests
 
 import (
-	"bytes"
-	"errors"
 	"fmt"
-	"math/big"
 
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/common/hexutil"
-	"github.com/ethereum/go-ethereum/common/math"
+	"github.com/ethereum/go-ethereum/core"
 	"github.com/ethereum/go-ethereum/core/types"
 	"github.com/ethereum/go-ethereum/params"
 	"github.com/ethereum/go-ethereum/rlp"
@@ -32,101 +29,80 @@ import (
 
 // TransactionTest checks RLP decoding and sender derivation of transactions.
 type TransactionTest struct {
-	json ttJSON
+	RLP            hexutil.Bytes `json:"rlp"`
+	Byzantium      ttFork
+	Constantinople ttFork
+	EIP150         ttFork
+	EIP158         ttFork
+	Frontier       ttFork
+	Homestead      ttFork
 }
 
-type ttJSON struct {
+type ttFork struct {
-	BlockNumber math.HexOrDecimal64 `json:"blockNumber"`
+	Sender common.UnprefixedAddress `json:"sender"`
-	RLP         hexutil.Bytes       `json:"rlp"`
+	Hash   common.UnprefixedHash    `json:"hash"`
-	Sender      hexutil.Bytes       `json:"sender"`
-	Transaction *ttTransaction      `json:"transaction"`
-}
-
-//go:generate gencodec -type ttTransaction -field-override ttTransactionMarshaling -out gen_tttransaction.go
-
-type ttTransaction struct {
-	Data     []byte         `gencodec:"required"`
-	GasLimit uint64         `gencodec:"required"`
-	GasPrice *big.Int       `gencodec:"required"`
-	Nonce    uint64         `gencodec:"required"`
-	Value    *big.Int       `gencodec:"required"`
-	R        *big.Int       `gencodec:"required"`
-	S        *big.Int       `gencodec:"required"`
-	V        *big.Int       `gencodec:"required"`
-	To       common.Address `gencodec:"required"`
-}
-
-type ttTransactionMarshaling struct {
-	Data     hexutil.Bytes
-	GasLimit math.HexOrDecimal64
-	GasPrice *math.HexOrDecimal256
-	Nonce    math.HexOrDecimal64
-	Value    *math.HexOrDecimal256
-	R        *math.HexOrDecimal256
-	S        *math.HexOrDecimal256
-	V        *math.HexOrDecimal256
 }
 
 func (tt *TransactionTest) Run(config *params.ChainConfig) error {
-	tx := new(types.Transaction)
-	if err := rlp.DecodeBytes(tt.json.RLP, tx); err != nil {
-		if tt.json.Transaction == nil {
-			return nil
-		}
-		return fmt.Errorf("RLP decoding failed: %v", err)
-	}
-	// Check sender derivation.
-	signer := types.MakeSigner(config, new(big.Int).SetUint64(uint64(tt.json.BlockNumber)))
-	sender, err := types.Sender(signer, tx)
-	if err != nil {
-		return err
-	}
-	if sender != common.BytesToAddress(tt.json.Sender) {
-		return fmt.Errorf("Sender mismatch: got %x, want %x", sender, tt.json.Sender)
-	}
-	// Check decoded fields.
-	err = tt.json.Transaction.verify(signer, tx)
-	if tt.json.Sender == nil && err == nil {
-		return errors.New("field validations succeeded but should fail")
-	}
-	if tt.json.Sender != nil && err != nil {
-		return fmt.Errorf("field validations failed after RLP decoding: %s", err)
-	}
-	return nil
-}
 
-func (tt *ttTransaction) verify(signer types.Signer, tx *types.Transaction) error {
+	validateTx := func(rlpData hexutil.Bytes, signer types.Signer, isHomestead bool) (*common.Address, *common.Hash, error) {
-	if !bytes.Equal(tx.Data(), tt.Data) {
+		tx := new(types.Transaction)
-		return fmt.Errorf("Tx input data mismatch: got %x want %x", tx.Data(), tt.Data)
+		if err := rlp.DecodeBytes(rlpData, tx); err != nil {
-	}
+			return nil, nil, err
-	if tx.Gas() != tt.GasLimit {
-		return fmt.Errorf("GasLimit mismatch: got %d, want %d", tx.Gas(), tt.GasLimit)
-	}
-	if tx.GasPrice().Cmp(tt.GasPrice) != 0 {
-		return fmt.Errorf("GasPrice mismatch: got %v, want %v", tx.GasPrice(), tt.GasPrice)
-	}
-	if tx.Nonce() != tt.Nonce {
-		return fmt.Errorf("Nonce mismatch: got %v, want %v", tx.Nonce(), tt.Nonce)
-	}
-	v, r, s := tx.RawSignatureValues()
-	if r.Cmp(tt.R) != 0 {
-		return fmt.Errorf("R mismatch: got %v, want %v", r, tt.R)
-	}
-	if s.Cmp(tt.S) != 0 {
-		return fmt.Errorf("S mismatch: got %v, want %v", s, tt.S)
-	}
-	if v.Cmp(tt.V) != 0 {
-		return fmt.Errorf("V mismatch: got %v, want %v", v, tt.V)
-	}
-	if tx.To() == nil {
-		if tt.To != (common.Address{}) {
-			return fmt.Errorf("To mismatch when recipient is nil (contract creation): %x", tt.To)
 		}
-	} else if *tx.To() != tt.To {
+		sender, err := types.Sender(signer, tx)
-		return fmt.Errorf("To mismatch: got %x, want %x", *tx.To(), tt.To)
+		if err != nil {
+			return nil, nil, err
+		}
+		// Intrinsic gas
+		requiredGas, err := core.IntrinsicGas(tx.Data(), tx.To() == nil, isHomestead)
+		if err != nil {
+			return nil, nil, err
+		}
+		if requiredGas > tx.Gas() {
+			return nil, nil, fmt.Errorf("insufficient gas ( %d < %d )", tx.Gas(), requiredGas)
+		}
+		h := tx.Hash()
+		return &sender, &h, nil
 	}
-	if tx.Value().Cmp(tt.Value) != 0 {
+
-		return fmt.Errorf("Value mismatch: got %x, want %x", tx.Value(), tt.Value)
+	for _, testcase := range []struct {
+		name        string
+		signer      types.Signer
+		fork        ttFork
+		isHomestead bool
+	}{
+		{"Frontier", types.FrontierSigner{}, tt.Frontier, false},
+		{"Homestead", types.HomesteadSigner{}, tt.Homestead, true},
+		{"EIP150", types.HomesteadSigner{}, tt.EIP150, true},
+		{"EIP158", types.NewEIP155Signer(config.ChainID), tt.EIP158, true},
+		{"Byzantium", types.NewEIP155Signer(config.ChainID), tt.Byzantium, true},
+		{"Constantinople", types.NewEIP155Signer(config.ChainID), tt.Constantinople, true},
+	} {
+		sender, txhash, err := validateTx(tt.RLP, testcase.signer, testcase.isHomestead)
+
+		if testcase.fork.Sender == (common.UnprefixedAddress{}) {
+			if err == nil {
+				return fmt.Errorf("Expected error, got none (address %v)", sender.String())
+			}
+			continue
+		}
+		// Should resolve the right address
+		if err != nil {
+			return fmt.Errorf("Got error, expected none: %v", err)
+		}
+		if sender == nil {
+			return fmt.Errorf("sender was nil, should be %x", common.Address(testcase.fork.Sender))
+		}
+		if *sender != common.Address(testcase.fork.Sender) {
+			return fmt.Errorf("Sender mismatch: got %x, want %x", sender, testcase.fork.Sender)
+		}
+		if txhash == nil {
+			return fmt.Errorf("txhash was nil, should be %x", common.Hash(testcase.fork.Hash))
+		}
+		if *txhash != common.Hash(testcase.fork.Hash) {
+			return fmt.Errorf("Hash mismatch: got %x, want %x", *txhash, testcase.fork.Hash)
+		}
 	}
 	return nil
 }