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", 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..d4271650 --- /dev/null +++ b/crates/dig-app-core/src/rewards/clawback.rs @@ -0,0 +1,533 @@ +//! 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 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` +//! 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 +//! +//! [`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. +//! +//! [`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; + +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 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 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 +/// +/// [`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: +/// +/// ```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; +/// +/// 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 is a use-of-moved-value error (`E0382`): +/// +/// ```compile_fail,E0382 +/// # fn fixture() -> dig_app_core::rewards::clawback::ClawbackAuthority { +/// # todo!() +/// # } +/// use dig_app_core::rewards::clawback::ProvenClawback; +/// +/// let authority = fixture(); +/// let _first = ProvenClawback::open(authority); +/// let _second = ProvenClawback::open(authority); // 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, + /// 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 { + /// 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, + commitment: *commitment, + }) + } +} + +/// The four already-formatted `rewards-clawback-*` sentences, produced ONLY by [`Self::open`] from +/// 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. + confirm_title: String, + /// `copy::CLAWBACK_CONFIRM_BODY` (`pub(super)`), rendered. + confirm_body: String, + /// `copy::CLAWBACK_WITHDRAW_BUTTON` (`pub(super)`), rendered. + withdraw_button: String, + /// `copy::CLAWBACK_KEEP_BUTTON` (`pub(super)`), rendered. + 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 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 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: 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); + 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. + /// + /// 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 = [ + "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", + ]; + 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:?}"); + } + } + for constant in forbidden_consts { + assert!( + !src.contains(constant), + "{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:?})" + ); + } + } + + /// 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) + .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).is_none()); + } +} diff --git a/crates/dig-app-core/src/rewards/copy.rs b/crates/dig-app-core/src/rewards/copy.rs index ad13c86f..5c00d555 100644 --- a/crates/dig-app-core/src/rewards/copy.rs +++ b/crates/dig-app-core/src/rewards/copy.rs @@ -43,14 +43,33 @@ 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`: 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, /// 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 @@ -129,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, 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..34ec6036 100644 --- a/crates/dig-app-core/src/rewards/pane.rs +++ b/crates/dig-app-core/src/rewards/pane.rs @@ -276,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 { @@ -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 at the top of this module. } /// 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 +} diff --git a/crates/dig-app-core/src/rewards/wire.rs b/crates/dig-app-core/src/rewards/wire.rs index f27c9e2f..6ad2ed1c 100644 --- a/crates/dig-app-core/src/rewards/wire.rs +++ b/crates/dig-app-core/src/rewards/wire.rs @@ -85,13 +85,18 @@ 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 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` ///