From e7498849e544903c9c79b507382fb973bac684a5 Mon Sep 17 00:00:00 2001 From: Michael Taylor Date: Wed, 2 Sep 2026 05:57:23 -0700 Subject: [PATCH 1/8] 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 --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index e6227d9e..710a96b4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -32,7 +32,7 @@ edition = "2021" # the ROOT manifest (`[workspace.package].version`), so it MUST be set here for a # release to fire (§3.6). The library crates (dig-node-core/dig-runtime/dig-wallet) # keep their own independent versions — only the released binary tracks the workspace version. -version = "0.247.0" +version = "0.251.0" # Release hardening, matching digstore: keep integer-overflow checks ON in release. # The node parses untrusted serialized input and does offset/length arithmetic over From b579f64a9dea63231a678f70451c3eb9f6d0ecd0 Mon Sep 17 00:00:00 2001 From: Michael Taylor Date: Wed, 2 Sep 2026 06:22:45 -0700 Subject: [PATCH 2/8] 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 --- Cargo.lock | 2 +- crates/dig-wallet/src/sage/db.rs | 142 ++++++++++++++++++++++++++++++ crates/dig-wallet/src/sage/rpc.rs | 21 ++++- 3 files changed, 163 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a68a9504..060fd2d1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3031,7 +3031,7 @@ dependencies = [ [[package]] name = "dig-node-service" -version = "0.247.0" +version = "0.251.0" dependencies = [ "async-trait", "axum", diff --git a/crates/dig-wallet/src/sage/db.rs b/crates/dig-wallet/src/sage/db.rs index 7ffca8c6..9738b4de 100644 --- a/crates/dig-wallet/src/sage/db.rs +++ b/crates/dig-wallet/src/sage/db.rs @@ -4691,6 +4691,7 @@ fn is_unique_violation(e: &sqlx::Error) -> bool { #[cfg(test)] mod tests { use super::*; + use super::super::rpc::{MAX_RESERVATION_HOLD_MS, RESERVATION_TTL_MS}; /// **Proves (dig-node#462):** the only public write of `initial_sync_complete` that production /// can reach DISARMS, and it really does clear a flag that was armed. @@ -6476,6 +6477,147 @@ mod tests { assert!(db.pending_transactions().await.unwrap().is_empty()); } + /// **The defect of dig-node#502, at the DB layer.** A caller that re-pushes the same bundle + /// more often than the TTL used to renew its hold forever: `expires_at` was recomputed as + /// `now + RESERVATION_TTL_MS` on every upsert, with no anchor to the FIRST push. The inputs + /// never came back, which is the lockout failure the TTL's own doc names as the worse of the + /// two. + /// + /// The fixture has to outlive a single TTL to see it: a one- or two-push fixture is satisfied + /// by the unfixed code, because the first hold has not lapsed yet. So the loop steps by less + /// than a TTL, past the cap, and the iteration count is asserted rather than assumed. + #[tokio::test] + async fn a_bundle_repushed_forever_still_releases_its_coins_at_the_total_cap() { + let db = WalletDb::open_in_memory().await.unwrap(); + db.upsert_coin(&coin("c1", 100, Some(10), None)) + .await + .unwrap(); + + let first_push = 1_000; + let step = 8 * 60 * 1000; // less than the TTL, so every re-push re-arms under the old rule + let deadline = first_push + MAX_RESERVATION_HOLD_MS; + + let mut pushes = 0; + let mut at = first_push; + while at <= deadline { + db.reserve_spend(&reservation("tx1", &["c1"], first_push, at + RESERVATION_TTL_MS)) + .await + .unwrap(); + assert_eq!( + db.prune_reservations(at).await.unwrap(), + 0, + "the hold must survive while the caller is actively re-pushing (t={at})" + ); + pushes += 1; + at += step; + } + assert!( + pushes >= 8, + "a fixture that pushes {pushes} times cannot outlive one TTL, so it cannot see this \ + defect" + ); + + assert_eq!( + db.prune_reservations(deadline).await.unwrap(), + 1, + "total hold is capped at submitted_at + MAX_RESERVATION_HOLD_MS however many re-pushes \ + arrive" + ); + assert_eq!( + db.unreserved_unspent_coins(None).await.unwrap().len(), + 1, + "the coins must come back to selection at the cap" + ); + } + + /// The cap must not break the property it is bolted onto: INSIDE the cap a re-push still + /// re-arms from `now`, which is what dig-node#348 and #497 rely on to keep a genuinely + /// in-flight bundle's inputs held for a full TTL after its LAST honest transmission. + #[tokio::test] + async fn a_repush_inside_the_cap_still_rearms_from_now() { + let db = WalletDb::open_in_memory().await.unwrap(); + db.upsert_coin(&coin("c1", 100, Some(10), None)) + .await + .unwrap(); + + let first = 1_000; + db.reserve_spend(&reservation("tx1", &["c1"], first, first + RESERVATION_TTL_MS)) + .await + .unwrap(); + + let again = first + 9 * 60 * 1000; + db.reserve_spend(&reservation("tx1", &["c1"], first, again + RESERVATION_TTL_MS)) + .await + .unwrap(); + + let row = &db.pending_transactions().await.unwrap()[0]; + assert_eq!( + row.expires_at, + again + RESERVATION_TTL_MS, + "a re-push inside the cap must extend the deadline, not pin it to the first push" + ); + } + + /// The cap is anchored on `submitted_at`, so the whole bound rests on the upsert never + /// rewriting it. It is not in the `DO UPDATE SET` list today; this pins that, because a later + /// edit adding it there would silently restore the unbounded renewal with every test above + /// still green. + #[tokio::test] + async fn a_repush_never_rewrites_the_first_push_anchor() { + let db = WalletDb::open_in_memory().await.unwrap(); + db.upsert_coin(&coin("c1", 100, Some(10), None)) + .await + .unwrap(); + + db.reserve_spend(&reservation("tx1", &["c1"], 1_000, 1_000 + RESERVATION_TTL_MS)) + .await + .unwrap(); + db.reserve_spend(&reservation("tx1", &["c1"], 500_000, 500_000 + RESERVATION_TTL_MS)) + .await + .unwrap(); + + let row = &db.pending_transactions().await.unwrap()[0]; + assert_eq!( + row.submitted_at, 1_000, + "submitted_at is the first-push anchor and a re-push must never move it" + ); + assert_eq!(row.attempts, 2, "the attempt count still counts re-pushes"); + } + + /// SQLite's `MIN`/`MAX` are AGGREGATES in their one-argument form and scalars only with two or + /// more arguments. The clamp is written with the two-argument scalar forms; a change that left + /// one argument would parse, and would silently mean something else. This drives the clamp + /// through both of its arms at once. + #[tokio::test] + async fn the_clamp_uses_the_scalar_two_argument_min_and_max() { + let db = WalletDb::open_in_memory().await.unwrap(); + db.upsert_coin(&coin("c1", 100, Some(10), None)) + .await + .unwrap(); + + let first = 1_000; + db.reserve_spend(&reservation("tx1", &["c1"], first, first + RESERVATION_TTL_MS)) + .await + .unwrap(); + + // A re-push far beyond the cap: MIN picks the cap, MAX keeps it above the stored value. + db.reserve_spend(&reservation( + "tx1", + &["c1"], + first, + first + 100 * RESERVATION_TTL_MS, + )) + .await + .unwrap(); + + let row = &db.pending_transactions().await.unwrap()[0]; + assert_eq!( + row.expires_at, + first + MAX_RESERVATION_HOLD_MS, + "an over-cap re-push must clamp to the cap exactly" + ); + } + /// Settlement retires the reservation without anything having to remember to release it: the /// coin's own `spent_height` is the signal. #[tokio::test] diff --git a/crates/dig-wallet/src/sage/rpc.rs b/crates/dig-wallet/src/sage/rpc.rs index f4b7aa8e..207548c7 100644 --- a/crates/dig-wallet/src/sage/rpc.rs +++ b/crates/dig-wallet/src/sage/rpc.rs @@ -541,7 +541,26 @@ const DEFAULT_FALLBACK_REFILL_PER_SEC: f64 = 2.0; /// ten minutes is roughly a dozen chances for the spend to land — well past the point where a /// still-unconfirmed bundle is more likely dropped than pending, and short enough that a stranded /// coin returns on a timescale a user waits out rather than reports as lost. -const RESERVATION_TTL_MS: i64 = 10 * 60 * 1000; +pub(crate) const RESERVATION_TTL_MS: i64 = 10 * 60 * 1000; + +/// The most a single bundle's reservation may hold its inputs in TOTAL, measured from the FIRST +/// push rather than from the latest one (dig-node#502): one hour. +/// +/// [`RESERVATION_TTL_MS`] bounds one hold. It does not bound a SEQUENCE of holds: the re-arm on +/// re-push is computed from `now`, so a caller re-pushing the same bundle more often than every +/// TTL renews the hold forever and the inputs never return. That is the lockout failure the TTL's +/// own doc names as the worse of the two, reachable without a single dishonest answer. +/// +/// Expressed as a MULTIPLE of the TTL, in this one place, so the two cannot drift: lengthening the +/// TTL because a bundle needs longer to land also lengthens the total a retrying caller may hold. +/// A cap shorter than the TTL would be a covert shortening of the TTL, which is forbidden. +/// +/// Six is sized by the same question as the TTL. Chia blocks are ~52s apart, so an hour is roughly +/// seventy chances for the spend to land — far past the point where an unconfirmed bundle is more +/// likely dropped than pending — while still returning a stranded coin on a timescale a user waits +/// out. The clamp is anchored on `submitted_at`, which the reservation upsert never rewrites, so it +/// is a bound on the bundle's whole life rather than on any one attempt. +pub(crate) const MAX_RESERVATION_HOLD_MS: i64 = 6 * RESERVATION_TTL_MS; /// The Sage-parity wallet backend. #[derive(Clone)] From 2728e57fb13a7a4a23153cb5d9db3a4fb8e0abc2 Mon Sep 17 00:00:00 2001 From: Michael Taylor Date: Wed, 2 Sep 2026 06:37:38 -0700 Subject: [PATCH 3/8] 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 --- crates/dig-wallet/src/sage/chain.rs | 109 +++++++++++ crates/dig-wallet/src/sage/db.rs | 289 ++++++++++++++++++++++++++-- crates/dig-wallet/src/sage/rpc.rs | 40 +++- 3 files changed, 416 insertions(+), 22 deletions(-) diff --git a/crates/dig-wallet/src/sage/chain.rs b/crates/dig-wallet/src/sage/chain.rs index 96ef1422..e95ccddc 100644 --- a/crates/dig-wallet/src/sage/chain.rs +++ b/crates/dig-wallet/src/sage/chain.rs @@ -285,6 +285,55 @@ pub(crate) fn refusal_is_bundle_intrinsic(stated: &str) -> bool { .any(|intrinsic| reason.eq_ignore_ascii_case(intrinsic)) } +/// Refusals that are about the BUNDLE'S OWN CONTENTS but are deliberately absent from +/// [`BUNDLE_INTRINSIC_REFUSALS`] because the verdict depends on the answering node's HEIGHT and +/// cost budget (dig-node#502). +/// +/// These are the four CLVM-execution / cost names the list above names explicitly as excluded, for +/// exactly that reason: `chia_consensus::spendbundle_validation` derives its flags from +/// `prev_tx_height` and runs under a caller-supplied `max_cost`, so two honest nodes can reach +/// different verdicts on identical bytes. They must keep HOLDING the inputs, and that is unchanged. +/// +/// What this list decides is narrower: whether such a refusal may EXTEND an existing hold. It may +/// not. Extending buys something only when some destination may plausibly still be holding the +/// bundle, and here nothing is: the complaint is about the bytes. Composed with a systematic +/// emitter — a broken wallet-construction path that answers `BLOCK_COST_EXCEEDS_MAX` on every +/// attempt — and a caller that keeps retrying, an unconditional re-arm holds the inputs for as long +/// as the retries continue, for a bundle that was never going to land. +/// +/// # What must NOT be added here, and why a future reader will want to +/// +/// The other absentees from [`BUNDLE_INTRINSIC_REFUSALS`] look like they belong and do not. The +/// timelock assertions (`ASSERT_HEIGHT_*`, `ASSERT_SECONDS_*`, `ASSERT_BEFORE_*`), the fee-policy +/// names, `UNKNOWN_UNSPENT` and `TOO_MANY_ANNOUNCEMENTS` can each be ADMITTED by a DIFFERENT node +/// than the one that refused — a node further ahead, with a different relay policy, or caught up. +/// A later push of the same bundle may therefore genuinely land, so extending the hold is CORRECT +/// for them. The four here differ only in degree, but the degree is the point: a cost or generator +/// verdict does not turn into an acceptance by asking a better-synced peer. +/// +/// Matching is EXACT, case-insensitive and trimmed, via [`refusal_reason`] — never substring or +/// prefix, for the same reason [`refusal_is_bundle_intrinsic`] gives. An unrecognised reason is not +/// on this list, so it EXTENDS: the conservative default, because a hold that lapses too early is +/// the double-select this family exists to close. +const VIEW_DEPENDENT_BUNDLE_CONTENT_REFUSALS: &[&str] = &[ + "GENERATOR_RUNTIME_ERROR", + "BLOCK_COST_EXCEEDS_MAX", + "INVALID_BLOCK_COST", + "INVALID_SPEND_BUNDLE", +]; + +/// Whether a stated refusal is a complaint about the bundle's own contents that no later push can +/// turn into an acceptance — the class that HOLDS but must not EXTEND the hold (dig-node#502). +/// +/// See [`VIEW_DEPENDENT_BUNDLE_CONTENT_REFUSALS`] for the membership rule and for the names that +/// deliberately stay out of it. +pub(crate) fn refusal_forecloses_a_later_push(stated: &str) -> bool { + let reason = refusal_reason(stated); + VIEW_DEPENDENT_BUNDLE_CONTENT_REFUSALS + .iter() + .any(|foreclosing| reason.eq_ignore_ascii_case(foreclosing)) +} + /// Pushes an ALREADY-SIGNED bundle to the network. /// /// A trait rather than a concrete client so the control surface can be driven end to end without a @@ -934,6 +983,66 @@ mod tests { } } + /// **Proves (dig-node#502):** the foreclosing class is recognised exactly, and everything else + /// EXTENDS. + /// + /// The membership rows are the four CLVM-execution / cost names. The shape rows are the ones + /// that matter more than the contents: an unrecognised reason must extend (the conservative + /// default, since a hold that lapses too early is the double-select this family exists to + /// close), and a name EMBEDDED in wider text must not match — the same exact-match discipline + /// `refusal_is_bundle_intrinsic` keeps, because a substring test lets a source turn any string + /// into a non-extending one by quoting a name inside it. + #[test] + fn the_foreclosing_refusals_match_exactly_and_everything_else_extends() { + for foreclosing in [ + "GENERATOR_RUNTIME_ERROR", + "BLOCK_COST_EXCEEDS_MAX", + "INVALID_BLOCK_COST", + "INVALID_SPEND_BUNDLE", + " block_cost_exceeds_max ", + "FAILED: BLOCK_COST_EXCEEDS_MAX", + ] { + assert!( + super::refusal_forecloses_a_later_push(foreclosing), + "{foreclosing} must not renew a hold: trimmed, case-insensitive, exact" + ); + } + + for extending in [ + "DOUBLE_SPEND", + "MEMPOOL_CONFLICT", + "ALREADY_INCLUDING_TRANSACTION", + "UNKNOWN_UNSPENT", + "TOO_MANY_ANNOUNCEMENTS", + "ASSERT_HEIGHT_ABSOLUTE_FAILED", + "ASSERT_SECONDS_RELATIVE_FAILED", + "ASSERT_BEFORE_HEIGHT_ABSOLUTE_FAILED", + "INVALID_FEE_LOW_FEE", + "SOMETHING_NOBODY_ENUMERATED", + "MEMPOOL_CONFLICT (see BLOCK_COST_EXCEEDS_MAX)", + "", + ] { + assert!( + !super::refusal_forecloses_a_later_push(extending), + "{extending:?} may be admitted by a better-synced or differently-configured node, \ + so a later push may land and the hold must still renew" + ); + } + } + + /// The two allowlists answer DIFFERENT questions — free the coins now, versus renew the hold — + /// and a name on both would mean the second list is dead code, because a bundle-intrinsic + /// refusal never reserves at all. + #[test] + fn the_foreclosing_list_is_disjoint_from_the_bundle_intrinsic_one() { + for name in super::VIEW_DEPENDENT_BUNDLE_CONTENT_REFUSALS { + assert!( + !super::refusal_is_bundle_intrinsic(name), + "{name} is on both lists, so its non-extending rule is unreachable" + ); + } + } + /// **Proves (dig-node#460):** the two refusal CLASSES are told apart, and the default is HOLD. /// /// The peer-local rows are not decoration: `ALREADY_INCLUDING_TRANSACTION`, `DOUBLE_SPEND` and diff --git a/crates/dig-wallet/src/sage/db.rs b/crates/dig-wallet/src/sage/db.rs index 9738b4de..cf79de02 100644 --- a/crates/dig-wallet/src/sage/db.rs +++ b/crates/dig-wallet/src/sage/db.rs @@ -224,6 +224,19 @@ pub struct PendingTransactionRow { pub expires_at: i64, /// How many times this bundle has been pushed (1 on first broadcast). pub attempts: i64, + /// Whether THIS attempt may push the deadline further out (dig-node#502). + /// + /// An INPUT to [`WalletDb::reserve_spend`], not a stored column: it is a property of the push + /// that produced this row, not of the bundle. Rows read back out of the database report `true`, + /// the conservative value, because the answer that produced them is not kept. + /// + /// `false` says the last destination complained about the bundle's own CONTENTS in a way no + /// later push can turn into an acceptance + /// (`chain::refusal_forecloses_a_later_push`). The coins still stay held for the + /// remaining hold — the verdict is height- and cost-budget-dependent, so this crate declines to + /// trust one node's view of it — but a retry loop against a systematically broken bundle must + /// not be able to renew that hold forever. + pub may_extend_expiry: bool, /// The coin ids the bundle spends — the coins held out of further selection while it is live. pub reserved_coin_ids: Vec, } @@ -2762,6 +2775,34 @@ impl WalletDb { /// /// Idempotent on the transaction id: re-pushing the same bundle updates its expiry and attempt /// count rather than duplicating it, because a resubmission is the same transaction. + /// + /// # The re-arm is BOUNDED, and the bound is in the SQL (dig-node#502) + /// + /// The expiry used to be taken straight from the incoming row, which is `now + TTL`. That + /// bounds ONE hold and not a SEQUENCE of them: a caller re-pushing the same bundle more often + /// than the TTL renewed its hold forever, and the inputs never returned — the lockout failure + /// `rpc::RESERVATION_TTL_MS`'s own doc names as the worse of the two. + /// + /// Three rules compose to bound it without shortening the TTL: + /// + /// 1. **Clamped to the FIRST push.** `submitted_at` is not in the `DO UPDATE SET` list, so the + /// stored value is a stable anchor, and the new deadline may not exceed + /// `submitted_at + MAX_RESERVATION_HOLD_MS`. + /// 2. **A foreclosing attempt does not extend at all** — see + /// [`PendingTransactionRow::may_extend_expiry`]. + /// 3. **A re-push may never move the deadline EARLIER.** This is what makes rules 1 and 2 add + /// up to the property the ticket asks for without a per-attempt history table: if every + /// attempt is non-extending the deadline never leaves the first `submitted_at + TTL`, and if + /// an extending attempt happened at any point its grant survives every later one. It also + /// keeps a second, later-anchored view of the same transaction from shortening a live hold. + /// + /// The clamp is expressed in SQL rather than computed in the caller because it must be atomic + /// against the STORED anchor: a read-then-write above this layer would race two concurrent + /// pushes of the same bundle. `MIN`/`MAX` are the two-argument SCALAR forms (the one-argument + /// forms are aggregates), which `the_clamp_uses_the_scalar_two_argument_min_and_max` pins. + /// + /// `coin_reservations`' `ON CONFLICT(coin_id) DO NOTHING` below is untouched: a coin still + /// backs exactly one bundle and the FIRST claim still wins. pub async fn reserve_spend(&self, tx: &PendingTransactionRow) -> sqlx::Result<()> { let mut conn = self.pool.begin().await?; sqlx::query( @@ -2769,7 +2810,13 @@ impl WalletDb { (transaction_id, bundle_hex, fee, submitted_at, expires_at, attempts) VALUES (?, ?, ?, ?, ?, ?) ON CONFLICT(transaction_id) DO UPDATE SET - expires_at = excluded.expires_at, + expires_at = CASE + WHEN ? = 0 THEN pending_transactions.expires_at + ELSE MAX( + pending_transactions.expires_at, + MIN(excluded.expires_at, pending_transactions.submitted_at + ?) + ) + END, attempts = pending_transactions.attempts + 1", ) .bind(&tx.transaction_id) @@ -2778,6 +2825,8 @@ impl WalletDb { .bind(tx.submitted_at) .bind(tx.expires_at) .bind(tx.attempts) + .bind(i64::from(tx.may_extend_expiry)) + .bind(super::rpc::MAX_RESERVATION_HOLD_MS) .execute(&mut *conn) .await?; for coin_id in &tx.reserved_coin_ids { @@ -3042,6 +3091,9 @@ impl WalletDb { submitted_at: r.get("submitted_at"), expires_at: r.get("expires_at"), attempts: r.get("attempts"), + // Not a stored column: a row read back describes a bundle, not the push that + // produced it. `true` is the conservative answer (see the field's own doc). + may_extend_expiry: true, reserved_coin_ids: coins.into_iter().map(|c| c.get("coin_id")).collect(), }); } @@ -4690,8 +4742,8 @@ fn is_unique_violation(e: &sqlx::Error) -> bool { #[cfg(test)] mod tests { - use super::*; use super::super::rpc::{MAX_RESERVATION_HOLD_MS, RESERVATION_TTL_MS}; + use super::*; /// **Proves (dig-node#462):** the only public write of `initial_sync_complete` that production /// can reach DISARMS, and it really does clear a flag that was armed. @@ -6390,6 +6442,7 @@ mod tests { submitted_at, expires_at, attempts: 1, + may_extend_expiry: true, reserved_coin_ids: coin_ids.iter().map(|c| (*c).to_string()).collect(), } } @@ -6500,9 +6553,14 @@ mod tests { let mut pushes = 0; let mut at = first_push; while at <= deadline { - db.reserve_spend(&reservation("tx1", &["c1"], first_push, at + RESERVATION_TTL_MS)) - .await - .unwrap(); + db.reserve_spend(&reservation( + "tx1", + &["c1"], + first_push, + at + RESERVATION_TTL_MS, + )) + .await + .unwrap(); assert_eq!( db.prune_reservations(at).await.unwrap(), 0, @@ -6541,14 +6599,24 @@ mod tests { .unwrap(); let first = 1_000; - db.reserve_spend(&reservation("tx1", &["c1"], first, first + RESERVATION_TTL_MS)) - .await - .unwrap(); + db.reserve_spend(&reservation( + "tx1", + &["c1"], + first, + first + RESERVATION_TTL_MS, + )) + .await + .unwrap(); let again = first + 9 * 60 * 1000; - db.reserve_spend(&reservation("tx1", &["c1"], first, again + RESERVATION_TTL_MS)) - .await - .unwrap(); + db.reserve_spend(&reservation( + "tx1", + &["c1"], + first, + again + RESERVATION_TTL_MS, + )) + .await + .unwrap(); let row = &db.pending_transactions().await.unwrap()[0]; assert_eq!( @@ -6569,12 +6637,22 @@ mod tests { .await .unwrap(); - db.reserve_spend(&reservation("tx1", &["c1"], 1_000, 1_000 + RESERVATION_TTL_MS)) - .await - .unwrap(); - db.reserve_spend(&reservation("tx1", &["c1"], 500_000, 500_000 + RESERVATION_TTL_MS)) - .await - .unwrap(); + db.reserve_spend(&reservation( + "tx1", + &["c1"], + 1_000, + 1_000 + RESERVATION_TTL_MS, + )) + .await + .unwrap(); + db.reserve_spend(&reservation( + "tx1", + &["c1"], + 500_000, + 500_000 + RESERVATION_TTL_MS, + )) + .await + .unwrap(); let row = &db.pending_transactions().await.unwrap()[0]; assert_eq!( @@ -6596,9 +6674,14 @@ mod tests { .unwrap(); let first = 1_000; - db.reserve_spend(&reservation("tx1", &["c1"], first, first + RESERVATION_TTL_MS)) - .await - .unwrap(); + db.reserve_spend(&reservation( + "tx1", + &["c1"], + first, + first + RESERVATION_TTL_MS, + )) + .await + .unwrap(); // A re-push far beyond the cap: MIN picks the cap, MAX keeps it above the stored value. db.reserve_spend(&reservation( @@ -6618,6 +6701,169 @@ mod tests { ); } + /// A reservation whose push may not extend the deadline (dig-node#502). + fn non_extending( + tx: &str, + coin_ids: &[&str], + submitted_at: i64, + expires_at: i64, + ) -> PendingTransactionRow { + PendingTransactionRow { + may_extend_expiry: false, + ..reservation(tx, coin_ids, submitted_at, expires_at) + } + } + + /// **The systematic-emitter case the adversarial gate on #497 named.** A broken + /// wallet-construction path answers `BLOCK_COST_EXCEEDS_MAX` on every attempt and a caller + /// keeps retrying. Every one of those refusals is a complaint about the bundle's own contents, + /// so no destination is plausibly holding it and renewing the hold buys nothing. The deadline + /// must therefore stay at the FIRST push's, however many attempts arrive — a strictly tighter + /// bound than the total cap, reached without any clock argument. + #[tokio::test] + async fn a_bundle_refused_only_for_its_own_contents_never_renews_its_hold() { + let db = WalletDb::open_in_memory().await.unwrap(); + db.upsert_coin(&coin("c1", 100, Some(10), None)) + .await + .unwrap(); + + let first = 1_000; + db.reserve_spend(&non_extending( + "tx1", + &["c1"], + first, + first + RESERVATION_TTL_MS, + )) + .await + .unwrap(); + + for attempt in 1..=20 { + let at = first + attempt * 60 * 1000; + db.reserve_spend(&non_extending( + "tx1", + &["c1"], + first, + at + RESERVATION_TTL_MS, + )) + .await + .unwrap(); + } + + let row = &db.pending_transactions().await.unwrap()[0]; + assert_eq!( + row.expires_at, + first + RESERVATION_TTL_MS, + "a refusal no later push can turn into an acceptance must not renew the hold" + ); + assert_eq!(row.attempts, 21, "the attempts are real re-pushes"); + + assert_eq!( + db.prune_reservations(first + RESERVATION_TTL_MS) + .await + .unwrap(), + 1, + "the coins must return at the first push's deadline" + ); + assert_eq!(db.unreserved_unspent_coins(None).await.unwrap().len(), 1); + } + + /// A re-push may never move the deadline EARLIER. This is what lets the two rules above compose + /// into "a bundle whose every observed refusal was foreclosing does not renew" without keeping + /// a per-attempt history: an extending attempt's grant survives every later non-extending one. + #[tokio::test] + async fn a_non_extending_repush_never_shortens_a_deadline_already_granted() { + let db = WalletDb::open_in_memory().await.unwrap(); + db.upsert_coin(&coin("c1", 100, Some(10), None)) + .await + .unwrap(); + + let first = 1_000; + db.reserve_spend(&reservation( + "tx1", + &["c1"], + first, + first + RESERVATION_TTL_MS, + )) + .await + .unwrap(); + + let extended_at = first + 9 * 60 * 1000; + db.reserve_spend(&reservation( + "tx1", + &["c1"], + first, + extended_at + RESERVATION_TTL_MS, + )) + .await + .unwrap(); + + // A later attempt that may not extend, carrying an EARLIER deadline than the granted one. + db.reserve_spend(&non_extending( + "tx1", + &["c1"], + first, + first + RESERVATION_TTL_MS, + )) + .await + .unwrap(); + + let row = &db.pending_transactions().await.unwrap()[0]; + assert_eq!( + row.expires_at, + extended_at + RESERVATION_TTL_MS, + "a non-extending attempt must leave an already-granted deadline alone, never shorten it" + ); + } + + /// The asymmetry the ticket requires to SURVIVE the fix: a coin backs exactly one in-flight + /// bundle and the FIRST claim wins. No path added for the cap may let a second bundle take a + /// coin already reserved, or shorten the first bundle's hold on it. + #[tokio::test] + async fn a_second_bundle_can_neither_take_nor_shorten_a_first_claim() { + let db = WalletDb::open_in_memory().await.unwrap(); + db.upsert_coin(&coin("c1", 100, Some(10), None)) + .await + .unwrap(); + + let first = 1_000; + db.reserve_spend(&reservation( + "tx1", + &["c1"], + first, + first + RESERVATION_TTL_MS, + )) + .await + .unwrap(); + // A second bundle claiming the same coin, non-extending and lapsing immediately. + db.reserve_spend(&non_extending("tx2", &["c1"], first, first + 1)) + .await + .unwrap(); + + assert_eq!( + db.prune_reservations(first + 1).await.unwrap(), + 1, + "only the second bundle lapses; the first still holds its coin" + ); + assert!( + db.unreserved_unspent_coins(None).await.unwrap().is_empty(), + "the first claim must keep the coin out of selection" + ); + + let held: Vec = db + .pending_transactions() + .await + .unwrap() + .into_iter() + .filter(|r| !r.reserved_coin_ids.is_empty()) + .map(|r| r.transaction_id) + .collect(); + assert_eq!( + held, + vec!["tx1".to_string()], + "the coin must still be reserved by the FIRST bundle" + ); + } + /// Settlement retires the reservation without anything having to remember to release it: the /// coin's own `spent_height` is the signal. #[tokio::test] @@ -7195,6 +7441,7 @@ mod tests { submitted_at: NOW, expires_at: NOW + 600_000, attempts: 1, + may_extend_expiry: true, reserved_coin_ids: vec!["aa".into()], }) .await @@ -7562,6 +7809,7 @@ mod tests { submitted_at: NOW, expires_at: NOW + 600_000, attempts: 1, + may_extend_expiry: true, reserved_coin_ids: vec!["bb".into()], }) .await @@ -7592,6 +7840,7 @@ mod tests { submitted_at: NOW, expires_at: NOW + 600_000, attempts: 1, + may_extend_expiry: true, reserved_coin_ids: vec!["aa".into()], }) .await diff --git a/crates/dig-wallet/src/sage/rpc.rs b/crates/dig-wallet/src/sage/rpc.rs index 207548c7..c6eb765a 100644 --- a/crates/dig-wallet/src/sage/rpc.rs +++ b/crates/dig-wallet/src/sage/rpc.rs @@ -2170,7 +2170,8 @@ impl WalletBackend { // mempool by this point, and reporting a push that did happen as an error would be a worse // lie than the double-selection this guards against. if !matches!(&pushed, Ok(o) if Self::is_definitive_rejection(o)) { - if let Err(e) = self.reserve_pushed_bundle(&bundle).await { + let may_extend = Self::attempt_may_extend_the_hold(&pushed); + if let Err(e) = self.reserve_pushed_bundle(&bundle, may_extend).await { tracing::warn!( error = %e, "pushed bundle may be in flight but its coins could not be reserved; a second \ @@ -2181,6 +2182,39 @@ impl WalletBackend { pushed.map_err(|e| PushError::Unreachable(e.to_string())) } + /// Whether THIS push attempt may push an existing reservation's deadline further out + /// (dig-node#502). + /// + /// The hold itself is decided by [`Self::is_definitive_rejection`] and is unchanged; this + /// decides only whether an attempt RENEWS one. The two questions differ because renewal is + /// worth something only while some destination may plausibly still be holding the bundle. When + /// the last answer was a complaint about the bundle's own CONTENTS that no later push can turn + /// into an acceptance, renewing buys nothing — and composed with a caller that retries, an + /// unconditional renewal held the inputs for as long as the retries continued, for a bundle + /// that was never going to land. + /// + /// The default is CONSERVATIVE: everything except a recognised foreclosing reason extends. + /// + /// | outcome | may extend | + /// |---|---| + /// | `Err` — no verdict was reached, so the bundle may be in flight | yes | + /// | accepted | yes | + /// | refused with no stated reason | yes | + /// | refused for a reason in `chain::refusal_forecloses_a_later_push` | **no** | + /// | refused for any other or unrecognised reason | yes | + /// + /// A bundle-intrinsic refusal never reaches this question at all: + /// [`Self::is_definitive_rejection`] skips the reservation entirely for those. + fn attempt_may_extend_the_hold(pushed: &Result) -> bool { + match pushed { + Ok(outcome) if !outcome.accepted => !outcome + .rejection + .as_deref() + .is_some_and(super::chain::refusal_forecloses_a_later_push), + _ => true, + } + } + /// Whether `outcome` is the network DEFINITIVELY refusing this bundle — the only case in which /// its inputs stay selectable (#348). /// @@ -2255,7 +2289,7 @@ impl WalletBackend { /// sign (§908), so a validation failure here says nothing about whether the bundle is /// legitimate — the mempool has already accepted it. An uncomputable fee is stored as `None` /// and reported as `null`, never as zero. - async fn reserve_pushed_bundle(&self, bundle: &SpendBundle) -> Result<()> { + async fn reserve_pushed_bundle(&self, bundle: &SpendBundle, may_extend: bool) -> Result<()> { let now = super::custody::now_ms() as i64; let row = super::db::PendingTransactionRow { transaction_id: hex::encode(bundle.name()), @@ -2266,6 +2300,7 @@ impl WalletBackend { submitted_at: now, expires_at: now + RESERVATION_TTL_MS, attempts: 1, + may_extend_expiry: may_extend, reserved_coin_ids: bundle .coin_spends .iter() @@ -10857,6 +10892,7 @@ mod tests { submitted_at: 1_000, expires_at, attempts: 1, + may_extend_expiry: true, reserved_coin_ids: coin_ids.to_vec(), } } From be772fbf2e9ca8c0d5e237da52bd3dca255344b0 Mon Sep 17 00:00:00 2001 From: Michael Taylor Date: Wed, 2 Sep 2026 08:01:54 -0700 Subject: [PATCH 4/8] chore: bump to 0.256.0, clear of #506's 0.251.0 --- Cargo.lock | 2 +- Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 060fd2d1..f73fb4af 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3031,7 +3031,7 @@ dependencies = [ [[package]] name = "dig-node-service" -version = "0.251.0" +version = "0.256.0" dependencies = [ "async-trait", "axum", diff --git a/Cargo.toml b/Cargo.toml index 710a96b4..d7340e04 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -32,7 +32,7 @@ edition = "2021" # the ROOT manifest (`[workspace.package].version`), so it MUST be set here for a # release to fire (§3.6). The library crates (dig-node-core/dig-runtime/dig-wallet) # keep their own independent versions — only the released binary tracks the workspace version. -version = "0.251.0" +version = "0.256.0" # Release hardening, matching digstore: keep integer-overflow checks ON in release. # The node parses untrusted serialized input and does offset/length arithmetic over From fce235895d7e9aa17c1e351e74b8ddbd99b79044 Mon Sep 17 00:00:00 2001 From: Michael Taylor Date: Wed, 2 Sep 2026 08:08:49 -0700 Subject: [PATCH 5/8] 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 --- SPEC.md | 17 +++ crates/dig-wallet/src/sage/chain.rs | 109 ------------- crates/dig-wallet/src/sage/db.rs | 227 ++++++++++++++++++---------- crates/dig-wallet/src/sage/rpc.rs | 71 ++++----- 4 files changed, 194 insertions(+), 230 deletions(-) diff --git a/SPEC.md b/SPEC.md index e59260cc..b7add90a 100644 --- a/SPEC.md +++ b/SPEC.md @@ -5580,6 +5580,23 @@ thereby return the coins to selection — a second send inside the confirmation reselect the same inputs. The TTL MUST NOT be shortened to compensate for the wider hold: that trades a double-select for a lockout, and a lockout is the worse failure. +The TTL bounds ONE hold and MUST NOT be relied on to bound a SEQUENCE of them. A re-push of the same +transaction renews the hold from the time of that push, so a caller re-pushing more often than the TTL +would otherwise hold the inputs for as long as it kept retrying — the lockout the previous paragraph +names as the worse failure, reachable without a single dishonest answer. A node MUST therefore also +bound the TOTAL hold: the deadline recorded for a transaction MUST NOT exceed the time of its FIRST +observed push plus `MAX_RESERVATION_HOLD_MS`, which MUST be `6 * RESERVATION_TTL_MS` (one hour). + +The first-push time MUST be a stable anchor: a re-push MUST update the deadline and the attempt count +and MUST NOT rewrite it. A re-push MUST NOT move a recorded deadline EARLIER, so that a clock which +steps backwards cannot shorten a hold that is already live. + +This bound is on CONTINUOUS hold. Once the deadline passes, the reservation and its coin claims are +released and the inputs become selectable again; a subsequent push of the same transaction is a new +reservation with a new anchor and MAY hold the inputs for a further full period. A node MUST NOT +refuse to re-hold a transaction on the grounds that it has already held one, since the transaction may +still be admitted and refusing would restore the double-select this section exists to close. + The set of bundle-intrinsic reasons MUST be an ALLOWLIST whose default is to HOLD. The reason text is supplied by an untrusted source (§13), so an unrecognised reason MUST hold rather than free: the enumeration cannot be complete, and a node MUST NOT be made to free a user's inputs by a reason diff --git a/crates/dig-wallet/src/sage/chain.rs b/crates/dig-wallet/src/sage/chain.rs index e95ccddc..96ef1422 100644 --- a/crates/dig-wallet/src/sage/chain.rs +++ b/crates/dig-wallet/src/sage/chain.rs @@ -285,55 +285,6 @@ pub(crate) fn refusal_is_bundle_intrinsic(stated: &str) -> bool { .any(|intrinsic| reason.eq_ignore_ascii_case(intrinsic)) } -/// Refusals that are about the BUNDLE'S OWN CONTENTS but are deliberately absent from -/// [`BUNDLE_INTRINSIC_REFUSALS`] because the verdict depends on the answering node's HEIGHT and -/// cost budget (dig-node#502). -/// -/// These are the four CLVM-execution / cost names the list above names explicitly as excluded, for -/// exactly that reason: `chia_consensus::spendbundle_validation` derives its flags from -/// `prev_tx_height` and runs under a caller-supplied `max_cost`, so two honest nodes can reach -/// different verdicts on identical bytes. They must keep HOLDING the inputs, and that is unchanged. -/// -/// What this list decides is narrower: whether such a refusal may EXTEND an existing hold. It may -/// not. Extending buys something only when some destination may plausibly still be holding the -/// bundle, and here nothing is: the complaint is about the bytes. Composed with a systematic -/// emitter — a broken wallet-construction path that answers `BLOCK_COST_EXCEEDS_MAX` on every -/// attempt — and a caller that keeps retrying, an unconditional re-arm holds the inputs for as long -/// as the retries continue, for a bundle that was never going to land. -/// -/// # What must NOT be added here, and why a future reader will want to -/// -/// The other absentees from [`BUNDLE_INTRINSIC_REFUSALS`] look like they belong and do not. The -/// timelock assertions (`ASSERT_HEIGHT_*`, `ASSERT_SECONDS_*`, `ASSERT_BEFORE_*`), the fee-policy -/// names, `UNKNOWN_UNSPENT` and `TOO_MANY_ANNOUNCEMENTS` can each be ADMITTED by a DIFFERENT node -/// than the one that refused — a node further ahead, with a different relay policy, or caught up. -/// A later push of the same bundle may therefore genuinely land, so extending the hold is CORRECT -/// for them. The four here differ only in degree, but the degree is the point: a cost or generator -/// verdict does not turn into an acceptance by asking a better-synced peer. -/// -/// Matching is EXACT, case-insensitive and trimmed, via [`refusal_reason`] — never substring or -/// prefix, for the same reason [`refusal_is_bundle_intrinsic`] gives. An unrecognised reason is not -/// on this list, so it EXTENDS: the conservative default, because a hold that lapses too early is -/// the double-select this family exists to close. -const VIEW_DEPENDENT_BUNDLE_CONTENT_REFUSALS: &[&str] = &[ - "GENERATOR_RUNTIME_ERROR", - "BLOCK_COST_EXCEEDS_MAX", - "INVALID_BLOCK_COST", - "INVALID_SPEND_BUNDLE", -]; - -/// Whether a stated refusal is a complaint about the bundle's own contents that no later push can -/// turn into an acceptance — the class that HOLDS but must not EXTEND the hold (dig-node#502). -/// -/// See [`VIEW_DEPENDENT_BUNDLE_CONTENT_REFUSALS`] for the membership rule and for the names that -/// deliberately stay out of it. -pub(crate) fn refusal_forecloses_a_later_push(stated: &str) -> bool { - let reason = refusal_reason(stated); - VIEW_DEPENDENT_BUNDLE_CONTENT_REFUSALS - .iter() - .any(|foreclosing| reason.eq_ignore_ascii_case(foreclosing)) -} - /// Pushes an ALREADY-SIGNED bundle to the network. /// /// A trait rather than a concrete client so the control surface can be driven end to end without a @@ -983,66 +934,6 @@ mod tests { } } - /// **Proves (dig-node#502):** the foreclosing class is recognised exactly, and everything else - /// EXTENDS. - /// - /// The membership rows are the four CLVM-execution / cost names. The shape rows are the ones - /// that matter more than the contents: an unrecognised reason must extend (the conservative - /// default, since a hold that lapses too early is the double-select this family exists to - /// close), and a name EMBEDDED in wider text must not match — the same exact-match discipline - /// `refusal_is_bundle_intrinsic` keeps, because a substring test lets a source turn any string - /// into a non-extending one by quoting a name inside it. - #[test] - fn the_foreclosing_refusals_match_exactly_and_everything_else_extends() { - for foreclosing in [ - "GENERATOR_RUNTIME_ERROR", - "BLOCK_COST_EXCEEDS_MAX", - "INVALID_BLOCK_COST", - "INVALID_SPEND_BUNDLE", - " block_cost_exceeds_max ", - "FAILED: BLOCK_COST_EXCEEDS_MAX", - ] { - assert!( - super::refusal_forecloses_a_later_push(foreclosing), - "{foreclosing} must not renew a hold: trimmed, case-insensitive, exact" - ); - } - - for extending in [ - "DOUBLE_SPEND", - "MEMPOOL_CONFLICT", - "ALREADY_INCLUDING_TRANSACTION", - "UNKNOWN_UNSPENT", - "TOO_MANY_ANNOUNCEMENTS", - "ASSERT_HEIGHT_ABSOLUTE_FAILED", - "ASSERT_SECONDS_RELATIVE_FAILED", - "ASSERT_BEFORE_HEIGHT_ABSOLUTE_FAILED", - "INVALID_FEE_LOW_FEE", - "SOMETHING_NOBODY_ENUMERATED", - "MEMPOOL_CONFLICT (see BLOCK_COST_EXCEEDS_MAX)", - "", - ] { - assert!( - !super::refusal_forecloses_a_later_push(extending), - "{extending:?} may be admitted by a better-synced or differently-configured node, \ - so a later push may land and the hold must still renew" - ); - } - } - - /// The two allowlists answer DIFFERENT questions — free the coins now, versus renew the hold — - /// and a name on both would mean the second list is dead code, because a bundle-intrinsic - /// refusal never reserves at all. - #[test] - fn the_foreclosing_list_is_disjoint_from_the_bundle_intrinsic_one() { - for name in super::VIEW_DEPENDENT_BUNDLE_CONTENT_REFUSALS { - assert!( - !super::refusal_is_bundle_intrinsic(name), - "{name} is on both lists, so its non-extending rule is unreachable" - ); - } - } - /// **Proves (dig-node#460):** the two refusal CLASSES are told apart, and the default is HOLD. /// /// The peer-local rows are not decoration: `ALREADY_INCLUDING_TRANSACTION`, `DOUBLE_SPEND` and diff --git a/crates/dig-wallet/src/sage/db.rs b/crates/dig-wallet/src/sage/db.rs index cf79de02..428ae8fd 100644 --- a/crates/dig-wallet/src/sage/db.rs +++ b/crates/dig-wallet/src/sage/db.rs @@ -224,19 +224,6 @@ pub struct PendingTransactionRow { pub expires_at: i64, /// How many times this bundle has been pushed (1 on first broadcast). pub attempts: i64, - /// Whether THIS attempt may push the deadline further out (dig-node#502). - /// - /// An INPUT to [`WalletDb::reserve_spend`], not a stored column: it is a property of the push - /// that produced this row, not of the bundle. Rows read back out of the database report `true`, - /// the conservative value, because the answer that produced them is not kept. - /// - /// `false` says the last destination complained about the bundle's own CONTENTS in a way no - /// later push can turn into an acceptance - /// (`chain::refusal_forecloses_a_later_push`). The coins still stay held for the - /// remaining hold — the verdict is height- and cost-budget-dependent, so this crate declines to - /// trust one node's view of it — but a retry loop against a systematically broken bundle must - /// not be able to renew that hold forever. - pub may_extend_expiry: bool, /// The coin ids the bundle spends — the coins held out of further selection while it is live. pub reserved_coin_ids: Vec, } @@ -2783,17 +2770,16 @@ impl WalletDb { /// than the TTL renewed its hold forever, and the inputs never returned — the lockout failure /// `rpc::RESERVATION_TTL_MS`'s own doc names as the worse of the two. /// - /// Three rules compose to bound it without shortening the TTL: + /// Two rules compose to bound it without shortening the TTL: /// /// 1. **Clamped to the FIRST push.** `submitted_at` is not in the `DO UPDATE SET` list, so the /// stored value is a stable anchor, and the new deadline may not exceed /// `submitted_at + MAX_RESERVATION_HOLD_MS`. - /// 2. **A foreclosing attempt does not extend at all** — see - /// [`PendingTransactionRow::may_extend_expiry`]. - /// 3. **A re-push may never move the deadline EARLIER.** This is what makes rules 1 and 2 add - /// up to the property the ticket asks for without a per-attempt history table: if every - /// attempt is non-extending the deadline never leaves the first `submitted_at + TTL`, and if - /// an extending attempt happened at any point its grant survives every later one. It also + /// 2. **A re-push may never move the deadline EARLIER.** The incoming `expires_at` is + /// `now + TTL`, so under a MONOTONIC clock the outer `MAX` never binds. It exists for the + /// non-monotonic one: an NTP step or a manual clock change moves `now` backwards, and + /// without the `MAX` that re-push would SHORTEN a live hold — returning the inputs of a + /// bundle that may still land, which is the direction #348/#497 exist to close. It also /// keeps a second, later-anchored view of the same transaction from shortening a live hold. /// /// The clamp is expressed in SQL rather than computed in the caller because it must be atomic @@ -2801,6 +2787,12 @@ impl WalletDb { /// pushes of the same bundle. `MIN`/`MAX` are the two-argument SCALAR forms (the one-argument /// forms are aggregates), which `the_clamp_uses_the_scalar_two_argument_min_and_max` pins. /// + /// The clamp fails OPEN, not closed, under an absurd `submitted_at`: SQLite does not trap + /// `INTEGER` overflow on `+` but promotes the result to `REAL`, so an anchor near `i64::MAX` + /// makes `submitted_at + MAX_RESERVATION_HOLD_MS` exceed any plausible incoming deadline and + /// the cap stops binding. `submitted_at` comes from `custody::now_ms()` and is never + /// caller-supplied, so reaching it needs a wildly wrong system clock rather than an attacker. + /// /// `coin_reservations`' `ON CONFLICT(coin_id) DO NOTHING` below is untouched: a coin still /// backs exactly one bundle and the FIRST claim still wins. pub async fn reserve_spend(&self, tx: &PendingTransactionRow) -> sqlx::Result<()> { @@ -2810,13 +2802,10 @@ impl WalletDb { (transaction_id, bundle_hex, fee, submitted_at, expires_at, attempts) VALUES (?, ?, ?, ?, ?, ?) ON CONFLICT(transaction_id) DO UPDATE SET - expires_at = CASE - WHEN ? = 0 THEN pending_transactions.expires_at - ELSE MAX( - pending_transactions.expires_at, - MIN(excluded.expires_at, pending_transactions.submitted_at + ?) - ) - END, + expires_at = MAX( + pending_transactions.expires_at, + MIN(excluded.expires_at, pending_transactions.submitted_at + ?) + ), attempts = pending_transactions.attempts + 1", ) .bind(&tx.transaction_id) @@ -2825,7 +2814,6 @@ impl WalletDb { .bind(tx.submitted_at) .bind(tx.expires_at) .bind(tx.attempts) - .bind(i64::from(tx.may_extend_expiry)) .bind(super::rpc::MAX_RESERVATION_HOLD_MS) .execute(&mut *conn) .await?; @@ -3093,7 +3081,6 @@ impl WalletDb { attempts: r.get("attempts"), // Not a stored column: a row read back describes a bundle, not the push that // produced it. `true` is the conservative answer (see the field's own doc). - may_extend_expiry: true, reserved_coin_ids: coins.into_iter().map(|c| c.get("coin_id")).collect(), }); } @@ -6442,7 +6429,6 @@ mod tests { submitted_at, expires_at, attempts: 1, - may_extend_expiry: true, reserved_coin_ids: coin_ids.iter().map(|c| (*c).to_string()).collect(), } } @@ -6701,34 +6687,94 @@ mod tests { ); } - /// A reservation whose push may not extend the deadline (dig-node#502). - fn non_extending( - tx: &str, - coin_ids: &[&str], - submitted_at: i64, - expires_at: i64, - ) -> PendingTransactionRow { - PendingTransactionRow { - may_extend_expiry: false, - ..reservation(tx, coin_ids, submitted_at, expires_at) - } + /// **The SAWTOOTH the cap deliberately leaves open (dig-node#502).** `prune_reservations` + /// DELETEs the row at the cap, so `submitted_at` anchors the clamp only while the row exists. + /// A re-push after the prune INSERTs a fresh row with a NEW anchor and a full new hour. + /// + /// Pinned as known, intended behaviour rather than left as an unmeasured surprise: the bound + /// is on CONTINUOUS hold, not aggregate hold. Closing it would mean permanently declining to + /// re-hold a bundle that already had its hour, which is the double-spend direction, and it + /// would need the per-bundle history this ticket deliberately avoided. + #[tokio::test] + async fn a_repushed_bundle_gets_a_fresh_anchor_after_the_cap_prunes_its_row() { + let db = WalletDb::open_in_memory().await.unwrap(); + db.upsert_coin(&coin("c1", 100, Some(10), None)) + .await + .unwrap(); + + // The clamp lives in the `ON CONFLICT` arm, so it binds on RE-push. The first INSERT takes + // its deadline verbatim, which in production is always `now + TTL`. + let first = 1_000; + db.reserve_spend(&reservation( + "tx1", + &["c1"], + first, + first + RESERVATION_TTL_MS, + )) + .await + .unwrap(); + db.reserve_spend(&reservation( + "tx1", + &["c1"], + first, + first + 100 * RESERVATION_TTL_MS, + )) + .await + .unwrap(); + assert_eq!( + db.pending_transactions().await.unwrap()[0].expires_at, + first + MAX_RESERVATION_HOLD_MS, + "a retrying caller's hold is clamped to the cap" + ); + + let capped_at = first + MAX_RESERVATION_HOLD_MS; + assert_eq!( + db.prune_reservations(capped_at).await.unwrap(), + 1, + "the cap must actually release the coins" + ); + assert_eq!(db.unreserved_unspent_coins(None).await.unwrap().len(), 1); + + // The same bundle, pushed again after the release: a NEW row, so a NEW anchor. + let second = capped_at + 1; + db.reserve_spend(&reservation( + "tx1", + &["c1"], + second, + second + RESERVATION_TTL_MS, + )) + .await + .unwrap(); + + let row = &db.pending_transactions().await.unwrap()[0]; + assert_eq!( + row.submitted_at, second, + "the pruned row is gone, so the re-push anchors on its own push time" + ); + assert_eq!( + row.attempts, 1, + "a fresh row starts its attempt count over: this is an INSERT, not the upsert path" + ); + assert_eq!( + row.expires_at, + second + RESERVATION_TTL_MS, + "and it may accumulate a full new hour from the new anchor" + ); } - /// **The systematic-emitter case the adversarial gate on #497 named.** A broken - /// wallet-construction path answers `BLOCK_COST_EXCEEDS_MAX` on every attempt and a caller - /// keeps retrying. Every one of those refusals is a complaint about the bundle's own contents, - /// so no destination is plausibly holding it and renewing the hold buys nothing. The deadline - /// must therefore stay at the FIRST push's, however many attempts arrive — a strictly tighter - /// bound than the total cap, reached without any clock argument. + /// **The clamp's boundary (dig-node#502).** Inside the last TTL before the cap the clamp binds, + /// so a re-push extends the deadline by strictly LESS than a full TTL, shrinking to nothing + /// exactly at the cap. Correct and intended — it is what a clamp does — but asserted from BOTH + /// sides, because a bound tested only from below can only confirm itself. #[tokio::test] - async fn a_bundle_refused_only_for_its_own_contents_never_renews_its_hold() { + async fn a_repush_inside_the_last_ttl_before_the_cap_buys_less_than_a_full_ttl() { let db = WalletDb::open_in_memory().await.unwrap(); db.upsert_coin(&coin("c1", 100, Some(10), None)) .await .unwrap(); let first = 1_000; - db.reserve_spend(&non_extending( + db.reserve_spend(&reservation( "tx1", &["c1"], first, @@ -6737,41 +6783,63 @@ mod tests { .await .unwrap(); - for attempt in 1..=20 { - let at = first + attempt * 60 * 1000; - db.reserve_spend(&non_extending( - "tx1", - &["c1"], - first, - at + RESERVATION_TTL_MS, - )) + // BELOW the boundary: a re-push at 4 x TTL wants 5 x TTL, which is under the cap, so the + // clamp does not bind and the full TTL is granted. + let under = first + 4 * RESERVATION_TTL_MS; + db.reserve_spend(&reservation("tx1", &["c1"], first, under + RESERVATION_TTL_MS)) .await .unwrap(); - } + assert_eq!( + db.pending_transactions().await.unwrap()[0].expires_at, + under + RESERVATION_TTL_MS, + "under the boundary the clamp must not bind: a full TTL is granted" + ); - let row = &db.pending_transactions().await.unwrap()[0]; + // OVER the boundary: a re-push half a TTL later wants 5.5 x TTL past the anchor. The cap + // is 6 x TTL, so it is granted in full — the last push that still buys everything it asks. + let at_edge = first + 5 * RESERVATION_TTL_MS; + db.reserve_spend(&reservation( + "tx1", + &["c1"], + first, + at_edge + RESERVATION_TTL_MS, + )) + .await + .unwrap(); assert_eq!( - row.expires_at, - first + RESERVATION_TTL_MS, - "a refusal no later push can turn into an acceptance must not renew the hold" + db.pending_transactions().await.unwrap()[0].expires_at, + first + MAX_RESERVATION_HOLD_MS, + "a push at 5 x TTL asks for exactly the cap and gets exactly the cap" ); - assert_eq!(row.attempts, 21, "the attempts are real re-pushes"); + // PAST it: the next re-push buys strictly less than a TTL — in fact nothing at all. + let past = first + 5 * RESERVATION_TTL_MS + 60_000; + db.reserve_spend(&reservation("tx1", &["c1"], first, past + RESERVATION_TTL_MS)) + .await + .unwrap(); + let row = &db.pending_transactions().await.unwrap()[0]; assert_eq!( - db.prune_reservations(first + RESERVATION_TTL_MS) - .await - .unwrap(), - 1, - "the coins must return at the first push's deadline" + row.expires_at, + first + MAX_RESERVATION_HOLD_MS, + "past the boundary a re-push buys strictly less than a TTL, and at the cap it buys zero" + ); + assert!( + row.expires_at < past + RESERVATION_TTL_MS, + "the granted deadline must be short of what the push asked for" ); - assert_eq!(db.unreserved_unspent_coins(None).await.unwrap().len(), 1); } - /// A re-push may never move the deadline EARLIER. This is what lets the two rules above compose - /// into "a bundle whose every observed refusal was foreclosing does not renew" without keeping - /// a per-attempt history: an extending attempt's grant survives every later non-extending one. + /// **Why the clamp keeps its outer `MAX` (dig-node#502).** With the deadline written as + /// `MIN(excluded.expires_at, submitted_at + cap)` alone, the incoming value is `now + TTL` and + /// a MONOTONIC clock can never make that smaller than a deadline already stored. A + /// non-monotonic one can: an NTP step or a manual clock change moves `now` backwards, and the + /// re-push then carries an EARLIER deadline than the live hold. + /// + /// Shortening a live hold is the dangerous direction — it returns the inputs of a bundle that + /// may still land, which is the double-select #348/#497 exist to close — so the outer `MAX` + /// makes a re-push able only ever to move the deadline outwards, never inwards. #[tokio::test] - async fn a_non_extending_repush_never_shortens_a_deadline_already_granted() { + async fn a_repush_under_a_clock_that_stepped_backwards_never_shortens_a_live_hold() { let db = WalletDb::open_in_memory().await.unwrap(); db.upsert_coin(&coin("c1", 100, Some(10), None)) .await @@ -6797,8 +6865,8 @@ mod tests { .await .unwrap(); - // A later attempt that may not extend, carrying an EARLIER deadline than the granted one. - db.reserve_spend(&non_extending( + // The clock steps backwards, so this re-push carries an EARLIER deadline than the live one. + db.reserve_spend(&reservation( "tx1", &["c1"], first, @@ -6811,7 +6879,7 @@ mod tests { assert_eq!( row.expires_at, extended_at + RESERVATION_TTL_MS, - "a non-extending attempt must leave an already-granted deadline alone, never shorten it" + "a backwards clock step must leave an already-granted deadline alone, never shorten it" ); } @@ -6834,8 +6902,8 @@ mod tests { )) .await .unwrap(); - // A second bundle claiming the same coin, non-extending and lapsing immediately. - db.reserve_spend(&non_extending("tx2", &["c1"], first, first + 1)) + // A second bundle claiming the same coin, lapsing immediately. + db.reserve_spend(&reservation("tx2", &["c1"], first, first + 1)) .await .unwrap(); @@ -7441,7 +7509,6 @@ mod tests { submitted_at: NOW, expires_at: NOW + 600_000, attempts: 1, - may_extend_expiry: true, reserved_coin_ids: vec!["aa".into()], }) .await @@ -7809,7 +7876,6 @@ mod tests { submitted_at: NOW, expires_at: NOW + 600_000, attempts: 1, - may_extend_expiry: true, reserved_coin_ids: vec!["bb".into()], }) .await @@ -7840,7 +7906,6 @@ mod tests { submitted_at: NOW, expires_at: NOW + 600_000, attempts: 1, - may_extend_expiry: true, reserved_coin_ids: vec!["aa".into()], }) .await diff --git a/crates/dig-wallet/src/sage/rpc.rs b/crates/dig-wallet/src/sage/rpc.rs index c6eb765a..6eb83d1b 100644 --- a/crates/dig-wallet/src/sage/rpc.rs +++ b/crates/dig-wallet/src/sage/rpc.rs @@ -558,8 +558,35 @@ pub(crate) const RESERVATION_TTL_MS: i64 = 10 * 60 * 1000; /// Six is sized by the same question as the TTL. Chia blocks are ~52s apart, so an hour is roughly /// seventy chances for the spend to land — far past the point where an unconfirmed bundle is more /// likely dropped than pending — while still returning a stranded coin on a timescale a user waits -/// out. The clamp is anchored on `submitted_at`, which the reservation upsert never rewrites, so it -/// is a bound on the bundle's whole life rather than on any one attempt. +/// out. +/// +/// # What this bound is NOT +/// +/// Three limitations are deliberate. Each is the price of having a finite cap at all, and each is +/// stated here because the bound is otherwise easy to read as stronger than it is. +/// +/// 1. **It bounds a CONTINUOUS hold, not an AGGREGATE one.** `submitted_at` anchors the clamp only +/// while the row exists, and `WalletDb::prune_reservations` DELETEs the row at the cap. The next +/// re-push therefore INSERTs a fresh row with a new `submitted_at` and a full new hour, so an +/// indefinitely retrying caller produces a SAWTOOTH — one-hour holds separated by an instant of +/// selectability — rather than one bounded total across the bundle's life. This is intended: at +/// each release the coins were genuinely selectable again, and refusing to ever re-hold a bundle +/// that already had its hour would mean permanently declining to protect a bundle that may still +/// land, which is the double-spend direction. Pinned by +/// `a_repushed_bundle_gets_a_fresh_anchor_after_the_cap_prunes_its_row`. +/// 2. **A bundle whose TIMELOCK matures later than the cap has its inputs freed while still +/// valid.** This is the one class where the network genuinely retains the bundle — a node +/// answers PENDING rather than FAILED for an unmet `ASSERT_HEIGHT_ABSOLUTE` or +/// `ASSERT_SECONDS_ABSOLUTE` — so the inputs are released while some mempool is really still +/// holding it, and it will be admitted once the condition is met. Accepted rather than fixed: the +/// release is CLOCK-driven, so no peer can advance it and the case carries no attacker leverage; +/// this node builds no timelocked bundles of its own; and the alternative is the indefinite +/// lockout this constant exists to close. +/// 3. **Near the cap, a re-push buys strictly LESS than a full TTL.** Between +/// `submitted_at + 5 * RESERVATION_TTL_MS` and the cap the clamp binds, so each renewal extends +/// the deadline by a shrinking amount that reaches zero exactly at the cap. That is what a clamp +/// does rather than a defect, and +/// `a_repush_inside_the_last_ttl_before_the_cap_buys_less_than_a_full_ttl` pins the boundary. pub(crate) const MAX_RESERVATION_HOLD_MS: i64 = 6 * RESERVATION_TTL_MS; /// The Sage-parity wallet backend. @@ -2170,8 +2197,7 @@ impl WalletBackend { // mempool by this point, and reporting a push that did happen as an error would be a worse // lie than the double-selection this guards against. if !matches!(&pushed, Ok(o) if Self::is_definitive_rejection(o)) { - let may_extend = Self::attempt_may_extend_the_hold(&pushed); - if let Err(e) = self.reserve_pushed_bundle(&bundle, may_extend).await { + if let Err(e) = self.reserve_pushed_bundle(&bundle).await { tracing::warn!( error = %e, "pushed bundle may be in flight but its coins could not be reserved; a second \ @@ -2182,39 +2208,6 @@ impl WalletBackend { pushed.map_err(|e| PushError::Unreachable(e.to_string())) } - /// Whether THIS push attempt may push an existing reservation's deadline further out - /// (dig-node#502). - /// - /// The hold itself is decided by [`Self::is_definitive_rejection`] and is unchanged; this - /// decides only whether an attempt RENEWS one. The two questions differ because renewal is - /// worth something only while some destination may plausibly still be holding the bundle. When - /// the last answer was a complaint about the bundle's own CONTENTS that no later push can turn - /// into an acceptance, renewing buys nothing — and composed with a caller that retries, an - /// unconditional renewal held the inputs for as long as the retries continued, for a bundle - /// that was never going to land. - /// - /// The default is CONSERVATIVE: everything except a recognised foreclosing reason extends. - /// - /// | outcome | may extend | - /// |---|---| - /// | `Err` — no verdict was reached, so the bundle may be in flight | yes | - /// | accepted | yes | - /// | refused with no stated reason | yes | - /// | refused for a reason in `chain::refusal_forecloses_a_later_push` | **no** | - /// | refused for any other or unrecognised reason | yes | - /// - /// A bundle-intrinsic refusal never reaches this question at all: - /// [`Self::is_definitive_rejection`] skips the reservation entirely for those. - fn attempt_may_extend_the_hold(pushed: &Result) -> bool { - match pushed { - Ok(outcome) if !outcome.accepted => !outcome - .rejection - .as_deref() - .is_some_and(super::chain::refusal_forecloses_a_later_push), - _ => true, - } - } - /// Whether `outcome` is the network DEFINITIVELY refusing this bundle — the only case in which /// its inputs stay selectable (#348). /// @@ -2289,7 +2282,7 @@ impl WalletBackend { /// sign (§908), so a validation failure here says nothing about whether the bundle is /// legitimate — the mempool has already accepted it. An uncomputable fee is stored as `None` /// and reported as `null`, never as zero. - async fn reserve_pushed_bundle(&self, bundle: &SpendBundle, may_extend: bool) -> Result<()> { + async fn reserve_pushed_bundle(&self, bundle: &SpendBundle) -> Result<()> { let now = super::custody::now_ms() as i64; let row = super::db::PendingTransactionRow { transaction_id: hex::encode(bundle.name()), @@ -2300,7 +2293,6 @@ impl WalletBackend { submitted_at: now, expires_at: now + RESERVATION_TTL_MS, attempts: 1, - may_extend_expiry: may_extend, reserved_coin_ids: bundle .coin_spends .iter() @@ -10892,7 +10884,6 @@ mod tests { submitted_at: 1_000, expires_at, attempts: 1, - may_extend_expiry: true, reserved_coin_ids: coin_ids.to_vec(), } } From 10f0533cc4e140d3058b98c78316fa90a61841e6 Mon Sep 17 00:00:00 2001 From: Michael Taylor Date: Wed, 2 Sep 2026 19:36:44 -0700 Subject: [PATCH 6/8] chore: renumber to 0.252.8, under the MSI ProductVersion ceiling (#521) --- Cargo.lock | 2 +- Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0099923a..7a9eb424 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3031,7 +3031,7 @@ dependencies = [ [[package]] name = "dig-node-service" -version = "0.256.0" +version = "0.252.8" dependencies = [ "async-trait", "axum", diff --git a/Cargo.toml b/Cargo.toml index d7340e04..09b383cf 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -32,7 +32,7 @@ edition = "2021" # the ROOT manifest (`[workspace.package].version`), so it MUST be set here for a # release to fire (§3.6). The library crates (dig-node-core/dig-runtime/dig-wallet) # keep their own independent versions — only the released binary tracks the workspace version. -version = "0.256.0" +version = "0.252.8" # Release hardening, matching digstore: keep integer-overflow checks ON in release. # The node parses untrusted serialized input and does offset/length arithmetic over From 11d46af97bb914d8cfd1d6c36a84a254eed7bfb7 Mon Sep 17 00:00:00 2001 From: Michael Taylor Date: Wed, 2 Sep 2026 19:56:27 -0700 Subject: [PATCH 7/8] 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 --- crates/dig-wallet/src/sage/db.rs | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/crates/dig-wallet/src/sage/db.rs b/crates/dig-wallet/src/sage/db.rs index 428ae8fd..1718cbf7 100644 --- a/crates/dig-wallet/src/sage/db.rs +++ b/crates/dig-wallet/src/sage/db.rs @@ -6786,9 +6786,14 @@ mod tests { // BELOW the boundary: a re-push at 4 x TTL wants 5 x TTL, which is under the cap, so the // clamp does not bind and the full TTL is granted. let under = first + 4 * RESERVATION_TTL_MS; - db.reserve_spend(&reservation("tx1", &["c1"], first, under + RESERVATION_TTL_MS)) - .await - .unwrap(); + db.reserve_spend(&reservation( + "tx1", + &["c1"], + first, + under + RESERVATION_TTL_MS, + )) + .await + .unwrap(); assert_eq!( db.pending_transactions().await.unwrap()[0].expires_at, under + RESERVATION_TTL_MS, @@ -6814,9 +6819,14 @@ mod tests { // PAST it: the next re-push buys strictly less than a TTL — in fact nothing at all. let past = first + 5 * RESERVATION_TTL_MS + 60_000; - db.reserve_spend(&reservation("tx1", &["c1"], first, past + RESERVATION_TTL_MS)) - .await - .unwrap(); + db.reserve_spend(&reservation( + "tx1", + &["c1"], + first, + past + RESERVATION_TTL_MS, + )) + .await + .unwrap(); let row = &db.pending_transactions().await.unwrap()[0]; assert_eq!( row.expires_at, From e01173c5e91ba72cd137b34838cd1e6503a9c2e8 Mon Sep 17 00:00:00 2001 From: Michael Taylor Date: Wed, 2 Sep 2026 20:05:52 -0700 Subject: [PATCH 8/8] 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 --- SPEC.md | 8 +++++--- crates/dig-wallet/src/sage/db.rs | 2 -- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/SPEC.md b/SPEC.md index 2aa3c563..9c91cddf 100644 --- a/SPEC.md +++ b/SPEC.md @@ -5751,9 +5751,11 @@ names as the worse failure, reachable without a single dishonest answer. A node bound the TOTAL hold: the deadline recorded for a transaction MUST NOT exceed the time of its FIRST observed push plus `MAX_RESERVATION_HOLD_MS`, which MUST be `6 * RESERVATION_TTL_MS` (one hour). -The first-push time MUST be a stable anchor: a re-push MUST update the deadline and the attempt count -and MUST NOT rewrite it. A re-push MUST NOT move a recorded deadline EARLIER, so that a clock which -steps backwards cannot shorten a hold that is already live. +The first-push time MUST be a stable anchor: a re-push MUST re-arm the deadline subject to the bound +above — which at or past the cap, or under a clock that has stepped backwards, leaves it unchanged — +MUST increment the attempt count, and MUST NOT rewrite the anchor. A re-push MUST NOT move a +recorded deadline EARLIER, so that a clock which steps backwards cannot shorten a hold that is +already live. This bound is on CONTINUOUS hold. Once the deadline passes, the reservation and its coin claims are released and the inputs become selectable again; a subsequent push of the same transaction is a new diff --git a/crates/dig-wallet/src/sage/db.rs b/crates/dig-wallet/src/sage/db.rs index 1718cbf7..1847110f 100644 --- a/crates/dig-wallet/src/sage/db.rs +++ b/crates/dig-wallet/src/sage/db.rs @@ -3079,8 +3079,6 @@ impl WalletDb { submitted_at: r.get("submitted_at"), expires_at: r.get("expires_at"), attempts: r.get("attempts"), - // Not a stored column: a row read back describes a bundle, not the push that - // produced it. `true` is the conservative answer (see the field's own doc). reserved_coin_ids: coins.into_iter().map(|c| c.get("coin_id")).collect(), }); }