feat(wasm-utxo): add ZIP-244 v6 signature digests (shielded + transparent) - #338
Conversation
OttoAllmendinger
left a comment
There was a problem hiding this comment.
Review
Overview
Adds two ZIP-244 §S.2 signature-hash computations to the v6 (Ironwood) codec: compute_v6_sig_digest (shielded SIGHASH_ALL, the message the binding signature signs) and compute_v6_transparent_sighash (per-input transparent SIGHASH_ALL). Both share a transparent_sig_digest_with_txin core that differs only in the per-input (txin) sub-digest. Well-scoped, single file, no new deps.
The construction is largely correct and matches the zcash_primitives reference on the points that matter most:
- Personalizations
ZTxTrAmountsHash/ZTxTrScriptsHash/Zcash___TxInHashare byte-exact. - Outer S.2 ordering (
hash_type ‖ prevouts ‖ amounts ‖ scripts ‖ sequence ‖ outputs ‖ txin) is correct. - The sig digest reuses the
ZcashTxHash_+branch-id outer personalization — correct; the reference shares that prefix between txid and sighash. Array-style (count-less) amounts/scripts encoding vs. plain concatenation for prevouts/sequence/outputs is right.- The golden test verifying a real external ECDSA signature against the computed digest is excellent — strong, independent proof of byte-correctness for the transparent shield path.
Issues
1. (Correctness — blocking) Empty-inputs fallback is too broad; wrong digest for the unshield case.
if inputs.is_empty() {
return transparent_txid_digest(tx);
}Per ZIP-244, the sig transparent digest collapses to the txid transparent digest only when the transaction has no transparent inputs AND no transparent outputs. With transparent outputs but no inputs (the unshield flow: shielded spend → transparent output), the reference still computes the full S.2 structure:
BLAKE2b(ZTxIdTranspaHash, hash_type ‖ prevouts(∅) ‖ amounts(∅) ‖ scripts(∅) ‖ sequence(∅) ‖ outputs ‖ txin(∅))
whereas transparent_txid_digest returns BLAKE2b(ZTxIdTranspaHash, prevouts(∅) ‖ sequence(∅) ‖ outputs) — no hash_type, no amounts/scripts/txin components. These differ whenever outputs is non-empty, so compute_v6_sig_digest would produce the wrong binding-signature message for an unshield transaction.
Suggested guard:
if inputs.is_empty() && tx.transparent.output.is_empty() {
return transparent_txid_digest(tx);
}The existing shielded_sig_digest_without_inputs_equals_txid test passes only because sample_transparent(false) has both empty inputs and outputs, so this path is untested. Recommend adding a case with outputs-but-no-inputs.
2. (Robustness) Silent miscomputation on mismatched slice lengths.
transparent_sig_digest_with_txin iterates input_amounts / input_script_pubkeys directly and never checks they line up with tx.transparent.input. If a caller passes fewer/more amounts or scripts than there are inputs, the digest is silently computed over the wrong commitment set rather than erroring — a hard-to-diagnose signing failure. A debug_assert! or an explicit length check against inputs.len() would fail fast. (The per-input path guards input_amounts.get(input_index) but not the aggregate lengths.)
Minor
compute_v6_transparent_sighashreturnsZcashV6Error::UnexpectedEoffor both an out-of-rangeinput_indexand a too-shortinput_amounts.UnexpectedEofreads oddly for a caller-argument error; a dedicated variant would be clearer, though reusing the existing one is pragmatic.i64amounts internally are fine here (ZIP-244 uses signed 8-byte LE); Convention #2'sbigintrule is TS-boundary-facing and doesn't apply to this Rust layer.
Conventions / style
Doc comments are thorough and cite the exact ZIP-244 sub-sections — very good. Error handling and expect("vec write is infallible") match surrounding code. Test coverage is strong for the transparent path; the one gap is the unshield digest (finding #1).
Bottom line: solid, well-documented work with a standout golden test. The one blocking concern is the empty-inputs fallback (#1) — a real correctness bug for unshield transactions, currently untested — worth fixing before merge even if that flow isn't exercised yet.
Generated by Claude Code
…rent) Extend the v6 (Ironwood/NU6.3) codec with the two ZIP-244 signature-hash computations needed to sign a v6 transaction. Previously the codec could only compute the ZIP-244 txid. - `compute_v6_sig_digest` — shielded SIGHASH_ALL digest (the message the Ironwood binding signature signs). - `compute_v6_transparent_sighash` — per-input transparent SIGHASH_ALL digest, folding in the ironwood_digest. Both share a single ZIP-244 §S.2 core that differs only in the per-input (txin) sub-digest: empty for a shielded signature, populated for the transparent input being signed. Adds the `ZTxTrAmountsHash`, `ZTxTrScriptsHash`, and `Zcash___TxInHash` personalizations, matching the `zcash_primitives` reference (amounts/scripts use the count-less `Array` encoding; txid sub-digests stay plain concatenation). Tests: - A golden oracle that verifies the REAL ECDSA signature of the signed `v6_shield1zec` transparent input against `compute_v6_transparent_sighash` (independent: the signature was produced by an external signer). Proves the transparent digest is byte-correct against a real signed v6 tx. - Regression coverage for determinism, that the digest commits to input amounts/scriptPubKeys, transparent-vs-shielded difference, per-input variation, and the no-transparent-inputs fallback (== txid). No new dependencies. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
051b0e8 to
92c080f
Compare
OttoAllmendinger
left a comment
There was a problem hiding this comment.
Re-review (rev 92c080f)
Both findings from the first pass are resolved — verified against the ZIP-244 §S.2 reference.
✅ #1 — empty-inputs fallback (was the blocking one). The guard is now:
if inputs.is_empty() && tx.transparent.output.is_empty() {
return transparent_txid_digest(tx);
}which is correct: the sig transparent digest collapses to the txid digest only when there are neither transparent inputs nor outputs. The unshield shape (transparent output, no transparent inputs) now runs the full S.2 structure (hash_type ‖ prevouts ‖ amounts ‖ scripts ‖ sequence ‖ outputs ‖ txin), which differs from the txid digest whenever outputs is non-empty. The explanatory comment is accurate. And shielded_sig_digest_unshield_case_differs_from_txid exercises exactly the previously-broken path (outputs present, input: vec[], asserts sig ≠ txid) — good targeted regression.
✅ #2 — length-mismatch robustness. debug_assert_eq! on input_amounts.len() / input_script_pubkeys.len() vs the transparent input count now fails fast on a caller mistake. The unshield path is consistent (0 amounts == 0 inputs, passes the assert).
Everything else I validated on the first pass still holds (personalizations byte-exact, S.2 ordering, count-less Array encoding for amounts/scripts, shared ZcashTxHash_+branch-id outer personalization, the real-signature golden oracle).
Non-blocking nits, unchanged from before — take or leave:
debug_assert_eq!is compiled out in release, so a length mismatch in a production/wasm build still silently computes a wrong-but-valid-looking digest. For internal-only callers with debug coverage that's a fine tradeoff; if you'd rather be strict, a returned error would also catch it in release. No test currently trips the assert.ZcashV6Error::UnexpectedEoffor an out-of-rangeinput_index/ shortinput_amountsstill reads oddly, but reusing the existing variant is pragmatic.
LGTM from my side — the correctness concern is closed. Leaving formal approval to @OttoAllmendinger.
Generated by Claude Code
Extend the v6 (Ironwood/NU6.3) codec with the two ZIP-244 signature-hash computations needed to sign a v6 transaction. Previously the codec could only compute the ZIP-244 txid.
compute_v6_sig_digest— shielded SIGHASH_ALL digest (the message the Ironwood binding signature signs).compute_v6_transparent_sighash— per-input transparent SIGHASH_ALL digest, folding in the ironwood_digest.Both share a single ZIP-244 §S.2 core that differs only in the per-input (txin) sub-digest: empty for a shielded signature, populated for the transparent input being signed. Adds the
ZTxTrAmountsHash,ZTxTrScriptsHash, andZcash___TxInHashpersonalizations, matching thezcash_primitivesreference (amounts/scripts use the count-lessArrayencoding; txid sub-digests stay plain concatenation).Tests:
v6_shield1zectransparent input againstcompute_v6_transparent_sighash(independent: the signature was produced by an external signer). Proves the transparent digest is byte-correct against a real signed v6 tx.No new dependencies.