Skip to content

Commit eb41aff

Browse files
claudeOttoAllmendinger
authored andcommitted
fix(wasm-utxo): keep v6 consensus branch id in the BITGO/ZEC/V6 namespace only
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.
1 parent 49c0e1d commit eb41aff

4 files changed

Lines changed: 83 additions & 32 deletions

File tree

packages/wasm-utxo/src/fixed_script_wallet/bitgo_psbt/mod.rs

Lines changed: 24 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -458,15 +458,18 @@ impl BitGoPsbt {
458458
expiry_height: Option<u32>,
459459
) -> Self {
460460
BitGoPsbt::Zcash(
461-
ZcashBitGoPsbt::new(
462-
network,
463-
wallet_keys,
464-
consensus_branch_id,
465-
version,
466-
lock_time,
467-
version_group_id,
468-
expiry_height,
469-
),
461+
{
462+
let mut z = ZcashBitGoPsbt::new(
463+
network,
464+
wallet_keys,
465+
version,
466+
lock_time,
467+
version_group_id,
468+
expiry_height,
469+
);
470+
propkv::set_zec_consensus_branch_id(&mut z.psbt, consensus_branch_id);
471+
z
472+
},
470473
network,
471474
)
472475
}
@@ -549,15 +552,18 @@ impl BitGoPsbt {
549552
let branch_id = propkv::get_zec_consensus_branch_id(&z.psbt)
550553
.ok_or("Template PSBT missing ZecConsensusBranchId")?;
551554
Ok(BitGoPsbt::Zcash(
552-
ZcashBitGoPsbt::new(
553-
network,
554-
wallet_keys,
555-
branch_id,
556-
Some(version),
557-
Some(lock_time),
558-
z.version_group_id,
559-
z.expiry_height,
560-
),
555+
{
556+
let mut built = ZcashBitGoPsbt::new(
557+
network,
558+
wallet_keys,
559+
Some(version),
560+
Some(lock_time),
561+
z.version_group_id,
562+
z.expiry_height,
563+
);
564+
propkv::set_zec_consensus_branch_id(&mut built.psbt, branch_id);
565+
built
566+
},
561567
network,
562568
))
563569
}

packages/wasm-utxo/src/fixed_script_wallet/bitgo_psbt/propkv.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -256,10 +256,11 @@ pub const BITGO_ZEC_V6: &[u8] = b"BITGO/ZEC/V6";
256256
/// This mirrors the v4 `ZecConsensusBranchId` (0x00 under the legacy `BITGO` prefix), but v4 is
257257
/// untouched: the two 0x00 branch-id keys are unambiguous because their prefixes differ.
258258
///
259-
/// Note that a v6 PSBT still carries the legacy `BITGO`/`ZecConsensusBranchId` key as well, because
260-
/// [`ZcashBitGoPsbt::new`] writes it for every Zcash PSBT. The v6 code paths read only the key in
261-
/// this namespace; the legacy one is redundant but harmless, and keeping it means the shared
262-
/// `new` constructor needs no v6 special case.
259+
/// A v6 PSBT carries its branch id under *this* namespace only: `ZcashBitGoPsbt::new_v6` writes
260+
/// it here (and never writes the legacy `BITGO` key), so v6 does not consume a slot in the shared
261+
/// single-byte `BITGO` subtype space. Readers that want the v6 branch id (including the wasm
262+
/// `consensus_branch_id()` getter) must use [`get_zec_v6_consensus_branch_id`], since
263+
/// [`get_zec_consensus_branch_id`] only matches the legacy prefix.
263264
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
264265
#[repr(u8)]
265266
pub enum ZecV6KeySubtype {

packages/wasm-utxo/src/fixed_script_wallet/bitgo_psbt/zcash_psbt.rs

Lines changed: 42 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -42,18 +42,21 @@ pub(crate) const V6_NOT_SUPPORTED_BY_V4_PATH: &str =
4242

4343
impl ZcashBitGoPsbt {
4444
/// Create an empty Zcash PSBT directly without going through `BitGoPsbt`.
45+
///
46+
/// Does not stamp a consensus-branch-id key — the caller writes it into the right proprietary
47+
/// namespace (legacy `BITGO` for v4/Sapling, `BITGO/ZEC/V6` for v6) so that the branch id lives
48+
/// under exactly one key and never pollutes the shared single-byte `BITGO` subtype space with a
49+
/// redundant copy.
4550
pub(crate) fn new(
4651
network: crate::Network,
4752
wallet_keys: &crate::fixed_script_wallet::RootWalletKeys,
48-
consensus_branch_id: u32,
4953
version: Option<i32>,
5054
lock_time: Option<u32>,
5155
version_group_id: Option<u32>,
5256
expiry_height: Option<u32>,
5357
) -> Self {
54-
let mut psbt =
58+
let psbt =
5559
super::make_psbt_with_xpubs(version.unwrap_or(4), lock_time.unwrap_or(0), wallet_keys);
56-
super::propkv::set_zec_consensus_branch_id(&mut psbt, consensus_branch_id);
5760
Self {
5861
psbt,
5962
network,
@@ -82,15 +85,16 @@ impl ZcashBitGoPsbt {
8285
if is_mainnet { "mainnet" } else { "testnet" }
8386
)
8487
})?;
85-
Ok(Self::new(
88+
let mut z = Self::new(
8689
network,
8790
wallet_keys,
88-
consensus_branch_id,
8991
version,
9092
lock_time,
9193
version_group_id,
9294
expiry_height,
93-
))
95+
);
96+
super::propkv::set_zec_consensus_branch_id(&mut z.psbt, consensus_branch_id);
97+
Ok(z)
9498
}
9599

96100
/// Get the network this PSBT is for
@@ -111,12 +115,12 @@ impl ZcashBitGoPsbt {
111115
let mut z = Self::new(
112116
network,
113117
wallet_keys,
114-
consensus_branch_id,
115118
Some(tx.version.0),
116119
Some(tx.lock_time.to_consensus_u32()),
117120
version_group_id,
118121
expiry_height,
119122
);
123+
super::propkv::set_zec_consensus_branch_id(&mut z.psbt, consensus_branch_id);
120124
super::BitGoPsbt::hydrate_psbt(&mut z.psbt, network, wallet_keys, tx, unspents)?;
121125
Ok(z)
122126
}
@@ -637,7 +641,6 @@ impl ZcashBitGoPsbt {
637641
let mut z = Self::new(
638642
network,
639643
wallet_keys,
640-
consensus_branch_id,
641644
Some(6),
642645
lock_time,
643646
Some(version_group_id),
@@ -1298,6 +1301,37 @@ mod ironwood_v6_tests {
12981301
z
12991302
}
13001303

1304+
/// A v6 PSBT keeps its consensus branch id under the `BITGO/ZEC/V6` namespace only: `new_v6`
1305+
/// writes it there and never writes the legacy `BITGO`/`ZecConsensusBranchId` key, so v6 does
1306+
/// not consume a slot in the shared single-byte `BITGO` subtype space. Guards the coupling with
1307+
/// the wasm `consensus_branch_id()` getter, which reads the v6 key for v6 PSBTs.
1308+
#[test]
1309+
fn v6_carries_branch_id_only_under_the_v6_namespace() {
1310+
use crate::fixed_script_wallet::bitgo_psbt::propkv::{
1311+
get_zec_consensus_branch_id, get_zec_v6_consensus_branch_id,
1312+
};
1313+
1314+
let z = build_shield_psbt("v6_branch_id_namespace");
1315+
// The same branch id `new_v6_at_height` derives for the testnet NU6.3 activation height.
1316+
let expected = crate::zcash::branch_id_for_height(
1317+
NetworkUpgrade::Nu6_3.testnet_activation_height(),
1318+
false,
1319+
)
1320+
.unwrap();
1321+
1322+
// Present under the v6 namespace...
1323+
assert_eq!(get_zec_v6_consensus_branch_id(&z.psbt), Some(expected));
1324+
// ...and the legacy shared-namespace key is absent (v6 never writes it).
1325+
assert_eq!(get_zec_consensus_branch_id(&z.psbt), None);
1326+
1327+
// Survives a v6 serialize/deserialize round-trip: the legacy key does not reappear, and the
1328+
// v6 branch id is still readable.
1329+
let round =
1330+
ZcashBitGoPsbt::deserialize_v6(&z.serialize_v6(), Network::ZcashTestnet).unwrap();
1331+
assert_eq!(get_zec_v6_consensus_branch_id(&round.psbt), Some(expected));
1332+
assert_eq!(get_zec_consensus_branch_id(&round.psbt), None);
1333+
}
1334+
13011335
/// `new_v6_at_height` rejects a height before NU6.3 rather than stamping the transaction with a
13021336
/// branch id that only fails at broadcast.
13031337
#[test]

packages/wasm-utxo/src/wasm/fixed_script_wallet/mod.rs

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -990,10 +990,20 @@ impl BitGoPsbt {
990990
/// Get the Zcash consensus branch ID from the PSBT proprietary map (returns None for non-Zcash PSBTs)
991991
pub fn consensus_branch_id(&self) -> Option<u32> {
992992
use crate::fixed_script_wallet::bitgo_psbt::{
993-
propkv::get_zec_consensus_branch_id, BitGoPsbt as InnerBitGoPsbt,
993+
propkv::{get_zec_consensus_branch_id, get_zec_v6_consensus_branch_id},
994+
BitGoPsbt as InnerBitGoPsbt,
994995
};
995996
match &self.psbt {
996-
InnerBitGoPsbt::Zcash(z, _) => get_zec_consensus_branch_id(&z.psbt),
997+
// v6 (Ironwood) PSBTs carry the branch id under the `BITGO/ZEC/V6` namespace; v4/Sapling
998+
// PSBTs under the legacy `BITGO` key. A v6 PSBT does not carry the legacy key at all
999+
// (see `ZcashBitGoPsbt::new_v6`), so it must be read from the namespace here.
1000+
InnerBitGoPsbt::Zcash(z, _) => {
1001+
if z.is_ironwood_v6() {
1002+
get_zec_v6_consensus_branch_id(&z.psbt)
1003+
} else {
1004+
get_zec_consensus_branch_id(&z.psbt)
1005+
}
1006+
}
9971007
_ => None,
9981008
}
9991009
}

0 commit comments

Comments
 (0)