From 64298b1d432a36d5c7a5aa283a1c3519a87c1e98 Mon Sep 17 00:00:00 2001 From: Michael Taylor Date: Thu, 10 Sep 2026 01:48:46 -0700 Subject: [PATCH 1/6] feat(app): gate rewards-clawback-* strings on a proven viewer authority Adds ClawbackAuthority: a witness constructible only from a wallet-key- derived ViewerPuzzleHash that is byte-equal to a commitment's own clawback_puzzle_hash, and the sole producer of the four finished rewards-clawback-* sentences via ProvenClawback::open. The gate takes the CAPABILITY and derives the comparand itself, closing the class of forgeable witness pane.rs:WarningsShown left open (that type is unchanged; out of scope). - New crates/dig-app-core/src/rewards/clawback.rs: ViewerPuzzleHash (single constructor, no Copy/Clone/Default/PartialEq/From), ClawbackAuthority::prove, ProvenClawback::open (consumes the witness by value; refuses the whole window on a checked_sub underflow rather than showing a stand-in zero). - copy.rs's four CLAWBACK_* Msg constants narrow to pub(super); ALL_KEYS untouched so the 14-locale/forbidden-phrase sweeps still walk them. No .ftl file touched; no affordance ships. - Moved pane.rs's test-only string-literal scanner to a shared rewards::test_scan module so clawback's key-isolation test reuses it instead of duplicating it. - Workspace root Cargo.toml 15.5.1 -> 15.5.2. Refs #3281 Co-Authored-By: Claude Sonnet 5 --- Cargo.toml | 2 +- crates/dig-app-core/src/rewards/clawback.rs | 366 +++++++++++++++++++ crates/dig-app-core/src/rewards/copy.rs | 20 +- crates/dig-app-core/src/rewards/mod.rs | 3 + crates/dig-app-core/src/rewards/pane.rs | 47 +-- crates/dig-app-core/src/rewards/test_scan.rs | 49 +++ 6 files changed, 439 insertions(+), 48 deletions(-) create mode 100644 crates/dig-app-core/src/rewards/clawback.rs create mode 100644 crates/dig-app-core/src/rewards/test_scan.rs diff --git a/Cargo.toml b/Cargo.toml index eea53d6a..e07f1eaf 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -15,7 +15,7 @@ resolver = "2" members = ["crates/dig-app-core", "crates/dig-app", "crates/diga"] [workspace.package] -version = "15.5.1" +version = "15.5.2" edition = "2021" rust-version = "1.75" license = "GPL-2.0-only" diff --git a/crates/dig-app-core/src/rewards/clawback.rs b/crates/dig-app-core/src/rewards/clawback.rs new file mode 100644 index 00000000..b2f1d6fb --- /dev/null +++ b/crates/dig-app-core/src/rewards/clawback.rs @@ -0,0 +1,366 @@ +//! The clawback authority gate (dig_ecosystem#3281): a [`ClawbackAuthority`] witness constructible +//! only from a wallet-key-derived [`ViewerPuzzleHash`] that is byte-equal to a commitment's +//! `clawback_puzzle_hash`, and the SOLE producer of the four finished `rewards-clawback-*` +//! sentences. +//! +//! # The defect class this closes +//! +//! [`super::pane::WarningsShown`] (`pane.rs:646-682`) is proven by comparing `REQUIRED_WARNING_KEYS` +//! (`pub`) against a caller-supplied list -- the caller supplies BOTH sides of that equality, so the +//! witness proves only that the caller named the right keys, never that anything was actually +//! painted; its own doc comment had to retract a stronger claim in the same file. That shape is out +//! of scope here and its warning strings ship unchanged, but the same mistake over +//! `clawback_puzzle_hash` would be worse: a per-user custody boundary, not a UI paint-order +//! convention. A `prove(viewer_ph: [u8; 32], commitment_ph: [u8; 32])` signature is forged in one +//! line -- `prove(c.clawback_puzzle_hash, c.clawback_puzzle_hash)` -- because the caller can name +//! both compared values. +//! +//! [`ClawbackAuthority::prove`] closes it by taking the CAPABILITY ([`ViewerPuzzleHash`], +//! constructible only from a real wallet key this process holds) and reading the OTHER side of the +//! comparison out of the record itself. A caller can supply neither compared value directly -- only +//! the wallet key `viewer` was derived from, and the commitment `prove` reads +//! `clawback_puzzle_hash` out of. +//! +//! # What this does NOT prove +//! +//! [`ViewerPuzzleHash::from_wallet_key`] derives at [`dig_account::ProfileIx::ROOT`] only. +//! `wallet/state.rs:262-270`'s `derivation_index_of` returns `Option` where `None` means +//! *unknown*, never index zero -- a range scan here would have to invent the bound that type +//! deliberately refuses to state, and the false-positive surface (a stranger's slot read as this +//! viewer's) would grow with the scan width. A profile spending from a non-root wallet index sees +//! every commitment as a stranger's; that is a real coverage gap, not a forgeable one. +//! +//! And `copy::CLAWBACK_*` dropping to `pub(super)` narrows reach to the `rewards` module, not to +//! this file alone: `pane.rs`, `reading.rs`, `cadence.rs`, `client.rs` and `tab_placement.rs` sit in +//! the SAME module and could still name a clawback key or fluent id. That last hop is covered by +//! [`tests::no_module_outside_clawback_names_a_clawback_key`], a source scan, NOT the compiler -- +//! writing "unreachable outside the gate" here would be exactly the retraction `pane.rs:646-658` +//! already had to publish once. + +use chia_protocol::Bytes32; +use dig_account::WalletKey; + +use crate::amount::{amount_with_unit, short_asset_id_str}; +use crate::i18n::Args; +use crate::wallet::state::Asset; + +use super::copy; +use super::wire::RewardDistributorCommitment; + +/// A wallet's own standard puzzle hash, derived ONLY from a real wallet-spending key. +/// +/// # Why there is no `from_bytes` +/// +/// The entire point of this type is that a caller cannot mint one from an arbitrary 32 bytes -- if +/// it could, [`ClawbackAuthority::prove`] would reduce to comparing two caller-supplied values, +/// exactly the forge the module doc above describes. The only legal input is a [`WalletKey`] this +/// process actually holds and controls; a second member (a non-root index) is added later by +/// extending this constructor, never by adding a `from_bytes`. +#[derive(Debug)] +pub struct ViewerPuzzleHash(Bytes32); + +impl ViewerPuzzleHash { + /// The ONLY constructor. `key.puzzle_hash()` already curries `StandardArgs` over the + /// `master_to_wallet_unhardened(master, ProfileIx::ROOT).derive_synthetic()` public key -- + /// `dig-account`'s own derivation, byte-identical to the longhand + /// `account/residency.rs:970-983` re-derives independently for its own test, and reused here + /// rather than re-derived (proven equal, not merely assumed, by + /// [`tests::from_wallet_key_matches_the_independently_derived_curry_tree_hash`]). ROOT index + /// only -- see the module doc. + pub fn from_wallet_key(key: &WalletKey) -> Self { + ViewerPuzzleHash(key.puzzle_hash()) + } +} + +/// Proof that the viewer who produced a [`ViewerPuzzleHash`] controls a commitment's +/// `clawback_puzzle_hash`. See the module doc for the defect class this closes. +/// +/// # Why not `Copy`/`Clone`/`Default` +/// +/// A copyable witness could be spent against two different commitments' worth of confirm text +/// without a second [`Self::prove`] -- [`ProvenClawback::open`] consumes this type by value for +/// exactly that reason: one proof, one confirm window. +/// +/// # Two compile-time properties a comment cannot hold +/// +/// `CLAWBACK_CONFIRM_BODY` is `pub(super)` inside `rewards` -- unreachable from outside the crate, +/// where this doctest runs: +/// +/// ```compile_fail +/// let _ = dig_app_core::rewards::copy::CLAWBACK_CONFIRM_BODY; +/// ``` +/// +/// A consumed witness cannot be reused -- `authority` is moved into the first [`ProvenClawback::open`] +/// call, so a second call with the same binding does not compile: +/// +/// ```compile_fail +/// # fn fixture() -> (dig_app_core::rewards::clawback::ClawbackAuthority, +/// # dig_app_core::rewards::wire::RewardDistributorCommitment) { +/// # todo!() +/// # } +/// use dig_app_core::rewards::clawback::ProvenClawback; +/// +/// let (authority, commitment) = fixture(); +/// let _first = ProvenClawback::open(authority, &commitment); +/// let _second = ProvenClawback::open(authority, &commitment); // moved -- does not compile +/// ``` +#[derive(Debug)] +pub struct ClawbackAuthority { + /// The matched hash, carried so [`ProvenClawback::open`] can render it without reading the + /// commitment's own `clawback_puzzle_hash` a second time -- see the module doc's + /// `clawback_ph_short` rule. + matched: Bytes32, +} + +impl ClawbackAuthority { + /// The ONLY constructor. Takes the CAPABILITY (`viewer`) and reads the comparand out of + /// `commitment` itself -- see the module doc for the one-line forge this shape closes. `None` + /// when the two hashes differ by even one bit: this is a per-user custody boundary, not a + /// display convenience, so there is no partial or "close enough" match. + pub fn prove( + viewer: &ViewerPuzzleHash, + commitment: &RewardDistributorCommitment, + ) -> Option { + let commitment_ph = Bytes32::new(commitment.clawback_puzzle_hash); + (viewer.0 == commitment_ph).then_some(ClawbackAuthority { matched: viewer.0 }) + } +} + +/// The four already-formatted `rewards-clawback-*` sentences, produced ONLY by [`Self::open`] from +/// a consumed [`ClawbackAuthority`]. `Clone` is fine here -- this is finished, post-proof text, not +/// a capability. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ProvenClawback { + /// [`copy::CLAWBACK_CONFIRM_TITLE`], rendered. + pub confirm_title: String, + /// [`copy::CLAWBACK_CONFIRM_BODY`], rendered. + pub confirm_body: String, + /// [`copy::CLAWBACK_WITHDRAW_BUTTON`], rendered. + pub withdraw_button: String, + /// [`copy::CLAWBACK_KEEP_BUTTON`], rendered. + pub keep_button: String, +} + +impl ProvenClawback { + /// Consumes `authority` BY VALUE -- one proof, one confirm window, four strings; a second + /// confirm window needs a second [`ClawbackAuthority::prove`]. + /// + /// `None` when `commitment.rewards_base_units` is less than its `recoverable_base_units`: an + /// underflow means the record is internally inconsistent, and the WHOLE window is refused + /// rather than shown with a stand-in zero forfeited figure (the #402 wrapper trap reappearing + /// under a different name). + pub fn open( + authority: ClawbackAuthority, + commitment: &RewardDistributorCommitment, + ) -> Option { + let forfeited_base_units = commitment + .rewards_base_units + .checked_sub(commitment.recoverable_base_units)?; + + // The witness's OWN field, never `commitment.clawback_puzzle_hash` -- see the module doc. + // Post-proof the two are equal, so this is not about today's value; it is about the + // direction a later loosening of `prove`'s equality drifts. Reading the record here would + // silently start printing a stranger's hash while this doc comment kept claiming otherwise. + let clawback_ph_short = short_asset_id_str(&authority.matched.to_string()); + + // dig-app-core has no date-formatting helper yet (`copy.rs`'s own `ENTRY_SET_KNOWN` doc + // names the same caveat), and the four-field wire commitment carries no per-slot ordinal -- + // only the raw Unix `epoch_start`. Rendered as-is for both `epoch_index` and + // `epoch_start_date` until a real ordinal/date formatter lands; a known display gap, not a + // security one -- the security-relevant field is `clawback_ph_short` above. + let epoch = commitment.epoch_start.to_string(); + + let slot_amount = amount_with_unit(Asset::DIG, commitment.rewards_base_units); + let returned_amount = amount_with_unit(Asset::DIG, commitment.recoverable_base_units); + let forfeited_amount = amount_with_unit(Asset::DIG, forfeited_base_units); + + let confirm_title = + copy::CLAWBACK_CONFIRM_TITLE.with(&Args::new().text("epoch_index", epoch.clone())); + let confirm_body = copy::CLAWBACK_CONFIRM_BODY.with( + &Args::new() + .text("slot_amount", slot_amount) + .text("epoch_index", epoch.clone()) + .text("epoch_start_date", epoch) + .text("returned_amount", returned_amount.clone()) + .text("forfeited_amount", forfeited_amount) + .text("clawback_ph_short", clawback_ph_short), + ); + let withdraw_button = copy::CLAWBACK_WITHDRAW_BUTTON + .with(&Args::new().text("returned_amount", returned_amount)); + let keep_button = copy::CLAWBACK_KEEP_BUTTON.text(); + + Some(ProvenClawback { + confirm_title, + confirm_body, + withdraw_button, + keep_button, + }) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::rewards::test_scan::string_literals; + + /// A fixed, arbitrary 32-byte BIP-39 entropy -- distinct from dig-account's own golden-vector + /// seed (`0x42` repeated) so this test module is not silently re-deriving that pinned value. + /// Expanded through a real mnemonic, never fed to `WalletKey::from_seed` raw, so the fixture + /// starts from a state production actually reaches (a recovery phrase), per the module doc's + /// "no `from_bytes`, no forge fixture" rule. + const ENTROPY: [u8; 32] = [0x7c; 32]; + + fn test_wallet_key() -> WalletKey { + let expanded = bip39::Mnemonic::from_entropy_in(bip39::Language::English, &ENTROPY) + .expect("32 bytes is valid 24-word BIP-39 entropy") + .to_seed(""); + WalletKey::from_seed(&expanded) + } + + /// The same longhand `account/residency.rs:970-983` uses, stopped one step short of the + /// bech32m address: BIP-39-expand -> `master_to_wallet_unhardened(master, ROOT).derive_synthetic()` + /// -> `StandardArgs::curry_tree_hash`. A SECOND implementation, so the provenance test below is + /// not the code under test agreeing with itself. + fn independently_derived_root_puzzle_hash() -> Bytes32 { + use chia_bls::{master_to_wallet_unhardened, SecretKey}; + use chia_puzzle_types::{standard::StandardArgs, DeriveSynthetic}; + use dig_account::ProfileIx; + + let expanded = bip39::Mnemonic::from_entropy_in(bip39::Language::English, &ENTROPY) + .expect("32 bytes is valid 24-word BIP-39 entropy") + .to_seed(""); + let master = SecretKey::from_seed(&expanded); + let synthetic = master_to_wallet_unhardened(&master, ProfileIx::ROOT.0).derive_synthetic(); + StandardArgs::curry_tree_hash(synthetic.public_key()).into() + } + + fn commitment_with_clawback_ph(clawback_puzzle_hash: [u8; 32]) -> RewardDistributorCommitment { + RewardDistributorCommitment { + epoch_start: 1_767_225_600, + clawback_puzzle_hash, + rewards_base_units: 10_000, + recoverable_base_units: 9_000, + } + } + + /// Step 6 -- ACCEPTANCE 1: no sibling module in `rewards` may name a clawback fluent key or + /// constant. Reuses [`string_literals`] (moved to `test_scan` so more than one test module can + /// share it, per the plan's "reuse it, do not write a second") rather than a second extractor. + #[test] + fn no_module_outside_clawback_names_a_clawback_key() { + let forbidden_keys = [ + "rewards-clawback-confirm-title", + "rewards-clawback-confirm-body", + "rewards-clawback-withdraw-button", + "rewards-clawback-keep-button", + ]; + let forbidden_consts = [ + "CLAWBACK_CONFIRM_TITLE", + "CLAWBACK_CONFIRM_BODY", + "CLAWBACK_WITHDRAW_BUTTON", + "CLAWBACK_KEEP_BUTTON", + ]; + let siblings: [(&str, &str); 5] = [ + ("pane.rs", include_str!("pane.rs")), + ("reading.rs", include_str!("reading.rs")), + ("cadence.rs", include_str!("cadence.rs")), + ("client.rs", include_str!("client.rs")), + ("tab_placement.rs", include_str!("tab_placement.rs")), + ]; + for (name, src) in siblings { + for literal in string_literals(src) { + for key in forbidden_keys { + assert_ne!(literal, key, "{name} names clawback key {key:?}"); + } + } + for constant in forbidden_consts { + assert!( + !src.contains(constant), + "{name} names clawback constant {constant}" + ); + } + } + } + + /// A sibling module that DOES name a clawback key must trip the guard above -- proves the scan + /// is not vacuously passing over its own fixture text. + #[test] + fn the_guard_itself_trips_on_a_planted_key() { + let planted = "let _bad = \"rewards-clawback-confirm-title\";"; + assert!(string_literals(planted).contains(&"rewards-clawback-confirm-title".to_string())); + } + + /// Step 8 -- provenance: [`ViewerPuzzleHash::from_wallet_key`] equals the longhand computed + /// independently in this test module. + #[test] + fn from_wallet_key_matches_the_independently_derived_curry_tree_hash() { + let key = test_wallet_key(); + let viewer = ViewerPuzzleHash::from_wallet_key(&key); + assert_eq!(viewer.0, independently_derived_root_puzzle_hash()); + } + + /// Step 9 -- a one-bit-flipped hash (not a random one) proves nothing. + #[test] + fn a_one_bit_flipped_commitment_hash_proves_nothing() { + let key = test_wallet_key(); + let viewer = ViewerPuzzleHash::from_wallet_key(&key); + let mut flipped = viewer.0.to_bytes(); + flipped[0] ^= 0b0000_0001; + let commitment = commitment_with_clawback_ph(flipped); + assert!(ClawbackAuthority::prove(&viewer, &commitment).is_none()); + } + + /// Step 10 -- ACCEPTANCE 3, THE NON-VACUOUS SUBJECT TEST. Delete the `==` in + /// `ClawbackAuthority::prove` and this test MUST go red (verified by hand per the RETURN + /// section; not automatable from inside the suite it would falsify). + #[test] + fn rendered_body_names_the_viewers_own_hash_and_a_strangers_commitment_renders_nothing() { + let key = test_wallet_key(); + let viewer = ViewerPuzzleHash::from_wallet_key(&key); + let own_hash = independently_derived_root_puzzle_hash(); + let own_commitment = commitment_with_clawback_ph(own_hash.to_bytes()); + + // (a) the viewer's own hash: proves, opens, and the body names the INDEPENDENTLY derived + // short hash -- never a hash borrowed back from the type under test. + let authority = ClawbackAuthority::prove(&viewer, &own_commitment) + .expect("viewer controls this commitment's clawback_puzzle_hash"); + let proven = ProvenClawback::open(authority, &own_commitment) + .expect("rewards_base_units >= recoverable_base_units in this fixture"); + let expected_short = short_asset_id_str(&own_hash.to_string()); + assert!( + proven.confirm_body.contains(&expected_short), + "confirm body {:?} does not name the viewer's own hash {expected_short:?}", + proven.confirm_body + ); + // (c) asserted on an owned `String` field of `proven`, never a `&str` borrowed from a + // dropped local (dig_ecosystem#3253 finding 2's undefined-behaviour shape). + assert!(!proven.confirm_title.is_empty()); + assert!(!proven.withdraw_button.is_empty()); + assert!(!proven.keep_button.is_empty()); + + // (b) a stranger's commitment (one bit flipped): `prove` is `None`, and -- structurally, + // not just in this assertion -- no `ProvenClawback` can exist without a `ClawbackAuthority` + // to consume, so no string is produced at all. + let mut strangers_ph = own_hash.to_bytes(); + strangers_ph[0] ^= 0b0000_0001; + let strangers_commitment = commitment_with_clawback_ph(strangers_ph); + assert!(ClawbackAuthority::prove(&viewer, &strangers_commitment).is_none()); + } + + /// [`ProvenClawback::open`] refuses the whole window on an internally inconsistent commitment + /// rather than showing a stand-in zero forfeited figure. + #[test] + fn checked_sub_underflow_refuses_the_whole_window() { + let key = test_wallet_key(); + let viewer = ViewerPuzzleHash::from_wallet_key(&key); + let own_hash = independently_derived_root_puzzle_hash(); + let inconsistent = RewardDistributorCommitment { + epoch_start: 1_767_225_600, + clawback_puzzle_hash: own_hash.to_bytes(), + rewards_base_units: 100, + recoverable_base_units: 101, // more recoverable than was ever committed + }; + let authority = ClawbackAuthority::prove(&viewer, &inconsistent).unwrap(); + assert!(ProvenClawback::open(authority, &inconsistent).is_none()); + } +} diff --git a/crates/dig-app-core/src/rewards/copy.rs b/crates/dig-app-core/src/rewards/copy.rs index ad13c86f..749e7adc 100644 --- a/crates/dig-app-core/src/rewards/copy.rs +++ b/crates/dig-app-core/src/rewards/copy.rs @@ -43,14 +43,26 @@ pub const WARNING_CLOSING_LINE_EN: &str = // --------------------------------------------------------------------------------------------- /// Placeable: `epoch_index`. -pub const CLAWBACK_CONFIRM_TITLE: Msg = Msg::new("rewards-clawback-confirm-title"); +/// +/// `pub(super)` (dig_ecosystem#3281), not `pub`: the finished, money-bound sentence is reachable +/// ONLY through [`super::clawback::ProvenClawback::open`], which requires a +/// [`super::clawback::ClawbackAuthority`] no caller can forge. A caller holding this bare `Msg` +/// handle could render it with an unbound/wrong-owner amount, which is not a gate -- see +/// `super::clawback`'s module doc. Narrowed to `pub(super)` rather than removed: `ALL_KEYS` below +/// (same module) still walks it for the 14-locale completeness and forbidden-phrase sweeps. +pub(super) const CLAWBACK_CONFIRM_TITLE: Msg = Msg::new("rewards-clawback-confirm-title"); /// Placeables: `slot_amount`, `epoch_index`, `epoch_start_date`, `returned_amount`, /// `forfeited_amount`, `clawback_ph_short`. Every value MUST come from the parsed commitment slot, /// never from pane state (DECISIONS Q3: "the confirm window reads NOTHING from the pane's state"). -pub const CLAWBACK_CONFIRM_BODY: Msg = Msg::new("rewards-clawback-confirm-body"); +/// +/// `pub(super)`, same reasoning as [`CLAWBACK_CONFIRM_TITLE`] above. +pub(super) const CLAWBACK_CONFIRM_BODY: Msg = Msg::new("rewards-clawback-confirm-body"); /// Placeable: `returned_amount` — the approving click names the amount. -pub const CLAWBACK_WITHDRAW_BUTTON: Msg = Msg::new("rewards-clawback-withdraw-button"); -pub const CLAWBACK_KEEP_BUTTON: Msg = Msg::new("rewards-clawback-keep-button"); +/// +/// `pub(super)`, same reasoning as [`CLAWBACK_CONFIRM_TITLE`] above. +pub(super) const CLAWBACK_WITHDRAW_BUTTON: Msg = Msg::new("rewards-clawback-withdraw-button"); +/// `pub(super)`, same reasoning as [`CLAWBACK_CONFIRM_TITLE`] above. +pub(super) const CLAWBACK_KEEP_BUTTON: Msg = Msg::new("rewards-clawback-keep-button"); // --------------------------------------------------------------------------------------------- // Q3 — the irrevocable donation (`AddIncentives`) disclosure. Never on/adjacent to the fund diff --git a/crates/dig-app-core/src/rewards/mod.rs b/crates/dig-app-core/src/rewards/mod.rs index 5ac9c63d..00da823a 100644 --- a/crates/dig-app-core/src/rewards/mod.rs +++ b/crates/dig-app-core/src/rewards/mod.rs @@ -25,9 +25,12 @@ //! is attempted vs. not. pub mod cadence; +pub mod clawback; pub mod client; pub mod copy; pub mod pane; pub mod reading; pub mod tab_placement; +#[cfg(test)] +pub(crate) mod test_scan; pub mod wire; diff --git a/crates/dig-app-core/src/rewards/pane.rs b/crates/dig-app-core/src/rewards/pane.rs index e07ed280..e9947c10 100644 --- a/crates/dig-app-core/src/rewards/pane.rs +++ b/crates/dig-app-core/src/rewards/pane.rs @@ -72,6 +72,7 @@ pub fn note_for(reading: &PaneReading) -> PaneNote { #[cfg(test)] mod tests { use super::*; + use crate::rewards::test_scan::{function_body, string_literals}; /// One case per state, asserting the painted note differs across all four -- the style /// `pane/mod.rs::painted_with_note` uses for every other tab's exhaustiveness check. @@ -595,49 +596,9 @@ mod rewards_sections_tests { } } - /// Slices `src` from `start_marker` (a `fn ...` signature) to `end_marker` (the next - /// function's signature) -- an explicit pair per builder, deliberately not a generic "next - /// `fn`" scan (see the guard above for why). - fn function_body<'a>(src: &'a str, start_marker: &str, end_marker: &str) -> &'a str { - let start = src - .find(start_marker) - .unwrap_or_else(|| panic!("{start_marker} not found in pane.rs")); - let rest = &src[start..]; - let end = rest - .find(end_marker) - .unwrap_or_else(|| panic!("{end_marker} not found after {start_marker} in pane.rs")); - &rest[..end] - } - - /// Every `"..."` string literal in `body`'s CODE lines, naively (no escape handling -- none - /// of this module's literals need it). Comment lines (`//`/`///`) are skipped first -- a - /// quoted phrase inside a doc comment (e.g. this very module's own prose) is not a Rust string - /// literal and must not trip the guard. - /// - /// Returns owned `String`s, not `&str`s borrowed from the local `code_only` buffer: an - /// earlier revision borrowed from that buffer via `unsafe { std::mem::transmute }` to escape - /// the borrow checker, which is undefined behaviour -- `code_only` drops at the end of this - /// function, so every caller was reading freed memory (dig_ecosystem#3253 adversarial gate, - /// finding 2). There is no reason to borrow here at all; owning the substrings costs a few - /// allocations in a `#[cfg(test)]`-only helper and removes the `unsafe` entirely. - fn string_literals(body: &str) -> Vec { - let code_only: String = body - .lines() - .filter(|line| !line.trim_start().starts_with("//")) - .collect::>() - .join("\n"); - let mut out = Vec::new(); - let mut rest: &str = &code_only; - while let Some(start) = rest.find('"') { - let after = &rest[start + 1..]; - let Some(end) = after.find('"') else { - break; - }; - out.push(after[..end].to_string()); - rest = &after[end + 1..]; - } - out - } + // `function_body`/`string_literals` moved to `super::super::test_scan` (dig_ecosystem#3281): + // `clawback`'s key-isolation guard needs the same string-literal extractor, and the plan calls + // for reusing it rather than writing a second one. Imported above via `use ... test_scan::*`. } /// Evidence that a caller supplied exactly the five required warning-block keys to diff --git a/crates/dig-app-core/src/rewards/test_scan.rs b/crates/dig-app-core/src/rewards/test_scan.rs new file mode 100644 index 00000000..aa0cc840 --- /dev/null +++ b/crates/dig-app-core/src/rewards/test_scan.rs @@ -0,0 +1,49 @@ +//! Shared `#[cfg(test)]` source-scanning helpers used by more than one `rewards` test module -- +//! [`super::pane`]'s hardcoded-English guard and [`super::clawback`]'s key-isolation guard +//! (dig_ecosystem#3281 plan step 6: "an in-file source-scanning string-literal extractor already +//! exists ... reuse it, do not write a second"). Moved out of `pane.rs` verbatim so both share the +//! ONE implementation rather than each carrying its own. + +#![cfg(test)] + +/// Slices `src` from `start_marker` (a `fn ...` signature) to `end_marker` (the next function's +/// signature) -- an explicit pair per caller, deliberately not a generic "next `fn`" scan (see +/// [`string_literals`]'s doc for why). +pub(crate) fn function_body<'a>(src: &'a str, start_marker: &str, end_marker: &str) -> &'a str { + let start = src + .find(start_marker) + .unwrap_or_else(|| panic!("{start_marker} not found in source")); + let rest = &src[start..]; + let end = rest + .find(end_marker) + .unwrap_or_else(|| panic!("{end_marker} not found after {start_marker} in source")); + &rest[..end] +} + +/// Every `"..."` string literal in `body`'s CODE lines, naively (no escape handling -- none of this +/// crate's guarded literals need it). Comment lines (`//`/`///`) are skipped first -- a quoted +/// phrase inside a doc comment is not a Rust string literal and must not trip a caller's guard. +/// +/// Returns owned `String`s, not `&str`s borrowed from the local `code_only` buffer: an earlier +/// revision of this function (when it lived only in `pane.rs`) borrowed from that buffer via +/// `unsafe { std::mem::transmute }` to escape the borrow checker, which is undefined behaviour -- +/// `code_only` drops at the end of the function, so every caller was reading freed memory +/// (dig_ecosystem#3253 adversarial gate, finding 2). There is no reason to borrow here at all. +pub(crate) fn string_literals(body: &str) -> Vec { + let code_only: String = body + .lines() + .filter(|line| !line.trim_start().starts_with("//")) + .collect::>() + .join("\n"); + let mut out = Vec::new(); + let mut rest: &str = &code_only; + while let Some(start) = rest.find('"') { + let after = &rest[start + 1..]; + let Some(end) = after.find('"') else { + break; + }; + out.push(after[..end].to_string()); + rest = &after[end + 1..]; + } + out +} From c660f22e7abb322a8403bc32266ade8b3faeb317 Mon Sep 17 00:00:00 2001 From: Michael Taylor Date: Thu, 10 Sep 2026 01:54:49 -0700 Subject: [PATCH 2/6] fix(app): sync Cargo.lock to workspace version 15.5.2 The version-bump commit updated Cargo.toml's workspace.package.version but left the three path-member entries in Cargo.lock (dig-app, dig-app-core, diga) pinned at 15.5.1, which every CI job's --locked cargo invocation refuses to reconcile. Refs #3281 Co-Authored-By: Claude Sonnet 5 --- Cargo.lock | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b034d19b..a0c55226 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2022,7 +2022,7 @@ dependencies = [ [[package]] name = "dig-app" -version = "15.5.1" +version = "15.5.2" dependencies = [ "dig-app-core", "dig-keystore", @@ -2041,7 +2041,7 @@ dependencies = [ [[package]] name = "dig-app-core" -version = "15.5.1" +version = "15.5.2" dependencies = [ "async-trait", "base64 0.22.1", @@ -2517,7 +2517,7 @@ dependencies = [ [[package]] name = "diga" -version = "15.5.1" +version = "15.5.2" dependencies = [ "clap", "dig-app-core", From 7e2b6b1d9b59dd9a44385de3a46b78863e4f1f73 Mon Sep 17 00:00:00 2001 From: Michael Taylor Date: Thu, 10 Sep 2026 04:07:58 -0700 Subject: [PATCH 3/6] fix(app): import the shared source scanner where it is called; unlink private copy consts Two CI root causes on #404, no behaviour change. Cause A (Clippy, Test + coverage, both Native confirmers): when the string-literal extractor moved out of `pane.rs` into `rewards::test_scan` for reuse by `rewards::clawback`, the `use` landed in `mod tests` while both call sites live in `mod rewards_sections_tests` -- so line 75 was an unused import and lines 588-589 were E0425. The import now sits in the module that calls it; there is still exactly ONE extractor (`rewards/test_scan.rs`). Cause B (Doc-link hygiene): three rustdoc failures, all doc text. 1-2. Intra-doc links into the `#[cfg(test)]` module cannot resolve under `cargo doc` (broken_intra_doc_links); the two test names are now plain backticks, not links. 3. Four `ProvenClawback` field docs linked `copy::CLAWBACK_*`, which this ticket deliberately downgraded to `pub(super)` (private_intra_doc_links). The DOC is fixed, not the visibility: the four constants stay `pub(super)` and no `pub` re-export, alias or accessor was added -- that downgrade is the security gate this PR exists to add. Co-Authored-By: Claude Opus 5 (1M context) --- crates/dig-app-core/src/rewards/clawback.rs | 12 ++++++------ crates/dig-app-core/src/rewards/pane.rs | 4 ++-- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/crates/dig-app-core/src/rewards/clawback.rs b/crates/dig-app-core/src/rewards/clawback.rs index b2f1d6fb..cdc59e9a 100644 --- a/crates/dig-app-core/src/rewards/clawback.rs +++ b/crates/dig-app-core/src/rewards/clawback.rs @@ -33,7 +33,7 @@ //! And `copy::CLAWBACK_*` dropping to `pub(super)` narrows reach to the `rewards` module, not to //! this file alone: `pane.rs`, `reading.rs`, `cadence.rs`, `client.rs` and `tab_placement.rs` sit in //! the SAME module and could still name a clawback key or fluent id. That last hop is covered by -//! [`tests::no_module_outside_clawback_names_a_clawback_key`], a source scan, NOT the compiler -- +//! `tests::no_module_outside_clawback_names_a_clawback_key`, a source scan, NOT the compiler -- //! writing "unreachable outside the gate" here would be exactly the retraction `pane.rs:646-658` //! already had to publish once. @@ -65,7 +65,7 @@ impl ViewerPuzzleHash { /// `dig-account`'s own derivation, byte-identical to the longhand /// `account/residency.rs:970-983` re-derives independently for its own test, and reused here /// rather than re-derived (proven equal, not merely assumed, by - /// [`tests::from_wallet_key_matches_the_independently_derived_curry_tree_hash`]). ROOT index + /// `tests::from_wallet_key_matches_the_independently_derived_curry_tree_hash`). ROOT index /// only -- see the module doc. pub fn from_wallet_key(key: &WalletKey) -> Self { ViewerPuzzleHash(key.puzzle_hash()) @@ -131,13 +131,13 @@ impl ClawbackAuthority { /// a capability. #[derive(Debug, Clone, PartialEq, Eq)] pub struct ProvenClawback { - /// [`copy::CLAWBACK_CONFIRM_TITLE`], rendered. + /// `copy::CLAWBACK_CONFIRM_TITLE` (`pub(super)`), rendered. pub confirm_title: String, - /// [`copy::CLAWBACK_CONFIRM_BODY`], rendered. + /// `copy::CLAWBACK_CONFIRM_BODY` (`pub(super)`), rendered. pub confirm_body: String, - /// [`copy::CLAWBACK_WITHDRAW_BUTTON`], rendered. + /// `copy::CLAWBACK_WITHDRAW_BUTTON` (`pub(super)`), rendered. pub withdraw_button: String, - /// [`copy::CLAWBACK_KEEP_BUTTON`], rendered. + /// `copy::CLAWBACK_KEEP_BUTTON` (`pub(super)`), rendered. pub keep_button: String, } diff --git a/crates/dig-app-core/src/rewards/pane.rs b/crates/dig-app-core/src/rewards/pane.rs index e9947c10..34ec6036 100644 --- a/crates/dig-app-core/src/rewards/pane.rs +++ b/crates/dig-app-core/src/rewards/pane.rs @@ -72,7 +72,6 @@ pub fn note_for(reading: &PaneReading) -> PaneNote { #[cfg(test)] mod tests { use super::*; - use crate::rewards::test_scan::{function_body, string_literals}; /// One case per state, asserting the painted note differs across all four -- the style /// `pane/mod.rs::painted_with_note` uses for every other tab's exhaustiveness check. @@ -277,6 +276,7 @@ pub fn rewards_sections( #[cfg(test)] mod rewards_sections_tests { use super::*; + use crate::rewards::test_scan::{function_body, string_literals}; use crate::rewards::wire::{ProverState, RewardCounters}; fn base_record() -> RewardDistributorStatusRecord { @@ -598,7 +598,7 @@ mod rewards_sections_tests { // `function_body`/`string_literals` moved to `super::super::test_scan` (dig_ecosystem#3281): // `clawback`'s key-isolation guard needs the same string-literal extractor, and the plan calls - // for reusing it rather than writing a second one. Imported above via `use ... test_scan::*`. + // for reusing it rather than writing a second one. Imported at the top of this module. } /// Evidence that a caller supplied exactly the five required warning-block keys to From fd3c69f95021388f6613f502cc09ad0f7af4c61e Mon Sep 17 00:00:00 2001 From: Michael Taylor Date: Thu, 10 Sep 2026 09:12:25 -0700 Subject: [PATCH 4/6] fix(app): clawback S1/F1/S2/F2/F3 -- single-arg open, private fields, ALL_KEYS Salvages a session-cap-killed dirty edit that half-applied S1's shape change (ClawbackAuthority gained a commitment field but its constructor never set it, and ProvenClawback::open still took a second, independent commitment parameter -- worse than either end state, since the splice remained expressible). - S1: ClawbackAuthority::prove now stores the matched commitment by value; ProvenClawback::open takes exactly one argument and reads every figure from authority.commitment, so the S1 splice (open(prove(&viewer,&mine).unwrap(), &strangers_slot)) is unrepresentable, not merely unchecked. Added a third compile_fail,E0061 doctest proving the second parameter no longer exists. - F1: ProvenClawback's four fields are private with &str accessors; #[non_exhaustive] was considered and rejected (leaves fields writable). Corrected the doc comments that claimed the compiler already held this. - S2: copy::ALL_KEYS is private and #[cfg(test)] (its only two callers are copy.rs's own test module; a non-test private const with no non-test caller is dead code under --no-default-features -D warnings). Corrected the "no caller can forge" / "ONLY reachable" overclaims to state reach-narrowing, since Msg::new + a literal key string still renders the sentence from any crate. - F2: no_module_outside_clawback_names_a_clawback_key now enumerates src/rewards/ at test time via std::fs::read_dir instead of a hardcoded five-file list, so mod.rs, wire.rs and test_scan.rs (previously uncovered) are scanned and a tenth file is covered automatically. - F3: corrected the doc classifying the raw-timestamp epoch_index/ epoch_start_date collision as "a known display gap, not a security one" -- it is the only identifier naming which commitment is being withdrawn and is not yet trustworthy. No wire field invented; the code fix is a follow-up ticket per the ruling. dig_ecosystem#3281 Co-Authored-By: Claude Sonnet 5 --- crates/dig-app-core/src/rewards/clawback.rs | 250 +++++++++++++++----- crates/dig-app-core/src/rewards/copy.rs | 34 ++- 2 files changed, 224 insertions(+), 60 deletions(-) diff --git a/crates/dig-app-core/src/rewards/clawback.rs b/crates/dig-app-core/src/rewards/clawback.rs index cdc59e9a..5b537617 100644 --- a/crates/dig-app-core/src/rewards/clawback.rs +++ b/crates/dig-app-core/src/rewards/clawback.rs @@ -1,7 +1,18 @@ //! The clawback authority gate (dig_ecosystem#3281): a [`ClawbackAuthority`] witness constructible //! only from a wallet-key-derived [`ViewerPuzzleHash`] that is byte-equal to a commitment's -//! `clawback_puzzle_hash`, and the SOLE producer of the four finished `rewards-clawback-*` -//! sentences. +//! `clawback_puzzle_hash`, and the only producer of a [`ProvenClawback`] -- the only value in this +//! crate that carries the four finished `rewards-clawback-*` sentences bound to a commitment's +//! real amounts and hash. +//! +//! That is narrower than "the sole producer of those sentences", and the difference matters. +//! [`crate::i18n::Msg::new`] is a public `const fn` and the fluent key is a plain `&'static str` +//! literal (`copy::ALL_KEYS`, the one place that used to re-export it by value, is now private -- +//! dig_ecosystem#3281 S2), so any crate that writes the literal key still renders the same +//! 14-locale sentence with any amounts and any hash it likes. What this module holds is that no +//! code outside it can produce or mutate a [`ProvenClawback`], and that a [`ProvenClawback`]'s +//! text is always bound to a commitment whose `clawback_puzzle_hash` a wallet key this process +//! holds actually controls. Reach-narrowing, not unreachability -- see "What this does NOT prove" +//! below. //! //! # The defect class this closes //! @@ -75,34 +86,70 @@ impl ViewerPuzzleHash { /// Proof that the viewer who produced a [`ViewerPuzzleHash`] controls a commitment's /// `clawback_puzzle_hash`. See the module doc for the defect class this closes. /// +/// # Why the proved commitment travels INSIDE the witness +/// +/// An earlier revision carried only `matched` and let [`ProvenClawback::open`] take a SECOND, +/// independent commitment, which it never re-checked. That made +/// `open(prove(&viewer, &mine).unwrap(), &strangers_slot)` render a stranger's amounts under +/// en.ftl's "You committed ..." and "... returns to this wallet" -- beside the viewer's own +/// genuine hash, which made the forgery more convincing rather than less (dig_ecosystem#3281 +/// security gate, S1). It is the predecessor PR's subject defect: correctly typed, correctly +/// formatted, and false about whose money it is. +/// +/// The fix is not a re-check inside `open` -- a re-check leaves the splice expressible and relies +/// on a future maintainer remembering it. The witness carries the commitment it was proved against +/// by value, and `open` takes exactly one argument, so there is no second commitment to pass and +/// the splice is unrepresentable. Same principle as [`Self::prove`] taking the capability instead +/// of a compared value. +/// /// # Why not `Copy`/`Clone`/`Default` /// -/// A copyable witness could be spent against two different commitments' worth of confirm text -/// without a second [`Self::prove`] -- [`ProvenClawback::open`] consumes this type by value for -/// exactly that reason: one proof, one confirm window. +/// A copyable witness could be spent for two confirm windows' worth of text from one +/// [`Self::prove`] -- [`ProvenClawback::open`] consumes this type by value for exactly that +/// reason: one proof, one confirm window. Note this is a REPLAY property only; it never protected +/// the subject, which is what carrying the commitment above does. +/// +/// # Three compile-time properties a comment cannot hold /// -/// # Two compile-time properties a comment cannot hold +/// [`ProvenClawback::open`] takes EXACTLY ONE argument, so passing a second, independent +/// commitment -- the S1 splice, `open(prove(&viewer, &mine).unwrap(), &strangers_slot)` -- is not +/// merely re-checked away, it is a wrong-number-of-arguments error (`E0061`) and does not compile: /// -/// `CLAWBACK_CONFIRM_BODY` is `pub(super)` inside `rewards` -- unreachable from outside the crate, -/// where this doctest runs: +/// ```compile_fail,E0061 +/// # fn fixture() -> dig_app_core::rewards::clawback::ClawbackAuthority { +/// # todo!() +/// # } +/// # fn strangers_commitment() -> dig_app_core::rewards::wire::RewardDistributorCommitment { +/// # todo!() +/// # } +/// use dig_app_core::rewards::clawback::ProvenClawback; /// -/// ```compile_fail +/// let authority = fixture(); +/// let _ = ProvenClawback::open(authority, &strangers_commitment()); // too many args -- does not compile +/// ``` +/// +/// `CLAWBACK_CONFIRM_BODY`'s PATH is `pub(super)` inside `rewards`, so naming the constant from +/// outside the crate -- where this doctest runs -- is a private-path error (`E0603`), asserted by +/// the error code rather than by "it failed to build somehow". That narrows REACH to the `rewards` +/// module; it does not make the sentence unrenderable, because the fluent key is a `&'static str` +/// any crate can hand to the public [`crate::i18n::Msg::new`]: +/// +/// ```compile_fail,E0603 /// let _ = dig_app_core::rewards::copy::CLAWBACK_CONFIRM_BODY; /// ``` /// /// A consumed witness cannot be reused -- `authority` is moved into the first [`ProvenClawback::open`] -/// call, so a second call with the same binding does not compile: +/// call, so a second call with the same binding is a use-of-moved-value error (`E0382`): /// -/// ```compile_fail -/// # fn fixture() -> (dig_app_core::rewards::clawback::ClawbackAuthority, -/// # dig_app_core::rewards::wire::RewardDistributorCommitment) { +/// ```compile_fail,E0382 +/// # fn fixture() -> dig_app_core::rewards::clawback::ClawbackAuthority { /// # todo!() /// # } /// use dig_app_core::rewards::clawback::ProvenClawback; /// -/// let (authority, commitment) = fixture(); -/// let _first = ProvenClawback::open(authority, &commitment); -/// let _second = ProvenClawback::open(authority, &commitment); // moved -- does not compile +/// let authority = fixture(); +/// let _first = ProvenClawback::open(authority); +/// let _second = ProvenClawback::open(authority); // moved -- does not compile /// ``` #[derive(Debug)] pub struct ClawbackAuthority { @@ -110,6 +157,10 @@ pub struct ClawbackAuthority { /// commitment's own `clawback_puzzle_hash` a second time -- see the module doc's /// `clawback_ph_short` rule. matched: Bytes32, + /// The commitment `matched` was proved against, by value -- the ONLY record + /// [`ProvenClawback::open`] may read a figure from. See "Why the proved commitment travels + /// INSIDE the witness" above. + commitment: RewardDistributorCommitment, } impl ClawbackAuthority { @@ -122,52 +173,105 @@ impl ClawbackAuthority { commitment: &RewardDistributorCommitment, ) -> Option { let commitment_ph = Bytes32::new(commitment.clawback_puzzle_hash); - (viewer.0 == commitment_ph).then_some(ClawbackAuthority { matched: viewer.0 }) + (viewer.0 == commitment_ph).then_some(ClawbackAuthority { + matched: viewer.0, + commitment: *commitment, + }) } } /// The four already-formatted `rewards-clawback-*` sentences, produced ONLY by [`Self::open`] from -/// a consumed [`ClawbackAuthority`]. `Clone` is fine here -- this is finished, post-proof text, not -/// a capability. +/// a consumed [`ClawbackAuthority`]. +/// +/// # Fields are PRIVATE, not `pub` +/// +/// An earlier revision left these four fields `pub`, so a struct literal built anywhere in the +/// crate -- with no [`ClawbackAuthority`] involved at all -- produced a value indistinguishable +/// from one this module actually proved, and a legitimately obtained one was mutable in place +/// (overwrite `withdraw_button` after the honest "returns to this wallet" body was rendered +/// beside it). `#[non_exhaustive]` would not have closed this: it blocks a struct literal from a +/// foreign crate but leaves every field publicly writable to anyone who already has a value, which +/// is the exact mutation this doc used to (wrongly) claim was impossible. Private fields plus the +/// accessors below are the only shape that makes both "constructed only by `open`" and "not +/// mutable after" true at once. +/// +/// `Clone`/`PartialEq`/`Eq` stay: this is finished, post-proof text, not a capability, so copying +/// or comparing it carries no custody meaning -- unlike [`ClawbackAuthority`], which is +/// deliberately not `Clone` (see its own doc). #[derive(Debug, Clone, PartialEq, Eq)] pub struct ProvenClawback { /// `copy::CLAWBACK_CONFIRM_TITLE` (`pub(super)`), rendered. - pub confirm_title: String, + confirm_title: String, /// `copy::CLAWBACK_CONFIRM_BODY` (`pub(super)`), rendered. - pub confirm_body: String, + confirm_body: String, /// `copy::CLAWBACK_WITHDRAW_BUTTON` (`pub(super)`), rendered. - pub withdraw_button: String, + withdraw_button: String, /// `copy::CLAWBACK_KEEP_BUTTON` (`pub(super)`), rendered. - pub keep_button: String, + keep_button: String, } impl ProvenClawback { + /// The rendered `rewards-clawback-confirm-title` sentence. + pub fn confirm_title(&self) -> &str { + &self.confirm_title + } + + /// The rendered `rewards-clawback-confirm-body` sentence -- the one that names amounts and the + /// viewer's own hash; see the module doc's S1 fix for why it can only ever name the commitment + /// [`ClawbackAuthority::prove`] matched. + pub fn confirm_body(&self) -> &str { + &self.confirm_body + } + + /// The rendered `rewards-clawback-withdraw-button` sentence. + pub fn withdraw_button(&self) -> &str { + &self.withdraw_button + } + + /// The rendered `rewards-clawback-keep-button` sentence. + pub fn keep_button(&self) -> &str { + &self.keep_button + } + /// Consumes `authority` BY VALUE -- one proof, one confirm window, four strings; a second /// confirm window needs a second [`ClawbackAuthority::prove`]. /// - /// `None` when `commitment.rewards_base_units` is less than its `recoverable_base_units`: an - /// underflow means the record is internally inconsistent, and the WHOLE window is refused - /// rather than shown with a stand-in zero forfeited figure (the #402 wrapper trap reappearing - /// under a different name). - pub fn open( - authority: ClawbackAuthority, - commitment: &RewardDistributorCommitment, - ) -> Option { + /// `None` when the held commitment's `rewards_base_units` is less than its + /// `recoverable_base_units`: an underflow means the record is internally inconsistent, and the + /// WHOLE window is refused rather than shown with a stand-in zero forfeited figure (the #402 + /// wrapper trap reappearing under a different name). + /// + /// Takes EXACTLY ONE argument -- see "Why the proved commitment travels INSIDE the witness" + /// above. There is no second `commitment` parameter to pass a stranger's record through, so + /// S1's splice (`open(prove(&viewer, &mine).unwrap(), &strangers_slot)`) is not merely + /// re-checked, it is unrepresentable: every figure below is read from `authority.commitment`, + /// the same record `prove` matched `authority.matched` against. + pub fn open(authority: ClawbackAuthority) -> Option { + let commitment = &authority.commitment; let forfeited_base_units = commitment .rewards_base_units .checked_sub(commitment.recoverable_base_units)?; - // The witness's OWN field, never `commitment.clawback_puzzle_hash` -- see the module doc. - // Post-proof the two are equal, so this is not about today's value; it is about the - // direction a later loosening of `prove`'s equality drifts. Reading the record here would - // silently start printing a stranger's hash while this doc comment kept claiming otherwise. + // The witness's OWN field, never a second, independently-supplied commitment -- see the + // module doc and this method's doc above. Post-proof the two are equal by construction + // (there is no other `commitment` in scope to diverge from), so this is not a defensive + // re-check; it is the only record this method can read from at all. let clawback_ph_short = short_asset_id_str(&authority.matched.to_string()); // dig-app-core has no date-formatting helper yet (`copy.rs`'s own `ENTRY_SET_KNOWN` doc // names the same caveat), and the four-field wire commitment carries no per-slot ordinal -- // only the raw Unix `epoch_start`. Rendered as-is for both `epoch_index` and - // `epoch_start_date` until a real ordinal/date formatter lands; a known display gap, not a - // security one -- the security-relevant field is `clawback_ph_short` above. + // `epoch_start_date` until a real ordinal/date formatter lands (dig_ecosystem#3281 F3). + // This is NOT merely a display gap: `epoch_index` is the only identifier in this sentence + // naming WHICH commitment is being withdrawn, and this raw timestamp is not it. Until the + // wire carries a real per-slot ordinal, the value rendered here is not a trustworthy epoch + // identifier and no future renderer should treat it as one, or key logic off it, the same + // way the `checked_sub` refusal two lines above treats an inconsistent commitment as + // unshowable rather than "close enough". The wire field this needs does not exist yet + // (four fields: `epoch_start`, `clawback_puzzle_hash`, `rewards_base_units`, + // `recoverable_base_units`) -- adding one, or deriving an index from a compiled-in epoch + // length, is the shape SPEC §2.6 clause 2 already rejected once; this stays a known-false + // display until the wire changes, tracked as a follow-up ticket rather than fixed here. let epoch = commitment.epoch_start.to_string(); let slot_amount = amount_with_unit(Asset::DIG, commitment.rewards_base_units); @@ -246,6 +350,17 @@ mod tests { /// Step 6 -- ACCEPTANCE 1: no sibling module in `rewards` may name a clawback fluent key or /// constant. Reuses [`string_literals`] (moved to `test_scan` so more than one test module can /// share it, per the plan's "reuse it, do not write a second") rather than a second extractor. + /// + /// ENUMERATES `src/rewards/` at test time (`std::fs::read_dir`, not a hardcoded file list) so + /// a tenth file added to this directory is covered automatically rather than silently + /// outside the scan -- dig_ecosystem#3281 F2 found the previous hardcoded five-file list had + /// already drifted behind `mod.rs`, `wire.rs` and `test_scan.rs` (`mod.rs` is the material + /// gap: a `pub use` there widens reach past both this scan and the compiler). Only two `.rs` + /// files are deliberately excluded, both self-evidently: `clawback.rs` (this file -- the gate + /// itself legitimately names every key and constant in its own doc comments) and `copy.rs` + /// (the module that OWNS and defines every `CLAWBACK_*` constant and fluent key -- the thing + /// this scan exists to keep OTHER modules from naming). `the_guard_itself_trips_on_a_planted_key` + /// below keeps this non-vacuous. #[test] fn no_module_outside_clawback_names_a_clawback_key() { let forbidden_keys = [ @@ -260,15 +375,30 @@ mod tests { "CLAWBACK_WITHDRAW_BUTTON", "CLAWBACK_KEEP_BUTTON", ]; - let siblings: [(&str, &str); 5] = [ - ("pane.rs", include_str!("pane.rs")), - ("reading.rs", include_str!("reading.rs")), - ("cadence.rs", include_str!("cadence.rs")), - ("client.rs", include_str!("client.rs")), - ("tab_placement.rs", include_str!("tab_placement.rs")), - ]; - for (name, src) in siblings { - for literal in string_literals(src) { + const EXCLUDED: &[&str] = &["clawback.rs", "copy.rs"]; + + let rewards_dir = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("src/rewards"); + let entries = std::fs::read_dir(&rewards_dir) + .unwrap_or_else(|e| panic!("{} must be readable: {e}", rewards_dir.display())); + + let mut scanned = Vec::new(); + for entry in entries { + let path = entry.expect("directory entry readable").path(); + if path.extension().and_then(|ext| ext.to_str()) != Some("rs") { + continue; + } + let name = path + .file_name() + .and_then(|n| n.to_str()) + .expect("utf-8 file name") + .to_string(); + if EXCLUDED.contains(&name.as_str()) { + continue; + } + let src = std::fs::read_to_string(&path) + .unwrap_or_else(|e| panic!("{} must be readable: {e}", path.display())); + + for literal in string_literals(&src) { for key in forbidden_keys { assert_ne!(literal, key, "{name} names clawback key {key:?}"); } @@ -279,6 +409,20 @@ mod tests { "{name} names clawback constant {constant}" ); } + scanned.push(name); + } + + // Non-vacuity of the ENUMERATION itself: if the directory read silently returned nothing + // (a moved crate root, a bad `CARGO_MANIFEST_DIR`), every assertion above would trivially + // pass over zero files. `wire.rs`, `mod.rs` and `test_scan.rs` are the three the previous + // hardcoded list was missing; require them by name so this scan cannot quietly narrow back + // to the old five without a test failure naming which one dropped out. + for must_be_scanned in ["mod.rs", "wire.rs", "test_scan.rs", "pane.rs"] { + assert!( + scanned.iter().any(|n| n == must_be_scanned), + "{must_be_scanned} was not scanned -- directory enumeration under-covered \ + (scanned: {scanned:?})" + ); } } @@ -324,19 +468,19 @@ mod tests { // short hash -- never a hash borrowed back from the type under test. let authority = ClawbackAuthority::prove(&viewer, &own_commitment) .expect("viewer controls this commitment's clawback_puzzle_hash"); - let proven = ProvenClawback::open(authority, &own_commitment) + let proven = ProvenClawback::open(authority) .expect("rewards_base_units >= recoverable_base_units in this fixture"); let expected_short = short_asset_id_str(&own_hash.to_string()); assert!( - proven.confirm_body.contains(&expected_short), + proven.confirm_body().contains(&expected_short), "confirm body {:?} does not name the viewer's own hash {expected_short:?}", - proven.confirm_body + proven.confirm_body() ); // (c) asserted on an owned `String` field of `proven`, never a `&str` borrowed from a // dropped local (dig_ecosystem#3253 finding 2's undefined-behaviour shape). - assert!(!proven.confirm_title.is_empty()); - assert!(!proven.withdraw_button.is_empty()); - assert!(!proven.keep_button.is_empty()); + assert!(!proven.confirm_title().is_empty()); + assert!(!proven.withdraw_button().is_empty()); + assert!(!proven.keep_button().is_empty()); // (b) a stranger's commitment (one bit flipped): `prove` is `None`, and -- structurally, // not just in this assertion -- no `ProvenClawback` can exist without a `ClawbackAuthority` @@ -361,6 +505,6 @@ mod tests { recoverable_base_units: 101, // more recoverable than was ever committed }; let authority = ClawbackAuthority::prove(&viewer, &inconsistent).unwrap(); - assert!(ProvenClawback::open(authority, &inconsistent).is_none()); + assert!(ProvenClawback::open(authority).is_none()); } } diff --git a/crates/dig-app-core/src/rewards/copy.rs b/crates/dig-app-core/src/rewards/copy.rs index 749e7adc..5c00d555 100644 --- a/crates/dig-app-core/src/rewards/copy.rs +++ b/crates/dig-app-core/src/rewards/copy.rs @@ -44,12 +44,19 @@ pub const WARNING_CLOSING_LINE_EN: &str = /// Placeable: `epoch_index`. /// -/// `pub(super)` (dig_ecosystem#3281), not `pub`: the finished, money-bound sentence is reachable -/// ONLY through [`super::clawback::ProvenClawback::open`], which requires a -/// [`super::clawback::ClawbackAuthority`] no caller can forge. A caller holding this bare `Msg` -/// handle could render it with an unbound/wrong-owner amount, which is not a gate -- see -/// `super::clawback`'s module doc. Narrowed to `pub(super)` rather than removed: `ALL_KEYS` below -/// (same module) still walks it for the 14-locale completeness and forbidden-phrase sweeps. +/// `pub(super)` (dig_ecosystem#3281), not `pub`: dropping this constant's own path to `pub(super)` +/// narrows who can name THIS `Msg` handle to the `rewards` module. That is reach-narrowing, not +/// unreachability -- see `super::clawback`'s module doc for the same distinction stated in full. +/// [`crate::i18n::Msg::new`] is a public `const fn` and the fluent key is a plain `&'static str` +/// literal, so any crate that writes `Msg::new("rewards-clawback-confirm-title")` still renders +/// this sentence with any amounts and any hash it likes -- no caller "can't forge" it in the sense +/// of being unable to reproduce the value; nothing here stops that. What IS true: the only value +/// in this crate whose text is bound to a commitment a real wallet key was proved to control is a +/// [`super::clawback::ProvenClawback`], producible only through +/// [`super::clawback::ProvenClawback::open`], which requires a +/// [`super::clawback::ClawbackAuthority`] -- see that module's doc for how `open` is gated. +/// `ALL_KEYS` below is private for the same reach-narrowing reason and stays walked only by this +/// module's own 14-locale completeness and forbidden-phrase sweeps. pub(super) const CLAWBACK_CONFIRM_TITLE: Msg = Msg::new("rewards-clawback-confirm-title"); /// Placeables: `slot_amount`, `epoch_index`, `epoch_start_date`, `returned_amount`, /// `forfeited_amount`, `clawback_ph_short`. Every value MUST come from the parsed commitment slot, @@ -141,7 +148,20 @@ pub const CADENCE_FAR_END: Msg = Msg::new("rewards-cadence-far-end"); /// Every key this module defines, for the exhaustiveness/render/sweep tests below. Keeping this /// list here (rather than re-deriving it per test) is the one place a new key must be added or the /// tests that iterate "every rewards key" silently stop covering it. -pub const ALL_KEYS: &[Msg] = &[ +/// +/// `#[cfg(test)]`, not `pub` (dig_ecosystem#3281 S2): its only callers are this module's own +/// `every_copy_key_renders_in_the_active_language` and `no_rewards_copy_contains_a_forbidden_phrase` +/// tests below (verified: no other file in the crate names `ALL_KEYS`), so gating it to test +/// builds costs nothing -- and is required, not just tidier: a plain private (non-`pub`) const +/// with no non-test caller is genuinely dead code in a `--no-default-features` release build and +/// trips `-D warnings`' dead-code lint. A `pub` `ALL_KEYS` re-exported all four `CLAWBACK_*` keys +/// BY VALUE regardless of their own `pub(super)`, so `ALL_KEYS[8].with(..)` rendered the full +/// confirm body from outside this crate with no [`super::clawback::ClawbackAuthority`] witness at +/// all -- narrower than a forbidden literal or constant name, so a source-scan guard could not see +/// it. Narrowing this is reach-narrowing too: a caller can still write the literal key and call +/// the public [`Msg::new`] directly (see [`CLAWBACK_CONFIRM_TITLE`]'s doc above). +#[cfg(test)] +const ALL_KEYS: &[Msg] = &[ WARNING_HEADING, WARNING_BLOCK_1, WARNING_BLOCK_2, From a22507467249c227ebbed7434fbc50d04d799a44 Mon Sep 17 00:00:00 2001 From: Michael Taylor Date: Thu, 10 Sep 2026 09:53:07 -0700 Subject: [PATCH 5/6] docs(app): name the clawback follow-up tickets, fix a doc staleness S1 created MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - wire.rs: RewardDistributorCommitment's doc claimed "nothing constructs, reads or paints from it" -- false since the S1 fix: ClawbackAuthority::prove now constructs it into the witness and ProvenClawback::open reads three of its fields. Corrected. - clawback.rs module doc: names the untracked gap this module's "bound to a commitment" claim does NOT cover -- record PROVENANCE (parsed chain read vs. in-crate struct literal) is not established by `prove`, tracked as dig_ecosystem#3294. - F3's promised follow-up ticket is now named: dig_ecosystem#3289 (the epoch_index/epoch_start_date raw-timestamp gap). - Names dig_ecosystem#3290 for the brief's SPEC §2.6 clause 5 note ("nothing committed" vs. "could not be read" is unrepresentable in these types today) next to the related wire-gap comment it sits beside, doc-only, no field added. dig_ecosystem#3281 Co-Authored-By: Claude Sonnet 5 --- crates/dig-app-core/src/rewards/clawback.rs | 49 +++++++++++++++------ crates/dig-app-core/src/rewards/wire.rs | 20 ++++++--- 2 files changed, 49 insertions(+), 20 deletions(-) diff --git a/crates/dig-app-core/src/rewards/clawback.rs b/crates/dig-app-core/src/rewards/clawback.rs index 5b537617..d4271650 100644 --- a/crates/dig-app-core/src/rewards/clawback.rs +++ b/crates/dig-app-core/src/rewards/clawback.rs @@ -1,8 +1,11 @@ //! The clawback authority gate (dig_ecosystem#3281): a [`ClawbackAuthority`] witness constructible //! only from a wallet-key-derived [`ViewerPuzzleHash`] that is byte-equal to a commitment's //! `clawback_puzzle_hash`, and the only producer of a [`ProvenClawback`] -- the only value in this -//! crate that carries the four finished `rewards-clawback-*` sentences bound to a commitment's -//! real amounts and hash. +//! crate that carries the four finished `rewards-clawback-*` sentences bound to the amounts carried +//! by the commitment [`ClawbackAuthority::prove`] was called against, and to the hash it matched. +//! "Bound to the commitment" is a binding to whatever fields that commitment value holds -- it is +//! NOT a claim that those fields' PROVENANCE (that the commitment came from a parsed chain read +//! rather than an in-crate construction) is established; see "What this does NOT prove" below. //! //! That is narrower than "the sole producer of those sentences", and the difference matters. //! [`crate::i18n::Msg::new`] is a public `const fn` and the fluent key is a plain `&'static str` @@ -47,6 +50,17 @@ //! `tests::no_module_outside_clawback_names_a_clawback_key`, a source scan, NOT the compiler -- //! writing "unreachable outside the gate" here would be exactly the retraction `pane.rs:646-658` //! already had to publish once. +//! +//! [`ClawbackAuthority::prove`] does not prove a commitment's ORIGIN. A commitment reaching `prove` +//! is not proven to have come from a parsed chain/RPC read rather than an in-crate struct literal +//! constructed by any code in this crate ([`super::wire::RewardDistributorCommitment`]'s fields are +//! `pub(crate)` for exactly this reason -- see that type's own doc). `prove` binds key control over +//! `clawback_puzzle_hash`; it does not and cannot bind the commitment's record provenance, because +//! nothing reaching it today carries that information -- see that type's own doc for the +//! `dig.listRewardDistributorCommitments` transport gap this sits behind. Closing this needs the +//! transport's parse-only constructor, tracked as dig_ecosystem#3294; it is NOT closed by widening +//! or narrowing any visibility here, and not by another doc sentence in this module -- see that +//! ticket for why. use chia_protocol::Bytes32; use dig_account::WalletKey; @@ -261,17 +275,26 @@ impl ProvenClawback { // dig-app-core has no date-formatting helper yet (`copy.rs`'s own `ENTRY_SET_KNOWN` doc // names the same caveat), and the four-field wire commitment carries no per-slot ordinal -- // only the raw Unix `epoch_start`. Rendered as-is for both `epoch_index` and - // `epoch_start_date` until a real ordinal/date formatter lands (dig_ecosystem#3281 F3). - // This is NOT merely a display gap: `epoch_index` is the only identifier in this sentence - // naming WHICH commitment is being withdrawn, and this raw timestamp is not it. Until the - // wire carries a real per-slot ordinal, the value rendered here is not a trustworthy epoch - // identifier and no future renderer should treat it as one, or key logic off it, the same - // way the `checked_sub` refusal two lines above treats an inconsistent commitment as - // unshowable rather than "close enough". The wire field this needs does not exist yet - // (four fields: `epoch_start`, `clawback_puzzle_hash`, `rewards_base_units`, - // `recoverable_base_units`) -- adding one, or deriving an index from a compiled-in epoch - // length, is the shape SPEC §2.6 clause 2 already rejected once; this stays a known-false - // display until the wire changes, tracked as a follow-up ticket rather than fixed here. + // `epoch_start_date` until a real ordinal/date formatter lands: tracked as + // dig_ecosystem#3289. This is NOT merely a display gap: `epoch_index` is the only + // identifier in this sentence naming WHICH commitment is being withdrawn, and this raw + // timestamp is not it. Until the wire carries a real per-slot ordinal, the value rendered + // here is not a trustworthy epoch identifier and no future renderer should treat it as one, + // or key logic off it, the same way the `checked_sub` refusal two lines above treats an + // inconsistent commitment as unshowable rather than "close enough". The wire field this + // needs does not exist yet (four fields: `epoch_start`, `clawback_puzzle_hash`, + // `rewards_base_units`, `recoverable_base_units`) -- adding one, or deriving an index from + // a compiled-in epoch length, is the SPEC §2.6-clause-2 shape dig_ecosystem#3253 already + // rejected once; this stays a known-false display until the wire changes (dig_ecosystem#3289). + // + // A related but distinct gap, tracked separately as dig_ecosystem#3290: SPEC §2.6 clause 5 + // distinguishes "nothing committed" from "could not be read" (a distributor with zero + // commitments vs. a read this process failed to perform), and none of the types in this + // module or `wire.rs` can represent that distinction -- an empty commitment set and a + // failed read are not separable here. Not this function's defect (`open` never sees an + // empty set, only a single already-obtained commitment), but the same wire gap this + // sentence's `epoch_index` caveat sits next to, so it is named here rather than left for + // the next reader to rediscover. let epoch = commitment.epoch_start.to_string(); let slot_amount = amount_with_unit(Asset::DIG, commitment.rewards_base_units); diff --git a/crates/dig-app-core/src/rewards/wire.rs b/crates/dig-app-core/src/rewards/wire.rs index f27c9e2f..6b78c334 100644 --- a/crates/dig-app-core/src/rewards/wire.rs +++ b/crates/dig-app-core/src/rewards/wire.rs @@ -85,13 +85,19 @@ pub struct RewardDistributorStatusRecord { /// whatever bps this crate happens to have compiled in. This type carries the chain's own already- /// computed answer instead, so there is nothing here to recompute. /// -/// This type is inert in this pass: nothing constructs, reads or paints from it. -/// [`super::client::RewardsClient`] does not adopt `dig.listRewardDistributorCommitments` (deleted -/// per the dig_ecosystem#3253 adversarial gate's finding 2 — the trait method wrapped only this -/// type's `Vec`, dropping three of the SPEC §2.6 result's five fields), and nothing in dig-node -/// serves the RPC yet either (PRs #593/#594 open, unmerged). This type itself stays: all four -/// fields are correct and complete for what THEY carry, and it is where the full five-field seam -/// lands once clawback is actually wired. +/// This type is no longer inert: [`super::clawback::ClawbackAuthority::prove`] (dig_ecosystem#3281) +/// constructs it into the witness it proves against, and [`super::clawback::ProvenClawback::open`] +/// reads `rewards_base_units`, `recoverable_base_units` and their difference through +/// `amount_with_unit` and paints all three as `$DIG`. The next reader must not assume no custody +/// gate reads this type — the clawback authority gate does, today. What is still true: no +/// constructor here takes a `dig.listRewardDistributorCommitments` RPC response — +/// [`super::client::RewardsClient`] does not adopt that method (deleted per the dig_ecosystem#3253 +/// adversarial gate's finding 2 — the trait method wrapped only this type's `Vec`, dropping three +/// of the SPEC §2.6 result's five fields), and nothing in dig-node serves the RPC yet either (PRs +/// #593/#594 open, unmerged). So every value of this type in the running program today is an +/// in-crate struct literal (fixture or otherwise), never a parsed chain read — see +/// dig_ecosystem#3294 for why that gap matters to [`super::clawback`]'s proof, and where closing it +/// lands once the transport is wired. /// /// # Fields are `pub(crate)`, not `pub` /// From 77b3d9fd2c24f3644e90a3e365ceaf85372604fa Mon Sep 17 00:00:00 2001 From: Michael Taylor Date: Thu, 10 Sep 2026 10:06:52 -0700 Subject: [PATCH 6/6] docs(app): stop citing dig-node PR state, drop production-value implication wire.rs's RewardDistributorCommitment doc cited dig-node#593/#594 as "open, unmerged" -- both had merged before this head, reintroducing the exact staleness pattern already banned on #402 (cite a durable version or code fact, never another repo's PR state). Replace it with a dig-app- internal fact instead: no constructor of this type anywhere in this crate sits outside #[cfg(test)] code (verified: clawback.rs:364,524 and wire.rs:180 are all under #[cfg(test)] mod tests). Also reworded "every value ... in the running program today" so it no longer implies a production value exists -- it does not, only test fixtures do. Co-Authored-By: Claude Sonnet 5 --- crates/dig-app-core/src/rewards/wire.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/crates/dig-app-core/src/rewards/wire.rs b/crates/dig-app-core/src/rewards/wire.rs index 6b78c334..6ad2ed1c 100644 --- a/crates/dig-app-core/src/rewards/wire.rs +++ b/crates/dig-app-core/src/rewards/wire.rs @@ -93,11 +93,10 @@ pub struct RewardDistributorStatusRecord { /// constructor here takes a `dig.listRewardDistributorCommitments` RPC response — /// [`super::client::RewardsClient`] does not adopt that method (deleted per the dig_ecosystem#3253 /// adversarial gate's finding 2 — the trait method wrapped only this type's `Vec`, dropping three -/// of the SPEC §2.6 result's five fields), and nothing in dig-node serves the RPC yet either (PRs -/// #593/#594 open, unmerged). So every value of this type in the running program today is an -/// in-crate struct literal (fixture or otherwise), never a parsed chain read — see -/// dig_ecosystem#3294 for why that gap matters to [`super::clawback`]'s proof, and where closing it -/// lands once the transport is wired. +/// of the SPEC §2.6 result's five fields), and no constructor of this type anywhere in this crate +/// sits outside `#[cfg(test)]` code. So this type has no production value at this head, only +/// test fixtures — see dig_ecosystem#3294 for why that gap matters to [`super::clawback`]'s proof, +/// and where closing it lands once the transport is wired. /// /// # Fields are `pub(crate)`, not `pub` ///