diff --git a/crates/cdk-signatory/src/db_signatory.rs b/crates/cdk-signatory/src/db_signatory.rs index c5a2d83ef..6bb7ea59c 100644 --- a/crates/cdk-signatory/src/db_signatory.rs +++ b/crates/cdk-signatory/src/db_signatory.rs @@ -1,6 +1,14 @@ //! Main Signatory implementation //! -//! It is named db_signatory because it uses a database to maintain state. +//! It is named db_signatory because it uses a database to persist state. The +//! database is a persistence layer only: it is read once on boot to hydrate the +//! in-memory keysets and written only when rotating keys. Every other operation +//! is served from and mutates in-memory state. See ADR-0003. +//! +//! Boot is strict: `new` attempts the initial keyset load from the database +//! once and bubbles up any error, so a failed load fails construction rather +//! than leaving a signatory without keys. On success the returned signatory is +//! loaded and serving. use std::collections::HashMap; use std::sync::Arc; @@ -10,7 +18,7 @@ use cdk_common::dhke::{sign_message, verify_message}; use cdk_common::mint::MintKeySetInfo; use cdk_common::nuts::{BlindSignature, BlindedMessage, CurrencyUnit, Id, MintKeySet, Proof}; use cdk_common::{database, Error, PublicKey}; -use tokio::sync::{watch, RwLock}; +use tokio::sync::{watch, Mutex, RwLock}; use tracing::instrument; use crate::common::{ @@ -28,9 +36,16 @@ use crate::signatory::{RotateKeyArguments, Signatory, SignatoryKeySet, Signatory pub struct DbSignatory { keysets: RwLock>, active_keysets: RwLock>, + /// Serializes keyset rotations. The standalone signatory gRPC server calls + /// rotate_keyset directly (no embedded single-runner), so two concurrent + /// rotations of the same unit could otherwise read the same path index and + /// derive duplicate keysets. + rotation_lock: Mutex<()>, localstore: Arc + Send + Sync>, secp_ctx: Secp256k1, custom_paths: HashMap, + /// Units to initialize on boot, as `init_keysets` expects them. + supported_units: HashMap)>, xpriv: Xpriv, xpub: PublicKey, /// Latest keyset snapshot, published on every reload (initial load and each @@ -39,7 +54,11 @@ pub struct DbSignatory { } impl DbSignatory { - /// Creates a new MemorySignatory instance + /// Creates a new signatory, loading its keysets from the database. + /// + /// The load is attempted once and any error is bubbled up: a failed load + /// fails construction rather than returning a signatory without keys. On + /// success the returned signatory is loaded and serving. /// /// # Panics /// @@ -47,16 +66,11 @@ impl DbSignatory { pub async fn new( localstore: Arc + Send + Sync>, seed: &[u8], - mut supported_units: HashMap)>, + supported_units: HashMap)>, custom_paths: HashMap, ) -> Result { let secp_ctx = Secp256k1::new(); let xpriv = Xpriv::new_master(bitcoin::Network::Bitcoin, seed).expect("RNG busted"); - init_keysets(xpriv, &secp_ctx, &localstore, &supported_units).await?; - - supported_units - .entry(CurrencyUnit::Auth) - .or_insert((0, vec![1])); let xpub: PublicKey = xpriv.to_keypair(&secp_ctx).public_key().into(); let (keyset_updates, _) = watch::channel(SignatoryKeysets { @@ -64,29 +78,48 @@ impl DbSignatory { keysets: vec![], }); - let keys = Self { + let signatory = Self { keysets: Default::default(), active_keysets: Default::default(), + rotation_lock: Default::default(), localstore, custom_paths, + supported_units, xpub, secp_ctx, xpriv, keyset_updates, }; - keys.reload_keys_from_db().await?; - Ok(keys) + signatory.boot_load().await?; + + Ok(signatory) } - /// Load all the keysets from the database, even if they are not active. + /// Load keysets from the database into memory. /// - /// Since the database is owned by this process, we can load all the keysets in memory, and use - /// it as the primary source, and the database as the persistence layer. + /// This runs the boot-time database reactivation (`init_keysets`) and then + /// hydrates memory from the database. + async fn boot_load(&self) -> Result<(), Error> { + init_keysets( + self.xpriv, + &self.secp_ctx, + &self.localstore, + &self.supported_units, + ) + .await?; + self.load_keys_from_db().await?; + Ok(()) + } + + /// Hydrate the in-memory keysets from the database. /// - /// Any operation performed with keysets, are done through this trait and never to the database - /// directly. - async fn reload_keys_from_db(&self) -> Result<(), Error> { + /// This is the only path that reads keysets from the database. Since the + /// database is owned by this process, all keysets are loaded into memory + /// and memory is the primary source afterwards; the database is only the + /// persistence layer. Any later operation reads from and mutates memory, + /// never the database directly. + async fn load_keys_from_db(&self) -> Result<(), Error> { let mut keysets = self.keysets.write().await; let mut active_keysets = self.active_keysets.write().await; keysets.clear(); @@ -104,15 +137,20 @@ impl DbSignatory { keysets.insert(id, (info, keyset)); } - // Publish the new snapshot to any keyset subscribers. Sending while the - // locks are held keeps the published set consistent with in-memory - // state. + self.publish_snapshot(&keysets); + + Ok(()) + } + + /// Publish the current keyset set to any subscribers of the watch channel. + /// + /// Callers hold the `keysets` write lock while calling this so the published + /// snapshot stays consistent with the in-memory state that produced it. + fn publish_snapshot(&self, keysets: &HashMap) { self.keyset_updates.send_replace(SignatoryKeysets { pubkey: self.xpub, keysets: keysets.values().map(|k| k.into()).collect(), }); - - Ok(()) } fn generate_keyset(&self, keyset_info: &MintKeySetInfo) -> MintKeySet { @@ -127,6 +165,20 @@ impl DbSignatory { keyset_info.id.get_version(), ) } + + /// Snapshot the current keysets from memory. + async fn keysets_snapshot(&self) -> SignatoryKeysets { + SignatoryKeysets { + pubkey: self.xpub, + keysets: self + .keysets + .read() + .await + .values() + .map(|k| k.into()) + .collect(), + } + } } #[async_trait::async_trait] @@ -190,16 +242,7 @@ impl Signatory for DbSignatory { #[tracing::instrument(skip_all)] async fn keysets(&self) -> Result { - Ok(SignatoryKeysets { - pubkey: self.xpub, - keysets: self - .keysets - .read() - .await - .values() - .map(|k| k.into()) - .collect::>(), - }) + Ok(self.keysets_snapshot().await) } #[tracing::instrument(skip_all)] @@ -211,21 +254,29 @@ impl Signatory for DbSignatory { /// Generate new keyset #[tracing::instrument(skip(self))] async fn rotate_keyset(&self, args: RotateKeyArguments) -> Result { - let (path_index, amounts) = if let Some(current_keyset_id) = - self.localstore.get_active_keyset_id(&args.unit).await? - { - let keyset_info = self - .localstore - .get_keyset_info(¤t_keyset_id) - .await? - .ok_or(Error::UnknownKeySet)?; - - ( - keyset_info.derivation_path_index.unwrap_or(1) + 1, - keyset_info.amounts, - ) - } else { - (1, vec![]) + // Serialize rotations. The standalone signatory gRPC server invokes this + // directly (no embedded single-runner), so without this two concurrent + // rotations of the same unit could read the same path index below and + // derive duplicate keysets. Held for the whole method. + let _rotation = self.rotation_lock.lock().await; + + // Derive the next path index and default amounts from the in-memory + // active keyset rather than the database. The rotation lock above keeps + // this read stable across the DB write and the in-memory update. + // Acquire keysets before active_keysets, the same order the write phase + // below uses (and load_keys_from_db), keeping lock ordering consistent. + let (path_index, amounts) = { + let keysets = self.keysets.read().await; + let active_keysets = self.active_keysets.read().await; + if let Some(current_keyset_id) = active_keysets.get(&args.unit) { + let (info, _) = keysets.get(current_keyset_id).ok_or(Error::UnknownKeySet)?; + ( + info.derivation_path_index.unwrap_or(1) + 1, + info.amounts.clone(), + ) + } else { + (1, vec![]) + } }; let derivation_path = match self.custom_paths.get(&args.unit) { @@ -255,18 +306,39 @@ impl Signatory for DbSignatory { args.keyset_id_type, ); - let keysets = self.keysets().await?; + let keysets = self.keysets_snapshot().await; check_unit_string_collision(keysets.keysets, &info)?; let id = info.id; + + // Persist the rotation. This is the only path that writes to the + // database. let mut tx = self.localstore.begin_transaction().await?; tx.add_keyset_info(info.clone()).await?; - tx.set_active_keyset(args.unit, id).await?; + tx.set_active_keyset(args.unit.clone(), id).await?; tx.commit().await?; - self.reload_keys_from_db().await?; + // Refresh the in-memory state to match what was just persisted, without + // reading the database back. This mirrors a fresh boot: the active + // pointer for the unit moves to the new keyset, so any previously active + // keyset for the unit becomes inactive. + let mut info = info; + info.active = true; + let signatory_keyset: SignatoryKeySet = (&(info.clone(), keyset.clone())).into(); + + let mut keysets = self.keysets.write().await; + let mut active_keysets = self.active_keysets.write().await; + + if let Some(prev_id) = active_keysets.insert(args.unit, id) { + if let Some((prev_info, _)) = keysets.get_mut(&prev_id) { + prev_info.active = false; + } + } + keysets.insert(id, (info, keyset)); + + self.publish_snapshot(&keysets); - Ok((&(info, keyset)).into()) + Ok(signatory_keyset) } } @@ -368,6 +440,61 @@ mod test { ); } + #[tokio::test(flavor = "multi_thread", worker_threads = 4)] + async fn concurrent_rotations_do_not_collide() { + let store = Arc::new( + cdk_sqlite::mint::memory::empty() + .await + .expect("in-memory db"), + ); + let signatory = Arc::new( + DbSignatory::new( + store, + b"test-seed-concurrent-rotations", + Default::default(), + Default::default(), + ) + .await + .expect("DbSignatory::new"), + ); + + // Fire several rotations of the same unit concurrently. Without the + // rotation lock they could read the same path index and derive + // duplicate keysets. + const ROTATIONS: usize = 8; + let mut handles = Vec::with_capacity(ROTATIONS); + for _ in 0..ROTATIONS { + let signatory = signatory.clone(); + handles.push(tokio::spawn(async move { + signatory + .rotate_keyset(RotateKeyArguments { + unit: CurrencyUnit::Sat, + amounts: vec![1, 2, 4, 8], + input_fee_ppk: 0, + keyset_id_type: cdk_common::nut02::KeySetVersion::Version00, + final_expiry: None, + }) + .await + .expect("rotate_keyset") + })); + } + + let mut ids = HashSet::new(); + let mut versions = HashSet::new(); + for handle in handles { + let keyset = handle.await.expect("join"); + assert!( + ids.insert(keyset.id), + "duplicate keyset id from concurrent rotation" + ); + assert!( + versions.insert(keyset.version), + "duplicate path index from concurrent rotation" + ); + } + assert_eq!(ids.len(), ROTATIONS); + } + #[test] fn mint_mod_generate_keyset_from_seed() { let seed = hex::decode("0000000000000000000000000000000000000000000000000000000000000001") diff --git a/crates/cdk/src/mint/mod.rs b/crates/cdk/src/mint/mod.rs index e4e5b4e21..cb38b7e19 100644 --- a/crates/cdk/src/mint/mod.rs +++ b/crates/cdk/src/mint/mod.rs @@ -277,24 +277,28 @@ impl Mint { // return immediately instead of being silently skipped. let mut keyset_updates = signatory.subscribe_keysets().await?; let keysets = keyset_updates.borrow_and_update().clone(); - if !keysets + let active_keys = keysets .keysets .iter() - .any(|keyset| keyset.active && keyset.unit != CurrencyUnit::Auth) - { - return Err(Error::NoActiveKeyset); + .filter(|keyset| keyset.active && keyset.unit != CurrencyUnit::Auth) + .count(); + // The signatory may not report an active keyset yet (for example a + // remote signatory that is still connecting). Start anyway and let the + // keyset drain task install keysets if they arrive over the + // subscription; endpoints return keyset errors until then. + if active_keys == 0 { + tracing::warn!( + "Signatory {} has no active keysets yet; starting without them", + signatory.name(), + ); + } else { + tracing::info!( + "Using Signatory {} with {} active keys", + signatory.name(), + active_keys, + ); } - tracing::info!( - "Using Signatory {} with {} active keys", - signatory.name(), - keysets - .keysets - .iter() - .filter(|keyset| keyset.active && keyset.unit != CurrencyUnit::Auth) - .count() - ); - // Persist missing pubkey early to avoid losing it on next boot and ensure stable identity across restarts let mut computed_info = mint_info; if computed_info.pubkey.is_none() { diff --git a/docs/adr/0003-signatory-database-persistence-only.md b/docs/adr/0003-signatory-database-persistence-only.md new file mode 100644 index 000000000..8010c0f3a --- /dev/null +++ b/docs/adr/0003-signatory-database-persistence-only.md @@ -0,0 +1,193 @@ +# Signatory database as persistence only + +* Status: accepted +* Authors: Cesar Rodas +* Date: 2026-07-27 +* Targeted modules: cdk-signatory (DbSignatory) +* Associated tickets/PRs: groundwork for cashubtc/cdk#2253 (auto-rotate + keysets on an age interval) + +## Context and Problem Statement + +`DbSignatory` owns its keys database as a single-process store and keeps the +full keyset set in memory (`keysets` and `active_keysets`, both behind an +`RwLock`). Signing, verification, and queries already read from memory, but +`rotate_keyset` did not: it re-read the active keyset from the database to +compute the next derivation index, and after committing the new keyset it called +`reload_keys_from_db`, which cleared and re-hydrated the entire in-memory set +from the database. What is the database's role relative to the in-memory state, +and where is it allowed to be touched? + +This also unblocks cashubtc/cdk#2253 (auto-rotate keysets on an age interval). +Making memory the single source of truth and having rotation update it in place, +rather than reloading from the database, means a periodic auto-rotation only has +to call `rotate_keyset`: no extra reads and no reload. + +## Decision Drivers + +* Memory is already the source of truth for every signing, verification, and + query path; only rotation still round-tripped the database. +* The database is owned by this single process. The code assumes no other + writer, so there is no external invalidation to reconcile against. +* Rotation should not re-read the whole keyset set to learn one new keyset it + just wrote itself. +* The watch-channel publish contract from ADR-0002 (snapshot on subscribe, one + per rotation) must be preserved. + +## Considered Options + +#### Database as the live store, read on every operation + +Every signing/verification/query reads keysets from the database. + +**Pros:** + +* Good, because memory can never drift from the persisted state. + +**Cons:** + +* Bad, because it puts a database read on the hot signing path for state that + never changes between rotations. +* Bad, because it discards the in-memory design the signatory already relies on. + +#### Database as a write-through cache, reloaded after each write (prior state) + +Rotation writes the database, then calls `reload_keys_from_db` to rebuild the +in-memory set from it. + +**Pros:** + +* Good, because one funnel (`reload_keys_from_db`) turns database state into + memory, so the refresh is simple. + +**Cons:** + +* Bad, because rotation reads the active keyset from the database to compute the + next index even though memory already holds it. +* Bad, because it re-reads and rebuilds every keyset to reflect a single + addition. + +#### Database as persistence only (chosen) + +Read the database once on boot, write it only when rotating, and serve +everything else (including the post-rotation refresh) from memory. + +**Pros:** + +* Good, because the hot paths never touch the database and rotation reads + nothing back. +* Good, because the database's role becomes a single clear rule. + +**Cons:** + +* Bad, because rotation must update the in-memory maps by hand instead of + leaning on a reload, so the deactivation of the prior active keyset is now + explicit code. + +## Decision Outcome + +Chosen option: "database as persistence only", because memory is already the +source of truth and the database only needs to survive restarts. + +The invariant, enforceable by `grep -n "self.localstore" +crates/cdk-signatory/src/db_signatory.rs`: + +* **Reads** hit the database only during `new`: `load_keys_from_db` (renamed + from `reload_keys_from_db`, now boot-only) plus the `init_keysets` + reactivation. Nothing else reads it. +* **Writes** hit the database only inside the `rotate_keyset` transaction + (`add_keyset_info` + `set_active_keyset` + `commit`). +* Every other operation (`blind_sign`, `verify_proofs`, `keysets`, + `subscribe_keysets`) and the post-rotation state refresh are served from and + mutate memory only. + +Rotation, after committing, updates memory to match what it just persisted +without reading the database back. It mirrors what a fresh boot would compute: +the active pointer for the unit moves to the new keyset, so the previously +active keyset for that unit is marked inactive in memory. + +```rust +// after tx.commit() +let mut info = info; +info.active = true; +let signatory_keyset: SignatoryKeySet = (&(info.clone(), keyset.clone())).into(); + +let mut keysets = self.keysets.write().await; +let mut active_keysets = self.active_keysets.write().await; + +if let Some(prev_id) = active_keysets.insert(args.unit, id) { + if let Some((prev_info, _)) = keysets.get_mut(&prev_id) { + prev_info.active = false; + } +} +keysets.insert(id, (info, keyset)); +self.publish_snapshot(&keysets); +``` + +The watch publish from ADR-0002 is preserved. The `watch::Sender::send_replace` +call moved into a `publish_snapshot` helper, now invoked from `load_keys_from_db` +(boot) and `rotate_keyset` (each rotation) instead of from a single +post-write reload. The observable contract, current snapshot on subscribe and +one per rotation, is unchanged. + +### Concurrent rotations + +`rotate_keyset` computes the next derivation index from the in-memory active +keyset for the unit, not from the database, consistent with the invariant above. +A `rotation_lock` mutex serializes rotations across the whole operation so two +concurrent rotations of the same unit cannot read the same index and derive +duplicate keysets. This matters for the standalone gRPC server, which invokes +`rotate_keyset` directly; the embedded deployment already serializes calls +through its single-runner service. + +### Positive Consequences + +* Signing, verification, and query paths never touch the database, and rotation + reads nothing back from it. +* The database's role is one rule that a single grep verifies. +* Memory remains the single source of truth; the watch contract is untouched. + +### Negative Consequences + +* Correctness depends on this process being the only writer. An out-of-band + database mutation is not observed until the next boot. +* Rotation keeps the in-memory maps and the database in lock-step by hand + (explicitly deactivating the prior active keyset) rather than relying on a + reload. +* Boot is still allowed to write: `init_keysets` reactivates the highest-index + matching keyset during `new`. This is a one-time boot concern, outside the + steady-state "write only on rotation" rule. + +## Boot load: strict, fail on construction + +Because the database is read only on boot, `new` performs that read once and +propagates any error. It runs the `init_keysets` reactivation plus +`load_keys_from_db`, and if either fails, construction fails. There is no +background retry and no partially-ready state: on success the returned signatory +is loaded and every operation serves immediately. + +An earlier iteration made boot resilient (return before the load succeeded, +retry it in the background with exponential backoff, and gate every key-using +operation on a `loaded` flag that produced a transient +`Error::KeysetsNotLoaded`). That was dropped. Both deployments, the embedded +mint and the standalone gRPC server, prefer a failed load to surface as a boot +failure rather than run a signatory without keys. Dropping it also removed the +`Arc` indirection whose only purpose was to let the retry task hold a +`Weak`; the state lives directly in `DbSignatory`'s own fields again. + +**Embedded mint.** `Mint::new_internal` treats a signatory that reports no active +keysets as a warning rather than `Error::NoActiveKeyset`, so a genuinely empty +(unconfigured) mint still starts and its endpoints return keyset errors until it +is configured. Under strict boot a configured embedded signatory is already +loaded before the mint observes it (it is built through `build_with_seed`), so +this path is reached only when the keyset set is really empty, not while it is +still loading. The `pubkey` bootstrap is unaffected: the watch snapshot always +carries `pubkey` even when the keyset list is empty. + +## Links + +* Refines [ADR-0001](0001-signatory-mint-key-segregation.md) +* Builds on [ADR-0002](0002-signatory-keyset-subscription.md): the watch publish + it defines is preserved, moved into `publish_snapshot`. +* Groundwork for [cashubtc/cdk#2253](https://github.com/cashubtc/cdk/pull/2253): + auto-rotate keysets on an age interval. diff --git a/docs/adr/README.md b/docs/adr/README.md index ff1e43d07..0008e8fe1 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -8,3 +8,4 @@ than editing an old one. |-----|-------|--------| | [0001](0001-signatory-mint-key-segregation.md) | Signatory and mint key segregation | Accepted | | [0002](0002-signatory-keyset-subscription.md) | Signatory keyset subscription and push injection | Accepted | +| [0003](0003-signatory-database-persistence-only.md) | Signatory database as persistence only | Accepted |