test(peer): prove bring-up installs the downstream engines on the real genesis (#240) - #533
Merged
Merged
Conversation
Co-Authored-By: Claude <noreply@anthropic.com>
…l genesis (#240) The acceptance test dig-node#240 has been open for: with the default DIG mainnet genesis and no DIG_NETWORK_GENESIS override, the peer-network bring-up gets past gossip-config validation and installs the pool handle, the P2P content engine and the DHT inventory-refresh hook, then binds the mTLS peer-RPC listener. Asserts the DOWNSTREAM post-conditions rather than peerStatus.running, which is set before GossipService::new and so holds even when the pool, DHT, content engine and PEX all fail. Also corrects a stale doc-comment that still described the genesis as a pre-launch placeholder that invalidates the gossip config -- the reason the #213 test sees no P2P convergence is environmental (relay off, loopback only), not a rejected config. Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
# Conflicts: # Cargo.lock # Cargo.toml
Co-Authored-By: Claude <noreply@anthropic.com>
# Conflicts: # Cargo.lock # Cargo.toml
# Conflicts: # Cargo.lock # Cargo.toml
MichaelTaylor3d
added a commit
that referenced
this pull request
Sep 3, 2026
MichaelTaylor3d
marked this pull request as ready for review
September 3, 2026 17:40
MichaelTaylor3d
added a commit
that referenced
this pull request
Sep 3, 2026
MichaelTaylor3d
added a commit
that referenced
this pull request
Sep 3, 2026
… only by the first push's (#528) * chore(wallet): open the #502 lane -- bound total reservation hold Salvage anchor for the dig-node#502 lane. Version assigned 0.251.0 (origin/main is 0.247.0; 0.248-0.250 are held by sibling lanes). Co-Authored-By: Claude <noreply@anthropic.com> * test(wallet): pin the total reservation hold a repushed bundle may take Four failing db-layer tests for dig-node#502, plus the constant they measure against. `MAX_RESERVATION_HOLD_MS` is defined as a multiple of `RESERVATION_TTL_MS` in one place so the two cannot drift; the TTL itself is unchanged. The acceptance test steps by less than a TTL past the cap and asserts its own iteration count: a one- or two-push fixture is satisfied by the unfixed code, because the first hold has not lapsed yet. Co-Authored-By: Claude <noreply@anthropic.com> * fix(wallet): bound the total hold a repushed bundle may take on its inputs `reserve_spend` re-armed `expires_at` to `now + RESERVATION_TTL_MS` on every push of a given transaction id, so a caller re-pushing the same signed bundle more often than the TTL renewed its hold forever and the inputs never returned. That is the lockout failure the TTL's own doc names as the worse of the two, reachable without a single dishonest answer. Two composed bounds, neither of which shortens the TTL: * A TOTAL cap anchored on the FIRST push. `MAX_RESERVATION_HOLD_MS` is defined as `6 * RESERVATION_TTL_MS` in one place, so lengthening the TTL scales the cap and the two cannot drift. `submitted_at` is not in the upsert's `DO UPDATE SET` list, so the stored value is a stable anchor; a test pins it. * A reason-conditional re-arm. `chain::refusal_forecloses_a_later_push` names the four CLVM-execution / cost refusals that complain about the bundle's own contents and that no better-synced node can turn into an acceptance. Those still HOLD -- the verdict is height-dependent, so this crate declines to trust one node's view of it -- but they may not RENEW the hold. Everything else extends, including an unrecognised reason, an `Err`, and a bare verdict. The two compose into the gate's "every observed refusal was foreclosing" case without a per-attempt history, because a re-push may never move the deadline EARLIER: if every attempt is non-extending the deadline never leaves the first `submitted_at + TTL`, and an extending attempt's grant survives every later one. The clamp lives in the SQL so it is atomic against the stored anchor; a read-then-write above this layer would race two concurrent pushes. `coin_reservations`' `ON CONFLICT(coin_id) DO NOTHING` first-claim-wins rule is untouched and pinned by a test. Closes #502 Co-Authored-By: Claude <noreply@anthropic.com> * chore: bump to 0.256.0, clear of #506's 0.251.0 * fix(wallet): drop the reason-conditional re-arm, keep the total-hold clamp The adversarial gate on #505 refuted the reason-conditional half and it is removed in full: VIEW_DEPENDENT_BUNDLE_CONTENT_REFUSALS, refusal_forecloses_a_later_push, attempt_may_extend_the_hold, the PendingTransactionRow::may_extend_expiry field and the CASE arm in reserve_spend. The four names it listed -- GENERATOR_RUNTIME_ERROR, BLOCK_COST_EXCEEDS_MAX, INVALID_BLOCK_COST, INVALID_SPEND_BUNDLE -- do not identify a bundle no destination is holding. push_tx relays to up to three destinations and only the LAST answer returns, so such a refusal from the last says nothing about the first, which may have admitted and gossiped the bundle. The removed code freed inputs up to 550s earlier than main for a bundle that lands, with no attacker: the double-spend direction #497 exists to close. What ships is the clamp alone: expires_at is bounded by submitted_at + 6 * RESERVATION_TTL_MS. The outer MAX is retained because a non-monotonic clock is the one case that can still drive an incoming deadline below a live one, and shortening a live hold is the dangerous direction. SPEC.md 18.9a gains the total-hold bound as a normative clause: without it a reimplementation built from the spec as written reproduces the unbounded re-arm this change fixes. Three limitations are now stated in MAX_RESERVATION_HOLD_MS' doc and two are pinned by tests: the bound is on CONTINUOUS hold and a re-push after the prune gets a fresh anchor; a bundle whose timelock matures past the cap has its inputs freed while the network genuinely still holds it; and inside the last TTL before the cap a re-push buys strictly less than a full TTL. The clamp also fails OPEN under an absurd clock, since SQLite promotes integer overflow to REAL. Refs #502 Co-Authored-By: Claude <noreply@anthropic.com> * chore: renumber to 0.252.8, under the MSI ProductVersion ceiling (#521) * style(wallet): rustfmt the two reserve_spend test call sites cargo fmt wanted the multi-line call form at both boundary tests. Formatted those two files only; the workspace-wide check is now clean at zero diffs. Co-Authored-By: Claude <noreply@anthropic.com> * docs(wallet): drop a comment left behind by the reverted re-arm field The comment sat on `reserved_coin_ids`, which IS assembled from a stored table and has no `true`. It described `may_extend_expiry`, the bool this branch removed in fce2358, and pointed at a field doc that no longer exists. Also tighten SPEC 18.9a: a re-push does not unconditionally 'update the deadline' -- at or past the cap, and under a backwards clock, it correctly leaves the deadline unchanged. State the re-arm as subject to the bound. Co-Authored-By: Claude <noreply@anthropic.com> * chore(wallet): open the lane for dig-node#525 (clock-anchored reservation freeze) Version anchor only. The fix follows: a far-forward clock at a bundle's FIRST push writes a `submitted_at` far in the future, and #505's outer `MAX` then pins `expires_at` there permanently, so no later correct-clock push and no prune can ever release the coins. Refs #525 Co-Authored-By: Claude <noreply@anthropic.com> * fix(wallet): repair a reservation whose deadline contradicts the clock (#525) `reserve_pushed_bundle` reads the clock once and writes both `submitted_at` and `expires_at` from that reading, so a single reading far in the future stores a deadline decades out. Nothing could retire it: `prune_reservations` deletes on `expires_at <= now`, which never arrives, and #502's upsert clamp is `MAX(stored, ...)`, so a later push under a corrected clock leaves the stored deadline alone. The coin was withheld from selection for ever and `reset_chain_cache` refused while the row existed. `prune_reservations` now repairs at OBSERVATION: a row whose deadline exceeds `now + MAX_RESERVATION_HOLD_MS` contradicts its own columns against the clock (an honest row satisfies `expires_at <= submitted_at + CAP` and `submitted_at <= now`), so it is re-anchored to `now` and granted one fresh `RESERVATION_TTL_MS`. `submitted_at` moves too, or #502's cap clause would stop binding on that row for ever. The client hold table gets the same repair, keyed on the SAME threshold so a five-minute backwards step cannot re-clamp a healthy hold, and granted its own ceiling since its requested TTL is unrecoverable. All four statements now share one write-first transaction. `reset_coin_db` prunes first, like every other reservation-sensitive entry point, so the refusal message telling a user to wait becomes true. Co-Authored-By: Claude <noreply@anthropic.com> * style(wallet): rustfmt the two files touched by #525 Co-Authored-By: Claude <noreply@anthropic.com> * fix(wallet): correct the clock-anchor SPEC claim and pin the 110-minute forward-glitch residue The adversarial gate on #525 found the normative sentence this PR added to SPEC.md was false: it claimed a reservation is never held beyond MAX_RESERVATION_HOLD_MS (60 min) from an observed instant. A forward clock glitch of up to CAP - TTL (50 min) at the first push evades the clock-contradiction detector by construction, and the true worst case is 2*CAP - TTL = 110 minutes, tight. - SPEC.md 18.9a now states the 110-minute bound explicitly, and names the CAP-TTL constant as both the forward-glitch evasion window and the backwards-step false-fire floor -- one constant, two sides. - db.rs: doc comments on the repair explain the residue instead of overclaiming past it. - A new compile-time assert pins RESERVATION_TTL_MS <= MAX_RESERVATION_HOLD_MS -- unreachable today (CAP = 6*TTL) but load-bearing if that ratio is ever narrowed, since a TTL above the cap would make every repaired row re-trigger the detector forever. - The existing boundary test is renamed and its doc comment states plainly that its past-bound row is synthetic and unreachable by any writer -- it pins the SQL predicate's `>` only, not production behaviour. - A new regression test, built entirely from real reserve_spend/prune_reservations calls (no hand-placed rows), measures the actual 110-minute residue under a real retry loop. No behaviour change: the repair itself is unchanged. This corrects a normative claim born false in the commit that wrote it, and pins the honest bound in its place. Co-Authored-By: Claude <noreply@anthropic.com> * chore(release): bump to 0.252.96 to clear sibling lanes Co-Authored-By: Claude <noreply@anthropic.com> * chore(release): bump to 0.253.7 to avoid collision with sibling release PRs Co-Authored-By: Claude <noreply@anthropic.com> * chore(release): bump to 0.254.3, resolve collision with #533 (0.254.1) --------- Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Task
dig-node#240 item 1 — the finite checklist's remaining substance: prove pool/DHT/PEX actually come
up against the real DIG L2 genesis, not merely that the genesis constant is correct.
What this PR does
Adds
crates/dig-node-core/tests/genesis_bringup.rs, a hermetic end-to-end test asserting theDOWNSTREAM post-conditions of a successful
run_peer_networkbring-up against the real defaultgenesis (no
DIG_NETWORK_GENESISoverride):Node::gossip_handle()isSome— the gossip pool started.Node::p2p_content()isSome— the P2P content engine is installed (wired only once the DHT is up).Node::has_inventory_refresher()(new observable, added to thePeerNetworktrait) — the DHTinventory-refresh hook is installed.
running == trueis deliberately NOT asserted — it is set before the pool exists and holds evenwhen every downstream step fails, which is exactly why #240 needed a new test rather than a
strengthened one (the same failure
serve_with_gate_on_starts_the_peer_network_and_still_serves_readshad).
Scope, stated plainly (also in the test's own module doc): this proves the bring-up completes
against the real genesis in a single hermetic process (relay off, loopback only). It does NOT prove
PEX behaviour, multi-peer convergence, or the relay reservation/accept loop — those need more than
one host and are the multi-node e2e's job (#1062), not this test's.
Also updates one stale
lib.rsdoc-comment (#213's chain-watch test) that described the old"placeholder genesis rejects the gossip config" world; it now cites this test and states the real
reason the pool doesn't converge in that test's env (no peer to reach, not a rejected config).
Item 2 — NOT included here, already covered elsewhere
#240's other checklist item (4 stale
SPEC.mdclauses describing the old placeholder-genesisbehaviour) is already addressed on open PR #530 (
loop/242-241-396-240), with more complete citationsthan a duplicate fix here would have (it cites
genesis_challenge_env_override's unit tests andcorrectly flags item 1 as open pending this test). I read that diff before touching SPEC.md and
reverted my own parallel edit to avoid two lanes rewriting the same lines. #530's SPEC.md text
currently says "no test in this crate yet exercises the full bring-up ... tracked as #240 item 1" —
once this PR merges that sentence should be updated to point at
tests/genesis_bringup.rsas aone-line follow-up (flagging for whoever sequences the two merges).
This PR alone leaves dig-node issue 240 open — item 2 lives on #530. Recommend a manual resolution of dig-node issue 240
once both PRs are merged, or adding a closing keyword to whichever of the two lands second.
How verified
cargo test -p dig-node-core --test genesis_bringup: 1 passed; 0 failed; 0 filtered out.has_inventory_refresher()to returnfalse— testcorrectly FAILED (
refresher=false, panic at the assertion), confirming it actually exercisesthe claimed post-condition rather than passing vacuously. Reverted before commit.
cargo check -p dig-node-core --all-targets: clean.Version
0.253.5(Cargo.toml + Cargo.lock). Mergedorigin/main(not rebased) into this branch first;0.253.0/0.253.1/0.253.2 are already claimed by concurrently open PRs #524/#528/#518, and 0.252.98/99
by #402/#530.
Contributes to (does not resolve on its own): #240