From 6fdbdbcb37283b6fe33eedf1124598d79d89021c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C4=99drzej=20Stuczy=C5=84ski?= Date: Wed, 19 Aug 2026 10:49:27 +0100 Subject: [PATCH 1/5] fix(nym-api): serve a past epoch's partial signatures from its archived key Credentials outlive the epoch that issued them by up to a week, but the keys that epoch was signed with were renamed to `epoch-{id}-{filename}.archived` at the next dealing exchange and never read again. Both partial-signature paths therefore refused any epoch but the current one, so material a signer had not already generated could never be produced - permanently unusable ticketbooks for that epoch. It matters most for a reset, where the master key changes and nothing else can serve those books. The keys are now retained per epoch. `KeyPair` keeps an archive alongside the live slot, populated at the two points that agree by construction: dealing exchange hands the outgoing key over instead of dropping it, and startup scans the archive files back in - so a process that stays up through a ceremony and one that restarts behave the same. `keys_for_epoch` is the single lookup; the live slot keeps its validity gate, the archive deliberately does not, since the chain already settled which shares were verified for a concluded epoch. Two consequences worth noting. `hazmat_into_secrets` now borrows rather than consumes, because resharing no longer destroys the key it has to keep. And both partial routes gate on the *requested* epoch's signer set: an api that has since dropped out of the group still holds the keys, and refusing it on the strength of the current epoch alone can leave a past epoch short of the threshold its aggregation needs. `ensure_dkg_not_in_progress` is deliberately untouched - serving old epochs mid-ceremony is a separate gap. Also renames the ecash key-file surface off the legacy "coconut" name (persist/archive/can_validate, and the `ecash_key_path` field). Mechanical, and bundled because it renames the very functions this change extends; the config alias for `coconut_key_path` stays for backwards compatibility. --- .../src/ecash/api_routes/partial_signing.rs | 20 +- nym-api/src/ecash/dkg/controller/keys.rs | 215 ++++++++++++++++-- nym-api/src/ecash/dkg/controller/mod.rs | 10 +- nym-api/src/ecash/dkg/dealing.rs | 69 +++++- nym-api/src/ecash/dkg/key_derivation.rs | 4 +- nym-api/src/ecash/dkg/state/mod.rs | 6 + nym-api/src/ecash/keys/mod.rs | 125 +++++++++- nym-api/src/ecash/state/mod.rs | 209 ++++++++++++++--- nym-api/src/ecash/tests/fixtures.rs | 8 +- nym-api/src/support/cli/run.rs | 18 +- 10 files changed, 603 insertions(+), 81 deletions(-) diff --git a/nym-api/src/ecash/api_routes/partial_signing.rs b/nym-api/src/ecash/api_routes/partial_signing.rs index 54c73ff376c..582b82cb32d 100644 --- a/nym-api/src/ecash/api_routes/partial_signing.rs +++ b/nym-api/src/ecash/api_routes/partial_signing.rs @@ -143,7 +143,6 @@ async fn partial_expiration_date_signatures( output, }): Query, ) -> AxumResult> { - state.ensure_signer().await?; let output = output.unwrap_or_default(); let expiration_date = match expiration_date { @@ -152,14 +151,17 @@ async fn partial_expiration_date_signatures( .map_err(|_| EcashError::MalformedExpirationDate { raw })?, }; - // see if we're not in the middle of new dkg - state.ensure_dkg_not_in_progress().await?; - let epoch_id = match epoch_id { Some(epoch_id) => epoch_id, None => state.current_dkg_epoch().await?, }; + // the caller wants this epoch's material, so it's this epoch's signers that have to answer + state.ensure_signer_for_epoch(epoch_id).await?; + + // see if we're not in the middle of new dkg + state.ensure_dkg_not_in_progress().await?; + let expiration_date_signatures = state .partial_expiration_date_signatures(expiration_date, epoch_id) .await?; @@ -191,12 +193,18 @@ async fn partial_coin_indices_signatures( State(state): State>, Query(EpochIdParam { epoch_id, output }): Query, ) -> AxumResult> { - state.ensure_signer().await?; + let epoch_id = match epoch_id { + Some(epoch_id) => epoch_id, + None => state.current_dkg_epoch().await?, + }; + + // the caller wants this epoch's material, so it's this epoch's signers that have to answer + state.ensure_signer_for_epoch(epoch_id).await?; // see if we're not in the middle of new dkg state.ensure_dkg_not_in_progress().await?; - let coin_indices_signatures = state.partial_coin_index_signatures(epoch_id).await?; + let coin_indices_signatures = state.partial_coin_index_signatures(Some(epoch_id)).await?; Ok(output .unwrap_or_default() diff --git a/nym-api/src/ecash/dkg/controller/keys.rs b/nym-api/src/ecash/dkg/controller/keys.rs index 35650921504..b2f8c538101 100644 --- a/nym-api/src/ecash/dkg/controller/keys.rs +++ b/nym-api/src/ecash/dkg/controller/keys.rs @@ -35,6 +35,39 @@ pub(crate) fn load_bte_keypair(config: &config::EcashSigner) -> anyhow::Result anyhow::Result<(&Path, &str)> { + let dir = store_path + .parent() + .ok_or(anyhow!("the ecash key does not have a valid parent"))?; + let filename = store_path + .file_name() + .ok_or(anyhow!("the ecash key does not have a valid filename"))? + .to_str() + .ok_or(anyhow!("the ecash key filename is not valid UTF8"))?; + + Ok((dir, filename)) +} + +/// The name the ecash key takes once `epoch_id` is no longer the epoch we sign for. +fn archived_key_filename(live_filename: &str, epoch_id: EpochId) -> String { + format!("{ARCHIVED_KEY_PREFIX}{epoch_id}-{live_filename}{ARCHIVED_KEY_SUFFIX}") +} + +/// The epoch `filename` encodes, if it is an archive of `live_filename` at all. +fn archived_key_epoch(live_filename: &str, filename: &str) -> Option { + filename + .strip_prefix(ARCHIVED_KEY_PREFIX)? + .strip_suffix(ARCHIVED_KEY_SUFFIX)? + .strip_suffix(live_filename)? + .strip_suffix('-')? + .parse() + .ok() +} + pub(crate) fn load_ecash_keypair_if_exists( config: &config::EcashSigner, ) -> anyhow::Result> { @@ -54,9 +87,78 @@ pub(crate) fn load_ecash_keypair_if_exists( Ok(Some(ecash_key)) } +/// Load every archived ecash keypair sitting alongside the live one. +/// +/// Credentials outlive the epoch that issued them, so the keys put aside by +/// [`archive_ecash_keypair`] have to be readable again after a restart. The epoch each one +/// belongs to is taken from the file *contents*; the name only locates the candidates. +/// +/// Individual failures are logged and skipped rather than propagated: a single unreadable +/// archive must not stop this api from serving the epoch it is currently signing for. +pub(crate) fn load_archived_ecash_keypairs>(store_path: P) -> Vec { + let store_path = store_path.as_ref(); + + let (dir, live_filename) = match key_path_parts(store_path) { + Ok(parts) => parts, + Err(err) => { + warn!("{err} - no archived ecash keys will be loaded"); + return Vec::new(); + } + }; + + let entries = match std::fs::read_dir(dir) { + Ok(entries) => entries, + Err(err) => { + // an api that has never derived a key has no directory yet, which is not a problem + debug!( + "could not read the ecash key directory {}: {err} - no archived keys will be loaded", + dir.display() + ); + return Vec::new(); + } + }; + + let mut loaded = Vec::new(); + for entry in entries.flatten() { + let path = entry.path(); + let Some(named_epoch) = path + .file_name() + .and_then(|name| name.to_str()) + .and_then(|name| archived_key_epoch(live_filename, name)) + else { + continue; + }; + + match nym_pemstore::load_key::(&path) { + Ok(keys) => { + if keys.issued_for_epoch != named_epoch { + // the contents win: that's what the signatures would be produced with + warn!( + "the archived ecash key at {} was issued for epoch {} rather than the {named_epoch} its name claims", + path.display(), + keys.issued_for_epoch + ); + } + debug!( + "loaded archived ecash keys for epoch {} from {}", + keys.issued_for_epoch, + path.display() + ); + loaded.push(keys) + } + Err(err) => warn!( + "failed to load the archived ecash key at {}: {err}. credentials from that epoch may not be servable", + path.display() + ), + } + } + + loaded +} + // the keys can be considered valid if they were generated for the current dkg epoch // and we're either in the "in progress" or "key finalization" states of the DKG -pub(crate) async fn can_validate_coconut_keys( +pub(crate) async fn can_validate_ecash_keys( nyxd_client: &nyxd::Client, issued_for: EpochId, ) -> anyhow::Result { @@ -64,45 +166,124 @@ pub(crate) async fn can_validate_coconut_keys( // and we're either in the "in progress" or "key finalization" states of the DKG let current_dkg_epoch = nyxd_client.get_current_epoch().await?; if issued_for != current_dkg_epoch.epoch_id { - warn!("managed to load coconut keys, but they were generated for epoch {issued_for}. The current epoch is {}. the keys won't be used for credential issuance", current_dkg_epoch.epoch_id); + warn!("managed to load ecash keys, but they were generated for epoch {issued_for}. The current epoch is {}. the keys won't be used for credential issuance", current_dkg_epoch.epoch_id); Ok(false) } else if !matches!( current_dkg_epoch.state, EpochState::InProgress | EpochState::VerificationKeyFinalization { .. } ) { - warn!("managed to load coconut keys, but the current DKG epoch is at {}. the keys won't (yet) be used for credential issuance", current_dkg_epoch.state); + warn!("managed to load ecash keys, but the current DKG epoch is at {}. the keys won't (yet) be used for credential issuance", current_dkg_epoch.state); Ok(false) } else { Ok(true) } } -pub(crate) fn persist_coconut_keypair>( +pub(crate) fn persist_ecash_keypair>( keys: &KeyPairWithEpoch, store_path: P, ) -> anyhow::Result<()> { - nym_pemstore::store_key(keys, store_path).context("coconut key store failure") + nym_pemstore::store_key(keys, store_path).context("ecash key store failure") } -pub(crate) fn archive_coconut_keypair>( +pub(crate) fn archive_ecash_keypair>( store_path: P, epoch_id: EpochId, ) -> anyhow::Result<()> { let store_path = store_path.as_ref(); if !store_path.exists() { - bail!("coconut key does not exist at {}", store_path.display()) + bail!("ecash key does not exist at {}", store_path.display()) } - let dir = store_path - .parent() - .ok_or(anyhow!("the coconut key does not have a valid parent"))?; - let filename = store_path - .file_name() - .ok_or(anyhow!("the coconut key does not have a valid filename"))? - .to_str() - .ok_or(anyhow!("the coconut key filename is not valid UTF8"))?; - let archive_path = dir.join(format!("epoch-{epoch_id}-{filename}.archived")); - std::fs::rename(store_path, archive_path)?; + let (dir, filename) = key_path_parts(store_path)?; + std::fs::rename( + store_path, + dir.join(archived_key_filename(filename, epoch_id)), + )?; Ok(()) } + +#[cfg(test)] +mod tests { + use super::*; + use nym_compact_ecash::ttp_keygen; + use tempfile::tempdir; + + fn dummy_keys(epoch_id: EpochId) -> KeyPairWithEpoch { + KeyPairWithEpoch::new(ttp_keygen(1, 1).unwrap().pop().unwrap(), epoch_id) + } + + #[test] + fn archived_key_names_round_trip() { + let name = archived_key_filename("ecash.pem", 42); + assert_eq!(name, "epoch-42-ecash.pem.archived"); + assert_eq!(archived_key_epoch("ecash.pem", &name), Some(42)); + + // anything that isn't an archive of *this* key is not ours to load + assert_eq!(archived_key_epoch("ecash.pem", "ecash.pem"), None); + assert_eq!( + archived_key_epoch("ecash.pem", "epoch-42-other.pem.archived"), + None + ); + assert_eq!( + archived_key_epoch("ecash.pem", "epoch-2a-ecash.pem.archived"), + None + ); + assert_eq!( + archived_key_epoch("ecash.pem", "epoch--ecash.pem.archived"), + None + ); + } + + #[test] + fn archived_keys_are_loaded_back_by_epoch() -> anyhow::Result<()> { + let dir = tempdir()?; + let key_path = dir.path().join("ecash.pem"); + + for epoch_id in [3, 7] { + persist_ecash_keypair(&dummy_keys(epoch_id), &key_path)?; + archive_ecash_keypair(&key_path, epoch_id)?; + } + + // the live key is not an archive, and neither is unrelated clutter + persist_ecash_keypair(&dummy_keys(8), &key_path)?; + std::fs::write(dir.path().join("persistent_state.json"), "{}")?; + + let mut epochs = load_archived_ecash_keypairs(&key_path) + .into_iter() + .map(|keys| keys.issued_for_epoch) + .collect::>(); + epochs.sort(); + assert_eq!(epochs, vec![3, 7]); + + Ok(()) + } + + #[test] + fn an_unreadable_archive_does_not_hide_the_others() -> anyhow::Result<()> { + let dir = tempdir()?; + let key_path = dir.path().join("ecash.pem"); + + persist_ecash_keypair(&dummy_keys(1), &key_path)?; + archive_ecash_keypair(&key_path, 1)?; + std::fs::write( + dir.path().join(archived_key_filename("ecash.pem", 2)), + "not a key", + )?; + + let loaded = load_archived_ecash_keypairs(&key_path); + assert_eq!(loaded.len(), 1); + assert_eq!(loaded[0].issued_for_epoch, 1); + + Ok(()) + } + + #[test] + fn a_missing_key_directory_yields_no_archives() { + let dir = tempdir().unwrap(); + let key_path = dir.path().join("never-created").join("ecash.pem"); + + assert!(load_archived_ecash_keypairs(&key_path).is_empty()); + } +} diff --git a/nym-api/src/ecash/dkg/controller/mod.rs b/nym-api/src/ecash/dkg/controller/mod.rs index a5751d8f78e..e442951424b 100644 --- a/nym-api/src/ecash/dkg/controller/mod.rs +++ b/nym-api/src/ecash/dkg/controller/mod.rs @@ -25,7 +25,7 @@ pub(crate) mod keys; pub(crate) struct DkgController { pub(crate) dkg_client: DkgClient, - pub(crate) coconut_key_path: PathBuf, + pub(crate) ecash_key_path: PathBuf, pub(crate) state: State, pub(super) rng: R, polling_rate: Duration, @@ -53,7 +53,7 @@ impl DkgController { Ok(DkgController { dkg_client: DkgClient::new(nyxd_client), - coconut_key_path: config.storage_paths.ecash_key_path.clone(), + ecash_key_path: config.storage_paths.ecash_key_path.clone(), state: State::new( config.storage_paths.dkg_persistent_state_path.clone(), persistent_state, @@ -377,7 +377,7 @@ impl DkgController { ) -> DkgController { DkgController { dkg_client, - coconut_key_path: Default::default(), + ecash_key_path: Default::default(), state, rng: crate::ecash::tests::fixtures::test_rng([1u8; 32]), polling_rate: Default::default(), @@ -388,11 +388,11 @@ impl DkgController { rng: rand_chacha::ChaCha20Rng, dkg_client: DkgClient, state: State, - coconut_key_path: PathBuf, + ecash_key_path: PathBuf, ) -> DkgController { DkgController { dkg_client, - coconut_key_path, + ecash_key_path, state, rng, polling_rate: Default::default(), diff --git a/nym-api/src/ecash/dkg/dealing.rs b/nym-api/src/ecash/dkg/dealing.rs index ee7ad5b5a3e..dd2c44e86b0 100644 --- a/nym-api/src/ecash/dkg/dealing.rs +++ b/nym-api/src/ecash/dkg/dealing.rs @@ -2,7 +2,7 @@ // SPDX-License-Identifier: GPL-3.0-only use crate::ecash::dkg; -use crate::ecash::dkg::controller::keys::archive_coconut_keypair; +use crate::ecash::dkg::controller::keys::archive_ecash_keypair; use crate::ecash::dkg::controller::DkgController; use crate::ecash::error::EcashError; use crate::ecash::keys::KeyPairWithEpoch; @@ -273,7 +273,7 @@ impl DkgController { &mut self, epoch_id: EpochId, expected_key_size: u32, - old_keypair: KeyPairWithEpoch, + old_keypair: &KeyPairWithEpoch, ) -> Result<(), DealingGenerationError> { // make sure we're allowed to participate in resharing if !self.can_reshare(epoch_id).await? { @@ -304,7 +304,7 @@ impl DkgController { } // generate resharing dealings - let prior_secrets = old_keypair.hazmat_into_secrets(); + let prior_secrets = old_keypair.hazmat_secrets(); // safety: // the prior secrets will be immediately converted into `Polynomial` with the specified coefficient // that does implement `ZeroizeOnDrop` @@ -405,7 +405,7 @@ impl DkgController { if resharing { debug!("resharing + prior key"); - self.handle_resharing_with_prior_key(epoch_id, expected_key_size, old_keypair) + self.handle_resharing_with_prior_key(epoch_id, expected_key_size, &old_keypair) .await?; } else { debug!("no resharing + prior key"); @@ -418,13 +418,19 @@ impl DkgController { // (so we won't be able to create resharing dealings again if we crashed since we won't be able to load the keys) self.state.persist()?; // archive the keypair - if let Err(source) = archive_coconut_keypair(&self.coconut_key_path, keypair_epoch) { + if let Err(source) = archive_ecash_keypair(&self.ecash_key_path, keypair_epoch) { return Err(DealingGenerationError::KeyArchiveFailure { epoch_id, - path: self.coconut_key_path.clone(), + path: self.ecash_key_path.clone(), source, }); } + + // it's no longer the key we sign with, but credentials issued under its epoch + // outlive the rotation and still need their auxiliary signatures, so keep it + // around rather than dropping it here. a restart recovers it from the archive + // we just wrote. + self.state.archive_ecash_keypair(old_keypair).await; } else { // sure, the if statements could be collapsed, but i prefer to explicitly repeat the block for readability if resharing { @@ -470,6 +476,7 @@ impl DkgController { #[cfg(test)] pub(crate) mod tests { use super::*; + use crate::ecash::dkg::controller::keys::load_archived_ecash_keypairs; use crate::ecash::dkg::state::registration::KeyRejectionReason; use crate::ecash::keys::KeyPair; use crate::ecash::tests::fixtures::{dealers_fixtures, test_rng, TestingDkgControllerBuilder}; @@ -667,6 +674,56 @@ pub(crate) mod tests { Ok(()) } + /// B3: rotating away from an epoch must not lose the keys it was signed with. Credentials + /// issued under it stay spendable for days afterwards and still need their auxiliary + /// signatures, which only those keys can produce. + /// + /// A restarted api recovers them from the archive on disk; a process that stays up through + /// the ceremony has to keep them itself, so both are asserted here. Neither depends on + /// whether the rotation is a resharing or a reset. + #[tokio::test] + async fn dealing_exchange_retains_the_prior_keys_for_their_own_epoch() -> anyhow::Result<()> { + for resharing in [true, false] { + let mut rng = test_rng([69u8; 32]); + let dealers = dealers_fixtures(&mut rng, 4); + let self_dealer = dealers[0].clone(); + + let epoch = 1; + let prior_epoch = epoch - 1; + + let mut keys = ttp_keygen(3, 4).unwrap(); + let ecash_keys = KeyPair::new(); + ecash_keys + .set(KeyPairWithEpoch::new(keys.pop().unwrap(), prior_epoch)) + .await; + + let mut controller = TestingDkgControllerBuilder::default() + .with_threshold(3) + .with_dealers(dealers.clone()) + .with_as_dealer(self_dealer.clone()) + .with_keypair(ecash_keys.clone()) + .with_initial_epoch_id(epoch) + .build() + .await; + + controller.dealing_exchange(epoch, resharing).await?; + + // we no longer sign for the epoch those keys belonged to ... + assert!(ecash_keys.keys_for_epoch(epoch).await.is_err()); + + // ... but we can still serve it, without having been restarted + let retained = ecash_keys.keys_for_epoch(prior_epoch).await?; + assert_eq!(retained.issued_for_epoch, prior_epoch); + + // and a restart would find the same keys on disk + let archived = load_archived_ecash_keypairs(&controller.ecash_key_path); + assert_eq!(archived.len(), 1); + assert_eq!(archived[0].issued_for_epoch, prior_epoch); + } + + Ok(()) + } + #[tokio::test] async fn resharing_inside_initial_set() -> anyhow::Result<()> { let mut rng = test_rng([69u8; 32]); diff --git a/nym-api/src/ecash/dkg/key_derivation.rs b/nym-api/src/ecash/dkg/key_derivation.rs index ea8f122a418..0f4411fbe8c 100644 --- a/nym-api/src/ecash/dkg/key_derivation.rs +++ b/nym-api/src/ecash/dkg/key_derivation.rs @@ -2,7 +2,7 @@ // SPDX-License-Identifier: GPL-3.0-only use crate::ecash::dkg; -use crate::ecash::dkg::controller::keys::persist_coconut_keypair; +use crate::ecash::dkg::controller::keys::persist_ecash_keypair; use crate::ecash::dkg::controller::DkgController; use crate::ecash::dkg::state::key_derivation::{DealerRejectionReason, DerivationFailure}; use crate::ecash::error::EcashError; @@ -664,7 +664,7 @@ impl DkgController { }; // before submitting our keys to the contract, persist the generated keypair - if let Err(source) = persist_coconut_keypair(&coconut_keypair, &self.coconut_key_path) { + if let Err(source) = persist_ecash_keypair(&coconut_keypair, &self.ecash_key_path) { return Err(KeyDerivationError::KeyPersistenceFailure { source }); } diff --git a/nym-api/src/ecash/dkg/state/mod.rs b/nym-api/src/ecash/dkg/state/mod.rs index d6401de37f1..a9f0b0d510f 100644 --- a/nym-api/src/ecash/dkg/state/mod.rs +++ b/nym-api/src/ecash/dkg/state/mod.rs @@ -360,6 +360,12 @@ impl State { self.coconut_keypair.take().await } + /// Retain a keypair whose epoch has rotated, so credentials issued under it can still be + /// served the auxiliary signatures they need. + pub async fn archive_ecash_keypair(&self, keypair: KeyPairWithEpoch) { + self.coconut_keypair.archive(keypair).await + } + pub fn invalidate_coconut_keypair(&self) { self.coconut_keypair.invalidate() } diff --git a/nym-api/src/ecash/keys/mod.rs b/nym-api/src/ecash/keys/mod.rs index e7e8bf94c64..25c9514f350 100644 --- a/nym-api/src/ecash/keys/mod.rs +++ b/nym-api/src/ecash/keys/mod.rs @@ -5,6 +5,7 @@ use crate::ecash::error::EcashError; use nym_coconut_dkg_common::types::EpochId; use nym_compact_ecash::{SecretKeyAuth, VerificationKeyAuth}; use nym_dkg::Scalar; +use std::collections::HashMap; use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::Arc; use tokio::sync::{RwLock, RwLockReadGuard}; @@ -13,8 +14,12 @@ mod persistence; #[derive(Clone, Debug)] pub struct KeyPair { - // keys: Arc>>, keys: Arc>>, + + /// Keys derived for epochs that have since rotated. Credentials outlive the epoch that + /// issued them, so their auxiliary signatures have to remain producible afterwards. + archived: Arc>>, + valid: Arc, } @@ -35,13 +40,14 @@ impl KeyPairWithEpoch { // extract underlying secrets from the coconut's secret key. // the caller of this function must exercise extreme care to not misuse the data and ensuring it gets zeroized // `KeyPair` and `SecretKey` implement ZeroizeOnDrop; `Scalar` does not (it implements `Copy` -> important to keep in mind) - pub(crate) fn hazmat_into_secrets(self) -> Vec { + // + // this borrows rather than consumes because the keypair outlives the resharing it feeds: + // it gets archived for the epoch it was issued for, whose credentials still need it + pub(crate) fn hazmat_secrets(&self) -> Vec { let (x, mut secrets) = self.keys.secret_key().hazmat_to_raw(); secrets.insert(0, x); secrets - // since `nym_coconut_interface::KeyPair` implements `ZeroizeOnDrop` and we took ownership of the keypair, - // it will get zeroized after we exit this scope } } @@ -49,6 +55,7 @@ impl KeyPair { pub fn new() -> Self { Self { keys: Arc::new(RwLock::new(None)), + archived: Arc::new(RwLock::new(HashMap::new())), valid: Arc::new(Default::default()), } } @@ -57,6 +64,51 @@ impl KeyPair { self.keys.write().await.take() } + /// Retain a keypair belonging to an epoch that is no longer current, so that credentials + /// issued under it can still be served their auxiliary signatures. + pub async fn archive(&self, keypair: KeyPairWithEpoch) { + let epoch_id = keypair.issued_for_epoch; + self.archived.write().await.insert(epoch_id, keypair); + } + + /// The epoch our currently held keys were derived for, regardless of whether they may + /// yet be used for issuance. + async fn current_key_epoch(&self) -> Option { + self.keys + .read() + .await + .as_ref() + .map(|keys| keys.issued_for_epoch) + } + + /// The keys derived for `epoch_id`, whether that is the epoch we're actively signing for + /// or one that has since rotated. + pub async fn keys_for_epoch( + &self, + epoch_id: EpochId, + ) -> Result, EcashError> { + let current = self.current_key_epoch().await; + + // the epoch we're actively signing for goes through the usual validity gate + if current == Some(epoch_id) { + return self.keys().await; + } + + // any other epoch comes out of the archive, and is deliberately not subject to that + // gate: it tracks whether the *current* keys may be used for issuance, and the chain + // is the authority on whether an archived share was ever verified + RwLockReadGuard::try_map(self.archived.read().await, |archived| { + archived.get(&epoch_id) + }) + .map_err(|_| match current { + Some(available) => EcashError::InvalidSigningKeyEpoch { + requested: epoch_id, + available, + }, + None => EcashError::KeyPairNotDerivedYet, + }) + } + pub async fn get(&self) -> Option>> { if self.is_valid() { Some(self.read_keys().await) @@ -108,3 +160,68 @@ impl KeyPair { self.valid.store(false, Ordering::SeqCst); } } + +#[cfg(test)] +mod tests { + use super::*; + use nym_compact_ecash::ttp_keygen; + + fn dummy_keys(epoch_id: EpochId) -> KeyPairWithEpoch { + KeyPairWithEpoch::new(ttp_keygen(1, 1).unwrap().pop().unwrap(), epoch_id) + } + + #[tokio::test] + async fn the_epoch_we_sign_for_is_still_subject_to_the_validity_gate() { + let keys = KeyPair::new(); + keys.set(dummy_keys(5)).await; + + // derived but not yet finalised on chain + assert!(matches!( + keys.keys_for_epoch(5).await, + Err(EcashError::KeyPairNotDerivedYet) + )); + + keys.validate(); + assert_eq!(keys.keys_for_epoch(5).await.unwrap().issued_for_epoch, 5); + } + + /// The gate above tracks whether the keys we sign *with* may be used for issuance. An + /// archived epoch is not covered by it: the chain already settled which shares were + /// verified for that epoch, and its credentials need their material regardless of what + /// the current ceremony is doing. + #[tokio::test] + async fn an_archived_epoch_is_served_whatever_the_current_keys_are_doing() { + let keys = KeyPair::new(); + keys.set(dummy_keys(5)).await; + keys.archive(dummy_keys(4)).await; + + assert_eq!(keys.keys_for_epoch(4).await.unwrap().issued_for_epoch, 4); + + // ... including in the middle of the next ceremony, when the live keys are unusable + keys.invalidate(); + assert_eq!(keys.keys_for_epoch(4).await.unwrap().issued_for_epoch, 4); + } + + #[tokio::test] + async fn an_epoch_we_never_held_keys_for_is_refused() { + let keys = KeyPair::new(); + + // nothing at all, so there is no epoch to report as the one we do have + assert!(matches!( + keys.keys_for_epoch(4).await, + Err(EcashError::KeyPairNotDerivedYet) + )); + + keys.set(dummy_keys(5)).await; + keys.validate(); + keys.archive(dummy_keys(4)).await; + + assert!(matches!( + keys.keys_for_epoch(3).await, + Err(EcashError::InvalidSigningKeyEpoch { + requested: 3, + available: 5 + }) + )); + } +} diff --git a/nym-api/src/ecash/state/mod.rs b/nym-api/src/ecash/state/mod.rs index ceb3c551bd2..28169654278 100644 --- a/nym-api/src/ecash/state/mod.rs +++ b/nym-api/src/ecash/state/mod.rs @@ -55,7 +55,7 @@ use std::sync::Arc; use time::{Date, OffsetDateTime}; use tokio::sync::{RwLockReadGuard, RwLockWriteGuard}; use tokio::task::JoinHandle; -use tracing::{debug, error, info, warn}; +use tracing::{debug, info, warn}; pub(crate) mod auxiliary; mod cleaner; @@ -199,14 +199,22 @@ impl EcashState { /// Ensures that this nym-api is one of ecash signers for the current epoch pub(crate) async fn ensure_signer(&self) -> Result<()> { + let epoch_id = self.current_dkg_epoch().await?; + self.ensure_signer_for_epoch(epoch_id).await + } + + /// Ensures that this nym-api was one of the ecash signers for the given epoch. + /// + /// Credentials outlive the epoch that issued them, so the material they need is asked of + /// *that* epoch's signers - which is not necessarily who signs today. An api that has since + /// dropped out of the set still holds the keys, and refusing on the strength of the current + /// epoch alone can leave a past epoch permanently short of the threshold it needs. + pub(crate) async fn ensure_signer_for_epoch(&self, epoch_id: EpochId) -> Result<()> { if self.local.explicitly_disabled { return Err(EcashError::NotASigner); } - let epoch_id = self.current_dkg_epoch().await?; - let is_epoch_signer = self.is_dkg_signer(epoch_id).await?; - - if !is_epoch_signer { + if !self.is_dkg_signer(epoch_id).await? { return Err(EcashError::NotASigner); } @@ -381,20 +389,11 @@ impl EcashState { }); } - // 2. perform actual issuance - let signing_keys = self.local.ecash_keypair.keys().await?; - if signing_keys.issued_for_epoch != epoch_id { - // TODO: this should get handled at some point, - // because if it was a past epoch we **do** have those keys. - // they're just archived - - error!("received partial coin index signature request for an invalid epoch ({epoch_id}). our key was derived for epoch {}", signing_keys.issued_for_epoch); - return Err(EcashError::InvalidSigningKeyEpoch { - requested: epoch_id, - available: signing_keys.issued_for_epoch, - }) - } + // + // a past epoch is answered from the key it was signed with, which we archived + // rather than destroyed when it rotated + let signing_keys = self.local.ecash_keypair.keys_for_epoch(epoch_id).await?; let master_vk = self.master_verification_key(Some(epoch_id)).await?; let signatures = sign_coin_indices( nym_compact_ecash::ecash_parameters(), @@ -403,7 +402,10 @@ impl EcashState { )?; // 3. save the signatures in the storage for when we reboot - self.aux.storage.insert_partial_coin_index_signatures(epoch_id, &signatures).await?; + self.aux + .storage + .insert_partial_coin_index_signatures(epoch_id, &signatures) + .await?; Ok(IssuedCoinIndicesSignatures { epoch_id, @@ -522,17 +524,10 @@ impl EcashState { } // 3. perform actual issuance - let signing_keys = self.local.ecash_keypair.keys().await?; - if signing_keys.issued_for_epoch != epoch_id { - // TODO: this should get handled at some point, - // because if it was a past epoch we **do** have those keys. - // they're just archived - error!("received partial expiration date signature request for an invalid epoch ({epoch_id}). our key was derived for epoch {}", signing_keys.issued_for_epoch); - return Err(EcashError::InvalidSigningKeyEpoch { - requested: epoch_id, - available: signing_keys.issued_for_epoch, - }); - } + // + // a past epoch is answered from the key it was signed with, which we archived + // rather than destroyed when it rotated + let signing_keys = self.local.ecash_keypair.keys_for_epoch(epoch_id).await?; let signatures = sign_expiration_date( signing_keys.keys.secret_key(), @@ -1092,6 +1087,9 @@ impl EcashState { #[cfg(test)] mod tests { use super::*; + use crate::ecash::dkg::controller::keys::{ + archive_ecash_keypair, load_archived_ecash_keypairs, persist_ecash_keypair, + }; use crate::ecash::keys::KeyPairWithEpoch; use crate::ecash::tests::contract_chain::SharedContractChain; use crate::ecash::tests::contract_harness::{ @@ -1272,12 +1270,153 @@ mod tests { Ok(()) } - /// The layer beneath: once the aggregation above stops silently switching epochs, it asks - /// this api for its *own* partial signatures for a past epoch. It cannot produce them - the - /// key it holds belongs to the current epoch and the old one is archived - so it has to say - /// so rather than sign with the wrong key and label the result with the wrong epoch. + /// B3: a ticketbook outlives the epoch that issued it, so after a rotation this api is + /// still asked for that epoch's partial signatures. It archived the key rather than + /// destroying it, so it can still produce them - and with a reset it *must*, because the + /// master key changed and no other epoch's material will verify for those books. /// - /// The coin index sibling has guarded this all along; only this path was missing it. + /// The archive is read back the way a restarted api reads it: off disk, by epoch. + #[tokio::test] + async fn partial_signatures_for_a_past_epoch_are_produced_from_the_archived_key( + ) -> anyhow::Result<()> { + let chain = SharedContractChain::new(1); + initiate_dkg(&chain); + + cheap::run_ceremony(&chain, false); + let past_epoch = chain.epoch().epoch_id; + let past_keys = cheap::install_real_verification_keys(&chain); + + // the key this api signed the first epoch with gets archived as the next ceremony begins + let key_dir = tempfile::tempdir()?; + let key_path = key_dir.path().join("ecash.pem"); + persist_ecash_keypair( + &KeyPairWithEpoch::new(past_keys.keypairs.into_iter().next().unwrap(), past_epoch), + &key_path, + )?; + archive_ecash_keypair(&key_path, past_epoch)?; + + // a reset, so the master key the first epoch's credentials verify against is gone + trigger_reset(&chain); + cheap::run_ceremony(&chain, false); + let current_epoch = chain.epoch().epoch_id; + let current_keys = cheap::install_real_verification_keys(&chain); + assert_ne!(past_keys.master, current_keys.master); + + let me = chain.group_member_addresses()[0].clone(); + let state = contract_backed_ecash_state(&chain, me).await; + + // as a restarting api would: the live key for the epoch it now signs for, plus + // whatever it finds archived alongside it + state + .local + .ecash_keypair + .set(KeyPairWithEpoch::new( + current_keys.keypairs.into_iter().next().unwrap(), + current_epoch, + )) + .await; + state.local.ecash_keypair.validate(); + let archived = load_archived_ecash_keypairs(&key_path); + assert_eq!(archived.len(), 1); + for keys in archived { + state.local.ecash_keypair.archive(keys).await; + } + + let expiration_date = ecash_today_date(); + + // both kinds of auxiliary material must come back stamped with the epoch asked for + let expiration_partial = state + .partial_expiration_date_signatures(expiration_date, past_epoch) + .await?; + assert_eq!(expiration_partial.epoch_id, past_epoch); + + let coin_index_partial = state + .partial_coin_index_signatures(Some(past_epoch)) + .await?; + assert_eq!(coin_index_partial.epoch_id, past_epoch); + drop(expiration_partial); + drop(coin_index_partial); + + // and it has to be the *right* material: aggregation verifies each partial against + // the epoch's master key, so signing with the current key would fail here + let master_expiration = state + .master_expiration_date_signatures(expiration_date, past_epoch) + .await?; + assert_eq!(master_expiration.epoch_id, past_epoch); + drop(master_expiration); + + let master_coin_indices = state.master_coin_index_signatures(Some(past_epoch)).await?; + assert_eq!(master_coin_indices.epoch_id, past_epoch); + + Ok(()) + } + + /// B3, at the gate sitting in front of it: the material a past epoch's credentials need is + /// asked of *that* epoch's signers, which is not necessarily whoever signs today. An api + /// that has since dropped out of the set still holds the keys, and refusing it on the + /// strength of the current epoch alone can leave a past epoch permanently short of the + /// threshold its aggregation needs. + #[tokio::test] + async fn a_signer_that_left_the_set_still_answers_for_the_epoch_it_signed() -> anyhow::Result<()> + { + let chain = SharedContractChain::new(4); + initiate_dkg(&chain); + + cheap::run_ceremony(&chain, false); + let past_epoch = chain.epoch().epoch_id; + let past_keys = cheap::install_real_verification_keys(&chain); + + // the last of the four archives the key it signed that epoch with + let me = chain.group_member_addresses()[3].clone(); + let key_dir = tempfile::tempdir()?; + let key_path = key_dir.path().join("ecash.pem"); + persist_ecash_keypair( + &KeyPairWithEpoch::new(past_keys.keypairs.into_iter().nth(3).unwrap(), past_epoch), + &key_path, + )?; + archive_ecash_keypair(&key_path, past_epoch)?; + + // a reset in which its share never gets verified. 3 of 4 still meets the threshold, + // so the epoch concludes without it + trigger_reset(&chain); + cheap::register_dealers(&chain, false); + cheap::advance(&chain); + cheap::submit_dealings(&chain, false); + cheap::advance(&chain); + cheap::submit_vk_shares(&chain, false); + cheap::advance(&chain); + cheap::advance(&chain); + cheap::verify_first_vk_shares(&chain, false, 3); + cheap::advance(&chain); + let current_epoch = chain.epoch().epoch_id; + assert_ne!(past_epoch, current_epoch); + cheap::install_real_verification_keys(&chain); + + let state = contract_backed_ecash_state(&chain, me).await; + for keys in load_archived_ecash_keypairs(&key_path) { + state.local.ecash_keypair.archive(keys).await; + } + + // it is not one of today's signers ... + assert!(matches!( + state.ensure_signer().await, + Err(EcashError::NotASigner) + )); + + // ... but it is still one of the epoch whose credentials are doing the asking, and it + // can still produce what they need + state.ensure_signer_for_epoch(past_epoch).await?; + let partial = state + .partial_expiration_date_signatures(ecash_today_date(), past_epoch) + .await?; + assert_eq!(partial.epoch_id, past_epoch); + + Ok(()) + } + + /// The archive only answers for epochs it actually holds. An api that never derived a key + /// for the requested epoch - because it was not yet in the group, or lost the file - has to + /// say so rather than sign with the wrong key and label the result with the wrong epoch. #[tokio::test] async fn partial_expiration_date_signatures_are_refused_for_an_epoch_we_have_no_key_for( ) -> anyhow::Result<()> { diff --git a/nym-api/src/ecash/tests/fixtures.rs b/nym-api/src/ecash/tests/fixtures.rs index 8ed69c1fb1d..46fbc706043 100644 --- a/nym-api/src/ecash/tests/fixtures.rs +++ b/nym-api/src/ecash/tests/fixtures.rs @@ -3,7 +3,7 @@ use crate::ecash::dkg; use crate::ecash::dkg::client::DkgClient; -use crate::ecash::dkg::controller::keys::persist_coconut_keypair; +use crate::ecash::dkg::controller::keys::persist_ecash_keypair; use crate::ecash::dkg::controller::DkgController; use crate::ecash::dkg::state::State; use crate::ecash::keys::KeyPair; @@ -194,13 +194,13 @@ impl TestingDkgControllerBuilder { let tmp_dir = tempdir().unwrap(); let dkg_state_path = tmp_dir.path().join("persistent_state.json"); - let coconut_key_path = tmp_dir.path().join("coconut_keypair.pem"); + let ecash_key_path = tmp_dir.path().join("ecash_keypair.pem"); // if we had a keypair, make sure to put it on disk otherwise, if we're testing dealing exchange, // we'll fail to archive it let keypair = if let Some(keypair) = self.keypair { if let Some(keys) = keypair.read_keys().await.as_ref() { - persist_coconut_keypair(keys, &coconut_key_path).unwrap(); + persist_ecash_keypair(keys, &ecash_key_path).unwrap(); } keypair } else { @@ -231,7 +231,7 @@ impl TestingDkgControllerBuilder { // } TestingDkgController { - controller: DkgController::test_mock(rng, dummy_client, state, coconut_key_path), + controller: DkgController::test_mock(rng, dummy_client, state, ecash_key_path), chain_state, _tmp_dir: tmp_dir, } diff --git a/nym-api/src/support/cli/run.rs b/nym-api/src/support/cli/run.rs index d4a79080e4a..7e6bb99eede 100644 --- a/nym-api/src/support/cli/run.rs +++ b/nym-api/src/support/cli/run.rs @@ -4,7 +4,8 @@ use crate::ecash::client::Client; use crate::ecash::comm::QueryCommunicationChannel; use crate::ecash::dkg::controller::keys::{ - can_validate_coconut_keys, load_bte_keypair, load_ecash_keypair_if_exists, + can_validate_ecash_keys, load_archived_ecash_keypairs, load_bte_keypair, + load_ecash_keypair_if_exists, }; use crate::ecash::dkg::controller::DkgController; use crate::ecash::state::EcashState; @@ -154,11 +155,24 @@ async fn start_nym_api_tasks(mut config: Config) -> anyhow::Result Date: Wed, 19 Aug 2026 16:14:18 +0100 Subject: [PATCH 2/5] fix(credential-proxy): stop a ceremony poisoning the proxy's signer cache The credential-proxy carried both defects that were fixed in nym-api but never had its own half done, and it sits on the issuance path, so a DKG re-run takes it out too. `epoch_clients` cached whatever the initialiser returned successfully and never expires. The moment a ceremony starts the epoch id increments, the new epoch has no verified shares yet, and the query returns an empty set - which was then remembered for the life of the process, so the proxy kept failing to fan out long after the ceremony had finished. It now only caches once the ceremony for that epoch has concluded, answering uncached before then. The same initialiser also collected the shares into a Result, so a single share that was never verified - or that carries an announce address the DKG contract never validated, since it validates none - failed the whole epoch rather than being skipped. It now goes through `usable_ecash_api_clients`, the same helper nym-api uses, which drops unusable shares and logs each one. `ecash_clients` consequently returns an owned Vec rather than a cache guard (the uncached path has no guard to hand out), which is what nym-api already does; both wrappers and their call sites follow, dropping two clones that the guard used to force. The "has this epoch's ceremony finished" comparison moves onto `Epoch` itself as `is_epoch_concluded`, since nym-api and the proxy were about to hold two copies of it, and gets unit tests there - the proxy cannot test its own use of it, because `ChainClient` is a concrete signing client with no stub. --- .../coconut-dkg/src/types.rs | 57 ++++++++++++++ .../src/shared_state/ecash_state.rs | 75 +++++++++++++++---- .../credential-proxy/src/shared_state/mod.rs | 2 +- .../src/ticketbook_manager/wallet_shares.rs | 2 +- nym-api/src/ecash/comm.rs | 15 ++-- .../src/ticketbook_manager/state.rs | 2 +- 6 files changed, 127 insertions(+), 26 deletions(-) diff --git a/common/cosmwasm-smart-contracts/coconut-dkg/src/types.rs b/common/cosmwasm-smart-contracts/coconut-dkg/src/types.rs index 20d4920db6e..c63679db803 100644 --- a/common/cosmwasm-smart-contracts/coconut-dkg/src/types.rs +++ b/common/cosmwasm-smart-contracts/coconut-dkg/src/types.rs @@ -4,6 +4,7 @@ #![allow(clippy::derivable_impls)] // MAX: surpressing warning for the moment, will be dealt with in a different PR (TODO) use cosmwasm_schema::cw_serde; +use std::cmp::Ordering; use std::fmt::{Display, Formatter}; use std::str::FromStr; @@ -200,6 +201,20 @@ impl Epoch { ) } + /// Whether the ceremony for `epoch_id` has finished, judged against `self` as the current + /// epoch. + /// + /// Anything before the current epoch has necessarily finished, and an epoch that has not + /// been reached yet certainly has not. Callers working from a cached copy of the current + /// epoch can only get a pessimistic answer out of a stale one, never a premature "yes". + pub fn is_epoch_concluded(&self, epoch_id: EpochId) -> bool { + match epoch_id.cmp(&self.epoch_id) { + Ordering::Less => true, + Ordering::Greater => false, + Ordering::Equal => self.state.is_in_progress(), + } + } + pub fn final_timestamp_secs(&self) -> Option { let mut finish = self.deadline?.seconds(); let time_configuration = self.time_configuration; @@ -340,3 +355,45 @@ impl EpochState { matches!(self, EpochState::WaitingInitialisation) } } + +#[cfg(test)] +mod tests { + use super::*; + + fn epoch_at(epoch_id: EpochId, state: EpochState) -> Epoch { + Epoch::new( + state, + epoch_id, + TimeConfiguration::default(), + Timestamp::from_seconds(0), + ) + } + + /// Signer sets are only settled once a ceremony finishes, and callers cache them per epoch. + /// Answering "concluded" for an epoch still mid-ceremony would have them remember a set + /// that is empty or partial. + #[test] + fn an_epoch_has_concluded_only_once_its_ceremony_is_done() { + let current = epoch_at(5, EpochState::InProgress); + + // earlier epochs are finished by definition, and later ones cannot have started + assert!(current.is_epoch_concluded(4)); + assert!(!current.is_epoch_concluded(6)); + + // the current one depends on where its ceremony has got to + assert!(current.is_epoch_concluded(5)); + for state in [ + EpochState::WaitingInitialisation, + EpochState::PublicKeySubmission { resharing: false }, + EpochState::DealingExchange { resharing: false }, + EpochState::VerificationKeySubmission { resharing: true }, + EpochState::VerificationKeyValidation { resharing: false }, + EpochState::VerificationKeyFinalization { resharing: false }, + ] { + assert!( + !epoch_at(5, state).is_epoch_concluded(5), + "{state} was treated as a concluded ceremony" + ); + } + } +} diff --git a/common/credential-proxy/src/shared_state/ecash_state.rs b/common/credential-proxy/src/shared_state/ecash_state.rs index 78187fa93d1..cd3cd3532bb 100644 --- a/common/credential-proxy/src/shared_state/ecash_state.rs +++ b/common/credential-proxy/src/shared_state/ecash_state.rs @@ -33,7 +33,7 @@ use nym_validator_client::nyxd::contract_traits::{DkgQueryClient, PagedDkgQueryC use std::time::Duration; use time::{Date, OffsetDateTime}; use tokio::sync::{RwLock, RwLockReadGuard}; -use tracing::info; +use tracing::{info, warn}; use url::Url; pub struct EcashState { @@ -55,6 +55,24 @@ pub struct EcashState { CachedImmutableItems<(EpochId, Date), AggregatedExpirationDateSignatures>, } +fn construct_usable_ecash_api_clients(shares: Vec) -> Vec { + let mut clients = Vec::with_capacity(shares.len()); + + for share in shares { + let owner = share.owner.clone(); + let epoch_id = share.epoch_id; + + match construct_ecash_api_client(share) { + Ok(client) => clients.push(client), + Err(err) => { + warn!("ignoring the key share of {owner} for epoch {epoch_id}: {err}") + } + } + } + + clients +} + fn construct_ecash_api_client(share: ContractVKShare) -> Result { if !share.verified { return Err(EcashApiError::UnverifiedShare); @@ -123,23 +141,55 @@ impl EcashState { self.required_deposit_cache.get_or_update(client).await } + /// Whether the ceremony for `epoch_id` has concluded, so its set of signers is settled. + async fn epoch_concluded( + &self, + client: &ChainClient, + epoch_id: EpochId, + ) -> Result { + Ok(self + .current_epoch(client) + .await? + .is_epoch_concluded(epoch_id)) + } + + /// The signers registered for `epoch_id`, skipping any whose share cannot be used. + /// + /// A single share that was never verified - or that carries an announce address the DKG + /// contract never validated - must not deny the caller every *other* signer of that epoch. + async fn registered_ecash_clients( + &self, + client: &ChainClient, + epoch_id: EpochId, + ) -> Result, CredentialProxyError> { + Ok(construct_usable_ecash_api_clients( + client + .query_chain() + .await + .get_all_verification_key_shares(epoch_id) + .await?, + )) + } + pub async fn ecash_clients( &self, client: &ChainClient, epoch_id: EpochId, - ) -> Result>, CredentialProxyError> { + ) -> Result, CredentialProxyError> { + // the moment a ceremony starts, the epoch id increments and the new epoch has no + // verified shares yet. this cache has no expiry, so answering from it then would + // remember an empty signer set for the life of the process - and this proxy would + // keep failing to fan out long after the ceremony finished. + if !self.epoch_concluded(client, epoch_id).await? { + return self.registered_ecash_clients(client, epoch_id).await; + } + self.epoch_clients .get_or_init(epoch_id, || async { - Ok(client - .query_chain() - .await - .get_all_verification_key_shares(epoch_id) - .await? - .into_iter() - .map(construct_ecash_api_client) - .collect::, EcashApiError>>()?) + self.registered_ecash_clients(client, epoch_id).await }) .await + .map(|guard| guard.clone()) } pub async fn current_epoch(&self, client: &ChainClient) -> Result { @@ -277,8 +327,7 @@ impl EcashState { }; let shares = - query_all_threshold_apis(all_apis.clone(), threshold, get_partial_signatures) - .await?; + query_all_threshold_apis(all_apis, threshold, get_partial_signatures).await?; let aggregated = aggregate_annotated_indices_signatures( nym_credentials_interface::ecash_parameters(), @@ -352,7 +401,7 @@ impl EcashState { }; let shares = - query_all_threshold_apis(all_apis.clone(), threshold, get_partial_signatures) + query_all_threshold_apis(all_apis, threshold, get_partial_signatures) .await?; let aggregated = aggregate_annotated_expiration_signatures( diff --git a/common/credential-proxy/src/shared_state/mod.rs b/common/credential-proxy/src/shared_state/mod.rs index 565f49a8f57..93626e04cad 100644 --- a/common/credential-proxy/src/shared_state/mod.rs +++ b/common/credential-proxy/src/shared_state/mod.rs @@ -222,7 +222,7 @@ impl CredentialProxyState { pub async fn ecash_clients( &self, epoch_id: EpochId, - ) -> Result>, CredentialProxyError> { + ) -> Result, CredentialProxyError> { self.ecash_state() .ecash_clients(self.client(), epoch_id) .await diff --git a/common/credential-proxy/src/ticketbook_manager/wallet_shares.rs b/common/credential-proxy/src/ticketbook_manager/wallet_shares.rs index de4c3f848f2..ab370514f0e 100644 --- a/common/credential-proxy/src/ticketbook_manager/wallet_shares.rs +++ b/common/credential-proxy/src/ticketbook_manager/wallet_shares.rs @@ -48,7 +48,7 @@ impl TicketbookManager { // before we commit to making the deposit, ensure we have required signatures cached and stored self.ensure_global_data_cached(epoch, expiration_date) .await?; - let ecash_api_clients = self.state.ecash_clients(epoch).await?.clone(); + let ecash_api_clients = self.state.ecash_clients(epoch).await?; let deposit_data = self .state diff --git a/nym-api/src/ecash/comm.rs b/nym-api/src/ecash/comm.rs index 640563dbe28..26d8a8e09ad 100644 --- a/nym-api/src/ecash/comm.rs +++ b/nym-api/src/ecash/comm.rs @@ -8,7 +8,7 @@ use async_trait::async_trait; use nym_coconut_dkg_common::types::{Epoch, EpochId}; use nym_dkg::Threshold; use nym_validator_client::EcashApiClient; -use std::cmp::{min, Ordering}; +use std::cmp::min; use time::OffsetDateTime; use tokio::sync::{RwLock, RwLockWriteGuard}; @@ -177,15 +177,10 @@ impl APICommunicationChannel for QueryCommunicationChannel { } async fn epoch_concluded(&self, epoch_id: EpochId) -> Result { - let current = self.current_epoch_data().await?; - - // anything before the current epoch has necessarily finished, and an epoch we - // have not reached yet certainly has not - match epoch_id.cmp(¤t.epoch_id) { - Ordering::Less => Ok(true), - Ordering::Greater => Ok(false), - Ordering::Equal => Ok(current.state.is_in_progress()), - } + Ok(self + .current_epoch_data() + .await? + .is_epoch_concluded(epoch_id)) } } diff --git a/nym-node-status-api/nym-node-status-api/src/ticketbook_manager/state.rs b/nym-node-status-api/nym-node-status-api/src/ticketbook_manager/state.rs index d6939ad1d99..ebfc6541bb3 100644 --- a/nym-node-status-api/nym-node-status-api/src/ticketbook_manager/state.rs +++ b/nym-node-status-api/nym-node-status-api/src/ticketbook_manager/state.rs @@ -224,7 +224,7 @@ impl TicketbookManagerState { pub async fn ecash_clients( &self, epoch_id: EpochId, - ) -> Result>, CredentialProxyError> { + ) -> Result, CredentialProxyError> { self.ecash_state() .ecash_clients(self.client(), epoch_id) .await From 86becca450d334a88cbec92b9b828e603922620d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C4=99drzej=20Stuczy=C5=84ski?= Date: Thu, 20 Aug 2026 09:47:44 +0100 Subject: [PATCH 3/5] fix(nym-api): serve a concluded epoch's data while a later ceremony runs `ensure_dkg_not_in_progress` was applied to every ecash route regardless of the epoch being asked about, so for the 11-22 minutes a ceremony takes (two of its phases cannot short-circuit) nothing was served at all - including for epochs that concluded long ago and whose data cannot change. That turns an issuance pause into a spending outage. The aggregated coin-index and expiration-date signatures are required inputs to `prepare_for_spending`, so a client holding an unexpired ticketbook from an earlier epoch, whose local cache does not already have them, cannot spend it until the ceremony ends. Nothing it needs has anything to do with the ceremony: the signer set, threshold and keys all belong to the epoch that issued the book. The three aggregated-data routes and the two partial-signature routes now refuse only the epoch whose ceremony is actually running. The partials matter as much as the aggregates, since aggregation fans out to peers' partial endpoints and would otherwise still fail on them. `post_blind_sign` keeps the blanket gate: issuing against a previous epoch is a separate question with its own decisions to make. Gateways were never affected - `credential-verification` builds its master key from the DKG contract directly rather than from these endpoints. `epoch_concluded` becomes `ceremony_concluded` (and `Epoch::is_epoch_concluded` becomes `is_ceremony_concluded`) throughout. The old name read as "this epoch is over and we have moved on", when what it reports is that the epoch's *ceremony* has finished - which is true for the whole time the epoch is in use. The dummy communication channel's knobs move into one shared `SharedCommState` handle rather than an `Arc` per flag threaded through the channel, the bundle and the fixture, and it now models a ceremony in flight so the route behaviour can be tested at all. --- .../coconut-dkg/src/types.rs | 28 +-- .../src/shared_state/ecash_state.rs | 6 +- nym-api/src/ecash/api_routes/aggregation.rs | 31 ++- .../src/ecash/api_routes/partial_signing.rs | 9 +- nym-api/src/ecash/comm.rs | 8 +- nym-api/src/ecash/error.rs | 5 + nym-api/src/ecash/state/mod.rs | 86 ++++++- nym-api/src/ecash/tests/mod.rs | 219 ++++++++++++++---- 8 files changed, 308 insertions(+), 84 deletions(-) diff --git a/common/cosmwasm-smart-contracts/coconut-dkg/src/types.rs b/common/cosmwasm-smart-contracts/coconut-dkg/src/types.rs index c63679db803..4a20a534aae 100644 --- a/common/cosmwasm-smart-contracts/coconut-dkg/src/types.rs +++ b/common/cosmwasm-smart-contracts/coconut-dkg/src/types.rs @@ -201,13 +201,15 @@ impl Epoch { ) } - /// Whether the ceremony for `epoch_id` has finished, judged against `self` as the current - /// epoch. + /// Whether the DKG ceremony that establishes `epoch_id`'s keys has finished, judged against + /// `self` as the current epoch. Note this says nothing about whether that epoch is *over* - + /// the current epoch's own ceremony is concluded for all of the time it is in use. /// - /// Anything before the current epoch has necessarily finished, and an epoch that has not - /// been reached yet certainly has not. Callers working from a cached copy of the current - /// epoch can only get a pessimistic answer out of a stale one, never a premature "yes". - pub fn is_epoch_concluded(&self, epoch_id: EpochId) -> bool { + /// The ceremony of any epoch before the current one has necessarily finished, and one that + /// has not been reached yet certainly has not started. Callers working from a cached copy of + /// the current epoch can only get a pessimistic answer out of a stale one, never a premature + /// "yes". + pub fn is_ceremony_concluded(&self, epoch_id: EpochId) -> bool { match epoch_id.cmp(&self.epoch_id) { Ordering::Less => true, Ordering::Greater => false, @@ -373,15 +375,15 @@ mod tests { /// Answering "concluded" for an epoch still mid-ceremony would have them remember a set /// that is empty or partial. #[test] - fn an_epoch_has_concluded_only_once_its_ceremony_is_done() { + fn a_ceremony_is_concluded_only_once_its_epoch_is_in_progress() { let current = epoch_at(5, EpochState::InProgress); - // earlier epochs are finished by definition, and later ones cannot have started - assert!(current.is_epoch_concluded(4)); - assert!(!current.is_epoch_concluded(6)); + // earlier ceremonies are finished by definition, and later ones cannot have started + assert!(current.is_ceremony_concluded(4)); + assert!(!current.is_ceremony_concluded(6)); - // the current one depends on where its ceremony has got to - assert!(current.is_epoch_concluded(5)); + // the current epoch's own ceremony is concluded for all the time it is in use + assert!(current.is_ceremony_concluded(5)); for state in [ EpochState::WaitingInitialisation, EpochState::PublicKeySubmission { resharing: false }, @@ -391,7 +393,7 @@ mod tests { EpochState::VerificationKeyFinalization { resharing: false }, ] { assert!( - !epoch_at(5, state).is_epoch_concluded(5), + !epoch_at(5, state).is_ceremony_concluded(5), "{state} was treated as a concluded ceremony" ); } diff --git a/common/credential-proxy/src/shared_state/ecash_state.rs b/common/credential-proxy/src/shared_state/ecash_state.rs index cd3cd3532bb..a375e92761a 100644 --- a/common/credential-proxy/src/shared_state/ecash_state.rs +++ b/common/credential-proxy/src/shared_state/ecash_state.rs @@ -142,7 +142,7 @@ impl EcashState { } /// Whether the ceremony for `epoch_id` has concluded, so its set of signers is settled. - async fn epoch_concluded( + async fn ceremony_concluded( &self, client: &ChainClient, epoch_id: EpochId, @@ -150,7 +150,7 @@ impl EcashState { Ok(self .current_epoch(client) .await? - .is_epoch_concluded(epoch_id)) + .is_ceremony_concluded(epoch_id)) } /// The signers registered for `epoch_id`, skipping any whose share cannot be used. @@ -180,7 +180,7 @@ impl EcashState { // verified shares yet. this cache has no expiry, so answering from it then would // remember an empty signer set for the life of the process - and this proxy would // keep failing to fan out long after the ceremony finished. - if !self.epoch_concluded(client, epoch_id).await? { + if !self.ceremony_concluded(client, epoch_id).await? { return self.registered_ecash_clients(client, epoch_id).await; } diff --git a/nym-api/src/ecash/api_routes/aggregation.rs b/nym-api/src/ecash/api_routes/aggregation.rs index 0b99e8dbd1f..e416009b230 100644 --- a/nym-api/src/ecash/api_routes/aggregation.rs +++ b/nym-api/src/ecash/api_routes/aggregation.rs @@ -61,10 +61,16 @@ async fn master_verification_key( trace!("aggregated_verification_key request"); let output = output.unwrap_or_default(); - // see if we're not in the middle of new dkg - state.ensure_dkg_not_in_progress().await?; + let epoch_id = match epoch_id { + Some(epoch_id) => epoch_id, + None => state.current_dkg_epoch().await?, + }; - let key = state.master_verification_key(epoch_id).await?; + // a concluded epoch's key is fixed, so a ceremony running for some *other* epoch is no + // reason to withhold it + state.ensure_ceremony_concluded(epoch_id).await?; + + let key = state.master_verification_key(Some(epoch_id)).await?; Ok(output.to_response(VerificationKeyResponse::new(key.clone()))) } @@ -108,14 +114,15 @@ async fn expiration_date_signatures( .map_err(|_| EcashError::MalformedExpirationDate { raw })?, }; - // see if we're not in the middle of new dkg - state.ensure_dkg_not_in_progress().await?; - let epoch_id = match epoch_id { Some(epoch_id) => epoch_id, None => state.current_dkg_epoch().await?, }; + // these signatures are an input to spending a ticketbook from that epoch, and they cannot + // change once its ceremony is done - so a later ceremony must not withhold them + state.ensure_ceremony_concluded(epoch_id).await?; + let expiration_date_signatures = state .master_expiration_date_signatures(expiration_date, epoch_id) .await?; @@ -151,10 +158,16 @@ async fn coin_indices_signatures( trace!("aggregated_coin_indices_signatures request"); let output = output.unwrap_or_default(); - // see if we're not in the middle of new dkg - state.ensure_dkg_not_in_progress().await?; - let coin_indices_signatures = state.master_coin_index_signatures(epoch_id).await?; + let epoch_id = match epoch_id { + Some(epoch_id) => epoch_id, + None => state.current_dkg_epoch().await?, + }; + + // as above: an input to spending, fixed once that epoch's ceremony concluded + state.ensure_ceremony_concluded(epoch_id).await?; + + let coin_indices_signatures = state.master_coin_index_signatures(Some(epoch_id)).await?; Ok(output.to_response(AggregatedCoinIndicesSignatureResponse { epoch_id: coin_indices_signatures.epoch_id, diff --git a/nym-api/src/ecash/api_routes/partial_signing.rs b/nym-api/src/ecash/api_routes/partial_signing.rs index 582b82cb32d..cb79b26830e 100644 --- a/nym-api/src/ecash/api_routes/partial_signing.rs +++ b/nym-api/src/ecash/api_routes/partial_signing.rs @@ -159,8 +159,9 @@ async fn partial_expiration_date_signatures( // the caller wants this epoch's material, so it's this epoch's signers that have to answer state.ensure_signer_for_epoch(epoch_id).await?; - // see if we're not in the middle of new dkg - state.ensure_dkg_not_in_progress().await?; + // an aggregator collecting a past epoch's signatures depends on us answering this while a + // later ceremony runs, so only that ceremony's own epoch is refused + state.ensure_ceremony_concluded(epoch_id).await?; let expiration_date_signatures = state .partial_expiration_date_signatures(expiration_date, epoch_id) @@ -201,8 +202,8 @@ async fn partial_coin_indices_signatures( // the caller wants this epoch's material, so it's this epoch's signers that have to answer state.ensure_signer_for_epoch(epoch_id).await?; - // see if we're not in the middle of new dkg - state.ensure_dkg_not_in_progress().await?; + // as above: refuse only the epoch whose ceremony is still running + state.ensure_ceremony_concluded(epoch_id).await?; let coin_indices_signatures = state.partial_coin_index_signatures(Some(epoch_id)).await?; diff --git a/nym-api/src/ecash/comm.rs b/nym-api/src/ecash/comm.rs index 26d8a8e09ad..4e2da553891 100644 --- a/nym-api/src/ecash/comm.rs +++ b/nym-api/src/ecash/comm.rs @@ -27,7 +27,7 @@ pub trait APICommunicationChannel { /// Anything derived from the signer set may only be cached once this is true: until /// then the set is still being filled in, and whatever partial view a caller happens /// to observe would be pinned for the lifetime of the process. - async fn epoch_concluded(&self, epoch_id: EpochId) -> Result; + async fn ceremony_concluded(&self, epoch_id: EpochId) -> Result; } struct CachedEpoch { @@ -138,7 +138,7 @@ impl APICommunicationChannel for QueryCommunicationChannel { // gateways poll continuously, so during a ceremony something will ask about the // new epoch while its shares are still being submitted. answer, but don't cache: // the entry has no expiry, so an empty or partial set would stick for good. - if !self.epoch_concluded(epoch_id).await? { + if !self.ceremony_concluded(epoch_id).await? { return self.client.get_registered_ecash_clients(epoch_id).await; } @@ -176,11 +176,11 @@ impl APICommunicationChannel for QueryCommunicationChannel { return Ok(!guard.current_epoch.state.is_in_progress()); } - async fn epoch_concluded(&self, epoch_id: EpochId) -> Result { + async fn ceremony_concluded(&self, epoch_id: EpochId) -> Result { Ok(self .current_epoch_data() .await? - .is_epoch_concluded(epoch_id)) + .is_ceremony_concluded(epoch_id)) } } diff --git a/nym-api/src/ecash/error.rs b/nym-api/src/ecash/error.rs index cd7a205eae6..ece6c90e9f3 100644 --- a/nym-api/src/ecash/error.rs +++ b/nym-api/src/ecash/error.rs @@ -98,6 +98,11 @@ pub enum EcashError { #[error("a new iteration of DKG is currently in progress. all ticket issuance is halted until that's completed")] DkgInProgress, + #[error( + "the DKG ceremony for epoch {epoch_id} has not concluded, so it has no data to serve yet" + )] + CeremonyNotConcluded { epoch_id: EpochId }, + #[error( "the node index value for epoch {epoch_id} is not available - are you sure we are a dealer?" )] diff --git a/nym-api/src/ecash/state/mod.rs b/nym-api/src/ecash/state/mod.rs index 28169654278..0acd02e0cbe 100644 --- a/nym-api/src/ecash/state/mod.rs +++ b/nym-api/src/ecash/state/mod.rs @@ -185,7 +185,7 @@ impl EcashState { // our own membership is only settled once the ceremony has concluded. this cache // never expires, so answering "not a signer" mid-ceremony and remembering it // would have us refuse to sign for the rest of the epoch. - if !self.aux.comm_channel.epoch_concluded(epoch_id).await? { + if !self.aux.comm_channel.ceremony_concluded(epoch_id).await? { return self.check_dkg_signer(epoch_id).await; } @@ -557,6 +557,20 @@ impl EcashState { Ok(()) } + /// Ensures the DKG ceremony that established `epoch_id`'s keys has finished, so everything + /// derived from them is settled. + /// + /// Only the epoch whose ceremony is running right now has nothing to give. Everything an + /// earlier one was ever asked for is fixed for good, and its credentials stay spendable for + /// days after it stops being used for issuance - so refusing those requests for the duration + /// of a ceremony takes credentials out of service for a reason that does not apply to them. + pub(crate) async fn ensure_ceremony_concluded(&self, epoch_id: EpochId) -> Result<()> { + if !self.aux.comm_channel.ceremony_concluded(epoch_id).await? { + return Err(EcashError::CeremonyNotConcluded { epoch_id }); + } + Ok(()) + } + /// Check if this nym-api has already issued a credential for the provided deposit id. /// If so, return it. pub async fn already_issued(&self, deposit_id: DepositId) -> Result> { @@ -1351,6 +1365,76 @@ mod tests { Ok(()) } + /// B2 at the layer beneath the routes: lifting the gate is only worth anything if the data + /// can actually be produced while a ceremony runs. Everything it depends on belongs to the + /// epoch being asked about - its signer set, its threshold, its keys - so none of it is + /// touched by the ceremony running for the *next* epoch. + #[tokio::test] + async fn a_concluded_epoch_can_still_be_served_while_the_next_ceremony_runs( + ) -> anyhow::Result<()> { + let chain = SharedContractChain::new(1); + initiate_dkg(&chain); + + cheap::run_ceremony(&chain, false); + let past_epoch = chain.epoch().epoch_id; + let past_keys = cheap::install_real_verification_keys(&chain); + + let key_dir = tempfile::tempdir()?; + let key_path = key_dir.path().join("ecash.pem"); + persist_ecash_keypair( + &KeyPairWithEpoch::new(past_keys.keypairs.into_iter().next().unwrap(), past_epoch), + &key_path, + )?; + archive_ecash_keypair(&key_path, past_epoch)?; + + // a fresh ceremony is under way and has not produced anything yet + trigger_reset(&chain); + cheap::register_dealers(&chain, false); + cheap::advance(&chain); + let current_epoch = chain.epoch().epoch_id; + assert_ne!(past_epoch, current_epoch); + + let me = chain.group_member_addresses()[0].clone(); + let state = contract_backed_ecash_state(&chain, me).await; + for keys in load_archived_ecash_keypairs(&key_path) { + state.local.ecash_keypair.archive(keys).await; + } + + // the blanket gate would have refused everything in this situation + assert!(state.ensure_dkg_not_in_progress().await.is_err()); + + // the epoch being built has nothing to give ... + assert!(matches!( + state.ensure_ceremony_concluded(current_epoch).await, + Err(EcashError::CeremonyNotConcluded { epoch_id }) if epoch_id == current_epoch + )); + + // ... while the one that finished is settled, and every layer can still serve it + state.ensure_ceremony_concluded(past_epoch).await?; + + let expiration_date = ecash_today_date(); + let partial = state + .partial_expiration_date_signatures(expiration_date, past_epoch) + .await?; + assert_eq!(partial.epoch_id, past_epoch); + drop(partial); + + let master = state + .master_expiration_date_signatures(expiration_date, past_epoch) + .await?; + assert_eq!(master.epoch_id, past_epoch); + drop(master); + + let coin_indices = state.master_coin_index_signatures(Some(past_epoch)).await?; + assert_eq!(coin_indices.epoch_id, past_epoch); + drop(coin_indices); + + let vk = state.master_verification_key(Some(past_epoch)).await?; + assert_eq!(*vk, past_keys.master); + + Ok(()) + } + /// B3, at the gate sitting in front of it: the material a past epoch's credentials need is /// asked of *that* epoch's signers, which is not necessarily whoever signs today. An api /// that has since dropped out of the set still holds the keys, and refusing it on the diff --git a/nym-api/src/ecash/tests/mod.rs b/nym-api/src/ecash/tests/mod.rs index 89a1cf899d0..184ecf46b2a 100644 --- a/nym-api/src/ecash/tests/mod.rs +++ b/nym-api/src/ecash/tests/mod.rs @@ -52,7 +52,7 @@ use nym_ecash_contract_common::deposit::{Deposit, DepositId, DepositResponse}; use nym_task::ShutdownManager; use nym_validator_client::nym_api::routes::{ ECASH_BLIND_SIGN, ECASH_ISSUED_TICKETBOOKS_CHALLENGE_COMMITMENT, ECASH_ISSUED_TICKETBOOKS_FOR, - ECASH_ROUTES, V1_API_VERSION, + ECASH_ROUTES, GLOBAL_EXPIRATION_DATE_SIGNATURES, V1_API_VERSION, }; use nym_validator_client::nyxd::cosmwasm_client::logs::Log; use nym_validator_client::nyxd::cosmwasm_client::types::ExecuteResult; @@ -60,10 +60,11 @@ use nym_validator_client::nyxd::{AccountId, ExecTxResult, Fee, Hash, TxResponse} use nym_validator_client::EcashApiClient; use rand::rngs::OsRng; use rand::RngCore; +use std::cmp::Ordering as CmpOrdering; use std::collections::{BTreeMap, HashMap}; use std::ops::Deref; use std::str::FromStr; -use std::sync::atomic::{AtomicU64, Ordering}; +use std::sync::atomic::{AtomicBool, AtomicU64, Ordering}; use std::sync::{Arc, Mutex}; use time::Date; use tokio::sync::RwLock; @@ -1096,21 +1097,96 @@ impl super::client::Client for DummyClient { } } +/// Everything a [`DummyCommunicationChannel`] reports, shared with whoever is driving the test. +/// +/// One handle rather than an `Arc` per knob: a new dial is added here instead of being threaded +/// through the channel, [`DummyEcashBundle`] and [`TestFixture`] in parallel. +#[allow(dead_code)] +#[derive(Clone)] +pub struct SharedCommState { + inner: Arc, +} + +struct CommStateInner { + current_epoch: AtomicU64, + + /// Whether the current epoch's ceremony is still running. + ceremony_in_flight: AtomicBool, + + ecash_clients: RwLock>>, +} + +#[allow(dead_code)] +impl SharedCommState { + pub fn new(epoch_id: EpochId, ecash_clients: Vec) -> Self { + SharedCommState { + inner: Arc::new(CommStateInner { + current_epoch: AtomicU64::new(epoch_id), + ceremony_in_flight: AtomicBool::new(false), + ecash_clients: RwLock::new(HashMap::from([(epoch_id, ecash_clients)])), + }), + } + } + + pub fn current_epoch(&self) -> EpochId { + self.inner.current_epoch.load(Ordering::Relaxed) + } + + /// Move to a new epoch, carrying the signers of the old one over to it. + pub async fn set_current_epoch(&self, epoch_id: EpochId) { + let previous = self.current_epoch(); + self.inner.current_epoch.store(epoch_id, Ordering::Relaxed); + + let existing = self.clients(previous).await; + if !existing.is_empty() { + self.set_clients(epoch_id, existing).await; + } + } + + /// Put the current epoch mid-ceremony, as a rotation would. + pub fn start_ceremony(&self) { + self.inner.ceremony_in_flight.store(true, Ordering::Relaxed); + } + + pub fn conclude_ceremony(&self) { + self.inner + .ceremony_in_flight + .store(false, Ordering::Relaxed); + } + + pub fn ceremony_in_flight(&self) -> bool { + self.inner.ceremony_in_flight.load(Ordering::Relaxed) + } + + pub async fn clients(&self, epoch_id: EpochId) -> Vec { + self.inner + .ecash_clients + .read() + .await + .get(&epoch_id) + .cloned() + .unwrap_or_default() + } + + pub async fn set_clients(&self, epoch_id: EpochId, clients: Vec) { + self.inner + .ecash_clients + .write() + .await + .insert(epoch_id, clients); + } +} + #[allow(dead_code)] #[derive(Clone)] pub struct DummyCommunicationChannel { - current_epoch: Arc, - ecash_clients: Arc>>>, + state: SharedCommState, } impl DummyCommunicationChannel { pub fn new(ecash_clients: Vec) -> Self { - let epoch_id = 1; - let mut ecash_clients_map = HashMap::new(); - ecash_clients_map.insert(epoch_id, ecash_clients); DummyCommunicationChannel { - current_epoch: Arc::new(AtomicU64::new(epoch_id)), - ecash_clients: Arc::new(RwLock::new(ecash_clients_map)), + state: SharedCommState::new(1, ecash_clients), } } @@ -1130,40 +1206,32 @@ impl DummyCommunicationChannel { Self::new(vec![client]) } - pub fn clients_arc(&self) -> Arc>>> { - Arc::clone(&self.ecash_clients) - } - - pub fn with_epoch(mut self, current_epoch: Arc) -> Self { - self.current_epoch = current_epoch; - self + pub fn shared_state(&self) -> SharedCommState { + self.state.clone() } } #[async_trait] impl super::comm::APICommunicationChannel for DummyCommunicationChannel { async fn current_epoch(&self) -> Result { - Ok(self.current_epoch.load(Ordering::Relaxed)) + Ok(self.state.current_epoch()) } async fn dkg_in_progress(&self) -> Result { - // deal with this later lol - Ok(false) + Ok(self.state.ceremony_in_flight()) } - async fn epoch_concluded(&self, _epoch_id: EpochId) -> Result { - // this chain never runs a ceremony, so its epochs are always settled - Ok(true) + async fn ceremony_concluded(&self, epoch_id: EpochId) -> Result { + // mirrors the real channel: only the current epoch's ceremony can be unfinished + match epoch_id.cmp(&self.state.current_epoch()) { + CmpOrdering::Less => Ok(true), + CmpOrdering::Greater => Ok(false), + CmpOrdering::Equal => Ok(!self.state.ceremony_in_flight()), + } } async fn ecash_clients(&self, epoch_id: EpochId) -> Result> { - Ok(self - .ecash_clients - .read() - .await - .get(&epoch_id) - .cloned() - .unwrap_or_default()) + Ok(self.state.clients(epoch_id).await) } async fn ecash_threshold(&self, _epoch_id: EpochId) -> Result { @@ -1242,22 +1310,20 @@ struct TestFixture { axum: TestServer, storage: NymApiStorage, chain_state: SharedFakeChain, - epoch: Arc, - ecash_clients: Arc>>>, + comm_state: SharedCommState, } /// Test-only bundle returned by [`build_dummy_ecash_state`]. Carries the /// constructed [`EcashState`] plus the test handles the caller may want to -/// poke at directly (chain state, registered ecash clients, epoch counter). +/// poke at directly (chain state, and the dummy channel's epoch/signer/ceremony state). pub(crate) struct DummyEcashBundle { pub ecash_state: EcashState, pub chain_state: SharedFakeChain, - pub ecash_clients: Arc>>>, + pub comm_state: SharedCommState, /// A real [`Client`] (not the [`DummyClient`]) suitable for the /// `nyxd_client` field on [`AppState`]. Built from a global env-var dance /// (see body), which is required because `AppState` is not generic. pub real_client: Client, - pub epoch: Arc, } /// Build a self-contained [`EcashState`] suitable for handler tests. Pulls @@ -1272,14 +1338,12 @@ pub(crate) async fn build_dummy_ecash_state( let mut rng = crate::ecash::tests::fixtures::test_rng(rng_seed); let coconut_keypair = ttp_keygen(1, 1).unwrap().remove(0); let identity = Arc::new(ed25519::KeyPair::new(&mut rng)); - let epoch = Arc::new(AtomicU64::new(1)); let address = AccountId::from_str(TEST_REWARDING_VALIDATOR_ADDRESS).unwrap(); let comm_channel = DummyCommunicationChannel::new_single_dummy( coconut_keypair.verification_key().clone(), address.clone(), - ) - .with_epoch(epoch.clone()); - let ecash_clients = comm_channel.clients_arc(); + ); + let comm_state = comm_channel.shared_state(); let staged_key_pair = crate::ecash::keys::KeyPair::new(); staged_key_pair @@ -1329,9 +1393,8 @@ pub(crate) async fn build_dummy_ecash_state( DummyEcashBundle { ecash_state, chain_state, - ecash_clients, + comm_state, real_client, - epoch, } } @@ -1359,21 +1422,13 @@ impl TestFixture { ), storage, chain_state: bundle.chain_state, - epoch: bundle.epoch, - ecash_clients: bundle.ecash_clients, + comm_state: bundle.comm_state, } } #[allow(dead_code)] async fn set_epoch(&self, epoch: u64) { - let current_epoch = self.epoch.load(Ordering::Relaxed); - self.epoch.store(epoch, Ordering::Relaxed); - - // copy the same epoch_signers as we had initially - let existing = self.ecash_clients.read().await.get(¤t_epoch).cloned(); - if let Some(clients) = existing { - self.ecash_clients.write().await.insert(epoch, clients); - } + self.comm_state.set_current_epoch(epoch).await } #[allow(dead_code)] @@ -1475,9 +1530,13 @@ impl TestFixture { #[cfg(test)] mod credential_tests { use super::*; + use crate::ecash::helpers::IssuedExpirationDateSignatures; use crate::ecash::storage::EcashStorageExt; use axum::http::StatusCode; + use nym_api_requests::ecash::models::AggregatedExpirationDateSignatureResponse; + use nym_ecash_time::ecash_today_date; use nym_ticketbooks_merkle::MerkleLeaf; + use nym_validator_client::nym_api::RFC_3339_DATE_FORMAT; #[tokio::test] async fn already_issued() { @@ -1705,6 +1764,66 @@ mod credential_tests { let _ = response.json::(); } + /// B2, at the routes: the aggregated data a client needs to *spend* an old ticketbook is + /// fixed once that epoch's ceremony finished, so a later ceremony is no reason to withhold + /// it. A client whose local cache is cold has no other source, and the book stays valid for + /// days after the rotation - so refusing here takes it out of service for the duration. + /// + /// The epoch being run right now is a different matter: it has nothing to give yet, and + /// says so. + #[tokio::test] + async fn aggregated_data_for_a_concluded_epoch_is_served_during_a_ceremony() { + let fixture = TestFixture::new().await; + let expiration_date = ecash_today_date(); + + let past_epoch = fixture.comm_state.current_epoch(); + + // the aggregate this api established while that epoch was current + fixture + .storage + .insert_master_expiration_date_signatures( + expiration_date, + &IssuedExpirationDateSignatures { + epoch_id: past_epoch, + signatures: Vec::new(), + }, + ) + .await + .unwrap(); + + // a new ceremony begins: the epoch id increments and its keys do not exist yet + fixture.set_epoch(past_epoch + 1).await; + fixture.comm_state.start_ceremony(); + let current_epoch = fixture.comm_state.current_epoch(); + + let request = |epoch_id: EpochId| { + fixture + .axum + .get(&format!( + "/{V1_API_VERSION}/{ECASH_ROUTES}/{GLOBAL_EXPIRATION_DATE_SIGNATURES}" + )) + .add_query_param("epoch_id", epoch_id) + .add_query_param( + "expiration_date", + expiration_date.format(RFC_3339_DATE_FORMAT).unwrap(), + ) + }; + + // the concluded epoch is still served, mid-ceremony + let response = request(past_epoch).await; + assert_eq!(response.status_code(), StatusCode::OK); + assert_eq!( + response + .json::() + .epoch_id, + past_epoch + ); + + // the one whose ceremony is running has nothing to offer yet + let response = request(current_epoch).await; + assert_eq!(response.status_code(), StatusCode::BAD_REQUEST); + } + #[test] fn blind_sign_request_body_serde() { let deposit_id = 123; From c734fe907c264709fa2aee8271fa2f5645f4b962 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C4=99drzej=20Stuczy=C5=84ski?= Date: Thu, 20 Aug 2026 11:18:45 +0100 Subject: [PATCH 4/5] fix(nym-api): serve a settled epoch before its keys reach the archive The previous commit lifted the ceremony-wide gate off the aggregated and partial signature routes, but a request for a settled epoch could still be refused one layer down, for the first phase of every ceremony. `invalidate_coconut_keypair` fires at public key submission, while the keys it applies to stay in the live slot until dealing exchange archives them. The lookup routed an exact-epoch match through that flag, so during the ~10 minutes of the first phase the previous epoch's auxiliary signatures were refused - and then served again afterwards, once the same keys had moved to the archive. Only material the signer had never generated was affected; anything already in its storage was served throughout. The flag answers "may we issue credentials right now", which is not the question a request naming a specific concluded epoch is asking. `keys_for_epoch` is now a lookup and nothing else, and the two signature paths establish their own right to use what it returns by checking that the epoch's ceremony is over. That check lives at the state layer as well as at the route, so neither depends on the other being correct - the same reason B10 needed fixing at two layers. Issuance keeps the flag, and `blind_sign` keeps the ceremony-wide gate. `KeyPair::keys` is dropped: it was the validity-gated accessor these paths used to share, and leaving it would invite the same conflation back in. Issuance goes through `signing_key`, lookups through `keys_for_epoch`. --- nym-api/src/ecash/keys/mod.rs | 66 +++++++++++++++++++----------- nym-api/src/ecash/state/mod.rs | 73 ++++++++++++++++++++++++++++++++-- 2 files changed, 113 insertions(+), 26 deletions(-) diff --git a/nym-api/src/ecash/keys/mod.rs b/nym-api/src/ecash/keys/mod.rs index 25c9514f350..f341dad72e8 100644 --- a/nym-api/src/ecash/keys/mod.rs +++ b/nym-api/src/ecash/keys/mod.rs @@ -81,22 +81,29 @@ impl KeyPair { .map(|keys| keys.issued_for_epoch) } - /// The keys derived for `epoch_id`, whether that is the epoch we're actively signing for - /// or one that has since rotated. + /// The keys derived for `epoch_id`, wherever we happen to be keeping them - the live slot + /// if it is still the epoch we sign for, otherwise the archive. + /// + /// This is a lookup and nothing more: it deliberately does **not** consult [`Self::valid`], + /// which answers a different question ("may we issue credentials right now"). A rotation + /// clears that flag the moment the next ceremony starts, while the keys it clears it for + /// stay in the live slot until dealing exchange moves them to the archive - so a gate here + /// would refuse a settled epoch's auxiliary signatures for exactly as long as that window + /// lasts, and serve them either side of it. + /// + /// Callers are responsible for establishing that they may use these keys at all; + /// `EcashState::ensure_ceremony_concluded` is how the signature paths do it. pub async fn keys_for_epoch( &self, epoch_id: EpochId, ) -> Result, EcashError> { let current = self.current_key_epoch().await; - // the epoch we're actively signing for goes through the usual validity gate if current == Some(epoch_id) { - return self.keys().await; + return RwLockReadGuard::try_map(self.read_keys().await, |keys| keys.as_ref()) + .map_err(|_| EcashError::KeyPairNotDerivedYet); } - // any other epoch comes out of the archive, and is deliberately not subject to that - // gate: it tracks whether the *current* keys may be used for issuance, and the chain - // is the authority on whether an archived share was ever verified RwLockReadGuard::try_map(self.archived.read().await, |archived| { archived.get(&epoch_id) }) @@ -117,12 +124,6 @@ impl KeyPair { } } - pub async fn keys(&self) -> Result, EcashError> { - let keypair_guard = self.get().await.ok_or(EcashError::KeyPairNotDerivedYet)?; - RwLockReadGuard::try_map(keypair_guard, |keypair| keypair.as_ref()) - .map_err(|_| EcashError::KeyPairNotDerivedYet) - } - pub async fn signing_key(&self) -> Result, EcashError> { let keypair_guard = self.get().await.ok_or(EcashError::KeyPairNotDerivedYet)?; @@ -170,25 +171,28 @@ mod tests { KeyPairWithEpoch::new(ttp_keygen(1, 1).unwrap().pop().unwrap(), epoch_id) } + /// The lookup is only about where the keys are, never about whether we may issue with them. + /// + /// That distinction is load bearing: a rotation clears `valid` when the next ceremony + /// starts, but the keys it clears it for stay in the live slot until dealing exchange + /// archives them. A gate here would refuse a settled epoch's auxiliary signatures for + /// precisely that window and serve them either side of it. #[tokio::test] - async fn the_epoch_we_sign_for_is_still_subject_to_the_validity_gate() { + async fn the_lookup_does_not_care_whether_the_keys_may_be_used_for_issuance() { let keys = KeyPair::new(); keys.set(dummy_keys(5)).await; - // derived but not yet finalised on chain - assert!(matches!( - keys.keys_for_epoch(5).await, - Err(EcashError::KeyPairNotDerivedYet) - )); + // never validated, e.g. derived but not yet finalised on chain + assert_eq!(keys.keys_for_epoch(5).await.unwrap().issued_for_epoch, 5); keys.validate(); assert_eq!(keys.keys_for_epoch(5).await.unwrap().issued_for_epoch, 5); + + // and once a later ceremony has cleared the flag again + keys.invalidate(); + assert_eq!(keys.keys_for_epoch(5).await.unwrap().issued_for_epoch, 5); } - /// The gate above tracks whether the keys we sign *with* may be used for issuance. An - /// archived epoch is not covered by it: the chain already settled which shares were - /// verified for that epoch, and its credentials need their material regardless of what - /// the current ceremony is doing. #[tokio::test] async fn an_archived_epoch_is_served_whatever_the_current_keys_are_doing() { let keys = KeyPair::new(); @@ -202,6 +206,22 @@ mod tests { assert_eq!(keys.keys_for_epoch(4).await.unwrap().issued_for_epoch, 4); } + /// Issuance keeps its own gate, and it is the only thing the flag governs. + #[tokio::test] + async fn issuance_is_still_refused_while_the_keys_are_not_usable() { + let keys = KeyPair::new(); + keys.set(dummy_keys(5)).await; + + assert!(keys.signing_key().await.is_err()); + assert!(keys.verification_key().await.is_none()); + + keys.validate(); + assert!(keys.signing_key().await.is_ok()); + + keys.invalidate(); + assert!(keys.signing_key().await.is_err()); + } + #[tokio::test] async fn an_epoch_we_never_held_keys_for_is_refused() { let keys = KeyPair::new(); diff --git a/nym-api/src/ecash/state/mod.rs b/nym-api/src/ecash/state/mod.rs index 0acd02e0cbe..4f5840fc4ae 100644 --- a/nym-api/src/ecash/state/mod.rs +++ b/nym-api/src/ecash/state/mod.rs @@ -392,7 +392,10 @@ impl EcashState { // 2. perform actual issuance // // a past epoch is answered from the key it was signed with, which we archived - // rather than destroyed when it rotated + // rather than destroyed when it rotated. what makes that safe is the epoch's + // ceremony being over, so check exactly that rather than inheriting the + // "may we issue right now" flag, which a later ceremony clears + self.ensure_ceremony_concluded(epoch_id).await?; let signing_keys = self.local.ecash_keypair.keys_for_epoch(epoch_id).await?; let master_vk = self.master_verification_key(Some(epoch_id)).await?; let signatures = sign_coin_indices( @@ -525,8 +528,9 @@ impl EcashState { // 3. perform actual issuance // - // a past epoch is answered from the key it was signed with, which we archived - // rather than destroyed when it rotated + // as with the coin index sibling: a settled epoch is answered from the key it + // was signed with, and it is the ceremony being over that makes that safe + self.ensure_ceremony_concluded(epoch_id).await?; let signing_keys = self.local.ecash_keypair.keys_for_epoch(epoch_id).await?; let signatures = sign_expiration_date( @@ -1435,6 +1439,69 @@ mod tests { Ok(()) } + /// The window the two tests above miss. A ceremony clears the "may we issue" flag as soon as + /// it starts, but the keys it clears it for are not archived until dealing exchange - so for + /// the first phase of every ceremony the previous epoch's keys sit in the live slot, unusable + /// for issuance and not yet in the archive. Its credentials still need serving throughout. + #[tokio::test] + async fn a_settled_epoch_is_served_from_the_live_slot_before_its_keys_are_archived( + ) -> anyhow::Result<()> { + let chain = SharedContractChain::new(1); + initiate_dkg(&chain); + + cheap::run_ceremony(&chain, false); + let past_epoch = chain.epoch().epoch_id; + let past_keys = cheap::install_real_verification_keys(&chain); + + let me = chain.group_member_addresses()[0].clone(); + let state = contract_backed_ecash_state(&chain, me).await; + + // the keys it signed that epoch with, in use and usable + state + .local + .ecash_keypair + .set(KeyPairWithEpoch::new( + past_keys.keypairs.into_iter().next().unwrap(), + past_epoch, + )) + .await; + state.local.ecash_keypair.validate(); + + // a ceremony begins: the flag is cleared at public key submission, but nothing has + // been archived yet - dealing exchange is what does that + trigger_reset(&chain); + state.local.ecash_keypair.invalidate(); + let current_epoch = chain.epoch().epoch_id; + assert_ne!(past_epoch, current_epoch); + + // issuance is indeed halted ... + assert!(state.ecash_signing_key().await.is_err()); + + // ... but the epoch that finished is still settled, and still has to be served + let expiration_date = ecash_today_date(); + let partial = state + .partial_expiration_date_signatures(expiration_date, past_epoch) + .await?; + assert_eq!(partial.epoch_id, past_epoch); + drop(partial); + + let coin_indices = state + .partial_coin_index_signatures(Some(past_epoch)) + .await?; + assert_eq!(coin_indices.epoch_id, past_epoch); + drop(coin_indices); + + // the epoch being built is refused, even though its keys are the ones we hold + assert!(matches!( + state + .partial_expiration_date_signatures(expiration_date, current_epoch) + .await, + Err(EcashError::CeremonyNotConcluded { epoch_id }) if epoch_id == current_epoch + )); + + Ok(()) + } + /// B3, at the gate sitting in front of it: the material a past epoch's credentials need is /// asked of *that* epoch's signers, which is not necessarily whoever signs today. An api /// that has since dropped out of the set still holds the keys, and refusing it on the From 7af5e6e8ad23e6a1bc9d5abf9d001c75964509b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C4=99drzej=20Stuczy=C5=84ski?= Date: Thu, 20 Aug 2026 11:29:25 +0100 Subject: [PATCH 5/5] docs(nym-api): correct the documented failure cases on the ecash data routes The partial-signature endpoints gate on the signer set of the epoch that was asked for rather than the current one, so their documented 400 no longer matched what they do. The three aggregated-data endpoints documented no failure at all, though they now refuse an epoch whose ceremony is still running. Blind-sign, the issued-ticketbook and the spending routes are untouched: they still gate on being a signer in the current epoch, which is what they say. --- nym-api/src/ecash/api_routes/aggregation.rs | 9 ++++++--- nym-api/src/ecash/api_routes/partial_signing.rs | 4 ++-- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/nym-api/src/ecash/api_routes/aggregation.rs b/nym-api/src/ecash/api_routes/aggregation.rs index e416009b230..3c405232a86 100644 --- a/nym-api/src/ecash/api_routes/aggregation.rs +++ b/nym-api/src/ecash/api_routes/aggregation.rs @@ -51,7 +51,8 @@ pub(crate) fn aggregation_routes() -> Router { (VerificationKeyResponse = "application/json"), (VerificationKeyResponse = "application/yaml"), (VerificationKeyResponse = "application/bincode") - )) + )), + (status = 400, body = String, description = "the requested epoch's DKG ceremony has not concluded, so it has no key yet"), ), )] async fn master_verification_key( @@ -94,7 +95,8 @@ struct ExpirationDateParam { (AggregatedExpirationDateSignatureResponse = "application/json"), (AggregatedExpirationDateSignatureResponse = "application/yaml"), (AggregatedExpirationDateSignatureResponse = "application/bincode") - )) + )), + (status = 400, body = String, description = "the requested epoch's DKG ceremony has not concluded, so it has no signatures yet"), ), )] async fn expiration_date_signatures( @@ -148,7 +150,8 @@ async fn expiration_date_signatures( (AggregatedCoinIndicesSignatureResponse = "application/json"), (AggregatedCoinIndicesSignatureResponse = "application/yaml"), (AggregatedCoinIndicesSignatureResponse = "application/bincode") - )) + )), + (status = 400, body = String, description = "the requested epoch's DKG ceremony has not concluded, so it has no signatures yet"), ), )] async fn coin_indices_signatures( diff --git a/nym-api/src/ecash/api_routes/partial_signing.rs b/nym-api/src/ecash/api_routes/partial_signing.rs index cb79b26830e..85e35f77907 100644 --- a/nym-api/src/ecash/api_routes/partial_signing.rs +++ b/nym-api/src/ecash/api_routes/partial_signing.rs @@ -132,7 +132,7 @@ struct ExpirationDateParam { (PartialExpirationDateSignatureResponse = "application/yaml"), (PartialExpirationDateSignatureResponse = "application/bincode") )), - (status = 400, body = String, description = "this nym-api is not an ecash signer in the current epoch"), + (status = 400, body = String, description = "this nym-api is not an ecash signer in the requested epoch, or that epoch's DKG ceremony has not concluded"), ) )] async fn partial_expiration_date_signatures( @@ -187,7 +187,7 @@ async fn partial_expiration_date_signatures( (PartialCoinIndicesSignatureResponse = "application/yaml"), (PartialCoinIndicesSignatureResponse = "application/bincode") )), - (status = 400, body = String, description = "this nym-api is not an ecash signer in the current epoch"), + (status = 400, body = String, description = "this nym-api is not an ecash signer in the requested epoch, or that epoch's DKG ceremony has not concluded"), ) )] async fn partial_coin_indices_signatures(