fix(tbtc): anchor the emergency rekey, and correct what the certified floor bounds - #4226
fix(tbtc): anchor the emergency rekey, and correct what the certified floor bounds#4226mswilkison wants to merge 6 commits into
Conversation
The engine has always exported frost_tbtc_trigger_emergency_rekey, but nothing in Go called it. With no Go caller the only way to arm the wallet kill switch was to stop the node and mutate its durable store out of band, which produces an uncertified local write: on restart the anchor sees local and remote agreeing, so an operator who restores the pre-rekey state file erases the kill switch aimed at them with no evidence anywhere in the anchor stream. Wire the call through callBuildTaggedTBTCSignerOperation so the durable write is compare-and-swapped onto the anchor stream before the call returns, which is what makes a later erasure detectable. Two properties worth stating, because both look like omissions: The path is deliberately not admission-gated. The operation wrapper takes no capacity reservation, and admission refuses all work once headroom reaches the rotation floor while the barrier keeps admitting until the certified window is genuinely exhausted. A kill switch that capacity accounting can veto is not a kill switch, so it runs unreserved in that band. The barrier, by contrast, stays mandatory: a poisoned anchor or an exhausted window fails the trigger. Every barrier refusal predicate is operation-independent, so a state in which this trigger is refused is one in which the node already refuses every signature-producing call - the switch is redundant there, not defeated, and its residual is availability rather than authority. No Rust change: the export exists at the pinned ref, so the ABI and ci/frost-signer-pin.env are untouched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The rotation runbook said the certified floor is "checked at startup against pins that the running node cannot influence." That holds against a compromised signer process, which cannot forge the offline authority's signature. It does not hold against the operator: every artifact naming the floor - certificate chain, init config, activation manifest - is a local file the operator owns, and the certificate sequence and digest are only ever compared against the init config. Replace the overclaim, and add a section stating plainly what does bound an operator-side downgrade: the artifact bundle moves in lockstep (each rotation must change the manifest hash and advance the manifest sequence and service epoch by one), the durable trust journal is append-only so a downgrade also needs a pre-rotation store snapshot, and the anchor service can retire the previous binding. Only the third is a real control, and it lives outside this repository. Two checklist items follow from that: the anchor-service binding-retirement rule, and a decision on where a monotonic floor is published if operator rollback is in scope. Nothing in keep-core can close that gap - every candidate pin is downstream of an operator-owned file - so the honest deliverable is the requirement, not a local check. The signed activation handshake already exports the values an external monitor needs to detect a downgrade today. Also name one condition that already failed closed obliquely: a durable trust head ahead of the configured pin now reports that the artifacts are older than the store, instead of surfacing as an endpoint identity mismatch. That is diagnosability only - reverting the bundle and the store together leaves the two equal and fires nothing - and the comment says so to stop a later reader mistaking it for a fix. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
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: Pro Plus 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 |
The Go and Rust descendant validators disagreed about which references are admissible, in both directions. Rust alone refused a checkpoint store fingerprint change across generations; Go alone required the certified floor to be revision 1 of its service epoch. Neither gap is reachable today, since certificate endpoints force a revision-1 To reference and the fingerprint is pinned per certificate. But a divergence between the trees is not a latent nicety: the more permissive tree would accept a chain the other refuses on every store open, with no truncation or rebase path back. That is fail-closed, which is the safe direction, and still a dead store. Take the union rather than the intersection. This adds Rust's rule here; the mirrored genesis-floor check lands on the Rust side. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ion warning The headroom mirror refreshed only when a full readiness reconciliation succeeded. That left two blind spots. A node signing steadily and a node that had stalled looked identical at the scrape, because nothing republished between reconciliations. And pre-sign authorization caches the readiness snapshot per finalized point, taking a branch that deliberately publishes nothing, so an entire authorized batch could burn window inside one finality window with the gauge unchanged throughout. Publish from the acknowledgement install instead. Every successful compare-and-swap already holds an authenticated tip, which is exactly where these numbers are both fresh and free; the recorder's own comment says it was built for a caller holding the anchor mutex, which is this one. Both install paths funnel through the same readback, so the ordinary commit, the startup local-ahead commit, and read-recovery are all covered. Also export the rotation warning as its own gauge. It was computed but reachable only through readiness snapshots and the activation handshake - a hand-assembled loopback challenge that refuses to answer once headroom hits zero, which is when an operator most needs it. It cannot be derived at scrape time from the two headroom gauges, because the workload term needs the node's largest local seat count, so the seat count now travels with the pair. That also lets the commit path recompute the warning without an inventory: seat count only changes on DKG and retirement, both of which force a reconciliation that republishes it. Failure to compute either half leaves the previous reading standing rather than publishing a partial pair - the two are only meaningful together, and a stale pair is more honest than a half-fresh one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Two further commits, both from the analysis in #4222 and both required under every candidate design there.
The rotation warning gets its own gauge. It was computed but reachable only via readiness snapshots and the activation handshake — a hand-assembled loopback challenge that hard-errors once headroom reaches zero, which is exactly when an operator needs it. It can't be derived at scrape time from the two headroom gauges, because the workload term needs the node's largest local seat count, so the seat count now travels with the pair; that also lets the commit path recompute the warning without an inventory of its own. A partial computation leaves the previous reading standing rather than publishing half a pair — the two dimensions are only meaningful together, and a stale pair is more honest than a half-fresh one.
Verified: |
…raming The rotation runbook's cadence table and its lifetime warning were built from a single burn figure per input. Real fault-free burn is a range - k+3 to 3k+2 generations - because a call whose sweep prologue mutates state advances more than one generation, and the document's own note already retracted an earlier set of figures for a related reason without correcting the table itself. Any capacity number taken from a point value understates the window by roughly a factor of two, and these are the figures that feed anchor-window sizing decisions. Rebuild the table from the range, and correct the lifetime warning: at the expected seat count a store admits roughly 630-1400 full-size sweeps over its 63 rotations, not the 64-128 previously stated. Also record that the 64-certificate cap is a size artifact - each certificate must fit a bounded journal record - rather than a security property, so a reader does not mistake the ceiling for something the chain proves. Signers are equal-weighted, so seats per operator is groupSize divided by operatorCount and the expected holder is around five. The high-seat rows are sensitivity checks on operator-set size, not a stake concentration tail, and two comments that motivated their reasoning from a fifty- or hundred-seat holder now cite the real distribution. The admission comment also read better than the truth: the superseded four-seat ceiling excluded every operator, not just the larger ones. Documentation and comments only; no behaviour change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Every anchor-window sizing and rotation-cadence figure this project has is a code reading, not an observation. Fault-free burn is only bounded - between k+3 and 3k+2 durable generations per signed input, because a call whose sweep prologue mutates state advances more than one generation - and a factor of two decides how much window a node actually has and how often it must stop for an offline ceremony. Nothing published today closes that. The headroom gauges report a level, and that level resets at every rotation, so no rate can be taken across one; and no counter records how much work produced the consumption. Add both halves. In pkg/frost/signing, monotonic totals for generations and revisions consumed, incremented at the barrier only after an acknowledgement is validated and durably read back - so they count work the anchor witnessed rather than work attempted. An operation that advanced no generation spends no revision either, since the barrier skips the compare-and-swap when the tip is unchanged, and is counted as neither. In pkg/tbtc, a counter of admitted workflows. Every other admission counter records a refusal; this one records work allowed to proceed, which is what gives the consumption totals a unit. For pre-sign it ticks once per input, so consumption divided by admissions is generations and revisions per signed input directly. The measurement matters beyond capacity planning: the runbook now documents cadence as a range pending live-testnet measurement, and the threshold for whether the anchor design needs to change at all is a burn rate nobody has yet observed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 participant
Two independent anchor-integrity fixes found while investigating #4222. Stacked on #4199 rather than folded into it, so that PR stays mergeable as-is.
Neither changes the Rust signer:
frost_tbtc_trigger_emergency_rekeyalready exists at the pinned ref, so the ABI andci/frost-signer-pin.envare untouched.1. The emergency rekey is written out of band, and the operator it targets can erase it
The engine has always exported the kill-switch trigger, but nothing in Go called it — a repo-wide grep for the symbol returns nothing. With no Go caller, the only way to arm the switch is to stop the node and mutate its durable store directly. That write never passes through the anchored operation path, so it is never compare-and-swapped onto the anchor stream. On restart the anchor sees local and remote agreeing, and an operator who restores the pre-rekey state file first has erased a kill switch aimed at them with no evidence anywhere in the stream.
The fix routes the call through
callBuildTaggedTBTCSignerOperation, so the durable write is CAS-ed onto the anchor stream before the call returns. That is what makes a later erasure detectable.Two properties that look like omissions but are deliberate, and are documented at the entrypoint:
The exported call is single-flight by contract: the engine treats an armed event as immutable and no export clears it.
Deliberately not included: an operator-facing trigger surface. An out-of-process CLI cannot work (the running node holds the store's exclusive
flock, and with the node stopped the write is unanchored again — the bug restated), and the remaining options each need decisions the code cannot supply: what names the target session, whether the switch is per-wallet or node-wide, and what the recovery path is for an accidental trigger given the event is immutable. This PR lands the anchored path so that any trigger built on it is certified; the surface itself should be a follow-up with those answers.Quarantine, fault scores, and
finalize_request_fingerprinthave the same missing-wiring shape but no runtime writers in the Rust crate at all — only test-support writers. A Go call into a nonexistent writer is not a fix, so those need a Rust-side writer first and are filed separately.2. The certified floor does not bound what the runbook says it bounds
docs/development/frost-anchor-rotation.adocsaid the floor is "checked at startup against pins that the running node cannot influence." That holds against a compromised signer process, which cannot forge the offline authority's signature. It does not hold against the operator: every artifact naming the floor — certificate chain, init config, activation manifest — is a local file the operator owns, and the certificate sequence and digest are only ever compared against the init config. The activation manifest is a signed envelope read from disk, not fetched from chain.The doc now says that, and adds what genuinely bounds an operator-side downgrade:
bindingHashandactivationManifestSequence.Only (3) is a real control, and it lives outside this repository, so it becomes a pre-activation checklist item alongside a decision on where a monotonic floor is published if operator-side rollback is in scope.
No code change closes this gap, and the PR does not pretend otherwise: every candidate in-repo pin is downstream of an operator-owned file. Putting the floor reference into the activation manifest is not merely awkward but unsatisfiable —
manifestHashfeedsbindingHashfeeds the event root, which is the floor, so the manifest would have to commit to a hash of itself. A plain minimum-sequence integer would dodge that circularity but is security-null here, because an old certificate is only presentable together with its own old manifest, which would carry the old minimum.Worth noting for whoever picks up the checklist item: the signed activation-handshake attestation already exports
trustCertificateSequence,anchorServiceEpochand the certified floor revision/generation, so an external monitor retaining the maximum ever seen per operator detects a downgrade today with no protocol change.The one code change here is diagnosability: a durable trust head ahead of the configured pin now says the artifacts are older than the store, rather than surfacing later as an opaque endpoint-identity mismatch. The comment states explicitly that this is not a bound on operator rollback — reverting the bundle and the store together leaves the two equal and fires nothing — so a later reader does not mistake it for a fix.
Verification
New tests cover the payload builder (empty/whitespace reason, over-long and malformed session IDs), the response decoder (six rejection cases, including the engine retargeting a per-signing session to its wallet session), the default-build stub failing closed, and both the honest and rejected paths of the new trust-head assertion.
🤖 Generated with Claude Code