Skip to content

Commit

Permalink
internal/ethapi: ethSendTransaction check baseFee (ethereum#27834)
Browse files Browse the repository at this point in the history
  • Loading branch information
gzliudan committed Sep 12, 2024
1 parent 27ca506 commit 42fac60
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 12 deletions.
35 changes: 25 additions & 10 deletions internal/ethapi/transaction_args.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
27 changes: 25 additions & 2 deletions internal/ethapi/transaction_args_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package ethapi

import (
"context"
"errors"
"fmt"
"math/big"
"reflect"
Expand Down Expand Up @@ -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}}}}
Expand All @@ -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
{
Expand Down Expand Up @@ -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",
Expand All @@ -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
{
Expand Down

0 comments on commit 42fac60

Please sign in to comment.