fix(wallet): route wallet_reset_coin_db's now through ClockGovernor - #543
Merged
Conversation
Reservation deadlines (#502/#525/#528) are anchored entirely on wall-clock readings. #528 closes the case where the clock is already wrong at the moment a reservation is FIRST written. It does not close the general form (#532): a wall clock stepped FORWARD while a reservation is already live, mid-hold -- an NTP step, a VM pause/resume, an operator setting the clock -- produces no self-contradiction for #528's check to catch, yet the very next prune reads the jump as elapsed time and can retire a bundle's hold while it is still genuinely in flight, with no bound on how far forward the step goes (the #348/#497 double-spend direction). Add ClockGovernor: it disciplines every reservation-lifecycle "now" reading so it cannot advance, between two observations, faster than a monotonic clock says real time has actually elapsed. A forward wall-clock jump is absorbed rather than trusted and the disciplined clock simply runs behind until real time catches up, at which point it resumes tracking the wall clock with no special unfreeze step. A backward step is passed straight through unclamped, since it can only lengthen a hold, never shorten one -- the safe direction #502/#528 already accept elsewhere. The governor lives for the process's lifetime and is not persisted: a restart re-seeds it from the wall clock at that moment, so a clock already wrong at boot remains #528's write-time contradiction check's problem, not this one's. Closes #532 Co-Authored-By: Claude <noreply@anthropic.com>
Root workspace version, per the main lane -- the minor field's version scheme is being fixed separately under #521/#522; this is the interim number to carry PR #539 (dig-node#532) through the version-increment gate. Cargo.lock refreshed in the same commit (cargo update -w --offline) so dig-node-service's locked entry matches -- every CI job runs --locked, and a manifest-only bump here fails Clippy/Test+coverage/all three package builds together on a change that cannot otherwise break a build. Co-Authored-By: Claude <noreply@anthropic.com>
…onotonic # Conflicts: # Cargo.lock # Cargo.toml
…onotonic # Conflicts: # Cargo.lock # Cargo.toml # SPEC.md
…onotonic # Conflicts: # Cargo.lock # Cargo.toml
…nto loop/541-reset-clock-governor # Conflicts: # Cargo.lock # Cargo.toml
wallet_reset_coin_db read its now_ms from a fresh, undisciplined SystemTime::now() rather than WalletBackend::reservation_now_ms(), so a wall-clock jump mid-hold (an NTP correction, a VM pause/resume) could make its in-flight-spend check see a still-live reservation as already expired and let the reset proceed -- the #348/#497 double-spend direction, no attacker required. reservation_now_ms() is now pub so the control plane (a different crate) can route through it, sharing the same ClockGovernor clamp state every other reservation call site (reserve_coins, prune_reservations) already uses. Swept every reservation-touching path in dig-wallet and dig-node-service for a direct SystemTime::now() read; this was the only production one. Closes #541 Co-Authored-By: Claude <noreply@anthropic.com>
dig-wallet: minor -- reservation_now_ms is now a public API surface (dig-node-service routes through it, dig-node#541). dig-node: patch -- behaviour fix, no breaking change. Co-Authored-By: Claude <noreply@anthropic.com>
MichaelTaylor3d
added a commit
that referenced
this pull request
Sep 3, 2026
Merged origin/main forward (7042f89, #535) rather than rebasing -- the queue across the five open PRs sequences this one last (#542 0.254.42, #539 0.254.43, #543 0.254.44, #536 0.254.50, this PR 0.254.51), deliberately: #542 touches ensure-version-increment.yml, which this PR's follow-up note also targets, so landing last means the final shape is on main rather than guessed at. Re-read the version from Cargo.toml on disk (not the commit log) after the merge, per the standing caution that a rebase can silently drop a bump commit as "already upstream" -- this was a merge, and the version file itself confirms 0.254.51 post-bump. Co-Authored-By: Claude <noreply@anthropic.com>
MichaelTaylor3d
added a commit
that referenced
this pull request
Sep 3, 2026
Per the main lane: main advanced to exactly 0.254.41 after #535's rebase, tying this branch's version. 0.254.43 clears main and every sibling PR in the version-bump queue (#542=0.254.42, #543=0.254.44, #536=0.254.50, #544=0.254.51). Cargo.lock re-synced with `git checkout origin/main -- Cargo.lock` followed by `cargo update -w --offline` (never hand-editing lock conflict markers), confirmed clean with `--dry-run` -> `Locking 0 packages`. Co-Authored-By: Claude <noreply@anthropic.com>
…governor # Conflicts: # Cargo.lock # Cargo.toml
MichaelTaylor3d
added a commit
that referenced
this pull request
Sep 3, 2026
#539) * fix(wallet): discipline reservation liveness against a monotonic clock Reservation deadlines (#502/#525/#528) are anchored entirely on wall-clock readings. #528 closes the case where the clock is already wrong at the moment a reservation is FIRST written. It does not close the general form (#532): a wall clock stepped FORWARD while a reservation is already live, mid-hold -- an NTP step, a VM pause/resume, an operator setting the clock -- produces no self-contradiction for #528's check to catch, yet the very next prune reads the jump as elapsed time and can retire a bundle's hold while it is still genuinely in flight, with no bound on how far forward the step goes (the #348/#497 double-spend direction). Add ClockGovernor: it disciplines every reservation-lifecycle "now" reading so it cannot advance, between two observations, faster than a monotonic clock says real time has actually elapsed. A forward wall-clock jump is absorbed rather than trusted and the disciplined clock simply runs behind until real time catches up, at which point it resumes tracking the wall clock with no special unfreeze step. A backward step is passed straight through unclamped, since it can only lengthen a hold, never shorten one -- the safe direction #502/#528 already accept elsewhere. The governor lives for the process's lifetime and is not persisted: a restart re-seeds it from the wall clock at that moment, so a clock already wrong at boot remains #528's write-time contradiction check's problem, not this one's. Closes #532 Co-Authored-By: Claude <noreply@anthropic.com> * chore(release): bump workspace version to 0.254.20 Root workspace version, per the main lane -- the minor field's version scheme is being fixed separately under #521/#522; this is the interim number to carry PR #539 (dig-node#532) through the version-increment gate. Cargo.lock refreshed in the same commit (cargo update -w --offline) so dig-node-service's locked entry matches -- every CI job runs --locked, and a manifest-only bump here fails Clippy/Test+coverage/all three package builds together on a change that cannot otherwise break a build. Co-Authored-By: Claude <noreply@anthropic.com> * chore(release): bump workspace version to 0.254.43 Per the main lane: main advanced to exactly 0.254.41 after #535's rebase, tying this branch's version. 0.254.43 clears main and every sibling PR in the version-bump queue (#542=0.254.42, #543=0.254.44, #536=0.254.50, #544=0.254.51). Cargo.lock re-synced with `git checkout origin/main -- Cargo.lock` followed by `cargo update -w --offline` (never hand-editing lock conflict markers), confirmed clean with `--dry-run` -> `Locking 0 packages`. Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
….254.62) # Conflicts: # Cargo.lock # Cargo.toml
MichaelTaylor3d
marked this pull request as ready for review
September 3, 2026 20:30
MichaelTaylor3d
marked this pull request as draft
September 3, 2026 20:31
….254.70) # Conflicts: # Cargo.lock # Cargo.toml # crates/dig-wallet/Cargo.toml # crates/dig-wallet/src/sage/rpc.rs
MichaelTaylor3d
marked this pull request as ready for review
September 3, 2026 20:35
MichaelTaylor3d
added a commit
that referenced
this pull request
Sep 3, 2026
…bump (#544) * fix(release): guard the MSI cross-release ordering hazard at a MAJOR bump scripts/package-version.sh's MINOR-overflow carry (#537, closing #521/#522) folds an overflowing MINOR into the idle MSI major field and is monotonic for the whole MAJOR==0 lifetime, but it cannot see a cross-release hazard: a carried release like 0.511.0 (MSI 1.255.0) can compare HIGHER under msiexec's numeric ordering than a later, perfectly legal 1.0.0 (MSI 1.0.0). An in-version guard can't catch this -- 1.0.0 alone is not illegal, only a specific predecessor makes it a downgrade. Add an optional second argument, PREV_VERSION (the previous stable release's bare X.Y.Z). When supplied, the script folds it with the same rule and refuses to emit a version whose MSI tuple compares LOWER than PREV_VERSION's -- turning the "pick a high-enough MAJOR-bump number" decision into a machine-checked one instead of an unchecked human call. Omitting it (every existing package.yml callsite) is byte-for-byte unchanged; ensure-version-increment.yml already checks out both the PR head and main, so a follow-up can wire main's version in at zero extra cost (sequenced separately against #542, which is adding a step to that same job). Measured (dig-node#540): MINOR has never exceeded 255 in any released dig-node version, so nothing shipped is affected today -- this closes the hazard before it can ever be reached rather than reacting once it is. Also corrects "dig_ecosystem#521/#522" references in this script/test/SPEC to "#521/#522" -- those are dig-node's own issues, not the unrelated dig_ecosystem tickets of the same numbers. Revert-proof: stripping the guard reproduces the exact hazard as an accepted exit 0 (0.511.0 -> 1.0.0), and reinstating it passes the full suite (58 ok / 0 fail). Closes #540 Co-Authored-By: Claude <noreply@anthropic.com> * chore(release): re-bump to 0.254.51 -- main took 0.254.41 (#535) Merged origin/main forward (7042f89, #535) rather than rebasing -- the queue across the five open PRs sequences this one last (#542 0.254.42, #539 0.254.43, #543 0.254.44, #536 0.254.50, this PR 0.254.51), deliberately: #542 touches ensure-version-increment.yml, which this PR's follow-up note also targets, so landing last means the final shape is on main rather than guessed at. Re-read the version from Cargo.toml on disk (not the commit log) after the merge, per the standing caution that a rebase can silently drop a bump commit as "already upstream" -- this was a merge, and the version file itself confirms 0.254.51 post-bump. Co-Authored-By: Claude <noreply@anthropic.com> --------- 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
1 participant
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
wallet_reset_coin_dbcomputed its ownnow_msinline fromSystemTime::now()(
crates/dig-node-service/src/control.rs, now moved a few lines by the merge) instead ofrouting through
ClockGovernor(#532/PR #539), so a wall-clock jump mid-reset was evaluatedagainst an undisciplined reading — the #348/#497 double-spend direction: a still-live
reservation reads as already-expired and
wallet_reset_coin_dbbypasses theSpendInFlightrefusal.Closes #541
Status: DRAFT — depends on unmerged PR #539
This branch merges in
loop/532-clock-jump-monotonic(PR #539, still open) to getClockGovernor/reservation_now_ms, which this fix depends on and which #539's own lanecorrectly declined to widen into this crate boundary. This PR must be rebased onto
mainonce #539 merges (a merge, not a rebase — see the standing rule in this repo about
git rebasesilently dropping a patch that already merged).What changed
WalletBackend::reservation_now_ms()(indig-wallet) is nowpub— it needs to becallable from
dig-node-service, a different crate.wallet_reset_coin_dbnow reads itsnow_msfromctx.wallet.reservation_now_ms()instead of a fresh
SystemTime::now()read, sharing the sameClockGovernorclamp stateevery other reservation call site (
reserve_coins,prune_reservations) already uses.SPEC.md'scontrol.wallet.resetCoinDbentry now states explicitly that its clock MUSTbe the same disciplined timeline, not an independent read.
sage::rpc::tests::reset_coin_db_now_must_come_from_the_disciplined_clock_not_a_fresh_read)built from a real
reserve_coinscall (not a hand-placed row, per fix(wallet): bound a reservation deadline by the observing clock, not only by the first push's #528's own lesson)demonstrates the decision point both ways: the disciplined clock still refuses
(
SpendInFlight) seconds after a 60s hold is taken even after a 2-minute wall-clock jump,while an undisciplined jumped reading incorrectly permits the reset — proving why the
control-plane call site must use the disciplined one.
prune_reservations,reserve_client_coins,reserve_spend,reset_chain_cachecallers) in both crates for a directSystemTime::now()read; this was the only production one.Verification
cargo build -p dig-wallet -p dig-node-service— clean.cargo clippy -p dig-wallet -p dig-node-service --all-targets— clean, zero warnings.cargo test -p dig-wallet --lib reservation— 14 passed, 0 failed (includes the newtest and all 13 pre-existing reservation tests, none regressed).
rustfmt --checkon both touched.rsfiles — clean.0.254.44(patch -- coordinator-assigned, avoids a collision with ci(release): make the MSI version-mapping check a required, fast-failing gate #542 which keeps 0.254.42; 0.254.43 reserved for fix(wallet): discipline reservation liveness against a monotonic clock #539), dig-wallet0.49.0(minor —reservation_now_msis a new public API surface).
Do not merge, do not undraft — waiting on #539.