Parcourir la source

Allow multiple melt quotes for a payment request (#1353)

  The duplicate melt quote check now happens during melt execution (when
  transitioning to PENDING state) rather than during quote creation.

  This allows wallets to create multiple quotes for the same invoice,
  which is useful for retry scenarios. The constraint that only one
  quote per request_lookup_id can be PENDING or PAID is still enforced,
  but at melt time via FOR UPDATE row locking.
tsk il y a 2 mois
Parent
commit
68099730b3

+ 104 - 0
crates/cdk-integration-tests/tests/fake_wallet.rs

@@ -1462,6 +1462,110 @@ async fn test_wallet_proof_recovery_after_failed_melt() {
     );
 }
 
+/// Tests that concurrent melt attempts for the same invoice result in exactly one success
+///
+/// This test verifies the race condition protection: when multiple melt quotes exist for the
+/// same invoice and all are attempted concurrently, only one should succeed due to
+/// the FOR UPDATE locking on quotes with the same request_lookup_id.
+#[tokio::test(flavor = "multi_thread", worker_threads = 4)]
+async fn test_concurrent_melt_same_invoice() {
+    const NUM_WALLETS: usize = 4;
+
+    // Create multiple wallets to simulate concurrent requests
+    let mut wallets = Vec::with_capacity(NUM_WALLETS);
+    for i in 0..NUM_WALLETS {
+        let wallet = Arc::new(
+            Wallet::new(
+                MINT_URL,
+                CurrencyUnit::Sat,
+                Arc::new(memory::empty().await.unwrap()),
+                Mnemonic::generate(12).unwrap().to_seed_normalized(""),
+                None,
+            )
+            .expect(&format!("failed to create wallet {}", i)),
+        );
+        wallets.push(wallet);
+    }
+
+    // Mint proofs for all wallets
+    for (i, wallet) in wallets.iter().enumerate() {
+        let mint_quote = wallet.mint_quote(100.into(), None).await.unwrap();
+        let mut proof_streams =
+            wallet.proof_stream(mint_quote.clone(), SplitTarget::default(), None);
+        proof_streams
+            .next()
+            .await
+            .expect(&format!("payment for wallet {}", i))
+            .expect("no error");
+    }
+
+    // Create a single invoice that all wallets will try to pay
+    let fake_description = FakeInvoiceDescription::default();
+    let invoice = create_fake_invoice(9000, serde_json::to_string(&fake_description).unwrap());
+
+    // All wallets create melt quotes for the same invoice
+    let mut melt_quotes = Vec::with_capacity(NUM_WALLETS);
+    for wallet in &wallets {
+        let melt_quote = wallet.melt_quote(invoice.to_string(), None).await.unwrap();
+        melt_quotes.push(melt_quote);
+    }
+
+    // Verify all quotes have the same request (same invoice = same lookup_id)
+    for quote in &melt_quotes[1..] {
+        assert_eq!(
+            melt_quotes[0].request, quote.request,
+            "All quotes should be for the same invoice"
+        );
+    }
+
+    // Attempt all melts concurrently
+    let mut handles = Vec::with_capacity(NUM_WALLETS);
+    for (wallet, quote) in wallets.iter().zip(melt_quotes.iter()) {
+        let wallet_clone = Arc::clone(wallet);
+        let quote_id = quote.id.clone();
+        handles.push(tokio::spawn(
+            async move { wallet_clone.melt(&quote_id).await },
+        ));
+    }
+
+    // Collect results
+    let mut results = Vec::with_capacity(NUM_WALLETS);
+    for handle in handles {
+        results.push(handle.await.expect("task panicked"));
+    }
+
+    // Count successes and failures
+    let success_count = results.iter().filter(|r| r.is_ok()).count();
+    let failure_count = results.iter().filter(|r| r.is_err()).count();
+
+    assert_eq!(
+        success_count, 1,
+        "Expected exactly one successful melt, got {}. Results: {:?}",
+        success_count, results
+    );
+    assert_eq!(
+        failure_count,
+        NUM_WALLETS - 1,
+        "Expected {} failed melts, got {}",
+        NUM_WALLETS - 1,
+        failure_count
+    );
+
+    // Verify all failures were due to duplicate detection
+    for result in &results {
+        if let Err(err) = result {
+            let err_str = err.to_string().to_lowercase();
+            assert!(
+                err_str.contains("duplicate")
+                    || err_str.contains("already paid")
+                    || err_str.contains("pending"),
+                "Expected duplicate/already paid/pending error, got: {}",
+                err
+            );
+        }
+    }
+}
+
 /// Tests that wallet automatically recovers proofs after a failed swap operation
 #[tokio::test(flavor = "multi_thread", worker_threads = 1)]
 async fn test_wallet_proof_recovery_after_failed_swap() {

+ 17 - 8
crates/cdk-integration-tests/tests/happy_path_mint_wallet.rs

@@ -628,17 +628,26 @@ async fn test_pay_invoice_twice() {
 
     let melt = wallet.melt(&melt_quote.id).await.unwrap();
 
-    let melt_two = wallet.melt_quote(invoice, None).await;
+    // Creating a second quote for the same invoice is allowed
+    let melt_quote_two = wallet.melt_quote(invoice, None).await.unwrap();
+
+    // But attempting to melt (pay) the second quote should fail
+    // since the first quote with the same lookup_id is already paid
+    let melt_two = wallet.melt(&melt_quote_two.id).await;
 
     match melt_two {
-        Err(err) => match err {
-            cdk::Error::RequestAlreadyPaid => (),
-            err => {
-                if !err.to_string().contains("Duplicate entry") {
-                    panic!("Wrong invoice already paid: {}", err.to_string());
-                }
+        Err(err) => {
+            let err_str = err.to_string().to_lowercase();
+            if !err_str.contains("duplicate")
+                && !err_str.contains("already paid")
+                && !err_str.contains("request already paid")
+            {
+                panic!(
+                    "Expected duplicate/already paid error, got: {}",
+                    err.to_string()
+                );
             }
-        },
+        }
         Ok(_) => {
             panic!("Should not have allowed second payment");
         }

+ 15 - 0
crates/cdk-sql-common/src/mint/migrations/postgres/20251127000000_allow_duplicate_melt_request_lookup_id.sql

@@ -0,0 +1,15 @@
+-- Remove unique constraint on request_lookup_id for melt_quote
+-- This allows multiple melt quotes for the same payment request
+-- The constraint that only one can be PENDING or PAID at a time is enforced by a partial unique index
+
+-- Drop the unique index on request_lookup_id
+DROP INDEX IF EXISTS unique_request_lookup_id_melt;
+
+-- Create a non-unique index for lookup performance
+CREATE INDEX IF NOT EXISTS idx_melt_quote_request_lookup_id ON melt_quote(request_lookup_id);
+
+-- Create a partial unique index to enforce that only one quote per lookup_id can be PENDING or PAID
+-- This provides database-level enforcement of the constraint
+CREATE UNIQUE INDEX IF NOT EXISTS unique_pending_paid_lookup_id
+ON melt_quote(request_lookup_id)
+WHERE state IN ('PENDING', 'PAID');

+ 9 - 0
crates/cdk-sql-common/src/mint/migrations/sqlite/20251127000000_allow_duplicate_melt_request_lookup_id.sql

@@ -0,0 +1,9 @@
+-- Remove unique constraint on request_lookup_id for melt_quote
+-- This allows multiple melt quotes for the same payment request
+-- The constraint that only one can be pending at a time is enforced in application logic
+
+-- Drop the unique index on request_lookup_id
+DROP INDEX IF EXISTS unique_request_lookup_id_melt;
+
+-- Create a non-unique index for lookup performance
+CREATE INDEX IF NOT EXISTS idx_melt_quote_request_lookup_id ON melt_quote(request_lookup_id);

+ 41 - 2
crates/cdk-sql-common/src/mint/mod.rs

@@ -1239,11 +1239,9 @@ VALUES (:quote_id, :amount, :timestamp);
                 melt_quote
             WHERE
                 id=:id
-                AND state != :state
             "#,
         )?
         .bind("id", quote_id.to_string())
-        .bind("state", state.to_string())
         .fetch_one(&self.inner)
         .await?
         .map(sql_row_to_melt_quote)
@@ -1252,6 +1250,47 @@ VALUES (:quote_id, :amount, :timestamp);
 
         check_melt_quote_state_transition(quote.state, state)?;
 
+        // When transitioning to Pending, lock all quotes with the same lookup_id
+        // and check if any are already pending or paid
+        if state == MeltQuoteState::Pending {
+            if let Some(ref lookup_id) = quote.request_lookup_id {
+                // Lock all quotes with the same lookup_id to prevent race conditions
+                let locked_quotes: Vec<(String, String)> = query(
+                    r#"
+                    SELECT id, state
+                    FROM melt_quote
+                    WHERE request_lookup_id = :lookup_id
+                    FOR UPDATE
+                    "#,
+                )?
+                .bind("lookup_id", lookup_id.to_string())
+                .fetch_all(&self.inner)
+                .await?
+                .into_iter()
+                .map(|row| {
+                    unpack_into!(let (id, state) = row);
+                    Ok((column_as_string!(id), column_as_string!(state)))
+                })
+                .collect::<Result<Vec<_>, Error>>()?;
+
+                // Check if any other quote with the same lookup_id is pending or paid
+                let has_conflict = locked_quotes.iter().any(|(id, state)| {
+                    id != &quote_id.to_string()
+                        && (state == &MeltQuoteState::Pending.to_string()
+                            || state == &MeltQuoteState::Paid.to_string())
+                });
+
+                if has_conflict {
+                    tracing::warn!(
+                        "Cannot transition quote {} to Pending: another quote with lookup_id {} is already pending or paid",
+                        quote_id,
+                        lookup_id
+                    );
+                    return Err(Error::Duplicate);
+                }
+            }
+        }
+
         let rec = if state == MeltQuoteState::Paid {
             let current_time = unix_time();
             query(r#"UPDATE melt_quote SET state = :state, paid_time = :paid_time, payment_preimage = :payment_preimage WHERE id = :id"#)?

+ 12 - 5
crates/cdk/src/mint/melt/melt_saga/mod.rs

@@ -250,16 +250,23 @@ impl MeltSaga<Initial> {
             );
         }
 
+        // Update quote state to Pending
+        let (state, quote) = match tx
+            .update_melt_quote_state(melt_request.quote(), MeltQuoteState::Pending, None)
+            .await
+        {
+            Ok(result) => result,
+            Err(err) => {
+                tx.rollback().await?;
+                return Err(err.into());
+            }
+        };
+
         // Publish proof state changes
         for pk in input_ys.iter() {
             self.pubsub.proof_state((*pk, State::Pending));
         }
 
-        // Update quote state to Pending
-        let (state, quote) = tx
-            .update_melt_quote_state(melt_request.quote(), MeltQuoteState::Pending, None)
-            .await?;
-
         if input_unit != Some(quote.unit.clone()) {
             tx.rollback().await?;
             return Err(Error::UnitMismatch);

+ 368 - 0
crates/cdk/src/mint/melt/melt_saga/tests.rs

@@ -2496,3 +2496,371 @@ async fn assert_proofs_state(
         assert_eq!(state, expected_state, "Proof state mismatch");
     }
 }
+
+// ============================================================================
+// Duplicate request_lookup_id Constraint Tests
+// ============================================================================
+
+/// Test: Cannot set melt quote to pending if another quote with same lookup_id is already pending
+///
+/// This test verifies that when two melt quotes share the same request_lookup_id,
+/// only one can be in PENDING state at a time.
+#[tokio::test]
+async fn test_duplicate_lookup_id_prevents_second_pending() {
+    use cdk_common::melt::MeltQuoteRequest;
+    use cdk_common::nuts::MeltQuoteBolt11Request;
+    use cdk_common::CurrencyUnit;
+    use cdk_fake_wallet::{create_fake_invoice, FakeInvoiceDescription};
+
+    // STEP 1: Setup test environment
+    let mint = create_test_mint().await.unwrap();
+
+    // Create a fake invoice description
+    let fake_description = FakeInvoiceDescription {
+        pay_invoice_state: MeltQuoteState::Paid,
+        check_payment_state: MeltQuoteState::Paid,
+        pay_err: false,
+        check_err: false,
+    };
+
+    // Create a single invoice that will be used for both quotes
+    let amount_msats: u64 = 9000;
+    let invoice = create_fake_invoice(
+        amount_msats,
+        serde_json::to_string(&fake_description).unwrap(),
+    );
+
+    // STEP 2: Create two melt quotes for the same invoice (same request_lookup_id)
+    let bolt11_request1 = MeltQuoteBolt11Request {
+        request: invoice.clone(),
+        unit: CurrencyUnit::Sat,
+        options: None,
+    };
+    let quote_response1 = mint
+        .get_melt_quote(MeltQuoteRequest::Bolt11(bolt11_request1))
+        .await
+        .unwrap();
+
+    let bolt11_request2 = MeltQuoteBolt11Request {
+        request: invoice,
+        unit: CurrencyUnit::Sat,
+        options: None,
+    };
+    let quote_response2 = mint
+        .get_melt_quote(MeltQuoteRequest::Bolt11(bolt11_request2))
+        .await
+        .unwrap();
+
+    // Retrieve full quotes
+    let quote1 = mint
+        .localstore
+        .get_melt_quote(&quote_response1.quote)
+        .await
+        .unwrap()
+        .expect("Quote 1 should exist");
+    let quote2 = mint
+        .localstore
+        .get_melt_quote(&quote_response2.quote)
+        .await
+        .unwrap()
+        .expect("Quote 2 should exist");
+
+    // Verify both quotes have the same lookup_id
+    assert_eq!(
+        quote1.request_lookup_id, quote2.request_lookup_id,
+        "Both quotes should have the same request_lookup_id"
+    );
+
+    // STEP 3: Setup first melt saga (puts quote1 in PENDING state)
+    let proofs1 = mint_test_proofs(&mint, Amount::from(10_000)).await.unwrap();
+    let melt_request1 = create_test_melt_request(&proofs1, &quote1);
+
+    let verification1 = mint.verify_inputs(melt_request1.inputs()).await.unwrap();
+    let saga1 = MeltSaga::new(
+        std::sync::Arc::new(mint.clone()),
+        mint.localstore(),
+        mint.pubsub_manager(),
+    );
+    let setup_saga1 = saga1
+        .setup_melt(&melt_request1, verification1)
+        .await
+        .unwrap();
+
+    // Continue through the payment flow to release any transaction locks
+    // The quote will stay in PENDING state because FakeWallet returns Paid
+    // but we don't call finalize()
+    let (payment_saga1, decision1) = setup_saga1
+        .attempt_internal_settlement(&melt_request1)
+        .await
+        .unwrap();
+
+    // Make payment but don't finalize - keeps quote in PENDING
+    let confirmed_saga1 = payment_saga1.make_payment(decision1).await.unwrap();
+
+    // Drop the saga to release resources (simulates crash before finalize)
+    drop(confirmed_saga1);
+
+    // Verify quote1 is now pending
+    let pending_quote1 = mint
+        .localstore
+        .get_melt_quote(&quote1.id)
+        .await
+        .unwrap()
+        .unwrap();
+    assert_eq!(
+        pending_quote1.state,
+        MeltQuoteState::Pending,
+        "Quote 1 should be pending"
+    );
+
+    // STEP 4: Try to setup second saga with quote2 (same lookup_id)
+    let proofs2 = mint_test_proofs(&mint, Amount::from(10_000)).await.unwrap();
+    let melt_request2 = create_test_melt_request(&proofs2, &quote2);
+
+    let verification2 = mint.verify_inputs(melt_request2.inputs()).await.unwrap();
+    let saga2 = MeltSaga::new(
+        std::sync::Arc::new(mint.clone()),
+        mint.localstore(),
+        mint.pubsub_manager(),
+    );
+    let setup_result2 = saga2.setup_melt(&melt_request2, verification2).await;
+
+    // STEP 5: Verify second setup fails due to duplicate pending lookup_id
+    assert!(
+        setup_result2.is_err(),
+        "Setup should fail when another quote with same lookup_id is already pending"
+    );
+
+    if let Err(error) = setup_result2 {
+        let error_msg = error.to_string().to_lowercase();
+        assert!(
+            error_msg.contains("duplicate") || error_msg.contains("pending"),
+            "Error should mention duplicate or pending, got: {}",
+            error
+        );
+    }
+
+    // Verify quote2 is still unpaid
+    let still_unpaid_quote2 = mint
+        .localstore
+        .get_melt_quote(&quote2.id)
+        .await
+        .unwrap()
+        .unwrap();
+    assert_eq!(
+        still_unpaid_quote2.state,
+        MeltQuoteState::Unpaid,
+        "Quote 2 should still be unpaid"
+    );
+
+    // SUCCESS: Duplicate pending lookup_id prevented!
+}
+
+/// Test: Cannot set melt quote to pending if another quote with same lookup_id is already paid
+///
+/// This test verifies that once a melt quote with a specific request_lookup_id is paid,
+/// no other quote with the same lookup_id can transition to pending.
+#[tokio::test]
+async fn test_paid_lookup_id_prevents_pending() {
+    use cdk_common::melt::MeltQuoteRequest;
+    use cdk_common::nuts::MeltQuoteBolt11Request;
+    use cdk_common::CurrencyUnit;
+    use cdk_fake_wallet::{create_fake_invoice, FakeInvoiceDescription};
+
+    // STEP 1: Setup test environment
+    let mint = create_test_mint().await.unwrap();
+
+    // Create a fake invoice description
+    let fake_description = FakeInvoiceDescription {
+        pay_invoice_state: MeltQuoteState::Paid,
+        check_payment_state: MeltQuoteState::Paid,
+        pay_err: false,
+        check_err: false,
+    };
+
+    // Create a single invoice that will be used for both quotes
+    let amount_msats: u64 = 9000;
+    let invoice = create_fake_invoice(
+        amount_msats,
+        serde_json::to_string(&fake_description).unwrap(),
+    );
+
+    // STEP 2: Create two melt quotes for the same invoice (same request_lookup_id)
+    let bolt11_request1 = MeltQuoteBolt11Request {
+        request: invoice.clone(),
+        unit: CurrencyUnit::Sat,
+        options: None,
+    };
+    let quote_response1 = mint
+        .get_melt_quote(MeltQuoteRequest::Bolt11(bolt11_request1))
+        .await
+        .unwrap();
+
+    let bolt11_request2 = MeltQuoteBolt11Request {
+        request: invoice,
+        unit: CurrencyUnit::Sat,
+        options: None,
+    };
+    let quote_response2 = mint
+        .get_melt_quote(MeltQuoteRequest::Bolt11(bolt11_request2))
+        .await
+        .unwrap();
+
+    // Retrieve full quotes
+    let quote1 = mint
+        .localstore
+        .get_melt_quote(&quote_response1.quote)
+        .await
+        .unwrap()
+        .expect("Quote 1 should exist");
+    let quote2 = mint
+        .localstore
+        .get_melt_quote(&quote_response2.quote)
+        .await
+        .unwrap()
+        .expect("Quote 2 should exist");
+
+    // STEP 3: Complete the first melt (marks quote1 as PAID)
+    let proofs1 = mint_test_proofs(&mint, Amount::from(10_000)).await.unwrap();
+    let melt_request1 = create_test_melt_request(&proofs1, &quote1);
+
+    let verification1 = mint.verify_inputs(melt_request1.inputs()).await.unwrap();
+    let saga1 = MeltSaga::new(
+        std::sync::Arc::new(mint.clone()),
+        mint.localstore(),
+        mint.pubsub_manager(),
+    );
+    let setup_saga1 = saga1
+        .setup_melt(&melt_request1, verification1)
+        .await
+        .unwrap();
+
+    // Complete the full melt flow for quote1
+    let (payment_saga, decision) = setup_saga1
+        .attempt_internal_settlement(&melt_request1)
+        .await
+        .unwrap();
+    let confirmed_saga = payment_saga.make_payment(decision).await.unwrap();
+    let _response = confirmed_saga.finalize().await.unwrap();
+
+    // Verify quote1 is now paid
+    let paid_quote1 = mint
+        .localstore
+        .get_melt_quote(&quote1.id)
+        .await
+        .unwrap()
+        .unwrap();
+    assert_eq!(
+        paid_quote1.state,
+        MeltQuoteState::Paid,
+        "Quote 1 should be paid"
+    );
+
+    // STEP 4: Try to setup second saga with quote2 (same lookup_id as paid quote)
+    let proofs2 = mint_test_proofs(&mint, Amount::from(10_000)).await.unwrap();
+    let melt_request2 = create_test_melt_request(&proofs2, &quote2);
+
+    let verification2 = mint.verify_inputs(melt_request2.inputs()).await.unwrap();
+    let saga2 = MeltSaga::new(
+        std::sync::Arc::new(mint.clone()),
+        mint.localstore(),
+        mint.pubsub_manager(),
+    );
+    let setup_result2 = saga2.setup_melt(&melt_request2, verification2).await;
+
+    // STEP 5: Verify second setup fails due to already paid lookup_id
+    assert!(
+        setup_result2.is_err(),
+        "Setup should fail when another quote with same lookup_id is already paid"
+    );
+
+    if let Err(error) = setup_result2 {
+        let error_msg = error.to_string().to_lowercase();
+        assert!(
+            error_msg.contains("duplicate")
+                || error_msg.contains("paid")
+                || error_msg.contains("pending"),
+            "Error should mention duplicate or paid, got: {}",
+            error
+        );
+    }
+
+    // SUCCESS: Paid lookup_id prevents new pending!
+}
+
+/// Test: Different lookup_ids allow concurrent pending quotes
+///
+/// This test verifies that melt quotes with different request_lookup_ids
+/// can both be in PENDING state simultaneously.
+#[tokio::test]
+async fn test_different_lookup_ids_allow_concurrent_pending() {
+    // STEP 1: Setup test environment
+    let mint = create_test_mint().await.unwrap();
+
+    // STEP 2: Create two quotes with different lookup_ids (different invoices)
+    let quote1 = create_test_melt_quote(&mint, Amount::from(9_000)).await;
+    let quote2 = create_test_melt_quote(&mint, Amount::from(8_000)).await;
+
+    // Verify quotes have different lookup_ids
+    assert_ne!(
+        quote1.request_lookup_id, quote2.request_lookup_id,
+        "Quotes should have different request_lookup_ids"
+    );
+
+    // STEP 3: Setup first saga (puts quote1 in PENDING state)
+    let proofs1 = mint_test_proofs(&mint, Amount::from(10_000)).await.unwrap();
+    let melt_request1 = create_test_melt_request(&proofs1, &quote1);
+
+    let verification1 = mint.verify_inputs(melt_request1.inputs()).await.unwrap();
+    let saga1 = MeltSaga::new(
+        std::sync::Arc::new(mint.clone()),
+        mint.localstore(),
+        mint.pubsub_manager(),
+    );
+    let _setup_saga1 = saga1
+        .setup_melt(&melt_request1, verification1)
+        .await
+        .unwrap();
+
+    // STEP 4: Setup second saga (puts quote2 in PENDING state)
+    let proofs2 = mint_test_proofs(&mint, Amount::from(10_000)).await.unwrap();
+    let melt_request2 = create_test_melt_request(&proofs2, &quote2);
+
+    let verification2 = mint.verify_inputs(melt_request2.inputs()).await.unwrap();
+    let saga2 = MeltSaga::new(
+        std::sync::Arc::new(mint.clone()),
+        mint.localstore(),
+        mint.pubsub_manager(),
+    );
+    let _setup_saga2 = saga2
+        .setup_melt(&melt_request2, verification2)
+        .await
+        .unwrap();
+
+    // STEP 5: Verify both quotes are pending
+    let pending_quote1 = mint
+        .localstore
+        .get_melt_quote(&quote1.id)
+        .await
+        .unwrap()
+        .unwrap();
+    let pending_quote2 = mint
+        .localstore
+        .get_melt_quote(&quote2.id)
+        .await
+        .unwrap()
+        .unwrap();
+
+    assert_eq!(
+        pending_quote1.state,
+        MeltQuoteState::Pending,
+        "Quote 1 should be pending"
+    );
+    assert_eq!(
+        pending_quote2.state,
+        MeltQuoteState::Pending,
+        "Quote 2 should be pending"
+    );
+
+    // SUCCESS: Different lookup_ids allow concurrent pending!
+}