fix(txpool): pendind pool reordering #3955
Conversation
rkrasiuk
added
C-bug
Codecov Report
... and 8 files with indirect coverage changes
Flags with carried forward coverage won't be shown. Click here to find out more.
|
| /// | ||
| /// NOTE: The implementation is incomplete for missing base fee. | ||
| fn priority(&self, transaction: &Self::Transaction, base_fee: u64) -> Self::Priority { | ||
| U256::from(transaction.effective_tip_per_gas(base_fee).expect("tx has been validated")) |
There was a problem hiding this comment.
while this should be accurate for txs in the pending pool
I don't like that this panics for txs that don't meet the basefee
I'd rather use a new priority enum type that works like an option
| fn clear_transactions(&mut self) -> BTreeMap<TransactionId, Arc<PendingTransaction<T>>> { | ||
| self.independent_transactions.clear(); | ||
| self.all.clear(); | ||
| std::mem::take(&mut self.by_id) |
There was a problem hiding this comment.
this needs a note that it does not update size tracker
| let mut removed = Vec::new(); | ||
|
|
||
| // Drain and iterate over all transactions. | ||
| let mut transactions_iter = self.clear_transactions().into_iter().peekable(); |
There was a problem hiding this comment.
why do we clear all lists?
we only need to rebuild the sorted list and only remove those that should be removed from the entire set?
There was a problem hiding this comment.
we need to update two sorted lists, but, yeah, no need to remove the map
| // TODO: temporary solution for ordering the queued pool. | ||
| impl<T: PoolTransaction> Ord for QueuedOrd<T> { | ||
| fn cmp(&self, other: &Self) -> Ordering { | ||
| // Higher cost is better | ||
| self.gas_cost().cmp(&other.gas_cost()).then_with(|| | ||
| // Higher price is better | ||
| self.priority_fee_or_price().cmp(&self.priority_fee_or_price()).then_with(|| |
There was a problem hiding this comment.
yeah we also want to consider nonce gap and distance to basefee I think
rkrasiuk
force-pushed
the
rkrasiuk/txpool-pending-reordering
branch
from
bc70ef5 to
6c6c993
Compare
Replaces #3853
Description
Changes the intrinsics of the ordering function to account for sorting context (currently, only
base_fee). Needed to determine the proper ordering of the transactions within the pool.The pending pool is now re-sorted on each base fee update.
NOTE: This PR does not fix everything and requires follow ups, most importantly for the
QueuedOrd.