Эх сурвалжийг харах

Simplify increment_issued_quote implementation

Fixes #1066

Removed explicit overflow and over-issuance checks from application code

Refactors the increment_issued_quote method to perform atomic SQL update
instead of read-modify-write pattern. This simplifies the code by using SQL's
atomic increment operation (amount_issued + :amount_issued) rather than
fetching, calculating, and updating separately.
Cesar Rodas 2 сар өмнө
parent
commit
910718f87f

+ 0 - 129
crates/cdk-common/src/database/mint/test/mint.rs

@@ -281,135 +281,6 @@ where
     assert_eq!(mint_quote_from_db.payments.len(), 1);
 }
 
-/// Reject over issue in same tx
-pub async fn reject_over_issue_same_tx<DB>(db: DB)
-where
-    DB: Database<Error> + KeysDatabase<Err = Error>,
-{
-    let mint_quote = MintQuote::new(
-        None,
-        "".to_owned(),
-        cashu::CurrencyUnit::Sat,
-        None,
-        0,
-        PaymentIdentifier::CustomId(unique_string()),
-        None,
-        0.into(),
-        0.into(),
-        cashu::PaymentMethod::Bolt12,
-        0,
-        vec![],
-        vec![],
-    );
-
-    let mut tx = Database::begin_transaction(&db).await.unwrap();
-    tx.add_mint_quote(mint_quote.clone()).await.unwrap();
-    assert!(tx
-        .increment_mint_quote_amount_issued(&mint_quote.id, 100.into())
-        .await
-        .is_err());
-}
-
-/// Reject over issue
-pub async fn reject_over_issue_different_tx<DB>(db: DB)
-where
-    DB: Database<Error> + KeysDatabase<Err = Error>,
-{
-    let mint_quote = MintQuote::new(
-        None,
-        "".to_owned(),
-        cashu::CurrencyUnit::Sat,
-        None,
-        0,
-        PaymentIdentifier::CustomId(unique_string()),
-        None,
-        0.into(),
-        0.into(),
-        cashu::PaymentMethod::Bolt12,
-        0,
-        vec![],
-        vec![],
-    );
-
-    let mut tx = Database::begin_transaction(&db).await.unwrap();
-    tx.add_mint_quote(mint_quote.clone()).await.unwrap();
-    tx.commit().await.unwrap();
-
-    let mut tx = Database::begin_transaction(&db).await.unwrap();
-    assert!(tx
-        .increment_mint_quote_amount_issued(&mint_quote.id, 100.into())
-        .await
-        .is_err());
-}
-
-/// Reject over issue with payment
-pub async fn reject_over_issue_with_payment<DB>(db: DB)
-where
-    DB: Database<Error> + KeysDatabase<Err = Error>,
-{
-    let mint_quote = MintQuote::new(
-        None,
-        "".to_owned(),
-        cashu::CurrencyUnit::Sat,
-        None,
-        0,
-        PaymentIdentifier::CustomId(unique_string()),
-        None,
-        0.into(),
-        0.into(),
-        cashu::PaymentMethod::Bolt12,
-        0,
-        vec![],
-        vec![],
-    );
-
-    let p1 = unique_string();
-    let mut tx = Database::begin_transaction(&db).await.unwrap();
-    tx.add_mint_quote(mint_quote.clone()).await.unwrap();
-    tx.increment_mint_quote_amount_paid(&mint_quote.id, 100.into(), p1.clone())
-        .await
-        .unwrap();
-    assert!(tx
-        .increment_mint_quote_amount_issued(&mint_quote.id, 101.into())
-        .await
-        .is_err());
-}
-
-/// Reject over issue with payment
-pub async fn reject_over_issue_with_payment_different_tx<DB>(db: DB)
-where
-    DB: Database<Error> + KeysDatabase<Err = Error>,
-{
-    let mint_quote = MintQuote::new(
-        None,
-        "".to_owned(),
-        cashu::CurrencyUnit::Sat,
-        None,
-        0,
-        PaymentIdentifier::CustomId(unique_string()),
-        None,
-        0.into(),
-        0.into(),
-        cashu::PaymentMethod::Bolt12,
-        0,
-        vec![],
-        vec![],
-    );
-
-    let p1 = unique_string();
-    let mut tx = Database::begin_transaction(&db).await.unwrap();
-    tx.add_mint_quote(mint_quote.clone()).await.unwrap();
-    tx.increment_mint_quote_amount_paid(&mint_quote.id, 100.into(), p1.clone())
-        .await
-        .unwrap();
-    tx.commit().await.unwrap();
-
-    let mut tx = Database::begin_transaction(&db).await.unwrap();
-    assert!(tx
-        .increment_mint_quote_amount_issued(&mint_quote.id, 101.into())
-        .await
-        .is_err());
-}
 /// Successful melt with unique blinded messages
 pub async fn add_melt_request_unique_blinded_messages<DB>(db: DB)
 where

+ 0 - 4
crates/cdk-common/src/database/mint/test/mod.rs

@@ -239,10 +239,6 @@ macro_rules! mint_db_test {
             get_proofs_by_keyset_id,
             reject_duplicate_payments_same_tx,
             reject_duplicate_payments_diff_tx,
-            reject_over_issue_same_tx,
-            reject_over_issue_different_tx,
-            reject_over_issue_with_payment,
-            reject_over_issue_with_payment_different_tx,
             add_melt_request_unique_blinded_messages,
             reject_melt_duplicate_blinded_signature,
             reject_duplicate_blinded_message_db_constraint,

+ 20 - 42
crates/cdk-sql-common/src/mint/mod.rs

@@ -1045,62 +1045,38 @@ where
         quote_id: &QuoteId,
         amount_issued: Amount,
     ) -> Result<Amount, Self::Err> {
-        // Get current amount_issued from quote
-        let current_amounts = query(
+        // Update the amount_issued
+        query(
             r#"
-            SELECT amount_issued, amount_paid
-            FROM mint_quote
+            UPDATE mint_quote
+            SET amount_issued = amount_issued + :amount_issued
             WHERE id = :quote_id
-            FOR UPDATE
             "#,
         )?
+        .bind("amount_issued", amount_issued.to_i64())
         .bind("quote_id", quote_id.to_string())
-        .fetch_one(&self.inner)
+        .execute(&self.inner)
         .await
         .inspect_err(|err| {
-            tracing::error!("SQLite could not get mint quote amount_issued: {}", err);
-        })?
-        .ok_or(Error::QuoteNotFound)?;
-
-        let new_amount_issued = {
-            // Make sure the db protects issuing not paid quotes
-            unpack_into!(
-                let (current_amount_issued, current_amount_paid) = current_amounts
-            );
-
-            let current_amount_issued: u64 = column_as_number!(current_amount_issued);
-            let current_amount_paid: u64 = column_as_number!(current_amount_paid);
-
-            let current_amount_issued = Amount::from(current_amount_issued);
-            let current_amount_paid = Amount::from(current_amount_paid);
-
-            // Calculate new amount_issued with overflow check
-            let new_amount_issued = current_amount_issued
-                .checked_add(amount_issued)
-                .ok_or_else(|| database::Error::AmountOverflow)?;
-
-            current_amount_paid
-                .checked_sub(new_amount_issued)
-                .ok_or(Error::Internal("Over-issued not allowed".to_owned()))?;
-
-            new_amount_issued
-        };
+            tracing::error!("SQLite could not update mint quote amount_issued: {}", err);
+        })?;
 
-        // Update the amount_issued
-        query(
+        // Get current amount_issued from quote
+        let current_amount_issued = query(
             r#"
-            UPDATE mint_quote
-            SET amount_issued = :amount_issued
+            SELECT amount_issued
+            FROM mint_quote
             WHERE id = :quote_id
+            FOR UPDATE
             "#,
         )?
-        .bind("amount_issued", new_amount_issued.to_i64())
         .bind("quote_id", quote_id.to_string())
-        .execute(&self.inner)
+        .pluck(&self.inner)
         .await
         .inspect_err(|err| {
-            tracing::error!("SQLite could not update mint quote amount_issued: {}", err);
-        })?;
+            tracing::error!("SQLite could not get mint quote amount_issued: {}", err);
+        })?
+        .ok_or(Error::QuoteNotFound)?;
 
         let current_time = unix_time();
 
@@ -1117,7 +1093,9 @@ VALUES (:quote_id, :amount, :timestamp);
         .execute(&self.inner)
         .await?;
 
-        Ok(new_amount_issued)
+        let current_amount_issued: u64 = column_as_number!(current_amount_issued);
+
+        Ok(Amount::from(current_amount_issued))
     }
 
     #[instrument(skip_all)]