Skip to content

spike: recovery Stage 3 - end-to-end experimental flow - #206

Closed
willemneal wants to merge 9 commits into
fm/nido-recovery-stage2-n7from
fm/nido-recovery-stage3-n8
Closed

willemneal wants to merge 9 commits into
fm/nido-recovery-stage2-n7from
fm/nido-recovery-stage3-n8

Conversation

@willemneal

@willemneal willemneal commented Sep 14, 2026 •

Copy link
Copy Markdown
Contributor

Scope

Stage 3 of the staged recovery plan (firstmate/data/perch-zk-recovery-scout-p5/follow-up.md
§8, "Complete the experimental Nido flow"), building on Stage 1's transition
spec (#204) and Stage 2's completion-mechanism recommendation (#205 —
Variant A, no blocker recorded, adopted here per the captain's Stage 3
authorization).

This is the largest stage. Per the brief: kept honest rather than polished —
every simplification is named below and in contracts/recovery-controller/src/lib.rs's
crate doc comment (the canonical, most detailed list). This is not
authorization to deploy Protected production accounts while follow-up.md §7
remains open.

What this ships

  1. contracts/recovery-controller — a shared controller (one deployed
    instance, many accounts) implementing guardian-only, ZK-only, and
    combined evidence paths against the SAME proposal-commitment model
    (docs/recovery/TRANSITION_SPEC.md, Stage 1). Completion is Variant A —
    the controller's Policy::enforce gates the account's EXISTING
    apply_doc, zero smart-account code changes, same call-ordering
    correctness argument Stage 2 already proved. GuardianOnly enrollment
    requires no ZK machinery at all (enforced at enroll, not just
    documented — follow-up.md §5.2 HARD requirement). Combined checks both
    factors against the identical frozen commitment before promotion.
    Cancellation uses its OWN action domain and per-attempt tally, separate
    storage from initiation (follow-up.md §2.2).

  2. contracts/recovery-verifier — a NEW, fully constructorless UltraHonk
    verifier: the VK is baked into the Wasm at compile time via
    include_bytes!, no constructor, no admin, no upgrade entry point at
    all. A circuit/VK change means a new Wasm deploy (new address), never a
    mutation — follow-up.md §5.4's "immutable until an explicit upgrade,"
    interpreted as strongly as the constructorless-registry pattern allows.

  3. circuits/zk_recovery_doc — a real, adapted Noir circuit. Starts from
    circuits/zk_recovery (the pre-existing M1 raw-signer-rotation circuit)
    and swaps its 5 raw-pubkey fields (pk_prefix/pk_x_hi/lo/pk_y_hi/lo)
    for 5 new fields (doc_hash_hi/lo, cfg_version, baseline_hi/lo) in
    the auth_hash binding — same arity-15 Poseidon2 sponge, same
    Merkle/nullifier logic untouched, real bb prove fixtures.

    This is a NEW, isolated circuit crate, not an in-place edit of
    circuits/zk_recovery.
    The in-place edit was the first approach taken
    and reverted after discovering it would break the M1 module's own
    still-referenced integration tests (zk_recovery_lifecycle.rs and 5
    siblings, 33 tests total), which pin real bb-proved fixtures against
    the OLD auth_hash formula via shared
    crates/integration-tests/src/zk_fixture.rs. Isolating Stage 3's circuit
    into its own crate (and its own Poseidon2 host-hash reconstruction in
    contracts/recovery-controller/src/zk.rs, NOT depending on
    nido-zk-recovery as a library) means this PR touches ZERO bytes of the
    M1 module or its tests — circuits/zk_recovery/, contracts/zk-recovery/,
    and every M1 integration test are unchanged and re-verified green after
    the revert.

  4. Client/SDK (packages/passkey-sdk/src/recoveryStage3/) — enrollment
    config builders, target-document construction + diff (lost-key vs
    compromise, per follow-up.md §4.2), attempt/evidence builders for all
    three modes, read wrappers, and docAuthHash.ts, a JS reimplementation
    of the Rust contract's compute_doc_auth_hash — parity-tested against
    the SAME pinned zk.rs fixture, cross-validating the JS↔Rust↔circuit
    path. scripts/generate-recovery-proof.mjs is a Node CLI shelling out to
    nargo/bb for proof generation (no in-browser/mobile proving — a named
    limit). It was run for real against the pinned circuit fixture during
    development: the computed root/nullifier/auth_hash AND the freshly
    generated VK's sha256 both matched the committed Rust-side values
    exactly — strong end-to-end correctness evidence spanning
    client→CLI→circuit→contract. packages/frontend/src/pages/security/recover-v3/
    is a plain, linear experimental page (enroll → status → begin attempt w/
    diff preview → evidence → complete) reusing existing signing
    infrastructure (signAndSubmit, walletConnect.ts's kit) rather than
    inventing a new flow; it does not touch the existing /security/recover
    (M1) page. Verified: tsc clean, 257/257 passkey-sdk tests pass
    (including the new parity test), npm run build + astro check clean.

  5. docs/recovery/stage3-measurements.md — proof generation (~68ms
    bb prove, 6,976-byte proof, on an Apple M5 Max dev machine — explicitly
    NOT a realistic replacement device), on-chain verification (179.3M CPU
    instructions, ~221M headroom under the 400M mainnet tx ceiling),
    restoration behavior, enrollment-data availability, and every limit named
    again with pointers.

Explicit limits (see contracts/recovery-controller/src/lib.rs for the

canonical, more detailed list — this is a summary)

  • No reconfigure entry point. Enrollment is one-shot; baseline/mode/
    guardian-set changes require a fresh account.
  • replaced_credential_ids is client-declared, not on-chain-verified
    against the target document — the contract's only real cryptographic
    guarantee is target_doc_hash exactness; content correctness (does the
    target really equal baseline+replacements) depends on evidence providers
    reviewing the client's target-document preview before approving.
  • No "ordinary execution" freeze — has_pending gates apply_doc and
    the signer/rule/policy-removal entry points (the pre-existing guard set),
    but NOT execute() (arbitrary contract calls), confirmed by reading that
    entry point directly (bare require_auth + invoke_contract, no guard).
  • Pre-existing, NOT introduced or closed by this PR: the smart account's
    add_context_rule has its own "completion window" check
    (has_pending() || completion_granted(), predates Stage 2/3) that admits
    ANY ordinarily-admin-authorized call — not just a doc-hash-bound
    completion — to install an arbitrary new ContextRule whenever a
    cross-called controller's has_pending() is true. This controller's
    Freeze-policy has_pending makes that condition true exactly like
    Stage 2's controller already does. Variant A's own binding only
    constrains completions through apply_doc; it does nothing to close this
    separate, inherited vehicle. Flagged per follow-up.md §3.1's own warning
    about exactly this class of gap — not fixed here (would need a
    smart-account change, out of scope for "zero smart-account changes").
  • PendingActivityPolicy::Restrict exists in the type (mirroring
    TRANSITION_SPEC.md's three-way enum) but is refused at enroll —
    follow-up.md §7's "no default" made concrete as a real, tested refusal.
    policyWriteConflictPolicy (Stage 1's separate invalidate-attempt-vs-block
    axis) is not implemented as its own axis at all.
  • No factory/registry wiring — and (correction, see "Fix" section below)
    this is a HARDER limit than first described.
    This bullet originally
    assumed new accounts are created with recovery_controller: None and
    just need a post-hoc enroll_zk_recovery(controller) call. A captain
    live-test plus a live testnet probe proved that's wrong for every account
    the doc-only factory actually mints today: they're ALREADY wired to the
    M1 nido-zk-recovery pool at construction (DEPLOYED.md's M2
    genesis-insert behavior), so enroll_zk_recovery against THIS controller
    is unreachable without first running the account's own real 7-day
    initiate_recovery_rule_removal → execute_recovery_rule_removal
    migration. Making this controller the factory's resolved default is a
    deploy-time registry governance action requiring testnet/mainnet registry
    author keys — named here per the brief's "report blocked naming exactly
    what's needed," but NOT a blocker for this experiment (not needed to
    complete Stage 3's scope).
  • Frontend page is manual-input, not auto-wired: recover-v3 requires
    pasting controller/verifier/pool addresses and current/baseline doc JSON
    by hand (no registry lookup or on-chain doc auto-fetch) — a time-boxed
    scope cut; the underlying SDK functions are fully built and tested.
  • recoverStage3Actions.ts submission helpers are classic-tx only — not
    wired to the Channels relayer, and assume the fee-payer is already funded.
  • Proof generation measured on this development machine, explicitly labeled
    — not a "realistic replacement device" claim.

Test plan

  • just test (full workspace, 119 passed / 0 failed / 3 pre-existing
    ignored) green
  • just check (fmt + clippy -D pedantic) green
  • nargo test in both circuits/zk_recovery (untouched, still 5/5
    green) and circuits/zk_recovery_doc (new, 5/5 green)
  • Guardian-only full lifecycle: enroll → initiate → quorum → delay →
    complete via real apply_doc → second attempt → expiry →
    cancellation (own action domain) → inactive-account restore →
    atomicity on failed install → ordinary-admin-cannot-complete — all 8
    tests in recovery_stage3_guardian_only.rs
  • ZK-only: real bb prove UltraHonk proof verified on-chain through
    promotion (tampered-proof / wrong-target-doc-hash / unknown-root all
    independently rejected) — recovery_stage3_zk_only.rs, 4/4
  • Combined-mode: neither guardian quorum alone nor a verifying ZK proof
    alone promotes — both required — recovery_stage3_combined.rs, 2/2
  • M1's full existing suite re-verified green after the circuit-isolation
    revert (zk_recovery_* 33/33, multisig_recovery 3/3)
  • Client SDK: tsc clean, 257/257 vitest, including a JS↔Rust
    compute_doc_auth_hash parity test
  • CLI proof generation run for real end-to-end: computed
    root/nullifier/auth_hash and VK sha256 both matched the committed
    Rust-side fixture exactly
  • Frontend: npm run build + astro check clean
  • CI (this PR): Contracts (cargo test --workspace), Scout audits for
    every touched contract crate, Unit (TestAuthenticator), E2E (fast UI
    + CDP), and Dependency audit all green

Fix: account-wiring bug found by a captain live-test

The captain live-tested this PR: created a test account and reported the
contract at CCI2HO73IVWPVYFPICNWMEJVF7ZZF36ZDI5HJ7WSEOWXZYIPCG2XOUH5
(testnet) "doesn't seem to use a policy doc at all."

Investigation (read-only, direct contract introspection):

  1. That contract IS a genuine NidoSmartAccount from the current doc-only
    factory (wasm hash fe3b1878…, matches the expected build). Not a stale
    factory, not a mistaken address.
  2. Its get_applied_doc()/applied_doc_hash() are both null — correct
    and expected: the doc-only factory never auto-applies a Perch doc at
    construction, only raw context rules. Not a bug.
  3. Its recovery_controller() is CAUZ6WFUTTZCJQNNL5D3BNZSG7FYYGX46BDJE6G2XVVCGN76RKE5ESAR
    — the OLD M1 nido-zk-recovery pool, not this PR's RecoveryController.
    RecoveryController::enroll only ever writes the CONTROLLER's own
    storage; nothing checked or established the ACCOUNT's own
    recovery_controller field before letting "Enroll" run. Clicking
    Enroll against this PR's controller silently wrote config nobody would
    ever cross-call — a complete, silent no-op with respect to actual
    on-chain protection. This was the real bug, not a doc/creation-path
    issue.
  4. Deeper finding beyond the captain's one account: a live probe
    (tests/e2e/testnet/recover-v3-wiring.testnet.spec.ts) created a
    BRAND NEW account via the doc-only factory and found it was already
    wired to that same M1 pool at construction — this is DEPLOYED.md's own
    documented M2 genesis-insert behavior ("every account this factory
    creates ... installs the recovery rule ... whether or not its owner ever
    uses recovery"). So the captain's account wasn't a one-off
    misconfiguration; it's the universal starting state of every account
    this factory has ever minted. The only way into this controller today is
    the account's own real 7-day initiate_recovery_rule_removal →
    execute_recovery_rule_removal migration — there is no faster path, by
    design (that delay is what keeps the removal safe against a
    stolen-key attacker racing to strip recovery protection).

Fix:

  • packages/passkey-sdk/src/recoveryStage3/accountWiring.ts (new):
    checkAccountWiring reads the account's own recovery_controller() and
    classifies it as 'wired-to-target' / 'unwired' /
    'wired-to-different-controller'; buildWireAccountTx handles the
    one-shot fresh-account case via enroll_zk_recovery.
  • recover-v3 now calls this before Enroll: the Enroll button starts
    disabled, a "Wire account" action appears for the unwired case, and a
    different-controller mismatch (the captain's exact scenario) blocks with
    an explanation instead of a silent no-op. The Enroll click handler ALSO
    re-checks wiring itself (defense in depth beyond the disabled attribute)
    — proven live in the probe by force-enabling the button and clicking
    anyway.
  • RecoveryController::config_hash(account) -> Option<BytesN<32>> (new,
    sha256(xdr(RecoveryConfig)), on-chain, deterministic) — investigated
    whether recovery config could instead be embedded in the account's own
    Perch policy document (follow-up.md §5.5's "reviewable configuration ...
    with an accurate commitment") and confirmed that's unreachable today, not
    merely unimplemented: perch's schema is .strict() (no extension
    fields), nido's own doc-lowering throws for the one principal shape that
    could fit, and — the decisive blocker — the deployed, pinned
    perch-doc-compiler's wire-level CompiledRule type has no field for an
    arbitrary policy address at all. config_hash is an equivalent,
    independently-verifiable substitute; surfaced in the recover-v3 Status
    panel via readConfigHash. Full reasoning in
    contracts/recovery-controller/src/lib.rs's "Known limits".
  • packages/contract-bindings/recovery-controller/package.json: the
    bindings regen needed to add config_hash to the TS client used a newer
    stellar-cli that emitted a package.json missing the @nidohq scope and
    publishConfig — broke npm workspace resolution for every job that runs
    a fresh npm install (PR preview deploys, TestAuthenticator unit tests,
    fast E2E). Restored to match every sibling bindings package.

Live probe evidence (tests/e2e/testnet/recover-v3-wiring.testnet.spec.ts,
real testnet, real relayer-sponsored passkey signing, @testnet tier):

  • Fresh probe account: CB5YS4DZD6EZDB74DDHP6PBVT6KMVJILVCVZEPUB27CVK2FI2JG26ALB
  • Probe RecoveryController deploy (this PR's contract, throwaway instance,
    deliberately not added to DEPLOYED.md): CDHB5B3GI63EQKPLSQWB6OKBZKMBLRAA3SHOBCYAPTDZ6ZN3YLADTHL6
  • checkAccountWiring → 'wired-to-different-controller',
    currentControllerId = CAUZ6WFUTTZCJQNNL5D3BNZSG7FYYGX46BDJE6G2XVVCGN76RKE5ESAR
    (independently confirmed via stellar contract invoke ... recovery_controller)
  • Enroll button disabled in this state; force-enabling it and clicking
    anyway is refused by the handler's own re-check ("inert no-op") before any
    transaction is built or signed
  • Status panel / on-chain read-back agree: config() and config_hash()
    on the probe controller are both null for this account — no orphaned
    state was written

Fix 2: legacy friend-recovery stub still reachable (captain live-fail #2)

A second captain live-test on this PR: installing 1-of-1 friend recovery on
the REAL /security/ page (not the recover-v3 spike) failed with
multisig-recovery.buildInstall: doc-only: the account has no rule mutators; M-of-N friend recovery is not yet expressible as a policy document (doc v1 has all-signers principals only) — a stale error from the
201 rework. buildInstall threw unconditionally for every account
regardless of state; the "doc v1 has all-signers principals only" claim was
also stale (threshold has existed since perch 0.2.0 — the real historical
gap was stateful timelock, not principal expressiveness).

Fix: multisigRecoveryModule now routes through Stage 3's
RecoveryController (GuardianOnly mode) instead of the dead stub:

  • buildInstall checks account wiring first (checkAccountWiring): wires
    then enrolls a fresh account (two sequential ops — Soroban allows one
    InvokeHostFunction per tx), just enrolls an already-wired account, and
    refuses with an accurate error (naming the real 7-day
    initiate_recovery_rule_removal → execute_recovery_rule_removal
    constraint) for an account already wired to a different controller —
    instead of the old false doc-schema claim.
  • Two real, documented implementer choices where the config type requires a
    value but nothing mandates a default: baseline_doc_hash is an inert
    sentinel hash (this simplified form never exposes Compromise-mode
    recovery — the only case that field is checked against);
    pending_activity_policy defaults to Freeze, the more conservative of
    the two options TRANSITION_SPEC.md §10 / follow-up.md §7 explicitly
    forbid a spec-level default for
    . This is an implementer default at the
    UI layer (the type requires SOME explicit value), not a silently-chosen
    spec default — documented prominently in multisigRecovery.ts and open
    to being overridden by an explicit product decision.
  • buildRevoke now explains the real constraint (no one-step revoke
    exists) instead of throwing the stale doc-only message.
  • fromChain recognizes BOTH the legacy multisig-policy on-chain-signers
    shape (unchanged — an already-installed legacy rule keeps its only
    Revoke path in the UI) and the new Stage 3 shape (guardians read from
    PolicyState, not on-chain signers, since Stage 3's rule is zero-signer
    CallContract(self)).
  • policyChainFetch.ts's fetchPolicyState gets a branch for the Stage 3
    controller address, reading RecoveryController::config and shaping it
    to {guardians, threshold}.
  • New packages/passkey-sdk/src/recoveryStage3/deployment.ts holds the
    canonical testnet controller/verifier addresses (not registry-resolvable
    yet — see "Known limits"); recorded in DEPLOYED.md with provenance
    notes (interface-checked against contracts/recovery-controller's own
    stellar contract info interface output before being recorded).

Live-probed end to end against real testnet
(tests/e2e/testnet/security-recovery-install.testnet.spec.ts): a
genuinely fresh, unwired account (raw-deployed directly against the
smart-account wasm, bypassing the doc-only factory — which, per Fix 1's
finding, pre-wires every account to the M1 pool at genesis, so it cannot
produce an unwired account at all) completes "Set up recovery" for 1 of 1
friend through the real production form, and reloading /security/ renders
"1 of 1 friend can rotate this account's signers and rules" — confirming
the full wire → enroll → render round trip. Independently confirmed
on-chain via stellar contract invoke ... config / recovery_controller
(both match exactly what the UI built).

Still true for essentially every REAL/existing account (per Fix 1's
universal-pre-wiring finding): reaching Stage 3 recovery requires the real
7-day rule-removal migration first. This fix makes the UI behave correctly
and honestly in that state (clear refusal, not a silent no-op or a false
error) rather than making it possible to skip that migration — no such
shortcut exists, by design.

Fix 3: ZK and guardian recovery couldn't coexist (captain live-fail #3)

A third captain live-test: enrolled ZK recovery first (via /security/'s
"Add ZK recovery"), then tried to add 1-of-1 friend recovery — refused with
the wired-to-different-controller error Fix 1 shipped. Diagnosis: ZK
enrollment wired the account directly to the OLD M1 nido-zk-recovery
pool, a different controller from the guardian flow's Stage 3
RecoveryController. On this stack ZK and guardians must be able to
coexist on ONE controller regardless of which is added first — that's
literally what AuthMode::Combined already existed to express.

A. New RecoveryController::reconfigure entry point — removes the "no
reconfigure entry point" Known Limit (lib.rs's crate doc comment explains
what replaced it and its remaining bound). Accepts only two strictly
additive transitions (GuardianOnly -> Combined, ZkOnly -> Combined);
every other field must match the stored config exactly or it refuses;
blocked while has_pending. Authorization: Profile::Loss needs only
account.require_auth(); Profile::Protected + existing GuardianOnly
needs the enrolled guardian quorum's nested require_auth_for_args in the
same transaction; Profile::Protected + existing ZkOnly explicitly
refuses (ReconfigureZkEvidenceUnsupported) — a real ZK
reconfigure-evidence path needs a new circuit auth_hash domain (out of
scope); reusing an existing domain was considered and rejected as a
cross-domain replay hazard. 12 new unit tests cover both transition
directions, every rejection case, and both profiles.

Deployed as a new testnet instance (v2) — the constructorless
controller has no upgrade/admin entry point at all, so the existing
deployed instance couldn't gain reconfigure in place. See
deployment.ts/DEPLOYED.md for the v1 → v2 addresses and the "explicit
upgrade = a new immutable artifact, not a rewrite" rationale (the same
philosophy already documented for the verifier, exercised here for the
controller itself for the first time).

B. Killed the factory's genesis trap. contracts/factory's
create_account/create_account_v2 no longer unconditionally wire every
new account to the M1 pool (recovery_controller: Some(M1), atomic genesis
Merkle-leaf insert) — Fix 1's live probe already proved this made it
impossible for any fresh account to ever reach Stage 3 without a real
7-day migration; the captain's bug wasn't a misconfiguration, it was the
universal starting state. Accounts now mint with recovery_controller: None; create_account_v2's commitment argument is ignored (kept for ABI
compatibility with existing callers). Redeployed the live factory in
place
(same address CCJFOM6U…, upgrade + refresh_account_wasm_hash)
and live-verified: a freshly minted account reads recovery_controller() == null.

C. Client flows route through Stage 3 consistently.
security/index.astro's runZkEnrollment and new-account/index.astro's
enrollRecoveryPostCreate both now check wiring (checkAccountWiring),
wire if needed, and enroll-or-reconfigure based on
RecoveryController::config(account)'s actual presence/mode — instead of
enroll_zk_recovery(M1 pool) directly. multisigRecoveryModule.buildInstall
gained the mirror case: reconfigure (adding guardians) when ZK was
already enrolled on the target controller. Shared enrollment defaults
(baseline-doc sentinel, pending-activity policy, delay/expiry/max-cancels)
extracted to defaults.ts so both flows agree on every field reconfigure
requires to match exactly — otherwise whichever factor enrolls second would
always fail the field-match check.

D. Live-probed both orderings against the real production /security/
forms (tests/e2e/testnet/recovery-stage3-combined.testnet.spec.ts) —
caught and fixed a real bug in the process: policyChainFetch.ts's
fetchRecoveryControllerState only returned guardian data for
GuardianOnly mode, so a Combined-mode account's "N of M friends can
rotate…" block silently vanished from the Security page even though the
on-chain config was correct (Combined has guardians too — only ZkOnly
has none). Fixed and re-verified. Both accounts independently confirmed via
RecoveryController::config() to reach identical Combined state (same
guardians/guardian_threshold/verifier/zk_pool) regardless of order.

Live-probe accounts (throwaway testnet test accounts, not real users):

  • ZK-first: CBY2E6AA7D5PTGB5JXASYAHDRJI6LY62JFEKWWQ45J3B76FNE4HIZI6B
  • Guardian-first: CAWNROKYENDO6W2FPF4VIDO7CBUH6GOKVG2D72TGWEM3HTKNCA6NIDFZ

Both independently read back via stellar contract invoke ... config
against the new controller (CBYSWPHNWAHYUBZO5TBTO5MCW2ZC45F2C3L4JSUZXYQFNMHTOBOCCHZU):
mode: "Combined", guardians: ["GAMPJROH…"], guardian_threshold: 1,
verifier/zk_pool both populated — identical on both accounts.

Stage 3 of the staged recovery plan (firstmate/data/perch-zk-recovery-scout-p5/follow-up.md
§8): contracts/recovery-controller (shared controller implementing
guardian-only, ZK-only, and combined evidence paths against Stage 1's
proposal model, completing via Stage 2's Variant A) and
contracts/recovery-verifier (a new, fully constructorless UltraHonk
verifier). circuits/zk_recovery_doc adapts the existing zk_recovery Noir
circuit to bind a target-document hash instead of a raw pubkey, as a
NEW, isolated circuit crate (not an in-place edit — the M1
circuits/zk_recovery module and its 33+ integration tests are
byte-for-byte untouched and re-verified green).

All three modes validated end-to-end against real contracts, including a
real bb-proved UltraHonk proof verified on-chain (recovery_stage3_zk_only.rs,
recovery_stage3_combined.rs) and the full guardian-only lifecycle through a
real apply_doc completion (recovery_stage3_guardian_only.rs), matching
Stage 2's Variant A call-ordering proof.

See contracts/recovery-controller/src/lib.rs's crate doc comment for the
architecture and the canonical "Known limits" list, and
docs/recovery/stage3-measurements.md for proof generation/verification
measurements. just test and just check both green.

Client/SDK work is in progress in a follow-up commit.
The matrix wasn't kept in sync when these two Stage 3 crates were added
(same maintenance gap the justfile's fmt-pkgs list already had for
recovery-doc-completion, fixed in the previous commit).
packages/passkey-sdk/src/recoveryStage3/: enrollment config builders,
target-document construction + diff (lost-key vs compromise, per
follow-up.md §4.2), attempt/evidence builders for all three modes, read
wrappers, and a JS reimplementation of the Rust contract's
compute_doc_auth_hash (parity-tested against the same pinned zk.rs
fixture, cross-validating the whole ZK path end to end).

scripts/generate-recovery-proof.mjs: a Node CLI shelling out to nargo/bb
for proof generation (no in-browser/mobile proving — a named limit, see
the crate doc comment) — verified for real against the pinned
circuits/zk_recovery_doc fixture: computed root/nullifier/auth_hash and
VK sha256 both matched the committed Rust-side values exactly.

packages/frontend/src/pages/security/recover-v3/: a plain, linear
experimental page (enroll -> status -> begin attempt w/ diff preview ->
evidence -> complete), reusing existing signing infrastructure
(signAndSubmit, walletConnect.ts's kit) rather than inventing a new flow.
Does not touch or modify the existing /security/recover (M1) page.

Contract bindings generated for recovery-controller/recovery-verifier.

Verified: tsc clean, 257/257 passkey-sdk tests, npm run build + astro
check clean for the frontend.
@github-actions

Copy link
Copy Markdown

Preview deployed!

https://206.nido.fyi

Account URLs use numeric preview suffixes, for example <contract-address>--206.nido.fyi.

@github-actions

Copy link
Copy Markdown

Example dApp preview deployed!

https://example-pr-206.mysoroban.pages.dev

The status-message example (testnet), wallet = THIS PR's preview (https://206.nido.fyi). The live home is https://nidohq.github.io/nido/ once merged.

…_hash commitment

Captain live-tested PR 206 and found a real testnet account's Enroll click
was a no-op: `RecoveryController::enroll` only writes the controller's own
storage — nothing ever checked or established the account's own
`recovery_controller` field, so an account already wired to a different
controller (or never wired at all) got orphaned, never-cross-called config.

Adds `packages/passkey-sdk/src/recoveryStage3/accountWiring.ts`
(checkAccountWiring/buildWireAccountTx) and wires it into the recover-v3
page: Enroll is disabled until wiring is confirmed, a "wire account" action
handles the fresh-account case, and a different-controller mismatch blocks
with an explanation instead of writing dead state.

Also investigated follow-up.md §5.5's "reviewable configuration commitment
in the doc" ask and confirmed it's unreachable today: perch's schema is
strict, nido's lowering throws for the one principal shape that could fit,
and the deployed/pinned perch-doc-compiler's wire-level CompiledRule has no
field for an arbitrary policy address at all. Added
RecoveryController::config_hash (sha256(xdr(RecoveryConfig)), on-chain,
recomputable) as an equivalent, independently verifiable substitute, and
documented the doc-embedding finding in the crate's Known Limits.
…-wired to M1

Added a live-testnet Playwright probe
(tests/e2e/testnet/recover-v3-wiring.testnet.spec.ts) for the account-wiring
fix. Running it surfaced something more specific than the wiring check was
written to handle: a BRAND NEW account from the doc-only factory is not
"fresh and unwired" — it already reports recovery_controller() ==
CAUZ6WFU... (the M1 nido-zk-recovery pool) at construction, per DEPLOYED.md's
M2 genesis-insert behavior. So the captain's bug wasn't a one-off
misconfiguration on his test account; it's the universal starting state for
every account this factory has ever minted, and the only way into this
Stage 3 controller today is the real 7-day rule-removal migration.

Updated recovery-controller's crate doc comment and accountWiring.ts's
module doc comment to state this as a confirmed, live-verified fact instead
of a hypothetical case. The probe itself asserts the mismatch-detection path
(the part that IS live-reachable): checkAccountWiring correctly reports
'wired-to-different-controller', Enroll stays disabled, and even a forced
click is refused by the handler's own defensive re-check before any
transaction is built.
Regenerating bindings (needed to add config_hash to the TS client) with a
newer stellar-cli emitted a package.json missing the @nidohq scope
(name: "recovery-controller" instead of "@nidohq/recovery-controller"),
downgraded version 0.1.0 -> 0.0.0, and dropped publishConfig. npm workspaces
no longer recognized it as satisfying passkey-sdk's
"@nidohq/recovery-controller": "^0.1.0" dependency, so any fresh `npm
install` (the PR-preview deploy jobs) tried the public registry and 404'd.
Restored the fields to match every sibling bindings package.
…sh in measurements

Adds two entries to stage3-measurements.md's limits/enrollment-data sections
mirroring what the account-wiring fix and its live probe established: (1)
account wiring is a separate, currently-unreachable-without-a-7-day-migration
precondition from enroll, and (2) config_hash is the on-chain substitute for
follow-up.md §5.5's reviewable-commitment ask, since literal doc embedding is
blocked by the deployed perch-doc-compiler's wire protocol.
…egacy stub)

Second captain live-fail on PR 206: installing 1-of-1 friend recovery on the
REAL /security/ page (not the recover-v3 spike) failed with
"multisig-recovery.buildInstall: doc-only: the account has no rule
mutators; M-of-N friend recovery is not yet expressible as a policy
document" — a stale, inaccurate error from the 201 rework. buildInstall
threw unconditionally for every account regardless of state; the stub's
"doc v1 has all-signers principals only" claim was also stale (threshold
has existed since perch 0.2.0).

Rewrites multisigRecoveryModule to route through Stage 3's
RecoveryController (GuardianOnly mode) instead of the dead stub:

- buildInstall checks account wiring first (checkAccountWiring): wires
  then enrolls a fresh account (two ops, one InvokeHostFunction each,
  submitted sequentially), just enrolls an already-wired account, and
  REFUSES with an accurate error (naming the real 7-day
  initiate_recovery_rule_removal -> execute_recovery_rule_removal
  constraint, not the false doc-schema claim) for an account already wired
  to a different controller.
- Two real, documented implementer choices where the config has no
  scriptable default: baseline_doc_hash is an inert sentinel hash (this
  simplified form never exposes Compromise-mode recovery, the only case
  that field is checked against); pending_activity_policy defaults to
  Freeze, the more conservative of the two options TRANSITION_SPEC.md §10 /
  follow-up.md §7 explicitly forbid a SPEC-level default for (this is an
  implementer default at the UI layer, not a silently-chosen spec default).
- buildRevoke now explains the real constraint (no one-step revoke exists;
  needs the same 7-day migration) instead of throwing the stale doc-only
  message.
- fromChain recognizes BOTH the legacy multisig-policy on-chain-signers
  shape (unchanged, so an already-installed legacy rule doesn't lose its
  only Revoke path from the UI) and the new Stage 3 shape (guardians from
  PolicyState, not on-chain signers — Stage 3's rule is zero-signer
  CallContract(self)).
- policyChainFetch.ts's fetchPolicyState gets a branch for the Stage 3
  controller address, reading RecoveryController::config and shaping it to
  {guardians, threshold} for fromChain.
- New packages/passkey-sdk/src/recoveryStage3/deployment.ts holds the
  canonical testnet controller/verifier addresses (not registry-resolvable
  yet); recorded in DEPLOYED.md with provenance/verification notes.

Live-probed end to end against real testnet
(tests/e2e/testnet/security-recovery-install.testnet.spec.ts): a genuinely
fresh, unwired account (raw-deployed, bypassing the doc-only factory's
universal M1 pre-wiring found while fixing captain issue #1) completes
"Set up recovery" for 1 of 1 friend through the real production form, and
reloading /security/ renders "1 of 1 friend can rotate this account's
signers and rules" — confirming the full wire -> enroll -> render round
trip. Independently confirmed on-chain via `stellar contract invoke
... config` / `recovery_controller` (both match exactly).

just check (fmt + clippy -D pedantic) green; passkey-sdk: tsc clean,
263/263 vitest; astro check + npm run build clean.
Third captain live-fail on PR 206: enrolling ZK recovery first (via
/security/'s "Add ZK recovery") wired the account directly to the M1
nido-zk-recovery pool -- a DIFFERENT controller from the guardian flow's
Stage 3 RecoveryController -- so adding guardian recovery second always hit
the wired-to-different-controller refusal shipped in the previous fix, and
vice versa. On this stack ZK and guardians must be able to coexist on ONE
controller (AuthMode::Combined) regardless of which is added first.

A. New RecoveryController::reconfigure entry point (removes the "no
   reconfigure entry point" Known Limit -- lib.rs's crate doc comment
   explains what replaced it and its remaining bound). Accepts only two
   strictly-additive transitions (GuardianOnly -> Combined, ZkOnly ->
   Combined); every other field must match the stored config exactly or it
   refuses; blocked while has_pending; Profile::Loss needs only
   account.require_auth(), Profile::Protected + existing GuardianOnly needs
   the enrolled guardian quorum's nested require_auth_for_args in the same
   transaction, Profile::Protected + existing ZkOnly explicitly refuses
   (ReconfigureZkEvidenceUnsupported -- a real ZK reconfigure-evidence path
   needs a new circuit auth_hash domain, out of scope here; reusing an
   existing domain was considered and rejected as a cross-domain replay
   hazard). 12 new unit tests cover both transitions, all rejection cases,
   and both profiles.

   Deployed as a NEW testnet instance (v2) -- the constructorless
   controller has no upgrade/admin entry point at all, so the existing
   deployed instance could not gain reconfigure in place. See
   deployment.ts/DEPLOYED.md for the v1 -> v2 addresses and the "explicit
   upgrade = new immutable artifact" rationale.

B. Killed the factory's genesis trap: contracts/factory::create_account/
   create_account_v2 no longer unconditionally wire every new account to
   the M1 pool (recovery_controller: Some(M1), atomic genesis Merkle-leaf
   insert) -- a live probe already proved this made it impossible for any
   fresh account to ever reach Stage 3 without a real 7-day migration.
   Accounts now mint with recovery_controller: None; create_account_v2's
   commitment argument is ignored (kept for ABI compatibility). Redeployed
   the live factory in place (same address, upgrade + refresh_account_wasm_hash)
   and live-verified a freshly minted account reads recovery_controller() ==
   null.

C. Client flows route through Stage 3 consistently: security/index.astro's
   runZkEnrollment and new-account/index.astro's enrollRecoveryPostCreate
   both now check wiring (checkAccountWiring), wire if needed, and
   enroll-or-reconfigure based on RecoveryController::config(account)'s
   actual presence/mode -- instead of enroll_zk_recovery(M1 pool) directly.
   multisigRecoveryModule.buildInstall gained the mirror case: reconfigure
   (adding guardians) when ZK was already enrolled on the target controller.
   Shared enrollment defaults (baseline-doc sentinel, pending-activity
   policy, delay/expiry/max-cancels) extracted to defaults.ts so both flows
   agree on every field reconfigure requires to match exactly.

D. Live-probed both orderings against the real production /security/ forms
   (tests/e2e/testnet/recovery-stage3-combined.testnet.spec.ts) -- caught
   and fixed a real bug in the process: policyChainFetch.ts's
   fetchRecoveryControllerState only returned guardian data for
   GuardianOnly mode, so a Combined-mode account's "N of M friends can
   rotate..." block silently vanished from the Security page even though
   the on-chain config was correct. Both accounts independently confirmed
   via RecoveryController::config() to reach identical Combined state
   (same guardians/threshold/verifier/zk_pool) regardless of order.

just check / just test green (workspace); tsc/vitest/astro check/npm build
green (client).
@willemneal

Copy link
Copy Markdown
Contributor Author

Superseded by #207, which merges this branch's work together with 200/201/202/204/205/206 into one reconciled, non-draft PR off main. This branch stays on origin for history.

@willemneal willemneal closed this Sep 14, 2026
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