diff --git a/README.md b/README.md index ea16ad919be..435104fc9a4 100644 --- a/README.md +++ b/README.md @@ -1,4 +1,4 @@ -# ZClassic 2.1.2-beta6 +# ZClassic 2.1.2-ZIP209-beta6 ZClassic is an Equihash-based proof-of-work implementation of the Zerocash protocol. It offers privacy through shielded transactions using zero-knowledge proofs that preserve transaction confidentiality. Based on Bitcoin's code and derived from Zcash, ZClassic builds three main binaries: **zclassicd** (daemon), **zclassic-cli** (RPC client), and **zclassic-tx** (transaction utility). @@ -16,6 +16,23 @@ This software is the ZClassic client. It synchronizes the entire blockchain hist --- +## Consensus Hardening (ZIP209 builds) + +These builds advertise the network subversion `/ZClassic:2.1.2-ZIP209-beta6/` and carry the following consensus-validation and reliability fixes on top of upstream ZClassic: + +- **ZIP-209 shielded turnstile (mainnet).** A block that would drive the Sprout or Sapling shielded value-pool balance negative is rejected as invalid. This bounds the damage of any shielded soundness bug: value forged inside a pool cannot be withdrawn past the pool boundary undetected. Enforcement starts from a hardcoded Sprout value-pool checkpoint — see [doc/zip209-mainnet-reactivation.md](doc/zip209-mainnet-reactivation.md). +- **CR-01 — contextual checks during initial block download.** `ContextualCheckTransaction()` no longer returns early while the node is in initial block download / import / reindex. Transaction version and network-upgrade activation enforcement, JoinSplit Ed25519 signature verification, and all Sapling spend/output/binding checks now run in every node state, closing a node-state-dependent validation bypass. DoS ban scores stay reduced while syncing; only the checks are no longer skipped. +- **Reindex genesis-block crash fix.** `AcceptBlockHeader()` no longer dereferences a NULL `pindexPrev` for the genesis block (which has no predecessor), fixing a segmentation fault that aborted every `-reindex` / block import at startup. This is a reliability fix, not a consensus change, and it is what makes the CR-01 re-validation reindex above actually completable. The bug was latent because fresh nodes use anchor-pinned fast-sync rather than a from-genesis reindex. +- **ConnectBlock check-queue use-after-free (CVE-2024-52911 parity).** Queued script checks held a raw pointer into a local `txdata` vector that `~CCheckQueueControl()` could outlive on an early `ConnectBlock` return, dereferencing freed memory while draining still-queued checks — a remotely-triggerable crash (DoS) reachable with parallel script verification (`-par>=2`) when a late-failing valid-PoW block is connected. Fixed by declaring `txdata` before the check-queue controller so its lifetime always outlives the controller's `Wait()`. Memory-safety / reliability fix, not a consensus change. +- **Deep-reorg parking skipped during IBD/reindex.** The deep-reorg park in `AcceptBlock` fired spuriously on the out-of-height-order block loads of `-reindex`, parking large swaths of the chain and stalling a from-genesis reindex. It is now gated behind `!IsInitialBlockDownload()`. At-tip behaviour is unchanged — the latch in `IsInitialBlockDownload()` keeps parking active once the node has synced — and the skip window stays backstopped by depth-10 auto-finalization and the 99-block reorg cap. Reliability fix. +- **Checkpoint hash lock-in at header acceptance.** A header presented at a hardcoded-checkpoint height with the wrong block hash is now rejected in `ContextualCheckBlockHeader`, independent of current chain state — closing an eclipse / bootstrap gap where a fresh node could follow a forged chain that does not pass through the compiled checkpoints. Rule-tightening (soft-fork class); honest peers are unaffected. + +Details and the full security write-up for the last three items are in [doc/security-hardening-2026-06.md](doc/security-hardening-2026-06.md). + +**Deployment notes.** ZIP-209, CR-01, and the checkpoint hash lock-in are rule-tightenings (soft-fork class) and want a coordinated upgrade; the UAF and parking fixes are reliability fixes and are safe to deploy independently. Because the CR-01 fix changes how blocks are validated during sync, run a `-reindex` on a CR-01-fixed binary to re-validate existing chain state (a reindex on an *un*fixed binary does not re-validate, since reindex keeps the node in IBD). Full validation is correspondingly slower; fresh nodes can still use anchor-pinned fast-sync. + +--- + ## Quick Start ### Release Binaries diff --git a/depends/packages/rust.mk b/depends/packages/rust.mk index 18d5b131647..1e2c835196b 100644 --- a/depends/packages/rust.mk +++ b/depends/packages/rust.mk @@ -1,12 +1,12 @@ package=rust -$(package)_version=1.32.0 +$(package)_version=1.70.0 $(package)_download_path=https://static.rust-lang.org/dist $(package)_file_name_linux=rust-$($(package)_version)-x86_64-unknown-linux-gnu.tar.gz -$(package)_sha256_hash_linux=e024698320d76b74daf0e6e71be3681a1e7923122e3ebd03673fcac3ecc23810 -$(package)_file_name_darwin=rust-$($(package)_version)-x86_64-apple-darwin.tar.gz -$(package)_sha256_hash_darwin=f0dfba507192f9b5c330b5984ba71d57d434475f3d62bd44a39201e36fa76304 +$(package)_sha256_hash_linux=8499c0b034dd881cd9a880c44021632422a28dc23d7a81ca0a97b04652245982 +$(package)_file_name_darwin=rust-$($(package)_version)-aarch64-apple-darwin.tar.gz +$(package)_sha256_hash_darwin=75cbc356a06c9b2daf6b9249febda0f0c46df2a427f7cc8467c7edbd44636e53 $(package)_file_name_mingw32=rust-$($(package)_version)-x86_64-pc-windows-gnu.tar.gz -$(package)_sha256_hash_mingw32=358e1435347c67dbf33aa9cad6fe501a833d6633ed5d5aa1863d5dffa0349be9 +$(package)_sha256_hash_mingw32=52945bf6ab861d05be100e88a95766760d2daff1a0c0a2eff32a7fd8071495bd ifeq ($(host_os),mingw32) $(package)_build_subdir=buildos diff --git a/doc/security-hardening-2026-06.md b/doc/security-hardening-2026-06.md new file mode 100644 index 00000000000..703c4453ba0 --- /dev/null +++ b/doc/security-hardening-2026-06.md @@ -0,0 +1,189 @@ +# Security hardening — June 2026 + +This document records the security work integrated via the merge of +`feature/parkdeepreorg-ibd-gate` into `master`. It covers one fix, one +reviewed behaviour change, one hardening commit, and the status of the +remaining items from the June-2026 upstream-parity audit. + +> Line numbers are accurate as of `master` `9bf04ef32` and drift with later +> edits. Every reference also names its symbol/function — if a number looks +> off, grep the named symbol (e.g. `GENEROUS_TX_SIZE_LIMIT`) rather than trust +> the line. + +Scope of the merge (commits, newest first): + +| Commit | Summary | +|--------|---------| +| `28382a7cd` | Fix use-after-free in `ConnectBlock` check-queue lifetime (CVE-2024-52911 parity) | +| `dcbdf14e5` | Deep-reorg parking: skip during IBD/reindex (fixes from-genesis reindex stall) | +| `fb585fd29` | docs: TVSP proposal (v2) Markdown source | +| `b2da5f69f` | docs: Transparent-Value Shielded Pool (TVSP) proposal (v2) | +| `bbea3177e` | Harden ZIP-209 pool accounting: checked `CAmount` delta arithmetic | + +--- + +## 1. ConnectBlock use-after-free — FIXED (`28382a7cd`) + +**Class:** memory-safety use-after-free. Bitcoin **CVE-2024-52911** / +Zcash **GHSA-fqr9-fxpx-rfpf** parity. + +### Root cause +In `ConnectBlock` (`src/main.cpp`): + +- Queued script checks (`CScriptCheck`) hold a **raw** + `PrecomputedTransactionData*` into a local `txdata` vector + (`CScriptCheck::txdata`, `src/main.h`). +- `~CCheckQueueControl()` calls `Wait()` on **every** return — including + early returns — and `Wait()` drains any still-queued checks **on the + calling thread** (the "master" in `CCheckQueue::Loop`). +- `control` was declared **before** `txdata`. C++ destroys locals in reverse + declaration order, so on an early return `txdata` was destroyed **first**, + then `~control`'s `Wait()` ran the queued checks that dereferenced the + freed `txdata` → **heap-use-after-free**. + +### Reachability and impact +Reachable in normal operation: `-par>=2` (default) and a block above the last +checkpoint (i.e. live blocks near tip). An attacker mines a valid-PoW block +crafted to fail **late** — e.g. coinbase overpay (`main.cpp:~2765`) or bad +Sapling root (`main.cpp:~2754`) — after script checks are queued but before the +explicit `control.Wait()`. The control destructor then drains those checks +against freed memory. **Impact: remote DoS (node crash).** RCE is theoretically +in the UAF class but not demonstrated. + +### Fix +Declare `txdata` (and its `reserve()`) **before** `control`, so reverse-order +destruction always runs `~control`'s `Wait()` before `txdata` is destroyed, on +every return path. One-line move; `reserve()` still precedes the first +`emplace_back`, preserving pointer stability. See the inline comment at the +declaration site in `ConnectBlock`. + +### Verification +- **gtest** (`src/gtest/test_validation.cpp`): + `Validation.CheckQueueControlDrainsQueuedCheckBeforeTxdataDestroyed`. + Deterministic (no worker threads → the queued check runs inside `~control`'s + `Wait()` on the calling thread). `EXPECT_TRUE(ran)` guards the + drain-before-destroy contract without needing a sanitizer; under + AddressSanitizer the buggy declaration order reports `heap-use-after-free`. + Compiles and passes. +- **AddressSanitizer proof** (standalone reproducer, throwaway, using the real + `CCheckQueue`/`CCheckQueueControl`): buggy declaration order → + `heap-use-after-free` in `~CCheckQueueControl → Wait → Loop → check()`; + fixed order → clean exit. Both observed. + Note: `--enable-asan` is unusable under Apple clang (`-static-libasan` + unsupported); the proof used `clang -fsanitize=address` directly. + +--- + +## 2. Deep-reorg parking IBD gate — REVIEWED, SAFE (`dcbdf14e5`) + +### What it changes +Gates the deep-reorg park in `AcceptBlock` behind `!IsInitialBlockDownload()`: + +```cpp +if (GetBoolArg("-parkdeepreorg", true) && !IsInitialBlockDownload()) { ... } +``` + +### Why +During `-reindex`, blocks are loaded from `blk*.dat` out of height order, so +`chainActive.FindFork(pindex)` sees spurious deep forks (`fork depth > 1`) and +parks large swaths of the chain — the stall that forced `-parkdeepreorg=0` on a +from-genesis reindex (observed at height ~478543/478544). + +### Review conclusion: safe +The "Deep Reorg Protection" feature (`d57bf7a5e`) has **three** layers; this +commit relaxes only the softest, and the two harder layers remain active during +the skip window: + +| Layer | Behaviour | During the IBD skip window | +|-------|-----------|----------------------------| +| Parking (this commit) | Preemptively parks deep-fork blocks; unparks at 2× work | **Skipped during IBD** | +| Auto-finalization, depth `-maxreorgdepth` (default 10) | Finalizes a block 10 deep; conflicting forks rejected (`bad-fork-prior-finalized`, `main.cpp:3089`) | **Still active** (`main.cpp:~3253`) | +| `MAX_REORG_LENGTH = COINBASE_MATURITY-1 = 99` | Node shuts down on any reorg > 99 blocks (`main.cpp:3667`) | **Always active** | + +Key safety property: `IsInitialBlockDownload()` **latches to false permanently** +after first catch-up. So after a node first reaches tip, parking is **always +on** again and cannot be forced off by an attacker staling the tip +(eclipse/partition). At-tip behaviour is therefore unchanged — the 51%/deep-reorg +defense is intact at tip. Parking is skipped only during `fReindex`/`fImporting` +or a node's genuine first sync. + +### Residual nuance (informational, not a blocker) +The latch resets on process restart. A node restarted after being offline +> `nMaxTipAge` (~24h) re-enters `IBD=true` until it catches the backlog, so +parking is skipped during that catch-up. Finalization (depth 10) and the 99-block +cap still apply, and a visibly-catching-up node should not be trusted for +confirmations. Exposure is bounded and acceptable. + +--- + +## 3. ZIP-209 checked arithmetic — included (`bbea3177e`) + +Overflow-safe `CAmount` delta arithmetic for the shielded-pool value tracking +(`CheckedAdd`/`CheckedAddTo`). Hardening of pool accounting; no behavioural +change for in-range values. + +--- + +## 4. Checkpoint hash enforcement at header acceptance — FIXED (audit #2, `6b53a6916`) + +Added in a follow-up merge after the items above. + +### Gap +`ContextualCheckBlockHeader` rejected only forks strictly **below** the last +checkpoint *present in `mapBlockIndex`* (`GetLastCheckpoint` + +`nHeight < pcheckpoint->nHeight`). It never rejected a header presented **at** a +checkpoint height with the wrong hash, and it depended on chain state — so a +fresh or eclipsed node could accept a forged chain that does not pass through the +compiled checkpoint hashes. Upstream's `CheckIndexAgainstCheckpoint` was absent. + +### Fix +New `Checkpoints::CheckBlock(data, nHeight, hash)` (mirrors upstream) returns +false only when there is a checkpoint at `nHeight` and the hash differs. Called +in `ContextualCheckBlockHeader` under `fCheckpointsEnabled`: a mismatch is +rejected with `DoS(100)` / `REJECT_CHECKPOINT` / `"bad-fork-checkpoint"`. It +reads the **hardcoded** checkpoint map directly, so it is independent of +`mapBlockIndex` state and protects a fresh/eclipsed node during bootstrap. + +### Safety +Pure tightening: canonical headers at checkpoint heights match by definition, so +honest peers are never rejected; an empty checkpoint map (regtest) is a no-op. No +activation height, no coordination. Unit test +`Checkpoints_tests/checkpoint_hash_lockin` covers no-checkpoint / match / +mismatch; all `Checkpoints_tests` pass and the UAF gtest still passes. + +--- + +## Audit items still OPEN (not yet implemented) + +From the June-2026 upstream-parity audit. Priority order (#2 now done — see §4): + +1. **#3 — reindex size band-aids loosen live consensus (High).** + `GENEROUS_BLOCK_SIZE_LIMIT = 2MB` (`main.cpp:4370`) vs `MAX_BLOCK_SIZE = + 200000` (`consensus.h:22`); `GENEROUS_TX_SIZE_LIMIT = 2MB` non-contextual + (`main.cpp:1230`) with the tight `MAX_TX_SIZE_AFTER_SAPLING = 102000` + (`consensus.h:27`) enforced only inside the **pre-Sapling** `!saplingActive` + branch (`main.cpp:1040`) → post-Sapling tx size effectively + unbounded to 2MB for live blocks. These are local patches, not an intentional + hardfork. **Load-bearing for reindex** (confirmed: real canonical tx at height + 478544 is 125,811 B; another 122,415 B at 478596 — both exceed 102000). Fix: + keep the non-contextual bounds generous (so historical reindex passes) and add + the tight limits in `ContextualCheckTransaction`/`ContextualCheckBlock` gated + on a future activation height (coordinated soft fork). + +2. **#4 — header-DoS hardening gap (Medium, unproven).** No modern + `nMinimumChainWork`/headers-presync staging. Has `MAX_HEADERS_RESULTS=160` + + checkpoint fork-rejection. Hardening gap, not a demonstrated exploit. + +3. **#5 — timestamp adjustment (Low).** Raw `nTime - GetTime()` (`main.cpp:6225`). + Self-limiting (200-sample freeze; protective per the in-code issue-#4521 + note). Signed-overflow hardening only. + +### Confirmed sound (no action) +Value conservation / no-inflation path: coinbase overpay rejected +(`main.cpp:~2765`), input/value conservation (`main.cpp:~2141`), ZIP-209 +negative-pool checks (`main.cpp:~2597`), Sapling/JoinSplit verification +(`main.cpp:~951`). Duplicate-input protection (`1342`), CVE-2012-2459 merkle +malleability (`4357`), Overwinter non-overwintered-tx rejection (`1027`). No +NU5/Orchard code in this fork. Structural block checks **do** run at +`AcceptBlock` (`CheckBlock` default `fCheckSizeLimits=true`, `main.cpp:4604`) — +only the redundant `ConnectBlock` re-check skips them below checkpoint. diff --git a/doc/zclassic-transparent-value-shielded-pool.html b/doc/zclassic-transparent-value-shielded-pool.html new file mode 100644 index 00000000000..0955f86f8f5 --- /dev/null +++ b/doc/zclassic-transparent-value-shielded-pool.html @@ -0,0 +1,315 @@ + + + + + +Transparent-Value Shielded Pool (TVSP) for Zclassic + + + +
+ +
+

Transparent-Value Shielded Pool (TVSP) for Zclassic

+

A shielded pool that makes per-transaction value conservation public and exact — removing the homomorphic value-balance / binding-signature soundness class — while keeping participant privacy. It narrows, but does not eliminate, proof-system counterfeiting risk.

+

Status: Draft proposal (v2, corrected security claims)  ·  Target: Zclassic, Sapling/Groth16 lineage  ·  Audience: protocol engineers, reviewers

+
+ +
+ What changed in v2. v1 of this document overclaimed. It said public amounts make the supply “provably non-inflatable” and that any soundness bug becomes “immediately visible.” That is false: the privacy that hides which note is spent also hides whether the value claimed for that note is legitimate. Public amounts make value arithmetic public — they do not make note existence or uniqueness public. v2 states the honest, narrower claim and is explicit about what remains inside the proof. The Orchard premise is also corrected to match the official Zcash Foundation disclosure. +
+ +

1. Abstract

+

Every privacy coin that hides transaction amounts inside a zero-knowledge proof concentrates a lot of trust in one place: the soundness of the shielded circuit (and, for Groth16, its trusted setup) plus the binding of its value commitments. If the value-conservation argument inside the proof is wrong, value can be forged inside the pool, and because the amounts are hidden the forgery carries no obvious on-chain signature — it is contained, but not necessarily seen, by a turnstile.

+

The June-2026 Zcash Orchard incident is the reference point, and it is worth stating precisely (see §2.1): it was a soundness bug in the halo2_gadgets Orchard Action circuit that could “accept invalid state transitions … potentially permitting double-spending of funds within Orchard.” The Zcash Foundation was explicit that there was “no ability to inflate the total ZEC supply, which is protected by Zcash's turnstile mechanism.” So the canonical 2026 incident is a double-spend / invalid-state-transition bug whose supply impact was contained by the turnstile — not the “unlimited undetectable minting” of market commentary.

+

This proposal takes a deliberate design choice for a new Zclassic shielded pool: make the note value public, keep the participants private. The amount of every note is published in the clear; the proof is reduced to spend authority + note membership + value-binding + nullifier correctness and no longer carries a homomorphic value balance. Value conservation becomes ordinary public integer arithmetic enforced by consensus.

+

The honest result:

+ +

And the honest limits (§6 in full):

+ +

This is a different privacy model than Zcash, not a strictly superior one. It trades amount-confidentiality for a smaller, public, exactly-auditable value surface — with the residual ZK risk contained, not eliminated.

+ +

2. Motivation

+ +

2.1 The Orchard incident, stated correctly

+

The official Zcash Foundation disclosure (Zebra 4.5.3 / 5.0.0 emergency soft fork, NU6.2) describes the bug as:

+
“a soundness bug in the implementation of the Orchard zero-knowledge proof circuit in the halo2_gadgets crate … could allow the Orchard pool to accept invalid state transitions … potentially permitting double-spending of funds within Orchard.”
+

and, crucially:

+
no ability to inflate the total ZEC supply, which is protected by Zcash's turnstile mechanism … [the turnstile] tracks the total ZEC balance across all value pools [and] provided a ground truth that ecosystem participants could use to confirm the supply cap remained intact.”
+

The advisory is tracked as GHSA-jfw5-j458-pfv6 (Critical), mitigated by a soft fork temporarily disabling Orchard actions. The disclosers' official post-mortem (The Orchard Counterfeiting Vulnerability — And Next Steps) gives the most precise primary root cause: “an under-constrained element of the Orchard circuit, because of which it was possible to put arbitrary false inputs into an elliptic curve multiplication and still have the multiplication check pass.” No primary source names a specific gadget; the finer “variable-base scalar-multiplication” attribution circulating in community and third-party write-ups is not in any primary disclosure, so this document does not assert it.

+

Two lessons for this proposal, and both cut against the v1 framing:

+
    +
  1. The 2026 incident is a double-spend / invalid-state-transition bug — exactly the class TVSP does not fully fix (see §6.2). It is not an example of undetectable supply inflation.
  2. +
  3. In that incident the turnstile provided the containment / ground-truth mechanism; had exploitation occurred, it would have bounded cross-pool extraction at the supply level. The turnstile is not “only a seatbelt that proves nothing”; it is a real, load-bearing layer — which is precisely why TVSP keeps it (made exact), rather than replacing it.
  4. +
+

For context, the April-2026 Zcash advisories also included a turnstile-accounting bypass (a re-seen block header silently overwriting a block index's pool-balance fields with nullopt, disabling enforcement across blocks) and a signed-integer overflow in per-pool value-delta computation that “could potentially have caused turnstile checks to be skipped, consensus validation to return early, or pool balance values to be computed incorrectly.” These are directly relevant to TVSP's implementation surface (§7): TVSP moves more logic into public pool arithmetic, so that arithmetic must be overflow-safe and applied on every path.

+ +

2.2 The structural problem TVSP targets

+

In Sapling (Zclassic's current shielded pool) the value v of a note is never revealed. It is carried in a homomorphic Pedersen value commitment

+
cv = [v]·V + [rcv]·R          (V, R = fixed independent generators)
+

and a binding signature over the sum of all cv proves

+
Σ cv(inputs) − Σ cv(outputs) = [valueBalance]·V
+

i.e. that inputs and outputs balance — without revealing any individual v. Conservation therefore rests on two in-circuit / commitment properties being simultaneously correct:

+
    +
  1. the value commitment being binding, and
  2. +
  3. the Spend/Output circuit being sound so the cv matches the note actually owned.
  4. +
+

If either fails, an attacker can make a transaction appear to conserve value when it does not, minting value inside the pool. TVSP removes this specific failure mode by deleting cv and the binding signature and checking conservation as public integers instead (§4.5).

+ +

2.3 What the turnstile does and does not do — precisely

+

A turnstile (Zcash's ZIP-209; the same mechanism Zclassic now enforces — see §2.4) tracks the pool's net balance and rejects any block that would drive it negative. It is genuinely valuable (it held supply during the Orchard incident), but its guarantee is net solvency bounded by liquidity, not per-note provenance:

+ + + + + + + +
The turnstile doesThe turnstile does not
Cap the maximum extractable counterfeit at the pool's real liquidityPrevent extraction up to that liquidity
Detect insolvency at the boundary (when net balance would go negative)Detect a counterfeit while the pool stays net-solvent (latent)
Contain counterfeit inside the pool boundaryRecover funds, identify the attacker, or prove no counterfeit exists internally
+

This is true under Sapling and under TVSP. TVSP makes the turnstile exact and public (anyone can recompute it), but does not change its essential nature: it bounds, it does not detect-per-note. The improvement TVSP brings is to the value-conservation check (§4.5), not to the turnstile's containment semantics.

+ +

2.4 Status in the current Zclassic tree

+

This is no longer aspirational: the current ZClassic tree enables ZIP-209 on mainnet (src/chainparams.cppfZIP209Enabled = true, Sprout checkpoint at height 3,000,000, balance 1316412375709), and ConnectBlock rejects negative Sprout/Sapling pool balances under if (chainparams.ZIP209Enabled()) (src/main.cpp:2590). Sapling nullifier double-spend checks are present (src/coins.cpp:596). An exhaustive --full history scan found negative_pool_events: 0 across all ~3.13M blocks, and both pools are net-positive at the tip — strong evidence no net drain has occurred, but (as the audit notes themselves state) not proof of the absence of a latent, unexploited circuit bug. That gap is exactly what motivates moving value out of the proof.

+ +

2.5 The design goal

+
Keep who private. Make how much public, conserved by plain arithmetic, and shrink the proof to the smallest authority+membership+nullifier statement.
+ +

3. Design overview

+

We introduce a new shielded transaction type — the Transparent-Value Shielded Pool (TVSP) — alongside (not replacing) the existing transparent and Sapling pools. Funds enter via a shield action and leave via an unshield action; internal transfers move value between TV-notes.

+

A TV-note is the same as a Sapling note except its value is public:

+
TV-note = ( diversifier d, pk_d, value v, rcm )      // v is published in the clear
+commitment  cm = NoteCommit( g_d, pk_d, v )           // unchanged: cm still commits to v
+

Compared to Sapling, the change is surgical:

+ + + + + + + + + + + + +
ComponentSapling (today)TVSP (proposed)
Note value vhidden (witness)public (in the tx, and a public input to the circuit)
Value commitment cvpresent ([v]·V+[rcv]·R)removed
Binding signaturepresent (proves Σcv balances)removed
Value-conservation checkhomomorphic commitment balance (in-proof)plain public arithmetic in consensus
Spend circuit provesauthority + membership + nullifier + value commitmentauthority + membership + nullifier + value-binding of public v to cm
Nullifier nfBLAKE2s(nk‖ρ)unchanged (own domain — see §7)
Spend authority rkak + [ar]·Gunchanged
Note encryptionencrypts v to recipientrecipient still gets a memo/key; v no longer needs hiding
+

Everything that provides participant privacy (membership-hiding spend, hidden spend authority, nullifiers, diversified addresses) is kept. The value commitment and binding signature are removed, and the amount is published and conserved by consensus arithmetic. The value-binding constraint (cm opens to the declared public v) moves into the proof and becomes a high-value audit target (§6.2).

+ +

4. Technical specification

+ +

4.1 Note and commitment

+

A TV-note commits to its value exactly as Sapling does:

+
cm = PedersenHash( NoteCommit_personalization, [ value(64 bits) ‖ g_d(256) ‖ pk_d(256) ] ) + [rcm]·NoteCommitRandomness
+

The only change is that value is also revealed in the clear in the spend/output description, and the circuit takes value as a public input (not a private witness). The circuit must prove that the opened cm uses exactly that public value. This binds the public value to the note — conditional on the NoteCommit gadget and the membership path being sound (see §6.2; this is not a free public guarantee, it is an in-circuit one).

+ +

4.2 Transaction format

+

A new transaction version TVSP_TX_VERSION (new nVersionGroupId, new consensus branch ID) carries:

+
tvSpends[]  : { anchor, nullifier nf, rk, value v_in (PUBLIC), zkproof }      // no cv, no bindingSig
+tvOutputs[] : { cmu, ephemeralKey, encCiphertext, value v_out (PUBLIC), zkproof }
+valueBalanceTVSP : signed integer (net public value leaving the pool to transparent)
+

v_in and v_out are explicit public integers. There is no bindingSig. The sighash must cover all public value fields and valueBalanceTVSP.

+ +

4.3 Spend circuit (TVSP)

+

Identical to the Sapling Spend circuit minus the value-commitment gadget. It proves, for a public value v_in:

+
    +
  1. The prover knows ak, nsknk = [nsk]·G, ivk = CRH(ak, nk).
  2. +
  3. rk = ak + [ar]·SpendAuthGenerator is exposed (re-randomized spend-auth key; the spend signature verifies against rk).
  4. +
  5. The note commitment cm = NoteCommit(g_d, pk_d, v_in) opens to the public v_in (value-binding constraint).
  6. +
  7. cm is a member of the note-commitment tree at the public anchor (Merkle path), gated by v_in ≠ 0 (dummy notes exempt).
  8. +
  9. nf = PRF^{nf}(nk, ρ) is correctly derived and exposed.
  10. +
+

It does not prove anything about a value commitment, and there is no homomorphic value balance inside the proof. Constraints (3), (4), (5) are the residual soundness-critical surface (§6.2).

+ +

4.4 Output circuit (TVSP)

+

Identical to Sapling Output minus the value-commitment gadget. It proves, for a public value v_out, that cmu is a correct note commitment to (g_d, pk_d, v_out).

+ +

4.5 Consensus: value conservation is now public arithmetic

+

TVSP mirrors the existing Sapling convention exactly — it does not invent a per-pool fee equation. Define the net public flow as a single signed quantity:

+
valueBalanceTVSP  :=  Σ tvSpends[i].v_in  −  Σ tvOutputs[j].v_out
+

with the same sign meaning as Sapling's valueBalance: positive = value leaving the TVSP pool into the transparent value pool (acts like a transparent input); negative = value entering the TVSP pool from transparent (acts like a transparent output). Consensus folds it into the global transaction value pool exactly as the code does for Sapling (GetShieldedValueIn() / GetValueOut()):

+ +

The fee is the single global remainder across all value pools, never attributed to the TVSP component:

+
nValueIn  =  Σ(transparent vin)  +  GetShieldedValueIn()     // incl. +valueBalanceTVSP, sprout vpub_new, sapling
+require    nValueIn  ≥  GetValueOut()                         // GetValueOut incl. −valueBalanceTVSP
+fee       =  nValueIn  −  GetValueOut()                       // one global subtraction (CheckTxInputs)
+

There is no + fee term inside any per-pool balance equation. (An earlier draft wrote Σ v_in == Σ v_out + fee + valueBalanceTVSP; that wrongly localizes the fee to the shielded pool and is rejected — see src/main.cpp:2135,2140,2146 and src/primitives/transaction.cpp:285,309, where the fee is the global remainder spanning transparent + sprout + sapling.) As CheckTransaction does for Sapling (src/main.cpp ~1252–1260), consensus also range-checks |valueBalanceTVSP| ≤ MAX_MONEY and rejects a non-zero valueBalanceTVSP when there are no TVSP spends/outputs.

+

The pool-level invariant (an exact, public turnstile) — accumulating the per-block delta as −valueBalanceTVSP, mirroring src/main.cpp:4117, with checked arithmetic (§7):

+
nChainTVSPValue  =  Σ (value shielded in)  −  Σ (value unshielded out)   ≥  0   at every height
+

What this does and does not guarantee. The global value-conservation rule is a hard public guarantee: a transaction whose inputs do not cover its outputs (across all value pools) is rejected in the clear, no circuit involved. The pool invariant is net solvency: it is exact and public, but — as §2.3 and §6.2 explain — it bounds counterfeit extraction at the pool's liquidity; it does not detect a counterfeit that is balanced internally or extracted within available liquidity. Per-note provenance is not public (privacy hides which note is spent), so the soundness of constraints §4.3(3)–(5) still matters.

+ +

4.6 What is no longer needed

+ + +

5. Privacy model — what you keep, what you lose

+ +

5.1 Kept (participant / graph privacy)

+ + +

5.2 Lost (amount privacy) — and the correlation caveat

+ + +

5.3 The fix: fixed denominations (and segregated denomination trees)

+

To preserve graph privacy with public amounts, TVSP should operate on fixed denominations (e.g. notes only in {0.01, 0.1, 1, 10, 100} ZCL). Then every spend and output of a given denomination is identical in value → no amount-correlation, and the anonymity set is “all notes of that denomination.” This turns TVSP into a denomination pool / mixer:

+ +

Security bonus: if each denomination has its own commitment tree and a spend declares which tree it draws from, then v_in is structurally fixed to that denomination. This upgrades the “open a real note to a higher v” defense (§6.2) from circuit-dependent to structural — you cannot spend a denom-1 note as denom-100 because it is not in the denom-100 tree. (Membership-forgery and double-spend within a denom tree still rest on the circuit.) Segregated denomination trees are therefore recommended both for privacy and for narrowing the residual soundness surface.

+

Honest core trade-off: strong graph privacy + public amounts is achievable only via fixed denominations. With free-form amounts you get public conservation but weak graph privacy.

+ +

6. Security analysis (corrected)

+ +

6.1 What TVSP genuinely fixes

+ + +

6.2 What TVSP does not fix (the honest limits)

+

TVSP does not make the supply “provably non-inflatable,” and over-claims are not “immediately visible.” Membership, value-binding, nullifier, and authorization soundness remain inside the proof, and the privacy that hides which note is spent also hides whether its claimed value is legitimate. Therefore:

+ +
+ The correct one-line claim: TVSP prevents incorrect hidden-value openings and removes the homomorphic-balance soundness class; it does not eliminate proof-system counterfeiting as a class. Any residual membership/value-binding/nullifier/authority bug still injects value that is bounded — not detected — by the (now-exact) turnstile. The win is a smaller, public, exactly-auditable value surface with the residual ZK risk contained, not eliminated. +
+ +

6.3 Where TVSP sits on the spectrum

+
Transparent (Bitcoin)   everything public · no privacy · supply trivially auditable · public UTXO arithmetic; no hidden-pool inflation opacity
+TVSP (this proposal)    WHO hidden · HOW MUCH public · per-tx conservation public+exact · net solvency public+exact
+                        · residual membership/nullifier/authority soundness still in-proof, turnstile-bounded
+                        · graph privacy strong only with fixed denominations                              ← here
+Sapling / Orchard       everything hidden · max privacy · supply rests entirely on circuit+binding+setup
+                        · value-forgery AND double-spend risk if a soundness bug exists (turnstile bounds supply)
+

TVSP is a coherent middle point: it sacrifices amount-confidentiality to make value conservation public/exact and to shrink the proof, while retaining identity/graph privacy and containing (not removing) the residual ZK risk.

+ +

7. Implementation risks — new code, new consensus surface

+

A correct TVSP fork is much more than new fields. Each item below is a place a bug would re-introduce the exact “accept bad value flows” class that the historical IBD bypass (CR-01) enabled for Sapling.

+ + +

8. Recommendations (if pursuing TVSP)

+
    +
  1. First, independently of TVSP: patch Zclassic's existing pool accounting with checked-delta arithmetic and add chain-value recomputation / checkpoint tests (this also hardens the live ZIP-209 path).
  2. +
  3. Treat the public-conservation and exact-turnstile enforcement as consensus-critical; add property-based / differential tests: replay historical flows, force over-claims and duplicate nullifiers, and assert that invalid TV txs are rejected even under isInitBlockDownload().
  4. +
  5. Use fixed denominations from the start for any claim of strong privacy, and prefer segregated denomination trees (privacy + structural value-binding). Document the anonymity-set math and correlation/timing risks.
  6. +
  7. Regenerate keys for the reduced circuit; strongly prefer a transparent-setup proving system for the residual authority+membership+nullifier statement (note: Halo 2-IPA is transparent but not post-quantum; only a hash-based STARK is plausibly post-quantum).
  8. +
  9. Separate activation height + a mixed-pool test matrix (Sapling spend + TV shield/unshield/internal in one block, reorgs, reindex from genesis).
  10. +
  11. Expose nChainTVSPValue (and the sum of public output values) via RPC + block explorer as a live, independently verifiable shielded-supply figure.
  12. +
  13. Commission a focused audit on the delta: removed value gadgets vs. added public checks and the value-binding constraint (§4.3(3)) + the new consensus arithmetic. Consider formal methods / circuit-equivalence checks for the retained NoteCommit + membership + nullifier logic.
  14. +
  15. Sunset policy: Sapling spendable long-term; new shielding into Sapling eventually disabled; users migrate at their pace — bounding ceremony risk for the old pool.
  16. +
  17. Public communication: this is a deliberate privacy-model shift (identity/graph privacy + public amounts + public conservation + contained residual ZK risk), not “Zcash but more private” and not “provably non-inflatable supply.” Set expectations accordingly.
  18. +
  19. Gate any TVSP work on completing the broader hardening already surfaced in the repo's own audit documents (CR-01 IBD discipline, ZIP-209 correctness, download verification).
  20. +
+ +

9. Conclusion

+

The Orchard episode showed that putting value inside a ZK proof makes supply integrity rest on the fallible circuit + binding + setup, and that a turnstile contains the blast radius at the supply level (it did, in 2026) without proving the absence of an internal bug. TVSP responds by publishing amounts and proving only authority + membership + value-binding + no-double-spend: per-transaction conservation becomes a public, exact, non-circuit consensus check; the proof shrinks; and net pool solvency is continuously, publicly auditable.

+

What TVSP does not do is equally important and was overstated in v1: it does not make the supply “provably non-inflatable,” and it does not make over-claims “immediately visible.” Membership, value-binding, nullifier, and authorization soundness remain inside the proof, and any bug there still injects value that the (now-exact) turnstile bounds but does not detect — the same containment semantics as today. The honest claim is narrower and still worthwhile: a smaller, public, exactly-auditable value surface, with the residual ZK risk contained rather than eliminated, in exchange for amount confidentiality and a denomination-based UX.

+

It is not “more private than Zcash.” It is differently private, with public value conservation — a defensible choice for a chain that wants its shielded value arithmetic in the open and its proving surface as small as possible, while being candid that a latent membership/nullifier/authority bug would still be contained, not impossible.

+ +
+ +

Appendix A — Why “value as a public input” binds the amount, and exactly how far that goes

+

Making value a public input and constraining cm = NoteCommit(g_d, pk_d, value) ties the declared public v_in to the specific committed cmprovided the NoteCommit gadget and the Merkle membership path are sound, and the spent cm is genuinely in the tree. What this does not provide is a public check that v_in equals the value the note was created with: the spend hides which note it is, so consensus cannot make that comparison. The binding is therefore an in-circuit guarantee (as strong as constraints §4.3(3)–(4)), not a free public one. Segregated denomination trees (§5.3) convert the value-binding into a structural public guarantee for the denomination dimension, which is why they are recommended.

+ +

Appendix B — Relationship to the turnstile (ZIP-209)

+

ZIP-209 enforces pool balance ≥ 0. In Sapling those flows are derived from hidden commitments; in TVSP the same constraint is enforced on public per-transaction values, so nChainTVSPValue is fully reconstructible from chain data and the turnstile is exact. But “exact” refers to the net balance, not to per-note provenance: TVSP's turnstile, like Sapling's, bounds counterfeit extraction at the pool's liquidity and surfaces only net insolvency. It is ZIP-209 with the amounts in the open — a better, public, recomputable seatbelt, not a per-note fraud detector.

+ +

Appendix C — Contrast with the Zinnia / STARK direction

+

The doc/zinnia-* proposals aim higher (post-quantum + no trusted setup) by introducing a new AIR circuit, a new hash (RPO256 on Goldilocks), a new Merkle (RpoHash FFI), large proofs (~80–100 KB), and (at the time) acknowledged ZK-completeness gaps on an unaudited Winterfell branch — a larger implementation/audit risk and a block-size impact. TVSP is the more conservative step: it reuses the relatively well-exercised Sapling gadgets minus the value parts, directly targets the value-conservation surface, and does not mandate a block-size jump. The two are compatible — TVSP can later adopt a transparent/PQ proof system for its reduced statement (recommendation 4).

+ +
+ + diff --git a/doc/zclassic-transparent-value-shielded-pool.md b/doc/zclassic-transparent-value-shielded-pool.md new file mode 100644 index 00000000000..d30d37ac691 --- /dev/null +++ b/doc/zclassic-transparent-value-shielded-pool.md @@ -0,0 +1,321 @@ +# Transparent-Value Shielded Pool (TVSP) for Zclassic + +**A shielded pool that makes per-transaction value conservation public and exact — removing the homomorphic value-balance / binding-signature soundness class — while keeping participant (sender / receiver / graph) privacy. It narrows, but does not eliminate, proof-system counterfeiting risk.** + +Status: Draft proposal (v2, corrected security claims) · Target: Zclassic, Sapling/Groth16 lineage · Audience: protocol engineers, reviewers + +> **What changed in v2.** v1 of this document overclaimed. It said public amounts make the supply "provably non-inflatable" and that any soundness bug becomes "immediately visible." That is false: the privacy that hides *which* note is spent also hides whether the value claimed for that note is legitimate. Public amounts make value *arithmetic* public — they do **not** make note *existence* or *uniqueness* public. v2 states the honest, narrower claim and is explicit about what remains inside the proof. The Orchard premise is also corrected to match the official Zcash Foundation disclosure. + +--- + +## 1. Abstract + +Every privacy coin that hides transaction **amounts** inside a zero-knowledge proof concentrates a lot of trust in one place: the soundness of the shielded circuit (and, for Groth16, its trusted setup) plus the binding of its value commitments. If the value-conservation argument inside the proof is wrong, value can be forged inside the pool, and because the amounts are hidden the forgery carries no obvious on-chain signature — it is contained, but not necessarily *seen*, by a turnstile. + +The June-2026 Zcash **Orchard** incident is the reference point, and it is worth stating precisely (see §2.1): it was *a soundness bug in the `halo2_gadgets` Orchard Action circuit* that could "accept invalid state transitions … potentially permitting **double-spending** of funds within Orchard." The Zcash Foundation was explicit that there was **"no ability to inflate the total ZEC supply, which is protected by Zcash's turnstile mechanism."** So the canonical 2026 incident is a *double-spend / invalid-state-transition* bug whose supply impact was **contained by the turnstile** — not the "unlimited undetectable minting" of market commentary. + +This proposal takes a deliberate design choice for a *new* Zclassic shielded pool: **make the note value public, keep the participants private.** The amount of every note is published in the clear; the proof is reduced to *spend authority + note membership + value-binding + nullifier correctness* and no longer carries a homomorphic value balance. Value conservation becomes ordinary public integer arithmetic enforced by consensus. + +The honest result: + +- **Per-transaction value conservation becomes a public, exact, non-circuit consensus check.** A transaction that does not conserve value is rejected in the clear. The *homomorphic value-balance / binding-signature* soundness class — a category of under-constrained in-circuit value arithmetic — is removed by construction. (This is **adjacent to but distinct from** the Orchard Action-circuit bug, which the official record describes as an invalid-state-transition / double-spend bug that TVSP does *not* fully fix — §6.2.) +- **The proving circuit shrinks** to authority + membership + value-binding + nullifier: a strictly smaller, easier-to-audit surface. +- **The pool's net solvency is continuously, publicly auditable** — `nChainTVSPValue` is reconstructible from public data at every height, so the turnstile becomes *exact* rather than derived from hidden commitments. +- **Sender, receiver, and the input↔output link remain hidden** — the proof still hides *which* note is spent and *who* owns it. + +And the honest limits (§6 in full): + +- **It does NOT make the supply "provably non-inflatable."** Membership, value-binding, nullifier, and authorization soundness all still live inside the proof. A bug in any of them still injects value, and that injection is **bounded — not detected — by the (now-exact) turnstile, exactly as today.** Public amounts prove arithmetic, not the existence or uniqueness of the notes behind it. +- **Amount privacy is given up**, and because public amounts leak linkage via amount-correlation, robust graph privacy requires **fixed denominations** (a mixer-style pool). + +This is a *different* privacy model than Zcash, not a strictly superior one. It trades amount-confidentiality for a smaller, public, exactly-auditable value surface — with the residual ZK risk **contained, not eliminated**. + +--- + +## 2. Motivation + +### 2.1 The Orchard incident, stated correctly + +The official Zcash Foundation disclosure (Zebra 4.5.3 / 5.0.0 emergency soft fork, NU6.2) describes the bug as: + +> "a soundness bug in the implementation of the Orchard zero-knowledge proof circuit in the `halo2_gadgets` crate … could allow the Orchard pool to accept invalid state transitions … potentially permitting **double-spending** of funds within Orchard." + +and, crucially: + +> "**no ability to inflate the total ZEC supply, which is protected by Zcash's turnstile mechanism** … [the turnstile] tracks the total ZEC balance across all value pools [and] provided a ground truth that ecosystem participants could use to confirm the supply cap remained intact." + +The advisory is tracked as **GHSA-jfw5-j458-pfv6** (Critical), mitigated by a soft fork temporarily disabling Orchard actions. The disclosers' official post-mortem (*The Orchard Counterfeiting Vulnerability — And Next Steps*) gives the most precise primary root cause: *"an under-constrained element of the Orchard circuit, because of which it was possible to put arbitrary false inputs into an elliptic curve multiplication and still have the multiplication check pass."* No primary source names a specific gadget; the finer "variable-base scalar-multiplication" attribution circulating in community and third-party write-ups is **not** in any primary disclosure, so this document does not assert it. + +Two lessons for this proposal, and both cut against the v1 framing: + +1. The 2026 incident is a **double-spend / invalid-state-transition** bug — exactly the class TVSP does **not** fully fix (see §6.2). It is *not* an example of undetectable supply inflation. +2. In that incident the **turnstile provided the containment / ground-truth mechanism**; had exploitation occurred, it would have bounded cross-pool extraction at the supply level. The turnstile is not "only a seatbelt that proves nothing"; it is a real, load-bearing layer — which is precisely why TVSP keeps it (made exact), rather than replacing it. + +For context, the April-2026 Zcash advisories also included a **turnstile-accounting bypass** (a re-seen block header silently overwriting a block index's pool-balance fields with `nullopt`, disabling enforcement across blocks) and a **signed-integer overflow in per-pool value-delta computation** that "could potentially have caused turnstile checks to be skipped, consensus validation to return early, or pool balance values to be computed incorrectly." These are directly relevant to TVSP's implementation surface (§7): TVSP moves *more* logic into public pool arithmetic, so that arithmetic must be overflow-safe and applied on every path. + +### 2.2 The structural problem TVSP targets + +In Sapling (Zclassic's current shielded pool) the value `v` of a note is never revealed. It is carried in a homomorphic **Pedersen value commitment** + +``` +cv = [v]·V + [rcv]·R (V, R = fixed independent generators) +``` + +and a **binding signature** over the sum of all `cv` proves + +``` +Σ cv(inputs) − Σ cv(outputs) = [valueBalance]·V +``` + +i.e. that inputs and outputs balance — without revealing any individual `v`. Conservation therefore rests on **two** in-circuit / commitment properties being simultaneously correct: + +1. the value commitment being **binding**, and +2. the Spend/Output **circuit being sound** so the `cv` matches the note actually owned. + +If either fails, an attacker can make a transaction *appear* to conserve value when it does not, minting value inside the pool. TVSP removes this specific failure mode by deleting `cv` and the binding signature and checking conservation as public integers instead (§4.5). + +### 2.3 What the turnstile does and does not do — precisely + +A turnstile (Zcash's ZIP-209; the same mechanism Zclassic now enforces — see §2.4) tracks the pool's net balance and **rejects any block that would drive it negative**. It is genuinely valuable (it held supply during the Orchard incident), but its guarantee is **net solvency bounded by liquidity**, not per-note provenance: + +| The turnstile **does** | The turnstile **does not** | +|---|---| +| Cap the maximum extractable counterfeit at the pool's real liquidity | Prevent extraction up to that liquidity | +| Detect insolvency **at the boundary** (when net balance would go negative) | Detect a counterfeit while the pool stays net-solvent (latent) | +| Contain counterfeit inside the pool boundary | Recover funds, identify the attacker, or prove no counterfeit exists internally | + +This is true under Sapling **and under TVSP**. TVSP makes the turnstile *exact and public* (anyone can recompute it), but does **not** change its essential nature: it bounds, it does not detect-per-note. The improvement TVSP brings is to the *value-conservation* check (§4.5), not to the turnstile's containment semantics. + +### 2.4 Status in the current Zclassic tree + +This is no longer aspirational: the current ZClassic tree enables ZIP-209 on mainnet (`src/chainparams.cpp` — `fZIP209Enabled = true`, Sprout checkpoint at height 3,000,000, balance `1316412375709`), and `ConnectBlock` rejects negative Sprout/Sapling pool balances under `if (chainparams.ZIP209Enabled())` (`src/main.cpp:2590`). Sapling nullifier double-spend checks are present (`src/coins.cpp:596`). An exhaustive `--full` history scan found `negative_pool_events: 0` across all ~3.13M blocks, and both pools are net-positive at the tip — strong evidence **no net drain has occurred**, but (as the audit notes themselves state) **not** proof of the absence of a latent, unexploited circuit bug. That gap is exactly what motivates moving value out of the proof. + +### 2.5 The design goal + +> Keep **who** private. Make **how much** public, conserved by plain arithmetic, and shrink the proof to the smallest authority+membership+nullifier statement. + +--- + +## 3. Design overview + +We introduce a new shielded transaction type — the **Transparent-Value Shielded Pool (TVSP)** — alongside (not replacing) the existing transparent and Sapling pools. Funds enter via a *shield* action and leave via an *unshield* action; internal transfers move value between TV-notes. + +A **TV-note** is the same as a Sapling note **except its value is public**: + +``` +TV-note = ( diversifier d, pk_d, value v, rcm ) // v is published in the clear +commitment cm = NoteCommit( g_d, pk_d, v ) // unchanged: cm still commits to v +``` + +Compared to Sapling, the change is surgical: + +| Component | Sapling (today) | TVSP (proposed) | +|---|---|---| +| Note value `v` | **hidden** (witness) | **public** (in the tx, and a *public input* to the circuit) | +| Value commitment `cv` | present (`[v]·V+[rcv]·R`) | **removed** | +| Binding signature | present (proves Σ`cv` balances) | **removed** | +| Value-conservation check | homomorphic commitment balance (in-proof) | **plain public arithmetic in consensus** | +| Spend circuit proves | authority + membership + nullifier + value commitment | authority + membership + nullifier + **value-binding of public `v` to `cm`** | +| Nullifier `nf` | `BLAKE2s(nk‖ρ)` | unchanged (own domain — see §7) | +| Spend authority `rk` | `ak + [ar]·G` | unchanged | +| Note encryption | encrypts `v` to recipient | recipient still gets a memo/key; `v` no longer needs hiding | + +Everything that provides **participant privacy** (membership-hiding spend, hidden spend authority, nullifiers, diversified addresses) is **kept**. The **value commitment and binding signature** are **removed**, and the amount is published and conserved by consensus arithmetic. The value-binding constraint (`cm` opens to the declared public `v`) **moves into the proof** and becomes a high-value audit target (§6.2). + +--- + +## 4. Technical specification + +### 4.1 Note and commitment + +A TV-note commits to its value exactly as Sapling does: + +``` +cm = PedersenHash( NoteCommit_personalization, [ value(64 bits) ‖ g_d(256) ‖ pk_d(256) ] ) + [rcm]·NoteCommitRandomness +``` + +The only change is that `value` is **also revealed in the clear** in the spend/output description, and the circuit takes `value` as a **public input** (not a private witness). The circuit must prove that the opened `cm` uses exactly that public `value`. This binds the public value to the note — **conditional on the NoteCommit gadget and the membership path being sound** (see §6.2; this is *not* a free public guarantee, it is an in-circuit one). + +### 4.2 Transaction format + +A new transaction version `TVSP_TX_VERSION` (new `nVersionGroupId`, new consensus branch ID) carries: + +``` +tvSpends[] : { anchor, nullifier nf, rk, value v_in (PUBLIC), zkproof } // no cv, no bindingSig +tvOutputs[] : { cmu, ephemeralKey, encCiphertext, value v_out (PUBLIC), zkproof } +valueBalanceTVSP : signed integer (net public value leaving the pool to transparent) +``` + +`v_in` and `v_out` are explicit public integers. There is **no** `bindingSig`. The sighash must cover all public value fields and `valueBalanceTVSP`. + +### 4.3 Spend circuit (TVSP) + +Identical to the Sapling Spend circuit **minus** the value-commitment gadget. It proves, for a public `value v_in`: + +1. The prover knows `ak, nsk` ⇒ `nk = [nsk]·G`, `ivk = CRH(ak, nk)`. +2. `rk = ak + [ar]·SpendAuthGenerator` is exposed (re-randomized spend-auth key; the spend signature verifies against `rk`). +3. The note commitment `cm = NoteCommit(g_d, pk_d, v_in)` opens to the **public** `v_in` (value-binding constraint). +4. `cm` is a member of the note-commitment tree at the public `anchor` (Merkle path), gated by `v_in ≠ 0` (dummy notes exempt). +5. `nf = PRF^{nf}(nk, ρ)` is correctly derived and exposed. + +It does **not** prove anything about a value commitment, and there is no homomorphic value balance inside the proof. Constraints (3), (4), (5) are the residual soundness-critical surface (§6.2). + +### 4.4 Output circuit (TVSP) + +Identical to Sapling Output **minus** the value-commitment gadget. It proves, for a public `value v_out`, that `cmu` is a correct note commitment to `(g_d, pk_d, v_out)`. + +### 4.5 Consensus: value conservation is now public arithmetic + +TVSP mirrors the existing Sapling convention exactly — it does **not** invent a per-pool fee equation. Define the net public flow as a single signed quantity: + +``` +valueBalanceTVSP := Σ tvSpends[i].v_in − Σ tvOutputs[j].v_out +``` + +with the same sign meaning as Sapling's `valueBalance`: **positive** = value leaving the TVSP pool *into* the transparent value pool (acts like a transparent input); **negative** = value entering the TVSP pool *from* transparent (acts like a transparent output). Consensus folds it into the **global** transaction value pool exactly as the code does for Sapling (`GetShieldedValueIn()` / `GetValueOut()`): + +- `GetShieldedValueIn()` adds `valueBalanceTVSP` when it is **positive** (like an input); +- `GetValueOut()` adds `−valueBalanceTVSP` when it is **negative** (like an output). + +The **fee is the single global remainder across all value pools**, never attributed to the TVSP component: + +``` +nValueIn = Σ(transparent vin) + GetShieldedValueIn() // incl. +valueBalanceTVSP, sprout vpub_new, sapling +require nValueIn ≥ GetValueOut() // GetValueOut incl. −valueBalanceTVSP +fee = nValueIn − GetValueOut() // one global subtraction (CheckTxInputs) +``` + +There is **no** `+ fee` term inside any per-pool balance equation. (An earlier draft wrote `Σ v_in == Σ v_out + fee + valueBalanceTVSP`; that wrongly localizes the fee to the shielded pool and is rejected — see `src/main.cpp:2135,2140,2146` and `src/primitives/transaction.cpp:285,309`, where the fee is the global remainder spanning transparent + sprout + sapling.) As `CheckTransaction` does for Sapling (`src/main.cpp` ~1252–1260), consensus also range-checks `|valueBalanceTVSP| ≤ MAX_MONEY` and rejects a non-zero `valueBalanceTVSP` when there are no TVSP spends/outputs. + +The pool-level invariant (an exact, public turnstile) — accumulating the per-block delta as `−valueBalanceTVSP`, mirroring `src/main.cpp:4117`, with checked arithmetic (§7): + +``` +nChainTVSPValue = Σ (value shielded in) − Σ (value unshielded out) ≥ 0 at every height +``` + +**What this does and does not guarantee.** The global value-conservation rule is a *hard public guarantee*: a transaction whose inputs do not cover its outputs (across all value pools) is rejected in the clear, no circuit involved. The pool invariant is *net solvency*: it is exact and public, but — as §2.3 and §6.2 explain — it bounds counterfeit extraction at the pool's liquidity; it does **not** detect a counterfeit that is balanced internally or extracted within available liquidity. Per-note provenance is not public (privacy hides which note is spent), so the soundness of constraints §4.3(3)–(5) still matters. + +### 4.6 What is no longer needed + +- The **binding signature** and its key. +- The **value-commitment generators** and the `[v]·V` / `[rcv]·R` gadgets in both circuits (a real reduction in circuit size and in the soundness-critical surface). +- Encrypting the value to the recipient is optional (the value is public); the encrypted memo channel is retained for the rest of the note plaintext. + +--- + +## 5. Privacy model — what you keep, what you lose + +### 5.1 Kept (participant / graph privacy) + +- **Which note is spent is hidden** — membership-hiding spend, exactly as Sapling. +- **Spender identity is hidden** — only `rk` (a re-randomization of the spend-auth key) is revealed; unlinkable across spends. +- **Recipient identity is hidden** — outputs reveal only `cmu` and an ephemeral key. +- **Nullifiers** prevent double-spends without linking to the note. + +### 5.2 Lost (amount privacy) — and the correlation caveat + +- **Amounts are public.** An observer sees that "13.37 ZCL" moved (but not who). +- **Public amounts leak linkage.** A spend of `13.37` and an output of `13.37` are probably the same flow — *amount-correlation* re-links inputs to outputs even though the specific note is hidden. With arbitrary values, graph privacy **largely collapses**. + +### 5.3 The fix: fixed denominations (and segregated denomination trees) + +To preserve graph privacy with public amounts, TVSP should operate on **fixed denominations** (e.g. notes only in {0.01, 0.1, 1, 10, 100} ZCL). Then every spend and output of a given denomination is identical in value → no amount-correlation, and the anonymity set is "all notes of that denomination." This turns TVSP into a **denomination pool / mixer**: + +- Arbitrary amounts are expressed as a **set** of fixed-denomination notes (like cash). +- The anonymity set for each note is the count of same-denomination notes ever created. +- UX cost: wallets split/merge into denominations (more notes per transaction); liquidity fragments per bucket; low-volume chains accumulate large anonymity sets slowly. Timing + public value + network observation still permit statistical attacks even with denominations. + +**Security bonus:** if each denomination has its **own commitment tree** and a spend declares which tree it draws from, then `v_in` is *structurally* fixed to that denomination. This upgrades the "open a real note to a higher `v`" defense (§6.2) from circuit-dependent to structural — you cannot spend a denom-1 note as denom-100 because it is not in the denom-100 tree. (Membership-forgery and double-spend *within* a denom tree still rest on the circuit.) Segregated denomination trees are therefore recommended both for privacy and for narrowing the residual soundness surface. + +**Honest core trade-off:** strong graph privacy + public amounts is achievable only via fixed denominations. With free-form amounts you get public conservation but weak graph privacy. + +--- + +## 6. Security analysis (corrected) + +### 6.1 What TVSP genuinely fixes + +- **Removes the homomorphic value-balance / binding-signature soundness class.** Conservation is checked as public integers; there is no `cv` to mis-bind and no binding signature to be unsound. A transaction that does not conserve value cannot be validly included — it is rejected in the clear by consensus, not by a circuit. This removes the *homomorphic value-balance / binding-signature* category of in-circuit value-arithmetic soundness bug. That category is **adjacent to but distinct from** the Orchard Action-circuit bug, which the official record describes as an invalid-state-transition / double-spend bug (an under-constrained element that let false inputs pass an elliptic-curve multiplication check; §2.1) — a class TVSP does *not* fully fix (§6.2). No primary source attributes the Orchard bug to a specific named gadget, so none is claimed here. +- **Shrinks the proving circuit** to authority + membership + value-binding + nullifier — fewer constraints, smaller audit surface, and (relative to Sapling) the deletion of the value gadgets is a net auditing win. +- **Makes net pool solvency continuously and publicly auditable.** `nChainTVSPValue` is exact and recomputable from public data at every block — the **non-negative net pool balance is publicly recomputable** at every height, with no forced migration or “prove-supply” ceremony required. (This is net balance only; it does **not** certify the absence of a latent, internally-balanced counterfeit, which remains turnstile-bounded, not detected — §6.2.) + +### 6.2 What TVSP does **not** fix (the honest limits) + +TVSP **does not** make the supply "provably non-inflatable," and over-claims are **not** "immediately visible." Membership, value-binding, nullifier, and authorization soundness remain inside the proof, and the privacy that hides *which* note is spent also hides whether its claimed value is legitimate. Therefore: + +- **Forged membership (counterfeit note from nothing).** A bug that proves membership of a `cm` never inserted into the tree, with public `v=10`, lets you spend it → output a real `v=10` note. The per-tx equation balances (you control both sides); `nChainTVSPValue` only drops on unshield, and only goes negative once cumulative fake extraction exceeds the pool's **total real liquidity**. Until then nothing looks wrong. **This is "accumulate undetectable fake value, drain later" — the seatbelt, not a real-time ledger.** +- **Value-binding break (open a real low-`v` note to a higher `v_in`).** If §4.3(3) is under-constrained, an attacker spends a real `v=5` note declaring `v_in=10`. Consensus cannot catch it, because membership privacy hides *which* note was spent, so it cannot compare `v_in=10` to the note's creation value `5`. Invisible per-tx; bounded only by liquidity. *(Segregated denomination trees, §5.3, neutralize this specific sub-case structurally.)* +- **Double-spend / nullifier soundness.** A bug that lets one note produce two valid nullifiers permits spending it twice; each spend can be internally balanced. The over-extraction is bounded by the turnstile but is **not** visible as a per-tx imbalance. Detectable only as eventual net insolvency, exactly as today. +- **Spend-authority break.** A bug allowing a forged authorization lets an attacker spend notes they don't own — theft up to available liquidity. Value-public does not address this. +- **Trusted-setup residue (if Groth16 is kept for the reduced statement).** A ceremony compromise can no longer forge *value* (value is public arithmetic) but can still forge spends (theft / double-nullify of real notes). Smaller scope than full value-forgery, still serious. Pairing TVSP with a **transparent-setup** proof system (Halo 2 with the IPA/inner-product commitment, or a STARK) removes the **ceremony-class residual entirely** — no trusted setup, no toxic waste, no parameter-generation ceremony to compromise. It does **not** remove proof-system or circuit (constraint-system) soundness risk, which is independent of how parameters are generated. And transparency ≠ post-quantum: **Halo 2-IPA rests on the elliptic-curve discrete-log assumption and is *not* post-quantum** (Shor breaks it); only a hash-based **STARK** is plausibly post-quantum. + +**The correct one-line claim:** TVSP prevents incorrect hidden-value *openings* and removes the homomorphic-balance soundness class; it **does not** eliminate proof-system counterfeiting as a class. Any residual membership/value-binding/nullifier/authority bug still injects value that is **bounded — not detected — by the (now-exact) turnstile.** The win is a smaller, public, exactly-auditable value surface with the residual ZK risk *contained*, not eliminated. + +### 6.3 Where TVSP sits on the spectrum + +``` +Transparent (Bitcoin) everything public · no privacy · supply trivially auditable · public UTXO arithmetic; no hidden-pool inflation opacity +TVSP (this proposal) WHO hidden · HOW MUCH public · per-tx conservation public+exact · net solvency public+exact + · residual membership/nullifier/authority soundness still in-proof, turnstile-bounded + · graph privacy strong only with fixed denominations ← here +Sapling / Orchard everything hidden · max privacy · supply rests entirely on circuit+binding+setup + · value-forgery AND double-spend risk if a soundness bug exists (turnstile bounds supply) +``` + +TVSP is a coherent **middle point**: it sacrifices amount-confidentiality to make value conservation public/exact and to shrink the proof, while retaining identity/graph privacy and *containing* (not removing) the residual ZK risk. + +--- + +## 7. Implementation risks — new code, new consensus surface + +A correct TVSP fork is much more than new fields. Each item below is a place a bug would re-introduce the exact "accept bad value flows" class that the historical IBD bypass (CR-01) enabled for Sapling. + +- **Checked arithmetic everywhere — already started.** The per-pool delta path has already been hardened in the working tree: `ReceivedBlockTransactions` accumulates the Sprout/Sapling deltas via `CheckedAddTo` (`src/main.cpp:4117`, `4123–4124`; helpers `CheckedAdd` / `CheckedAddTo` at `src/amount.h:40–52`), falling back to `boost::none` on overflow, and chain-value propagation uses `CheckedAdd` — mirroring the April-2026 Zcash fix for the signed-integer per-pool-delta overflow class. *(As of this writing these changes are uncommitted in the working tree — unbuilt/untested.)* TVSP must extend the same `CheckedAddTo` / `CheckedAdd` discipline to `nChainTVSPValue` and every new per-tx check, including the sign handling on `valueBalanceTVSP`. +- **Separate pool plumbing.** New nullifier set, anchor DB, commitment tree, mempool nullifier maps, and a chain-index running total (`nChainTVSPValue`) — likely with a block-header commitment analogous to `hashFinalSaplingRoot` (`src/primitives/block.h`). +- **Domain separation.** TVSP commitments and nullifiers must use distinct personalization / a strictly separate tree from Sapling, or cross-pool replay/confusion becomes possible. +- **Consensus-enforced denominations.** Wallet-only denominations do not protect privacy or bound the value-binding surface; the denomination set (and, ideally, segregated trees, §5.3) must be a consensus rule. +- **Apply the check on every path.** The per-tx conservation and exact turnstile must run in `CheckTransaction` / `ContextualCheckTransaction` / `ConnectBlock` for **all** node states. Apply the CR-01 discipline (no validation shortcut during IBD/import/reindex) to the new paths from day one. A reindex under a buggy TVSP binary would be dangerous. +- **Reorg / reindex correctness.** `nChainTVSPValue` and the TVSP nullifier/anchor sets must rewind correctly on reorg and re-accumulate correctly during reindex/IBD. +- **Format surface.** New `nVersionGroupId`, branch ID, parser/serialization (public value fields, no `bindingSig`), sighash coverage, RPC encoding, `getblocktemplate`/miner policy, wallet note structures. +- **Legacy-pool policy.** While Sapling/Sprout shielding remains enabled, global supply still inherits their hidden-pool risk. Draining a legacy pool makes its *final net* auditable but does **not** retro-prove no prior inflation. +- **Orthogonal hardening still applies.** Bootstrap/fast-sync trust (no signed manifest in the reviewed tree), proving-key/parameter trust, DoS via many small-denom notes or large proofs, side-channels in proof generation — none are solved by TVSP. +- **Not post-quantum by itself.** The retained ZK is still discrete-log / Jubjub / Groth16; transparent ECDSA unchanged. TVSP can be *paired* with a PQ proof system but does not provide PQ guarantees on its own. + +--- + +## 8. Recommendations (if pursuing TVSP) + +1. **First, independently of TVSP:** patch Zclassic's existing pool accounting with checked-delta arithmetic and add chain-value recomputation / checkpoint tests (this also hardens the live ZIP-209 path). +2. Treat the public-conservation and exact-turnstile enforcement as consensus-critical; add property-based / differential tests: replay historical flows, force over-claims and duplicate nullifiers, and assert that invalid TV txs are rejected **even under `isInitBlockDownload()`**. +3. Use **fixed denominations from the start** for any claim of strong privacy, and prefer **segregated denomination trees** (privacy + structural value-binding). Document the anonymity-set math and correlation/timing risks. +4. Regenerate keys for the reduced circuit; strongly prefer a **transparent-setup** proving system for the residual authority+membership+nullifier statement (note: Halo 2-IPA is transparent but **not** post-quantum; only a hash-based STARK is plausibly post-quantum). +5. Separate activation height + a **mixed-pool test matrix** (Sapling spend + TV shield/unshield/internal in one block, reorgs, reindex from genesis). +6. Expose `nChainTVSPValue` (and the sum of public output values) via **RPC + block explorer** as a live, independently verifiable shielded-supply figure. +7. Commission a **focused audit on the delta**: removed value gadgets vs. added public checks and the value-binding constraint (§4.3(3)) + the new consensus arithmetic. Consider formal methods / circuit-equivalence checks for the retained NoteCommit + membership + nullifier logic. +8. **Sunset policy:** Sapling spendable long-term; new shielding into Sapling eventually disabled; users migrate at their pace — bounding ceremony risk for the old pool. +9. **Public communication:** this is a deliberate privacy-model shift (identity/graph privacy + public amounts + public conservation + *contained* residual ZK risk), **not** "Zcash but more private" and **not** "provably non-inflatable supply." Set expectations accordingly. +10. Gate any TVSP work on completing the broader hardening already surfaced in the repo's own audit documents (CR-01 IBD discipline, ZIP-209 correctness, download verification). + +--- + +## 9. Conclusion + +The Orchard episode showed that putting value *inside* a ZK proof makes supply integrity rest on the fallible circuit + binding + setup, and that a turnstile contains the blast radius at the supply level (it did, in 2026) without proving the absence of an internal bug. TVSP responds by publishing amounts and proving only *authority + membership + value-binding + no-double-spend*: per-transaction conservation becomes a public, exact, non-circuit consensus check; the proof shrinks; and net pool solvency is continuously, publicly auditable. + +What TVSP does **not** do is equally important and was overstated in v1: it does not make the supply "provably non-inflatable," and it does not make over-claims "immediately visible." Membership, value-binding, nullifier, and authorization soundness remain inside the proof, and any bug there still injects value that the (now-exact) turnstile **bounds but does not detect** — the same containment semantics as today. The honest claim is narrower and still worthwhile: **a smaller, public, exactly-auditable value surface, with the residual ZK risk contained rather than eliminated, in exchange for amount confidentiality and a denomination-based UX.** + +It is not "more private than Zcash." It is **differently private, with public value conservation** — a defensible choice for a chain that wants its shielded value arithmetic in the open and its proving surface as small as possible, while being candid that a latent membership/nullifier/authority bug would still be contained, not impossible. + +--- + +### Appendix A — Why "value as a public input" binds the amount, and exactly how far that goes + +Making `value` a public input and constraining `cm = NoteCommit(g_d, pk_d, value)` ties the declared public `v_in` to the specific committed `cm` — **provided the NoteCommit gadget and the Merkle membership path are sound, and the spent `cm` is genuinely in the tree.** What this does *not* provide is a *public* check that `v_in` equals the value the note was created with: the spend hides which note it is, so consensus cannot make that comparison. The binding is therefore an **in-circuit** guarantee (as strong as constraints §4.3(3)–(4)), not a free public one. Segregated denomination trees (§5.3) convert the value-binding into a *structural* public guarantee for the denomination dimension, which is why they are recommended. + +### Appendix B — Relationship to the turnstile (ZIP-209) + +ZIP-209 enforces `pool balance ≥ 0`. In Sapling those flows are derived from hidden commitments; in TVSP the same constraint is enforced on **public** per-transaction values, so `nChainTVSPValue` is fully reconstructible from chain data and the turnstile is *exact*. But "exact" refers to the **net** balance, not to per-note provenance: TVSP's turnstile, like Sapling's, **bounds** counterfeit extraction at the pool's liquidity and surfaces only **net** insolvency. It is ZIP-209 with the amounts in the open — a better, public, recomputable seatbelt, not a per-note fraud detector. + +### Appendix C — Contrast with the Zinnia / STARK direction + +The `doc/zinnia-*` proposals aim higher (post-quantum + no trusted setup) by introducing a new AIR circuit, a new hash (RPO256 on Goldilocks), a new Merkle (RpoHash FFI), large proofs (~80–100 KB), and (at the time) acknowledged ZK-completeness gaps on an unaudited Winterfell branch — a larger implementation/audit risk and a block-size impact. TVSP is the more conservative step: it reuses the relatively well-exercised Sapling gadgets *minus* the value parts, directly targets the value-conservation surface, and does not mandate a block-size jump. The two are compatible — TVSP can later adopt a transparent/PQ proof system for its reduced statement (recommendation 4). diff --git a/doc/zip209-cr01-testing.md b/doc/zip209-cr01-testing.md new file mode 100644 index 00000000000..f74cd47e4f2 --- /dev/null +++ b/doc/zip209-cr01-testing.md @@ -0,0 +1,101 @@ +# Testing ZIP-209 + CR-01 (ZClassic consensus hardening) + +These builds carry two consensus-validation fixes on top of upstream ZClassic: + +- **ZIP-209 shielded turnstile (mainnet)** — a block that would drive the Sprout + or Sapling shielded value-pool balance negative is rejected as invalid. +- **CR-01** — `ContextualCheckTransaction()` no longer skips contextual crypto + checks during initial block download / import / reindex. + +This document is the checklist for testers. Everything except step 4 is +read-only and safe to run on a production node. + +> **Prerequisite:** a **fully-synced mainnet** node. The auditor reads the +> chainstate via RPC, so RPC must be enabled in `zclassic.conf`. + +--- + +## 1. Pre-flight: confirm your node matches the compiled checkpoint (~1s) + +ZIP-209 ships a hardcoded Sprout value-pool checkpoint. A genesis-tracking node +asserts its own computed balance against it at startup — a mismatch aborts the +daemon. Verify the value before running a ZIP-209 binary: + +```bash +./src/zclassic-cli getblock $(./src/zclassic-cli getblockhash 3000000) | grep -A4 '"id": "sprout"' +# Expected: "chainValueZat": 1316412375709 +``` + +If `chainValueZat` is **not** `1316412375709`, stop and report it — your +chainstate differs from the canonical chain (or is corrupt). + +## 2. Full turnstile audit (read-only, ~20–30 min) + +Prove that **no** historical block ever drove a shielded pool negative, so +enabling ZIP-209 — and a later `-reindex` — will not reject the chain: + +```bash +python3 scripts/audit-mainnet-history.py --full --supply-check --json-output audit-full.json +``` + +Expected: + +``` +negative pool events found in scan: 0 +status=OK: observable supply does not exceed expected issuance +``` + +This only issues read-only RPCs (`getblockchaininfo`, `getblock`, +`gettxoutsetinfo`); it never modifies the node, datadir, or chain. + +> The default (without `--full`) is a sampled scan and cannot prove absence of a +> transient historical negative pool. Use `--full` for the real proof. + +## 3. Build, run, and confirm the turnstile build + +```bash +./zcutil/build.sh -j$(nproc) # macOS: -j$(sysctl -n hw.ncpu) +./src/zclassicd +./src/zclassic-cli getnetworkinfo | grep subversion +# Expected: "/ZClassic:2.1.2-ZIP209-beta6/" +``` + +A clean startup (no `turnstile violation ... shielded value pool` abort) means +ZIP-209 loaded and validated the checkpoint against your chainstate. + +## 4. Optional: full historical re-validation (slow) + +Re-validate every block from genesis with the turnstile **and** the CR-01 +contextual checks enforced (this is the CR-01 deployment step — a reindex on an +*un*fixed binary does not re-validate, since reindex keeps the node in IBD): + +```bash +./src/zclassic-cli stop +./src/zclassicd -reindex +``` + +Watch the log. It must reach the chain tip **without** printing: + +- `turnstile violation in Sprout/Sapling shielded value pool` (ZIP-209), or +- any `ConnectBlock` / contextual-check block rejection (CR-01). + +A reindex that reaches the tip cleanly is independent confirmation that the +whole chain validates under the hardened ruleset. + +--- + +## Do testers need to run the audit? + +Not strictly — the mainnet chain history is identical for everyone, and it has +already been proven clean (`negative pool events: 0` across all blocks). But +running step 1 is the **minimum** (it prevents a startup assert on a node whose +chainstate differs), and step 2 is recommended as independent verification. + +## Network note + +ZIP-209 is a soft-fork-class rule tightening. A single node enforcing it while +the rest of the network does not is harmless (stricter, and the chain never +violates the rule today) but only protects that node. For ZIP-209 to be a +network-wide defense, a majority of nodes/miners must run a ZIP-209 build with +the **same** checkpoint, ideally behind a coordinated activation height — see +[zip209-mainnet-reactivation.md](zip209-mainnet-reactivation.md). diff --git a/qa/pull-tester/rpc-tests.sh b/qa/pull-tester/rpc-tests.sh index 7d8b0b7d4fb..a4bbc27dd31 100755 --- a/qa/pull-tester/rpc-tests.sh +++ b/qa/pull-tester/rpc-tests.sh @@ -10,25 +10,29 @@ export BITCOIND=${REAL_BITCOIND} #Run the tests +# SUP-04: the shielded-pool regression tests below were re-enabled (they had been +# commented out, leaving ZClassic's core privacy features unguarded in CI). If any +# fails, triage and fix the underlying issue or file a tracking ticket with a code +# comment — do NOT silently re-comment it. testScripts=( - # 'paymentdisclosure.py' + 'paymentdisclosure.py' 'prioritisetransaction.py' - # 'wallet_treestate.py' - # 'wallet_anchorfork.py' + 'wallet_treestate.py' + 'wallet_anchorfork.py' # 'wallet_changeindicator.py' 'wallet_import_export.py' - # 'wallet_protectcoinbase.py' - # 'wallet_shieldcoinbase_sprout.py' - # 'wallet_shieldcoinbase_sapling.py' - # 'wallet_listreceived.py' + 'wallet_protectcoinbase.py' + 'wallet_shieldcoinbase_sprout.py' + 'wallet_shieldcoinbase_sapling.py' + 'wallet_listreceived.py' # 'wallet.py' # 'wallet_overwintertx.py' 'wallet_persistence.py' - # 'wallet_nullifiers.py' + 'wallet_nullifiers.py' # 'wallet_1941.py' 'wallet_addresses.py' 'wallet_sapling.py' - # 'wallet_listnotes.py' + 'wallet_listnotes.py' # 'mergetoaddress_sprout.py' # 'mergetoaddress_sapling.py' 'listtransactions.py' @@ -40,8 +44,8 @@ testScripts=( 'rest.py' 'mempool_spendcoinbase.py' 'mempool_reorg.py' - # 'mempool_tx_input_limit.py' - # 'mempool_nu_activation.py' + 'mempool_tx_input_limit.py' + 'mempool_nu_activation.py' 'mempool_tx_expiry.py' 'httpbasics.py' 'zapwallettxes.py' @@ -58,8 +62,8 @@ testScripts=( 'blockchain.py' 'disablewallet.py' 'zcjoinsplit.py' - # 'zcjoinsplitdoublespend.py' - # 'zkey_import_export.py' + 'zcjoinsplitdoublespend.py' + 'zkey_import_export.py' 'reorg_limit.py' 'getblocktemplate.py' 'bip65-cltv-p2p.py' diff --git a/scripts/audit-circuits.py b/scripts/audit-circuits.py new file mode 100644 index 00000000000..ce668a196d7 --- /dev/null +++ b/scripts/audit-circuits.py @@ -0,0 +1,367 @@ +#!/usr/bin/env python3 +""" +Reusable zk-SNARK circuit soundness auditor for Zclassic's shielded circuits. + +Operationalizes the "find under-constrained circuit elements" methodology (the +class of bug behind the June-2026 Zcash Orchard counterfeiting flaw) across every +shielded-circuit source file Zclassic actually compiles: + + - Sprout C++ circuit (libsnark/R1CS, legacy BCTV14 verify path): src/zcash/circuit/*.tcc + - Sprout Rust circuit (bellman/Groth16, ACTIVE path for new JoinSplits) and the + Sapling Spend/Output circuit + supporting gadgets, both bundled inside the + pinned librustzcash source tarball under depends/sources/. + +For each file it builds a soundness-audit prompt tailored to that file's proving +system (the assign/copy-constraint pattern differs between halo2, bellman, and +libsnark), then either: + + --emit (default) writes one ready-to-run prompt per file + a manifest and a + SOUND/GAP checklist. No network, no API key. Feed the prompts to + Claude Code agents (or paste them in) and record verdicts in the + checklist. This is the safe default. + + --run calls the Claude API (Opus 4.8, adaptive thinking, streaming, effort + high) once per file and collects a structured SOUND / GAP_FOUND / + INCONCLUSIVE verdict into a JSON report. Requires `pip install anthropic` + and ANTHROPIC_API_KEY (or `ant auth login`). Read-only: it only reads + circuit source; it never edits the repo. + +This does NOT prove a circuit correct. A SOUND verdict means no missing-constraint +(Orchard-class) gap was found in that file. It does not cover the proving-system +parameters / trusted setup, the bellman/libsnark verifier internals, or non-circuit +bugs. +""" + +from __future__ import annotations + +import argparse +import glob +import json +import os +import sys +import tarfile +import tempfile +from pathlib import Path +from typing import Dict, List, Optional + +REPO_ROOT = Path(__file__).resolve().parent.parent +MODEL = "claude-opus-4-8" + +# --------------------------------------------------------------------------- +# Per-proving-system description of the soundness-bug pattern. The class is the +# same everywhere (a value is witnessed but never constrained); the *mechanism* +# differs, so the prompt names the exact thing to look for. +# --------------------------------------------------------------------------- +SYSTEMS = { + "libsnark": ( + "libsnark R1CS (C++ protoboard gadgets). The bug pattern is a value set in " + "`generate_r1cs_witness()` with no matching constraint added in " + "`generate_r1cs_constraints()` (e.g. a `pb.val(x) = ...` or `fill_with_bits` " + "whose variable is never pinned by an `add_r1cs_constraint` / " + "`generate_boolean_r1cs_constraint`). Check parity between the two methods of " + "every gadget." + ), + "bellman": ( + "bellman R1CS / Groth16 over Jubjub. The bug pattern is a variable produced by " + "`AllocatedNum::alloc` / `AllocatedBit::alloc` / `.get_value()` with no matching " + "`cs.enforce(...)` binding it (a coordinate computed but not constrained on-curve, " + "a conditional select whose result isn't enforced, scalar bits not boolean-" + "constrained, a doubling/addition output coordinate witnessed but not pinned). " + "This is the bellman analog of Orchard's halo2 assign_advice-without-copy_advice." + ), + "halo2": ( + "halo2 PLONKish (advice columns + permutation copy constraints). The exact " + "Orchard bug: `assign_advice()` used where `copy_advice()` was required, so a " + "value isn't bound by an equality/permutation constraint." + ), +} + +PROMPT_TEMPLATE = """\ +You are auditing a zk-SNARK circuit for SOUNDNESS bugs — the class that lets a +malicious prover satisfy the circuit with values that should be impossible, +enabling counterfeiting or double-spends. This file is part of {role}. + +Proving system: {system_label} +Bug pattern to hunt: {system_desc} + +Context: the June-2026 Zcash Orchard counterfeiting flaw was an under-constrained +element of the variable-base scalar-multiplication gadget — the diversified-address +integrity check pk_d = [ivk]·g_d could be satisfied for arbitrary inputs, so a note +could be spent repeatedly under different nullifiers. Find the analog here, if any. + +For EACH gadget / function in the file: + 1. List every variable that is ALLOCATED or whose witness value is computed. + 2. List every constraint actually added. + 3. Check parity: is every witnessed value bound by a constraint? Are points kept + on-curve? Are bit decompositions boolean-constrained? Are conditional selects / + sign negations ENFORCED (not merely witnessed)? Are summed values range-bounded + so the field sum cannot wrap (the classic inflation bug)? + 4. Flag any value used downstream but NOT constrained — that is the bug. + +Priorities: in-circuit EC scalar multiplication; range/overflow guards on value +balance; booleanity of every bit. Spend authority, nullifier derivation, note- +commitment binding, and Merkle membership (correctly gated for dummy/zero-value +inputs) must each be constrained. + +Be rigorous and skeptical — do NOT assume correctness because it is upstream code. +But do NOT invent bugs: if a value is correctly constrained, say so and cite the +exact constraint line as evidence. Quote file:line for every claim. Do not modify +any file. + +FILE: {path} +-------------------------------------------------------------------------------- +{source} +-------------------------------------------------------------------------------- + +End with a final line of strict JSON (no prose after it) matching: +{{"file": "...", "proving_system": "{system}", "verdict": "SOUND"|"GAP_FOUND"|"INCONCLUSIVE", + "findings": [{{"gadget": "...", "location": "file:line", "witnessed": "...", + "constrained": true|false, "confidence": "low"|"medium"|"high", "note": "..."}}], + "summary": "..."}} +""" + +VERDICT_SCHEMA = { + "type": "object", + "additionalProperties": False, + "properties": { + "file": {"type": "string"}, + "proving_system": {"type": "string"}, + "verdict": {"type": "string", "enum": ["SOUND", "GAP_FOUND", "INCONCLUSIVE"]}, + "findings": { + "type": "array", + "items": { + "type": "object", + "additionalProperties": False, + "properties": { + "gadget": {"type": "string"}, + "location": {"type": "string"}, + "witnessed": {"type": "string"}, + "constrained": {"type": "boolean"}, + "confidence": {"type": "string", "enum": ["low", "medium", "high"]}, + "note": {"type": "string"}, + }, + "required": ["gadget", "location", "witnessed", "constrained", "confidence", "note"], + }, + }, + "summary": {"type": "string"}, + }, + "required": ["file", "proving_system", "verdict", "findings", "summary"], +} + + +class Circuit: + def __init__(self, name: str, path: Path, system: str, role: str): + self.name = name + self.path = path + self.system = system + self.role = role + + @property + def system_label(self) -> str: + return {"libsnark": "libsnark/R1CS", "bellman": "bellman/Groth16", "halo2": "halo2/PLONKish"}[self.system] + + +def extract_rust_circuits(workdir: Path) -> Optional[Path]: + """Extract sapling-crypto/src/circuit/** from the pinned librustzcash tarball. + + Returns the extracted sapling-crypto/src/circuit dir, or None if the tarball + is absent (e.g. a clean checkout that hasn't fetched depends sources).""" + tarballs = glob.glob(str(REPO_ROOT / "depends/sources/librustzcash-*.tar.gz")) + if not tarballs: + return None + tarball = Path(sorted(tarballs)[-1]) + base = tarball.name[: -len(".tar.gz")] + circuit_dir = workdir / base / "sapling-crypto" / "src" / "circuit" + if not circuit_dir.exists(): + with tarfile.open(tarball, "r:gz") as tf: + prefix = f"{base}/sapling-crypto/src/" + members = [m for m in tf.getmembers() if m.name.startswith(prefix)] + # Defensive: skip any path-traversing members. + safe = [m for m in members if ".." not in Path(m.name).parts] + tf.extractall(workdir, members=safe) + return circuit_dir if circuit_dir.exists() else None + + +def classify_rust(rel: str) -> tuple[str, str]: + """Map a sapling-crypto circuit/*.rs path to (system, role).""" + if rel.startswith("sapling/"): + return "bellman", "the Sapling Spend/Output circuit (bellman/Groth16)" + if rel.startswith("sprout/"): + return "bellman", "the ACTIVE Groth16 Sprout JoinSplit circuit (new-note verify path)" + return "bellman", "a load-bearing Sapling circuit gadget (bellman/Groth16)" + + +def discover(workdir: Path) -> List[Circuit]: + circuits: List[Circuit] = [] + + # 1. Sprout C++ libsnark circuit — in-repo, complete. + sprout_cpp = sorted((REPO_ROOT / "src/zcash/circuit").glob("*.tcc")) + for p in sprout_cpp: + circuits.append( + Circuit( + name=f"sprout-cpp/{p.name}", + path=p, + system="libsnark", + role="the Sprout JoinSplit circuit (libsnark/BCTV14, legacy verify path)", + ) + ) + + # 2. Rust circuits (Sapling + Groth16 Sprout) from the pinned tarball. + circuit_dir = extract_rust_circuits(workdir) + if circuit_dir is not None: + for p in sorted(circuit_dir.rglob("*.rs")): + rel = p.relative_to(circuit_dir).as_posix() + if rel.startswith("test/") or rel.endswith("/test/mod.rs"): + continue # skip the test harness + system, role = classify_rust(rel) + circuits.append(Circuit(name=f"rust/{rel}", path=p, system=system, role=role)) + + return circuits + + +def build_prompt(c: Circuit) -> str: + source = c.path.read_text(errors="replace") + return PROMPT_TEMPLATE.format( + role=c.role, + system=c.system, + system_label=c.system_label, + system_desc=SYSTEMS[c.system], + path=c.name, + source=source, + ) + + +# --------------------------------------------------------------------------- +# --emit : write prompt-packs + manifest + checklist (no API). +# --------------------------------------------------------------------------- +def emit(circuits: List[Circuit], out: Path) -> None: + out.mkdir(parents=True, exist_ok=True) + manifest = [] + for c in circuits: + slug = c.name.replace("/", "__") + prompt_path = out / f"{slug}.prompt.md" + prompt_path.write_text(build_prompt(c)) + manifest.append( + { + "name": c.name, + "source_path": str(c.path), + "proving_system": c.system, + "role": c.role, + "prompt": str(prompt_path), + "verdict": None, # fill in after auditing + } + ) + (out / "manifest.json").write_text(json.dumps(manifest, indent=2) + "\n") + + lines = ["# Circuit soundness audit checklist", ""] + lines.append("Run each prompt through Opus 4.8 (a Claude Code agent, or `--run`), record the verdict.") + lines.append("") + by_sys: Dict[str, List[dict]] = {} + for m in manifest: + by_sys.setdefault(m["proving_system"], []).append(m) + for sysname, items in by_sys.items(): + lines.append(f"## {sysname} ({len(items)} files)") + for m in items: + lines.append(f"- [ ] `{m['name']}` — {m['role']} → prompt: `{m['prompt']}`") + lines.append("") + if not any(c.system == "halo2" for c in circuits): + lines.append("## halo2 (Orchard)") + lines.append("- n/a — no halo2/Orchard code in this codebase (pre-NU5). The Orchard bug class does not apply.") + lines.append("") + (out / "CHECKLIST.md").write_text("\n".join(lines)) + + print(f"Wrote {len(circuits)} prompt(s) + manifest.json + CHECKLIST.md to {out}") + print("Next: feed each *.prompt.md to an Opus 4.8 agent, or rerun with --run to call the API.") + + +# --------------------------------------------------------------------------- +# --run : call the Claude API per file, collect structured verdicts. +# --------------------------------------------------------------------------- +def run(circuits: List[Circuit], out: Path, effort: str) -> int: + try: + import anthropic + except ImportError: + print("error: --run needs the Anthropic SDK. Install with: pip install anthropic", file=sys.stderr) + return 2 + + client = anthropic.Anthropic() # ANTHROPIC_API_KEY / ant auth profile from env + out.mkdir(parents=True, exist_ok=True) + results = [] + for c in circuits: + print(f"[audit] {c.name} ({c.system}) ...", file=sys.stderr, flush=True) + prompt = build_prompt(c) + # Stream (large circuit + reasoning), adaptive thinking, effort high, + # structured output for the verdict. + with client.messages.stream( + model=MODEL, + max_tokens=64000, + thinking={"type": "adaptive"}, + output_config={"effort": effort, "format": {"type": "json_schema", "schema": VERDICT_SCHEMA}}, + messages=[{"role": "user", "content": prompt}], + ) as stream: + message = stream.get_final_message() + text = next((b.text for b in message.content if b.type == "text"), "") + try: + verdict = json.loads(text) + except json.JSONDecodeError: + verdict = {"file": c.name, "proving_system": c.system, "verdict": "INCONCLUSIVE", + "findings": [], "summary": "could not parse model JSON output"} + verdict.setdefault("file", c.name) + results.append(verdict) + v = verdict.get("verdict", "INCONCLUSIVE") + gaps = [f for f in verdict.get("findings", []) if f.get("constrained") is False] + print(f" -> {v}" + (f" ({len(gaps)} unconstrained finding(s))" if gaps else ""), file=sys.stderr) + + (out / "report.json").write_text(json.dumps(results, indent=2) + "\n") + + print("\n== Circuit soundness audit ==") + for r in results: + flag = "" if r["verdict"] == "SOUND" else " <-- REVIEW" + print(f"{r['verdict']:13} {r['file']}{flag}") + any_gap = any(r["verdict"] == "GAP_FOUND" for r in results) + print(f"\nReport: {out / 'report.json'}") + print("VERDICT: " + ("GAP(S) FOUND — review report.json" if any_gap else + "no Orchard-class soundness gap found in the audited circuits")) + return 1 if any_gap else 0 + + +def parse_args() -> argparse.Namespace: + p = argparse.ArgumentParser(description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter) + mode = p.add_mutually_exclusive_group() + mode.add_argument("--emit", action="store_true", help="Write prompt-packs + manifest (default; no API).") + mode.add_argument("--run", action="store_true", help="Call the Claude API per file and collect verdicts.") + p.add_argument("--out", default=str(REPO_ROOT / "circuit-audit"), help="Output directory.") + p.add_argument("--workdir", default=None, help="Where to extract the Rust tarball (default: a temp dir).") + p.add_argument("--only", action="append", default=[], help="Substring filter on circuit name (repeatable).") + p.add_argument("--effort", default="high", choices=["low", "medium", "high", "xhigh", "max"], + help="Effort for --run (default high).") + p.add_argument("--list", action="store_true", help="List discovered circuits and exit.") + return p.parse_args() + + +def main() -> int: + args = parse_args() + workdir = Path(args.workdir) if args.workdir else Path(tempfile.mkdtemp(prefix="zcl-circuit-audit-")) + workdir.mkdir(parents=True, exist_ok=True) + + circuits = discover(workdir) + if args.only: + circuits = [c for c in circuits if any(s in c.name for s in args.only)] + if not circuits: + print("error: no circuit files found (is depends/sources fetched?).", file=sys.stderr) + return 2 + + if args.list: + for c in circuits: + print(f"{c.system:9} {c.name}\t{c.path}") + return 0 + + out = Path(args.out) + if args.run: + return run(circuits, out, args.effort) + emit(circuits, out) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/scripts/audit-mainnet-history.py b/scripts/audit-mainnet-history.py new file mode 100755 index 00000000000..420079d4659 --- /dev/null +++ b/scripts/audit-mainnet-history.py @@ -0,0 +1,650 @@ +#!/usr/bin/env python3 +""" +Read-only Zclassic chain-history auditor for shielded-pool and block-size risks. + +This script calls only read-only RPCs through direct HTTP JSON-RPC by default: + getblockchaininfo, gettxoutsetinfo, getblock + +It does not create transactions, does not submit blocks, and does not modify the +node or datadir. Use --full for an exhaustive block walk; the default stride is +sampling and cannot prove absence of a transient historical negative pool. + +ZIP-209 / CR-01 deployment prerequisite & check +----------------------------------------------- +This is the prerequisite validation for the ZIP-209 shielded turnstile (mainnet) +and the CR-01 IBD-validation fix. Run it (read-only) on a fully-synced mainnet +node BEFORE building/running a ZIP-209 binary or re-validating with `-reindex`: + + python3 scripts/audit-mainnet-history.py --full --supply-check + +Expected on a healthy chain: + * "negative pool events found in scan: 0" — no block ever drove a Sprout or + Sapling value-pool balance negative, so enabling ZIP-209 (and a reindex) + will NOT reject the existing chain. + * supply "status=OK" — observable supply does not exceed expected issuance. + +It also reports the Sprout value-pool balance: this MUST equal the compiled +ZIP-209 checkpoint (CMainParams::nSproutValuePoolCheckpointBalance), otherwise a +ZIP-209 node aborts on the FallbackSproutValuePoolBalance assert at startup. See +doc/zip209-cr01-testing.md for the full tester checklist. +""" + +from __future__ import annotations + +import argparse +import base64 +import http.client +import json +import subprocess +import sys +import time +from decimal import Decimal, ROUND_DOWN +from pathlib import Path +from typing import Any, Dict, Iterable, List, Optional, Tuple + +COIN = 100_000_000 +INITIAL_SUBSIDY_ZAT = int(Decimal("12.5") * COIN) + +MAINNET_OVERWINTER_SAPLING_HEIGHT = 476_969 +MAINNET_BUBBLES_HEIGHT = 585_318 +MAINNET_DIFFADJ_HEIGHT = 585_322 +MAINNET_BUTTERCUP_HEIGHT = 707_000 +PRE_BUTTERCUP_HALVING_INTERVAL = 840_000 +POST_BUTTERCUP_HALVING_INTERVAL = PRE_BUTTERCUP_HALVING_INTERVAL * 2 +SUBSIDY_SLOW_START_INTERVAL = 2 +SUBSIDY_SLOW_START_SHIFT = SUBSIDY_SLOW_START_INTERVAL // 2 +DEFAULT_MAINNET_RPC_PORT = 8023 +DEFAULT_TEST_RPC_PORT = 18023 + + +def parse_args() -> argparse.Namespace: + default_cli = Path("src/zclassic-cli") + parser = argparse.ArgumentParser( + description="Read-only shielded-pool and block-size audit against a local zclassicd node." + ) + parser.add_argument("--use-cli", action="store_true", help="Use zclassic-cli subprocess calls instead of direct HTTP RPC.") + parser.add_argument("--cli", default=str(default_cli if default_cli.exists() else "zclassic-cli")) + parser.add_argument("--datadir", help="Optional zclassic datadir.") + parser.add_argument( + "--cli-arg", + action="append", + default=[], + help="Extra zclassic-cli-compatible option, e.g. --cli-arg=-rpcuser=... --cli-arg=-rpcpassword=...", + ) + parser.add_argument("--rpcconnect", help="RPC host for direct HTTP mode. Default: zclassic.conf or 127.0.0.1.") + parser.add_argument("--rpcport", type=int, help="RPC port for direct HTTP mode. Default: zclassic.conf or chain default.") + parser.add_argument("--rpcuser", help="RPC username for direct HTTP mode.") + parser.add_argument("--rpcpassword", help="RPC password for direct HTTP mode.") + parser.add_argument("--rpccookiefile", help="RPC auth cookie path. Relative paths resolve under the network datadir.") + parser.add_argument("--rpc-timeout", type=int, default=600, help="Direct HTTP RPC timeout in seconds.") + parser.add_argument("--batch-size", type=int, default=250, help="getblock RPC calls per direct HTTP batch.") + parser.add_argument("--allow-non-main", action="store_true", help="Allow testnet/regtest auditing.") + parser.add_argument("--start", type=int, default=0) + parser.add_argument("--end", type=int, help="Default: current tip height.") + parser.add_argument("--stride", type=int, default=10_000, help="Block sampling stride. Use --full for stride 1.") + parser.add_argument("--full", action="store_true", help="Scan every block height.") + parser.add_argument("--extra-height", type=int, action="append", default=[], help="Additional height to force-scan.") + parser.add_argument("--block-size-limit", type=int, default=200_000, help="Expected MAX_BLOCK_SIZE threshold.") + parser.add_argument("--skip-txoutset", action="store_true", help="Skip gettxoutsetinfo and supply comparison.") + parser.add_argument("--supply-check", action="store_true", help="Compute expected mainnet issuance and compare observable supply.") + parser.add_argument("--json-output", help="Write full audit result to this JSON file.") + parser.add_argument("--progress-every", type=int, default=1_000, help="Progress interval in checked blocks; 0 disables.") + return parser.parse_args() + + +class Cli: + def __init__(self, binary: str, datadir: Optional[str], extra_args: List[str]): + self.base = [binary] + if datadir: + self.base.append(f"-datadir={datadir}") + self.base.extend(extra_args) + self.mode = "zclassic-cli subprocess" + + def call(self, method: str, *params: Any) -> Any: + cmd = self.base + [method] + [cli_arg(param) for param in params] + proc = subprocess.run(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True) + if proc.returncode != 0: + raise RuntimeError( + "zclassic-cli failed\n" + f"command: {' '.join(cmd)}\n" + f"stderr: {proc.stderr.strip()}\n" + f"stdout: {proc.stdout.strip()}" + ) + out = proc.stdout.strip() + if not out: + return None + return json.loads(out, parse_float=Decimal) + + def batch(self, calls: List[Tuple[str, List[Any]]]) -> List[Any]: + return [self.call(method, *params) for method, params in calls] + + +def cli_arg(value: Any) -> str: + if isinstance(value, bool): + return "1" if value else "0" + return str(value) + + +class HttpRpc: + def __init__(self, host: str, port: int, auth: str, timeout: int): + self.host = host + self.port = port + self.timeout = timeout + self.auth_header = "Basic " + base64.b64encode(auth.encode("utf8")).decode("ascii") + self.conn = http.client.HTTPConnection(host, port, timeout=timeout) + self.next_id = 0 + self.mode = f"direct HTTP JSON-RPC batch ({host}:{port})" + + def _next_id(self) -> int: + self.next_id += 1 + return self.next_id + + def _request_once(self, body: str) -> Any: + headers = { + "Host": self.host, + "User-Agent": "zclassic-audit-mainnet-history/1.0", + "Authorization": self.auth_header, + "Content-Type": "application/json", + } + self.conn.request("POST", "/", body=body, headers=headers) + response = self.conn.getresponse() + raw = response.read().decode("utf8") + if response.status == 401: + raise RuntimeError("RPC authorization failed; check rpcuser/rpcpassword or the auth cookie.") + if not raw: + raise RuntimeError(f"RPC server returned empty HTTP {response.status} {response.reason}") + try: + parsed = json.loads(raw, parse_float=Decimal) + except json.JSONDecodeError as exc: + raise RuntimeError(f"RPC server returned non-JSON HTTP {response.status} {response.reason}: {raw[:200]}") from exc + if response.status >= 400 and not _rpc_response_has_error(parsed): + raise RuntimeError(f"RPC server returned HTTP {response.status} {response.reason}: {raw[:200]}") + return parsed + + def _request(self, payload: Any) -> Any: + body = json.dumps(payload) + retry_errors = ( + http.client.BadStatusLine, + http.client.CannotSendRequest, + http.client.RemoteDisconnected, + http.client.ResponseNotReady, + BrokenPipeError, + ConnectionResetError, + ) + for attempt in range(2): + try: + return self._request_once(body) + except retry_errors: + self.conn.close() + self.conn = http.client.HTTPConnection(self.host, self.port, timeout=self.timeout) + if attempt: + raise + except OSError as exc: + raise RuntimeError(f"could not connect to RPC server at {self.host}:{self.port}: {exc}") from exc + raise RuntimeError("unreachable RPC retry state") + + def call(self, method: str, *params: Any) -> Any: + req_id = self._next_id() + payload = {"version": "1.1", "method": method, "params": list(params), "id": req_id} + response = self._request(payload) + if not isinstance(response, dict): + raise RuntimeError(f"RPC {method} returned unexpected batch response") + _raise_for_rpc_error(method, response) + if "result" not in response: + raise RuntimeError(f"RPC {method} response is missing result") + return response["result"] + + def batch(self, calls: List[Tuple[str, List[Any]]]) -> List[Any]: + if not calls: + return [] + id_to_label: Dict[int, str] = {} + payload = [] + order = [] + for method, params in calls: + req_id = self._next_id() + id_to_label[req_id] = f"{method} {params}" + order.append(req_id) + payload.append({"version": "1.1", "method": method, "params": params, "id": req_id}) + response = self._request(payload) + if not isinstance(response, list): + raise RuntimeError(f"RPC batch returned non-list response: {response!r}") + + by_id: Dict[int, Dict[str, Any]] = {} + for item in response: + if not isinstance(item, dict) or "id" not in item: + raise RuntimeError(f"RPC batch returned malformed item: {item!r}") + by_id[int(item["id"])] = item + + results = [] + for req_id in order: + item = by_id.get(req_id) + if item is None: + raise RuntimeError(f"RPC batch response is missing id {req_id}") + _raise_for_rpc_error(id_to_label[req_id], item) + if "result" not in item: + raise RuntimeError(f"RPC batch response for {id_to_label[req_id]} is missing result") + results.append(item["result"]) + return results + + +def _rpc_response_has_error(response: Any) -> bool: + if isinstance(response, dict): + return "error" in response + if isinstance(response, list): + return any(isinstance(item, dict) and "error" in item for item in response) + return False + + +def _raise_for_rpc_error(label: str, response: Dict[str, Any]) -> None: + error = response.get("error") + if error is None: + return + if isinstance(error, dict): + message = error.get("message", error) + code = error.get("code") + raise RuntimeError(f"RPC {label} failed with code {code}: {message}") + raise RuntimeError(f"RPC {label} failed: {error}") + + +def parse_dash_options(args: Iterable[str]) -> Dict[str, str]: + options: Dict[str, str] = {} + for raw in args: + if not raw.startswith("-"): + continue + item = raw.lstrip("-") + if not item: + continue + if "=" in item: + key, value = item.split("=", 1) + options[key] = value + else: + options[item] = "1" + return options + + +def truthy(value: Optional[str]) -> bool: + if value is None: + return False + return value.lower() not in ("", "0", "false", "no") + + +def read_config_file(path: Path) -> Dict[str, str]: + config: Dict[str, str] = {} + if not path.exists(): + return config + for raw_line in path.read_text(errors="replace").splitlines(): + line = raw_line.strip() + if not line or line.startswith("#") or line.startswith(";") or line.startswith("["): + continue + for marker in (" #", "\t#", " ;", "\t;"): + if marker in line: + line = line.split(marker, 1)[0].strip() + if "=" not in line: + continue + key, value = line.split("=", 1) + config[key.strip()] = value.strip() + return config + + +def default_datadir() -> Path: + home = Path.home() + if sys.platform == "darwin": + return home / "Library/Application Support/ZClassic" + if sys.platform.startswith("win"): + return home / "AppData/Roaming/ZClassic" + return home / ".zclassic" + + +def network_from_options(options: Dict[str, str], config: Dict[str, str]) -> str: + if truthy(options.get("regtest")) or truthy(config.get("regtest")): + return "regtest" + if truthy(options.get("testnet")) or truthy(config.get("testnet")): + return "testnet" + return "main" + + +def network_datadir(base_datadir: Path, network: str) -> Path: + if network == "testnet": + return base_datadir / "testnet3" + if network == "regtest": + return base_datadir / "regtest" + return base_datadir + + +def resolve_rpc_settings(args: argparse.Namespace) -> Tuple[str, int, str]: + cli_options = parse_dash_options(args.cli_arg) + base_datadir = Path(args.datadir or cli_options.get("datadir") or default_datadir()).expanduser() + + conf_name = cli_options.get("conf", "zclassic.conf") + conf_path = Path(conf_name).expanduser() + if not conf_path.is_absolute(): + conf_path = base_datadir / conf_path + config = read_config_file(conf_path) + + network = network_from_options(cli_options, config) + rpc_host = args.rpcconnect or cli_options.get("rpcconnect") or config.get("rpcconnect") or "127.0.0.1" + default_port = DEFAULT_TEST_RPC_PORT if network in ("testnet", "regtest") else DEFAULT_MAINNET_RPC_PORT + rpc_port = int(args.rpcport or cli_options.get("rpcport") or config.get("rpcport") or default_port) + + rpc_user = args.rpcuser or cli_options.get("rpcuser") or config.get("rpcuser") or "" + rpc_password = args.rpcpassword or cli_options.get("rpcpassword") or config.get("rpcpassword") or "" + if rpc_password: + return rpc_host, rpc_port, f"{rpc_user}:{rpc_password}" + + cookie_name = args.rpccookiefile or cli_options.get("rpccookiefile") or config.get("rpccookiefile") or ".cookie" + cookie_path = Path(cookie_name).expanduser() + if not cookie_path.is_absolute(): + cookie_path = network_datadir(base_datadir, network) / cookie_path + if cookie_path.exists(): + cookie = cookie_path.read_text(errors="replace").strip() + if cookie: + return rpc_host, rpc_port, cookie + + raise RuntimeError( + "Could not locate RPC credentials. Pass --rpcuser/--rpcpassword, " + f"--rpccookiefile, --datadir, or use --use-cli. Tried cookie: {cookie_path}; config: {conf_path}" + ) + + +def make_rpc(args: argparse.Namespace) -> Any: + if args.use_cli: + return Cli(args.cli, args.datadir, args.cli_arg) + host, port, auth = resolve_rpc_settings(args) + return HttpRpc(host, port, auth, args.rpc_timeout) + + +def amount_to_zat(value: Any) -> int: + dec = value if isinstance(value, Decimal) else Decimal(str(value)) + return int((dec * COIN).to_integral_value(rounding=ROUND_DOWN)) + + +def cxx_div_toward_zero(a: int, b: int) -> int: + q = abs(a) // abs(b) + return q if (a >= 0) == (b >= 0) else -q + + +def mainnet_block_subsidy_zat(height: int) -> int: + n_subsidy = INITIAL_SUBSIDY_ZAT + if height < SUBSIDY_SLOW_START_INTERVAL // 2: + return (n_subsidy // SUBSIDY_SLOW_START_INTERVAL) * height + if height < SUBSIDY_SLOW_START_INTERVAL: + return (n_subsidy // SUBSIDY_SLOW_START_INTERVAL) * (height + 1) + + if height >= MAINNET_BUTTERCUP_HEIGHT: + halvings = cxx_div_toward_zero( + height - SUBSIDY_SLOW_START_SHIFT - MAINNET_BUTTERCUP_HEIGHT, + POST_BUTTERCUP_HALVING_INTERVAL, + ) + 3 + base = n_subsidy // 2 + else: + halvings = (height - SUBSIDY_SLOW_START_SHIFT) // PRE_BUTTERCUP_HALVING_INTERVAL + base = n_subsidy + + if halvings >= 64: + return 0 + return base >> halvings + + +def expected_issued_zat_mainnet(tip_height: int) -> int: + # Genesis coinbase is not counted here; mining starts at height 1. + return sum(mainnet_block_subsidy_zat(h) for h in range(1, tip_height + 1)) + + +def pool_map(block_or_info: Dict[str, Any]) -> Dict[str, Dict[str, Any]]: + return {pool.get("id"): pool for pool in block_or_info.get("valuePools", [])} + + +def pool_chain_value(pool: Dict[str, Any]) -> Optional[int]: + if not pool.get("monitored", False): + return None + if "chainValueZat" not in pool: + return None + return int(pool["chainValueZat"]) + + +def selected_heights( + start: int, + end: int, + stride: int, + extras: Iterable[int], + include_mainnet_landmarks: bool, +) -> List[int]: + heights = set() + if stride <= 0: + raise ValueError("stride must be positive") + h = start + while h <= end: + heights.add(h) + h += stride + heights.add(end) + for extra in extras: + if start <= extra <= end: + heights.add(extra) + if include_mainnet_landmarks: + for landmark in ( + MAINNET_OVERWINTER_SAPLING_HEIGHT - 1, + MAINNET_OVERWINTER_SAPLING_HEIGHT, + MAINNET_BUBBLES_HEIGHT, + MAINNET_DIFFADJ_HEIGHT, + MAINNET_BUTTERCUP_HEIGHT - 1, + MAINNET_BUTTERCUP_HEIGHT, + ): + if start <= landmark <= end: + heights.add(landmark) + return sorted(heights) + + +def chunks(values: List[int], size: int) -> Iterable[List[int]]: + if size <= 0: + raise ValueError("batch size must be positive") + for offset in range(0, len(values), size): + yield values[offset : offset + size] + + +def scan_blocks(rpc: Any, heights: List[int], block_size_limit: int, progress_every: int, batch_size: int) -> Dict[str, Any]: + result: Dict[str, Any] = { + "blocks_checked": 0, + "negative_pool_events": [], + "unmonitored_pool_events": [], + "oversized_blocks": [], + "max_block_size": {"height": None, "hash": None, "size": 0}, + "pools": { + "sprout": {"first_monitored": None, "first_nonzero_delta": None, "min": None, "max": None}, + "sapling": {"first_monitored": None, "first_nonzero_delta": None, "min": None, "max": None}, + }, + } + started = time.time() + total = len(heights) + for batch_heights in chunks(heights, batch_size): + calls = [("getblock", [str(height), True]) for height in batch_heights] + blocks = rpc.batch(calls) + for height, block in zip(batch_heights, blocks): + result["blocks_checked"] += 1 + + size = int(block.get("size", 0)) + if size > result["max_block_size"]["size"]: + result["max_block_size"] = {"height": height, "hash": block.get("hash"), "size": size} + if size > block_size_limit: + result["oversized_blocks"].append({"height": height, "hash": block.get("hash"), "size": size}) + + pools = pool_map(block) + for pool_name in ("sprout", "sapling"): + pool = pools.get(pool_name, {}) + chain_value = pool_chain_value(pool) + stats = result["pools"][pool_name] + if chain_value is None: + if len(result["unmonitored_pool_events"]) < 50: + result["unmonitored_pool_events"].append({"height": height, "pool": pool_name}) + continue + + if stats["first_monitored"] is None: + stats["first_monitored"] = height + stats["min"] = chain_value if stats["min"] is None else min(stats["min"], chain_value) + stats["max"] = chain_value if stats["max"] is None else max(stats["max"], chain_value) + if chain_value < 0: + result["negative_pool_events"].append( + {"height": height, "hash": block.get("hash"), "pool": pool_name, "chainValueZat": chain_value} + ) + if pool.get("valueDeltaZat") not in (None, 0, "0") and stats["first_nonzero_delta"] is None: + stats["first_nonzero_delta"] = { + "height": height, + "hash": block.get("hash"), + "valueDeltaZat": int(pool["valueDeltaZat"]), + } + + if progress_every and result["blocks_checked"] % progress_every == 0: + elapsed = time.time() - started + print(f"checked {result['blocks_checked']}/{total} selected heights in {elapsed:.1f}s", file=sys.stderr) + + return result + + +def print_summary(result: Dict[str, Any]) -> None: + print("== Zclassic Read-Only Chain Audit ==") + print(f"chain: {result['chain']}") + print(f"tip: {result['tip_height']} {result['tip_hash']}") + print(f"scan: start={result['scan']['start']} end={result['scan']['end']} stride={result['scan']['stride']} full={result['scan']['full']}") + print(f"rpc: {result['scan'].get('rpc_mode')} batch_size={result['scan'].get('batch_size')}") + print(f"selected heights checked: {result['scan']['blocks_checked']}") + print() + + print("== Shielded Pools ==") + for name, pool in result["tip_value_pools"].items(): + print(f"tip {name}: monitored={pool.get('monitored')} chainValueZat={pool.get('chainValueZat')}") + print(f"negative pool events found in scan: {len(result['scan']['negative_pool_events'])}") + if result["scan"]["negative_pool_events"]: + for event in result["scan"]["negative_pool_events"][:10]: + print(f" NEGATIVE {event}") + print(f"unmonitored pool samples: {len(result['scan']['unmonitored_pool_events'])}") + for name, stats in result["scan"]["pools"].items(): + print(f"{name}: first_monitored={stats['first_monitored']} min={stats['min']} max={stats['max']} first_nonzero_delta={stats['first_nonzero_delta']}") + print() + + print("== Block Size ==") + print(f"max observed block size: {result['scan']['max_block_size']}") + print(f"blocks over configured limit: {len(result['scan']['oversized_blocks'])}") + for event in result["scan"]["oversized_blocks"][:10]: + print(f" OVERSIZE {event}") + print() + + if "supply" in result: + print("== Observable Supply ==") + supply = result["supply"] + print(f"transparent_utxo_zat={supply.get('transparent_utxo_zat')}") + print(f"monitored_pool_sum_zat={supply.get('monitored_pool_sum_zat')}") + print(f"observable_zat={supply.get('observable_zat')}") + print(f"expected_issued_zat={supply.get('expected_issued_zat')}") + print(f"observable_minus_expected_zat={supply.get('observable_minus_expected_zat')}") + print(f"status={supply.get('status')}") + print() + + print("== Warnings ==") + for warning in result["warnings"]: + print(f"- {warning}") + + +def main() -> int: + args = parse_args() + if args.full: + args.stride = 1 + if args.start < 0: + raise SystemExit("--start must be non-negative") + + rpc = make_rpc(args) + info = rpc.call("getblockchaininfo") + chain = info["chain"] + tip_height = int(info["blocks"]) + tip_hash = info["bestblockhash"] + end = tip_height if args.end is None else min(args.end, tip_height) + if end < args.start: + raise SystemExit("--end must be >= --start") + if chain != "main" and not args.allow_non_main: + raise SystemExit(f"Refusing to audit non-main chain {chain!r}; pass --allow-non-main if intentional.") + + warnings: List[str] = [] + if args.stride != 1: + warnings.append("Scan is sampled, not exhaustive. Use --full to prove absence across every block height.") + if info.get("pruned"): + warnings.append("Node is pruned; historical getblock calls may fail for old blocks.") + if Decimal(str(info.get("verificationprogress", "0"))) < Decimal("0.999"): + warnings.append("Node verificationprogress is below 0.999; audit may be against an incomplete chain.") + + heights = selected_heights( + args.start, + end, + args.stride, + args.extra_height, + include_mainnet_landmarks=(chain == "main"), + ) + scan = scan_blocks(rpc, heights, args.block_size_limit, args.progress_every, args.batch_size) + scan.update( + { + "start": args.start, + "end": end, + "stride": args.stride, + "full": args.stride == 1, + "rpc_mode": rpc.mode, + "batch_size": args.batch_size, + } + ) + + tip_pools = pool_map(info) + result: Dict[str, Any] = { + "chain": chain, + "tip_height": tip_height, + "tip_hash": tip_hash, + "verificationprogress": str(info.get("verificationprogress")), + "pruned": info.get("pruned"), + "tip_value_pools": tip_pools, + "scan": scan, + "warnings": warnings, + } + + if not args.skip_txoutset: + txoutset = rpc.call("gettxoutsetinfo") + result["txoutset"] = txoutset + if args.supply_check: + if chain != "main": + warnings.append("Supply schedule comparison is implemented only for mainnet params.") + else: + transparent = amount_to_zat(txoutset["total_amount"]) + monitored_pool_values = [] + unmonitored_tip_pools = [] + for name in ("sprout", "sapling"): + value = pool_chain_value(tip_pools.get(name, {})) + if value is None: + unmonitored_tip_pools.append(name) + else: + monitored_pool_values.append(value) + expected = expected_issued_zat_mainnet(tip_height) + observable = transparent + sum(monitored_pool_values) + status = "OK: observable supply does not exceed expected issuance" + if observable > expected: + status = "ALERT: observable supply exceeds expected issuance" + if unmonitored_tip_pools: + status += f"; incomplete because unmonitored tip pools: {', '.join(unmonitored_tip_pools)}" + result["supply"] = { + "transparent_utxo_zat": transparent, + "monitored_pool_sum_zat": sum(monitored_pool_values), + "observable_zat": observable, + "expected_issued_zat": expected, + "observable_minus_expected_zat": observable - expected, + "status": status, + } + + print_summary(result) + if args.json_output: + out_path = Path(args.json_output) + out_path.write_text(json.dumps(result, indent=2, default=str) + "\n") + print(f"\nWrote JSON report: {out_path}") + return 0 + + +if __name__ == "__main__": + try: + raise SystemExit(main()) + except RuntimeError as exc: + print(f"error: {exc}", file=sys.stderr) + raise SystemExit(1) + except KeyboardInterrupt: + raise SystemExit(130) diff --git a/src/amount.h b/src/amount.h index 3d7eefc589c..1587eb8f900 100644 --- a/src/amount.h +++ b/src/amount.h @@ -8,6 +8,7 @@ #include "serialize.h" +#include #include #include @@ -30,6 +31,26 @@ extern const std::string CURRENCY_UNIT; static const CAmount MAX_MONEY = 21000000 * COIN; inline bool MoneyRange(const CAmount& nValue) { return (nValue >= 0 && nValue <= MAX_MONEY); } +/** + * Checked arithmetic helpers for CAmount (signed 64-bit). + * These prevent undefined behavior from signed overflow and are used for + * shielded value pool delta and chain-value tracking (see ZIP-209 turnstile paths + * and the April-2026 signed-integer overflow class in per-pool accounting). + */ +inline bool CheckedAdd(CAmount a, CAmount b, CAmount& result) { + if (b > 0 && a > std::numeric_limits::max() - b) return false; + if (b < 0 && a < std::numeric_limits::min() - b) return false; + result = a + b; + return true; +} + +inline bool CheckedAddTo(CAmount& a, CAmount b) { + CAmount tmp; + if (!CheckedAdd(a, b, tmp)) return false; + a = tmp; + return true; +} + /** Type-safe wrapper class to for fee rates * (how much to pay based on transaction size) */ diff --git a/src/bootstrap.cpp b/src/bootstrap.cpp index a2a5d1979c6..7246127b1ee 100644 --- a/src/bootstrap.cpp +++ b/src/bootstrap.cpp @@ -268,17 +268,25 @@ static size_t DiscoverBootstrapPeersFromSocket(SOCKET socket, const CService& pe std::vector vAddr; try { - addrPayload >> vAddr; + // MEM-03: read and bound the element COUNT before allocating/deserializing + // the vector, mirroring the normal addr handler's 1000-entry bound. The + // previous `addrPayload >> vAddr` deserialized the whole list (up to the + // 2 MiB message cap) before the size guard below could fire. + uint64_t nAddr = ReadCompactSize(addrPayload); + if (nAddr > 1000) { + LogPrint("net", "bootstrap discovery: oversized addr (%llu) from %s\n", (unsigned long long)nAddr, peerAddress.ToStringIPPort()); + return 0; + } + vAddr.reserve(nAddr); + for (uint64_t i = 0; i < nAddr; ++i) { + CAddress a; + addrPayload >> a; + vAddr.push_back(a); + } } catch (const std::exception& e) { LogPrint("net", "bootstrap discovery: malformed addr from %s: %s\n", peerAddress.ToStringIPPort(), e.what()); return 0; } - // Mirror the addr-message bound enforced by the normal net handler so a - // misbehaving peer cannot make us iterate an enormous list. - if (vAddr.size() > 1000) { - LogPrint("net", "bootstrap discovery: oversized addr (%u) from %s\n", (unsigned int)vAddr.size(), peerAddress.ToStringIPPort()); - return 0; - } size_t appended = 0; for (size_t i = 0; i < vAddr.size() && out.size() < BOOTSTRAP_DISCOVERY_MAX_RESULTS; ++i) { diff --git a/src/bootstrapvalidation.cpp b/src/bootstrapvalidation.cpp index d831b887ba6..3fa3fa2c6e2 100644 --- a/src/bootstrapvalidation.cpp +++ b/src/bootstrapvalidation.cpp @@ -103,7 +103,10 @@ static void RefreshFinalizationHoldLocked() g_finalizationHold.store(provisional || tipHold, std::memory_order_relaxed); } -static const int64_t BOOTSTRAPVAL_BATCH_MS = 80; // cs_main per-batch budget +// PERF-04: cs_main per-batch budget. Lowered 80 -> 20 ms so the background UTXO +// validator holds cs_main for far shorter spans, reducing stalls to live message +// processing / block relay (and peer timeouts on slower hardware) while it runs. +static const int64_t BOOTSTRAPVAL_BATCH_MS = 20; // cs_main per-batch budget static const size_t BOOTSTRAPVAL_FLUSH_CAP = 300 * (1 << 20); // in-mem coin cache cap static boost::filesystem::path ScratchDir() diff --git a/src/chainparams.cpp b/src/chainparams.cpp index fab7224ba22..bc42d3c235e 100644 --- a/src/chainparams.cpp +++ b/src/chainparams.cpp @@ -242,6 +242,24 @@ class CMainParams : public CChainParams { vBootstrapPeers.push_back("74.50.74.102"); vBootstrapPeers.push_back("205.209.104.118"); + // ZIP-209: reject any block that drives a shielded value pool balance + // negative (turnstile enforcement). Previously enabled on testnet only; + // this mirrors that configuration for mainnet. + // + // The hardcoded Sprout fallback below re-seeds nChainSproutValue for + // nodes whose block index predates pool monitoring (#2795), so they can + // enforce from this height forward WITHOUT a full reindex. Nodes that + // tracked the pool from genesis instead assert that their computed + // balance equals this value (see FallbackSproutValuePoolBalance in + // main.cpp), so it MUST be the exact genesis-derived Sprout balance at + // this block. Verified against a synced mainnet node on 2026-06-05: + // getblock 0000038aee939c8017f4ad353e3fd1313c6a0da565bbc1d3269bbe855fe33505 + // -> valuePools[id=sprout].chainValueZat == 1316412375709 (height 3000000) + nSproutValuePoolCheckpointHeight = 3000000; + nSproutValuePoolCheckpointBalance = 1316412375709; + fZIP209Enabled = true; + hashSproutValuePoolCheckpointBlock = uint256S("0000038aee939c8017f4ad353e3fd1313c6a0da565bbc1d3269bbe855fe33505"); + // Founders reward script expects a vector of 2-of-3 multisig addresses vFoundersRewardAddress = { "t3Vz22vK5z2LcKEdg16Yv4FFneEL1zg9ojd", /* main-index: 0*/ diff --git a/src/checkpoints.cpp b/src/checkpoints.cpp index 5d09f7dbee2..78b5c011367 100644 --- a/src/checkpoints.cpp +++ b/src/checkpoints.cpp @@ -81,6 +81,16 @@ namespace Checkpoints { return NULL; } + bool CheckBlock(const CCheckpointData& data, int nHeight, const uint256& hash) + { + const MapCheckpoints& checkpoints = data.mapCheckpoints; + + MapCheckpoints::const_iterator i = checkpoints.find(nHeight); + if (i == checkpoints.end()) + return true; + return hash == i->second; + } + static std::string FastSyncAnchorPayload(const CChainParams& chainparams, const CFastSyncAnchorData& anchor) { return strprintf("zclassic-fastsync-anchor-v1|%s|%d|%s", diff --git a/src/checkpoints.h b/src/checkpoints.h index 5b65ba5736a..baa410e6932 100644 --- a/src/checkpoints.h +++ b/src/checkpoints.h @@ -27,6 +27,11 @@ int GetTotalBlocksEstimate(const CCheckpointData& data); //! Returns last CBlockIndex* in mapBlockIndex that is a checkpoint CBlockIndex* GetLastCheckpoint(const CCheckpointData& data); +//! Returns false only if there is a checkpoint at nHeight and hash does not +//! match it. A block presented at a checkpoint height with a different hash is +//! a forgery and must be rejected (eclipse / bootstrap lock-in). +bool CheckBlock(const CCheckpointData& data, int nHeight, const uint256& hash); + double GuessVerificationProgress(const CCheckpointData& data, CBlockIndex* pindex, bool fSigchecks = true); //! Validate the compiled fast-sync anchor against the checkpoint set and digest fields. diff --git a/src/clientversion.cpp b/src/clientversion.cpp index ae67e678f1a..122a3619cc0 100644 --- a/src/clientversion.cpp +++ b/src/clientversion.cpp @@ -19,12 +19,12 @@ * for both bitcoind and bitcoin-core, to make it harder for attackers to * target servers or GUI users specifically. */ -const std::string CLIENT_NAME("MagicBean"); +const std::string CLIENT_NAME("ZClassic"); /** * Client version number */ -#define CLIENT_VERSION_SUFFIX "" +#define CLIENT_VERSION_SUFFIX "-ZClassic" /** @@ -103,7 +103,7 @@ const std::string CLIENT_DATE(BUILD_DATE); std::string FormatVersion(int nVersion) { if (nVersion % 100 < 25) - return strprintf("%d.%d.%d-beta%d", nVersion / 1000000, (nVersion / 10000) % 100, (nVersion / 100) % 100, (nVersion % 100)+1); + return strprintf("%d.%d.%d-ZIP209-beta%d", nVersion / 1000000, (nVersion / 10000) % 100, (nVersion / 100) % 100, (nVersion % 100)+1); if (nVersion % 100 < 50) return strprintf("%d.%d.%d-rc%d", nVersion / 1000000, (nVersion / 10000) % 100, (nVersion / 100) % 100, (nVersion % 100)-24); else if (nVersion % 100 == 50) diff --git a/src/consensus/consensus.h b/src/consensus/consensus.h index 2878edaa0de..734092f7c35 100644 --- a/src/consensus/consensus.h +++ b/src/consensus/consensus.h @@ -20,6 +20,21 @@ static const int32_t SAPLING_MIN_TX_VERSION = 4; static const int32_t SAPLING_MAX_TX_VERSION = 4; /** The maximum allowed size for a serialized block, in bytes (network rule) */ static const unsigned int MAX_BLOCK_SIZE = 200000; + +/** The maximum block size we tolerate when loading blocks from disk during -reindex, + * -loadblock, or bootstrap.dat import. The canonical mainnet chain contains 1,272 blocks + * whose serialized size is strictly between 200000 and 2000000 bytes (max observed + * 1,999,599 B as of height ~3.1M). These blocks are accepted on the normal P2P/ConnectBlock + * path via the local GENEROUS_BLOCK_SIZE_LIMIT in CheckBlock ("checkpoint validates + * correctness" + hash chain). LoadExternalBlockFile was not widened when the generous + * tolerance was added, causing silent drops (nSize check + undersized CBufferedFile). + * This constant makes the import path match CheckBlock so that -reindex can rebuild + * real mainnet history. Safety remains: subsequent ProcessNewBlock still runs CheckBlock + * (which applies the same generous limit) and the checkpoint hash proof for historical + * blocks. See BLK-01 (Critical in 2026-06 full source review, rev 3). + */ +static const unsigned int GENEROUS_BLOCK_SIZE_LIMIT = 2000000; + /** The maximum allowed number of signature check operations in a block (network rule) */ static const unsigned int MAX_BLOCK_SIGOPS = 20000; /** The maximum size of a transaction (network rule) */ diff --git a/src/gtest/test_checktransaction.cpp b/src/gtest/test_checktransaction.cpp index 481f021f71f..7ad3f07ded6 100644 --- a/src/gtest/test_checktransaction.cpp +++ b/src/gtest/test_checktransaction.cpp @@ -167,8 +167,8 @@ TEST(checktransaction_tests, BadTxnsOversize) { MockCValidationState state; EXPECT_TRUE(CheckTransactionWithoutProofVerification(tx, state)); - // ... but fails contextual ones! (Force IBD off: ZClassic skips - // ContextualCheckTransaction during initial block download.) + // ... but fails contextual ones! (isInitBlockDownload() is forced false + // here; with the CR-01 fix the contextual checks also run during IBD.) EXPECT_CALL(state, DoS(100, false, REJECT_INVALID, "bad-txns-oversize", false, ::testing::_)).Times(1); EXPECT_FALSE(ContextualCheckTransaction(tx, state, 1, 100, []() { return false; })); } @@ -528,10 +528,9 @@ TEST(checktransaction_tests, bad_txns_invalid_joinsplit_signature) { CTransaction tx(mtx); MockCValidationState state; - // ZClassic skips ContextualCheckTransaction entirely during initial block - // download (see commit "speed up initial sync"), so no DoS is reported in - // IBD. Once IBD has finished, the invalid joinsplit signature is rejected - // with the full DoS ban score. + // Contextual checks now run during IBD too (CR-01 fix); only the DoS ban + // score is reduced while syncing. The invalid joinsplit signature is + // rejected with the full DoS ban score. EXPECT_CALL(state, DoS(100, false, REJECT_INVALID, "bad-txns-invalid-joinsplit-signature", false, ::testing::_)).Times(1); ContextualCheckTransaction(tx, state, 0, 100, []() { return false; }); } @@ -565,10 +564,9 @@ TEST(checktransaction_tests, non_canonical_ed25519_signature) { CTransaction tx(mtx); MockCValidationState state; - // ZClassic skips ContextualCheckTransaction entirely during initial block - // download (see commit "speed up initial sync"), so no DoS is reported in - // IBD. Once IBD has finished, the non-canonical signature is rejected with - // the full DoS ban score. + // Contextual checks now run during IBD too (CR-01 fix); only the DoS ban + // score is reduced while syncing. The non-canonical signature is rejected + // with the full DoS ban score. EXPECT_CALL(state, DoS(100, false, REJECT_INVALID, "bad-txns-invalid-joinsplit-signature", false, ::testing::_)).Times(1); ContextualCheckTransaction(tx, state, 0, 100, []() { return false; }); } diff --git a/src/gtest/test_validation.cpp b/src/gtest/test_validation.cpp index 13d14892663..8493c83e2f4 100644 --- a/src/gtest/test_validation.cpp +++ b/src/gtest/test_validation.cpp @@ -1,9 +1,11 @@ #include +#include "checkqueue.h" #include "consensus/upgrades.h" #include "consensus/validation.h" #include "main.h" #include "pow.h" +#include "script/interpreter.h" #include "txdb.h" #include "utiltest.h" @@ -371,3 +373,79 @@ TEST(Validation, ChainstateCommitmentBindsCoinMetadata) EXPECT_EQ(base.hashSerialized, bumpHeight.hashSerialized); EXPECT_NE(base.hashSerializedFull, bumpHeight.hashSerializedFull); } + +// Regression guard for the CVE-2024-52911-parity use-after-free in ConnectBlock. +// +// In ConnectBlock, queued CScriptChecks hold a raw PrecomputedTransactionData* +// into a local `txdata` vector, and ~CCheckQueueControl() calls Wait() on every +// (including early) return. Crucially, Wait() drains any still-queued checks ON +// THE CALLING THREAD (the "master" in CCheckQueue::Loop). So if `txdata` is +// declared AFTER `control`, reverse-order destruction frees txdata BEFORE +// ~control's Wait() runs the queued check that dereferences it -> heap UAF. +// +// The check below mirrors CScriptCheck: it holds a PrecomputedTransactionData* +// and dereferences it when run. Because no background worker threads are started, +// the queued check is guaranteed to execute inside ~control's Wait() on this +// thread -> the reproduction is DETERMINISTIC, not racy. +// +// * EXPECT_TRUE(ran) deterministically guards the property the fix depends on: +// ~CCheckQueueControl drains queued checks. If that contract regresses, the +// fix becomes a silent no-op and this fails WITHOUT needing a sanitizer. +// * Under AddressSanitizer, swapping the declaration order of `txdata` and +// `control` below makes the drained check read freed memory -> ASan reports +// heap-use-after-free. Build with: ./configure --with-sanitizers=address +// +// NOTE: this guards the lifetime *mechanism*; the exact declaration order inside +// ConnectBlock itself is guarded by the inline comment there + code review. +struct LifetimeCheck { + const PrecomputedTransactionData* pdata; + uint256* sink; + bool* ran; + LifetimeCheck() : pdata(nullptr), sink(nullptr), ran(nullptr) {} + LifetimeCheck(const PrecomputedTransactionData* pdataIn, uint256* sinkIn, bool* ranIn) + : pdata(pdataIn), sink(sinkIn), ran(ranIn) {} + bool operator()() { + // Dereference pdata exactly as CScriptCheck dereferences its txdata. + if (pdata != nullptr && sink != nullptr) { + *sink = pdata->hashPrevouts; + } + if (ran != nullptr) { + *ran = true; + } + return true; + } + void swap(LifetimeCheck& other) { + std::swap(pdata, other.pdata); + std::swap(sink, other.sink); + std::swap(ran, other.ran); + } +}; + +TEST(Validation, CheckQueueControlDrainsQueuedCheckBeforeTxdataDestroyed) { + CCheckQueue queue(128); + uint256 sink; + bool ran = false; + { + // ConnectBlock's FIXED ordering: txdata declared BEFORE control, so + // ~control's Wait() (which drains the queued check on this thread) runs + // BEFORE txdata is destroyed. Swapping these two lines reproduces the + // use-after-free under AddressSanitizer. + std::vector txdata; + txdata.reserve(1); + CMutableTransaction mtx; + mtx.vin.resize(1); + txdata.emplace_back(CTransaction(mtx)); + + CCheckQueueControl control(&queue); + + std::vector vChecks; + vChecks.emplace_back(&txdata[0], &sink, &ran); + control.Add(vChecks); + + // Simulate an early consensus-failure return (coinbase overpay / bad + // Sapling root): leave scope WITHOUT calling control.Wait(). + } + // ~control must have drained the queued check on this thread while txdata + // was still alive. If it did not, the fix is void. + EXPECT_TRUE(ran); +} diff --git a/src/httpserver.cpp b/src/httpserver.cpp index b6d810531c9..4cd0b7145f7 100644 --- a/src/httpserver.cpp +++ b/src/httpserver.cpp @@ -336,6 +336,14 @@ static bool HTTPBindAddresses(struct evhttp* http) endpoints.push_back(std::make_pair(host, port)); } } else { // No specific bind address specified, bind to any + // WAL-02: -rpcallowip was set without -rpcbind, so the RPC port binds on + // ALL interfaces and is reachable from the network, protected only by the + // IP ACL plus the RPC password. Warn loudly — a broad ACL (e.g. + // -rpcallowip=0.0.0.0/0) then exposes dumpprivkey/dumpwallet/z_exportkey to + // the internet behind the password alone. + LogPrintf("WARNING: -rpcallowip was specified without -rpcbind; binding RPC to all interfaces " + "(0.0.0.0 and ::). The RPC port is now network-reachable, guarded only by the IP ACL " + "and password. Set -rpcbind=127.0.0.1 (or a specific address) unless this is intended.\n"); endpoints.push_back(std::make_pair("::", defaultPort)); endpoints.push_back(std::make_pair("0.0.0.0", defaultPort)); } diff --git a/src/init.cpp b/src/init.cpp index ac853e6b495..8b5de9a3254 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -766,6 +766,14 @@ static bool check_file_hash(const std::string& path, const std::string& hash) SHA256 buff; while (!feof(file)){ size = fread(buffer.data(), 1, kHashReadBufSize, file); + // RUST-02: on a real I/O error fread sets ferror() (not feof()) and returns + // 0, so the original `while(!feof)` loop would spin forever at 100% CPU. + // Bail out instead of hanging node startup. + if (size == 0 && ferror(file)) { + LogPrintf("%s: I/O error while reading for hash check\n", path); + fclose(file); + return false; + } buff.update(buffer.data(), size); } std::string buff_hash = buff.hash(); diff --git a/src/main.cpp b/src/main.cpp index eb98a68c72d..69d626794ff 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -955,9 +955,14 @@ bool ContextualCheckTransaction( const int dosLevel, bool (*isInitBlockDownload)()) { - if (isInitBlockDownload()) { - return true; - } + // CR-01 fix: never skip contextual consensus checks during IBD / import / + // reindex. The previous early return here (when isInitBlockDownload() was + // true) bypassed tx version/activation enforcement, JoinSplit Ed25519 + // signature verification, and ALL Sapling spend/output/binding checks — a + // node-state-dependent consensus validation bypass (a syncing node could + // accept a block path a fully-synced node would reject). The DoS ban scores + // below stay reduced while syncing (isInitBlockDownload() ? 0 : ...), but + // the consensus checks themselves now always run. bool overwinterActive = Params().GetConsensus().NetworkUpgradeActive(nHeight, Consensus::UPGRADE_OVERWINTER); bool saplingActive = Params().GetConsensus().NetworkUpgradeActive(nHeight, Consensus::UPGRADE_SAPLING); bool isSprout = !overwinterActive; @@ -1211,7 +1216,27 @@ bool CheckTransactionWithoutProofVerification(const CTransaction& tx, CValidatio // Size limits BOOST_STATIC_ASSERT(MAX_BLOCK_SIZE >= MAX_TX_SIZE_AFTER_SAPLING); // sanity BOOST_STATIC_ASSERT(MAX_TX_SIZE_AFTER_SAPLING > MAX_TX_SIZE_BEFORE_SAPLING); // sanity - if (::GetSerializeSize(tx, SER_NETWORK, PROTOCOL_VERSION) > MAX_TX_SIZE_AFTER_SAPLING) + // This is the NON-contextual transaction-size check, so its bound must hold + // for EVERY block in chain history. Some historical (pre-reduction) blocks + // carry transactions larger than the current MAX_TX_SIZE_AFTER_SAPLING + // (102000), so enforcing that tight limit here rejects valid canonical + // blocks during -reindex / from-genesis validation (the daemon stalls at + // the first such block, ~height 478544, early Sapling era). Use a generous + // bound here, exactly mirroring GENEROUS_BLOCK_SIZE_LIMIT in CheckBlock + // ("checkpoint validates correctness"). If the tighter MAX_TX_SIZE_AFTER_SAPLING + // limit is meant to be a live consensus rule for new blocks, enforce it in + // ContextualCheckTransaction (which has nHeight), gated on its activation + // height — not in this non-contextual check. + // + // CON-04 (DECISION REQUIRED — intentionally NOT changed here): restoring the + // 102000-byte post-Sapling tx limit for new transactions is a TIGHTENING of the + // rules, i.e. a soft fork. It MUST be staged at a fixed future activation height + // (so the 1,272 historical >200 KB blocks and any historical >102 KB txs stay + // valid) and added in ContextualCheckTransaction. That height is a policy choice + // for the maintainers; this review does not pick one, so runtime behavior is + // left unchanged (current rule = generous 2 MB). + const unsigned int GENEROUS_TX_SIZE_LIMIT = 2000000; // 2MB, matches GENEROUS_BLOCK_SIZE_LIMIT + if (::GetSerializeSize(tx, SER_NETWORK, PROTOCOL_VERSION) > GENEROUS_TX_SIZE_LIMIT) return state.DoS(100, error("CheckTransaction(): size limits failed"), REJECT_INVALID, "bad-txns-oversize"); @@ -2613,6 +2638,14 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin CBlockUndo blockundo; + // txdata must be declared before `control`: queued CScriptChecks hold raw + // PrecomputedTransactionData* into this vector, and ~CCheckQueueControl() + // calls Wait() for in-flight worker checks on every (including early) return. + // Declaring txdata first guarantees reverse-order destruction runs that Wait() + // before txdata is destroyed, closing the use-after-free (CVE-2024-52911 parity). + std::vector txdata; + txdata.reserve(block.vtx.size()); // Required so that pointers to individual PrecomputedTransactionData don't get invalidated + CCheckQueueControl control(fExpensiveChecks && nScriptCheckThreads ? &scriptcheckqueue : NULL); int64_t nTimeStart = GetTimeMicros(); @@ -2649,8 +2682,6 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin // Grab the consensus branch ID for the block's height auto consensusBranchId = CurrentEpochBranchId(pindex->nHeight, Params().GetConsensus()); - std::vector txdata; - txdata.reserve(block.vtx.size()); // Required so that pointers to individual PrecomputedTransactionData don't get invalidated for (unsigned int i = 0; i < block.vtx.size(); i++) { const CTransaction &tx = block.vtx[i]; @@ -4063,8 +4094,12 @@ void FallbackSproutValuePoolBalance( // this point onwards (assuming the checkpoint is late enough) pindex->nChainSproutValue = chainparams.SproutValuePoolCheckpointBalance(); } else { - // Apparently we have been. So, we should expect the current - // value to match the hardcoded one. + // Chain-value recomputation cross-check against the ZIP-209 Sprout checkpoint. + // The nChainSproutValue was computed by summing per-block nSproutValue deltas + // (via checked arithmetic in ReceivedBlockTransactions + propagation). + // It must match the independently known balance at this height. + // This is the live recomputation test for the pool accounting (see also + // scripts/audit-mainnet-history.py for the full-history version). assert(*pindex->nChainSproutValue == chainparams.SproutValuePoolCheckpointBalance()); // And we should expect non-none for the delta stored in the block index here, // or the checkpoint is too early. @@ -4087,21 +4122,53 @@ bool ReceivedBlockTransactions(const CBlock &block, CValidationState& state, CBl pindexNew->nChainTx = 0; CAmount sproutValue = 0; CAmount saplingValue = 0; + bool blockDeltaOk = true; for (auto tx : block.vtx) { // Negative valueBalance "takes" money from the transparent value pool // and adds it to the Sapling value pool. Positive valueBalance "gives" // money to the transparent value pool, removing from the Sapling value // pool. So we invert the sign here. - saplingValue += -tx.valueBalance; + if (!CheckedAddTo(saplingValue, -tx.valueBalance)) { + blockDeltaOk = false; + break; + } for (auto js : tx.vjoinsplit) { - sproutValue += js.vpub_old; - sproutValue -= js.vpub_new; + if (!CheckedAddTo(sproutValue, js.vpub_old) || + !CheckedAddTo(sproutValue, -js.vpub_new)) { + blockDeltaOk = false; + break; + } } + if (!blockDeltaOk) break; + } + if (blockDeltaOk) { + pindexNew->nSproutValue = sproutValue; + pindexNew->nSaplingValue = saplingValue; + } else { + // Overflow in per-block shielded pool delta (see April-2026 per-pool + // signed overflow class). Do not trust this block's delta for chain + // value propagation; descendants will get nChain*Value = none until + // a later checkpoint or reindex can re-establish a known-good total. + pindexNew->nSproutValue = boost::none; + // CON-02/03: nSaplingValue is a plain CAmount (not boost::optional, see + // chain.h), so unlike Sprout it cannot carry an "unknown" sentinel; we + // store 0 here. nChainSaplingValue is forced to none just below and the + // accumulation loops propagate none to every descendant (they guard on + // pprev->nChainSaplingValue). The ZIP-209 turnstile in ConnectBlock only + // runs when nChainSaplingValue is present, so for an overflowed block (and + // its descendants) the turnstile is SKIPPED, not enforced — i.e. the + // substituted 0 can never cause a wrong turnstile PASS on corrupted data, + // but the turnstile is also not enforced across the unknown window. That + // trade-off (check-skipped, not fail-closed rejection) is acceptable for + // this corruption-recovery path; a checkpoint/reindex re-establishes a + // known-good total. Carrying a true per-block "unknown" would require + // making nSaplingValue optional, which changes the on-disk + // CDiskBlockIndex format; deferred. The actual UB (raw signed '+') is + // fixed at the CON-01 sites. + pindexNew->nSaplingValue = 0; } - pindexNew->nSproutValue = sproutValue; pindexNew->nChainSproutValue = boost::none; - pindexNew->nSaplingValue = saplingValue; pindexNew->nChainSaplingValue = boost::none; pindexNew->nFile = pos.nFile; pindexNew->nDataPos = pos.nPos; @@ -4122,12 +4189,22 @@ bool ReceivedBlockTransactions(const CBlock &block, CValidationState& state, CBl pindex->nChainTx = (pindex->pprev ? pindex->pprev->nChainTx : 0) + pindex->nTx; if (pindex->pprev) { if (pindex->pprev->nChainSproutValue && pindex->nSproutValue) { - pindex->nChainSproutValue = *pindex->pprev->nChainSproutValue + *pindex->nSproutValue; + CAmount chainSprout; + if (CheckedAdd(*pindex->pprev->nChainSproutValue, *pindex->nSproutValue, chainSprout)) { + pindex->nChainSproutValue = chainSprout; + } else { + pindex->nChainSproutValue = boost::none; + } } else { pindex->nChainSproutValue = boost::none; } if (pindex->pprev->nChainSaplingValue) { - pindex->nChainSaplingValue = *pindex->pprev->nChainSaplingValue + pindex->nSaplingValue; + CAmount chainSapling; + if (CheckedAdd(*pindex->pprev->nChainSaplingValue, pindex->nSaplingValue, chainSapling)) { + pindex->nChainSaplingValue = chainSapling; + } else { + pindex->nChainSaplingValue = boost::none; + } } else { pindex->nChainSaplingValue = boost::none; } @@ -4312,9 +4389,16 @@ bool CheckBlock(const CBlock& block, CValidationState& state, // Skip all structural validation (size, coinbase, transactions, sigops) for pre-checkpoint blocks. if (fCheckSizeLimits) { // Size limits - // Allow larger blocks for historical chain variations - checkpoint validates correctness - const unsigned int GENEROUS_BLOCK_SIZE_LIMIT = 2000000; // 2MB to accommodate any historical forks - if (block.vtx.empty() || block.vtx.size() > GENEROUS_BLOCK_SIZE_LIMIT || ::GetSerializeSize(block, SER_NETWORK, PROTOCOL_VERSION) > GENEROUS_BLOCK_SIZE_LIMIT) + // Allow larger blocks for historical chain variations - checkpoint validates correctness. + // The real mainnet history contains 1,272 blocks in (200000, 2000000] bytes (see + // audit-full.json + scripts/audit-mainnet-history.py). This generous limit (defined + // once in consensus/consensus.h) is also used by the -reindex/-loadblock path so that + // LoadExternalBlockFile can successfully import the canonical chain. See BLK-01. + // CON-05: the serialized-size check below is the real block-size guard. + // The former `block.vtx.size() > GENEROUS_BLOCK_SIZE_LIMIT` conjunct compared a + // transaction *count* against a *byte* constant (a no-op in practice, since vtx + // is already bounded by the serialized size) and was dropped for clarity. + if (block.vtx.empty() || ::GetSerializeSize(block, SER_NETWORK, PROTOCOL_VERSION) > GENEROUS_BLOCK_SIZE_LIMIT) return state.DoS(100, error("CheckBlock(): size limits failed"), REJECT_INVALID, "bad-blk-length"); @@ -4389,6 +4473,17 @@ bool ContextualCheckBlockHeader(const CBlockHeader& block, CValidationState& sta CBlockIndex* pcheckpoint = Checkpoints::GetLastCheckpoint(chainParams.Checkpoints()); if (pcheckpoint && nHeight < pcheckpoint->nHeight) return state.DoS(100, error("%s: forked chain older than last checkpoint (height %d)", __func__, nHeight)); + + // Enforce the exact hash at checkpoint heights. A header presented at a + // checkpoint height with a different hash is a forgery. The forked-chain + // check above only covers heights strictly below the last checkpoint + // *present in mapBlockIndex*; this closes the gap at the checkpoint + // height itself and is independent of mapBlockIndex state, so it also + // protects a fresh/eclipsed node during bootstrap. + if (!Checkpoints::CheckBlock(chainParams.Checkpoints(), nHeight, hash)) + return state.DoS(100, error("%s: rejected by checkpoint lock-in at height %d (hash %s)", + __func__, nHeight, hash.ToString()), + REJECT_CHECKPOINT, "bad-fork-checkpoint"); } // Reject block.nVersion < 4 blocks @@ -4473,7 +4568,11 @@ bool AcceptBlockHeader(const CBlockHeader& block, CValidationState& state, CBloc return false; - if (!pindexPrev->IsValid(BLOCK_VALID_SCRIPTS)) { + // pindexPrev is NULL for the genesis block (set only for non-genesis above). + // The genesis block has no ancestors, so the invalid-ancestor walk below is + // meaningless for it; guard the dereference to avoid a NULL segfault during + // -reindex / import, where LoadExternalBlockFile re-accepts the genesis header. + if (pindexPrev != NULL && !pindexPrev->IsValid(BLOCK_VALID_SCRIPTS)) { for (const CBlockIndex *failedit : g_failed_blocks) { if (pindexPrev->GetAncestor(failedit->nHeight) == failedit) { assert(failedit->nStatus & BLOCK_FAILED_MASK); @@ -4544,7 +4643,14 @@ bool AcceptBlock(CBlock& block, CValidationState& state, CBlockIndex** ppindex, // mark the chain as parked. If it has enough work, it'll unpark // automatically. We mark the block as parked at the very last minute so we // can make sure everything is ready to be reorged if needed. - if (GetBoolArg("-parkdeepreorg", true)) { + // + // Skip during initial block download / reindex: there, blocks are loaded + // from disk out of height order, so the "would cause a deep reorg" test + // (fork depth > 1) fires spuriously and parks large swaths of the chain, + // stalling the sync (the bug that forces -parkdeepreorg=0 on a from-genesis + // reindex). Deep-reorg protection is an at-tip defense; during IBD the node + // only follows the best chain forward, so there is nothing to protect. + if (GetBoolArg("-parkdeepreorg", true) && !IsInitialBlockDownload()) { const CBlockIndex *pindexFork = chainActive.FindFork(pindex); if (pindexFork && pindexFork->nHeight + 1 < pindex->nHeight) { LogPrintf("Park block %s as it would cause a deep reorg.\n", @@ -4849,13 +4955,28 @@ bool static LoadBlockIndexDB() if (pindex->pprev) { if (pindex->pprev->nChainTx) { pindex->nChainTx = pindex->pprev->nChainTx + pindex->nTx; + // CON-01: use overflow-safe CheckedAdd here, exactly as the live + // ReceivedBlockTransactions path does. Raw signed '+' on CAmount + // (int64_t) is undefined behaviour on overflow; on a corrupted + // on-disk delta it could wrap the chain totals that the ZIP-209 + // turnstile reads. Fall back to boost::none (unknown) on overflow. if (pindex->pprev->nChainSproutValue && pindex->nSproutValue) { - pindex->nChainSproutValue = *pindex->pprev->nChainSproutValue + *pindex->nSproutValue; + CAmount chainSprout; + if (CheckedAdd(*pindex->pprev->nChainSproutValue, *pindex->nSproutValue, chainSprout)) { + pindex->nChainSproutValue = chainSprout; + } else { + pindex->nChainSproutValue = boost::none; + } } else { pindex->nChainSproutValue = boost::none; } if (pindex->pprev->nChainSaplingValue) { - pindex->nChainSaplingValue = *pindex->pprev->nChainSaplingValue + pindex->nSaplingValue; + CAmount chainSapling; + if (CheckedAdd(*pindex->pprev->nChainSaplingValue, pindex->nSaplingValue, chainSapling)) { + pindex->nChainSaplingValue = chainSapling; + } else { + pindex->nChainSaplingValue = boost::none; + } } else { pindex->nChainSaplingValue = boost::none; } @@ -5371,8 +5492,16 @@ bool LoadExternalBlockFile(FILE* fileIn, CDiskBlockPos *dbp) int nLoaded = 0; try { - // This takes over fileIn and calls fclose() on it in the CBufferedFile destructor - CBufferedFile blkdat(fileIn, 2*MAX_BLOCK_SIZE, MAX_BLOCK_SIZE+8, SER_DISK, CLIENT_VERSION); + // This takes over fileIn and calls fclose() on it in the CBufferedFile destructor. + // + // Use GENEROUS_BLOCK_SIZE_LIMIT (not MAX_BLOCK_SIZE) for both the buffer and the + // nSize filter. The canonical chain contains 1,272 blocks larger than the original + // 200000-byte MAX_BLOCK_SIZE (all < 2 MB). Without this, -reindex and -loadblock + // (see call sites in init.cpp) silently skip them via the size check or fail to + // buffer them, producing a divergent chainstate even though CheckBlock + checkpoints + // would have accepted them. ProcessNewBlock below will still validate each block + // through the normal (generous) CheckBlock path. See BLK-01 (Critical). + CBufferedFile blkdat(fileIn, 2*GENEROUS_BLOCK_SIZE_LIMIT, GENEROUS_BLOCK_SIZE_LIMIT+8, SER_DISK, CLIENT_VERSION); uint64_t nRewind = blkdat.GetPos(); while (!blkdat.eof()) { boost::this_thread::interruption_point(); @@ -5391,7 +5520,7 @@ bool LoadExternalBlockFile(FILE* fileIn, CDiskBlockPos *dbp) continue; // read size blkdat >> nSize; - if (nSize < 80 || nSize > MAX_BLOCK_SIZE) + if (nSize < 80 || nSize > GENEROUS_BLOCK_SIZE_LIMIT) continue; } catch (const std::exception&) { // no valid block header found; don't complain @@ -6382,9 +6511,14 @@ bool ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, int64_t { if (pnode->nVersion < CADDR_TIME_VERSION) continue; - unsigned int nPointer; + // NET-01: use uintptr_t (not unsigned int) so the full pointer + // value mixes into the relay-selection hash. On 64-bit a 4-byte + // copy kept only the low 32 bits (ASLR randomizes the high bits), + // letting distinct CNode* collide and biasing which peers receive + // freshly-relayed addresses. + uintptr_t nPointer; memcpy(&nPointer, &pnode, sizeof(nPointer)); - uint256 hashKey = ArithToUint256(UintToArith256(hashRand) ^ nPointer); + uint256 hashKey = ArithToUint256(UintToArith256(hashRand) ^ (uint64_t)nPointer); hashKey = Hash(BEGIN(hashKey), END(hashKey)); mapMix.insert(make_pair(hashKey, pnode)); } diff --git a/src/miner.cpp b/src/miner.cpp index 07b30750bad..935bacdf86f 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -334,14 +334,18 @@ CBlockTemplate* CreateNewBlock(const CScript& scriptPubKeyIn) CAmount sproutValueDummy = sproutValue; CAmount saplingValueDummy = saplingValue; - saplingValueDummy += -tx.valueBalance; - - for (auto js : tx.vjoinsplit) { - sproutValueDummy += js.vpub_old; - sproutValueDummy -= js.vpub_new; + bool dummyOk = CheckedAddTo(saplingValueDummy, -tx.valueBalance); + if (dummyOk) { + for (auto js : tx.vjoinsplit) { + if (!CheckedAddTo(sproutValueDummy, js.vpub_old) || + !CheckedAddTo(sproutValueDummy, -js.vpub_new)) { + dummyOk = false; + break; + } + } } - if (sproutValueDummy < 0) { + if (!dummyOk || sproutValueDummy < 0) { LogPrintf("CreateNewBlock(): tx %s appears to violate Sprout turnstile\n", tx.GetHash().ToString()); continue; } diff --git a/src/net.cpp b/src/net.cpp index 4aab7bda1c0..99a7122f9db 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -496,6 +496,17 @@ void CNode::Ban(const CSubNet& subNet, int64_t bantimeoffset, bool sinceUnixEpoc banTime = (sinceUnixEpoch ? 0 : GetTime() )+bantimeoffset; LOCK(cs_setBanned); + // NET-02: prune expired entries on each new ban so setBanned cannot grow + // unbounded. Without this, an attacker cycling distinct source IPs accumulates + // dead entries that IsBanned() linearly scans under cs_setBanned on every + // inbound connection, degrading into a lock-contention bottleneck over time. + int64_t nowPrune = GetTime(); + for (std::map::iterator it = setBanned.begin(); it != setBanned.end(); ) { + if (it->second < nowPrune) + setBanned.erase(it++); + else + ++it; + } if (setBanned[subNet] < banTime) setBanned[subNet] = banTime; } @@ -1883,6 +1894,17 @@ void RelayTransaction(const CTransaction& tx, const CDataStream& ss) vRelayExpiration.pop_front(); } + // PERF-03: hard cap on relay entries. Time-based expiry alone lets a flood + // of unique-txid transactions grow mapRelay (full CDataStream per entry) to + // hundreds of MB before the 15-minute timer reclaims the oldest. Evict the + // oldest entries once the cap is reached. + static const size_t MAX_RELAY_ENTRIES = 100000; + while (mapRelay.size() >= MAX_RELAY_ENTRIES && !vRelayExpiration.empty()) + { + mapRelay.erase(vRelayExpiration.front().second); + vRelayExpiration.pop_front(); + } + // Save original serialized message so newer versions are preserved mapRelay.insert(std::make_pair(inv, ss)); vRelayExpiration.push_back(std::make_pair(GetTime() + 15 * 60, inv)); diff --git a/src/rpc/misc.cpp b/src/rpc/misc.cpp index f6d31c1641f..1ab134644f2 100644 --- a/src/rpc/misc.cpp +++ b/src/rpc/misc.cpp @@ -100,7 +100,7 @@ UniValue getinfo(const UniValue& params, bool fHelp) obj.push_back(Pair("keypoolsize", (int)pwalletMain->GetKeyPoolSize())); } if (pwalletMain && pwalletMain->IsCrypted()) - obj.push_back(Pair("unlocked_until", nWalletUnlockTime)); + obj.push_back(Pair("unlocked_until", nWalletUnlockTime.load())); // WAL-06 obj.push_back(Pair("paytxfee", ValueFromAmount(payTxFee.GetFeePerK()))); #endif obj.push_back(Pair("relayfee", ValueFromAmount(::minRelayTxFee.GetFeePerK()))); diff --git a/src/rpc/server.h b/src/rpc/server.h index 6ae4279287a..4fb2e3cc251 100644 --- a/src/rpc/server.h +++ b/src/rpc/server.h @@ -10,6 +10,7 @@ #include "rpc/protocol.h" #include "uint256.h" +#include #include #include #include @@ -169,7 +170,9 @@ extern uint256 ParseHashO(const UniValue& o, std::string strKey); extern std::vector ParseHexV(const UniValue& v, std::string strName); extern std::vector ParseHexO(const UniValue& o, std::string strKey); -extern int64_t nWalletUnlockTime; +// WAL-06: atomic so unlocked reads in getwalletinfo/getinfo cannot tear against +// the relock timer's write (writes still occur under cs_nWalletUnlockTime). +extern std::atomic nWalletUnlockTime; extern CAmount AmountFromValue(const UniValue& value); extern UniValue ValueFromAmount(const CAmount& amount); extern double GetDifficulty(const CBlockIndex* blockindex = NULL); diff --git a/src/serialize.h b/src/serialize.h index a945650d6f5..965ba62da4d 100644 --- a/src/serialize.h +++ b/src/serialize.h @@ -366,13 +366,25 @@ template I ReadVarInt(Stream& is) { I n = 0; + // MEM-02: guard against overflow of I BEFORE shifting and BEFORE the + // increment, so a malformed max-length encoding cannot wrap the value or hit + // signed UB (VARINT is also used for signed fields, e.g. CDiskBlockIndex + // nFile/nPos). This is the upstream Bitcoin Core form; it also bounds the + // iteration count, since n grows past the limit within ceil(bits/7) bytes. while(true) { unsigned char chData = ser_readdata8(is); + if (n > (std::numeric_limits::max() >> 7)) { + throw std::ios_base::failure("ReadVarInt(): size too large"); + } n = (n << 7) | (chData & 0x7F); - if (chData & 0x80) + if (chData & 0x80) { + if (n == std::numeric_limits::max()) { + throw std::ios_base::failure("ReadVarInt(): size too large"); + } n++; - else + } else { return n; + } } } diff --git a/src/streams.h b/src/streams.h index 9d4a2e39e04..79f4185f260 100644 --- a/src/streams.h +++ b/src/streams.h @@ -281,20 +281,21 @@ class CBaseDataStream throw std::ios_base::failure("CBaseDataStream::read(): cannot read from null pointer"); } - // Read from the beginning of the buffer - unsigned int nReadPosNext = nReadPos + nSize; - if (nReadPosNext >= vch.size()) + // MEM-01: do the bounds arithmetic in size_t and check for overflow + // BEFORE adding. Previously `unsigned int nReadPosNext = nReadPos + nSize` + // narrowed the size_t nSize on LP64, so a large nSize could wrap to a small + // value that slipped past the guard before the full-size memcpy ran. + if (nReadPos > vch.size() || nSize > vch.size() - nReadPos) { + throw std::ios_base::failure("CBaseDataStream::read(): end of data"); + } + size_t nReadPosNext = (size_t)nReadPos + nSize; + memcpy(pch, &vch[nReadPos], nSize); + if (nReadPosNext == vch.size()) { - if (nReadPosNext > vch.size()) - { - throw std::ios_base::failure("CBaseDataStream::read(): end of data"); - } - memcpy(pch, &vch[nReadPos], nSize); nReadPos = 0; vch.clear(); return; } - memcpy(pch, &vch[nReadPos], nSize); nReadPos = nReadPosNext; } @@ -304,11 +305,15 @@ class CBaseDataStream if (nSize < 0) { throw std::ios_base::failure("CDataStream::ignore(): nSize negative"); } - unsigned int nReadPosNext = nReadPos + nSize; - if (nReadPosNext >= vch.size()) + // MEM-01: size_t arithmetic with an explicit pre-addition overflow guard + // (see read() above). + size_t snSize = (size_t)nSize; + if (nReadPos > vch.size() || snSize > vch.size() - nReadPos) { + throw std::ios_base::failure("CBaseDataStream::ignore(): end of data"); + } + size_t nReadPosNext = (size_t)nReadPos + snSize; + if (nReadPosNext == vch.size()) { - if (nReadPosNext > vch.size()) - throw std::ios_base::failure("CBaseDataStream::ignore(): end of data"); nReadPos = 0; vch.clear(); return; diff --git a/src/test/Checkpoints_tests.cpp b/src/test/Checkpoints_tests.cpp index e31c8903cf9..755bfbb91e1 100644 --- a/src/test/Checkpoints_tests.cpp +++ b/src/test/Checkpoints_tests.cpp @@ -97,4 +97,26 @@ BOOST_AUTO_TEST_CASE(fast_sync_anchor_negative_branches) } } +BOOST_AUTO_TEST_CASE(checkpoint_hash_lockin) +{ + const uint256 hashA = uint256S("0x00000000000000000000000000000000000000000000000000000000000000aa"); + const uint256 hashB = uint256S("0x00000000000000000000000000000000000000000000000000000000000000bb"); + + CCheckpointData data{}; + data.mapCheckpoints[30000] = hashA; + data.mapCheckpoints[160000] = hashB; + + // No checkpoint at this height -> always accepted, regardless of hash. + BOOST_CHECK(Checkpoints::CheckBlock(data, 12345, hashA)); + BOOST_CHECK(Checkpoints::CheckBlock(data, 12345, hashB)); + + // Correct hash at a checkpoint height -> accepted. + BOOST_CHECK(Checkpoints::CheckBlock(data, 30000, hashA)); + BOOST_CHECK(Checkpoints::CheckBlock(data, 160000, hashB)); + + // Wrong hash at a checkpoint height -> rejected (the forgery case #2 closes). + BOOST_CHECK(!Checkpoints::CheckBlock(data, 30000, hashB)); + BOOST_CHECK(!Checkpoints::CheckBlock(data, 160000, hashA)); +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp index 0fcdd653067..0d91e4a8e17 100644 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -409,16 +409,16 @@ BOOST_AUTO_TEST_CASE(test_FormatSubVersion) std::vector comments2; comments2.push_back(std::string("comment1")); comments2.push_back(SanitizeString(std::string("Comment2; .,_?@; !\"#$%&'()*+-/<=>[]\\^`{|}~"), SAFE_CHARS_UA_COMMENT)); // Semicolon is discouraged but not forbidden by BIP-0014 - BOOST_CHECK_EQUAL(FormatSubVersion("Test", 99900, std::vector()), std::string("/Test:0.9.99-beta1/")); - BOOST_CHECK_EQUAL(FormatSubVersion("Test", 99924, std::vector()), std::string("/Test:0.9.99-beta25/")); + BOOST_CHECK_EQUAL(FormatSubVersion("Test", 99900, std::vector()), std::string("/Test:0.9.99-ZIP209-beta1/")); + BOOST_CHECK_EQUAL(FormatSubVersion("Test", 99924, std::vector()), std::string("/Test:0.9.99-ZIP209-beta25/")); BOOST_CHECK_EQUAL(FormatSubVersion("Test", 99925, std::vector()), std::string("/Test:0.9.99-rc1/")); BOOST_CHECK_EQUAL(FormatSubVersion("Test", 99949, std::vector()), std::string("/Test:0.9.99-rc25/")); BOOST_CHECK_EQUAL(FormatSubVersion("Test", 99950, std::vector()), std::string("/Test:0.9.99/")); BOOST_CHECK_EQUAL(FormatSubVersion("Test", 99951, std::vector()), std::string("/Test:0.9.99-1/")); BOOST_CHECK_EQUAL(FormatSubVersion("Test", 99999, std::vector()), std::string("/Test:0.9.99-49/")); - BOOST_CHECK_EQUAL(FormatSubVersion("Test", 99900, comments), std::string("/Test:0.9.99-beta1(comment1)/")); + BOOST_CHECK_EQUAL(FormatSubVersion("Test", 99900, comments), std::string("/Test:0.9.99-ZIP209-beta1(comment1)/")); BOOST_CHECK_EQUAL(FormatSubVersion("Test", 99950, comments), std::string("/Test:0.9.99(comment1)/")); - BOOST_CHECK_EQUAL(FormatSubVersion("Test", 99900, comments2), std::string("/Test:0.9.99-beta1(comment1; Comment2; .,_?@; )/")); + BOOST_CHECK_EQUAL(FormatSubVersion("Test", 99900, comments2), std::string("/Test:0.9.99-ZIP209-beta1(comment1; Comment2; .,_?@; )/")); BOOST_CHECK_EQUAL(FormatSubVersion("Test", 99950, comments2), std::string("/Test:0.9.99(comment1; Comment2; .,_?@; )/")); } diff --git a/src/transaction_builder.cpp b/src/transaction_builder.cpp index 30808e7200e..7239d1a448b 100644 --- a/src/transaction_builder.cpp +++ b/src/transaction_builder.cpp @@ -285,18 +285,29 @@ TransactionBuilderResult TransactionBuilder::Build() } // Create Sapling spendAuth and binding signatures + // RUST-01: these FFI calls return bool and fail on an invalid ask/ar (e.g. a + // corrupted spending key). Previously the returns were discarded, so on + // failure the tx was built with an all-zero signature that every node rejects + // while the wallet treated the note as spent (funds stranded, hard to + // diagnose). Check both and surface an error instead. for (size_t i = 0; i < spends.size(); i++) { - librustzcash_sapling_spend_sig( - spends[i].expsk.ask.begin(), - spends[i].alpha.begin(), + if (!librustzcash_sapling_spend_sig( + spends[i].expsk.ask.begin(), + spends[i].alpha.begin(), + dataToBeSigned.begin(), + mtx.vShieldedSpend[i].spendAuthSig.data())) { + librustzcash_sapling_proving_ctx_free(ctx); + return TransactionBuilderResult("Failed to create Sapling spend signature"); + } + } + if (!librustzcash_sapling_binding_sig( + ctx, + mtx.valueBalance, dataToBeSigned.begin(), - mtx.vShieldedSpend[i].spendAuthSig.data()); + mtx.bindingSig.data())) { + librustzcash_sapling_proving_ctx_free(ctx); + return TransactionBuilderResult("Failed to create Sapling binding signature"); } - librustzcash_sapling_binding_sig( - ctx, - mtx.valueBalance, - dataToBeSigned.begin(), - mtx.bindingSig.data()); librustzcash_sapling_proving_ctx_free(ctx); diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 9a0f71e3f70..590f0926237 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -487,6 +487,17 @@ UniValue dumpwallet_impl(const UniValue& params, bool fHelp, bool fDumpZKeys) if (!file.is_open()) throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot open wallet dump file"); + // WAL-05: this file contains every private key AND the HD seed (root entropy + // for all derived keys) in plaintext. Restrict it to owner read/write (0600) + // immediately so it is not left world/group-readable in the export directory. + try { + boost::filesystem::permissions(exportfilepath, + boost::filesystem::owner_read | boost::filesystem::owner_write); + } catch (const boost::filesystem::filesystem_error& e) { + LogPrintf("dumpwallet: warning: could not set 0600 permissions on %s: %s\n", + exportfilepath.string(), e.what()); + } + std::map mapKeyBirth; std::set setKeyPool; pwalletMain->GetKeyBirthTimes(mapKeyBirth); diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 86149e415f2..9cfe397aaf5 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -50,8 +50,8 @@ const std::string ADDR_TYPE_SAPLING = "sapling"; extern UniValue TxJoinSplitToJSON(const CTransaction& tx); -int64_t nWalletUnlockTime; -static CCriticalSection cs_nWalletUnlockTime; +std::atomic nWalletUnlockTime; // WAL-06: atomic (see rpc/server.h) +static CCriticalSection cs_nWalletUnlockTime; // still serializes check-then-set writes // Private method: UniValue z_getoperationstatus_IMPL(const UniValue&, bool); @@ -2299,7 +2299,7 @@ UniValue getwalletinfo(const UniValue& params, bool fHelp) obj.push_back(Pair("keypoololdest", pwalletMain->GetOldestKeyPoolTime())); obj.push_back(Pair("keypoolsize", (int)pwalletMain->GetKeyPoolSize())); if (pwalletMain->IsCrypted()) - obj.push_back(Pair("unlocked_until", nWalletUnlockTime)); + obj.push_back(Pair("unlocked_until", nWalletUnlockTime.load())); // WAL-06 obj.push_back(Pair("paytxfee", ValueFromAmount(payTxFee.GetFeePerK()))); uint256 seedFp = pwalletMain->GetHDChain().seedFp; if (!seedFp.IsNull()) @@ -3743,6 +3743,11 @@ UniValue z_sendmany(const UniValue& params, bool fHelp) + strprintf("%s", FormatMoney(ASYNC_RPC_OPERATION_DEFAULT_MINERS_FEE)) + ") The fee amount to attach to this transaction.\n" "\nResult:\n" "\"operationid\" (string) An operationid to pass to z_getoperationstatus to get the result of the operation.\n" + "\nWARNING: running with -debug=zrpcunsafe (or -debug=all) writes the\n" + "sender address, every recipient address, amounts and memo fields of this\n" + "call to debug.log in plaintext, defeating shielded-pool privacy. Do not\n" + "enable that category on a node whose debug.log is shipped to log\n" + "aggregators or shared.\n" "\nExamples:\n" + HelpExampleCli("z_sendmany", "\"t1M72Sfpbz1BPpXFHz9m3CdqATR44Jvaydd\" '[{\"address\": \"ztfaW34Gj9FrnGUEf833ywDVL62NWXBM81u6EQnM6VR45eYnXhwztecW1SjxA7JrmAXKJhxhj3vDNEpVCQoSvVoSpmbhtjf\" ,\"amount\": 5.0}]'") + HelpExampleRpc("z_sendmany", "\"t1M72Sfpbz1BPpXFHz9m3CdqATR44Jvaydd\", [{\"address\": \"ztfaW34Gj9FrnGUEf833ywDVL62NWXBM81u6EQnM6VR45eYnXhwztecW1SjxA7JrmAXKJhxhj3vDNEpVCQoSvVoSpmbhtjf\" ,\"amount\": 5.0}]") diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 10602ebf5d6..cb32aab346f 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -537,8 +537,13 @@ bool CWallet::ChangeWalletPassphrase(const SecureString& strOldWalletPassphrase, crypter.SetKeyFromPassphrase(strNewWalletPassphrase, pMasterKey.second.vchSalt, pMasterKey.second.nDeriveIterations, pMasterKey.second.nDerivationMethod); pMasterKey.second.nDeriveIterations = (pMasterKey.second.nDeriveIterations + pMasterKey.second.nDeriveIterations * 100 / ((double)(GetTimeMillis() - nStartTime))) / 2; - if (pMasterKey.second.nDeriveIterations < 25000) - pMasterKey.second.nDeriveIterations = 25000; + // WAL-03: raise the KDF iteration floor 25000 -> 100000. The + // dynamic calibration targets ~0.1s and normally lands far higher; + // the floor only binds on very fast machines, where 25000 rounds of + // (non-memory-hard) PBKDF2-SHA512 is too cheap against an offline + // wallet.dat dictionary attack. + if (pMasterKey.second.nDeriveIterations < 100000) + pMasterKey.second.nDeriveIterations = 100000; LogPrintf("Wallet passphrase changed to an nDeriveIterations of %i\n", pMasterKey.second.nDeriveIterations); @@ -1195,8 +1200,9 @@ bool CWallet::EncryptWallet(const SecureString& strWalletPassphrase) crypter.SetKeyFromPassphrase(strWalletPassphrase, kMasterKey.vchSalt, kMasterKey.nDeriveIterations, kMasterKey.nDerivationMethod); kMasterKey.nDeriveIterations = (kMasterKey.nDeriveIterations + kMasterKey.nDeriveIterations * 100 / ((double)(GetTimeMillis() - nStartTime))) / 2; - if (kMasterKey.nDeriveIterations < 25000) - kMasterKey.nDeriveIterations = 25000; + // WAL-03: raise the KDF iteration floor 25000 -> 100000 (see ChangeWalletPassphrase). + if (kMasterKey.nDeriveIterations < 100000) + kMasterKey.nDeriveIterations = 100000; LogPrintf("Encrypting Wallet with an nDeriveIterations of %i\n", kMasterKey.nDeriveIterations); @@ -2779,7 +2785,11 @@ void CWallet::ReacceptWalletTransactions() { CWalletTx& wtx = *(item.second); - LOCK(mempool.cs); + // PERF-01: do not take an explicit LOCK(mempool.cs) here. AcceptToMemoryPool + // already acquires mempool.cs internally (under cs_main); taking it here too + // only added a redundant cs_wallet -> mempool.cs lock-order edge that a + // lock-order checker flags as an inversion against the pool.cs -> cs_wallet + // path (NotifyRecentlyAdded -> SyncWithWallets). wtx.AcceptToMemoryPool(false); } } diff --git a/src/zcash/NoteEncryption.cpp b/src/zcash/NoteEncryption.cpp index 63e07326542..eee0a61c736 100644 --- a/src/zcash/NoteEncryption.cpp +++ b/src/zcash/NoteEncryption.cpp @@ -4,9 +4,26 @@ #include #include "prf.h" #include "librustzcash.h" +#include "support/cleanse.h" // CRY-01: zeroize symmetric keys / DH secrets after use #define NOTEENCRYPTION_CIPHER_KEYSIZE 32 +namespace { +// CRY-01: zeroes a fixed buffer when it leaves scope, so a derived symmetric key +// or DH secret is cleared on EVERY exit path — normal return, early return, and +// any throw (e.g. a should-never-happen KDF hash-failure between key derivation +// and the end of the function). Declaring one of these right after the secret +// buffer guarantees cleanup without relying on manual calls before each return. +struct MemoryCleanser { + void* p; + size_t n; + MemoryCleanser(void* p_, size_t n_) : p(p_), n(n_) {} + ~MemoryCleanser() { memory_cleanse(p, n); } + MemoryCleanser(const MemoryCleanser&) = delete; + MemoryCleanser& operator=(const MemoryCleanser&) = delete; +}; +} // namespace + void clamp_curve25519(unsigned char key[crypto_scalarmult_SCALARBYTES]) { key[0] &= 248; @@ -38,8 +55,10 @@ void PRF_ock( personalization ) != 0) { + memory_cleanse(block, sizeof(block)); // CRY-01 throw std::logic_error("hash function failure"); } + memory_cleanse(block, sizeof(block)); // CRY-01: block held ovk/cv/cm/epk } void KDF_Sapling( @@ -62,8 +81,10 @@ void KDF_Sapling( personalization ) != 0) { + memory_cleanse(block, sizeof(block)); // CRY-01 throw std::logic_error("hash function failure"); } + memory_cleanse(block, sizeof(block)); // CRY-01: block held the DH secret } void KDF(unsigned char K[NOTEENCRYPTION_CIPHER_KEYSIZE], @@ -95,8 +116,10 @@ void KDF(unsigned char K[NOTEENCRYPTION_CIPHER_KEYSIZE], personalization ) != 0) { + memory_cleanse(block, sizeof(block)); // CRY-01 throw std::logic_error("hash function failure"); } + memory_cleanse(block, sizeof(block)); // CRY-01: block held hSig/dhsecret/epk/pk_enc } namespace libzcash { @@ -126,6 +149,7 @@ boost::optional SaplingNoteEncryption::encrypt_to_recipien } uint256 dhsecret; + MemoryCleanser _cleanseDhsecret(dhsecret.begin(), 32); // CRY-01: zeroed on every exit if (!librustzcash_sapling_ka_agree(pk_d.begin(), esk.begin(), dhsecret.begin())) { return boost::none; @@ -133,6 +157,7 @@ boost::optional SaplingNoteEncryption::encrypt_to_recipien // Construct the symmetric key unsigned char K[NOTEENCRYPTION_CIPHER_KEYSIZE]; + MemoryCleanser _cleanseK(K, sizeof(K)); // CRY-01: zeroed on every exit KDF_Sapling(K, dhsecret, epk); // The nonce is zero because we never reuse keys @@ -147,6 +172,7 @@ boost::optional SaplingNoteEncryption::encrypt_to_recipien NULL, cipher_nonce, K ); + already_encrypted_enc = true; return ciphertext; @@ -159,6 +185,7 @@ boost::optional AttemptSaplingEncDecryption( ) { uint256 dhsecret; + MemoryCleanser _cleanseDhsecret(dhsecret.begin(), 32); // CRY-01: zeroed on every exit if (!librustzcash_sapling_ka_agree(epk.begin(), ivk.begin(), dhsecret.begin())) { return boost::none; @@ -166,6 +193,7 @@ boost::optional AttemptSaplingEncDecryption( // Construct the symmetric key unsigned char K[NOTEENCRYPTION_CIPHER_KEYSIZE]; + MemoryCleanser _cleanseK(K, sizeof(K)); // CRY-01: zeroed on every exit KDF_Sapling(K, dhsecret, epk); // The nonce is zero because we never reuse keys @@ -195,6 +223,7 @@ boost::optional AttemptSaplingEncDecryption ( ) { uint256 dhsecret; + MemoryCleanser _cleanseDhsecret(dhsecret.begin(), 32); // CRY-01: zeroed on every exit if (!librustzcash_sapling_ka_agree(pk_d.begin(), esk.begin(), dhsecret.begin())) { return boost::none; @@ -202,6 +231,7 @@ boost::optional AttemptSaplingEncDecryption ( // Construct the symmetric key unsigned char K[NOTEENCRYPTION_CIPHER_KEYSIZE]; + MemoryCleanser _cleanseK(K, sizeof(K)); // CRY-01: zeroed on every exit KDF_Sapling(K, dhsecret, epk); // The nonce is zero because we never reuse keys @@ -237,6 +267,7 @@ SaplingOutCiphertext SaplingNoteEncryption::encrypt_to_ourselves( // Construct the symmetric key unsigned char K[NOTEENCRYPTION_CIPHER_KEYSIZE]; + MemoryCleanser _cleanseK(K, sizeof(K)); // CRY-01: zeroed on every exit PRF_ock(K, ovk, cv, cm, epk); // The nonce is zero because we never reuse keys @@ -251,6 +282,7 @@ SaplingOutCiphertext SaplingNoteEncryption::encrypt_to_ourselves( NULL, cipher_nonce, K ); + already_encrypted_out = true; return ciphertext; @@ -266,6 +298,7 @@ boost::optional AttemptSaplingOutDecryption( { // Construct the symmetric key unsigned char K[NOTEENCRYPTION_CIPHER_KEYSIZE]; + MemoryCleanser _cleanseK(K, sizeof(K)); // CRY-01: zeroed on every exit PRF_ock(K, ovk, cv, cm, epk); // The nonce is zero because we never reuse keys @@ -313,6 +346,7 @@ typename NoteEncryption::Ciphertext NoteEncryption::encrypt ) { uint256 dhsecret; + MemoryCleanser _cleanseDhsecret(dhsecret.begin(), 32); // CRY-01: zeroed on every exit if (crypto_scalarmult(dhsecret.begin(), esk.begin(), pk_enc.begin()) != 0) { throw std::logic_error("Could not create DH secret"); @@ -320,6 +354,7 @@ typename NoteEncryption::Ciphertext NoteEncryption::encrypt // Construct the symmetric key unsigned char K[NOTEENCRYPTION_CIPHER_KEYSIZE]; + MemoryCleanser _cleanseK(K, sizeof(K)); // CRY-01: zeroed on every exit KDF(K, dhsecret, epk, pk_enc, hSig, nonce); // Increment the number of encryptions we've performed @@ -335,6 +370,7 @@ typename NoteEncryption::Ciphertext NoteEncryption::encrypt NULL, 0, // no "additional data" NULL, cipher_nonce, K); + return ciphertext; } @@ -347,12 +383,14 @@ typename NoteDecryption::Plaintext NoteDecryption::decrypt ) const { uint256 dhsecret; + MemoryCleanser _cleanseDhsecret(dhsecret.begin(), 32); // CRY-01: zeroed on every exit if (crypto_scalarmult(dhsecret.begin(), sk_enc.begin(), epk.begin()) != 0) { throw std::logic_error("Could not create DH secret"); } unsigned char K[NOTEENCRYPTION_CIPHER_KEYSIZE]; + MemoryCleanser _cleanseK(K, sizeof(K)); // CRY-01: zeroed on every exit KDF(K, dhsecret, epk, pk_enc, hSig, nonce); // The nonce is zero because we never reuse keys @@ -387,6 +425,7 @@ typename PaymentDisclosureNoteDecryption::Plaintext PaymentDisclosureNoteD ) const { uint256 dhsecret; + MemoryCleanser _cleanseDhsecret(dhsecret.begin(), 32); // CRY-01: zeroed on every exit if (crypto_scalarmult(dhsecret.begin(), esk.begin(), pk_enc.begin()) != 0) { throw std::logic_error("Could not create DH secret"); @@ -396,6 +435,7 @@ typename PaymentDisclosureNoteDecryption::Plaintext PaymentDisclosureNoteD uint256 epk = NoteEncryption::generate_pubkey(esk); unsigned char K[NOTEENCRYPTION_CIPHER_KEYSIZE]; + MemoryCleanser _cleanseK(K, sizeof(K)); // CRY-01: zeroed on every exit KDF(K, dhsecret, epk, pk_enc, hSig, nonce); // The nonce is zero because we never reuse keys diff --git a/src/zcash/Proof.cpp b/src/zcash/Proof.cpp index af87d1b8188..9eb40a500ea 100644 --- a/src/zcash/Proof.cpp +++ b/src/zcash/Proof.cpp @@ -127,6 +127,15 @@ curve_G1 CompressedG1::to_libsnark_g1() const assert(r.is_well_formed()); + // CRY-03: explicitly verify the point is in the prime-order subgroup, + // mirroring the G2 check in to_libsnark_g2(). For alt_bn128 G1 the cofactor + // is 1 (every well-formed point is already in the subgroup), so this rejects + // nothing new today, but it removes the implicit, undocumented dependence on + // that cofactor-1 property and makes the G1/G2 defense posture symmetric. + if (alt_bn128_modulus_r * r != curve_G1::zero()) { + throw std::runtime_error("point is not in G1"); + } + return r; }