fix(zcash): NU6.2 Orchard proof fix + verify-device diagnostic & headless harness#277
Merged
Merged
Conversation
…s device?)
Adds a read-only check that derives the Orchard FVK fresh from the
connected device (silent — no button) and compares its ak to the cached
FVK that owns the shielded balance. Answers 'is this shielded balance
actually spendable by the connected device, or is it stale/another
wallet?' without a send attempt or a DB wipe.
- RPC: zcashVerifyDevice
- REST: GET /api/zcash/shielded/verify-device
→ { match, deviceAk, cachedAk, cachedAddress, message }
match=false means the displayed balance is from a different wallet and
will fail consensus on spend (wrong-signer). Does not mutate state.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…oot guard Two additions to drive + diagnose z→z spends without the UI: 1. POST /api/zcash/shielded/send — full build→sign→finalize→broadcast. On an emulator (autonomous test rig, emu-window auto-approves the confirm) it skips auth so the flow can be driven via curl. Real devices still require a paired bearer token. 2. build_pczt now verifies the orchard-crate MerklePath recomputes the tx anchor BEFORE proving. The proof consumes this converted path, not the ShardTree witness — a conversion/encoding divergence (e.g. for an aged note deep in a completed shard) would bind a wrong anchor and fail consensus with 'could not validate orchard proof'. Now it fails loudly at build time with both roots instead. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The emulator confirm gate waits for a window click (interactive sign). Driven headlessly via /api/zcash/shielded/send the confirm was never clicked → 'rejected by user'. Add a scoped auto-approve flag (setEmuAutoApprove) the REST send enables on an emulator so the full build→sign→finalize→broadcast runs via curl with no manual clicks. Tightly scoped: set only around the send, emulator-only, finally-reset. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The shield path verifies the proof in-process before broadcast (catches the exact 'could not validate orchard proof' chain rejection with the real halo2 error). The z→z spend path skipped this, so the only signal was the opaque consensus error. Mirror it: verify_proof(&vk) after binding sig. A local FAIL pinpoints the proof as the bug (vs serialization) and yields the underlying constraint error for an aged deep-shard spend. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ze_pczt verify_proof() only checks the zk proof (always passes for a self-consistent bundle). Add the orchard BatchValidator — the exact consensus check (proof + spend-auth sigs + binding sig) — run with the signing sighash AND with the sighash recomputed from the final tx. Outcomes: - FAIL on signing sighash → reproduces the chain rejection locally (proof/ sig/binding bug), narrowable from here. - signing != consensus sighash → the device signed a stale digest (the bug). - PASS on both but chain still rejects → consensus STATE (already-spent nullifier / unknown anchor), i.e. a public-seed note spent elsewhere — not a code bug. Diagnostic logging only; does not change the tx. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ld/deshield endpoints - build_deshield_pczt had the same insert-roots-then-append-note-shard ordering bug as build_pczt (pre-#273): spending an aged note in a deep completed shard failed with 'Failed to get root'. Apply the same ascending-order construction. - Add POST /api/zcash/shielded/{deshield,shield} headless endpoints (emulator: auto-approve + no auth) so the full t→z / z→t flows can be driven via curl for testing with fresh funds. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
handle_broadcast only hit na.zec.rocks (fallbacks were connect-only). A valid tx (BatchValidator passes, note unspent, anchor verified) rejected by one node may be node-specific (policy / reported-branch-id mismatch). Broadcast to every node (na/sa/eu zec.rocks + mainnet.lightwalletd) and log each accept/reject. If another node accepts what na.zec.rocks rejects, the bug is the node, not our transaction. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…not validate orchard proof" Mainnet hard-forked to NU6.2 on 2026-06-03 (branch 0x5437f330) to fix an Orchard circuit soundness bug, shipping a new verifying key (FixedPostNu6_2). The sidecar built proofs with orchard 0.12, which produces pre-fork proofs: they pass our local verify_proof/BatchValidator but every current Zebra/zcashd node rejects them as "could not validate orchard proof". Local verification cannot distinguish pre- from post-fork proofs, so the only signal was the opaque consensus rejection. Upgrade the whole NU6.2 crate cohort (all published 2026-06-03; every prior version is yanked): orchard 0.12 -> 0.14 (FixedPostNu6_2 circuit + halo2_gadgets 0.5) zcash_primitives 0.19 -> 0.28 zcash_protocol 0.7 -> 0.9 zcash_address 0.10 -> 0.12 zcash_keys 0.8 -> 0.14 API fixups required by the bump: - scanner: Action::from_parts now returns Result; skip malformed actions. - pczt_builder: disambiguate epk_bytes slice; rebuild the test Authorized bundle via EffectsOnly::from_parts + map_authorization (0.14 dropped the public Authorized from_parts); use TxOut value()/script_pubkey() accessors. - drop the orchard 0.10 / zcash_protocol 0.4 dev-dep aliases (they only matched the old 0.19 pin); point the BatchValidator doc-test at the main crates. Verified on mainnet: z->z send confirmed in tx 9e699c9b09320a16e1cef40f31655b8d42c386c030b4da52c63fadcc708d896e. Release build clean; 61/61 unit tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
BitHighlander
changed the title
… zatoshi validation, doomed-tx abort Four review findings on the headless REST send/shield/deshield paths: - amount is ZATOSHIS, not ZEC. The value goes straight to the sidecar's build_pczt (u64), so the old "(number, ZEC)" doc was wrong: 0.01 would be rejected and `1` sends 1 zatoshi. Correct the doc and validate positive integers on all three endpoints. - The headless paths skipped the RPC flow's fail-closed preflight, so a stale sidecar DB or a device swap could build from old notes and fail late after signing (ensureFvkLoaded no-ops when any cached FVK exists). Add a shared zcashPreSendGate callback (ensureZcashDeviceMatch + ensureZcashScanFresh) and run it before signing on send/shield/deshield. Missing gate => 503. - deshield loaded account 0 but passed body.account downstream, so a non-zero account could init the sidecar on 0 while the device signs N. Resolve `account = body.account ?? 0` once and use it consistently (send/deshield/ shield), threading it into the builder calls. - finalize_pczt's BatchValidator consensus check only logged FAIL and kept going, broadcasting a doomed tx. Now returns an error on !ok_signing and on any signing-vs-consensus sighash divergence, before serialize/broadcast. Sidecar release build clean; 61/61 tests pass. TS typechecks (pre-existing minimatch type-def error unrelated). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…every send, emulator included The headless zcash REST endpoints (send/shield/deshield/verify-device) shipped two bypasses scoped to `engine.isEmulator`, for autonomous curl-driven testing: 1. auth skip — no bearer token required when an emulator was active 2. setEmuAutoApprove — auto-pressed every device confirm with no user click Per review, remove both outright. Even on the emulator we want the UI to gate. - Every endpoint now calls auth.requireAuth(req) unconditionally (no `if (!engine.isEmulator)`). - Delete the autoApproveAll global + setEmuAutoApprove export; emuGatedConfirm only auto-presses non-interactive SETUP ops (wipe/load/settings). All signing ops are interactive:true and route through requestUserConfirm, so the user must approve in the UI. The remaining engine.isEmulator use is emuWrap's loop-freeze workaround (routes signing through the interactive confirm), not a security bypass. Net: no path can move funds without a paired token AND an explicit approval. TS typechecks (pre-existing minimatch type-def error unrelated). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…hVerifyDevice RPC Two review findings on account handling: - HIGH: the new REST send/shield/deshield routes accept `account`, but the Zcash sidecar/FVK/scan state is global. ensureFvkLoaded only checks "any FVK loaded" and ensureZcashDeviceMatch short-circuits once ANY account has set zcashDeviceVerified, so a request with account:1 would silently build from account 0's cached state. Until the state is genuinely account-scoped, reject account != 0 with a 400 on all three routes. (verify-device already pins account 0, so it's now consistent — the Low false-mismatch case can no longer arise.) - MEDIUM: the zcashVerifyDevice RPC handler had no entry in the shared RPC schema. Declare its params/response shape. TS typechecks (pre-existing minimatch type-def error unrelated). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the "could not validate orchard proof" failure on Zcash shielded sends (z→z and deshield), plus the diagnostic + headless test harness built while tracking it down.
Root cause (now fixed)
Mainnet hard-forked to NU6.2 on 2026-06-03 (branch
0x5437f330), an emergency fork fixing an Orchard circuit soundness bug. It retired the old verifying key (InsecurePreNu6_2→FixedPostNu6_2). The sidecar built proofs withorchard 0.12, which produces pre-fork proofs: they pass our localverify_proof/BatchValidatorbut every current Zebra/zcashd node rejects them. Local verification cannot distinguish pre- from post-fork proofs, so the only signal was the opaque consensus rejection.Fix: upgrade the whole NU6.2 crate cohort (all published 2026-06-03; every prior version yanked):
Firmware needs no change — it signs sighashes; the proof is 100% sidecar-side.
Also in this PR (diagnostic + harness work)
build_pcztandbuild_deshield_pczt(fixes "Failed to get root" on aged deep-shard notes)MerklePathroot guardBatchValidator+ sighash-divergence instrumentation infinalize_pczt/send,/shield,/deshield,/verify-device) + auto-approve, so a full send is curl-drivable with no UIverify-devicediagnostic: does the shielded balance belong to this device?setTxVersionTest plan
cargo build --releaseclean; 61/61 unit tests pass (serializer/txid canaries round-trip throughzcash_primitivesreference reader on the new stack)9e699c9b09320a16e1cef40f31655b8d42c386c030b4da52c63fadcc708d896e