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

Implement new transaction replacement mechanism when adding transactions to the pool. #906

Closed
AbdelStark opened this issue May 12, 2020 · 4 comments · Fixed by #930
Closed
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed

Comments

@AbdelStark
Copy link
Contributor

AbdelStark commented May 12, 2020

Description

We should implement a new transaction replacement mechanism, similar to other client implementations.
Currently a new transaction (same user and same nonce) replaces an old one if the new gas price is higher than the old one. Other client implementations require the delta to be higher than a minimum percentage.
For instance this option is called pricebump in Geth implementation, configurable using --txpool.pricebump . This command line option is described as Price bump percentage to replace an already existing transaction and is defaulted to 10%.

Acceptance criteria

  • New replacement mechanism is implemented
  • Used for remote transactions received via P2P
  • Used for local transactions received via eth_sendRawTransaction

Linked issues

@AbdelStark AbdelStark added enhancement New feature or request good first issue Good for newcomers labels May 12, 2020
@timbeiko timbeiko added the help wanted Extra attention is needed label May 12, 2020
@sambacha
Copy link

would the existing transaction replacement mechanism still be available for non-PoW Besu clients?

@AbdelStark
Copy link
Contributor Author

would the existing transaction replacement mechanism still be available for non-PoW Besu clients?

Good question. Actually we have multiple options:

  • Set the price bump to 0 by default, this would behave like the existing mechanism.
  • Set the price bump to 10 by default to align with default behaviour of other mainnet clients, non-PoW Besu client would have to explicitly set it to 0 via CLI option.
  • Set the default price bump depending on the consensus engine.

I will raise the point and see what other contributors think about it. Do you have a personal preference ?

@brianmcgee
Copy link

Setting a sane default based on the consensus engine would IMHO reduce the likelihood of any surprises for not much effort.

@AbdelStark
Copy link
Contributor Author

Setting a sane default based on the consensus engine would IMHO reduce the likelihood of any surprises for not much effort.

@brianmcgee Thanks for the feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants