From 5226921267a252fa893ba5ac6403dcab49765824 Mon Sep 17 00:00:00 2001 From: Cesar Rodas Date: Mon, 27 Jul 2026 18:22:33 -0300 Subject: [PATCH 1/3] 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. --- .../src/bin/start_fake_mint.rs | 1 + crates/cdk-mintd/README.md | 15 + crates/cdk-mintd/example.config.toml | 6 + crates/cdk-mintd/src/config.rs | 15 + crates/cdk-mintd/src/config_migration.rs | 1 + crates/cdk-mintd/src/config_service.rs | 1 + crates/cdk-mintd/src/env_vars/common.rs | 2 + crates/cdk-mintd/src/env_vars/signatory.rs | 9 + crates/cdk-mintd/src/lib.rs | 14 + crates/cdk-signatory/src/bin/cli/mod.rs | 21 + crates/cdk-signatory/src/db_signatory.rs | 971 +++++++++++++++++- crates/cdk/src/mint/builder.rs | 86 +- crates/cdk/src/mint/mod.rs | 219 +++- .../0004-signatory-multi-instance-sharing.md | 22 + 14 files changed, 1352 insertions(+), 31 deletions(-) diff --git a/crates/cdk-integration-tests/src/bin/start_fake_mint.rs b/crates/cdk-integration-tests/src/bin/start_fake_mint.rs index 151d2c21a..20a9aae94 100644 --- a/crates/cdk-integration-tests/src/bin/start_fake_mint.rs +++ b/crates/cdk-integration-tests/src/bin/start_fake_mint.rs @@ -59,6 +59,7 @@ async fn start_fake_mint( port: 15060, tls_dir: Some(temp_dir.to_path_buf()), allow_insecure: false, + keyset_rotation_interval_seconds: None, }) } else { None diff --git a/crates/cdk-mintd/README.md b/crates/cdk-mintd/README.md index d508cc942..0a10173a7 100644 --- a/crates/cdk-mintd/README.md +++ b/crates/cdk-mintd/README.md @@ -316,6 +316,21 @@ cdk-mint-cli rotate-next-keyset --use-keyset-v2 true # Rotate to V2 cdk-mint-cli rotate-next-keyset --use-keyset-v2 false # Rotate to V1 ``` +**Automatic Rotation:** +An embedded signatory rotates active keysets automatically once they reach a +given age. The replacement keeps the previous amounts, input fee and version. +Meant for long periods (days); the default is 90 days. + +- **Default**: active keysets rotate once they are 90 days old (7776000 + seconds). +- `[signatory].keyset_rotation_interval_seconds = ` (or + `CDK_MINTD_SIGNATORY_KEYSET_ROTATION_INTERVAL_SECONDS=`): override the + interval. +- Set the value to `0` to disable auto-rotation. + +This applies only to an embedded signatory; a remote signatory manages its own +rotation schedule. + ## Production Examples ### With LDK Node (Recommended for Testing) diff --git a/crates/cdk-mintd/example.config.toml b/crates/cdk-mintd/example.config.toml index fc7e780a8..875a85212 100644 --- a/crates/cdk-mintd/example.config.toml +++ b/crates/cdk-mintd/example.config.toml @@ -40,6 +40,12 @@ enabled = false # tls_dir = "/path/to/tls" # allow_insecure = false +# Automatically rotate active keysets once they reach this age, in seconds. +# Applies to the embedded signatory (enabled = false); a remote signatory +# rotates on its own schedule. Defaults to 7776000 (90 days); set to 0 to +# disable auto-rotation. +# keyset_rotation_interval_seconds = 7776000 + # Optional existing management RPC for immediate mint metadata and keyset # operations. Configuration commands access the database directly. [mint_management_rpc] diff --git a/crates/cdk-mintd/src/config.rs b/crates/cdk-mintd/src/config.rs index 9db81d26f..39fe5e19c 100644 --- a/crates/cdk-mintd/src/config.rs +++ b/crates/cdk-mintd/src/config.rs @@ -137,6 +137,14 @@ pub struct Signatory { pub tls_dir: Option, #[serde(default)] pub allow_insecure: bool, + /// Automatically rotate active keysets once they reach this age, in seconds. + /// + /// Applies to the embedded signatory the mint runs when `enabled` is false. + /// Defaults to 90 days; set to `0` to disable auto-rotation. A remote + /// signatory (`enabled = true`) manages its own rotation schedule and + /// ignores this value. + #[serde(default = "default_keyset_rotation_interval_seconds")] + pub keyset_rotation_interval_seconds: Option, } impl Default for Signatory { @@ -147,6 +155,7 @@ impl Default for Signatory { port: default_signatory_port(), tls_dir: None, allow_insecure: false, + keyset_rotation_interval_seconds: default_keyset_rotation_interval_seconds(), } } } @@ -155,6 +164,12 @@ fn default_signatory_address() -> String { "127.0.0.1".to_string() } +/// Default keyset auto-rotation interval: 90 days, matching common mint +/// deployments. Set the config value to `0` to disable. +fn default_keyset_rotation_interval_seconds() -> Option { + Some(90 * 24 * 60 * 60) +} + fn default_signatory_port() -> u16 { 15060 } diff --git a/crates/cdk-mintd/src/config_migration.rs b/crates/cdk-mintd/src/config_migration.rs index f0dce3cb5..2037cdb7c 100644 --- a/crates/cdk-mintd/src/config_migration.rs +++ b/crates/cdk-mintd/src/config_migration.rs @@ -619,6 +619,7 @@ fn apply_released_v017_signatory( port, allow_insecure: tls_dir.is_none(), tls_dir, + ..Default::default() }); // Released v0.17 selected the remote signatory before either local source. // Remove ignored local material so the new mutually-exclusive model keeps diff --git a/crates/cdk-mintd/src/config_service.rs b/crates/cdk-mintd/src/config_service.rs index 7661c5da9..344ae65da 100644 --- a/crates/cdk-mintd/src/config_service.rs +++ b/crates/cdk-mintd/src/config_service.rs @@ -1396,6 +1396,7 @@ engine = "sqlite" port: 15060, tls_dir: None, allow_insecure: true, + ..Default::default() }), ..Default::default() }; diff --git a/crates/cdk-mintd/src/env_vars/common.rs b/crates/cdk-mintd/src/env_vars/common.rs index 0f75bbad6..f472c12f6 100644 --- a/crates/cdk-mintd/src/env_vars/common.rs +++ b/crates/cdk-mintd/src/env_vars/common.rs @@ -13,6 +13,8 @@ pub const ENV_SIGNATORY_ADDRESS: &str = "CDK_MINTD_SIGNATORY_ADDRESS"; pub const ENV_SIGNATORY_PORT: &str = "CDK_MINTD_SIGNATORY_PORT"; pub const ENV_SIGNATORY_TLS_DIR: &str = "CDK_MINTD_SIGNATORY_TLS_DIR"; pub const ENV_SIGNATORY_ALLOW_INSECURE: &str = "CDK_MINTD_SIGNATORY_ALLOW_INSECURE"; +pub const ENV_SIGNATORY_KEYSET_ROTATION_INTERVAL_SECONDS: &str = + "CDK_MINTD_SIGNATORY_KEYSET_ROTATION_INTERVAL_SECONDS"; pub const ENV_SECONDS_QUOTE_VALID: &str = "CDK_MINTD_SECONDS_QUOTE_VALID"; pub const ENV_CACHE_SECONDS: &str = "CDK_MINTD_CACHE_SECONDS"; pub const ENV_EXTEND_CACHE_SECONDS: &str = "CDK_MINTD_EXTEND_CACHE_SECONDS"; diff --git a/crates/cdk-mintd/src/env_vars/signatory.rs b/crates/cdk-mintd/src/env_vars/signatory.rs index e2dce5b7c..d2f83c3e1 100644 --- a/crates/cdk-mintd/src/env_vars/signatory.rs +++ b/crates/cdk-mintd/src/env_vars/signatory.rs @@ -33,6 +33,12 @@ impl Signatory { } } + if let Ok(interval_str) = env::var(ENV_SIGNATORY_KEYSET_ROTATION_INTERVAL_SECONDS) { + if let Ok(interval) = interval_str.parse() { + self.keyset_rotation_interval_seconds = Some(interval); + } + } + self } } @@ -53,6 +59,7 @@ mod tests { env::remove_var(ENV_SIGNATORY_PORT); env::remove_var(ENV_SIGNATORY_TLS_DIR); env::remove_var(ENV_SIGNATORY_ALLOW_INSECURE); + env::remove_var(ENV_SIGNATORY_KEYSET_ROTATION_INTERVAL_SECONDS); } #[test] @@ -65,6 +72,7 @@ mod tests { env::set_var(ENV_SIGNATORY_PORT, "15061"); env::set_var(ENV_SIGNATORY_TLS_DIR, "/var/lib/cdk/signatory-tls"); env::set_var(ENV_SIGNATORY_ALLOW_INSECURE, "true"); + env::set_var(ENV_SIGNATORY_KEYSET_ROTATION_INTERVAL_SECONDS, "7776000"); let signatory = Signatory::default().from_env(); @@ -76,6 +84,7 @@ mod tests { Some(PathBuf::from("/var/lib/cdk/signatory-tls")) ); assert!(signatory.allow_insecure); + assert_eq!(signatory.keyset_rotation_interval_seconds, Some(7776000)); clear_env_vars(); } diff --git a/crates/cdk-mintd/src/lib.rs b/crates/cdk-mintd/src/lib.rs index 2c1c1a010..de448635b 100644 --- a/crates/cdk-mintd/src/lib.rs +++ b/crates/cdk-mintd/src/lib.rs @@ -1186,6 +1186,19 @@ fn configure_basic_info(settings: &config::Settings, mint_builder: MintBuilder) builder = builder.with_keyset_v2(settings.info.use_keyset_v2); + // Fall back to the default interval when no `[signatory]` section is + // present, so an embedded mint auto-rotates without explicit config. + builder = builder.with_keyset_rotation_interval( + settings + .signatory + .as_ref() + .map_or_else( + || crate::config::Signatory::default().keyset_rotation_interval_seconds, + |signatory| signatory.keyset_rotation_interval_seconds, + ) + .map(std::time::Duration::from_secs), + ); + builder } /// Configures payment backends based on the specified backend types @@ -3138,6 +3151,7 @@ engine = "sqlite" port: 15060, tls_dir: Some("/tmp/certs".into()), allow_insecure: false, + keyset_rotation_interval_seconds: None, }), ..Default::default() }; diff --git a/crates/cdk-signatory/src/bin/cli/mod.rs b/crates/cdk-signatory/src/bin/cli/mod.rs index c3574eeaa..9c1c61c09 100644 --- a/crates/cdk-signatory/src/bin/cli/mod.rs +++ b/crates/cdk-signatory/src/bin/cli/mod.rs @@ -20,6 +20,7 @@ use { std::sync::Arc, std::time::Duration, std::{env, fs}, + tokio::sync::watch, tracing_subscriber::EnvFilter, }; @@ -100,6 +101,11 @@ struct Cli { /// another's rotations without a restart. #[arg(long, default_value = "0")] keyset_refresh_interval_ms: u64, + /// Automatically rotate active keysets once they reach this age, in seconds. + /// Defaults to 7776000 (90 days), matching the embedded mint. A value of 0 + /// disables auto-rotation. + #[arg(long, default_value = "7776000")] + rotation_interval_secs: u64, } /// Main function for the signatory standalone binary @@ -207,6 +213,21 @@ pub async fn cli_main() -> Result<()> { .then(|| Duration::from_millis(args.keyset_refresh_interval_ms)); signatory.spawn_keyset_refresh(refresh_interval); + // Hold the shutdown sender for the process lifetime so the rotation loop + // keeps running until the server exits; dropping it would stop rotation. + let _rotation_shutdown = if args.rotation_interval_secs > 0 { + let interval = Duration::from_secs(args.rotation_interval_secs); + tracing::info!( + "Enabling keyset auto-rotation every {}s", + args.rotation_interval_secs + ); + let (shutdown_tx, shutdown_rx) = watch::channel(false); + signatory.spawn_auto_rotation(interval, shutdown_rx); + Some(shutdown_tx) + } else { + None + }; + let socket_addr = SocketAddr::from_str(&format!("{}:{}", args.listen_addr, args.listen_port))?; start_grpc_server(signatory, socket_addr, certs).await?; diff --git a/crates/cdk-signatory/src/db_signatory.rs b/crates/cdk-signatory/src/db_signatory.rs index 6f14ad145..857446a0a 100644 --- a/crates/cdk-signatory/src/db_signatory.rs +++ b/crates/cdk-signatory/src/db_signatory.rs @@ -10,7 +10,7 @@ //! than leaving a signatory without keys. On success the returned signatory is //! loaded and serving. use std::collections::HashMap; -use std::sync::Arc; +use std::sync::{Arc, Weak}; use std::time::{Duration, SystemTime, UNIX_EPOCH}; use arc_swap::ArcSwap; @@ -20,8 +20,10 @@ use cdk_common::database::MintKeyDatabaseTransaction; 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::util::unix_time; use cdk_common::{database, Error, PublicKey}; use tokio::sync::{watch, Mutex}; +use tokio::task::JoinHandle; use tracing::instrument; use crate::common::{ @@ -29,6 +31,11 @@ use crate::common::{ }; use crate::signatory::{RotateKeyArguments, Signatory, SignatoryKeySet, SignatoryKeysets}; +/// Longest gap between auto-rotation checks. The rotation loop polls at +/// `min(interval, DEFAULT_TICKET)` so a large interval (hours/days) still wakes +/// periodically, while a small interval is not checked slower than itself. +const DEFAULT_TICKET: Duration = Duration::from_secs(120); + /// Immutable in-memory view of the keysets, swapped atomically on every change. /// /// Readers load it lock-free; the periodic refresh and local rotations build a @@ -59,11 +66,9 @@ pub struct DbSignatory { /// coordination point (advisory lock and epoch token); this only mirrors /// it. keysets: ArcSwap, - /// Serializes local keyset rotations. The standalone signatory gRPC server - /// calls rotate_keyset directly (no embedded single-runner), so two - /// concurrent local rotations of the same unit could otherwise open nested - /// transactions on a single-connection backend. Cross-process - /// serialization is the database's job. + /// Stops two local rotations from opening nested transactions on a + /// single-connection backend. Cross-process serialization is the database's + /// job. rotation_lock: Mutex<()>, localstore: Arc + Send + Sync>, secp_ctx: Secp256k1, @@ -336,6 +341,150 @@ impl DbSignatory { } } +impl DbSignatory { + /// Rotate every active keyset that has been valid for at least `max_age`. + /// + /// Each replacement keeps the previous keyset's amounts, input fee and id + /// version. When the previous keyset had a `final_expiry`, the new one is + /// pushed forward by the keyset's active age so it stays valid at least as + /// long as the keyset it replaces. + /// + /// The age check needs each keyset's `valid_from`, which does not cross the + /// `Signatory` trait, so this reads the in-memory keysets directly. + /// + /// Best effort per unit: a failure is logged and the unit keeps its current + /// keyset until the next tick. The sweep needs no coordination of its own, + /// `rotate` is race-free across processes. + async fn rotate_aged_keysets(&self, max_age: Duration) { + let now = unix_time(); + let max_age = max_age.as_secs(); + + if max_age == 0 { + return; + } + + // The due set comes from the current snapshot, which may be behind a + // peer's committed rotation. `rotate` re-checks each unit under the + // global keyset lock, so a unit a peer already rotated is skipped rather + // than rotated twice. + let due: Vec = { + let keysets = self.keysets.load(); + keysets + .active_by_unit + .values() + .filter_map(|id| keysets.by_id.get(id).map(|(info, _)| info.clone())) + .filter(|info| now.saturating_sub(info.valid_from) >= max_age) + .collect() + }; + + for info in due { + let active_age = now.saturating_sub(info.valid_from); + let final_expiry = info + .final_expiry + .map(|expiry| expiry.saturating_add(active_age)); + + tracing::info!( + "Auto-rotating keyset {} for unit {} (active for {}s, interval {}s)", + info.id, + info.unit, + active_age, + max_age + ); + + match self + .rotate( + RotateKeyArguments { + unit: info.unit.clone(), + amounts: info.amounts.clone(), + input_fee_ppk: info.input_fee_ppk, + keyset_id_type: info.id.get_version(), + final_expiry, + }, + Some(info.id), + ) + .await + { + Ok(_) => {} + // Another process sharing this database rotated the unit first. + // A benign skip, not a failure. + Err(Error::ConcurrentUpdate) => { + tracing::info!( + "Auto-rotation for unit {} skipped: another instance already rotated it", + info.unit + ); + } + Err(err) => { + tracing::error!( + "Auto-rotating keyset {} for unit {} failed: {}", + info.id, + info.unit, + err + ); + } + } + } + } + + /// Spawn the background keyset auto-rotation task. + /// + /// Every `interval` the task rotates each active keyset that has been valid + /// for at least `interval`. Rotations are published to keyset subscribers + /// through the same path as manual rotations, so mints learn about them + /// without a restart. + /// + /// A zero `interval` disables auto-rotation: the spawned task returns + /// immediately, so callers can spawn unconditionally without a panic. + /// + /// The task holds a weak reference to the signatory, so it stops on its own + /// once the signatory is dropped. Sending `true` on `shutdown` stops it + /// between sweeps, so shutdown never interrupts a rotation mid-flight. + pub fn spawn_auto_rotation( + self: &Arc, + interval: Duration, + shutdown: watch::Receiver, + ) -> JoinHandle<()> { + let weak = Arc::downgrade(self); + tokio::spawn(async move { + Self::auto_rotation_loop(weak, interval, shutdown).await; + }) + } + + async fn auto_rotation_loop( + weak: Weak, + interval: Duration, + mut shutdown: watch::Receiver, + ) { + // Auto-rotation disabled. Return before building the ticker: + // `tokio::time::interval(Duration::ZERO)` panics. + if interval.is_zero() { + return; + } + + // Capped at `DEFAULT_TICKET` so a large interval (hours/days) still + // checks periodically, floored at the interval so a small one is not + // checked slower than itself. + let mut ticker = tokio::time::interval(interval.min(DEFAULT_TICKET)); + + loop { + // `biased` checks shutdown first, so once it is signalled the loop + // exits instead of running one more sweep. `wait_for` re-checks the + // latched value, so a signal sent mid-sweep is not missed. An `Err` + // means the sender was dropped (mint gone), which also stops us. + tokio::select! { + biased; + _ = shutdown.wait_for(|stop| *stop) => break, + _ = ticker.tick() => {} + } + + let Some(signatory) = weak.upgrade() else { + break; + }; + + signatory.rotate_aged_keysets(interval).await; + } + } +} + #[async_trait::async_trait] impl Signatory for DbSignatory { fn name(&self) -> String { @@ -412,11 +561,27 @@ impl Signatory for DbSignatory { /// Generate new keyset #[tracing::instrument(skip(self))] async fn rotate_keyset(&self, args: RotateKeyArguments) -> Result { - // Serialize local rotations. The standalone signatory gRPC server - // invokes this directly (no embedded single-runner), so without this two - // concurrent local rotations could open nested transactions on a - // single-connection backend. Held for the whole method. Cross-process - // rotations are serialized by the global keyset lock in the database. + // No `expected_prev`: a mint-initiated rotation adopts whatever the + // database says is active, rather than failing because a peer rotated + // first. + self.rotate(args, None).await + } +} + +impl DbSignatory { + /// Rotate a single keyset for `args.unit`. + /// + /// `expected_prev` is the keyset the caller decided to replace. When set, + /// the rotation is abandoned with [`Error::ConcurrentUpdate`] if the unit's + /// active keyset is no longer that one by the time the rotation transaction + /// holds the global keyset lock. Only the auto-rotation sweep passes it: its + /// due-check reads a process-local snapshot, so without the re-check a peer's + /// rotation would be followed by a redundant second one. + async fn rotate( + &self, + args: RotateKeyArguments, + expected_prev: Option, + ) -> Result { let _rotation = self.rotation_lock.lock().await; // Persist the rotation. This is the only path that writes to the @@ -433,6 +598,15 @@ impl Signatory for DbSignatory { // committed rotations rather than a possibly-stale in-memory snapshot. self.reload_from_tx(&mut *tx).await?; + // The reload above is authoritative under the global lock, so this sees + // any peer rotation that beat us. Return before writing anything: the + // transaction rolls back and the caller skips the unit. + if let Some(prev) = expected_prev { + if self.keysets.load().active_by_unit.get(&args.unit) != Some(&prev) { + return Err(Error::ConcurrentUpdate); + } + } + // Default amounts come from the in-memory active keyset and are only // used when the caller does not specify any. The authoritative // derivation index is allocated from the database above, not memory, so @@ -502,13 +676,17 @@ impl Signatory for DbSignatory { #[cfg(test)] mod test { use std::collections::HashSet; + use std::sync::Mutex; use bitcoin::key::Secp256k1; use bitcoin::Network; - use cdk_common::database::MintKeysDatabase; + use cdk_common::database::{ + DbTransactionFinalizer, MintKeyDatabaseTransaction, MintKeysDatabase, + }; use cdk_common::nuts::SecretKey; use cdk_common::util::{hex, unix_time}; use cdk_common::{Amount, MintKeySet, PublicKey}; + use cdk_sqlite::mint::MintSqliteDatabase; use super::*; @@ -1550,4 +1728,773 @@ mod test { "025b6c1ca8bb741a6f2321c953266df7bf3f3f2c3be8c54c0a6e41bb00976046a4".to_string() ); } + + #[tokio::test] + async fn rotate_aged_keysets_respects_age() { + let signatory = test_signatory(b"test-seed-for-aged-rotation").await; + + // Seed one keyset aged 120 seconds. + let seeded = seed_aged_keyset( + &signatory, + CurrencyUnit::Sat, + &[1, 2, 4, 8], + 0, + cdk_common::nut02::KeySetVersion::Version00, + None, + 120, + ) + .await; + + // An interval larger than the keyset's age leaves it in place. + signatory + .rotate_aged_keysets(Duration::from_secs(600)) + .await; + assert_eq!( + memory_active_keyset_id(&signatory, &CurrencyUnit::Sat), + Some(seeded), + "keyset younger than the interval must not rotate" + ); + + // An interval below the keyset's age makes it due, so it rotates. + signatory + .rotate_aged_keysets(Duration::from_secs(60)) + .await; + assert_ne!( + memory_active_keyset_id(&signatory, &CurrencyUnit::Sat), + Some(seeded), + "keyset at or past the interval must rotate" + ); + } + + #[tokio::test] + async fn spawn_auto_rotation_pushes_new_keyset() { + let signatory = test_signatory(b"test-seed-for-auto-rotation").await; + + // Seed an already-aged active Sat keyset for the task to rotate. + seed_aged_keyset( + &signatory, + CurrencyUnit::Sat, + &[1, 2, 4, 8], + 0, + cdk_common::nut02::KeySetVersion::Version00, + None, + 120, + ) + .await; + + let mut updates = signatory.subscribe_keysets().await.expect("subscribe"); + let before = updates.borrow_and_update().keysets.len(); + + // The seeded keyset is older than this one second interval, so the + // first (immediate) tick rotates it. Keep the shutdown sender alive so + // the loop is not asked to stop. + let (_shutdown_tx, shutdown_rx) = watch::channel(false); + let _handle = signatory.spawn_auto_rotation(Duration::from_secs(1), shutdown_rx); + + tokio::time::timeout(Duration::from_secs(5), updates.changed()) + .await + .expect("auto rotation should push within timeout") + .expect("keyset update"); + + let after = updates.borrow_and_update().keysets.len(); + assert!( + after > before, + "auto rotation should add a keyset ({after} > {before})" + ); + } + + /// Build an in-memory signatory with no keysets. + async fn test_signatory(seed: &[u8]) -> Arc { + let store = Arc::new( + cdk_sqlite::mint::memory::empty() + .await + .expect("in-memory db"), + ); + Arc::new( + DbSignatory::new(store, seed, Default::default(), Default::default()) + .await + .expect("DbSignatory::new"), + ) + } + + /// Keys database that wraps a real one and fails `swap_active_keyset` for a + /// single armed unit, to exercise a mid-sweep rotation failure. The armed + /// unit is settable so seeding runs before failure is armed. + struct FailUnitDb { + inner: MintSqliteDatabase, + fail_unit: Arc>>, + } + + struct FailUnitTx<'a> { + inner: Box + Send + Sync + 'a>, + fail_unit: Arc>>, + } + + #[async_trait::async_trait] + impl DbTransactionFinalizer for FailUnitTx<'_> { + type Err = database::Error; + + async fn commit(self: Box) -> Result<(), Self::Err> { + self.inner.commit().await + } + + async fn rollback(self: Box) -> Result<(), Self::Err> { + self.inner.rollback().await + } + } + + #[async_trait::async_trait] + impl<'a> MintKeyDatabaseTransaction<'a, database::Error> for FailUnitTx<'a> { + async fn set_active_keyset( + &mut self, + unit: CurrencyUnit, + id: Id, + ) -> Result<(), database::Error> { + // Every rotation reassigns the active pointer, so injecting here + // fails the whole rotation transaction. An Internal error, not a + // `ConcurrentUpdate`, so the sweep treats it as a real failure rather + // than a benign lost race. + if self.fail_unit.lock().expect("lock").as_ref() == Some(&unit) { + return Err(database::Error::Internal( + "simulated set_active_keyset failure".to_string(), + )); + } + self.inner.set_active_keyset(unit, id).await + } + + async fn add_keyset_info(&mut self, keyset: MintKeySetInfo) -> Result<(), database::Error> { + self.inner.add_keyset_info(keyset).await + } + + async fn next_derivation_index( + &mut self, + unit: &CurrencyUnit, + ) -> Result { + self.inner.next_derivation_index(unit).await + } + + async fn get_keyset_infos_by_unit( + &mut self, + unit: &CurrencyUnit, + ) -> Result, database::Error> { + self.inner.get_keyset_infos_by_unit(unit).await + } + + async fn get_active_keysets( + &mut self, + ) -> Result, database::Error> { + self.inner.get_active_keysets().await + } + + async fn get_keyset_infos(&mut self) -> Result, database::Error> { + self.inner.get_keyset_infos().await + } + + async fn keysets_epoch(&mut self) -> Result { + self.inner.keysets_epoch().await + } + } + + #[async_trait::async_trait] + impl MintKeysDatabase for FailUnitDb { + type Err = database::Error; + + async fn begin_transaction<'a>( + &'a self, + ) -> Result< + Box + Send + Sync + 'a>, + database::Error, + > { + Ok(Box::new(FailUnitTx { + inner: self.inner.begin_transaction().await?, + fail_unit: self.fail_unit.clone(), + })) + } + + async fn keysets_epoch(&self) -> Result { + self.inner.keysets_epoch().await + } + } + + /// Return the single active keyset for `unit`, panicking if there isn't one. + async fn active_keyset(sig: &DbSignatory, unit: &CurrencyUnit) -> SignatoryKeySet { + sig.keysets() + .await + .expect("keysets") + .keysets + .into_iter() + .find(|k| k.active && &k.unit == unit) + .expect("active keyset for unit") + } + + /// Return the unit's active keyset id as recorded in the database, bypassing + /// the signatory's in-memory snapshot. Keyset reads go through a transaction, + /// so this opens one. + async fn db_active_keyset_id(sig: &DbSignatory, unit: &CurrencyUnit) -> Option { + let mut tx = sig.localstore.begin_transaction().await.expect("begin tx"); + let active = tx.get_active_keysets().await.expect("active keysets"); + tx.commit().await.expect("commit"); + active.get(unit).copied() + } + + /// Return the unit's active keyset id as the signatory currently sees it. + fn memory_active_keyset_id(sig: &DbSignatory, unit: &CurrencyUnit) -> Option { + sig.keysets.load().active_by_unit.get(unit).copied() + } + + /// Seed an active keyset for `unit` whose `valid_from` is backdated by + /// `age` seconds, so `rotate_aged_keysets` treats it as due for any + /// interval below `age`. Returns the seeded keyset id. + async fn seed_aged_keyset( + sig: &DbSignatory, + unit: CurrencyUnit, + amounts: &[u64], + input_fee_ppk: u64, + version: cdk_common::nut02::KeySetVersion, + final_expiry: Option, + age: u64, + ) -> Id { + let derivation_path = derivation_path_from_unit(unit.clone(), 1).expect("derivation path"); + let (keyset, mut info) = create_new_keyset( + &sig.secp_ctx, + sig.xpriv, + derivation_path, + Some(1), + unit.clone(), + amounts, + input_fee_ppk, + final_expiry, + version, + ); + // Backdate the keyset so it reads as aged without waiting. + info.valid_from = unix_time() - age; + let id = keyset.id; + + let mut tx = sig.localstore.begin_transaction().await.expect("begin tx"); + tx.add_keyset_info(info).await.expect("add keyset info"); + tx.set_active_keyset(unit, id) + .await + .expect("set active keyset"); + tx.commit().await.expect("commit"); + sig.load_keys_from_db().await.expect("reload"); + + id + } + + /// Backdate the active keyset for `unit` by `age` seconds so the next + /// `rotate_aged_keysets` treats it as due, then reload. Used to trigger a + /// second rotation after the first one produced a fresh (age zero) keyset. + async fn age_active_keyset(sig: &DbSignatory, unit: &CurrencyUnit, age: u64) { + let mut tx = sig.localstore.begin_transaction().await.expect("begin tx"); + let active_id = *tx + .get_active_keysets() + .await + .expect("active keysets") + .get(unit) + .expect("active keyset for unit"); + let mut info = tx + .get_keyset_infos_by_unit(unit) + .await + .expect("keyset infos") + .into_iter() + .find(|info| info.id == active_id) + .expect("keyset info present"); + info.valid_from = unix_time() - age; + + tx.add_keyset_info(info).await.expect("update keyset info"); + tx.commit().await.expect("commit"); + sig.load_keys_from_db().await.expect("reload"); + } + + async fn assert_rotation_preserves_metadata(version: cdk_common::nut02::KeySetVersion) { + let sig = test_signatory(b"test-seed-preserve").await; + let amounts = vec![1, 2, 4, 8, 16]; + let fee = 100; + + seed_aged_keyset(&sig, CurrencyUnit::Sat, &amounts, fee, version, None, 120).await; + let original = active_keyset(&sig, &CurrencyUnit::Sat).await; + + sig.rotate_aged_keysets(Duration::from_secs(60)) + .await; + + let rotated = active_keyset(&sig, &CurrencyUnit::Sat).await; + assert_ne!(rotated.id, original.id, "a new keyset must be created"); + assert_eq!(rotated.amounts, amounts, "amounts must be preserved"); + assert_eq!(rotated.input_fee_ppk, fee, "input fee must be preserved"); + assert_eq!( + rotated.final_expiry, None, + "a keyset without a final_expiry rotates into one without a final_expiry" + ); + assert_eq!( + rotated.id.get_version(), + version, + "keyset id version must be preserved" + ); + assert_eq!( + rotated.version, + original.version + 1, + "derivation index must increment on rotation" + ); + } + + #[tokio::test] + async fn auto_rotation_preserves_amounts_fee_and_version_v1() { + assert_rotation_preserves_metadata(cdk_common::nut02::KeySetVersion::Version00).await; + } + + #[tokio::test] + async fn auto_rotation_preserves_amounts_fee_and_version_v2() { + assert_rotation_preserves_metadata(cdk_common::nut02::KeySetVersion::Version01).await; + } + + #[tokio::test] + async fn rotate_aged_keysets_pushes_final_expiry_forward() { + let sig = test_signatory(b"test-seed-final-expiry").await; + let age = 100; + // Far enough in the future that the keyset is not treated as expired. + let expiry = unix_time() + 10_000; + + seed_aged_keyset( + &sig, + CurrencyUnit::Sat, + &[1, 2, 4, 8], + 0, + cdk_common::nut02::KeySetVersion::Version00, + Some(expiry), + age, + ) + .await; + + sig.rotate_aged_keysets(Duration::from_secs(60)) + .await; + + let rotated = active_keyset(&sig, &CurrencyUnit::Sat).await; + let bumped = rotated + .final_expiry + .expect("rotated keyset carries a final_expiry"); + // The new expiry is the old one pushed forward by the active age, which + // is at least `age`. Use `>=` since the clock may tick during the test. + assert!( + bumped >= expiry + age, + "final_expiry must be pushed forward by the active age (got {bumped}, expected >= {})", + expiry + age + ); + } + + #[tokio::test] + async fn rotate_aged_keysets_noop_without_active_keysets() { + let sig = test_signatory(b"test-seed-noop").await; + + let before = sig.keysets().await.expect("keysets").keysets.len(); + sig.rotate_aged_keysets(Duration::ZERO) + .await; + let after = sig.keysets().await.expect("keysets").keysets.len(); + + assert_eq!(before, 0, "fresh signatory has no keysets"); + assert_eq!(after, before, "no active keysets means nothing to rotate"); + } + + #[tokio::test] + async fn rotate_aged_keysets_ignores_inactive_keysets() { + let sig = test_signatory(b"test-seed-inactive").await; + + seed_aged_keyset( + &sig, + CurrencyUnit::Sat, + &[1, 2, 4, 8], + 0, + cdk_common::nut02::KeySetVersion::Version00, + None, + 120, + ) + .await; + + let total = |ks: &SignatoryKeysets| ks.keysets.len(); + let active_sat = |ks: &SignatoryKeysets| { + ks.keysets + .iter() + .filter(|k| k.active && k.unit == CurrencyUnit::Sat) + .count() + }; + + let after_seed = sig.keysets().await.expect("keysets"); + assert_eq!(total(&after_seed), 1); + assert_eq!(active_sat(&after_seed), 1); + + sig.rotate_aged_keysets(Duration::from_secs(60)) + .await; + let after_first = sig.keysets().await.expect("keysets"); + assert_eq!( + total(&after_first), + 2, + "one rotation adds exactly one keyset" + ); + assert_eq!(active_sat(&after_first), 1, "exactly one active Sat keyset"); + + // The first rotation left a fresh active keyset (age zero). Age it so + // the next pass finds it due. The keyset the first pass retired stays + // inactive and aged, so if inactive keysets were re-rotated the count + // would grow by more than one. + age_active_keyset(&sig, &CurrencyUnit::Sat, 120).await; + + sig.rotate_aged_keysets(Duration::from_secs(60)) + .await; + let after_second = sig.keysets().await.expect("keysets"); + assert_eq!( + total(&after_second), + 3, + "second rotation adds exactly one more; inactive keysets are not re-rotated" + ); + assert_eq!( + active_sat(&after_second), + 1, + "still exactly one active Sat keyset" + ); + } + + #[tokio::test] + async fn rotate_aged_keysets_rotates_all_aged_units() { + let sig = test_signatory(b"test-seed-multi-unit").await; + + let sat = seed_aged_keyset( + &sig, + CurrencyUnit::Sat, + &[1, 2, 4, 8], + 0, + cdk_common::nut02::KeySetVersion::Version00, + None, + 120, + ) + .await; + let usd = seed_aged_keyset( + &sig, + CurrencyUnit::Usd, + &[1, 2, 4, 8], + 0, + cdk_common::nut02::KeySetVersion::Version00, + None, + 120, + ) + .await; + + sig.rotate_aged_keysets(Duration::from_secs(60)) + .await; + + let new_sat = active_keyset(&sig, &CurrencyUnit::Sat).await; + let new_usd = active_keyset(&sig, &CurrencyUnit::Usd).await; + assert_ne!(new_sat.id, sat, "Sat keyset should rotate"); + assert_ne!(new_usd.id, usd, "Usd keyset should rotate"); + } + + #[tokio::test] + async fn rotate_aged_keysets_continues_after_one_unit_fails() { + // One unit's rotation failing must not starve the others: the sweep + // rotates every other due unit and only the failed unit is left on its + // current keyset, to be retried on the next tick. + let fail_unit = Arc::new(Mutex::new(None)); + let store = Arc::new(FailUnitDb { + inner: cdk_sqlite::mint::memory::empty() + .await + .expect("in-memory db"), + fail_unit: fail_unit.clone(), + }); + let sig = Arc::new( + DbSignatory::new( + store, + b"test-seed-partial-fail", + Default::default(), + Default::default(), + ) + .await + .expect("DbSignatory::new"), + ); + + // Seed two aged units while failure is disarmed. + let sat = seed_aged_keyset( + &sig, + CurrencyUnit::Sat, + &[1, 2, 4, 8], + 0, + cdk_common::nut02::KeySetVersion::Version00, + None, + 120, + ) + .await; + let usd = seed_aged_keyset( + &sig, + CurrencyUnit::Usd, + &[1, 2, 4, 8], + 0, + cdk_common::nut02::KeySetVersion::Version00, + None, + 120, + ) + .await; + + // Arm failure for Usd only, then sweep. + *fail_unit.lock().expect("lock") = Some(CurrencyUnit::Usd); + sig.rotate_aged_keysets(Duration::from_secs(60)).await; + + // Sat rotated despite Usd failing (order within the sweep is + // nondeterministic, so this proves the sweep did not abort early). + let new_sat = active_keyset(&sig, &CurrencyUnit::Sat).await; + assert_ne!(new_sat.id, sat, "Sat rotates even though Usd failed"); + + // Usd unchanged in memory and in the DB: its transaction rolled back. + let cur_usd = active_keyset(&sig, &CurrencyUnit::Usd).await; + assert_eq!( + cur_usd.id, usd, + "failed unit keeps its previous active keyset" + ); + assert_eq!( + db_active_keyset_id(&sig, &CurrencyUnit::Usd).await, + Some(usd), + "DB active pointer for the failed unit is unchanged", + ); + } + + #[tokio::test] + async fn spawn_auto_rotation_stops_when_signatory_dropped() { + let sig = test_signatory(b"test-seed-drop").await; + // Keep the shutdown sender alive so the loop can only exit through the + // dropped-signatory path, not a shutdown signal. + let (_shutdown_tx, shutdown_rx) = watch::channel(false); + let handle = sig.spawn_auto_rotation(Duration::from_millis(50), shutdown_rx); + + // Drop the only strong reference; the task's weak upgrade then fails and + // the loop exits on its next tick. + drop(sig); + + tokio::time::timeout(Duration::from_secs(5), handle) + .await + .expect("auto rotation task should stop after the signatory is dropped") + .expect("task should not panic"); + } + + #[tokio::test] + async fn spawn_auto_rotation_stops_on_shutdown_signal() { + // The signatory stays alive; only the shutdown signal ends the loop. + let sig = test_signatory(b"test-seed-shutdown").await; + let (shutdown_tx, shutdown_rx) = watch::channel(false); + let handle = sig.spawn_auto_rotation(Duration::from_millis(50), shutdown_rx); + + shutdown_tx.send(true).expect("receiver alive"); + + tokio::time::timeout(Duration::from_secs(5), handle) + .await + .expect("auto rotation task should stop after shutdown is signalled") + .expect("task should not panic"); + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 4)] + async fn concurrent_rotations_all_land_in_memory() { + // Consistency guard for concurrent rotations: several run at once on + // distinct units (as auto-rotation and a mint-initiated rotate might). + // `rotate_keyset` serializes them through `rotation_lock`, commits each + // to the DB, and updates memory in place. After they settle, the + // in-memory map and the published watch snapshot must both equal the + // full DB set, with no keyset lost to a race. + let sig = test_signatory(b"test-seed-concurrent-rotations").await; + + // Distinct units so each rotation creates its own keyset without + // contending on a shared unit's derivation index. + let units: Vec = (0..12) + .map(|i| CurrencyUnit::custom(format!("UNIT{i}"))) + .collect(); + + let mut handles = Vec::new(); + for unit in units.iter().cloned() { + let sig = Arc::clone(&sig); + handles.push(tokio::spawn(async move { + sig.rotate_keyset(RotateKeyArguments { + unit, + 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"); + })); + } + for handle in handles { + handle.await.expect("rotation task should not panic"); + } + + let db_ids: HashSet = { + let mut tx = sig.localstore.begin_transaction().await.expect("begin tx"); + let infos = tx.get_keyset_infos().await.expect("keyset infos"); + tx.commit().await.expect("commit"); + infos.into_iter().map(|info| info.id).collect() + }; + assert_eq!( + db_ids.len(), + units.len(), + "every rotation committed a keyset" + ); + + let memory_ids: HashSet = sig + .keysets() + .await + .expect("keysets") + .keysets + .into_iter() + .map(|k| k.id) + .collect(); + assert_eq!( + memory_ids, db_ids, + "in-memory keysets must match the DB after concurrent rotations" + ); + + let watch_ids: HashSet = sig + .subscribe_keysets() + .await + .expect("subscribe") + .borrow() + .keysets + .iter() + .map(|k| k.id) + .collect(); + assert_eq!( + watch_ids, db_ids, + "published keyset snapshot must match the DB after concurrent rotations" + ); + } + + /// The auto-rotation sweep picks its due set from a process-local snapshot, + /// so a peer can rotate the unit before the sweep gets to it. The + /// `expected_prev` re-check inside the rotation transaction must catch that + /// and abandon the rotation rather than rotate a second time. + #[tokio::test] + async fn rotate_skips_unit_a_peer_already_rotated() { + let sig = test_signatory(b"test-seed-peer-rotation-skip").await; + + // The keyset the sweep would decide is due. + let seeded = seed_aged_keyset( + &sig, + CurrencyUnit::Sat, + &[1, 2, 4, 8], + 0, + cdk_common::nut02::KeySetVersion::Version00, + None, + 120, + ) + .await; + + // A peer rotates the unit: move the DB active pointer without touching + // this signatory's snapshot, exactly what a second instance sharing the + // database would leave behind. + let external_path = + derivation_path_from_unit(CurrencyUnit::Sat, 99).expect("derivation path"); + let (external_keyset, mut external_info) = create_new_keyset( + &sig.secp_ctx, + sig.xpriv, + external_path, + Some(99), + CurrencyUnit::Sat, + &[1, 2, 4, 8], + 0, + None, + cdk_common::nut02::KeySetVersion::Version00, + ); + external_info.valid_from = unix_time(); + let external_id = external_keyset.id; + let mut tx = sig.localstore.begin_transaction().await.expect("begin tx"); + tx.add_keyset_info(external_info).await.expect("add keyset"); + tx.set_active_keyset(CurrencyUnit::Sat, external_id) + .await + .expect("set active"); + tx.commit().await.expect("commit"); + + let result = sig + .rotate( + 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, + }, + Some(seeded), + ) + .await; + assert!( + matches!(result, Err(Error::ConcurrentUpdate)), + "a unit the peer already rotated must be skipped, got {result:?}" + ); + + // The peer's keyset stands, in the database and in memory: the abandoned + // rotation neither committed nor left a half-applied snapshot. + assert_eq!( + db_active_keyset_id(&sig, &CurrencyUnit::Sat).await, + Some(external_id), + "the peer's active pointer must survive the abandoned rotation", + ); + assert_eq!( + memory_active_keyset_id(&sig, &CurrencyUnit::Sat), + Some(external_id), + "the abandoned rotation still adopts the peer's keyset it read under the lock" + ); + } + + /// A mint-initiated rotation passes no `expected_prev`, so it adopts a peer's + /// keyset instead of failing. + #[tokio::test] + async fn rotate_keyset_succeeds_when_a_peer_rotated_first() { + let sig = test_signatory(b"test-seed-peer-rotation-adopt").await; + + seed_aged_keyset( + &sig, + CurrencyUnit::Sat, + &[1, 2, 4, 8], + 0, + cdk_common::nut02::KeySetVersion::Version00, + None, + 120, + ) + .await; + + let external_path = + derivation_path_from_unit(CurrencyUnit::Sat, 99).expect("derivation path"); + let (external_keyset, mut external_info) = create_new_keyset( + &sig.secp_ctx, + sig.xpriv, + external_path, + Some(99), + CurrencyUnit::Sat, + &[1, 2, 4, 8], + 0, + None, + cdk_common::nut02::KeySetVersion::Version00, + ); + external_info.valid_from = unix_time(); + let external_id = external_keyset.id; + let mut tx = sig.localstore.begin_transaction().await.expect("begin tx"); + tx.add_keyset_info(external_info).await.expect("add keyset"); + tx.set_active_keyset(CurrencyUnit::Sat, external_id) + .await + .expect("set active"); + tx.commit().await.expect("commit"); + + let rotated = sig + .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"); + + assert_ne!( + rotated.id, external_id, + "the rotation creates a keyset of its own" + ); + assert_eq!( + memory_active_keyset_id(&sig, &CurrencyUnit::Sat), + Some(rotated.id), + "the new keyset becomes active" + ); + } } diff --git a/crates/cdk/src/mint/builder.rs b/crates/cdk/src/mint/builder.rs index 11a021d1f..9aebf1521 100644 --- a/crates/cdk/src/mint/builder.rs +++ b/crates/cdk/src/mint/builder.rs @@ -18,7 +18,7 @@ use super::nut19::{self, CachedEndpoint}; use super::Nuts; use crate::amount::Amount; use crate::cdk_database; -use crate::mint::Mint; +use crate::mint::{Mint, RotationSpawner}; use crate::nuts::{ AuthRequired, ContactInfo, CurrencyUnit, MeltMethodSettings, MintInfo, MintMethodSettings, MintVersion, MppMethodSettings, PaymentMethod, ProtectedEndpoint, @@ -72,6 +72,12 @@ pub struct MintBuilder { custom_paths: HashMap, use_keyset_v2: Option, keyset_rotations: Vec, + keyset_rotation_interval: Option, + /// Rotation spawner for the embedded auto-rotation loop, built in + /// `build_with_seed` and handed to the `Mint` in `build_with_signatory` so + /// that `Mint::start` can spawn (and re-spawn) it. `None` when no rotation + /// interval is configured. + rotation_spawner: Option, max_inputs: usize, max_outputs: usize, max_batch_size: Option, @@ -120,6 +126,8 @@ impl MintBuilder { custom_paths: HashMap::new(), use_keyset_v2: None, keyset_rotations: Vec::new(), + keyset_rotation_interval: None, + rotation_spawner: None, max_inputs: 1000, max_outputs: 1000, max_batch_size: None, @@ -150,6 +158,21 @@ impl MintBuilder { self } + /// Automatically rotate active keysets once they reach `interval`. + /// + /// Only applies to the embedded signatory built through + /// [`MintBuilder::build_with_seed`]. A `None` value, or an interval of zero, + /// leaves auto-rotation disabled. A remote signatory manages its own + /// rotation schedule. + /// + /// The rotation loop is spawned by [`Mint::start`] and halted by + /// [`Mint::stop`], resuming on a later `start()` like the other background + /// services. + pub fn with_keyset_rotation_interval(mut self, interval: Option) -> Self { + self.keyset_rotation_interval = interval.filter(|i| !i.is_zero()); + self + } + /// Set clear auth settings pub fn with_auth( mut self, @@ -598,9 +621,13 @@ impl MintBuilder { /// Build the mint with the provided signatory pub async fn build_with_signatory( - #[allow(unused_mut)] mut self, + mut self, signatory: Arc, ) -> Result { + // Taken now so the field is not caught in the piecemeal moves of `self` + // into the `Mint` constructors below. + let rotation_spawner = self.rotation_spawner.take(); + // Check active keysets and rotate if necessary let active_keysets = signatory.keysets().await?; @@ -703,7 +730,7 @@ impl MintBuilder { )); } - if let Some(auth_localstore) = self.auth_localstore { + let mint = if let Some(auth_localstore) = self.auth_localstore { let mut protected_endpoints = HashMap::new(); for endpoint in self.clear_auth_endpoints { protected_endpoints.insert(endpoint, AuthRequired::Clear); @@ -718,7 +745,7 @@ impl MintBuilder { tx.commit().await?; } - return Mint::new_with_auth( + Mint::new_with_auth( self.mint_info, signatory, self.localstore, @@ -727,25 +754,36 @@ impl MintBuilder { self.max_inputs, self.max_outputs, ) - .await; + .await? + } else { + Mint::new( + self.mint_info, + signatory, + self.localstore, + self.payment_processors, + self.max_inputs, + self.max_outputs, + ) + .await? + }; + + // Bind the embedded auto-rotation spawner to the mint so `start()` runs + // it and `stop()` halts it cooperatively. + if let Some(spawner) = rotation_spawner { + mint.set_rotation_spawner(spawner).await; } - Mint::new( - self.mint_info, - signatory, - self.localstore, - self.payment_processors, - self.max_inputs, - self.max_outputs, - ) - .await + + Ok(mint) } /// Build the mint with the provided keystore and seed pub async fn build_with_seed( - self, + mut self, keystore: Arc + Send + Sync>, seed: &[u8], ) -> Result { + // Wrapped in an `Arc` so the auto-rotation spawner below can hold a weak + // handle without keeping the signatory alive. let in_memory_signatory = Arc::new( cdk_signatory::db_signatory::DbSignatory::new( keystore, @@ -762,6 +800,24 @@ impl MintBuilder { // instance is picked up on the next refresh without a restart. in_memory_signatory.spawn_keyset_refresh(self.keyset_refresh_interval); + if let Some(interval) = self.keyset_rotation_interval { + tracing::info!( + "Enabling keyset auto-rotation every {}s", + interval.as_secs() + ); + // Capture a weak handle so the spawner does not keep the signatory + // alive; the embedded `Service` owns the only strong reference. + // `Mint::start` calls this to spawn the loop (and re-spawn it after a + // `stop()`); if the signatory has been dropped the loop has nothing + // to rotate, so spawn a no-op. + let weak = Arc::downgrade(&in_memory_signatory); + let spawner: RotationSpawner = Arc::new(move |shutdown_rx| match weak.upgrade() { + Some(signatory) => signatory.spawn_auto_rotation(interval, shutdown_rx), + None => tokio::spawn(async {}), + }); + self.rotation_spawner = Some(spawner); + } + let signatory = Arc::new(cdk_signatory::embedded::Service::new(in_memory_signatory)); self.build_with_signatory(signatory).await diff --git a/crates/cdk/src/mint/mod.rs b/crates/cdk/src/mint/mod.rs index f47a66a50..1d752fa99 100644 --- a/crates/cdk/src/mint/mod.rs +++ b/crates/cdk/src/mint/mod.rs @@ -103,6 +103,18 @@ impl std::fmt::Debug for Mint { } } +/// Factory that spawns the embedded signatory's keyset auto-rotation loop. +/// +/// It captures a weak handle to the concrete embedded signatory plus the +/// interval; each call spawns a fresh loop wired to the given shutdown receiver. +/// The mint keeps it so `start()` can spawn the loop and re-spawn it after a +/// `stop()`, keeping rotation symmetric with the other background services. The +/// concrete signatory is only reachable at build time (not through the +/// `Signatory` trait the mint holds), so the closure is what carries that reach +/// into `start()`. +pub(crate) type RotationSpawner = + Arc) -> JoinHandle<()> + Send + Sync>; + /// State for managing background tasks #[derive(Default)] struct TaskState { @@ -115,6 +127,16 @@ struct TaskState { /// Keyset subscription retained from construction, drained once by the first /// `start()`. `None` after it has been taken; a restart re-subscribes. keyset_updates: Option>, + /// Factory to spawn the embedded auto-rotation loop, set once at build time + /// and kept across restarts so `start()` can (re)spawn it. `None` when no + /// embedded rotation interval is configured. + rotation_spawner: Option, + /// The running auto-rotation task, if started: the `watch::Sender` signals + /// the loop to stop and the `JoinHandle` lets `stop()` await an in-flight + /// rotation to completion before returning, so rotation is never aborted + /// mid-flight. Cleared by `stop()` and re-created by the next `start()` from + /// `rotation_spawner`. + rotation_handle: Option<(watch::Sender, JoinHandle<()>)>, } /// Supervised subscription to a single payment processor's event stream. @@ -410,6 +432,15 @@ impl Mint { }) } + /// Bind the embedded signatory's rotation spawner to this mint. [`Mint::start`] + /// uses it to spawn the auto-rotation loop and to re-spawn it after a + /// [`Mint::stop`]; `stop()` halts the running loop cooperatively. Called once + /// at build time when an embedded signatory is configured with a rotation + /// interval. + pub(crate) async fn set_rotation_spawner(&self, spawner: RotationSpawner) { + self.task_state.lock().await.rotation_spawner = Some(spawner); + } + /// Start the mint's background services and operations /// /// This function immediately starts background services and returns. The background @@ -569,6 +600,18 @@ impl Mint { None }; + // Spawn embedded keyset auto-rotation, if configured and not already + // running. A fresh shutdown channel each start means a later + // stop()/start() cycle resumes rotation, like the other background + // services. + if task_state.rotation_handle.is_none() { + if let Some(spawner) = task_state.rotation_spawner.clone() { + let (shutdown_tx, shutdown_rx) = watch::channel(false); + let handle = spawner(shutdown_rx); + task_state.rotation_handle = Some((shutdown_tx, handle)); + } + } + // Store the handles task_state.shutdown_notify = Some(shutdown_notify); task_state.supervisor_handle = Some(supervisor_handle); @@ -586,6 +629,11 @@ impl Mint { /// This function signals all background tasks to shut down and waits for them /// to complete gracefully. It's safe to call multiple times. /// + /// Embedded keyset auto-rotation (configured via + /// [`MintBuilder::with_keyset_rotation_interval`]) is halted here + /// cooperatively, letting an in-flight rotation finish, and is resumed by a + /// later [`Mint::start`] like the other background services. + /// /// # Returns /// /// Returns `Ok(())` when all background services have shut down cleanly, or an @@ -593,11 +641,31 @@ impl Mint { pub async fn stop(&self) -> Result<(), Error> { let mut task_state = self.task_state.lock().await; - // Take the handles out of the state + // Take the handles out of the state. Leave `rotation_spawner` in place + // so the next `start()` can re-spawn rotation. + let rotation_handle = task_state.rotation_handle.take(); let shutdown_notify = task_state.shutdown_notify.take(); let supervisor_handle = task_state.supervisor_handle.take(); let keyset_drain_handle = task_state.keyset_drain_handle.take(); + // Drop the lock before awaiting any task. + drop(task_state); + + // Halt embedded keyset auto-rotation cooperatively, if running. Signal + // the loop and await it so an in-flight rotation finishes rather than + // being aborted mid-rotation. Done before the early-return below so + // rotation stops even when no other background services were started. + if let Some((shutdown, handle)) = rotation_handle { + // A send error means the loop already exited (receiver dropped); the + // await then returns immediately. + let _ = shutdown.send(true); + if let Err(join_error) = handle.await { + if !join_error.is_cancelled() { + tracing::error!("Auto-rotation task panicked: {:?}", join_error); + } + } + } + // If nothing to stop, return early let (shutdown_notify, supervisor_handle) = match (shutdown_notify, supervisor_handle) { (Some(notify), Some(handle)) => (notify, handle), @@ -608,9 +676,6 @@ impl Mint { } }; - // Drop the lock before waiting - drop(task_state); - tracing::info!("Stopping mint background services..."); // Signal shutdown @@ -1654,6 +1719,152 @@ mod tests { .unwrap() } + #[tokio::test] + async fn stop_halts_embedded_auto_rotation() { + let localstore = Arc::new( + new_with_state( + Default::default(), + Default::default(), + Default::default(), + Default::default(), + Default::default(), + Default::default(), + MintInfo::default(), + ) + .await + .unwrap(), + ); + let keystore = Arc::new(cdk_sqlite::mint::memory::empty().await.unwrap()); + + // One second interval: the loop polls every second (cadence is + // min(interval, DEFAULT_TICKET)) and rotates the build-time keyset once + // it ages past one second. The 5s growth timeout below leaves margin for + // the second-granularity `valid_from`. + let mut builder = MintBuilder::new(localstore) + .with_keyset_rotation_interval(Some(Duration::from_secs(1))); + builder + .configure_unit( + CurrencyUnit::Sat, + UnitConfig { + amounts: vec![1, 2, 4, 8], + input_fee_ppk: 0, + }, + ) + .unwrap(); + let mint = builder + .build_with_seed(keystore, b"stop-halts-rotation-seed") + .await + .unwrap(); + + // Rotation is spawned by `start()`, like the other background services. + mint.start().await.expect("mint should start"); + + // Observe rotation through the signatory directly, not the mint's + // drained view, so we measure the producer rather than the consumer. + let initial = mint.signatory.keysets().await.unwrap().keysets.len(); + let grew = tokio::time::timeout(Duration::from_secs(5), async { + loop { + if mint.signatory.keysets().await.unwrap().keysets.len() > initial { + break; + } + tokio::time::sleep(Duration::from_millis(20)).await; + } + }) + .await; + assert!( + grew.is_ok(), + "auto-rotation should grow the keyset set before stop" + ); + + mint.stop().await.expect("mint should stop"); + + // After stop the rotation task is aborted, so the count no longer grows. + // Wait longer than two intervals: if rotation were still running it would + // fire at least twice in this window. + let after_stop = mint.signatory.keysets().await.unwrap().keysets.len(); + tokio::time::sleep(Duration::from_millis(2500)).await; + let later = mint.signatory.keysets().await.unwrap().keysets.len(); + assert_eq!(after_stop, later, "stop() must halt embedded auto-rotation"); + } + + /// Rotation is spawned by `start()`, so a `stop()` + `start()` cycle resumes + /// it like the other background services. + #[tokio::test] + async fn auto_rotation_resumes_after_restart() { + let localstore = Arc::new( + new_with_state( + Default::default(), + Default::default(), + Default::default(), + Default::default(), + Default::default(), + Default::default(), + MintInfo::default(), + ) + .await + .unwrap(), + ); + let keystore = Arc::new(cdk_sqlite::mint::memory::empty().await.unwrap()); + + // One second interval so the loop polls every second and rotation is + // observable within the 5s growth timeout below. + let mut builder = MintBuilder::new(localstore) + .with_keyset_rotation_interval(Some(Duration::from_secs(1))); + builder + .configure_unit( + CurrencyUnit::Sat, + UnitConfig { + amounts: vec![1, 2, 4, 8], + input_fee_ppk: 0, + }, + ) + .unwrap(); + let mint = builder + .build_with_seed(keystore, b"rotation-restart-seed") + .await + .unwrap(); + + mint.start().await.expect("mint should start"); + + // Rotation is live before the restart: wait for the keyset set to grow. + let initial = mint.signatory.keysets().await.unwrap().keysets.len(); + let grew = tokio::time::timeout(Duration::from_secs(5), async { + loop { + if mint.signatory.keysets().await.unwrap().keysets.len() > initial { + break; + } + tokio::time::sleep(Duration::from_millis(20)).await; + } + }) + .await; + assert!( + grew.is_ok(), + "auto-rotation should be running before the restart" + ); + + mint.stop().await.expect("mint should stop"); + mint.start().await.expect("mint should restart"); + + // Rotation is respawned on restart, so the keyset set grows again from + // its post-restart baseline. + let after_restart = mint.signatory.keysets().await.unwrap().keysets.len(); + let regrew = tokio::time::timeout(Duration::from_secs(5), async { + loop { + if mint.signatory.keysets().await.unwrap().keysets.len() > after_restart { + break; + } + tokio::time::sleep(Duration::from_millis(20)).await; + } + }) + .await; + assert!( + regrew.is_ok(), + "auto-rotation should resume after stop() then start()" + ); + + mint.stop().await.expect("mint should stop"); + } + #[tokio::test] async fn mock_injection_updates_mint_keysets() { let snaps = rotated_snapshots(2).await; diff --git a/docs/adr/0004-signatory-multi-instance-sharing.md b/docs/adr/0004-signatory-multi-instance-sharing.md index 20238dd07..0f8bf159c 100644 --- a/docs/adr/0004-signatory-multi-instance-sharing.md +++ b/docs/adr/0004-signatory-multi-instance-sharing.md @@ -148,6 +148,28 @@ sees no change and does not reload. Recording the epoch from a bare post-commit read would be unsafe: it could already reflect a peer change this instance has not loaded, and the instance would then skip it. +### Auto-rotation across instances + +Keyset auto-rotation (`spawn_auto_rotation`, off when the interval is zero) adds +a third writer to the picture: every instance sweeps its own keysets and rotates +the ones older than the interval. + +The due set is read from the process-local snapshot, which may predate a peer's +committed rotation, so the sweep alone cannot tell a genuinely aged keyset from +one a peer has already replaced. `rotate` therefore takes the keyset the caller +decided to replace and re-checks it inside the rotation transaction, after the +reload and under the global keyset lock, where the database view is +authoritative. A mismatch means a peer got there first: the method returns +`ConcurrentUpdate` before writing anything, the transaction rolls back, and the +sweep logs the unit as skipped rather than failed. Only auto-rotation passes the +expectation; a mint-initiated `rotate_keyset` passes none and adopts whatever is +active, as before. + +That re-check is the whole coordination story, so the sweep itself carries no +machinery of its own: it takes no lock spanning the units it rotates, needs no +stagger between instances, and treats each unit as best effort, logging a +failure and leaving the unit for the next tick. + ### Positive Consequences * Multiple signatory instances can run active/active against one database. From 42f1e2c700979fc7fe85b24aecc9e2a0d3f943be Mon Sep 17 00:00:00 2001 From: Cesar Rodas Date: Tue, 11 Aug 2026 22:34:22 -0300 Subject: [PATCH 2/3] Signatory: sweep every aged keyset in one rotation transaction The sweep read its due set from a process-local snapshot, so it could not tell an aged keyset from one a peer had already replaced. Every rotation had to carry the keyset it meant to replace and re-check it under the global lock, returning ConcurrentUpdate when a peer won, which the sweep then had to tell apart from a real failure. Reading the due set inside the rotation transaction removes the question. The reload runs under the global keyset lock, held to commit, so a unit a peer just rotated carries a fresh valid_from and is not due. Age decides, and rotate splits into begin/stage/finish so the sweep and a mint-initiated rotation share the loading and the commit. The sweep is now all-or-nothing, which Postgres forces once the units share a transaction. --- crates/cdk-signatory/src/db_signatory.rs | 427 +++++++++++------- .../0004-signatory-multi-instance-sharing.md | 33 +- 2 files changed, 291 insertions(+), 169 deletions(-) diff --git a/crates/cdk-signatory/src/db_signatory.rs b/crates/cdk-signatory/src/db_signatory.rs index 857446a0a..063e5f03f 100644 --- a/crates/cdk-signatory/src/db_signatory.rs +++ b/crates/cdk-signatory/src/db_signatory.rs @@ -339,6 +339,26 @@ impl DbSignatory { .collect(), } } + + /// Active keysets valid for at least `max_age`, read from the in-memory + /// snapshot and sorted by unit so a sweep is deterministic. + fn due_keysets(&self, now: u64, max_age: u64) -> Vec { + let keysets = self.keysets.load(); + let mut due: Vec = keysets + .active_by_unit + .values() + .filter_map(|id| keysets.by_id.get(id).map(|(info, _)| info)) + .filter(|info| now.saturating_sub(info.valid_from) >= max_age) + // A keyset with no amounts fails staging identically every tick, and + // the sweep is one transaction, so it would hold back every other + // unit forever. + .filter(|info| !info.amounts.is_empty()) + .cloned() + .collect(); + + due.sort_by(|a, b| a.unit.cmp(&b.unit)); + due + } } impl DbSignatory { @@ -349,39 +369,42 @@ impl DbSignatory { /// pushed forward by the keyset's active age so it stays valid at least as /// long as the keyset it replaces. /// - /// The age check needs each keyset's `valid_from`, which does not cross the - /// `Signatory` trait, so this reads the in-memory keysets directly. - /// - /// Best effort per unit: a failure is logged and the unit keeps its current - /// keyset until the next tick. The sweep needs no coordination of its own, - /// `rotate` is race-free across processes. + /// The whole sweep is one transaction, so it is all-or-nothing: if any unit + /// fails, none of them rotate and the tick is retried later. Postgres aborts + /// a transaction on the first failed statement, so continuing past a failure + /// is not an option once the units share a transaction. async fn rotate_aged_keysets(&self, max_age: Duration) { - let now = unix_time(); - let max_age = max_age.as_secs(); - - if max_age == 0 { + if max_age.is_zero() { return; } - // The due set comes from the current snapshot, which may be behind a - // peer's committed rotation. `rotate` re-checks each unit under the - // global keyset lock, so a unit a peer already rotated is skipped rather - // than rotated twice. - let due: Vec = { - let keysets = self.keysets.load(); - keysets - .active_by_unit - .values() - .filter_map(|id| keysets.by_id.get(id).map(|(info, _)| info.clone())) - .filter(|info| now.saturating_sub(info.valid_from) >= max_age) - .collect() - }; + if let Err(err) = self.sweep_aged_keysets(max_age.as_secs()).await { + tracing::error!("Automatic keyset rotation failed: {}", err); + } + } + + async fn sweep_aged_keysets(&self, max_age: u64) -> Result<(), Error> { + let _rotation = self.rotation_lock.lock().await; + let mut tx = self.begin_rotation().await?; + + // Read the due set only now. The reload above ran under the global + // keyset lock, held until this transaction ends, so a unit a peer + // already rotated carries that peer's fresh `valid_from` and is not due. + // Age alone decides; nothing has to re-check what it is replacing. + // + // `now` is taken after `begin_rotation`, which can block on the lock for + // an unbounded time: a stale one would skew each replacement's expiry. + let now = unix_time(); + let due = self.due_keysets(now, max_age); + + if due.is_empty() { + tx.rollback().await?; + return Ok(()); + } - for info in due { + let mut staged = Ok(()); + for info in &due { let active_age = now.saturating_sub(info.valid_from); - let final_expiry = info - .final_expiry - .map(|expiry| expiry.saturating_add(active_age)); tracing::info!( "Auto-rotating keyset {} for unit {} (active for {}s, interval {}s)", @@ -391,38 +414,30 @@ impl DbSignatory { max_age ); - match self - .rotate( + staged = self + .stage_rotation( + &mut *tx, RotateKeyArguments { unit: info.unit.clone(), amounts: info.amounts.clone(), input_fee_ppk: info.input_fee_ppk, keyset_id_type: info.id.get_version(), - final_expiry, + // Pushed forward by the age the keyset reached, so the + // replacement stays valid as long as it did. + final_expiry: info + .final_expiry + .map(|expiry| expiry.saturating_add(active_age)), }, - Some(info.id), ) .await - { - Ok(_) => {} - // Another process sharing this database rotated the unit first. - // A benign skip, not a failure. - Err(Error::ConcurrentUpdate) => { - tracing::info!( - "Auto-rotation for unit {} skipped: another instance already rotated it", - info.unit - ); - } - Err(err) => { - tracing::error!( - "Auto-rotating keyset {} for unit {} failed: {}", - info.id, - info.unit, - err - ); - } + .map(|_| ()); + + if staged.is_err() { + break; } } + + self.finish_rotation(tx, staged).await } /// Spawn the background keyset auto-rotation task. @@ -561,51 +576,78 @@ impl Signatory for DbSignatory { /// Generate new keyset #[tracing::instrument(skip(self))] async fn rotate_keyset(&self, args: RotateKeyArguments) -> Result { - // No `expected_prev`: a mint-initiated rotation adopts whatever the - // database says is active, rather than failing because a peer rotated - // first. - self.rotate(args, None).await + let _rotation = self.rotation_lock.lock().await; + let mut tx = self.begin_rotation().await?; + + // Unconditional: a mint-initiated rotation adopts whatever the database + // says is active rather than failing because a peer rotated first. + let staged = self.stage_rotation(&mut *tx, args).await; + + self.finish_rotation(tx, staged).await } } impl DbSignatory { - /// Rotate a single keyset for `args.unit`. + /// Open the rotation transaction and refresh in-memory keysets through it. /// - /// `expected_prev` is the keyset the caller decided to replace. When set, - /// the rotation is abandoned with [`Error::ConcurrentUpdate`] if the unit's - /// active keyset is no longer that one by the time the rotation transaction - /// holds the global keyset lock. Only the auto-rotation sweep passes it: its - /// due-check reads a process-local snapshot, so without the re-check a peer's - /// rotation would be followed by a redundant second one. - async fn rotate( - &self, - args: RotateKeyArguments, - expected_prev: Option, - ) -> Result { - let _rotation = self.rotation_lock.lock().await; - - // Persist the rotation. This is the only path that writes to the - // database. Opening the transaction takes the global keyset advisory - // lock, held to commit, so all keyset transactions serialize across - // processes: index allocation is authoritative and two rotations cannot - // interleave. + /// Opening the transaction takes the global keyset advisory lock, held until + /// the caller commits or rolls back, so all keyset transactions serialize + /// across processes: index allocation is authoritative and two rotations + /// cannot interleave. + /// + /// This is the only reload on the rotation path and it must precede every + /// write: `reload_from_tx` reads the keyset epoch through the transaction, + /// so after a write a second call would publish uncommitted rows into the + /// live snapshot. Finish the transaction through [`Self::finish_rotation`]. + async fn begin_rotation<'a>( + &'a self, + ) -> Result + Send + Sync + 'a>, Error> + { let mut tx = self.localstore.begin_transaction().await?; - let path_index = tx.next_derivation_index(&args.unit).await?; - - // Reload in-memory keysets from committed state through this - // transaction, under the global lock just taken. Both the default - // amounts below and the collision check further down then see peers' - // committed rotations rather than a possibly-stale in-memory snapshot. self.reload_from_tx(&mut *tx).await?; + Ok(tx) + } - // The reload above is authoritative under the global lock, so this sees - // any peer rotation that beat us. Return before writing anything: the - // transaction rolls back and the caller skips the unit. - if let Some(prev) = expected_prev { - if self.keysets.load().active_by_unit.get(&args.unit) != Some(&prev) { - return Err(Error::ConcurrentUpdate); + /// Commit and refresh memory when everything staged, roll back otherwise. + /// Never let the transaction die by drop: that defers the rollback to a + /// detached task holding the connection, which stalls the next caller on a + /// single-connection backend. + /// + /// The refresh reloads full database state rather than patching memory by + /// hand, so it picks up any concurrent peer rotation and records the epoch + /// it loaded. (Recording the epoch from a bare post-commit read would be + /// unsafe: it could already include a peer change this instance has not + /// loaded.) A rollback wrote nothing, so there is nothing to refresh. + async fn finish_rotation( + &self, + tx: Box + Send + Sync + '_>, + staged: Result, + ) -> Result { + match staged { + Ok(staged) => { + tx.commit().await?; + self.load_keys_from_db().await?; + Ok(staged) + } + Err(err) => { + tx.rollback().await?; + Err(err) } } + } + + /// Stage one unit's rotation into an open rotation transaction. + /// + /// Nothing is visible to anyone until the caller commits, so several units + /// can be staged into one transaction. `commit` and `rollback` take + /// `self: Box`, so taking the transaction by reference here means + /// staging cannot finish it by accident. + async fn stage_rotation( + &self, + tx: &mut (dyn MintKeyDatabaseTransaction<'_, database::Error> + Send + Sync), + args: RotateKeyArguments, + ) -> Result { + let path_index = tx.next_derivation_index(&args.unit).await?; // Default amounts come from the in-memory active keyset and are only // used when the caller does not specify any. The authoritative @@ -655,27 +697,19 @@ impl DbSignatory { tx.add_keyset_info(info.clone()).await?; tx.set_active_keyset(args.unit.clone(), id).await?; - tx.commit().await?; + // Built from what was staged rather than read back after the commit: a + // peer may rotate the unit again before this instance reloads. let mut info = info; info.active = true; - let signatory_keyset: SignatoryKeySet = (&(info, keyset)).into(); - - // Refresh from the full database state rather than patching memory by - // hand. This picks up any concurrent peer rotation too, and records the - // keyset epoch we loaded, so the periodic refresh does not reload - // again on its next tick. (Recording the epoch from a bare post-commit - // read would be unsafe: it could already include a peer change this - // instance has not loaded.) - self.load_keys_from_db().await?; - - Ok(signatory_keyset) + Ok((&(info, keyset)).into()) } } #[cfg(test)] mod test { use std::collections::HashSet; + use std::sync::atomic::{AtomicUsize, Ordering}; use std::sync::Mutex; use bitcoin::key::Secp256k1; @@ -1817,12 +1851,29 @@ mod test { ) } - /// Keys database that wraps a real one and fails `swap_active_keyset` for a - /// single armed unit, to exercise a mid-sweep rotation failure. The armed - /// unit is settable so seeding runs before failure is armed. + /// Keys database that wraps a real one, counts the transactions opened + /// through it, and fails `set_active_keyset` for a single armed unit to + /// exercise a mid-sweep rotation failure. The armed unit is settable so + /// seeding runs before failure is armed. struct FailUnitDb { inner: MintSqliteDatabase, fail_unit: Arc>>, + transactions: Arc, + } + + impl FailUnitDb { + async fn new() -> (Arc, Arc>>, Arc) { + let fail_unit = Arc::new(Mutex::new(None)); + let transactions = Arc::new(AtomicUsize::new(0)); + let db = Arc::new(Self { + inner: cdk_sqlite::mint::memory::empty() + .await + .expect("in-memory db"), + fail_unit: fail_unit.clone(), + transactions: transactions.clone(), + }); + (db, fail_unit, transactions) + } } struct FailUnitTx<'a> { @@ -1851,9 +1902,7 @@ mod test { id: Id, ) -> Result<(), database::Error> { // Every rotation reassigns the active pointer, so injecting here - // fails the whole rotation transaction. An Internal error, not a - // `ConcurrentUpdate`, so the sweep treats it as a real failure rather - // than a benign lost race. + // fails the transaction the whole sweep is staged into. if self.fail_unit.lock().expect("lock").as_ref() == Some(&unit) { return Err(database::Error::Internal( "simulated set_active_keyset failure".to_string(), @@ -1905,6 +1954,7 @@ mod test { Box + Send + Sync + 'a>, database::Error, > { + self.transactions.fetch_add(1, Ordering::SeqCst); Ok(Box::new(FailUnitTx { inner: self.inner.begin_transaction().await?, fail_unit: self.fail_unit.clone(), @@ -1942,6 +1992,14 @@ mod test { sig.keysets.load().active_by_unit.get(unit).copied() } + /// Every keyset id recorded in the database, bypassing the in-memory snapshot. + async fn db_keyset_ids(sig: &DbSignatory) -> HashSet { + let mut tx = sig.localstore.begin_transaction().await.expect("begin tx"); + let infos = tx.get_keyset_infos().await.expect("keyset infos"); + tx.commit().await.expect("commit"); + infos.into_iter().map(|info| info.id).collect() + } + /// Seed an active keyset for `unit` whose `valid_from` is backdated by /// `age` seconds, so `rotate_aged_keysets` treats it as due for any /// interval below `age`. Returns the seeded keyset id. @@ -2187,20 +2245,15 @@ mod test { } #[tokio::test] - async fn rotate_aged_keysets_continues_after_one_unit_fails() { - // One unit's rotation failing must not starve the others: the sweep - // rotates every other due unit and only the failed unit is left on its - // current keyset, to be retried on the next tick. - let fail_unit = Arc::new(Mutex::new(None)); - let store = Arc::new(FailUnitDb { - inner: cdk_sqlite::mint::memory::empty() - .await - .expect("in-memory db"), - fail_unit: fail_unit.clone(), - }); + async fn rotate_aged_keysets_rolls_back_every_unit_when_one_fails() { + // The sweep stages every due unit into one transaction, so one unit + // failing leaves none of them rotated. The failure must also be + // transient rather than a poison pill: once it clears, the next sweep + // rotates everything. + let (store, fail_unit, _transactions) = FailUnitDb::new().await; let sig = Arc::new( DbSignatory::new( - store, + store.clone(), b"test-seed-partial-fail", Default::default(), Default::default(), @@ -2231,25 +2284,93 @@ mod test { ) .await; - // Arm failure for Usd only, then sweep. + // Arm failure for Usd only, then sweep. Sat sorts before Usd, so Sat is + // staged first and is the unit the rollback has to undo. + let epoch_before = store.keysets_epoch().await.expect("epoch"); *fail_unit.lock().expect("lock") = Some(CurrencyUnit::Usd); sig.rotate_aged_keysets(Duration::from_secs(60)).await; - // Sat rotated despite Usd failing (order within the sweep is - // nondeterministic, so this proves the sweep did not abort early). - let new_sat = active_keyset(&sig, &CurrencyUnit::Sat).await; - assert_ne!(new_sat.id, sat, "Sat rotates even though Usd failed"); - - // Usd unchanged in memory and in the DB: its transaction rolled back. - let cur_usd = active_keyset(&sig, &CurrencyUnit::Usd).await; + for (unit, seeded) in [(CurrencyUnit::Sat, sat), (CurrencyUnit::Usd, usd)] { + assert_eq!( + db_active_keyset_id(&sig, &unit).await, + Some(seeded), + "{unit} must keep its active pointer when the sweep rolls back" + ); + assert_eq!( + memory_active_keyset_id(&sig, &unit), + Some(seeded), + "{unit} must be unchanged in memory: only committed state is published" + ); + } assert_eq!( - cur_usd.id, usd, - "failed unit keeps its previous active keyset" + store.keysets_epoch().await.expect("epoch"), + epoch_before, + "a rolled-back sweep must write nothing at all" ); assert_eq!( + db_keyset_ids(&sig).await.len(), + 2, + "the staged keyset of the unit that did succeed must not survive" + ); + + // Disarm and sweep again: the failure was transient, not a poison pill. + *fail_unit.lock().expect("lock") = None; + sig.rotate_aged_keysets(Duration::from_secs(60)).await; + + assert_ne!( + db_active_keyset_id(&sig, &CurrencyUnit::Sat).await, + Some(sat), + "Sat rotates once the failure clears" + ); + assert_ne!( db_active_keyset_id(&sig, &CurrencyUnit::Usd).await, Some(usd), - "DB active pointer for the failed unit is unchanged", + "Usd rotates once the failure clears" + ); + } + + #[tokio::test] + async fn rotate_aged_keysets_stages_every_unit_in_one_transaction() { + let (store, _fail_unit, transactions) = FailUnitDb::new().await; + let sig = Arc::new( + DbSignatory::new( + store, + b"test-seed-one-transaction", + Default::default(), + Default::default(), + ) + .await + .expect("DbSignatory::new"), + ); + + for unit in [CurrencyUnit::Sat, CurrencyUnit::Usd, CurrencyUnit::Eur] { + seed_aged_keyset( + &sig, + unit, + &[1, 2, 4, 8], + 0, + cdk_common::nut02::KeySetVersion::Version00, + None, + 120, + ) + .await; + } + + transactions.store(0, Ordering::SeqCst); + sig.rotate_aged_keysets(Duration::from_secs(60)).await; + + // One transaction stages and commits all three units, and the + // post-commit `load_keys_from_db` opens the second. A per-unit sweep + // would open six. + assert_eq!( + transactions.load(Ordering::SeqCst), + 2, + "the sweep must stage every unit in a single transaction" + ); + assert_eq!( + db_keyset_ids(&sig).await.len(), + 6, + "all three units rotated" ); } @@ -2361,16 +2482,16 @@ mod test { ); } - /// The auto-rotation sweep picks its due set from a process-local snapshot, - /// so a peer can rotate the unit before the sweep gets to it. The - /// `expected_prev` re-check inside the rotation transaction must catch that - /// and abandon the rotation rather than rotate a second time. + /// A peer's rotation must not be followed by a redundant second one. The + /// sweep reads its due set only after reloading inside the rotation + /// transaction, so it sees the peer's fresh `valid_from` and the unit is + /// simply not due. No identity re-check is involved. #[tokio::test] - async fn rotate_skips_unit_a_peer_already_rotated() { + async fn rotate_aged_keysets_skips_unit_a_peer_already_rotated() { let sig = test_signatory(b"test-seed-peer-rotation-skip").await; - // The keyset the sweep would decide is due. - let seeded = seed_aged_keyset( + // The keyset this signatory's stale snapshot still thinks is due. + let _seeded = seed_aged_keyset( &sig, CurrencyUnit::Sat, &[1, 2, 4, 8], @@ -2397,6 +2518,7 @@ mod test { None, cdk_common::nut02::KeySetVersion::Version00, ); + // The freshness of this `valid_from` is the entire skip mechanism. external_info.valid_from = unix_time(); let external_id = external_keyset.id; let mut tx = sig.localstore.begin_transaction().await.expect("begin tx"); @@ -2406,39 +2528,36 @@ mod test { .expect("set active"); tx.commit().await.expect("commit"); - let result = sig - .rotate( - 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, - }, - Some(seeded), - ) - .await; - assert!( - matches!(result, Err(Error::ConcurrentUpdate)), - "a unit the peer already rotated must be skipped, got {result:?}" - ); + let epoch_before = sig.localstore.keysets_epoch().await.expect("epoch"); + sig.rotate_aged_keysets(Duration::from_secs(60)).await; - // The peer's keyset stands, in the database and in memory: the abandoned - // rotation neither committed nor left a half-applied snapshot. + assert_eq!( + sig.localstore.keysets_epoch().await.expect("epoch"), + epoch_before, + "a unit the peer already rotated must not be rotated again" + ); + assert_eq!( + db_keyset_ids(&sig).await.len(), + 2, + "no third keyset was created" + ); assert_eq!( db_active_keyset_id(&sig, &CurrencyUnit::Sat).await, Some(external_id), - "the peer's active pointer must survive the abandoned rotation", + "the peer's active pointer stands", ); + // Load-bearing: this can only hold if the sweep opened its transaction + // and adopted the peer's view, rather than deciding from the stale + // snapshot alone. assert_eq!( memory_active_keyset_id(&sig, &CurrencyUnit::Sat), Some(external_id), - "the abandoned rotation still adopts the peer's keyset it read under the lock" + "the sweep's under-lock reload adopts the peer's keyset" ); } - /// A mint-initiated rotation passes no `expected_prev`, so it adopts a peer's - /// keyset instead of failing. + /// A mint-initiated rotation is unconditional: it adopts whatever the + /// database says is active instead of failing because a peer rotated first. #[tokio::test] async fn rotate_keyset_succeeds_when_a_peer_rotated_first() { let sig = test_signatory(b"test-seed-peer-rotation-adopt").await; diff --git a/docs/adr/0004-signatory-multi-instance-sharing.md b/docs/adr/0004-signatory-multi-instance-sharing.md index 0f8bf159c..4c5dfd77c 100644 --- a/docs/adr/0004-signatory-multi-instance-sharing.md +++ b/docs/adr/0004-signatory-multi-instance-sharing.md @@ -154,21 +154,24 @@ Keyset auto-rotation (`spawn_auto_rotation`, off when the interval is zero) adds a third writer to the picture: every instance sweeps its own keysets and rotates the ones older than the interval. -The due set is read from the process-local snapshot, which may predate a peer's -committed rotation, so the sweep alone cannot tell a genuinely aged keyset from -one a peer has already replaced. `rotate` therefore takes the keyset the caller -decided to replace and re-checks it inside the rotation transaction, after the -reload and under the global keyset lock, where the database view is -authoritative. A mismatch means a peer got there first: the method returns -`ConcurrentUpdate` before writing anything, the transaction rolls back, and the -sweep logs the unit as skipped rather than failed. Only auto-rotation passes the -expectation; a mint-initiated `rotate_keyset` passes none and adopts whatever is -active, as before. - -That re-check is the whole coordination story, so the sweep itself carries no -machinery of its own: it takes no lock spanning the units it rotates, needs no -stagger between instances, and treats each unit as best effort, logging a -failure and leaving the unit for the next tick. +The sweep needs no coordination of its own because it runs entirely inside one +rotation transaction. `begin_rotation` opens that transaction, which takes the +global keyset lock, and reloads the in-memory keysets through it; only then does +the sweep read its due set. A unit a peer already rotated therefore carries that +peer's fresh `valid_from` and is not due, and the lock is held until commit, so +no peer can rotate in between. Age alone decides, with no identity check and no +lost-race error to distinguish from a real failure. + +Reading the due set outside the transaction was what made an identity check +necessary, and it is also what forced a transaction per unit. Staging every due +unit into the one transaction makes the sweep all-or-nothing: if any unit fails, +none of them rotate and the tick is retried later. That is not a preference, +Postgres aborts a transaction on the first failed statement, so continuing past +a failure is not available once the units share a transaction. The realistic +failure is a database error, which would have failed every unit anyway. + +`rotate_keyset` runs through the same two pieces with a single unit, so a +mint-initiated rotation and a sweep share the loading and the commit. ### Positive Consequences From 5b062db87da77f4f878a92626b6a173c8b66a692 Mon Sep 17 00:00:00 2001 From: C Date: Sat, 22 Aug 2026 22:11:37 -0300 Subject: [PATCH 3/3] Signatory: keep idle rotation ticks off the shared keyset lock The auto-rotation loop polls faster than its interval so that age, not process uptime, drives rotation. Every tick opened a transaction and took the global keyset lock before checking whether anything was due, taxing every instance in the fleet for nothing. Check the in-memory snapshot first: it only ever lags the database, so it over-reports due-ness and can never miss a keyset that is actually due. Mint::stop no longer waits without bound for an in-flight sweep, which can block on that same lock. After a timeout it leaves the rotation to finish detached rather than holding up shutdown. An unparsable rotation interval environment variable now fails startup instead of falling back to the 90-day default, which would rotate keys for an operator who was trying to disable rotation. --- crates/cdk-mintd/src/env_vars/mod.rs | 2 +- crates/cdk-mintd/src/env_vars/signatory.rs | 42 +++++++-- crates/cdk-signatory/src/db_signatory.rs | 89 ++++++++++++++---- crates/cdk/src/mint/mod.rs | 91 +++++++++++++++++-- .../0004-signatory-multi-instance-sharing.md | 9 ++ 5 files changed, 200 insertions(+), 33 deletions(-) diff --git a/crates/cdk-mintd/src/env_vars/mod.rs b/crates/cdk-mintd/src/env_vars/mod.rs index 05a63553a..78a643f98 100644 --- a/crates/cdk-mintd/src/env_vars/mod.rs +++ b/crates/cdk-mintd/src/env_vars/mod.rs @@ -92,7 +92,7 @@ impl Settings { }); self.info = self.info.clone().from_env(); - self.signatory = Some(self.signatory.clone().unwrap_or_default().from_env()); + self.signatory = Some(self.signatory.clone().unwrap_or_default().from_env()?); self.mint_info = self.mint_info.clone().from_env(); // CDK_MINTD_PAYMENT_BACKEND_* env vars only apply when there is exactly diff --git a/crates/cdk-mintd/src/env_vars/signatory.rs b/crates/cdk-mintd/src/env_vars/signatory.rs index d2f83c3e1..e66ec2298 100644 --- a/crates/cdk-mintd/src/env_vars/signatory.rs +++ b/crates/cdk-mintd/src/env_vars/signatory.rs @@ -2,11 +2,13 @@ use std::env; +use anyhow::{Context, Result}; + use super::common::*; use crate::config::Signatory; impl Signatory { - pub fn from_env(mut self) -> Self { + pub fn from_env(mut self) -> Result { if let Ok(enabled) = env::var(ENV_SIGNATORY_ENABLED) { if let Ok(enabled) = enabled.parse() { self.enabled = enabled; @@ -33,13 +35,20 @@ impl Signatory { } } + // Hard failure rather than a silent fallback: an unparsable value here + // would otherwise leave the 90-day default in place, so an operator + // trying to disable auto-rotation would get keys rotating instead. if let Ok(interval_str) = env::var(ENV_SIGNATORY_KEYSET_ROTATION_INTERVAL_SECONDS) { - if let Ok(interval) = interval_str.parse() { - self.keyset_rotation_interval_seconds = Some(interval); - } + let interval = interval_str.parse().with_context(|| { + format!( + "{ENV_SIGNATORY_KEYSET_ROTATION_INTERVAL_SECONDS} must be a whole number of \ + seconds; 0 disables keyset auto-rotation" + ) + })?; + self.keyset_rotation_interval_seconds = Some(interval); } - self + Ok(self) } } @@ -74,7 +83,7 @@ mod tests { env::set_var(ENV_SIGNATORY_ALLOW_INSECURE, "true"); env::set_var(ENV_SIGNATORY_KEYSET_ROTATION_INTERVAL_SECONDS, "7776000"); - let signatory = Signatory::default().from_env(); + let signatory = Signatory::default().from_env().expect("valid env"); assert!(signatory.enabled); assert_eq!(signatory.address, "0.0.0.0"); @@ -88,4 +97,25 @@ mod tests { clear_env_vars(); } + + /// An operator writing `off` means "disable rotation". Falling back to the + /// 90-day default would silently rotate keys instead, so the parse fails. + #[test] + fn signatory_from_env_rejects_unparsable_rotation_interval() { + let _guard = env_lock(); + clear_env_vars(); + + env::set_var(ENV_SIGNATORY_KEYSET_ROTATION_INTERVAL_SECONDS, "off"); + + let err = Signatory::default() + .from_env() + .expect_err("an unparsable rotation interval must fail configuration"); + assert!( + err.to_string() + .contains(ENV_SIGNATORY_KEYSET_ROTATION_INTERVAL_SECONDS), + "the error must name the offending variable, got: {err}" + ); + + clear_env_vars(); + } } diff --git a/crates/cdk-signatory/src/db_signatory.rs b/crates/cdk-signatory/src/db_signatory.rs index 063e5f03f..01e39e684 100644 --- a/crates/cdk-signatory/src/db_signatory.rs +++ b/crates/cdk-signatory/src/db_signatory.rs @@ -342,6 +342,9 @@ impl DbSignatory { /// Active keysets valid for at least `max_age`, read from the in-memory /// snapshot and sorted by unit so a sweep is deterministic. + /// + /// A sweep calls this as a lock-free pre-check, and again inside the + /// rotation transaction where the reloaded snapshot is authoritative. fn due_keysets(&self, now: u64, max_age: u64) -> Vec { let keysets = self.keysets.load(); let mut due: Vec = keysets @@ -384,13 +387,24 @@ impl DbSignatory { } async fn sweep_aged_keysets(&self, max_age: u64) -> Result<(), Error> { + // Quiet ticks cost one atomic snapshot read: no rotation lock, no + // transaction, no global keyset lock. Safe because the snapshot only + // ever lags the database, and a lagging snapshot carries an older + // `valid_from`, so it can over-report due-ness but never under-report. + // A false positive is discarded by the authoritative under-lock read + // below. + if self.due_keysets(unix_time(), max_age).is_empty() { + return Ok(()); + } + let _rotation = self.rotation_lock.lock().await; let mut tx = self.begin_rotation().await?; - // Read the due set only now. The reload above ran under the global - // keyset lock, held until this transaction ends, so a unit a peer - // already rotated carries that peer's fresh `valid_from` and is not due. - // Age alone decides; nothing has to re-check what it is replacing. + // The authoritative read, which the pre-check above never substitutes + // for. The reload above ran under the global keyset lock, held until + // this transaction ends, so a unit a peer already rotated carries that + // peer's fresh `valid_from` and is not due. Age alone decides; nothing + // has to re-check what it is replacing. // // `now` is taken after `begin_rotation`, which can block on the lock for // an unbounded time: a stale one would skew each replacement's expiry. @@ -1790,9 +1804,7 @@ mod test { ); // An interval below the keyset's age makes it due, so it rotates. - signatory - .rotate_aged_keysets(Duration::from_secs(60)) - .await; + signatory.rotate_aged_keysets(Duration::from_secs(60)).await; assert_ne!( memory_active_keyset_id(&signatory, &CurrencyUnit::Sat), Some(seeded), @@ -1862,7 +1874,11 @@ mod test { } impl FailUnitDb { - async fn new() -> (Arc, Arc>>, Arc) { + async fn new() -> ( + Arc, + Arc>>, + Arc, + ) { let fail_unit = Arc::new(Mutex::new(None)); let transactions = Arc::new(AtomicUsize::new(0)); let db = Arc::new(Self { @@ -2072,8 +2088,7 @@ mod test { seed_aged_keyset(&sig, CurrencyUnit::Sat, &amounts, fee, version, None, 120).await; let original = active_keyset(&sig, &CurrencyUnit::Sat).await; - sig.rotate_aged_keysets(Duration::from_secs(60)) - .await; + sig.rotate_aged_keysets(Duration::from_secs(60)).await; let rotated = active_keyset(&sig, &CurrencyUnit::Sat).await; assert_ne!(rotated.id, original.id, "a new keyset must be created"); @@ -2123,8 +2138,7 @@ mod test { ) .await; - sig.rotate_aged_keysets(Duration::from_secs(60)) - .await; + sig.rotate_aged_keysets(Duration::from_secs(60)).await; let rotated = active_keyset(&sig, &CurrencyUnit::Sat).await; let bumped = rotated @@ -2144,8 +2158,7 @@ mod test { let sig = test_signatory(b"test-seed-noop").await; let before = sig.keysets().await.expect("keysets").keysets.len(); - sig.rotate_aged_keysets(Duration::ZERO) - .await; + sig.rotate_aged_keysets(Duration::ZERO).await; let after = sig.keysets().await.expect("keysets").keysets.len(); assert_eq!(before, 0, "fresh signatory has no keysets"); @@ -2179,8 +2192,7 @@ mod test { assert_eq!(total(&after_seed), 1); assert_eq!(active_sat(&after_seed), 1); - sig.rotate_aged_keysets(Duration::from_secs(60)) - .await; + sig.rotate_aged_keysets(Duration::from_secs(60)).await; let after_first = sig.keysets().await.expect("keysets"); assert_eq!( total(&after_first), @@ -2195,8 +2207,7 @@ mod test { // would grow by more than one. age_active_keyset(&sig, &CurrencyUnit::Sat, 120).await; - sig.rotate_aged_keysets(Duration::from_secs(60)) - .await; + sig.rotate_aged_keysets(Duration::from_secs(60)).await; let after_second = sig.keysets().await.expect("keysets"); assert_eq!( total(&after_second), @@ -2235,8 +2246,7 @@ mod test { ) .await; - sig.rotate_aged_keysets(Duration::from_secs(60)) - .await; + sig.rotate_aged_keysets(Duration::from_secs(60)).await; let new_sat = active_keyset(&sig, &CurrencyUnit::Sat).await; let new_usd = active_keyset(&sig, &CurrencyUnit::Usd).await; @@ -2374,6 +2384,45 @@ mod test { ); } + #[tokio::test] + async fn sweep_with_nothing_due_touches_the_database_not_at_all() { + // The loop polls at most every DEFAULT_TICKET, so nearly every tick has + // nothing due. Those ticks must not open a transaction: opening one + // takes the global keyset lock every process shares, and parks the + // rotation lock a real rotation needs. + let (store, _fail_unit, transactions) = FailUnitDb::new().await; + let sig = Arc::new( + DbSignatory::new( + store, + b"test-seed-quiet-tick", + Default::default(), + Default::default(), + ) + .await + .expect("DbSignatory::new"), + ); + + seed_aged_keyset( + &sig, + CurrencyUnit::Sat, + &[1, 2, 4, 8], + 0, + cdk_common::nut02::KeySetVersion::Version00, + None, + 120, + ) + .await; + + transactions.store(0, Ordering::SeqCst); + sig.rotate_aged_keysets(Duration::from_secs(600)).await; + + assert_eq!( + transactions.load(Ordering::SeqCst), + 0, + "a sweep with nothing due must not open a transaction" + ); + } + #[tokio::test] async fn spawn_auto_rotation_stops_when_signatory_dropped() { let sig = test_signatory(b"test-seed-drop").await; diff --git a/crates/cdk/src/mint/mod.rs b/crates/cdk/src/mint/mod.rs index 1d752fa99..a98e7662b 100644 --- a/crates/cdk/src/mint/mod.rs +++ b/crates/cdk/src/mint/mod.rs @@ -115,6 +115,13 @@ impl std::fmt::Debug for Mint { pub(crate) type RotationSpawner = Arc) -> JoinHandle<()> + Send + Sync>; +/// How long [`Mint::stop`] waits for the auto-rotation loop to finish an +/// in-flight sweep before it stops waiting. A sweep can block on the +/// cross-process keyset lock for an unbounded time, and shutdown must not +/// inherit that wait. Giving up drops the `JoinHandle`, which detaches the task +/// rather than aborting it, so the rotation still runs to its own conclusion. +const ROTATION_SHUTDOWN_TIMEOUT: Duration = Duration::from_secs(30); + /// State for managing background tasks #[derive(Default)] struct TaskState { @@ -632,7 +639,9 @@ impl Mint { /// Embedded keyset auto-rotation (configured via /// [`MintBuilder::with_keyset_rotation_interval`]) is halted here /// cooperatively, letting an in-flight rotation finish, and is resumed by a - /// later [`Mint::start`] like the other background services. + /// later [`Mint::start`] like the other background services. A rotation + /// still running after [`ROTATION_SHUTDOWN_TIMEOUT`] is left to finish + /// detached rather than holding up shutdown. /// /// # Returns /// @@ -656,13 +665,19 @@ impl Mint { // being aborted mid-rotation. Done before the early-return below so // rotation stops even when no other background services were started. if let Some((shutdown, handle)) = rotation_handle { - // A send error means the loop already exited (receiver dropped); the - // await then returns immediately. - let _ = shutdown.send(true); - if let Err(join_error) = handle.await { - if !join_error.is_cancelled() { + if shutdown.send(true).is_err() { + tracing::debug!("Auto-rotation loop had already exited before shutdown"); + } + match tokio::time::timeout(ROTATION_SHUTDOWN_TIMEOUT, handle).await { + Ok(Ok(())) => {} + Ok(Err(join_error)) if join_error.is_cancelled() => {} + Ok(Err(join_error)) => { tracing::error!("Auto-rotation task panicked: {:?}", join_error); } + Err(_) => tracing::warn!( + "Auto-rotation did not stop within {}s, continuing shutdown without it", + ROTATION_SHUTDOWN_TIMEOUT.as_secs() + ), } } @@ -1787,6 +1802,70 @@ mod tests { assert_eq!(after_stop, later, "stop() must halt embedded auto-rotation"); } + /// A sweep blocked on the cross-process keyset lock never reaches the point + /// where it observes the shutdown signal, so `stop()` has to give up on it + /// rather than inherit an unbounded wait. + #[tokio::test(start_paused = true)] + async fn stop_gives_up_on_a_wedged_rotation() { + let localstore = Arc::new( + new_with_state( + Default::default(), + Default::default(), + Default::default(), + Default::default(), + Default::default(), + Default::default(), + MintInfo::default(), + ) + .await + .unwrap(), + ); + let keystore = Arc::new(cdk_sqlite::mint::memory::empty().await.unwrap()); + + let mut builder = MintBuilder::new(localstore); + builder + .configure_unit( + CurrencyUnit::Sat, + UnitConfig { + amounts: vec![1, 2, 4, 8], + input_fee_ppk: 0, + }, + ) + .unwrap(); + let mint = builder + .build_with_seed(keystore, b"wedged-rotation-seed") + .await + .unwrap(); + + // Stands in for a sweep stuck on the database lock: it ignores the + // shutdown receiver and never completes. + let spawned = Arc::new(std::sync::atomic::AtomicBool::new(false)); + let flag = Arc::clone(&spawned); + mint.set_rotation_spawner(Arc::new(move |_shutdown| { + let flag = Arc::clone(&flag); + tokio::spawn(async move { + flag.store(true, std::sync::atomic::Ordering::SeqCst); + std::future::pending::<()>().await; + }) + })) + .await; + + mint.start().await.expect("mint should start"); + tokio::task::yield_now().await; + assert!( + spawned.load(std::sync::atomic::Ordering::SeqCst), + "the wedged rotation task must actually be running, or this proves nothing" + ); + + // Generous relative to ROTATION_SHUTDOWN_TIMEOUT: the point is that + // stop() returns at all, not when. Time is paused, so this costs no + // wall clock. + tokio::time::timeout(Duration::from_secs(600), mint.stop()) + .await + .expect("stop() must not block forever on a wedged rotation") + .expect("mint should stop"); + } + /// Rotation is spawned by `start()`, so a `stop()` + `start()` cycle resumes /// it like the other background services. #[tokio::test] diff --git a/docs/adr/0004-signatory-multi-instance-sharing.md b/docs/adr/0004-signatory-multi-instance-sharing.md index 4c5dfd77c..d4e281a22 100644 --- a/docs/adr/0004-signatory-multi-instance-sharing.md +++ b/docs/adr/0004-signatory-multi-instance-sharing.md @@ -162,6 +162,15 @@ peer's fresh `valid_from` and is not due, and the lock is held until commit, so no peer can rotate in between. Age alone decides, with no identity check and no lost-race error to distinguish from a real failure. +Because the loop polls faster than the interval (age has to drive rotation, not +process uptime), nearly every tick has nothing to do, and taking the global lock +that often would tax every instance in the fleet. So a tick first checks the +in-memory snapshot lock-free and returns when nothing looks due. That cannot +skip a due keyset: the snapshot only ever lags the database, and a lagging +snapshot carries an older `valid_from`, so it over-reports due-ness and never +under-reports. A false positive costs one transaction that the authoritative +under-lock read then discards. + Reading the due set outside the transaction was what made an identity check necessary, and it is also what forced a transaction per unit. Staging every due unit into the one transaction makes the sweep all-or-nothing: if any unit fails,