Return vector instead of iterator

An earlier implementation called collect() before returning the results.
Instead of calling collect(), the API was changed to just return the
iterator since it's possible dependant applications may prefer to use
the iterator.  However, after a refactor, the call to collect() was
removed, and now it no longer makes sense to return an iterator.
Furthermore, return an opaque type makes for a more complicated return
type, making it hard to alias for example.  Returning a concrete type
therefore simplifies the API.

Author: yancyribbens

benches/coin_selection.rs

@@ -34,7 +34,7 @@ pub fn criterion_benchmark(c: &mut Criterion) {
 
     c.bench_function("bnb 1000", |b| {
         b.iter(|| {
-            let (iteration_count, inputs_iter) = select_coins_bnb(
+            let (iteration_count, inputs) = select_coins_bnb(
                 black_box(target),
                 black_box(cost_of_change),
                 black_box(FeeRate::ZERO),
@@ -43,7 +43,6 @@ pub fn criterion_benchmark(c: &mut Criterion) {
             )
             .unwrap();
             assert_eq!(iteration_count, 100000);
-            let inputs: Vec<_> = inputs_iter.collect();
 
             assert_eq!(2, inputs.len());
             assert_eq!(Amount::from_sat(1_000), inputs[0].value());

src/branch_and_bound.rs

@@ -4,8 +4,6 @@
 //!
 //! This module introduces the Branch and Bound Coin-Selection Algorithm.
 
-use std::vec::IntoIter;
-
 use bitcoin::amount::CheckedSum;
 use bitcoin::{Amount, FeeRate, SignedAmount};
 
@@ -158,7 +156,7 @@ pub fn select_coins_bnb<Utxo: WeightedUtxo>(
     fee_rate: FeeRate,
     long_term_fee_rate: FeeRate,
     weighted_utxos: &[Utxo],
-) -> Option<(u32, IntoIter<&Utxo>)> {
+) -> Option<(u32, Vec<&Utxo>)> {
     // Total_Tries in Core:
     // https://github.com/bitcoin/bitcoin/blob/1d9da8da309d1dbf9aef15eb8dc43b4a2dc3d309/src/wallet/coinselection.cpp#L74
     const ITERATION_LIMIT: u32 = 100_000;
@@ -310,7 +308,7 @@ fn index_to_utxo_list<Utxo: WeightedUtxo>(
     iterations: u32,
     index_list: Vec<usize>,
     wu: Vec<(Amount, SignedAmount, &Utxo)>,
-) -> Option<(u32, std::vec::IntoIter<&Utxo>)> {
+) -> Option<(u32, Vec<&Utxo>)> {
     let mut result: Vec<_> = Vec::new();
     let list = index_list;
 
@@ -322,7 +320,7 @@ fn index_to_utxo_list<Utxo: WeightedUtxo>(
     if result.is_empty() {
         None
     } else {
-        Some((iterations, result.into_iter()))
+        Some((iterations, result))
     }
 }
 
@@ -337,7 +335,7 @@ mod tests {
     use bitcoin::{Amount, Weight};
 
     use super::*;
-    use crate::tests::{assert_proptest_bnb, Utxo, UtxoPool};
+    use crate::tests::{assert_proptest_bnb, assert_ref_eq, Utxo, UtxoPool};
     use crate::WeightedUtxo;
 
     const TX_IN_BASE_WEIGHT: u64 = 160;
@@ -363,15 +361,13 @@ mod tests {
     ) {
         let target = Amount::from_str(target_str).unwrap();
         let pool = build_pool();
-        let (iterations, inputs_iter) =
+        let (iterations, inputs) =
             select_coins_bnb(target, Amount::ZERO, FeeRate::ZERO, FeeRate::ZERO, &pool.utxos)
                 .unwrap();
-
         assert_eq!(iterations, expected_iterations);
 
-        let inputs: Vec<_> = inputs_iter.cloned().collect();
-        let expected_inputs: UtxoPool = UtxoPool::from_str_list(expected_inputs_str);
-        assert_eq!(expected_inputs.utxos, inputs);
+        let expected: UtxoPool = UtxoPool::from_str_list(expected_inputs_str);
+        assert_ref_eq(inputs, expected.utxos);
     }
 
     fn assert_coin_select_params(
@@ -395,12 +391,11 @@ mod tests {
         let pool: UtxoPool = UtxoPool::from_str_list(&p.weighted_utxos);
         let result = select_coins_bnb(target, cost_of_change, fee_rate, lt_fee_rate, &pool.utxos);
 
-        if let Some((iterations, inputs_iter)) = result {
+        if let Some((iterations, inputs)) = result {
             assert_eq!(iterations, expected_iterations);
 
-            let inputs: Vec<Utxo> = inputs_iter.cloned().collect();
-            let expected_inputs: UtxoPool = UtxoPool::from_str_list(expected_inputs_str.unwrap());
-            assert_eq!(expected_inputs.utxos, inputs);
+            let expected: UtxoPool = UtxoPool::from_str_list(expected_inputs_str.unwrap());
+            assert_ref_eq(inputs, expected.utxos);
         }
     }
 
@@ -746,7 +741,7 @@ mod tests {
         amts.push(Amount::from_sat(target));
         let pool: Vec<_> = amts.into_iter().map(|a| Utxo::new(a, Weight::ZERO)).collect();
 
-        let (iterations, mut utxos) = select_coins_bnb(
+        let (iterations, utxos) = select_coins_bnb(
             Amount::from_sat(target),
             Amount::ONE_SAT,
             FeeRate::ZERO,
@@ -756,7 +751,7 @@ mod tests {
         .unwrap();
 
         assert_eq!(utxos.len(), 1);
-        assert_eq!(utxos.next().unwrap().value(), Amount::from_sat(target));
+        assert_eq!(utxos[0].value(), Amount::from_sat(target));
         assert_eq!(100000, iterations);
     }
 
@@ -774,8 +769,7 @@ mod tests {
                 select_coins_bnb(utxo.value(), Amount::ZERO, FeeRate::ZERO, FeeRate::ZERO, &pool)
                     .unwrap();
 
-            let coins: Vec<Utxo> = utxos.cloned().collect();
-
+            let coins: Vec<Utxo> = utxos.into_iter().cloned().collect();
             assert_eq!(coins, pool);
 
             Ok(())
@@ -803,6 +797,7 @@ mod tests {
 
                     if let Some((_i, utxos)) = result {
                         let sum: SignedAmount = utxos
+                            .into_iter()
                             .map(|u| {
                                 effective_value(fee_rate, u.satisfaction_weight(), u.value())
                                     .unwrap()
@@ -812,7 +807,7 @@ mod tests {
                         assert_eq!(amount_sum, target);
                     } else {
                         // if result was none, then assert that fail happened because overflow when
-                        // ssumming pool.  In the future, assert specific error when added.
+                        // summing pool.  In the future, assert specific error when added.
                         let available_value = utxos.into_iter().map(|u| u.value()).checked_sum();
                         assert!(available_value.is_none());
                     }
@@ -872,6 +867,7 @@ mod tests {
 
                     if let Some((_i, utxos)) = result {
                         let effective_value_sum: Amount = utxos
+                            .into_iter()
                             .map(|u| {
                                 effective_value(fee_rate, u.satisfaction_weight(), u.value())
                                     .unwrap()

src/lib.rs

@@ -93,7 +93,7 @@ pub fn select_coins<Utxo: WeightedUtxo>(
     fee_rate: FeeRate,
     long_term_fee_rate: FeeRate,
     weighted_utxos: &[Utxo],
-) -> Option<(u32, impl Iterator<Item = &Utxo>)> {
+) -> Option<(u32, Vec<&Utxo>)> {
     let bnb =
         select_coins_bnb(target, cost_of_change, fee_rate, long_term_fee_rate, weighted_utxos);
 
@@ -133,6 +133,11 @@ mod tests {
         utxos
     }
 
+    pub fn assert_ref_eq(inputs: Vec<&Utxo>, expected: Vec<Utxo>) {
+        let expected_ref: Vec<&Utxo> = expected.iter().collect();
+        assert_eq!(inputs, expected_ref);
+    }
+
     #[derive(Debug, Clone, PartialEq, Ord, Eq, PartialOrd, Arbitrary)]
     pub struct Utxo {
         pub output: TxOut,
@@ -230,8 +235,8 @@ mod tests {
         let result = select_coins(target, cost_of_change, fee_rate, lt_fee_rate, &pool);
 
         // This yields no solution because:
-        //  * BnB fails because the sum overage is greater than cost_of_change
-        //  * SRD fails because the sum is greater the utxo sum + CHANGE_LOWER
+        // * BnB fails because the sum overage is greater than cost_of_change
+        // * SRD fails because the sum is greater the utxo sum + CHANGE_LOWER
         assert!(result.is_none());
     }
 
@@ -245,7 +250,7 @@ mod tests {
 
         let result = select_coins(target, cost_of_change, fee_rate, lt_fee_rate, &pool);
         let (_iterations, utxos) = result.unwrap();
-        let sum: Amount = utxos.map(|u| u.value()).sum();
+        let sum: Amount = utxos.into_iter().map(|u| u.value()).sum();
         assert!(sum > target);
     }
 
@@ -264,7 +269,7 @@ mod tests {
 
         let result = select_coins(target, cost_of_change, fee_rate, lt_fee_rate, &pool);
         let (iterations, utxos) = result.unwrap();
-        let sum: Amount = utxos.map(|u| u.value()).sum();
+        let sum: Amount = utxos.into_iter().map(|u| u.value()).sum();
         assert!(sum > target);
         assert!(sum <= target + cost_of_change);
         assert_eq!(16, iterations);
@@ -325,18 +330,19 @@ mod tests {
         }
     }
 
-    pub fn assert_proptest_bnb<'a, T: Iterator<Item = &'a Utxo>>(
+    pub fn assert_proptest_bnb(
         target: Amount,
         cost_of_change: Amount,
         fee_rate: FeeRate,
         pool: UtxoPool,
-        result: Option<(u32, T)>,
+        result: Option<(u32, Vec<&Utxo>)>,
     ) {
         let mut bnb_solutions: Vec<Vec<&Utxo>> = Vec::new();
         build_possible_solutions_bnb(&pool, fee_rate, target, cost_of_change, &mut bnb_solutions);
 
         if let Some((_i, utxos)) = result {
             let utxo_sum: Amount = utxos
+                .into_iter()
                 .map(|u| {
                     effective_value(fee_rate, u.satisfaction_weight(), u.value())
                         .unwrap()
@@ -354,17 +360,18 @@ mod tests {
         }
     }
 
-    pub fn assert_proptest_srd<'a, T: Iterator<Item = &'a Utxo>>(
+    pub fn assert_proptest_srd(
         target: Amount,
         fee_rate: FeeRate,
         pool: UtxoPool,
-        result: Option<(u32, T)>,
+        result: Option<(u32, Vec<&Utxo>)>,
     ) {
         let mut srd_solutions: Vec<Vec<&Utxo>> = Vec::new();
         build_possible_solutions_srd(&pool, fee_rate, target, &mut srd_solutions);
 
         if let Some((_i, utxos)) = result {
             let utxo_sum: Amount = utxos
+                .into_iter()
                 .map(|u| {
                     effective_value(fee_rate, u.satisfaction_weight(), u.value())
                         .unwrap()
@@ -381,12 +388,12 @@ mod tests {
         }
     }
 
-    pub fn assert_proptest<'a, T: Iterator<Item = &'a Utxo>>(
+    pub fn assert_proptest(
         target: Amount,
         cost_of_change: Amount,
         fee_rate: FeeRate,
         pool: UtxoPool,
-        result: Option<(u32, T)>,
+        result: Option<(u32, Vec<&Utxo>)>,
     ) {
         let mut bnb_solutions: Vec<Vec<&Utxo>> = Vec::new();
         build_possible_solutions_bnb(&pool, fee_rate, target, cost_of_change, &mut bnb_solutions);
@@ -396,6 +403,7 @@ mod tests {
 
         if let Some((_i, utxos)) = result {
             let utxo_sum: Amount = utxos
+                .into_iter()
                 .map(|u| {
                     effective_value(fee_rate, u.satisfaction_weight(), u.value())
                         .unwrap()

src/single_random_draw.rs

@@ -45,7 +45,7 @@ pub fn select_coins_srd<'a, R: rand::Rng + ?Sized, Utxo: WeightedUtxo>(
     fee_rate: FeeRate,
     weighted_utxos: &'a [Utxo],
     rng: &mut R,
-) -> Option<(u32, std::vec::IntoIter<&'a Utxo>)> {
+) -> Option<(u32, Vec<&'a Utxo>)> {
     if target > Amount::MAX_MONEY {
         return None;
     }
@@ -73,7 +73,7 @@ pub fn select_coins_srd<'a, R: rand::Rng + ?Sized, Utxo: WeightedUtxo>(
                 result.push(w_utxo);
 
                 if value >= threshold {
-                    return Some((iteration, result.into_iter()));
+                    return Some((iteration, result));
                 }
             }
         }
@@ -93,7 +93,7 @@ mod tests {
 
     use super::*;
     use crate::single_random_draw::select_coins_srd;
-    use crate::tests::{assert_proptest_srd, Utxo, UtxoPool};
+    use crate::tests::{assert_proptest_srd, assert_ref_eq, UtxoPool};
 
     const FEE_RATE: FeeRate = FeeRate::from_sat_per_kwu(10);
 
@@ -140,12 +140,11 @@ mod tests {
         let pool: UtxoPool = UtxoPool::from_str_list(&p.weighted_utxos);
         let result = select_coins_srd(target, fee_rate, &pool.utxos, &mut get_rng());
 
-        if let Some((iterations, inputs_iter)) = result {
+        if let Some((iterations, inputs)) = result {
             assert_eq!(iterations, expected_iterations);
 
-            let inputs: Vec<Utxo> = inputs_iter.cloned().collect();
-            let expected_pool: UtxoPool = UtxoPool::from_str_list(expected_inputs_str.unwrap());
-            assert_eq!(inputs, expected_pool.utxos);
+            let expected: UtxoPool = UtxoPool::from_str_list(expected_inputs_str.unwrap());
+            assert_ref_eq(inputs, expected.utxos);
         }
     }
 
@@ -157,13 +156,12 @@ mod tests {
         let target = Amount::from_str(target_str).unwrap();
         let pool = build_pool();
 
-        let (iterations, inputs_iter) =
+        let (iterations, inputs) =
             select_coins_srd(target, FEE_RATE, &pool.utxos, &mut get_rng()).unwrap();
         assert_eq!(iterations, expected_iterations);
 
-        let inputs: Vec<_> = inputs_iter.cloned().collect();
-        let expected_pool: UtxoPool = UtxoPool::from_str_list(expected_inputs_str);
-        assert_eq!(inputs, expected_pool.utxos);
+        let expected: UtxoPool = UtxoPool::from_str_list(expected_inputs_str);
+        assert_ref_eq(inputs, expected.utxos);
     }
 
     #[test]