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.

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

Explain and improve price validation for London transactions during s… #4602

Merged
merged 8 commits into from
Nov 8, 2022

Conversation

fab-10
Copy link
Contributor

@fab-10 fab-10 commented Nov 4, 2022

…election

Signed-off-by: Fabio Di Fabio fabio.difabio@consensys.net

PR description

Explain and improve price validation for London transactions during block proposal selection.

For EIP1559 transactions, the price is dynamic and depends on network conditions, so we can
only calculate at this time the current minimum price the transaction is willing to pay
and if it is above the minimum accepted by the node.
If below we do not delete the transaction, since when we added the transaction to the pool,
we assured sure that the maxFeePerGas is >= of the minimum price accepted by the node
and so the price of the transaction could satisfy this rule in the future.

Also if a EIP1559 fails validation because its maxFeePerGas is below current baseFee, this should be considered a specific transient error, and not wrongly report INVALID_TRANSACTION_FORMAT.

Moreover this is applied only to remoteTransaction because local transactions are allowed to have gas price
below the configured minimum, so we need to track local senders and do not enforce the rule for them.

Also fixed an edge case when a local transaction is readded to the pool due to a block reorg, and it was wrongly
considered remote with the risk of being rejected due to low gas price.

Fixed Issue(s)

Documentation

  • I thought about documentation and added the doc-change-required label to this PR if
    updates are required.

Changelog

…election

Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
@fab-10 fab-10 marked this pull request as ready for review November 4, 2022 11:42
Copy link
Contributor

@shemnon shemnon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
@fab-10 fab-10 requested a review from macfarla November 7, 2022 08:15
Copy link
Contributor

@macfarla macfarla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
@fab-10 fab-10 enabled auto-merge (squash) November 8, 2022 08:48
@fab-10 fab-10 merged commit db9b623 into hyperledger:main Nov 8, 2022
@fab-10 fab-10 deleted the txselector-improvements branch November 8, 2022 09:48
macfarla pushed a commit to macfarla/besu that referenced this pull request Jan 10, 2023
…election (hyperledger#4602)

Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>
eum602 pushed a commit to lacchain/besu that referenced this pull request Nov 3, 2023
…election (hyperledger#4602)

Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants