fix(hygiene): port the lost-continuation guard to 4 crates, fix 48 corrupted strings (#3190) - #583
Conversation
MichaelTaylor3d
left a comment
There was a problem hiding this comment.
CHANGES-REQUIRED — this PR contains no work.
Audited head SHA 5754d001b6c418e5d2640c34848301eec4e3de68.
Same shape as #582: additions:0, deletions:0, changedFiles:0, one commit
(chore: open lane for #3190), PR body itself is titled "DO NOT MERGE — lane in progress" and
states no code was written yet. The lane was capped before doing any of the described
continuation-guard replication into dig-node-core/dig-wallet/dig-runtime/dig-chat-protocol.
Nothing to gate; needs a real implementer pass first.
…rrupted strings Replicates dig-node-service::continuation_guard (dig-node#526/#501) into dig-node-core, dig-wallet, dig-runtime and dig-chat-protocol, line-for-line apart from crate-specific constants -- ported rather than reinvented, per dig_ecosystem#3190. Wiring the guard in surfaced 48 pre-existing lost-continuation defects the ticket's own "no measured corruption in these four crates" note did not anticipate: 36 in dig-node-core, 12 in dig-wallet, mostly test-assertion prose where a multi-line message lost its `\` continuation and shipped the source's own indentation as a mid-sentence space run (one as the worse `\n`-plus-indentation variant). All 48 are collapsed to the single space the sentence always meant, with surrounding indentation and wording otherwise untouched. Two lines are real column-alignment, not defects, and get a targeted EXCLUDED_LINE_RANGES entry on dig-node-core instead of a rewrite: download.rs's `claimed(...)` fixture-table trailing comments, and net.rs's `label : value` debug-print alignment. Refs DIG-Network/dig_ecosystem#3190 Refs DIG-Network/dig_ecosystem#3130 Co-Authored-By: Claude <noreply@anthropic.com>
MichaelTaylor3d
left a comment
There was a problem hiding this comment.
loop-reviewer verdict: PASS
Head audited: 1a084c4dbab1b7a6a1d4d0efa8e3456cfa309e99 (base develop).
All 48 string edits verified programmatically, not by eye. Diffed every -/+ pair in
the PR: after normalizing (collapse runs of 2+ spaces to one space, replace literal \n
with a space) 47 of 48 pairs are byte-identical between old and new -- pure whitespace-run
collapse, no wording changed, no reflow. The one exception, store_melted.rs:1124, also
removes a literal \n escape sequence (not just the trailing space run) -- this is still
the same defect class (a botched line-continuation left both a stray escaped newline and
the source's leftover indentation mid-sentence) and is the one the PR body itself flags as
"the worse variant." Not a stray reword.
Indentation restoration verified byte-exact: for all 48 edited lines, leading
whitespace (the part before the first non-space character) is identical between the old and
new line -- confirmed programmatically, zero mismatches. The claimed "clobbered-then-fixed"
indentation repair holds.
Discriminator (10+ spaces, exempt if followed by // or listed): spot-checked both
new exemptions against dig-node-core's live source at this SHA.
download.rs:4595--claimed(7, None), // claims nothing -> Unverified--
the exempted run is inside the trailing comment (aligning->across three sibling
fixture-table lines), not before//, so it needed the line-range exemption; the run
before//itself is already exempt structurally. Genuine alignment, not a suppressed
defect.net.rs:2259--"warning : {:?}",-- padding aligns this line's colon
with the precedingprintln!("TIERED (this change) : ...")'s colon (whose own run is
only 3 spaces, well under the 10-space floor, so it needs no exemption). Genuine column
alignment.
Guard wiring across all four crates: confirmed mod continuation_guard; added under
#[cfg(test)] in dig-chat-protocol, dig-node-core, dig-runtime, dig-wallet's
lib.rs. MIN_FILES_SCANNED floors (2/50/0/30) sit safely below each crate's claimed file
count (4/78/2/43), so a scan that silently reads zero/too-few files fails rather than
passing vacuously. The revert-proof was demonstrated directly in dig-chat-protocol only;
the other three are exercised for real by the pre-existing corruption this PR fixes (a
guard with a wrong constant in dig-node-core/dig-wallet would have failed to find their
36/12 real corruptions, which is equivalent proof by construction). Reasonable, not a gap.
Rival-implementation risk: the four copies are line-for-line identical apart from the
module doc and the three per-crate constants (EXCLUDED_LINE_RANGES, CLI_COLUMN_FILES,
MIN_FILES_SCANNED) -- confirmed by direct comparison of the diff bodies. No drift found.
A shared home (moving the scanner to a crate all five could depend on) isn't possible today
without a same-level/upward dependency edge for at least one of these crates relative to
dig-node-service; the PR correctly does not attempt it and the residual notes a
cross-repo drift-check as a follow-up once other repos land their own copy.
CI: all required checks green. Zero open review threads. PR body's "48 total... nothing else
about the string, or the surrounding code, was reworded" claim is accurate per the
programmatic diff above (with the store_melted.rs nuance noted, which the body itself
separately flags).
develop -> main batch should carry: Closes dig_ecosystem#3190 and #3130 explicitly in the
batch PR body.
…t-code guard, copy hygiene (#584) * feat(mirror): persist mirror-bond coin ids (#575) * chore: open lane for #574 * feat(mirror): persist mirror-bond coin ids so a restart cannot double-create Bond identity was reconstructed from a live chain scan on every read (`mirror/observe.rs`), with no persistence of its own. A restart, a cold replica, or a lagging/flaky chain source all rendered a real, unspent, confirmed bond as "no bonds" -- and because the in-flight suppression is keyed on pending/submitted audit entries, a bond whose create had already CONFIRMED was not suppressed either, so the same short scan that emptied the read surface also cleared the one thing that would have stopped a second coin being paid for collateral that already exists (dig-node#574). Persist the (store, root, epoch) -> coin_id mapping in the EXISTING spend audit record (spend-audit.jsonl) rather than a new store: a mirror-coin create already writes store_id + AuditedBond{root, epoch} + amount there, and the coin id itself becomes durable the moment resolve_landed_spends confirms it. This adds the one missing piece -- the advertised URL a create carries -- and a read-side query, confirmed_mirror_bond, that returns the newest CONFIRMED record naming a triple. Chain stays authoritative. mirror::local_bond::recheck_missing_bonds never trusts the record: for a held bond the live scan did not cover, it asks the record for a candidate coin id, then re-verifies that SPECIFIC coin against chain via the same independent check (chain_bond_verdict) that verifies an untrusted peer's claimed bond. Only a fresh `Bonded` verdict is folded back in, as covered; `Unbonded`/`Unverified` fall through to an ordinary create, exactly as if no record existed. Version: 0.254.86 (patch -- per #522 the MSI ProductVersion minor field is exhausted and the counter lives in patch). Co-Authored-By: Claude <noreply@anthropic.com> * test(mirror): prove the recovery wiring end to end through PassRunner::run Adds two integration-level tests over the REAL pass pipeline, not just the isolated recheck_missing_bonds unit tests: a bond missing from the live scan with a chain-reverified durable record is recovered (no double create, correct Bonded state reported), and the control -- the same record but chain disproves it -- correctly falls through to an ordinary create. Together these are the concrete regression test for the cold-start/lagging-chain-source double-create scenario the ticket asked to have measured. Also refactors in_flight_creates to take the already-folded SpendLedger instead of re-reading the log itself, so PassRunner::run reads the audit file once per pass and shares it with the new recovery step, and fixes a doc comment on in_flight_creates that the recovery step would otherwise have made stale on landing ("a Confirmed create has a coin the chain observation already sees" is no longer unconditionally true). Co-Authored-By: Claude <noreply@anthropic.com> * chore(fmt): wrap long test signatures to satisfy rustfmt Co-Authored-By: Claude <noreply@anthropic.com> * chore(clippy): use slice::from_ref instead of cloning for a single-element slice Co-Authored-By: Claude <noreply@anthropic.com> * chore(release): bump to v0.254.89 Base branch moved to develop after PR #576 merged there at v0.254.88 (main and develop are currently identical), leaving this branch's carried-forward .88 as a zero-increment against the new base. Bumped to the next free integer after fetching and verifying both origin/main and origin/develop tip at .88. Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com> * fix(peer): count accepted relayed circuits in the connected pool (#579) serve_accepted_relay_conn served every accepted relayed circuit (full mTLS auth, full L7 peer RPC) while registering it nowhere, so connected_peers under-reported every relayed inbound peer -- the relay-leg twin of the direct-inbound defect #402/#523 already fixed. adopt_inbound_peer_in_pool now dispatches by TraversalKind: Relayed routes to dig-gossip's already-published adopt_relayed_inbound_handle (v0.32.0, the rev this repo already pins), every other tier keeps the unchanged adopt_direct_inbound_handle path. serve_accepted_relay_conn adopts before serving and releases after, mirroring the direct listener exactly. Refs: DIG-Network/dig_ecosystem#3124 * fix(cli): guard the exit-code namespace shared with diga against collisions (#582) * chore: open lane for #3189 * fix(cli): guard the exit-code namespace shared with diga against collisions dign and diga deliberately share one process exit-code numbering (dig-app's outcome.rs says so in its own doc comment), so a number is free only if it is unoccupied ecosystem-wide. dig-node#407 assigned exit 7 to NODE_UNREACHABLE by checking only this repo's own table, where 7 genuinely was free -- and collided with diga's NOT_CONNECTED. A reviewer caught it by hand; nothing failed automatically. Adds scripts/check-exit-code-collisions.sh: parses both enums' code()/name() match arms straight from their own source -- this repo's ExitCode, and a live fetch of dig-app's outcome.rs at its default branch -- and fails if a number carries two different names, or if either side draws a number from the reserved shell signal range (126, 127, 128+N). Ships with an 18-case hermetic test harness (scripts/tests/check-exit-code-collisions.test.sh) covering the actual #407 collision shape, arm-order independence, arm-count mismatch, the reserved-range boundary from both sides, the live-fetch path itself, and fail-closed behaviour on an empty/missing/unreachable table. Wires a real (unstubbed) invocation into ci.yml's existing "Release-script tests" job so a collision introduced by a future PR, on either side, is a red required check on that PR -- not a note a reviewer has to catch. The fetch retries twice (2s backoff) since this becomes a required, network- dependent check; a fetch failure still fails closed after retrying, never silently passing as "diga has no codes". Updates SPEC.md 8.4 to point at the mechanical guard instead of leaving "re-check both tables" as unenforced prose, and records that the extension's WALLET_WS_ERR.NOT_CONNECTED = -33001 is a separate JSON-RPC error-code space, not a rival of this one. Adds a doc-comment to the existing transcribed collision test pointing future readers at the live script as the authoritative check; the transcription remains as a narrower, hermetic regression pin for the #407 shape specifically. No renumbering: every currently-assigned code is unchanged. Refs #3189 Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com> * fix(hygiene): port the lost-continuation guard to 4 crates, fix 48 corrupted strings (#3190) (#583) * chore: open lane for #3190 * fix(hygiene): port the lost-continuation guard to 4 crates, fix 48 corrupted strings Replicates dig-node-service::continuation_guard (dig-node#526/#501) into dig-node-core, dig-wallet, dig-runtime and dig-chat-protocol, line-for-line apart from crate-specific constants -- ported rather than reinvented, per dig_ecosystem#3190. Wiring the guard in surfaced 48 pre-existing lost-continuation defects the ticket's own "no measured corruption in these four crates" note did not anticipate: 36 in dig-node-core, 12 in dig-wallet, mostly test-assertion prose where a multi-line message lost its `\` continuation and shipped the source's own indentation as a mid-sentence space run (one as the worse `\n`-plus-indentation variant). All 48 are collapsed to the single space the sentence always meant, with surrounding indentation and wording otherwise untouched. Two lines are real column-alignment, not defects, and get a targeted EXCLUDED_LINE_RANGES entry on dig-node-core instead of a rewrite: download.rs's `claimed(...)` fixture-table trailing comments, and net.rs's `label : value` debug-print alignment. Refs DIG-Network/dig_ecosystem#3190 Refs DIG-Network/dig_ecosystem#3130 Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
Task
Port
dig-node-service::continuation_guard(dig-node#526/#501) into this repo's other fourcrates and fix whatever it catches:
dig-node-core,dig-wallet,dig-runtime,dig-chat-protocol. Refs https://github.com/DIG-Network/dig_ecosystem/issues/3190,https://github.com/DIG-Network/dig_ecosystem/issues/3130.
Base is
develop(no version bump — the version gate fires only on PRs intomain).Do not merge — draft, awaiting gate.
What landed
Ported line-for-line, adapting only the crate-specific constants (module doc, per-crate
EXCLUDED_LINE_RANGES/CLI_COLUMN_FILES/MIN_FILES_SCANNED) — same algorithm, not arival shape. No sibling repo (digs/dig-account/dig-updater) had pushed a guard yet at the time
this branch started; nothing to adopt from them.
Wiring the guard in found real, pre-existing corruption the ticket's own note
(
"no measured corruption in these four crates — prophylactic") did not anticipate:dig-node-coredig-walletdig-runtimedig-chat-protocol48 total, almost all test-assertion prose (
assert!/assert_eq!messages) where amulti-line message lost its
\continuation and shipped the source's own indentation as arun of 10-18 spaces mid-sentence; one (
store_melted.rs:1124) was the worse variant — aliteral
\nplus indentation. Every fix collapses the run to the single space the sentencealways meant; nothing else about the string, or the surrounding code, was reworded or
reflowed. Indentation on lines whose fix touched a leading run ≥10 spaces was verified
byte-exact against the pre-fix committed source (
git show HEAD:<path>) after an initialmechanical pass clobbered it — caught before push by that same verification, not by CI.
Two lines are real column-alignment, not defects — given a targeted
EXCLUDED_LINE_RANGESentry ondig-node-coreinstead of a rewrite, mirroring thereference crate's own escape-hatch mechanism:
download.rs:4595— aclaimed(...)fixture table's trailing comments, column-alignedacross three sibling lines (
// a coin bonding something else -> Unbonded, etc.). The runbefore the first
//is exempt structurally; the run inside the comment's own text(before its
->) is not, so the whole line needs the exemption.net.rs:2259— twoprintln!debug lines column-align theirlabel : valueprefix sothe colons line up in terminal output. Same idiom
dig-node-service'sCLI_COLUMN_FILESexempts, expressed as plain padding with no literal
\nfor that exemption to key off.The discriminator (unchanged from the reference, stated here for review)
A non-comment line's run of 10+ spaces is a defect unless it is immediately followed
by
//(trailing-comment column alignment) or the line falls in a per-fileEXCLUDED_LINE_RANGES/CLI_COLUMN_FILESentry. Ten spaces sits comfortably above everylegitimate pad measured in these crates (max 8) and comfortably below every real instance
found here (10-18).
Proof it's load-bearing (revert-proof, not just "the checker is present")
Injected a deliberate 15-space run into
dig-chat-protocol/src/types.rs:36(a file theguard's own tests don't touch), then reverted:
cargo test -p dig-chat-protocol --lib continuation_guard→0 passed; 1 failed,panic message names
types.rs:36and quotes the exact injected fragment.git diff --statempty).1 passed; 0 failed.Demonstrated once (algorithm is identical across all four ports); the other three crates'
guards are exercised for real by the pre-existing corruption this PR fixes — each guard
fails against the original committed content and passes against the fixed content, which
is the same proof by construction.
Verify
cargo fmt --all -- --check— clean.the trailing-comment continue-scanning subtlety) run in Python against all four crates
post-fix: zero remaining offenses, file counts match each crate's
MIN_FILES_SCANNEDfloor (78/43/2/4 vs floors of 50/30/0/2).
cargo test -p dig-node-core -p dig-wallet -p dig-runtime -p dig-chat-protocol --lib --jobs 4 continuation_guard— scoped run avoiding the two integration-test binaries thathit local Windows linker (
LNK1318/LNK1140) contention under concurrent sibling-lanebuilds on this machine (see below).
Test + coveragepending.A local
cargo test --workspace --all-targets --all-features --locked --jobs 4run hittwo
link.exefailures (dig-node-service'sserver/lib-test binaries,dig-node-core'sinbound_pool_membership) — bothLNK1318 Unexpected PDB error/LNK1140, a known MSVCconcurrent-linker symptom, not a rustc/code error: every crate's library compiled
cleanly first (
dig-node-service v0.254.89,dig-chat-protocol v0.2.0,dig-node-core v0.68.1,dig-wallet v0.49.0,dig-runtime v0.4.0all showCompiling ...with no preceding error), and only two specific test binaries' link stepfailed, while 2-3 sibling lanes' own cargo processes were concurrently building on this
same machine. Confirmed environmental, not code, by (a) the failure being
link.exe-stageonly, after successful rustc compilation, and (b) CI's independent, isolated Clippy pass
(which requires a full successful compile) going green.
Blast radius checked
Every touched symbol is a
&'static strliteral argument toassert!/assert_eq!/panic!or a
Displayformat string, plus four new test-only modules gated#[cfg(test)]. Nopublic API, no non-test code path, no behavior change outside test-failure-message text.
detect_changes()-equivalent:git diff --statshows only the 19 pre-existing files(string-literal-only edits) + 4 new
continuation_guard.rsfiles + the 4lib.rsmodulewires.
Residual / out of scope
digs/dig-account/dig-updaterlandone shaped differently, worth a follow-up drift check across the family for consistency.
\nfollowed by a realnewline, rather than staying on one source line, slips past) is inherited unchanged and
was not rediscovered as a new instance here — only the same-source-line
\n-plus-indentvariant was found (
store_melted.rs:1124), which the guard does catch.informational and will drift as the crates grow; not load-bearing (
MIN_FILES_SCANNEDhaswide margin below every one).
Co-Authored-By: Claude noreply@anthropic.com