wip: feat(txpool): new txpool design #22
Conversation
mattsse
force-pushed
the
matt/add-tx-pool-2
branch
from
0d81a00
to
eff5c30
Compare
|
@rakita wdyt about these changes? one thing I think needs to be changed is the I guess we could instead make this a generic And for I kept the internal mapping for |
mattsse
force-pushed
the
matt/add-tx-pool-2
branch
from
39272d1
to
5846a3f
Compare
|
As discussed, trust you to design this according to Erigon & the patterns for multi-tx pool, let's make sure to add a diagram later. |
There was a problem hiding this comment.
Priority seems okay for this level of abstraction, as you can put anything that you want to compare inside it. And it has Ord.
for anything you would do inside compare(tx, tx) -> Ordering as in tx1.price < tx1.price you are extracting data to Priority= tx.price and comparing price after that price1 < price2 it is same principle with having compare(tx,tx)
Maybe it would be a cleaner implementation but as I can tell you are getting similar functionality.
| /// Actual transaction. | ||
| pub(crate) transaction: Arc<ValidPoolTransaction<T::Transaction>>, | ||
| /// A transaction that is ready to be included in a block. | ||
| pub(crate) struct PendingTransactionRef<T: TransactionOrdering> { | ||
| /// Identifier that tags when transaction was submitted in the pool. | ||
| pub(crate) submission_id: u64, |
There was a problem hiding this comment.
Could you use timestamp from ValidPoolTransaction for this?
There was a problem hiding this comment.
I've thought about this as well, but since Instant depends on the OS this could lead to issues. I saw weird timestamps in the past, especially after reboot.
There was a problem hiding this comment.
Instant can report time in past sometimes (few ns or very small margin), and panic would happen if you sub them with that assumption (had a bug with this :) ), for txpool where transactions come in arbitrary order in the span of seconds/minutes we just need a comparison we wouldn't care about value.
There was a problem hiding this comment.
if you sub them with that assumption (had a bug with this :)
same :D
makes sense, I'll use the timestamp instead of the id then
mattsse
force-pushed
the
matt/add-tx-pool-2
branch
from
30ba481
to
02b0d58
Compare
onbjerg
added
C-enhancement
|
sg! I wrote additional tests, will merge this as is, and then start smaller iterations. starting with docs + definition |
Yeah let's start being explicit about what it provides, can provide in the future, and any limitations. |
…rli-forkid-test Fix optimism_goerli_forkids test with genesis and regolith conditions
WIP
This is another txpool iteration that replaces the previous graph based pool with a tier-pool design described here https://github.com/ledgerwatch/erigon/wiki/Transaction-Pool-Design#design-proposal
The previous pool design, essentially was only able to handle the nonce gap condition. Introducing additional state-related conditions turned out to be tricky.
Instead of two (pending, queued) sub-pools, this has three subpools (pending, basefee, queued) and can also handle: dynamic fee + balance related restrictions.
The state of a tx is maintained in a bitfield from which the subpool the tx should be moved to is derived.
There's only one
BTreeMap((Sender,Nonce), Tx)that tracks all transactions in the pool. The subpools then contain a reference (or rather an Arc clone) to the transaction.By storing all transactions in one place, state changes can be derived more efficiently than checking separate pools.
By sorting by
(Sender,Nonce)all descendant tx of a tx can be visited efficiently, this is imported to update the state of transactions from the same sender.If a new tx is added, or a block was mined, appropriate updates are derived which are then applied to the subpools (e.g.: move tx from queued to ready).
The pending pool returns an iterator (
BestTransactions) that always returns the next best transaction.todo: