Переглянути джерело

Delete melt and swap saga in compensation instead of separate tx (#1389)

* test: add melt saga deleted tests

* fix: delete melt saga in compensation

* feat: delete swap saga in compensation
tsk 1 місяць тому
батько
коміт
4bf727eac6

+ 33 - 0
crates/cdk-sql-common/src/mint/mod.rs

@@ -258,6 +258,39 @@ where
         .await?;
 
         if total_deleted != ys.len() {
+            // Query current states to provide detailed logging
+            let current_states = get_current_states(&self.inner, ys).await?;
+
+            let missing_count = ys.len() - current_states.len();
+            let spent_count = current_states
+                .values()
+                .filter(|s| **s == State::Spent)
+                .count();
+
+            if missing_count > 0 {
+                tracing::warn!(
+                    "remove_proofs: {} of {} proofs do not exist in database (already removed?)",
+                    missing_count,
+                    ys.len()
+                );
+            }
+
+            if spent_count > 0 {
+                tracing::warn!(
+                    "remove_proofs: {} of {} proofs are in Spent state and cannot be removed",
+                    spent_count,
+                    ys.len()
+                );
+            }
+
+            tracing::debug!(
+                "remove_proofs details: requested={}, deleted={}, missing={}, spent={}",
+                ys.len(),
+                total_deleted,
+                missing_count,
+                spent_count
+            );
+
             return Err(Self::Err::AttemptRemoveSpentProof);
         }
 

+ 8 - 2
crates/cdk/src/mint/melt/melt_saga/compensation.rs

@@ -7,6 +7,7 @@ use async_trait::async_trait;
 use cdk_common::database::DynMintDatabase;
 use cdk_common::{Error, PublicKey, QuoteId};
 use tracing::instrument;
+use uuid::Uuid;
 
 /// Trait for compensating actions in the saga pattern.
 ///
@@ -25,6 +26,7 @@ pub trait CompensatingAction: Send + Sync {
 /// - Input proofs (identified by input_ys)
 /// - Output blinded messages (identified by blinded_secrets)
 /// - Melt request tracking record
+/// - Saga state record
 ///
 ///   And resets:
 /// - Quote state from Pending back to Unpaid
@@ -37,6 +39,8 @@ pub struct RemoveMeltSetup {
     pub blinded_secrets: Vec<PublicKey>,
     /// Quote ID to reset state
     pub quote_id: QuoteId,
+    /// Operation ID (saga ID) to delete
+    pub operation_id: Uuid,
 }
 
 #[async_trait]
@@ -44,10 +48,11 @@ impl CompensatingAction for RemoveMeltSetup {
     #[instrument(skip_all)]
     async fn execute(&self, db: &DynMintDatabase) -> Result<(), Error> {
         tracing::info!(
-            "Compensation: Removing melt setup for quote {} ({} proofs, {} blinded messages)",
+            "Compensation: Removing melt setup for quote {} ({} proofs, {} blinded messages, saga {})",
             self.quote_id,
             self.input_ys.len(),
-            self.blinded_secrets.len()
+            self.blinded_secrets.len(),
+            self.operation_id
         );
 
         super::super::shared::rollback_melt_quote(
@@ -55,6 +60,7 @@ impl CompensatingAction for RemoveMeltSetup {
             &self.quote_id,
             &self.input_ys,
             &self.blinded_secrets,
+            &self.operation_id,
         )
         .await
     }

+ 1 - 0
crates/cdk/src/mint/melt/melt_saga/mod.rs

@@ -385,6 +385,7 @@ impl MeltSaga<Initial> {
                 input_ys: input_ys.clone(),
                 blinded_secrets,
                 quote_id: quote.id.clone(),
+                operation_id: *self.operation.id(),
             }));
 
         // Transition to SetupComplete state

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

@@ -1012,6 +1012,107 @@ async fn test_compensation_idempotent() {
     // SUCCESS: Compensation is idempotent and safe to run multiple times!
 }
 
+/// Test: Saga is deleted after direct payment failure (not via recovery)
+///
+/// This test validates that when make_payment() fails and compensate_all()
+/// is called directly during normal operation (not through crash recovery),
+/// the saga is properly deleted from the database.
+///
+/// This is different from the recovery tests which simulate crashes - this
+/// tests the actual payment failure path in normal operation.
+#[tokio::test]
+async fn test_saga_deleted_after_payment_failure() {
+    use cdk_common::CurrencyUnit;
+    use cdk_fake_wallet::{create_fake_invoice, FakeInvoiceDescription};
+
+    // STEP 1: Setup test environment
+    let mint = create_test_mint().await.unwrap();
+    let proofs = mint_test_proofs(&mint, Amount::from(10_000)).await.unwrap();
+    let input_ys = proofs.ys().unwrap();
+
+    // STEP 2: Create a quote that will FAIL payment
+    let fake_description = FakeInvoiceDescription {
+        pay_invoice_state: MeltQuoteState::Failed, // Payment will fail
+        check_payment_state: MeltQuoteState::Failed, // Check will also show failed
+        pay_err: false,
+        check_err: false,
+    };
+
+    let amount_msats: u64 = Amount::from(9_000).into();
+    let invoice = create_fake_invoice(
+        amount_msats,
+        serde_json::to_string(&fake_description).unwrap(),
+    );
+
+    let bolt11_request = cdk_common::nuts::MeltQuoteBolt11Request {
+        request: invoice,
+        unit: CurrencyUnit::Sat,
+        options: None,
+    };
+
+    let request = cdk_common::melt::MeltQuoteRequest::Bolt11(bolt11_request);
+    let quote_response = mint.get_melt_quote(request).await.unwrap();
+    let quote = mint
+        .localstore
+        .get_melt_quote(&quote_response.quote)
+        .await
+        .unwrap()
+        .expect("Quote should exist");
+
+    let melt_request = create_test_melt_request(&proofs, &quote);
+
+    // STEP 3: Setup melt saga
+    let verification = mint.verify_inputs(melt_request.inputs()).await.unwrap();
+    let saga = MeltSaga::new(
+        std::sync::Arc::new(mint.clone()),
+        mint.localstore(),
+        mint.pubsub_manager(),
+    );
+    let setup_saga = saga.setup_melt(&melt_request, verification).await.unwrap();
+    let operation_id = *setup_saga.operation.id();
+
+    // Verify saga exists after setup
+    assert_saga_exists(&mint, &operation_id).await;
+    assert_proofs_state(&mint, &input_ys, Some(State::Pending)).await;
+
+    // STEP 4: Attempt internal settlement (will return RequiresExternalPayment)
+    let (payment_saga, decision) = setup_saga
+        .attempt_internal_settlement(&melt_request)
+        .await
+        .unwrap();
+
+    // STEP 5: Make payment - this should FAIL and trigger compensate_all()
+    let result = payment_saga.make_payment(decision).await;
+
+    // Payment should fail
+    assert!(
+        result.is_err(),
+        "Payment should fail with Failed status from FakeWallet"
+    );
+
+    // STEP 6: Verify saga was deleted after compensation
+    // This is the key assertion - the saga should be cleaned up after compensate_all()
+    assert_saga_not_exists(&mint, &operation_id).await;
+
+    // STEP 7: Verify proofs were returned (removed from database)
+    assert_proofs_state(&mint, &input_ys, None).await;
+
+    // STEP 8: Verify quote state was reset to Unpaid
+    let recovered_quote = mint
+        .localstore
+        .get_melt_quote(&quote.id)
+        .await
+        .unwrap()
+        .expect("Quote should still exist");
+    assert_eq!(
+        recovered_quote.state,
+        MeltQuoteState::Unpaid,
+        "Quote state should be reset to Unpaid after compensation"
+    );
+
+    // SUCCESS: Saga properly deleted after direct payment failure!
+}
+
 // ============================================================================
 // Saga Content Validation Tests
 // ============================================================================

+ 19 - 3
crates/cdk/src/mint/melt/shared.rs

@@ -76,16 +76,18 @@ pub async fn rollback_melt_quote(
     quote_id: &QuoteId,
     input_ys: &[PublicKey],
     blinded_secrets: &[PublicKey],
+    operation_id: &uuid::Uuid,
 ) -> Result<(), Error> {
     if input_ys.is_empty() && blinded_secrets.is_empty() {
         return Ok(());
     }
 
     tracing::info!(
-        "Rolling back melt quote {} ({} proofs, {} blinded messages)",
+        "Rolling back melt quote {} ({} proofs, {} blinded messages, saga {})",
         quote_id,
         input_ys.len(),
-        blinded_secrets.len()
+        blinded_secrets.len(),
+        operation_id
     );
 
     let mut tx = db.begin_transaction().await?;
@@ -115,9 +117,23 @@ pub async fn rollback_melt_quote(
     // Delete melt request tracking record
     tx.delete_melt_request(quote_id).await?;
 
+    // Delete saga state record
+    if let Err(e) = tx.delete_saga(operation_id).await {
+        tracing::warn!(
+            "Failed to delete saga {} during rollback: {}",
+            operation_id,
+            e
+        );
+        // Continue anyway - saga cleanup is best-effort
+    }
+
     tx.commit().await?;
 
-    tracing::info!("Successfully rolled back melt quote {}", quote_id);
+    tracing::info!(
+        "Successfully rolled back melt quote {} and deleted saga {}",
+        quote_id,
+        operation_id
+    );
 
     Ok(())
 }

+ 3 - 10
crates/cdk/src/mint/start_up_check.rs

@@ -126,12 +126,14 @@ impl Mint {
             );
 
             // Use the same compensation logic as in-process failures
+            // Saga deletion is included in the compensation transaction
             let compensation = RemoveSwapSetup {
                 blinded_secrets: saga.blinded_secrets.clone(),
                 input_ys: saga.input_ys.clone(),
+                operation_id: saga.operation_id,
             };
 
-            // Execute compensation
+            // Execute compensation (includes saga deletion)
             if let Err(e) = compensation.execute(&self.localstore).await {
                 tracing::error!(
                     "Failed to compensate saga {}: {}. Continuing...",
@@ -141,15 +143,6 @@ impl Mint {
                 continue;
             }
 
-            // Delete saga after successful compensation
-            let mut tx = self.localstore.begin_transaction().await?;
-            if let Err(e) = tx.delete_saga(&saga.operation_id).await {
-                tracing::error!("Failed to delete saga for {}: {}", saga.operation_id, e);
-                tx.rollback().await?;
-                continue;
-            }
-            tx.commit().await?;
-
             tracing::info!("Successfully recovered saga {}", saga.operation_id);
         }
 

+ 22 - 2
crates/cdk/src/mint/swap/swap_saga/compensation.rs

@@ -2,6 +2,7 @@ use async_trait::async_trait;
 use cdk_common::database::DynMintDatabase;
 use cdk_common::{Error, PublicKey};
 use tracing::instrument;
+use uuid::Uuid;
 
 #[async_trait]
 pub trait CompensatingAction: Send + Sync {
@@ -15,6 +16,7 @@ pub trait CompensatingAction: Send + Sync {
 /// the setup transaction has committed. It removes:
 /// - Output blinded messages (identified by blinded_secrets)
 /// - Input proofs (identified by input_ys)
+/// - Saga state record
 ///
 /// This restores the database to its pre-swap state.
 pub struct RemoveSwapSetup {
@@ -22,6 +24,8 @@ pub struct RemoveSwapSetup {
     pub blinded_secrets: Vec<PublicKey>,
     /// Y values (public keys) from the input proofs
     pub input_ys: Vec<PublicKey>,
+    /// Operation ID (saga ID) to delete
+    pub operation_id: Uuid,
 }
 
 #[async_trait]
@@ -33,9 +37,10 @@ impl CompensatingAction for RemoveSwapSetup {
         }
 
         tracing::info!(
-            "Compensation: Removing swap setup ({} blinded messages, {} proofs)",
+            "Compensation: Removing swap setup ({} blinded messages, {} proofs, saga {})",
             self.blinded_secrets.len(),
-            self.input_ys.len()
+            self.input_ys.len(),
+            self.operation_id
         );
 
         let mut tx = db.begin_transaction().await?;
@@ -50,8 +55,23 @@ impl CompensatingAction for RemoveSwapSetup {
             tx.remove_proofs(&self.input_ys, None).await?;
         }
 
+        // Delete saga state record
+        if let Err(e) = tx.delete_saga(&self.operation_id).await {
+            tracing::warn!(
+                "Failed to delete saga {} during compensation: {}",
+                self.operation_id,
+                e
+            );
+            // Continue anyway - saga cleanup is best-effort
+        }
+
         tx.commit().await?;
 
+        tracing::info!(
+            "Successfully compensated swap and deleted saga {}",
+            self.operation_id
+        );
+
         Ok(())
     }
 

+ 1 - 21
crates/cdk/src/mint/swap/swap_saga/mod.rs

@@ -256,6 +256,7 @@ impl<'a> SwapSaga<'a, Initial> {
             .push_front(Box::new(RemoveSwapSetup {
                 blinded_secrets: blinded_secrets.clone(),
                 input_ys: ys.clone(),
+                operation_id: *self.operation.id(),
             }));
 
         // Transition to SetupComplete state
@@ -488,27 +489,6 @@ impl<S> SwapSaga<'_, S> {
             }
         }
 
-        // Delete saga - swap was compensated
-        // Use a separate transaction since compensations already ran
-        // Don't fail the compensation if saga cleanup fails (log only)
-        let mut tx = match self.db.begin_transaction().await {
-            Ok(tx) => tx,
-            Err(e) => {
-                tracing::error!(
-                    "Failed to begin tx for saga cleanup after compensation: {}",
-                    e
-                );
-                return Ok(()); // Compensations already ran, don't fail now
-            }
-        };
-
-        if let Err(e) = tx.delete_saga(self.operation.id()).await {
-            tracing::warn!("Failed to delete saga after compensation: {}", e);
-        } else if let Err(e) = tx.commit().await {
-            tracing::error!("Failed to commit saga cleanup after compensation: {}", e);
-        }
-        // Always succeed - compensations are done, saga cleanup is best-effort
-
         Ok(())
     }
 }