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

New option tx-pool-min-gas-price to set a lower bound when accepting txs to the pool #6098

Merged
merged 16 commits into from
Nov 20, 2023

Conversation

fab-10
Copy link
Contributor

@fab-10 fab-10 commented Oct 26, 2023

PR description

This PR, is related to #6097, and decouples the min gas price (existing min-gas-price option) used when selecting txs during block production, from the min gas price (new tx-pool-min-gas-price option) that is required when txs are added to the txpool, since as consequence of #6097, the former min gas price (min-gas-price) can be changed at runtime and using it for evaluation incoming txs in the txpool, could result in strange behavior, where we refuse txs just because their price is just less than the min-gas-price, but since the latter can be later lowered, the same txs could have been included in a block just after a while, for this we want to accept them in the pool.

Thus this new option (tx-pool-min-gas-price) is a static value to set only if you are sure that you never want to process txs with a gas price lower than it, so to exclude what you consider spam txs from polluting your txpool.
For that reason tx-pool-min-gas-price can only be less than equal to the start value of min-gas-price.

…tx to the pool

Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
@github-actions
Copy link

github-actions bot commented Oct 26, 2023

  • I thought about documentation and added the doc-change-required label to this PR if updates are required.
  • I thought about the changelog and included a changelog update if required.
  • If my PR includes database changes (e.g. KeyValueSegmentIdentifier) I have thought about compatibility and performed forwards and backwards compatibility tests

fab-10 added 11 commits October 27, 2023 15:34
…tx to the pool

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>
Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
# Conflicts:
#	besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java
#	ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/AbstractTransactionPoolTest.java
# Conflicts:
#	ethereum/blockcreation/src/test/java/org/hyperledger/besu/ethereum/blockcreation/LegacyFeeMarketBlockTransactionSelectorTest.java
Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
@fab-10 fab-10 added the doc-change-required Indicates an issue or PR that requires doc to be updated label Nov 9, 2023
@fab-10 fab-10 marked this pull request as ready for review November 15, 2023 10:49
@fab-10 fab-10 force-pushed the txpool-min-gas-price branch from fb7e306 to e0c1d5a Compare November 15, 2023 10:51
Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
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.

suggestion re wording, otherwise LGTM

fab-10 and others added 2 commits November 20, 2023 09:32
Co-authored-by: Sally MacFarlane <macfarla.github@gmail.com>
Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
# Conflicts:
#	ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/AbstractTransactionPoolTest.java
@fab-10 fab-10 enabled auto-merge (squash) November 20, 2023 11:00
@fab-10 fab-10 merged commit eaf5682 into hyperledger:main Nov 20, 2023
18 checks passed
@fab-10 fab-10 deleted the txpool-min-gas-price branch November 20, 2023 12:12
jflo pushed a commit to jflo/besu that referenced this pull request Nov 20, 2023
…txs to the pool (hyperledger#6098)

Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Co-authored-by: Sally MacFarlane <macfarla.github@gmail.com>
Signed-off-by: Justin Florentine <justin+github@florentine.us>
jflo pushed a commit to jflo/besu that referenced this pull request Dec 4, 2023
…txs to the pool (hyperledger#6098)

Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Co-authored-by: Sally MacFarlane <macfarla.github@gmail.com>
Signed-off-by: Justin Florentine <justin+github@florentine.us>
@alexandratran alexandratran removed the doc-change-required Indicates an issue or PR that requires doc to be updated label Dec 12, 2023
gfukushima pushed a commit to gfukushima/besu that referenced this pull request Dec 15, 2023
…txs to the pool (hyperledger#6098)

Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Co-authored-by: Sally MacFarlane <macfarla.github@gmail.com>
Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>
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.

3 participants