From 226c3cce9652b1f3eff225cb6c41138318fa0f40 Mon Sep 17 00:00:00 2001 From: Daniel Liu 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 | 27 ++++++++++++++++-- 2 files changed, 50 insertions(+), 12 deletions(-) diff --git a/internal/ethapi/transaction_args.go b/internal/ethapi/transaction_args.go index f23926592d2a..ca69427c7a1d 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 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) diff --git a/internal/ethapi/transaction_args_test.go b/internal/ethapi/transaction_args_test.go index c1e624f0486c..7e7f2499948b 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 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 {