Skip to content

bugfix: fix various bugs within nym-api making future re-DKG smoother [vol5] - #7107

Open
jstuczyn wants to merge 8 commits into
bugfix/dkg-fixes-vol4from
bugfix/dkg-fixes-vol5
Open

bugfix: fix various bugs within nym-api making future re-DKG smoother [vol5]#7107
jstuczyn wants to merge 8 commits into
bugfix/dkg-fixes-vol4from
bugfix/dkg-fixes-vol5

Conversation

@jstuczyn

@jstuczyn jstuczyn commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Upgrade-mode claims at ticket exhaustion + admin forced DKG reset

Two changes that close out the reset-mandatory client and contract work.

Claim with the upgrade-mode token when tickets run out

LP registration - the client default - ignored upgrade mode entirely: it raised NoTicketsAvailable before ever building a claim, and the mixnet websocket path did the same with NoMoreBandwidthCredentials. An exhausted stock is precisely the situation the upgrade-mode token exists for: during an upgrade window a gateway stops metering, so a client with nothing to spend can still connect.

Both paths now fall back to the stored token only once the ticket stock is exhausted - a held ticket always wins. Ticket-first is deliberate: signers now keep issuing through a ceremony (earlier on this branch), so tickets are replaceable mid-ceremony, while a refused JWT (e.g. the gateway's 30s recheck rate limit) would fail registrations a held ticket would have carried. Without a token, exhaustion errors exactly as before.

Everything gateway-side already existed; this is the missing client branch. The wire format and all function signatures are unchanged, so app-side integration is a dependency bump. send_upgrade_mode_jwt gains its first caller instead of being dead surface. Arm selection is unit-tested against a provider holding both a ticket and a token (mutation-checked).

Admin forced DKG reset (TriggerForcedReset)

A ceremony that keeps ending sub-threshold auto-resets straight into the next attempt without ever passing through InProgress - the only state TriggerReset accepts - so a looping ceremony locks the admin out entirely.

TriggerForcedReset is admin-only, callable from any post-initialisation state, and always resets (never reshares - an aborted resharing's registrants may hold nothing to reshare). keys_in_service carries over, so aborting an exchange retires nothing and issuance continues under the epoch in service. It is a separate message rather than a widened TriggerReset: the state gate on the ordinary lever is a safety interlock, and the chain log stays self-describing in a post-mortem. Also fixes the reset handler misreporting its refusal as CantReshareDuringExchange.


This change is Reviewable

@vercel

vercel Bot commented Aug 27, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
docs-nextra Ready Ready Preview Sep 1, 2026 10:37am UTC
nym-explorer-v2 Ready Ready Preview Sep 1, 2026 10:37am UTC
1 Skipped Deployment
Project Deployment Actions Updated
nym-node-status Ignored Ignored Preview Sep 1, 2026 10:37am UTC

Request Review

@coderabbitai

coderabbitai Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Team

Run ID: 0c27c585-b0c4-4f3a-b982-a1ddaab3c64f

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

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.
…rved

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.
…n 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.
… 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.
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.
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.
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant