Fix spurious failure in mempool tests

mempool/src/pool/tests/basic.rs

@@ -213,7 +213,8 @@ async fn reject_txs_during_ibd(#[case] seed: Seed) {
     let mock_time = tf.time_value.as_ref().unwrap().shallow_clone();
     let mock_clock = tf.time_getter.clone();
 
-    let mut mempool = setup_with_chainstate_and_clock(tf.chainstate(), mock_clock);
+    let mut mempool =
+        setup_with_chainstate_generic(tf.chainstate(), create_mempool_config(), mock_clock);
     let chainstate = mempool.chainstate_handle().shallow_clone();
 
     assert!(mempool.is_initial_block_download());
@@ -322,7 +323,8 @@ async fn ibd_transition(#[case] seed: Seed) {
     let mock_time = tf.time_value.as_ref().unwrap().shallow_clone();
     let mock_clock = tf.time_getter.clone();
 
-    let mut mempool = setup_with_chainstate_and_clock(tf.chainstate(), mock_clock);
+    let mut mempool =
+        setup_with_chainstate_generic(tf.chainstate(), create_mempool_config(), mock_clock);
     let chainstate = mempool.chainstate_handle().shallow_clone();
 
     assert!(mempool.is_initial_block_download());

mempool/src/pool/tests/orphans.rs

@@ -236,10 +236,22 @@ async fn transaction_graph_subset_permutation(#[case] seed: Seed) {
         subseq
     };
 
+    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_txs.into(),
+
+        max_cluster_size_bytes: Default::default(),
+    };
+
     let mut results: Vec<Vec<Option<TxStatus>>> = Vec::new();
     for tx_subseq in [tx_subseq_0, tx_subseq_1] {
         let tf = TestFramework::builder(&mut rng).build();
-        let mut mempool = setup_with_chainstate(tf.chainstate());
+        let mut mempool = setup_with_chainstate_generic(
+            tf.chainstate(),
+            mempool_config.clone(),
+            Default::default(),
+        );
 
         // Now add each transaction in the subsequence
         tx_subseq.iter().for_each(|tx| {

mempool/src/pool/tests/utils.rs

@@ -26,23 +26,25 @@ use common::{
 use crypto::vrf::{VRFKeyKind, VRFPrivateKey};
 use mempool_types::{TxOptions, TxStatus, tx_origin::TxOrigin};
 
-pub use crate::pool::tx_pool::tests::utils::*;
-pub use rstest::rstest;
+use crate::MempoolConfig;
 
 use super::{Error, MemoryUsageEstimator, Mempool, TxEntry};
 
+pub use crate::pool::tx_pool::tests::utils::*;
+pub use rstest::rstest;
+
 pub fn setup_with_chainstate(
     chainstate: Box<dyn ChainstateInterface>,
 ) -> Mempool<StoreMemoryUsageEstimator> {
-    setup_with_chainstate_and_clock(chainstate, Default::default())
+    setup_with_chainstate_generic(chainstate, create_mempool_config(), Default::default())
 }
 
-pub fn setup_with_chainstate_and_clock(
+pub fn setup_with_chainstate_generic(
     chainstate: Box<dyn ChainstateInterface>,
+    mempool_config: MempoolConfig,
     clock: TimeGetter,
 ) -> Mempool<StoreMemoryUsageEstimator> {
     let chain_config = std::sync::Arc::clone(chainstate.get_chain_config());
-    let mempool_config = create_mempool_config();
     let chainstate_handle = start_chainstate(chainstate);
     Mempool::new(
         chain_config,

mempool/src/pool/tx_pool/tests/utils.rs

@@ -232,9 +232,12 @@ pub fn make_tx(
 
 /// Generate a valid transaction graph.
 ///
-/// This produces an infinite iterator but taking too many items may not be valid:
-/// * The transaction fees may drop below minimum threshold.
-/// * In extreme, 0-value outputs may be generated.
+/// Note: this produces an infinite iterator but taking too many items will fail, because at some
+/// point the remaining total amount will become less than the minimum tx fee. If you need a large
+/// number of txs, use a negligible fee rate and/or a large initial utxo amount (each tx pays the
+/// minimum fee plus up to 5% of its selected input amount as extra fee, so in the worst case after
+/// N txs the remaining amount will be no more than 0.95^N of the initial amount minus the accumulated
+/// minimum fees).
 pub fn generate_transaction_graph_generic(
     rng: &mut impl CryptoRng,
     time: Time,
@@ -248,34 +251,60 @@ pub fn generate_transaction_graph_generic(
     )];
 
     std::iter::from_fn(move || {
-        let n_inputs = rng.random_range(1..=std::cmp::min(3, utxos.len()));
-        let n_outputs = rng.random_range(1..=3);
-
-        let estimated_tx_size = estimate_tx_size(n_inputs, n_outputs);
-        let estimated_fee = min_tx_relay_fee_rate.compute_fee(estimated_tx_size).unwrap();
-
         let mut builder = TransactionBuilder::new();
         let mut total = 0u128;
         let mut amts = Vec::new();
 
-        // the number is chosen to avoid generating empty range below
-        let min_valid_total_amount = 2;
+        let min_outputs_amt = 2;
+
+        let mut inputs_count = rng.random_range(1..=std::cmp::min(3, utxos.len()));
+        let outputs_count = rng.random_range(1..=3);
+
+        let mut estimated_fee = min_tx_relay_fee_rate
+            .compute_fee(estimate_tx_size(inputs_count, outputs_count))
+            .unwrap();
 
-        let mut input_count = 0;
-        while input_count < n_inputs || total < estimated_fee.into_atoms() + min_valid_total_amount
         {
-            let (outpt, amt) = utxos.swap_remove(rng.random_range(0..utxos.len()));
-            total += amt;
-            builder = builder.add_input(outpt, empty_witness(rng));
-            input_count += 1;
+            let mut i = 0;
+            loop {
+                let (outpt, amt) = utxos.swap_remove(rng.random_range(0..utxos.len()));
+                total += amt;
+                builder = builder.add_input(outpt, empty_witness(rng));
+
+                if total >= estimated_fee.into_atoms() + min_outputs_amt {
+                    break;
+                }
+
+                if i == inputs_count - 1 {
+                    inputs_count += 1;
+
+                    estimated_fee = min_tx_relay_fee_rate
+                        .compute_fee(estimate_tx_size(inputs_count, outputs_count))
+                        .unwrap();
+                }
+
+                i += 1;
+            }
         }
 
-        for _ in 0..n_outputs {
-            if total < min_valid_total_amount {
+        let estimated_fee = estimated_fee;
+        let mut remaining_outputs_total = total - estimated_fee.into_atoms();
+
+        for i in 0..outputs_count {
+            if remaining_outputs_total == 0 {
                 break;
             }
-            let amt = rng.random_range((total / 2)..(95 * total / 100));
-            total -= amt;
+
+            let (min_amt_percentage, max_amt_percentage) = if i < outputs_count - 1 {
+                (25, 95)
+            } else {
+                (95, 100)
+            };
+            let min_amt = min_amt_percentage * remaining_outputs_total / 100;
+            let max_amt = max_amt_percentage * remaining_outputs_total / 100;
+            let amt = std::cmp::max(rng.random_range(min_amt..=max_amt), 1);
+
+            remaining_outputs_total -= amt;
             builder = builder.add_output(TxOutput::Transfer(
                 OutputValue::Coin(Amount::from_atoms(amt)),
                 Destination::AnyoneCanSpend,
@@ -297,7 +326,7 @@ pub fn generate_transaction_graph_generic(
         let entry = TxEntry::new(tx, time, origin, options);
         Some(TxEntryWithFee::new(
             entry,
-            Fee::new(Amount::from_atoms(total)),
+            (estimated_fee + Fee::new(Amount::from_atoms(remaining_outputs_total))).unwrap(),
         ))
     })
 }