bugfix: fix various bugs within nym-api making future re-DKG smoother [vol4] - #7096
Open
jstuczyn wants to merge 5 commits into
Open
bugfix: fix various bugs within nym-api making future re-DKG smoother [vol4]#7096jstuczyn wants to merge 5 commits into
jstuczyn wants to merge 5 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
jstuczyn
force-pushed
the
bugfix/dkg-fixes-vol4
branch
from
September 1, 2026 08:58
77f5a7b to
7afb971
Compare
The epoch id counts ceremonies started, not key generations produced, so `epoch_id - 1` names the wrong epoch the moment a ceremony fails: the id moves on and leaves behind an epoch that concluded nothing and never will. Everything that resolves an issuing epoch derives it that way today, so a single failed ceremony would have signers issuing under an epoch with no aggregate key. `update` records the epoch coming into service where it already stamps the conclusion, and a fresh ceremony leaves both fields alone - starting one, or failing one, retires nothing. `is_ceremony_concluded` moves onto the same footing. Sitting below the current id is not enough: an abandoned epoch does too, and treating it as concluded lets callers memoise its empty signer set with no expiry.
An epoch already in service concluded its own ceremony, so recording it is reading a fact off the chain rather than guessing one. Any other state is left unknown: which epoch is in service is exactly what was never stored, and deriving it from the epoch id is the guess this field exists to remove. Unknown refuses issuance until the next conclusion writes the truth, which is the behaviour from before mid-ceremony issuance existed. Seeding matters for the first rotation rather than the second: a fresh ceremony carries the recorded epoch forward, so without the seed the next reset would run with nothing in service and stop issuance for its duration - the very case mid-ceremony issuance was built for. Skipped once anything is recorded. Re-seeding a concluded epoch would put the same value back while clearing the epoch it superseded along with it, dropping the window a collection begun under that epoch still needs.
`current - 1` is only the epoch in service while every ceremony has concluded. A failed one moves the id on without producing keys, so the signer would have named an epoch with no aggregate key: some signers hold a derived key for it and would sign, others refuse, and the client is left with sub-threshold shares bound to an epoch that can never reach it. The grace window had the same arithmetic, so after a failure it would have honoured an epoch nobody was ever collecting under while refusing the one they were - stranding exactly the deposits it exists to protect. Both now come off a single epoch snapshot, which also removes a smaller hazard: the three separate cache reads this used to make could straddle a ceremony concluding and answer from either side of it. `current_ceremony_concluded_at` goes with them; the snapshot carries it, and `issuable_epochs` was its only caller. The dummy channel keeps the keys in service explicitly rather than deriving them, gains `fail_ceremony`, and delegates the concluded check to `Epoch::is_ceremony_concluded` so it cannot drift from the real rule.
…elow Same defect as the signer side, in the proxy's own copy of the arithmetic. `current - 1` names an epoch with no keys once a ceremony has failed, and the liveness check treated any epoch id above zero as proof that something had concluded - so a first ceremony being retried would have read as healthy while nothing was issuable at all. Both now ask the epoch which generation is in service. The retry case refuses as `CredentialsNotYetIssuable`, which is a 503: the condition clears the moment a ceremony concludes, so a caller should come back.
…elow The client's own copy of the same two derivations. It states the issuance epoch on every request now, so getting this wrong is worse here than elsewhere: the epoch it names is the epoch its shares are signed under, and an epoch a failed ceremony abandoned has no key to unblind them with. The wait loop had the matching flaw - a non-zero epoch id was taken as proof that something had concluded, so a first ceremony being retried would have let the client through to buy a book nobody could sign.
jstuczyn
force-pushed
the
bugfix/dkg-fixes-vol4
branch
from
September 1, 2026 09:52
7afb971 to
b86a490
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Record which epoch's keys are in service
The epoch id counts DKG ceremonies started, not key generations produced. Three places derived "the epoch signers are issuing under" as
current - 1while a ceremony runs - the signer, the credential proxy, and the client fetcher - and thatidentity is only correct while no ceremony has ever failed. When one does, the contract resets into a fresh id and leaves an epoch behind that concluded nothing.
Consequences, all in the window a reset is riskiest:
is_ceremony_concludedtreated anything below the current id as concluded, so callers would memoise the abandoned epoch's empty signer set with no expiryEpochnow recordskeys_in_serviceandoutgoing_keys, written whereceremony_concluded_atalready is; a ceremony starting or failing leaves both alone. All three resolvers delegate toEpoch::issuing_epoch_id(), so thearithmetic exists once and is tested once.
This change is