feat(wasm-utxo): wire v6 (Ironwood) build/sign/combine into ZcashBitG…

[veetragjain]

…oPsbt

Add a dedicated Zcash v6 (Ironwood / NU6.3) shielding flow on ZcashBitGoPsbt, mirroring the microservice build → sign → combine PSBT lifecycle. The ~20 generic BitGoPsbt::Zcash dispatch arms are left untouched; v6 uses dedicated methods.

propkv: add ProprietaryKeySubtype::ZecIronwoodPczt (0x07, the serialized orchard PCZT) and ZecV6Params (0x08, versionGroupId + expiryHeight); the latter marks a PSBT as v6 so it round-trips through a plain PSBT serialization.

ZcashBitGoPsbt v6 methods:

• new_v6 / new_v6_at_height: version-6, Ironwood VGID, NU6.3 branch.
• add_ironwood_output (Constructor): build the shielded note as an orchard PCZT and store it in the PSBT.
• ironwood_action_data / v6_txid / v6_transparent_sighash: derive the ZIP-244 txid and per-input transparent sighash from the transparent skeleton + stored PCZT action data.
• add_v6_transparent_signature: verify a client/HSM signature against the v6 sighash and insert it into partial_sigs.
• combine_ironwood_proof (Extractor): finalize the transparent inputs, splice in the external prover's zkproof, apply the binding signature, and encode the broadcast-ready v6 transaction.
• serialize_v6 / deserialize_v6: round-trip the v6 PSBT (transparent skeleton + PCZT + params).

BitGoPsbt::new_zcash_v6_at_height: a builder so transparent inputs/outputs use the existing add_wallet_input / add_wallet_output machinery.

Tests (native):

• build a 2-of-3 P2SH → Ironwood shield PSBT, round-trip it, sign the transparent input over the ZIP-244 sighash, combine with a placeholder proof, and assert the result decodes, keeps a stable txid across signing, and is accepted by zebra-chain.
• golden oracle: reproduce the on-chain shield1zec transaction inside a PSBT and assert its v6_transparent_sighash both equals the codec-golden sighash and verifies the transaction's real ECDSA signature — proving the PSBT threads the true spent-output value + scriptCode into the sighash (a path the synthetic and zebra checks cannot cover).
• propkv round-trip unit test.

[linear-code]

CSHLD-1294

[OttoAllmendinger]

this list is becoming a little disorganized, I wonder if we should namespace it somehow and put all the zcash thing under the same prefix

[OttoAllmendinger]

Strong +1 on namespacing — and there's a concrete reason it matters here: the subtype is a hard-limited subspace. In rust-bitcoin 0.32 (bitcoin-0.32.8-bitgo.2), ProprietaryKey<Subtype = u8> is bounded Subtype: Copy + From<u8> + Into<u8> and the subtype serializes as a single byte, so every BitGo key — MuSig2, PayGo, BIP322, WasmUtxo, and now Zcash — is competing for the same 256-value BITGO space. We can't widen the subtype without hand-rolling raw PSBT keys and abandoning ProprietaryKey. The right lever is the other field: the identifier prefix, which is an arbitrary Vec<u8>. Giving a domain its own prefix hands it a fresh, private 0x00–0xFF subtype space.

Proposal for this PR — put the v6 keys under a dedicated prefix and revert the two additions to the shared enum:

/// Zcash v6 (Ironwood) proprietary namespace — its own private subtype space,
/// so v6 keys don't consume slots in the shared BITGO space.
pub const BITGO_ZEC_V6: &[u8] = b"BITGO/ZEC/V6";

#[derive(Debug, Clone, Copy, PartialEq, Eq)]
#[repr(u8)]
pub enum ZecV6KeySubtype {
    ConsensusBranchId = 0x00,
    IronwoodPczt = 0x01,
    V6Params = 0x02,
}

• ZecIronwoodPczt/ZecV6Params (0x07/0x08) come back out of ProprietaryKeySubtype — they were added in this PR and never shipped, so there's nothing to migrate; they move to BITGO/ZEC/V6 at 0x01/0x02.
• The v6 consensus branch id moves under the namespace too (BITGO/ZEC/V6 / 0x00), read/written by v6-only accessors.
• v4 is untouched. The shipped ZecConsensusBranchId = 0x00 under the legacy BITGO prefix stays exactly as-is — v4 and v6 are distinct construction paths, so no dual-read or migration is needed for in-the-wild v4 PSBTs.

Note the two 0x00 branch-id keys are unambiguous because the prefix differs: BITGO/0x00 (v4) vs BITGO/ZEC/V6/0x00 (v6). Keeping a separate ZecV6KeySubtype enum (rather than extending ProprietaryKeySubtype) keeps the two subtype spaces from being looked up against the wrong prefix.

Happy to push this as a follow-up commit if you'd like it in before merge.

---

Generated by Claude Code

[OttoAllmendinger]

Addendum: once the v6 keys have their own subtype space, it's worth splitting ZecV6Params (the packed versionGroupId ‖ expiryHeight blob) into two single-field keys. Spending a subtype per field is free under the namespace, and each accessor becomes a self-describing u32 instead of manual offset-slicing + a len() == 8 guard:

pub enum ZecV6KeySubtype {
    ConsensusBranchId = 0x00,
    IronwoodPczt      = 0x01,
    VersionGroupId    = 0x02,
    ExpiryHeight      = 0x03,
}

fn set_u32(psbt: &mut Psbt, st: ZecV6KeySubtype, v: u32) {
    set_zec(psbt, st, v.to_le_bytes().to_vec());
}
fn get_u32(psbt: &Psbt, st: ZecV6KeySubtype) -> Option<u32> {
    let v = get_zec(psbt, st)?;
    Some(u32::from_le_bytes(v.as_slice().try_into().ok()?))
}

No more [0..4]/[4..8] slicing or packed-length check, and a future header field is just another subtype rather than a repack of a growing blob.

Two things to keep right when splitting:

1. What marks a PSBT as v6. Today it's "ZecV6Params present." After the split, pick one field as the marker — VersionGroupId is the natural one (the Ironwood VGID), so deserialize_v6 should key off its presence, not expiry.
2. Always write both fields, even when expiry is 0. expiry_height == 0 ("no expiry") is a valid value; the packed blob stores it explicitly, so set must write the ExpiryHeight key unconditionally — otherwise you can't tell "0" from "absent" on read.

Minor: two keys repeat the ~12-byte prefix, so it's ~15 bytes larger on the wire than the packed form — negligible.

Aside: since VersionGroupId is constant for Ironwood v6, you could drop it and let ConsensusBranchId's presence under the v6 prefix be the marker — but storing it is cheap and leaves room for a future non-Ironwood v6 variant, so I'd keep it.

---

Generated by Claude Code

[veetragjain]

Done

[OttoAllmendinger]

Review

(Namespacing of the proprietary keys is covered in the propkv.rs thread — this covers the rest.)

Overview

Wires the Ironwood PCZT bridge into ZcashBitGoPsbt: proprietary subtypes for the shielded state, the v6 constructor/txid/sighash/sign/combine methods, and plain-PSBT serialize/deserialize that round-trips the v6 state without the v4 tx-replacement dance. Two strong tests — a synthetic end-to-end (build→sign→combine + zebra cross-check) and a PSBT-level golden oracle verifying the real shield1zec signature against the PSBT-derived sighash.

✅ Correction to my #341 finding (bsk leak) — the production flow is safe

Reviewing combine_ironwood_proof here resolves the bsk concern I raised on #339/#341. The real flow is: add_ironwood_output stores the Constructor PCZT (no bsk); the external prover receives that; then combine_ironwood_proof(proof) runs finalize_shield_io (derives bsk) → with_zkproof → combine all locally, after the proof returns. So bsk is created and consumed inside one local call and is never serialized into anything sent to the prover. The leak-prone sequence I flagged was inferred from #341's test ordering (finalize → serialize → hand to prover → combine); the actual consumer inverts that safely. Recommend aligning the #341 test/docstring to this order so the bridge doesn't read as if bsk ships to the prover. Net: I'd downgrade that finding from "security" to "tighten the #341 example."

Issues

1. (Correctness) finalized_transparent_tx doesn't enforce the 2-of-3 threshold.
It emits OP_0 <sig>… <redeemScript> from whatever partial_sigs happen to be present. With only one signature collected it silently produces an incomplete, unspendable scriptSig rather than erroring — the failure only surfaces at broadcast. Worth asserting the required number of sigs per input (≥ m) before assembling.

2. (API nit) combine_ironwood_proof ignores its rng for the binding signature.
The rng: R param is moved into finalize_shield_io, then the binding signature hardcodes rand::rngs::OsRng (per the comment). A caller passing a seeded RNG for reproducibility still gets OS-sourced binding-sig randomness. Consider taking &mut R and reusing it, or documenting that binding randomness is always OS-sourced.

Minor

• get_zec_v6_params correctly length-checks (== 8) and overwrite semantics are tested — good. (If you take the split-into-two-keys suggestion from the propkv thread, this check goes away entirely.)
• The golden test overloads redeem_script to carry a P2PKH scriptPubKey as the scriptCode — fine for the test, but a one-line comment noting it's a P2PKH stand-in (not a real P2SH redeem script) would prevent confusion.

Praise

add_v6_transparent_signature verifying each signature against the sighash before inserting is a nice guardrail; the PSBT-level golden oracle genuinely closes a gap the synthetic test can't (a real external signature over the true spent-output value/script); and the plain-PSBT round-trip via a marker proprietary key is clean.

---

Generated by Claude Code

[OttoAllmendinger]

Re-review of 7388b83 (now rebased directly on master, +835/-2).

Prior review + the namespacing thread: all addressed ✅

Namespacing (thread #discussion_r3664486652) — fully adopted, both comments.

• v6 keys moved under a dedicated BITGO_ZEC_V6 = b"BITGO/ZEC/V6" prefix with its own private ZecV6KeySubtype space; the 0x07/0x08 additions to the shared ProprietaryKeySubtype enum are reverted, so the hard-limited single-byte BITGO subspace is no longer consumed by Zcash keys.
• The split-fields addendum is in too: ConsensusBranchId=0x00, IronwoodPczt=0x01, VersionGroupId=0x02, ExpiryHeight=0x03, self-describing u32 accessors (no more offset-slicing + len()==8 guard), VersionGroupId as the v6 marker, and ExpiryHeight always written (the roundtrip test asserts Some((vgid, 0)) is distinct from absent).
• v4 is untouched, and test_zec_v6_consensus_branch_id_roundtrip pins the key point: the two 0x00 branch-id keys coexist unambiguously because the prefix differs.

Both findings from the first pass — fixed.

1. finalized_transparent_tx now enforces REQUIRED_SIGS = 2 and returns an error on under-collection, so a single collected signature can no longer silently produce an incomplete, unspendable scriptSig.
2. combine_ironwood_proof threads &mut rng into both finalize_shield_io and combine — the hardcoded OsRng is gone.

Also good: the dangling docs/ironwood-proof-service-contract.md doc reference is removed, and combine_ironwood_proof still derives the binding signing key locally (post-proof), so bsk never leaves the process.

One new minor thing (non-blocking)

new_v6 writes the consensus branch id twice: Self::new (zcash_psbt.rs:47) writes the legacy set_zec_consensus_branch_id (BITGO/0x00), and then new_v6 writes set_zec_v6_consensus_branch_id (BITGO/ZEC/V6/0x00). The values agree so it isn't a correctness bug, but every v6 PSBT ends up carrying a redundant key in the legacy shared namespace — the exact pollution the namespacing was meant to avoid, and a v6 PSBT would then also satisfy the generic v4 require_branch_id check at zcash_psbt.rs:376. It isn't caught by test_ironwood_pczt_and_v6_params_roundtrip's prefix-purity assertion because that test builds the PSBT via the setters rather than through new_v6. Worth dropping the legacy write on the v6 path (or asserting prefix purity on a new_v6-built PSBT).

Nit: the add_ironwood_output doc comment has a garbled sentence ("Must be called after the transparent inputs/outputs are in place is NOT required …").

LGTM once the double-write is cleaned up (or consciously kept). Nice work threading the golden shield1zec oracle through the PSBT layer — verifying the real on-chain ECDSA signature against the PSBT-derived sighash is a strong check that the spent-output value/scriptCode are threaded correctly.

---

Generated by Claude Code

[prajwalu142]

lgtm