From 241b2b2b14c0c9e3758ec990e243789d4506e5ae Mon Sep 17 00:00:00 2001
From: Daniel Liu <liudaniel@qq.com>
Date: Fri, 14 Jun 2024 11:41:25 +0800
Subject: [PATCH] internal/ethapi: ethSendTransaction check baseFee (#27834)

---
 internal/ethapi/transaction_args.go      | 35 +++++++++++++++++-------
 internal/ethapi/transaction_args_test.go | 23 ++++++++++++++++
 2 files changed, 48 insertions(+), 10 deletions(-)

diff --git a/internal/ethapi/transaction_args.go b/internal/ethapi/transaction_args.go
index f23926592d2a..3c44d18ad4e9 100644
--- a/internal/ethapi/transaction_args.go
+++ b/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 london 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 London is active")
 		}
 		// London not active, set gas price.
 		price, err := b.SuggestGasTipCap(ctx)
diff --git a/internal/ethapi/transaction_args_test.go b/internal/ethapi/transaction_args_test.go
index c1e624f0486c..3676b8ec802f 100644
--- a/internal/ethapi/transaction_args_test.go
+++ b/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,6 +73,13 @@ 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,
@@ -78,6 +87,13 @@ func TestSetFeeDefaults(t *testing.T) {
 			&TransactionArgs{GasPrice: fortytwo},
 			nil,
 		},
+		{
+			"legacy tx post-London with zero price",
+			true,
+			&TransactionArgs{GasPrice: zero},
+			nil,
+			errors.New("gasPrice must be non-zero after london fork"),
+		},
 
 		// Access list txs
 		{
@@ -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
 		{