feat(wasm-utxo): add orchard (no-circuit) dep + Ironwood PCZT serialization

[veetragjain]

Introduce the orchard crate WITHOUT the circuit feature (Sinsemilla / pasta / note-encryption only — no halo2 prover) plus rand, postcard, and ff, and add the zcash::ironwood_pczt module that (de)serializes an orchard::pczt::Bundle.

orchard::pczt::Bundle is not serde-serializable, so this bridges it to a compact wire form: a 1-byte format version + a postcard-encoded mirror struct whose fields are exactly what orchard's Bundle/Action/Spend/ Output::parse(...) consume. Serialize reads the bundle's public getters; deserialize reconstructs via parse(...). This is the witness payload exchanged with the external Ironwood proof service and carried through the PSBT (proving is delegated; no halo2 in wasm-utxo).

Notes:

• >32-byte fields are stored as Vec<u8> (serde derives arrays only up to length 32) and length-checked on the way back in.
• note_version is derived from the bundle version rather than carried on the wire; zip32_derivation (wallet metadata the prover doesn't need) is dropped.
• serde is now a non-optional dependency (the module derives Serialize/Deserialize); the inspect feature drops its dep:serde.

Verified: compiles for wasm32-unknown-unknown and native; a build ->
serialize -> deserialize -> re-serialize round-trip is byte-stable.

[OttoAllmendinger]

Review

Scope: the GitHub diff here also shows the v6.rs signature-digest block, but that's inherited from #338 (this PR's base) and was already reviewed there — the #338 empty-inputs fallback finding remains the stack root. This review covers only what #339 adds: ironwood_pczt.rs, the Cargo.toml/Cargo.lock changes, and mod.rs.

Overview

Adds zcash::ironwood_pczt, a compact wire codec bridging the non-serde orchard::pczt::Bundle to a postcard-encoded mirror struct (1-byte FORMAT_VERSION + body), used both as the external proof-service payload and the PSBT carry. Also brings orchard in without the circuit feature (no halo2 prover) and makes serde non-optional. Clean, well-documented, and the default-features = false on orchard to avoid the heavy prover is exactly right.

Good choices worth calling out: BTreeMap for proprietary (deterministic ordering → stable bytes), the FORMAT_VERSION gate with explicit rejection tests, and deriving note_version from the bundle version rather than trusting it on the wire.

Questions / issues

1. (Security — please confirm) Does bsk need to cross to the external prover?
BundleWire.bsk (the binding-signature signing key) is serialized into the same wire form the module docstring says is "exchanged with the external Ironwood proof service." Per the stack (#341), bsk is produced by finalize_shield_io (local) and consumed by combine to apply the binding signature (local) — the Prover role doesn't need it. If the finalized PCZT is shipped to the proof service with bsk populated, a binding-signature secret leaves the signing boundary and is disclosed to that service. If the intent is that bsk is only ever present on the PSBT-carry path and stripped before the prover call, that's fine — but the single bidirectional format makes that easy to get wrong. Can you confirm the prover payload is built with bsk = None (or add a note/assertion enforcing it)? Same question, lower stakes, for dummy_sk — though the prover legitimately needs spend-witness material, so that one is likely expected.

2. (Robustness) enc_ciphertext (580) / out_ciphertext (80) aren't length-checked like their siblings.
The module explicitly length-validates spend_auth_sig/recipient/fvk/auth_path via arr::<N> on the way back in, but enc_ciphertext and out_ciphertext are passed straight into PcztOutput::parse as Vec<u8>. If parse validates their lengths, fine — but the asymmetry is surprising given the module's stated ">32-byte fields are length-checked" contract. Either add explicit arr::<580> / arr::<80> checks or drop a comment noting parse enforces them, so a malformed payload fails with a precise BadLength rather than an opaque Parse error (or worse).

3. (Test coverage) bundle_roundtrip_is_byte_stable proves idempotence, not losslessness.
The test round-trips serialize→deserialize→serialize and asserts byte-equality — but a field that bundle_to_wire silently drops (and that parse reconstructs to the same default) would still pass, since it's absent in both encodings. Consider also asserting a few decoded getters against known values from sample_pczt() (anchor, flags, action count, output cmx), so an accidentally-dropped field is actually caught. Right now the strongest correctness guarantee is deferred entirely to the #341 golden oracle.

Minor

• postcard (default features) pulls heapless → atomic-polyfill + critical-section into a wasm crate. You've verified it compiles for wasm32-unknown-unknown, so this is informational — but postcard with default-features = false, features = ["use-std"] may trim the heapless/atomic-polyfill chain if you'd rather not carry it.
• FORMAT_VERSION rejection and empty-input paths are tested — nice. A malformed-body (postcard decode failure → Codec) test would round out the error-path coverage cheaply.

Overall this is solid, careful work; #1 is the only one I'd consider blocking, and only pending your confirmation on the prover payload.

---

Generated by Claude Code

[veetragjain]

1. (Security — please confirm) Does bsk need to cross to the external prover?
   BundleWire.bsk (the binding-signature signing key) is serialized into the same wire form the module docstring says is "exchanged with the external Ironwood proof service." Per the stack (feat(wasm-utxo): add Ironwood PCZT constructor/combine bridge #341), bsk is produced by finalize_shield_io (local) and consumed by combine to apply the binding signature (local) — the Prover role doesn't need it. If the finalized PCZT is shipped to the proof service with bsk populated, a binding-signature secret leaves the signing boundary and is disclosed to that service. If the intent is that bsk is only ever present on the PSBT-carry path and stripped before the prover call, that's fine — but the single bidirectional format makes that easy to get wrong. Can you confirm the prover payload is built with bsk = None (or add a note/assertion enforcing it)? Same question, lower stakes, for dummy_sk — though the prover legitimately needs spend-witness material, so that one is likely expected.

BSK is required by the combiner i.e. IMS, when IMS calls the KMS(proof generation service), it will not include bsk in the request, and even if it did bsk is not a secret to KMS, KMS can recompute it by aggregating rcv.

2. (Robustness) enc_ciphertext (580) / out_ciphertext (80) aren't length-checked like their siblings.
   The module explicitly length-validates spend_auth_sig/recipient/fvk/auth_path via arr:: on the way back in, but enc_ciphertext and out_ciphertext are passed straight into PcztOutput::parse as Vec. If parse validates their lengths, fine — but the asymmetry is surprising given the module's stated ">32-byte fields are length-checked" contract. Either add explicit arr::<580> / arr::<80> checks or drop a comment noting parse enforces them, so a malformed payload fails with a precise BadLength rather than an opaque Parse error (or worse).

It does enc_ciphertext.as_slice().try_into() → ParseError::InvalidEncCiphertext / InvalidOutCiphertext. So a wrong length does not pass silently — but it surfaces as our opaque IronwoodPcztError::Parse("InvalidEncCiphertext") instead of the typed BadLength(field, 580, actual). enc_ciphertext/out_ciphertext are now length-checked with arr::<580>/arr::<80> in wire_to_output, giving BadLength instead of the opaque Parse

3. (Test coverage) bundle_roundtrip_is_byte_stable proves idempotence, not losslessness.
   The test round-trips serialize→deserialize→serialize and asserts byte-equality — but a field that bundle_to_wire silently drops (and that parse reconstructs to the same default) would still pass, since it's absent in both encodings. Consider also asserting a few decoded getters against known values from sample_pczt() (anchor, flags, action count, output cmx), so an accidentally-dropped field is actually caught. Right now the strongest correctness guarantee is deferred entirely to the feat(wasm-utxo): add Ironwood PCZT constructor/combine bridge #341 golden oracle.

Added value assertions (anchor, flags, action count, value_sum sign/magnitude, non-default cmx) decoded from the actual wire struct, so a symmetric drop/default bug would now fail.

[veetragjain]

Minor
postcard (default features) pulls heapless → atomic-polyfill + critical-section into a wasm crate. You've verified it compiles for wasm32-unknown-unknown, so this is informational — but postcard with default-features = false, features = ["use-std"] may trim the heapless/atomic-polyfill chain if you'd rather not carry it.
FORMAT_VERSION rejection and empty-input paths are tested — nice. A malformed-body (postcard decode failure → Codec) test would round out the error-path coverage cheaply.

postcard: default-features = false — drops heapless/atomic-polyfill/spin entirely and added rejects_malformed_body test (valid version byte + truncated postcard body → Codec(_))

[OttoAllmendinger]

Re-review (rev 29230c2)

All three findings from the first pass are resolved, including both minors.

• ✅ #2 — ciphertext length checks. wire_to_output now length-validates both variable fields (arr::<580>(&o.enc_ciphertext, …)?, arr::<80>(&o.out_ciphertext, …)?) before PcztOutput::parse, matching the treatment of the other >32-byte fields. A malformed payload now fails with a precise BadLength(field, exp, act) instead of an opaque Parse.
• ✅ #3 — test now proves losslessness, not just idempotence. bundle_roundtrip_is_byte_stable additionally decodes the wire and asserts against sample_pczt()'s known values (anchor, flags, action count, value sum + sign, non-zero cmx), so a field silently dropped in both directions would be caught.
• ✅ #1 — bsk. Resolved by the flow (confirmed on #342): the prover payload is the Constructor PCZT with bsk = None, and bsk only round-trips locally inside combine_ironwood_proof. The Option<bsk> field on BundleWire is correct as-is — no change needed.
• ✅ minors. postcard is now default-features = false, features = ["use-std"], which drops the heapless/atomic-polyfill/critical-section chain (confirmed gone from Cargo.lock); and a rejects_malformed_body test rounds out the error-path coverage.

The deny.toml additions (skip the transitive thiserror 1.x/2.x duplicate from reddsa/redjubjub vs postcard's cobs; allow BSD-2-Clause) are reasonable and clearly commented — no concern.

LGTM from my side. Leaving formal approval to @OttoAllmendinger.

---

Generated by Claude Code