From ed189f1858e3f74e99d01176fc24ac47e1f1a425 Mon Sep 17 00:00:00 2001 From: allnil Date: Mon, 19 Feb 2024 15:39:23 +0000 Subject: [PATCH 1/9] merge --- crates/transaction-pool/src/pool/txpool.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/transaction-pool/src/pool/txpool.rs b/crates/transaction-pool/src/pool/txpool.rs index ab877ff0c183..5993930b0099 100644 --- a/crates/transaction-pool/src/pool/txpool.rs +++ b/crates/transaction-pool/src/pool/txpool.rs @@ -291,7 +291,6 @@ impl TxPool { pub(crate) fn best_transactions(&self) -> BestTransactions { self.pending_pool.best() } - /// Returns an iterator that yields transactions that are ready to be included in the block with /// the given base fee and optional blob fee. pub(crate) fn best_transactions_with_attributes( From fa16bd4e10d85380b28c09051f6489c2f46f3771 Mon Sep 17 00:00:00 2001 From: allnil Date: Mon, 19 Feb 2024 16:25:53 +0000 Subject: [PATCH 2/9] feat: refactor iter, add blob fee --- crates/transaction-pool/src/pool/best.rs | 10 +++++++++- crates/transaction-pool/src/pool/blob.rs | 22 +++++++++++++++++++++- 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/crates/transaction-pool/src/pool/best.rs b/crates/transaction-pool/src/pool/best.rs index 4e874f5c7a42..b90e23f8421b 100644 --- a/crates/transaction-pool/src/pool/best.rs +++ b/crates/transaction-pool/src/pool/best.rs @@ -17,10 +17,11 @@ use tracing::debug; /// /// This is a wrapper around [`BestTransactions`] that also enforces a specific basefee. /// -/// This iterator guarantees that all transaction it returns satisfy the base fee. +/// This iterator guarantees that all transaction it returns satisfy both the base fee and blob fee! pub(crate) struct BestTransactionsWithBasefee { pub(crate) best: BestTransactions, pub(crate) base_fee: u64, + pub(crate) base_fee_per_blob_gas: u128, } impl crate::traits::BestTransactions for BestTransactionsWithBasefee { @@ -52,6 +53,13 @@ impl Iterator for BestTransactionsWithBasefee { // tx violates base fee, mark it as invalid and continue crate::traits::BestTransactions::mark_invalid(self, &best); } else { + // tx is EIP4844 and violates blob fee, mark it as invalid and continue + if best.transaction.max_fee_per_blob_gas().is_some_and(|max_fee_per_blob_gas| { + max_fee_per_blob_gas < self.base_fee_per_blob_gas + }) { + crate::traits::BestTransactions::mark_invalid(self, &best); + continue; + }; return Some(best) } } diff --git a/crates/transaction-pool/src/pool/blob.rs b/crates/transaction-pool/src/pool/blob.rs index 6d5b6ce1b114..65ecf6238296 100644 --- a/crates/transaction-pool/src/pool/blob.rs +++ b/crates/transaction-pool/src/pool/blob.rs @@ -85,7 +85,27 @@ impl BlobTransactions { &self, _best_transactions_attributes: BestTransactionsAttributes, ) -> Vec>> { - Vec::new() + let mut transactions = Vec::new(); + { + let mut iter = self.by_id.iter().peekable(); + + while let Some((id, tx)) = iter.next() { + if tx.transaction.max_fee_per_blob_gas() < Some(pending_fees.blob_fee) || + tx.transaction.max_fee_per_gas() < pending_fees.base_fee as u128 + { + // still parked in blob pool -> skip descendant transactions + 'this: while let Some((peek, _)) = iter.peek() { + if peek.sender != id.sender { + break 'this + } + iter.next(); + } + } else { + transactions.push(*id); + } + } + } + transactions } /// The reported size of all transactions in this pool. From 7636d006c0234d50c0fa4635348c90997b78879c Mon Sep 17 00:00:00 2001 From: allnil Date: Mon, 19 Feb 2024 17:23:17 +0000 Subject: [PATCH 3/9] feat: add blob fees --- crates/transaction-pool/src/pool/best.rs | 4 ++-- crates/transaction-pool/src/pool/blob.rs | 8 ++++---- crates/transaction-pool/src/pool/pending.rs | 10 +++++++--- crates/transaction-pool/src/pool/txpool.rs | 5 ++++- 4 files changed, 17 insertions(+), 10 deletions(-) diff --git a/crates/transaction-pool/src/pool/best.rs b/crates/transaction-pool/src/pool/best.rs index b90e23f8421b..a36e505101c3 100644 --- a/crates/transaction-pool/src/pool/best.rs +++ b/crates/transaction-pool/src/pool/best.rs @@ -21,7 +21,7 @@ use tracing::debug; pub(crate) struct BestTransactionsWithBasefee { pub(crate) best: BestTransactions, pub(crate) base_fee: u64, - pub(crate) base_fee_per_blob_gas: u128, + pub(crate) base_fee_per_blob_gas: u64, } impl crate::traits::BestTransactions for BestTransactionsWithBasefee { @@ -55,7 +55,7 @@ impl Iterator for BestTransactionsWithBasefee { } else { // tx is EIP4844 and violates blob fee, mark it as invalid and continue if best.transaction.max_fee_per_blob_gas().is_some_and(|max_fee_per_blob_gas| { - max_fee_per_blob_gas < self.base_fee_per_blob_gas + max_fee_per_blob_gas < self.base_fee_per_blob_gas as u128 }) { crate::traits::BestTransactions::mark_invalid(self, &best); continue; diff --git a/crates/transaction-pool/src/pool/blob.rs b/crates/transaction-pool/src/pool/blob.rs index 65ecf6238296..24984205be75 100644 --- a/crates/transaction-pool/src/pool/blob.rs +++ b/crates/transaction-pool/src/pool/blob.rs @@ -81,7 +81,7 @@ impl BlobTransactions { } /// Returns all transactions that satisfy the given basefee and blob_fee. - pub(crate) const fn satisfy_attributes( + pub(crate) fn satisfy_attributes( &self, _best_transactions_attributes: BestTransactionsAttributes, ) -> Vec>> { @@ -90,8 +90,8 @@ impl BlobTransactions { let mut iter = self.by_id.iter().peekable(); while let Some((id, tx)) = iter.next() { - if tx.transaction.max_fee_per_blob_gas() < Some(pending_fees.blob_fee) || - tx.transaction.max_fee_per_gas() < pending_fees.base_fee as u128 + if tx.transaction.max_fee_per_blob_gas() < Some(self.pending_fees.blob_fee) || + tx.transaction.max_fee_per_gas() < self.pending_fees.base_fee as u128 { // still parked in blob pool -> skip descendant transactions 'this: while let Some((peek, _)) = iter.peek() { @@ -101,7 +101,7 @@ impl BlobTransactions { iter.next(); } } else { - transactions.push(*id); + transactions.push(tx.transaction.clone()); } } } diff --git a/crates/transaction-pool/src/pool/pending.rs b/crates/transaction-pool/src/pool/pending.rs index 0f852f008500..a6109dc2fe4b 100644 --- a/crates/transaction-pool/src/pool/pending.rs +++ b/crates/transaction-pool/src/pool/pending.rs @@ -115,9 +115,13 @@ impl PendingPool { } } - /// Same as `best` but only returns transactions that satisfy the given basefee. - pub(crate) fn best_with_basefee(&self, base_fee: u64) -> BestTransactionsWithBasefee { - BestTransactionsWithBasefee { best: self.best(), base_fee } + /// Same as `best` but only returns transactions that satisfy the given basefee and blobfee. + pub(crate) fn best_with_basefee_and_blobfee( + &self, + base_fee: u64, + base_fee_per_blob_gas: u64, + ) -> BestTransactionsWithBasefee { + BestTransactionsWithBasefee { best: self.best(), base_fee, base_fee_per_blob_gas } } /// Same as `best` but also includes the given unlocked transactions. diff --git a/crates/transaction-pool/src/pool/txpool.rs b/crates/transaction-pool/src/pool/txpool.rs index aeaa5d280b16..672965511ea1 100644 --- a/crates/transaction-pool/src/pool/txpool.rs +++ b/crates/transaction-pool/src/pool/txpool.rs @@ -306,7 +306,10 @@ impl TxPool { } Ordering::Greater => { // base fee increased, we only need to enforce this on the pending pool - Box::new(self.pending_pool.best_with_basefee(best_transactions_attributes.basefee)) + Box::new(self.pending_pool.best_with_basefee_and_blobfee( + best_transactions_attributes.basefee, + best_transactions_attributes.blob_fee.unwrap_or_default(), + )) } Ordering::Less => { // base fee decreased, we need to move transactions from the basefee pool to the From e0142c80eeba2c6ec62d63e41f63bb86b5d7ed84 Mon Sep 17 00:00:00 2001 From: allnil Date: Tue, 20 Feb 2024 17:14:05 +0000 Subject: [PATCH 4/9] chore: rename iterator to consider both fees --- crates/transaction-pool/src/pool/best.rs | 6 +++--- crates/transaction-pool/src/pool/pending.rs | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/transaction-pool/src/pool/best.rs b/crates/transaction-pool/src/pool/best.rs index a36e505101c3..027dfa6b6c79 100644 --- a/crates/transaction-pool/src/pool/best.rs +++ b/crates/transaction-pool/src/pool/best.rs @@ -18,13 +18,13 @@ use tracing::debug; /// This is a wrapper around [`BestTransactions`] that also enforces a specific basefee. /// /// This iterator guarantees that all transaction it returns satisfy both the base fee and blob fee! -pub(crate) struct BestTransactionsWithBasefee { +pub(crate) struct BestTransactionsWithFees { pub(crate) best: BestTransactions, pub(crate) base_fee: u64, pub(crate) base_fee_per_blob_gas: u64, } -impl crate::traits::BestTransactions for BestTransactionsWithBasefee { +impl crate::traits::BestTransactions for BestTransactionsWithFees { fn mark_invalid(&mut self, tx: &Self::Item) { BestTransactions::mark_invalid(&mut self.best, tx) } @@ -42,7 +42,7 @@ impl crate::traits::BestTransactions for BestTransaction } } -impl Iterator for BestTransactionsWithBasefee { +impl Iterator for BestTransactionsWithFees { type Item = Arc>; fn next(&mut self) -> Option { diff --git a/crates/transaction-pool/src/pool/pending.rs b/crates/transaction-pool/src/pool/pending.rs index a6109dc2fe4b..3345636098cc 100644 --- a/crates/transaction-pool/src/pool/pending.rs +++ b/crates/transaction-pool/src/pool/pending.rs @@ -1,7 +1,7 @@ use crate::{ identifier::{SenderId, TransactionId}, pool::{ - best::{BestTransactions, BestTransactionsWithBasefee}, + best::{BestTransactions, BestTransactionsWithFees}, size::SizeTracker, }, Priority, SubPoolLimit, TransactionOrdering, ValidPoolTransaction, @@ -120,8 +120,8 @@ impl PendingPool { &self, base_fee: u64, base_fee_per_blob_gas: u64, - ) -> BestTransactionsWithBasefee { - BestTransactionsWithBasefee { best: self.best(), base_fee, base_fee_per_blob_gas } + ) -> BestTransactionsWithFees { + BestTransactionsWithFees { best: self.best(), base_fee, base_fee_per_blob_gas } } /// Same as `best` but also includes the given unlocked transactions. From 8c2394eac5b43e8204c9309ab23cf4766c538dbd Mon Sep 17 00:00:00 2001 From: allnil Date: Tue, 20 Feb 2024 17:20:43 +0000 Subject: [PATCH 5/9] feat: fix parameters --- crates/transaction-pool/src/pool/blob.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/crates/transaction-pool/src/pool/blob.rs b/crates/transaction-pool/src/pool/blob.rs index 24984205be75..3fe512ca4496 100644 --- a/crates/transaction-pool/src/pool/blob.rs +++ b/crates/transaction-pool/src/pool/blob.rs @@ -90,8 +90,10 @@ impl BlobTransactions { let mut iter = self.by_id.iter().peekable(); while let Some((id, tx)) = iter.next() { - if tx.transaction.max_fee_per_blob_gas() < Some(self.pending_fees.blob_fee) || - tx.transaction.max_fee_per_gas() < self.pending_fees.base_fee as u128 + if tx.transaction.max_fee_per_blob_gas().unwrap_or_default() < + _best_transactions_attributes.blob_fee.unwrap_or_default() as u128 || + tx.transaction.max_fee_per_gas() < + _best_transactions_attributes.basefee as u128 { // still parked in blob pool -> skip descendant transactions 'this: while let Some((peek, _)) = iter.peek() { From 9232c0ec38b0495e6f292d41f187b65ca742eb1b Mon Sep 17 00:00:00 2001 From: allnil Date: Tue, 20 Feb 2024 17:45:19 +0000 Subject: [PATCH 6/9] feat: base fee the same and blob_fee goes down condition --- crates/transaction-pool/src/pool/txpool.rs | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/crates/transaction-pool/src/pool/txpool.rs b/crates/transaction-pool/src/pool/txpool.rs index 672965511ea1..3699f2a2b869 100644 --- a/crates/transaction-pool/src/pool/txpool.rs +++ b/crates/transaction-pool/src/pool/txpool.rs @@ -301,8 +301,19 @@ impl TxPool { match best_transactions_attributes.basefee.cmp(&self.all_transactions.pending_fees.base_fee) { Ordering::Equal => { - // fee unchanged, nothing to shift - Box::new(self.best_transactions()) + // check if blob_fee become lower + let mut unlocked_with_blob = Vec::new(); + if best_transactions_attributes.blob_fee.unwrap_or_default() < + self.all_transactions.pending_fees.blob_fee as u64 + { + unlocked_with_blob = + self.blob_pool.satisfy_attributes(best_transactions_attributes); + } + + Box::new(self.pending_pool.best_with_unlocked( + unlocked_with_blob, + self.all_transactions.pending_fees.base_fee, + )) } Ordering::Greater => { // base fee increased, we only need to enforce this on the pending pool From 18e74adf283131fce8293dcd636200610ae3e6a4 Mon Sep 17 00:00:00 2001 From: allnil Date: Wed, 21 Feb 2024 19:15:53 +0000 Subject: [PATCH 7/9] chore: style nit, parameter is used now --- crates/transaction-pool/src/pool/blob.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/transaction-pool/src/pool/blob.rs b/crates/transaction-pool/src/pool/blob.rs index 3fe512ca4496..f5ce385f5b9e 100644 --- a/crates/transaction-pool/src/pool/blob.rs +++ b/crates/transaction-pool/src/pool/blob.rs @@ -83,7 +83,7 @@ impl BlobTransactions { /// Returns all transactions that satisfy the given basefee and blob_fee. pub(crate) fn satisfy_attributes( &self, - _best_transactions_attributes: BestTransactionsAttributes, + best_transactions_attributes: BestTransactionsAttributes, ) -> Vec>> { let mut transactions = Vec::new(); { @@ -91,9 +91,9 @@ impl BlobTransactions { while let Some((id, tx)) = iter.next() { if tx.transaction.max_fee_per_blob_gas().unwrap_or_default() < - _best_transactions_attributes.blob_fee.unwrap_or_default() as u128 || + best_transactions_attributes.blob_fee.unwrap_or_default() as u128 || tx.transaction.max_fee_per_gas() < - _best_transactions_attributes.basefee as u128 + best_transactions_attributes.basefee as u128 { // still parked in blob pool -> skip descendant transactions 'this: while let Some((peek, _)) = iter.peek() { From c40b8cb5f02318a65d39e4c0bbbe79abd529e4e9 Mon Sep 17 00:00:00 2001 From: allnil Date: Wed, 21 Feb 2024 19:33:30 +0000 Subject: [PATCH 8/9] chore: early exit if blob_fee is none --- crates/transaction-pool/src/pool/blob.rs | 38 ++++++++++++++---------- 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/crates/transaction-pool/src/pool/blob.rs b/crates/transaction-pool/src/pool/blob.rs index f5ce385f5b9e..8e20fe5d352f 100644 --- a/crates/transaction-pool/src/pool/blob.rs +++ b/crates/transaction-pool/src/pool/blob.rs @@ -87,23 +87,31 @@ impl BlobTransactions { ) -> Vec>> { let mut transactions = Vec::new(); { - let mut iter = self.by_id.iter().peekable(); - - while let Some((id, tx)) = iter.next() { - if tx.transaction.max_fee_per_blob_gas().unwrap_or_default() < - best_transactions_attributes.blob_fee.unwrap_or_default() as u128 || - tx.transaction.max_fee_per_gas() < - best_transactions_attributes.basefee as u128 - { - // still parked in blob pool -> skip descendant transactions - 'this: while let Some((peek, _)) = iter.peek() { - if peek.sender != id.sender { - break 'this + // short path if blob_fee is None in provided best transactions attributes + if best_transactions_attributes.blob_fee.is_some() { + let blob_fee_to_satisfy = best_transactions_attributes + .blob_fee + .expect("blob fee is some and already checked") + as u128; + + let mut iter = self.by_id.iter().peekable(); + + while let Some((id, tx)) = iter.next() { + if tx.transaction.max_fee_per_blob_gas().unwrap_or_default() < + blob_fee_to_satisfy || + tx.transaction.max_fee_per_gas() < + best_transactions_attributes.basefee as u128 + { + // still parked in blob pool -> skip descendant transactions + 'this: while let Some((peek, _)) = iter.peek() { + if peek.sender != id.sender { + break 'this + } + iter.next(); } - iter.next(); + } else { + transactions.push(tx.transaction.clone()); } - } else { - transactions.push(tx.transaction.clone()); } } } From 888be4967d1114e19f98a080362cbf07862c69a2 Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Thu, 22 Feb 2024 12:14:51 +0100 Subject: [PATCH 9/9] chore: touchups --- crates/transaction-pool/src/pool/blob.rs | 14 +++--- crates/transaction-pool/src/pool/txpool.rs | 56 ++++++++++++++-------- 2 files changed, 43 insertions(+), 27 deletions(-) diff --git a/crates/transaction-pool/src/pool/blob.rs b/crates/transaction-pool/src/pool/blob.rs index 8e20fe5d352f..734d7dacdbc3 100644 --- a/crates/transaction-pool/src/pool/blob.rs +++ b/crates/transaction-pool/src/pool/blob.rs @@ -80,7 +80,9 @@ impl BlobTransactions { Some(tx.transaction) } - /// Returns all transactions that satisfy the given basefee and blob_fee. + /// Returns all transactions that satisfy the given basefee and blobfee. + /// + /// Note: This does not remove any the transactions from the pool. pub(crate) fn satisfy_attributes( &self, best_transactions_attributes: BestTransactionsAttributes, @@ -88,12 +90,9 @@ impl BlobTransactions { let mut transactions = Vec::new(); { // short path if blob_fee is None in provided best transactions attributes - if best_transactions_attributes.blob_fee.is_some() { - let blob_fee_to_satisfy = best_transactions_attributes - .blob_fee - .expect("blob fee is some and already checked") - as u128; - + if let Some(blob_fee_to_satisfy) = + best_transactions_attributes.blob_fee.map(|fee| fee as u128) + { let mut iter = self.by_id.iter().peekable(); while let Some((id, tx)) = iter.next() { @@ -102,6 +101,7 @@ impl BlobTransactions { tx.transaction.max_fee_per_gas() < best_transactions_attributes.basefee as u128 { + // does not satisfy the blob fee or base fee // still parked in blob pool -> skip descendant transactions 'this: while let Some((peek, _)) = iter.peek() { if peek.sender != id.sender { diff --git a/crates/transaction-pool/src/pool/txpool.rs b/crates/transaction-pool/src/pool/txpool.rs index 7ddec68a1f41..5a3a312085f2 100644 --- a/crates/transaction-pool/src/pool/txpool.rs +++ b/crates/transaction-pool/src/pool/txpool.rs @@ -287,33 +287,45 @@ impl TxPool { } } - /// Returns an iterator that yields transactions that are ready to be included in the block. + /// Returns an iterator that yields transactions that are ready to be included in the block with + /// the tracked fees. pub(crate) fn best_transactions(&self) -> BestTransactions { self.pending_pool.best() } + /// Returns an iterator that yields transactions that are ready to be included in the block with /// the given base fee and optional blob fee. + /// + /// If the provided attributes differ from the currently tracked fees, this will also include + /// transactions that are unlocked by the new fees, or exclude transactions that are no longer + /// valid with the new fees. pub(crate) fn best_transactions_with_attributes( &self, best_transactions_attributes: BestTransactionsAttributes, ) -> Box>>> { + // First we need to check if the given base fee is different than what's currently being + // tracked match best_transactions_attributes.basefee.cmp(&self.all_transactions.pending_fees.base_fee) { Ordering::Equal => { - // check if blob_fee become lower - let mut unlocked_with_blob = Vec::new(); - if best_transactions_attributes.blob_fee.unwrap_or_default() < - self.all_transactions.pending_fees.blob_fee as u64 + // for EIP-4844 transactions we also need to check if the blob fee is now lower than + // what's currently being tracked, if so we need to include transactions from the + // blob pool that are valid with the lower blob fee + if best_transactions_attributes + .blob_fee + .map_or(false, |fee| fee < self.all_transactions.pending_fees.blob_fee as u64) { - unlocked_with_blob = + let unlocked_by_blob_fee = self.blob_pool.satisfy_attributes(best_transactions_attributes); - } - Box::new(self.pending_pool.best_with_unlocked( - unlocked_with_blob, - self.all_transactions.pending_fees.base_fee, - )) + Box::new(self.pending_pool.best_with_unlocked( + unlocked_by_blob_fee, + self.all_transactions.pending_fees.base_fee, + )) + } else { + Box::new(self.pending_pool.best()) + } } Ordering::Greater => { // base fee increased, we only need to enforce this on the pending pool @@ -323,15 +335,19 @@ impl TxPool { )) } Ordering::Less => { - // base fee decreased, we need to move transactions from the basefee pool to the - // pending pool and satisfy blob fee transactions as well - let unlocked_with_blob = - self.blob_pool.satisfy_attributes(best_transactions_attributes); - - Box::new(self.pending_pool.best_with_unlocked( - unlocked_with_blob, - self.all_transactions.pending_fees.base_fee, - )) + // base fee decreased, we need to move transactions from the basefee + blob pool to + // the pending pool that might be unlocked by the lower base fee + let mut unlocked = self + .basefee_pool + .satisfy_base_fee_transactions(best_transactions_attributes.basefee); + + // also include blob pool transactions that are now unlocked + unlocked.extend(self.blob_pool.satisfy_attributes(best_transactions_attributes)); + + Box::new( + self.pending_pool + .best_with_unlocked(unlocked, self.all_transactions.pending_fees.base_fee), + ) } } }