From 277582438a5f458fbb7f665f756c2803260a2e8f Mon Sep 17 00:00:00 2001 From: Mykhailo Kremniov Date: Wed, 20 May 2026 11:42:32 +0300 Subject: [PATCH 1/6] mempool: introduce cluster size limits --- .../test-framework/src/transaction_builder.rs | 7 + common/src/chain/config/mod.rs | 6 +- mempool/src/config.rs | 10 + mempool/src/error/ban_score.rs | 46 ++- mempool/src/error/mod.rs | 71 ++-- .../src/interface/mempool_interface_impl.rs | 4 +- mempool/src/pool/mod.rs | 15 +- mempool/src/pool/tx_pool/collect_txs.rs | 19 +- mempool/src/pool/tx_pool/mod.rs | 47 ++- mempool/src/pool/tx_pool/store/mod.rs | 339 ++++++++++++++---- mempool/src/pool/tx_pool/tests/basic.rs | 170 +++++++-- mempool/src/pool/tx_pool/tests/mod.rs | 2 +- .../tests/tx_ids_by_score_and_ancestry.rs | 2 + mempool/src/pool/tx_pool/tests/utils.rs | 4 + node-lib/src/config_files/mempool.rs | 14 + .../tests/no_discouragement_after_tx_reorg.rs | 2 + p2p/src/sync/tests/tx_announcement.rs | 8 + wallet/src/account/mod.rs | 6 + 18 files changed, 587 insertions(+), 185 deletions(-) diff --git a/chainstate/test-framework/src/transaction_builder.rs b/chainstate/test-framework/src/transaction_builder.rs index 85adeeadc1..6623b05bf8 100644 --- a/chainstate/test-framework/src/transaction_builder.rs +++ b/chainstate/test-framework/src/transaction_builder.rs @@ -72,6 +72,13 @@ impl TransactionBuilder { self } + pub fn add_output_n_times(mut self, n: usize, output: &TxOutput) -> Self { + for _ in 0..n { + self = self.add_output(output.clone()) + } + self + } + pub fn outputs(&self) -> &[TxOutput] { &self.outputs } diff --git a/common/src/chain/config/mod.rs b/common/src/chain/config/mod.rs index f93ddb2a06..899a87eacb 100644 --- a/common/src/chain/config/mod.rs +++ b/common/src/chain/config/mod.rs @@ -522,7 +522,11 @@ impl ChainConfig { self.max_block_size_with_smart_contracts } - /// The maximum size of any transaction submitted to the node for the mempool + /// The maximum size of any transaction submitted to the node for the mempool. + /// + /// Note that this is just an upper limit - creating a tx bigger than this doesn't make much + /// sense, because it won't fit into a block. Normally, the cluster size limit (specified in + /// the mempool config) will be the actual limit. pub fn max_tx_size_for_mempool(&self) -> usize { // Reserve some space in the block for the data it needs to store beyond the transaction // data itself, namely the transaction count due to how sequences of elements are encoded. diff --git a/mempool/src/config.rs b/mempool/src/config.rs index 129c4596a2..679399c098 100644 --- a/mempool/src/config.rs +++ b/mempool/src/config.rs @@ -141,9 +141,19 @@ make_config_setting!( FeeRate::from_amount_per_kb(Amount::from_atoms(100_000_000_000)) ); +make_config_setting!(MaxClusterTxCount, usize, 64); +make_config_setting!(MaxClusterSizeBytes, usize, 100_000); + #[derive(Debug, Clone, Default)] pub struct MempoolConfig { + /// Minimum transaction relay fee rate (in atoms per 1000 bytes). pub min_tx_relay_fee_rate: MinTxRelayFeeRate, + + /// Maximum number of transactions that a single cluster can contain. + pub max_cluster_tx_count: MaxClusterTxCount, + + /// Maximum total size of transactions that is allowed in a single cluster. + pub max_cluster_size_bytes: MaxClusterSizeBytes, } impl MempoolConfig { diff --git a/mempool/src/error/ban_score.rs b/mempool/src/error/ban_score.rs index e7a7386bb6..ffd0ec2c48 100644 --- a/mempool/src/error/ban_score.rs +++ b/mempool/src/error/ban_score.rs @@ -24,7 +24,10 @@ use chainstate::{ }; use common::chain::IdCreationError; -use crate::error::{Error, MempoolPolicyError, ReorgError, TxCollectionError, TxValidationError}; +use crate::error::{ + Error, MempoolPolicyError, MempoolStoreError, MempoolStoreInvariantError, ReorgError, + TxCollectionError, TxValidationError, +}; /// Ban score for transactions pub trait MempoolBanScore { @@ -60,9 +63,13 @@ impl MempoolBanScore for MempoolPolicyError { // Basic transaction integrity checks failed, ban peer. MempoolPolicyError::NoInputs => 100, MempoolPolicyError::NoOutputs => 100, - MempoolPolicyError::ExceedsMaxBlockSize => 100, + MempoolPolicyError::TxSizeExceedsMaxBlockSize => 100, MempoolPolicyError::RelayFeeOverflow => 100, + // Don't punish the peer for this, because max cluster size is technically configurable + // and the peer doesn't know the node's mempool configuration. + MempoolPolicyError::TxSizeExceedsMaxClusterSize => 0, + // Errors to do with transaction conflicts and replacements are not punished since the // peer may not be aware of all the transactions the node has in the mempool. // This could be refined later. @@ -73,6 +80,11 @@ impl MempoolBanScore for MempoolPolicyError { MempoolPolicyError::ReplacementFeeLowerThanOriginal { .. } => 0, MempoolPolicyError::AdditionalFeesUnderflow => 0, + // Same as above, cluster size violations are not punished because the peer may not + // be aware of the node's mempool contents. + MempoolPolicyError::ClusterMaxTxCountLimitViolated { .. } + | MempoolPolicyError::ClusterTotalTxSizeLimitViolated { .. } => 0, + // Sending transactions with a fee below the minimum should not be punished. MempoolPolicyError::InsufficientFeesToRelay { .. } => 0, MempoolPolicyError::InsufficientFeesToRelayRBF => 0, @@ -85,8 +97,28 @@ impl MempoolBanScore for MempoolPolicyError { MempoolPolicyError::AncestorFeeOverflow => 0, MempoolPolicyError::AncestorFeeUpdateOverflow => 0, MempoolPolicyError::FeeOverflow => 0, - MempoolPolicyError::GetParentError => 0, MempoolPolicyError::DescendantOfExpiredTransaction => 0, + + MempoolPolicyError::MempoolStoreInvariantError(err) => err.mempool_ban_score(), + MempoolPolicyError::MempoolStoreError(err) => err.mempool_ban_score(), + } + } +} + +impl MempoolBanScore for MempoolStoreInvariantError { + fn mempool_ban_score(&self) -> u32 { + match self { + // This whole enum is supposed to contain only invariant violations, but just in case + // we explicitly list its variants here. + MempoolStoreInvariantError::SupposedlyExistingEntryNotFound(_) => 0, + } + } +} + +impl MempoolBanScore for MempoolStoreError { + fn mempool_ban_score(&self) -> u32 { + match self { + MempoolStoreError::TxEntryNotFound(_) => 0, } } } @@ -141,12 +173,8 @@ impl MempoolBanScore for ReorgError { impl MempoolBanScore for TxCollectionError { fn mempool_ban_score(&self) -> u32 { match self { - // This represents a function contract violation by the caller code. - TxCollectionError::SpecifiedTxNotFound(_) => 0, - - // These 2 are mempool invariant errors. - TxCollectionError::TxParentNotFound { .. } - | TxCollectionError::TxChildNotFound { .. } => 0, + TxCollectionError::MempoolStoreInvariantError(err) => err.mempool_ban_score(), + TxCollectionError::MempoolStoreError(err) => err.mempool_ban_score(), } } } diff --git a/mempool/src/error/mod.rs b/mempool/src/error/mod.rs index 2dd3725c3e..7f4ab57203 100644 --- a/mempool/src/error/mod.rs +++ b/mempool/src/error/mod.rs @@ -44,20 +44,11 @@ pub enum BlockConstructionError { /// block construction. #[derive(Debug, Clone, Error, PartialEq, Eq)] pub enum TxCollectionError { - #[error("Specified transaction {0:x} not found in mempool")] - SpecifiedTxNotFound(Id), - - #[error("Transaction {tx_id:x} has a parent {parent_tx_id:x} that is not in mempool")] - TxParentNotFound { - tx_id: Id, - parent_tx_id: Id, - }, + #[error(transparent)] + MempoolStoreInvariantError(#[from] MempoolStoreInvariantError), - #[error("Transaction {tx_id:x} has a child {child_tx_id:x} that is not in mempool")] - TxChildNotFound { - tx_id: Id, - child_tx_id: Id, - }, + #[error(transparent)] + MempoolStoreError(#[from] MempoolStoreError), } #[derive(Debug, Clone, Error, PartialEq, Eq)] @@ -95,14 +86,17 @@ pub enum MempoolPolicyError { #[error("Mempool is full")] MempoolFull, - #[error("Transaction has no inputs.")] + #[error("Transaction has no inputs")] NoInputs, - #[error("Transaction has no outputs.")] + #[error("Transaction has no outputs")] NoOutputs, - #[error("Transaction exceeds the maximum block size.")] - ExceedsMaxBlockSize, + #[error("Transaction size exceeds the maximum block size")] + TxSizeExceedsMaxBlockSize, + + #[error("Transaction size exceeds the maximum cluster size")] + TxSizeExceedsMaxClusterSize, #[error( "Replacement transaction has fee lower than the original. Replacement fee is {replacement_fee:?}, original fee {original_fee:?}" @@ -114,29 +108,29 @@ pub enum MempoolPolicyError { original_fee: Fee, }, - #[error("The sum of the fees of this transaction's conflicts overflows.")] + #[error("The sum of the fees of this transaction's conflicts overflows")] ConflictsFeeOverflow, #[error( - "Transaction pays a fee that is lower than the fee of its conflicts with their descendants." + "Transaction pays a fee that is lower than the fee of its conflicts with their descendants" )] TransactionFeeLowerThanConflictsWithDescendants, - #[error("Underflow in computing transaction's additional fees.")] + #[error("Underflow in computing transaction's additional fees")] AdditionalFeesUnderflow, #[error( - "Transaction does not pay sufficient fees to be relayed (tx_fee: {tx_fee}, min_relay_fee: {min_relay_fee})." + "Transaction does not pay sufficient fees to be relayed (tx_fee: {tx_fee}, min_relay_fee: {min_relay_fee})" )] InsufficientFeesToRelay { tx_fee: DisplayAmount, min_relay_fee: DisplayAmount, }, - #[error("Replacement transaction does not pay enough for its bandwidth.")] + #[error("Replacement transaction does not pay enough for its bandwidth")] InsufficientFeesToRelayRBF, - #[error("Rolling fee threshold not met (fee is {tx_fee}, minimum {minimum_fee}).")] + #[error("Rolling fee threshold not met (fee is {tx_fee}, minimum {minimum_fee})")] RollingFeeThresholdNotMet { minimum_fee: DisplayAmount, tx_fee: DisplayAmount, @@ -145,20 +139,41 @@ pub enum MempoolPolicyError { #[error("Overflow encountered while computing fee with ancestors")] AncestorFeeOverflow, - #[error("Overflow encountered while updating ancestor fee.")] + #[error("Overflow encountered while updating ancestor fee")] AncestorFeeUpdateOverflow, #[error("Fee overflow")] FeeOverflow, - #[error("Get parent error")] - GetParentError, - - #[error("Transaction is a descendant of expired transaction.")] + #[error("Transaction is a descendant of expired transaction")] DescendantOfExpiredTransaction, #[error("Relay fee overflow error")] RelayFeeOverflow, + + #[error("Cluster max transaction count limit ({limit}) violated")] + ClusterMaxTxCountLimitViolated { limit: usize }, + + #[error("Cluster total transaction size ({actual_size}) exceeds the limit ({limit})")] + ClusterTotalTxSizeLimitViolated { actual_size: usize, limit: usize }, + + #[error(transparent)] + MempoolStoreError(#[from] MempoolStoreError), + + #[error(transparent)] + MempoolStoreInvariantError(#[from] MempoolStoreInvariantError), +} + +#[derive(Debug, Clone, Error, PartialEq, Eq)] +pub enum MempoolStoreInvariantError { + #[error("Mempool entry for transaction {0:x} must exist but it wasn't found")] + SupposedlyExistingEntryNotFound(Id), +} + +#[derive(Debug, Clone, Error, PartialEq, Eq)] +pub enum MempoolStoreError { + #[error("Mempool entry for transaction {0:x} wasn't found")] + TxEntryNotFound(Id), } #[derive(Debug, Clone, Error, PartialEq, Eq)] diff --git a/mempool/src/interface/mempool_interface_impl.rs b/mempool/src/interface/mempool_interface_impl.rs index 2a2d16a16f..1405ba03a3 100644 --- a/mempool/src/interface/mempool_interface_impl.rs +++ b/mempool/src/interface/mempool_interface_impl.rs @@ -23,7 +23,7 @@ use common::{ }; use logging::log; use mempool_types::TransactionDuplicateStatus; -use utils::{const_value::ConstValue, debug_panic_or_log, tap_log::TapLog}; +use utils::{debug_panic_or_log, tap_log::TapLog}; use crate::{ FeeRate, MempoolInterface, MempoolMaxSize, TxOptions, TxStatus, @@ -42,7 +42,7 @@ type Mempool = crate::pool::Mempool; /// Contains all the information required to spin up the mempool subsystem pub struct MempoolInit { chain_config: Arc, - mempool_config: ConstValue, + mempool_config: MempoolConfig, chainstate_handle: chainstate::ChainstateHandle, time_getter: TimeGetter, } diff --git a/mempool/src/pool/mod.rs b/mempool/src/pool/mod.rs index 6099a35bfa..74a02e930a 100644 --- a/mempool/src/pool/mod.rs +++ b/mempool/src/pool/mod.rs @@ -23,16 +23,15 @@ use common::{ }; use logging::log; use utils::{ - const_value::ConstValue, debug_assert_or_log, ensure, eventhandler::EventsController, - shallow_clone::ShallowClone, + debug_assert_or_log, ensure, eventhandler::EventsController, shallow_clone::ShallowClone, }; use utils_networking::broadcaster; use crate::{ MempoolConfig, MempoolMaxSize, TxStatus, config, error::{ - BlockConstructionError, Error, MempoolPolicyError, OrphanPoolError, TxCollectionError, - TxValidationError, + BlockConstructionError, Error, MempoolPolicyError, MempoolStoreError, OrphanPoolError, + TxCollectionError, TxValidationError, }, event::{ MempoolEvent, NewTipEvent, make_local_duplicate_tx_event, make_new_tx_accepted_event, @@ -86,7 +85,7 @@ enum MempoolState { struct MempoolStateInIbd { chain_config: Arc, - mempool_config: ConstValue, + mempool_config: MempoolConfig, chainstate_handle: chainstate::ChainstateHandle, clock: TimeGetter, memory_usage_estimator: M, @@ -107,7 +106,7 @@ struct MempoolStateAfterIbd { impl Mempool { pub fn new( chain_config: Arc, - mempool_config: ConstValue, + mempool_config: MempoolConfig, chainstate_handle: chainstate::ChainstateHandle, clock: TimeGetter, memory_usage_estimator: M, @@ -121,7 +120,7 @@ impl Mempool { let mut this = Self(MempoolState::InIbd(MempoolStateInIbd { chain_config, - mempool_config, + mempool_config: mempool_config.into(), chainstate_handle, clock, memory_usage_estimator, @@ -459,7 +458,7 @@ impl Mempool { // The mempool is empty during IBD; return an error if `tx_ids` is non-empty, // so that the function's contract is the same in and out of IBD. if let Some(tx_id) = tx_ids.first() { - Err(TxCollectionError::SpecifiedTxNotFound(*tx_id)) + Err(MempoolStoreError::TxEntryNotFound(*tx_id).into()) } else { Ok(Vec::new()) } diff --git a/mempool/src/pool/tx_pool/collect_txs.rs b/mempool/src/pool/tx_pool/collect_txs.rs index 217a1a5e7f..9e7fdb47fa 100644 --- a/mempool/src/pool/tx_pool/collect_txs.rs +++ b/mempool/src/pool/tx_pool/collect_txs.rs @@ -297,10 +297,7 @@ pub fn get_best_tx_ids_by_score_and_ancestry( let mut ready_txs = BinaryHeap::::with_capacity(selected_tx_ids.len()); for tx_id in selected_tx_ids { - let entry = mempool - .store - .get_entry(tx_id) - .ok_or(TxCollectionError::SpecifiedTxNotFound(*tx_id))?; + let entry = mempool.store.get_specified_entry(tx_id)?; collect_nearest_selected_ancestors( mempool, @@ -344,12 +341,7 @@ pub fn get_best_tx_ids_by_score_and_ancestry( } 1 => { missing_selected_ancestors_count.remove(); - let child = mempool.store.get_entry(child_id).ok_or( - TxCollectionError::TxChildNotFound { - tx_id: *tx_id, - child_tx_id: *child_id, - }, - )?; + let child = mempool.store.get_existing_entry(child_id)?; ready_txs.push(child.into()); } missing_count => *missing_count -= 1, @@ -396,12 +388,7 @@ mod get_best_tx_ids_by_score_and_ancestry_impl { // The nearest selected ancestor has been found, no need to go deeper. tx_ancestors.insert(*parent_id); } else { - let parent_tx_entry = mempool.store.get_entry(parent_id).ok_or( - TxCollectionError::TxParentNotFound { - tx_id: *tx_id, - parent_tx_id: *parent_id, - }, - )?; + let parent_tx_entry = mempool.store.get_existing_entry(parent_id)?; collect_nearest_selected_ancestors( mempool, parent_tx_entry, diff --git a/mempool/src/pool/tx_pool/mod.rs b/mempool/src/pool/tx_pool/mod.rs index db5a09b730..1f79a5d346 100644 --- a/mempool/src/pool/tx_pool/mod.rs +++ b/mempool/src/pool/tx_pool/mod.rs @@ -47,7 +47,7 @@ use common::{ time_getter::TimeGetter, }; use logging::log; -use utils::{const_value::ConstValue, debug_panic_or_log, ensure, shallow_clone::ShallowClone}; +use utils::{debug_panic_or_log, ensure, shallow_clone::ShallowClone}; use utxo::UtxosStorageRead; use crate::{ @@ -76,7 +76,7 @@ use self::{ pub struct TxPool { chain_config: Arc, - mempool_config: ConstValue, + mempool_config: Arc, store: MempoolStore, rolling_fee_rate: RwLock, max_size: config::MempoolMaxSize, @@ -96,22 +96,22 @@ impl std::fmt::Debug for TxPool { impl TxPool { pub fn new( chain_config: Arc, - mempool_config: ConstValue, + mempool_config: MempoolConfig, chainstate_handle: chainstate::ChainstateHandle, clock: TimeGetter, memory_usage_estimator: M, ) -> Self { - log::trace!("Setting up mempool transaction verifier"); let tx_verifier = tx_verifier::create( chain_config.shallow_clone(), chainstate_handle.shallow_clone(), ); + let mempool_config = Arc::new(mempool_config); + let store = MempoolStore::new(Arc::clone(&mempool_config)); - log::trace!("Creating mempool object"); Self { chain_config, mempool_config, - store: MempoolStore::new(), + store, chainstate_handle, max_size: Self::initial_max_size(), max_tx_age: config::DEFAULT_MEMPOOL_EXPIRY, @@ -153,7 +153,11 @@ impl TxPool { self.chainstate_handle.shallow_clone(), ); - std::mem::replace(&mut self.store, MempoolStore::new()).into_transactions() + std::mem::replace( + &mut self.store, + MempoolStore::new(Arc::clone(&self.mempool_config)), + ) + .into_transactions() } pub fn is_ibd(&self) -> bool { @@ -268,8 +272,18 @@ impl TxPool { ensure!(has_outputs, MempoolPolicyError::NoOutputs); let size = entry.size().get(); - let max_size = self.chain_config.max_tx_size_for_mempool(); - ensure!(size <= max_size, MempoolPolicyError::ExceedsMaxBlockSize); + let absolute_max_size = self.chain_config.max_tx_size_for_mempool(); + ensure!( + size <= absolute_max_size, + MempoolPolicyError::TxSizeExceedsMaxBlockSize + ); + + // A tx cannot be bigger than a cluster. + let max_cluster_size = *self.mempool_config.max_cluster_size_bytes; + ensure!( + size <= max_cluster_size, + MempoolPolicyError::TxSizeExceedsMaxClusterSize + ); Ok(()) } @@ -540,7 +554,7 @@ impl TxPool { ) -> StoreHashSet> { conflicts .iter() - .flat_map(|conflict| conflict.unconfirmed_descendants(&self.store).take()) + .flat_map(|conflict| conflict.collect_descendants(&self.store).take()) .chain(conflicts.iter().map(|conflict| *conflict.tx_id())) .collect() } @@ -548,10 +562,18 @@ impl TxPool { // Transaction Finalization impl TxPool { - fn finalize_tx(&mut self, entry: TxEntryWithFee) -> Result<(), Error> { + fn finalize_tx( + &mut self, + entry: TxEntryWithFee, + tx_verifier_delta: TransactionVerifierDelta, + ) -> Result<(), Error> { let tx_id = *entry.tx_id(); self.store.add_transaction(entry)?; + // Note: the call to `add_transaction` above can fail e.g. if the cluster-related limits + // are violated. So we flush the delta to storage only after `add_transaction` has succeeded. + tx_verifier::flush_to_storage(&mut self.tx_verifier, tx_verifier_delta)?; + self.remove_expired_transactions(); ensure!( self.store.contains(&tx_id), @@ -814,8 +836,7 @@ impl TxPool { if config::ENABLE_RBF { self.store.drop_conflicts(conflicts); } - tx_verifier::flush_to_storage(&mut self.tx_verifier, delta)?; - self.finalize_tx(tx)?; + self.finalize_tx(tx, delta)?; self.store.assert_valid(); Ok(TxAdditionAttemptOutcome::Added) diff --git a/mempool/src/pool/tx_pool/store/mod.rs b/mempool/src/pool/tx_pool/store/mod.rs index a8daff2619..77c5375454 100644 --- a/mempool/src/pool/tx_pool/store/mod.rs +++ b/mempool/src/pool/tx_pool/store/mod.rs @@ -20,6 +20,7 @@ use std::{ collections::{BTreeMap, BTreeSet}, num::NonZeroUsize, ops::Deref, + sync::Arc, }; use common::{ @@ -27,13 +28,13 @@ use common::{ primitives::Id, }; use logging::log; -use utils::{debug_assert_or_log, newtype}; +use utils::{debug_assert_or_log, ensure, newtype}; use super::{Fee, Time, TxEntry, TxEntryWithFee}; use crate::{ - FeeRate, - error::MempoolPolicyError, + FeeRate, MempoolConfig, + error::{MempoolPolicyError, MempoolStoreError, MempoolStoreInvariantError}, pool::{entry::TxDependency, tx_pool::store::mem_usage::MemUsageTracker}, }; @@ -54,19 +55,30 @@ pub type StoreHashMap = pub type StoreHashSet = hashbrown::hash_set::HashSet; newtype! { + /// A set of ids of in-mempool (or "unconfirmed") ancestors of a certain tx. #[derive(Debug)] pub struct Ancestors(StoreHashSet>); } -impl Ancestors { - pub fn len(&self) -> usize { - self.0.len() - } +newtype! { + /// A set of ids of descendants of a certain tx. + #[derive(Debug)] + pub struct Descendants(StoreHashSet>); } newtype! { + /// A set of ids of txs that will form a cluster after a certain new tx is added to the mempool + /// (the new tx id itself is not included). + /// + /// A cluster is a connected component of the in-mempool dependency graph. #[derive(Debug)] - pub struct Descendants(StoreHashSet>); + pub struct NewTxCluster(StoreHashSet>); +} + +newtype! { + /// A set of ids of txs that currently form a cluster in the mempool. + #[derive(Debug)] + pub struct Cluster(StoreHashSet>); } newtype! { @@ -103,6 +115,9 @@ pub type TrackedTxIdMultiMap = TrackedSet<(K, Id)>; #[derive(Debug)] pub struct MempoolStore { + /// Mempool config + mempool_config: Arc, + // This is the "main" data structure storing Mempool entries. All other structures in the // MempoolStore contain ids (hashes) of entries, sorted according to some order of interest. // (Note: TxMempoolEntry is boxed, because the hashbrown table stores items directly @@ -167,8 +182,9 @@ pub enum MempoolRemovalReason { } impl MempoolStore { - pub fn new() -> Self { + pub fn new(mempool_config: Arc) -> Self { Self { + mempool_config, txs_by_descendant_score: Tracked::default(), txs_by_ancestor_score: Tracked::default(), txs_by_id: Tracked::default(), @@ -189,6 +205,31 @@ impl MempoolStore { self.txs_by_id.get(id).map(|tx| tx.as_ref()) } + /// A helper function to reduce the noise of error mapping. + /// + /// The tx is supposed to be present in the mempool by construction, so it's an invariant + /// error if it's not found. + pub fn get_existing_entry( + &self, + id: &Id, + ) -> Result<&TxMempoolEntry, MempoolStoreInvariantError> { + self.get_entry(id) + .ok_or(MempoolStoreInvariantError::SupposedlyExistingEntryNotFound( + *id, + )) + } + + /// A helper function to reduce the noise of error mapping. + /// + /// The tx id is supposed to come from user code, so it's not an invariant violation if + /// the tx can't be found. + pub fn get_specified_entry( + &self, + id: &Id, + ) -> Result<&TxMempoolEntry, MempoolStoreError> { + self.get_entry(id).ok_or(MempoolStoreError::TxEntryNotFound(*id)) + } + pub fn contains(&self, id: &Id) -> bool { self.txs_by_id.contains_key(id) } @@ -317,7 +358,7 @@ impl MempoolStore { } fn update_ancestor_state_for_drop(&mut self, entry: &TxMempoolEntry) { - for ancestor in entry.unconfirmed_ancestors(self).0 { + for ancestor in entry.collect_ancestors(self).0 { self.mem_tracker.modify(&mut self.txs_by_id, |txs_by_id, tracker| { tracker.modify( txs_by_id.get_mut(&ancestor).expect("ancestor"), @@ -453,7 +494,7 @@ impl MempoolStore { } fn update_descendant_state_for_drop(&mut self, entry: &TxMempoolEntry) { - for descendant_id in entry.unconfirmed_descendants(self).0 { + for descendant_id in entry.collect_descendants(self).0 { self.mem_tracker.modify(&mut self.txs_by_id, |by_id, tracker| { tracker.modify( by_id.get_mut(&descendant_id).expect("descendant"), @@ -581,6 +622,33 @@ impl MempoolStore { MemUsageTracker::forget(txs_by_id.remove(&id).expect("entry must be present")).entry }) } + + /// Collect all ancestors of the specified existing transaction. + /// Mainly intended for testing. + pub fn collect_ancestors( + &self, + tx_id: &Id, + ) -> Result { + let entry = self.get_existing_entry(tx_id)?; + Ok(entry.collect_ancestors(&self)) + } + + /// Collect all descendants of the specified existing transaction. + /// Mainly intended for testing. + pub fn collect_descendants( + &self, + tx_id: &Id, + ) -> Result { + let entry = self.get_existing_entry(tx_id)?; + Ok(entry.collect_descendants(&self)) + } + + /// Collect the cluster that the specified existing transaction belongs to. + /// Mainly intended for testing. + pub fn collect_cluster(&self, tx_id: &Id) -> Result { + let entry = self.get_existing_entry(tx_id)?; + Ok(entry.collect_cluster(&self)?) + } } #[cfg(test)] @@ -715,7 +783,6 @@ impl TxMempoolEntry { } pub fn size(&self) -> NonZeroUsize { - // TODO(Roy) this should follow Bitcoin's GetTxSize, which weighs in sigops, etc. self.entry.size() } @@ -723,7 +790,8 @@ impl TxMempoolEntry { self.entry.creation_time() } - // Note: only the parents that are currently in the mempool are included here. + // Note: only the parents that are currently in the mempool are included here (i.e. the + // "unconfirmed" parents). pub fn parents(&self) -> impl Iterator> { self.parents.iter() } @@ -742,42 +810,11 @@ impl TxMempoolEntry { pub fn is_replaceable(&self, store: &MempoolStore) -> bool { self.entry.transaction().is_replaceable() - || self.unconfirmed_ancestors(store).0.iter().any(|ancestor| { + || self.collect_ancestors(store).0.iter().any(|ancestor| { store.get_entry(ancestor).expect("entry").entry.transaction().is_replaceable() }) } - pub fn unconfirmed_ancestors(&self, store: &MempoolStore) -> Ancestors { - let mut visited = Ancestors(Default::default()); - self.unconfirmed_ancestors_inner(&mut visited, store); - visited - } - - pub fn unconfirmed_ancestors_from_parents( - parents: &StoreHashSet>, - store: &MempoolStore, - ) -> Result { - let mut ancestors = parents.clone().into(); - for parent in parents { - let parent = store.get_entry(parent).ok_or(MempoolPolicyError::GetParentError)?; - parent.unconfirmed_ancestors_inner(&mut ancestors, store); - } - Ok(ancestors) - } - - fn unconfirmed_ancestors_inner(&self, visited: &mut Ancestors, store: &MempoolStore) { - // TODO: change this from recursive to iterative - visited.reserve(self.count_with_ancestors - 1); - for parent in self.parents.iter() { - if visited.insert(*parent) { - store - .get_entry(parent) - .expect("entry") - .unconfirmed_ancestors_inner(visited, store); - } - } - } - pub fn depth_postorder_descendants<'a>( &'a self, store: &'a MempoolStore, @@ -791,22 +828,61 @@ impl TxMempoolEntry { utils::graph_traversals::dag_depth_postorder(self, children_fn) } - pub fn unconfirmed_descendants(&self, store: &MempoolStore) -> Descendants { - let mut visited = Descendants(Default::default()); - self.unconfirmed_descendants_inner(&mut visited, store); - visited + /// Collect ancestors of this transaction. + /// + /// The transaction itself may not be in the store. + pub fn collect_ancestors(&self, store: &MempoolStore) -> Ancestors { + let result = collect_relatives( + store, + self.parents.iter().copied(), + RelativesKind::Ancestors, + |_| Ok::<_, MempoolStoreInvariantError>(()), + ); + + match result { + Ok(ancestors) => Ancestors::new(ancestors), + Err(MempoolStoreInvariantError::SupposedlyExistingEntryNotFound(tx_id)) => { + // Note: this panic existed here for ages, but in the form of a direct call + // of `expect` after `get_entry`. + // TODO: it's better to get rid of this panic and all `get_entry().expect()` + // calls, which are still abundant. + panic!("Tx with id {tx_id:x} not found in mempool"); + } + } } - fn unconfirmed_descendants_inner(&self, visited: &mut Descendants, store: &MempoolStore) { - for child in self.children.iter() { - if visited.insert(*child) { - store - .get_entry(child) - .expect("entry") - .unconfirmed_descendants_inner(visited, store); + /// Collect descendants of this transaction. + /// + /// The transaction itself may not be in the store. + pub fn collect_descendants(&self, store: &MempoolStore) -> Descendants { + let result = collect_relatives( + store, + self.children.iter().copied(), + RelativesKind::Descendants, + |_| Ok::<_, MempoolStoreInvariantError>(()), + ); + + match result { + Ok(descendants) => Descendants::new(descendants), + Err(MempoolStoreInvariantError::SupposedlyExistingEntryNotFound(tx_id)) => { + // Same note/TODO as in collect_ancestors. + panic!("Tx with id {tx_id:x} not found in mempool"); } } } + + /// Collect the cluster that this transaction belongs to. + /// + /// The transaction itself *must* be in the store. + /// + /// This function is mainly intended for testing purposes. + pub fn collect_cluster(&self, store: &MempoolStore) -> Result { + let cluster = collect_relatives(store, [*self.tx_id()], RelativesKind::Cluster, |_| { + Ok::<_, MempoolPolicyError>(()) + })?; + + Ok(Cluster::new(cluster)) + } } #[allow(clippy::non_canonical_partial_ord_impl)] @@ -836,20 +912,30 @@ pub struct TxMempoolEntryWithAncestors { impl TxMempoolEntryWithAncestors { pub fn new(store: &MempoolStore, entry: TxEntryWithFee) -> Result { - // Genesis transaction has no parent, hence the first filter_map - let parents = entry - .transaction() - .inputs() - .iter() - .filter_map(|input| match input { - TxInput::Utxo(outpoint) => outpoint.source_id().get_tx_id().cloned(), - TxInput::Account(..) - | TxInput::AccountCommand(..) - | TxInput::OrderAccountCommand(..) => None, - }) - .filter(|id| store.txs_by_id.contains_key(id)) - .collect::>(); - let ancestors = TxMempoolEntry::unconfirmed_ancestors_from_parents(&parents, store)?; + let parents = collect_tx_parents(store, entry.transaction().transaction()); + + // Collect the cluster first, checking its size on each iteration. + // After that, the ancestors may be collected without the tx count check. + // Note: it's technically possible to unite ancestor and cluster collecting, to reduce + // the total number of steps during traversal. But it's probably not worth the extra + // complexity. + let cluster = NewTxCluster::new(collect_relatives( + store, + parents.iter().copied(), + RelativesKind::Cluster, + |collected_size| { + ensure_cluster_tx_count_limit(&store.mempool_config, 1 + collected_size) + }, + )?); + enforce_cluster_size_limit(store, &entry, &cluster)?; + + let ancestors = Ancestors::new(collect_relatives( + store, + parents.iter().copied(), + RelativesKind::Ancestors, + |_| Ok::<_, MempoolPolicyError>(()), + )?); + let ancestor_entries_iter = ancestors .deref() .iter() @@ -861,7 +947,7 @@ impl TxMempoolEntryWithAncestors { #[cfg(test)] pub fn new_from_existing_entry(store: &MempoolStore, entry: TxMempoolEntry) -> Self { - let ancestors = entry.unconfirmed_ancestors(store); + let ancestors = entry.collect_ancestors(store); Self { entry, ancestors } } @@ -877,3 +963,114 @@ impl TxMempoolEntryWithAncestors { &self.ancestors } } + +#[derive(Clone, Copy)] +pub enum RelativesKind { + Ancestors, + Descendants, + Cluster, +} + +/// Collect relatives of the specified "initial" transactions. +/// +/// At the end the result will contain the initial tx ids and, depending on `kind`: +/// a) their ancestors, +/// b) their descendants, +/// c) the union of clusters that they belong to (if initial_tx_ids are parents of a new tx, +/// then it's the cluster that will form after the tx is added to the mempool). +/// +/// After each tx is added to the result, `on_new_tx_added` is called with the current size +/// of the result as the argument. +pub fn collect_relatives( + store: &MempoolStore, + initial_tx_ids: impl IntoIterator>, + kind: RelativesKind, + mut on_new_tx_added: impl FnMut(/*cur_result_size:*/ usize) -> Result<(), E>, +) -> Result>, E> +where + E: From, +{ + let mut stack = Vec::new(); + let mut result = StoreHashSet::default(); + + let mut visit = |stack: &mut Vec>, tx_id: &Id| -> Result<(), E> { + if result.insert(*tx_id) { + on_new_tx_added(result.len())?; + stack.push(*tx_id); + } + Ok(()) + }; + + for tx_id in initial_tx_ids { + visit(&mut stack, &tx_id)?; + } + + while let Some(tx_id) = stack.pop() { + let entry = store.get_existing_entry(&tx_id)?; + + let (iter1, iter2) = match kind { + RelativesKind::Ancestors => (Some(entry.parents()), None), + RelativesKind::Descendants => (None, Some(entry.children())), + RelativesKind::Cluster => (Some(entry.parents()), Some(entry.children())), + }; + for tx_id in iter1.into_iter().flatten().chain(iter2.into_iter().flatten()) { + visit(&mut stack, tx_id)?; + } + } + + Ok(result) +} + +fn collect_tx_parents(store: &MempoolStore, tx: &Transaction) -> StoreHashSet> { + // Genesis transaction has no parent, hence the first filter_map + tx.inputs() + .iter() + .filter_map(|input| match input { + TxInput::Utxo(outpoint) => outpoint.source_id().get_tx_id().cloned(), + TxInput::Account(..) + | TxInput::AccountCommand(..) + | TxInput::OrderAccountCommand(..) => None, + }) + .filter(|id| store.txs_by_id.contains_key(id)) + .collect::>() +} + +fn ensure_cluster_tx_count_limit( + mempool_config: &MempoolConfig, + cluster_size: usize, +) -> Result<(), MempoolPolicyError> { + let limit = *mempool_config.max_cluster_tx_count; + ensure!( + cluster_size <= limit, + MempoolPolicyError::ClusterMaxTxCountLimitViolated { limit } + ); + Ok(()) +} + +fn enforce_cluster_size_limit( + store: &MempoolStore, + new_tx_entry: &TxEntryWithFee, + cluster: &NewTxCluster, +) -> Result<(), MempoolPolicyError> { + let cluster_size_bytes = { + let mut total_size = new_tx_entry.tx_entry().size().get(); + + for tx_id in cluster.iter() { + let entry = store.get_existing_entry(tx_id)?; + total_size += entry.size().get(); + } + + total_size + }; + + let limit = *store.mempool_config.max_cluster_size_bytes; + ensure!( + cluster_size_bytes <= limit, + MempoolPolicyError::ClusterTotalTxSizeLimitViolated { + actual_size: cluster_size_bytes, + limit + } + ); + + Ok(()) +} diff --git a/mempool/src/pool/tx_pool/tests/basic.rs b/mempool/src/pool/tx_pool/tests/basic.rs index 73b54c049e..c35c36c64c 100644 --- a/mempool/src/pool/tx_pool/tests/basic.rs +++ b/mempool/src/pool/tx_pool/tests/basic.rs @@ -323,38 +323,76 @@ async fn outpoint_not_found(#[case] seed: Seed) -> anyhow::Result<()> { Ok(()) } +// Create a tx bigger than `ChainConfig::max_tx_size_for_mempool`. #[rstest] #[trace] #[case(Seed::from_entropy())] #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn tx_too_big(#[case] seed: Seed) -> anyhow::Result<()> { +async fn tx_bigger_than_block_size(#[case] seed: Seed) -> anyhow::Result<()> { let mut rng = make_seedable_rng(seed); let tf = TestFramework::builder(&mut rng).build(); let genesis = tf.genesis(); - let single_output_size = TxOutput::Transfer( + let output = TxOutput::Transfer( OutputValue::Coin(Amount::from_atoms(100)), Destination::AnyoneCanSpend, - ) - .encoded_size(); - let too_many_outputs = - tf.chainstate.get_chain_config().max_tx_size_for_mempool() / single_output_size; - let mut tx_builder = TransactionBuilder::new().add_input( - TxInput::from_utxo(OutPointSourceId::BlockReward(genesis.get_id().into()), 0), - empty_witness(&mut rng), ); - for _ in 0..too_many_outputs { - tx_builder = tx_builder.add_output(TxOutput::Transfer( - OutputValue::Coin(Amount::from_atoms(100)), - Destination::AnyoneCanSpend, - )) - } - let tx = tx_builder.build(); + let output_size = output.encoded_size(); + let outputs_count = + tf.chainstate.get_chain_config().max_tx_size_for_mempool() / output_size + 1; + let tx = TransactionBuilder::new() + .add_input( + TxInput::from_utxo(OutPointSourceId::BlockReward(genesis.get_id().into()), 0), + empty_witness(&mut rng), + ) + .add_output_n_times(outputs_count, &output) + .build(); let mut mempool = setup_with_chainstate(tf.chainstate()); assert_eq!( mempool.add_transaction_test(tx), - Err(MempoolPolicyError::ExceedsMaxBlockSize.into()) + Err(MempoolPolicyError::TxSizeExceedsMaxBlockSize.into()) + ); + mempool.store.assert_valid(); + Ok(()) +} + +// Create a tx bigger than the cluster size specified in the mempool config. +#[rstest] +#[trace] +#[case(Seed::from_entropy())] +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn tx_bigger_than_cluster_size(#[case] seed: Seed) -> anyhow::Result<()> { + let mut rng = make_seedable_rng(seed); + let tf = TestFramework::builder(&mut rng).build(); + let genesis = tf.genesis(); + + let max_cluster_size = rng.random_range(10_000..500_000); + let output = TxOutput::Transfer( + OutputValue::Coin(Amount::from_atoms(100)), + Destination::AnyoneCanSpend, + ); + let output_size = output.encoded_size(); + let outputs_count = max_cluster_size / output_size + 1; + let tx = TransactionBuilder::new() + .add_input( + TxInput::from_utxo(OutPointSourceId::BlockReward(genesis.get_id().into()), 0), + empty_witness(&mut rng), + ) + .add_output_n_times(outputs_count, &output) + .build(); + let mempool_config = MempoolConfig { + min_tx_relay_fee_rate: TEST_MIN_TX_RELAY_FEE_RATE.into(), + max_cluster_size_bytes: max_cluster_size.into(), + + max_cluster_tx_count: Default::default(), + }; + let mut mempool = + setup_with_chainstate_generic(tf.chainstate(), mempool_config, Default::default()); + + assert_eq!( + mempool.add_transaction_test(tx), + Err(MempoolPolicyError::TxSizeExceedsMaxClusterSize.into()) ); mempool.store.assert_valid(); Ok(()) @@ -464,12 +502,12 @@ async fn tx_mempool_entry() -> anyhow::Result<()> { let entry4 = mempool.store.get_entry(ids.get(3).expect("index")).expect("entry"); let entry5 = mempool.store.get_entry(ids.get(4).expect("index")).expect("entry"); let entry6 = mempool.store.get_entry(ids.get(5).expect("index")).expect("entry"); - assert_eq!(entry1.unconfirmed_ancestors(&mempool.store).len(), 0); - assert_eq!(entry2.unconfirmed_ancestors(&mempool.store).len(), 0); - assert_eq!(entry3.unconfirmed_ancestors(&mempool.store).len(), 2); - assert_eq!(entry4.unconfirmed_ancestors(&mempool.store).len(), 3); - assert_eq!(entry5.unconfirmed_ancestors(&mempool.store).len(), 3); - assert_eq!(entry6.unconfirmed_ancestors(&mempool.store).len(), 5); + assert_eq!(entry1.collect_ancestors(&mempool.store).len(), 0); + assert_eq!(entry2.collect_ancestors(&mempool.store).len(), 0); + assert_eq!(entry3.collect_ancestors(&mempool.store).len(), 2); + assert_eq!(entry4.collect_ancestors(&mempool.store).len(), 3); + assert_eq!(entry5.collect_ancestors(&mempool.store).len(), 3); + assert_eq!(entry6.collect_ancestors(&mempool.store).len(), 5); assert_eq!(entry1.fees_with_ancestors(), Amount::from_atoms(1).into()); assert_eq!(entry2.fees_with_ancestors(), Amount::from_atoms(1).into()); @@ -1285,7 +1323,7 @@ async fn mempool_full_real(#[case] seed: Seed) { // Get total memory size without memory limit let memory_size = { - let mut storage = MempoolStore::new(); + let mut storage = MempoolStore::new(Default::default()); for entry in &txs { storage.add_transaction(entry.clone()).expect("tx insertion to succeed"); log::trace!("Storage mem usage updated: {}", storage.memory_usage()); @@ -1361,13 +1399,22 @@ async fn no_empty_bags_in_indices(#[case] seed: Seed) -> anyhow::Result<()> { )); } let parent = tx_builder.build(); - let mut mempool = setup_with_chainstate(tf.chainstate()); + let num_child_txs = num_outputs; + + let mempool_config = MempoolConfig { + min_tx_relay_fee_rate: TEST_MIN_TX_RELAY_FEE_RATE.into(), + // Make sure we don't hit the max cluster tx count limit. + max_cluster_tx_count: (num_child_txs + 1).into(), + max_cluster_size_bytes: Default::default(), + }; + let mut mempool = + setup_with_chainstate_generic(tf.chainstate(), mempool_config, Default::default()); let parent_id = parent.transaction().get_id(); let outpoint_source_id = OutPointSourceId::Transaction(parent.transaction().get_id()); mempool.add_transaction_test(parent)?.assert_in_mempool(); - let num_child_txs = num_outputs; + let flags = 0; let fee = get_relay_fee_from_tx_size(estimate_tx_size(1, num_outputs)); let mut txs = Vec::new(); @@ -1399,6 +1446,11 @@ async fn no_empty_bags_in_indices(#[case] seed: Seed) -> anyhow::Result<()> { Ok(()) } +// If use_cluster_size_limit is false, the max cluster size will be set to some artificially large +// value, so the test tx size values will be checked against `ChainConfig::max_tx_size_for_mempool`. +// Otherwise, `ChainConfig::max_tx_size_for_mempool` will be set to some artificially large value +// and the max cluster size will be set to the default value of `ChainConfig::max_tx_size_for_mempool`, +// so the test tx size values will be checked against the max cluster size. #[rstest] #[case(Seed::from_entropy(), 300, true)] #[case(Seed::from_entropy(), 1_000, true)] @@ -1413,15 +1465,48 @@ async fn no_empty_bags_in_indices(#[case] seed: Seed) -> anyhow::Result<()> { #[case::at_block_limit(Seed::from_entropy(), 1_048_576, false)] #[trace] #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn accepted_tx_size(#[case] seed: Seed, #[case] tx_size: usize, #[case] accept: bool) { +async fn accepted_tx_size( + #[case] seed: Seed, + #[case] tx_size: usize, + #[case] accept: bool, + #[values(false, true)] use_cluster_size_limit: bool, +) { let mut rng = make_seedable_rng(seed); + let default_max_tx_size_from_blocks = + chain::config::create_unit_test_config().max_tx_size_for_mempool(); + + // Note: we can't modify `max_tx_size_for_mempool` directly, but its value depends on the "max_block_size_" + // values, so we modify them instead. + let large_limit = default_max_tx_size_from_blocks * 2; + let (max_block_size_with_standard_txs, max_block_size_with_smart_contracts, max_cluster_size) = + if use_cluster_size_limit { + ( + Some(large_limit), + Some(large_limit), + default_max_tx_size_from_blocks, + ) + } else { + (None, None, large_limit) + }; + let tf = { - let chain_config = - common::chain::config::Builder::new(common::chain::config::ChainType::Regtest) - .data_deposit_max_size(Some(2_000_000)) - .build(); - TestFramework::builder(&mut rng).with_chain_config(chain_config).build() + let mut chain_config_builder = + chain::config::create_unit_test_config_builder().data_deposit_max_size(Some(2_000_000)); + + if let Some(max_block_size_with_standard_txs) = max_block_size_with_standard_txs { + chain_config_builder = chain_config_builder + .max_block_size_with_standard_txs(max_block_size_with_standard_txs); + } + + if let Some(max_block_size_with_smart_contracts) = max_block_size_with_smart_contracts { + chain_config_builder = chain_config_builder + .max_block_size_with_smart_contracts(max_block_size_with_smart_contracts); + } + + TestFramework::builder(&mut rng) + .with_chain_config(chain_config_builder.build()) + .build() }; let transaction = { @@ -1451,18 +1536,29 @@ async fn accepted_tx_size(#[case] seed: Seed, #[case] tx_size: usize, #[case] ac transaction }; - let max_tx_size = tf.chain_config().max_tx_size_for_mempool(); - let mut mempool = setup_with_chainstate(tf.chainstate()); + let mempool_config = MempoolConfig { + min_tx_relay_fee_rate: TEST_MIN_TX_RELAY_FEE_RATE.into(), + max_cluster_size_bytes: max_cluster_size.into(), + + max_cluster_tx_count: Default::default(), + }; + let mut mempool = + setup_with_chainstate_generic(tf.chainstate(), mempool_config, Default::default()); let result = mempool.add_transaction_test(transaction); + let expected_err = if use_cluster_size_limit { + MempoolPolicyError::TxSizeExceedsMaxClusterSize + } else { + MempoolPolicyError::TxSizeExceedsMaxBlockSize + }; let expected = match accept { true => Ok(TxStatus::InMempool), - false => Err(Error::Policy(MempoolPolicyError::ExceedsMaxBlockSize)), + false => Err(Error::Policy(expected_err)), }; assert_eq!( result, expected, - "tx_size: {tx_size}, max tx size: {max_tx_size}" + "tx_size: {tx_size}, max tx size: {default_max_tx_size_from_blocks}" ); } @@ -1482,6 +1578,8 @@ async fn initial_values(#[case] seed: Seed) { rng.random_range(100_000..100_000_000_000_000), ) .into(), + max_cluster_tx_count: Default::default(), + max_cluster_size_bytes: Default::default(), }; let tx_pool = setup_with_chainstate_generic(tf.chainstate(), mempool_config.clone(), Default::default()); diff --git a/mempool/src/pool/tx_pool/tests/mod.rs b/mempool/src/pool/tx_pool/tests/mod.rs index 2a5ad1e2b0..349a476d4c 100644 --- a/mempool/src/pool/tx_pool/tests/mod.rs +++ b/mempool/src/pool/tx_pool/tests/mod.rs @@ -28,7 +28,7 @@ use chainstate::{ }; use common::{ chain::{ - OutPointSourceId, Transaction, UtxoOutPoint, + self, OutPointSourceId, Transaction, UtxoOutPoint, block::{Block, BlockReward, ConsensusData, timestamp::BlockTimestamp}, config::ChainConfig, output_value::OutputValue, diff --git a/mempool/src/pool/tx_pool/tests/tx_ids_by_score_and_ancestry.rs b/mempool/src/pool/tx_pool/tests/tx_ids_by_score_and_ancestry.rs index 47c8af6953..eaf5f24fa5 100644 --- a/mempool/src/pool/tx_pool/tests/tx_ids_by_score_and_ancestry.rs +++ b/mempool/src/pool/tx_pool/tests/tx_ids_by_score_and_ancestry.rs @@ -50,6 +50,8 @@ async fn tx_ids_by_score_and_ancestry(#[case] seed: Seed) { tf.chainstate(), MempoolConfig { min_tx_relay_fee_rate: FeeRate::from_amount_per_kb(Amount::ZERO).into(), + max_cluster_tx_count: Default::default(), + max_cluster_size_bytes: Default::default(), }, Default::default(), ); diff --git a/mempool/src/pool/tx_pool/tests/utils.rs b/mempool/src/pool/tx_pool/tests/utils.rs index dc20c8bcc1..192c71f067 100644 --- a/mempool/src/pool/tx_pool/tests/utils.rs +++ b/mempool/src/pool/tx_pool/tests/utils.rs @@ -45,6 +45,8 @@ pub const TEST_MIN_TX_RELAY_FEE_RATE: FeeRate = pub fn create_mempool_config() -> MempoolConfig { MempoolConfig { min_tx_relay_fee_rate: TEST_MIN_TX_RELAY_FEE_RATE.into(), + max_cluster_tx_count: Default::default(), + max_cluster_size_bytes: Default::default(), } } @@ -328,6 +330,8 @@ pub fn setup_with_min_tx_relay_fee_rate(fee_rate: FeeRate) -> TxPool, + + /// Maximum number of transactions that a single cluster can contain. + pub max_cluster_tx_count: Option, + + /// Maximum total size of transactions that is allowed in a single cluster. + pub max_cluster_size_bytes: Option, } impl MempoolConfigFile { @@ -37,12 +43,16 @@ impl MempoolConfigFile { pub fn with_run_options(config: MempoolConfigFile, options: &RunOptions) -> MempoolConfigFile { let MempoolConfigFile { min_tx_relay_fee_rate, + max_cluster_tx_count, + max_cluster_size_bytes, } = config; let min_tx_relay_fee_rate = min_tx_relay_fee_rate.or(options.min_tx_relay_fee_rate); MempoolConfigFile { min_tx_relay_fee_rate, + max_cluster_tx_count, + max_cluster_size_bytes, } } } @@ -51,12 +61,16 @@ impl From for MempoolConfig { fn from(config_file: MempoolConfigFile) -> Self { let MempoolConfigFile { min_tx_relay_fee_rate, + max_cluster_tx_count, + max_cluster_size_bytes, } = config_file; Self { min_tx_relay_fee_rate: min_tx_relay_fee_rate .map(|val| FeeRate::from_amount_per_kb(Amount::from_atoms(val.into()))) .into(), + max_cluster_tx_count: max_cluster_tx_count.into(), + max_cluster_size_bytes: max_cluster_size_bytes.into(), } } } diff --git a/p2p/src/sync/tests/no_discouragement_after_tx_reorg.rs b/p2p/src/sync/tests/no_discouragement_after_tx_reorg.rs index 88e7dbda1e..e508c16751 100644 --- a/p2p/src/sync/tests/no_discouragement_after_tx_reorg.rs +++ b/p2p/src/sync/tests/no_discouragement_after_tx_reorg.rs @@ -287,6 +287,8 @@ async fn no_discouragement_after_tx_reorg(#[case] seed: Seed) { let mempool_config = MempoolConfig { min_tx_relay_fee_rate: FeeRate::from_amount_per_kb(Amount::ZERO).into(), + max_cluster_tx_count: Default::default(), + max_cluster_size_bytes: Default::default(), }; let mut node = TestNode::builder(protocol_version) .with_chain_config(Arc::clone(tfxt.tfrm.chain_config())) diff --git a/p2p/src/sync/tests/tx_announcement.rs b/p2p/src/sync/tests/tx_announcement.rs index ea2ce0dea1..5017a21384 100644 --- a/p2p/src/sync/tests/tx_announcement.rs +++ b/p2p/src/sync/tests/tx_announcement.rs @@ -466,6 +466,8 @@ async fn valid_transaction_with_fee_below_minimum(#[case] seed: Seed) { let p2p_config = Arc::new(test_p2p_config()); let mempool_config = MempoolConfig { min_tx_relay_fee_rate: min_fee_rate.into(), + max_cluster_tx_count: Default::default(), + max_cluster_size_bytes: Default::default(), }; let mut node = TestNode::builder(protocol_version) .with_p2p_config(Arc::clone(&p2p_config)) @@ -569,6 +571,8 @@ async fn transaction_sequence_via_orphan_pool(#[case] seed: Seed) { .with_mempool_config(MempoolConfig { min_tx_relay_fee_rate: FeeRate::from_amount_per_kb(Amount::from_atoms(100_000_000)) .into(), + max_cluster_tx_count: Default::default(), + max_cluster_size_bytes: Default::default(), }) .with_p2p_config(Arc::clone(&p2p_config)) .with_chainstate(tf.into_chainstate()) @@ -843,6 +847,8 @@ async fn transaction_announcements_are_batched_and_sorted(#[case] seed: Seed) { .with_p2p_config(Arc::clone(&p2p_config)) .with_mempool_config(MempoolConfig { min_tx_relay_fee_rate: FeeRate::from_amount_per_kb(Amount::ZERO).into(), + max_cluster_tx_count: Default::default(), + max_cluster_size_bytes: Default::default(), }) .with_chainstate(tf.into_chainstate()) .with_common_time_getter(&time_getter) @@ -981,6 +987,8 @@ async fn unconfirmed_local_transactions_reannouncement( .with_p2p_config(Arc::clone(&p2p_config)) .with_mempool_config(MempoolConfig { min_tx_relay_fee_rate: FeeRate::from_amount_per_kb(Amount::ZERO).into(), + max_cluster_tx_count: Default::default(), + max_cluster_size_bytes: Default::default(), }) .with_chainstate(tf.into_chainstate()) .with_common_time_getter(&time_getter) diff --git a/wallet/src/account/mod.rs b/wallet/src/account/mod.rs index 0b340c9e57..3e2d659dfb 100644 --- a/wallet/src/account/mod.rs +++ b/wallet/src/account/mod.rs @@ -338,6 +338,12 @@ impl Account { // when we allow paying fees with different currency Amount::ZERO, selection_algo, + // FIXME: Get mempool config from the node and use cluster size as the limit + // for the size of 1 tx (or, better, take the minimum with chain_config.max_tx_size_for_mempool, + // just in case). + // Same below. + // Create an issue to make the wallet take the cluster limit into account when + // selecting utxos. self.chain_config.max_tx_size_for_mempool(), )?; From 65e0e30f6e75e8f7ccbd1f370d183d18233c411a Mon Sep 17 00:00:00 2001 From: Mykhailo Kremniov Date: Mon, 18 May 2026 15:11:59 +0300 Subject: [PATCH 2/6] mempool: add tests for cluster size limits; appease clippy --- Cargo.lock | 1 + chainstate/test-framework/src/helpers.rs | 40 +- chainstate/test-framework/src/utils.rs | 13 +- mempool/Cargo.toml | 1 + .../src/interface/mempool_interface_impl.rs | 2 +- mempool/src/pool/mod.rs | 9 +- mempool/src/pool/orphans/mod.rs | 4 + mempool/src/pool/tests/mod.rs | 1 + mempool/src/pool/tests/orders_v1.rs | 6 +- mempool/src/pool/tests/relatives.rs | 946 ++++++++++++++++++ mempool/src/pool/tests/utils.rs | 2 +- mempool/src/pool/tx_pool/mod.rs | 5 + mempool/src/pool/tx_pool/store/mod.rs | 20 +- mempool/src/pool/tx_pool/tests/basic.rs | 120 ++- mempool/src/pool/tx_pool/tests/expiry.rs | 4 +- mempool/src/pool/tx_pool/tests/utils.rs | 39 +- p2p/src/sync/tests/tx_announcement.rs | 42 +- utils/src/lib.rs | 1 + utils/src/shuffled.rs | 34 + 19 files changed, 1238 insertions(+), 52 deletions(-) create mode 100644 mempool/src/pool/tests/relatives.rs create mode 100644 utils/src/shuffled.rs diff --git a/Cargo.lock b/Cargo.lock index bc7dce207a..ddb5a40bab 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5034,6 +5034,7 @@ dependencies = [ "ctor", "hashbrown 0.17.1", "hex", + "itertools 0.14.0", "jsonrpsee", "logging", "mempool-types", diff --git a/chainstate/test-framework/src/helpers.rs b/chainstate/test-framework/src/helpers.rs index f7a9254e0b..a64fcba0ef 100644 --- a/chainstate/test-framework/src/helpers.rs +++ b/chainstate/test-framework/src/helpers.rs @@ -30,7 +30,10 @@ use orders_accounting::{OrdersAccountingDB, OrdersAccountingView as _}; use randomness::{CryptoRng, Rng, RngExt as _, SliceRandom as _}; use test_utils::{random_ascii_alphanumeric_string, token_utils::random_nft_issuance}; -use crate::{TestFramework, TransactionBuilder, empty_witness, get_output_value}; +use crate::{ + TestFramework, TransactionBuilder, get_output_value, + utils::{EMPTY_WITNESS_DEFAULT_SIZE, empty_witness_with_size}, +}; // Note: this function will create 2 blocks pub fn issue_and_mint_random_token_from_best_block( @@ -396,15 +399,34 @@ pub fn output_value_with_amount(output_value: &OutputValue, new_amount: Amount) pub fn make_simple_coin_tx( rng: &mut impl CryptoRng, - ins: &[(OutPointSourceId, u32)], - outs: &[u128], + ins: impl IntoIterator, + outs: impl IntoIterator, ) -> SignedTransaction { - let builder = ins.iter().fold(TransactionBuilder::new(), |b, (s, n)| { - b.add_input(TxInput::from_utxo(s.clone(), *n), empty_witness(rng)) - }); - let builder = outs.iter().fold(builder, |b, a| { - b.add_output(TxOutput::Transfer( - OutputValue::Coin(Amount::from_atoms(*a)), + make_simple_coin_tx_with_witness_sizes( + rng, + ins.into_iter() + .map(|(utxo_src, utxo_idx)| (utxo_src, utxo_idx, EMPTY_WITNESS_DEFAULT_SIZE)), + outs, + ) +} + +pub fn make_simple_coin_tx_with_witness_sizes( + rng: &mut impl CryptoRng, + ins: impl IntoIterator, + outs: impl IntoIterator, +) -> SignedTransaction { + let builder = ins.into_iter().fold( + TransactionBuilder::new(), + |tx_builder, (utxo_src, utxo_idx, witness_size)| { + tx_builder.add_input( + TxInput::from_utxo(utxo_src.clone(), utxo_idx), + empty_witness_with_size(rng, witness_size), + ) + }, + ); + let builder = outs.into_iter().fold(builder, |tx_builder, output_amount| { + tx_builder.add_output(TxOutput::Transfer( + OutputValue::Coin(Amount::from_atoms(output_amount)), Destination::AnyoneCanSpend, )) }); diff --git a/chainstate/test-framework/src/utils.rs b/chainstate/test-framework/src/utils.rs index 0553a5be0d..86a9fa7a6f 100644 --- a/chainstate/test-framework/src/utils.rs +++ b/chainstate/test-framework/src/utils.rs @@ -57,12 +57,19 @@ use crypto::{ use orders_accounting::OrdersAccountingView; use pos_accounting::{PoSAccountingDB, PoSAccountingView}; use randomness::{CryptoRng, Rng, RngExt as _}; +use test_utils::random::gen_random_bytes; use utxo::UtxosView; +// Note: 99 is because originally `empty_witness` returned a randomly shuffled vec of numbers (0..100) +// and now some tests that check transaction sizes depend on this. +pub const EMPTY_WITNESS_DEFAULT_SIZE: usize = 99; + pub fn empty_witness(rng: &mut impl Rng) -> InputWitness { - use randomness::SliceRandom; - let mut msg: Vec = (1..100).collect(); - msg.shuffle(rng); + empty_witness_with_size(rng, EMPTY_WITNESS_DEFAULT_SIZE) +} + +pub fn empty_witness_with_size(rng: &mut impl Rng, size: usize) -> InputWitness { + let msg = gen_random_bytes(rng, size, size); InputWitness::NoSignature(Some(msg)) } diff --git a/mempool/Cargo.toml b/mempool/Cargo.toml index 26f28b5090..e672ef18fb 100644 --- a/mempool/Cargo.toml +++ b/mempool/Cargo.toml @@ -48,5 +48,6 @@ crypto = { path = "../crypto" } test-utils = { path = "../test-utils" } ctor.workspace = true +itertools.workspace = true mockall.workspace = true rstest.workspace = true diff --git a/mempool/src/interface/mempool_interface_impl.rs b/mempool/src/interface/mempool_interface_impl.rs index 1405ba03a3..d546e37e25 100644 --- a/mempool/src/interface/mempool_interface_impl.rs +++ b/mempool/src/interface/mempool_interface_impl.rs @@ -56,7 +56,7 @@ impl MempoolInit { ) -> Self { Self { chain_config, - mempool_config: mempool_config.into(), + mempool_config, chainstate_handle, time_getter, } diff --git a/mempool/src/pool/mod.rs b/mempool/src/pool/mod.rs index 74a02e930a..668adda5a7 100644 --- a/mempool/src/pool/mod.rs +++ b/mempool/src/pool/mod.rs @@ -120,7 +120,7 @@ impl Mempool { let mut this = Self(MempoolState::InIbd(MempoolStateInIbd { chain_config, - mempool_config: mempool_config.into(), + mempool_config, chainstate_handle, clock, memory_usage_estimator, @@ -278,6 +278,13 @@ impl Mempool { } } + pub fn get_all_orphan_transaction_ids(&self) -> Vec> { + match &self.0 { + MempoolState::InIbd(_) => Vec::new(), + MempoolState::AfterIbd(state) => state.orphans.get_all_transaction_ids(), + } + } + pub fn best_block_id(&self) -> Id { match &self.0 { MempoolState::InIbd(state) => state.best_block_id, diff --git a/mempool/src/pool/orphans/mod.rs b/mempool/src/pool/orphans/mod.rs index 8e39481268..147b8e44e6 100644 --- a/mempool/src/pool/orphans/mod.rs +++ b/mempool/src/pool/orphans/mod.rs @@ -280,6 +280,10 @@ impl TxOrphanPool { .map(|(_origin, iid)| *iid) .next() } + + pub fn get_all_transaction_ids(&self) -> Vec> { + self.transactions.iter().map(|entry| *entry.tx_id()).collect() + } } /// Entry-like access to orphans in the pool (somewhat similar to `btree_map::OccupiedEntry`) diff --git a/mempool/src/pool/tests/mod.rs b/mempool/src/pool/tests/mod.rs index 397b24a445..386528008d 100644 --- a/mempool/src/pool/tests/mod.rs +++ b/mempool/src/pool/tests/mod.rs @@ -28,6 +28,7 @@ use common::{ mod basic; mod orders_v1; mod orphans; +mod relatives; mod utils; #[ctor::ctor] diff --git a/mempool/src/pool/tests/orders_v1.rs b/mempool/src/pool/tests/orders_v1.rs index 7f1ed7e26e..8385e11883 100644 --- a/mempool/src/pool/tests/orders_v1.rs +++ b/mempool/src/pool/tests/orders_v1.rs @@ -168,7 +168,7 @@ async fn non_orphans(#[case] seed: Seed) { let create_mempool = || { Mempool::new( Arc::clone(&chain_config), - mempool_config.clone().into(), + mempool_config.clone(), chainstate_handle.clone(), Default::default(), StoreMemoryUsageEstimator, @@ -531,7 +531,7 @@ async fn orphans_with_missing_utxo(#[case] seed: Seed) { let create_mempool = || { Mempool::new( Arc::clone(&chain_config), - mempool_config.clone().into(), + mempool_config.clone(), chainstate_handle.clone(), Default::default(), StoreMemoryUsageEstimator, @@ -773,7 +773,7 @@ async fn orphans_with_missing_order(#[case] seed: Seed) { let create_mempool = || { Mempool::new( Arc::clone(&chain_config), - mempool_config.clone().into(), + mempool_config.clone(), chainstate_handle.clone(), Default::default(), StoreMemoryUsageEstimator, diff --git a/mempool/src/pool/tests/relatives.rs b/mempool/src/pool/tests/relatives.rs new file mode 100644 index 0000000000..d3e73d29b6 --- /dev/null +++ b/mempool/src/pool/tests/relatives.rs @@ -0,0 +1,946 @@ +// Copyright (c) 2026 RBB S.r.l +// opensource@mintlayer.org +// SPDX-License-Identifier: MIT +// Licensed under the MIT License; +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://github.com/mintlayer/mintlayer-core/blob/master/LICENSE +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +use std::{ + collections::BTreeSet, + ops::DerefMut, + sync::{Arc, Mutex, MutexGuard}, +}; + +use itertools::Itertools as _; +use rstest::rstest; + +use ::utils::shuffled::Shuffled as _; +use chainstate_test_framework::{ + TestFramework as ChainstateTestFramework, + helpers::{make_simple_coin_tx, make_simple_coin_tx_with_witness_sizes}, +}; +use common::{ + chain::{self, ChainConfig, Genesis, OutPointSourceId, SignedTransaction}, + primitives::{Amount, Id, Idable}, +}; +use logging::log; +use randomness::{CryptoRng, RngExt as _}; +use serialization::Encode as _; +use test_utils::{ + assert_matches, + random::{Seed, make_seedable_rng}, +}; + +use crate::{ + FeeRate, + config::{MaxClusterSizeBytes, MaxClusterTxCount, MempoolConfig}, + error::{Error, MempoolPolicyError}, + pool::{ + Mempool, TxStatus, + memory_usage_estimator::StoreMemoryUsageEstimator, + tx_pool::{self, tests::utils::start_chainstate}, + }, +}; + +// A general test that creates a tx tree and adds the txs to the mempool, checking clusters, +// ancestors and descendants of each tx. +// Several subtests exist, which use different cluster tx count limits and avoid/force orphans +// creation. +#[rstest] +#[trace] +#[case(Seed::from_entropy())] +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_relatives_and_cluster_tx_count_limit(#[case] seed: Seed) { + let mut rng = make_seedable_rng(seed); + let mut ctf = ChainstateTestFramework::builder(&mut rng).build(); + let tf = TestFramework::new(&mut rng, ctf.genesis().get_id()); + + ctf.make_block_builder() + .add_transaction(tf.funding_tx().clone()) + .build_and_process(&mut rng) + .unwrap(); + + let id = |tx: &SignedTransaction| tx.transaction().get_id(); + + let chain_config = Arc::clone(ctf.chain_config()); + let chainstate_handle = start_chainstate(ctf.chainstate()); + + let create_mempool = |max_cluster_tx_count: usize| { + self::create_mempool( + Arc::clone(&chain_config), + chainstate_handle.clone(), + max_cluster_tx_count.into(), + Default::default(), + ) + }; + + let a0 = tf.mk_base_tx(); + let a1 = tf.mk_base_tx(); + let a2 = tf.mk_base_tx(); + let a3 = tf.mk_base_tx(); + let a4 = tf.mk_base_tx(); + + // b0 depends on a0, a1 + let b0 = tf.mk_tx(&[(id(&a0).into(), 0), (id(&a1).into(), 0)], 1); + // b1 depends on a2, a3, a4 + let b1 = tf.mk_tx( + &[(id(&a2).into(), 0), (id(&a3).into(), 0), (id(&a4).into(), 0)], + 1, + ); + // b2 depends on a2, a3 + let b2 = tf.mk_tx(&[(id(&a2).into(), 1), (id(&a3).into(), 1)], 1); + // b3 depends on a2 + let b3 = tf.mk_tx(&[(id(&a2).into(), 2)], 1); + // b4, b5, b6 all depend on a4 + let b4 = tf.mk_tx(&[(id(&a4).into(), 1)], 1); + let b5 = tf.mk_tx(&[(id(&a4).into(), 2)], 1); + let b6 = tf.mk_tx(&[(id(&a4).into(), 3)], 1); + // c0 depends on b0, b1 + let c0 = tf.mk_tx(&[(id(&b0).into(), 0), (id(&b1).into(), 0)], 2); + // d0, d1 depend on c0 + let d0 = tf.mk_tx(&[(id(&c0).into(), 0)], 3); + let d1 = tf.mk_tx(&[(id(&c0).into(), 1)], 3); + + let all_a = [&a0, &a1, &a2, &a3, &a4]; + let all_b = [&b0, &b1, &b2, &b3, &b4, &b5, &b6]; + + for (idx, tx) in all_a.iter().enumerate() { + log::debug!("a{idx}: {:x}", id(tx)); + } + for (idx, tx) in all_b.iter().enumerate() { + log::debug!("b{idx}: {:x}", id(tx)); + } + log::debug!("c0: {:x}", id(&c0)); + log::debug!("d0: {:x}", id(&d0)); + log::debug!("d1: {:x}", id(&d1)); + + // The tx tree looks like this: + // a0 ->----\ + // b3 --\ a1 ->---- b0 -\ + // \ \ /--- d0 + // b2-------<- a2 ->---\ -- c0 --- d1 + // \------<- a3 ->---- b1 --/ + // / + // /- b4 + // a4 ->---- b5 + // \- b6 + + // Simple subtest with a small cluster size limit. + { + let mut mempool = create_mempool(3); + + for tx in all_a.iter().collect_vec().shuffled(&mut rng) { + add_tx_ok(&mut mempool, tx); + } + + assert_txs(&mempool, &all_a); + assert_orphans(&mempool, &[]); + + // Each 'a' is a separate cluster at this moment. + for tx in &all_a { + assert_cluster(&mempool, &[tx]); + } + + add_tx_ok(&mut mempool, &b0); + assert_txs(&mempool, &[&a0, &a1, &a2, &a3, &a4, &b0]); + assert_orphans(&mempool, &[]); + assert_cluster(&mempool, &[&a0, &a1, &b0]); + assert_cluster(&mempool, &[&a2]); + assert_cluster(&mempool, &[&a3]); + assert_cluster(&mempool, &[&a4]); + + // This would create a cluster of 4 + add_tx_expect_cluster_cnt_failure(&mut mempool, &b1); + assert_txs(&mempool, &[&a0, &a1, &a2, &a3, &a4, &b0]); + assert_orphans(&mempool, &[]); + assert_cluster(&mempool, &[&a0, &a1, &b0]); + assert_cluster(&mempool, &[&a2]); + assert_cluster(&mempool, &[&a3]); + assert_cluster(&mempool, &[&a4]); + + add_tx_ok(&mut mempool, &b2); + assert_txs(&mempool, &[&a0, &a1, &a2, &a3, &a4, &b0, &b2]); + assert_orphans(&mempool, &[]); + assert_cluster(&mempool, &[&a0, &a1, &b0]); + assert_cluster(&mempool, &[&a2, &a3, &b2]); + assert_cluster(&mempool, &[&a4]); + + // This would create a cluster of 4 + add_tx_expect_cluster_cnt_failure(&mut mempool, &b3); + assert_txs(&mempool, &[&a0, &a1, &a2, &a3, &a4, &b0, &b2]); + assert_orphans(&mempool, &[]); + assert_cluster(&mempool, &[&a0, &a1, &b0]); + assert_cluster(&mempool, &[&a2, &a3, &b2]); + assert_cluster(&mempool, &[&a4]); + + add_tx_ok(&mut mempool, &b4); + add_tx_ok(&mut mempool, &b5); + assert_txs(&mempool, &[&a0, &a1, &a2, &a3, &a4, &b0, &b2, &b4, &b5]); + assert_orphans(&mempool, &[]); + assert_cluster(&mempool, &[&a0, &a1, &b0]); + assert_cluster(&mempool, &[&a2, &a3, &b2]); + assert_cluster(&mempool, &[&a4, &b4, &b5]); + + add_tx_expect_cluster_cnt_failure(&mut mempool, &b6); + assert_txs(&mempool, &[&a0, &a1, &a2, &a3, &a4, &b0, &b2, &b4, &b5]); + assert_orphans(&mempool, &[]); + assert_cluster(&mempool, &[&a0, &a1, &b0]); + assert_cluster(&mempool, &[&a2, &a3, &b2]); + assert_cluster(&mempool, &[&a4, &b4, &b5]); + + // c0 will be an orphan, since b1 was rejected and di depend on c0 + for tx in [&c0, &d0, &d1].shuffled(&mut rng) { + add_orphan_ok(&mut mempool, tx); + } + + assert_txs(&mempool, &[&a0, &a1, &a2, &a3, &a4, &b0, &b2, &b4, &b5]); + assert_orphans(&mempool, &[&c0, &d0, &d1]); + assert_cluster(&mempool, &[&a0, &a1, &b0]); + assert_cluster(&mempool, &[&a2, &a3, &b2]); + assert_cluster(&mempool, &[&a4, &b4, &b5]); + + // Now also check ancestors and descendants of each tx, grouping the checks by cluster. + // Cluster 1. + assert_ancestors(&mempool, &a0, &[]); + assert_descendants(&mempool, &a0, &[&b0]); + assert_ancestors(&mempool, &a1, &[]); + assert_descendants(&mempool, &a1, &[&b0]); + assert_ancestors(&mempool, &b0, &[&a0, &a1]); + assert_descendants(&mempool, &b0, &[]); + // Cluster 2. + assert_ancestors(&mempool, &a2, &[]); + assert_descendants(&mempool, &a2, &[&b2]); + assert_ancestors(&mempool, &a3, &[]); + assert_descendants(&mempool, &a3, &[&b2]); + assert_ancestors(&mempool, &b2, &[&a2, &a3]); + assert_descendants(&mempool, &b2, &[]); + // Cluster 3. + assert_ancestors(&mempool, &a4, &[]); + assert_descendants(&mempool, &a4, &[&b4, &b5]); + assert_ancestors(&mempool, &b4, &[&a4]); + assert_descendants(&mempool, &b4, &[]); + assert_ancestors(&mempool, &b5, &[&a4]); + assert_descendants(&mempool, &b5, &[]); + } + + // Same as above, but we add a's at the end, so all other txs are initially orphans + { + let mut mempool = create_mempool(3); + + // Add all txs except a's and b6, they will all become orphans (b6 is omitted because + // it'd make clusters non-predictable - they'd depend on which of b4, b5, b6 is picked + // from the orphan pool first). + for tx in [&b0, &b1, &b2, &b3, &b4, &b5, &c0, &d0, &d1].shuffled(&mut rng) { + add_orphan_ok(&mut mempool, tx); + } + + assert_txs(&mempool, &[]); + assert_orphans(&mempool, &[&b0, &b1, &b2, &b3, &b4, &b5, &c0, &d0, &d1]); + + // Now add a's one by one. Note that due to tx dependencies, b's will be entering + // the mempool in a different order, so different clusters will form and different + // txs will be rejected compared to the test above. + + add_tx_ok(&mut mempool, &a0); + assert_txs(&mempool, &[&a0]); + assert_orphans(&mempool, &[&b0, &b1, &b2, &b3, &b4, &b5, &c0, &d0, &d1]); + assert_cluster(&mempool, &[&a0]); + + // After this, b0 is no longer an orphan + add_tx_ok(&mut mempool, &a1); + assert_txs(&mempool, &[&a0, &a1, &b0]); + assert_orphans(&mempool, &[&b1, &b2, &b3, &b4, &b5, &c0, &d0, &d1]); + assert_cluster(&mempool, &[&a0, &a1, &b0]); + + // After this, b3 is no longer an orphan + add_tx_ok(&mut mempool, &a2); + assert_txs(&mempool, &[&a0, &a1, &a2, &b0, &b3]); + assert_orphans(&mempool, &[&b1, &b2, &b4, &b5, &c0, &d0, &d1]); + assert_cluster(&mempool, &[&a0, &a1, &b0]); + assert_cluster(&mempool, &[&a2, &b3]); + + // This will attempt to add b2 to the pool, but it would result in a cluster of size 4, + // so b2 will be rejected + add_tx_ok(&mut mempool, &a3); + assert_txs(&mempool, &[&a0, &a1, &a2, &a3, &b0, &b3]); + assert_orphans(&mempool, &[&b1, &b4, &b5, &c0, &d0, &d1]); + assert_cluster(&mempool, &[&a0, &a1, &b0]); + assert_cluster(&mempool, &[&a2, &b3]); + assert_cluster(&mempool, &[&a3]); + + // After this, b1, b4 and b5 are no longer orphans. But adding b1 would result in a cluster + // of size 4 (even if it's selected first), so it will be dropped. + // (b4 and b5 will be added successfully because we didn't add b6 initially). + add_tx_ok(&mut mempool, &a4); + assert_txs(&mempool, &[&a0, &a1, &a2, &a3, &a4, &b0, &b3, &b4, &b5]); + assert_orphans(&mempool, &[&c0, &d0, &d1]); + assert_cluster(&mempool, &[&a0, &a1, &b0]); + assert_cluster(&mempool, &[&a2, &b3]); + assert_cluster(&mempool, &[&a3]); + assert_cluster(&mempool, &[&a4, &b4, &b5]); + + // Check ancestors and descendants of each tx. + // Cluster 1. + assert_ancestors(&mempool, &a0, &[]); + assert_descendants(&mempool, &a0, &[&b0]); + assert_ancestors(&mempool, &a1, &[]); + assert_descendants(&mempool, &a1, &[&b0]); + assert_ancestors(&mempool, &b0, &[&a0, &a1]); + assert_descendants(&mempool, &b0, &[]); + // Cluster 2. + assert_ancestors(&mempool, &a2, &[]); + assert_descendants(&mempool, &a2, &[&b3]); + assert_ancestors(&mempool, &b3, &[&a2]); + assert_descendants(&mempool, &b3, &[]); + // Cluster 3. + assert_ancestors(&mempool, &a3, &[]); + assert_descendants(&mempool, &a3, &[]); + // Cluster 4. + assert_ancestors(&mempool, &a4, &[]); + assert_descendants(&mempool, &a4, &[&b4, &b5]); + assert_ancestors(&mempool, &b4, &[&a4]); + assert_descendants(&mempool, &b4, &[]); + assert_ancestors(&mempool, &b5, &[&a4]); + assert_descendants(&mempool, &b5, &[]); + } + + // A non-orphan subtest with a bigger limit + { + let mut mempool = create_mempool(5); + + for tx in all_a.iter().collect_vec().shuffled(&mut rng) { + add_tx_ok(&mut mempool, tx); + } + + assert_txs(&mempool, &all_a); + assert_orphans(&mempool, &[]); + + // Each 'a' is a separate cluster at this moment. + for tx in &all_a { + assert_cluster(&mempool, &[tx]); + } + + add_tx_ok(&mut mempool, &b0); + assert_txs(&mempool, &[&a0, &a1, &a2, &a3, &a4, &b0]); + assert_orphans(&mempool, &[]); + assert_cluster(&mempool, &[&a0, &a1, &b0]); + assert_cluster(&mempool, &[&a2]); + assert_cluster(&mempool, &[&a3]); + assert_cluster(&mempool, &[&a4]); + + add_tx_ok(&mut mempool, &b1); + assert_txs(&mempool, &[&a0, &a1, &a2, &a3, &a4, &b0, &b1]); + assert_orphans(&mempool, &[]); + assert_cluster(&mempool, &[&a0, &a1, &b0]); + assert_cluster(&mempool, &[&a2, &a3, &a4, &b1]); + + add_tx_ok(&mut mempool, &b2); + assert_txs(&mempool, &[&a0, &a1, &a2, &a3, &a4, &b0, &b1, &b2]); + assert_orphans(&mempool, &[]); + assert_cluster(&mempool, &[&a0, &a1, &b0]); + assert_cluster(&mempool, &[&a2, &a3, &a4, &b1, &b2]); + + // Any of these would contribute to the larger cluster, which is already of size 5, + // so they will be rejected. + for tx in [&b3, &b4, &b5, &b6, &c0].shuffled(&mut rng) { + add_tx_expect_cluster_cnt_failure(&mut mempool, tx); + assert_txs(&mempool, &[&a0, &a1, &a2, &a3, &a4, &b0, &b1, &b2]); + assert_orphans(&mempool, &[]); + assert_cluster(&mempool, &[&a0, &a1, &b0]); + assert_cluster(&mempool, &[&a2, &a3, &a4, &b1, &b2]); + } + + // d's will become orphans + for tx in [&d0, &d1].shuffled(&mut rng) { + add_orphan_ok(&mut mempool, tx); + } + + assert_orphans(&mempool, &[&d0, &d1]); + assert_cluster(&mempool, &[&a0, &a1, &b0]); + assert_cluster(&mempool, &[&a2, &a3, &a4, &b1, &b2]); + + // Check ancestors and descendants of each tx. + // Cluster 1. + assert_ancestors(&mempool, &a0, &[]); + assert_descendants(&mempool, &a0, &[&b0]); + assert_ancestors(&mempool, &a1, &[]); + assert_descendants(&mempool, &a1, &[&b0]); + assert_ancestors(&mempool, &b0, &[&a0, &a1]); + assert_descendants(&mempool, &b0, &[]); + // Cluster 2. + assert_ancestors(&mempool, &a2, &[]); + assert_descendants(&mempool, &a2, &[&b1, &b2]); + assert_ancestors(&mempool, &a3, &[]); + assert_descendants(&mempool, &a3, &[&b1, &b2]); + assert_ancestors(&mempool, &a4, &[]); + assert_descendants(&mempool, &a4, &[&b1]); + assert_ancestors(&mempool, &b1, &[&a2, &a3, &a4]); + assert_descendants(&mempool, &b1, &[]); + assert_ancestors(&mempool, &b2, &[&a2, &a3]); + assert_descendants(&mempool, &b2, &[]); + } + + // A non-orphan subtest with the default limit. All txs will be added and form a single cluster. + { + let mut mempool = create_mempool(*MaxClusterTxCount::default()); + + let all_txs = { + let mut all_txs = all_a.iter().copied().collect_vec(); + all_txs.extend(&all_b); + all_txs.extend([&c0, &d0, &d1]); + all_txs + }; + let all_txs_shuffled = all_txs.clone().shuffled(&mut rng); + + for (idx, tx) in all_txs_shuffled.iter().enumerate() { + log::debug!("shuffled tx #{idx}: {:x}", id(tx)); + } + + for tx in &all_txs_shuffled { + let status = mempool.add_transaction_test((*tx).clone()).unwrap(); + assert!(status == TxStatus::InMempool || status == TxStatus::InOrphanPool); + } + + assert_orphans(&mempool, &[]); + assert_txs(&mempool, &all_txs); + assert_cluster(&mempool, &all_txs); + + // Check ancestors and descendants of each tx. + // A's + assert_ancestors(&mempool, &a0, &[]); + assert_descendants(&mempool, &a0, &[&b0, &c0, &d0, &d1]); + assert_ancestors(&mempool, &a1, &[]); + assert_descendants(&mempool, &a1, &[&b0, &c0, &d0, &d1]); + assert_ancestors(&mempool, &a2, &[]); + assert_descendants(&mempool, &a2, &[&b1, &b2, &b3, &c0, &d0, &d1]); + assert_ancestors(&mempool, &a3, &[]); + assert_descendants(&mempool, &a3, &[&b1, &b2, &c0, &d0, &d1]); + assert_ancestors(&mempool, &a4, &[]); + assert_descendants(&mempool, &a4, &[&b1, &b4, &b5, &b6, &c0, &d0, &d1]); + // B's + assert_ancestors(&mempool, &b0, &[&a0, &a1]); + assert_descendants(&mempool, &b0, &[&c0, &d0, &d1]); + assert_ancestors(&mempool, &b1, &[&a2, &a3, &a4]); + assert_descendants(&mempool, &b1, &[&c0, &d0, &d1]); + assert_ancestors(&mempool, &b2, &[&a2, &a3]); + assert_descendants(&mempool, &b2, &[]); + assert_ancestors(&mempool, &b3, &[&a2]); + assert_descendants(&mempool, &b3, &[]); + assert_ancestors(&mempool, &b4, &[&a4]); + assert_descendants(&mempool, &b4, &[]); + assert_ancestors(&mempool, &b5, &[&a4]); + assert_descendants(&mempool, &b5, &[]); + assert_ancestors(&mempool, &b6, &[&a4]); + assert_descendants(&mempool, &b6, &[]); + // C's + assert_ancestors(&mempool, &c0, &[&a0, &a1, &a2, &a3, &a4, &b0, &b1]); + assert_descendants(&mempool, &c0, &[&d0, &d1]); + // D's + assert_ancestors(&mempool, &d0, &[&a0, &a1, &a2, &a3, &a4, &b0, &b1, &c0]); + assert_descendants(&mempool, &d0, &[]); + assert_ancestors(&mempool, &d1, &[&a0, &a1, &a2, &a3, &a4, &b0, &b1, &c0]); + assert_descendants(&mempool, &d1, &[]); + } +} + +#[rstest] +#[trace] +#[case(Seed::from_entropy())] +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_cluster_byte_size(#[case] seed: Seed) { + let mut rng = make_seedable_rng(seed); + let chain_config = chain::config::create_unit_test_config_builder() + // In this test we'll vary witness data length to make txs of the desired size. + .data_in_no_signature_witness_max_size(10_000) + .build(); + let mut ctf = ChainstateTestFramework::builder(&mut rng) + .with_chain_config(chain_config) + .build(); + let tf = TestFramework::new(&mut rng, ctf.genesis().get_id()); + + ctf.make_block_builder() + .add_transaction(tf.funding_tx().clone()) + .build_and_process(&mut rng) + .unwrap(); + + let id = |tx: &SignedTransaction| tx.transaction().get_id(); + + let chain_config = Arc::clone(ctf.chain_config()); + let chainstate_handle = start_chainstate(ctf.chainstate()); + + let create_mempool = |max_cluster_size_bytes: usize| { + self::create_mempool( + Arc::clone(&chain_config), + chainstate_handle.clone(), + Default::default(), + max_cluster_size_bytes.into(), + ) + }; + + // a's and d0 will all have 1 input (and all txs have the same number of outputs), + // so they will have the same size, not counting the witness. + let ad_tx = tf.mk_base_tx_with_sig_size(1000); + let ad_tx_base_size = ad_tx.encoded_size() - 1000; + + // same for b0, b1 and c0, which will all have 2 inputs. + let bc_tx_base_size = tf + .mk_tx_with_sig_size( + &[(id(&ad_tx).into(), 0, 1000), (id(&ad_tx).into(), 0, 0)], + 1, + ) + .encoded_size() + - 1000; + + let a_size = 1000; + let b0_size = 1000; + let b1_size = 2000; + let c0_size = 1000; + let d0_size = 1000; + + let a0 = tf.mk_base_tx_with_sig_size(a_size - ad_tx_base_size); + let a1 = tf.mk_base_tx_with_sig_size(a_size - ad_tx_base_size); + let a2 = tf.mk_base_tx_with_sig_size(a_size - ad_tx_base_size); + let a3 = tf.mk_base_tx_with_sig_size(a_size - ad_tx_base_size); + + // b0 depends on a0, a1 + let b0 = tf.mk_tx_with_sig_size( + &[(id(&a0).into(), 0, b0_size - bc_tx_base_size), (id(&a1).into(), 0, 0)], + 1, + ); + // b1 depends on a2, a3 + let b1 = tf.mk_tx_with_sig_size( + &[(id(&a2).into(), 0, b1_size - bc_tx_base_size), (id(&a3).into(), 0, 0)], + 1, + ); + // c0 depends on b0, b1 + let c0 = tf.mk_tx_with_sig_size( + &[(id(&b0).into(), 0, c0_size - bc_tx_base_size), (id(&b1).into(), 0, 0)], + 2, + ); + // d0 depends on c0 + let d0 = tf.mk_tx_with_sig_size(&[(id(&c0).into(), 0, d0_size - ad_tx_base_size)], 3); + + let all_a = [&a0, &a1, &a2, &a3]; + + for (idx, tx) in all_a.iter().enumerate() { + log::debug!("a{idx}: id={:x}, size={}", id(tx), tx.encoded_size()); + } + log::debug!("b0: id={:x}, size={}", id(&b0), b0.encoded_size()); + log::debug!("b1: id={:x}, size={}", id(&b1), b1.encoded_size()); + log::debug!("c0: id={:x}, size={}", id(&c0), c0.encoded_size()); + log::debug!("d0: id={:x}, size={}", id(&d0), d0.encoded_size()); + + // The tx tree looks like this: + // a0 ->----\ + // a1 ->---- b0 -\ + // \ + // a2 ->---\ -- c0 --- d0 + // a3 ->---- b1 --/ + + let b0_cluster_size = a_size * 2 + b0_size; + let b1_cluster_size = a_size * 2 + b1_size; + let c0_cluster_size = b0_cluster_size + b1_cluster_size + c0_size; + let d0_cluster_size = c0_cluster_size + d0_size; + + // Sanity check + assert!(b1_cluster_size > b0_cluster_size); + + // A simple subtest with a small byte size limit + { + let limit = b0_cluster_size + rng.random_range(0..(b1_cluster_size - b0_cluster_size)); + let mut mempool = create_mempool(limit); + + for tx in all_a.iter().collect_vec().shuffled(&mut rng) { + add_tx_ok(&mut mempool, tx); + } + + assert_txs(&mempool, &all_a); + assert_orphans(&mempool, &[]); + + // Each 'a' is a separate cluster at this moment. + for tx in &all_a { + assert_cluster(&mempool, &[tx]); + } + + add_tx_ok(&mut mempool, &b0); + assert_txs(&mempool, &[&a0, &a1, &a2, &a3, &b0]); + assert_orphans(&mempool, &[]); + assert_cluster_with_size(&mempool, &[&a0, &a1, &b0], b0_cluster_size); + assert_cluster_with_size(&mempool, &[&a2], a_size); + assert_cluster_with_size(&mempool, &[&a3], a_size); + + // This would result in cluster with size b1_cluster_size + add_tx_expect_cluster_byte_size_failure(&mut mempool, &b1); + assert_txs(&mempool, &[&a0, &a1, &a2, &a3, &b0]); + assert_orphans(&mempool, &[]); + assert_cluster_with_size(&mempool, &[&a0, &a1, &b0], b0_cluster_size); + assert_cluster_with_size(&mempool, &[&a2], a_size); + assert_cluster_with_size(&mempool, &[&a3], a_size); + + // c0 will be an orphan, since b1 was rejected and d0 depends on c0 + add_orphan_ok(&mut mempool, &c0); + add_orphan_ok(&mut mempool, &d0); + assert_txs(&mempool, &[&a0, &a1, &a2, &a3, &b0]); + assert_orphans(&mempool, &[&c0, &d0]); + assert_cluster_with_size(&mempool, &[&a0, &a1, &b0], b0_cluster_size); + assert_cluster_with_size(&mempool, &[&a2], a_size); + assert_cluster_with_size(&mempool, &[&a3], a_size); + } + + // Same as above, but we add a's at the end, so all other txs are initially orphans + { + let limit = b0_cluster_size + rng.random_range(0..(b1_cluster_size - b0_cluster_size)); + let mut mempool = create_mempool(limit); + + for tx in [&b0, &b1, &c0, &d0].shuffled(&mut rng) { + add_orphan_ok(&mut mempool, tx); + } + + assert_txs(&mempool, &[]); + assert_orphans(&mempool, &[&b0, &b1, &c0, &d0]); + + // b0 is still an orphan + add_tx_ok(&mut mempool, &a0); + assert_txs(&mempool, &[&a0]); + assert_orphans(&mempool, &[&b0, &b1, &c0, &d0]); + assert_cluster_with_size(&mempool, &[&a0], a_size); + + // b0 is no longer an orphan + add_tx_ok(&mut mempool, &a1); + assert_txs(&mempool, &[&a0, &a1, &b0]); + assert_orphans(&mempool, &[&b1, &c0, &d0]); + assert_cluster_with_size(&mempool, &[&a0, &a1, &b0], b0_cluster_size); + + // b1 is still an orphan + add_tx_ok(&mut mempool, &a2); + assert_txs(&mempool, &[&a0, &a1, &a2, &b0]); + assert_orphans(&mempool, &[&b1, &c0, &d0]); + assert_cluster_with_size(&mempool, &[&a0, &a1, &b0], b0_cluster_size); + assert_cluster_with_size(&mempool, &[&a2], a_size); + + // b1 is no longer an orphan, but adding it to the mempool would create a cluster of + // size b1_cluster_size, so it'll be discarded. + add_tx_ok(&mut mempool, &a3); + assert_txs(&mempool, &[&a0, &a1, &a2, &a3, &b0]); + assert_orphans(&mempool, &[&c0, &d0]); + assert_cluster_with_size(&mempool, &[&a0, &a1, &b0], b0_cluster_size); + assert_cluster_with_size(&mempool, &[&a2], a_size); + assert_cluster_with_size(&mempool, &[&a3], a_size); + } + + // A non-orphan subtest with a bigger byte size limit, enough to include b1, but not c0. + { + let limit = b1_cluster_size + rng.random_range(0..(c0_cluster_size - b1_cluster_size)); + let mut mempool = create_mempool(limit); + + for tx in all_a.iter().collect_vec().shuffled(&mut rng) { + add_tx_ok(&mut mempool, tx); + } + + assert_txs(&mempool, &all_a); + assert_orphans(&mempool, &[]); + + // Each 'a' is a separate cluster at this moment. + for tx in &all_a { + assert_cluster(&mempool, &[tx]); + } + + add_tx_ok(&mut mempool, &b0); + assert_txs(&mempool, &[&a0, &a1, &a2, &a3, &b0]); + assert_orphans(&mempool, &[]); + assert_cluster_with_size(&mempool, &[&a0, &a1, &b0], b0_cluster_size); + assert_cluster_with_size(&mempool, &[&a2], a_size); + assert_cluster_with_size(&mempool, &[&a3], a_size); + + add_tx_ok(&mut mempool, &b1); + assert_txs(&mempool, &[&a0, &a1, &a2, &a3, &b0, &b1]); + assert_orphans(&mempool, &[]); + assert_cluster_with_size(&mempool, &[&a0, &a1, &b0], b0_cluster_size); + assert_cluster_with_size(&mempool, &[&a2, &a3, &b1], b1_cluster_size); + + // This would result in cluster with size c0_cluster_size + add_tx_expect_cluster_byte_size_failure(&mut mempool, &c0); + assert_txs(&mempool, &[&a0, &a1, &a2, &a3, &b0, &b1]); + assert_orphans(&mempool, &[]); + assert_cluster_with_size(&mempool, &[&a0, &a1, &b0], b0_cluster_size); + assert_cluster_with_size(&mempool, &[&a2, &a3, &b1], b1_cluster_size); + + // d0 will be an orphan, since c0 was rejected + add_orphan_ok(&mut mempool, &d0); + assert_txs(&mempool, &[&a0, &a1, &a2, &a3, &b0, &b1]); + assert_orphans(&mempool, &[&d0]); + assert_cluster_with_size(&mempool, &[&a0, &a1, &b0], b0_cluster_size); + assert_cluster_with_size(&mempool, &[&a2, &a3, &b1], b1_cluster_size); + } + + // A non-orphan subtest with a bigger byte size limit, enough to include c0, but not d0. + { + let limit = c0_cluster_size + rng.random_range(0..(d0_cluster_size - c0_cluster_size)); + let mut mempool = create_mempool(limit); + + for tx in all_a.iter().collect_vec().shuffled(&mut rng) { + add_tx_ok(&mut mempool, tx); + } + + assert_txs(&mempool, &all_a); + assert_orphans(&mempool, &[]); + + // Each 'a' is a separate cluster at this moment. + for tx in &all_a { + assert_cluster(&mempool, &[tx]); + } + + add_tx_ok(&mut mempool, &b0); + assert_txs(&mempool, &[&a0, &a1, &a2, &a3, &b0]); + assert_orphans(&mempool, &[]); + assert_cluster_with_size(&mempool, &[&a0, &a1, &b0], b0_cluster_size); + assert_cluster_with_size(&mempool, &[&a2], a_size); + assert_cluster_with_size(&mempool, &[&a3], a_size); + + add_tx_ok(&mut mempool, &b1); + assert_txs(&mempool, &[&a0, &a1, &a2, &a3, &b0, &b1]); + assert_orphans(&mempool, &[]); + assert_cluster_with_size(&mempool, &[&a0, &a1, &b0], b0_cluster_size); + assert_cluster_with_size(&mempool, &[&a2, &a3, &b1], b1_cluster_size); + + add_tx_ok(&mut mempool, &c0); + assert_txs(&mempool, &[&a0, &a1, &a2, &a3, &b0, &b1, &c0]); + assert_orphans(&mempool, &[]); + assert_cluster_with_size( + &mempool, + &[&a0, &a1, &a2, &a3, &b0, &b1, &c0], + c0_cluster_size, + ); + + // This would result in cluster with size d0_cluster_size + add_tx_expect_cluster_byte_size_failure(&mut mempool, &d0); + assert_txs(&mempool, &[&a0, &a1, &a2, &a3, &b0, &b1, &c0]); + assert_orphans(&mempool, &[]); + assert_cluster_with_size( + &mempool, + &[&a0, &a1, &a2, &a3, &b0, &b1, &c0], + c0_cluster_size, + ); + } +} + +struct TestFramework { + rng: Mutex>, + fund_amount: u128, + funding_tx: SignedTransaction, + next_fund_tx_output_idx: Mutex, +} + +impl TestFramework { + pub fn new(orig_rng: &mut impl CryptoRng, genesis_id: Id) -> Self { + let fund_amount = 1_000_000_000; + let funding_tx = make_simple_coin_tx( + orig_rng, + [(OutPointSourceId::BlockReward(genesis_id.into()), 0)], + [fund_amount; 100], + ); + + Self { + rng: Mutex::new(Box::new(make_seedable_rng(orig_rng.random()))), + fund_amount: 1_000_000_000, + funding_tx, + next_fund_tx_output_idx: Mutex::new(0), + } + } + + fn lock_rng(&self) -> MutexGuard<'_, Box> { + self.rng.lock().unwrap() + } + + pub fn funding_tx(&self) -> &SignedTransaction { + &self.funding_tx + } + + // For simplicity, all txs in these tests will have 10 outputs. + // The "level" here must be the max "distance" from this tx and the funding tx, and it determines + // the size of the outputs. + fn mk_tx_outputs(&self, level: u32) -> impl IntoIterator { + [self.fund_amount / 10u128.pow(level + 1); 10] + } + + pub fn mk_tx(&self, ins: &[(OutPointSourceId, u32)], level: u32) -> SignedTransaction { + make_simple_coin_tx( + self.lock_rng().deref_mut(), + ins.iter().cloned(), + self.mk_tx_outputs(level), + ) + } + + pub fn mk_tx_with_sig_size( + &self, + ins: &[(OutPointSourceId, u32, usize)], + level: u32, + ) -> SignedTransaction { + make_simple_coin_tx_with_witness_sizes( + self.lock_rng().deref_mut(), + ins.iter().cloned(), + self.mk_tx_outputs(level), + ) + } + + pub fn mk_base_tx(&self) -> SignedTransaction { + self.mk_tx( + &[( + self.funding_tx.transaction().get_id().into(), + self.next_fund_tx_output_idx(), + )], + 0, + ) + } + + pub fn mk_base_tx_with_sig_size(&self, sig_size: usize) -> SignedTransaction { + self.mk_tx_with_sig_size( + &[( + self.funding_tx.transaction().get_id().into(), + self.next_fund_tx_output_idx(), + sig_size, + )], + 0, + ) + } + + fn next_fund_tx_output_idx(&self) -> u32 { + let mut next_idx = self.next_fund_tx_output_idx.lock().unwrap(); + let old_idx = *next_idx; + + *next_idx += 1; + + old_idx + } +} + +type MempoolType = Mempool; + +fn create_mempool( + chain_config: Arc, + chainstate_handle: chainstate::ChainstateHandle, + max_cluster_tx_count: MaxClusterTxCount, + max_cluster_size_bytes: MaxClusterSizeBytes, +) -> MempoolType { + let mempool_config = MempoolConfig { + min_tx_relay_fee_rate: FeeRate::from_amount_per_kb(Amount::ZERO).into(), + max_cluster_tx_count, + max_cluster_size_bytes, + }; + Mempool::new( + chain_config, + mempool_config, + chainstate_handle, + Default::default(), + StoreMemoryUsageEstimator, + ) + .unwrap() +} + +fn add_tx_ok(mempool: &mut MempoolType, tx: &SignedTransaction) { + let status = mempool.add_transaction_test(tx.clone()).unwrap(); + assert_eq!(status, TxStatus::InMempool); +} + +fn add_orphan_ok(mempool: &mut MempoolType, tx: &SignedTransaction) { + let status = mempool.add_transaction_test(tx.clone()).unwrap(); + assert_eq!(status, TxStatus::InOrphanPool); +} + +fn add_tx_expect_cluster_cnt_failure(mempool: &mut MempoolType, tx: &SignedTransaction) { + let err = mempool.add_transaction_test(tx.clone()).unwrap_err(); + assert_matches!( + err, + Error::Policy(MempoolPolicyError::ClusterMaxTxCountLimitViolated { .. }) + ); +} + +fn add_tx_expect_cluster_byte_size_failure(mempool: &mut MempoolType, tx: &SignedTransaction) { + let err = mempool.add_transaction_test(tx.clone()).unwrap_err(); + assert_matches!( + err, + Error::Policy(MempoolPolicyError::ClusterTotalTxSizeLimitViolated { .. }) + ); +} + +fn assert_txs(mempool: &MempoolType, txs: &[&SignedTransaction]) { + let actual_tx_ids = mempool + .get_all() + .iter() + .map(|tx| tx.transaction().get_id()) + .collect::>(); + let expected_tx_ids = txs.iter().map(|tx| tx.transaction().get_id()).collect::>(); + assert_eq!(actual_tx_ids, expected_tx_ids); +} + +fn assert_orphans(mempool: &MempoolType, txs: &[&SignedTransaction]) { + let actual_tx_ids = + mempool.get_all_orphan_transaction_ids().into_iter().collect::>(); + let expected_tx_ids = txs.iter().map(|tx| tx.transaction().get_id()).collect::>(); + assert_eq!(actual_tx_ids, expected_tx_ids); +} + +fn assert_cluster(mempool: &MempoolType, txs: &[&SignedTransaction]) { + let tx_ids = txs.iter().map(|tx| tx.transaction().get_id()).collect::>(); + // txs must be unique + assert_eq!(tx_ids.len(), txs.len()); + tx_pool::tests::utils::assert_cluster(mempool.tx_store(), &tx_ids); +} + +fn assert_cluster_with_size( + mempool: &MempoolType, + txs: &[&SignedTransaction], + expected_total_size: usize, +) { + assert_cluster(mempool, txs); + + let mut total_size = 0; + for tx in txs { + let tx_id = tx.transaction().get_id(); + let entry = mempool.tx_store().get_entry(&tx_id).unwrap(); + let recorded_tx_size = entry.size().get(); + let actual_tx_size = tx.encoded_size(); + assert_eq!(recorded_tx_size, actual_tx_size, "tx_id = {tx_id:x}"); + total_size += actual_tx_size; + } + + assert_eq!(total_size, expected_total_size); +} + +fn assert_ancestors( + mempool: &MempoolType, + tx: &SignedTransaction, + ancestor_txs: &[&SignedTransaction], +) { + let ancestor_tx_ids = + ancestor_txs.iter().map(|tx| tx.transaction().get_id()).collect::>(); + tx_pool::tests::utils::assert_ancestors( + mempool.tx_store(), + &tx.transaction().get_id(), + &ancestor_tx_ids, + ); +} + +fn assert_descendants( + mempool: &MempoolType, + tx: &SignedTransaction, + descendant_txs: &[&SignedTransaction], +) { + let descendant_tx_ids = descendant_txs + .iter() + .map(|tx| tx.transaction().get_id()) + .collect::>(); + tx_pool::tests::utils::assert_descendants( + mempool.tx_store(), + &tx.transaction().get_id(), + &descendant_tx_ids, + ); +} diff --git a/mempool/src/pool/tests/utils.rs b/mempool/src/pool/tests/utils.rs index e08c7d1f84..f80c3f2045 100644 --- a/mempool/src/pool/tests/utils.rs +++ b/mempool/src/pool/tests/utils.rs @@ -41,7 +41,7 @@ pub fn setup_with_chainstate_and_clock( let chainstate_handle = start_chainstate(chainstate); Mempool::new( chain_config, - mempool_config.into(), + mempool_config, chainstate_handle, clock, StoreMemoryUsageEstimator, diff --git a/mempool/src/pool/tx_pool/mod.rs b/mempool/src/pool/tx_pool/mod.rs index 1f79a5d346..505b43ce0f 100644 --- a/mempool/src/pool/tx_pool/mod.rs +++ b/mempool/src/pool/tx_pool/mod.rs @@ -173,6 +173,11 @@ impl TxPool { .map(|(_score, id)| self.store.get_entry(id).expect("entry").transaction().clone()) .collect() } + + #[cfg(test)] + pub fn disable_heavy_validity_checks(&mut self) { + self.store.disable_heavy_validity_checks() + } } // Rolling-fee-related methods diff --git a/mempool/src/pool/tx_pool/store/mod.rs b/mempool/src/pool/tx_pool/store/mod.rs index 77c5375454..8be45e5072 100644 --- a/mempool/src/pool/tx_pool/store/mod.rs +++ b/mempool/src/pool/tx_pool/store/mod.rs @@ -164,6 +164,9 @@ pub struct MempoolStore { /// Memory usage accumulator mem_tracker: mem_usage::MemUsageTracker, + + #[cfg(test)] + heavy_validity_checks_enabled: bool, } // If a transaction is removed from the mempool for any reason other than inclusion in a block, @@ -194,6 +197,8 @@ impl MempoolStore { seq_nos_by_tx: Tracked::default(), next_seq_no: 0, mem_tracker: mem_usage::MemUsageTracker::new(), + #[cfg(test)] + heavy_validity_checks_enabled: true, } } @@ -243,8 +248,17 @@ impl MempoolStore { self.assert_valid_inner() } + #[cfg(test)] + pub fn disable_heavy_validity_checks(&mut self) { + self.heavy_validity_checks_enabled = false; + } + #[cfg(test)] fn assert_valid_inner(&self) { + if !self.heavy_validity_checks_enabled { + return; + } + use mem_usage::MemoryUsage; fn hash_map_size_deep( map: &StoreHashMap>, @@ -630,7 +644,7 @@ impl MempoolStore { tx_id: &Id, ) -> Result { let entry = self.get_existing_entry(tx_id)?; - Ok(entry.collect_ancestors(&self)) + Ok(entry.collect_ancestors(self)) } /// Collect all descendants of the specified existing transaction. @@ -640,14 +654,14 @@ impl MempoolStore { tx_id: &Id, ) -> Result { let entry = self.get_existing_entry(tx_id)?; - Ok(entry.collect_descendants(&self)) + Ok(entry.collect_descendants(self)) } /// Collect the cluster that the specified existing transaction belongs to. /// Mainly intended for testing. pub fn collect_cluster(&self, tx_id: &Id) -> Result { let entry = self.get_existing_entry(tx_id)?; - Ok(entry.collect_cluster(&self)?) + entry.collect_cluster(self) } } diff --git a/mempool/src/pool/tx_pool/tests/basic.rs b/mempool/src/pool/tx_pool/tests/basic.rs index c35c36c64c..7e12829493 100644 --- a/mempool/src/pool/tx_pool/tests/basic.rs +++ b/mempool/src/pool/tx_pool/tests/basic.rs @@ -677,7 +677,7 @@ async fn rolling_fee(#[case] seed: Seed) -> anyhow::Result<()> { log::debug!("before adding parent"); let mut tx_pool = TxPool::new( Arc::clone(&chain_config), - create_mempool_config().into(), + create_mempool_config(), chainstate_interface, mock_clock, mock_usage, @@ -1281,7 +1281,7 @@ async fn mempool_full_mock(#[case] seed: Seed) -> anyhow::Result<()> { let mut tx_pool = TxPool::new( chain_config, - create_mempool_config().into(), + create_mempool_config(), chainstate_handle, Default::default(), mock_usage, @@ -1596,3 +1596,119 @@ async fn initial_values(#[case] seed: Seed) { .unwrap(); assert_eq!(initial_fee_rate_points, fee_rate_points); } + +// Originally, when adding a transaction, a recursive function was used to collect its ancestors, +// which could potentially result in a stack overflow. +// Here we create a long chain of transactions and use a small stack size, the combination that +// would crash the original implementation. Two cases exist: +// a) The default cluster tx count limit is used; adding the tx that violates the limit should fail. +// b) A huge cluster tx count limit is used; all txs should be added successfully. +// In either case, the test shouldn't crash. +#[rstest] +#[case(Seed::from_entropy())] +#[trace] +fn stack_overflow_on_transaction_addition( + #[case] seed: Seed, + #[values(false, true)] unlimited_cluster_tx_count: bool, +) { + let runtime = tokio::runtime::Builder::new_multi_thread() + .worker_threads(2) + // Use a small stack - 256Kb. Note that the underlying OS may have a minimum stack size + // (and different OSs have different minimums), so this value is not a guarantee and the + // actual stack may be bigger, but it's unlikely that the minimum is more than 128Kb (but + // we can't use 128Kb here because in debug builds it's not enough to even start the test). + .thread_stack_size(256 * 1024) + .enable_all() + .build() + .unwrap(); + + let test_body = move || { + let mut rng = make_seedable_rng(seed); + let mut tf = TestFramework::builder(&mut rng).build(); + let genesis_id = tf.genesis().get_id(); + + // With the original recursive implementation, about 900 txs were enough to crash the test + // in debug builds and 2200 in release builds (with 256Kb of stack). + // We use bigger values just in case, but not too big, because the test will become too slow + // in the unlimited cluster size case. + let chain_len: usize = if cfg!(debug_assertions) { 2000 } else { 5_000 }; + + let mut amount = 1_000_000; + let funding_tx = make_tx( + &mut rng, + &[(OutPointSourceId::BlockReward(genesis_id.into()), 0)], + &[amount], + ); + let funding_tx_id = funding_tx.transaction().get_id(); + tf.make_block_builder() + .add_transaction(funding_tx) + .build_and_process(&mut rng) + .unwrap(); + + let max_cluster_tx_count = if unlimited_cluster_tx_count { + usize::MAX + } else { + *MaxClusterTxCount::default() + }; + + let mut mempool = setup_with_chainstate_generic( + tf.chainstate(), + MempoolConfig { + min_tx_relay_fee_rate: FeeRate::from_amount_per_kb(Amount::ZERO).into(), + max_cluster_size_bytes: usize::MAX.into(), + max_cluster_tx_count: max_cluster_tx_count.into(), + }, + Default::default(), + ); + + if unlimited_cluster_tx_count { + // Disable heavy checks because this case is already pretty slow. + mempool.disable_heavy_validity_checks(); + } + + let mut prev_source: OutPointSourceId = funding_tx_id.into(); + let mut tx_ids = BTreeSet::new(); + + for i in 0..chain_len { + let tx = make_tx(&mut rng, &[(prev_source, 0)], &[amount - 1]); + amount -= 1; + let tx_id = tx.transaction().get_id(); + prev_source = tx_id.into(); + + let result = mempool.add_transaction_test(tx); + + if unlimited_cluster_tx_count || i < max_cluster_tx_count { + assert_eq!(result, Ok(TxStatus::InMempool)); + tx_ids.insert(tx_id); + } else { + assert_eq!( + result, + Err(Error::Policy( + MempoolPolicyError::ClusterMaxTxCountLimitViolated { + limit: max_cluster_tx_count + } + )) + ); + break; + } + } + + let actual_tx_ids = mempool + .get_all() + .iter() + .map(|tx| tx.transaction().get_id()) + .collect::>(); + assert_eq!(actual_tx_ids, tx_ids); + }; + + runtime.block_on(async move { + // Note: need to use `spawn` or `spawn_blocking` here, to make sure the stack size actually + // applies (the `runtime.block_on` itself waits on the test thread, which has the default + // stack size). + tokio::task::spawn_blocking(move || { + test_body(); + }) + .await + .unwrap(); + }); +} diff --git a/mempool/src/pool/tx_pool/tests/expiry.rs b/mempool/src/pool/tx_pool/tests/expiry.rs index 37b8815348..788347c6ca 100644 --- a/mempool/src/pool/tx_pool/tests/expiry.rs +++ b/mempool/src/pool/tx_pool/tests/expiry.rs @@ -50,7 +50,7 @@ async fn descendant_of_expired_entry(#[case] seed: Seed) -> anyhow::Result<()> { let chainstate = tf.chainstate(); let mut mempool = TxPool::new( Arc::clone(chainstate.get_chain_config()), - create_mempool_config().into(), + create_mempool_config(), start_chainstate(chainstate), mock_clock, StoreMemoryUsageEstimator, @@ -111,7 +111,7 @@ async fn only_expired_entries_removed(#[case] seed: Seed) -> anyhow::Result<()> let mut mempool = TxPool::new( chain_config, - create_mempool_config().into(), + create_mempool_config(), chainstate_interface, mock_clock, StoreMemoryUsageEstimator, diff --git a/mempool/src/pool/tx_pool/tests/utils.rs b/mempool/src/pool/tx_pool/tests/utils.rs index 192c71f067..bc9ae9a84c 100644 --- a/mempool/src/pool/tx_pool/tests/utils.rs +++ b/mempool/src/pool/tx_pool/tests/utils.rs @@ -227,7 +227,7 @@ pub fn make_tx( ins: &[(OutPointSourceId, u32)], outs: &[u128], ) -> SignedTransaction { - make_simple_coin_tx(rng, ins, outs) + make_simple_coin_tx(rng, ins.iter().cloned(), outs.iter().cloned()) } /// Generate a valid transaction graph. @@ -319,7 +319,7 @@ pub fn setup() -> TxPool { let chainstate_interface = start_chainstate_with_config(Arc::clone(&chain_config)); TxPool::new( chain_config, - create_mempool_config().into(), + create_mempool_config(), chainstate_interface, Default::default(), StoreMemoryUsageEstimator, @@ -336,7 +336,7 @@ pub fn setup_with_min_tx_relay_fee_rate(fee_rate: FeeRate) -> TxPool( SignedTransaction::new(tx, witnesses.to_vec()) .map_err(|_| anyhow::Error::msg("invalid witness count")) } + +/// Assert that the specified txs form a cluster +pub fn assert_cluster(store: &MempoolStore, tx_ids: &BTreeSet>) { + for tx_id in tx_ids { + let cluster = store.collect_cluster(tx_id).unwrap(); + let cluster = cluster.iter().copied().collect::>(); + assert_eq!(&cluster, tx_ids, "tx_id = {tx_id:x}"); + } +} + +/// Assert that the specified tx has the specified ancestors +pub fn assert_ancestors( + store: &MempoolStore, + tx_id: &Id, + ancestor_tx_ids: &BTreeSet>, +) { + let ancestors = store.collect_ancestors(tx_id).unwrap(); + let ancestors = ancestors.iter().copied().collect::>(); + assert_eq!(&ancestors, ancestor_tx_ids); +} + +/// Assert that the specified tx has the specified descendants +pub fn assert_descendants( + store: &MempoolStore, + tx_id: &Id, + descendant_tx_ids: &BTreeSet>, +) { + let descendants = store.collect_descendants(tx_id).unwrap(); + let descendants = descendants.iter().copied().collect::>(); + assert_eq!(&descendants, descendant_tx_ids); +} diff --git a/p2p/src/sync/tests/tx_announcement.rs b/p2p/src/sync/tests/tx_announcement.rs index 5017a21384..89634f5547 100644 --- a/p2p/src/sync/tests/tx_announcement.rs +++ b/p2p/src/sync/tests/tx_announcement.rs @@ -869,22 +869,22 @@ async fn transaction_announcements_are_batched_and_sorted(#[case] seed: Seed) { let independent_tx = make_simple_coin_tx( &mut rng, - &[(funding_tx_id.into(), 0)], - &[output_amount - independent_tx_fee], + [(funding_tx_id.into(), 0)], + [output_amount - independent_tx_fee], ); let independent_tx_id = independent_tx.transaction().get_id(); let parent_tx = make_simple_coin_tx( &mut rng, - &[(funding_tx_id.into(), 1)], - &[1, output_amount - parent_tx_fee - 1], + [(funding_tx_id.into(), 1)], + [1, output_amount - parent_tx_fee - 1], ); let parent_tx_id = parent_tx.transaction().get_id(); let child_tx = make_simple_coin_tx( &mut rng, - &[(funding_tx_id.into(), 2), (parent_tx_id.into(), 0)], - &[1, output_amount - child_tx_fee], + [(funding_tx_id.into(), 2), (parent_tx_id.into(), 0)], + [1, output_amount - child_tx_fee], ); let child_tx_id = child_tx.transaction().get_id(); @@ -1008,22 +1008,22 @@ async fn unconfirmed_local_transactions_reannouncement( let local_independent_tx = make_simple_coin_tx( &mut rng, - &[(funding_tx_id.into(), 0)], - &[output_amount - local_independent_tx_fee], + [(funding_tx_id.into(), 0)], + [output_amount - local_independent_tx_fee], ); let local_independent_tx_id = local_independent_tx.transaction().get_id(); let local_parent_tx = make_simple_coin_tx( &mut rng, - &[(funding_tx_id.into(), 1)], - &[1, output_amount - local_parent_tx_fee - 1], + [(funding_tx_id.into(), 1)], + [1, output_amount - local_parent_tx_fee - 1], ); let local_parent_tx_id = local_parent_tx.transaction().get_id(); let local_child_tx = make_simple_coin_tx( &mut rng, - &[(funding_tx_id.into(), 2), (local_parent_tx_id.into(), 0)], - &[1, output_amount - local_child_tx_fee], + [(funding_tx_id.into(), 2), (local_parent_tx_id.into(), 0)], + [1, output_amount - local_child_tx_fee], ); let local_child_tx_id = local_child_tx.transaction().get_id(); @@ -1033,22 +1033,22 @@ async fn unconfirmed_local_transactions_reannouncement( let remote_independent_tx = make_simple_coin_tx( &mut rng, - &[(funding_tx_id.into(), 3)], - &[output_amount - remote_independent_tx_fee], + [(funding_tx_id.into(), 3)], + [output_amount - remote_independent_tx_fee], ); let remote_independent_tx_id = remote_independent_tx.transaction().get_id(); let remote_parent_tx = make_simple_coin_tx( &mut rng, - &[(funding_tx_id.into(), 4)], - &[1, output_amount - remote_parent_tx_fee - 1], + [(funding_tx_id.into(), 4)], + [1, output_amount - remote_parent_tx_fee - 1], ); let remote_parent_tx_id = remote_parent_tx.transaction().get_id(); let remote_child_tx = make_simple_coin_tx( &mut rng, - &[(funding_tx_id.into(), 5), (remote_parent_tx_id.into(), 0)], - &[1, output_amount - remote_child_tx_fee], + [(funding_tx_id.into(), 5), (remote_parent_tx_id.into(), 0)], + [1, output_amount - remote_child_tx_fee], ); let remote_child_tx_id = remote_child_tx.transaction().get_id(); @@ -1225,11 +1225,7 @@ fn transaction_with_amount( block_id: Id, amount_atoms: u128, ) -> SignedTransaction { - make_simple_coin_tx( - rng, - &[(OutPointSourceId::from(block_id), 0)], - &[amount_atoms], - ) + make_simple_coin_tx(rng, [(OutPointSourceId::from(block_id), 0)], [amount_atoms]) } fn transaction(rng: &mut impl CryptoRng, block_id: Id) -> SignedTransaction { diff --git a/utils/src/lib.rs b/utils/src/lib.rs index b26bd093da..b23899e798 100644 --- a/utils/src/lib.rs +++ b/utils/src/lib.rs @@ -46,6 +46,7 @@ pub mod rust_backtrace; pub mod sender_with_id; pub mod set_flag; pub mod shallow_clone; +pub mod shuffled; pub mod sorted; pub mod tap_log; pub mod try_as; diff --git a/utils/src/shuffled.rs b/utils/src/shuffled.rs new file mode 100644 index 0000000000..903e8a2333 --- /dev/null +++ b/utils/src/shuffled.rs @@ -0,0 +1,34 @@ +// Copyright (c) 2026 RBB S.r.l +// opensource@mintlayer.org +// SPDX-License-Identifier: MIT +// Licensed under the MIT License; +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://github.com/mintlayer/mintlayer-core/blob/master/LICENSE +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +use std::convert::AsMut; + +use randomness::{CryptoRng, SliceRandom as _}; + +/// An extension trait allowing to shuffle a container without creating an intermediate mutable +/// variable. +pub trait Shuffled: AsMut<[Item]> { + fn shuffled(self, rng: &mut impl CryptoRng) -> Self; +} + +impl Shuffled for T +where + T: AsMut<[Item]>, +{ + fn shuffled(mut self, rng: &mut impl CryptoRng) -> Self { + self.as_mut().shuffle(rng); + self + } +} From 59e2c535f10b7a3ff49988a6a80d9a2e57feba25 Mon Sep 17 00:00:00 2001 From: Mykhailo Kremniov Date: Thu, 21 May 2026 20:57:25 +0300 Subject: [PATCH 3/6] Add mempool_max_cluster_transaction_count/mempool_max_cluster_size_bytes to RunOptions; fix test wallet_high_fee.py --- node-lib/src/config_files/mempool.rs | 4 ++++ node-lib/src/options.rs | 10 ++++++++++ node-lib/tests/cli.rs | 16 +++++++++++++++- test/functional/wallet_high_fee.py | 7 +++++-- 4 files changed, 34 insertions(+), 3 deletions(-) diff --git a/node-lib/src/config_files/mempool.rs b/node-lib/src/config_files/mempool.rs index 622f0f2c0c..cbfb84164b 100644 --- a/node-lib/src/config_files/mempool.rs +++ b/node-lib/src/config_files/mempool.rs @@ -48,6 +48,10 @@ impl MempoolConfigFile { } = config; let min_tx_relay_fee_rate = min_tx_relay_fee_rate.or(options.min_tx_relay_fee_rate); + let max_cluster_tx_count = + max_cluster_tx_count.or(options.mempool_max_cluster_transaction_count); + let max_cluster_size_bytes = + max_cluster_size_bytes.or(options.mempool_max_cluster_size_bytes); MempoolConfigFile { min_tx_relay_fee_rate, diff --git a/node-lib/src/options.rs b/node-lib/src/options.rs index 9016d2e258..da61692d88 100644 --- a/node-lib/src/options.rs +++ b/node-lib/src/options.rs @@ -301,6 +301,14 @@ pub struct RunOptions { #[clap(long, value_name = "COUNT")] pub max_orphan_blocks: Option, + /// The maximum number of transactions that a single mempool transaction cluster may have. + #[clap(long, value_name = "COUNT")] + pub mempool_max_cluster_transaction_count: Option, + + /// The maximum size in bytes that a single mempool transaction cluster may have. + #[clap(long, value_name = "SIZE")] + pub mempool_max_cluster_size_bytes: Option, + /// Whether p2p networking should be enabled. #[clap(long, value_name = "VAL")] pub p2p_networking_enabled: Option, @@ -478,6 +486,8 @@ mod tests { max_db_commit_attempts: Default::default(), enable_db_reckless_mode_in_ibd: Default::default(), max_orphan_blocks: Default::default(), + mempool_max_cluster_transaction_count: Default::default(), + mempool_max_cluster_size_bytes: Default::default(), p2p_networking_enabled: Default::default(), p2p_bind_addresses: Default::default(), p2p_socks5_proxy: Default::default(), diff --git a/node-lib/tests/cli.rs b/node-lib/tests/cli.rs index 8463a2ed4d..7fbf9820b3 100644 --- a/node-lib/tests/cli.rs +++ b/node-lib/tests/cli.rs @@ -102,6 +102,8 @@ fn read_config_override_values() { let max_db_commit_attempts = 1; let enable_db_reckless_mode_in_ibd = true; let max_orphan_blocks = 2; + let mempool_max_cluster_transaction_count = 123; + let mempool_max_cluster_size_bytes = 12345; let p2p_networking_enabled = false; let p2p_bind_addr = "127.0.0.1:44444".parse::().unwrap(); let p2p_socks5_proxy = "socks5_proxy"; @@ -144,6 +146,8 @@ fn read_config_override_values() { max_db_commit_attempts: Some(max_db_commit_attempts), enable_db_reckless_mode_in_ibd: Some(enable_db_reckless_mode_in_ibd), max_orphan_blocks: Some(max_orphan_blocks), + mempool_max_cluster_transaction_count: Some(mempool_max_cluster_transaction_count), + mempool_max_cluster_size_bytes: Some(mempool_max_cluster_size_bytes), p2p_networking_enabled: Some(p2p_networking_enabled), p2p_bind_addresses: Some(vec![p2p_bind_addr]), p2p_socks5_proxy: Some(p2p_socks5_proxy.to_owned()), @@ -220,10 +224,20 @@ fn read_config_override_values() { ); assert_eq!( - config.mempool.unwrap().min_tx_relay_fee_rate, + config.mempool.as_ref().unwrap().min_tx_relay_fee_rate, Some(min_tx_relay_fee_rate) ); + assert_eq!( + config.mempool.as_ref().unwrap().max_cluster_tx_count, + Some(mempool_max_cluster_transaction_count) + ); + + assert_eq!( + config.mempool.as_ref().unwrap().max_cluster_size_bytes, + Some(mempool_max_cluster_size_bytes) + ); + assert_eq!( config.chainstate.as_ref().unwrap().chainstate_config.enable_heavy_checks, Some(enable_chainstate_heavy_checks) diff --git a/test/functional/wallet_high_fee.py b/test/functional/wallet_high_fee.py index 58308db729..0071c5ec2b 100644 --- a/test/functional/wallet_high_fee.py +++ b/test/functional/wallet_high_fee.py @@ -42,6 +42,10 @@ def set_test_params(self): self.num_nodes = 1 self.extra_args = [[ "--blockprod-min-peers-to-produce-blocks=0", + # Note: this test uses test_functions_generate_transactions, which creates a chain + # of dependant txs, so they end up in the same cluster. And the number of txs is large, + # so the default max cluster size of 100_000 bytes is not enough. + "--mempool-max-cluster-size-bytes=250000", ]] def setup_network(self): @@ -100,7 +104,7 @@ async def async_test(self): # Submit a valid transaction output = { - 'Transfer': [ { 'Coin': 10 * ATOMS_PER_COIN }, { 'PublicKey': {'key': {'Secp256k1Schnorr' : {'pubkey_data': pub_key_bytes}}} } ], + 'Transfer': [ { 'Coin': 10 * ATOMS_PER_COIN }, { 'PublicKey': {'key': {'Secp256k1Schnorr' : {'pubkey_data': pub_key_bytes}}} } ], } inputs = list(range(5)) total = 300000//len(inputs) @@ -142,7 +146,6 @@ async def async_test(self): node.mempool_submit_transaction(encoded_tx, {}) total_size_of_txs_in_mempool += len(bytes.fromhex(encoded_tx)) - self.log.info(f"total size {total_size_of_txs_in_mempool}") #check total size of txs are more then 1MB assert_greater_than(total_size_of_txs_in_mempool, 1_000_000) From 7008bcbd88710707909e13e338bbbd07da52fa7e Mon Sep 17 00:00:00 2001 From: Mykhailo Kremniov Date: Thu, 21 May 2026 17:36:23 +0300 Subject: [PATCH 4/6] Expose mempool config in the node rpc. Use it in the wallet to limit the size of a single tx. Update the changelog. --- CHANGELOG.md | 9 ++ common/src/chain/config/mod.rs | 2 +- mempool/src/config.rs | 38 +++++++++ mempool/src/interface/mempool_interface.rs | 5 +- .../src/interface/mempool_interface_impl.rs | 4 + mempool/src/lib.rs | 6 +- mempool/src/pool/mod.rs | 7 ++ mempool/src/pool/tx_pool/mod.rs | 4 + mempool/src/rpc.rs | 10 ++- mocks/src/mempool.rs | 4 +- node-daemon/docs/RPC.md | 19 +++++ wallet/src/account/mod.rs | 71 ++++++++++++++-- .../src/account/utxo_selector/output_group.rs | 12 +++ wallet/src/wallet/mod.rs | 85 ++++++++++++++++++- wallet/src/wallet/test_helpers.rs | 2 + wallet/src/wallet/tests.rs | 11 +++ wallet/wallet-controller/src/lib.rs | 14 ++- .../wallet-controller/src/sync/tests/mod.rs | 6 +- .../src/handles_client/mod.rs | 7 +- wallet/wallet-node-client/src/mock.rs | 8 +- wallet/wallet-node-client/src/node_traits.rs | 6 +- .../src/rpc_client/client_impl.rs | 11 ++- .../src/rpc_client/cold_wallet_client.rs | 8 +- wallet/wallet-rpc-lib/src/service/mod.rs | 16 ++++ wallet/wallet-rpc-lib/src/service/worker.rs | 13 ++- wallet/wallet-rpc-lib/tests/utils.rs | 1 + 26 files changed, 353 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c5268a12bc..01b2a9b28a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,11 @@ The format is loosely based on [Keep a Changelog](https://keepachangelog.com/en/ - `wallet-create`/`wallet-recover`/`wallet-open` support the `ledger` subcommand, in addition to the existing `software` and `trezor`, which specifies the type of the wallet to operate on. + - Node: + - New options to control the newly introduced mempool cluster limits: `--mempool-max-cluster-transaction-count` + and `--mempool-max-cluster-size-bytes`. + - New RPC method `mempool_get_config`, which returns the current mempool configuration. + ### Changed - P2p: - Various improvements to transaction announcement: @@ -33,6 +38,10 @@ The format is loosely based on [Keep a Changelog](https://keepachangelog.com/en/ - Mempool: - Various optimizations were made. + - There are now limits on how big a single mempool transaction cluster can be (a cluster is a set of unconfirmed + transactions that depend on each other); the mempool will reject transactions that would result in the formation + of a cluster that exceeds the limits. The default cluster limits are: max 64 transactions, max 100'000 bytes + in total. ### Fixed - Wallet: diff --git a/common/src/chain/config/mod.rs b/common/src/chain/config/mod.rs index 899a87eacb..a71b81888a 100644 --- a/common/src/chain/config/mod.rs +++ b/common/src/chain/config/mod.rs @@ -522,7 +522,7 @@ impl ChainConfig { self.max_block_size_with_smart_contracts } - /// The maximum size of any transaction submitted to the node for the mempool. + /// The approximate maximum size of any transaction submitted to the node for the mempool. /// /// Note that this is just an upper limit - creating a tx bigger than this doesn't make much /// sense, because it won't fit into a block. Normally, the cluster size limit (specified in diff --git a/mempool/src/config.rs b/mempool/src/config.rs index 679399c098..c362799807 100644 --- a/mempool/src/config.rs +++ b/mempool/src/config.rs @@ -160,4 +160,42 @@ impl MempoolConfig { pub fn new() -> Self { Self::default() } + + pub fn to_rpc_type(&self) -> RpcMempoolConfig { + let MempoolConfig { + min_tx_relay_fee_rate, + max_cluster_tx_count, + max_cluster_size_bytes, + } = self; + + RpcMempoolConfig { + min_tx_relay_fee_rate: **min_tx_relay_fee_rate, + max_cluster_tx_count: **max_cluster_tx_count, + max_cluster_size_bytes: **max_cluster_size_bytes, + } + } +} + +impl From for MempoolConfig { + fn from(value: RpcMempoolConfig) -> Self { + let RpcMempoolConfig { + min_tx_relay_fee_rate, + max_cluster_tx_count, + max_cluster_size_bytes, + } = value; + + Self { + min_tx_relay_fee_rate: min_tx_relay_fee_rate.into(), + max_cluster_tx_count: max_cluster_tx_count.into(), + max_cluster_size_bytes: max_cluster_size_bytes.into(), + } + } +} + +/// Same as MempoolConfig but for usage in RPC. +#[derive(Debug, Clone, serde::Serialize, serde::Deserialize, rpc::description::HasValueHint)] +pub struct RpcMempoolConfig { + pub min_tx_relay_fee_rate: FeeRate, + pub max_cluster_tx_count: usize, + pub max_cluster_size_bytes: usize, } diff --git a/mempool/src/interface/mempool_interface.rs b/mempool/src/interface/mempool_interface.rs index efffd2e34b..d786f1c5e2 100644 --- a/mempool/src/interface/mempool_interface.rs +++ b/mempool/src/interface/mempool_interface.rs @@ -22,7 +22,7 @@ use common::{ use mempool_types::TransactionDuplicateStatus; use crate::{ - FeeRate, MempoolMaxSize, TxOptions, TxStatus, + FeeRate, MempoolConfig, MempoolMaxSize, TxOptions, TxStatus, error::{BlockConstructionError, Error}, event::MempoolEvent, tx_accumulator::{PackingStrategy, TransactionAccumulator}, @@ -63,6 +63,9 @@ pub trait MempoolInterface: Send + Sync { /// Best block ID according to mempool. May be temporarily out of sync with chainstate. fn best_block_id(&self) -> Id; + /// Return the mempool config. + fn config(&self) -> &MempoolConfig; + /// Collect transactions by putting them in given accumulator /// Returns the accumulator with the collected transactions /// Ok(None) is returned on recoverable errors, such as if diff --git a/mempool/src/interface/mempool_interface_impl.rs b/mempool/src/interface/mempool_interface_impl.rs index d546e37e25..21924f0a23 100644 --- a/mempool/src/interface/mempool_interface_impl.rs +++ b/mempool/src/interface/mempool_interface_impl.rs @@ -157,6 +157,10 @@ impl MempoolInterface for Mempool { self.best_block_id() } + fn config(&self) -> &MempoolConfig { + self.mempool_config() + } + #[tracing::instrument(skip_all)] fn collect_txs( &self, diff --git a/mempool/src/lib.rs b/mempool/src/lib.rs index afbc86a8c6..b23aa391b6 100644 --- a/mempool/src/lib.rs +++ b/mempool/src/lib.rs @@ -28,7 +28,11 @@ pub mod rpc; pub mod rpc_event; pub mod tx_accumulator; -pub use {config::MempoolConfig, pool::FeeRate, pool::feerate_points::find_interpolated_value}; +pub use { + config::{MempoolConfig, RpcMempoolConfig}, + pool::FeeRate, + pool::feerate_points::find_interpolated_value, +}; pub type MempoolHandle = subsystem::Handle; diff --git a/mempool/src/pool/mod.rs b/mempool/src/pool/mod.rs index 668adda5a7..35e26a6732 100644 --- a/mempool/src/pool/mod.rs +++ b/mempool/src/pool/mod.rs @@ -299,6 +299,13 @@ impl Mempool { } } + pub fn mempool_config(&self) -> &MempoolConfig { + match &self.0 { + MempoolState::InIbd(state) => &state.mempool_config, + MempoolState::AfterIbd(state) => state.tx_pool.mempool_config(), + } + } + pub fn has_work(&self) -> bool { match &self.0 { MempoolState::InIbd(_) => false, diff --git a/mempool/src/pool/tx_pool/mod.rs b/mempool/src/pool/tx_pool/mod.rs index 505b43ce0f..3b821e0cbd 100644 --- a/mempool/src/pool/tx_pool/mod.rs +++ b/mempool/src/pool/tx_pool/mod.rs @@ -141,6 +141,10 @@ impl TxPool { .expect("best block to exist") } + pub fn mempool_config(&self) -> &MempoolConfig { + &self.mempool_config + } + pub fn max_size(&self) -> config::MempoolMaxSize { self.max_size } diff --git a/mempool/src/rpc.rs b/mempool/src/rpc.rs index d424c1ef0e..ec5244f7a0 100644 --- a/mempool/src/rpc.rs +++ b/mempool/src/rpc.rs @@ -25,7 +25,7 @@ use mempool_types::{TxOptions, tx_options::TxOptionsOverrides, tx_origin::LocalT use serialization::hex_encoded::HexEncoded; use utils::tap_log::TapLog; -use crate::{FeeRate, MempoolMaxSize, TxStatus, rpc_event::RpcEvent}; +use crate::{FeeRate, MempoolMaxSize, RpcMempoolConfig, TxStatus, rpc_event::RpcEvent}; use rpc::RpcResult; @@ -103,6 +103,10 @@ trait MempoolRpc { #[method(name = "get_fee_rate_points")] async fn get_fee_rate_points(&self) -> RpcResult>; + /// Get the mempool config. + #[method(name = "get_config")] + async fn get_config(&self) -> RpcResult; + /// Subscribe to mempool events, such as tx processed. /// /// After a successful subscription, the node will message the subscriber with a message on every event. @@ -189,6 +193,10 @@ impl MempoolRpcServer for super::MempoolHandle { rpc::handle_result(self.call(move |this| this.get_fee_rate_points(NUM_POINTS)).await) } + async fn get_config(&self) -> RpcResult { + rpc::handle_result(self.call(move |this| this.config().to_rpc_type()).await) + } + async fn subscribe_to_events( &self, pending: rpc::subscription::Pending, diff --git a/mocks/src/mempool.rs b/mocks/src/mempool.rs index b2dcfc6d82..bc55eeb873 100644 --- a/mocks/src/mempool.rs +++ b/mocks/src/mempool.rs @@ -22,7 +22,8 @@ use common::{ primitives::Id, }; use mempool::{ - FeeRate, MempoolInterface, MempoolMaxSize, TransactionDuplicateStatus, TxOptions, TxStatus, + FeeRate, MempoolConfig, MempoolInterface, MempoolMaxSize, TransactionDuplicateStatus, + TxOptions, TxStatus, error::{BlockConstructionError, Error}, event::MempoolEvent, tx_accumulator::{PackingStrategy, TransactionAccumulator}, @@ -53,6 +54,7 @@ mockall::mock! { fn contains_transaction(&self, tx: &Id) -> bool; fn contains_orphan_transaction(&self, tx: &Id) -> bool; fn best_block_id(&self) -> Id; + fn config(&self) -> &MempoolConfig; fn collect_txs( &self, diff --git a/node-daemon/docs/RPC.md b/node-daemon/docs/RPC.md index c8808df13a..311b2939ba 100644 --- a/node-daemon/docs/RPC.md +++ b/node-daemon/docs/RPC.md @@ -1000,6 +1000,25 @@ Returns: ], .. ] ``` +### Method `mempool_get_config` + +Get the mempool config. + + +Parameters: +``` +{} +``` + +Returns: +``` +{ + "min_tx_relay_fee_rate": { "amount_per_kb": { "atoms": number string } }, + "max_cluster_tx_count": number, + "max_cluster_size_bytes": number, +} +``` + ### Subscription `mempool_subscribe_to_events` Subscribe to mempool events, such as tx processed. diff --git a/wallet/src/account/mod.rs b/wallet/src/account/mod.rs index 3e2d659dfb..d742230bfa 100644 --- a/wallet/src/account/mod.rs +++ b/wallet/src/account/mod.rs @@ -64,7 +64,7 @@ use crypto::{ }, vrf::VRFPublicKey, }; -use mempool::FeeRate; +use mempool::{FeeRate, MempoolConfig}; use serialization::hex_encoded::HexEncoded; use utils::{ensure, iter::CloneableExactSizeIterator}; use wallet_storage::{ @@ -215,6 +215,17 @@ impl Account { } // Note: the default selection algo depends on whether input_utxos are empty. + // TODO: utxo selection algorithm must be improved, so that it avoids creating transaction + // clusters that violate the corresponding mempool limits. Bitcoin's `AutomaticCoinSelection` + // may be used as an inspiration, e.g. we may want to: + // *) prefer confirmed utxos is general; + // *) prefer utxos that have fewer unconfirmed dependencies; + // *) take into account the number of confirmations, especially for utxos coming from a + // different wallet. + // Also note that at the moment of writing this mempool only considers utxo-based relations to + // determine tx clusters. But this will be fixed in the (hopefully near) future, and all + // dependencies will be considered (e.g. the account-based ones and things like order creation + // vs order commands). The new algorithm will have to take this into account as well. #[allow(clippy::too_many_arguments)] pub fn select_inputs_for_send_request( &mut self, @@ -226,6 +237,7 @@ impl Account { median_time: BlockTimestamp, fee_rates: CurrentFeeRate, order_info: Option>, + mempool_config: &MempoolConfig, ) -> WalletResult { // TODO: allow to pay fees with different currency? let pay_fee_with_currency = Currency::Coin; @@ -314,6 +326,17 @@ impl Account { } }; + // The maximum possible size for a tx. ChainConfig::max_tx_size_for_mempool() is the + // absolute limit, but a transaction cannot be bigger than the cluster size limit. + // TODO: currently this value is only passed to select_coins, but the actual tx size + // may become bigger after change is added (also, select_coins will ignore the passed value + // in the `CoinSelectionAlgo::UsePreselected` case). This should probably be fixed as a part + // of a general utxo selection revamp, see the TODO at the top of this function. + let max_tx_size = std::cmp::min( + self.chain_config.max_tx_size_for_mempool(), + *mempool_config.max_cluster_size_bytes, + ); + let mut selected_inputs: BTreeMap<_, _> = output_currency_amounts .iter() .map(|(currency, output_amount)| -> WalletResult<_> { @@ -338,13 +361,7 @@ impl Account { // when we allow paying fees with different currency Amount::ZERO, selection_algo, - // FIXME: Get mempool config from the node and use cluster size as the limit - // for the size of 1 tx (or, better, take the minimum with chain_config.max_tx_size_for_mempool, - // just in case). - // Same below. - // Create an issue to make the wallet take the cluster limit into account when - // selecting utxos. - self.chain_config.max_tx_size_for_mempool(), + max_tx_size, )?; total_tx_size += selection_result.get_weight(); @@ -426,7 +443,7 @@ impl Account { PayFee::PayFeeWithThisCurrency, cost_of_change, selection_algo, - self.chain_config.max_tx_size_for_mempool(), + max_tx_size, )?; let mut selection_result = selection_result.add_change( @@ -941,6 +958,7 @@ impl Account { htlc: HashedTimelockContract, median_time: BlockTimestamp, fee_rate: CurrentFeeRate, + mempool_config: &MempoolConfig, ) -> WalletResult { let output = TxOutput::Htlc(output_value, Box::new(htlc)); let request = SendRequest::new().with_outputs([output]); @@ -954,6 +972,7 @@ impl Account { median_time, fee_rate, None, + mempool_config, ) } @@ -965,6 +984,7 @@ impl Account { conclude_address: Address, median_time: BlockTimestamp, fee_rate: CurrentFeeRate, + mempool_config: &MempoolConfig, ) -> WalletResult { let order_data = common::chain::OrderData::new(conclude_address.into_object(), ask_value, give_value); @@ -980,6 +1000,7 @@ impl Account { median_time, fee_rate, None, + mempool_config, ) } @@ -991,6 +1012,7 @@ impl Account { output_address: Option, median_time: BlockTimestamp, fee_rate: CurrentFeeRate, + mempool_config: &MempoolConfig, ) -> WalletResult { let output_destination = if let Some(dest) = output_address { dest @@ -1049,6 +1071,7 @@ impl Account { median_time, fee_rate, Some(BTreeMap::from_iter([(order_id, &order_info)])), + mempool_config, ) } @@ -1062,6 +1085,7 @@ impl Account { output_address: Option, median_time: BlockTimestamp, fee_rate: CurrentFeeRate, + mempool_config: &MempoolConfig, ) -> WalletResult { let output_destination = if let Some(dest) = output_address { dest @@ -1144,6 +1168,7 @@ impl Account { median_time, fee_rate, Some(BTreeMap::from_iter([(order_id, &order_info)])), + mempool_config, ) } @@ -1154,6 +1179,7 @@ impl Account { order_info: RpcOrderInfo, median_time: BlockTimestamp, fee_rate: CurrentFeeRate, + mempool_config: &MempoolConfig, ) -> WalletResult { let request = SendRequest::new().with_inputs_and_destinations([( TxInput::OrderAccountCommand(OrderAccountCommand::FreezeOrder(order_id)), @@ -1169,6 +1195,7 @@ impl Account { median_time, fee_rate, Some(BTreeMap::from_iter([(order_id, &order_info)])), + mempool_config, ) } @@ -1180,6 +1207,7 @@ impl Account { htlc_secret: Option, median_time: BlockTimestamp, fee_rate: CurrentFeeRate, + mempool_config: &MempoolConfig, ) -> WalletResult { let utxo = self .output_cache @@ -1298,6 +1326,7 @@ impl Account { median_time, fee_rate, None, + mempool_config, ) } } @@ -1308,6 +1337,7 @@ impl Account { nft_issue_arguments: IssueNftArguments, median_time: BlockTimestamp, fee_rate: CurrentFeeRate, + mempool_config: &MempoolConfig, ) -> WalletResult { let nft_issuance = NftIssuanceV0 { metadata: nft_issue_arguments.metadata, @@ -1333,6 +1363,7 @@ impl Account { median_time, fee_rate, None, + mempool_config, )?; let new_token_id = make_token_id( @@ -1375,6 +1406,7 @@ impl Account { amount: Amount, median_time: BlockTimestamp, fee_rate: CurrentFeeRate, + mempool_config: &MempoolConfig, ) -> WalletResult { let token_id = token_info.token_id(); let outputs = make_mint_token_outputs(token_id, amount, address); @@ -1392,6 +1424,7 @@ impl Account { db_tx, median_time, fee_rate, + mempool_config, ) } @@ -1402,6 +1435,7 @@ impl Account { amount: Amount, median_time: BlockTimestamp, fee_rate: CurrentFeeRate, + mempool_config: &MempoolConfig, ) -> WalletResult { let token_id = token_info.token_id(); let outputs = make_unmint_token_outputs(token_id, amount); @@ -1419,6 +1453,7 @@ impl Account { db_tx, median_time, fee_rate, + mempool_config, ) } @@ -1428,6 +1463,7 @@ impl Account { token_info: &UnconfirmedTokenInfo, median_time: BlockTimestamp, fee_rate: CurrentFeeRate, + mempool_config: &MempoolConfig, ) -> WalletResult { let token_id = token_info.token_id(); token_info.check_can_lock()?; @@ -1443,6 +1479,7 @@ impl Account { db_tx, median_time, fee_rate, + mempool_config, ) } @@ -1453,6 +1490,7 @@ impl Account { is_token_unfreezable: IsTokenUnfreezable, median_time: BlockTimestamp, fee_rate: CurrentFeeRate, + mempool_config: &MempoolConfig, ) -> WalletResult { token_info.check_can_freeze()?; @@ -1470,6 +1508,7 @@ impl Account { db_tx, median_time, fee_rate, + mempool_config, ) } @@ -1479,6 +1518,7 @@ impl Account { token_info: &UnconfirmedTokenInfo, median_time: BlockTimestamp, fee_rate: CurrentFeeRate, + mempool_config: &MempoolConfig, ) -> WalletResult { token_info.check_can_unfreeze()?; @@ -1494,6 +1534,7 @@ impl Account { db_tx, median_time, fee_rate, + mempool_config, ) } @@ -1504,6 +1545,7 @@ impl Account { address: Address, median_time: BlockTimestamp, fee_rate: CurrentFeeRate, + mempool_config: &MempoolConfig, ) -> WalletResult { let new_authority = address.into_object(); @@ -1521,6 +1563,7 @@ impl Account { db_tx, median_time, fee_rate, + mempool_config, ) } @@ -1531,6 +1574,7 @@ impl Account { metadata_uri: Vec, median_time: BlockTimestamp, fee_rate: CurrentFeeRate, + mempool_config: &MempoolConfig, ) -> WalletResult { let nonce = token_info.get_next_nonce()?; let tx_input = TxInput::AccountCommand( @@ -1546,6 +1590,7 @@ impl Account { db_tx, median_time, fee_rate, + mempool_config, ) } @@ -1557,6 +1602,7 @@ impl Account { db_tx: &mut impl WalletStorageWriteLocked, median_time: BlockTimestamp, fee_rate: CurrentFeeRate, + mempool_config: &MempoolConfig, ) -> Result { let request = SendRequest::new() .with_outputs(outputs) @@ -1571,6 +1617,7 @@ impl Account { median_time, fee_rate, None, + mempool_config, ) } @@ -1580,6 +1627,7 @@ impl Account { mut stake_pool_arguments: StakePoolCreationArguments, median_time: BlockTimestamp, fee_rate: CurrentFeeRate, + mempool_config: &MempoolConfig, ) -> WalletResult { let Some(vrf_public_key) = stake_pool_arguments.vrf_public_key.take() else { return Err(WalletError::VrfKeyMustBeProvided); @@ -1591,6 +1639,7 @@ impl Account { vrf_public_key, median_time, fee_rate, + mempool_config, ) } @@ -1601,6 +1650,7 @@ impl Account { vrf_public_key: VRFPublicKey, median_time: BlockTimestamp, fee_rate: CurrentFeeRate, + mempool_config: &MempoolConfig, ) -> Result { let staker = match stake_pool_arguments.staker_key { Some(staker) => match staker { @@ -1645,6 +1695,7 @@ impl Account { median_time, fee_rate, None, + mempool_config, )?; let new_pool_id = common::chain::make_pool_id(request.inputs())?; @@ -2626,6 +2677,7 @@ impl Account { mut stake_pool_arguments: StakePoolCreationArguments, median_time: BlockTimestamp, fee_rate: CurrentFeeRate, + mempool_config: &MempoolConfig, ) -> WalletResult { let vrf_public_key = match stake_pool_arguments.vrf_public_key.take() { Some(vrf_public_key) => vrf_public_key, @@ -2637,6 +2689,7 @@ impl Account { vrf_public_key, median_time, fee_rate, + mempool_config, ) } } diff --git a/wallet/src/account/utxo_selector/output_group.rs b/wallet/src/account/utxo_selector/output_group.rs index a13240722a..f15acf8814 100644 --- a/wallet/src/account/utxo_selector/output_group.rs +++ b/wallet/src/account/utxo_selector/output_group.rs @@ -22,6 +22,18 @@ use super::UtxoSelectorError; /// A group of UTXOs paid to the same output script. /// This helps reduce privacy leaks resulting from address reuse. +// TODO: at this moment, we always create a separate output group for each utxo, i.e. we don't have +// by-destination grouping. +// Note that in Bitcoin, transactions are not grouped unconditionally: +// *) They are grouped if either m_avoid_partial_spends or m_avoid_address_reuse is true (both are +// false by default). +// *) Otherwise, if m_max_aps_fee is non-negative (it's zero by default), then a second attempt +// to construct the tx is made, this time with grouping enabled, and the result is chosen only +// if its fee difference with the "non-grouped" tx is not bigger than m_max_aps_fee (see +// CreateTransaction in wallet/spend.cpp). +// *) In any case, there is a notion of "unsafe" outputs (see COutput::safe in wallet/coinselection.h), +// which are normally completely omitted from utxo selection (m_include_unsafe_inputs is false +// by default), and there is a limit on how many utxos can be in a single group. #[derive(Clone, Debug)] pub struct OutputGroup { /// The list of UTXOs contained in this output group. diff --git a/wallet/src/wallet/mod.rs b/wallet/src/wallet/mod.rs index fc6b5af712..2afd91d504 100644 --- a/wallet/src/wallet/mod.rs +++ b/wallet/src/wallet/mod.rs @@ -52,7 +52,7 @@ use crypto::{ }, vrf::VRFPublicKey, }; -use mempool::FeeRate; +use mempool::{FeeRate, MempoolConfig}; use tx_verifier::{CheckTransactionError, check_transaction, error::TokenIssuanceError}; use utils::{debug_panic_or_log, ensure}; use wallet_storage::{ @@ -327,6 +327,12 @@ pub enum WalletPoolsFilter { pub struct Wallet { chain_config: Arc, + // Note: unlike the chain config, the mempool config may potentially change, e.g. if the node + // is restarted with different parameters. At this moment, node restart requires the wallet + // to be restarted as well, so it's not an issue for now. But when we implement the automatic + // reconnection, the mempool config stored here should be updated each time we reconnect to + // the node. + mempool_config: Arc, db: Store, accounts: BTreeMap>, latest_median_time: BlockTimestamp, @@ -388,12 +394,20 @@ where { pub async fn create_new_wallet) -> WalletResult

>( chain_config: Arc, + mempool_config: Arc, db: Store, best_block: (BlockHeight, Id), wallet_type: WalletType, signer_provider: F, ) -> WalletResult> { - let mut wallet = Self::new_wallet(chain_config, db, wallet_type, signer_provider).await?; + let mut wallet = Self::new_wallet( + chain_config, + mempool_config, + db, + wallet_type, + signer_provider, + ) + .await?; match &mut wallet { WalletCreation::Wallet(w) => { @@ -408,15 +422,24 @@ where pub async fn recover_wallet) -> WalletResult

>( chain_config: Arc, + mempool_config: Arc, db: Store, wallet_type: WalletType, signer_provider: F, ) -> WalletResult> { - Self::new_wallet(chain_config, db, wallet_type, signer_provider).await + Self::new_wallet( + chain_config, + mempool_config, + db, + wallet_type, + signer_provider, + ) + .await } async fn new_wallet) -> WalletResult

>( chain_config: Arc, + mempool_config: Arc, mut db: Store, wallet_type: WalletType, signer_provider: F, @@ -463,6 +486,7 @@ where let latest_median_time = chain_config.genesis_block().timestamp(); let wallet = Wallet { chain_config, + mempool_config, db, accounts: [default_account].into(), latest_median_time, @@ -880,6 +904,7 @@ where F2: AsyncFnOnce(StoreTxRo) -> WalletResult

, >( chain_config: Arc, + mempool_config: Arc, mut db: Store, password: Option, pre_migration: F, @@ -957,6 +982,7 @@ where Ok(WalletCreation::Wallet(Wallet { chain_config, + mempool_config, db, accounts, latest_median_time, @@ -1771,6 +1797,8 @@ where additional_info: TxAdditionalInfo, ) -> WalletResult<(SignedTxWithFees, AddlData)> { let latest_median_time = self.latest_median_time; + let mempool_config = Arc::clone(&self.mempool_config); + self.async_for_account_rw_unlocked_and_check_tx_generic( account_index, additional_info, @@ -1787,6 +1815,7 @@ where consolidate_fee_rate, }, None, + &mempool_config, )?; let additional_data = additional_data_getter(&request); @@ -1811,6 +1840,8 @@ where ) -> WalletResult<(PartiallySignedTransaction, BTreeMap)> { let request = SendRequest::new().with_outputs(outputs); let latest_median_time = self.latest_median_time; + let mempool_config = Arc::clone(&self.mempool_config); + self.for_account_rw(account_index, |account, db_tx| { let mut request = account.select_inputs_for_send_request( request, @@ -1824,6 +1855,7 @@ where consolidate_fee_rate, }, None, + &mempool_config, )?; let fees = request.get_fees(); @@ -1910,6 +1942,8 @@ where ) -> WalletResult { let latest_median_time = self.latest_median_time; let additional_info = to_token_additional_info(token_info); + let mempool_config = Arc::clone(&self.mempool_config); + self.async_for_account_rw_unlocked_and_check_tx( account_index, additional_info, @@ -1924,6 +1958,7 @@ where current_fee_rate, consolidate_fee_rate, }, + &mempool_config, ) }, ) @@ -1940,6 +1975,8 @@ where ) -> WalletResult { let latest_median_time = self.latest_median_time; let additional_info = to_token_additional_info(token_info); + let mempool_config = Arc::clone(&self.mempool_config); + self.async_for_account_rw_unlocked_and_check_tx( account_index, additional_info, @@ -1953,6 +1990,7 @@ where current_fee_rate, consolidate_fee_rate, }, + &mempool_config, ) }, ) @@ -1968,6 +2006,8 @@ where ) -> WalletResult { let latest_median_time = self.latest_median_time; let additional_info = to_token_additional_info(token_info); + let mempool_config = Arc::clone(&self.mempool_config); + self.async_for_account_rw_unlocked_and_check_tx( account_index, additional_info, @@ -1980,6 +2020,7 @@ where current_fee_rate, consolidate_fee_rate, }, + &mempool_config, ) }, ) @@ -1996,6 +2037,8 @@ where ) -> WalletResult { let latest_median_time = self.latest_median_time; let additional_info = to_token_additional_info(token_info); + let mempool_config = Arc::clone(&self.mempool_config); + self.async_for_account_rw_unlocked_and_check_tx( account_index, additional_info, @@ -2009,6 +2052,7 @@ where current_fee_rate, consolidate_fee_rate, }, + &mempool_config, ) }, ) @@ -2024,6 +2068,8 @@ where ) -> WalletResult { let latest_median_time = self.latest_median_time; let additional_info = to_token_additional_info(token_info); + let mempool_config = Arc::clone(&self.mempool_config); + self.async_for_account_rw_unlocked_and_check_tx( account_index, additional_info, @@ -2036,6 +2082,7 @@ where current_fee_rate, consolidate_fee_rate, }, + &mempool_config, ) }, ) @@ -2052,6 +2099,8 @@ where ) -> WalletResult { let latest_median_time = self.latest_median_time; let additional_info = to_token_additional_info(token_info); + let mempool_config = Arc::clone(&self.mempool_config); + self.async_for_account_rw_unlocked_and_check_tx( account_index, additional_info, @@ -2065,6 +2114,7 @@ where current_fee_rate, consolidate_fee_rate, }, + &mempool_config, ) }, ) @@ -2081,6 +2131,8 @@ where ) -> WalletResult { let latest_median_time = self.latest_median_time; let additional_info = to_token_additional_info(token_info); + let mempool_config = Arc::clone(&self.mempool_config); + self.async_for_account_rw_unlocked_and_check_tx( account_index, additional_info, @@ -2094,6 +2146,7 @@ where current_fee_rate, consolidate_fee_rate, }, + &mempool_config, ) }, ) @@ -2177,6 +2230,7 @@ where ) -> WalletResult<(TokenId, SignedTxWithFees)> { let destination = address.into_object(); let latest_median_time = self.latest_median_time; + let mempool_config = Arc::clone(&self.mempool_config); let signed_transaction = self .async_for_account_rw_unlocked_and_check_tx( @@ -2194,6 +2248,7 @@ where current_fee_rate, consolidate_fee_rate, }, + &mempool_config, ) }, ) @@ -2215,6 +2270,8 @@ where stake_pool_arguments: StakePoolCreationArguments, ) -> WalletResult { let latest_median_time = self.latest_median_time; + let mempool_config = Arc::clone(&self.mempool_config); + self.async_for_account_rw_unlocked_and_check_tx( account_index, TxAdditionalInfo::new(), @@ -2227,6 +2284,7 @@ where current_fee_rate, consolidate_fee_rate, }, + &mempool_config, ) }, ) @@ -2321,6 +2379,8 @@ where additional_info: TxAdditionalInfo, ) -> WalletResult { let latest_median_time = self.latest_median_time; + let mempool_config = Arc::clone(&self.mempool_config); + self.async_for_account_rw_unlocked_and_check_tx( account_index, additional_info, @@ -2334,6 +2394,7 @@ where current_fee_rate, consolidate_fee_rate, }, + &mempool_config, ) }, ) @@ -2359,6 +2420,8 @@ where additional_info: TxAdditionalInfo, ) -> WalletResult<(OrderId, SignedTxWithFees)> { let latest_median_time = self.latest_median_time; + let mempool_config = Arc::clone(&self.mempool_config); + let tx = self .async_for_account_rw_unlocked_and_check_tx( account_index, @@ -2374,6 +2437,7 @@ where current_fee_rate, consolidate_fee_rate, }, + &mempool_config, ) }, ) @@ -2394,6 +2458,8 @@ where additional_info: TxAdditionalInfo, ) -> WalletResult { let latest_median_time = self.latest_median_time; + let mempool_config = Arc::clone(&self.mempool_config); + self.async_for_account_rw_unlocked_and_check_tx( account_index, additional_info, @@ -2408,6 +2474,7 @@ where current_fee_rate, consolidate_fee_rate, }, + &mempool_config, ) }, ) @@ -2427,6 +2494,8 @@ where additional_info: TxAdditionalInfo, ) -> WalletResult { let latest_median_time = self.latest_median_time; + let mempool_config = Arc::clone(&self.mempool_config); + self.async_for_account_rw_unlocked_and_check_tx( account_index, additional_info, @@ -2442,6 +2511,7 @@ where current_fee_rate, consolidate_fee_rate, }, + &mempool_config, ) }, ) @@ -2458,6 +2528,8 @@ where additional_info: TxAdditionalInfo, ) -> WalletResult { let latest_median_time = self.latest_median_time; + let mempool_config = Arc::clone(&self.mempool_config); + self.async_for_account_rw_unlocked_and_check_tx( account_index, additional_info, @@ -2471,6 +2543,7 @@ where current_fee_rate, consolidate_fee_rate, }, + &mempool_config, ) }, ) @@ -2489,6 +2562,8 @@ where additional_info: TxAdditionalInfo, ) -> WalletResult { let latest_median_time = self.latest_median_time; + let mempool_config = Arc::clone(&self.mempool_config); + self.async_for_account_rw_unlocked_and_check_tx( account_index, additional_info, @@ -2503,6 +2578,7 @@ where current_fee_rate, consolidate_fee_rate, }, + &mempool_config, ) }, ) @@ -2771,6 +2847,8 @@ where stake_pool_arguments: StakePoolCreationArguments, ) -> WalletResult { let latest_median_time = self.latest_median_time; + let mempool_config = Arc::clone(&self.mempool_config); + self.async_for_account_rw_unlocked_and_check_tx( account_index, TxAdditionalInfo::new(), @@ -2783,6 +2861,7 @@ where current_fee_rate, consolidate_fee_rate, }, + &mempool_config, ) }, ) diff --git a/wallet/src/wallet/test_helpers.rs b/wallet/src/wallet/test_helpers.rs index 16aeb99db6..59fd24a57b 100644 --- a/wallet/src/wallet/test_helpers.rs +++ b/wallet/src/wallet/test_helpers.rs @@ -42,6 +42,7 @@ pub async fn create_wallet_with_mnemonic( let genesis_block_id = chain_config.genesis_block_id(); Wallet::create_new_wallet( chain_config.clone(), + Default::default(), db, (BlockHeight::new(0), genesis_block_id), WalletType::Hot, @@ -78,6 +79,7 @@ pub async fn create_wallet_with_mnemonic_and_named_db( let genesis_block_id = chain_config.genesis_block_id(); Wallet::create_new_wallet( chain_config.clone(), + Default::default(), db, (BlockHeight::new(0), genesis_block_id), WalletType::Hot, diff --git a/wallet/src/wallet/tests.rs b/wallet/src/wallet/tests.rs index fbb216aed6..b3f102ec0b 100644 --- a/wallet/src/wallet/tests.rs +++ b/wallet/src/wallet/tests.rs @@ -279,6 +279,7 @@ async fn verify_wallet_balance( let db_copy = reopen_db_func(); let wallet = Wallet::load_wallet( Arc::clone(chain_config), + Default::default(), db_copy, None, |_| Ok(()), @@ -387,6 +388,7 @@ async fn wallet_creation_in_memory() { // fail to load an empty wallet match Wallet::load_wallet( Arc::clone(&chain_config), + Default::default(), empty_db, None, |_| Ok(()), @@ -408,6 +410,7 @@ async fn wallet_creation_in_memory() { let wrong_network_chain_config = Arc::new(create_mainnet()); match Wallet::load_wallet( wrong_network_chain_config.clone(), + Default::default(), initialized_db, None, |_| Ok(()), @@ -429,6 +432,7 @@ async fn wallet_creation_in_memory() { // successfully load a wallet from initialized db let _wallet = Wallet::load_wallet( chain_config.clone(), + Default::default(), initialized_db, None, |_| Ok(()), @@ -459,6 +463,7 @@ async fn wallet_creation_in_memory() { eprintln!("load again."); match Wallet::load_wallet( Arc::clone(&chain_config), + Default::default(), wallet.db, None, |_| Ok(()), @@ -505,6 +510,7 @@ async fn wallet_migration_to_v2(#[case] seed: Seed) { let genesis_block_id = chain_config.genesis_block_id(); let mut wallet = Wallet::create_new_wallet( Arc::clone(&chain_config), + Default::default(), db, (BlockHeight::new(0), genesis_block_id), WalletType::Hot, @@ -572,6 +578,7 @@ async fn wallet_migration_to_v2(#[case] seed: Seed) { let wallet = Wallet::load_wallet( Arc::clone(&chain_config), + Default::default(), new_db, password, |_| Ok(()), @@ -629,6 +636,7 @@ async fn wallet_seed_phrase_retrieval(#[case] seed: Seed) { let genesis_block_id = chain_config.genesis_block_id(); let mut wallet = Wallet::create_new_wallet( Arc::clone(&chain_config), + Default::default(), db, (BlockHeight::new(0), genesis_block_id), WalletType::Hot, @@ -728,6 +736,7 @@ async fn wallet_seed_phrase_check_address() { let wallet_passphrase: Option = None; let mut wallet = Wallet::create_new_wallet( Arc::clone(&chain_config), + Default::default(), db, (BlockHeight::new(0), genesis_block_id), WalletType::Hot, @@ -773,6 +782,7 @@ async fn wallet_seed_phrase_check_address() { let wallet_passphrase: Option = Some("phrase123".into()); let mut wallet = Wallet::create_new_wallet( Arc::clone(&chain_config), + Default::default(), db, (BlockHeight::new(0), genesis_block_id), WalletType::Hot, @@ -1153,6 +1163,7 @@ async fn test_wallet_accounts( let db_copy = reopen_db_func(); let wallet = Wallet::load_wallet( Arc::clone(chain_config), + Default::default(), db_copy, None, |_| Ok(()), diff --git a/wallet/wallet-controller/src/lib.rs b/wallet/wallet-controller/src/lib.rs index fb257bc0a0..be76223bed 100644 --- a/wallet/wallet-controller/src/lib.rs +++ b/wallet/wallet-controller/src/lib.rs @@ -38,6 +38,7 @@ use helpers::{ fetch_input_infos, fetch_rpc_token_info, fetch_utxo, fetch_utxo_extra_info, into_balances, }; use itertools::Itertools as _; +use mempool::MempoolConfig; use node_comm::node_traits::NodeInterfaceError as _; use runtime_wallet::RuntimeWallet; use std::{ @@ -302,6 +303,7 @@ where pub async fn create_wallet( chain_config: Arc, + mempool_config: Arc, file_path: impl AsRef, args: WalletTypeArgsComputed, best_block: (BlockHeight, Id), @@ -328,6 +330,7 @@ where wallet::Wallet::create_new_wallet( Arc::clone(&chain_config), + mempool_config, db, best_block, wallet_type, @@ -349,6 +352,7 @@ where #[cfg(feature = "trezor")] WalletTypeArgsComputed::Trezor { device_id } => wallet::Wallet::create_new_wallet( Arc::clone(&chain_config), + mempool_config, db, best_block, wallet_type, @@ -366,6 +370,7 @@ where #[cfg(feature = "ledger")] WalletTypeArgsComputed::Ledger => wallet::Wallet::create_new_wallet( Arc::clone(&chain_config), + mempool_config, db, best_block, wallet_type, @@ -382,6 +387,7 @@ where pub async fn recover_wallet( chain_config: Arc, + mempool_config: Arc, file_path: impl AsRef, args: WalletTypeArgsComputed, wallet_type: WalletType, @@ -407,6 +413,7 @@ where let wallet = wallet::Wallet::recover_wallet( Arc::clone(&chain_config), + mempool_config, db, wallet_type, async |db_tx| { @@ -428,6 +435,7 @@ where WalletTypeArgsComputed::Trezor { device_id } => { let wallet = wallet::Wallet::recover_wallet( Arc::clone(&chain_config), + mempool_config, db, wallet_type, async |_db_tx| { @@ -446,6 +454,7 @@ where WalletTypeArgsComputed::Ledger => { let wallet = wallet::Wallet::recover_wallet( Arc::clone(&chain_config), + mempool_config, db, wallet_type, async |_db_tx| LedgerSignerProvider::new().await.map_err(Into::into), @@ -512,6 +521,7 @@ where pub async fn open_wallet( chain_config: Arc, + mempool_config: Arc, file_path: impl AsRef, password: Option, current_controller_mode: WalletControllerMode, @@ -534,6 +544,7 @@ where WalletType::Cold | WalletType::Hot => { let wallet = wallet::Wallet::load_wallet( Arc::clone(&chain_config), + mempool_config, db, password, |version| Self::make_backup_wallet_file(file_path.as_ref(), version), @@ -551,6 +562,7 @@ where WalletType::Trezor => { let wallet = wallet::Wallet::load_wallet( Arc::clone(&chain_config), + mempool_config, db, password, |version| Self::make_backup_wallet_file(file_path.as_ref(), version), @@ -572,6 +584,7 @@ where WalletType::Ledger => { let wallet = wallet::Wallet::load_wallet( Arc::clone(&chain_config), + mempool_config, db, password, |version| Self::make_backup_wallet_file(file_path.as_ref(), version), @@ -1588,7 +1601,6 @@ where log::error!("Mempool notifications channel is closed."); tokio::time::sleep(ERROR_DELAY).await; - match self.rpc_client .mempool_subscribe_to_events() .await { diff --git a/wallet/wallet-controller/src/sync/tests/mod.rs b/wallet/wallet-controller/src/sync/tests/mod.rs index 8bdc2d0c2b..579489538b 100644 --- a/wallet/wallet-controller/src/sync/tests/mod.rs +++ b/wallet/wallet-controller/src/sync/tests/mod.rs @@ -38,7 +38,7 @@ use common::{ use consensus::GenerateBlockInputData; use crypto::ephemeral_e2e::EndToEndPublicKey; use logging::log; -use mempool::{FeeRate, tx_accumulator::PackingStrategy}; +use mempool::{FeeRate, MempoolConfig, tx_accumulator::PackingStrategy}; use mempool_types::tx_options::TxOptionsOverrides; use node_comm::{ node_traits::{ConnectedPeer, MempoolEvents, PeerId}, @@ -450,6 +450,10 @@ impl NodeInterface for MockNode { unreachable!() } + async fn mempool_get_config(&self) -> Result, Self::Error> { + unreachable!() + } + async fn mempool_subscribe_to_events(&self) -> Result { unreachable!() } diff --git a/wallet/wallet-node-client/src/handles_client/mod.rs b/wallet/wallet-node-client/src/handles_client/mod.rs index 925e8b75bf..32ce9ffc61 100644 --- a/wallet/wallet-node-client/src/handles_client/mod.rs +++ b/wallet/wallet-node-client/src/handles_client/mod.rs @@ -34,7 +34,7 @@ use common::{ use consensus::GenerateBlockInputData; use crypto::ephemeral_e2e::EndToEndPublicKey; use mempool::{ - FeeRate, MempoolHandle, event::MempoolEvent, tx_accumulator::PackingStrategy, + FeeRate, MempoolConfig, MempoolHandle, event::MempoolEvent, tx_accumulator::PackingStrategy, tx_options::TxOptionsOverrides, }; use p2p::{ @@ -531,4 +531,9 @@ impl NodeInterface for WalletHandlesClient { let res = self.mempool.call(move |this| this.get_all()).await?; Ok(res) } + + async fn mempool_get_config(&self) -> Result, Self::Error> { + let config = self.mempool.call(move |this| this.config().clone()).await?; + Ok(Some(config)) + } } diff --git a/wallet/wallet-node-client/src/mock.rs b/wallet/wallet-node-client/src/mock.rs index 13fa44b6b8..1f7770ac48 100644 --- a/wallet/wallet-node-client/src/mock.rs +++ b/wallet/wallet-node-client/src/mock.rs @@ -33,7 +33,9 @@ use common::{ }; use consensus::GenerateBlockInputData; use crypto::ephemeral_e2e::EndToEndPublicKey; -use mempool::{FeeRate, tx_accumulator::PackingStrategy, tx_options::TxOptionsOverrides}; +use mempool::{ + FeeRate, MempoolConfig, tx_accumulator::PackingStrategy, tx_options::TxOptionsOverrides, +}; use p2p::{ interface::types::ConnectedPeer, types::{PeerId, bannable_address::BannableAddress, socket_address::SocketAddress}, @@ -330,6 +332,10 @@ impl NodeInterface for ClonableMockNodeInterface { self.lock().await.mempool_subscribe_to_events().await } + async fn mempool_get_config(&self) -> Result, Self::Error> { + self.lock().await.mempool_get_config().await + } + async fn get_utxo(&self, outpoint: UtxoOutPoint) -> Result, Self::Error> { self.lock().await.get_utxo(outpoint).await } diff --git a/wallet/wallet-node-client/src/node_traits.rs b/wallet/wallet-node-client/src/node_traits.rs index 3a6f2a5696..2b4cb47392 100644 --- a/wallet/wallet-node-client/src/node_traits.rs +++ b/wallet/wallet-node-client/src/node_traits.rs @@ -32,7 +32,9 @@ use common::{ }; use consensus::GenerateBlockInputData; use crypto::ephemeral_e2e::EndToEndPublicKey; -use mempool::{FeeRate, tx_accumulator::PackingStrategy, tx_options::TxOptionsOverrides}; +use mempool::{ + FeeRate, MempoolConfig, tx_accumulator::PackingStrategy, tx_options::TxOptionsOverrides, +}; use p2p::types::{bannable_address::BannableAddress, socket_address::SocketAddress}; use utils_networking::IpOrSocketAddress; use wallet_types::wallet_type::WalletControllerMode; @@ -174,6 +176,8 @@ pub trait NodeInterface { ) -> Result, Self::Error>; async fn mempool_get_transactions(&self) -> Result, Self::Error>; async fn mempool_subscribe_to_events(&self) -> Result; + /// Obtain MempoolConfig from the node. None is returned if the wallet is in the cold mode. + async fn mempool_get_config(&self) -> Result, Self::Error>; async fn get_utxo(&self, outpoint: UtxoOutPoint) -> Result, Self::Error>; } diff --git a/wallet/wallet-node-client/src/rpc_client/client_impl.rs b/wallet/wallet-node-client/src/rpc_client/client_impl.rs index 34c814ab44..3f42478c0d 100644 --- a/wallet/wallet-node-client/src/rpc_client/client_impl.rs +++ b/wallet/wallet-node-client/src/rpc_client/client_impl.rs @@ -35,8 +35,8 @@ use common::{ use consensus::GenerateBlockInputData; use crypto::ephemeral_e2e::EndToEndPublicKey; use mempool::{ - FeeRate, rpc::MempoolRpcClient, rpc_event::RpcEvent, tx_accumulator::PackingStrategy, - tx_options::TxOptionsOverrides, + FeeRate, MempoolConfig, rpc::MempoolRpcClient, rpc_event::RpcEvent, + tx_accumulator::PackingStrategy, tx_options::TxOptionsOverrides, }; use p2p::{ interface::types::ConnectedPeer, @@ -443,6 +443,13 @@ impl NodeInterface for NodeRpcClient { Ok(Box::new(subscription)) } + async fn mempool_get_config(&self) -> Result, Self::Error> { + let config = MempoolRpcClient::get_config(&*self.rpc_client) + .await + .map_err(NodeRpcError::ResponseError)?; + Ok(Some(config.into())) + } + async fn get_utxo(&self, outpoint: UtxoOutPoint) -> Result, Self::Error> { ChainstateRpcClient::get_utxo(&*self.rpc_client, outpoint.into()) .await diff --git a/wallet/wallet-node-client/src/rpc_client/cold_wallet_client.rs b/wallet/wallet-node-client/src/rpc_client/cold_wallet_client.rs index f48130bd74..2a90442eba 100644 --- a/wallet/wallet-node-client/src/rpc_client/cold_wallet_client.rs +++ b/wallet/wallet-node-client/src/rpc_client/cold_wallet_client.rs @@ -31,7 +31,9 @@ use common::{ }; use consensus::GenerateBlockInputData; use crypto::ephemeral_e2e::EndToEndPublicKey; -use mempool::{FeeRate, tx_accumulator::PackingStrategy, tx_options::TxOptionsOverrides}; +use mempool::{ + FeeRate, MempoolConfig, tx_accumulator::PackingStrategy, tx_options::TxOptionsOverrides, +}; use p2p::{ interface::types::ConnectedPeer, types::{PeerId, bannable_address::BannableAddress, socket_address::SocketAddress}, @@ -311,6 +313,10 @@ impl NodeInterface for ColdWalletClient { Err(ColdWalletRpcError::NotAvailable) } + async fn mempool_get_config(&self) -> Result, Self::Error> { + Ok(None) + } + async fn get_utxo( &self, _outpoint: common::chain::UtxoOutPoint, diff --git a/wallet/wallet-rpc-lib/src/service/mod.rs b/wallet/wallet-rpc-lib/src/service/mod.rs index a6ecfa729b..4951d193d5 100644 --- a/wallet/wallet-rpc-lib/src/service/mod.rs +++ b/wallet/wallet-rpc-lib/src/service/mod.rs @@ -21,6 +21,7 @@ use std::{path::PathBuf, sync::Arc}; use common::chain::ChainConfig; use crypto::key::hdkd::u31::U31; +use mempool::MempoolConfig; use utils::shallow_clone::ShallowClone; pub use events::{Event, TxState}; @@ -45,10 +46,15 @@ pub struct WalletService { pub enum InitError { #[error(transparent)] Wallet(#[from] wallet::WalletError), + #[error(transparent)] NodeRpc(#[from] node_comm::rpc_client::NodeRpcError), + #[error(transparent)] Controller(#[from] WalletControllerError), + + #[error("Node call error: {0}")] + NodeCallError(N::Error), } impl WalletService @@ -65,12 +71,16 @@ where let (wallet_events, events_rx) = WalletServiceEvents::new(); let (command_tx, command_rx) = tokio::sync::mpsc::unbounded_channel(); + let mempool_config = + get_mempool_config(&node_rpc).await.map_err(InitError::NodeCallError)?; + let controller = if let Some((wallet_file, open_as_wallet_type)) = &wallet_file { let wallet = { // TODO: Allow user to set password (config file only) let wallet_password = None; WalletController::open_wallet( chain_config.shallow_clone(), + Arc::new(mempool_config), wallet_file, wallet_password, node_rpc.is_cold_wallet_node().await, @@ -139,3 +149,9 @@ where self.task.await } } + +async fn get_mempool_config(node_rpc: &N) -> Result { + // If node_rpc.mempool_get_config() returns None (i.e. the wallet is in the cold mode), + // use the default mempool config. + Ok(node_rpc.mempool_get_config().await?.unwrap_or_default()) +} diff --git a/wallet/wallet-rpc-lib/src/service/worker.rs b/wallet/wallet-rpc-lib/src/service/worker.rs index 5fd26b0e85..36ed6a2b64 100644 --- a/wallet/wallet-rpc-lib/src/service/worker.rs +++ b/wallet/wallet-rpc-lib/src/service/worker.rs @@ -30,7 +30,7 @@ use wallet_types::{scan_blockchain::ScanBlockchain, wallet_type::WalletType}; use crate::{Event, types::RpcError}; -use super::WalletServiceEvents; +use super::{WalletServiceEvents, get_mempool_config}; pub type WalletController = wallet_controller::RpcController; pub type WalletControllerError = wallet_controller::ControllerError; @@ -162,8 +162,13 @@ where ControllerError::WalletFileAlreadyOpen ); + let mempool_config = get_mempool_config(&self.node_rpc) + .await + .map_err(ControllerError::NodeCallError)?; + let wallet = WalletController::open_wallet( self.chain_config.clone(), + Arc::new(mempool_config), wallet_path, password, self.node_rpc.is_cold_wallet_node().await, @@ -217,10 +222,15 @@ where let (computed_args, wallet_created) = args.parse_or_generate_mnemonic_if_needed().map_err(RpcError::InvalidMnemonic)?; + let mempool_config = get_mempool_config(&self.node_rpc) + .await + .map_err(ControllerError::NodeCallError)?; + let wallet = if options.scan_blockchain.skip_scanning_the_blockchain() { let info = self.node_rpc.chainstate_info().await.map_err(RpcError::RpcError)?; WalletController::create_wallet( self.chain_config.clone(), + Arc::new(mempool_config), wallet_path, computed_args, (info.best_block_height, info.best_block_id), @@ -231,6 +241,7 @@ where } else { WalletController::recover_wallet( self.chain_config.clone(), + Arc::new(mempool_config), wallet_path, computed_args, wallet_type, diff --git a/wallet/wallet-rpc-lib/tests/utils.rs b/wallet/wallet-rpc-lib/tests/utils.rs index fc23e83d57..e2a0c658a2 100644 --- a/wallet/wallet-rpc-lib/tests/utils.rs +++ b/wallet/wallet-rpc-lib/tests/utils.rs @@ -67,6 +67,7 @@ impl TestFramework { let _wallet = wallet::Wallet::create_new_wallet( Arc::clone(&chain_config), + Default::default(), db, (BlockHeight::new(0), chain_config.genesis_block_id()), WalletType::Hot, From 82f7dd318b2a12b174ff26b8a4c0de4d27fdcd10 Mon Sep 17 00:00:00 2001 From: Mykhailo Kremniov Date: Fri, 22 May 2026 17:05:01 +0300 Subject: [PATCH 5/6] Reject zero max_cluster_tx_count/max_cluster_size_bytes at node start --- blockprod/src/lib.rs | 3 ++- mempool/src/config.rs | 27 ++++++++++++++++++- mempool/src/error/ban_score.rs | 3 +++ mempool/src/error/mod.rs | 5 +++- .../src/interface/mempool_interface_impl.rs | 8 +++--- node-lib/src/runner.rs | 2 +- p2p/src/sync/tests/helpers/mod.rs | 3 ++- p2p/test-utils/src/lib.rs | 3 ++- p2p/tests/shutdown.rs | 3 ++- wallet/wallet-node-client/tests/call_tests.rs | 3 ++- wallet/wallet-test-node/src/lib.rs | 3 ++- 11 files changed, 51 insertions(+), 12 deletions(-) diff --git a/blockprod/src/lib.rs b/blockprod/src/lib.rs index 36425dfd14..510ff8d8bc 100644 --- a/blockprod/src/lib.rs +++ b/blockprod/src/lib.rs @@ -327,7 +327,8 @@ mod tests { mempool_config, subsystem::Handle::clone(&chainstate), time_getter.clone(), - ); + ) + .unwrap(); let mempool = manager.add_custom_subsystem("mempool", |hdl, _| mempool_init.init(hdl)); let mut p2p_config = test_p2p_config(); diff --git a/mempool/src/config.rs b/mempool/src/config.rs index c362799807..2972da38ee 100644 --- a/mempool/src/config.rs +++ b/mempool/src/config.rs @@ -17,7 +17,7 @@ use std::{str::FromStr, time::Duration}; use common::primitives::{Amount, BlockDistance}; use rpc::description::HasValueHint; -use utils::make_config_setting; +use utils::{ensure, make_config_setting}; use crate::FeeRate; @@ -174,6 +174,22 @@ impl MempoolConfig { max_cluster_size_bytes: **max_cluster_size_bytes, } } + + pub fn validate(&self) -> Result<(), ConfigError> { + // Note: setting these values to 0 makes no sense (unless the goal is to prevent + // any tx from entering the mempool). Also, `max_cluster_tx_count=0` will behave + // the same way as `max_cluster_tx_count=1`, because parentless txs currently + // skip the cluster tx count check. So we just forbid passing zeros here. + ensure!( + *self.max_cluster_tx_count > 0, + ConfigError::MaxClusterTxCountCannotBeZero + ); + ensure!( + *self.max_cluster_size_bytes > 0, + ConfigError::MaxClusterSizeBytesCannotBeZero + ); + Ok(()) + } } impl From for MempoolConfig { @@ -199,3 +215,12 @@ pub struct RpcMempoolConfig { pub max_cluster_tx_count: usize, pub max_cluster_size_bytes: usize, } + +#[derive(Debug, Clone, thiserror::Error, PartialEq, Eq)] +pub enum ConfigError { + #[error("Maximum cluster transaction count cannot be zero")] + MaxClusterTxCountCannotBeZero, + + #[error("Maximum cluster size in bytes cannot be zero")] + MaxClusterSizeBytesCannotBeZero, +} diff --git a/mempool/src/error/ban_score.rs b/mempool/src/error/ban_score.rs index ffd0ec2c48..e27715e569 100644 --- a/mempool/src/error/ban_score.rs +++ b/mempool/src/error/ban_score.rs @@ -53,6 +53,9 @@ impl MempoolBanScore for Error { // Internal error Error::SubsystemCallError(_) => 0, + + // A configuration error is not the peer's fault. + Error::ConfigError(_) => 0, } } } diff --git a/mempool/src/error/mod.rs b/mempool/src/error/mod.rs index 7f4ab57203..470f250faf 100644 --- a/mempool/src/error/mod.rs +++ b/mempool/src/error/mod.rs @@ -23,7 +23,7 @@ use common::{ primitives::{H256, Id, amount::DisplayAmount}, }; -use crate::pool::fee::Fee; +use crate::{config::ConfigError, pool::fee::Fee}; pub use ban_score::MempoolBanScore; @@ -76,6 +76,9 @@ pub enum Error { #[error("Transaction collection error: {0}")] TxCollectionError(#[from] TxCollectionError), + + #[error(transparent)] + ConfigError(#[from] ConfigError), } #[derive(Debug, Clone, Error, PartialEq, Eq)] diff --git a/mempool/src/interface/mempool_interface_impl.rs b/mempool/src/interface/mempool_interface_impl.rs index 21924f0a23..4fbe4561c1 100644 --- a/mempool/src/interface/mempool_interface_impl.rs +++ b/mempool/src/interface/mempool_interface_impl.rs @@ -53,13 +53,15 @@ impl MempoolInit { mempool_config: MempoolConfig, chainstate_handle: chainstate::ChainstateHandle, time_getter: TimeGetter, - ) -> Self { - Self { + ) -> Result { + mempool_config.validate()?; + + Ok(Self { chain_config, mempool_config, chainstate_handle, time_getter, - } + }) } pub async fn init( diff --git a/node-lib/src/runner.rs b/node-lib/src/runner.rs index 5f0feca81c..78fdc86765 100644 --- a/node-lib/src/runner.rs +++ b/node-lib/src/runner.rs @@ -107,7 +107,7 @@ async fn initialize( node_config.mempool.unwrap_or_default().into(), subsystem::Handle::clone(&chainstate), Default::default(), - ); + )?; let mempool = manager.add_custom_subsystem("mempool", |handle, _| mempool_init.init(handle)); // P2P subsystem diff --git a/p2p/src/sync/tests/helpers/mod.rs b/p2p/src/sync/tests/helpers/mod.rs index cf613b504b..6bfc9db367 100644 --- a/p2p/src/sync/tests/helpers/mod.rs +++ b/p2p/src/sync/tests/helpers/mod.rs @@ -694,7 +694,8 @@ impl TestNodeBuilder { mempool_config, chainstate.clone(), time_getter.clone(), - ); + ) + .unwrap(); let mempool = manager.add_custom_subsystem("p2p-sync-test-mempool", |h, _| mempool_init.init(h)); diff --git a/p2p/test-utils/src/lib.rs b/p2p/test-utils/src/lib.rs index 46ce2d0a90..27cb49e5f0 100644 --- a/p2p/test-utils/src/lib.rs +++ b/p2p/test-utils/src/lib.rs @@ -93,7 +93,8 @@ pub fn start_subsystems_generic( mempool_config, chainstate.clone(), time_getter, - ); + ) + .unwrap(); let mempool = manager.add_custom_subsystem("p2p-test-mempool", |handle, _| mempool_init.init(handle)); diff --git a/p2p/tests/shutdown.rs b/p2p/tests/shutdown.rs index ca44f8c129..477564a20e 100644 --- a/p2p/tests/shutdown.rs +++ b/p2p/tests/shutdown.rs @@ -60,7 +60,8 @@ async fn shutdown_timeout() { mempool_config, chainstate.clone(), Default::default(), - ); + ) + .unwrap(); let mempool = manager.add_custom_subsystem("shutdown-test-mempool", |hdl, _| mempool_init.init(hdl)); diff --git a/wallet/wallet-node-client/tests/call_tests.rs b/wallet/wallet-node-client/tests/call_tests.rs index e68dcbf12e..4714ba389d 100644 --- a/wallet/wallet-node-client/tests/call_tests.rs +++ b/wallet/wallet-node-client/tests/call_tests.rs @@ -91,7 +91,8 @@ pub async fn start_subsystems( mempool_config, chainstate_handle.clone(), Default::default(), - ); + ) + .unwrap(); let mempool_handle = manager.add_custom_subsystem("test-mempool", |hdl, _| mempool_init.init(hdl)); diff --git a/wallet/wallet-test-node/src/lib.rs b/wallet/wallet-test-node/src/lib.rs index 55717eb437..3472c5534b 100644 --- a/wallet/wallet-test-node/src/lib.rs +++ b/wallet/wallet-test-node/src/lib.rs @@ -213,7 +213,8 @@ pub async fn start_node(chain_config: Arc) -> (subsystem::Manager, mempool_config, chainstate.clone(), Default::default(), - ); + ) + .unwrap(); let mempool = manager.add_custom_subsystem("wallet-cli-test-mempool", |hdl, _| mempool_init.init(hdl)); From 9661f7f4712471a3b6d0a80ad20fa5e771a5f788 Mon Sep 17 00:00:00 2001 From: Mykhailo Kremniov Date: Fri, 22 May 2026 18:19:24 +0300 Subject: [PATCH 6/6] Minor improvements and cleanup; appease clippy --- mempool/src/error/ban_score.rs | 4 +-- mempool/src/error/mod.rs | 4 +-- mempool/src/pool/tests/relatives.rs | 4 +-- mempool/src/pool/tx_pool/collect_txs.rs | 1 + mempool/src/pool/tx_pool/mod.rs | 5 +++- mempool/src/pool/tx_pool/store/mod.rs | 27 +++++++++++++++---- mempool/src/pool/tx_pool/tests/basic.rs | 12 +++++++-- wallet/src/account/mod.rs | 6 +++++ .../src/account/utxo_selector/output_group.rs | 4 +-- wallet/src/wallet/mod.rs | 5 ++-- wallet/wallet-controller/src/lib.rs | 1 + 11 files changed, 55 insertions(+), 18 deletions(-) diff --git a/mempool/src/error/ban_score.rs b/mempool/src/error/ban_score.rs index e27715e569..51a288f287 100644 --- a/mempool/src/error/ban_score.rs +++ b/mempool/src/error/ban_score.rs @@ -54,7 +54,7 @@ impl MempoolBanScore for Error { // Internal error Error::SubsystemCallError(_) => 0, - // A configuration error is not the peer's fault. + // A configuration error is not peer's fault. Error::ConfigError(_) => 0, } } @@ -71,7 +71,7 @@ impl MempoolBanScore for MempoolPolicyError { // Don't punish the peer for this, because max cluster size is technically configurable // and the peer doesn't know the node's mempool configuration. - MempoolPolicyError::TxSizeExceedsMaxClusterSize => 0, + MempoolPolicyError::TxSizeExceedsMaxClusterSize { .. } => 0, // Errors to do with transaction conflicts and replacements are not punished since the // peer may not be aware of all the transactions the node has in the mempool. diff --git a/mempool/src/error/mod.rs b/mempool/src/error/mod.rs index 470f250faf..6275c92200 100644 --- a/mempool/src/error/mod.rs +++ b/mempool/src/error/mod.rs @@ -98,8 +98,8 @@ pub enum MempoolPolicyError { #[error("Transaction size exceeds the maximum block size")] TxSizeExceedsMaxBlockSize, - #[error("Transaction size exceeds the maximum cluster size")] - TxSizeExceedsMaxClusterSize, + #[error("Transaction size ({tx_size}) exceeds the maximum cluster size ({limit})")] + TxSizeExceedsMaxClusterSize { tx_size: usize, limit: usize }, #[error( "Replacement transaction has fee lower than the original. Replacement fee is {replacement_fee:?}, original fee {original_fee:?}" diff --git a/mempool/src/pool/tests/relatives.rs b/mempool/src/pool/tests/relatives.rs index d3e73d29b6..c9bb1eb215 100644 --- a/mempool/src/pool/tests/relatives.rs +++ b/mempool/src/pool/tests/relatives.rs @@ -312,7 +312,7 @@ async fn test_relatives_and_cluster_tx_count_limit(#[case] seed: Seed) { assert_descendants(&mempool, &b5, &[]); } - // A non-orphan subtest with a bigger limit + // A non-orphan subtest with a bigger limit { let mut mempool = create_mempool(5); @@ -388,7 +388,7 @@ async fn test_relatives_and_cluster_tx_count_limit(#[case] seed: Seed) { assert_descendants(&mempool, &b2, &[]); } - // A non-orphan subtest with the default limit. All txs will be added and form a single cluster. + // A non-orphan subtest with the default limit. All txs will be added and form a single cluster. { let mut mempool = create_mempool(*MaxClusterTxCount::default()); diff --git a/mempool/src/pool/tx_pool/collect_txs.rs b/mempool/src/pool/tx_pool/collect_txs.rs index 9e7fdb47fa..e938cc4c06 100644 --- a/mempool/src/pool/tx_pool/collect_txs.rs +++ b/mempool/src/pool/tx_pool/collect_txs.rs @@ -207,6 +207,7 @@ pub fn collect_txs( // 6) pool creation vs delegation id creation. // Need to update TxDependency to handle these relationships too and use TxDependency // when determining "parents" for TxMempoolEntry. + // Also see https://github.com/mintlayer/mintlayer-core/issues/2065. // The old TODO goes below. // TODO Narrow down when the critical error is presented. Printing the error may be a diff --git a/mempool/src/pool/tx_pool/mod.rs b/mempool/src/pool/tx_pool/mod.rs index 3b821e0cbd..f877902f2f 100644 --- a/mempool/src/pool/tx_pool/mod.rs +++ b/mempool/src/pool/tx_pool/mod.rs @@ -291,7 +291,10 @@ impl TxPool { let max_cluster_size = *self.mempool_config.max_cluster_size_bytes; ensure!( size <= max_cluster_size, - MempoolPolicyError::TxSizeExceedsMaxClusterSize + MempoolPolicyError::TxSizeExceedsMaxClusterSize { + tx_size: size, + limit: max_cluster_size + } ); Ok(()) diff --git a/mempool/src/pool/tx_pool/store/mod.rs b/mempool/src/pool/tx_pool/store/mod.rs index 8be45e5072..3a605ffe2b 100644 --- a/mempool/src/pool/tx_pool/store/mod.rs +++ b/mempool/src/pool/tx_pool/store/mod.rs @@ -851,6 +851,7 @@ impl TxMempoolEntry { self.parents.iter().copied(), RelativesKind::Ancestors, |_| Ok::<_, MempoolStoreInvariantError>(()), + Some(self.count_with_ancestors - 1), ); match result { @@ -874,6 +875,7 @@ impl TxMempoolEntry { self.children.iter().copied(), RelativesKind::Descendants, |_| Ok::<_, MempoolStoreInvariantError>(()), + Some(self.count_with_descendants - 1), ); match result { @@ -891,9 +893,13 @@ impl TxMempoolEntry { /// /// This function is mainly intended for testing purposes. pub fn collect_cluster(&self, store: &MempoolStore) -> Result { - let cluster = collect_relatives(store, [*self.tx_id()], RelativesKind::Cluster, |_| { - Ok::<_, MempoolPolicyError>(()) - })?; + let cluster = collect_relatives( + store, + [*self.tx_id()], + RelativesKind::Cluster, + |_| Ok::<_, MempoolPolicyError>(()), + Some(self.count_with_ancestors + self.count_with_descendants - 1), + )?; Ok(Cluster::new(cluster)) } @@ -940,6 +946,7 @@ impl TxMempoolEntryWithAncestors { |collected_size| { ensure_cluster_tx_count_limit(&store.mempool_config, 1 + collected_size) }, + None, )?); enforce_cluster_size_limit(store, &entry, &cluster)?; @@ -948,6 +955,7 @@ impl TxMempoolEntryWithAncestors { parents.iter().copied(), RelativesKind::Ancestors, |_| Ok::<_, MempoolPolicyError>(()), + None, )?); let ancestor_entries_iter = ancestors @@ -995,17 +1003,26 @@ pub enum RelativesKind { /// /// After each tx is added to the result, `on_new_tx_added` is called with the current size /// of the result as the argument. +/// +/// `expected_result_size`, if specified, is used to reserve the needed capacity in the result +/// to avoid redundant reallocations. pub fn collect_relatives( store: &MempoolStore, initial_tx_ids: impl IntoIterator>, kind: RelativesKind, mut on_new_tx_added: impl FnMut(/*cur_result_size:*/ usize) -> Result<(), E>, + expected_result_size: Option, ) -> Result>, E> where E: From, { - let mut stack = Vec::new(); - let mut result = StoreHashSet::default(); + let initial_tx_ids = initial_tx_ids.into_iter(); + let initial_tx_ids_min_size_hint = initial_tx_ids.size_hint().0; + let expected_result_size = expected_result_size.unwrap_or(initial_tx_ids_min_size_hint); + + let mut stack = Vec::with_capacity(initial_tx_ids_min_size_hint); + let mut result = + StoreHashSet::with_capacity_and_hasher(expected_result_size, Default::default()); let mut visit = |stack: &mut Vec>, tx_id: &Id| -> Result<(), E> { if result.insert(*tx_id) { diff --git a/mempool/src/pool/tx_pool/tests/basic.rs b/mempool/src/pool/tx_pool/tests/basic.rs index 7e12829493..22c5a7d329 100644 --- a/mempool/src/pool/tx_pool/tests/basic.rs +++ b/mempool/src/pool/tx_pool/tests/basic.rs @@ -381,6 +381,7 @@ async fn tx_bigger_than_cluster_size(#[case] seed: Seed) -> anyhow::Result<()> { ) .add_output_n_times(outputs_count, &output) .build(); + let tx_size = tx.encoded_size(); let mempool_config = MempoolConfig { min_tx_relay_fee_rate: TEST_MIN_TX_RELAY_FEE_RATE.into(), max_cluster_size_bytes: max_cluster_size.into(), @@ -392,7 +393,11 @@ async fn tx_bigger_than_cluster_size(#[case] seed: Seed) -> anyhow::Result<()> { assert_eq!( mempool.add_transaction_test(tx), - Err(MempoolPolicyError::TxSizeExceedsMaxClusterSize.into()) + Err(MempoolPolicyError::TxSizeExceedsMaxClusterSize { + tx_size, + limit: max_cluster_size + } + .into()) ); mempool.store.assert_valid(); Ok(()) @@ -1547,7 +1552,10 @@ async fn accepted_tx_size( let result = mempool.add_transaction_test(transaction); let expected_err = if use_cluster_size_limit { - MempoolPolicyError::TxSizeExceedsMaxClusterSize + MempoolPolicyError::TxSizeExceedsMaxClusterSize { + tx_size, + limit: max_cluster_size, + } } else { MempoolPolicyError::TxSizeExceedsMaxBlockSize }; diff --git a/wallet/src/account/mod.rs b/wallet/src/account/mod.rs index d742230bfa..c733f41b24 100644 --- a/wallet/src/account/mod.rs +++ b/wallet/src/account/mod.rs @@ -226,6 +226,7 @@ impl Account { // determine tx clusters. But this will be fixed in the (hopefully near) future, and all // dependencies will be considered (e.g. the account-based ones and things like order creation // vs order commands). The new algorithm will have to take this into account as well. + // See https://github.com/mintlayer/mintlayer-core/issues/2066. #[allow(clippy::too_many_arguments)] pub fn select_inputs_for_send_request( &mut self, @@ -976,6 +977,7 @@ impl Account { ) } + #[allow(clippy::too_many_arguments)] pub fn create_order_tx( &mut self, db_tx: &mut impl WalletStorageWriteLocked, @@ -1004,6 +1006,7 @@ impl Account { ) } + #[allow(clippy::too_many_arguments)] pub fn create_conclude_order_tx( &mut self, db_tx: &mut impl WalletStorageWriteLocked, @@ -1199,6 +1202,7 @@ impl Account { ) } + #[allow(clippy::too_many_arguments)] pub fn create_spend_utxo_tx( &mut self, db_tx: &mut impl WalletStorageWriteLocked, @@ -1398,6 +1402,7 @@ impl Account { Ok(request) } + #[allow(clippy::too_many_arguments)] pub fn mint_tokens( &mut self, db_tx: &mut impl WalletStorageWriteLocked, @@ -1594,6 +1599,7 @@ impl Account { ) } + #[allow(clippy::too_many_arguments)] fn make_change_token_transaction( &mut self, authority: Destination, diff --git a/wallet/src/account/utxo_selector/output_group.rs b/wallet/src/account/utxo_selector/output_group.rs index f15acf8814..5f96bab793 100644 --- a/wallet/src/account/utxo_selector/output_group.rs +++ b/wallet/src/account/utxo_selector/output_group.rs @@ -23,8 +23,8 @@ use super::UtxoSelectorError; /// A group of UTXOs paid to the same output script. /// This helps reduce privacy leaks resulting from address reuse. // TODO: at this moment, we always create a separate output group for each utxo, i.e. we don't have -// by-destination grouping. -// Note that in Bitcoin, transactions are not grouped unconditionally: +// by-destination grouping. See https://github.com/mintlayer/mintlayer-core/issues/2067. +// Note that in Bitcoin transactions are not grouped unconditionally: // *) They are grouped if either m_avoid_partial_spends or m_avoid_address_reuse is true (both are // false by default). // *) Otherwise, if m_max_aps_fee is non-negative (it's zero by default), then a second attempt diff --git a/wallet/src/wallet/mod.rs b/wallet/src/wallet/mod.rs index 2afd91d504..7059051c27 100644 --- a/wallet/src/wallet/mod.rs +++ b/wallet/src/wallet/mod.rs @@ -329,9 +329,9 @@ pub struct Wallet { chain_config: Arc, // Note: unlike the chain config, the mempool config may potentially change, e.g. if the node // is restarted with different parameters. At this moment, node restart requires the wallet - // to be restarted as well, so it's not an issue for now. But when we implement the automatic + // to be restarted as well, so it's not an issue for now. But when we implement automatic // reconnection, the mempool config stored here should be updated each time we reconnect to - // the node. + // the node. See https://github.com/mintlayer/mintlayer-core/issues/2064. mempool_config: Arc, db: Store, accounts: BTreeMap>, @@ -899,6 +899,7 @@ where Ok(()) } + #[allow(clippy::too_many_arguments)] pub async fn load_wallet< F: Fn(u32) -> WalletResult<()>, F2: AsyncFnOnce(StoreTxRo) -> WalletResult

, diff --git a/wallet/wallet-controller/src/lib.rs b/wallet/wallet-controller/src/lib.rs index be76223bed..cfc0208af0 100644 --- a/wallet/wallet-controller/src/lib.rs +++ b/wallet/wallet-controller/src/lib.rs @@ -519,6 +519,7 @@ where Ok(()) } + #[allow(clippy::too_many_arguments)] pub async fn open_wallet( chain_config: Arc, mempool_config: Arc,