Reorganize tests, add mint quote/payment coverage, and prevent over-issuing (#1048)

* Add consistent ordering of sql migrations

Also sort the prefix and not only the filenames

* Reorganize tests, add mint quote/payment coverage, and prevent over-issuing

Reorganizes the mint test suite into clear modules, adds comprehensive mint
quote & payment scenarios, enhances the shared test macro, and hardens SQL
logic to forbid issuing more than what’s been paid.

These tests were added:

* Add quote once; reject duplicates.
* Register multiple payments and verify aggregated amount_paid.
* Read parity between DB and in-TX views.
* Reject duplicate payment_id in same and different transactions.
* Reject over-issuing (same TX, different TX, with/without prior payments).

---------

Co-authored-by: thesimplekid <tsk@thesimplekid.com>

crates/cdk-common/src/database/mint/mod.rs

@@ -80,9 +80,6 @@ mod auth;
 #[cfg(feature = "test")]
 pub mod test;
 
-#[cfg(test)]
-mod test_kvstore;
-
 #[cfg(feature = "auth")]
 pub use auth::{MintAuthDatabase, MintAuthTransaction};
 

crates/cdk-common/src/database/mint/test/mint.rs

@@ -0,0 +1,406 @@
+//! Payments
+
+use crate::database::mint::test::unique_string;
+use crate::database::mint::{Database, Error, KeysDatabase};
+use crate::mint::MintQuote;
+use crate::payment::PaymentIdentifier;
+
+/// Add a mint quote
+pub async fn add_mint_quote<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();
+    assert!(tx.add_mint_quote(mint_quote.clone()).await.is_ok());
+    tx.commit().await.unwrap();
+}
+
+/// Dup mint quotes fails
+pub async fn add_mint_quote_only_once<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();
+    assert!(tx.add_mint_quote(mint_quote.clone()).await.is_ok());
+    tx.commit().await.unwrap();
+
+    let mut tx = Database::begin_transaction(&db).await.unwrap();
+    assert!(tx.add_mint_quote(mint_quote).await.is_err());
+    tx.commit().await.unwrap();
+}
+
+/// Register payments
+pub async fn register_payments<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();
+    assert!(tx.add_mint_quote(mint_quote.clone()).await.is_ok());
+
+    let p1 = unique_string();
+    let p2 = unique_string();
+
+    let new_paid_amount = tx
+        .increment_mint_quote_amount_paid(&mint_quote.id, 100.into(), p1.clone())
+        .await
+        .unwrap();
+
+    assert_eq!(new_paid_amount, 100.into());
+
+    let new_paid_amount = tx
+        .increment_mint_quote_amount_paid(&mint_quote.id, 250.into(), p2.clone())
+        .await
+        .unwrap();
+
+    assert_eq!(new_paid_amount, 350.into());
+
+    tx.commit().await.unwrap();
+
+    let mint_quote_from_db = db
+        .get_mint_quote(&mint_quote.id)
+        .await
+        .unwrap()
+        .expect("mint_quote_from_db");
+    assert_eq!(mint_quote_from_db.amount_paid(), 350.into());
+    assert_eq!(
+        mint_quote_from_db
+            .payments
+            .iter()
+            .map(|x| (x.payment_id.clone(), x.amount))
+            .collect::<Vec<_>>(),
+        vec![(p1, 100.into()), (p2, 250.into())]
+    );
+}
+
+/// Read mint and payments from db and tx objects
+pub async fn read_mint_from_db_and_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 p2 = unique_string();
+
+    let mut tx = Database::begin_transaction(&db).await.unwrap();
+    tx.add_mint_quote(mint_quote.clone()).await.unwrap();
+    let new_paid_amount = tx
+        .increment_mint_quote_amount_paid(&mint_quote.id, 100.into(), p1.clone())
+        .await
+        .unwrap();
+
+    assert_eq!(new_paid_amount, 100.into());
+
+    let new_paid_amount = tx
+        .increment_mint_quote_amount_paid(&mint_quote.id, 250.into(), p2.clone())
+        .await
+        .unwrap();
+    assert_eq!(new_paid_amount, 350.into());
+    tx.commit().await.unwrap();
+
+    let mint_quote_from_db = db
+        .get_mint_quote(&mint_quote.id)
+        .await
+        .unwrap()
+        .expect("mint_quote_from_db");
+    assert_eq!(mint_quote_from_db.amount_paid(), 350.into());
+    assert_eq!(
+        mint_quote_from_db
+            .payments
+            .iter()
+            .map(|x| (x.payment_id.clone(), x.amount))
+            .collect::<Vec<_>>(),
+        vec![(p1, 100.into()), (p2, 250.into())]
+    );
+
+    let mut tx = Database::begin_transaction(&db).await.unwrap();
+    let mint_quote_from_tx = tx
+        .get_mint_quote(&mint_quote.id)
+        .await
+        .unwrap()
+        .expect("mint_quote_from_tx");
+    assert_eq!(mint_quote_from_db, mint_quote_from_tx);
+}
+
+/// Reject duplicate payments in the same txs
+pub async fn reject_duplicate_payments_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 p1 = unique_string();
+
+    let mut tx = Database::begin_transaction(&db).await.unwrap();
+    tx.add_mint_quote(mint_quote.clone()).await.unwrap();
+    let amount_paid = tx
+        .increment_mint_quote_amount_paid(&mint_quote.id, 100.into(), p1.clone())
+        .await
+        .unwrap();
+
+    assert!(tx
+        .increment_mint_quote_amount_paid(&mint_quote.id, 100.into(), p1)
+        .await
+        .is_err());
+    tx.commit().await.unwrap();
+
+    let mint_quote_from_db = db
+        .get_mint_quote(&mint_quote.id)
+        .await
+        .unwrap()
+        .expect("mint_from_db");
+    assert_eq!(mint_quote_from_db.amount_paid(), amount_paid);
+    assert_eq!(mint_quote_from_db.payments.len(), 1);
+}
+
+/// Reject duplicate payments in different txs
+pub async fn reject_duplicate_payments_diff_tx<DB>(db: DB)
+where
+    DB: Database<Error> + KeysDatabase<Err = Error>,
+{
+    let p1 = unique_string();
+
+    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();
+    let amount_paid = 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_paid(&mint_quote.id, 100.into(), p1)
+        .await
+        .is_err());
+    tx.commit().await.unwrap(); // although in theory nothing has changed, let's try it out
+
+    let mint_quote_from_db = db
+        .get_mint_quote(&mint_quote.id)
+        .await
+        .unwrap()
+        .expect("mint_from_db");
+    assert_eq!(mint_quote_from_db.amount_paid(), amount_paid);
+    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());
+}

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

@@ -3,6 +3,8 @@
 //! This set is generic and checks the default and expected behaviour for a mint database
 //! implementation
 use std::str::FromStr;
+use std::sync::atomic::{AtomicU64, Ordering};
+use std::time::{SystemTime, UNIX_EPOCH};
 
 // For derivation path parsing
 use bitcoin::bip32::DerivationPath;
@@ -13,6 +15,13 @@ use super::*;
 use crate::database::MintKVStoreDatabase;
 use crate::mint::MintKeySetInfo;
 
+mod kvstore;
+mod mint;
+mod proofs;
+
+pub use self::mint::*;
+pub use self::proofs::*;
+
 #[inline]
 async fn setup_keyset<DB>(db: &DB) -> Id
 where
@@ -81,52 +90,6 @@ where
     tx.commit().await.unwrap();
 }
 
-/// Test the basic storing and retrieving proofs from the database. Probably the database would use
-/// binary/`Vec<u8>` to store data, that's why this test would quickly identify issues before running
-/// other tests
-pub async fn add_and_find_proofs<DB>(db: DB)
-where
-    DB: Database<crate::database::Error> + KeysDatabase<Err = crate::database::Error>,
-{
-    let keyset_id = setup_keyset(&db).await;
-
-    let quote_id = QuoteId::new_uuid();
-
-    let proofs = vec![
-        Proof {
-            amount: Amount::from(100),
-            keyset_id,
-            secret: Secret::generate(),
-            c: SecretKey::generate().public_key(),
-            witness: None,
-            dleq: None,
-        },
-        Proof {
-            amount: Amount::from(200),
-            keyset_id,
-            secret: Secret::generate(),
-            c: SecretKey::generate().public_key(),
-            witness: None,
-            dleq: None,
-        },
-    ];
-
-    // Add proofs to database
-    let mut tx = Database::begin_transaction(&db).await.unwrap();
-    tx.add_proofs(proofs.clone(), Some(quote_id.clone()))
-        .await
-        .unwrap();
-    assert!(tx.commit().await.is_ok());
-
-    let proofs_from_db = db.get_proofs_by_ys(&[proofs[0].c, proofs[1].c]).await;
-    assert!(proofs_from_db.is_ok());
-    assert_eq!(proofs_from_db.unwrap().len(), 2);
-
-    let proofs_from_db = db.get_proof_ys_by_quote_id(&quote_id).await;
-    assert!(proofs_from_db.is_ok());
-    assert_eq!(proofs_from_db.unwrap().len(), 2);
-}
-
 /// Test KV store functionality including write, read, list, update, and remove operations
 pub async fn kvstore_functionality<DB>(db: DB)
 where
@@ -213,18 +176,73 @@ where
     }
 }
 
+static COUNTER: AtomicU64 = AtomicU64::new(0);
+
+/// Returns a unique, random-looking Base62 string (no external crates).
+/// Not cryptographically secure, but great for ids, keys, temp names, etc.
+fn unique_string() -> String {
+    // 1) high-res timestamp (nanos since epoch)
+    let now = SystemTime::now()
+        .duration_since(UNIX_EPOCH)
+        .unwrap()
+        .as_nanos();
+
+    // 2) per-process monotonic counter to avoid collisions in the same instant
+    let n = COUNTER.fetch_add(1, Ordering::Relaxed) as u128;
+
+    // 3) process id to reduce collision chance across processes
+    let pid = std::process::id() as u128;
+
+    // Mix the components (simple XOR/shift mix; good enough for "random-looking")
+    let mixed = now ^ (pid << 64) ^ (n << 32);
+
+    base62_encode(mixed)
+}
+
+fn base62_encode(mut x: u128) -> String {
+    const ALPHABET: &[u8; 62] = b"0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz";
+    if x == 0 {
+        return "0".to_string();
+    }
+    let mut buf = [0u8; 26]; // enough for base62(u128)
+    let mut i = buf.len();
+    while x > 0 {
+        let rem = (x % 62) as usize;
+        x /= 62;
+        i -= 1;
+        buf[i] = ALPHABET[rem];
+    }
+    String::from_utf8_lossy(&buf[i..]).into_owned()
+}
+
 /// Unit test that is expected to be passed for a correct database implementation
 #[macro_export]
 macro_rules! mint_db_test {
     ($make_db_fn:ident) => {
-        mint_db_test!(state_transition, $make_db_fn);
-        mint_db_test!(add_and_find_proofs, $make_db_fn);
-        mint_db_test!(kvstore_functionality, $make_db_fn);
+        mint_db_test!(
+            $make_db_fn,
+            state_transition,
+            add_and_find_proofs,
+            add_duplicate_proofs,
+            kvstore_functionality,
+            add_mint_quote,
+            add_mint_quote_only_once,
+            register_payments,
+            read_mint_from_db_and_tx,
+            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
+        );
     };
-    ($name:ident, $make_db_fn:ident) => {
-        #[tokio::test]
-        async fn $name() {
-            cdk_common::database::mint::test::$name($make_db_fn().await).await;
-        }
+    ($make_db_fn:ident, $($name:ident),+ $(,)?) => {
+        $(
+            #[tokio::test]
+            async fn $name() {
+                cdk_common::database::mint::test::$name($make_db_fn().await).await;
+            }
+        )+
     };
 }

crates/cdk-common/src/database/mint/test/proofs.rs

@@ -0,0 +1,97 @@
+//! Proofs tests
+
+use cashu::secret::Secret;
+use cashu::{Amount, SecretKey};
+
+use crate::database::mint::test::setup_keyset;
+use crate::database::mint::{Database, Error, KeysDatabase, Proof, QuoteId};
+
+/// Test the basic storing and retrieving proofs from the database. Probably the database would use
+/// binary/`Vec<u8>` to store data, that's why this test would quickly identify issues before running
+/// other tests
+pub async fn add_and_find_proofs<DB>(db: DB)
+where
+    DB: Database<Error> + KeysDatabase<Err = Error>,
+{
+    let keyset_id = setup_keyset(&db).await;
+
+    let quote_id = QuoteId::new_uuid();
+
+    let proofs = vec![
+        Proof {
+            amount: Amount::from(100),
+            keyset_id,
+            secret: Secret::generate(),
+            c: SecretKey::generate().public_key(),
+            witness: None,
+            dleq: None,
+        },
+        Proof {
+            amount: Amount::from(200),
+            keyset_id,
+            secret: Secret::generate(),
+            c: SecretKey::generate().public_key(),
+            witness: None,
+            dleq: None,
+        },
+    ];
+
+    // Add proofs to database
+    let mut tx = Database::begin_transaction(&db).await.unwrap();
+    tx.add_proofs(proofs.clone(), Some(quote_id.clone()))
+        .await
+        .unwrap();
+    assert!(tx.commit().await.is_ok());
+
+    let proofs_from_db = db.get_proofs_by_ys(&[proofs[0].c, proofs[1].c]).await;
+    assert!(proofs_from_db.is_ok());
+    assert_eq!(proofs_from_db.unwrap().len(), 2);
+
+    let proofs_from_db = db.get_proof_ys_by_quote_id(&quote_id).await;
+    assert!(proofs_from_db.is_ok());
+    assert_eq!(proofs_from_db.unwrap().len(), 2);
+}
+
+/// Test to add duplicate proofs
+pub async fn add_duplicate_proofs<DB>(db: DB)
+where
+    DB: Database<Error> + KeysDatabase<Err = Error>,
+{
+    let keyset_id = setup_keyset(&db).await;
+
+    let quote_id = QuoteId::new_uuid();
+
+    let proofs = vec![
+        Proof {
+            amount: Amount::from(100),
+            keyset_id,
+            secret: Secret::generate(),
+            c: SecretKey::generate().public_key(),
+            witness: None,
+            dleq: None,
+        },
+        Proof {
+            amount: Amount::from(200),
+            keyset_id,
+            secret: Secret::generate(),
+            c: SecretKey::generate().public_key(),
+            witness: None,
+            dleq: None,
+        },
+    ];
+
+    // Add proofs to database
+    let mut tx = Database::begin_transaction(&db).await.unwrap();
+    tx.add_proofs(proofs.clone(), Some(quote_id.clone()))
+        .await
+        .unwrap();
+    assert!(tx.commit().await.is_ok());
+
+    let mut tx = Database::begin_transaction(&db).await.unwrap();
+    let result = tx.add_proofs(proofs.clone(), Some(quote_id.clone())).await;
+
+    assert!(
+        matches!(result.unwrap_err(), Error::Duplicate),
+        "Duplicate entry"
+    );
+}

crates/cdk-sql-common/src/mint/mod.rs

@@ -654,9 +654,9 @@ where
         amount_issued: Amount,
     ) -> Result<Amount, Self::Err> {
         // Get current amount_issued from quote
-        let current_amount = query(
+        let current_amounts = query(
             r#"
-            SELECT amount_issued
+            SELECT amount_issued, amount_paid
             FROM mint_quote
             WHERE id = :quote_id
             FOR UPDATE
@@ -667,20 +667,33 @@ where
         .await
         .inspect_err(|err| {
             tracing::error!("SQLite could not get mint quote amount_issued: {}", err);
-        })?;
+        })?
+        .ok_or(Error::QuoteNotFound)?;
 
-        let current_amount_issued = if let Some(current_amount) = current_amount {
-            let amount: u64 = column_as_number!(current_amount[0].clone());
-            Amount::from(amount)
-        } else {
-            Amount::ZERO
+        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
         };
 
-        // Calculate new amount_issued with overflow check
-        let new_amount_issued = current_amount_issued
-            .checked_add(amount_issued)
-            .ok_or_else(|| database::Error::AmountOverflow)?;
-
         // Update the amount_issued
         query(
             r#"