Skip to content

feat(custody)!: remove the second node-side USER custody + signing plane - #338

Merged
MichaelTaylor3d merged 9 commits into
mainfrom
loop/327-remove-second-custody-plane
Aug 24, 2026
Merged

feat(custody)!: remove the second node-side USER custody + signing plane#338
MichaelTaylor3d merged 9 commits into
mainfrom
loop/327-remove-second-custody-plane

Conversation

@MichaelTaylor3d

@MichaelTaylor3dMichaelTaylor3d commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

DRAFT — DO NOT MERGE. Custody change; the full triple gate has not returned.

Closes#327
Parent epic: https://github.com/DIG-Network/dig_ecosystem/issues/1701 (its last child)

Removes the second node-side USER custody + signing plane — the one inside dig-wallet's own
[[bin]] dig-wallet loopback server, which the #1701 freeze never covered because the
zero-population count that justified PR#326 ranged over the custody manifest
(<config_dir>/wallets/index.json) and not over seed_path().

wc_dispatch now answers only the three keyless handshake methods and forwards every other method
to the user's Sage wallet. There is no local branch to fall through to and no setting that selects
one.


1. The population count — taken first, before a line was edited

Full measurement with commands and controls: #327 (comment 5394698931). Summary:

Machine: Windows 11, micha, the loop's primary dev host.
Ranged over: BOTH bases seed_path() can resolve — %LOCALAPPDATA%and$HOME — not only
the one this platform resolves. Widening past the platform's own base is what found blob 2.

#pathcontainersidecardevice keyorigin
1%LOCALAPPDATA%\DigWallet\seed.bindig-keystore/DIGOP1origin:"auto"presentauto (operator)
2$HOME\DigWallet\seed.binlegacy/EncryptedSeed(v1)absentabsentUSER-ORIGIN

Total 2; user-origin 1, not 0. Blob 2 is what an origin-keyed count misses, in the unsafe
direction: it has no sidecar to read an origin from, and it cannot be auto-minted — autoseed has
only ever sealed DIGOP1 under a device key, there is no device key under $HOME, and the legacy
layout was written exclusively by the user import/create path.

Funding, from chain state. No claim here cites ever_funded; latch_ever_funded has zero
production callers (#286), so blob 1's "ever_funded": false is not evidence of anything.
get_coin_records_by_puzzle_hash, include_spent_coins: true:

  • positive control — burn ph 0x00..00success: true, 6527 records
  • negative control — well-formed bogus ph → success: true, 0 records
  • blob 1, all 25 HD indices 0..240 records ever, 0 unspent, every index
  • blob 2 → undeterminable; password-sealed, and no attempt was made to open it

Why this proceeded rather than splitting. Read literally the gate does not clear. Its stated
reason is that the offline export path (#1701 step 3) comes first — and that shipped in 0.145.0
(#331)
: dig_wallet::seed_export::export_mnemonic has no network surface, reads both on-disk
formats, and takes an explicit path so a file under a base this build no longer resolves stays
reachable — blob 2's exact situation. It is a real verb, dign wallet export-seed --path <p>, routed
at entrypoint.rs:678. The precondition the gate protects is already satisfied by shipped, reachable
code, so removing the plane strands nobody.


2. Blast radius

gitnexus, per-worktree index (dn-327, 225 files / 11,751 nodes / 32,040 edges), taken at the
lane's stub commit — i.e. against the pre-edit call graph:

impact wc_dispatch --direction upstream16 impacted, 7 direct, risk CRITICAL.

⚠️CRITICAL risk, as required to be stated. The full radius, read row by row, is 15 of 16 inside
crates/dig-wallet/src/lib.rs (its own callers plus the tests that drive them). Exactly one
crosses a crate boundary, at depth 3:

  • dig_wallet_rpccrates/dig-runtime/src/lib.rs, the C-ABI FFI entrypoint the browser process
    calls. It reaches wc_dispatch through wallet_dispatch, whose signature is unchanged.

detect_changes was not available: this gitnexus CLI build exposes no such subcommand, and the
MCP server is bound to the superproject index rather than this worktree. Falling back is permitted
(§2.0 bound 2) and is stated here. The substitutes are stronger for the "what breaks" question:

  1. The complete cross-crate symbol set is byte-identical before and after — every
    dig_wallet::* path referenced outside the crate, enumerated on both trees: sage::*
    (40 symbols incl. rpc::WalletBackend), seed_export, autoseed, run, wallet_dispatch.
    The removed plane had zero external consumers; it was entirely private to crate::lib.
  2. cargo clippy --workspace --all-targets -- -D warnings — clean. The compiler is exhaustive
    where a call-graph query is heuristic.
  3. Full workspace suite: 2257 passed / 0 failed across 40 suites.

dig-node-service builds against WalletBackend, before and after. It consumes only
dig_wallet::sage::rpc::* (+ seed_export, autoseed) — control.rs:820 holds
Arc<dig_wallet::sage::rpc::WalletBackend>. The RPC control plane is untouched by this diff.


3. autoseed.rs survives — proven by tests, not asserted

autoseed is the node's own DIGOP1/DIGVK1operator identity: a machine credential that never
leaves the host, not custody of anyone's funds. Deleting it would break node auth rather than tighten
anything.

  • cargo test -p dig-wallet --lib autoseed:: → 15 passed, 0 failed. Untouched by this PR.
  • seed_store keeps its one production caller (autoseed), and seed_store::4 passed.
  • SPEC §16.4 now says explicitly that this seed is the node's own identity and that §18.20 does not
    retire it, so a future reader does not "tighten custody" by deleting the node's credential.

4. §908 made structural, not merely policed

  • AppState has no unlocked-session field — a signer would have no material to read.
  • seed_store::encrypt_seed is #[cfg(test)] — production code that sealed a user seed would
    not compile. Its only remaining callers are the fixtures that prove old blobs still open.
  • No wallet-source setting exists, so there is no second route to select.

5. Tests

dig-wallet lib: 606 passed, 0 failed. Coverage 85.08% lines / 80.64% functions / 87.28%
regions
(floor is 80; CI gates the workspace at --fail-under-lines 80).

New

  • seed_export_rescue.rs (2 tests) — drives the real dign binary against a legacy
    EncryptedSeed
    blob at a foreign base with no sidecar and no device key: blob 2's exact
    shape. Both nearest-wrong implementations pass a friendlier fixture and fail this one — a rescue
    reading only the current container cannot decrypt it, and a rescue that resolves the default path
    cannot reach it. Plus a wrong-password control asserting no fragment of the phrase appears on the
    failure path.
  • an_approved_origins_sign_request_is_parked_for_sage_not_answered_locally — the positive half of
    the existing refusal test. This is a placement change, so the observable that must move is
    where the request ends up: asserting a 501 or an empty result would be satisfied identically by a
    local signer that happened to be locked, which is the state the old build sat in most of the time.
    Reaching the delegate queue cannot be.

Revert-proofs — each fix reverted individually, after committing

mutationresult
decrypt_seed format dispatch → keystore-onlylegacy-rescue test FAILS
export-seed ignores --pathlegacy-rescue test FAILS
export name added to WC_METHOD_CATALOGUEdisjointness test FAILS
catalogue check disabledunsupported_advanced_legs…FAILS

Six guards restored that a compile-driven prune had silently taken. They referenced
WalletSource/Session, so they stopped compiling — but the behaviour they guard survives.
Losing them would have left the export-class refusal, the unknown-method 501 and the
handshake/delegate split with no coverage at all. Re-expressed against current behaviour; two got
stronger, because the routing split was a pure-function assertion over a table that no longer exists
and is now a behavioural check that the request reaches Sage.

6. ⚠️ A vacuity found and reported rather than papered over

The runtime export-class guard in wc_dispatch is currently UNREACHABLE. Measured: disabling it
leaves the entire suite green, because every export spelling is already outside
WC_METHOD_CATALOGUE, so the check below refuses all of them with an identical 501. The two checks
mask each other.

This is pre-existing and was faithfully carried over from the original test, which had the same blind
spot. Handled by saying so in the code comment, correcting the test's own claim about what it proves,
and pinning the invariant that is load-bearing —
export_class_methods_are_absent_from_the_catalogue, written over the catalogue constant because
dispatch answers 501 either way and a behavioural check cannot see the difference. The guard stays,
and stays ordered first, because that ordering is what survives the change that makes it matter.

7. Docs, in the same unit of work

  • SPEC.md §18.20 rewritten from "§908 is NOT yet whole" to "§908 is satisfied on BOTH
    planes"
    , with the recovery contract stated.
  • sage/custody.rs's "not yet on that one" sentence corrected — it named this gap.
  • §16.2 (the local broadcast gate went with the signer it gated), §16.3 (no local reveal
    route), §16.4 (autoseed is the node's own identity), and the §16 conformance row.
  • Swept 10 further sentences in dig-wallet and dig-runtime still describing a local signer, an
    unlocked session or a wallet source.

8. UI — both surfaces made honest

Neither page may imply a capability that is gone. Settings loses the backup/restore and identity
cards and gains an explicit "this browser holds no wallet keys" statement plus a residual-seed card
pointing at the terminal command; the wallet page's four-view auth gate becomes one connect gate. The
phrase is deliberately never revealed in a browser: a page that can display it is a page that can be
tricked into sending it. Escape hatches kept — reading DIG content needs no wallet, and that is said
on both pages.

Balance is asked of Sage and an unanswered balance renders as unknown, never 0 — an empty
wallet and an unreachable one look identical as "0", and that is the figure a user decides whether to
spend against.

9. Versions + deps

Workspace 0.146.0 → 0.147.0, dig-wallet0.31.0 → 0.32.0. Breaking (routes removed), minor
under 0.x. CHANGELOG is git-cliff-generated on release and deliberately not hand-edited.

§2.4b:dig-wallet's dig-* deps are already at latest — dig-node-control-interface 0.19,
dig-offers 0.2, dig-keystore 0.9, each confirmed against the crates.io index. No chia-* bump
is takeable here
: the chia-0.36 cascade (#3152) is blocked behind #2761, and forcing one would ship
this crate split across two chia lines.


⚠️dig-node's midnight-UTC cron cuts a real stable release unattended, so whatever lands on main
reaches users at 00:00Z. This gate is the last line of defence, not the first of two.

Co-Authored-By: Claude <noreply@anthropic.com>
MichaelTaylor3dand others added 3 commits August 24, 2026 05:00
…llet
WIP: lib compiles; tests + UI + docs still to follow.
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
@MichaelTaylor3d
MichaelTaylor3dforce-pushed the loop/327-remove-second-custody-plane branch from b39044b to d016f03CompareAugust 24, 2026 12:33
MichaelTaylor3dand others added 5 commits August 24, 2026 05:53
Co-Authored-By: Claude <noreply@anthropic.com>
…moved plane
Co-Authored-By: Claude <noreply@anthropic.com>
…s on
The runtime export-class guard is unreachable today: every export spelling is
already outside WC_METHOD_CATALOGUE, so removing the guard leaves the suite green.
Measured, not assumed. The invariant that IS load-bearing is the disjointness, and
it is now pinned directly over the catalogue constant.
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
A `git checkout -- Cargo.lock` during a revert-proof restored the pre-bump entries,
so CI's --locked builds refused the tree while local unlocked builds regenerated it
silently. Both member versions now match their manifests.
Co-Authored-By: Claude <noreply@anthropic.com>
@MichaelTaylor3d

Copy link
Copy Markdown
ContributorAuthor

loop-security — INTERIM STATUS, NOT A VERDICT. Do not merge on this comment.

Audited head: 3d98567551a60666191e1193c3eda9612353db4a (resolved from gh pr view 338 --json headRefOid, not from the dispatch prompt).

Posting mid-audit so the record survives. The verdict follows as a separate comment plus inline threads.

Confirmed so far (blob 2 recoverability)

  • crates/dig-wallet/src/seed_export.rs and crates/dig-node-service/src/seed_export_cli.rs are byte-identical between the merge-base 7b4caab and this headgit diff base..head on both paths is empty. The rescue path is pre-existing shipped code (landed in feat(custody)!: carve node-side USER custody out of dig-node #326), not something this PR introduces or perturbs.
  • crates/dig-wallet/src/seed_store.rs:81decrypt_seed is untouched by this PR. The only change in that file is encrypt_seed gaining #[cfg(test)]. The read path is intact; only the write path was gated.
  • The legacy branch at seed_store.rs:87-91 is EncryptedSeed::from_bytes(bytes) then legacy_decrypt(&enc, password). It is self-contained: it consults no wallet.meta.json sidecar and no device key. That is exactly blob 2's shape.
  • crates/dig-node-service/tests/seed_export_rescue.rs (new in this PR) builds its fixture with digstore_chain::seed::encrypt_seed — the actual legacy writer, pulled in as a dev-dep for this purpose — asserts bytes[0] == 1, asserts !home.join("wallet.meta.json").exists(), writes no device key, places it under a foreign base, and drives the real dign binary as a process. Fixture shape matches blob 2 on all four axes.
  • CLI reachability: entrypoint.rs:377 defines the verb, :677 routes it before control_cli, :787 returns None from the control mapping so it can never become a network method. --path is a real override (seed_export_cli.rs:51), and --json is refused at :46-49before the file is read.

A correction to my own method, recorded because it nearly produced a false clean

My first sweep for surviving /api/* routes returned zero hits tree-wide and I almost banked that. It was vacuous: MSYS rewrites any git grep pattern beginning with /, so "/api" was passed to git as C:/Program Files/Git/api and matched nothing. A filter that matches nothing reports clean.

Re-run with the leading slash removed, the true picture is the good one anyway — the router at lib.rs:1038-1057 registers none of the ten removed routes, and the residual textual hits are absence-guard tests (lib.rs:1468-1480, :1929-1933, :2001) plus a historical note in ui.html:634. Flagging it because any reviewer repeating a leading-slash grep on this diff will get a clean answer for the wrong reason.

Still open at the time of writing

  • Running seed_export_rescue and reading the passed-count (build in progress).
  • Remainder of the §908 enumeration: local BLS sign call sites, WalletSigner constructibility, with_signer production callers.
  • autoseed intactness and its retained caller.
  • The export_class_methods_are_absent_from_the_catalogue load-bearingness check and a sweep for sibling masked guards.
  • The My Stores honest-unavailable state.

The rescue test has now RUN (added after the above was drafted)

Cut my own detached worktree at C:\tmp\worktrees\sec338audit on 3d98567 — I did not use the lane's dn-327 worktree and I did not touch the shared submodule checkout, which currently holds another lane's staged work.

Finished `test` profile [unoptimized + debuginfo] target(s) in 4m 36s
Running tests\seed_export_rescue.rs (target\debug\deps\seed_export_rescue-f9dc4c2e8d9c61a8.exe)
test a_legacy_seed_under_a_foreign_base_is_still_recoverable_through_the_cli ... ok
test a_wrong_password_is_refused_and_prints_no_phrase ... ok
test result: ok. 2 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 8.39s

2 passed, 0 filtered out — the filter matched real tests rather than silently selecting nothing, and dig-wallet v0.32.0 was compiled from this worktree, so the run is against this head and not a stale artifact.

@MichaelTaylor3d

Copy link
Copy Markdown
ContributorAuthor

loop-security: PASS

Head audited: 3d98567551a60666191e1193c3eda9612353db4a (resolved from gh pr view 338 --json headRefOid; re-confirmed unchanged at the end of the audit).

No gating security defect. Two non-gating findings and one correction to the lane's report are named at the bottom. I have deliberately filed no inline threads, because an unresolved thread would deadlock a PASS.


1. THE GATING QUESTION: is blob 2 recoverable after this removal?

YES — and this PR makes it MORE recoverable than the tree it replaces, not less. I accept the lane's reasoning, on stronger evidence than the lane gave.

The test result, by passed-count

Cut my own detached worktree at C:\tmp\worktrees\sec338audit on 3d98567. I did not use the lane's dn-327 worktree and did not touch the shared submodule checkout (which currently holds a different lane's staged work).

Running tests\seed_export_rescue.rs (target\debug\deps\seed_export_rescue-f9dc4c2e8d9c61a8.exe)
test a_legacy_seed_under_a_foreign_base_is_still_recoverable_through_the_cli ... ok
test a_wrong_password_is_refused_and_prints_no_phrase ... ok
test result: ok. 2 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 8.39s
running 27 tests
test seed_export::tests::legacy_seed_file_exports ... ok
test seed_export::tests::explicit_path_reaches_a_non_default_location ... ok
test seed_export::tests::export_leaves_the_file_byte_identical ... ok
test seed_export::tests::current_format_seed_file_also_exports ... ok
test seed_export::tests::wrong_password_fails_without_leaking ... ok
test seed_store::tests::legacy_digstore_chain_seed_file_still_decrypts ... ok
test result: ok. 27 passed; 0 failed; 0 ignored; 0 measured; 580 filtered out; finished in 10.55s

0 filtered out on the first and a real 27 on the second: the filters selected actual tests rather than nothing, and dig-wallet v0.32.0 was compiled from this worktree, so both runs are against this head.

Does the fixture genuinely match blob 2's shape?

Yes, on all four axes, and tests/seed_export_rescue.rs asserts three of them in-test rather than assuming them:

blob 2 propertyfixture
legacy/EncryptedSeed(v1)built by the actual legacy writerdigstore_chain::seed::encrypt_seed (:49), with assert_eq!(bytes[0], 1) at :51-55
no sidecarassert!(!home.join("wallet.meta.json").exists()) at :57
no device keynone written; the legacy branch never consults one
base this build cannot resolvedir/some-other-home/DigWallet/seed.bin at :45, reachable only via --path

The legacy read path at seed_store.rs:87-91 is EncryptedSeed::from_bytes(bytes) then legacy_decrypt(&enc, password)self-contained, consulting no sidecar and no device key. That is exactly why blob 2's missing sidecar and missing device key are irrelevant to its recovery.

The test drives env!("CARGO_BIN_EXE_dign") as a process, not the library function, so argument routing and the password read are covered too. export_mnemonic being correct would otherwise say nothing about whether any shipped command reaches it.

Does this PR remove anything that makes blob 2 LESS recoverable?

No — the opposite. Every route out of that blob, before and after:

base 7b4caabhead 3d98567
POST /api/exportpresent, but reads std::fs::read(seed_path()) with no path overrideremoved
dign wallet export-seed [--path]already present (shipped in #326, untouched here)present

The decisive fact: the removed /api/export handler read seed_path() and nothing else. seed_path() (lib.rs:161-166) is LOCALAPPDATA first, falling back to HOME. On the Windows host where blob 2 was measured, LOCALAPPDATA is set, so the resolver returns blob 1's path and can never return blob 2's. The removed route would have answered blob 2's owner 404 "no wallet on this device".

So blob 2 was unreachable through the surface this PR deletes, and is reachable through the CLI verb that survives, because --path is capability the deleted route never had. Removing the route strands nobody.

Reachability and discoverability of the CLI verb:

  • entrypoint.rs:377 defines ExportSeed { path }; :677 routes it locally beforecontrol_cli; :787 returns None from the control mapping so it can never become a network method. Platform-independent: a plain local command with no OS-conditional compilation.
  • Discoverable via dign wallet --help (the clap doc-comment at :368-376 is user-facing prose).
  • --json is refused at seed_export_cli.rs:46-49before the file is read, and json_is_refused_before_the_file_is_read pins the ORDER by using an absent path.

Is the blob file itself untouched on disk?

Yes. Verified four ways:

  • seed_store::encrypt_seed is now #[cfg(test)] (seed_store.rs:68), so no production code in the crate can seal a user seed at all — a new production caller is a compile error, not a policy.
  • The only fs::write/remove_file touching a seed anywhere in the head tree are inside autoseed.rs (byte-identical to base, see section 3) and test fixtures. autoseed.rs:537's remove_file is a rollback of a file the same call just created with create_new(true) — it can only delete what it made.
  • Blob 2 cannot be disposed: is_disposable (autoseed.rs:341-342) is read_meta(&paths.meta).is_some_and(...), which fails closed to false when there is no sidecar. Blob 2 has no sidecar. Pinned by latching_a_wallet_with_no_sidecar_records_it_as_not_disposable, which passed.
  • Both the unit test export_leaves_the_file_byte_identical and the integration test's byte-comparison at :88-92 assert the file is unchanged after a successful AND a failed export.

2. §908 — verified structural, not policed

Independently re-derived on this head, including what an enumeration misses.

  • Every removed route is genuinely gone from the router.lib.rs:1038-1057 registers only /, /wc-bundle.js, /settings, /api/status, /api/dig-config, /api/dig-cache/{clear,list,remove,fetch}, /api/wc/{project-id,request,delegate/next,delegate/result,connections,approve,reject,revoke}. None of the ten removed routes appears. Residual textual hits are absence-guard tests (lib.rs:1468-1480, :1929-1933, :2001) and a historical note in ui.html:634.
  • WalletSource survives only as a comment at lib.rs:1581. Zero declarations, zero uses.
  • No unlock session. Base status was async fn status(State(st)) reading st.session.lock().await; head status() (lib.rs:232) takes no state at all. Every remaining session: field tree-wide is peer-mux or chain-sync (dig-node-core/src/peer.rs, sage/sync.rs, sage/sync_supervisor.rs), none of it wallet custody. sage/rpc.rs:620SessionIdentity is explicitly "Scoping data only — no key": a fingerprint plus PUBLIC puzzle hashes.
  • No local signing in lib.rs. Grepping SecretKey|master_to_wallet|mnemonic_to_seed|AugSchemeMPL|sign_message|Mnemonic over crates/dig-wallet/src/lib.rs returns 10 hits, every one a method-NAME string literal in deny-lists and tests ("exportMnemonic", "getSecretKeys", ...). Not one is a type, a call, or an import.
  • decrypt_seed production callers are exactly two concerns: autoseed.rs:326,449,726 (the node's own device-keyed operator identity) and seed_export.rs:104 (the rescue). Nothing in the second plane opens a user seed.
  • The thing an enumeration misses — I chased it.WalletSigner still exists in crate::sage, so I traced whether it can be attached in production. #[cfg(test)] mod tests begins at sage/rpc.rs:4724, and everywith_signer call site — 5641, 5679, 7332, 8237, 8291, 8373, 8450, 8790, 9367 — is above that line, i.e. inside the test module. The production backend is built at sage/service.rs:252-259 and never calls .with_signer(...), so signer: None in production. The PR's claim that §908 now holds on both planes is therefore accurate.
  • sage/service.rs:257.with_node_custodied_spending(cfg.enable_live_broadcast) is production, but it gates broadcasting an already-signed bundle (rpc.rs:1958), not signing. With signer: None, nothing local can produce a signature. Pre-existing and untouched by this diff.
  • wc_dispatch answers nothing keyed locally.lib.rs:630+: only chip0002_chainId returns "mainnet", chip0002_connect returns true, and chip0002_getMethods returns the catalogue. All three are keyless. Every other method reaches delegate_to_sage. There is no local-signer branch to fall through to.
  • dig-node-service imports nothing from the removed plane. Its full dig_wallet:: surface is autoseed, seed_export, and sage::{arrivals, events, fallback, network, routing, rpc, service, sync, sync_supervisor, transport, types, watchlist}.

3. autoseed survives, for the right reason

  • Byte-identical between merge-base and head: git diff 7b4caab..3d98567 -- crates/dig-wallet/src/autoseed.rs is empty, 1180 lines on both sides. The lane's "module intact at 1180 lines" is exact.
  • Still declared pub mod autoseed; at lib.rs:73.
  • Its production caller is retained: dig-node-service/src/wallet_bootstrap.rs:21,27,37 (autoseed::ensure_wallet). lib.rs:206 additionally uses autoseed::presence.
  • Its tests contributed to the 27 passed / 0 failed above, including secrets_are_created_with_an_explicit_owner_only_dacl, the_funded_latch_ends_disposability_permanently and a_seed_that_does_not_decrypt_is_left_untouched.
  • The SPEC sentence is accurate. §16.4 now reads "This seed is the node's OWN machine identity (DIGOP1/DIGVK1, sealed under a device key), NOT a user wallet, and §18.20 does not retire it", which matches the code: autoseed seals through dig_keystore::opaque under a device key, and is the sole surviving seed_store consumer besides the rescue.

4. The vacuous guard — fix verified by probe, siblings swept

The new test is genuinely load-bearing. I proved it rather than accepting the report. In my own worktree I inserted "chip0002_export" into WC_METHOD_CATALOGUE and re-ran both tests:

thread 'tests::export_class_methods_are_absent_from_the_catalogue' panicked at crates\dig-wallet\src\lib.rs:1694:13:
chip0002_export is advertised as dispatchable AND is export-class
test tests::export_class_methods_are_absent_from_the_catalogue ... FAILED
test tests::export_is_not_a_dispatchable_wc_method ... ok
test result: FAILED. 1 passed; 1 failed; 0 ignored; 0 measured; 605 filtered out

This reproduces the exact blind spot: the behavioural test still passes, because the export-class guard ordered first refuses it, while the new structural test is the only thing that fails. That is the definition of load-bearing, and it confirms the lane's claim precisely.

File restored by copy, not git checkout — sha256 back to 1dea33569aaed5266a59f3f95e7cd870b6435a734d9ec2e18cb08aebf78dfa67, git status --porcelain empty, HEAD still 3d98567.

Sibling sweep for similarly-masked guards on the changed surface: the only other candidate pair is wc_gate (lib.rs:636-655) versus the catalogue check. It is not masked — wc_gate returns Forbidden (403) for a key/sign method from an unapproved origin, whereas the catalogue check returns 501: different codes on different inputs. unsupported() is shared only between the two 501 paths, so export spellings cannot be probed apart from unknown methods. That sharing is intentional and documented, not an accident.


5. Secrets, logging, and the capability regression

  • Zero logging statements added anywhere in the diff — grepping every + line for println|eprintln|tracing::|info!|debug!|warn!|error!|trace!|dbg!|log:: returns 0.
  • No new Debug/Display impl on a secret-carrying type; the only derive added in the whole diff is #[derive(Default)].
  • ExportError (seed_export.rs:45-77) carries only a path and a failure class. wrong_password_fails_without_leaking checks the rendered error against every WORD of the phrase, not just the whole phrase — a stronger assertion than the obvious one, and it passed.
  • The mnemonic goes to stdout while all guidance goes to stderr, and --json is refused, so the phrase is never emitted in a shape built for redirection.
  • My Stores is honest and does not trap.loadStores/loadHomeStores render STORES_UNAVAILABLE, which says listing needed a wallet on this device and points at hub.dig.net. It explicitly refuses to render an empty list, on the stated grounds that an empty list would assert the user owns nothing — a different and false claim. That is the right call. The connect gate keeps a genuine escape ("Just reading DIG content? You don't need a wallet at all" with a dig:// link), so no user is trapped.
  • Balance honesty holds: an unanswered balance renders unknown / an em dash, never 0, with the reasoning stated in-comment that an empty wallet and an unreachable one otherwise render identically. This is the money-lie class the contract does not defer, and it is handled correctly.
  • Cargo.lock correctly carries both bumps (dig-node-service 0.147.0, dig-wallet 0.32.0) plus the new digstore-chain dev-dep, verified with git diff base..head -- Cargo.lock.

Non-gating findings (recommend follow-up tickets; do NOT hold this merge)

F1 — MEDIUM, defense-in-depth: the residual-seed banner cannot fire for the one real user-origin blob.wallet_exists() (lib.rs:204-209) probes only autoseed::presence(&seed_path()), and seed_path() (:161-166) prefers LOCALAPPDATA over HOME. On the measured Windows host it therefore resolves blob 1 and can never see blob 2 under $HOME. So /api/status reports "delegated", and both surfaces that name the rescue command stay hidden — ui.html:1314 and settings.html:422 each gate on state === 'custodied', and the entire residualSeed card containing the copyable dign wallet export-seed is class="card hidden" (settings.html:143). Meanwhile the permanently-visible copy at settings.html:137-139 asserts "This browser holds no wallet keys ... there is nothing here to back up, unlock, or lose", which is not true for that user.

Why this does not gate: it is not a regression. At base, wallet_exists() used the same resolver, and the always-visible export control would have called /api/export and hit std::fs::read(seed_path()) for a 404 "no wallet on this device". Blob 2's owner learned nothing then and learns nothing now — but can now actually recover the file via --path, which was impossible before. The PR improves the outcome and leaves a pre-existing discovery gap unsolved.

Suggested fix for the follow-up: have the presence probe check both candidate bases (%LOCALAPPDATA% and $HOME) and, when a seed is found off the default path, surface the resolved path in the banner so the user is handed the exact --path argument.

F2 — LOW, hygiene:tests/seed_export_rescue.rs:38 uses a literal const PASSWORD: &str = "the-users-own-password";. The neighbouring seed_export.rs:115-125 in this same PR deliberately derives its test password from a label precisely so that "a password-shaped literal [stays] out of the source, which static analysis cannot tell apart from a real credential." The new file does not follow the convention its sibling establishes. No exposure — it seals the public abandon...art BIP-39 vector — but it is the kind of literal a secret scanner flags.

F3 — accuracy correction to the lane's report (not a defect): the dispatch brief states dig-node-service "consumes only dig_wallet::sage::rpc::* plus seed_export/autoseed". It also imports sage::{arrivals, events, fallback, network, routing, service, sync, sync_supervisor, transport, types, watchlist}. The material claim — that nothing from the removed plane has an external consumer — is correct and I verified it; the enumeration behind it was merely incomplete.


Methodology note worth carrying forward

My first sweep for surviving /api/* routes returned zero hits tree-wide, and I nearly banked it as clean. It was vacuous: MSYS rewrites any git grep pattern beginning with /, so "/api" reached git as C:/Program Files/Git/api and matched nothing. Re-run without the leading slash the answer happened to be favourable anyway — but any reviewer repeating a leading-slash grep on this diff gets a clean result for the wrong reason. Drop the leading slash ("api/export", not "/api/export").

Shared-state disclosure

I mutated nothing shared. No git checkout, reset, stash or any mutating git command in the shared submodule checkout at modules/apps/dig-node (which holds another lane's staged work), and I never entered the lane's dn-327 worktree. All reading was via git show / git diff / git cat-file. My probe ran in my own detached worktree C:\tmp\worktrees\sec338audit, was reverted by file copy, verified by sha256, and that worktree has been removed.

Verdict: PASS. The removal is safe: blob 2 is recoverable and measurably more so than before, the file is untouched, §908 is structural on both planes, autoseed is byte-identical and correctly justified, and the guard fix is genuinely load-bearing.

@MichaelTaylor3d

Copy link
Copy Markdown
ContributorAuthor

Correctness gate — part 1/5: by-name audit of deleted tests (head 3d98567)

Method: enumerated every #[test]/#[tokio::test] fn name in each touched file at base 7b4caab and at head, then for each deleted name checked whether the production symbol it exercised still exists at head.

Counts:crates/dig-wallet/src/lib.rs 71 tests at base -> 43 at head (36 deleted, 8 added). dig-runtime/src/lib.rs 14 -> 14, seed_store.rs 6 -> 6, sage/custody.rs 0 -> 0 (no test deltas in any of those three). New file crates/dig-node-service/tests/seed_export_rescue.rs adds 2.

A. Deleted with their subject — property genuinely gone (22). Each of these exercised a local spend-building/parsing/serialising helper that is now absent from head (verified: 0 occurrences of the symbol at head, vs base counts in parens):

asset_id_parses_with_or_without_0x_and_rejects_bad_len (parse_asset_id, 20->0), cat_asset_id_defaults_to_dig_and_accepts_any_tail (9->0), clawback_coin_parses_and_renders, coin_entry_json_matches_sage_spendable_coin_shape (7->0), coin_spend_hex_round_trips_from_components (5->0), find_owned_index_matches_coin_or_launcher_id (8->0), json_u64_tolerates_number_and_decimal_string (34->0), k1_pubkey_parses_and_rejects_bad_len (5->0), memo_hashes_parse_and_reject_non_hash (6->0), mint_spec_parses_metadata_royalty_and_did (6->0), offer_legs_parse_xch_and_cat (7->0), offer_summary_json_shape (7->0), option_strike_parses_xch_and_rejects_unsupported_underlyings (7->0), ph_or_address_accepts_either_form (6->0), pubkey_window_defaults_and_clamps (6->0), requested_store_id_accepts_storeid_launcherid_or_id (9->0), store_dig_amount_defaults_and_rejects_zero (7->0), tx_json_renders_direction_asset_and_amounts (5->0), vault_config_parses_members_and_threshold (7->0), vc_verify_is_pure_and_deterministic (6->0), parse_delegates_builds_admin_writer_oracle_and_rejects_unknown, broadcast_requires_both_request_and_env_optin (local broadcast path gone).

Note on clawback: the word still occurs 15x at head, but only as method names in WC_METHOD_CATALOGUE (dig_clawbackSend/Claim/Recover, dig_streamClawback) which now delegate to Sage. The parser the test covered is gone. Not a false clear.

B. Deleted because the surface was removed, with a replacement or negative guard (4).

  • export_requires_self_origin_and_correct_password/api/export removed; replaced by seed_export_rescue.rs::a_legacy_seed_under_a_foreign_base_is_still_recoverable_through_the_cli + ..::a_wrong_password_is_refused_and_prints_no_phrase, and by the export-class refusal battery (export_class_methods_are_absent_from_the_catalogue, delegate_never_forwards_export_class_methods, export_is_not_a_dispatchable_wc_method, wc_dispatch_method_set_has_no_export_path, method_catalogue_matches_the_gate_and_never_leaks_export).
  • wallet_pubkey_is_self_origin_and_needs_unlock — no wallet_pubkey symbol at head; there is no node-held key to publish.
  • store_endpoints_require_an_unlocked_wallet/api/stores removed; the negation is pinned at lib.rs:1411 (assert!(!UI_HTML.contains("api('/api/stores"))), which is the right shape: it fails if the endpoint is re-wired.
  • wallet_source_round_trips_through_its_wire_token / wallet_source_set_is_self_origin_only_and_persistsWalletSource survives only as a comment (41->1); replaced by no_persisted_setting_can_route_a_sign_method_back_into_this_process, which guards the property that outlived the type.

C. The six whose behaviour SURVIVES — confirmed restored, 1:1 (6). This is the class the lane flagged, and each has a successor asserting the same property:

deleted (base)restored (head)
sage_mode_delegates_signing_methods_but_answers_the_handshake_locallythe_handshake_is_local_and_every_key_method_goes_to_sage
sage_mode_dispatch_surfaces_sage_errorsdispatch_surfaces_sage_errors
sage_mode_dispatch_routes_through_the_delegate_bridge_not_local_keysno_persisted_setting_can_route_a_sign_method_back_into_this_process
wallet_dispatch_self_origin_routes_through_to_the_signerwallet_dispatch_self_origin_routes_through_to_sage
wallet_dispatch_approved_origin_routes_throughan_approved_origins_sign_request_is_parked_for_sage_not_answered_locally
native_mode_routes_every_method_to_the_local_signerinverted, correctly: no_persisted_setting_can_route_a_sign_method_back_into_this_process
settings_page_wires_the_wallet_source_controlsettings_page_wires_the_sage_connection_and_offers_no_alternative_to_it
settings_page_wires_the_new_settings_apissettings_page_offers_no_route_to_a_key_and_says_where_the_keys_are

(8 rows for 6 flagged — the two settings-page guards are in the same class.) The incident is also recorded in-source at lib.rs:1583-1585, which is the right place for it.

Unknown-method 501: retained and covered — the shared refusal is documented at lib.rs:672-674 (export-class and unknown-method refusals are deliberately indistinguishable on the wire, so a caller cannot probe which export spellings exist), with lib.rs:1689 and 1702 explicitly reasoning about why a bare 501 assertion would be vacuous. That is the correct treatment.

Verdict on part 1: no necessary coverage was lost. Every deleted test either lost its subject or has a named successor. No blocking finding; nothing to resolve here.

@MichaelTaylor3d

Copy link
Copy Markdown
ContributorAuthor

Correctness gate — parts 2 & 3/5: doc sweep, and wc_dispatch's remaining split (head 3d98567)

2. Doc sweep — complete, with a positive control

I ran the retraction greps myself rather than reading the diff. Leading slashes dropped (MSYS rewrites /api/* into C:/Program Files/Git/api, which is how a tree-wide sweep goes vacuous). Positive control first:git grep -c custody 3d98567 -- SPEC.md -> 35, so the grep is reaching the tree.

Retracted phrasing, at head, across *.md*.rs*.html:

phrasefiles at head
NOT yet whole / not yet whole0
not yet on that one0
second custody0
native mode0
node-held seed0
WalletSource1 — lib.rs:1581, a comment about the removal
seed at rest1 — lib.rs:9, "It used to...", retrospective
api/export4 — all negations (SPEC.md:4127 "was removed", SPEC.md:5458 in the removed list, lib.rs:1476/2001 absence assertions, ui.html:634 "this UI once called")
local signer3 files — SPEC.md:3669/4115 and lib.rs all negative ("no local signer", "went with the local signer it gated")

No surviving sentence asserts the old rule. The sweep is a sweep, not an edit.

One non-gating nit, resolving myself:crates/dig-wallet/wc/entry.js:86 and wc/requester.js:147 still say "forward to the local signer". Both are untouched by this PR and describe the browser-side WC bridge posting to the loopback responder — which now forwards to Sage — so the phrase is imprecise rather than false, and neither file is normative. Not worth a round trip; noting it so a future reader does not mistake it for surviving custody.

Does the new §18.20 over-claim? No.

Every claim it makes is structural and independently checkable, and I checked the four load-bearing ones at head:

  • "The process holds no unlocked-session state — the field is gone from AppState"lib.rs:110-114, AppState is exactly { approvals, delegate }. True.
  • "seed_store::encrypt_seed is #[cfg(test)], so a production caller ... would not compile"seed_store.rs:68. True.
  • "wc_dispatch answers only the keyless handshake methods and forwards every other method" — confirmed below. True.
  • "GET /api/status reports custodied while such a file exists, delegated otherwise"lib.rs:213-237. True.

It is also careful in the two places over-claiming would have been easy: §16.4 explicitly says §18.20 does not retire autoseed (the node's own DIGOP1 identity is a separate concern sharing an at-rest primitive), and §18.20 says the remaining WalletCustody is a read of one non-secret file. Both are the honest framing.

3. wc_dispatch — the split is right

  • Keyless local arm is exactly three:chip0002_chainId, chip0002_connect, chip0002_getMethods (lib.rs:623). Nothing keyed is answered locally; chainId is public (wallet_dispatch_chain_id_is_public_and_returns_mainnet), connect from an unapproved origin parks as 202 (wallet_dispatch_connect_from_unapproved_origin_is_pending_202).
  • Everything else forwards, gated first by the per-origin consent gate: unapproved + sign method -> 403 (wallet_dispatch_gates_unapproved_origin_for_sign_methods), approved -> parked for Sage (an_approved_origins_sign_request_is_parked_for_sage_not_answered_locally), self-origin -> wallet_dispatch_self_origin_routes_through_to_sage. Sage's own errors surface rather than being swallowed (dispatch_surfaces_sage_errors).
  • Unknown methods 501, and deliberately indistinguishable from the export-class refusal (lib.rs:672-674) so the set of export spellings cannot be probed. The export-class check at lib.rs:658 is documented as currently unreachable (lib.rs:649-655) and retained as defence-in-depth for a future catalogue addition — with export_class_methods_are_absent_from_the_catalogue pinning the reason it is unreachable. That is the correct pairing: the guard and the fact that makes it dormant are each pinned separately, so neither can drift silently.
  • The tests are also honest about their own limits — lib.rs:1625, 1689, 1702 and 1864 each explain why asserting only a 501 (or only an error) would be satisfied by the wrong implementation, and assert the pump instead. That is the property-not-outcome shape.

No blocking finding in parts 2 or 3.

@MichaelTaylor3d

Copy link
Copy Markdown
ContributorAuthor

Correctness gate: PASS

Head reviewed: 3d985675 1a60666191e1193c3eda9612353db4a — resolved from the remote myself (gh pr view 338 --json headRefOid), not from the dispatch. DRAFT, 12 files, +894 / -4263. Security leg's PASS not duplicated.

Parts 1-3 are posted above (deleted-test audit; doc sweep + §18.20 over-claim check; wc_dispatch split). Parts 4-5 below.

4. The My Stores regression — honest, remedied, does not trap

ui.html:986-990. Both surfaces render one string:

"Listing your stores needed a wallet on this device, and DIG no longer keeps one. Manage and publish your stores at hub.dig.net."

Against the professional-ui bar:

  • Honest — it states the cause and does not imply a fact it lacks. The in-source comment gets the important part right: "inventing an empty one would assert that the user owns nothing — a different and false claim." An empty list here would have been the false-green shape (an outcome that the wrong implementation produces identically); this is the property.
  • Names a remedy — a working link to hub.dig.net, which is where the capability actually lives now.
  • Does not trap — it is an inline empty state in a card, not a blocking element. Nothing else on the page is gated on it.

Minor, resolving myself: the anchor has no target/rel="noopener". It is a loopback page navigating to a first-party origin; not worth a round trip.

On the follow-up ticket: yes, file one — re-sourcing My Stores from Sage (or from the node's own non-secret wallets/index.json public keys, which §18.20 confirms survive as a read) is a real capability the user had and no longer has. It is kind:business under §1.3c rule 2 (user-visible on a shipped surface), so it clears the bar for an issue rather than a comment. It is not a blocker for this PR: the current state is honest, which is the standard this contract sets for a deliberate regression.

5. Versions, lock, CI — confirmed by name and by count

  • Workspace 0.146.0 -> 0.147.0, dig-wallet0.31.0 -> 0.32.0. Correct as minor: a removed HTTP surface and removed public API is breaking in the ordinary sense, but pre-1.0 0.x minor is the breaking slot, and Check version increment is green.
  • Cargo.lock at head is fresh, not stale. Compared the two committed blobs (git diff 7b4caab 3d98567 -- Cargo.lock), not the working file: both bumps are present (dig-node-service 0.146.0->0.147.0, dig-wallet 0.31.0->0.32.0) plus the new digstore-chain dev-dep the rescue test needs. Zero 0.146 residue anywhere in the lock. The earlier trap is resolved.
  • SPEC.md is complete, not truncated — 6,725 lines at head vs 6,702 at base (+23, consistent with the +45/-22 diff), and the tail ends on the intended final block. The spec.md clobber is fully recovered.
  • Required contexts, asserted by name against branch protection (Lint commit messages, Check version increment, Rustfmt, Clippy, Test + coverage): all five present and SUCCESS. None absent, none action_required. CodeQL (3 analyses) also green.
  • Read from the run log, not the ok:Summary [839.422s] 2271 tests run: 2271 passed (18 slow), 1 skipped. Coverage TOTAL ... 89.45% lines / 86.02% functions / 90.38% regions, against an 80% floor.

On the flaky test

sage::sync_supervisor::tests::stall_evidence_survives_the_end_of_a_session — 123s vs a usual 14s under CPU contention, masked by --retries 2, untouched by this diff. Worth a ticket, not a blocker. A retry-masked timing test is a gate that has partially rotted: it will keep reporting green while degrading, and the day it fails it will be attributed to contention rather than to a regression. kind:maintenance, low band. Not this PR's to fix.

Blocking findings: none

No inline threads opened, deliberately — an unresolved thread bars merge under required_conversation_resolution, and nothing here rises to a blocker. The two nits above (wc/*.js "local signer" phrasing, the anchor rel) and the two follow-ups (My Stores re-sourcing, the flaky test) are recorded here and resolved by me.

Merge is unblocked from the correctness leg once the PR is undrafted. I am not merging and not undrafting.

@MichaelTaylor3d
MichaelTaylor3d marked this pull request as ready for review August 24, 2026 15:46
@MichaelTaylor3d
MichaelTaylor3d merged commit 251b4c1 into mainAug 24, 2026
15 checks passed
@MichaelTaylor3d
MichaelTaylor3d deleted the loop/327-remove-second-custody-plane branch August 24, 2026 15:46
Sign up for freeto 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.

fix(custody)!: a second node-side USER custody + signing surface survives in dig-wallet's self-origin UI

1 participant

@MichaelTaylor3d