-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Txpool 4844 upgrades Part 2 #8213
Conversation
backend.notifications.Accumulator.StartChange(currentBlock.NumberU64(), currentBlock.Hash(), nil, false) | ||
backend.notifications.Accumulator.SendAndReset(ctx, backend.notifications.StateChangesConsumer, baseFee, currentBlock.GasLimit(), 0) | ||
|
||
backend.notifications.Accumulator.SendAndReset(ctx, backend.notifications.StateChangesConsumer, baseFee, blobFee, currentBlock.GasLimit(), 0) |
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.
If we are sending pending rather than current blobFee, it makes sense to do the same for baseFee (pending instead of current).
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, i will make that change as well. Meanwhile, in this case we are sending values (to txPool) with respect to the CurrentBlock, which is where the sync is at. Should we consider sending values for the canonical head instead?
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.
This piece of code looks like a kludge. I'd leave currentBlock
as is since it seems to be doing the job.
Fix some peer-review comments from the last related PR, and add some enhancements #### Change summary - Addition of a flag for BlobSlots - for max allowed blobs per account in txpool - Use BlobFee from the block to validate txs in the pool - Let go of unwound blob txn's onNewBlock, if not present in the cache; we don't request them again from the network Related: erigontech/erigon#8213 erigontech/interfaces#195
Some peer-review changes from the last related PR.
Addition of a flag for BlobSlots - for max allowed blobs per account in txpool.
Use BlobFee from the block to validate txs in the pool.
See also ledgerwatch/erigon-lib#1125