From 7a0c18bd31eb4ce7e78cc9515556d52fc5aa66ab Mon Sep 17 00:00:00 2001 From: rjl493456442 Date: Thu, 8 Jul 2021 15:06:34 +0800 Subject: [PATCH 1/4] internal/ethapi: fix transaction APIs --- internal/ethapi/api.go | 25 ++++++++++++++----------- internal/ethapi/transaction_args.go | 4 ++-- 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index becfc2f5b011..c51a4e0f1ff6 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -448,14 +448,15 @@ func (s *PrivateAccountAPI) SignTransaction(ctx context.Context, args Transactio if args.Gas == nil { return nil, fmt.Errorf("gas not specified") } - if args.GasPrice == nil { - return nil, fmt.Errorf("gasPrice not specified") + if args.GasPrice == nil && (args.MaxFeePerGas == nil || args.MaxPriorityFeePerGas == nil) { + return nil, fmt.Errorf("either gasPrice or (maxFeePerGas and maxPriorityFeePerGas) is specified") } if args.Nonce == nil { return nil, fmt.Errorf("nonce not specified") } // Before actually sign the transaction, ensure the transaction fee is reasonable. - if err := checkTxFee(args.GasPrice.ToInt(), uint64(*args.Gas), s.b.RPCTxFeeCap()); err != nil { + tx := args.toTransaction() + if err := checkTxFee(tx.GasPrice(), tx.Gas(), s.b.RPCTxFeeCap()); err != nil { return nil, err } signed, err := s.signTransaction(ctx, &args, passwd) @@ -1697,8 +1698,9 @@ func (s *PublicTransactionPoolAPI) SendTransaction(ctx context.Context, args Tra return SubmitTransaction(ctx, s.b, signed) } -// FillTransaction fills the defaults (nonce, gas, gasPrice) on a given unsigned transaction, -// and returns it to the caller for further processing (signing + broadcast) +// FillTransaction fills the defaults (nonce, gas, gasPrice or 1559 stype fields) +// on a given unsigned transaction, and returns it to the caller for further +// processing (signing + broadcast). func (s *PublicTransactionPoolAPI) FillTransaction(ctx context.Context, args TransactionArgs) (*SignTransactionResult, error) { // Set some sanity defaults and terminate on failure if err := args.setDefaults(ctx, s.b); err != nil { @@ -1761,8 +1763,8 @@ func (s *PublicTransactionPoolAPI) SignTransaction(ctx context.Context, args Tra if args.Gas == nil { return nil, fmt.Errorf("gas not specified") } - if args.GasPrice == nil { - return nil, fmt.Errorf("gasPrice not specified") + if args.GasPrice == nil && (args.MaxPriorityFeePerGas == nil || args.MaxFeePerGas == nil) { + return nil, fmt.Errorf("either gasPrice or (maxFeePerGas and maxPriorityFeePerGas) is specified") } if args.Nonce == nil { return nil, fmt.Errorf("nonce not specified") @@ -1771,18 +1773,19 @@ func (s *PublicTransactionPoolAPI) SignTransaction(ctx context.Context, args Tra return nil, err } // Before actually sign the transaction, ensure the transaction fee is reasonable. - if err := checkTxFee(args.GasPrice.ToInt(), uint64(*args.Gas), s.b.RPCTxFeeCap()); err != nil { + tx := args.toTransaction() + if err := checkTxFee(tx.GasPrice(), tx.Gas(), s.b.RPCTxFeeCap()); err != nil { return nil, err } - tx, err := s.sign(args.from(), args.toTransaction()) + signed, err := s.sign(args.from(), tx) if err != nil { return nil, err } - data, err := tx.MarshalBinary() + data, err := signed.MarshalBinary() if err != nil { return nil, err } - return &SignTransactionResult{data, tx}, nil + return &SignTransactionResult{data, signed}, nil } // PendingTransactions returns the transactions that are in the transaction pool diff --git a/internal/ethapi/transaction_args.go b/internal/ethapi/transaction_args.go index 220eb22b7092..b8ee371126ba 100644 --- a/internal/ethapi/transaction_args.go +++ b/internal/ethapi/transaction_args.go @@ -49,7 +49,7 @@ type TransactionArgs struct { Data *hexutil.Bytes `json:"data"` Input *hexutil.Bytes `json:"input"` - // For non-legacy transactions + // Introduced by AccessListTxType transaction. AccessList *types.AccessList `json:"accessList,omitempty"` ChainID *hexutil.Big `json:"chainId,omitempty"` } @@ -108,7 +108,7 @@ func (args *TransactionArgs) setDefaults(ctx context.Context, b Backend) error { return err } if b.ChainConfig().IsLondon(head.Number) { - price.Add(price, head.BaseFee) + price.Add(price, new(big.Int).Mul(head.BaseFee, big.NewInt(2))) } args.GasPrice = (*hexutil.Big)(price) } From 32f5a6166a4822e8d6d90c388bb61f3abb947edc Mon Sep 17 00:00:00 2001 From: rjl493456442 Date: Thu, 8 Jul 2021 15:11:33 +0800 Subject: [PATCH 2/4] internal/ethapi: fix typo --- internal/ethapi/api.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index c51a4e0f1ff6..a91e0e3a5328 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -1698,7 +1698,7 @@ func (s *PublicTransactionPoolAPI) SendTransaction(ctx context.Context, args Tra return SubmitTransaction(ctx, s.b, signed) } -// FillTransaction fills the defaults (nonce, gas, gasPrice or 1559 stype fields) +// FillTransaction fills the defaults (nonce, gas, gasPrice or 1559 fields) // on a given unsigned transaction, and returns it to the caller for further // processing (signing + broadcast). func (s *PublicTransactionPoolAPI) FillTransaction(ctx context.Context, args TransactionArgs) (*SignTransactionResult, error) { From 3f83d9f38a40b1a646f26de0e346c98479fae5d3 Mon Sep 17 00:00:00 2001 From: rjl493456442 Date: Thu, 8 Jul 2021 18:43:14 +0800 Subject: [PATCH 3/4] internal/ethapi: address comments --- internal/ethapi/api.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index a91e0e3a5328..8383d18cc1ba 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -449,7 +449,7 @@ func (s *PrivateAccountAPI) SignTransaction(ctx context.Context, args Transactio return nil, fmt.Errorf("gas not specified") } if args.GasPrice == nil && (args.MaxFeePerGas == nil || args.MaxPriorityFeePerGas == nil) { - return nil, fmt.Errorf("either gasPrice or (maxFeePerGas and maxPriorityFeePerGas) is specified") + return nil, fmt.Errorf("missing gasPrice or maxFeePerGas/maxPriorityFeePerGas") } if args.Nonce == nil { return nil, fmt.Errorf("nonce not specified") @@ -1764,7 +1764,7 @@ func (s *PublicTransactionPoolAPI) SignTransaction(ctx context.Context, args Tra return nil, fmt.Errorf("gas not specified") } if args.GasPrice == nil && (args.MaxPriorityFeePerGas == nil || args.MaxFeePerGas == nil) { - return nil, fmt.Errorf("either gasPrice or (maxFeePerGas and maxPriorityFeePerGas) is specified") + return nil, fmt.Errorf("missing gasPrice or maxFeePerGas/maxPriorityFeePerGas") } if args.Nonce == nil { return nil, fmt.Errorf("nonce not specified") From 78bda097b336482dd9b2031f778269f9ae33c18e Mon Sep 17 00:00:00 2001 From: rjl493456442 Date: Tue, 13 Jul 2021 17:07:14 +0800 Subject: [PATCH 4/4] internal/ethapi: address comment from Peter --- internal/ethapi/transaction_args.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/internal/ethapi/transaction_args.go b/internal/ethapi/transaction_args.go index b8ee371126ba..1fbaaeacbf06 100644 --- a/internal/ethapi/transaction_args.go +++ b/internal/ethapi/transaction_args.go @@ -108,7 +108,10 @@ func (args *TransactionArgs) setDefaults(ctx context.Context, b Backend) error { return err } if b.ChainConfig().IsLondon(head.Number) { - price.Add(price, new(big.Int).Mul(head.BaseFee, big.NewInt(2))) + // The legacy tx gas price suggestion should not add 2x base fee + // because all fees are consumed, so it would result in a spiral + // upwards. + price.Add(price, head.BaseFee) } args.GasPrice = (*hexutil.Big)(price) }