Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

internal/ethapi: rework setDefaults for tx args so fee logic is separate #25197

Merged
merged 5 commits into from
Aug 11, 2022
+410 −50
Merged

internal/ethapi: rework setDefaults for tx args so fee logic is separate #25197

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
cfadaf6
legacy tx must be IsLondon(head.Number) is false.
Jun 29, 2022
55f0005
internal/ethapi: improve clarity of fee default logic in setDefaults
lightclient Jun 29, 2022
e30e0a7
internal/ethapi: rework setDefaults for tx args so fee logic is separate
lightclient Jun 29, 2022
b1b38e7
internal/ethapi: add additional context to test
lightclient Jun 29, 2022
9d13f8d
internal/ethapi: fix comment typo
lightclient Jun 30, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
118 changes: 68 additions & 50 deletions internal/ethapi/transaction_args.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,56 +75,8 @@ func (args *TransactionArgs) data() []byte {

// setDefaults fills in default values for unspecified tx fields.
func (args *TransactionArgs) setDefaults(ctx context.Context, b Backend) error {
if args.GasPrice != nil && (args.MaxFeePerGas != nil || args.MaxPriorityFeePerGas != nil) {
return errors.New("both gasPrice and (maxFeePerGas or maxPriorityFeePerGas) specified")
}
// After london, default to 1559 unless gasPrice is set
head := b.CurrentHeader()
// 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)
}
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 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)
}
}
} else {
// Both maxPriorityfee and maxFee set by caller. Sanity-check their internal relation
if args.MaxFeePerGas.ToInt().Cmp(args.MaxPriorityFeePerGas.ToInt()) < 0 {
return fmt.Errorf("maxFeePerGas (%v) < maxPriorityFeePerGas (%v)", args.MaxFeePerGas, args.MaxPriorityFeePerGas)
}
if err := args.setFeeDefaults(ctx, b); err != nil {
return err
}
if args.Value == nil {
args.Value = new(hexutil.Big)
Expand Down Expand Up @@ -172,6 +124,72 @@ func (args *TransactionArgs) setDefaults(ctx context.Context, b Backend) error {
return nil
}

// setFeeDefaults fills in default fee values for unspecified tx fields.
func (args *TransactionArgs) setFeeDefaults(ctx context.Context, b Backend) error {
// If both gasPrice and at least one of the EIP-1559 fee parameters are specified, error.
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.
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 {
return fmt.Errorf("maxFeePerGas (%v) < maxPriorityFeePerGas (%v)", args.MaxFeePerGas, args.MaxPriorityFeePerGas)
}
return nil
}
// Now attempt to fill in default value depending on whether London is active or not.
head := b.CurrentHeader()
if b.ChainConfig().IsLondon(head.Number) {
// 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")
}
// London not active, set gas price.
price, err := b.SuggestGasTipCap(ctx)
if err != nil {
return err
}
args.GasPrice = (*hexutil.Big)(price)
}
return nil
}

// setLondonFeeDefaults fills in reasonable default fee values for unspecified fields.
func (args *TransactionArgs) setLondonFeeDefaults(ctx context.Context, head *types.Header, b Backend) error {
// Set maxPriorityFeePerGas if it is missing.
if args.MaxPriorityFeePerGas == nil {
tip, err := b.SuggestGasTipCap(ctx)
if err != nil {
return err
}
args.MaxPriorityFeePerGas = (*hexutil.Big)(tip)
}
// Set maxFeePerGas if it is missing.
if args.MaxFeePerGas == nil {
// Set the max fee to be 2 times larger than the previous block's base fee.
// The additional slack allows the tx to not become invalidated if the base
// fee is rising.
val := new(big.Int).Add(
args.MaxPriorityFeePerGas.ToInt(),
new(big.Int).Mul(head.BaseFee, big.NewInt(2)),
)
args.MaxFeePerGas = (*hexutil.Big)(val)
}
// Both EIP-1559 fee parameters are now set; sanity check them.
if args.MaxFeePerGas.ToInt().Cmp(args.MaxPriorityFeePerGas.ToInt()) < 0 {
return fmt.Errorf("maxFeePerGas (%v) < maxPriorityFeePerGas (%v)", args.MaxFeePerGas, args.MaxPriorityFeePerGas)
}
return nil
}

// ToMessage converts the transaction arguments to the Message type used by the
// core evm. This method is used in calls and traces that do not require a real
// live transaction.
Expand Down
Loading