feat(wallet)!: the gate mints an owned SpendApproval; the signer accepts nothing else - #4
feat(wallet)!: the gate mints an owned SpendApproval; the signer accepts nothing else#4MichaelTaylor3d wants to merge 13 commits into
Conversation
3fe60f9 to
c144883
Compare
…pts nothing else dig-account 0.1.2 shipped tier classification, per-transaction limits, a rolling period cap and vault clawback, and none of it reached a running code path. Two design seams are why, and both are closed here rather than patched. The `SpendAuthorizer` TRAIT was a custody gate whose simplest implementation approves everything, and the only consumer duly shipped exactly that: a default authorizer whose entire body was `Ok(())`. And `Result<()>` cannot say "not yet, ask the human", so a consumer handed only Ok/Err collapses every refusal into one, which makes the confirm ceremony unreachable for precisely the Confirm and Vault tiers that exist to require it. The shape: - `PolicyAuthorizer::authorize_op(&[CoinSpend], SpendOpClass) -> Result<SpendRuling>` derives the summary ITSELF, so no caller-supplied description exists to disagree with. - It mints a `SpendApproval` that OWNS the exact coin spends it authorized. Deliberately not a digest binding: a comparison is a step that can be skipped, mis-scoped, or run over the wrong bytes, whereas a single owned value cannot be mismatched. - `MoneySigner::sign_approved(SpendApproval) -> SpendBundle` is the only signing entry point, and the bundle it returns carries the approval's own spends, so a caller cannot pair the signature with different bytes. - The token's properties are type-system properties: single-use BY MOVE (not `Clone`, taken by value), no nonce, no expiry, never `Serialize`/`Deserialize`/`Debug`. - `SpendRuling::RequiresConfirmation` is the third state, so a Confirm/Vault spend reaches the ceremony, and `SpendOpClass::Undeclared` (what every out-of-process request is) now escalates to the human instead of dead-ending as `PolicyIndeterminate`. The enforcement is the REMOVALS; the additions are inert without them: 1. `MoneySigner::sign_coin_spends` DELETED. No function turns `&[CoinSpend]` into a signature, which is the enforcement point SPEC 6.2's "authorized before signed" MUST never had. 2. `LocalMoneySigner::sign_unsigned` demoted to `pub(crate)`: it took a caller-supplied summary, the same defect one layer over. 3. The `SpendAuthorizer` trait DELETED. `PolicyAuthorizer` is the concrete gate, and since `SpendApproval`'s constructor is `pub(crate)` it is mechanically the only minter. 4. `AccountError::RequireAuth` DELETED. Nothing can produce it once escalation is a ruling, so leaving it would invite consumers to keep collapsing. 5. The tier-disagreement guard DELETED as vacuous: the gate classifies its own derivation, so the two sides can no longer disagree. 6. The two SPEC carve-outs at the old 216-228 DELETED. Enforcement is no longer opt-in, and the authorization is no longer unbound from the signed bytes. Also fixed, found while delete-probing the guards: dig-wallet-backend 0.16 accumulates the spent coins' amounts with an unchecked `+=` (`client/verify.rs:153`). Those amounts arrive in a caller-supplied unsigned skeleton, so they are attacker-chosen and need not name coins that exist. An unsummable input total therefore PANICS in a debug build and WRAPS in a release build, after which the wrapped figure is what value conservation is checked against. dig-account now sums them checked before deriving anything, and refuses. Evidence: 144 unit tests + 5 shape tests green; fmt and clippy -D warnings clean; 19 custody guards delete-probed with 18 load-bearing and the one exception documented in `summary.rs` at its site; cargo-mutants over the changed modules with zero survivors. BREAKING CHANGE: `PolicyAuthorizer::authorize_op` changes both its parameter and its return type; `MoneySigner::sign_coin_spends`, the `SpendAuthorizer` trait and `AccountError::RequireAuth` are removed. A 0.x MINOR is semver-breaking, since `^0.1` cannot resolve `0.2.0`, so the dig-app bump is mandatory rather than optional. Co-Authored-By: Claude <noreply@anthropic.com>
c144883 to
053a640
Compare
MichaelTaylor3d
left a comment
There was a problem hiding this comment.
VERDICT: CHANGES-REQUIRED (recorded as a comment review — GitHub rejects request-changes on a self-authored PR, 422).
CHANGES-REQUIRED — correctness review (independent, fresh context; the security + adversarial legs run separately and I have not duplicated them).
The shape is right and I could not break the custody substance. I independently delete-probed four guards on this head — G14 rolling cap, G10 per-tx limit, G5 vault destination, G6 global off switch — and all four came back RED with the named tests. cargo test on 053a640 is 144 unit + 5 shape, 0 failures. SemVer 0.2.0 is correct and the single manifest agrees; the frozen §3 byte-contracts are untouched by diff (only error.rs, lib.rs, wallet/*) and the golden money-key vector still asserts 884cc9a2. dig-app does carry exactly ONE version literal (Cargo.toml:82); crates/dig-app-core/Cargo.toml:37 inherits — ticket correction confirmed.
What blocks is the evidence layer, and one blocker is a demonstrated false green in the headline invariant. Four blocking findings, all cheap; details inline.
tests/the_shape_is_unwritable.rs:146— the "no signing function takes bare coin spends" invariant is bypassed by a rustfmt-wrapped signature. Proved it: addedpub fn sign_these_spends_without_any_approval_whatsoever(&self, coin_spends: &[CoinSpend])to productionmoney_signer.rsand the shape suite stayed GREEN (5 passed).scripts/probe-guards.sh:10— the harness cannot run at all as committed; every probe reports PATTERN-MISS. Ran it: 4/4 miss. It also holds 4 of the 19 disclosed guards, and a miss exits 0.SPEC.md:303—AccountError::PolicyDeniedis double-mapped in the normative 6.3.1 table (-33053 and -33052).src/wallet/money_signer.rs:380and:411— the two relocated tests assert ondig_wallet_backend::client::derive_summarydirectly; no dig-account symbol is under test, so they do not prove the ordering they claim.
Non-gating, resolved by me, nothing to do for merge:
- The mutants
--filelist omitssrc/wallet/authorizer.rs, a changed production module — the same class of omission the PR says it corrected from the previous batch. I ran it myself: 7 mutants, 4 caught, 3 unviable, 0 survivors. No real gap; only the scope of the claim needs correcting. - G2: I agree with retain-and-document. Deleting it means reverting to the saturating accessor — the fail-open form — so the alternative is strictly worse. It IS a hard local check returning
PolicyIndeterminate; it is merely unreachable while the dependency conserves value.summary.rs:230-243states that honestly and names the dependency the bound rests on. Correct call, and the right place for it. - The single-element-fixture class is otherwise swept: the vault destination rule,
checked_native_total_mojos, the rolling window and the input sum all have two-or-more-element fixtures. The one survivor of the class is theunbounded.join(", ")inenforcer.rs, only ever exercised with one asset id — message-only, no branch, so a nit not a gap. AutoSendPolicy::limits_for(autosend.rs:134) now has no caller in-crate and none in the only consumer; the gate usesconfigured_limits. Public surface with no user.production_sources()splits on an LF-only"#[cfg(test)]\nmod tests {"; on a CRLF checkout the split misses and every structural invariant silently scans the whole file — looser, not stricter. Worth normalizing newlines.
Readability (2.5) is genuinely high across approval.rs, enforcer.rs and summary.rs: one purpose per function, guard-clause style, no nesting past 3, doc-comments on every public item, WHY-comments that name the defect each guard exists for. The SpendRuling helpers (enforcer.rs:158-194) are exhaustive per variant and panic with the outcome they demanded, so they cannot silently accept the wrong variant. RequireAuth is gone from code, SPEC 9, 6.3 and the tests with no stale reference surviving (the mentions in error.rs:50, authorizer.rs:5 and the CHANGELOG are deliberate history). 6.1.1 still states all three deferrals as limits.
Verified by running: the suite, four guard probes, the multi-line-signature bypass, the probe harness, cargo mutants --file src/wallet/authorizer.rs, the SPEC/README/CHANGELOG stale-symbol sweep, the dig-app manifest claim. Inferred by reading only: the 20-unviable Default::default() explanation (consistent with SpendApproval/SpendRuling having no Default, and with 3 of my 7 authorizer mutants also being unviable), and that the fail-closed arms of sign_approved still hold via the dependency. Not examined: the full 59-mutant run over the six listed files (I re-ran only the omitted module), the 96.06% coverage report, dig-wallet-backend internals, and whether the guard probe was re-run after the LAST fix — that last one is not verifiable at all, which is finding 2.
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
R5: a PolicyAuthorizer now stamps a CustodyScope (profile index, plus the configured wallet for a hot profile) onto every permission it mints, and sign_approved refuses one minted by a different profile's gate. Without it a host holding a hot gate for profile 0 and a signer for vault profile 1 could route profile 1's spend through profile 0's gate: it auto-approves (the gate judges outputs, never whose key owns the inputs) and profile 1's signer holds the key, so the vault destination rule and clawback window never ran. The two sides have genuinely different provenance -- the scope comes from the host's persisted custody configuration at gate construction, the signer's from the live master seed at sign time -- so this is a real comparison rather than two derivations of one input. Also: C4 routes the three gate-refusal tests through dig-account's own derivation instead of the dependency's; C5 LF-normalizes the production/test split so the structural invariants hold on a CRLF checkout; the summary single-derivation invariant now tracks analyze(); and the confirm-twice compile-fail fixture follows confirm_with. Refs #1702 Co-Authored-By: Claude <noreply@anthropic.com>
…dening Recovered from an interrupted lane. Pushed as-is before further work so a third interruption is not lossy. Co-Authored-By: Claude <noreply@anthropic.com>
…kout The structural invariants in the shape suite are all written as LF-only needles, so an un-normalized scan on a CRLF checkout silently misses the `#[cfg(test)]` marker (in-crate test code is then scanned as production) and misses every needle (the invariants pass on an empty match set). The normalization that fixes this is a no-op in the only environments it currently runs in -- this checkout and CI are both LF -- so it was shipped untested. Extract the split into `production_half` and pin it with an explicit CRLF fixture; removing the `replace` now fails the suite. Closes C5. Co-Authored-By: Claude <noreply@anthropic.com>
…total R1's hot-wallet leg was covered; its vault leg was not. The destination rule iterates `summary.recipients`, so while that list held only hinted outputs a vault spend paying a stranger with no memo presented an EMPTY recipient list: the rule looped zero times, returned Ok, and the spend escalated as an ordinary vault move -- skipping the 24h clawback window entirely. Varies exactly one field (the memo) from the hinted third-party vault spend that is already refused, so the refusal is attributable to hint-blindness and the hinted sibling stands as its control. RED evidence: dropping `.chain(effect.change.iter())` from summary.rs fails this with "expected a refusal, got an escalation to the human". Closes R1 (vault leg). Co-Authored-By: Claude <noreply@anthropic.com>
…ng summary Co-Authored-By: Claude <noreply@anthropic.com>
… boundary The dependency's four amount accumulations now route through a fallible `accumulate`, so a spend whose created-output amounts sum past u64::MAX is refused instead of wrapping modulo 2^64 and passing value conservation. `a_spend_whose_output_amounts_overflow_is_never_approved` pins dig-account's side of that boundary and FAILS against 0.16.0, so the version floor is load-bearing. Its companion pins the same bound from below, so the guard cannot degrade into "refuses large amounts". BREAKING CHANGE: the dig-wallet-backend floor moves from 0.16 to 0.16.1. Co-Authored-By: Claude <noreply@anthropic.com>
…comes SPEC.md's wire table gave SPEND_DENIED -33053 to a declined ceremony and SPEND_NOT_AUTHORIZED -33052 to PolicyDenied, while the decline path already returned PolicyDenied -- so the normative mapping named two codes for one value and no conforming host could exist. The table now keys every row on a distinct AccountError variant, and states that it MUST be a function of the outcome alone. Co-Authored-By: Claude <noreply@anthropic.com>
…er it probe-guards.sh shares ONE backup file across every probe. A `cp` that failed left the PREVIOUS probe's contents in it, and the restore path then wrote that unrelated source into the target -- so a stale file path could silently corrupt a different module. The backup is now taken before the file is claimed, and a probe that cannot read its target is a counted PATTERN-MISS that never reaches restore. Co-Authored-By: Claude <noreply@anthropic.com>
|
Round 4 pushed at Answering each thread directly:
R2 is real. dig-wallet-backend 0.16.0 -> 0.16.1; Not merging — handing back for the gates. The security leg has not yet run on this PR. |
… spender A wrapper layer's coin.puzzle_hash is not a payable address. Excusing an output because it paid some spent coin's puzzle_hash was therefore wrong for every wrapped coin: a CAT coin's hash is the CAT layer curried over its tail and inner puzzle, so mojos paid there are destroyed rather than returned, because the CAT layer demands a lineage proof no XCH parent can supply. Executed exploit, now a regression test: with any CAT in the wallet (a dust airdrop suffices - permissionless, needs only the synthetic pk), spend it with un-hinted CAT change, then pay 999_999 mojos to the CAT coin's own puzzle hash with a 1 mojo fee. The gate weighed 1, auto-approved under a 10-mojo limit and a 10-mojo cap, charged the ledger 1, escalated PAST the vault destination rule, and produced a real aggregate. Four bounds bypassed at once: per-transaction limit, rolling cap, vault destination rule, 24h clawback window. Impact is irreversible destruction of the victim's XCH, not theft - exfiltration stays closed because every spend must be key-owned to sign at all. The repair is not a denylist of wrapper layers; the next layer would walk past it. It inverts the burden: an output is excused ONLY when this crate can PROVE its destination is a bare p2_delegated_puzzle_or_hidden_puzzle whose curry reproduces that coin's own puzzle_hash. Every uncertain case - a wrapper, an undecodable reveal, a driver error, a curry that does not match - omits the hash and so COUNTS the output. NFT, DID, singleton and offer coins all have the same property and are all handled without naming them. Co-Authored-By: Claude <noreply@anthropic.com>
…false SPEC.md carried two claims that were false in BOTH directions and two that named code which no longer exists - the same false-security-claim class this PR set out to delete, in the document written to prevent it: - 6.4's charged-total rule justified excusing any spent coin's puzzle_hash as equivalent to "xch_in minus change returning to the spender". That equivalence holds only for a bare p2 coin; for every wrapper it is false, which is the live defect just fixed. Restated as a proof obligation, with the wrapper hazard and the allowlist-not-denylist requirement made normative. - 6.4 step 2 said the vault rule reads "every recipient", implying the hinted list. It reads the charged destination list. - 6.4 said the dependency's output accumulation is unchecked and "a test reaching it cannot yet be written". Both false since 0.16.1 is the required floor and a_spend_whose_output_amounts_overflow_is_never_approved executes. - 6.3 named PendingApproval::confirmed, which no longer exists; the normative route is confirm_with through the AuthProvider seam, and that there is NO public method taking a SpendDecision is itself the property worth stating. Cargo.toml's include omitted tests/compile_fail/*.stderr, so the trybuild custody proofs - this crate's headline evidence - could not run from a published tarball. Verified with cargo package --list. Also pin the two guards this round added (the confirmation-prompt ceiling and the ledger entry ceiling), so the probe harness reports 24 of 25 load-bearing with one documented exemption rather than two untested guards. Co-Authored-By: Claude <noreply@anthropic.com>
Implements the #1552 decision (comment
5094382373) for dig-account 0.2.0. Tracked asDIG-Network/dig_ecosystem#1702. dig-account only — release-first, dig-app adopts as a separate unit.The problem this closes
0.1.2 shipped tier classification, per-transaction limits, a rolling period cap and vault clawback, verified hard (138 tests, 96.11% coverage, 24/24 mutants killed). None of it reached a running code path. Two design seams are why, and both are closed here rather than patched.
SpendAuthorizerwas a custody gate whose simplest implementation approves everything, and the only consumer duly shipped exactly that:dig-app's default authorizer body wasOk(())(crates/dig-app-core/src/account/auth.rs:110-115). So the crate is not fixed by retyping the trait; the trait is the defect.Result<()>cannot say "not yet, ask the human". A consumer handed only Ok/Err collapses every refusal into one (money.rs:137-139), which makes the confirm ceremony unreachable for precisely theConfirmandVaulttiers that exist to require it.The shape
PolicyAuthorizer::authorize_op(&[CoinSpend], SpendOpClass) -> Result<SpendRuling>derives the summary itself, so no caller-supplied description exists to disagree with.SpendApprovalthat OWNS the exactCoinSpends it authorized. Deliberately not a digest binding: a comparison is a step that can be skipped, mis-scoped, or run over the wrong bytes, whereas a single owned value cannot be mismatched.MoneySigner::sign_approved(SpendApproval) -> SpendBundleis the only signing entry point, and the bundle carries the approval's own spends, so a caller cannot pair the signature with different bytes.Clone, taken by value), no nonce, no expiry, neverSerialize/Deserialize/Debug.SpendRuling::RequiresConfirmationis the third state.SpendOpClass::Undeclared— what every out-of-process request is — now escalates to the human rather than dead-ending asPolicyIndeterminate.The enforcement is the REMOVALS
MoneySigner::sign_coin_spends&[CoinSpend]into a signature. This is the enforcement pointSPEC.md§6.2's "authorized before signed" MUST never had.LocalMoneySigner::sign_unsigned→pub(crate)summary— the same defect one layer over. Zero external callers.SpendAuthorizertraitAccountError::RequireAuthreclassifySPEC.md:216-228carve-outsSpendSummary::newandWalletOps::summarizestay public and now confer no authority: nothing accepts a&SpendSummaryfor a custody decision, so a hand-built summary has nowhere to go.WalletOps::money_signerstayspubfor the same reason — holding a signer confers no ability to sign anything unauthorized.Blast radius checked
gitnexus is disabled in the loop (CLAUDE.md §2.0 temp override), so this was done by direct read + ripgrep across
modules/.dig-account: exactly one.grep dig-account --include=Cargo.tomlacrossmodules/findsdig-appand nothing else.dig-wallet-backendis a dependency, not a consumer, and is untouched.sign_coin_spends,SpendAuthorizer,authorize,authorize_op,sign_unsigned,RequireAuth,SpendSummary::new. In-crate call sites all migrated; the only external references are indig-app(below).git show --stat): 25 files, +2070/-855. The only production modules touched aresrc/error.rs,src/lib.rs,src/wallet/{approval,authorizer,autosend,enforcer,money_signer,mod,summary}.rs. No key-derivation, keystore, DEK, session, or mint code is touched, so the frozen byte-contracts ofSPEC.md§3 are untouched and their golden vectors still pass.0.xMINOR is semver-breaking:^0.1cannot resolve0.2.0, so the consumer bump is mandatory, not optional. Requesting the full triple gate.Consumer work this creates (NOT done here)
dig-appmust bump and migrate. One correction to the ticket: only ONE manifest carries the version.dig-app/Cargo.toml:82isdig-account = "0.1.1"and must become"0.2";crates/dig-app-core/Cargo.toml:37isdig-account.workspace = true, which inherits it — there is no version literal there to change.Migration: replace
authorize(&summary)/authorize_op(&summary, class)withauthorize_op(&coin_spends, class); match the ruling (Approved(a)→sign_approved(a),RequiresConfirmation(p)→ renderp.summary()thenp.confirmed(decision)); deleteAlwaysConfirmAuthorizer; construct a realPolicyAuthorizerfrom persisted config (#1560 becomes required); fix theSpendSummary::newfixtures atauth.rs:228andceremony.rs:299.Acceptance evidence
The #1698 exploit is UNWRITABLE, not merely rejected.
tests/compile_fail/the_1698_exploit.rs+ its recorded.stderr: authorizing a hand-built one-mojo summary isE0308(the gate takes spends, not a description) and signing the real billion-mojo spends isE0599(no such method). Both halves fail independently.Three more compile-fail cases, each with the recorded verdict checked to be the right error rather than an incidental one:
E0624× 2, privacy only (the constructors are merely named, so no argument-count noise could stand in for the privacy error)E0382use-after-moveE0382use-after-movePlus non-brittle trait-absence assertions (
static_assertions) that neither token implementsClone/Serialize/DeserializeOwned/Debug, and four structural invariants as tests: exactly one production module mints an approval (the gate), one assembles its fields, one callsderive_summary, and no production function declares a signing signature over&[CoinSpend].E0599above, plusno_signing_function_accepts_bare_coin_spends, which catches the same door being reopened under a different name.the_rolling_cap_is_charged_the_spends_own_value_so_a_small_description_cannot_understate_itapproves a 900_000 spend and assertsledger_total == 900_000, then requires a 200_000 spend to escalate against a 1_000_000 cap. Under the old API the same sequence with hand-built 1-mojo summaries left the ledger at 1 and approved both.Confirm-tier spend reaches the confirm path and, once confirmed, becomes signable over the very same spends (a_confirm_tier_spend_reaches_the_ceremony_and_a_confirmation_makes_it_signable).an_undeclared_intent_escalates_to_the_human_rather_than_dead_endingpins the undeclared case from both sides of the auto-send switch, since only the switch-ON case reaches the op-class lookup at all.rg 'SpendAuthorizer'insrc/returns one hit, a doc-comment inauthorizer.rs:5explaining why the trait is gone.Every fixture is now a real coin spend. The old enforcer suite built hand-made
SpendSummarys; it could describe a spend as something it was not. Every amount asserted is now an amount the coin spends really move.Gates
cargo test: 144 unit + 5 shape tests, 0 failures.cargo fmt --checkclean;cargo clippy --all-targets -- -D warningsclean.cargo-mutantsoverenforcer.rs,approval.rs,summary.rs,money_signer.rs,autosend.rs,error.rs: 59 mutants, 39 caught, 20 unviable, ZERO SURVIVORS. The first run had 6 survivors, both real gaps, both now pinned (see below). The 20 unviable are mutants cargo-mutants cannot compile because it substitutesDefault::default()for a return type — andSpendApproval/SpendRulinghave noDefault, which is itself a consequence of this shape.Per-guard delete-probe (18 of 19 load-bearing)
Committed first, then each guard neutralized one at a time and the suite run —
scripts/probe-guards.shis checked in so this is repeatable rather than a claim.u64Two guards were discovered vacuous during probing, and both were dealt with rather than reported around:
u64::MAX, which the projection check already prevents from ever being recorded. The two are now one fold, and it is reachable (G13 RED).summary.rs,DerivedSpend::derive). G19 bounds the input total and the dependency then requires value conservation, so the native total is bounded before that line runs — the guard provably cannot fire. It is retained because that bound rests on a dependency's conservation check rather than on anything this crate can see: if that check is ever relaxed for a new spend shape, "what is this spend worth" must become a refusal, neveru64::MAXquietly standing in for a number nobody computed. Flagged in the code rather than left for a reader to discover.The first mutants run's 6 survivors, now pinned:
DEFAULT_PERIOD_SECONDS = 24 * 60 * 60— three survivors. It reads as self-evidently a day, which is exactly why nothing checked it: any arithmetic slip yields a plausible-looking window while a user told "a daily cap" gets some other period. The literal86_400is now asserted.SpendSummary'sDisplayseparator — three survivors. Every existing fixture had ONE recipient, so the "is this the first?" test was true every time and>=/</==all rendered identically. A two-recipient fixture asserting the whole line can see it; acontainscould not.Findings that need their own tickets
dig-wallet-backend0.16client/verify.rs:153accumulates spent-coin amounts with an unchecked+=. Those amounts arrive in a caller-supplied unsigned skeleton, so they are attacker-chosen and need not name coins that exist: an unsummable input total PANICS in a debug build and WRAPS in a release build, after which the wrapped figure is what value conservation is checked against. Found by trying to build an overflow fixture — the fixture panicked inside the dependency. Defended in dig-account here (G19, with a load-bearing test and a truthful control at halved amounts), but the upstream+=should be fixed too.SPEND_POLICY_INDETERMINATE -33056cannot be added in this repo. dig-account carries no wire error-code taxonomy at all — theSPEND_*set lives indig-app'sloopback/dispatch.rsand the extension'serrors.ts. NewSPEC.md§6.3.1 records the normative crate-outcome → wire-code mapping including-33056, so the dig-app step is byte-exact, but the constant itself must land there.Deliberately not done
SYSTEM.mdrow, thecanonicalentry for the-33050band and the invariant "the requester supplies unsignedcoin_spendsonly, never a summary and never an op class" are the orchestrator's.SpendApproval, no coin-to-tier linkage, no persisted rolling ledger — all named as deferred in the decision, and the SPEC still states them as limits (§6.1.1) rather than quietly dropping them.Version
0.2.0. A
0.xMINOR is semver-breaking —^0.1cannot resolve0.2.0— which is the correct level for removingsign_coin_spends, theSpendAuthorizertrait andAccountError::RequireAuth, and for changingauthorize_op's parameter and return types. It also makes the consumer bump mandatory rather than optional, which is the desired property: this crate must not be adoptable by accident.Round 4 — the four blocking review threads, plus R2
Every fix below is evidenced by EXECUTING the mutation (guard reverted, output observed, restored), never by reasoning about it.
T1 —
the_shape_is_unwritable.rsscanned per LINE (demonstrated false green)Fixed by normalizing each source to a whitespace-collapsed form before matching, so the crate's own formatter is out of the equation. Pinned by a fixture that reproduces the reviewer's exact rustfmt-wrapped door.
Re-verified by re-inserting that door into production
src/wallet/money_signer.rs:T2 —
scripts/probe-guards.shcould not run, and passed silently when it did notcd "$(dirname "$0")/.."fixed the cwd; PATTERN-MISS / INCONCLUSIVE / unexpected-GREEN each now fail the run. Full run against this head:And the silent-pass property itself, probed by pointing a guard at a file that is not there:
That probe found a second, worse defect in the harness, now fixed.
BACKUPis ONE shared temp file. Acpthat failed left the previous probe's contents in it, and the restore path then wrote that unrelated source into the target file — so a stale path could silently corrupt a different module (the first run of this probe literally createdsrc/wallet/enforcer.rs.MISSINGon disk). The backup is now taken before the file is claimed, and a probe that cannot read its target never reaches restore.T3 —
PolicyDeniedwas double-mapped in a normative cross-repo tableDecision: make the outcome distinguishable at the TYPE level, not document the ambiguity away. A host holding one variant cannot tell "the user said no" from "the rules say no"; those have different deciders, warrant different UI, and §6.3.1 assigns them different wire codes, so a single variant makes the normative mapping a non-function and no conforming host can exist.
The distinct variant
AccountError::UserDeclinedand thedecided()arm that returns it were already present in the code; what was stale was the specification and one doc-comment.SPEC.mdnow keys every row of both tables on a distinct variant (UserDeclined→SPEND_DENIED -33053,PolicyDenied→SPEND_NOT_AUTHORIZED -33052), states that the mapping MUST be a function of the outcome alone, and records why the previous revision could not be implemented.error.rsno longer documentsPolicyDeniedas covering a decline.Pinned by
a_user_refusal_and_a_structural_refusal_are_distinguishable_outcomes, which produces both halves from one gate and asserts the discriminants differ — two separate single-variant tests would survive a merge that updated only one of them. Mutation (decided()returnsPolicyDeniedagain):T4 — two tests asserted on the DEPENDENCY, discriminating no dig-account code
Both now enter through a
gate_derivationhelper that calls dig-account's ownSpendSummary::from_coin_spends. Proven by the stated mutation — makingderive_effectstop consultinganalyzeand return a fabricated effect:R2 — unblocked and now real
dig-wallet-backend0.16.0 → 0.16.1 (Cargo.toml + lock). All four accumulation sites route through a fallibleaccumulate, so a spend whose created-output amounts sum pastu64::MAXis refused rather than wrapping modulo 2^64 and passing value conservation.a_spend_whose_output_amounts_overflow_is_never_approvedpins dig-account's side of that boundary, entering throughauthorize_op. Its fixture is chosen FROM the bound rather than picked large: a1_000-mojo coin creatingu64::MAXand1_001, which wraps back to exactly1_000and therefore looks conserving. Executed against 0.16.0 (--precise 0.16.0):so the
0.16.1floor is load-bearing, not cosmetic. A companion pins the same bound from below — outputs ofu64::MAX - 1and1from a coin of exactlyu64::MAXmust still be accounted for and reach a ruling — so the guard cannot degrade into "refuses large amounts".Consequence for G2, stated honestly. With 0.16.1 the checked total at
DerivedSpend::deriveis provably unreachable: the driver accumulates every created XCH coin plus the fee throughaccumulateand then requiresxch_in == xch_out + fee, and this crate's native total is a subset of those coins plus that same fee. It is retained as defence-in-depth because the proof lives inside a dependency, and the probe exemption is re-argued from "blocked on #1708" to that proof. It still self-expires the moment the guard goes RED.Also fixed:
summary.rs:345cited a testa_spend_whose_output_amounts_overflow_is_never_approvedthat did not exist. It exists now. The enforcer module doc still claimed aSpendSummary"accounts only for HINTED outputs", contradicting the destination-based counting this PR shipped; corrected.Round-4 blast radius
Same single external consumer (
dig-app), re-confirmed bygrep dig-account --include=Cargo.tomlacrossmodules/. Symbols touched this round:derive_effect,DerivedSpend::derive(docs only),AccountError::PolicyDenied(doc only — no variant added or removed beyond what already shipped in this PR), plus test-only additions. The round's diff is limited toCargo.toml,Cargo.lock,SPEC.md,src/error.rs,src/wallet/{enforcer,summary}.rsandscripts/probe-guards.sh. Risk stays HIGH (breaking + custody); the security gate has not yet run on this PR.Consumer work this round ADDS for dig-app (deliberately NOT done here)
dig-app's loopback dispatch must map the newAccountError::UserDeclinedtoSPEND_DENIED -33053, and stop routing a declined ceremony throughPolicyDenied. Without it a user's refusal reports asSPEND_NOT_AUTHORIZED -33052. Single-writer: reported for a sibling lane, not edited here.