fix(wallet): retire the last largest-first rival, and make the unguarded sync setter unreachable

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.209.0"
+version = "0.213.0"
 
 # Release hardening, matching digstore: keep integer-overflow checks ON in release.
 # The node parses untrusted serialized input and does offset/length arithmetic over

crates/dig-wallet/src/sage/db.rs

@@ -1395,14 +1395,47 @@ impl WalletDb {
     /// only the terminal answer of a full address-history catch-up produces. A caller that has not
     /// replayed history has nothing to build one out of, which is what makes the unsafe arming hard
     /// to write rather than merely discouraged.
-    pub async fn set_initial_sync_complete(&self, complete: bool) -> sqlx::Result<()> {
+    ///
+    /// # The `true` direction is UNREPRESENTABLE here, by type (dig-node#462)
+    ///
+    /// This takes no argument, so it can only DISARM. That is the whole fix: dig-node#454 made
+    /// every real ARMING writer take a `reset_epoch` guard, because an unguarded arm is that PR's
+    /// money-lie defect — a catch-up completing after a cache reset set `initial_sync_complete = 1`
+    /// over an EMPTIED table, giving `balance 0, synced true` on a funded wallet. A `bool` setter
+    /// beside those guards is a third path to that state which bypasses them, and the next author
+    /// who needs to set the flag will find a function whose name says it does exactly that.
+    ///
+    /// Removing the parameter discharges it STRUCTURALLY rather than by convention: there is no
+    /// `true` to pass. Arming remains reachable only through [`Self::complete_catch_up`] and
+    /// [`Self::record_coverage`], which is what the compiler now enforces.
+    ///
+    /// **Disarming stays public and unguarded, deliberately.** `sync.rs`'s reorg handler calls it
+    /// when the replica moves backwards, so wallet reads fall back until a fresh catch-up. That is
+    /// the conservative direction — it can only make the node claim LESS than it knows — and it is
+    /// the direction a guard would be protecting nothing by blocking.
+    pub async fn clear_initial_sync_complete(&self) -> sqlx::Result<()> {
+        self.write_initial_sync_complete(false).await
+    }
+
+    /// The raw write, private so that the only ways to reach the `true` direction are the two
+    /// guarded arming paths and the test-only forcing hatch below.
+    async fn write_initial_sync_complete(&self, complete: bool) -> sqlx::Result<()> {
         sqlx::query("UPDATE sync_state SET initial_sync_complete = ? WHERE id = 0")
             .bind(i64::from(complete))
             .execute(&self.pool)
             .await?;
         Ok(())
     }
 
+    /// Force the flag from a test, in either direction.
+    ///
+    /// A test-only escape hatch is fine; an AMBIGUOUS one is what dig-node#462 is about, so the
+    /// name carries its own danger and `#[cfg(test)]` keeps it out of every production build.
+    #[cfg(test)]
+    pub async fn force_initial_sync_complete_for_test(&self, complete: bool) -> sqlx::Result<()> {
+        self.write_initial_sync_complete(complete).await
+    }
+
     /// Record the puzzle-hash set a NON-catch-up sync path covered — the oracle-tier refresh
     /// ([`crate::sage::rpc::WalletBackend::refresh_tracked_coins`]), which fetches coins for a set
     /// of addresses by point read and latches `initial_sync_complete` without replaying history.
@@ -4580,6 +4613,45 @@ fn is_unique_violation(e: &sqlx::Error) -> bool {
 mod tests {
     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.
+    ///
+    /// # What enforces the other half, and why it is not asserted here
+    ///
+    /// The ticket asks that production be unable to ARM except through a `reset_epoch`-guarded
+    /// path, "enforced by the compiler rather than by convention", and explicitly rules out a test
+    /// that greps for callers. That half is discharged STRUCTURALLY and cannot be written as a
+    /// runtime assertion: [`WalletDb::clear_initial_sync_complete`] takes no argument, so there is
+    /// no `true` to pass, and the raw `write_initial_sync_complete` is private. The type cannot
+    /// express the unguarded arm — the same shape as a scalar that cannot represent a set of two.
+    ///
+    /// So this test pins the direction that IS representable, and the compiler pins the one that is
+    /// not. A future change that re-widens this to `fn set_initial_sync_complete(&self, bool)` is
+    /// caught by review against this doc-comment, not by a green suite — which is stated here
+    /// rather than implied, because a reader who finds only this test could otherwise conclude the
+    /// arming direction was never considered.
+    ///
+    /// **Catches:** a `clear_` that no longer clears — e.g. one wired to the wrong column, or one
+    /// that writes `true` because the parameter was removed by editing the call rather than the
+    /// body.
+    #[tokio::test]
+    async fn clear_initial_sync_complete_disarms_an_armed_flag() {
+        let db = WalletDb::open_in_memory().await.unwrap();
+
+        db.force_initial_sync_complete_for_test(true).await.unwrap();
+        assert!(
+            db.sync_state().await.unwrap().initial_sync_complete,
+            "the fixture must start ARMED, or the clear below would prove nothing"
+        );
+
+        db.clear_initial_sync_complete().await.unwrap();
+        assert!(
+            !db.sync_state().await.unwrap().initial_sync_complete,
+            "the reorg path's disarm must actually reach the flag — sync.rs relies on it to make \
+             wallet reads fall back after the replica moves backwards"
+        );
+    }
+
     fn coin(id: &str, amount: u64, created: Option<i64>, spent: Option<i64>) -> CoinRow {
         CoinRow {
             coin_id: id.into(),
@@ -4882,7 +4954,7 @@ mod tests {
         let db = WalletDb::open_in_memory().await.unwrap();
 
         // What `refresh_tracked_coins` does on a fresh install with nothing in the DB yet.
-        db.set_initial_sync_complete(true).await.unwrap();
+        db.force_initial_sync_complete_for_test(true).await.unwrap();
         assert_eq!(
             db.arrival_baseline().await.unwrap(),
             None,
@@ -5292,7 +5364,7 @@ mod tests {
         db.upsert_coin(&coin("c1", 12_345, Some(10), None))
             .await
             .unwrap();
-        db.set_initial_sync_complete(true).await.unwrap();
+        db.force_initial_sync_complete_for_test(true).await.unwrap();
         assert!(db.is_synced().await.unwrap(), "pre-state: authoritative");
 
         let report = db.reset_chain_cache(0).await.unwrap().expect("not refused");
@@ -5321,7 +5393,7 @@ mod tests {
         db.upsert_coin(&coin("c1", 12_345, Some(10), None))
             .await
             .unwrap();
-        db.set_initial_sync_complete(true).await.unwrap();
+        db.force_initial_sync_complete_for_test(true).await.unwrap();
         db.reserve_client_coins(&["c1".to_string()], Some(60_000), 0)
             .await
             .expect("reserve");

crates/dig-wallet/src/sage/offers.rs

@@ -409,24 +409,30 @@ fn cat_offer_asset(asset_id: Bytes32, amount: u64) -> OfferAsset {
 }
 
 /// Greedily select XCH coins (largest first) covering `need`.
+///
+/// The ordering, the tiebreak and the refusal are
+/// [`super::selection::select_largest_first`]'s — the same ones the wallet's row selector, the CAT
+/// leg selector and the mirror lifecycle's operator-scoped selector already use (dig-node#428).
+///
+/// It was the LAST of the four to still carry its own loop, and the divergence had become
+/// asymmetric rather than merely duplicated: this one accumulated with a bare `+=`, where the other
+/// three saturate. On a coin set summing past `u64::MAX` that panics in a debug build and wraps in a
+/// release one, and a wrapped total is read back as a shortfall — so the shipped failure was a
+/// spurious "insufficient XCH" on a set that could in fact pay, rather than the false success the
+/// ticket anticipated. Safe direction, wrong answer, and a debug-build panic on a money path.
+///
+/// The tiebreak was CONFIRMED identical before deleting, not assumed: this sorted
+/// `b.amount.cmp(&a.amount).then(a.coin_id().cmp(&b.coin_id()))`, which is exactly the canonical
+/// key `(amount, coin_id)` — descending amount, ascending coin id. The refusal message is kept
+/// verbatim, so an operator's log line does not change spelling under a refactor.
 fn select_xch(coins: &[Coin], need: u64) -> Result<Vec<Coin>> {
-    let mut sorted: Vec<Coin> = coins.to_vec();
-    sorted.sort_by(|a, b| b.amount.cmp(&a.amount).then(a.coin_id().cmp(&b.coin_id())));
-    let mut sum = 0u64;
-    let mut out = Vec::new();
-    for c in sorted {
-        if sum >= need {
-            break;
-        }
-        sum += c.amount;
-        out.push(c);
-    }
-    if sum < need {
-        return Err(Error::api(format!(
-            "insufficient XCH to offer: need {need} have {sum}"
-        )));
-    }
-    Ok(out)
+    super::selection::select_largest_first(coins.to_vec(), need, |c| (c.amount, c.coin_id()))
+        .map_err(|s| {
+            Error::api(format!(
+                "insufficient XCH to offer: need {} have {}",
+                s.need, s.have
+            ))
+        })
 }
 
 /// Greedily select CAT coins of `asset_id` (largest first) covering `need`.
@@ -453,6 +459,72 @@ fn select_cats(cats: &[Cat], asset_id: Bytes32, need: u64) -> Result<Vec<Cat>> {
 #[cfg(test)]
 mod tests {
     use super::*;
+
+    /// **Proves (dig-node#428):** `select_xch` accumulates its running total the way every other
+    /// selector in this crate does, so a coin set whose amounts sum past `u64::MAX` is handled
+    /// rather than overflowed.
+    ///
+    /// # The fixture, and the assertion it took two attempts to get right
+    ///
+    /// Two coins of `2^63` with a target just above one of them is the ONLY shape that reaches the
+    /// overflow at all:
+    ///
+    /// - the walk stops the moment `total >= target` and the set is sorted DESCENDING, so the
+    ///   largest coin is added to a zero total and cannot overflow on its own;
+    /// - to overflow on the second coin, `a1 + a2` must exceed `u64::MAX` while `a1 < target`,
+    ///   which forces BOTH `a1 > u64::MAX / 2` and `target > u64::MAX / 2`.
+    ///
+    /// So the intuitive "one huge coin plus small change" fixture proves nothing — the huge coin is
+    /// taken first, covers the target, and the loop breaks before a second addition happens.
+    ///
+    /// **The expected outcome is SUCCESS, not refusal.** Two coins of `2^63` genuinely do cover
+    /// `2^63 + 5`; a refusal would be the wrong answer. This test first asserted a shortfall and
+    /// failed against the correct implementation, which is the useful half of writing it: any set
+    /// that overflows sums to more than `u64::MAX`, hence more than any `u64` target, so
+    /// **saturation cannot manufacture a false success here** — the saturated total is only ever
+    /// reached when the real total is larger still.
+    ///
+    /// **Catches:** the bare `sum += c.amount` this replaces. In a debug build — which is how tests
+    /// run — that PANICS with `attempt to add with overflow` at the accumulation, so this test
+    /// fails loudly rather than by assertion. In a release build it wraps to a small total and the
+    /// caller reads back a spurious "insufficient XCH" on a set that could in fact pay.
+    #[test]
+    fn select_xch_covers_a_target_from_coins_that_sum_past_u64_max() {
+        let half = u64::MAX / 2 + 1; // 2^63
+        let coins = vec![
+            Coin::new(Bytes32::default(), Bytes32::default(), half),
+            Coin::new(Bytes32::new([1u8; 32]), Bytes32::default(), half),
+        ];
+        // Above the first coin, so the walk must add the second and cross u64::MAX.
+        let target = half + 5;
+
+        let selected = select_xch(&coins, target)
+            .expect("two coins of 2^63 cover 2^63 + 5; only the arithmetic could fail here");
+        assert_eq!(
+            selected.len(),
+            2,
+            "both coins are needed — one 2^63 alone does not reach 2^63 + 5"
+        );
+    }
+
+    /// **The control.** The saturating total must not turn a genuinely-covered target into a
+    /// refusal — a selector that always refused would pass the test above.
+    #[test]
+    fn select_xch_still_covers_an_ordinary_target_largest_first() {
+        let coins = vec![
+            Coin::new(Bytes32::new([1u8; 32]), Bytes32::default(), 30),
+            Coin::new(Bytes32::new([2u8; 32]), Bytes32::default(), 100),
+            Coin::new(Bytes32::new([3u8; 32]), Bytes32::default(), 70),
+        ];
+        let selected = select_xch(&coins, 90).expect("100 alone covers 90");
+        assert_eq!(
+            selected.len(),
+            1,
+            "largest-first must take the single 100 rather than 70 + 30"
+        );
+        assert_eq!(selected[0].amount, 100);
+    }
+
     use chia_sdk_test::Simulator;
     use chia_wallet_sdk::types::TESTNET11_CONSTANTS;