Skip to content

Commit 2fa4753

Browse files
feat(wallet)!: require dig-wallet-backend 0.16.1 and pin the overflow boundary
The dependency's four amount accumulations now route through a fallible `accumulate`, so a spend whose created-output amounts sum past u64::MAX is refused instead of wrapping modulo 2^64 and passing value conservation. `a_spend_whose_output_amounts_overflow_is_never_approved` pins dig-account's side of that boundary and FAILS against 0.16.0, so the version floor is load-bearing. Its companion pins the same bound from below, so the guard cannot degrade into "refuses large amounts". BREAKING CHANGE: the dig-wallet-backend floor moves from 0.16 to 0.16.1. Co-Authored-By: Claude <noreply@anthropic.com>
1 parent f8d765b commit 2fa4753

5 files changed

Lines changed: 142 additions & 41 deletions

File tree

Cargo.lock

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ dig-social-profile = "0.2"
1818
# The canonical money signer + independent spend verification live in dig-wallet-backend's `client`
1919
# seam (LocalSigner, verify::derive_summary). Only the `client` half is pulled in — dig-account is the
2020
# key-holding custody side, never the running engine (no tokio runtime / rusqlite / offer builders).
21-
dig-wallet-backend = { version = "0.16", default-features = false, features = ["client"] }
21+
dig-wallet-backend = { version = "0.16.1", default-features = false, features = ["client"] }
2222
dig-session = "0.4"
2323
dig-identity = "0.5"
2424
dig-keystore = "0.4.1"

scripts/probe-guards.sh

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -87,16 +87,17 @@ probe "G7 an undeclared intent escalates" $E \
8787
declared => declared,
8888
})'
8989

90-
# G2 is REACHABLE -- dig-wallet-backend sums OUTPUT amounts from CLVM conditions with an unchecked
91-
# `+=` (client/verify.rs:165), so a spend can wrap `xch_out`, pass value conservation, and hand back
92-
# an effect whose recipient amounts cannot be totalled. The input that reaches this guard PANICS
93-
# inside the dependency before the guard evaluates, so the test cannot be written until
94-
# dig-wallet-backend #1708 ships; a mock that bypassed the dependency would prove nothing about the
95-
# real path. The exemption below expires by itself the moment the guard goes RED.
90+
# G2 is knowingly VACUOUS against dig-wallet-backend >= 0.16.1, and provably so rather than
91+
# untested: the driver accumulates every created XCH coin plus the fee through a fallible
92+
# `accumulate` and then requires `xch_in == xch_out + fee`, so this crate's native total -- a subset
93+
# of those coins plus that fee -- is bounded by `xch_in` and cannot overflow. The guard is kept as
94+
# defence-in-depth because that proof lives inside a dependency. What IS pinned is the boundary:
95+
# `a_spend_whose_output_amounts_overflow_is_never_approved` fails against 0.16.0, so the version
96+
# floor is load-bearing. The exemption below still expires by itself the moment the guard goes RED.
9697
probe "G2 custody total is CHECKED, not saturating" $S \
9798
'let native_total_mojos = summary.checked_native_total_mojos()?;' \
9899
'let native_total_mojos = summary.native_total_mojos();' \
99-
'vacuous:blocked on dig-wallet-backend #1708; see summary.rs DerivedSpend::derive'
100+
'vacuous:unreachable given dig-wallet-backend >=0.16.1; proof in summary.rs DerivedSpend::derive'
100101

101102
probe "G19 input coin amounts must sum in a u64" $S \
102103
'coin_spends

src/wallet/enforcer.rs

Lines changed: 98 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -50,14 +50,14 @@
5050
//! a property of the AUTHORIZER THE CALLER BUILT, not a property of the funds — the caller MUST
5151
//! construct the authorizer that matches the coins it is about to spend.
5252
//!
53-
//! **A [`SpendSummary`] accounts only for HINTED outputs plus the fee.** An un-hinted output is
54-
//! change, which `dig-wallet-backend`'s re-derivation excludes from the recipient list, so no amount
55-
//! limit here can see it. That invariant is held one layer down by
56-
//! [`LocalMoneySigner`](crate::wallet::money_signer::LocalMoneySigner), which refuses to sign a spend
57-
//! with a change output the wallet does not own — which bounds where such value can GO (somewhere
58-
//! under the same seed) but not that it obeyed a policy.
59-
//! `refuses_to_sign_unhinted_value_leaving_the_wallet_even_when_the_policy_approves` pins the
60-
//! composition.
53+
//! **A [`SpendSummary`] counts every output by DESTINATION, never by hint status** — so this
54+
//! paragraph's former warning (that an un-hinted output was invisible to every amount limit) no
55+
//! longer holds, and neither does any custody claim built on it. An output is weighed unless it pays
56+
//! a puzzle hash the spend is itself spending from. The signer's change-ownership check remains a
57+
//! required second layer, because this one bounds how MUCH value leaves while that one bounds where
58+
//! it may go; `an_unhinted_output_to_an_owned_derivation_is_counted_not_hidden` and
59+
//! `refuses_to_sign_unhinted_value_leaving_the_wallet_even_when_the_policy_approves` pin the two
60+
//! halves.
6161
6262
use std::collections::VecDeque;
6363
use std::sync::{Arc, Mutex};
@@ -1761,4 +1761,94 @@ mod tests {
17611761
"the projection must refuse rather than wrap to 0: {err}"
17621762
);
17631763
}
1764+
1765+
/// A standard-layer spend of `coin_amount` that creates one hinted output per entry in
1766+
/// `outputs`, with NO total pre-check — the fixture the summable-total rules need.
1767+
///
1768+
/// `spend_paying` deliberately refuses to build an unrepresentable total (it `checked_add`s and
1769+
/// panics), which is right for every fixture that must be a *well-formed* spend. The rules below
1770+
/// are about the spends that are NOT well-formed, so they need a builder that will emit one.
1771+
fn spend_creating(coin_amount: u64, outputs: &[(Bytes32, u64)]) -> Vec<CoinSpend> {
1772+
let mut ctx = SpendContext::new();
1773+
let mut conditions = Conditions::new();
1774+
for (puzzle_hash, amount) in outputs {
1775+
let hint = ctx.hint(*puzzle_hash).unwrap();
1776+
conditions = conditions.create_coin(*puzzle_hash, *amount, hint);
1777+
}
1778+
StandardLayer::new(spender().public_key())
1779+
.spend(
1780+
&mut ctx,
1781+
Coin::new(Bytes32::new([9u8; 32]), spender().puzzle_hash(), coin_amount),
1782+
conditions,
1783+
)
1784+
.unwrap();
1785+
ctx.take()
1786+
}
1787+
1788+
/// **A spend whose CREATED-OUTPUT amounts do not sum in a `u64` never becomes an approval.**
1789+
///
1790+
/// This is the value-conservation bypass, and it is a bypass precisely because the wrap makes the
1791+
/// spend look conserving. A `1_000`-mojo coin creating `u64::MAX` and `1_001` sums, modulo 2^64,
1792+
/// back to exactly `1_000` — so an implementation that accumulates output amounts with a wrapping
1793+
/// `+=` finds `xch_in == xch_out`, declares value conserved, and hands back an effect describing
1794+
/// a spend of every mojo that will ever exist. The dependency's security gate showed
1795+
/// `LocalSigner::sign_unsigned` will emit a real aggregated BLS signature over such a spend, so
1796+
/// the refusal has to happen before an approval exists, not at the signature.
1797+
///
1798+
/// The fixture's amounts are chosen FROM the bound rather than picked large: `u64::MAX + 1_001`
1799+
/// is the smallest pair that both overflows and lands back on a plausible coin amount, so the
1800+
/// test cannot pass merely because the numbers were too big to be believed.
1801+
///
1802+
/// Requires `dig-wallet-backend` >= 0.16.1, where all four accumulation sites route through a
1803+
/// fallible `accumulate`. Pinned by EXECUTION against 0.16.0, where it fails in both build
1804+
/// profiles for the same underlying reason: debug panics on the unchecked `+=` at
1805+
/// `client/verify.rs:165`, release wraps and the gate returns an approval. That version
1806+
/// difference is the whole content of the guarantee, which is why the dependency floor is
1807+
/// `0.16.1` and not `0.16`.
1808+
#[test]
1809+
fn a_spend_whose_output_amounts_overflow_is_never_approved() {
1810+
let gate = gate_with(hot_custody(), permissive_auto_send());
1811+
let overflowing = spend_creating(
1812+
1_000,
1813+
&[
1814+
(third_party().puzzle_hash(), u64::MAX),
1815+
(third_party().puzzle_hash(), 1_001),
1816+
],
1817+
);
1818+
1819+
let err = refusal(gate.authorize_op(&overflowing, SpendOpClass::Tip));
1820+
1821+
assert!(
1822+
matches!(&err, AccountError::Spend(_)),
1823+
"an unaccountable spend must be refused at the derivation, before any approval or signature exists: {err}"
1824+
);
1825+
}
1826+
1827+
/// THE OTHER SIDE OF THE SAME BOUND — the refusal above must be about overflow, not about size.
1828+
///
1829+
/// A guard tested only from over the bound cannot distinguish "rejects what does not sum" from
1830+
/// "rejects large amounts", and the second would refuse every legitimate whale spend while every
1831+
/// test stayed green. So the largest total that DOES sum — outputs of `u64::MAX - 1` and `1`,
1832+
/// exactly `u64::MAX`, from a coin of exactly `u64::MAX` — must be accounted for and reach a
1833+
/// ruling.
1834+
#[test]
1835+
fn a_spend_whose_output_amounts_sum_to_exactly_u64_max_is_still_accountable() {
1836+
let gate = gate_with(hot_custody(), permissive_auto_send());
1837+
let at_the_bound = spend_creating(
1838+
u64::MAX,
1839+
&[
1840+
(third_party().puzzle_hash(), u64::MAX - 1),
1841+
(third_party().puzzle_hash(), 1),
1842+
],
1843+
);
1844+
1845+
// It escalates rather than auto-sends (it is far over the hot allowance), which is a RULING:
1846+
// the derivation accounted for it. The point is that it is not refused as unaccountable.
1847+
let ruling = gate.authorize_op(&at_the_bound, SpendOpClass::Tip);
1848+
assert!(
1849+
!matches!(&ruling, Err(AccountError::Spend(_))),
1850+
"a spend whose outputs sum to exactly u64::MAX is accountable and must not be refused as though it overflowed"
1851+
);
1852+
let _ = pending(ruling);
1853+
}
17641854
}

src/wallet/summary.rs

Lines changed: 33 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -251,20 +251,23 @@ fn destination_line(output: &DecodedOutput) -> SpendRecipient {
251251
/// the sole-`AGG_SIG_ME` rule are checked here, so a spend the driver cannot fully account for is
252252
/// refused before any custody decision — and before any signature — exists.
253253
///
254-
/// # Why the input amounts are summed first
254+
/// # Why the input amounts are summed first, even though the driver now sums them too
255255
///
256-
/// `dig-wallet-backend` 0.16 accumulates the spent coins' amounts with an unchecked `+=`
257-
/// (`client/verify.rs:153`), so a coin-spend set whose INPUT amounts do not sum in a `u64` panics in a
258-
/// debug build and WRAPS in a release build — and a wrapped input total is what value conservation is
259-
/// then checked against. The amounts come from an unsigned skeleton, which a dapp supplies, so they are
260-
/// attacker-chosen and need not correspond to coins that exist. Refusing an unsummable input total here
261-
/// keeps that reachable from nowhere: the answer is
262-
/// [`PolicyIndeterminate`](AccountError::PolicyIndeterminate), because a spend whose inputs cannot be
263-
/// totalled is not forbidden — it simply cannot be judged.
256+
/// `dig-wallet-backend` 0.16.1 routes all four of its accumulations through a fallible `accumulate`
257+
/// (#1708), so an unsummable total is refused there rather than panicking in debug and wrapping in
258+
/// release. This pre-check is therefore no longer the only thing standing between an attacker-chosen
259+
/// amount and a wrapped total — but it is kept, and deliberately:
264260
///
265-
/// The driver's OUTPUT accumulators (`xch_out` at `:165`, `cat_out` at `:131`) are summed from CLVM
266-
/// conditions rather than coin amounts, so no pre-check here can bound them; they are guarded after the
267-
/// fact by [`SpendSummary::checked_native_total_mojos`], and upstream in dig-wallet-backend (#1708).
261+
/// - It makes the ANSWER right, not merely the refusal. An unsummable input total is
262+
/// [`PolicyIndeterminate`](AccountError::PolicyIndeterminate) — the spend cannot be JUDGED — whereas
263+
/// the driver's own refusal arrives as [`Spend`](AccountError::Spend), "this spend is malformed".
264+
/// Those are different facts, and `SPEC.md` §6.3 forbids collapsing them.
265+
/// - The amounts come from an unsigned skeleton a dapp supplies, so they are attacker-chosen and need
266+
/// not correspond to coins that exist. A custody crate does not delegate its fail-closed behaviour
267+
/// to a dependency's minor version.
268+
///
269+
/// `a_spend_whose_input_amounts_do_not_sum_in_a_u64_is_refused_rather_than_wrapped` pins the variant,
270+
/// which is what makes this guard load-bearing rather than a duplicate of the driver's.
268271
fn derive_effect(coin_spends: &[CoinSpend]) -> Result<SpendEffect> {
269272
coin_spends
270273
.iter()
@@ -332,19 +335,26 @@ impl DerivedSpend {
332335
/// [`PolicyIndeterminate`](AccountError::PolicyIndeterminate) rather than clamped to `u64::MAX` and
333336
/// then tiered as though the clamp were its value.
334337
///
335-
/// # The checked sum is REACHABLE, and is why the saturating accessor must not be used here
338+
/// # The checked sum is now UNREACHABLE here, and is kept anyway — read why before removing it
339+
///
340+
/// Against `dig-wallet-backend` >= 0.16.1 no input can reach this `?`. The proof is short: the
341+
/// driver accumulates EVERY created XCH coin into `xch_out` through its fallible `accumulate`,
342+
/// accumulates the fee the same way, and then requires `xch_in == xch_out + fee` with `xch_in`
343+
/// itself checked. This summary's native total is a SUBSET of those same created coins (change
344+
/// returning to a spent puzzle hash is dropped, CAT outputs are excluded) plus that same fee, so
345+
/// it is bounded above by `xch_in`, which fits in a `u64` by construction.
336346
///
337-
/// [`derive_effect`] bounds the spent coins' amounts, but the driver's own output accumulator
338-
/// (`xch_out`, `client/verify.rs:165`) is summed from CLVM conditions with an unchecked `+=`. So a
339-
/// spend creating hinted outputs of `u64::MAX` and `1_001` from a `1_000` mojo coin wraps `xch_out`
340-
/// back to `1_000` in a release build, value conservation *passes*, and the driver hands back an
341-
/// effect whose outputs cannot be totalled. This sum is what refuses it.
347+
/// It is retained as defence-in-depth because that proof rests entirely on an invariant INSIDE a
348+
/// dependency, which no test in this crate can enforce and a patch release could weaken. What the
349+
/// crate can pin is the boundary itself, and
350+
/// `a_spend_whose_output_amounts_overflow_is_never_approved` does: it fails against 0.16.0, so the
351+
/// `0.16.1` floor is load-bearing rather than cosmetic. `scripts/probe-guards.sh` records this
352+
/// guard as knowingly vacuous, and that exemption self-expires the moment any input makes it RED.
342353
///
343-
/// Were this the saturating accessor, that spend would total `u64::MAX`, classify `Confirm`, and be
344-
/// offered to a human as a spend of every mojo that will ever exist — a fiction presented as a
345-
/// figure. `a_spend_whose_output_amounts_overflow_is_never_approved` pins it. (In a debug build the
346-
/// driver panics on the wrap before returning; that is dig-wallet-backend #1708, and the test
347-
/// asserts the "never approved" property under either build profile.)
354+
/// The accessor must nonetheless stay the CHECKED one. Were it the saturating form and the
355+
/// dependency ever regressed, an overflowing spend would total `u64::MAX`, classify `Confirm`, and
356+
/// be offered to a human as a spend of every mojo that will ever exist — a fiction presented as a
357+
/// figure.
348358
pub(crate) fn derive(coin_spends: &[CoinSpend], policy: &CustodyPolicy) -> Result<Self> {
349359
let effect = derive_effect(coin_spends)?;
350360
// Tiered `Confirm` first — the stricter of the two hot tiers, so a bug that skipped the

0 commit comments

Comments
 (0)