From 968172013f7ff37bf205fd1a67e2a147fb2133e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C4=99drzej=20Stuczy=C5=84ski?= Date: Thu, 27 Aug 2026 15:36:55 +0100 Subject: [PATCH 1/8] feat(coconut-dkg): let the admin force a reset from any state A ceremony that keeps ending sub-threshold auto-resets straight into the next attempt without ever passing through `InProgress` - and `InProgress` is the only state `TriggerReset` accepts. So the one failure mode that needs admin intervention is the one state the admin lever cannot reach: a looping ceremony can never be stopped or redirected. `TriggerForcedReset` is a separate message rather than a widening of `TriggerReset`, deliberately. The state gate on the ordinary lever is a safety interlock - a slip mid-ceremony errors instead of aborting a running exchange - and widening it would make the same message mean different things on either side of the migration. A distinct name also keeps the chain log self-describing: an emergency intervention reads as one in a post-mortem. The forced reset is refused only before initialisation. Its successor is always a non-resharing attempt, aborted resharings included, since their registrants may hold nothing to reshare; and `keys_in_service` is carried over, so aborting an exchange retires nothing and issuance continues under the epoch in service throughout. Also fixes `try_trigger_reset` misreporting its refusal as `CantReshareDuringExchange`; the reset variant existed unused, and nothing matches on either. The variant is additive and no query or state shape changes, so laggard signers are unaffected. --- .../contract_traits/dkg_signing_client.rs | 9 + .../coconut-dkg/src/msg.rs | 6 + .../coconut-dkg/schema/nym-coconut-dkg.json | 14 ++ contracts/coconut-dkg/schema/raw/execute.json | 14 ++ contracts/coconut-dkg/src/contract.rs | 4 +- .../src/epoch_state/transactions/mod.rs | 168 +++++++++++++++++- 6 files changed, 213 insertions(+), 2 deletions(-) diff --git a/common/client-libs/validator-client/src/nyxd/contract_traits/dkg_signing_client.rs b/common/client-libs/validator-client/src/nyxd/contract_traits/dkg_signing_client.rs index 6316ac10777..13a1cba8d48 100644 --- a/common/client-libs/validator-client/src/nyxd/contract_traits/dkg_signing_client.rs +++ b/common/client-libs/validator-client/src/nyxd/contract_traits/dkg_signing_client.rs @@ -139,6 +139,14 @@ pub trait DkgSigningClient { .await } + /// The admin escape hatch: forces a DKG reset from any epoch state, including mid-exchange. + async fn trigger_dkg_forced_reset(&self, fee: Option) -> Result { + let req = DkgExecuteMsg::TriggerForcedReset {}; + + self.execute_dkg_contract(fee, req, "trigger forced DKG reset".to_string(), vec![]) + .await + } + async fn transfer_ownership( &self, transfer_to: String, @@ -238,6 +246,7 @@ mod tests { DkgExecuteMsg::AdvanceEpochState {} => client.advance_dkg_epoch_state(None).ignore(), DkgExecuteMsg::TriggerReset {} => client.trigger_dkg_reset(None).ignore(), DkgExecuteMsg::TriggerResharing {} => client.trigger_dkg_resharing(None).ignore(), + DkgExecuteMsg::TriggerForcedReset {} => client.trigger_dkg_forced_reset(None).ignore(), ExecuteMsg::TransferOwnership { transfer_to } => { client.transfer_ownership(transfer_to, None).ignore() } diff --git a/common/cosmwasm-smart-contracts/coconut-dkg/src/msg.rs b/common/cosmwasm-smart-contracts/coconut-dkg/src/msg.rs index 4612afd4cc5..e7a4226be3a 100644 --- a/common/cosmwasm-smart-contracts/coconut-dkg/src/msg.rs +++ b/common/cosmwasm-smart-contracts/coconut-dkg/src/msg.rs @@ -78,6 +78,12 @@ pub enum ExecuteMsg { TriggerResharing {}, + /// Admin-only escape hatch: force a reset from any epoch state, including mid-exchange. + /// A ceremony that keeps ending sub-threshold auto-resets straight into the next attempt + /// without ever passing through `InProgress`, which is the only state [`Self::TriggerReset`] + /// accepts - so without this, a looping ceremony can never be stopped or redirected. + TriggerForcedReset {}, + /// Transfers ownership of the epoch dealer to another address. /// This assumes off-chain hand-over of keys TransferOwnership { diff --git a/contracts/coconut-dkg/schema/nym-coconut-dkg.json b/contracts/coconut-dkg/schema/nym-coconut-dkg.json index b52590d77e3..855c396568e 100644 --- a/contracts/coconut-dkg/schema/nym-coconut-dkg.json +++ b/contracts/coconut-dkg/schema/nym-coconut-dkg.json @@ -290,6 +290,20 @@ }, "additionalProperties": false }, + { + "description": "Admin-only escape hatch: force a reset from any epoch state, including mid-exchange. A ceremony that keeps ending sub-threshold auto-resets straight into the next attempt without ever passing through `InProgress`, which is the only state [`Self::TriggerReset`] accepts - so without this, a looping ceremony can never be stopped or redirected.", + "type": "object", + "required": [ + "trigger_forced_reset" + ], + "properties": { + "trigger_forced_reset": { + "type": "object", + "additionalProperties": false + } + }, + "additionalProperties": false + }, { "description": "Transfers ownership of the epoch dealer to another address. This assumes off-chain hand-over of keys", "type": "object", diff --git a/contracts/coconut-dkg/schema/raw/execute.json b/contracts/coconut-dkg/schema/raw/execute.json index 7b7e626407f..2138add3be6 100644 --- a/contracts/coconut-dkg/schema/raw/execute.json +++ b/contracts/coconut-dkg/schema/raw/execute.json @@ -199,6 +199,20 @@ }, "additionalProperties": false }, + { + "description": "Admin-only escape hatch: force a reset from any epoch state, including mid-exchange. A ceremony that keeps ending sub-threshold auto-resets straight into the next attempt without ever passing through `InProgress`, which is the only state [`Self::TriggerReset`] accepts - so without this, a looping ceremony can never be stopped or redirected.", + "type": "object", + "required": [ + "trigger_forced_reset" + ], + "properties": { + "trigger_forced_reset": { + "type": "object", + "additionalProperties": false + } + }, + "additionalProperties": false + }, { "description": "Transfers ownership of the epoch dealer to another address. This assumes off-chain hand-over of keys", "type": "object", diff --git a/contracts/coconut-dkg/src/contract.rs b/contracts/coconut-dkg/src/contract.rs index 9cb50088bff..c0e0bf0cb1a 100644 --- a/contracts/coconut-dkg/src/contract.rs +++ b/contracts/coconut-dkg/src/contract.rs @@ -20,7 +20,8 @@ use crate::epoch_state::queries::{ }; use crate::epoch_state::storage::{load_current_epoch, save_epoch}; use crate::epoch_state::transactions::{ - try_advance_epoch_state, try_initiate_dkg, try_trigger_reset, try_trigger_resharing, + try_advance_epoch_state, try_initiate_dkg, try_trigger_forced_reset, try_trigger_reset, + try_trigger_resharing, }; use crate::error::ContractError; use crate::state::queries::query_state; @@ -130,6 +131,7 @@ pub fn execute( ExecuteMsg::AdvanceEpochState {} => try_advance_epoch_state(deps, env), ExecuteMsg::TriggerReset {} => try_trigger_reset(deps, env, info), ExecuteMsg::TriggerResharing {} => try_trigger_resharing(deps, env, info), + ExecuteMsg::TriggerForcedReset {} => try_trigger_forced_reset(deps, env, info), ExecuteMsg::TransferOwnership { transfer_to } => { try_transfer_ownership(deps, env, info, transfer_to) } diff --git a/contracts/coconut-dkg/src/epoch_state/transactions/mod.rs b/contracts/coconut-dkg/src/epoch_state/transactions/mod.rs index e206fcb3790..67b7eeed225 100644 --- a/contracts/coconut-dkg/src/epoch_state/transactions/mod.rs +++ b/contracts/coconut-dkg/src/epoch_state/transactions/mod.rs @@ -53,7 +53,36 @@ pub(crate) fn try_trigger_reset( // only allow reset when the DKG exchange isn't in progress if !current_epoch.state.is_in_progress() { - return Err(ContractError::CantReshareDuringExchange); + return Err(ContractError::CantResetDuringExchange); + } + + let next_epoch = current_epoch.next_reset(env.block.time); + save_epoch(deps.storage, env.block.height, &next_epoch)?; + + reset_dkg_state(deps.storage)?; + + Ok(Response::default()) +} + +/// The admin's escape hatch: a reset callable from any state past initialisation. +/// +/// [`try_trigger_reset`] is deliberately gated on `InProgress`, but a ceremony that keeps ending +/// sub-threshold auto-resets straight into the next attempt without ever getting there, so the +/// ordinary lever can never stop or redirect a looping ceremony. This one can, and it can also +/// abort an exchange already in flight. Aborting leaves the in-flight epoch abandoned, which +/// issuance already tolerates: `keys_in_service` is carried over, not derived from the epoch id. +pub(crate) fn try_trigger_forced_reset( + deps: DepsMut<'_>, + env: Env, + info: MessageInfo, +) -> Result { + // only the admin is allowed to force a DKG reset + DKG_ADMIN.assert_admin(deps.as_ref(), &info.sender)?; + let current_epoch = load_current_epoch(deps.storage)?; + + // there is nothing to reset before the DKG has been initiated + if matches!(current_epoch.state, EpochState::WaitingInitialisation) { + return Err(ContractError::WaitingInitialisation); } let next_epoch = current_epoch.next_reset(env.block.time); @@ -154,4 +183,141 @@ pub(crate) mod tests { assert!(THRESHOLD.may_load(&deps.storage).unwrap().is_none()); } + + #[cfg(test)] + mod forced_reset { + use super::*; + use nym_coconut_dkg_common::types::{StateProgress, TimeConfiguration}; + + #[test] + fn only_the_admin_may_force_a_reset() { + let mut deps = init_contract(); + let env = mock_env(); + + try_initiate_dkg( + deps.as_mut(), + env.clone(), + message_info(&Addr::unchecked(ADMIN_ADDRESS), &[]), + ) + .unwrap(); + + let not_admin = deps.api.addr_make("not an admin"); + let res = + try_trigger_forced_reset(deps.as_mut(), env.clone(), message_info(¬_admin, &[])) + .unwrap_err(); + assert_eq!(ContractError::Admin(AdminError::NotAdmin {}), res); + + // and the epoch was left alone + assert_eq!(0, load_current_epoch(&deps.storage).unwrap().epoch_id); + } + + #[test] + fn there_is_nothing_to_force_before_initialisation() { + let mut deps = init_contract(); + let env = mock_env(); + + let res = try_trigger_forced_reset( + deps.as_mut(), + env, + message_info(&Addr::unchecked(ADMIN_ADDRESS), &[]), + ) + .unwrap_err(); + assert_eq!(ContractError::WaitingInitialisation, res); + } + + /// The reason this message exists: a ceremony that keeps ending sub-threshold auto-resets + /// into the next attempt without ever reaching `InProgress`, which is the only state the + /// ordinary `TriggerReset` accepts - so a looping ceremony locks the admin out entirely. + #[test] + fn a_forced_reset_escapes_the_sub_threshold_loop() { + let mut deps = init_contract(); + let mut env = mock_env(); + let admin = message_info(&Addr::unchecked(ADMIN_ADDRESS), &[]); + + // epoch 7's keys are in service and the ceremony for 11 is about to end sub-threshold + THRESHOLD.save(deps.as_mut().storage, &42).unwrap(); + let failing = Epoch { + state_progress: StateProgress { + verified_keys: 41, + ..Default::default() + }, + keys_in_service: Some(7), + ..Epoch::new( + EpochState::VerificationKeyFinalization { resharing: false }, + 11, + TimeConfiguration::default(), + env.block.time, + ) + }; + save_epoch(deps.as_mut().storage, env.block.height, &failing).unwrap(); + + env.block.time = env.block.time.plus_seconds( + TimeConfiguration::default().verification_key_finalization_time_secs + 1, + ); + try_advance_epoch_state(deps.as_mut(), env.clone()).unwrap(); + + // the loop: a fresh attempt is already running, and the ordinary lever is refused + // (with the reset error, not the resharing one it used to misreport) + let looping = load_current_epoch(&deps.storage).unwrap(); + assert_eq!(12, looping.epoch_id); + assert!(!looping.state.is_in_progress()); + let res = try_trigger_reset(deps.as_mut(), env.clone(), admin.clone()).unwrap_err(); + assert_eq!(ContractError::CantResetDuringExchange, res); + + // the escape hatch is not + try_trigger_forced_reset(deps.as_mut(), env.clone(), admin).unwrap(); + + let after = load_current_epoch(&deps.storage).unwrap(); + assert_eq!(13, after.epoch_id); + assert_eq!( + EpochState::PublicKeySubmission { resharing: false }, + after.state + ); + // the abandoned attempts retired nothing: epoch 7 keeps issuing throughout + assert_eq!(Some(7), after.issuing_epoch_id()); + assert!(THRESHOLD.may_load(&deps.storage).unwrap().is_none()); + } + + #[test] + fn a_forced_reset_works_from_every_post_initialisation_state() { + let states = [ + EpochState::PublicKeySubmission { resharing: false }, + EpochState::PublicKeySubmission { resharing: true }, + EpochState::DealingExchange { resharing: false }, + EpochState::DealingExchange { resharing: true }, + EpochState::VerificationKeySubmission { resharing: false }, + EpochState::VerificationKeySubmission { resharing: true }, + EpochState::VerificationKeyValidation { resharing: false }, + EpochState::VerificationKeyValidation { resharing: true }, + EpochState::VerificationKeyFinalization { resharing: false }, + EpochState::VerificationKeyFinalization { resharing: true }, + EpochState::InProgress, + ]; + + for state in states { + let mut deps = init_contract(); + let env = mock_env(); + + let epoch = Epoch::new(state, 5, TimeConfiguration::default(), env.block.time); + save_epoch(deps.as_mut().storage, env.block.height, &epoch).unwrap(); + + try_trigger_forced_reset( + deps.as_mut(), + env, + message_info(&Addr::unchecked(ADMIN_ADDRESS), &[]), + ) + .unwrap(); + + let after = load_current_epoch(&deps.storage).unwrap(); + assert_eq!(6, after.epoch_id, "from {state}"); + // always a reset, never a resharing: aborted resharings included, since their + // registrants may hold nothing to reshare + assert_eq!( + EpochState::PublicKeySubmission { resharing: false }, + after.state, + "from {state}" + ); + } + } + } } From 72ebe49304248a6cc4dab44e6952d509ea9ca972 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C4=99drzej=20Stuczy=C5=84ski?= Date: Tue, 1 Sep 2026 09:52:45 +0100 Subject: [PATCH 2/8] fix(nym-api): decide the signing keys' epoch on the guard actually served keys_for_epoch read which epoch the live slot held, dropped the lock, and re-acquired it to serve the keys - so a rotation landing between the two reads served a keypair for a different epoch than requested, and both partial-signature paths would persist the wrong-key result for good. The check now happens on the guard that is returned, and the archive answers whenever the live slot does not match - which also finds keys that moved to the archive between the reads instead of erroring on where they were. --- nym-api/src/ecash/keys/mod.rs | 35 ++++++++++++++++++----------------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/nym-api/src/ecash/keys/mod.rs b/nym-api/src/ecash/keys/mod.rs index d39b57bb459..f7772cf6cae 100644 --- a/nym-api/src/ecash/keys/mod.rs +++ b/nym-api/src/ecash/keys/mod.rs @@ -75,16 +75,6 @@ impl KeyPair { 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`, wherever we happen to be keeping them - the live slot /// if it is still the epoch we sign for, otherwise the archive. /// @@ -101,17 +91,28 @@ impl KeyPair { &self, epoch_id: EpochId, ) -> Result, EcashError> { - let current = self.current_key_epoch().await; - - if current == Some(epoch_id) { - return RwLockReadGuard::try_map(self.read_keys().await, |keys| keys.as_ref()) - .map_err(|_| EcashError::KeyPairNotDerivedYet); - } + // the epoch check happens on the guard actually returned: were "which epoch is + // live" a separate read, the controller could rotate the slot between it and the + // re-acquisition, and a keypair for a different epoch than requested would be + // served - with its wrong-epoch partials persisted + let live = match RwLockReadGuard::try_map(self.read_keys().await, |keys| { + keys.as_ref() + .filter(|keys| keys.issued_for_epoch == epoch_id) + }) { + Ok(keys) => return Ok(keys), + Err(live) => live, + }; + + // the archive is consulted whenever the live slot did not match, so keys that + // rotated out between the two reads are found where they now live rather than + // erroring on where they used to + let available = live.as_ref().map(|keys| keys.issued_for_epoch); + drop(live); RwLockReadGuard::try_map(self.archived.read().await, |archived| { archived.get(&epoch_id) }) - .map_err(|_| match current { + .map_err(|_| match available { Some(available) => EcashError::InvalidSigningKeyEpoch { requested: epoch_id, available, From 0d949c6cd174ea703bab4a7fdcab84aafbba20ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C4=99drzej=20Stuczy=C5=84ski?= Date: Tue, 1 Sep 2026 09:52:58 +0100 Subject: [PATCH 3/8] fix(ecash): treat an already-lapsed epoch deadline as no deadline when caching The settled epoch keeps the deadline of its last self-extension, and since the extension's removal nothing will ever move it. Once that timestamp passes, min(state_end - now, ceiling) goes negative and the cached copy expires in the past - permanently invalid, so every epoch read on every signer, the proxy and node-status-api goes to the chain from that moment until the next ceremony stores a fresh epoch. A deadline already behind us now falls back to the staleness ceiling, in both copies of the cache. --- .../credential-proxy/src/nym_api_helpers.rs | 48 +++++++++++++++- nym-api/src/ecash/comm.rs | 56 ++++++++++++++++--- 2 files changed, 94 insertions(+), 10 deletions(-) diff --git a/common/credential-proxy/src/nym_api_helpers.rs b/common/credential-proxy/src/nym_api_helpers.rs index 71caa2806f6..43a5028402b 100644 --- a/common/credential-proxy/src/nym_api_helpers.rs +++ b/common/credential-proxy/src/nym_api_helpers.rs @@ -43,9 +43,15 @@ impl CachedEpoch { #[allow(clippy::unwrap_used)] let state_end = OffsetDateTime::from_unix_timestamp(epoch_finish.seconds() as i64).unwrap(); - let until_epoch_state_end = state_end - now; - // make it valid until the next epoch transition or next 5min, whichever is smaller - min(until_epoch_state_end, 5 * time::Duration::MINUTE) + if state_end <= now { + // a deadline nothing will ever advance (the settled epoch's lapsed + // self-extension) - an expiry computed against it would land in the + // past and disable the cache for good + 5 * time::Duration::MINUTE + } else { + // valid until the next epoch transition or next 5min, whichever is smaller + min(state_end - now, 5 * time::Duration::MINUTE) + } } else { 5 * time::Duration::MINUTE }; @@ -109,3 +115,39 @@ where Ok(shares) } + +#[cfg(test)] +mod tests { + use super::*; + use cosmwasm_std::Timestamp; + + /// The settled mainnet epoch keeps the deadline of its last self-extension, and since + /// the extension's removal nothing will ever move it. Once that timestamp passes, an + /// expiry computed against it lands in the past - a permanently invalid cache, sending + /// every epoch read to the chain until the next ceremony stores a fresh epoch. + #[test] + fn a_lapsed_deadline_does_not_disable_the_cache() { + let mut epoch = Epoch::default(); + epoch.deadline = Some(Timestamp::from_seconds(1)); + + let mut cached = CachedEpoch::default(); + cached.update(epoch); + + assert!(cached.is_valid()); + } + + /// The counterpart boundary: a deadline still ahead caps the expiry below the refresh + /// interval, so the copy dies at the state change it cannot see past and never claims + /// a ceremony has concluded when it has not. + #[test] + fn a_nearer_state_end_still_caps_the_cache_validity() { + let now = OffsetDateTime::now_utc().unix_timestamp() as u64; + let mut epoch = Epoch::default(); + epoch.deadline = Some(Timestamp::from_seconds(now + 60)); + + let mut cached = CachedEpoch::default(); + cached.update(epoch); + + assert!(cached.valid_until <= OffsetDateTime::now_utc() + time::Duration::seconds(60)); + } +} diff --git a/nym-api/src/ecash/comm.rs b/nym-api/src/ecash/comm.rs index c4ebdbb7bc3..8e719121a42 100644 --- a/nym-api/src/ecash/comm.rs +++ b/nym-api/src/ecash/comm.rs @@ -69,10 +69,16 @@ impl CachedEpoch { #[allow(clippy::unwrap_used)] let state_end = OffsetDateTime::from_unix_timestamp(epoch_finish.seconds() as i64).unwrap(); - let until_epoch_state_end = state_end - now; - // make it valid until the next epoch transition or the staleness ceiling, whichever - // is smaller - min(until_epoch_state_end, max_staleness) + if state_end <= now { + // a deadline nothing will ever advance (the settled epoch's lapsed + // self-extension) - an expiry computed against it would land in the + // past and disable the cache for good + max_staleness + } else { + // valid until the next epoch transition or the staleness ceiling, + // whichever is smaller + min(state_end - now, max_staleness) + } } else { max_staleness }; @@ -329,9 +335,10 @@ mod tests { /// whatever it sees under that epoch id with no expiry and no invalidation, so a /// single mid-ceremony query pins an empty signer set for good. /// - /// Currently RED. Note the fix for the unverified-share handling widened this: - /// mid-ceremony queries used to fail (and errors are not cached), whereas now they - /// succeed with an empty list, which is exactly what gets cached. + /// Note the fix for the unverified-share handling had widened this before the + /// mid-ceremony bypass closed it: mid-ceremony queries used to fail (and errors are + /// not cached), whereas they now succeed with an empty list, which is exactly what + /// would have been cached. /// /// The ceremony here is a precondition, not the subject, so it runs against the /// contract without any DKG cryptography. @@ -381,4 +388,39 @@ mod tests { Ok(()) } + + /// The settled mainnet epoch keeps the deadline of its last self-extension, and since + /// the extension's removal nothing will ever move it. Once that timestamp passes, an + /// expiry computed against it lands in the past - a permanently invalid cache, sending + /// every epoch read to the chain until the next ceremony stores a fresh epoch. + #[test] + fn a_lapsed_deadline_does_not_disable_the_cache() { + use nym_coconut_dkg_common::types::Timestamp; + + let mut epoch = Epoch::default(); + epoch.deadline = Some(Timestamp::from_seconds(1)); + + let mut cached = CachedEpoch::default(); + cached.update(epoch, Duration::from_secs(300)).unwrap(); + + assert!(cached.is_valid()); + } + + /// The counterpart boundary: a deadline still ahead caps the expiry below the staleness + /// ceiling. This is the pessimism guarantee `current_epoch_details` documents - the copy + /// dies at the state change it cannot see past, so it can never claim a ceremony has + /// concluded when it has not. + #[test] + fn a_nearer_state_end_still_caps_the_cache_validity() { + use nym_coconut_dkg_common::types::Timestamp; + + let now = OffsetDateTime::now_utc().unix_timestamp() as u64; + let mut epoch = Epoch::default(); + epoch.deadline = Some(Timestamp::from_seconds(now + 60)); + + let mut cached = CachedEpoch::default(); + cached.update(epoch, Duration::from_secs(300)).unwrap(); + + assert!(cached.valid_until <= OffsetDateTime::now_utc() + time::Duration::seconds(60)); + } } From 93dc096819cf6b7c0cd301dc83c82e40c0deb66f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C4=99drzej=20Stuczy=C5=84ski?= Date: Tue, 1 Sep 2026 09:53:14 +0100 Subject: [PATCH 4/8] feat(coconut-dkg): say on the transaction when an advance is held for dealers The zero-dealer hold succeeds looking exactly like a real advance, so a ceremony stuck waiting for dealers - the reset's likeliest failure mode if signers are not ready - could only be noticed by diffing successive epoch queries. The held branch now emits awaiting_dealers with the held epoch's id, making the condition visible on the transaction itself and alertable from the event stream. A real advance stays attribute-free, pinned by test. --- .../coconut-dkg/src/event_attributes.rs | 6 ++++ .../transactions/advance_epoch_state.rs | 30 ++++++++++++++++--- 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/common/cosmwasm-smart-contracts/coconut-dkg/src/event_attributes.rs b/common/cosmwasm-smart-contracts/coconut-dkg/src/event_attributes.rs index 28166677af5..a138c677a63 100644 --- a/common/cosmwasm-smart-contracts/coconut-dkg/src/event_attributes.rs +++ b/common/cosmwasm-smart-contracts/coconut-dkg/src/event_attributes.rs @@ -3,3 +3,9 @@ pub const NODE_INDEX: &str = "node_index"; pub const DKG_PROPOSAL_ID: &str = "proposal_id"; + +/// Emitted (with the held epoch's id as the value) when an epoch-state advance is held +/// because nobody has registered as a dealer - without it, the held transaction succeeds +/// looking exactly like a real advance, and a ceremony stuck waiting for dealers can only +/// be noticed by diffing successive epoch queries. +pub const AWAITING_DEALERS: &str = "awaiting_dealers"; diff --git a/contracts/coconut-dkg/src/epoch_state/transactions/advance_epoch_state.rs b/contracts/coconut-dkg/src/epoch_state/transactions/advance_epoch_state.rs index 7c00e6ec6d0..1bd4e7a5ea9 100644 --- a/contracts/coconut-dkg/src/epoch_state/transactions/advance_epoch_state.rs +++ b/contracts/coconut-dkg/src/epoch_state/transactions/advance_epoch_state.rs @@ -70,7 +70,12 @@ pub fn try_advance_epoch_state(deps: DepsMut<'_>, env: Env) -> Result Date: Tue, 1 Sep 2026 09:53:31 +0100 Subject: [PATCH 5/8] docs(ecash): true up the comments the stack review flagged Drop the stale 'Currently RED' markers (green since the fixes landed in the same commits), state the guarantee is_ceremony_concluded actually provides (nothing at-or-below the in-service epoch can change again; an abandoned epoch below it reads concluded but is equally frozen), match corrupt_vk_share's doc to the closure it takes, and move a doc comment above its #[instrument] attribute. --- .../cosmwasm-smart-contracts/coconut-dkg/src/types.rs | 10 ++++++---- .../src/ticketbook_manager/wallet_shares.rs | 10 +++++----- contracts/coconut-dkg/src/testable_dkg_contract/mod.rs | 4 ++-- nym-api/src/ecash/state/mod.rs | 8 ++++---- 4 files changed, 17 insertions(+), 15 deletions(-) diff --git a/common/cosmwasm-smart-contracts/coconut-dkg/src/types.rs b/common/cosmwasm-smart-contracts/coconut-dkg/src/types.rs index 3e3e37a4234..3454e65504c 100644 --- a/common/cosmwasm-smart-contracts/coconut-dkg/src/types.rs +++ b/common/cosmwasm-smart-contracts/coconut-dkg/src/types.rs @@ -278,10 +278,12 @@ impl Epoch { /// /// Sitting below the current epoch is not enough: a failed ceremony moves the id on and /// leaves an epoch behind that concluded nothing, and calling that concluded would let - /// callers cache its empty signer set for good. So the boundary is the epoch in service, - /// which no unconcluded epoch is ever ahead of. 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". + /// callers cache its empty signer set for good. So the boundary is the epoch in service: + /// nothing at or below it can ever change again, and no epoch that can still change reads + /// concluded. (An epoch a failed ceremony abandoned below the boundary reads concluded + /// too - its records are equally frozen, merely empty.) 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 self.issuing_epoch_id() { Some(in_service) => epoch_id <= in_service, diff --git a/common/credential-proxy/src/ticketbook_manager/wallet_shares.rs b/common/credential-proxy/src/ticketbook_manager/wallet_shares.rs index 211afc25662..6c761299ec9 100644 --- a/common/credential-proxy/src/ticketbook_manager/wallet_shares.rs +++ b/common/credential-proxy/src/ticketbook_manager/wallet_shares.rs @@ -24,6 +24,11 @@ use tracing::{debug, error, info, instrument}; use uuid::Uuid; impl TicketbookManager { + /// `epoch` is resolved once by the caller and used for everything here: the signers asked, + /// the threshold required of them, the auxiliary data returned alongside, the epoch stated on + /// each request, and the epoch the shares are stored under. Resolving it again here would let + /// a ceremony concluding in between hand back shares and auxiliary data from two different + /// epochs, which a client cannot combine. #[instrument( skip(self, request_data, request, requested_on), fields( @@ -31,11 +36,6 @@ impl TicketbookManager { ticketbook_type = %request_data.ticketbook_type ) )] - /// `epoch` is resolved once by the caller and used for everything here: the signers asked, - /// the threshold required of them, the auxiliary data returned alongside, the epoch stated on - /// each request, and the epoch the shares are stored under. Resolving it again here would let - /// a ceremony concluding in between hand back shares and auxiliary data from two different - /// epochs, which a client cannot combine. pub async fn try_obtain_wallet_shares( &self, request: Uuid, diff --git a/contracts/coconut-dkg/src/testable_dkg_contract/mod.rs b/contracts/coconut-dkg/src/testable_dkg_contract/mod.rs index 9dd7582ebf3..c1c9ddc982a 100644 --- a/contracts/coconut-dkg/src/testable_dkg_contract/mod.rs +++ b/contracts/coconut-dkg/src/testable_dkg_contract/mod.rs @@ -333,8 +333,8 @@ pub trait DkgContractTesterExt: .unwrap() } - /// Replace a dealer's verification key share with an arbitrary string, so that it - /// no longer pairs with the dealings that dealer actually distributed. + /// Corrupt a dealer's stored verification key share with the supplied mutation, so + /// that it no longer pairs with the dealings that dealer actually distributed. fn corrupt_vk_share(&mut self, epoch_id: EpochId, owner: &Addr, mutate: F) where F: FnOnce(&mut VerificationKeyShare), diff --git a/nym-api/src/ecash/state/mod.rs b/nym-api/src/ecash/state/mod.rs index e3361abdc9f..3d90ddfde38 100644 --- a/nym-api/src/ecash/state/mod.rs +++ b/nym-api/src/ecash/state/mod.rs @@ -1398,8 +1398,8 @@ mod tests { /// thing every ticket verification does, and gateways poll continuously, so a query /// landing mid-ceremony is a certainty rather than a risk. /// - /// Currently RED. Note `active_signer` is a *separate* cache from the communication - /// channel's `epoch_clients`, so fixing that one alone would not fix this. + /// Note `active_signer` is a *separate* cache from the communication channel's + /// `epoch_clients`, so the bypass fixing that one alone would not have fixed this. #[tokio::test] async fn a_signer_still_recognises_itself_after_a_ceremony() -> anyhow::Result<()> { let chain = SharedContractChain::new(3); @@ -1440,8 +1440,8 @@ mod tests { /// signer discovery, so a poisoned signer set leaves it permanently unavailable. /// /// It is guarded by a threshold check and `get_or_init` never caches errors, so it - /// is *designed* to recover on the next request. Currently RED because the layer - /// beneath it cannot. + /// is *designed* to recover on the next request - which it could not do while the + /// layer beneath it could not. #[tokio::test] async fn the_master_verification_key_becomes_available_after_a_ceremony() -> anyhow::Result<()> { From 00de82643106c767b9dbe2fbc539b89ef0b6f174 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C4=99drzej=20Stuczy=C5=84ski?= Date: Tue, 1 Sep 2026 09:53:35 +0100 Subject: [PATCH 6/8] test(nym-api): make the dummy's ceremony start mirror the chain's start_ceremony now clears ceremony_concluded_at and carries outgoing_keys across, as next_ceremony does; previously the two divergences compensated for each other through issuable_epochs, which a future test relying on either mid-ceremony field would not survive. keys_in_service stays synthesized from the id below: the helper doubles as a state constructor paired with set_epoch, and its callers build their states through it. --- nym-api/src/ecash/tests/mod.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/nym-api/src/ecash/tests/mod.rs b/nym-api/src/ecash/tests/mod.rs index 5968ff84fb8..c82d494503f 100644 --- a/nym-api/src/ecash/tests/mod.rs +++ b/nym-api/src/ecash/tests/mod.rs @@ -1198,10 +1198,15 @@ impl SharedCommState { /// Put the current epoch mid-ceremony, as a rotation would: its own keys do not exist yet, so /// the epoch before it is the one in service. + /// A ceremony for the current epoch id begins. This doubles as a state constructor + /// (pair it with `set_epoch`), so unlike the chain it synthesizes `keys_in_service` + /// from the id below; the other fields follow `next_ceremony` - the recorded + /// conclusion is cleared (starting a ceremony revokes the grace window) and + /// `outgoing_keys` carries over untouched. pub async fn start_ceremony(&self) { self.inner.ceremony_in_flight.store(true, Ordering::Relaxed); *self.inner.keys_in_service.write().await = self.current_epoch().checked_sub(1); - *self.inner.outgoing_keys.write().await = None; + *self.inner.ceremony_concluded_at.write().await = None; } /// A ceremony that failed: the contract resets into a fresh epoch id and the keys already in From a679c5774ff25eb61c4e2d55384b2b2bf37295ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C4=99drzej=20Stuczy=C5=84ski?= Date: Tue, 1 Sep 2026 10:27:56 +0100 Subject: [PATCH 7/8] test(ecash): pin the reason in the review's flagged broad asserts Three assertions accepted any failure where a specific one was the point: the non-member registration now asserts the cw4 group-check refusal (and derives its outsider address like every other, instead of hardcoding a bech32), the stale-order replay asserts the exact StaleVerificationOrder variant with both epoch ids, and the test dummy's verify arm now binds the order's epoch_id and asserts it against the chain's, mirroring the gate the real contract applies. --- .../src/verification_key_shares/transactions.rs | 15 +++++++++++---- nym-api/src/ecash/tests/contract_chain.rs | 12 ++++++++---- nym-api/src/ecash/tests/mod.rs | 5 ++++- 3 files changed, 23 insertions(+), 9 deletions(-) diff --git a/contracts/coconut-dkg/src/verification_key_shares/transactions.rs b/contracts/coconut-dkg/src/verification_key_shares/transactions.rs index 715b3626b4a..4b3bc1d4199 100644 --- a/contracts/coconut-dkg/src/verification_key_shares/transactions.rs +++ b/contracts/coconut-dkg/src/verification_key_shares/transactions.rs @@ -537,11 +537,18 @@ mod tests { .plus_seconds(timings.verification_key_validation_time_secs); try_advance_epoch_state(deps.as_mut(), env.clone()).unwrap(); - // now the stale order finally gets executed - let replayed = crate::contract::execute(deps.as_mut(), env, multisig_info, epoch_0_order); + // now the stale order finally gets executed, and it is the epoch guard that + // refuses it, not some incidental failure + let replayed = + crate::contract::execute(deps.as_mut(), env, multisig_info, epoch_0_order).unwrap_err(); - assert!( - replayed.is_err(), + assert_eq!( + replayed, + ContractError::StaleVerificationOrder { + owner: owner.to_string(), + order_epoch_id: 0, + current_epoch_id: 1, + }, "an order minted for epoch 0 verified a share in epoch 1" ); let share = vk_shares().load(&deps.storage, (&owner, 1)).unwrap(); diff --git a/nym-api/src/ecash/tests/contract_chain.rs b/nym-api/src/ecash/tests/contract_chain.rs index 01b42946c9f..ff1ab17ae46 100644 --- a/nym-api/src/ecash/tests/contract_chain.rs +++ b/nym-api/src/ecash/tests/contract_chain.rs @@ -848,10 +848,10 @@ mod tests { ); assert_eq!(node_index.as_deref(), Some("1")); - // and a non-member is rejected by the real group check - let outsider: AccountId = "n19lc9u84cz0yz3fww5283nucc9yvr8gsjmgeul0".parse().unwrap(); + // and a non-member is rejected by the real group check, not for some incidental reason + let outsider = chain.make_address("outsider".to_string()); let outsider_client = ContractChainClient::new(outsider, chain.clone()); - assert!(outsider_client + let refusal = outsider_client .register_dealer( "bte-key-2".to_string(), "identity-2".to_string(), @@ -859,7 +859,11 @@ mod tests { false, ) .await - .is_err()); + .unwrap_err(); + assert!( + format!("{refusal:?}").contains("not in the coconut signer group"), + "rejected, but not by the group check: {refusal:?}" + ); Ok(()) } diff --git a/nym-api/src/ecash/tests/mod.rs b/nym-api/src/ecash/tests/mod.rs index c82d494503f..08fd176f632 100644 --- a/nym-api/src/ecash/tests/mod.rs +++ b/nym-api/src/ecash/tests/mod.rs @@ -393,7 +393,7 @@ impl FakeChainState { nym_coconut_dkg_common::msg::ExecuteMsg::VerifyVerificationKeyShare { owner, resharing, - .. + epoch_id: order_epoch_id, } => { if sender.sender != self.multisig_contract.address { panic!("not multisig") @@ -403,6 +403,9 @@ impl FakeChainState { EpochState::VerificationKeyFinalization { resharing } ); let epoch_id = self.dkg_contract.epoch.epoch_id; + // mirror the contract's StaleVerificationOrder gate: an order names the + // epoch it was minted for, and only that epoch's share may be verified by it + assert_eq!(order_epoch_id, epoch_id, "stale verification order"); let Some(shares) = self.dkg_contract.verification_shares.get_mut(&epoch_id) else { panic!("no shares for epoch") }; From ffd94f6ee25d50a6e988d3728b0476af3ad96148 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C4=99drzej=20Stuczy=C5=84ski?= Date: Tue, 1 Sep 2026 11:29:15 +0100 Subject: [PATCH 8/8] clippy --- common/credential-proxy/src/nym_api_helpers.rs | 12 ++++++++---- nym-api/src/ecash/comm.rs | 12 ++++++++---- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/common/credential-proxy/src/nym_api_helpers.rs b/common/credential-proxy/src/nym_api_helpers.rs index 43a5028402b..24118cbbd7a 100644 --- a/common/credential-proxy/src/nym_api_helpers.rs +++ b/common/credential-proxy/src/nym_api_helpers.rs @@ -127,8 +127,10 @@ mod tests { /// every epoch read to the chain until the next ceremony stores a fresh epoch. #[test] fn a_lapsed_deadline_does_not_disable_the_cache() { - let mut epoch = Epoch::default(); - epoch.deadline = Some(Timestamp::from_seconds(1)); + let epoch = Epoch { + deadline: Some(Timestamp::from_seconds(1)), + ..Default::default() + }; let mut cached = CachedEpoch::default(); cached.update(epoch); @@ -142,8 +144,10 @@ mod tests { #[test] fn a_nearer_state_end_still_caps_the_cache_validity() { let now = OffsetDateTime::now_utc().unix_timestamp() as u64; - let mut epoch = Epoch::default(); - epoch.deadline = Some(Timestamp::from_seconds(now + 60)); + let epoch = Epoch { + deadline: Some(Timestamp::from_seconds(now + 60)), + ..Default::default() + }; let mut cached = CachedEpoch::default(); cached.update(epoch); diff --git a/nym-api/src/ecash/comm.rs b/nym-api/src/ecash/comm.rs index 8e719121a42..ed868908a9a 100644 --- a/nym-api/src/ecash/comm.rs +++ b/nym-api/src/ecash/comm.rs @@ -397,8 +397,10 @@ mod tests { fn a_lapsed_deadline_does_not_disable_the_cache() { use nym_coconut_dkg_common::types::Timestamp; - let mut epoch = Epoch::default(); - epoch.deadline = Some(Timestamp::from_seconds(1)); + let epoch = Epoch { + deadline: Some(Timestamp::from_seconds(1)), + ..Default::default() + }; let mut cached = CachedEpoch::default(); cached.update(epoch, Duration::from_secs(300)).unwrap(); @@ -415,8 +417,10 @@ mod tests { use nym_coconut_dkg_common::types::Timestamp; let now = OffsetDateTime::now_utc().unix_timestamp() as u64; - let mut epoch = Epoch::default(); - epoch.deadline = Some(Timestamp::from_seconds(now + 60)); + let epoch = Epoch { + deadline: Some(Timestamp::from_seconds(now + 60)), + ..Default::default() + }; let mut cached = CachedEpoch::default(); cached.update(epoch, Duration::from_secs(300)).unwrap();