-
Notifications
You must be signed in to change notification settings - Fork 277
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
feat: try to avoid reading mempool under load #1047
Conversation
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.
Would this optimization lead to "not full" blocks from time to time?
85fef6a
Yes, there is a risk of decreasing the overall block space utilization. |
// Fill the block with all available pending transactions. | ||
pending := w.eth.TxPool().PendingWithMax(false, w.config.MaxAccountsNum) | ||
|
||
pending := w.pendingTxns |
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.
Hmm, ownership of pending gets transferred to TransactionsByPriceAndNonce
once NewTransactionsByPriceAndNonce
is called. So we can't just reuse the map as a whole without cloning it first.
1. Purpose or design rationale of this PR
In the cases where we close a block due to some limit other than the timeout, there is a high chance that worker already has a bunch of txns at hand that it can include in the next block.
2. PR title
Your PR title must follow conventional commits (as we are doing squash merge for each PR), so it must start with one of the following types:
3. Deployment tag versioning
Has the version in
params/version.go
been updated?4. Breaking change label
Does this PR have the
breaking-change
label?