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

core: ignore basefee when comparing with pool gasprice in txpool #24080

Merged
merged 1 commit into from Dec 17, 2021
Merged

core: ignore basefee when comparing with pool gasprice in txpool #24080

merged 1 commit into from Dec 17, 2021

Conversation

XWJACK
Copy link
Contributor

@XWJACK XWJACK commented Dec 7, 2021

fix #24079

pool.gasPrice is config by pricelimit.

gasPrice: new(big.Int).SetUint64(config.PriceLimit),

baseFee will always changed by tx pool . so if you do that, users will not able to send tx below baseFee rather than pricelimit. highly recommended revert this pr #23855.

baseFee *big.Int // heap should always be re-sorted after baseFee is changed

image

#23855
#23837

@holiman holiman changed the title Fix transaction underpriced core: ignore basefee when comparing with pool gasprice in txpool Dec 14, 2021
pendingBaseFee := pool.priced.urgent.baseFee
if !local && tx.EffectiveGasTipIntCmp(pool.gasPrice, pendingBaseFee) < 0 {
// Drop non-local transactions under our own minimal accepted gas price or tip
if !local && tx.GasTipCapIntCmp(pool.gasPrice) < 0 {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it should be GasFeeCapIntCmp here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, not sure yet.

Copy link
Contributor

Choose a reason for hiding this comment

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

After some more discusssion, it seems just reverting to GasTipCapIntCmp is the best choice for now.

Copy link

Choose a reason for hiding this comment

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

WHY?I think tx.EffectiveGasTipIntCmp(pool.gasPrice, pendingBaseFee) < 0 Is better here.

@fjl fjl merged commit 8bbf83e into ethereum:master Dec 17, 2021
@fjl fjl removed the status:triage label Dec 17, 2021
@fjl fjl added this to the 1.10.14 milestone Dec 17, 2021
sidhujag pushed a commit to syscoin/go-ethereum that referenced this pull request Dec 19, 2021
JacekGlen pushed a commit to JacekGlen/go-ethereum that referenced this pull request May 26, 2022
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.

transaction underpriced
4 participants