feat(mirror): drive the mirror pass from disk events, timer as backstop - #470
feat(mirror): drive the mirror pass from disk events, timer as backstop#470MichaelTaylor3d wants to merge 11 commits into
Conversation
…er as the backstop (#465)
72fd757 to
ed2dfc4
Compare
Both sides added a module declaration -- `pub mod events` here, `mod converge_tests` from #457 -- so the union is correct.
|
Greened the red Clippy lints on loop/mc-event. Two manual modulo checks replaced with
Verified with local clippy (exit 0, no errors). Pushed to origin/loop/mc-event. New PR head: |
Lane update — conflicts resolved, rebased onto main via mergeOutcome 1: still needed and distinct. §2.0 already-shipped check against State
Next action: wait for CI on |
|
Local verification on the merged head 131 tests actually executed -- count checked rather than exit status alone. |
|
Head moved —
Next action: watch CI on |
|
IN PROGRESS — not the verdict. Gate on head Confirmed so far, with evidence:
Still open: whether the SPEC/instruction scope ( |
|
IN PROGRESS — not the verdict (2/2). Head
|
MichaelTaylor3d
left a comment
There was a problem hiding this comment.
Gate on b172413. One gating finding, inline. Everything else in this PR checks out and is recorded in the two interim comments above.
MichaelTaylor3d
left a comment
There was a problem hiding this comment.
CHANGES-REQUIRED
Head read: b172413664f099883c3c051ef668d42c07d16bb7, resolved from the remote at review time. Read via git show <ref>:<path> and an isolated worktree; the primary checkout was not touched.
One gating finding, on the inline thread at crates/dig-node-service/src/server.rs:2950 — wait_for_next_pass is the entire seam this PR exists to create, and no test exercises it. All six WakeCoalescer tests and the watcher test pass unchanged if it is wrong. Three cases requested there, all reachable with #[tokio::test(start_paused = true)] and without touching either constant.
Everything else clears, and I want that on the record because the rest of this diff is unusually careful:
- Reentrancy (the money question) is answered structurally. One sequential task; an event moves the next pass earlier and never starts a second. A wedged pass blocks only its own successor, and the events arriving meanwhile collapse into one owed wake rather than a queue that drains as a burst.
- The timer is a real ceiling.
deadlineis computed on entry and every return is at or before it;MIRROR_ROUND_LENGTH_MSis unchanged, and a test asserts it is still10*60*1000— the "do not lengthen the timer to compensate" clause of #465, made mechanical. - The watched path is the observed path.
<cache>/modules, recursive (server.rs:2765-2772) versusscan_cached_modules'sread_dir(self.cache_dir.join("modules"))(dig-node-core/src/lib.rs:2276). The accelerant is not watching a directory nothing writes to. - Self-heal (#464) is not narrowed. The pass body is untouched; only the tail sleep changed. A prior failed attempt is retried on the next pass exactly as before.
observe_chainstaying on the timer is #465's own settled fork, decided with reasons and recorded inSPEC.md. Not a scope miss. Flagged for the user because their verbatim namedobserve_chain.
Evidence. cargo test -p dig-node-service --lib mirror:: → 131 passed; 0 failed; 591 filtered out — 131 tests genuinely executed, not a filter that matched nothing. Revert proof, in my own worktree, by file copy: deleting the MIN_EVENT_PASS_INTERVAL_MS clause from due_at_ms turns no_two_event_driven_passes_are_closer_than_the_floor RED with a real assertion (a pass at 1700000011000 followed one at 1700000005000, closer than the 30000ms floor), not a compile error; the other six stay green. So the floor test is discriminating.
What I could not reach: I did not see an event drive a pass on a real running node — only the watcher signalling in a test, and the coalescer owing a wake in tests. That gap is precisely the gating finding. gitnexus was not used: the dig-node index is ~301 commits stale and returns a false-safe impactedCount: 0, so blast radius was done by grep and direct read.
…_chain is timer-only The gate found the seam this change creates untested: every property the PR is about lives in `wait_for_next_pass`, and both halves it composes were already covered while their composition was not. Three cases on a paused clock: the round timer with no events (both the no-watcher path and the silent-watcher path), a single event waking a quiet period later, and a storm that OUTLASTS the round still returning by its deadline. The third is the one that matters -- it is the branch where the `events.changed()` arm keeps winning the `select!` and the deadline check in the sibling arm is re-entered rather than reached. Testing it at all required one clock rather than two: the deadline and every sleep are on tokio's monotonic clock while the coalescer's windows are unix milliseconds, and reading the wall clock per turn compared two quantities free to disagree -- under a wall-clock step (NTP, suspend/resume) as much as under a paused test clock. The unix figure is now anchored to elapsed monotonic time at entry. SPEC.md §25.5 also stated the chain half as a preference. It is a limitation: `ChainSource` is request/response with no subscription surface, §14.2 chain-watch is itself a poll loop, and the one real push path publishes a FIELDLESS `SyncEvent::CoinState` that names no coin. Recorded with the mechanism that would have to exist, as #482. Refs #465 Co-Authored-By: Claude <noreply@anthropic.com>
# Conflicts: # Cargo.lock # Cargo.toml
DRAFT — do not merge. The gate round has not run.
Drives the §25 mirror pass from disk events, with the round timer kept unchanged as the backstop.
Closes #465
What already existed (measured before building, §2.0)
lifecycle::observe_disk→Node::cache_list_cached()is a directory scan. There is no capsule-cached broadcast/watch channel anywhere indig-node-coreordig-node-service, and no filesystem-watcher crate was inCargo.lock. The debounced presence deltas inrunner.rs:243-245are computed from consecutive scans byPresenceTracker— a settling filter over polled state, not an event source. So the watcher is genuinely new and is not a rival implementation.chia-block-listeneris not a dependency of this repo at all (zero hits forchia_block_listenerundercrates/). Peak is read from the wallet replica'ssync_state, polled.The invariant
No behaviour depends on an event arriving. A wake decides only when the next pass runs, never whether one runs and never what it concludes. The timer is neither deleted nor lengthened.
silencing_every_event_source_leaves_the_timer_untouchedasserts both halves, and pinsMIRROR_ROUND_LENGTH_MSat 10 minutes so a later edit cannot lengthen the backstop to compensate for having events.The chain fork, decided
A chain event does not trigger a pass. Three reasons, in order of weight: a new peak arrives roughly every 18.75s and each pass reads chain and may spend money, so peaks are the largest amplification source available here; chain activity cannot make a create newly correct, because creates are decided from disk presence; and the epoch rollover a reclaim waits on is wall-clock, already derived locally by
current_epoch_now(). A peak subscription would be new integration bought to obtain the one signal most worth suppressing. Chain stays observed inside the pass, on the timer, where one round already bounds reclaim latency far inside an epoch.The coalescing bound
QUIET_PERIOD_MS.digin causes one pass at the end rather than one per write eventMIN_EVENT_PASS_INTERVAL_MSSETTLING_WINDOW_MS(30_000)PresenceTrackerstampssince_msat the first observation, not at file creation, so a single wake could only ever RECORD an appearance. One burst yields an observing wake and one settling wake a window later, which does not re-arm. N events produce exactly one of each, never N.wait_for_next_passcomputes the round deadline once on entry and every return happens at or before it, so an event can only lower the next wake instant, never postpone it.Verification
cargo test -p dig-node-service --lib mirror::— 113 passed, 0 failed (7 of them new inmirror::events). Count checked, not just the exit status.cargo clippy -p dig-node-service --lib --all-features -- -D warnings— clean.no_two_event_driven_passes_are_closer_than_the_floorFAILED (6 passed, 1 failed)a_burst_of_events_in_one_window_produces_exactly_one_observing_passFAILED (6 passed, 1 failed)a_burst_produces_a_settling_pass_and_then_stopsFAILED (6 passed, 1 failed)tokio::time::pause()— a paused clock auto-advances past the wall-clock interval under test.Blast radius checked
impactcould not be trusted here: the registered gitnexus indexes point at the primary checkout and are stale by hundreds of commits (dig-node~301 behind), and a stale index returns a false-safeimpactedCount: 0. Radius was established by direct read plus ripgrep instead, and stated rather than assumed.mirror/events.rsis new. The only change to existing code is insidespawn_mirror_passes(server.rs), plus one new private free functionwait_for_next_passwith no other callers.runner.rs,resolve.rsandspend_audit.rsare untouched — the live The mirror lifecycle must self-heal: every store gets a coin, every orphaned coin is spent, even after a failed attempt #464 lane's surface.PassRunner,MirrorEffects,observe_diskandobserve_chainkeep their exact signatures and semantics; the event source sits entirely outside the pass and changes only when the loop waits.notify = "8", and the lock refreshed in the same commit (cargo update -w --offline).Version
0.202.0(minor — new capability, no behaviour removed),Cargo.lockrefreshed in the same commit.