fix(wallet): say when a wallet-isolating override was not honoured - #489
Conversation
Salvage anchor so a cap is never lossy. No production change yet. Refs #392 Co-Authored-By: Claude <noreply@anthropic.com>
Two resolvers decide "the per-user base directory" in one dig-node process and disagree under a LOCALAPPDATA override: dig_wallet's is env-first and owns the seed/meta/device key, while dig-node-core's asks the OS known-folder API first and owns cache/config and therefore wallet.sqlite. The node came up with a newly minted seed under one root and a coin replica under the other, saying only that it had minted a wallet. Resolution is deliberately unchanged -- deriving the wallet base from the node's cache dir would move the seed off every existing service install and mint a fresh wallet there. Instead the split is announced, and the one irreversible consequence (minting into a split layout) is refused. Also announces an inert DIG_WALLET_PORT: dig-node never starts a wallet host, so nothing binds the port an operator set. Co-Authored-By: Claude <noreply@anthropic.com>
The replica is a SIBLING of the config file, not a child of the cache dir, so an operator who sets DIG_NODE_CACHE and then looks for wallet.sqlite inside that directory concludes the second lever failed too. Name the resolved file. Co-Authored-By: Claude <noreply@anthropic.com>
…47.0 Minor on both libraries: each gains an additive public fn (platform_user_base, autoseed::user_base) with no behaviour change. Co-Authored-By: Claude <noreply@anthropic.com>
|
Implemented to the locked design at
Tests: |
MichaelTaylor3d
left a comment
MichaelTaylor3d
left a comment
There was a problem hiding this comment.
IN PROGRESS — not the verdict. Independent correctness gate, fresh context, reading head 2b22c7aa7bed9fe25e1be4df2d4f3fc27eb928fc. Two findings established so far; posting them now rather than holding them in context.
GATING 1 — the refusal names a remedy that cannot clear the refusal
REFUSED_SPLIT_MINT (crates/dig-node-service/src/wallet_env.rs:74-79) tells the operator:
Set DIG_NODE_CACHE as well as LOCALAPPDATA so both halves land in one place ... then start the node again.
DIG_NODE_CACHE cannot clear the split. wallet_root_split() (wallet_env.rs:104-109) compares dig_wallet::autoseed::user_base() against dig_node_core::platform_user_base(). DIG_NODE_CACHE is read in canonical_cache_dir() (crates/dig-node-core/src/lib.rs:606-612), which returns before platform_user_base() is ever called — so it changes the cache dir and leaves platform_user_base() byte-identical.
Consequence: an operator who does exactly what the message says (LOCALAPPDATA=<scratch> + DIG_NODE_CACHE=<scratch>/DigNode/cache, which genuinely puts seed, cache, config and wallet.sqlite all under the scratch root) is still refused, on every subsequent start, forever. The only configuration that clears the refusal is unset LOCALAPPDATA — i.e. abandoning the isolated wallet run that #392 exists to make possible. The triggering state is durable, so one env setting becomes permanent denial with no escape.
The same wrong remedy is in SPLIT_ROOTS (wallet_env.rs:61-69) and in the SPEC edit (SPEC.md, §16, "DIG_NODE_CACHE is the variable that moves those").
A fix must not be "delete the refusal": either make the split predicate the thing the operator can actually influence (compare the wallet base against the effective node root, i.e. config_path().parent(), so DIG_NODE_CACHE clears it), or change the prose to state the only remedy that works and accept that the isolated run is unsupported — but then say that, per the ticket's third scope bullet.
GATING 2 — the refusal is reachable on a stock Linux .deb install, where it stops a wallet ever being minted
platform_user_base() (crates/dig-node-core/src/lib.rs:628-641) tries directories::BaseDirs::new() first; on Linux that resolves home_dir via dirs-sys 0.5.0, which reads $HOME and falls back to getpwuid_r (dirs-sys-0.5.0/src/lib.rs:33-47). dig_wallet::autoseed::user_base() (crates/dig-wallet/src/autoseed.rs:110-120) has no such fallback: LOCALAPPDATA → HOME → ".".
So with $HOME unset the two disagree: node base = the passwd home (e.g. /root), wallet base = ".". Verified the passwd fallback returns a real path with HOME unset on this machine's WSL (env -u HOME getent passwd $(id -u) → /home/micha), so BaseDirs::new() is Some there, not None — the "both collapse to ." reading is wrong.
The packaged unit packaging/linux/systemd/net.dignetwork.dig-node.service runs ExecStart=/usr/bin/dig-node run as root with no User= and no Environment=HOME=.... systemd does not set $HOME for a system service without User=. If that holds, every stock .deb install hits wallet_root_split() == Some and, on a fresh install with no seed, ensure_wallet_seed() now returns None and mints nothing — where today it mints. The operator is then handed a message naming two Windows-only environment variables.
I have not booted a .deb under systemd to confirm $HOME is unset there, and I am not treating my reasoning as sufficient for a merge: the burden is on the PR, because the refusal is a new denial path and the change ships no test, no doc reasoning and no #[cfg(unix)] coverage for the non-Windows shape at all. Please measure it (systemctl show -p Environment / strings /proc/<pid>/environ on a real unit) and make the split predicate not fire when the disagreement is merely "one resolver has a passwd fallback and the other does not".
Remaining items (announce side effects, test vacuity, once-guard, SPEC, extraction verbatim) still in progress; verdict to follow.
loop-security — IN PROGRESS, not the verdictAuditing head Posting findings as they resolve so none is lost to a watchdog. Four of the seven brief items have resolved CLEAN so far. Item 6 —
|
MichaelTaylor3d
left a comment
MichaelTaylor3d
left a comment
There was a problem hiding this comment.
CHANGES-REQUIRED
Head read: 2b22c7aa7bed9fe25e1be4df2d4f3fc27eb928fc (resolved from the remote; unchanged during the review). Base origin/main = 1e94c7f. Independent correctness gate, fresh context. gitnexus was NOT used (its index for this repo is stale and returns a false-safe impactedCount: 0); everything below is grep + direct read of the worktree at that SHA, plus test runs.
The diagnosis in this PR is right, the announce-don't-unify decision is right, and the extraction is clean. Two findings block it, both on the new refusal path — the refusal is a new way to prevent a wallet existing, and neither its escape hatch nor its non-Windows reachability holds up.
GATING 1 — the refusal names a remedy that cannot clear it (wallet_env.rs:74-79)
REFUSED_SPLIT_MINT tells the operator to "Set DIG_NODE_CACHE as well as LOCALAPPDATA so both halves land in one place ... then start the node again." SPLIT_ROOTS (wallet_env.rs:66-68) and the SPEC edit say the same.
DIG_NODE_CACHE cannot clear the split. wallet_root_split() (wallet_env.rs:104-109) compares dig_wallet::autoseed::user_base() against dig_node_core::platform_user_base(). DIG_NODE_CACHE is read in canonical_cache_dir() (crates/dig-node-core/src/lib.rs:606-612) and returns before platform_user_base() is called (lib.rs:628) — so it moves the cache and leaves the split predicate byte-identical.
An operator who does exactly what the message says — LOCALAPPDATA=<scratch> plus DIG_NODE_CACHE=<scratch>/DigNode/cache, which genuinely puts seed, cache, config.json and wallet.sqlite all under the scratch root — is refused anyway, on that start and every subsequent one. The only configuration that clears the refusal is unsetting LOCALAPPDATA, i.e. abandoning the isolated wallet run that #392 exists to enable. The triggering state is durable, so one env var becomes permanent denial with no working escape. The test mint_is_refused_only_when_a_split_would_create_a_new_wallet cannot see this: it feeds the pure core a synthetic split and never asks what an operator could do to make the split go away.
The fix must not be to delete the refusal. Two shapes that would work: make the predicate compare against the effective node root (dig_node_core::config_path().parent(), which DIG_NODE_CACHE does move), so the documented remedy actually clears it; or keep the predicate and rewrite both constants to state the only remedy that works, settling the ticket's third scope bullet explicitly. Whichever is chosen, the constants, the SPEC paragraph, and a test asserting "the documented remedy clears the refusal" must all move together.
GATING 2 — the refusal is reachable on a stock Linux .deb, where it stops any wallet being minted
platform_user_base() (crates/dig-node-core/src/lib.rs:628-641) tries directories::BaseDirs::new() first; on Linux that resolves home_dir through dirs-sys 0.5.0, which reads $HOME and falls back to getpwuid_r (dirs-sys-0.5.0/src/lib.rs:33-47). dig_wallet::autoseed::user_base() (crates/dig-wallet/src/autoseed.rs:110-120) has no such fallback: LOCALAPPDATA -> HOME -> ".".
So with $HOME unset the two roots differ — node base = the passwd home (/root), wallet base = "." — and wallet_root_split() returns Some. I checked the "both collapse to ." reading and it is wrong: with HOME unset, getent passwd for the current uid on this machine's WSL returns a real path, which is exactly what BaseDirs::new() uses, so it is Some, not None.
packaging/linux/systemd/net.dignetwork.dig-node.service runs ExecStart=/usr/bin/dig-node run as root with no User= and sets no HOME; systemd does not set $HOME for a system service without User=. If that holds on a real unit, every stock .deb install with no seed now gets ensure_wallet_seed() returning None and mints nothing where it previously minted, and is handed a message naming two variables that mean nothing on Linux.
I did not boot a .deb under systemd, so I am not asserting the regression as measured — but the burden is on the PR, and it carries no #[cfg(unix)] coverage, no Linux reasoning and no test for the non-Windows shape of a change that adds a denial path. Please measure HOME on a real unit, and make the predicate not fire when the only disagreement is "one resolver has a passwd fallback and the other does not" (equivalently: give autoseed::user_base the same last-resort, or compare canonicalised roots).
Related, and it is your canary: the final assertion of an_overridden_localappdata_is_reported_as_a_split (wallet_env.rs:280-284, "restoring the variable removes the split") fails on any host where HOME is unset, because the restore path does remove_var and the two resolvers then disagree. A CI container without HOME turns this test red — for exactly the reason above.
Non-gating (do not let these block; noted for the record)
announce_from_envacquires a disk side effect (wallet_env.rs:143-145intodig_node_core::config_path()intoresolve_cache_dir()intodir_is_writable(),lib.rs:671-685): itcreate_dir_alls the cache dir and writes+removes a probe file, now at the very top ofblock_on_serve. I judge this acceptable: the directory is the one this process uses regardless, it is created moments later by config load, and under a split it probes the real user cache dir — which is correct, since that is genuinely where the replica opens. Worth one sentence in the doc-comment, because "a warning function that creates directories" is surprising on sight.- No wiring guard for the announcement.
tests/wallet_bootstrap_wiring.rsexists precisely because "a correct function nobody calls mints nothing", and pinsensure_wallet_seed()in bothblock_on_serveandwin_service.rs. The newannounce_from_env()call has the identical failure mode and no such guard. Two more asserts in that file, with the samebetween(...)helper. - SPEC 16 inherits GATING 1's wrong remedy (DIG_NODE_CACHE named as the variable that moves those, plus "REFUSES to mint") and must be corrected in the same unit.
Items checked and found clean
- Ticket scope. All three bullets are met by code, not prose: bullet 1 by the two warnings reachable from both serve entrypoints (
entrypoint.rs:1415inblock_on_serve, andwin_service.rs:128, which does not go through it — verified by reading both); bullet 2 was already shipped in #458; bullet 3 by the SPEC paragraph stating the shape and the refusal. - The
ANNOUNCEDonce-guard (wallet_env.rs:148,155) cannot suppress a legitimate second announcement (the two call sites are mutually exclusive per process) and introduces no test-order dependence: nothing in the lib-test suite callsannounceorannounce_from_env, so the flag is never set during tests. seed_present(wallet_bootstrap.rs:63-65) takes unknown-means-present, matchingdig_wallet::wallet_exists(crates/dig-wallet/src/lib.rs:209-214) exactly. The inverse is safe: an erroringpresence()yieldsProceed, andensure_wallet_seed_atthen runs its own check — so a permanently-unreadable seed path cannot cause a silent never-mint through this path.- Test quality. Ran them; counts, not exit status:
cargo test -p dig-node-service --lib wallet_envgave 8 passed (748 filtered) and--lib wallet_bootstrapgave 1 passed (755 filtered) — 9 new tests, matching the claim.a_split_root_with_no_seed_mints_nothingasserts on the disk (seed, meta and device key all absent), not on the return value, as required.an_overridden_localappdata_is_reported_as_a_splitis non-vacuous and binds the real pair: a wrapper wired to two env-first resolvers gives equal roots and fails theexpect; one wired to two OS-first resolvers givesNoneand fails the same line. I established this by inspection plus the passing runs rather than by a revert-proof in a scratch worktree — a cold build here is ~9m and the verdict does not turn on it; saying so explicitly so it does not read as verified by mutation. - The extraction is verbatim.
platform_user_base()(lib.rs:628-641) preserves BaseDirs -> LOCALAPPDATA -> HOME -> "." in that order, with the samecfg!(windows)arm, andcanonical_cache_dirstill appendsDigNode/cache. No install's cache resolution moves. - Both SPEC claims are true. dig-wallet holds no key and signs nothing —
crates/dig-wallet/src/lib.rs:7-18says so and #327 removed the signer, so deleting "with native BLS signing" corrects text that had been false since then.DIG_WALLET_PORThas exactly one read site,crates/dig-wallet/src/lib.rs:148-150, reached only fromdig_wallet::run, so "inert in dig-node" is correct. Nothing else in section 16 was broken by the edit.
Nothing is resolved and nothing is merged by me; the orchestrator owns the merge.
MichaelTaylor3d
left a comment
MichaelTaylor3d
left a comment
There was a problem hiding this comment.
Two GATING findings, anchored inline so they bar merge under required_conversation_resolution. Full verdict in the review above.
loop-security: PASSAudited head: No LIVE vulnerability. Four defense-in-depth findings and one born-false doc claim, all non-gating — recommend follow-up tickets, do not hold the merge. Method: grep + direct read throughout. gitnexus was NOT used — its index here is ~301 commits stale and returns a false-safe Items 4, 5, 6 and most of 2 were cleared in my interim comment above with evidence; not repeated in full here. Item 1 — denial-of-wallet reachability. CLEAN.The attacker who can set
And the refusal is fail-safe in the direction that matters. A genuine true-positive class exists, which is a point in the change's favour rather than against it: on Unix with Item 2 — log-content disclosure. CLEAN, two LOW notes.No key material can reach any new field, on any arm. The refusal logs the
Item 3 — side effect in a warning path. Real, but does NOT cross the #501 boundary. One LOW finding.The side effect is confirmed: It is nonetheless safe, for two measured reasons:
So no directory is created earlier than
Item 4 —
|
…override The refusal shipped in this branch fired on every stock Linux `.deb` install and named a remedy that did nothing, reintroducing the silent-start defect (#392) one layer up. Both halves were wrong for the same reason: the predicate asked whether the two per-user roots DIFFER, when the question it needed to ask was whether an operator made them differ. - `directories::BaseDirs` reads `$HOME` and then falls back to `getpwuid_r`, while `dig_wallet::autoseed::user_base` has no such fallback. The shipped systemd unit sets no `User=`, no `Group=` and no `HOME=`, so on every stock install the roots diverged with nobody at fault and the node came up wallet-less. - `REFUSED_SPLIT_MINT` told the operator to set `DIG_NODE_CACHE`, which neither resolver reads. On a service run the anchor has already set it and the refusal fired anyway. The predicate now refuses on the conjunction of an OVERRIDE-caused split, no seed, and no `DIG_NODE_CACHE` - so the escape it names is one it honours. Roots are compared through a pure `same_root` that ignores trailing separators and, when asked, case; case-insensitivity is a PARAMETER rather than a `cfg!(windows)` read, because CI runs on ubuntu-latest only and an internal `cfg!` would leave the Windows arm untested on every runner this repo has. Exact equality would have reported a false split, and therefore a permanent refusal to mint, on an ordinary Windows host whose environment and known-folder result differ only in spelling. An ambient divergence now warns with its own sentence, which names both resolved roots and the seed file and does NOT prescribe the inert remedy. Also states the `anchor_service_data_dirs` mechanism conditionally in the module doc - it is gated on `state::running_as_service` - and corrects SPEC 16 to the narrowed predicate. Co-Authored-By: Claude <noreply@anthropic.com>
Fix round on
|
| case | expected | result |
|---|---|---|
stock Linux .deb service (LOCALAPPDATA unset, HOME unset) |
Proceed | Proceed (Ambient) |
| ordinary Windows host / LocalSystem (env == API modulo case) | Proceed | not a split at all after normalization |
LOCALAPPDATA=<scratch>, no DIG_NODE_CACHE, no seed |
Refuse | Refuse, remedy now works |
LOCALAPPDATA + DIG_NODE_CACHE both set |
Proceed, warn only | Proceed |
container, no HOME, no LOCALAPPDATA |
Proceed, warn | Proceed (Ambient) |
The table is in the module doc. Ambiguity resolves toward Proceed.
Two messages, each with a true remedy
SPLIT_ROOTS (override) keeps its text - its remedy is now actually honoured. New
AMBIENT_SPLIT_ROOTS names both resolved roots and the seed file, says nothing is refused, and
deliberately does NOT prescribe DIG_NODE_CACHE; the_ambient_announcement_does_not_prescribe_an_inert_remedy
asserts that absence. concat! discipline and the no-run-of-4-spaces guard cover the new constant.
SPEC.md §16 corrected from "resolves the two roots differently WARNS, and REFUSES" to the narrowed
predicate, including the normalization rule and the ambient MUST-proceed clause.
Evidence
TDD, red first: red_a_stock_linux_service_shaped_split_must_proceed failed with
left: RefuseSplitRoot, right: Proceed (8 passed, 1 failed) before any fix.
| run | result |
|---|---|
cargo test -p dig-node-service --lib wallet |
55 passed, 0 failed |
cargo test -p dig-node-service --test wallet_bootstrap_wiring |
5 passed, 0 failed |
cargo test -p dig-node-service --lib (full) |
764 passed, 0 failed |
cargo clippy -p dig-node-service --all-targets -- -D warnings |
clean |
cargo fmt --check (touched files only) |
clean |
Revert-proofs, each committed first, reverting only the fix:
- predicate ->
(Some(_), false, _): 3 tests FAIL -a_stock_linux_service_shaped_split_proceeds,
mint_is_refused_only_for_an_unanswered_override_with_no_wallet,
setting_the_named_cache_override_lifts_the_refusal. same_root-> exact equality: 2 tests FAIL -a_case_differing_windows_root_is_not_a_split,
same_root_normalizes_trailing_separators_and_optionally_case.
Blast radius
gitnexus is ~301 commits stale on this repo and returns a false-safe impactedCount: 0, so this was
done by grep + direct read and is stated as such. The changed symbols are split_of,
mint_decision, announce and WalletRootSplit, all pub in dig_node_service::wallet_env, whose
only callers ecosystem-wide are wallet_bootstrap.rs, entrypoint.rs:1418 and win_service.rs:128
- all in this crate, all updated. No consumer outside
dig-node-servicenames the module.
Nothing the gates cleared was churned: the verbatim platform_user_base extraction,
seed_present's unknown-means-present direction, the ANNOUNCED once-guard, the log-field content,
the refusal writing nothing, and mirror/lifecycle.rs are untouched. No dig-*/chia-* dep bumps.
Versions unchanged at 0.235.0 / core 0.66.0 / wallet 0.47.0.
# Conflicts: # Cargo.lock # Cargo.toml
Heads-up: this branch's version now COLLIDES with
|
Merge origin/main and take the next unclaimed version; 0.235.0 collided with main after #488 landed. Co-Authored-By: Claude <noreply@anthropic.com>
READY FOR REVIEWER RE-GATE (scoped: the two
|
| finding | outcome |
|---|---|
"This remedy cannot clear the refusal" (wallet_root_split()) |
FIXED. mint_decision() (crates/dig-node-service/src/wallet_env.rs:257-268) now takes cache_override as a third input and refuses only on (Overridden, no seed, no DIG_NODE_CACHE). The remedy the message names flips the verdict to Proceed. SPEC.md:4393-4407 rewritten to match. |
| "This predicate fires on non-Windows hosts where nobody set an override" | FIXED. SplitCause::{Overridden, Ambient} (:76-87) splits the two causes; split_of() (:196-221) assigns the cause from the raw LOCALAPPDATA, and only Overridden can refuse. A stock .deb service mints. |
Tests are load-bearing, and bound to the predicate not the prose
setting_the_named_cache_override_lifts_the_refusal(:450-471) — asserts BOTH directions, so the original defect stays caught while the remedy is proven to work. Deliberately bound tomint_decision(): the earlierREFUSED_SPLIT_MINT.contains("DIG_NODE_CACHE")grep passed against the broken version, which is the exact false green the reviewer flagged.a_stock_linux_service_shaped_split_proceeds(:441-448) — the shipped systemd unit's shape (HOMEunset, noLOCALAPPDATA) mustProceed.mint_is_refused_only_for_an_unanswered_override_with_no_wallet(:474-496) — all eight arms of the three-input table.- The reviewer's named canary,
an_overridden_localappdata_is_reported_as_a_split, is no longerHOME-dependent: it setsLOCALAPPDATAto a tempdir, so the split isOverriddenby construction.
Run: cargo test -p dig-node-service --lib wallet_env → 14 passed; 0 failed (14 tests actually executed, not a zero-match green).
Mergeability
origin/mainmerged (not rebased —--ontosilently drops a version-bump commit whose patch matches upstream). No conflicts.- Version was
0.235.0, colliding with main after perf(serve): stream the capsule hash paths instead of reading whole capsules into RAM #488 landed; the version gate had been reading SUCCESS against a stale base. Re-read from disk after the merge and set to0.239.0(0.236/0.237/0.238 are claimed by feat(mirror): verify a peer's mirror-coin bond against chain on the download path #467/fix(rpc): source the shell error catalogue from dig-rpc-protocol so -32004 carries one name #494/feat(digsex): wire the dig-sex economic layer into the node #456 in planned merge order).Cargo.lockre-resolved.
Next action
Reviewer re-gate, scoped to the two wallet_env.rs findings. loop-security PASSed at 21:46Z on 2b22c7a; the delta since is the two fixes plus a merge commit, and touches no custody or crypto seam — so a security re-run is not indicated. PR stays DRAFT until the gate verdict returns; the orchestrator undrafts and merges.
…t-start # Conflicts: # Cargo.lock # Cargo.toml
Rebased onto a moved
|
|
IN PROGRESS — not the verdict. loop-reviewer scoped re-gate, audited head Confirmed so far by direct read (gitnexus dig-node index is ~300 commits stale, so this is grep + direct read):
Still outstanding: the test run + count, and the failure-direction pass over the 8-arm table. |
CI GREEN on
|
loop-reviewer — VERDICT: PASSAudited head: 1. Finding (a) — the refusal's named remedy now flips the verdict. RESOLVED.
Revert-proof, measured, not reasoned. In my own worktree I mutated the arm to 2. Finding (b) — no rival predicate survives. RESOLVED.
3. Failure direction of the 8-arm table. No arm mints into the wrong root through a misclassification.The only unsafe direction is 4. User-facing sentences. Correct.
5. Tests. 14 passed, 0 failed, 750 filtered out.
6. Delta since the security PASS head
|
MichaelTaylor3d
left a comment
MichaelTaylor3d
left a comment
There was a problem hiding this comment.
loop-reviewer scoped re-gate — PASS at head 547a44f2. Both prior GATING findings are genuinely fixed (revert-proof measured: mutating the cache_override arm fails 2 tests). Full evidence in the verdict comment. No security re-run needed — the only non-merge production commit since the security PASS head is 404076d, which touches no custody, signing or spend path.
… a code span The `crate::wallet_env` reference in `service_data_dir_overrides`' doc was a plain code span naming a module that did not exist on this branch at the time it was written. The module arrived with #489 and is now merged in, so the reference resolves — but a code span is not checked by anything, which is how it came to name a non-existent path in the first place. Upgraded to an intra-doc link, so rustdoc fails if the module is renamed or moved rather than the reference going quietly stale. The one sentence of the argument being distinguished is inlined alongside it, so a reader learns what the paragraph is contrasting without leaving the page. Doc-only; no behaviour change. Co-Authored-By: Claude <noreply@anthropic.com>
… a code span The `crate::wallet_env` reference in `service_data_dir_overrides`' doc was a plain code span naming a module that did not exist on this branch at the time it was written. The module arrived with #489 and is now merged in, so the reference resolves — but a code span is not checked by anything, which is how it came to name a non-existent path in the first place. Upgraded to an intra-doc link, so rustdoc fails if the module is renamed or moved rather than the reference going quietly stale. The one sentence of the argument being distinguished is inlined alongside it, so a reader learns what the paragraph is contrasting without leaving the page. Doc-only; no behaviour change. Co-Authored-By: Claude <noreply@anthropic.com>
…to 0.249.0 Brings origin/main (0.245.0, including #467, #489, #492, #497) onto the branch and sets the workspace version to the pre-assigned 0.249.0. Conflicts and how they were resolved: - `Cargo.toml` — a pure version collision (branch 0.242.0 vs main 0.245.0). Every other main-side hunk was already applied by the auto-merge; the only difference from `origin/main` in this file is the version line, now 0.249.0. - `Cargo.lock` — taken wholesale from `origin/main`, then re-locked with `cargo update -w`, which re-points the two workspace members whose manifests moved (`dig-node-service` 0.245.0 -> 0.249.0, `dig-wallet` 0.47.0 -> 0.48.0). Nothing in the tree still reads 0.242.0. - `crates/dig-wallet/src/sage/rpc.rs` — reported as a conflict by an earlier attempt; on this merge git resolved it textually because the two sides touch disjoint regions of the file. The result was read against BOTH parents rather than accepted on git's word: * MAIN's hunks are intact. `is_definitive_rejection` keeps the #497 narrowing — a refusal frees inputs only when its stated reason is bundle-intrinsic (`super::chain::refusal_is_bundle_intrinsic`), with a HOLD default — and the #492 doc block stating that `synced` is a CURRENCY test computed independently of the routing tier, so `{source: "db", synced: false}` is a reachable state. * THE BRANCH's hunk is intact. `replica_answer_is_current` still delegates to `sync_supervisor::FollowingEvidence::measure`, which withholds the evidence when EITHER the replica or the peer height is unmeasured, so a `synced` phase cannot be emitted without the peak height that bounds it (#495). * No rival implementation survives the merge. The pre-#495 `is_following` predicate is gone from the tree; `FollowingEvidence` is the single producer consumed by both the money reads (`rpc.rs:1085`) and the status endpoint (`sync_supervisor.rs:490`), which is what makes the `{phase: "synced", peak_height: null}` pairing unrepresentable rather than merely unlikely. No behaviour was chosen over the other side: both guards are load-bearing on different questions — one on whether a refusal may free inputs, the other on whether a currency claim may be made at all. dig-wallet: 772 passed, 0 failed, 1 ignored. dig-node-service: 774 passed, 0 failed. Note: `cargo test` on this Windows host needs RUST_MIN_STACK raised (default hits a rustc STATUS_STACK_BUFFER_OVERRUN ICE while encoding dig-node-service metadata) — an environment limit, not a code fault. Co-Authored-By: Claude <noreply@anthropic.com>
…elative to cwd (#491) (#499) * chore(release): anchor v0.241.0 for #491 Salvage anchor for the wallet-seed-path lane. Co-Authored-By: Claude <noreply@anthropic.com> * fix(wallet): anchor the wallet base for a service run without orphaning an existing wallet On a stock Linux .deb install the seed was CREATED at /DigWallet/seed.bin - the filesystem root - and the write SUCCEEDED. The shipped unit sets no User=, so systemd sets no $HOME, and no WorkingDirectory=, so the working directory is /; the wallet's LOCALAPPDATA -> HOME -> "." chain therefore collapsed to a relative base, and ProtectSystem=full leaves / writable for the root-run service. The device key resolved from the same broken base, so the pair stayed consistent and the wallet opened normally, which is why nothing surfaced it. Introduce DIG_WALLET_BASE, a single override of the BASE that both wallet roots hang off, and route seed_path() and autoseed::user_base() through one resolver. Overriding the base rather than either directory keeps DigWallet/ and DigNode/device/ siblings by construction - that separation is the partial-exfiltration boundary autoseed's module docs describe, and a per-directory override would let configuration alone collapse it. A service run anchors that base at the machine state dir, but ONLY when no wallet is present at the base it would otherwise have resolved. The operator wallet holds real $DIG for mirror-coin collateral, so re-rooting an existing host would strand the funded seed and mint an empty replacement; Windows LocalSystem services are the live case, since %LOCALAPPDATA% IS set for them. Presence is read through autoseed::presence and an undeterminable answer counts as present. No key material is moved or copied. The resolver is pure and takes its inputs explicitly so the service environment (no LOCALAPPDATA, no HOME) and the Windows ordering are both exercised on the Linux CI runner, without the process-global env serialization this crate already works around. Refs #491 Co-Authored-By: Claude <noreply@anthropic.com> * test(wallet): assert the fail-closed direction of legacy_wallet_present N1 from the #499 review: the safety-critical half of the change had no test. An unreadable legacy seed path must count as PRESENT, never as absent, or a service start re-anchors the wallet base away from a funded wallet and mints an empty one beside it. Split the mapping out as presence_counts_as_present so the property is assertable without a platform-specific unreadable-path fixture - the portable NUL-in-the-path fixture cannot be delivered through the environment on Windows, because set_var rejects an interior NUL. The split also makes a refactor to Path::exists() a compile error rather than a silent regression. Co-Authored-By: Claude <noreply@anthropic.com> * docs(wallet): make the wallet_env cross-reference a checked link, not a code span The `crate::wallet_env` reference in `service_data_dir_overrides`' doc was a plain code span naming a module that did not exist on this branch at the time it was written. The module arrived with #489 and is now merged in, so the reference resolves — but a code span is not checked by anything, which is how it came to name a non-existent path in the first place. Upgraded to an intra-doc link, so rustdoc fails if the module is renamed or moved rather than the reference going quietly stale. The one sentence of the argument being distinguished is inlined alongside it, so a reader learns what the paragraph is contrasting without leaving the page. Doc-only; no behaviour change. Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
1 participant
DRAFT — DO NOT MERGE. Gate round in progress.
Closes #392
The defect, measured
Two rival resolvers of "the per-user base directory" live in one
dig-nodeprocess and disagreethe moment someone overrides
LOCALAPPDATA— which is exactly when someone is trying to isolate awallet run:
dig_wallet::autoseed::user_base()LOCALAPPDATA->HOME->.DigWallet/seed.bin,wallet.meta.json,DigNode/device/device.keydig_node_core::canonical_cache_dir()DIG_NODE_CACHE->directories::BaseDirs(the OS Known Folder API — it does not read the env) ->LOCALAPPDATA->HOME->.cache/,config.json, and thereforewallet.sqliteReproduced on a real Windows host running the installed node service. With
LOCALAPPDATA=<scratch>and no
DIG_NODE_CACHE, the node created<scratch>/DigWallet/seed.binand<scratch>/DigNode/device/device.keywhilecache.getConfigreportedcache_dir = C:\Users\...\AppData\Local\DigNode\cache— the real path. A brand new seed under oneroot, the machine's existing coin replica under the other, and the only thing said about it was
INFO no wallet was present; minted one, which reads as a clean first run.Separately,
DIG_WALLET_PORTis read only bydig_wallet::wallet_port(), reached only fromdig_wallet::run()— invoked bydig-runtimeand the standalonedig-walletbinary, never bydig-node. Nothing listened on the port. Accepted and ignored, with no error.What this does NOT do, deliberately
Resolution is unchanged. Making either resolver defer to the other is the obvious fix and it is
the destructive one: a service run has
DIG_NODE_CACHE = C:\ProgramData\DigNode\cache(
state::anchor_service_data_dirs), so deriving the wallet base from the node's cache dir wouldmove the seed off
...systemprofile\AppData\Local\DigWallet\seed.bin, find nothing there, and minta fresh wallet on every existing install — orphaning the operator wallet and any $DIG in it.
What it does
Per the ticket's own scope ("a refused override ... is fine to refuse, but refusing silently is
not"):
wallet.sqlitepath. Naming the filematters: the replica sits beside
DIG_NODE_CACHE, not inside it, so an operator who sets thatvariable and looks in the cache directory concludes the second lever failed too.
minted and nothing is written. A host that already HAS a seed proceeds unchanged — refusing there
would break a working install to enforce a layout rule.
DIG_WALLET_PORTis inert in this binary, naming the port and where it ishonoured.
entrypoint.rs::block_on_serveandwin_service.rs::run_service, which does not go through it — before the bootstrap, so the refusalis read before the line that would have said a wallet was minted.
The second finding on #392, the silent logging degrade, was already fixed by #458 and is verified
working on real hardware in this ticket's comments: a non-admin foreground run's first line is the
dig-loggingWARN naming the denied directory. Nothing here re-does it.Shape
A pure decision core (
split_of,mint_decision,inert_wallet_port,replica_beside) taking itsinputs as arguments, plus thin env-reading and log-emitting wrappers — the same split
logging::degrade_announcementandstate::service_data_dir_overridesuse, because the interestingbranch is the one that does not occur on the machine the tests run on.
Announcement prose is pinned in
concat!constants, never\-continued literals: acargo fmtrunrejoined a continuation in this exact module before and materialised the source indentation into an
operator-facing string. A test asserts no run of four or more spaces survives.
How verified
cache.getConfiganswer ratherthan inferring from file mtimes — an earlier mtime-based inference was retracted on the
ticket, because a running
dig-app.exewrites that file continuously regardless.LOCALAPPDATA+DIG_NODE_CACHE+DIG_NODE_STATE_DIR+DIG_NODE_PORT): seed under one, replica at<iso>/wallet.sqlite, on amachine already running the installed service.
LOCALAPPDATAunder a lock and restores it — a testrunning under a normal path cannot see this bug.
returns a false-safe
impactedCount: 0.Bump
0.229.0 -> 0.235.0(0.230-0.234 held by sibling lanes). Minor: new announced behaviour and twoadditive public functions.
dig-node-core 0.65.0 -> 0.66.0anddig-wallet 0.46.0 -> 0.47.0, eachgaining one additive
pub fn.SPEC.md§16 also corrects a claim that had been false since dig-node#327: it said dig-wallet had"native BLS signing", while that crate's own module doc states it holds no user key and signs
nothing (§908).