Skip to content

Reservation lifetimes are anchored on wall-clock time, so any clock jump misjudges a live hold #532

Description

@MichaelTaylor3d

Task

Every coin-reservation deadline in dig-wallet is an absolute wall-clock timestamp read from custody::now_ms(), and every release decision compares it against a later reading of the same clock. Neither reading is monotonic, so any jump between them misjudges the hold. Make reservation lifetimes robust to a clock that jumps, in either direction.

Context

Two symptoms of this one root cause are already known and behave oppositely:

  1. Forward jump frees a live bundle. WalletDb::prune_reservations (crates/dig-wallet/src/sage/db.rs) deletes on expires_at <= now_ms with no lower bound, so a forward clock jump past a live deadline retires a bundle that may still be in a mempool and returns its inputs to the selectable set. That is the double-spend direction dig-node#348 and dig-node#497 exist to close. This is present on origin/main today and is NOT changed by PR fix(wallet): bound a reservation deadline by the observing clock, not only by the first push's #528.
  2. Far-forward reading at first push froze a row for ever. dig-node#525. Bounded by PR fix(wallet): bound a reservation deadline by the observing clock, not only by the first push's #528, which repairs a row whose expires_at exceeds now + MAX_RESERVATION_HOLD_MS by re-anchoring it. That fix is an interim bound on this class, not a cure: it detects a contradiction after the fact rather than removing the dependency on wall-clock time.

A monotonic anchor, or a stored boot-epoch that lets elapsed time be computed independently of wall-clock corrections, would make the whole class unreachable. That is a schema change plus a change to every writer and reader of pending_transactions.submitted_at/.expires_at and client_coin_reservations.expires_at_ms, which is why it is filed separately rather than folded into #528.

Act

Decide the anchor representation before implementing — this is a shape decision and a wrong one is expensive to unwind. Weigh at minimum: a monotonic instant is not durable across a process restart, so a purely monotonic anchor cannot survive the node stopping, which is exactly when a stranded reservation matters most. A stored boot-epoch plus a monotonic offset, or a pair of (wall, monotonic) readings whose disagreement is measurable, are the obvious candidates. Say which and why on the ticket before code is written.

Scope

crates/dig-wallet/src/sage/db.rs and rpc.rs, the two reservation tables, and any consumer that reads those columns. A schema migration is required, and existing rows written under the current scheme must be handled rather than assumed absent.

Evidence

Symptom 1 must be reproduced with a failing test asserting at unreserved_unspent_coins — the selection surface — before it is fixed, and the fix must be mutation-proven. A test asserting on a raw table count sits one layer below the decision and would pass under a broken cascade. Check the test COUNT on every run: a filter matching nothing prints 0 passed; N filtered out and exits 0.

Not doing

This ticket does not re-open PR #528's bound, which stands on its own merits as the interim fix.

Refs

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingkind:maintenancereal work that changes nothing a user can perceive

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions