From 1711054890c773c1c7cd190e70bed11dd95d70fb Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 31 Jul 2026 10:37:26 +0000 Subject: [PATCH] fix(wasm-utxo): keep v6 consensus branch id in the BITGO/ZEC/V6 namespace only MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A v6 (Ironwood) PSBT previously carried its consensus branch id twice: once under the namespaced BITGO/ZEC/V6 key written by new_v6, and once under the legacy BITGO/ZecConsensusBranchId key that the shared ZcashBitGoPsbt::new constructor wrote for every Zcash PSBT. The legacy copy was redundant namespace pollution in the hard-limited single-byte BITGO subtype space. ZcashBitGoPsbt::new no longer takes or stamps consensus_branch_id — callers write it into the correct proprietary namespace directly: v4/ Sapling callers use set_zec_consensus_branch_id (legacy BITGO key), new_v6 uses set_zec_v6_consensus_branch_id (BITGO/ZEC/V6 namespace). The branch id now lands under exactly one key on first write, so v6 never pollutes the shared single-byte BITGO subtype space. The wasm consensus_branch_id() getter is made v6-aware: it reads the BITGO/ZEC/V6 key for v6 (Ironwood) PSBTs and the legacy BITGO key for v4/Sapling PSBTs. A test asserts a v6 PSBT exposes its branch id only under BITGO/ZEC/V6, both freshly built and across a v6 serialize/ deserialize round-trip. 527 tests pass, clippy clean. --- .../src/fixed_script_wallet/bitgo_psbt/mod.rs | 42 +++++++++------- .../fixed_script_wallet/bitgo_psbt/propkv.rs | 9 ++-- .../bitgo_psbt/zcash_psbt.rs | 50 ++++++++++++++++--- .../src/wasm/fixed_script_wallet/mod.rs | 14 +++++- 4 files changed, 83 insertions(+), 32 deletions(-) diff --git a/packages/wasm-utxo/src/fixed_script_wallet/bitgo_psbt/mod.rs b/packages/wasm-utxo/src/fixed_script_wallet/bitgo_psbt/mod.rs index c81c1f2fe77..3421bf37d8a 100644 --- a/packages/wasm-utxo/src/fixed_script_wallet/bitgo_psbt/mod.rs +++ b/packages/wasm-utxo/src/fixed_script_wallet/bitgo_psbt/mod.rs @@ -458,15 +458,18 @@ impl BitGoPsbt { expiry_height: Option, ) -> Self { BitGoPsbt::Zcash( - ZcashBitGoPsbt::new( - network, - wallet_keys, - consensus_branch_id, - version, - lock_time, - version_group_id, - expiry_height, - ), + { + let mut z = ZcashBitGoPsbt::new( + network, + wallet_keys, + version, + lock_time, + version_group_id, + expiry_height, + ); + propkv::set_zec_consensus_branch_id(&mut z.psbt, consensus_branch_id); + z + }, network, ) } @@ -549,15 +552,18 @@ impl BitGoPsbt { let branch_id = propkv::get_zec_consensus_branch_id(&z.psbt) .ok_or("Template PSBT missing ZecConsensusBranchId")?; Ok(BitGoPsbt::Zcash( - ZcashBitGoPsbt::new( - network, - wallet_keys, - branch_id, - Some(version), - Some(lock_time), - z.version_group_id, - z.expiry_height, - ), + { + let mut built = ZcashBitGoPsbt::new( + network, + wallet_keys, + Some(version), + Some(lock_time), + z.version_group_id, + z.expiry_height, + ); + propkv::set_zec_consensus_branch_id(&mut built.psbt, branch_id); + built + }, network, )) } diff --git a/packages/wasm-utxo/src/fixed_script_wallet/bitgo_psbt/propkv.rs b/packages/wasm-utxo/src/fixed_script_wallet/bitgo_psbt/propkv.rs index 4c9552e4bb7..b22249c4075 100644 --- a/packages/wasm-utxo/src/fixed_script_wallet/bitgo_psbt/propkv.rs +++ b/packages/wasm-utxo/src/fixed_script_wallet/bitgo_psbt/propkv.rs @@ -256,10 +256,11 @@ pub const BITGO_ZEC_V6: &[u8] = b"BITGO/ZEC/V6"; /// This mirrors the v4 `ZecConsensusBranchId` (0x00 under the legacy `BITGO` prefix), but v4 is /// untouched: the two 0x00 branch-id keys are unambiguous because their prefixes differ. /// -/// Note that a v6 PSBT still carries the legacy `BITGO`/`ZecConsensusBranchId` key as well, because -/// [`ZcashBitGoPsbt::new`] writes it for every Zcash PSBT. The v6 code paths read only the key in -/// this namespace; the legacy one is redundant but harmless, and keeping it means the shared -/// `new` constructor needs no v6 special case. +/// A v6 PSBT carries its branch id under *this* namespace only: `ZcashBitGoPsbt::new_v6` writes +/// it here (and never writes the legacy `BITGO` key), so v6 does not consume a slot in the shared +/// single-byte `BITGO` subtype space. Readers that want the v6 branch id (including the wasm +/// `consensus_branch_id()` getter) must use [`get_zec_v6_consensus_branch_id`], since +/// [`get_zec_consensus_branch_id`] only matches the legacy prefix. #[derive(Debug, Clone, Copy, PartialEq, Eq)] #[repr(u8)] pub enum ZecV6KeySubtype { diff --git a/packages/wasm-utxo/src/fixed_script_wallet/bitgo_psbt/zcash_psbt.rs b/packages/wasm-utxo/src/fixed_script_wallet/bitgo_psbt/zcash_psbt.rs index a1836ee0408..a0a4d7a13cd 100644 --- a/packages/wasm-utxo/src/fixed_script_wallet/bitgo_psbt/zcash_psbt.rs +++ b/packages/wasm-utxo/src/fixed_script_wallet/bitgo_psbt/zcash_psbt.rs @@ -42,18 +42,21 @@ pub(crate) const V6_NOT_SUPPORTED_BY_V4_PATH: &str = impl ZcashBitGoPsbt { /// Create an empty Zcash PSBT directly without going through `BitGoPsbt`. + /// + /// Does not stamp a consensus-branch-id key — the caller writes it into the right proprietary + /// namespace (legacy `BITGO` for v4/Sapling, `BITGO/ZEC/V6` for v6) so that the branch id lives + /// under exactly one key and never pollutes the shared single-byte `BITGO` subtype space with a + /// redundant copy. pub(crate) fn new( network: crate::Network, wallet_keys: &crate::fixed_script_wallet::RootWalletKeys, - consensus_branch_id: u32, version: Option, lock_time: Option, version_group_id: Option, expiry_height: Option, ) -> Self { - let mut psbt = + let psbt = super::make_psbt_with_xpubs(version.unwrap_or(4), lock_time.unwrap_or(0), wallet_keys); - super::propkv::set_zec_consensus_branch_id(&mut psbt, consensus_branch_id); Self { psbt, network, @@ -82,15 +85,16 @@ impl ZcashBitGoPsbt { if is_mainnet { "mainnet" } else { "testnet" } ) })?; - Ok(Self::new( + let mut z = Self::new( network, wallet_keys, - consensus_branch_id, version, lock_time, version_group_id, expiry_height, - )) + ); + super::propkv::set_zec_consensus_branch_id(&mut z.psbt, consensus_branch_id); + Ok(z) } /// Get the network this PSBT is for @@ -111,12 +115,12 @@ impl ZcashBitGoPsbt { let mut z = Self::new( network, wallet_keys, - consensus_branch_id, Some(tx.version.0), Some(tx.lock_time.to_consensus_u32()), version_group_id, expiry_height, ); + super::propkv::set_zec_consensus_branch_id(&mut z.psbt, consensus_branch_id); super::BitGoPsbt::hydrate_psbt(&mut z.psbt, network, wallet_keys, tx, unspents)?; Ok(z) } @@ -637,7 +641,6 @@ impl ZcashBitGoPsbt { let mut z = Self::new( network, wallet_keys, - consensus_branch_id, Some(6), lock_time, Some(version_group_id), @@ -1298,6 +1301,37 @@ mod ironwood_v6_tests { z } + /// A v6 PSBT keeps its consensus branch id under the `BITGO/ZEC/V6` namespace only: `new_v6` + /// writes it there and never writes the legacy `BITGO`/`ZecConsensusBranchId` key, so v6 does + /// not consume a slot in the shared single-byte `BITGO` subtype space. Guards the coupling with + /// the wasm `consensus_branch_id()` getter, which reads the v6 key for v6 PSBTs. + #[test] + fn v6_carries_branch_id_only_under_the_v6_namespace() { + use crate::fixed_script_wallet::bitgo_psbt::propkv::{ + get_zec_consensus_branch_id, get_zec_v6_consensus_branch_id, + }; + + let z = build_shield_psbt("v6_branch_id_namespace"); + // The same branch id `new_v6_at_height` derives for the testnet NU6.3 activation height. + let expected = crate::zcash::branch_id_for_height( + NetworkUpgrade::Nu6_3.testnet_activation_height(), + false, + ) + .unwrap(); + + // Present under the v6 namespace... + assert_eq!(get_zec_v6_consensus_branch_id(&z.psbt), Some(expected)); + // ...and the legacy shared-namespace key is absent (v6 never writes it). + assert_eq!(get_zec_consensus_branch_id(&z.psbt), None); + + // Survives a v6 serialize/deserialize round-trip: the legacy key does not reappear, and the + // v6 branch id is still readable. + let round = + ZcashBitGoPsbt::deserialize_v6(&z.serialize_v6(), Network::ZcashTestnet).unwrap(); + assert_eq!(get_zec_v6_consensus_branch_id(&round.psbt), Some(expected)); + assert_eq!(get_zec_consensus_branch_id(&round.psbt), None); + } + /// `new_v6_at_height` rejects a height before NU6.3 rather than stamping the transaction with a /// branch id that only fails at broadcast. #[test] diff --git a/packages/wasm-utxo/src/wasm/fixed_script_wallet/mod.rs b/packages/wasm-utxo/src/wasm/fixed_script_wallet/mod.rs index fe27ce704a8..4b8c765c74f 100644 --- a/packages/wasm-utxo/src/wasm/fixed_script_wallet/mod.rs +++ b/packages/wasm-utxo/src/wasm/fixed_script_wallet/mod.rs @@ -990,10 +990,20 @@ impl BitGoPsbt { /// Get the Zcash consensus branch ID from the PSBT proprietary map (returns None for non-Zcash PSBTs) pub fn consensus_branch_id(&self) -> Option { use crate::fixed_script_wallet::bitgo_psbt::{ - propkv::get_zec_consensus_branch_id, BitGoPsbt as InnerBitGoPsbt, + propkv::{get_zec_consensus_branch_id, get_zec_v6_consensus_branch_id}, + BitGoPsbt as InnerBitGoPsbt, }; match &self.psbt { - InnerBitGoPsbt::Zcash(z, _) => get_zec_consensus_branch_id(&z.psbt), + // v6 (Ironwood) PSBTs carry the branch id under the `BITGO/ZEC/V6` namespace; v4/Sapling + // PSBTs under the legacy `BITGO` key. A v6 PSBT does not carry the legacy key at all + // (see `ZcashBitGoPsbt::new_v6`), so it must be read from the namespace here. + InnerBitGoPsbt::Zcash(z, _) => { + if z.is_ironwood_v6() { + get_zec_v6_consensus_branch_id(&z.psbt) + } else { + get_zec_consensus_branch_id(&z.psbt) + } + } _ => None, } }