Merge pull request fedimint#6231 from elsirion/2024-10-safe-ecash-spend

fix: make `spend_ecash` not overspend by default

fedimint-load-test-tool/src/common.rs

@@ -19,7 +19,9 @@ use fedimint_ln_client::{
     LightningClientInit, LightningClientModule, LnPayState, OutgoingLightningPayment,
 };
 use fedimint_ln_common::LightningGateway;
-use fedimint_mint_client::{MintClientInit, MintClientModule, MintCommonInit, OOBNotes};
+use fedimint_mint_client::{
+    MintClientInit, MintClientModule, MintCommonInit, OOBNotes, SelectNotesWithAtleastAmount,
+};
 use fedimint_wallet_client::WalletClientInit;
 use futures::StreamExt;
 use lightning_invoice::Bolt11Invoice;
@@ -90,7 +92,13 @@ pub async fn do_spend_notes(
 ) -> anyhow::Result<(OperationId, OOBNotes)> {
     let mint = &mint.get_first_module::<MintClientModule>()?;
     let (operation_id, oob_notes) = mint
-        .spend_notes(amount, Duration::from_secs(600), false, ())
+        .spend_notes_with_selector(
+            &SelectNotesWithAtleastAmount,
+            amount,
+            Duration::from_secs(600),
+            false,
+            (),
+        )
         .await?;
     let mut updates = mint
         .subscribe_spend_notes(operation_id)

fedimint-wasm-tests/src/lib.rs

@@ -117,7 +117,9 @@ mod tests {
     };
     use fedimint_ln_common::lightning_invoice::{Bolt11InvoiceDescription, Description};
     use fedimint_ln_common::LightningGateway;
-    use fedimint_mint_client::{MintClientModule, ReissueExternalNotesState, SpendOOBState};
+    use fedimint_mint_client::{
+        MintClientModule, ReissueExternalNotesState, SelectNotesWithAtleastAmount, SpendOOBState,
+    };
     use futures::StreamExt;
     use wasm_bindgen_test::wasm_bindgen_test;
 
@@ -262,7 +264,13 @@ mod tests {
     ) -> Result<(), anyhow::Error> {
         let mint = client.get_first_module::<MintClientModule>()?;
         let (_, notes) = mint
-            .spend_notes(Amount::from_sats(11), Duration::from_secs(10000), false, ())
+            .spend_notes_with_selector(
+                &SelectNotesWithAtleastAmount,
+                Amount::from_sats(11),
+                Duration::from_secs(10000),
+                false,
+                (),
+            )
             .await?;
         let operation_id = mint.reissue_external_notes(notes, ()).await?;
         let mut updates = mint
@@ -291,7 +299,13 @@ mod tests {
         let mint = client.get_first_module::<MintClientModule>()?;
         'retry: loop {
             let (operation_id, notes) = mint
-                .spend_notes(amount, Duration::from_secs(10000), false, ())
+                .spend_notes_with_selector(
+                    &SelectNotesWithAtleastAmount,
+                    amount,
+                    Duration::from_secs(10000),
+                    false,
+                    (),
+                )
                 .await?;
             if notes.total_amount() == amount {
                 return Ok(());

modules/fedimint-mint-client/src/lib.rs

@@ -457,7 +457,7 @@ pub enum ReissueExternalNotesState {
 }
 
 /// The high-level state of a raw e-cash spend operation started with
-/// [`MintClientModule::spend_notes`].
+/// [`MintClientModule::spend_notes_with_selector`].
 #[derive(Debug, Clone, Eq, PartialEq, Serialize, Deserialize)]
 pub enum SpendOOBState {
     /// The e-cash has been selected and given to the caller
@@ -859,7 +859,24 @@ impl ClientModule for MintClientModule {
                 }
                 "spend_notes" => {
                     let req: SpendNotesRequest = serde_json::from_value(request)?;
-                    let result = self.spend_notes(req.min_amount, req.try_cancel_after, req.include_invite, req.extra_meta).await?;
+                    let result = self.spend_notes_with_selector(
+                        &SelectNotesWithExactAmount,
+                        req.amount,
+                        req.try_cancel_after,
+                        req.include_invite,
+                        req.extra_meta
+                    ).await?;
+                    yield serde_json::to_value(result)?;
+                }
+                "spend_notes_expert" => {
+                    let req: SpendNotesExpertRequest = serde_json::from_value(request)?;
+                    let result = self.spend_notes_with_selector(
+                        &SelectNotesWithAtleastAmount,
+                        req.min_amount,
+                        req.try_cancel_after,
+                        req.include_invite,
+                        req.extra_meta
+                    ).await?;
                     yield serde_json::to_value(result)?;
                 }
                 "validate_notes" => {
@@ -904,14 +921,24 @@ struct SubscribeReissueExternalNotesRequest {
     operation_id: OperationId,
 }
 
+/// Caution: if no notes of the correct denomination are available the next
+/// bigger note will be selected. You might want to use `spend_notes` instead.
 #[derive(Deserialize)]
-struct SpendNotesRequest {
+struct SpendNotesExpertRequest {
     min_amount: Amount,
     try_cancel_after: Duration,
     include_invite: bool,
     extra_meta: serde_json::Value,
 }
 
+#[derive(Deserialize)]
+struct SpendNotesRequest {
+    amount: Amount,
+    try_cancel_after: Duration,
+    include_invite: bool,
+    extra_meta: serde_json::Value,
+}
+
 #[derive(Deserialize)]
 struct ValidateNotesRequest {
     oob_notes: OOBNotes,
@@ -1547,6 +1574,10 @@ impl MintClientModule {
     /// should be chosen such that the recipient (who is potentially offline at
     /// the time of receiving the e-cash notes) had a reasonable timeframe to
     /// come online and reissue the notes themselves.
+    #[deprecated(
+        since = "0.5.0",
+        note = "Use `spend_notes_with_selector` instead, with `SelectNotesWithAtleastAmount` to maintain the same behavior"
+    )]
     pub async fn spend_notes<M: Serialize + Send>(
         &self,
         min_amount: Amount,
@@ -1564,7 +1595,21 @@ impl MintClientModule {
         .await
     }
 
-    /// Same as `spend_notes` but allows different to select notes to be used.
+    /// Fetches and removes notes from the wallet to be sent to the recipient
+    /// out of band. The not selection algorithm is determined by
+    /// `note_selector`. See the [`NotesSelector`] trait for available
+    /// implementations.
+    ///
+    /// These spends can be canceled by calling
+    /// [`MintClientModule::try_cancel_spend_notes`] as long
+    /// as the recipient hasn't reissued the e-cash notes themselves yet.
+    ///
+    /// The client will also automatically attempt to cancel the operation after
+    /// `try_cancel_after` time has passed. This is a safety mechanism to avoid
+    /// users forgetting about failed out-of-band transactions. The timeout
+    /// should be chosen such that the recipient (who is potentially offline at
+    /// the time of receiving the e-cash notes) had a reasonable timeframe to
+    /// come online and reissue the notes themselves.
     pub async fn spend_notes_with_selector<M: Serialize + Send>(
         &self,
         notes_selector: &impl NotesSelector,
@@ -1672,9 +1717,9 @@ impl MintClientModule {
     }
 
     /// Try to cancel a spend operation started with
-    /// [`MintClientModule::spend_notes`]. If the e-cash notes have already been
-    /// spent this operation will fail which can be observed using
-    /// [`MintClientModule::subscribe_spend_notes`].
+    /// [`MintClientModule::spend_notes_with_selector`]. If the e-cash notes
+    /// have already been spent this operation will fail which can be
+    /// observed using [`MintClientModule::subscribe_spend_notes`].
     pub async fn try_cancel_spend_notes(&self, operation_id: OperationId) {
         let mut dbtx = self.client_ctx.module_db().begin_transaction().await;
         dbtx.insert_entry(&CancelledOOBSpendKey(operation_id), &())
@@ -1685,7 +1730,7 @@ impl MintClientModule {
     }
 
     /// Subscribe to updates on the progress of a raw e-cash spend operation
-    /// started with [`MintClientModule::spend_notes`].
+    /// started with [`MintClientModule::spend_notes_with_selector`].
     pub async fn subscribe_spend_notes(
         &self,
         operation_id: OperationId,
@@ -1813,6 +1858,8 @@ pub struct SpendOOBRefund {
     pub transaction_id: TransactionId,
 }
 
+/// Defines a strategy for selecting e-cash notes given a specific target amount
+/// and fee per note transaction input.
 #[apply(async_trait_maybe_send!)]
 pub trait NotesSelector<Note = SpendableNoteUndecoded>: Send + Sync {
     /// Select notes from stream for requested_amount.

modules/fedimint-mint-tests/tests/tests.rs

@@ -16,7 +16,8 @@ use fedimint_dummy_common::config::DummyGenParams;
 use fedimint_dummy_server::DummyInit;
 use fedimint_logging::LOG_TEST;
 use fedimint_mint_client::{
-    MintClientInit, MintClientModule, Note, OOBNotes, ReissueExternalNotesState, SpendOOBState,
+    MintClientInit, MintClientModule, Note, OOBNotes, ReissueExternalNotesState,
+    SelectNotesWithAtleastAmount, SpendOOBState,
 };
 use fedimint_mint_common::config::{FeeConsensus, MintGenParams, MintGenParamsConsensus};
 use fedimint_mint_common::{MintInput, MintInputV0, Nonce};
@@ -114,7 +115,7 @@ async fn sends_ecash_out_of_band() -> anyhow::Result<()> {
     let client2_mint = client2.get_first_module::<MintClientModule>()?;
     info!("### SPEND NOTES");
     let (op, notes) = client1_mint
-        .spend_notes(sats(750), TIMEOUT, false, ())
+        .spend_notes_with_selector(&SelectNotesWithAtleastAmount, sats(750), TIMEOUT, false, ())
         .await?;
     let sub1 = &mut client1_mint.subscribe_spend_notes(op).await?.into_stream();
     assert_eq!(sub1.ok().await?, SpendOOBState::Created);
@@ -166,7 +167,13 @@ async fn sends_ecash_oob_highly_parallel() -> anyhow::Result<()> {
                 info!("Starting spend {num_spend}");
                 let client1_mint = task_client1.get_first_module::<MintClientModule>().unwrap();
                 let (op, notes) = client1_mint
-                    .spend_notes(sats(30), ECASH_TIMEOUT, false, ())
+                    .spend_notes_with_selector(
+                        &SelectNotesWithAtleastAmount,
+                        sats(30),
+                        ECASH_TIMEOUT,
+                        false,
+                        (),
+                    )
                     .await
                     .unwrap();
                 let sub1 = &mut client1_mint
@@ -291,7 +298,7 @@ async fn sends_ecash_out_of_band_cancel() -> anyhow::Result<()> {
     // Spend from client1 to client2
     let mint_module = client.get_first_module::<MintClientModule>()?;
     let (op, _) = mint_module
-        .spend_notes(sats(750), TIMEOUT, false, ())
+        .spend_notes_with_selector(&SelectNotesWithAtleastAmount, sats(750), TIMEOUT, false, ())
         .await?;
     let sub1 = &mut mint_module.subscribe_spend_notes(op).await?.into_stream();
     assert_eq!(sub1.ok().await?, SpendOOBState::Created);
@@ -328,7 +335,13 @@ async fn error_zero_value_oob_spend() -> anyhow::Result<()> {
     // Spend from client1 to client2
     let err_msg = client1
         .get_first_module::<MintClientModule>()?
-        .spend_notes(Amount::ZERO, TIMEOUT, false, ())
+        .spend_notes_with_selector(
+            &SelectNotesWithAtleastAmount,
+            Amount::ZERO,
+            TIMEOUT,
+            false,
+            (),
+        )
         .await
         .expect_err("Zero-amount spends should be forbidden")
         .to_string();