From e48cc77e0a3f3c3660a94eb91f756dc233f83b0e Mon Sep 17 00:00:00 2001 From: yancy Date: Tue, 8 Oct 2024 18:20:37 -0500 Subject: [PATCH 1/4] Refactor SRD test module --- src/single_random_draw.rs | 189 ++++++++++++++++++++++---------------- 1 file changed, 109 insertions(+), 80 deletions(-) diff --git a/src/single_random_draw.rs b/src/single_random_draw.rs index 6e2c297..511fcf1 100644 --- a/src/single_random_draw.rs +++ b/src/single_random_draw.rs @@ -74,7 +74,7 @@ mod tests { use super::*; use crate::single_random_draw::select_coins_srd; - use crate::{WeightedUtxo, CHANGE_LOWER}; + use crate::WeightedUtxo; const FEE_RATE: FeeRate = FeeRate::from_sat_per_kwu(10); const SATISFACTION_WEIGHT: Weight = Weight::from_wu(204); @@ -85,6 +85,13 @@ mod tests { satisfaction_weight: Weight, } + #[derive(Debug)] + pub struct ParamsStr<'a> { + target: &'a str, + fee_rate: &'a str, + weighted_utxos: Vec<&'a str>, + } + impl WeightedUtxo for Utxo { fn satisfaction_weight(&self) -> Weight { self.satisfaction_weight } @@ -123,127 +130,149 @@ mod tests { StepRng::new(0, 0) } - #[test] - fn select_coins_srd_with_solution() { - let target: Amount = Amount::from_str("1.5 cBTC").unwrap(); - let pool = build_pool(); - - let result: Vec<&Utxo> = select_coins_srd(target, FEE_RATE, &pool, &mut get_rng()) - .expect("unexpected error") - .collect(); + fn format_utxo_list(i: &[&Utxo]) -> Vec { + i.iter().map(|u| u.value().to_string()).collect() + } - let expected_result = Amount::from_str("2 cBTC").unwrap(); - assert_eq!(result.len(), 1); - assert_eq!(expected_result, result[0].output.value); + fn format_expected_str_list(e: &[&str]) -> Vec { + e.iter().map(|s| Amount::from_str(s).unwrap().to_string()).collect() } - #[test] - fn select_coins_srd_no_solution() { - let target: Amount = Amount::from_str("4 cBTC").unwrap(); - let pool = build_pool(); + fn assert_coin_select_params(p: &ParamsStr, expected_inputs: Option<&[&str]>) { + let fee_rate = p.fee_rate.parse::().unwrap(); // would be nice if FeeRate had + // from_str like Amount::from_str() + let target = Amount::from_str(p.target).unwrap(); + let fee_rate = FeeRate::from_sat_per_kwu(fee_rate); + + let w_utxos: Vec<_> = p + .weighted_utxos + .iter() + .map(|s| { + let v: Vec<_> = s.split("/").collect(); + match v.len() { + 2 => { + let a = Amount::from_str(v[0]).unwrap(); + let w = Weight::from_wu(v[1].parse().unwrap()); + (a, w) + } + 1 => { + let a = Amount::from_str(v[0]).unwrap(); + (a, Weight::ZERO) + } + _ => panic!(), + } + }) + .map(|(a, w)| build_utxo(a, w)) + .collect(); - let result = select_coins_srd(target, FEE_RATE, &pool, &mut get_rng()); - assert!(result.is_none()) + let result = select_coins_srd(target, fee_rate, &w_utxos, &mut get_rng()); + + if expected_inputs.is_none() { + assert!(result.is_none()); + } else { + let inputs: Vec<_> = result.unwrap().collect(); + let expected_str_list: Vec = expected_inputs + .unwrap() + .iter() + .map(|s| Amount::from_str(s).unwrap().to_string()) + .collect(); + let input_str_list: Vec = format_utxo_list(&inputs); + assert_eq!(input_str_list, expected_str_list); + } } - #[test] - fn select_coins_srd_all_solution() { - let target: Amount = Amount::from_str("2.5 cBTC").unwrap(); + fn assert_coin_select(target_str: &str, expected_inputs: &[&str]) { + let target = Amount::from_str(target_str).unwrap(); let pool = build_pool(); - let result: Vec<&Utxo> = select_coins_srd(target, FeeRate::ZERO, &pool, &mut get_rng()) + let inputs: Vec<&Utxo> = select_coins_srd(target, FEE_RATE, &pool, &mut get_rng()) .expect("unexpected error") .collect(); - let expected_second_element = Amount::from_str("1 cBTC").unwrap(); - let expected_first_element = Amount::from_str("2 cBTC").unwrap(); - - assert_eq!(result.len(), 2); - assert_eq!(result[0].output.value, expected_first_element); - assert_eq!(result[1].output.value, expected_second_element); + let input_str_list: Vec<_> = format_utxo_list(&inputs); + let expected_str_list: Vec<_> = format_expected_str_list(expected_inputs); + assert_eq!(input_str_list, expected_str_list); } #[test] - fn select_coins_skip_negative_effective_value() { - let target: Amount = Amount::from_str("2 cBTC").unwrap() - CHANGE_LOWER; - let mut pool = build_pool(); + fn select_coins_srd_with_solution() { assert_coin_select("1.5 cBTC", &["2 cBTC"]); } - let negative_eff_value = Amount::from_str("1 sat").unwrap(); - let utxo = build_utxo(negative_eff_value, SATISFACTION_WEIGHT); - pool.push(utxo); + #[test] + fn select_coins_srd_all_solution() { assert_coin_select("2.5 cBTC", &["2 cBTC", "1 cBTC"]); } - let result: Vec<_> = select_coins_srd(target, FEE_RATE, &pool, &mut get_rng()) - .expect("unexpected error") - .collect(); + #[test] + fn select_coins_srd_no_solution() { + let params = + ParamsStr { target: "4 cBTC", fee_rate: "0", weighted_utxos: vec!["1 cBTC", "2 cBTC"] }; - let expected_second_element = Amount::from_str("1 cBTC").unwrap(); - let expected_first_element = Amount::from_str("2 cBTC").unwrap(); + assert_coin_select_params(¶ms, None); + } + + #[test] + fn select_coins_skip_negative_effective_value() { + let params = ParamsStr { + target: "1.95 cBTC", // 2 cBTC - CHANGE_LOWER + fee_rate: "10", + weighted_utxos: vec!["1 cBTC", "2 cBTC", "1 sat/204"], // 1 sat @ 204 wu has negative effective_value + }; - assert_eq!(result.len(), 2); - assert_eq!(result[0].output.value, expected_first_element); - assert_eq!(result[1].output.value, expected_second_element); + assert_coin_select_params(¶ms, Some(&["2 cBTC", "1 cBTC"])); } #[test] fn select_coins_srd_fee_rate_error() { - let target: Amount = Amount::from_str("2 cBTC").unwrap(); - let pool = build_pool(); + println!("test"); + + let params = ParamsStr { + target: "1 cBTC", + fee_rate: "18446744073709551615", + weighted_utxos: vec!["1 cBTC", "2 cBTC"], + }; - let result = select_coins_srd(target, FeeRate::MAX, &pool, &mut get_rng()); - assert!(result.is_none()); + assert_coin_select_params(¶ms, None); } #[test] fn select_coins_srd_change_output_too_small() { - let target: Amount = Amount::from_str("3 cBTC").unwrap(); - let pool = build_pool(); + let params = ParamsStr { + target: "3 cBTC", + fee_rate: "10", + weighted_utxos: vec!["1 cBTC", "2 cBTC"], + }; - let result = select_coins_srd(target, FEE_RATE, &pool, &mut get_rng()); - assert!(result.is_none()); + assert_coin_select_params(¶ms, None); } #[test] fn select_coins_srd_with_high_fee() { - // the first UTXO is 2 cBTC. If the fee is greater than 10 sats, - // then more than the single 2 cBTC output will need to be selected - // if the target is 1.99999 cBTC. That is, 2 cBTC - 1.9999 cBTC = 10 sats. - let target: Amount = Amount::from_str("1.99999 cBTC").unwrap(); - - // fee = 15 sats, since - // 40 sat/kwu * (204 + BASE_WEIGHT) = 15 sats - let fee_rate: FeeRate = FeeRate::from_sat_per_kwu(40); - let pool = build_pool(); - - let result: Vec<_> = select_coins_srd(target, fee_rate, &pool, &mut get_rng()) - .expect("unexpected error") - .collect(); - let expected_second_element = Amount::from_str("1 cBTC").unwrap(); - let expected_first_element = Amount::from_str("2 cBTC").unwrap(); + let params = ParamsStr { + target: "1.99999 cBTC", + fee_rate: "10", + weighted_utxos: vec!["1 cBTC", "2 cBTC"], + }; - assert_eq!(result.len(), 2); - assert_eq!(result[0].output.value, expected_first_element); - assert_eq!(result[1].output.value, expected_second_element); + assert_coin_select_params(¶ms, Some(&["2 cBTC", "1 cBTC"])); } #[test] fn select_coins_srd_addition_overflow() { - let target: Amount = Amount::from_str("2 cBTC").unwrap(); - let amt = Amount::from_str("1 cBTC").unwrap(); - let utxo = build_utxo(amt, Weight::MAX); - let pool = vec![utxo]; + let params = ParamsStr { + target: "2 cBTC", + fee_rate: "10", + weighted_utxos: vec!["1 cBTC/18446744073709551615"], // weight= u64::MAX + }; - let result = select_coins_srd(target, FEE_RATE, &pool, &mut get_rng()); - assert!(result.is_none()); + assert_coin_select_params(¶ms, None); } #[test] fn select_coins_srd_threshold_overflow() { - let target: Amount = Amount::MAX; - let amt = Amount::from_str("1 cBTC").unwrap(); - let utxo = build_utxo(amt, Weight::MAX); - let pool = vec![utxo]; + let params = ParamsStr { + target: "18446744073709551615 sat", // u64::MAX + fee_rate: "10", + weighted_utxos: vec!["1 cBTC/18446744073709551615"], + }; - let result = select_coins_srd(target, FEE_RATE, &pool, &mut get_rng()); - assert!(result.is_none()); + assert_coin_select_params(¶ms, None); } } From f7dd2d0a9654a89acb0af545137085eb00fd2c66 Mon Sep 17 00:00:00 2001 From: yancy Date: Tue, 8 Oct 2024 18:25:04 -0500 Subject: [PATCH 2/4] Fix how SRD handles the effective_value return If the UTXO value exceeds i64 max, the effective_value returns None causing the SRD algorithem to exit early. Instead, if effective_value returns None, discard that UTXO and continue. --- src/single_random_draw.rs | 31 +++++++++++++++++++++++-------- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/src/single_random_draw.rs b/src/single_random_draw.rs index 511fcf1..1d10e15 100644 --- a/src/single_random_draw.rs +++ b/src/single_random_draw.rs @@ -48,17 +48,18 @@ pub fn select_coins_srd<'a, R: rand::Rng + ?Sized, Utxo: WeightedUtxo>( for w_utxo in origin { let utxo_value = w_utxo.value(); - let effective_value = effective_value(fee_rate, w_utxo.satisfaction_weight(), utxo_value)?; + let effective_value = effective_value(fee_rate, w_utxo.satisfaction_weight(), utxo_value); - value += match effective_value.to_unsigned() { - Ok(amt) => amt, - Err(_) => continue, - }; + if let Some(e) = effective_value { + if let Ok(v) = e.to_unsigned() { + value += v; - result.push(w_utxo); + result.push(w_utxo); - if value >= threshold { - return Some(result.into_iter()); + if value >= threshold { + return Some(result.into_iter()); + } + } } } @@ -275,4 +276,18 @@ mod tests { assert_coin_select_params(¶ms, None); } + + #[test] + fn select_coins_srd_none_effective_value() { + let params = ParamsStr { + target: ".95 cBTC", + fee_rate: "0", + weighted_utxos: vec![ + "1 cBTC", + "9223372036854775808 sat", //i64::MAX + 1 + ], + }; + + assert_coin_select_params(¶ms, Some(&["1 cBTC"])); + } } From 49655c0966dffcd8d01cfcfb436acdce10d579d7 Mon Sep 17 00:00:00 2001 From: yancy Date: Tue, 8 Oct 2024 18:33:57 -0500 Subject: [PATCH 3/4] Version bump --- CHANGELOG.md | 7 +++++++ Cargo.toml | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0f56b3b..53a4d2f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,3 +39,10 @@ - Add WeightedUtxo trait replacing WeightedUtxo struct. - Add check for overflow to SRD. - Correction to README parameter definitions. + +# 0.6.0 - 2024-10-08 + +- Add Libfuzzer and fuzz targets +- Refactor SRD, BnB test modules +- Minor refactor to SRD selection +- Fix early return bug in SRD if a UTXO value exceeds i64::MAX. diff --git a/Cargo.toml b/Cargo.toml index 41154e8..8210390 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -7,7 +7,7 @@ homepage = "https://github.com/rust-bitcoin/rust-bitcoin-coin-selection/" license = "CC0-1.0" name = "bitcoin-coin-selection" repository = "https://github.com/rust-bitcoin/rust-bitcoin-coin-selection/" -version = "0.5.0" +version = "0.6.0" # documentation = "https://docs.rs/bitcoin-coin-selection/" description = "Libary providing utility functions to efficiently select a set of UTXOs." keywords = ["bitcoin", "coin-selection", "coin", "coinselection", "utxo"] From f46622a2801de490ed3077687025b359c0ee6a0e Mon Sep 17 00:00:00 2001 From: yancy Date: Tue, 8 Oct 2024 18:38:03 -0500 Subject: [PATCH 4/4] Refactor SRD selection --- src/single_random_draw.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/single_random_draw.rs b/src/single_random_draw.rs index 1d10e15..43a5d75 100644 --- a/src/single_random_draw.rs +++ b/src/single_random_draw.rs @@ -48,7 +48,8 @@ pub fn select_coins_srd<'a, R: rand::Rng + ?Sized, Utxo: WeightedUtxo>( for w_utxo in origin { let utxo_value = w_utxo.value(); - let effective_value = effective_value(fee_rate, w_utxo.satisfaction_weight(), utxo_value); + let utxo_weight = w_utxo.satisfaction_weight(); + let effective_value = effective_value(fee_rate, utxo_weight, utxo_value); if let Some(e) = effective_value { if let Ok(v) = e.to_unsigned() {