From 5c13012b5681218aec6cf6039349adb7308d58e0 Mon Sep 17 00:00:00 2001
From: Martin Holst Swende <martin@swende.se>
Date: Thu, 29 Jul 2021 14:00:06 +0200
Subject: [PATCH] accounts/external, internal/ethapi: fixes for London tx
signing (#23274)
Ticket #23273 found a flaw where we were unable to sign legacy-transactions
using the external signer, even if we're still on non-london network. That's
fixed in this PR.
Additionally, I found that even when supplying all parameters, it was impossible
to sign a london-transaction on an unsynched node. It's a pretty common usecase
that someone wants to sign a transaction using an unsynced 'vanilla' node,
providing all necessary data. Our setDefaults, however, insisted on checking the
current block against the config. This PR therefore adds a case, so that if both
MaxPriorityFeePerGas and MaxFeePerGas are provided, we accept them as given.
OBS This PR fixes a regression -- on current master, we are unable to sign a
london-transaction unless the node is synched, which may break scenarios where
geth (or clef) is used as a cold wallet.
Fixes #23273
---
accounts/external/backend.go | 9 ++--
internal/ethapi/transaction_args.go | 67 ++++++++++++++++-------------
2 files changed, 42 insertions(+), 34 deletions(-)
diff --git a/accounts/external/backend.go b/accounts/external/backend.go
index 4f6ca3996..f6715a84e 100644
--- a/accounts/external/backend.go
+++ b/accounts/external/backend.go
@@ -211,11 +211,14 @@ func (api *ExternalSigner) SignTx(account accounts.Account, tx *types.Transactio
To: to,
From: common.NewMixedcaseAddress(account.Address),
}
- if tx.GasFeeCap() != nil {
+ switch tx.Type() {
+ case types.LegacyTxType, types.AccessListTxType:
+ args.GasPrice = (*hexutil.Big)(tx.GasPrice())
+ case types.DynamicFeeTxType:
args.MaxFeePerGas = (*hexutil.Big)(tx.GasFeeCap())
args.MaxPriorityFeePerGas = (*hexutil.Big)(tx.GasTipCap())
- } else {
- args.GasPrice = (*hexutil.Big)(tx.GasPrice())
+ default:
+ return nil, fmt.Errorf("Unsupported tx type %d", tx.Type())
}
// We should request the default chain id that we're operating with
// (the chain we're executing on)
diff --git a/internal/ethapi/transaction_args.go b/internal/ethapi/transaction_args.go
index 1fbaaeacb..9076e49b2 100644
--- a/internal/ethapi/transaction_args.go
+++ b/internal/ethapi/transaction_args.go
@@ -80,40 +80,45 @@ func (args *TransactionArgs) setDefaults(ctx context.Context, b Backend) error {
}
// After london, default to 1559 unless gasPrice is set
head := b.CurrentHeader()
- if b.ChainConfig().IsLondon(head.Number) && args.GasPrice == nil {
- if args.MaxPriorityFeePerGas == nil {
- tip, err := b.SuggestGasTipCap(ctx)
- if err != nil {
- return err
+ // If user specifies both maxPriorityfee and maxFee, then we do not
+ // need to consult the chain for defaults. It's definitely a London tx.
+ if args.MaxPriorityFeePerGas == nil || args.MaxFeePerGas == nil {
+ // In this clause, user left some fields unspecified.
+ if b.ChainConfig().IsLondon(head.Number) && args.GasPrice == nil {
+ if args.MaxPriorityFeePerGas == nil {
+ tip, err := b.SuggestGasTipCap(ctx)
+ if err != nil {
+ return err
+ }
+ args.MaxPriorityFeePerGas = (*hexutil.Big)(tip)
}
- args.MaxPriorityFeePerGas = (*hexutil.Big)(tip)
- }
- if args.MaxFeePerGas == nil {
- gasFeeCap := new(big.Int).Add(
- (*big.Int)(args.MaxPriorityFeePerGas),
- new(big.Int).Mul(head.BaseFee, big.NewInt(2)),
- )
- args.MaxFeePerGas = (*hexutil.Big)(gasFeeCap)
- }
- if args.MaxFeePerGas.ToInt().Cmp(args.MaxPriorityFeePerGas.ToInt()) < 0 {
- return fmt.Errorf("maxFeePerGas (%v) < maxPriorityFeePerGas (%v)", args.MaxFeePerGas, args.MaxPriorityFeePerGas)
- }
- } else {
- if args.MaxFeePerGas != nil || args.MaxPriorityFeePerGas != nil {
- return errors.New("maxFeePerGas or maxPriorityFeePerGas specified but london is not active yet")
- }
- if args.GasPrice == nil {
- price, err := b.SuggestGasTipCap(ctx)
- if err != nil {
- return err
+ if args.MaxFeePerGas == nil {
+ gasFeeCap := new(big.Int).Add(
+ (*big.Int)(args.MaxPriorityFeePerGas),
+ new(big.Int).Mul(head.BaseFee, big.NewInt(2)),
+ )
+ args.MaxFeePerGas = (*hexutil.Big)(gasFeeCap)
+ }
+ if args.MaxFeePerGas.ToInt().Cmp(args.MaxPriorityFeePerGas.ToInt()) < 0 {
+ return fmt.Errorf("maxFeePerGas (%v) < maxPriorityFeePerGas (%v)", args.MaxFeePerGas, args.MaxPriorityFeePerGas)
+ }
+ } else {
+ if args.MaxFeePerGas != nil || args.MaxPriorityFeePerGas != nil {
+ return errors.New("maxFeePerGas or maxPriorityFeePerGas specified but london is not active yet")
}
- if b.ChainConfig().IsLondon(head.Number) {
- // 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)
+ if args.GasPrice == nil {
+ price, err := b.SuggestGasTipCap(ctx)
+ if err != nil {
+ return err
+ }
+ if b.ChainConfig().IsLondon(head.Number) {
+ // 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)
}
- args.GasPrice = (*hexutil.Big)(price)
}
}
if args.Value == nil {
--
GitLab