From c0bb1c676df9e3c9ca0c907168693ee4bfeee115 Mon Sep 17 00:00:00 2001 From: David Caseria Date: Thu, 3 Oct 2024 10:00:14 -0400 Subject: [PATCH 01/18] Provide sorting method for selecting proofs to swap --- crates/cdk/src/nuts/nut01/public_key.rs | 10 +++ crates/cdk/src/nuts/nut02.rs | 8 ++ crates/cdk/src/wallet/melt.rs | 8 +- crates/cdk/src/wallet/proofs.rs | 97 ++++++++++++++++++++++++- crates/cdk/src/wallet/send.rs | 11 ++- crates/cdk/src/wallet/swap.rs | 6 +- 6 files changed, 133 insertions(+), 7 deletions(-) diff --git a/crates/cdk/src/nuts/nut01/public_key.rs b/crates/cdk/src/nuts/nut01/public_key.rs index ddb58b81f..d061be344 100644 --- a/crates/cdk/src/nuts/nut01/public_key.rs +++ b/crates/cdk/src/nuts/nut01/public_key.rs @@ -92,6 +92,16 @@ impl PublicKey { SECP256K1.verify_schnorr(sig, &msg, &self.inner.x_only_public_key().0)?; Ok(()) } + + #[cfg(test)] + pub fn random() -> Self { + Self { + inner: secp256k1::PublicKey::from_secret_key( + &SECP256K1, + &secp256k1::SecretKey::new(&mut rand::thread_rng()), + ), + } + } } impl FromStr for PublicKey { diff --git a/crates/cdk/src/nuts/nut02.rs b/crates/cdk/src/nuts/nut02.rs index fbe12ba77..14489bda2 100644 --- a/crates/cdk/src/nuts/nut02.rs +++ b/crates/cdk/src/nuts/nut02.rs @@ -105,6 +105,14 @@ impl Id { id: bytes[1..].try_into()?, }) } + + #[cfg(test)] + pub fn random() -> Self { + Self { + version: KeySetVersion::Version00, + id: rand::random(), + } + } } impl TryFrom for u64 { diff --git a/crates/cdk/src/wallet/melt.rs b/crates/cdk/src/wallet/melt.rs index c7217c287..1abe5afd6 100644 --- a/crates/cdk/src/wallet/melt.rs +++ b/crates/cdk/src/wallet/melt.rs @@ -11,7 +11,7 @@ use crate::{ Amount, Error, Wallet, }; -use super::MeltQuote; +use super::{proofs::SelectProofsMethod, MeltQuote}; impl Wallet { /// Melt Quote @@ -294,7 +294,11 @@ impl Wallet { let available_proofs = self.get_proofs().await?; let input_proofs = self - .select_proofs_to_swap(inputs_needed_amount, available_proofs) + .select_proofs_to_swap( + inputs_needed_amount, + available_proofs, + SelectProofsMethod::ClosestFirst, + ) .await?; self.melt_proofs(quote_id, input_proofs).await diff --git a/crates/cdk/src/wallet/proofs.rs b/crates/cdk/src/wallet/proofs.rs index 3eff5cf8c..cd25c0c9e 100644 --- a/crates/cdk/src/wallet/proofs.rs +++ b/crates/cdk/src/wallet/proofs.rs @@ -235,11 +235,16 @@ impl Wallet { } /// Select proofs to send + /// + /// This method will first select inactive proofs and then active proofs. + /// Inactive proofs are always sorted largest first. + /// The active proofs are sorted by the [`SelectProofsMethod`] provided. #[instrument(skip_all)] pub async fn select_proofs_to_swap( &self, amount: Amount, proofs: Proofs, + method: SelectProofsMethod, ) -> Result { let active_keyset_id = self.get_active_mint_keyset().await?.id; @@ -260,7 +265,7 @@ impl Wallet { } } - active_proofs.sort_by(|a: &Proof, b: &Proof| b.cmp(a)); + sort_proofs(&mut active_proofs, method, amount); for active_proof in active_proofs { selected_proofs.push(active_proof); @@ -275,3 +280,93 @@ impl Wallet { Err(Error::InsufficientFunds) } } + +fn sort_proofs(proofs: &mut Proofs, method: SelectProofsMethod, amount: Amount) { + match method { + SelectProofsMethod::LargestFirst => proofs.sort_by(|a: &Proof, b: &Proof| b.cmp(a)), + SelectProofsMethod::ClosestFirst => proofs.sort_by(|a: &Proof, b: &Proof| { + let a_diff = if a.amount > amount { + a.amount - amount + } else { + amount - a.amount + }; + let b_diff = if b.amount > amount { + b.amount - amount + } else { + amount - b.amount + }; + a_diff.cmp(&b_diff) + }), + SelectProofsMethod::SmallestFirst => proofs.sort(), + } +} + +/// Select proofs method +#[derive(Debug, Default, Clone, Copy, Hash, PartialEq, Eq)] +pub enum SelectProofsMethod { + /// Select proofs with the largest amount first + #[default] + LargestFirst, + /// Select proofs closest to the amount first + ClosestFirst, + /// Select proofs with the smallest amount first + SmallestFirst, +} + +#[cfg(test)] +mod tests { + use crate::{ + nuts::{Id, Proof, PublicKey}, + secret::Secret, + Amount, + }; + + use super::{sort_proofs, SelectProofsMethod}; + + #[test] + fn test_sort_proofs_by_method() { + let amount = Amount::from(256); + let keyset_id = Id::random(); + let mut proofs = vec![ + Proof { + amount: 1.into(), + keyset_id, + secret: Secret::generate(), + c: PublicKey::random(), + witness: None, + dleq: None, + }, + Proof { + amount: 256.into(), + keyset_id, + secret: Secret::generate(), + c: PublicKey::random(), + witness: None, + dleq: None, + }, + Proof { + amount: 1024.into(), + keyset_id, + secret: Secret::generate(), + c: PublicKey::random(), + witness: None, + dleq: None, + }, + ]; + + fn assert_proof_order(proofs: &Vec, order: Vec) { + for (p, a) in proofs.iter().zip(order.iter()) { + assert_eq!(p.amount, Amount::from(*a)); + } + } + + sort_proofs(&mut proofs, SelectProofsMethod::LargestFirst, amount); + assert_proof_order(&proofs, vec![1024, 256, 1]); + + sort_proofs(&mut proofs, SelectProofsMethod::ClosestFirst, amount); + assert_proof_order(&proofs, vec![256, 1, 1024]); + + sort_proofs(&mut proofs, SelectProofsMethod::SmallestFirst, amount); + assert_proof_order(&proofs, vec![1, 256, 1024]); + } +} diff --git a/crates/cdk/src/wallet/send.rs b/crates/cdk/src/wallet/send.rs index a3024bde7..ba10348f3 100644 --- a/crates/cdk/src/wallet/send.rs +++ b/crates/cdk/src/wallet/send.rs @@ -6,7 +6,7 @@ use crate::{ Amount, Error, Wallet, }; -use super::SendKind; +use super::{proofs::SelectProofsMethod, SendKind}; impl Wallet { /// Send specific proofs @@ -85,8 +85,13 @@ impl Wallet { let available_proofs = available_proofs.into_iter().map(|p| p.proof).collect(); - let proofs_to_swap = - self.select_proofs_to_swap(amount, available_proofs).await?; + let proofs_to_swap = self + .select_proofs_to_swap( + amount, + available_proofs, + SelectProofsMethod::LargestFirst, + ) + .await?; let proofs_with_conditions = self .swap( diff --git a/crates/cdk/src/wallet/swap.rs b/crates/cdk/src/wallet/swap.rs index 7f59866ae..3758aa07b 100644 --- a/crates/cdk/src/wallet/swap.rs +++ b/crates/cdk/src/wallet/swap.rs @@ -15,6 +15,8 @@ use crate::Amount; use crate::Error; use crate::Wallet; +use super::proofs::SelectProofsMethod; + impl Wallet { /// Swap #[instrument(skip(self, input_proofs))] @@ -173,7 +175,9 @@ impl Wallet { return Err(Error::InsufficientFunds); } - let proofs = self.select_proofs_to_swap(amount, available_proofs).await?; + let proofs = self + .select_proofs_to_swap(amount, available_proofs, SelectProofsMethod::LargestFirst) + .await?; self.swap( Some(amount), From 314df0260b7a36907bb75e63904dabb9581a6e3c Mon Sep 17 00:00:00 2001 From: David Caseria Date: Thu, 3 Oct 2024 10:56:15 -0400 Subject: [PATCH 02/18] Fix clippy --- crates/cdk/src/wallet/melt.rs | 2 +- crates/cdk/src/wallet/proofs.rs | 20 ++++++++++---------- crates/cdk/src/wallet/send.rs | 2 +- crates/cdk/src/wallet/swap.rs | 2 +- 4 files changed, 13 insertions(+), 13 deletions(-) diff --git a/crates/cdk/src/wallet/melt.rs b/crates/cdk/src/wallet/melt.rs index 1abe5afd6..4a29012d4 100644 --- a/crates/cdk/src/wallet/melt.rs +++ b/crates/cdk/src/wallet/melt.rs @@ -297,7 +297,7 @@ impl Wallet { .select_proofs_to_swap( inputs_needed_amount, available_proofs, - SelectProofsMethod::ClosestFirst, + SelectProofsMethod::Closest, ) .await?; diff --git a/crates/cdk/src/wallet/proofs.rs b/crates/cdk/src/wallet/proofs.rs index cd25c0c9e..d43b6a254 100644 --- a/crates/cdk/src/wallet/proofs.rs +++ b/crates/cdk/src/wallet/proofs.rs @@ -283,8 +283,8 @@ impl Wallet { fn sort_proofs(proofs: &mut Proofs, method: SelectProofsMethod, amount: Amount) { match method { - SelectProofsMethod::LargestFirst => proofs.sort_by(|a: &Proof, b: &Proof| b.cmp(a)), - SelectProofsMethod::ClosestFirst => proofs.sort_by(|a: &Proof, b: &Proof| { + SelectProofsMethod::Largest => proofs.sort_by(|a: &Proof, b: &Proof| b.cmp(a)), + SelectProofsMethod::Closest => proofs.sort_by(|a: &Proof, b: &Proof| { let a_diff = if a.amount > amount { a.amount - amount } else { @@ -297,7 +297,7 @@ fn sort_proofs(proofs: &mut Proofs, method: SelectProofsMethod, amount: Amount) }; a_diff.cmp(&b_diff) }), - SelectProofsMethod::SmallestFirst => proofs.sort(), + SelectProofsMethod::Smallest => proofs.sort(), } } @@ -306,11 +306,11 @@ fn sort_proofs(proofs: &mut Proofs, method: SelectProofsMethod, amount: Amount) pub enum SelectProofsMethod { /// Select proofs with the largest amount first #[default] - LargestFirst, + Largest, /// Select proofs closest to the amount first - ClosestFirst, + Closest, /// Select proofs with the smallest amount first - SmallestFirst, + Smallest, } #[cfg(test)] @@ -354,19 +354,19 @@ mod tests { }, ]; - fn assert_proof_order(proofs: &Vec, order: Vec) { + fn assert_proof_order(proofs: &[Proof], order: Vec) { for (p, a) in proofs.iter().zip(order.iter()) { assert_eq!(p.amount, Amount::from(*a)); } } - sort_proofs(&mut proofs, SelectProofsMethod::LargestFirst, amount); + sort_proofs(&mut proofs, SelectProofsMethod::Largest, amount); assert_proof_order(&proofs, vec![1024, 256, 1]); - sort_proofs(&mut proofs, SelectProofsMethod::ClosestFirst, amount); + sort_proofs(&mut proofs, SelectProofsMethod::Closest, amount); assert_proof_order(&proofs, vec![256, 1, 1024]); - sort_proofs(&mut proofs, SelectProofsMethod::SmallestFirst, amount); + sort_proofs(&mut proofs, SelectProofsMethod::Smallest, amount); assert_proof_order(&proofs, vec![1, 256, 1024]); } } diff --git a/crates/cdk/src/wallet/send.rs b/crates/cdk/src/wallet/send.rs index ba10348f3..2285fad96 100644 --- a/crates/cdk/src/wallet/send.rs +++ b/crates/cdk/src/wallet/send.rs @@ -89,7 +89,7 @@ impl Wallet { .select_proofs_to_swap( amount, available_proofs, - SelectProofsMethod::LargestFirst, + SelectProofsMethod::Largest, ) .await?; diff --git a/crates/cdk/src/wallet/swap.rs b/crates/cdk/src/wallet/swap.rs index 3758aa07b..215f45844 100644 --- a/crates/cdk/src/wallet/swap.rs +++ b/crates/cdk/src/wallet/swap.rs @@ -176,7 +176,7 @@ impl Wallet { } let proofs = self - .select_proofs_to_swap(amount, available_proofs, SelectProofsMethod::LargestFirst) + .select_proofs_to_swap(amount, available_proofs, SelectProofsMethod::Largest) .await?; self.swap( From d01de88b0adefa5323af136005bd8087bdba2c8f Mon Sep 17 00:00:00 2001 From: David Caseria Date: Thu, 3 Oct 2024 17:52:58 -0400 Subject: [PATCH 03/18] Add least proof selection method --- crates/cdk/src/wallet/melt.rs | 4 +- crates/cdk/src/wallet/proofs.rs | 150 +++++++++++++++++++++++++++++--- crates/cdk/src/wallet/send.rs | 4 +- crates/cdk/src/wallet/swap.rs | 4 +- 4 files changed, 143 insertions(+), 19 deletions(-) diff --git a/crates/cdk/src/wallet/melt.rs b/crates/cdk/src/wallet/melt.rs index 4a29012d4..c88d1eb7e 100644 --- a/crates/cdk/src/wallet/melt.rs +++ b/crates/cdk/src/wallet/melt.rs @@ -11,7 +11,7 @@ use crate::{ Amount, Error, Wallet, }; -use super::{proofs::SelectProofsMethod, MeltQuote}; +use super::{proofs::ProofSelectionMethod, MeltQuote}; impl Wallet { /// Melt Quote @@ -297,7 +297,7 @@ impl Wallet { .select_proofs_to_swap( inputs_needed_amount, available_proofs, - SelectProofsMethod::Closest, + ProofSelectionMethod::Closest, ) .await?; diff --git a/crates/cdk/src/wallet/proofs.rs b/crates/cdk/src/wallet/proofs.rs index d43b6a254..571261dc7 100644 --- a/crates/cdk/src/wallet/proofs.rs +++ b/crates/cdk/src/wallet/proofs.rs @@ -1,11 +1,12 @@ -use std::collections::HashSet; +use std::collections::{HashMap, HashSet}; use tracing::instrument; use crate::{ amount::SplitTarget, dhke::hash_to_curve, - nuts::{Proof, ProofState, Proofs, PublicKey, State}, + fees::calculate_fee, + nuts::{Id, Proof, ProofState, Proofs, PublicKey, State}, types::ProofInfo, Amount, Error, Wallet, }; @@ -244,7 +245,7 @@ impl Wallet { &self, amount: Amount, proofs: Proofs, - method: SelectProofsMethod, + method: ProofSelectionMethod, ) -> Result { let active_keyset_id = self.get_active_mint_keyset().await?.id; @@ -253,7 +254,7 @@ impl Wallet { .partition(|p| p.keyset_id == active_keyset_id); let mut selected_proofs: Proofs = Vec::new(); - inactive_proofs.sort_by(|a: &Proof, b: &Proof| b.cmp(a)); + sort_proofs(&mut inactive_proofs, ProofSelectionMethod::Largest, amount); for inactive_proof in inactive_proofs { selected_proofs.push(inactive_proof); @@ -265,6 +266,24 @@ impl Wallet { } } + if method == ProofSelectionMethod::Least { + let selected_amount = Amount::try_sum(selected_proofs.iter().map(|p| p.amount))?; + let keyset_fees = self + .get_mint_keysets() + .await? + .into_iter() + .map(|k| (k.id, k.input_fee_ppk)) + .collect(); + if let Some(proofs) = select_least_proofs_over_amount( + &active_proofs, + amount.checked_sub(selected_amount).unwrap_or(Amount::ZERO), + keyset_fees, + ) { + selected_proofs.extend(proofs); + return Ok(selected_proofs); + } + } + sort_proofs(&mut active_proofs, method, amount); for active_proof in active_proofs { @@ -281,10 +300,13 @@ impl Wallet { } } -fn sort_proofs(proofs: &mut Proofs, method: SelectProofsMethod, amount: Amount) { +fn sort_proofs(proofs: &mut Proofs, method: ProofSelectionMethod, amount: Amount) { match method { - SelectProofsMethod::Largest => proofs.sort_by(|a: &Proof, b: &Proof| b.cmp(a)), - SelectProofsMethod::Closest => proofs.sort_by(|a: &Proof, b: &Proof| { + // Least fallback to largest + ProofSelectionMethod::Largest | ProofSelectionMethod::Least => { + proofs.sort_by(|a: &Proof, b: &Proof| b.cmp(a)) + } + ProofSelectionMethod::Closest => proofs.sort_by(|a: &Proof, b: &Proof| { let a_diff = if a.amount > amount { a.amount - amount } else { @@ -297,13 +319,68 @@ fn sort_proofs(proofs: &mut Proofs, method: SelectProofsMethod, amount: Amount) }; a_diff.cmp(&b_diff) }), - SelectProofsMethod::Smallest => proofs.sort(), + ProofSelectionMethod::Smallest => proofs.sort(), + } +} + +fn select_least_proofs_over_amount( + proofs: &Proofs, + amount: Amount, + fees: HashMap, +) -> Option> { + let max_sum = Amount::try_sum(proofs.iter().map(|p| p.amount)).ok()? + 1.into(); + if max_sum < amount || proofs.is_empty() || amount == Amount::ZERO { + return None; + } + let table_len = Into::::into(max_sum + 1.into()) as usize; + let mut dp = vec![None; table_len]; + let mut paths = vec![Vec::::new(); table_len]; + + dp[0] = Some(Amount::ZERO); + + // Fill DP table and track paths + for proof in proofs { + let max_other_amounts: u64 = (max_sum - proof.amount).into(); + for t in (0..=max_other_amounts).rev() { + if let Some(current_sum) = dp[t as usize] { + let new_sum = current_sum + proof.amount; + let target_index = (t + Into::::into(proof.amount)) as usize; + + // If we found a smaller sum or this sum has not been reached yet + if dp[target_index].is_none() || dp[target_index].unwrap() > new_sum { + dp[target_index] = Some(new_sum); + paths[target_index] = paths[t as usize].clone(); + paths[target_index].push(proof.clone()); + } + } + } + } + + // Find the smallest sum greater than or equal to the target + for t in Into::::into(amount)..=Into::::into(max_sum) { + if let Some(proofs_amount) = dp[t as usize] { + let proofs = &paths[t as usize]; + let mut proofs_count = HashMap::new(); + for proof in proofs { + proofs_count + .entry(proof.keyset_id) + .and_modify(|count| *count += 1) + .or_insert(1); + } + let fee = calculate_fee(&proofs_count, &fees).unwrap_or(Amount::ZERO); + + if proofs_amount >= amount + fee { + return Some(paths[t as usize].clone()); + } + } } + + None } /// Select proofs method #[derive(Debug, Default, Clone, Copy, Hash, PartialEq, Eq)] -pub enum SelectProofsMethod { +pub enum ProofSelectionMethod { /// Select proofs with the largest amount first #[default] Largest, @@ -311,17 +388,21 @@ pub enum SelectProofsMethod { Closest, /// Select proofs with the smallest amount first Smallest, + /// Select least number of proofs over the amount + Least, } #[cfg(test)] mod tests { + use std::collections::HashMap; + use crate::{ nuts::{Id, Proof, PublicKey}, secret::Secret, Amount, }; - use super::{sort_proofs, SelectProofsMethod}; + use super::{select_least_proofs_over_amount, sort_proofs, ProofSelectionMethod}; #[test] fn test_sort_proofs_by_method() { @@ -360,13 +441,56 @@ mod tests { } } - sort_proofs(&mut proofs, SelectProofsMethod::Largest, amount); + sort_proofs(&mut proofs, ProofSelectionMethod::Largest, amount); assert_proof_order(&proofs, vec![1024, 256, 1]); - sort_proofs(&mut proofs, SelectProofsMethod::Closest, amount); + sort_proofs(&mut proofs, ProofSelectionMethod::Closest, amount); assert_proof_order(&proofs, vec![256, 1, 1024]); - sort_proofs(&mut proofs, SelectProofsMethod::Smallest, amount); + sort_proofs(&mut proofs, ProofSelectionMethod::Smallest, amount); assert_proof_order(&proofs, vec![1, 256, 1024]); + + // Least should fallback to largest + sort_proofs(&mut proofs, ProofSelectionMethod::Least, amount); + assert_proof_order(&proofs, vec![1024, 256, 1]); + } + + #[test] + fn test_select_least_proofs_over_amount() { + let amount = Amount::from(1025); + let keyset_id = Id::random(); + let proofs = vec![ + Proof { + amount: 1.into(), + keyset_id, + secret: Secret::generate(), + c: PublicKey::random(), + witness: None, + dleq: None, + }, + Proof { + amount: 256.into(), + keyset_id, + secret: Secret::generate(), + c: PublicKey::random(), + witness: None, + dleq: None, + }, + Proof { + amount: 1024.into(), + keyset_id, + secret: Secret::generate(), + c: PublicKey::random(), + witness: None, + dleq: None, + }, + ]; + + let selected_proofs = + select_least_proofs_over_amount(&proofs, amount, HashMap::new()).unwrap(); + assert_eq!(selected_proofs.len(), 2); + let amounts: Vec = selected_proofs.iter().map(|p| p.amount.into()).collect(); + assert!(amounts.contains(&1024)); + assert!(amounts.contains(&1)); } } diff --git a/crates/cdk/src/wallet/send.rs b/crates/cdk/src/wallet/send.rs index 2285fad96..bc1ea1011 100644 --- a/crates/cdk/src/wallet/send.rs +++ b/crates/cdk/src/wallet/send.rs @@ -6,7 +6,7 @@ use crate::{ Amount, Error, Wallet, }; -use super::{proofs::SelectProofsMethod, SendKind}; +use super::{proofs::ProofSelectionMethod, SendKind}; impl Wallet { /// Send specific proofs @@ -89,7 +89,7 @@ impl Wallet { .select_proofs_to_swap( amount, available_proofs, - SelectProofsMethod::Largest, + ProofSelectionMethod::Largest, ) .await?; diff --git a/crates/cdk/src/wallet/swap.rs b/crates/cdk/src/wallet/swap.rs index 215f45844..d91563b26 100644 --- a/crates/cdk/src/wallet/swap.rs +++ b/crates/cdk/src/wallet/swap.rs @@ -15,7 +15,7 @@ use crate::Amount; use crate::Error; use crate::Wallet; -use super::proofs::SelectProofsMethod; +use super::proofs::ProofSelectionMethod; impl Wallet { /// Swap @@ -176,7 +176,7 @@ impl Wallet { } let proofs = self - .select_proofs_to_swap(amount, available_proofs, SelectProofsMethod::Largest) + .select_proofs_to_swap(amount, available_proofs, ProofSelectionMethod::Largest) .await?; self.swap( From 7bd56f2783bf7dad474185fc1170751cf3e06aac Mon Sep 17 00:00:00 2001 From: David Caseria Date: Thu, 3 Oct 2024 18:03:09 -0400 Subject: [PATCH 04/18] Fix least selection doc --- crates/cdk/src/wallet/proofs.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/cdk/src/wallet/proofs.rs b/crates/cdk/src/wallet/proofs.rs index 571261dc7..5ed285b2c 100644 --- a/crates/cdk/src/wallet/proofs.rs +++ b/crates/cdk/src/wallet/proofs.rs @@ -388,7 +388,7 @@ pub enum ProofSelectionMethod { Closest, /// Select proofs with the smallest amount first Smallest, - /// Select least number of proofs over the amount + /// Select least total proof amount over the specified amount Least, } From 22e000d95a613543ad3c1e1dd680c5bbeb91aad0 Mon Sep 17 00:00:00 2001 From: David Caseria Date: Sat, 5 Oct 2024 06:46:28 -0400 Subject: [PATCH 05/18] Combine select_proofs --- crates/cdk/examples/proof-selection.rs | 8 +- crates/cdk/src/wallet/melt.rs | 9 +- crates/cdk/src/wallet/mod.rs | 1 + crates/cdk/src/wallet/proofs.rs | 171 ++++++++++++------------- crates/cdk/src/wallet/send.rs | 12 +- crates/cdk/src/wallet/swap.rs | 8 +- 6 files changed, 108 insertions(+), 101 deletions(-) diff --git a/crates/cdk/examples/proof-selection.rs b/crates/cdk/examples/proof-selection.rs index 46d687a29..1d0b8138e 100644 --- a/crates/cdk/examples/proof-selection.rs +++ b/crates/cdk/examples/proof-selection.rs @@ -6,7 +6,7 @@ use std::time::Duration; use cdk::amount::SplitTarget; use cdk::cdk_database::WalletMemoryDatabase; use cdk::nuts::{CurrencyUnit, MintQuoteState}; -use cdk::wallet::Wallet; +use cdk::wallet::{SelectProofsOptions, Wallet}; use cdk::Amount; use rand::Rng; use tokio::time::sleep; @@ -51,7 +51,11 @@ async fn main() { let proofs = wallet.get_proofs().await.unwrap(); let selected = wallet - .select_proofs_to_send(Amount::from(64), proofs, false) + .select_proofs( + Amount::from(64), + proofs, + SelectProofsOptions::default().include_fees(false), + ) .await .unwrap(); diff --git a/crates/cdk/src/wallet/melt.rs b/crates/cdk/src/wallet/melt.rs index c88d1eb7e..7df7b86f8 100644 --- a/crates/cdk/src/wallet/melt.rs +++ b/crates/cdk/src/wallet/melt.rs @@ -11,7 +11,10 @@ use crate::{ Amount, Error, Wallet, }; -use super::{proofs::ProofSelectionMethod, MeltQuote}; +use super::{ + proofs::{ProofSelectionMethod, SelectProofsOptions}, + MeltQuote, +}; impl Wallet { /// Melt Quote @@ -294,10 +297,10 @@ impl Wallet { let available_proofs = self.get_proofs().await?; let input_proofs = self - .select_proofs_to_swap( + .select_proofs( inputs_needed_amount, available_proofs, - ProofSelectionMethod::Closest, + SelectProofsOptions::default().method(ProofSelectionMethod::Least), ) .await?; diff --git a/crates/cdk/src/wallet/mod.rs b/crates/cdk/src/wallet/mod.rs index 93e1b76b5..7ee27ff86 100644 --- a/crates/cdk/src/wallet/mod.rs +++ b/crates/cdk/src/wallet/mod.rs @@ -36,6 +36,7 @@ pub mod types; pub mod util; pub use multi_mint_wallet::MultiMintWallet; +pub use proofs::{ProofSelectionMethod, SelectProofsOptions}; pub use types::{MeltQuote, MintQuote, SendKind}; /// CDK Wallet diff --git a/crates/cdk/src/wallet/proofs.rs b/crates/cdk/src/wallet/proofs.rs index 5ed285b2c..2546674ce 100644 --- a/crates/cdk/src/wallet/proofs.rs +++ b/crates/cdk/src/wallet/proofs.rs @@ -167,113 +167,47 @@ impl Wallet { Ok(balance) } - /// Select proofs to send + /// Select proofs to send or swap for a specific amount. #[instrument(skip_all)] - pub async fn select_proofs_to_send( + pub async fn select_proofs( &self, amount: Amount, proofs: Proofs, - include_fees: bool, + opts: SelectProofsOptions, ) -> Result { - // TODO: Check all proofs are same unit - - if Amount::try_sum(proofs.iter().map(|p| p.amount))? < amount { - return Err(Error::InsufficientFunds); - } - - let (mut proofs_larger, mut proofs_smaller): (Proofs, Proofs) = - proofs.into_iter().partition(|p| p.amount > amount); - - let next_bigger_proof = proofs_larger.first().cloned(); - let mut selected_proofs: Proofs = Vec::new(); - let mut remaining_amount = amount; - while remaining_amount > Amount::ZERO { - proofs_larger.sort(); - // Sort smaller proofs in descending order - proofs_smaller.sort_by(|a: &Proof, b: &Proof| b.cmp(a)); - - let selected_proof = if let Some(next_small) = proofs_smaller.clone().first() { - next_small.clone() - } else if let Some(next_bigger) = proofs_larger.first() { - next_bigger.clone() - } else { - break; - }; - - let proof_amount = selected_proof.amount; - - selected_proofs.push(selected_proof); - - let fees = match include_fees { - true => self.get_proofs_fee(&selected_proofs).await?, - false => Amount::ZERO, - }; - - if proof_amount >= remaining_amount + fees { - remaining_amount = Amount::ZERO; - break; - } - - remaining_amount = amount.checked_add(fees).ok_or(Error::AmountOverflow)? - - Amount::try_sum(selected_proofs.iter().map(|p| p.amount))?; - (proofs_larger, proofs_smaller) = proofs_smaller - .into_iter() - .skip(1) - .partition(|p| p.amount > remaining_amount); - } - - if remaining_amount > Amount::ZERO { - if let Some(next_bigger) = next_bigger_proof { - return Ok(vec![next_bigger.clone()]); - } - - return Err(Error::InsufficientFunds); - } - - Ok(selected_proofs) - } - - /// Select proofs to send - /// - /// This method will first select inactive proofs and then active proofs. - /// Inactive proofs are always sorted largest first. - /// The active proofs are sorted by the [`SelectProofsMethod`] provided. - #[instrument(skip_all)] - pub async fn select_proofs_to_swap( - &self, - amount: Amount, - proofs: Proofs, - method: ProofSelectionMethod, - ) -> Result { let active_keyset_id = self.get_active_mint_keyset().await?.id; let (mut active_proofs, mut inactive_proofs): (Proofs, Proofs) = proofs .into_iter() .partition(|p| p.keyset_id == active_keyset_id); - let mut selected_proofs: Proofs = Vec::new(); - sort_proofs(&mut inactive_proofs, ProofSelectionMethod::Largest, amount); + if opts.allow_inactive_keys { + sort_proofs(&mut inactive_proofs, ProofSelectionMethod::Largest, amount); - for inactive_proof in inactive_proofs { - selected_proofs.push(inactive_proof); - let selected_total = Amount::try_sum(selected_proofs.iter().map(|p| p.amount))?; - let fees = self.get_proofs_fee(&selected_proofs).await?; + for inactive_proof in inactive_proofs { + selected_proofs.push(inactive_proof); + let selected_total = Amount::try_sum(selected_proofs.iter().map(|p| p.amount))?; + let fees = self.get_proofs_fee(&selected_proofs).await?; - if selected_total >= amount + fees { - return Ok(selected_proofs); + if selected_total >= amount + fees { + return Ok(selected_proofs); + } } } - if method == ProofSelectionMethod::Least { + if opts.method == ProofSelectionMethod::Least { let selected_amount = Amount::try_sum(selected_proofs.iter().map(|p| p.amount))?; - let keyset_fees = self - .get_mint_keysets() - .await? - .into_iter() - .map(|k| (k.id, k.input_fee_ppk)) - .collect(); + let keyset_fees = if opts.include_fees { + self.get_mint_keysets() + .await? + .into_iter() + .map(|k| (k.id, k.input_fee_ppk)) + .collect() + } else { + HashMap::new() + }; if let Some(proofs) = select_least_proofs_over_amount( &active_proofs, amount.checked_sub(selected_amount).unwrap_or(Amount::ZERO), @@ -284,12 +218,16 @@ impl Wallet { } } - sort_proofs(&mut active_proofs, method, amount); + sort_proofs(&mut active_proofs, opts.method, amount); for active_proof in active_proofs { selected_proofs.push(active_proof); let selected_total = Amount::try_sum(selected_proofs.iter().map(|p| p.amount))?; - let fees = self.get_proofs_fee(&selected_proofs).await?; + let fees = if opts.include_fees { + self.get_proofs_fee(&selected_proofs).await? + } else { + Amount::ZERO + }; if selected_total >= amount + fees { return Ok(selected_proofs); @@ -378,6 +316,59 @@ fn select_least_proofs_over_amount( None } +/// Select proofs options +pub struct SelectProofsOptions { + /// Allow inactive keys (if `true`, inactive keys will be selected first in largest order) + pub allow_inactive_keys: bool, + /// Include fees to add to the selection amount + pub include_fees: bool, + /// Proof selection method + pub method: ProofSelectionMethod, +} + +impl SelectProofsOptions { + /// Create new [`SelectProofsOptions`] + pub fn new( + allow_inactive_keys: bool, + include_fees: bool, + method: ProofSelectionMethod, + ) -> Self { + Self { + allow_inactive_keys, + include_fees, + method, + } + } + + /// Allow inactive keys (if `true`, inactive keys will be selected first in largest order) + pub fn allow_inactive_keys(mut self, allow_inactive_keys: bool) -> Self { + self.allow_inactive_keys = allow_inactive_keys; + self + } + + /// Include fees to add to the selection amount + pub fn include_fees(mut self, include_fees: bool) -> Self { + self.include_fees = include_fees; + self + } + + /// Proof selection method + pub fn method(mut self, method: ProofSelectionMethod) -> Self { + self.method = method; + self + } +} + +impl Default for SelectProofsOptions { + fn default() -> Self { + Self { + allow_inactive_keys: false, + include_fees: true, + method: ProofSelectionMethod::Largest, + } + } +} + /// Select proofs method #[derive(Debug, Default, Clone, Copy, Hash, PartialEq, Eq)] pub enum ProofSelectionMethod { diff --git a/crates/cdk/src/wallet/send.rs b/crates/cdk/src/wallet/send.rs index bc1ea1011..b269ef327 100644 --- a/crates/cdk/src/wallet/send.rs +++ b/crates/cdk/src/wallet/send.rs @@ -6,7 +6,7 @@ use crate::{ Amount, Error, Wallet, }; -use super::{proofs::ProofSelectionMethod, SendKind}; +use super::{proofs::SelectProofsOptions, SendKind}; impl Wallet { /// Send specific proofs @@ -86,10 +86,10 @@ impl Wallet { let available_proofs = available_proofs.into_iter().map(|p| p.proof).collect(); let proofs_to_swap = self - .select_proofs_to_swap( + .select_proofs( amount, available_proofs, - ProofSelectionMethod::Largest, + SelectProofsOptions::default().include_fees(include_fees), ) .await?; @@ -113,7 +113,11 @@ impl Wallet { }; let selected = self - .select_proofs_to_send(amount, available_proofs, include_fees) + .select_proofs( + amount, + available_proofs, + SelectProofsOptions::default().include_fees(include_fees), + ) .await; let send_proofs: Proofs = match (send_kind, selected, conditions.clone()) { diff --git a/crates/cdk/src/wallet/swap.rs b/crates/cdk/src/wallet/swap.rs index d91563b26..167c5b48d 100644 --- a/crates/cdk/src/wallet/swap.rs +++ b/crates/cdk/src/wallet/swap.rs @@ -15,7 +15,7 @@ use crate::Amount; use crate::Error; use crate::Wallet; -use super::proofs::ProofSelectionMethod; +use super::proofs::SelectProofsOptions; impl Wallet { /// Swap @@ -176,7 +176,11 @@ impl Wallet { } let proofs = self - .select_proofs_to_swap(amount, available_proofs, ProofSelectionMethod::Largest) + .select_proofs( + amount, + available_proofs, + SelectProofsOptions::default().include_fees(include_fees), + ) .await?; self.swap( From 9519bbd6d11eccaf1ba972ae19a618877568eea8 Mon Sep 17 00:00:00 2001 From: David Caseria Date: Sat, 5 Oct 2024 07:22:52 -0400 Subject: [PATCH 06/18] Add more tests and trace logging --- crates/cdk/src/wallet/proofs.rs | 83 ++++++++++++++++++++++++++++----- 1 file changed, 72 insertions(+), 11 deletions(-) diff --git a/crates/cdk/src/wallet/proofs.rs b/crates/cdk/src/wallet/proofs.rs index 2546674ce..134bee256 100644 --- a/crates/cdk/src/wallet/proofs.rs +++ b/crates/cdk/src/wallet/proofs.rs @@ -168,6 +168,7 @@ impl Wallet { } /// Select proofs to send or swap for a specific amount. + /// If inactive keys are allowed via [`SelectProofsOptions`], they will be selected first in largest order. #[instrument(skip_all)] pub async fn select_proofs( &self, @@ -266,6 +267,11 @@ fn select_least_proofs_over_amount( amount: Amount, fees: HashMap, ) -> Option> { + tracing::trace!( + "Selecting LEAST proofs for amount {} with fees {:?}", + amount, + fees + ); let max_sum = Amount::try_sum(proofs.iter().map(|p| p.amount)).ok()? + 1.into(); if max_sum < amount || proofs.is_empty() || amount == Amount::ZERO { return None; @@ -284,20 +290,31 @@ fn select_least_proofs_over_amount( let new_sum = current_sum + proof.amount; let target_index = (t + Into::::into(proof.amount)) as usize; - // If we found a smaller sum or this sum has not been reached yet - if dp[target_index].is_none() || dp[target_index].unwrap() > new_sum { + // If this sum has not been reached yet, or if the new sum is smaller, or if the new path is shorter + if dp[target_index].is_none() + || dp[target_index].unwrap() > new_sum + || paths[target_index].len() > paths[t as usize].len() + 1 + { + tracing::trace!("Updating DP table: {} -> {}", target_index, new_sum); dp[target_index] = Some(new_sum); paths[target_index] = paths[t as usize].clone(); paths[target_index].push(proof.clone()); + tracing::trace!("Path: {:?}", paths[target_index]); } } } } - // Find the smallest sum greater than or equal to the target + // Find the smallest sum greater than or equal to the target amount for t in Into::::into(amount)..=Into::::into(max_sum) { if let Some(proofs_amount) = dp[t as usize] { let proofs = &paths[t as usize]; + let proofs_sum = + Amount::try_sum(proofs.iter().map(|p| p.amount)).unwrap_or(Amount::ZERO); + if proofs_sum != proofs_amount { + tracing::error!("Proofs sum does not match DP table sum"); + continue; + } let mut proofs_count = HashMap::new(); for proof in proofs { proofs_count @@ -308,11 +325,19 @@ fn select_least_proofs_over_amount( let fee = calculate_fee(&proofs_count, &fees).unwrap_or(Amount::ZERO); if proofs_amount >= amount + fee { - return Some(paths[t as usize].clone()); + let proofs = paths[t as usize].clone(); + tracing::trace!( + "Selected proofs for amount {} with fee {}: {:?}", + amount, + fee, + proofs + ); + return Some(proofs); } } } + tracing::trace!("No proofs found for amount {}", amount); None } @@ -448,13 +473,29 @@ mod tests { #[test] fn test_select_least_proofs_over_amount() { - let amount = Amount::from(1025); let keyset_id = Id::random(); + let c_1 = PublicKey::random(); let proofs = vec![ Proof { amount: 1.into(), keyset_id, secret: Secret::generate(), + c: c_1, + witness: None, + dleq: None, + }, + Proof { + amount: 1.into(), + keyset_id, + secret: Secret::generate(), + c: c_1, + witness: None, + dleq: None, + }, + Proof { + amount: 2.into(), + keyset_id, + secret: Secret::generate(), c: PublicKey::random(), witness: None, dleq: None, @@ -477,11 +518,31 @@ mod tests { }, ]; - let selected_proofs = - select_least_proofs_over_amount(&proofs, amount, HashMap::new()).unwrap(); - assert_eq!(selected_proofs.len(), 2); - let amounts: Vec = selected_proofs.iter().map(|p| p.amount.into()).collect(); - assert!(amounts.contains(&1024)); - assert!(amounts.contains(&1)); + fn assert_amounts(proofs: &mut [Proof], amounts: &mut [u64]) { + println!("{:?}", proofs); + println!("{:?}", amounts); + assert_eq!(proofs.len(), amounts.len()); + proofs.sort_by(|a, b| a.amount.cmp(&b.amount)); + amounts.sort(); + for (p, a) in proofs.iter().zip(amounts.iter()) { + assert_eq!(p.amount, Amount::from(*a)); + } + } + + let mut selected_proofs = + select_least_proofs_over_amount(&proofs, Amount::from(1025), HashMap::new()).unwrap(); + assert_amounts(&mut selected_proofs, &mut [1024, 1]); + + let mut selected_proofs = + select_least_proofs_over_amount(&proofs, Amount::from(2), HashMap::new()).unwrap(); + assert_amounts(&mut selected_proofs, &mut [2]); + + let mut selected_proofs = + select_least_proofs_over_amount(&proofs, Amount::from(1284), HashMap::new()).unwrap(); + assert_amounts(&mut selected_proofs, &mut [1024, 256, 2, 1, 1]); + + assert!( + select_least_proofs_over_amount(&proofs, Amount::from(2048), HashMap::new()).is_none() + ); } } From 558df43ec7638f9a766dc2fda35b329495b63305 Mon Sep 17 00:00:00 2001 From: David Caseria Date: Sat, 5 Oct 2024 07:32:13 -0400 Subject: [PATCH 07/18] Update swap options --- crates/cdk/src/wallet/send.rs | 6 ++++-- crates/cdk/src/wallet/swap.rs | 4 +++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/crates/cdk/src/wallet/send.rs b/crates/cdk/src/wallet/send.rs index b269ef327..5f9de706b 100644 --- a/crates/cdk/src/wallet/send.rs +++ b/crates/cdk/src/wallet/send.rs @@ -6,7 +6,7 @@ use crate::{ Amount, Error, Wallet, }; -use super::{proofs::SelectProofsOptions, SendKind}; +use super::{proofs::SelectProofsOptions, ProofSelectionMethod, SendKind}; impl Wallet { /// Send specific proofs @@ -116,7 +116,9 @@ impl Wallet { .select_proofs( amount, available_proofs, - SelectProofsOptions::default().include_fees(include_fees), + SelectProofsOptions::default() + .include_fees(include_fees) + .method(ProofSelectionMethod::Least), ) .await; diff --git a/crates/cdk/src/wallet/swap.rs b/crates/cdk/src/wallet/swap.rs index 167c5b48d..80f92b534 100644 --- a/crates/cdk/src/wallet/swap.rs +++ b/crates/cdk/src/wallet/swap.rs @@ -179,7 +179,9 @@ impl Wallet { .select_proofs( amount, available_proofs, - SelectProofsOptions::default().include_fees(include_fees), + SelectProofsOptions::default() + .allow_inactive_keys(true) + .include_fees(include_fees), ) .await?; From e55fe41cfabe41efbed97d4f56949e3fb5b36404 Mon Sep 17 00:00:00 2001 From: David Caseria Date: Mon, 7 Oct 2024 08:22:40 -0400 Subject: [PATCH 08/18] Add more tests; least -> fewest --- crates/cdk/src/wallet/melt.rs | 2 +- crates/cdk/src/wallet/proofs.rs | 43 +++++++++++++++++++++------------ crates/cdk/src/wallet/send.rs | 2 +- 3 files changed, 29 insertions(+), 18 deletions(-) diff --git a/crates/cdk/src/wallet/melt.rs b/crates/cdk/src/wallet/melt.rs index 7df7b86f8..131236907 100644 --- a/crates/cdk/src/wallet/melt.rs +++ b/crates/cdk/src/wallet/melt.rs @@ -300,7 +300,7 @@ impl Wallet { .select_proofs( inputs_needed_amount, available_proofs, - SelectProofsOptions::default().method(ProofSelectionMethod::Least), + SelectProofsOptions::default().method(ProofSelectionMethod::Fewest), ) .await?; diff --git a/crates/cdk/src/wallet/proofs.rs b/crates/cdk/src/wallet/proofs.rs index 134bee256..4fac9b512 100644 --- a/crates/cdk/src/wallet/proofs.rs +++ b/crates/cdk/src/wallet/proofs.rs @@ -198,7 +198,7 @@ impl Wallet { } } - if opts.method == ProofSelectionMethod::Least { + if opts.method == ProofSelectionMethod::Fewest { let selected_amount = Amount::try_sum(selected_proofs.iter().map(|p| p.amount))?; let keyset_fees = if opts.include_fees { self.get_mint_keysets() @@ -209,7 +209,7 @@ impl Wallet { } else { HashMap::new() }; - if let Some(proofs) = select_least_proofs_over_amount( + if let Some(proofs) = select_fewest_proofs_over_amount( &active_proofs, amount.checked_sub(selected_amount).unwrap_or(Amount::ZERO), keyset_fees, @@ -242,7 +242,7 @@ impl Wallet { fn sort_proofs(proofs: &mut Proofs, method: ProofSelectionMethod, amount: Amount) { match method { // Least fallback to largest - ProofSelectionMethod::Largest | ProofSelectionMethod::Least => { + ProofSelectionMethod::Largest | ProofSelectionMethod::Fewest => { proofs.sort_by(|a: &Proof, b: &Proof| b.cmp(a)) } ProofSelectionMethod::Closest => proofs.sort_by(|a: &Proof, b: &Proof| { @@ -262,7 +262,7 @@ fn sort_proofs(proofs: &mut Proofs, method: ProofSelectionMethod, amount: Amount } } -fn select_least_proofs_over_amount( +fn select_fewest_proofs_over_amount( proofs: &Proofs, amount: Amount, fees: HashMap, @@ -397,15 +397,15 @@ impl Default for SelectProofsOptions { /// Select proofs method #[derive(Debug, Default, Clone, Copy, Hash, PartialEq, Eq)] pub enum ProofSelectionMethod { - /// Select proofs with the largest amount first + /// The largest value proofs first #[default] Largest, - /// Select proofs closest to the amount first + /// The closest in value to the amount first Closest, - /// Select proofs with the smallest amount first + /// The smallest value proofs first Smallest, - /// Select least total proof amount over the specified amount - Least, + /// Select fewest number of proofs over the specified amount + Fewest, } #[cfg(test)] @@ -418,7 +418,7 @@ mod tests { Amount, }; - use super::{select_least_proofs_over_amount, sort_proofs, ProofSelectionMethod}; + use super::{select_fewest_proofs_over_amount, sort_proofs, ProofSelectionMethod}; #[test] fn test_sort_proofs_by_method() { @@ -467,12 +467,12 @@ mod tests { assert_proof_order(&proofs, vec![1, 256, 1024]); // Least should fallback to largest - sort_proofs(&mut proofs, ProofSelectionMethod::Least, amount); + sort_proofs(&mut proofs, ProofSelectionMethod::Fewest, amount); assert_proof_order(&proofs, vec![1024, 256, 1]); } #[test] - fn test_select_least_proofs_over_amount() { + fn test_select_fewest_proofs_over_amount() { let keyset_id = Id::random(); let c_1 = PublicKey::random(); let proofs = vec![ @@ -530,19 +530,30 @@ mod tests { } let mut selected_proofs = - select_least_proofs_over_amount(&proofs, Amount::from(1025), HashMap::new()).unwrap(); + select_fewest_proofs_over_amount(&proofs, Amount::from(1025), HashMap::new()).unwrap(); assert_amounts(&mut selected_proofs, &mut [1024, 1]); let mut selected_proofs = - select_least_proofs_over_amount(&proofs, Amount::from(2), HashMap::new()).unwrap(); + select_fewest_proofs_over_amount(&proofs, Amount::from(1), HashMap::new()).unwrap(); + assert_amounts(&mut selected_proofs, &mut [1]); + + let mut selected_proofs = + select_fewest_proofs_over_amount(&proofs, Amount::from(2), HashMap::new()).unwrap(); assert_amounts(&mut selected_proofs, &mut [2]); let mut selected_proofs = - select_least_proofs_over_amount(&proofs, Amount::from(1284), HashMap::new()).unwrap(); + select_fewest_proofs_over_amount(&proofs, Amount::from(1284), HashMap::new()).unwrap(); assert_amounts(&mut selected_proofs, &mut [1024, 256, 2, 1, 1]); + // Edge cases + assert!( + select_fewest_proofs_over_amount(&proofs, Amount::from(2048), HashMap::new()).is_none() + ); + assert!( + select_fewest_proofs_over_amount(&proofs, Amount::from(0), HashMap::new()).is_none() + ); assert!( - select_least_proofs_over_amount(&proofs, Amount::from(2048), HashMap::new()).is_none() + select_fewest_proofs_over_amount(&vec![], Amount::from(1), HashMap::new()).is_none() ); } } diff --git a/crates/cdk/src/wallet/send.rs b/crates/cdk/src/wallet/send.rs index 5f9de706b..cbf9b57a5 100644 --- a/crates/cdk/src/wallet/send.rs +++ b/crates/cdk/src/wallet/send.rs @@ -118,7 +118,7 @@ impl Wallet { available_proofs, SelectProofsOptions::default() .include_fees(include_fees) - .method(ProofSelectionMethod::Least), + .method(ProofSelectionMethod::Fewest), ) .await; From e5c7cdc7b1c16cc1a4f0b6a19ad6106bc0a354b6 Mon Sep 17 00:00:00 2001 From: David Caseria Date: Mon, 7 Oct 2024 08:36:55 -0400 Subject: [PATCH 09/18] Fewest -> least :facepalm: --- crates/cdk/src/wallet/melt.rs | 2 +- crates/cdk/src/wallet/proofs.rs | 32 ++++++++++++++++---------------- crates/cdk/src/wallet/send.rs | 2 +- 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/crates/cdk/src/wallet/melt.rs b/crates/cdk/src/wallet/melt.rs index 131236907..7df7b86f8 100644 --- a/crates/cdk/src/wallet/melt.rs +++ b/crates/cdk/src/wallet/melt.rs @@ -300,7 +300,7 @@ impl Wallet { .select_proofs( inputs_needed_amount, available_proofs, - SelectProofsOptions::default().method(ProofSelectionMethod::Fewest), + SelectProofsOptions::default().method(ProofSelectionMethod::Least), ) .await?; diff --git a/crates/cdk/src/wallet/proofs.rs b/crates/cdk/src/wallet/proofs.rs index 4fac9b512..349d85d1f 100644 --- a/crates/cdk/src/wallet/proofs.rs +++ b/crates/cdk/src/wallet/proofs.rs @@ -198,7 +198,7 @@ impl Wallet { } } - if opts.method == ProofSelectionMethod::Fewest { + if opts.method == ProofSelectionMethod::Least { let selected_amount = Amount::try_sum(selected_proofs.iter().map(|p| p.amount))?; let keyset_fees = if opts.include_fees { self.get_mint_keysets() @@ -209,7 +209,7 @@ impl Wallet { } else { HashMap::new() }; - if let Some(proofs) = select_fewest_proofs_over_amount( + if let Some(proofs) = select_least_proofs_over_amount( &active_proofs, amount.checked_sub(selected_amount).unwrap_or(Amount::ZERO), keyset_fees, @@ -242,7 +242,7 @@ impl Wallet { fn sort_proofs(proofs: &mut Proofs, method: ProofSelectionMethod, amount: Amount) { match method { // Least fallback to largest - ProofSelectionMethod::Largest | ProofSelectionMethod::Fewest => { + ProofSelectionMethod::Largest | ProofSelectionMethod::Least => { proofs.sort_by(|a: &Proof, b: &Proof| b.cmp(a)) } ProofSelectionMethod::Closest => proofs.sort_by(|a: &Proof, b: &Proof| { @@ -262,13 +262,13 @@ fn sort_proofs(proofs: &mut Proofs, method: ProofSelectionMethod, amount: Amount } } -fn select_fewest_proofs_over_amount( +fn select_least_proofs_over_amount( proofs: &Proofs, amount: Amount, fees: HashMap, ) -> Option> { tracing::trace!( - "Selecting LEAST proofs for amount {} with fees {:?}", + "Selecting LEAST proofs over amount {} with fees {:?}", amount, fees ); @@ -404,8 +404,8 @@ pub enum ProofSelectionMethod { Closest, /// The smallest value proofs first Smallest, - /// Select fewest number of proofs over the specified amount - Fewest, + /// Select the least value of proofs equal to or over the specified amount + Least, } #[cfg(test)] @@ -418,7 +418,7 @@ mod tests { Amount, }; - use super::{select_fewest_proofs_over_amount, sort_proofs, ProofSelectionMethod}; + use super::{select_least_proofs_over_amount, sort_proofs, ProofSelectionMethod}; #[test] fn test_sort_proofs_by_method() { @@ -467,7 +467,7 @@ mod tests { assert_proof_order(&proofs, vec![1, 256, 1024]); // Least should fallback to largest - sort_proofs(&mut proofs, ProofSelectionMethod::Fewest, amount); + sort_proofs(&mut proofs, ProofSelectionMethod::Least, amount); assert_proof_order(&proofs, vec![1024, 256, 1]); } @@ -530,30 +530,30 @@ mod tests { } let mut selected_proofs = - select_fewest_proofs_over_amount(&proofs, Amount::from(1025), HashMap::new()).unwrap(); + select_least_proofs_over_amount(&proofs, Amount::from(1025), HashMap::new()).unwrap(); assert_amounts(&mut selected_proofs, &mut [1024, 1]); let mut selected_proofs = - select_fewest_proofs_over_amount(&proofs, Amount::from(1), HashMap::new()).unwrap(); + select_least_proofs_over_amount(&proofs, Amount::from(1), HashMap::new()).unwrap(); assert_amounts(&mut selected_proofs, &mut [1]); let mut selected_proofs = - select_fewest_proofs_over_amount(&proofs, Amount::from(2), HashMap::new()).unwrap(); + select_least_proofs_over_amount(&proofs, Amount::from(2), HashMap::new()).unwrap(); assert_amounts(&mut selected_proofs, &mut [2]); let mut selected_proofs = - select_fewest_proofs_over_amount(&proofs, Amount::from(1284), HashMap::new()).unwrap(); + select_least_proofs_over_amount(&proofs, Amount::from(1284), HashMap::new()).unwrap(); assert_amounts(&mut selected_proofs, &mut [1024, 256, 2, 1, 1]); // Edge cases assert!( - select_fewest_proofs_over_amount(&proofs, Amount::from(2048), HashMap::new()).is_none() + select_least_proofs_over_amount(&proofs, Amount::from(2048), HashMap::new()).is_none() ); assert!( - select_fewest_proofs_over_amount(&proofs, Amount::from(0), HashMap::new()).is_none() + select_least_proofs_over_amount(&proofs, Amount::from(0), HashMap::new()).is_none() ); assert!( - select_fewest_proofs_over_amount(&vec![], Amount::from(1), HashMap::new()).is_none() + select_least_proofs_over_amount(&vec![], Amount::from(1), HashMap::new()).is_none() ); } } diff --git a/crates/cdk/src/wallet/send.rs b/crates/cdk/src/wallet/send.rs index cbf9b57a5..5f9de706b 100644 --- a/crates/cdk/src/wallet/send.rs +++ b/crates/cdk/src/wallet/send.rs @@ -118,7 +118,7 @@ impl Wallet { available_proofs, SelectProofsOptions::default() .include_fees(include_fees) - .method(ProofSelectionMethod::Fewest), + .method(ProofSelectionMethod::Least), ) .await; From f60499316c51fd867925bfc294d4bac9fc80eace Mon Sep 17 00:00:00 2001 From: David Caseria Date: Mon, 7 Oct 2024 08:37:48 -0400 Subject: [PATCH 10/18] Fewest -> least :facepalm: --- crates/cdk/src/wallet/proofs.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/cdk/src/wallet/proofs.rs b/crates/cdk/src/wallet/proofs.rs index 349d85d1f..6f9562fd3 100644 --- a/crates/cdk/src/wallet/proofs.rs +++ b/crates/cdk/src/wallet/proofs.rs @@ -472,7 +472,7 @@ mod tests { } #[test] - fn test_select_fewest_proofs_over_amount() { + fn test_select_least_proofs_over_amount() { let keyset_id = Id::random(); let c_1 = PublicKey::random(); let proofs = vec![ From 35bd336d9e3b16f732ace18108b24a3b41936be2 Mon Sep 17 00:00:00 2001 From: David Caseria Date: Tue, 8 Oct 2024 07:33:08 -0400 Subject: [PATCH 11/18] Address PR feedback --- crates/cdk/src/wallet/proofs.rs | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/crates/cdk/src/wallet/proofs.rs b/crates/cdk/src/wallet/proofs.rs index 6f9562fd3..302cadae7 100644 --- a/crates/cdk/src/wallet/proofs.rs +++ b/crates/cdk/src/wallet/proofs.rs @@ -245,18 +245,12 @@ fn sort_proofs(proofs: &mut Proofs, method: ProofSelectionMethod, amount: Amount ProofSelectionMethod::Largest | ProofSelectionMethod::Least => { proofs.sort_by(|a: &Proof, b: &Proof| b.cmp(a)) } - ProofSelectionMethod::Closest => proofs.sort_by(|a: &Proof, b: &Proof| { - let a_diff = if a.amount > amount { - a.amount - amount + ProofSelectionMethod::Closest => proofs.sort_by_key(|p| { + if p.amount > amount { + p.amount - amount } else { - amount - a.amount - }; - let b_diff = if b.amount > amount { - b.amount - amount - } else { - amount - b.amount - }; - a_diff.cmp(&b_diff) + amount - p.amount + } }), ProofSelectionMethod::Smallest => proofs.sort(), } @@ -272,7 +266,9 @@ fn select_least_proofs_over_amount( amount, fees ); - let max_sum = Amount::try_sum(proofs.iter().map(|p| p.amount)).ok()? + 1.into(); + let max_sum = Amount::try_sum(proofs.iter().map(|p| p.amount)) + .ok()? + .checked_add(1.into())?; if max_sum < amount || proofs.is_empty() || amount == Amount::ZERO { return None; } From 552fff9a43c1cf1394a1fc421a53ca7c88c807a0 Mon Sep 17 00:00:00 2001 From: David Caseria Date: Tue, 8 Oct 2024 12:48:31 -0400 Subject: [PATCH 12/18] Address PR feedback --- crates/cdk/src/wallet/proofs.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/crates/cdk/src/wallet/proofs.rs b/crates/cdk/src/wallet/proofs.rs index 302cadae7..149fea940 100644 --- a/crates/cdk/src/wallet/proofs.rs +++ b/crates/cdk/src/wallet/proofs.rs @@ -184,7 +184,7 @@ impl Wallet { .into_iter() .partition(|p| p.keyset_id == active_keyset_id); - if opts.allow_inactive_keys { + if opts.prefer_inactive_keys { sort_proofs(&mut inactive_proofs, ProofSelectionMethod::Largest, amount); for inactive_proof in inactive_proofs { @@ -284,11 +284,11 @@ fn select_least_proofs_over_amount( for t in (0..=max_other_amounts).rev() { if let Some(current_sum) = dp[t as usize] { let new_sum = current_sum + proof.amount; - let target_index = (t + Into::::into(proof.amount)) as usize; + let target_index = (t + u64::from(proof.amount)) as usize; // If this sum has not been reached yet, or if the new sum is smaller, or if the new path is shorter if dp[target_index].is_none() - || dp[target_index].unwrap() > new_sum + || dp[target_index].expect("None checked") > new_sum || paths[target_index].len() > paths[t as usize].len() + 1 { tracing::trace!("Updating DP table: {} -> {}", target_index, new_sum); @@ -340,7 +340,7 @@ fn select_least_proofs_over_amount( /// Select proofs options pub struct SelectProofsOptions { /// Allow inactive keys (if `true`, inactive keys will be selected first in largest order) - pub allow_inactive_keys: bool, + pub prefer_inactive_keys: bool, /// Include fees to add to the selection amount pub include_fees: bool, /// Proof selection method @@ -355,7 +355,7 @@ impl SelectProofsOptions { method: ProofSelectionMethod, ) -> Self { Self { - allow_inactive_keys, + prefer_inactive_keys: allow_inactive_keys, include_fees, method, } @@ -363,7 +363,7 @@ impl SelectProofsOptions { /// Allow inactive keys (if `true`, inactive keys will be selected first in largest order) pub fn allow_inactive_keys(mut self, allow_inactive_keys: bool) -> Self { - self.allow_inactive_keys = allow_inactive_keys; + self.prefer_inactive_keys = allow_inactive_keys; self } @@ -383,7 +383,7 @@ impl SelectProofsOptions { impl Default for SelectProofsOptions { fn default() -> Self { Self { - allow_inactive_keys: false, + prefer_inactive_keys: true, include_fees: true, method: ProofSelectionMethod::Largest, } From bb9634ed4819d5ee1c622a42dcfc6b4affd01ac9 Mon Sep 17 00:00:00 2001 From: David Caseria Date: Tue, 8 Oct 2024 16:32:41 -0400 Subject: [PATCH 13/18] Improve invalid access guarding --- crates/cdk/src/wallet/proofs.rs | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/crates/cdk/src/wallet/proofs.rs b/crates/cdk/src/wallet/proofs.rs index 149fea940..c46d02362 100644 --- a/crates/cdk/src/wallet/proofs.rs +++ b/crates/cdk/src/wallet/proofs.rs @@ -272,7 +272,7 @@ fn select_least_proofs_over_amount( if max_sum < amount || proofs.is_empty() || amount == Amount::ZERO { return None; } - let table_len = Into::::into(max_sum + 1.into()) as usize; + let table_len = u64::from(max_sum + 1.into()) as usize; let mut dp = vec![None; table_len]; let mut paths = vec![Vec::::new(); table_len]; @@ -280,20 +280,30 @@ fn select_least_proofs_over_amount( // Fill DP table and track paths for proof in proofs { - let max_other_amounts: u64 = (max_sum - proof.amount).into(); + let max_other_amounts = u64::from(max_sum - proof.amount) as usize; for t in (0..=max_other_amounts).rev() { + // Double check bounds + if t >= dp.len() || t >= paths.len() { + continue; + } + if let Some(current_sum) = dp[t as usize] { let new_sum = current_sum + proof.amount; - let target_index = (t + u64::from(proof.amount)) as usize; + let target_index = (t as u64 + u64::from(proof.amount)) as usize; + + // Double check new bounds + if target_index >= dp.len() || target_index >= paths.len() { + continue; + } // If this sum has not been reached yet, or if the new sum is smaller, or if the new path is shorter if dp[target_index].is_none() || dp[target_index].expect("None checked") > new_sum - || paths[target_index].len() > paths[t as usize].len() + 1 + || paths[target_index].len() > paths[t].len() + 1 { tracing::trace!("Updating DP table: {} -> {}", target_index, new_sum); dp[target_index] = Some(new_sum); - paths[target_index] = paths[t as usize].clone(); + paths[target_index] = paths[t].clone(); paths[target_index].push(proof.clone()); tracing::trace!("Path: {:?}", paths[target_index]); } @@ -302,7 +312,7 @@ fn select_least_proofs_over_amount( } // Find the smallest sum greater than or equal to the target amount - for t in Into::::into(amount)..=Into::::into(max_sum) { + for t in u64::from(amount)..=u64::from(max_sum) { if let Some(proofs_amount) = dp[t as usize] { let proofs = &paths[t as usize]; let proofs_sum = From 5ef0766e50dc09a235b457055495e910e30f97a9 Mon Sep 17 00:00:00 2001 From: David Caseria Date: Tue, 8 Oct 2024 16:34:53 -0400 Subject: [PATCH 14/18] Improve invalid access guarding --- crates/cdk/src/wallet/proofs.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/crates/cdk/src/wallet/proofs.rs b/crates/cdk/src/wallet/proofs.rs index c46d02362..aecd823a0 100644 --- a/crates/cdk/src/wallet/proofs.rs +++ b/crates/cdk/src/wallet/proofs.rs @@ -313,8 +313,13 @@ fn select_least_proofs_over_amount( // Find the smallest sum greater than or equal to the target amount for t in u64::from(amount)..=u64::from(max_sum) { - if let Some(proofs_amount) = dp[t as usize] { - let proofs = &paths[t as usize]; + let idx = t as usize; + if idx >= dp.len() || idx >= paths.len() { + continue; + } + + if let Some(proofs_amount) = dp[idx] { + let proofs = &paths[idx]; let proofs_sum = Amount::try_sum(proofs.iter().map(|p| p.amount)).unwrap_or(Amount::ZERO); if proofs_sum != proofs_amount { @@ -331,7 +336,7 @@ fn select_least_proofs_over_amount( let fee = calculate_fee(&proofs_count, &fees).unwrap_or(Amount::ZERO); if proofs_amount >= amount + fee { - let proofs = paths[t as usize].clone(); + let proofs = paths[idx].clone(); tracing::trace!( "Selected proofs for amount {} with fee {}: {:?}", amount, From d45f636e63bdeaa92de8a632a25a03ae0a7025b2 Mon Sep 17 00:00:00 2001 From: David Caseria Date: Wed, 9 Oct 2024 07:59:30 -0400 Subject: [PATCH 15/18] Filter proofs to optimized least proof selection --- crates/cdk/src/wallet/proofs.rs | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/crates/cdk/src/wallet/proofs.rs b/crates/cdk/src/wallet/proofs.rs index aecd823a0..bf4964d35 100644 --- a/crates/cdk/src/wallet/proofs.rs +++ b/crates/cdk/src/wallet/proofs.rs @@ -266,20 +266,36 @@ fn select_least_proofs_over_amount( amount, fees ); - let max_sum = Amount::try_sum(proofs.iter().map(|p| p.amount)) + + // Filter proofs that are less than or equal to the amount and the next highest proof + let mut filtered_proofs: Vec = proofs + .iter() + .filter(|p| p.amount <= amount) + .cloned() + .collect(); + + if let Some(next_highest_proof) = proofs + .iter() + .filter(|p| p.amount > amount) + .min_by_key(|p| p.amount) + { + filtered_proofs.push(next_highest_proof.clone()); + } + + let max_sum = Amount::try_sum(filtered_proofs.iter().map(|p| p.amount)) .ok()? .checked_add(1.into())?; - if max_sum < amount || proofs.is_empty() || amount == Amount::ZERO { + if max_sum < amount || filtered_proofs.is_empty() || amount == Amount::ZERO { return None; } let table_len = u64::from(max_sum + 1.into()) as usize; let mut dp = vec![None; table_len]; - let mut paths = vec![Vec::::new(); table_len]; + let mut paths = vec![Vec::::with_capacity(0); table_len]; dp[0] = Some(Amount::ZERO); // Fill DP table and track paths - for proof in proofs { + for proof in filtered_proofs { let max_other_amounts = u64::from(max_sum - proof.amount) as usize; for t in (0..=max_other_amounts).rev() { // Double check bounds From 7c6d74b5f70fccf102c0a935bf0de2f8d47e4231 Mon Sep 17 00:00:00 2001 From: David Caseria Date: Wed, 9 Oct 2024 09:51:10 -0400 Subject: [PATCH 16/18] Fix docs; protect against OOM --- crates/cdk/src/wallet/proofs.rs | 47 ++++++++++++++++++++++++++------- crates/cdk/src/wallet/swap.rs | 2 +- 2 files changed, 38 insertions(+), 11 deletions(-) diff --git a/crates/cdk/src/wallet/proofs.rs b/crates/cdk/src/wallet/proofs.rs index bf4964d35..919132cd5 100644 --- a/crates/cdk/src/wallet/proofs.rs +++ b/crates/cdk/src/wallet/proofs.rs @@ -282,13 +282,17 @@ fn select_least_proofs_over_amount( filtered_proofs.push(next_highest_proof.clone()); } - let max_sum = Amount::try_sum(filtered_proofs.iter().map(|p| p.amount)) - .ok()? - .checked_add(1.into())?; + let max_sum = Amount::try_sum(filtered_proofs.iter().map(|p| p.amount)).ok()?; if max_sum < amount || filtered_proofs.is_empty() || amount == Amount::ZERO { return None; } + + // Determine the size of the DP table, return None if it is too large let table_len = u64::from(max_sum + 1.into()) as usize; + if table_len > 1_000_000 { + return None; + } + let mut dp = vec![None; table_len]; let mut paths = vec![Vec::::with_capacity(0); table_len]; @@ -305,7 +309,7 @@ fn select_least_proofs_over_amount( if let Some(current_sum) = dp[t as usize] { let new_sum = current_sum + proof.amount; - let target_index = (t as u64 + u64::from(proof.amount)) as usize; + let target_index = t + u64::from(proof.amount) as usize; // Double check new bounds if target_index >= dp.len() || target_index >= paths.len() { @@ -370,7 +374,7 @@ fn select_least_proofs_over_amount( /// Select proofs options pub struct SelectProofsOptions { - /// Allow inactive keys (if `true`, inactive keys will be selected first in largest order) + /// Prefer inactive keys (if `true`, inactive keys will be selected first in largest order) pub prefer_inactive_keys: bool, /// Include fees to add to the selection amount pub include_fees: bool, @@ -381,19 +385,19 @@ pub struct SelectProofsOptions { impl SelectProofsOptions { /// Create new [`SelectProofsOptions`] pub fn new( - allow_inactive_keys: bool, + prefer_inactive_keys: bool, include_fees: bool, method: ProofSelectionMethod, ) -> Self { Self { - prefer_inactive_keys: allow_inactive_keys, + prefer_inactive_keys, include_fees, method, } } - /// Allow inactive keys (if `true`, inactive keys will be selected first in largest order) - pub fn allow_inactive_keys(mut self, allow_inactive_keys: bool) -> Self { + /// Prefer inactive keys (if `true`, inactive keys will be selected first in largest order) + pub fn prefer_inactive_keys(mut self, allow_inactive_keys: bool) -> Self { self.prefer_inactive_keys = allow_inactive_keys; self } @@ -431,7 +435,7 @@ pub enum ProofSelectionMethod { Closest, /// The smallest value proofs first Smallest, - /// Select the least value of proofs equal to or over the specified amount + /// Select the least value of proofs equal to or over the specified amount (best-effort, may fallback to largest) Least, } @@ -582,5 +586,28 @@ mod tests { assert!( select_least_proofs_over_amount(&vec![], Amount::from(1), HashMap::new()).is_none() ); + + // OOM protection + let proofs = vec![ + Proof { + amount: Amount::from(1_048_576), + keyset_id, + secret: Secret::generate(), + c: PublicKey::random(), + witness: None, + dleq: None, + }, + Proof { + amount: Amount::from(2_097_152), + keyset_id, + secret: Secret::generate(), + c: PublicKey::random(), + witness: None, + dleq: None, + }, + ]; + assert!( + select_least_proofs_over_amount(&proofs, Amount::from(1), HashMap::new()).is_none() + ); } } diff --git a/crates/cdk/src/wallet/swap.rs b/crates/cdk/src/wallet/swap.rs index 80f92b534..36d0a1448 100644 --- a/crates/cdk/src/wallet/swap.rs +++ b/crates/cdk/src/wallet/swap.rs @@ -180,7 +180,7 @@ impl Wallet { amount, available_proofs, SelectProofsOptions::default() - .allow_inactive_keys(true) + .prefer_inactive_keys(true) .include_fees(include_fees), ) .await?; From d68ffadedf9f519ff218f299d6a5cf4bec37a382 Mon Sep 17 00:00:00 2001 From: David Caseria Date: Wed, 9 Oct 2024 09:52:31 -0400 Subject: [PATCH 17/18] Fix clippy --- crates/cdk/src/wallet/proofs.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/cdk/src/wallet/proofs.rs b/crates/cdk/src/wallet/proofs.rs index 919132cd5..bc2338a19 100644 --- a/crates/cdk/src/wallet/proofs.rs +++ b/crates/cdk/src/wallet/proofs.rs @@ -307,7 +307,7 @@ fn select_least_proofs_over_amount( continue; } - if let Some(current_sum) = dp[t as usize] { + if let Some(current_sum) = dp[t] { let new_sum = current_sum + proof.amount; let target_index = t + u64::from(proof.amount) as usize; From d60da75c7dd9b5df4c613c5137a0fbde60f47280 Mon Sep 17 00:00:00 2001 From: David Caseria Date: Wed, 16 Oct 2024 12:15:01 +0100 Subject: [PATCH 18/18] Smallest over --- crates/cdk/src/wallet/melt.rs | 7 ++-- crates/cdk/src/wallet/proofs.rs | 58 +++++++++++++++++++++++++-------- crates/cdk/src/wallet/send.rs | 6 ++-- 3 files changed, 49 insertions(+), 22 deletions(-) diff --git a/crates/cdk/src/wallet/melt.rs b/crates/cdk/src/wallet/melt.rs index 7df7b86f8..5e1aea932 100644 --- a/crates/cdk/src/wallet/melt.rs +++ b/crates/cdk/src/wallet/melt.rs @@ -11,10 +11,7 @@ use crate::{ Amount, Error, Wallet, }; -use super::{ - proofs::{ProofSelectionMethod, SelectProofsOptions}, - MeltQuote, -}; +use super::{proofs::SelectProofsOptions, MeltQuote}; impl Wallet { /// Melt Quote @@ -300,7 +297,7 @@ impl Wallet { .select_proofs( inputs_needed_amount, available_proofs, - SelectProofsOptions::default().method(ProofSelectionMethod::Least), + SelectProofsOptions::default(), ) .await?; diff --git a/crates/cdk/src/wallet/proofs.rs b/crates/cdk/src/wallet/proofs.rs index bc2338a19..fb774b9bc 100644 --- a/crates/cdk/src/wallet/proofs.rs +++ b/crates/cdk/src/wallet/proofs.rs @@ -241,10 +241,16 @@ impl Wallet { fn sort_proofs(proofs: &mut Proofs, method: ProofSelectionMethod, amount: Amount) { match method { - // Least fallback to largest - ProofSelectionMethod::Largest | ProofSelectionMethod::Least => { - proofs.sort_by(|a: &Proof, b: &Proof| b.cmp(a)) - } + // Least fallback to smallest over + ProofSelectionMethod::SmallestOver | ProofSelectionMethod::Least => proofs.sort_by( + |a: &Proof, b: &Proof| match (a.amount >= amount, b.amount >= amount) { + (true, true) => a.amount.cmp(&b.amount), + (true, false) => std::cmp::Ordering::Less, + (false, true) => std::cmp::Ordering::Greater, + (false, false) => b.amount.cmp(&a.amount), + }, + ), + ProofSelectionMethod::Largest => proofs.sort_by(|a: &Proof, b: &Proof| b.cmp(a)), ProofSelectionMethod::Closest => proofs.sort_by_key(|p| { if p.amount > amount { p.amount - amount @@ -428,17 +434,32 @@ impl Default for SelectProofsOptions { /// Select proofs method #[derive(Debug, Default, Clone, Copy, Hash, PartialEq, Eq)] pub enum ProofSelectionMethod { - /// The largest value proofs first + /// The smallest over the specified amount proofs first, then largest #[default] + SmallestOver, + /// The largest value proofs first Largest, /// The closest in value to the amount first Closest, /// The smallest value proofs first Smallest, - /// Select the least value of proofs equal to or over the specified amount (best-effort, may fallback to largest) + /// Select the least value of proofs equal to or over the specified amount. + /// **CAUTION**: This method can be slow or OOM for large proof sets. Least, } +impl std::fmt::Display for ProofSelectionMethod { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + ProofSelectionMethod::SmallestOver => write!(f, "SmallestOver"), + ProofSelectionMethod::Largest => write!(f, "Largest"), + ProofSelectionMethod::Closest => write!(f, "Closest"), + ProofSelectionMethod::Smallest => write!(f, "Smallest"), + ProofSelectionMethod::Least => write!(f, "Least"), + } + } +} + #[cfg(test)] mod tests { use std::collections::HashMap; @@ -482,24 +503,35 @@ mod tests { }, ]; - fn assert_proof_order(proofs: &[Proof], order: Vec) { + fn assert_proof_order( + proofs: &[Proof], + order: Vec, + test_method: ProofSelectionMethod, + ) { for (p, a) in proofs.iter().zip(order.iter()) { - assert_eq!(p.amount, Amount::from(*a)); + assert_eq!(p.amount, Amount::from(*a), "{}", test_method); } } + sort_proofs(&mut proofs, ProofSelectionMethod::SmallestOver, amount); + assert_proof_order( + &proofs, + vec![256, 1024, 1], + ProofSelectionMethod::SmallestOver, + ); + sort_proofs(&mut proofs, ProofSelectionMethod::Largest, amount); - assert_proof_order(&proofs, vec![1024, 256, 1]); + assert_proof_order(&proofs, vec![1024, 256, 1], ProofSelectionMethod::Largest); sort_proofs(&mut proofs, ProofSelectionMethod::Closest, amount); - assert_proof_order(&proofs, vec![256, 1, 1024]); + assert_proof_order(&proofs, vec![256, 1, 1024], ProofSelectionMethod::Closest); sort_proofs(&mut proofs, ProofSelectionMethod::Smallest, amount); - assert_proof_order(&proofs, vec![1, 256, 1024]); + assert_proof_order(&proofs, vec![1, 256, 1024], ProofSelectionMethod::Smallest); - // Least should fallback to largest + // Least should fallback to smallest over sort_proofs(&mut proofs, ProofSelectionMethod::Least, amount); - assert_proof_order(&proofs, vec![1024, 256, 1]); + assert_proof_order(&proofs, vec![256, 1024, 1], ProofSelectionMethod::Least); } #[test] diff --git a/crates/cdk/src/wallet/send.rs b/crates/cdk/src/wallet/send.rs index 5f9de706b..b269ef327 100644 --- a/crates/cdk/src/wallet/send.rs +++ b/crates/cdk/src/wallet/send.rs @@ -6,7 +6,7 @@ use crate::{ Amount, Error, Wallet, }; -use super::{proofs::SelectProofsOptions, ProofSelectionMethod, SendKind}; +use super::{proofs::SelectProofsOptions, SendKind}; impl Wallet { /// Send specific proofs @@ -116,9 +116,7 @@ impl Wallet { .select_proofs( amount, available_proofs, - SelectProofsOptions::default() - .include_fees(include_fees) - .method(ProofSelectionMethod::Least), + SelectProofsOptions::default().include_fees(include_fees), ) .await;