-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Remove protocolBaseFee checks #8367
Conversation
@@ -1551,10 +1532,8 @@ func promote(pending *PendingPool, baseFee, queued *SubPool, pendingBaseFee uint | |||
tx := pending.PopWorst() | |||
announcements.Append(tx.Tx.Type, tx.Tx.Size, tx.Tx.IDHash[:]) | |||
baseFee.Add(tx, logger) | |||
} else if worst.subPool >= QueuedPoolBits { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don’t understand why this line removed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Popping off from the worst of Pending could potentially either get injected to baseFee subpool or queued subpool. The only check for queued subpool was that the txn is not impossible to include at a future time.
At this stage, this check was with >= QueuedPoolBits (= EnoughFeeCapProtocol), which has now been removed. All other bits could change in the future making txn eligible for inclusion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are you sure that QueuedPoolBits has only 1 bit?
Even if it’s true - still “bad txs need move to queued sub-pool” because maybe they will improve in future (sender may increase it’s balance, etc…)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes QueuedPoolBits only has 1 bit.
Moving worst txns down from pending and baseFee to Queued does happen still. Just that, discarding of txns from queued based on EnoughFeeCapProtocol won't be there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, make sense for me now
IsLocal = 0b000001 | ||
|
||
BaseFeePoolBits = EnoughFeeCapProtocol + NoNonceGaps + EnoughBalance + NotTooMuchGas | ||
QueuedPoolBits = EnoughFeeCapProtocol |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In my head: QueuedPoolBits=0 doesn’t mean that need remove it. We just reducing minimal requirements for queued sub-pool - it doesn’t make it useless - actually opposite - more txs will now go there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
EnoughFeeCapProtocol
was basically the first barrier to entry (because all txns first go to queued). The Subpool bits can never be less than 0. So, this barrier, QueuedPoolBits=0 is redundant.
The only requirement aside from txn validity was that the baseFee is at least greater than 7 wei for queued pool.
Now, any valid transaction could go to Queued pool, irrespective of their baseFee.
As per EIP-1559 , there is only initial base fee after which it adjusts automatically. There are no current references to a minimum protocol base fee on Ethereum, Gnosis or Polygon.
The existing code would put a 7 wei minimum limit on transactions to be included in the pool. This PR removes this check within txPool. Any transaction can now be added to the
queued
sub-pool after which it could get promoted topending
orbaseFee
subpools.The
pricelimit
flag still exists and acts on non-local transactions as a minimum feeCap for inclusion.