Skip to content

feat(wasm-utxo): add ZIP-244 v6 signature digests (shielded + transparent) - #338

Merged
OttoAllmendinger merged 1 commit into
masterfrom
veetragjain/cshld-1291-wasm-utxo-add-zip-244-v6-signature-digests-shielded
Jul 29, 2026
Merged

feat(wasm-utxo): add ZIP-244 v6 signature digests (shielded + transparent)#338
OttoAllmendinger merged 1 commit into
masterfrom
veetragjain/cshld-1291-wasm-utxo-add-zip-244-v6-signature-digests-shielded

Conversation

@veetragjain

Copy link
Copy Markdown
Contributor

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.

@linear-code

linear-code Bot commented Jul 28, 2026

Copy link
Copy Markdown

CSHLD-1291

@veetragjain
veetragjain marked this pull request as ready for review July 28, 2026 08:41
@veetragjain
veetragjain requested a review from a team as a code owner July 28, 2026 08:41

@OttoAllmendinger OttoAllmendinger left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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___TxInHash are 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_sighash returns ZcashV6Error::UnexpectedEof for both an out-of-range input_index and a too-short input_amounts. UnexpectedEof reads oddly for a caller-argument error; a dedicated variant would be clearer, though reusing the existing one is pragmatic.
  • i64 amounts internally are fine here (ZIP-244 uses signed 8-byte LE); Convention #2's bigint rule 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>

@OttoAllmendinger OttoAllmendinger left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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::UnexpectedEof for an out-of-range input_index / short input_amounts still 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

@OttoAllmendinger
OttoAllmendinger merged commit eba18e5 into master Jul 29, 2026
14 checks passed
@OttoAllmendinger
OttoAllmendinger deleted the veetragjain/cshld-1291-wasm-utxo-add-zip-244-v6-signature-digests-shielded branch July 29, 2026 09:59
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