-
Notifications
You must be signed in to change notification settings - Fork 20.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
all: change TxPreEvent to TxsPreEvent #16720
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.
LGTM
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.
one typo
core/tx_journal.go
Outdated
for _, err := range errs { | ||
// Create a method to load a limited batch of transactions and bump the | ||
// appropriate progress counters. Then use this method to load all the | ||
// journelled transactions in small-ish batches. |
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.
s/journelled/journalled/
core/tx_pool.go
Outdated
@@ -653,7 +653,7 @@ func (pool *TxPool) add(tx *types.Transaction, local bool) (bool, error) { | |||
log.Trace("Pooled new executable transaction", "hash", hash, "from", from, "to", tx.To()) | |||
|
|||
// We've directly injected a replacement transaction, notify subsystems | |||
go pool.txFeed.Send(TxsPreEvent{types.Transactions{tx}}) | |||
go pool.txFeed.Send(NewTxsEvent{types.Transactions{tx}}) |
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.
Will it be better if we post the NewTxsEvent
synchronously here?
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.
Felix started looking into the synchronous postings, so I'd rather keep that can of worms in a separate pr.
…lay, Raft minter with NewTxsEvent
Currently, when a transaction is promoted, a
TxPreEvent
will be pushed to subscribers immediately.While when the number of new transactions received is large, there are too many
TxPreEvent
will be pushed leads totxFeed
lock resource contention.So in this PR, i replace all
TxPreEvent
toTxsPreEvent
. So that we can push a batch of new promoted txs together. It will help in these scenario:Besides, in this PR, also fix a small problem.
When we apply new promoted transaction to mining block, it should sill abide original gas limit restriction instead of creating a new gas limit.