Unverified Commit 5c13012b authored by Martin Holst Swende's avatar Martin Holst Swende Committed by GitHub

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 
parent 523866c2
...@@ -211,11 +211,14 @@ func (api *ExternalSigner) SignTx(account accounts.Account, tx *types.Transactio ...@@ -211,11 +211,14 @@ func (api *ExternalSigner) SignTx(account accounts.Account, tx *types.Transactio
To: to, To: to,
From: common.NewMixedcaseAddress(account.Address), 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.MaxFeePerGas = (*hexutil.Big)(tx.GasFeeCap())
args.MaxPriorityFeePerGas = (*hexutil.Big)(tx.GasTipCap()) args.MaxPriorityFeePerGas = (*hexutil.Big)(tx.GasTipCap())
} else { default:
args.GasPrice = (*hexutil.Big)(tx.GasPrice()) return nil, fmt.Errorf("Unsupported tx type %d", tx.Type())
} }
// We should request the default chain id that we're operating with // We should request the default chain id that we're operating with
// (the chain we're executing on) // (the chain we're executing on)
......
...@@ -80,40 +80,45 @@ func (args *TransactionArgs) setDefaults(ctx context.Context, b Backend) error { ...@@ -80,40 +80,45 @@ func (args *TransactionArgs) setDefaults(ctx context.Context, b Backend) error {
} }
// After london, default to 1559 unless gasPrice is set // After london, default to 1559 unless gasPrice is set
head := b.CurrentHeader() head := b.CurrentHeader()
if b.ChainConfig().IsLondon(head.Number) && args.GasPrice == nil { // If user specifies both maxPriorityfee and maxFee, then we do not
if args.MaxPriorityFeePerGas == nil { // need to consult the chain for defaults. It's definitely a London tx.
tip, err := b.SuggestGasTipCap(ctx) if args.MaxPriorityFeePerGas == nil || args.MaxFeePerGas == nil {
if err != nil { // In this clause, user left some fields unspecified.
return err 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(
if args.MaxFeePerGas == nil { (*big.Int)(args.MaxPriorityFeePerGas),
gasFeeCap := new(big.Int).Add( new(big.Int).Mul(head.BaseFee, big.NewInt(2)),
(*big.Int)(args.MaxPriorityFeePerGas), )
new(big.Int).Mul(head.BaseFee, big.NewInt(2)), args.MaxFeePerGas = (*hexutil.Big)(gasFeeCap)
) }
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)
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 {
} else { return errors.New("maxFeePerGas or maxPriorityFeePerGas specified but london is not active yet")
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 b.ChainConfig().IsLondon(head.Number) { if args.GasPrice == nil {
// The legacy tx gas price suggestion should not add 2x base fee price, err := b.SuggestGasTipCap(ctx)
// because all fees are consumed, so it would result in a spiral if err != nil {
// upwards. return err
price.Add(price, head.BaseFee) }
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 { if args.Value == nil {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment