Move LockedRows to cdk-sql-common behind testing feature

The row locking mechanism was originally in cdk-common to enforce that database
records must be read (and locked) before they can be modified.  This prevents
race conditions by ensuring upper layers always work with fresh data from the
database.

However, this mechanism is only useful for testing - in production, the SQL FOR
UPDATE clause already provides the necessary row-level locking.  The LockedRows
tracking adds overhead without benefit in release builds.

This change moves LockedRows to cdk-sql-common and gates it behind a `testing`
feature flag. When the feature is disabled:
- The locked_row module is not compiled at all
- Helper methods (lock_record, verify_locked, etc.) become empty no-ops
- All methods are marked #[inline(always)] so they're completely elided

When the feature is enabled:
- LockedRows tracks which records have been read in the transaction
- Verification methods panic if code attempts to modify unlocked records
- This catches bugs during testing where code skips the read step

The verification methods now panic instead of returning errors because: 1. This
is test-only code - panics provide clear failure messages 2. Removes the need
for Error::UpdatingUnlockedRecord in cdk-common 3. Simplifies the API - no
error handling needed at call sites

Related test cases that depended on catching UpdatingUnlockedRecord errors have
been removed since the mechanism now panics instead.

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

@@ -1429,114 +1429,6 @@ where
     assert_eq!(retrieved.amount_paid(), 500.into());
 }
 
-/// Test that modifying a mint quote without loading it first fails
-///
-/// This test verifies that the database layer rejects attempts to modify
-/// records that weren't first read (and locked) within the transaction.
-pub async fn modify_mint_quote_without_loading_fails<DB>(db: DB)
-where
-    DB: Database<Error> + KeysDatabase<Err = Error>,
-{
-    use crate::database::mint::test::unique_string;
-
-    // First, create a mint quote in the database
-    let mint_quote = MintQuote::new(
-        None,
-        "".to_owned(),
-        cashu::CurrencyUnit::Sat,
-        None,
-        0,
-        PaymentIdentifier::CustomId(unique_string()),
-        None,
-        1000.into(),
-        500.into(), // already has some amount paid
-        cashu::PaymentMethod::Bolt11,
-        0,
-        vec![],
-        vec![],
-    );
-
-    let mut tx = Database::begin_transaction(&db).await.unwrap();
-    let mint_quote = tx.add_mint_quote(mint_quote).await.unwrap();
-    tx.commit().await.unwrap();
-
-    // Now try to modify the quote in a new transaction WITHOUT loading it first
-    let mut tx = Database::begin_transaction(&db).await.unwrap();
-
-    // Clone the quote from our local variable (simulating having a stale reference)
-    // and try to increment amount_paid without first calling get_mint_quote
-    let result = tx
-        .increment_mint_quote_amount_paid(mint_quote.clone(), 100.into(), unique_string())
-        .await;
-
-    // This should fail because we didn't load (and lock) the quote first
-    assert!(
-        result.is_err(),
-        "Modifying mint quote without loading should fail"
-    );
-
-    // Verify the error is the expected UpdatingUnlockedRecord error
-    let err = result.unwrap_err();
-    assert!(
-        matches!(err, Error::UpdatingUnlockedRecord(_)),
-        "Expected UpdatingUnlockedRecord error, got: {:?}",
-        err
-    );
-
-    tx.rollback().await.unwrap();
-}
-
-/// Test that incrementing amount_issued without loading the quote first fails
-pub async fn increment_amount_issued_without_loading_fails<DB>(db: DB)
-where
-    DB: Database<Error> + KeysDatabase<Err = Error>,
-{
-    use crate::database::mint::test::unique_string;
-
-    // Create a mint quote with amount_paid already set
-    let mint_quote = MintQuote::new(
-        None,
-        "".to_owned(),
-        cashu::CurrencyUnit::Sat,
-        None,
-        0,
-        PaymentIdentifier::CustomId(unique_string()),
-        None,
-        1000.into(),
-        500.into(), // has paid amount to allow issuing
-        cashu::PaymentMethod::Bolt11,
-        0,
-        vec![],
-        vec![],
-    );
-
-    let mut tx = Database::begin_transaction(&db).await.unwrap();
-    let mint_quote = tx.add_mint_quote(mint_quote).await.unwrap();
-    tx.commit().await.unwrap();
-
-    // Try to increment amount_issued without loading the quote first
-    let mut tx = Database::begin_transaction(&db).await.unwrap();
-
-    let result = tx
-        .increment_mint_quote_amount_issued(mint_quote.clone(), 100.into())
-        .await;
-
-    // This should fail
-    assert!(
-        result.is_err(),
-        "Incrementing amount_issued without loading should fail"
-    );
-
-    let err = result.unwrap_err();
-    assert!(
-        matches!(err, Error::UpdatingUnlockedRecord(_)),
-        "Expected UpdatingUnlockedRecord error, got: {:?}",
-        err
-    );
-
-    tx.rollback().await.unwrap();
-}
-
 /// Test that loading the quote first allows modifications
 pub async fn modify_mint_quote_after_loading_succeeds<DB>(db: DB)
 where

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

@@ -255,9 +255,6 @@ macro_rules! mint_db_test {
             get_blind_signatures_in_transaction,
             reject_duplicate_payment_ids,
             remove_spent_proofs_should_fail,
-            modify_mint_quote_without_loading_fails,
-            increment_amount_issued_without_loading_fails,
-            modify_mint_quote_after_loading_succeeds
         );
     };
     ($make_db_fn:ident, $($name:ident),+ $(,)?) => {

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

@@ -1,7 +1,6 @@
 //! CDK Database
 
 mod kvstore;
-pub mod locked_row;
 
 #[cfg(feature = "mint")]
 pub mod mint;
@@ -33,8 +32,6 @@ pub use wallet::{
     DynWalletDatabaseTransaction,
 };
 
-use crate::database::locked_row::RowId;
-
 /// Type alias for dynamic Wallet Database
 #[cfg(feature = "wallet")]
 pub type DynWalletDatabase = std::sync::Arc<dyn WalletDatabase<Error> + Send + Sync>;
@@ -134,10 +131,6 @@ pub enum Error {
     #[error(transparent)]
     Database(Box<dyn std::error::Error + Send + Sync>),
 
-    /// Misusage of update
-    #[error("Attempting to update record without previously locking ita {0:?}")]
-    UpdatingUnlockedRecord(RowId),
-
     /// Duplicate entry
     #[error("Duplicate entry")]
     Duplicate,

crates/cdk-postgres/Cargo.toml

@@ -35,5 +35,8 @@ native-tls = "0.2"
 once_cell.workspace = true
 paste = "1.0.15"
 
+[dev-dependencies]
+cdk-sql-common = { workspace = true, features = ["testing"] }
+
 [lints]
 workspace = true

crates/cdk-sql-common/Cargo.toml

@@ -17,6 +17,7 @@ mint = ["cdk-common/mint"]
 wallet = ["cdk-common/wallet"]
 auth = ["cdk-common/auth"]
 prometheus = ["cdk-prometheus"]
+testing = []
 [dependencies]
 async-trait.workspace = true
 cdk-common = { workspace = true, features = ["test"] }

crates/cdk-sql-common/src/lib.rs

@@ -20,3 +20,9 @@ pub mod wallet;
 pub use mint::SQLMintDatabase;
 #[cfg(feature = "wallet")]
 pub use wallet::SQLWalletDatabase;
+
+#[cfg(feature = "testing")]
+mod locked_row;
+
+#[cfg(feature = "testing")]
+pub use locked_row::{LockedRows, RowId};

crates/cdk-common/src/database/locked_row.rs → crates/cdk-sql-common/src/locked_row.rs

@@ -7,13 +7,16 @@
 //! By requiring explicit locking before updates, this prevents race conditions and ensures
 //! data consistency when multiple operations might attempt to modify the same resources
 //! concurrently.
+//!
+//! This module is only available when the `testing` feature is enabled and is intended
+//! for use in test environments to validate proper row locking behavior.
 
-use std::collections::HashSet;
+#![allow(clippy::panic)]
 
-use cashu::quote_id::QuoteId;
-use cashu::PublicKey;
+use std::collections::HashSet;
 
-use crate::database::Error;
+use cdk_common::nuts::PublicKey;
+use cdk_common::quote_id::QuoteId;
 
 /// Identifies a database row that can be locked.
 ///
@@ -28,18 +31,21 @@ pub enum RowId {
 }
 
 impl From<PublicKey> for RowId {
+    #[inline(always)]
     fn from(value: PublicKey) -> Self {
         RowId::Proof(value)
     }
 }
 
 impl From<&PublicKey> for RowId {
+    #[inline(always)]
     fn from(value: &PublicKey) -> Self {
         RowId::Proof(*value)
     }
 }
 
 impl From<&QuoteId> for RowId {
+    #[inline(always)]
     fn from(value: &QuoteId) -> Self {
         RowId::Quote(value.to_owned())
     }
@@ -64,6 +70,7 @@ impl LockedRows {
     ///
     /// After locking, any subsequent calls to [`is_locked`](Self::is_locked) for this
     /// row will succeed. This should be called when reading a row that will be modified.
+    #[inline(always)]
     pub fn lock<T>(&mut self, record_id: T)
     where
         T: Into<RowId>,
@@ -75,6 +82,7 @@ impl LockedRows {
     ///
     /// This is a convenience method equivalent to calling [`lock`](Self::lock)
     /// for each item in the collection.
+    #[inline(always)]
     pub fn lock_many<T>(&mut self, records_id: Vec<T>)
     where
         T: Into<RowId>,
@@ -86,39 +94,43 @@ impl LockedRows {
 
     /// Verifies that all specified rows are currently locked.
     ///
-    /// # Errors
+    /// # Panics
     ///
-    /// Returns [`Error::UpdatingUnlockedRecord`] if any of the specified rows
-    /// have not been locked. This prevents modifications to rows that weren't
-    /// properly read first.
-    pub fn is_locked_many<T>(&self, records_id: Vec<T>) -> Result<(), Error>
+    /// Panics if any of the specified rows have not been locked. This is intentional
+    /// as this module is only used in tests to validate proper row locking behavior.
+    #[inline(always)]
+    pub fn is_locked_many<T>(&self, records_id: Vec<T>)
     where
         T: Into<RowId>,
     {
-        records_id.into_iter().try_for_each(|resource_id| {
+        for resource_id in records_id {
             let id = resource_id.into();
-            self.inner
-                .contains(&id)
-                .then_some(())
-                .ok_or(Error::UpdatingUnlockedRecord(id))
-        })
+            if !self.inner.contains(&id) {
+                panic!(
+                    "Attempting to update record without previously locking it: {:?}",
+                    id
+                );
+            }
+        }
     }
 
     /// Verifies that a single row is currently locked.
     ///
-    /// # Errors
+    /// # Panics
     ///
-    /// Returns [`Error::UpdatingUnlockedRecord`] if the specified row has not
-    /// been locked. This prevents modifications to rows that weren't properly
-    /// read first.
-    pub fn is_locked<T>(&self, resource_id: T) -> Result<(), Error>
+    /// Panics if the specified row has not been locked. This is intentional
+    /// as this module is only used in tests to validate proper row locking behavior.
+    #[inline(always)]
+    pub fn is_locked<T>(&self, resource_id: T)
     where
         T: Into<RowId>,
     {
         let id = resource_id.into();
-        self.inner
-            .contains(&id)
-            .then_some(())
-            .ok_or(Error::UpdatingUnlockedRecord(id))
+        if !self.inner.contains(&id) {
+            panic!(
+                "Attempting to update record without previously locking it: {:?}",
+                id
+            );
+        }
     }
 }

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

@@ -140,7 +140,7 @@ where
         {
             tracing::debug!("Attempting to add known proof. Skipping.... {:?}", err);
         }
-        self.locked_records.lock(y);
+        self.lock_record(y);
         Ok(())
     }
 
@@ -149,7 +149,7 @@ where
         y: &PublicKey,
         proofs_state: State,
     ) -> Result<Option<State>, Self::Err> {
-        self.locked_records.is_locked(y)?;
+        self.verify_locked(y);
         let current_state = query(r#"SELECT state FROM proof WHERE y = :y FOR UPDATE"#)?
             .bind("y", y.to_bytes().to_vec())
             .pluck(&self.inner)
@@ -254,6 +254,7 @@ where
                 self.pool.get().map_err(|e| Error::Database(Box::new(e)))?,
             )
             .await?,
+            #[cfg(feature = "testing")]
             locked_records: Default::default(),
         }))
     }

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

@@ -15,7 +15,6 @@ use std::sync::Arc;
 
 use async_trait::async_trait;
 use bitcoin::bip32::DerivationPath;
-use cdk_common::database::locked_row::LockedRows;
 use cdk_common::database::mint::{
     CompletedOperationsDatabase, CompletedOperationsTransaction, SagaDatabase, SagaTransaction,
 };
@@ -79,7 +78,61 @@ where
     RM: DatabasePool + 'static,
 {
     inner: ConnectionWithTransaction<RM::Connection, PooledResource<RM>>,
-    locked_records: LockedRows,
+    #[cfg(feature = "testing")]
+    locked_records: crate::LockedRows,
+}
+
+impl<RM> SQLTransaction<RM>
+where
+    RM: DatabasePool + 'static,
+{
+    /// Lock a record for modification (only active when testing feature is enabled)
+    #[cfg(feature = "testing")]
+    #[inline(always)]
+    fn lock_record<T: Into<crate::RowId>>(&mut self, record_id: T) {
+        self.locked_records.lock(record_id);
+    }
+
+    /// Lock a record for modification (no-op when testing feature is disabled)
+    #[cfg(not(feature = "testing"))]
+    #[inline(always)]
+    fn lock_record<T>(&mut self, _record_id: T) {}
+
+    /// Lock multiple records for modification (only active when testing feature is enabled)
+    #[cfg(feature = "testing")]
+    #[inline(always)]
+    fn lock_records<T: Into<crate::RowId>>(&mut self, records: Vec<T>) {
+        self.locked_records.lock_many(records);
+    }
+
+    /// Lock multiple records for modification (no-op when testing feature is disabled)
+    #[cfg(not(feature = "testing"))]
+    #[inline(always)]
+    fn lock_records<T>(&mut self, _records: Vec<T>) {}
+
+    /// Verify records are locked (only active when testing feature is enabled)
+    #[cfg(feature = "testing")]
+    #[inline(always)]
+    fn verify_locked<T: Into<crate::RowId>>(&self, record_id: T) {
+        self.locked_records.is_locked(record_id);
+    }
+
+    /// Verify records are locked (no-op when testing feature is disabled)
+    #[cfg(not(feature = "testing"))]
+    #[inline(always)]
+    fn verify_locked<T>(&self, _record_id: T) {}
+
+    /// Verify multiple records are locked (only active when testing feature is enabled)
+    #[cfg(feature = "testing")]
+    #[inline(always)]
+    fn verify_locked_many<T: Into<crate::RowId>>(&self, records: Vec<T>) {
+        self.locked_records.is_locked_many(records);
+    }
+
+    /// Verify multiple records are locked (no-op when testing feature is disabled)
+    #[cfg(not(feature = "testing"))]
+    #[inline(always)]
+    fn verify_locked_many<T>(&self, _records: Vec<T>) {}
 }
 
 impl<RM> SQLMintDatabase<RM>
@@ -144,7 +197,7 @@ where
 
         for proof in proofs {
             let y = proof.y()?;
-            self.locked_records.lock(y);
+            self.lock_record(y);
 
             query(
                 r#"
@@ -180,7 +233,7 @@ where
         ys: &[PublicKey],
         new_state: State,
     ) -> Result<Vec<Option<State>>, Self::Err> {
-        self.locked_records.is_locked_many(ys.to_owned())?;
+        self.verify_locked_many(ys.to_owned());
 
         let mut current_states = get_current_states(&self.inner, ys, true).await?;
 
@@ -302,7 +355,7 @@ where
         .into_iter()
         .map(|row| {
             sql_row_to_proof(row).inspect(|row| {
-                let _ = row.y().map(|c| self.locked_records.lock(c));
+                let _ = row.y().map(|c| self.lock_record(c));
             })
         })
         .collect::<Result<Vec<Proof>, _>>()?
@@ -345,8 +398,7 @@ where
             get_current_states(&self.inner, ys, true)
                 .await
                 .inspect(|public_keys| {
-                    self.locked_records
-                        .lock_many(public_keys.keys().collect::<Vec<_>>());
+                    self.lock_records(public_keys.keys().collect::<Vec<_>>());
                 })?;
 
         Ok(ys.iter().map(|y| current_states.remove(y)).collect())
@@ -745,6 +797,7 @@ where
                 self.pool.get().map_err(|e| Error::Database(Box::new(e)))?,
             )
             .await?,
+            #[cfg(feature = "testing")]
             locked_records: Default::default(),
         };
 
@@ -1035,7 +1088,7 @@ where
         amount_paid: Amount,
         payment_id: String,
     ) -> Result<mint::MintQuote, Self::Err> {
-        self.locked_records.is_locked(&quote.id)?;
+        self.verify_locked(&quote.id);
         if amount_paid == Amount::ZERO {
             tracing::warn!("Amount payments of zero amount should not be recorded.");
             return Err(Error::Duplicate);
@@ -1115,7 +1168,7 @@ where
         mut quote: mint::MintQuote,
         amount_issued: Amount,
     ) -> Result<mint::MintQuote, Self::Err> {
-        self.locked_records.is_locked(&quote.id)?;
+        self.verify_locked(&quote.id);
 
         let new_amount_issued = quote
             .increment_amount_issued(amount_issued)
@@ -1183,7 +1236,7 @@ VALUES (:quote_id, :amount, :timestamp);
         .execute(&self.inner)
         .await?;
 
-        self.locked_records.lock(&quote.id);
+        self.lock_record(&quote.id);
 
         Ok(quote)
     }
@@ -1232,7 +1285,7 @@ VALUES (:quote_id, :amount, :timestamp);
         .execute(&self.inner)
         .await?;
 
-        self.locked_records.lock(&quote.id);
+        self.lock_record(&quote.id);
 
         Ok(())
     }
@@ -1370,7 +1423,7 @@ VALUES (:quote_id, :amount, :timestamp);
             .await
             .inspect(|quote| {
                 quote.as_ref().inspect(|mint_quote| {
-                    self.locked_records.lock(&mint_quote.id);
+                    self.lock_record(&mint_quote.id);
                 });
             })
     }
@@ -1383,7 +1436,7 @@ VALUES (:quote_id, :amount, :timestamp);
             .await
             .inspect(|quote| {
                 quote.as_ref().inspect(|melt_quote| {
-                    self.locked_records.lock(&melt_quote.id);
+                    self.lock_record(&melt_quote.id);
                 });
             })
     }
@@ -1396,7 +1449,7 @@ VALUES (:quote_id, :amount, :timestamp);
             .await
             .inspect(|quote| {
                 quote.as_ref().inspect(|mint_quote| {
-                    self.locked_records.lock(&mint_quote.id);
+                    self.lock_record(&mint_quote.id);
                 });
             })
     }
@@ -1409,7 +1462,7 @@ VALUES (:quote_id, :amount, :timestamp);
             .await
             .inspect(|quote| {
                 quote.as_ref().inspect(|mint_quote| {
-                    self.locked_records.lock(&mint_quote.id);
+                    self.lock_record(&mint_quote.id);
                 });
             })
     }
@@ -2181,6 +2234,7 @@ where
                 self.pool.get().map_err(|e| Error::Database(Box::new(e)))?,
             )
             .await?,
+            #[cfg(feature = "testing")]
             locked_records: Default::default(),
         }))
     }
@@ -2475,6 +2529,7 @@ where
                 self.pool.get().map_err(|e| Error::Database(Box::new(e)))?,
             )
             .await?,
+            #[cfg(feature = "testing")]
             locked_records: Default::default(),
         };
 

crates/cdk-sqlite/Cargo.toml

@@ -37,5 +37,8 @@ paste = "1.0.15"
 [target.'cfg(target_arch = "wasm32")'.dependencies]
 uuid = { workspace = true, features = ["js"] }
 
+[dev-dependencies]
+cdk-sql-common = { workspace = true, features = ["testing"] }
+
 [lints]
 workspace = true