11 Commits 3626dd2f6a ... 15e10c0e90

Auteur SHA1 Message Date
  C 15e10c0e90 Merge pull request #730 from crodas/fix/race-condition-state-update il y a 3 semaines
  Cesar Rodas 25fad98aa8 Fix formatting il y a 3 semaines
  C 5505f0b5d7 Apply suggestions from code review il y a 3 semaines
  thesimplekid db14c1aecc Merge pull request #731 from thesimplekid/fix_p2pk_multi_sig il y a 3 semaines
  thesimplekid 570782e3b6 chore: clippy il y a 3 semaines
  thesimplekid 8379da2655 fix: only could sigs from unique pubkeys il y a 4 semaines
  thesimplekid 48fd3c652b test: multiple sigs il y a 1 mois
  thesimplekid 204ff46dc4 Merge pull request #732 from thesimplekid/update_stable_rust il y a 3 semaines
  thesimplekid 06a3237977 chore: update stable rust to 1.86.0 il y a 3 semaines
  Cesar Rodas 81a6d68ef3 Remove wallet states from `check_state_transition` il y a 3 semaines
  Cesar Rodas abdde307c6 Fix race conditions with proof state updates. il y a 3 semaines

+ 39 - 9
crates/cashu/src/nuts/nut11/mod.rs

@@ -60,6 +60,9 @@ pub enum Error {
     /// Witness Signatures not provided
     #[error("Witness signatures not provided")]
     SignaturesNotProvided,
+    /// Duplicate signature from same pubkey
+    #[error("Duplicate signature from the same pubkey detected")]
+    DuplicateSignature,
     /// Parse Url Error
     #[error(transparent)]
     UrlParseError(#[from] url::ParseError),
@@ -128,7 +131,7 @@ impl Proof {
             secret.secret_data.tags.unwrap_or_default().try_into()?;
         let msg: &[u8] = self.secret.as_bytes();
 
-        let mut valid_sigs = 0;
+        let mut verified_pubkeys = HashSet::new();
 
         let witness_signatures = match &self.witness {
             Some(witness) => witness.signatures(),
@@ -148,7 +151,10 @@ impl Proof {
                 let sig = Signature::from_str(signature)?;
 
                 if v.verify(msg, &sig).is_ok() {
-                    valid_sigs += 1;
+                    // If the pubkey is already verified, return a duplicate signature error
+                    if !verified_pubkeys.insert(*v) {
+                        return Err(Error::DuplicateSignature);
+                    }
                 } else {
                     tracing::debug!(
                         "Could not verify signature: {sig} on message: {}",
@@ -158,6 +164,8 @@ impl Proof {
             }
         }
 
+        let valid_sigs = verified_pubkeys.len() as u64;
+
         if valid_sigs >= spending_conditions.num_sigs.unwrap_or(1) {
             return Ok(());
         }
@@ -185,19 +193,27 @@ impl Proof {
     }
 }
 
-/// Returns count of valid signatures
-pub fn valid_signatures(msg: &[u8], pubkeys: &[PublicKey], signatures: &[Signature]) -> u64 {
-    let mut count = 0;
+/// Returns count of valid signatures (each public key is only counted once)
+/// Returns error if the same pubkey has multiple valid signatures
+pub fn valid_signatures(
+    msg: &[u8],
+    pubkeys: &[PublicKey],
+    signatures: &[Signature],
+) -> Result<u64, Error> {
+    let mut verified_pubkeys = HashSet::new();
 
     for pubkey in pubkeys {
         for signature in signatures {
             if pubkey.verify(msg, signature).is_ok() {
-                count += 1;
+                // If the pubkey is already verified, return a duplicate signature error
+                if !verified_pubkeys.insert(*pubkey) {
+                    return Err(Error::DuplicateSignature);
+                }
             }
         }
     }
 
-    count
+    Ok(verified_pubkeys.len() as u64)
 }
 
 impl BlindedMessage {
@@ -224,7 +240,7 @@ impl BlindedMessage {
 
     /// Verify P2PK conditions on [BlindedMessage]
     pub fn verify_p2pk(&self, pubkeys: &Vec<PublicKey>, required_sigs: u64) -> Result<(), Error> {
-        let mut valid_sigs = 0;
+        let mut verified_pubkeys = HashSet::new();
         if let Some(witness) = &self.witness {
             for signature in witness
                 .signatures()
@@ -236,7 +252,10 @@ impl BlindedMessage {
                     let sig = Signature::from_str(signature)?;
 
                     if v.verify(msg, &sig).is_ok() {
-                        valid_sigs += 1;
+                        // If the pubkey is already verified, return a duplicate signature error
+                        if !verified_pubkeys.insert(*v) {
+                            return Err(Error::DuplicateSignature);
+                        }
                     } else {
                         tracing::debug!(
                             "Could not verify signature: {sig} on message: {}",
@@ -247,6 +266,8 @@ impl BlindedMessage {
             }
         }
 
+        let valid_sigs = verified_pubkeys.len() as u64;
+
         if valid_sigs.ge(&required_sigs) {
             Ok(())
         } else {
@@ -928,4 +949,13 @@ mod tests {
 
         assert!(invalid_proof.verify_p2pk().is_err());
     }
+
+    #[test]
+    fn test_duplicate_signatures_counting() {
+        let proof: Proof = serde_json::from_str(
+            r#"{"amount":1,"id":"009a1f293253e41e","secret":"[\"P2PK\",{\"nonce\":\"e434a9efbc5f65d144a620e368c9a6dc12c719d0ebc57e0c74f7341864dc449a\",\"data\":\"02a60c27104cf6023581e790970fc33994a320abe36e7ceed16771b0f8d76f0666\",\"tags\":[[\"pubkeys\",\"039c6a20a6ba354b7bb92eb9750716c1098063006362a1fa2afca7421f262d45c5\",\"0203eb2f7cd72a4f725d3327216365d2df18bb4bbc810522fd973c9af987e9b05b\"],[\"locktime\",\"1744876528\"],[\"n_sigs\",\"2\"],[\"sigflag\",\"SIG_INPUTS\"]]}]","C":"02698c4e2b5f9534cd0687d87513c759790cf829aa5739184a3e3735471fbda904","witness":"{\"signatures\":[\"3e9ff9e55c9eccb9e5aa0b6c62d54500b40d0eebadb06efcc8e76f3ce38e0923f956ec1bccb9080db96a17c1e98a1b857abfd1a56bb25670037cea3db1f73d81\",\"c5e29c38e60c4db720cf3f78e590358cf1291a06b9eadf77c1108ae84d533520c2707ffda224eb6a63fddaee9abd5ecf8f2cd263d2556950550e3061a5511f65\"]}"}"#,
+        ).unwrap();
+
+        assert!(proof.verify_p2pk().is_err());
+    }
 }

+ 2 - 2
crates/cashu/src/nuts/nut14/mod.rs

@@ -94,7 +94,7 @@ impl Proof {
                         .collect::<Result<Vec<Signature>, _>>()?;
 
                     // If secret includes refund keys check that there is a valid signature
-                    if valid_signatures(self.secret.as_bytes(), &refund_key, &signatures).ge(&1) {
+                    if valid_signatures(self.secret.as_bytes(), &refund_key, &signatures)?.ge(&1) {
                         return Ok(());
                     }
                 }
@@ -113,7 +113,7 @@ impl Proof {
                     .map(|s| Signature::from_str(s))
                     .collect::<Result<Vec<Signature>, _>>()?;
 
-                let valid_sigs = valid_signatures(self.secret.as_bytes(), &pubkey, &signatures);
+                let valid_sigs = valid_signatures(self.secret.as_bytes(), &pubkey, &signatures)?;
                 ensure_cdk!(valid_sigs >= req_sigs, Error::IncorrectSecretKind);
             }
         }

+ 1 - 0
crates/cdk-common/Cargo.toml

@@ -13,6 +13,7 @@ readme = "README.md"
 [features]
 default = ["mint", "wallet"]
 swagger = ["dep:utoipa", "cashu/swagger"]
+test = []
 bench = []
 wallet = ["cashu/wallet"]
 mint = ["cashu/mint", "dep:uuid"]

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

@@ -17,6 +17,9 @@ use crate::nuts::{
 #[cfg(feature = "auth")]
 mod auth;
 
+#[cfg(feature = "test")]
+pub mod test;
+
 #[cfg(feature = "auth")]
 pub use auth::MintAuthDatabase;
 

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

@@ -0,0 +1,83 @@
+//! Macro with default tests
+//!
+//! This set is generic and checks the default and expected behaviour for a mint database
+//! implementation
+use std::fmt::Debug;
+use std::str::FromStr;
+
+use cashu::secret::Secret;
+use cashu::{Amount, CurrencyUnit, SecretKey};
+
+use super::*;
+use crate::mint::MintKeySetInfo;
+
+#[inline]
+async fn setup_keyset<E: Debug, DB: Database<E>>(db: &DB) -> Id {
+    let keyset_id = Id::from_str("00916bbf7ef91a36").unwrap();
+    let keyset_info = MintKeySetInfo {
+        id: keyset_id,
+        unit: CurrencyUnit::Sat,
+        active: true,
+        valid_from: 0,
+        valid_to: None,
+        derivation_path: bitcoin::bip32::DerivationPath::from_str("m/0'/0'/0'").unwrap(),
+        derivation_path_index: Some(0),
+        max_order: 32,
+        input_fee_ppk: 0,
+    };
+    db.add_keyset_info(keyset_info).await.unwrap();
+    keyset_id
+}
+
+/// State transition test
+pub async fn state_transition<E: Debug, DB: Database<E>>(db: DB) {
+    let keyset_id = setup_keyset(&db).await;
+
+    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
+    db.add_proofs(proofs.clone(), None).await.unwrap();
+
+    // Mark one proof as `pending`
+    assert!(db
+        .update_proofs_states(&[proofs[0].y().unwrap()], State::Pending)
+        .await
+        .is_ok());
+
+    // Attempt to select the `pending` proof, as `pending` again (which should fail)
+    assert!(db
+        .update_proofs_states(&[proofs[0].y().unwrap()], State::Pending)
+        .await
+        .is_err());
+}
+
+/// 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);
+    };
+    ($name:ident, $make_db_fn:ident) => {
+        #[tokio::test]
+        async fn $name() {
+            cdk_common::database::mint::test::$name($make_db_fn().await).await;
+        }
+    };
+}

+ 15 - 1
crates/cdk-common/src/database/mod.rs

@@ -1,7 +1,7 @@
 //! CDK Database
 
 #[cfg(feature = "mint")]
-mod mint;
+pub mod mint;
 #[cfg(feature = "wallet")]
 mod wallet;
 
@@ -53,4 +53,18 @@ pub enum Error {
     /// Invalid keyset
     #[error("Unknown or invalid keyset")]
     InvalidKeysetId,
+    #[cfg(feature = "mint")]
+    /// Invalid state transition
+    #[error("Invalid state transition")]
+    InvalidStateTransition(crate::state::Error),
+}
+
+#[cfg(feature = "mint")]
+impl From<crate::state::Error> for Error {
+    fn from(state: crate::state::Error) -> Self {
+        match state {
+            crate::state::Error::AlreadySpent => Error::AttemptUpdateSpentProof,
+            _ => Error::InvalidStateTransition(state),
+        }
+    }
 }

+ 22 - 1
crates/cdk-common/src/error.rs

@@ -317,7 +317,7 @@ pub enum Error {
     NUT22(#[from] crate::nuts::nut22::Error),
     /// Database Error
     #[error(transparent)]
-    Database(#[from] crate::database::Error),
+    Database(crate::database::Error),
     /// Payment Error
     #[error(transparent)]
     #[cfg(feature = "mint")]
@@ -502,6 +502,27 @@ impl From<Error> for ErrorResponse {
     }
 }
 
+#[cfg(feature = "mint")]
+impl From<crate::database::Error> for Error {
+    fn from(db_error: crate::database::Error) -> Self {
+        match db_error {
+            crate::database::Error::InvalidStateTransition(state) => match state {
+                crate::state::Error::Pending => Self::TokenPending,
+                crate::state::Error::AlreadySpent => Self::TokenAlreadySpent,
+                state => Self::Database(crate::database::Error::InvalidStateTransition(state)),
+            },
+            db_error => Self::Database(db_error),
+        }
+    }
+}
+
+#[cfg(not(feature = "mint"))]
+impl From<crate::database::Error> for Error {
+    fn from(db_error: crate::database::Error) -> Self {
+        Self::Database(db_error)
+    }
+}
+
 impl From<ErrorResponse> for Error {
     fn from(err: ErrorResponse) -> Error {
         match err.code {

+ 2 - 0
crates/cdk-common/src/lib.rs

@@ -16,6 +16,8 @@ pub mod mint;
 #[cfg(feature = "mint")]
 pub mod payment;
 pub mod pub_sub;
+#[cfg(feature = "mint")]
+pub mod state;
 pub mod subscription;
 #[cfg(feature = "wallet")]
 pub mod wallet;

+ 39 - 0
crates/cdk-common/src/state.rs

@@ -0,0 +1,39 @@
+//! State transition rules
+
+use cashu::State;
+
+/// State transition Error
+#[derive(thiserror::Error, Debug)]
+pub enum Error {
+    /// Pending Token
+    #[error("Token already pending for another update")]
+    Pending,
+    /// Already spent
+    #[error("Token already spent")]
+    AlreadySpent,
+    /// Invalid transition
+    #[error("Invalid transition: From {0} to {1}")]
+    InvalidTransition(State, State),
+}
+
+#[inline]
+/// Check if the state transition is allowed
+pub fn check_state_transition(current_state: State, new_state: State) -> Result<(), Error> {
+    let is_valid_transition = match current_state {
+        State::Unspent => matches!(new_state, State::Pending | State::Spent),
+        State::Pending => matches!(new_state, State::Unspent | State::Spent),
+        // Any other state shouldn't be updated by the mint, and the wallet does not use this
+        // function
+        _ => false,
+    };
+
+    if !is_valid_transition {
+        Err(match current_state {
+            State::Pending => Error::Pending,
+            State::Spent => Error::AlreadySpent,
+            _ => Error::InvalidTransition(current_state, new_state),
+        })
+    } else {
+        Ok(())
+    }
+}

+ 1 - 1
crates/cdk-redb/Cargo.toml

@@ -19,7 +19,7 @@ auth = ["cdk-common/auth"]
 
 [dependencies]
 async-trait.workspace = true
-cdk-common.workspace = true
+cdk-common = { workspace = true, features = ["test"] }
 redb = "2.4.0"
 thiserror.workspace = true
 tracing.workspace = true

+ 15 - 8
crates/cdk-redb/src/mint/mod.rs

@@ -15,6 +15,7 @@ use cdk_common::database::{
 use cdk_common::dhke::hash_to_curve;
 use cdk_common::mint::{self, MintKeySetInfo, MintQuote};
 use cdk_common::nut00::ProofsMethods;
+use cdk_common::state::check_state_transition;
 use cdk_common::util::unix_time;
 use cdk_common::{
     BlindSignature, CurrencyUnit, Id, MeltBolt11Request, MeltQuoteState, MintInfo, MintQuoteState,
@@ -787,21 +788,19 @@ impl MintProofsDatabase for MintRedbDatabase {
                 for y in ys {
                     let current_state = match table.get(y.to_bytes()).map_err(Error::from)? {
                         Some(state) => {
-                            Some(serde_json::from_str(state.value()).map_err(Error::from)?)
+                            let current_state =
+                                serde_json::from_str(state.value()).map_err(Error::from)?;
+                            check_state_transition(current_state, proofs_state)?;
+                            Some(current_state)
                         }
                         None => None,
                     };
+
                     states.push(current_state);
                 }
             }
         }
 
-        // Check if any proofs are spent
-        if states.contains(&Some(State::Spent)) {
-            write_txn.abort().map_err(Error::from)?;
-            return Err(database::Error::AttemptUpdateSpentProof);
-        }
-
         {
             let mut table = write_txn
                 .open_table(PROOFS_STATE_TABLE)
@@ -1007,7 +1006,7 @@ impl MintDatabase<database::Error> for MintRedbDatabase {
 #[cfg(test)]
 mod tests {
     use cdk_common::secret::Secret;
-    use cdk_common::{Amount, SecretKey};
+    use cdk_common::{mint_db_test, Amount, SecretKey};
     use tempfile::tempdir;
 
     use super::*;
@@ -1136,4 +1135,12 @@ mod tests {
         assert_eq!(states[0], Some(State::Spent));
         assert_eq!(states[1], Some(State::Unspent));
     }
+
+    async fn provide_db() -> MintRedbDatabase {
+        let tmp_dir = tempdir().unwrap();
+
+        MintRedbDatabase::new(&tmp_dir.path().join("mint.redb")).unwrap()
+    }
+
+    mint_db_test!(provide_db);
 }

+ 2 - 2
crates/cdk-sqlite/Cargo.toml

@@ -20,11 +20,11 @@ sqlcipher = ["libsqlite3-sys"]
 
 [dependencies]
 async-trait.workspace = true
-cdk-common.workspace = true
+cdk-common = { workspace = true, features = ["test"] }
 bitcoin.workspace = true
 sqlx = { version = "0.7.4", default-features = false, features = [
     "runtime-tokio-rustls",
-    "sqlite", 
+    "sqlite",
     "macros",
     "migrate",
     "uuid",

+ 11 - 9
crates/cdk-sqlite/src/mint/mod.rs

@@ -15,6 +15,7 @@ use cdk_common::mint::{self, MintKeySetInfo, MintQuote};
 use cdk_common::nut00::ProofsMethods;
 use cdk_common::nut05::QuoteState;
 use cdk_common::secret::Secret;
+use cdk_common::state::check_state_transition;
 use cdk_common::util::unix_time;
 use cdk_common::{
     Amount, BlindSignature, BlindSignatureDleq, CurrencyUnit, Id, MeltBolt11Request,
@@ -1311,10 +1312,8 @@ WHERE keyset_id=?;
 
         let states = current_states.values().collect::<HashSet<_>>();
 
-        if states.contains(&State::Spent) {
-            transaction.rollback().await.map_err(Error::from)?;
-            tracing::warn!("Attempted to update state of spent proof");
-            return Err(database::Error::AttemptUpdateSpentProof);
+        for state in states {
+            check_state_transition(*state, proofs_state)?;
         }
 
         // If no proofs are spent, proceed with update
@@ -1843,7 +1842,7 @@ fn sqlite_row_to_melt_request(
 #[cfg(test)]
 mod tests {
     use cdk_common::mint::MintKeySetInfo;
-    use cdk_common::Amount;
+    use cdk_common::{mint_db_test, Amount};
 
     use super::*;
 
@@ -1963,10 +1962,7 @@ mod tests {
 
         // Try to update both proofs - should fail because one is spent
         let result = db
-            .update_proofs_states(
-                &[proofs[0].y().unwrap(), proofs[1].y().unwrap()],
-                State::Reserved,
-            )
+            .update_proofs_states(&[proofs[0].y().unwrap()], State::Unspent)
             .await;
 
         assert!(result.is_err());
@@ -1985,4 +1981,10 @@ mod tests {
         assert_eq!(states[0], Some(State::Spent));
         assert_eq!(states[1], Some(State::Unspent));
     }
+
+    async fn provide_db() -> MintSqliteDatabase {
+        memory::empty().await.unwrap()
+    }
+
+    mint_db_test!(provide_db);
 }

+ 1 - 1
crates/cdk-sqlite/src/wallet/mod.rs

@@ -1116,7 +1116,7 @@ mod tests {
             .await
             .unwrap();
 
-        db.migrate().await;
+        db.migrate().await.unwrap();
 
         let mint_info = MintInfo::new().description("test");
         let mint_url = MintUrl::from_str("https://mint.xyz").unwrap();

+ 12 - 12
flake.lock

@@ -2,11 +2,11 @@
   "nodes": {
     "crane": {
       "locked": {
-        "lastModified": 1742394900,
-        "narHash": "sha256-vVOAp9ahvnU+fQoKd4SEXB2JG2wbENkpqcwlkIXgUC0=",
+        "lastModified": 1745022865,
+        "narHash": "sha256-tXL4qUlyYZEGOHUKUWjmmcvJjjLQ+4U38lPWSc8Cgdo=",
         "owner": "ipetkov",
         "repo": "crane",
-        "rev": "70947c1908108c0c551ddfd73d4f750ff2ea67cd",
+        "rev": "25ca4c50039d91ad88cc0b8feacb9ad7f748dedf",
         "type": "github"
       },
       "original": {
@@ -23,11 +23,11 @@
         "rust-analyzer-src": []
       },
       "locked": {
-        "lastModified": 1742452566,
-        "narHash": "sha256-sVuLDQ2UIWfXUBbctzrZrXM2X05YjX08K7XHMztt36E=",
+        "lastModified": 1745303921,
+        "narHash": "sha256-zYucemS2QvJUR5GKJ/u3eZAoe82AKhcxMtNVZDERXsw=",
         "owner": "nix-community",
         "repo": "fenix",
-        "rev": "7d9ba794daf5e8cc7ee728859bc688d8e26d5f06",
+        "rev": "14850d5984f3696a2972f85f19085e5fb46daa95",
         "type": "github"
       },
       "original": {
@@ -93,11 +93,11 @@
     },
     "nixpkgs": {
       "locked": {
-        "lastModified": 1743501102,
-        "narHash": "sha256-7PCBQ4aGVF8OrzMkzqtYSKyoQuU2jtpPi4lmABpe5X4=",
+        "lastModified": 1744440957,
+        "narHash": "sha256-FHlSkNqFmPxPJvy+6fNLaNeWnF1lZSgqVCl/eWaJRc4=",
         "owner": "NixOS",
         "repo": "nixpkgs",
-        "rev": "02f2af8c8a8c3b2c05028936a1e84daefa1171d4",
+        "rev": "26d499fc9f1d567283d5d56fcf367edd815dba1d",
         "type": "github"
       },
       "original": {
@@ -160,11 +160,11 @@
         ]
       },
       "locked": {
-        "lastModified": 1743561237,
-        "narHash": "sha256-dd97LXek202OWmUXvKYFdYWj0jHrn3p+L5Ojh1SEOqs=",
+        "lastModified": 1745289264,
+        "narHash": "sha256-7nt+UJ7qaIUe2J7BdnEEph9n2eKEwxUwKS/QIr091uA=",
         "owner": "oxalica",
         "repo": "rust-overlay",
-        "rev": "1de27ae43712a971c1da100dcd84386356f03ec7",
+        "rev": "3b7171858c20d5293360042936058fb0c4cb93a9",
         "type": "github"
       },
       "original": {

+ 1 - 1
flake.nix

@@ -49,7 +49,7 @@
 
         # Toolchains
         # latest stable
-        stable_toolchain = pkgs.rust-bin.stable."1.83.0".default.override {
+        stable_toolchain = pkgs.rust-bin.stable."1.86.0".default.override {
           targets = [ "wasm32-unknown-unknown" ]; # wasm
           extensions = [ "rustfmt" "clippy" "rust-analyzer" ];
         };

+ 1 - 1
rust-toolchain.toml

@@ -1,4 +1,4 @@
 [toolchain]
-channel="1.83.0"
+channel="1.86.0"
 components = ["rustfmt", "clippy", "rust-analyzer"]