-
Notifications
You must be signed in to change notification settings - Fork 20.5k
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/txpool, eth/catalyst: clear transaction pool in Rollback #30534
Conversation
core/txpool/subpool.go
Outdated
@@ -162,4 +162,7 @@ type SubPool interface { | |||
// Status returns the known status (unknown/pending/queued) of a transaction | |||
// identified by their hashes. | |||
Status(hash common.Hash) TxStatus | |||
|
|||
// DropTransactions removes all tracked transactions from the pool | |||
DropTransactions() |
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.
Pls rename to Clear
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.
p.lock.Lock() | ||
defer p.lock.Unlock() | ||
|
||
// manually iterating and deleting every entry is super sub-optimal |
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.
We should probably iterate and unreserve the addresses here. It's less optimal that recreating the reservations map in the parent pool, but that's very sensitive datarace wise, so it's simpler to iterate for now.
core/txpool/legacypool/legacypool.go
Outdated
p.mu.Lock() | ||
defer p.mu.Unlock() | ||
|
||
p.all = newLookup() |
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.
We should probably iterate and unreserve the addresses here. It's less optimal that recreating the reservations map in the parent pool, but that's very sensitive datarace wise, so it's simpler to iterate for now.
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
…interface. Make simulated beacon rollback use the new method, fix old hack
…it's not done at the TxPool level.
0bc3138
to
e4e0c45
Compare
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
This adds an API method `DropTransactions` to legacy pool, blob pool and txpool interface. This method removes all txs currently tracked in the pools. It modifies the simulated beacon to use the new method in `Rollback` which removes previous hacky implementation that also erroneously reset the gas tip to 1 gwei. --------- Co-authored-by: Felix Lange <[email protected]>
…um#30534) This adds an API method `DropTransactions` to legacy pool, blob pool and txpool interface. This method removes all txs currently tracked in the pools. It modifies the simulated beacon to use the new method in `Rollback` which removes previous hacky implementation that also erroneously reset the gas tip to 1 gwei. --------- Co-authored-by: Felix Lange <[email protected]>
This adds an API method
DropTransactions
to legacy pool, blob pool and txpool interface. This method removes all txs currently tracked in the pools.It modifies the simulated beacon to use the new method in
Rollback
which removes previous hacky implementation that also erroneously reset the gas tip to 1 gwei.