From 783efcb259c6a2542619261bb1cfdcb29669c47a Mon Sep 17 00:00:00 2001 From: Cesar Rodas Date: Mon, 27 Jul 2026 18:22:33 -0300 Subject: [PATCH] Signatory: serve keys from memory, load from the DB at boot The signatory round-tripped the database on the hot path: rotate_keyset re-read the active keyset to compute the next derivation index and then reloaded the whole keyset set to refresh memory. The database's role in steady-state operation was never pinned down. Treat the database as a persistence layer only. It is read once on boot and written only inside the rotate_keyset transaction; every other operation is served from and mutates in-memory state. Rotation reads the next index from memory and updates the in-memory maps directly after committing, mirroring what a fresh boot would compute, so it never reads the database back. Load strictly at boot. new runs the load once and propagates any error, so a failed load fails construction rather than leaving a signatory without keys; both the embedded mint and the standalone signatory server want a bad database to surface at startup. The in-memory state lives in DbSignatory's own fields, with no wrapper indirection. Serialize rotations. rotate_keyset holds a mutex across the whole operation, so two concurrent rotations of the same unit cannot read the same derivation index and derive duplicate keysets. This guards the standalone gRPC server, which calls rotate_keyset directly rather than through the embedded single-runner service. Relax the embedded mint boot: a signatory reporting no active keyset is a warning rather than a hard error, so an unconfigured mint still starts and its endpoints return keyset errors until it is configured. This is groundwork to simplify cashubtc/cdk#2253 (auto-rotate keysets on an age interval): with memory as the source of truth and rotation updating it in place, a periodic rotator only has to call rotate_keyset. Document the persistence model, the strict boot, and rotation serialization in ADR-0003. --- crates/cdk-signatory/src/db_signatory.rs | 231 ++++++++++++++---- crates/cdk/src/mint/mod.rs | 32 +-- ...003-signatory-database-persistence-only.md | 193 +++++++++++++++ docs/adr/README.md | 1 + 4 files changed, 391 insertions(+), 66 deletions(-) create mode 100644 docs/adr/0003-signatory-database-persistence-only.md 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 |