internal/ethapi: ethSendTransaction check baseFee (ethereum#27834)

internal/ethapi/transaction_args.go

@@ -136,27 +136,42 @@ func (args *TransactionArgs) setFeeDefaults(ctx context.Context, b Backend) erro
 	if args.GasPrice != nil && (args.MaxFeePerGas != nil || args.MaxPriorityFeePerGas != nil) {
 		return errors.New("both gasPrice and (maxFeePerGas or maxPriorityFeePerGas) specified")
 	}
-	// If the tx has completely specified a fee mechanism, no default is needed. This allows users
-	// who are not yet synced past London to get defaults for other tx values. See
-	// https://github.com/ethereum/go-ethereum/pull/23274 for more information.
+	// If the tx has completely specified a fee mechanism, no default is needed.
+	// This allows users who are not yet synced past London to get defaults for
+	// other tx values. See https://github.com/ethereum/go-ethereum/pull/23274
+	// for more information.
 	eip1559ParamsSet := args.MaxFeePerGas != nil && args.MaxPriorityFeePerGas != nil
-	if (args.GasPrice != nil && !eip1559ParamsSet) || (args.GasPrice == nil && eip1559ParamsSet) {
-		// Sanity check the EIP-1559 fee parameters if present.
-		if args.GasPrice == nil && args.MaxFeePerGas.ToInt().Cmp(args.MaxPriorityFeePerGas.ToInt()) < 0 {
+
+	// Sanity check the EIP-1559 fee parameters if present.
+	if args.GasPrice == nil && eip1559ParamsSet {
+		if args.MaxFeePerGas.ToInt().Sign() == 0 {
+			return errors.New("maxFeePerGas must be non-zero")
+		}
+		if args.MaxFeePerGas.ToInt().Cmp(args.MaxPriorityFeePerGas.ToInt()) < 0 {
 			return fmt.Errorf("maxFeePerGas (%v) < maxPriorityFeePerGas (%v)", args.MaxFeePerGas, args.MaxPriorityFeePerGas)
 		}
-		return nil
+		return nil // No need to set anything, user already set MaxFeePerGas and MaxPriorityFeePerGas
 	}
-	// Now attempt to fill in default value depending on whether London is active or not.
+	// Sanity check the non-EIP-1559 fee parameters.
 	head := b.CurrentHeader()
-	if b.ChainConfig().IsEIP1559(head.Number) {
+	isEIP1559 := b.ChainConfig().IsEIP1559(head.Number)
+	if args.GasPrice != nil && !eip1559ParamsSet {
+		// Zero gas-price is not allowed after London fork
+		if args.GasPrice.ToInt().Sign() == 0 && isEIP1559 {
+			return errors.New("gasPrice must be non-zero after EIP-1559 fork")
+		}
+		return nil // No need to set anything, user already set GasPrice
+	}
+
+	// Now attempt to fill in default value depending on whether London is active or not.
+	if isEIP1559 {
 		// London is active, set maxPriorityFeePerGas and maxFeePerGas.
 		if err := args.setLondonFeeDefaults(ctx, head, b); err != nil {
 			return err
 		}
 	} else {
 		if args.MaxFeePerGas != nil || args.MaxPriorityFeePerGas != nil {
-			return fmt.Errorf("maxFeePerGas and maxPriorityFeePerGas are not valid before London is active")
+			return errors.New("maxFeePerGas and maxPriorityFeePerGas are not valid before EIP-1559 is active")
 		}
 		// London not active, set gas price.
 		price, err := b.SuggestGasTipCap(ctx)

internal/ethapi/transaction_args_test.go

@@ -18,6 +18,7 @@ package ethapi
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"math/big"
 	"reflect"
@@ -57,6 +58,7 @@ func TestSetFeeDefaults(t *testing.T) {
 
 	var (
 		b        = newBackendMock()
+		zero     = (*hexutil.Big)(big.NewInt(0))
 		fortytwo = (*hexutil.Big)(big.NewInt(42))
 		maxFee   = (*hexutil.Big)(new(big.Int).Add(new(big.Int).Mul(b.current.BaseFee, big.NewInt(2)), fortytwo.ToInt()))
 		al       = &types.AccessList{types.AccessTuple{Address: common.Address{0xaa}, StorageKeys: []common.Hash{{0x01}}}}
@@ -71,13 +73,27 @@ func TestSetFeeDefaults(t *testing.T) {
 			&TransactionArgs{GasPrice: fortytwo},
 			nil,
 		},
+		{
+			"legacy tx pre-London with zero price",
+			false,
+			&TransactionArgs{GasPrice: zero},
+			&TransactionArgs{GasPrice: zero},
+			nil,
+		},
 		{
 			"legacy tx post-London, explicit gas price",
 			true,
 			&TransactionArgs{GasPrice: fortytwo},
 			&TransactionArgs{GasPrice: fortytwo},
 			nil,
 		},
+		{
+			"legacy tx post-London with zero price",
+			true,
+			&TransactionArgs{GasPrice: zero},
+			nil,
+			errors.New("gasPrice must be non-zero after EIP-1559 fork"),
+		},
 
 		// Access list txs
 		{
@@ -143,14 +159,14 @@ func TestSetFeeDefaults(t *testing.T) {
 			false,
 			&TransactionArgs{MaxFeePerGas: maxFee},
 			nil,
-			fmt.Errorf("maxFeePerGas and maxPriorityFeePerGas are not valid before London is active"),
+			fmt.Errorf("maxFeePerGas and maxPriorityFeePerGas are not valid before EIP-1559 is active"),
 		},
 		{
 			"dynamic fee tx pre-London, priorityFee set",
 			false,
 			&TransactionArgs{MaxPriorityFeePerGas: fortytwo},
 			nil,
-			fmt.Errorf("maxFeePerGas and maxPriorityFeePerGas are not valid before London is active"),
+			fmt.Errorf("maxFeePerGas and maxPriorityFeePerGas are not valid before EIP-1559 is active"),
 		},
 		{
 			"dynamic fee tx, maxFee < priorityFee",
@@ -166,6 +182,13 @@ func TestSetFeeDefaults(t *testing.T) {
 			nil,
 			fmt.Errorf("maxFeePerGas (0x7) < maxPriorityFeePerGas (0x2a)"),
 		},
+		{
+			"dynamic fee tx post-London, explicit gas price",
+			true,
+			&TransactionArgs{MaxFeePerGas: zero, MaxPriorityFeePerGas: zero},
+			nil,
+			errors.New("maxFeePerGas must be non-zero"),
+		},
 
 		// Misc
 		{