diff --git a/crates/transaction-pool/src/pool/blob.rs b/crates/transaction-pool/src/pool/blob.rs index 4503bf2670119..15f337ecc8929 100644 --- a/crates/transaction-pool/src/pool/blob.rs +++ b/crates/transaction-pool/src/pool/blob.rs @@ -27,6 +27,8 @@ pub(crate) struct BlobTransactions { by_id: BTreeMap>, /// _All_ transactions sorted by blob priority. all: BTreeSet>, + /// Keeps track of the current fees, so transaction priority can be calculated on insertion. + pending_fees: PendingFees, /// Keeps track of the size of this pool. /// /// See also [`PoolTransaction::size`]. @@ -55,13 +57,19 @@ impl BlobTransactions { // keep track of size self.size_of += tx.size(); - let ord = BlobOrd { submission_id }; - let transaction = BlobTransaction { ord, transaction: tx }; + // set transaction, which will also calculate priority based on current pending fees + let transaction = BlobTransaction::new(tx, submission_id, &self.pending_fees); self.by_id.insert(id, transaction.clone()); self.all.insert(transaction); } + fn next_id(&mut self) -> u64 { + let id = self.submission_id; + self.submission_id = self.submission_id.wrapping_add(1); + id + } + /// Removes the transaction from the pool pub(crate) fn remove_transaction( &mut self, @@ -86,12 +94,6 @@ impl BlobTransactions { Vec::new() } - fn next_id(&mut self) -> u64 { - let id = self.submission_id; - self.submission_id = self.submission_id.wrapping_add(1); - id - } - /// The reported size of all transactions in this pool. pub(crate) fn size(&self) -> usize { self.size_of.into() @@ -136,10 +138,31 @@ impl BlobTransactions { transactions } + /// Resorts the transactions in the pool based on the pool's current [PendingFees]. + pub(crate) fn reprioritize(&mut self) { + // mem::take to modify without allocating, then collect to rebuild the BTreeSet + self.all = std::mem::take(&mut self.all) + .into_iter() + .map(|mut tx| { + tx.update_priority(&self.pending_fees); + tx + }) + .collect(); + + // we need to update `by_id` as well because removal from `all` can only happen if the + // `BlobTransaction`s in each struct are consistent + for tx in self.by_id.values_mut() { + tx.update_priority(&self.pending_fees); + } + } + /// Removes all transactions (and their descendants) which: /// * have a `max_fee_per_blob_gas` greater than or equal to the given `blob_fee`, _and_ /// * have a `max_fee_per_gas` greater than or equal to the given `base_fee` /// + /// This also sets the [PendingFees] for the pool, resorting transactions based on their + /// updated priority. + /// /// Note: the transactions are not returned in a particular order. pub(crate) fn enforce_pending_fees( &mut self, @@ -152,6 +175,10 @@ impl BlobTransactions { removed.push(self.remove_transaction(&id).expect("transaction exists")); } + // set pending fees and reprioritize / resort + self.pending_fees = pending_fees.clone(); + self.reprioritize(); + removed } @@ -176,11 +203,13 @@ impl Default for BlobTransactions { by_id: Default::default(), all: Default::default(), size_of: Default::default(), + pending_fees: Default::default(), } } } /// A transaction that is ready to be included in a block. +#[derive(Debug)] struct BlobTransaction { /// Actual blob transaction. transaction: Arc>, @@ -188,6 +217,35 @@ struct BlobTransaction { ord: BlobOrd, } +impl BlobTransaction { + /// Creates a new blob transaction, based on the pool transaction, submission id, and current + /// pending fees. + pub(crate) fn new( + transaction: Arc>, + submission_id: u64, + pending_fees: &PendingFees, + ) -> Self { + let priority = blob_tx_priority( + pending_fees.blob_fee, + transaction.max_fee_per_blob_gas().unwrap_or_default(), + pending_fees.base_fee as u128, + transaction.max_fee_per_gas(), + ); + let ord = BlobOrd { priority, submission_id }; + Self { transaction, ord } + } + + /// Updates the priority for the transaction based on the current pending fees. + pub(crate) fn update_priority(&mut self, pending_fees: &PendingFees) { + self.ord.priority = blob_tx_priority( + pending_fees.blob_fee, + self.transaction.max_fee_per_blob_gas().unwrap_or_default(), + pending_fees.base_fee as u128, + self.transaction.max_fee_per_gas(), + ); + } +} + impl Clone for BlobTransaction { fn clone(&self) -> Self { Self { transaction: self.transaction.clone(), ord: self.ord.clone() } @@ -214,11 +272,47 @@ impl Ord for BlobTransaction { } } +/// The blob step function, attempting to compute the delta given the `max_tx_fee`, and +/// `current_fee`. +/// +/// The `max_tx_fee` is the maximum fee that the transaction is willing to pay, this +/// would be the priority fee for the EIP1559 component of transaction fees, and the blob fee cap +/// for the blob component of transaction fees. +/// +/// The `current_fee` is the current value of the fee, this would be the base fee for the EIP1559 +/// component, and the blob fee (computed from the current head) for the blob component. +/// +/// This is supposed to get the number of fee jumps required to get from the current fee to the fee +/// cap, or where the transaction would not be executable any more. +fn fee_delta(max_tx_fee: u128, current_fee: u128) -> f64 { + // jumps = log1.125(txfee) - log1.125(basefee) + // TODO: should we do this without f64? + let jumps = (max_tx_fee as f64).log(1.125) - (current_fee as f64).log(1.125); + // delta = sign(jumps) * log(abs(jumps)) + jumps.signum() * jumps.abs().log2() +} + +/// Returns the priority for the transaction, based on the "delta" blob fee and priority fee. +fn blob_tx_priority( + blob_fee_cap: u128, + blob_fee: u128, + max_priority_fee: u128, + base_fee: u128, +) -> f64 { + let delta_blob_fee = fee_delta(blob_fee_cap, blob_fee); + let delta_priority_fee = fee_delta(max_priority_fee, base_fee); + + // priority = min(delta-basefee, delta-blobfee, 0) + delta_blob_fee.min(delta_priority_fee).min(0.0) +} + #[derive(Debug, Clone)] struct BlobOrd { /// Identifier that tags when transaction was submitted in the pool. pub(crate) submission_id: u64, - // TODO(mattsse): add ord values + // The priority for this transaction, calculated using the [`blob_tx_priority`] function, + // taking into account both the blob and priority fee. + pub(crate) priority: f64, } impl Eq for BlobOrd {} @@ -237,6 +331,238 @@ impl PartialOrd for BlobOrd { impl Ord for BlobOrd { fn cmp(&self, other: &Self) -> Ordering { - other.submission_id.cmp(&self.submission_id) + let ord = other.priority.total_cmp(&self.priority); + + // use submission_id to break ties + if ord == Ordering::Equal { + self.submission_id.cmp(&other.submission_id) + } else { + ord + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::test_utils::{MockTransaction, MockTransactionFactory}; + + /// Represents the fees for a single transaction, which will be built inside of a test. + #[derive(Debug, Clone, Copy, PartialEq, Eq)] + struct TransactionFees { + /// The blob fee cap for the transaction. + max_blob_fee: u128, + /// The max priority fee for the transaction. + max_priority_fee_per_gas: u128, + /// The base fee for the transaction. + max_fee_per_gas: u128, + } + + /// Represents an ordering of transactions based on their fees and the current network fees. + #[derive(Debug, Clone)] + struct TransactionOrdering { + /// The transaction fees, in the order that they're expected to be returned + fees: Vec, + /// The network fees + network_fees: PendingFees, + } + + #[test] + fn test_blob_ordering() { + // Tests are from: + // + let mut factory = MockTransactionFactory::default(); + + let vectors = vec![ + // If everything is above basefee and blobfee, order by miner tip + TransactionOrdering { + fees: vec![ + TransactionFees { + max_blob_fee: 2, + max_priority_fee_per_gas: 0, + max_fee_per_gas: 2, + }, + TransactionFees { + max_blob_fee: 3, + max_priority_fee_per_gas: 1, + max_fee_per_gas: 1, + }, + TransactionFees { + max_blob_fee: 1, + max_priority_fee_per_gas: 2, + max_fee_per_gas: 3, + }, + ], + network_fees: PendingFees { base_fee: 0, blob_fee: 0 }, + }, + // If only basefees are used (blob fee matches with network), return the ones the + // furthest below the current basefee, splitting same ones with the tip. Anything above + // the basefee should be split by tip. + TransactionOrdering { + fees: vec![ + TransactionFees { + max_blob_fee: 0, + max_priority_fee_per_gas: 50, + max_fee_per_gas: 500, + }, + TransactionFees { + max_blob_fee: 0, + max_priority_fee_per_gas: 100, + max_fee_per_gas: 500, + }, + TransactionFees { + max_blob_fee: 0, + max_priority_fee_per_gas: 50, + max_fee_per_gas: 1000, + }, + TransactionFees { + max_blob_fee: 0, + max_priority_fee_per_gas: 100, + max_fee_per_gas: 1000, + }, + TransactionFees { + max_blob_fee: 0, + max_priority_fee_per_gas: 1, + max_fee_per_gas: 2000, + }, + TransactionFees { + max_blob_fee: 0, + max_priority_fee_per_gas: 2, + max_fee_per_gas: 2000, + }, + TransactionFees { + max_blob_fee: 0, + max_priority_fee_per_gas: 3, + max_fee_per_gas: 2000, + }, + ], + network_fees: PendingFees { base_fee: 1999, blob_fee: 0 }, + }, + // If only blobfees are used (base fee matches with network), return the + // ones the furthest below the current blobfee, splitting same ones with + // the tip. Anything above the blobfee should be split by tip. + TransactionOrdering { + fees: vec![ + TransactionFees { + max_blob_fee: 500, + max_priority_fee_per_gas: 50, + max_fee_per_gas: 0, + }, + TransactionFees { + max_blob_fee: 500, + max_priority_fee_per_gas: 100, + max_fee_per_gas: 0, + }, + TransactionFees { + max_blob_fee: 1000, + max_priority_fee_per_gas: 50, + max_fee_per_gas: 0, + }, + TransactionFees { + max_blob_fee: 1000, + max_priority_fee_per_gas: 100, + max_fee_per_gas: 0, + }, + TransactionFees { + max_blob_fee: 2000, + max_priority_fee_per_gas: 1, + max_fee_per_gas: 0, + }, + TransactionFees { + max_blob_fee: 2000, + max_priority_fee_per_gas: 2, + max_fee_per_gas: 0, + }, + TransactionFees { + max_blob_fee: 2000, + max_priority_fee_per_gas: 3, + max_fee_per_gas: 0, + }, + ], + network_fees: PendingFees { base_fee: 0, blob_fee: 1999 }, + }, + // If both basefee and blobfee is specified, sort by the larger distance + // of the two from the current network conditions, splitting same (loglog) + // ones via the tip. + // + // Basefee: 1000 + // Blobfee: 100 + // + // Tx #0: (800, 80) - 2 jumps below both => priority -1 + // Tx #1: (630, 63) - 4 jumps below both => priority -2 + // Tx #2: (800, 63) - 2 jumps below basefee, 4 jumps below blobfee => priority -2 (blob + // penalty dominates) Tx #3: (630, 80) - 4 jumps below basefee, 2 jumps + // below blobfee => priority -2 (base penalty dominates) + // + // Txs 1, 2, 3 share the same priority, split via tip, prefer 0 as the best + TransactionOrdering { + fees: vec![ + TransactionFees { + max_blob_fee: 80, + max_priority_fee_per_gas: 4, + max_fee_per_gas: 630, + }, + TransactionFees { + max_blob_fee: 63, + max_priority_fee_per_gas: 3, + max_fee_per_gas: 800, + }, + TransactionFees { + max_blob_fee: 63, + max_priority_fee_per_gas: 2, + max_fee_per_gas: 630, + }, + TransactionFees { + max_blob_fee: 80, + max_priority_fee_per_gas: 1, + max_fee_per_gas: 800, + }, + ], + network_fees: PendingFees { base_fee: 1000, blob_fee: 100 }, + }, + ]; + + for ordering in vectors { + // create a new pool each time + let mut pool = BlobTransactions::default(); + + // create tx from fees + let txs = ordering + .fees + .iter() + .map(|fees| { + MockTransaction::eip4844() + .with_blob_fee(fees.max_blob_fee) + .with_priority_fee(fees.max_priority_fee_per_gas) + .with_max_fee(fees.max_fee_per_gas) + }) + .collect::>(); + + for tx in txs.iter() { + pool.add_transaction(factory.validated_arc(tx.clone())); + } + + // update fees and resort the pool + pool.pending_fees = ordering.network_fees.clone(); + pool.reprioritize(); + + // now iterate through the pool and make sure they're in the same order as the original + // fees - map to TransactionFees so it's easier to compare the ordering without having + // to see irrelevant fields + let actual_txs = pool + .all + .iter() + .map(|tx| TransactionFees { + max_blob_fee: tx.transaction.max_fee_per_blob_gas().unwrap_or_default(), + max_priority_fee_per_gas: tx.transaction.priority_fee_or_price(), + max_fee_per_gas: tx.transaction.max_fee_per_gas(), + }) + .collect::>(); + assert_eq!( + ordering.fees, actual_txs, + "ordering mismatch, expected: {:#?}, actual: {:#?}", + ordering.fees, actual_txs + ); + } } }