Skip to content

feat(zcash): PCZT signing flow + Keystone watch-only support#11

Merged
hitchho merged 10 commits into
mainfrom
feat/pczt-keystone-migration
Jun 7, 2026
Merged

feat(zcash): PCZT signing flow + Keystone watch-only support#11
hitchho merged 10 commits into
mainfrom
feat/pczt-keystone-migration

Conversation

@hitchho

@hitchho hitchho commented May 3, 2026

Copy link
Copy Markdown

Summary

  • Migrates Zcash cold-signing from the legacy [sighash][alphas][summary] simple format to the canonical PCZT format, closing the display↔sighash decoupling: the cold device recomputes the sighash from PCZT contents itself, so what the user sees on-screen is bound to what gets signed by construction.
  • Adds Keystone hardware wallet support as a watch-only Zcash cold signer paired with zafu — same byte format as zigner, distinct UI/onboarding affordance.
  • Pre-PCZT housekeeping fixes (clear-cache deferral + zitadel async build fix) batched in a separate first commit so the feature commit stays focused.

Companion PR (must land first): rotkonetworks/zcli#6 ships the Rust wasm code that produces the artifacts in this PR.

Why this matters

Today's flow ships signZcashSimple to zigner: a sighash, per-action alphas, and a free-text summary string. The summary is a display hint that isn't cryptographically tied to the sighash — a compromised zafu can show "send 1 ZEC to alice" while shipping a sighash that commits to "send 999 ZEC to attacker". User approves on the cold device's screen, real bytes signed.

The PCZT path closes that. Zigner (and Keystone) recompute the shielded sighash via v5_signature_hash(pczt_to_tx_data(global, transparent, sapling, orchard)) — derived from the PCZT bytes themselves. Display and signed bytes are bound by construction.

What lands

PCZT signing pipeline

  • New worker handlers send-tx-pczt / send-tx-pczt-complete.
  • AnimatedQrScanner extended with BC-UR fountain mode (auto-detected from first frame's prefix) feeding ur_decode_frames for multi-frame reassembly.
  • Animated UR fragment-size now configurable (default 200 B/frame; tuneable for hardware-specific cameras).
  • Real chain-tip threaded into target_height instead of a hardcoded value — branch_id derivation is correct on testnet.

Keystone integration

  • New "Scan Keystone (animated UR, zcash only)" button on onboarding.
  • New coldSignerType: 'zigner' | 'keystone' discriminant on zcashWallets (storage v2 schema). User's button click is source of truth; byte heuristic kept only as a consistency check.
  • Stable Keystone deviceId derived from UFVK hash so re-imports dedup correctly.

Security guards

  • UFVK validated structurally on import (HRP prefix + bech32m alphabet + length bounds). Malformed ur:zcash-accounts payloads now throw at parse time instead of corrupting the wallet store silently.
  • Pure-byte CBOR {1: bytes} envelope helpers extracted into their own module + Node-runner tests pin the round-trip across all four CBOR length-prefix paths (tag 0x40, 0x58, 0x59, 0x5a).

Tests

  • 18 Node test runs across 3 files (CBOR envelope round-trip, fragmentSize threading, UFVK validator structural checks).
  • All security-relevant Rust tests live in the companion zcli PR (sighash invariance under redaction, Signer::new acceptance after redaction, full prove+sign+extract round-trip).

Test plan

  • pnpm exec tsc --noEmit — typecheck clean (modulo pre-existing unrelated noise in send/index.tsx).
  • npx webpack --config webpack.prod-config.ts — both browser and worker bundles compile.
  • node --test — 18 passing.
  • On-device test against real zigner running latest firmware. Most likely failure modes if any: (1) parallel wasm fails to load build_unsigned_pczt in offscreen prover, (2) animated UR density too aggressive at 200 B/frame for some camera, (3) PCZT bytes parse-rejected by zigner — the redactor strips fields zigner expects.
  • On-device test against real Keystone. Hardware required; the integration is byte-format compatible by construction (same canonical pczt::Pczt as zashi/keystone-sdk consume) but untested in practice.
  • After hardware passes: drop a zashi-emitted reference PCZT into zcli tests/fixtures/ to activate the #[ignore]'d interop snapshot test.

Deferred (call-outs for reviewer)

  • FROST flows still use simple format. PCZT-FROST integration is a follow-up (the standard pczt::Signer is single-signer-only; we'd need a custom round protocol to extract sighash+alpha for the threshold signing).
  • No UI gating on coldSignerType yet. The discriminant lands in storage but no screen hides anything based on it. Speculative gates aren't useful before we know which features matter to gate. One-line if (wallet.coldSignerType === 'keystone') hide per screen later.
  • PCZT redactor's drop-list is conservative. We strip witness, zip32_derivation, dummy_sk, proprietary. We leave alpha, fvk, value, recipient, nullifier, zkproof, bsk untouched — the cold signer needs each. If Keystone rejects PCZTs at our current redaction level, the trade-off list lives in zcli/crates/zcash-wasm/src/lib.rs::redact_pczt_for_signer.

🤖 Generated with Claude Code

hitchho and others added 8 commits May 3, 2026 20:15
… async)

Two unrelated fixes that landed during the PCZT migration session and got
batched here so the main feature commit stays clean. Both are independent
bug fixes that improve correctness on flows orthogonal to PCZT signing.

clear-cache deferral
--------------------
The Penumbra clear-cache flow was failing silently. The service worker held
an open IndexedDB connection while attempting to delete the same DB, and the
unsafe close cast `(indexedDb as { db: { close } }).db?.close?.()` didn't
reliably close the handle — so deleteDatabase fired `onblocked`, our handler
silently resolve()'d, and the SW reload re-seeded fullSyncHeight from the
intact IDB. User saw "syncing 93%" right after clear cache, exactly because
the cache was never cleared.

Fix: defer the actual IDB delete to next SW startup. Set a `pendingClearCache`
flag in local storage, reload, then on `initHandler()` boot delete the DBs
*before* any wallet services open new connections (so onblocked can't fire).

- New `clear-cache-startup.ts` runs at SW init and processes the flag.
- `internal-services.ts` `clearPenumbraCache` no longer attempts in-process
  deletion; it just sets the flag and triggers reload.
- `service-worker.ts` awaits `performPendingClears()` before
  `startWalletServices`.
- `storage-chrome` v2 schema gains optional `pendingClearCache?: ('penumbra' | 'zcash')[]`.

zitadel async keydown + DM channel types
----------------------------------------
The slash-command dispatcher inside `inp.addEventListener('keydown', ...)`
called `await navigator.clipboard.writeText`, `await runLogin()`, and
`await Notification.requestPermission()` — but the listener wasn't `async`,
so prod builds failed `Module parse failed: Cannot use keyword 'await'
outside an async function`. The listener returns void anyway; making it
async is cosmetic at the runtime level but unblocks the build.

Same commit also drops an unused `dmMessages()` helper and tightens
`openDmChannel`'s return type from `Option<…|null>` to `Option<…|undefined>`
(matches the `Map.get()` convention every call site already used).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Migrates zafu's Zcash cold-signing path from the legacy [sighash][alphas][summary]
"simple" QR format to the canonical Partially Created Zcash Transaction (PCZT)
format. Closes the display↔sighash decoupling that the simple format had: the
cold device now recomputes the sighash from PCZT contents itself, so what the
user sees on the zigner/Keystone screen is bound to what gets signed by
construction. A compromised hot wallet can no longer ship a sighash for one
tx while displaying another.

The same byte format is consumed by zashi-android and keystone-sdk, so this
PR also opens the door to Keystone hardware wallets as a watch-only Zcash
cold signer paired with zafu — the flip side of zigner without buying our
hardware.

What lands
==========

PCZT signing pipeline
---------------------
- Worker handlers `send-tx-pczt` / `send-tx-pczt-complete` in zcash-worker.
  The unsigned-build phase calls the new wasm `build_unsigned_pczt` which
  goes through Builder → Creator → Prover → IoFinalizer → redactor →
  serialize. Result is CBOR-wrapped {1: bytes} and UR-encoded as
  `ur:zcash-pczt/...` for animated QR transport. Density configurable
  (default 200 B/frame; drop to 150 for Keystone-class cameras, raise to
  400 on flagship Pixels).
- `extract_signed_tx_from_pczt` on the receive side runs TransactionExtractor
  over the signed PCZT; no more manual signature-offset patching.
- AnimatedQrScanner extended to detect BC-UR fountain mode from the first
  scanned frame's prefix and accumulate parts via the wasm `ur_decode_frames`
  decoder. Single-frame UR works through the same path.
- Real chain-tip is threaded into `target_height` from the merkle-witness
  fetch instead of being hardcoded — branch-id derivation is correct on
  testnet where activation heights diverge from mainnet.

Keystone integration
--------------------
- New "Scan Keystone (animated UR, zcash only)" button in onboarding, distinct
  from the static-QR Zigner scan path. Routes through AnimatedQrScanner with
  `urTypeFilter: 'zcash-accounts'` and parses CBOR via the new
  `parseZcashAccountsCbor`. Different scanner, different state action, same
  underlying watch-only wallet record.
- New `coldSignerType: 'zigner' | 'keystone'` discriminant on zcashWallets
  (storage v2). The user's button click is the source of truth; we keep a
  byte-level heuristic (zid_pubkey presence) only as a consistency-warning
  check — flagged in console if declaration disagrees with bytes, but the
  declaration wins. UI gating on this field is a follow-up; the schema
  field lands now so future feature gates have a stable substrate.
- Keystone deviceId derived from a DJB2 hash of the UFVK so reimporting
  the same wallet dedups against itself, instead of getting a fresh
  timestamp-based id every time.

Security guards
---------------
- UFVK validated structurally on import (HRP prefix + bech32m alphabet +
  length bounds). Stops malformed `ur:zcash-accounts` payloads writing
  garbage into the wallet store; failure surfaces in the import flow rather
  than at first-send time.
- Pure-byte CBOR `{1: bytes}` envelope helpers in their own module so
  unit tests exercise them without the rest of the extension state. Two
  Node-runner test suites (CBOR round-trip + fragment-size threading)
  catch format drift between emitter and parser.

WASM artifacts
--------------
Both single-thread (apps/extension/packages/zcash-wasm/) and parallel rayon
(apps/extension/public/zafu-wasm-parallel/) artifacts rebuilt with the new
exports. Size went 5.3 MB → 6.2 MB due to sapling-crypto + pczt overhead;
acceptable.

Companion PR: rotkonetworks/zcli#6 ships the Rust-side wasm code.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three small things:

1. Content scripts (injected-session.ts, injected-penumbra-global.ts):
   reject the literal 'invalid' that chrome.runtime.id returns for
   orphaned scripts (extension reloaded while a tab was already open).
   Without this the orphan would inject
   chrome-extension://invalid/manifest.json into window[PenumbraSymbol]
   and break wallet-picker manifest fetches in penumbra-web.

2. penumbra-swap-claim.ts: classify ConnectRPC '[unavailable]' /
   'Connection closed' errors as transient port-recycle events and
   log them at debug level instead of error. The 30s retry tick
   handles them — there's no real failure to surface.

3. routes/popup/home/cosmos-subwallets.tsx (NEW): renders the user's
   unshielded Cosmos balances (Noble, Cosmos Hub) as sub-wallets
   under the Penumbra view. Uses the existing useAllCosmosBalances
   hook; auto-hides when the user has zero balances. Lazy-loaded so
   non-Penumbra views don't pay the chunk.

Also added an ADR at docs/adrs/cosmos-as-penumbra-subwallets.md
covering the broader plan: web-injection of a Cosmos provider,
cosmos-kit adapter, balance/sign data flow, file-by-file work.
Includes the orphan chrome.runtime.id === 'invalid' guard in both
content scripts (commit 9aace42c) so reloading the extension in one
tab no longer poisons window[PenumbraSymbol] in other open tabs.

Build artifact: /tmp/zafu-beta-24.1.3.zip ready for Web Store upload.
buildWitnesses anchored at the live tip, where zidecar replicas diverge and
reorgs happen, across 3 unsynchronized RPCs → intermittent "tree root
mismatch". Decouple: anchorHeight = tip - ANCHOR_CONFIRMATIONS(3); keep
target_height at the live tip for branch_id/expiry. Orchard anchors are
historical roots with no max-age, so a slightly-behind anchor is fully valid.

precise `sibling i/j not canonical: <hex>` error instead of a lossy generic
count error. (zcli commit 2038eb6e.)

zcash_keys decode (new wasm `validate_ufvk`) at the persistence boundary in
wallet-entries.ts, before any record is written. Throws loud on a bogus
UFVK instead of deferring to first-send and poisoning dedup. @repo/wallet
stays structural-only and wasm-free.

used signed `<<` → high-bit length went negative, bypassed the bounds
guard, returned a silently-truncated PCZT. Rewrote with *256 accumulation,
per-byte bounds, exact-consume canonical check. TDD: 3 RED adversarial
cases → 12/12 green.

without awaiting the bindgen default() initializer (same class of bug Al
fixed in the scanner) — silently degraded contact-card privacy. Added
await default().

"dead/corrupt stream" → unbounded "scanning...". Added a 12s
no-new-unique-part watchdog that surfaces a hard error via onError.

zero binding to a zcli source rev. Added BUILD_PROVENANCE.md pinning the
exact zcli commit + per-artifact sha256 + reproduction commands. Both
single-thread and parallel artifacts rebuilt from zcli 2038eb6e.

Not done (documented, needs human decision / hardware, NOT silently shipped):
- Behavioral merkle-rejection test — needs notes+path fixture infra that
  doesn't exist; would be a contorted box-tick. Tracked as the remaining
  coverage gap (matches QA/redshiftzero).
- #7 commit hygiene — branch is shared with Al; rewriting history is a
  coordination hazard. Disposition: curate the squash message at merge and
  split the unrelated release commits into their own PR off main. Process
  instruction, not an autonomous shared-history rewrite.
- The #1b anchor fix MUST be re-validated on hardware before the merkle
  bug is claimed resolved.

zafu prod build green (browser + worker). zcli tests green.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@hitchho
hitchho force-pushed the feat/pczt-keystone-migration branch from 67b8c24 to 885d030 Compare May 17, 2026 02:14
hitchho added a commit that referenced this pull request Jun 1, 2026
… tests

Two reviewer-flagged blockers on zafu PR #11, fixed in-tree on staging so
tomorrow's hardware probe runs against a known-good integration:

1. **UR fountain accumulator was unbounded.** A hostile or buggy QR
   source emitting an endless stream of unique-but-junk frames bypassed
   the stall watchdog (each frame looked healthy because it was *new*)
   and the Set grew until tab OOM. Cap at 4096 unique parts + 2048 B
   per frame. Hitting the cap surfaces a hard error via `setError(...)`
   rather than continuing to accept frames.

2. **`*.node.test.mjs` files were being picked up by vitest.** They use
   `node --test` semantics (no Vitest globals, no jsdom) and Vitest's
   `tests-setup.ts` chain imports `navigator.locks` which dies in jsdom
   with `Cannot read properties of undefined (reading 'getItem')`.
   Exclude them from vitest discovery so they run via
   `pnpm exec node --test packages/wallet/src/*.node.test.mjs` (passes
   8/8 there). `wallet.test.ts` still fails for the same navigator.locks
   reason — that failure is pre-existing on `origin/main`, not
   introduced by this PR.

STAGING-ONLY commits. Both fixes should land on the original PR branch
(`feat/pczt-keystone-migration`) before any `main` merge.

Assisted-by: Claude:claude-opus-4-7
hitchho added a commit that referenced this pull request Jun 1, 2026
Previous version (in the same staging push) only setError'd and return'd
on cap exhaustion — the camera + ZXing pipeline kept running, every new
unique frame re-entered the branch and re-fired setError. The stall
watchdog (12s) already had the right shape (latch completedRef → call
stopScanning → invoke onErrorRef); the cap branch now mirrors it so the
abort is symmetric.

Defends against a hostile source that emits a sustained stream of
unique-but-junk frames: stall watchdog can't catch it (frames are new
every time), and the cap is what stops it — but only if the cap branch
actually halts the pipeline.

STAGING-ONLY commit; should land on the original PR #11 branch before
main merge.

Assisted-by: Claude:claude-opus-4-7
@hitchho
hitchho merged commit 9e60dea into main Jun 7, 2026
1 of 3 checks passed
hitchho added a commit that referenced this pull request Jun 7, 2026
…ightwalletd, DKG external API, security review pass

Includes:
- PCZT signing flow + Keystone watch-only support (PR #11)
- Mempool-watch backend (PR #16, hdevalence-hardened, off by default + zidecar gate)
- Modular memo-sync service + filter pattern (PR #14)
- DKG external API (zafu_dkg_join), joiner-side PCZT co-signing (zafu_frost_sign_orchard)
- vault-unlock gate before runPokerSign + runSign FROST share decrypt
- UR scanner caps (4096 parts / 2KiB each) + stall watchdog + wasm-decoded type filter
- Parallel wasm rebuild with shared imported memory
- Security review pass: multisigLabel sanitization, runSign auth gate, lightwalletd response-size caps, reconnect bound
- zafu_frost_sign_orchard disabled at build-time pending display↔sighash binding (see f3efd94)

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants