Change mempool tx eviction policy

mining/errors/src/mempool.rs

@@ -33,9 +33,9 @@ pub enum RuleError {
     #[error("replace by fee found more than one double spending transaction in the mempool")]
     RejectRbfTooManyDoubleSpendingTransactions,
 
-    /// New behavior: a transaction is rejected if the mempool is full
-    #[error("number of high-priority transactions in mempool ({0}) has reached the maximum allowed ({1})")]
-    RejectMempoolIsFull(usize, u64),
+    /// a transaction is rejected if the mempool is full
+    #[error("transaction could not be added to the mempool because it's full with transactions with higher priority")]
+    RejectMempoolIsFull,
 
     /// An error emitted by mining\src\mempool\check_transaction_standard.rs
     #[error("transaction {0} is not standard: {1}")]

mining/src/mempool/config.rs

@@ -26,7 +26,8 @@ pub(crate) const DEFAULT_MAXIMUM_STANDARD_TRANSACTION_VERSION: u16 = TX_VERSION;
 
 #[derive(Clone, Debug)]
 pub struct Config {
-    pub maximum_transaction_count: u32,
+    pub maximum_transaction_count: usize,
+    pub maximum_transaction_compute_mass: u64,
     pub maximum_build_block_template_attempts: u64,
     pub transaction_expire_interval_daa_score: u64,
     pub transaction_expire_scan_interval_daa_score: u64,
@@ -49,7 +50,7 @@ pub struct Config {
 impl Config {
     #[allow(clippy::too_many_arguments)]
     pub fn new(
-        maximum_transaction_count: u32,
+        maximum_transaction_count: usize,
         maximum_build_block_template_attempts: u64,
         transaction_expire_interval_daa_score: u64,
         transaction_expire_scan_interval_daa_score: u64,

mining/src/mempool/model/frontier.rs

@@ -5,12 +5,12 @@ use crate::{
 };
 
 use feerate_key::FeerateTransactionKey;
-use kaspa_consensus_core::block::TemplateTransactionSelector;
+use kaspa_consensus_core::{block::TemplateTransactionSelector, tx::Transaction};
 use kaspa_core::trace;
 use rand::{distributions::Uniform, prelude::Distribution, Rng};
 use search_tree::SearchTree;
 use selectors::{SequenceSelector, SequenceSelectorInput, TakeAllSelector};
-use std::collections::HashSet;
+use std::{collections::HashSet, iter::FusedIterator, sync::Arc};
 
 pub(crate) mod feerate_key;
 pub(crate) mod search_tree;
@@ -254,6 +254,10 @@ impl Frontier {
         }
         estimator
     }
+
+    pub fn ascending_iter(&self) -> impl DoubleEndedIterator<Item = &Arc<Transaction>> + ExactSizeIterator + FusedIterator {
+        self.search_tree.ascending_iter().map(|key| &key.tx)
+    }
 }
 
 #[cfg(test)]

mining/src/mempool/model/transactions_pool.rs

@@ -14,6 +14,7 @@ use crate::{
     model::topological_index::TopologicalIndex,
     Policy,
 };
+use itertools::Itertools;
 use kaspa_consensus_core::{
     block::TemplateTransactionSelector,
     tx::{MutableTransaction, TransactionId, TransactionOutpoint},
@@ -197,58 +198,43 @@ impl TransactionsPool {
     }
 
     /// Returns the exceeding low-priority transactions having the lowest fee rates in order
-    /// to have room for at least `free_slots` new transactions. The returned transactions
+    /// to make room for `transaction`. The returned transactions
     /// are guaranteed to be unchained (no successor in mempool) and to not be parent of
     /// `transaction`.
     ///
-    /// An error is returned if the mempool is filled with high priority transactions.
-    pub(crate) fn limit_transaction_count(
-        &self,
-        free_slots: usize,
-        transaction: &MutableTransaction,
-    ) -> RuleResult<Vec<TransactionId>> {
-        assert!(free_slots > 0);
+    /// An error is returned if the mempool is filled with high priority transactions, or
+    /// at least one candidate for removal has a higher fee rate than `transaction`.
+    pub(crate) fn limit_transaction_count(&self, transaction: &MutableTransaction) -> RuleResult<Vec<TransactionId>> {
         // Returns a vector of transactions to be removed that the caller has to remove actually.
         // The caller is golang validateAndInsertTransaction equivalent.
         // This behavior differs from golang impl.
-        let trim_size = self.len() + free_slots - usize::min(self.len() + free_slots, self.config.maximum_transaction_count as usize);
-        let mut transactions_to_remove = Vec::with_capacity(trim_size);
-        if trim_size > 0 {
-            // TODO: consider introducing an index on all_transactions low-priority items instead.
-            //
-            // Sorting this vector here may be sub-optimal compared with maintaining a sorted
-            // index of all_transactions low-priority items if the proportion of low-priority txs
-            // in all_transactions is important.
-            let low_priority_txs = self
-                .all_transactions
-                .values()
-                .filter(|x| x.priority == Priority::Low && self.transaction_is_unchained(&x.id()) && !x.is_parent_of(transaction));
-
-            if trim_size == 1 {
-                // This is the most likely case. Here we just search the minimum, thus avoiding the need to sort altogether.
-                if let Some(tx) = low_priority_txs.min_by(|a, b| a.fee_rate().partial_cmp(&b.fee_rate()).unwrap()) {
-                    transactions_to_remove.push(tx);
-                }
-            } else {
-                let mut low_priority_txs = low_priority_txs.collect::<Vec<_>>();
-                if low_priority_txs.len() > trim_size {
-                    low_priority_txs.sort_by(|a, b| a.fee_rate().partial_cmp(&b.fee_rate()).unwrap());
-                    transactions_to_remove.extend_from_slice(&low_priority_txs[0..usize::min(trim_size, low_priority_txs.len())]);
+        self.ready_transactions
+            .ascending_iter()
+            .map(|tx| self.all_transactions.get(&tx.id()).unwrap())
+            .filter(|mtx| mtx.priority == Priority::Low && !mtx.is_parent_of(transaction))
+            .scan((self.len() + 1, self.total_compute_mass + transaction.calculated_compute_mass.unwrap()), |state, mtx| {
+                (*state).0 -= 1;
+                (*state).1 -= mtx.calculated_compute_mass().unwrap();
+                Some((mtx, *state))
+            })
+            .take_while(|(_, state)| {
+                self.len() - state.0 > self.config.maximum_transaction_count as usize
+                    && self.total_compute_mass - state.1 > self.config.maximum_transaction_compute_mass
+            })
+            .map(|(mtx, _)| {
+                if mtx.fee_rate() <= transaction.calculated_feerate().unwrap() {
+                    Ok(mtx.id())
                 } else {
-                    transactions_to_remove = low_priority_txs;
+                    let err = RuleError::RejectMempoolIsFull;
+                    warn!("{}", err);
+                    Err(err)
                 }
-            }
-        }
-
-        // An error is returned if the mempool is filled with high priority and other unremovable transactions.
-        let tx_count = self.len() + free_slots - transactions_to_remove.len();
-        if tx_count as u64 > self.config.maximum_transaction_count as u64 {
-            let err = RuleError::RejectMempoolIsFull(tx_count - free_slots, self.config.maximum_transaction_count as u64);
-            warn!("{}", err.to_string());
-            return Err(err);
-        }
+            })
+            .collect()
+    }
 
-        Ok(transactions_to_remove.iter().map(|x| x.id()).collect())
+    pub(crate) fn get_total_compute_mass(&self) -> u64 {
+        self.total_compute_mass
     }
 
     pub(crate) fn all_transaction_ids_with_priority(&self, priority: Priority) -> Vec<TransactionId> {

mining/src/mempool/validate_and_insert_transaction.rs

@@ -73,9 +73,19 @@ impl Mempool {
         self.validate_transaction_in_context(&transaction)?;
 
         // Before adding the transaction, check if there is room in the pool
-        self.transaction_pool.limit_transaction_count(1, &transaction)?.iter().try_for_each(|x| {
-            self.remove_transaction(x, true, TxRemovalReason::MakingRoom, format!(" for {}", transaction_id).as_str())
-        })?;
+        for x in self.transaction_pool.limit_transaction_count(&transaction)?.iter() {
+            self.remove_transaction(x, true, TxRemovalReason::MakingRoom, format!(" for {}", transaction_id).as_str())?;
+            // self.transaction_pool.limit_transaction_count(&transaction) returns the
+            // smallest prefix of `ready_transactions` (sorted by ascending fee-rate)
+            // that makes enough room for `transaction`, but since each call to `self.remove_transaction`
+            // also removes all transactions dependant on `x` we might already have enough room.
+            if self.transaction_pool.len() + 1 <= self.config.maximum_transaction_count
+                && self.transaction_pool.get_total_compute_mass() + transaction.calculated_compute_mass.unwrap()
+                    <= self.config.maximum_transaction_compute_mass
+            {
+                break;
+            }
+        }
 
         // Add the transaction to the mempool as a MempoolTransaction and return a clone of the embedded Arc<Transaction>
         let accepted_transaction =