bugfix: fix various bugs within nym-api making future re-DKG smoother [vol2] - #7082
Open
jstuczyn wants to merge 5 commits into
Open
bugfix: fix various bugs within nym-api making future re-DKG smoother [vol2]#7082jstuczyn 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-vol2
branch
from
September 1, 2026 08:58
f8c38bb to
cd03016
Compare
…ed 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.
…ache 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.
`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.
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`.
… 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.
jstuczyn
force-pushed
the
bugfix/dkg-fixes-vol2
branch
from
September 1, 2026 09:52
cd03016 to
d4d1499
Compare
mmsinclair
approved these changes
Sep 2, 2026
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
2 participants
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.
Keep credentials usable across a DKG re-run
A ticketbook stays valid for up to 7 days after it is issued, so it outlives the epoch that signed it. Several paths assumed otherwise: they answered only for the epoch currently being signed for, and refused everything for the duration of a ceremony. A re-run of the DKG would therefore take existing, unexpired ticketbooks out of service - and with a reset, where the master key changes, nothing else can serve them.
What changes
Prior epochs' keys are kept and read back. Each api renames its ecash key to
epoch-{id}-{filename}.archivedwhen it rotates, and nothing ever read those files again, so the auxiliary signatures a past epoch's credentials need could not be produced. Keys are now retained per epoch, in memory from the moment of rotation and reloaded from the archive on startup, so a process that stays up through a ceremony and one that restarts behave the same. Resharing borrows the prior secrets rather than consuming them, since it no longer destroys the key it has to keep.A concluded epoch is served while a later ceremony runs. The DKG-in-progress gate applied to every route regardless of the epoch asked about, so for the 11-22 minutes a ceremony takes nothing was served at all - turning an issuance pause into a spending outage for clients whose local cache lacked the aggregated signatures that spending requires. The three aggregated-data routes and the two partial-signature routes now refuse only the epoch whose ceremony is actually running. Blind-sign keeps the blanket gate: issuing against a previous epoch is a separate question.
The signer set consulted is the requested epoch's. An api that signed an earlier epoch but is no longer in the current set still holds those keys, and refusing it could leave a past epoch short of the threshold its aggregation needs.
The credential-proxy gets the two fixes nym-api already had. Its per-epoch signer cache had no expiry and would remember the empty set a ceremony briefly presents, disabling its fan-out for the life of the process; and it failed an entire epoch on a single unverified key share instead of skipping it. Both now match nym-api's behaviour.
Also:
epoch_concludedis renamedceremony_concluded, since it reports that an epoch's ceremony has finished - true for the whole time that epoch is in use - rather than that the epoch is over. The comparison moves ontoEpochitself so nym-api and the proxy share one implementation.This change is