From 92a674599b8b8db69cb05c610a8c9a416ab79c68 Mon Sep 17 00:00:00 2001 From: Luis Covarrubias Date: Thu, 24 Sep 2026 00:45:16 +0000 Subject: [PATCH] fix(wasm-utxo): reject duplicate wallet role keys Reject duplicate root xpubs and derived keys before wallet scripts or PSBT metadata creation. Propagate typed Taproot aggregation errors. Prevent one authority from occupying multiple quorum positions and trapping WASM when aggregation fails. Ticket: WCN-1863 Session-Id: d899491b-4b54-442d-bfa3-99fc93352b00 Task-Id: 05aabeb7-a6fa-45bb-adb0-5b6323263435 Requested-By: David Kaplan --- packages/wasm-utxo/src/bip322/bitgo_psbt.rs | 15 ++- packages/wasm-utxo/src/error.rs | 20 +++ .../src/fixed_script_wallet/bitgo_psbt/mod.rs | 39 ++++-- .../bitgo_psbt/psbt_wallet_input.rs | 12 +- .../bitgo_psbt/zcash_psbt.rs | 61 ++++++--- .../test_utils/fixtures.rs | 1 + .../src/fixed_script_wallet/test_utils/mod.rs | 2 +- .../src/fixed_script_wallet/wallet_keys.rs | 121 +++++++++++++++++- .../wallet_scripts/bitgo_musig.rs | 3 +- .../wallet_scripts/checkmultisig.rs | 47 +++++-- .../wallet_scripts/checksigverify.rs | 75 ++++++++--- .../fixed_script_wallet/wallet_scripts/mod.rs | 50 +++++++- packages/wasm-utxo/src/inspect/psbt.rs | 3 +- packages/wasm-utxo/src/wasm/wallet_keys.rs | 11 +- .../wasm-utxo/test/fixedScript/walletKeys.ts | 77 +++++++++++ 15 files changed, 437 insertions(+), 100 deletions(-) create mode 100644 packages/wasm-utxo/test/fixedScript/walletKeys.ts diff --git a/packages/wasm-utxo/src/bip322/bitgo_psbt.rs b/packages/wasm-utxo/src/bip322/bitgo_psbt.rs index 384bccb7f23..6f6b58afbbb 100644 --- a/packages/wasm-utxo/src/bip322/bitgo_psbt.rs +++ b/packages/wasm-utxo/src/bip322/bitgo_psbt.rs @@ -494,28 +494,31 @@ fn build_output_script_from_pubkeys( ) -> Result { match script_type { "p2sh" => { - let redeem_script = build_multisig_script_2_of_3(pubkeys); + let redeem_script = build_multisig_script_2_of_3(pubkeys) + .map_err(|error| error.to_string())?; Ok(redeem_script.to_p2sh()) } "p2shP2wsh" => { - let witness_script = build_multisig_script_2_of_3(pubkeys); + let witness_script = build_multisig_script_2_of_3(pubkeys) + .map_err(|error| error.to_string())?; let redeem_script = witness_script.to_p2wsh(); Ok(redeem_script.to_p2sh()) } "p2wsh" => { - let witness_script = build_multisig_script_2_of_3(pubkeys); + let witness_script = build_multisig_script_2_of_3(pubkeys) + .map_err(|error| error.to_string())?; Ok(witness_script.to_p2wsh()) } "p2tr" => { - let script_p2tr = ScriptP2tr::new(pubkeys, false); + let script_p2tr = ScriptP2tr::new(pubkeys, false).map_err(|error| error.to_string())?; Ok(script_p2tr.output_script()) } "p2trMusig2" => { - let script_p2tr = ScriptP2tr::new(pubkeys, true); + let script_p2tr = ScriptP2tr::new(pubkeys, true).map_err(|error| error.to_string())?; Ok(script_p2tr.output_script()) } "p2mr" => { - let script_p2mr = ScriptP2mr::new(pubkeys); + let script_p2mr = ScriptP2mr::new(pubkeys).map_err(|error| error.to_string())?; Ok(script_p2mr.output_script()) } _ => Err(format!( diff --git a/packages/wasm-utxo/src/error.rs b/packages/wasm-utxo/src/error.rs index da476d1b21b..275ed24d425 100644 --- a/packages/wasm-utxo/src/error.rs +++ b/packages/wasm-utxo/src/error.rs @@ -2,6 +2,8 @@ use core::fmt; use crate::fixed_script_wallet::bitgo_psbt::zcash_psbt::VerifyV6SignatureError; use crate::fixed_script_wallet::bitgo_psbt::ParseTransactionError; +use crate::fixed_script_wallet::wallet_scripts::BitGoMusigError; +use crate::fixed_script_wallet::WalletKeyError; pub trait WasmErrorCode { fn code(&self) -> String; @@ -28,6 +30,8 @@ pub enum WasmUtxoError { ZcashV6(crate::zcash::v6::ZcashV6Error), Ironwood(crate::zcash::ironwood_build::IronwoodBuildError), VerifyV6Signature(VerifyV6SignatureError), + WalletKey(WalletKeyError), + BitGoMusig(BitGoMusigError), } impl std::error::Error for WasmUtxoError {} @@ -41,6 +45,8 @@ impl fmt::Display for WasmUtxoError { WasmUtxoError::ZcashV6(e) => write!(f, "{}", e), WasmUtxoError::Ironwood(e) => write!(f, "{}", e), WasmUtxoError::VerifyV6Signature(e) => write!(f, "{}", e), + WasmUtxoError::WalletKey(e) => write!(f, "{}", e), + WasmUtxoError::BitGoMusig(e) => write!(f, "{}", e), } } } @@ -54,6 +60,8 @@ impl WasmErrorCode for WasmUtxoError { WasmUtxoError::ZcashV6(e) => e.code(), WasmUtxoError::Ironwood(e) => e.code(), WasmUtxoError::VerifyV6Signature(e) => e.code(), + WasmUtxoError::WalletKey(e) => e.code(), + WasmUtxoError::BitGoMusig(e) => e.code(), } } } @@ -117,6 +125,18 @@ impl From for WasmUtxoError { } } +impl From for WasmUtxoError { + fn from(err: WalletKeyError) -> Self { + WasmUtxoError::WalletKey(err) + } +} + +impl From for WasmUtxoError { + fn from(err: BitGoMusigError) -> Self { + WasmUtxoError::BitGoMusig(err) + } +} + impl WasmUtxoError { pub fn new(s: &str) -> WasmUtxoError { WasmUtxoError::StringError(s.to_string()) 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 a47b1489ec4..ed650f8f8e6 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 @@ -1277,14 +1277,18 @@ impl BitGoPsbt { // We reuse taproot PSBT fields (tap_tree, tap_key_origins) since // all tested PSBT parsers accept them on witness v2 outputs. // No tap_internal_key (P2MR has no internal key or tweak). - psbt_output.tap_tree = Some(build_tap_tree_for_output(&pub_triple, false)); + psbt_output.tap_tree = Some( + build_tap_tree_for_output(&pub_triple, false) + .map_err(|error| error.to_string())?, + ); psbt_output.tap_key_origins = create_tap_bip32_derivation_for_output( wallet_keys, chain, derivation_index, &pub_triple, false, - ); + ) + .map_err(|error| error.to_string())?; } WalletScripts::P2trLegacy(script) | WalletScripts::P2trMusig2(script) => { let is_musig2 = matches!(scripts, WalletScripts::P2trMusig2(_)); @@ -1292,7 +1296,10 @@ impl BitGoPsbt { let internal_key = script.spend_info.internal_key(); psbt_output.tap_internal_key = Some(internal_key); - psbt_output.tap_tree = Some(build_tap_tree_for_output(&pub_triple, is_musig2)); + psbt_output.tap_tree = Some( + build_tap_tree_for_output(&pub_triple, is_musig2) + .map_err(|error| error.to_string())?, + ); psbt_output.tap_key_origins = create_tap_bip32_derivation_for_output( wallet_keys, @@ -1300,7 +1307,8 @@ impl BitGoPsbt { derivation_index, &pub_triple, is_musig2, - ); + ) + .map_err(|error| error.to_string())?; } } @@ -3673,7 +3681,7 @@ pub fn to_wallet_keys( for perm in &XPUB_TRIPLE_PERMUTATIONS { let permuted = [xpubs[perm[0]], xpubs[perm[1]], xpubs[perm[2]]]; - let wallet_keys = RootWalletKeys::new(permuted); + let wallet_keys = RootWalletKeys::new(permuted).map_err(|error| error.to_string())?; let all_match = wallet_inputs.iter().all(|(tx_input, psbt_input)| { let output_script = psbt_wallet_input::get_output_script_and_value( @@ -3770,7 +3778,8 @@ mod tests { use crate::fixed_script_wallet::test_utils::get_test_wallet_keys; use crate::zcash::NetworkUpgrade; - let keys = RootWalletKeys::new(get_test_wallet_keys("test_zcash_at_height")); + let keys = RootWalletKeys::new(get_test_wallet_keys("test_zcash_at_height")) + .expect("test wallet xpubs are distinct"); // Test with Nu5 activation height (mainnet) let nu5_height = NetworkUpgrade::Nu5.mainnet_activation_height(); @@ -3822,7 +3831,8 @@ mod tests { use crate::fixed_script_wallet::test_utils::get_test_wallet_keys; use crate::zcash::NetworkUpgrade; - let keys = RootWalletKeys::new(get_test_wallet_keys("test_zcash_at_height")); + let keys = RootWalletKeys::new(get_test_wallet_keys("test_zcash_at_height")) + .expect("test wallet xpubs are distinct"); // Test with Nu5 activation height (testnet) let nu5_height = NetworkUpgrade::Nu5.testnet_activation_height(); @@ -5341,7 +5351,8 @@ mod tests { use crate::fixed_script_wallet::test_utils::get_test_wallet_keys; let other_wallet_keys = crate::fixed_script_wallet::RootWalletKeys::new( get_test_wallet_keys("too many secrets"), - ); + ) + .expect("test wallet xpubs are distinct"); // Load the original PSBT and parse inputs/outputs using existing methods let original_psbt = fixture @@ -5724,7 +5735,8 @@ mod tests { use std::str::FromStr; let wallet_keys = - crate::fixed_script_wallet::RootWalletKeys::new(get_test_wallet_keys("doge_1e19")); + crate::fixed_script_wallet::RootWalletKeys::new(get_test_wallet_keys("doge_1e19")) + .expect("test wallet xpubs are distinct"); let mut psbt = BitGoPsbt::new(Network::Dogecoin, &wallet_keys, Some(2), Some(0)); @@ -5810,7 +5822,7 @@ mod tests { use crate::fixed_script_wallet::test_utils::get_test_wallet_keys; let xpubs = get_test_wallet_keys("test_global_xpubs"); - let wallet_keys = RootWalletKeys::new(xpubs); + let wallet_keys = RootWalletKeys::new(xpubs).expect("test wallet xpubs are distinct"); let psbt = BitGoPsbt::new(Network::Bitcoin, &wallet_keys, Some(2), Some(0)); let global = psbt.get_global_xpubs().expect("should have global xpubs"); @@ -5828,7 +5840,7 @@ mod tests { use miniscript::bitcoin::hashes::Hash; let xpubs = get_test_wallet_keys("test_to_wallet_keys"); - let wallet_keys = RootWalletKeys::new(xpubs); + let wallet_keys = RootWalletKeys::new(xpubs).expect("test wallet xpubs are distinct"); let mut psbt = BitGoPsbt::new(Network::Bitcoin, &wallet_keys, Some(2), Some(0)); let txid = Txid::all_zeros(); @@ -5855,7 +5867,7 @@ mod tests { use miniscript::bitcoin::hashes::Hash; let xpubs = get_test_wallet_keys("test_to_wallet_keys_shuffled"); - let wallet_keys = RootWalletKeys::new(xpubs); + let wallet_keys = RootWalletKeys::new(xpubs).expect("test wallet xpubs are distinct"); let mut psbt = BitGoPsbt::new(Network::Bitcoin, &wallet_keys, Some(2), Some(0)); let txid = Txid::all_zeros(); @@ -5908,7 +5920,8 @@ mod tests { let seed = "zcash_block_aligned"; let secp = Secp256k1::new(); - let wallet_keys = RootWalletKeys::new(get_test_wallet_keys(seed)); + let wallet_keys = RootWalletKeys::new(get_test_wallet_keys(seed)) + .expect("test wallet xpubs are distinct"); let sapling_height = NetworkUpgrade::Sapling.testnet_activation_height(); let mut psbt = BitGoPsbt::new_zcash_at_height( diff --git a/packages/wasm-utxo/src/fixed_script_wallet/bitgo_psbt/psbt_wallet_input.rs b/packages/wasm-utxo/src/fixed_script_wallet/bitgo_psbt/psbt_wallet_input.rs index 3bc9f6b6600..7423793c622 100644 --- a/packages/wasm-utxo/src/fixed_script_wallet/bitgo_psbt/psbt_wallet_input.rs +++ b/packages/wasm-utxo/src/fixed_script_wallet/bitgo_psbt/psbt_wallet_input.rs @@ -1141,7 +1141,7 @@ pub mod test_helpers { .collect::>() .try_into() .expect("Failed to convert to XpubTriple"); - RootWalletKeys::new(triple) + RootWalletKeys::new(triple).expect("test wallet xpubs are distinct") } crate::test_psbt_fixtures!(test_validate_psbt_wallet_inputs, network, format, { @@ -1250,7 +1250,7 @@ mod infer_tests { #[test] fn tier1_witness_script_2_of_3_no_derivations_is_p2wsh() { let triple = test_pub_triple(); - let multisig = build_multisig_script_2_of_3(&triple); + let multisig = build_multisig_script_2_of_3(&triple).unwrap(); let input = p2wsh_input(multisig); let result = infer_input_script_type(&input, dummy_prevout()).expect("should classify"); @@ -1260,7 +1260,7 @@ mod infer_tests { #[test] fn tier1_witness_script_plus_redeem_script_is_p2shp2wsh() { let triple = test_pub_triple(); - let multisig = build_multisig_script_2_of_3(&triple); + let multisig = build_multisig_script_2_of_3(&triple).unwrap(); // P2shP2wsh: witness_script = multisig, redeem_script = P2WSH wrapper, // output = P2SH of the P2WSH wrapper. let redeem_script = multisig.to_p2wsh(); @@ -1300,7 +1300,7 @@ mod infer_tests { #[test] fn tier1_redeem_script_2_of_3_no_derivations_is_p2sh() { let triple = test_pub_triple(); - let multisig = build_multisig_script_2_of_3(&triple); + let multisig = build_multisig_script_2_of_3(&triple).unwrap(); let output_script = multisig.to_p2sh(); let input = psbt::Input { redeem_script: Some(multisig), @@ -1318,7 +1318,7 @@ mod infer_tests { #[test] fn bare_input_with_only_witness_utxo_errors() { let triple = test_pub_triple(); - let output_script = build_multisig_script_2_of_3(&triple).to_p2wsh(); + let output_script = build_multisig_script_2_of_3(&triple).unwrap().to_p2wsh(); let input = input_with_output(output_script); let result = infer_input_script_type(&input, dummy_prevout()); @@ -1329,7 +1329,7 @@ mod infer_tests { fn witness_script_shape_cross_check_failure_errors() { // witness_script parses as 2-of-3, but output is P2SH (not P2WSH). let triple = test_pub_triple(); - let multisig = build_multisig_script_2_of_3(&triple); + let multisig = build_multisig_script_2_of_3(&triple).unwrap(); let p2sh_output = multisig.to_p2sh(); let input = psbt::Input { witness_script: Some(multisig), 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 7c7aa65014a..33afec50cdd 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 @@ -1821,7 +1821,8 @@ mod tests { use crate::fixed_script_wallet::RootWalletKeys; use crate::networks::Network; - let wallet_keys = RootWalletKeys::new(get_test_wallet_keys("empty-zcash-round-trip")); + let wallet_keys = RootWalletKeys::new(get_test_wallet_keys("empty-zcash-round-trip")) + .expect("test wallet xpubs are distinct"); let psbt = ZcashBitGoPsbt::new( Network::Zcash, &wallet_keys, @@ -1958,7 +1959,8 @@ mod ironwood_v6_tests { #[test] fn build_sign_combine_produces_valid_v6_tx() { let seed = "ironwood_v6_psbt"; - let wallet_keys = RootWalletKeys::new(get_test_wallet_keys(seed)); + let wallet_keys = RootWalletKeys::new(get_test_wallet_keys(seed)) + .expect("test wallet xpubs are distinct"); let nu6_3 = NetworkUpgrade::Nu6_3.testnet_activation_height(); // Build: one 2-of-3 P2SH transparent input (2 ZEC), a transparent change output, and a @@ -2072,7 +2074,8 @@ mod ironwood_v6_tests { use orchard::{Action as OrchardAction, Proof}; let seed = "ironwood_v6_local_proof"; - let wallet_keys = RootWalletKeys::new(get_test_wallet_keys(seed)); + let wallet_keys = RootWalletKeys::new(get_test_wallet_keys(seed)) + .expect("test wallet xpubs are distinct"); let nu6_3 = NetworkUpgrade::Nu6_3.testnet_activation_height(); let mut psbt = BitGoPsbt::new_zcash_v6_at_height( @@ -2185,7 +2188,8 @@ mod ironwood_v6_tests { #[test] fn keyless_server_build_client_sets_out_ciphertext_via_ecdh_then_signs_and_combines() { let seed = "ironwood_v6_ovk_psbt"; - let wallet_keys = RootWalletKeys::new(get_test_wallet_keys(seed)); + let wallet_keys = RootWalletKeys::new(get_test_wallet_keys(seed)) + .expect("test wallet xpubs are distinct"); let nu6_3 = NetworkUpgrade::Nu6_3.testnet_activation_height(); // ---- Server: build a keyless PSBT (one transparent input, one Ironwood output). ---- @@ -2353,7 +2357,8 @@ mod ironwood_v6_tests { /// A minimal v6 PSBT with one 2-of-3 P2SH input, a change output, and a shielded output. fn build_shield_psbt(seed: &str) -> ZcashBitGoPsbt { - let wallet_keys = RootWalletKeys::new(get_test_wallet_keys(seed)); + let wallet_keys = RootWalletKeys::new(get_test_wallet_keys(seed)) + .expect("test wallet xpubs are distinct"); let mut psbt = BitGoPsbt::new_zcash_v6_at_height( Network::ZcashTestnet, &wallet_keys, @@ -2393,6 +2398,7 @@ mod ironwood_v6_tests { /// `bitgo_key()`'s raw pubkey matches what's actually in this PSBT's `bip32_derivation` entries. fn root_wallet_keys(seed: &str) -> RootWalletKeys { RootWalletKeys::new(get_test_wallet_keys(seed)) + .expect("test wallet xpubs are distinct") } /// `sign_ironwood_v6`: the user signs first (setting `out_ciphertext` via its ECDH-derived @@ -2702,7 +2708,8 @@ mod ironwood_v6_tests { #[test] fn sign_ironwood_v6_rejects_a_non_v6_psbt() { let seed = "ironwood_v6_sign_api_v4"; - let wallet_keys = RootWalletKeys::new(get_test_wallet_keys(seed)); + let wallet_keys = RootWalletKeys::new(get_test_wallet_keys(seed)) + .expect("test wallet xpubs are distinct"); let mut z = ZcashBitGoPsbt::new( Network::ZcashTestnet, &wallet_keys, @@ -2725,7 +2732,8 @@ mod ironwood_v6_tests { /// branch id that only fails at broadcast. #[test] fn new_v6_at_height_rejects_pre_nu6_3_height() { - let wallet_keys = RootWalletKeys::new(get_test_wallet_keys("v6_height")); + let wallet_keys = RootWalletKeys::new(get_test_wallet_keys("v6_height")) + .expect("test wallet xpubs are distinct"); let nu6_3 = NetworkUpgrade::Nu6_3.testnet_activation_height(); let err = ZcashBitGoPsbt::new_v6_at_height( Network::ZcashTestnet, @@ -2834,7 +2842,8 @@ mod ironwood_v6_tests { /// `ironwood_shielded_outputs_info` reports an empty Vec, neither erroring. #[test] fn unsigned_v6_txid_and_shielded_output_info_handle_no_shielded_output_ever_added() { - let wallet_keys = RootWalletKeys::new(get_test_wallet_keys("v6_never_shielded")); + let wallet_keys = RootWalletKeys::new(get_test_wallet_keys("v6_never_shielded")) + .expect("test wallet xpubs are distinct"); let mut psbt = BitGoPsbt::new_zcash_v6_at_height( Network::ZcashTestnet, &wallet_keys, @@ -3015,7 +3024,8 @@ mod ironwood_v6_tests { /// fields rather than anything sighash/signature-dependent. #[test] fn ironwood_shielded_output_info_reflects_added_output() { - let wallet_keys = RootWalletKeys::new(get_test_wallet_keys("v6_output_info")); + let wallet_keys = RootWalletKeys::new(get_test_wallet_keys("v6_output_info")) + .expect("test wallet xpubs are distinct"); let mut psbt = BitGoPsbt::new_zcash_v6_at_height( Network::ZcashTestnet, &wallet_keys, @@ -3083,7 +3093,8 @@ mod ironwood_v6_tests { /// `recipient`, which alone cannot reconstruct a multi-receiver UA. #[test] fn add_ironwood_output_unified_address_round_trips() { - let wallet_keys = RootWalletKeys::new(get_test_wallet_keys("v6_ua_roundtrip")); + let wallet_keys = RootWalletKeys::new(get_test_wallet_keys("v6_ua_roundtrip")) + .expect("test wallet xpubs are distinct"); let mut psbt = BitGoPsbt::new_zcash_v6_at_height( Network::ZcashTestnet, &wallet_keys, @@ -3163,7 +3174,8 @@ mod ironwood_v6_tests { .unwrap() .expect("fixture UA has a transparent receiver"); - let wallet_keys = RootWalletKeys::new(get_test_wallet_keys("v4_ua_transparent_roundtrip")); + let wallet_keys = RootWalletKeys::new(get_test_wallet_keys("v4_ua_transparent_roundtrip")) + .expect("test wallet xpubs are distinct"); let mut psbt = BitGoPsbt::new_zcash( Network::ZcashTestnet, &wallet_keys, @@ -3254,7 +3266,8 @@ mod ironwood_v6_tests { .unwrap(); let ua = fixtures["testnetWallet"]["unified"].as_str().unwrap(); - let wallet_keys = RootWalletKeys::new(get_test_wallet_keys("v4_ua_transparent_mismatch")); + let wallet_keys = RootWalletKeys::new(get_test_wallet_keys("v4_ua_transparent_mismatch")) + .expect("test wallet xpubs are distinct"); let mut z = ZcashBitGoPsbt::new( Network::ZcashTestnet, &wallet_keys, @@ -3281,7 +3294,8 @@ mod ironwood_v6_tests { /// bug, not something to fall back to the plain `script`/`value` behavior for. #[test] fn add_transparent_output_rejects_an_unparseable_unified_address() { - let wallet_keys = RootWalletKeys::new(get_test_wallet_keys("v4_ua_transparent_bad_ua")); + let wallet_keys = RootWalletKeys::new(get_test_wallet_keys("v4_ua_transparent_bad_ua")) + .expect("test wallet xpubs are distinct"); let mut z = ZcashBitGoPsbt::new( Network::ZcashTestnet, &wallet_keys, @@ -3316,7 +3330,8 @@ mod ironwood_v6_tests { crate::zcash::unified_address::encode_orchard_receiver(&recipient, "tzec").unwrap(); let wallet_keys = - RootWalletKeys::new(get_test_wallet_keys("v4_ua_transparent_no_transparent")); + RootWalletKeys::new(get_test_wallet_keys("v4_ua_transparent_no_transparent")) + .expect("test wallet xpubs are distinct"); let mut z = ZcashBitGoPsbt::new( Network::ZcashTestnet, &wallet_keys, @@ -3358,7 +3373,8 @@ mod ironwood_v6_tests { let parsed = crate::zcash::unified_address::UnifiedAddress::parse(ua, "tzec").unwrap(); let transparent_script = parsed.transparent_script().unwrap().unwrap(); - let wallet_keys = RootWalletKeys::new(get_test_wallet_keys("v6_ua_transparent_rejected")); + let wallet_keys = RootWalletKeys::new(get_test_wallet_keys("v6_ua_transparent_rejected")) + .expect("test wallet xpubs are distinct"); let mut z = ZcashBitGoPsbt::new_v6_at_height( Network::ZcashTestnet, &wallet_keys, @@ -3502,7 +3518,8 @@ mod ironwood_v6_tests { /// `add_ironwood_output` is just its one-recipient special case. #[test] fn add_ironwood_outputs_supports_multiple_recipients() { - let wallet_keys = RootWalletKeys::new(get_test_wallet_keys("v6_multi_recipient")); + let wallet_keys = RootWalletKeys::new(get_test_wallet_keys("v6_multi_recipient")) + .expect("test wallet xpubs are distinct"); let mut psbt = BitGoPsbt::new_zcash_v6_at_height( Network::ZcashTestnet, &wallet_keys, @@ -3769,7 +3786,8 @@ mod ironwood_v6_tests { /// serialize-per-command round trip does. #[test] fn new_v6_bare_supports_the_cli_build_flow_end_to_end() { - let wallet_keys = RootWalletKeys::new(get_test_wallet_keys("v6_bare_cli_flow")); + let wallet_keys = RootWalletKeys::new(get_test_wallet_keys("v6_bare_cli_flow")) + .expect("test wallet xpubs are distinct"); let consensus_branch_id = crate::zcash::branch_id_for_height( NetworkUpgrade::Nu6_3.testnet_activation_height(), @@ -4014,7 +4032,8 @@ mod ironwood_v6_tests { // Reconstruct the fixture's state inside a v6 PSBT: transparent skeleton (scriptSigs // stripped), the spent output hydrated as witness_utxo (this is the extraction under test), // and the shielded action data as a stored PCZT. - let wallet_keys = RootWalletKeys::new(get_test_wallet_keys("shield1zec_psbt")); + let wallet_keys = RootWalletKeys::new(get_test_wallet_keys("shield1zec_psbt")) + .expect("test wallet xpubs are distinct"); let mut z = ZcashBitGoPsbt::new_v6( Network::ZcashTestnet, &wallet_keys, @@ -4117,7 +4136,8 @@ mod ironwood_v6_tests { .verify_v6_signature_with_xpub(&secp, 0, wallet_keys.backup_key()) .unwrap()); // A stranger's xpub has no matching fingerprint in the input at all. - let stranger = RootWalletKeys::new(get_test_wallet_keys("a-different-wallet")); + let stranger = RootWalletKeys::new(get_test_wallet_keys("a-different-wallet")) + .expect("test wallet xpubs are distinct"); assert!(!z .verify_v6_signature_with_xpub(&secp, 0, stranger.user_key()) .unwrap()); @@ -4178,7 +4198,8 @@ mod ironwood_v6_tests { // "no derivation path" must not silently answer false before the v6 check. Backup key is // absent from this input's 2-of-3 script, and a stranger's xpub matches no fingerprint at // all; both would have returned Ok(false) before the guard was hoisted. - let stranger_wallet = RootWalletKeys::new(get_test_wallet_keys("an-unrelated-wallet")); + let stranger_wallet = RootWalletKeys::new(get_test_wallet_keys("an-unrelated-wallet")) + .expect("test wallet xpubs are distinct"); for xpub in [wallet_keys.backup_key(), stranger_wallet.user_key()] { let err = generic .verify_signature_with_xpub(&secp, 0, xpub) diff --git a/packages/wasm-utxo/src/fixed_script_wallet/test_utils/fixtures.rs b/packages/wasm-utxo/src/fixed_script_wallet/test_utils/fixtures.rs index 5ff696d848a..6981532a235 100644 --- a/packages/wasm-utxo/src/fixed_script_wallet/test_utils/fixtures.rs +++ b/packages/wasm-utxo/src/fixed_script_wallet/test_utils/fixtures.rs @@ -84,6 +84,7 @@ impl XprvTriple { pub fn to_root_wallet_keys(&self) -> RootWalletKeys { let secp = crate::bitcoin::secp256k1::Secp256k1::new(); RootWalletKeys::new(self.0.map(|x| Xpub::from_priv(&secp, &x))) + .expect("fixture wallet xpubs are distinct") } } diff --git a/packages/wasm-utxo/src/fixed_script_wallet/test_utils/mod.rs b/packages/wasm-utxo/src/fixed_script_wallet/test_utils/mod.rs index f0bc210298a..497334e52d7 100644 --- a/packages/wasm-utxo/src/fixed_script_wallet/test_utils/mod.rs +++ b/packages/wasm-utxo/src/fixed_script_wallet/test_utils/mod.rs @@ -38,7 +38,7 @@ pub fn get_test_wallet_keys(seed: &str) -> XpubTriple { pub fn create_external_output(seed: &str) -> PsbtOutput { let xpubs = get_test_wallet_keys(seed); let _scripts = WalletScripts::from_wallet_keys( - &RootWalletKeys::new(xpubs), + &RootWalletKeys::new(xpubs).expect("test wallet xpubs are distinct"), OutputScriptType::P2wsh, &chain_index_path( Chain::new(OutputScriptType::P2wsh, Scope::External).value(), diff --git a/packages/wasm-utxo/src/fixed_script_wallet/wallet_keys.rs b/packages/wasm-utxo/src/fixed_script_wallet/wallet_keys.rs index 706133bf718..6a09eecc4c0 100644 --- a/packages/wasm-utxo/src/fixed_script_wallet/wallet_keys.rs +++ b/packages/wasm-utxo/src/fixed_script_wallet/wallet_keys.rs @@ -11,6 +11,63 @@ pub type XpubTriple = [Xpub; 3]; pub type PubTriple = [CompressedPublicKey; 3]; +#[derive(Debug, strum::IntoStaticStr)] +pub enum WalletKeyError { + DuplicateRootKeys { first: &'static str, second: &'static str }, + DuplicateDerivedKeys { first: &'static str, second: &'static str }, +} + +impl std::fmt::Display for WalletKeyError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::DuplicateRootKeys { first, second } => write!( + f, + "Wallet role xpubs must be distinct: {first} and {second} are identical" + ), + Self::DuplicateDerivedKeys { first, second } => write!( + f, + "Wallet role public keys must be distinct: {first} and {second} are identical" + ), + } + } +} + +impl std::error::Error for WalletKeyError {} +crate::impl_wasm_error_code!(WalletKeyError); + +const ROLE_NAMES: [&str; 3] = ["user", "backup", "BitGo"]; + +fn duplicate_pair(keys: &[T; 3]) -> Option<(usize, usize)> { + for first in 0..keys.len() { + for second in first + 1..keys.len() { + if keys[first] == keys[second] { + return Some((first, second)); + } + } + } + None +} + +fn require_distinct_root_xpubs(xpubs: &XpubTriple) -> Result<(), WalletKeyError> { + if let Some((first, second)) = duplicate_pair(xpubs) { + return Err(WalletKeyError::DuplicateRootKeys { + first: ROLE_NAMES[first], + second: ROLE_NAMES[second], + }); + } + Ok(()) +} + +pub fn require_distinct_pubkeys(keys: &PubTriple) -> Result<(), WalletKeyError> { + if let Some((first, second)) = duplicate_pair(keys) { + return Err(WalletKeyError::DuplicateDerivedKeys { + first: ROLE_NAMES[first], + second: ROLE_NAMES[second], + }); + } + Ok(()) +} + pub fn to_pub_triple(xpubs: &XpubTriple) -> PubTriple { xpubs .iter() @@ -44,7 +101,8 @@ impl RootWalletKeys { pub fn new_with_derivation_prefixes( xpubs: XpubTriple, derivation_prefixes: [DerivationPath; 3], - ) -> Self { + ) -> Result { + require_distinct_root_xpubs(&xpubs)?; let secp = Secp256k1::new(); // Pre-derive keys to prefix level (e.g., m/0/0) @@ -53,19 +111,19 @@ impl RootWalletKeys { .zip(derivation_prefixes.iter()) .map(|(xpub, prefix)| { xpub.derive_pub(&secp, prefix) - .expect("valid prefix derivation") + .map_err(|e| WasmUtxoError::new(&format!("Error deriving xpub: {}", e))) }) - .collect::>() + .collect::, _>>()? .try_into() - .expect("3 keys"); + .map_err(|_| WasmUtxoError::new("Expected exactly 3 derived xpubs"))?; - Self { + Ok(Self { xpubs, derivation_prefixes, prefix_derived, derivation_cache: RefCell::new(HashMap::new()), secp, - } + }) } pub fn user_key(&self) -> &Xpub { @@ -80,7 +138,7 @@ impl RootWalletKeys { &self.xpubs[2] } - pub fn new(xpubs: XpubTriple) -> Self { + pub fn new(xpubs: XpubTriple) -> Result { Self::new_with_derivation_prefixes( xpubs, [ @@ -186,6 +244,55 @@ pub mod tests { let xprvs = get_test_wallet_xprvs(seed); let secp = crate::bitcoin::key::Secp256k1::new(); RootWalletKeys::new(xprvs.map(|x| Xpub::from_priv(&secp, &x))) + .expect("test wallet xpubs are distinct") + } + + #[test] + fn duplicate_root_roles_are_rejected_with_default_and_custom_prefixes() { + use super::WalletKeyError; + use crate::bitcoin::bip32::DerivationPath; + use crate::error::WasmUtxoError; + use std::str::FromStr; + + let xprvs = get_test_wallet_xprvs("duplicate-root-roles"); + let secp = crate::bitcoin::secp256k1::Secp256k1::new(); + let xpubs = xprvs.map(|x| Xpub::from_priv(&secp, &x)); + + for (first, second) in [(0, 1), (0, 2), (1, 2)] { + let mut duplicate = xpubs; + duplicate[second] = duplicate[first]; + let error = RootWalletKeys::new(duplicate).unwrap_err(); + assert!(matches!( + error, + WasmUtxoError::WalletKey(WalletKeyError::DuplicateRootKeys { .. }) + )); + } + + let error = RootWalletKeys::new([xpubs[0]; 3]).unwrap_err(); + assert!(matches!( + error, + WasmUtxoError::WalletKey(WalletKeyError::DuplicateRootKeys { .. }) + )); + + let prefixes = [ + DerivationPath::from_str("m/0/0").unwrap(), + DerivationPath::from_str("m/1/0").unwrap(), + DerivationPath::from_str("m/2/0").unwrap(), + ]; + let error = RootWalletKeys::new_with_derivation_prefixes( + [xpubs[0], xpubs[0], xpubs[2]], + prefixes.clone(), + ) + .unwrap_err(); + assert!(matches!( + error, + WasmUtxoError::WalletKey(WalletKeyError::DuplicateRootKeys { .. }) + )); + + let distinct = RootWalletKeys::new_with_derivation_prefixes(xpubs, prefixes).unwrap(); + assert!(distinct + .derive_path(&crate::fixed_script_wallet::wallet_scripts::chain_index_path(0, 0)) + .is_ok()); } #[test] diff --git a/packages/wasm-utxo/src/fixed_script_wallet/wallet_scripts/bitgo_musig.rs b/packages/wasm-utxo/src/fixed_script_wallet/wallet_scripts/bitgo_musig.rs index e6caf397b4f..b773f157214 100644 --- a/packages/wasm-utxo/src/fixed_script_wallet/wallet_scripts/bitgo_musig.rs +++ b/packages/wasm-utxo/src/fixed_script_wallet/wallet_scripts/bitgo_musig.rs @@ -14,7 +14,7 @@ use crate::bitcoin::hashes::{sha256, Hash, HashEngine}; use crate::bitcoin::secp256k1::{Parity, PublicKey, Scalar, Secp256k1, XOnlyPublicKey}; /// Error types for BitGo MuSig2 operations -#[derive(Debug)] +#[derive(Debug, strum::IntoStaticStr)] pub enum BitGoMusigError { InvalidPubkeyCount(String), InvalidPubkey(String), @@ -32,6 +32,7 @@ impl std::fmt::Display for BitGoMusigError { } impl std::error::Error for BitGoMusigError {} +crate::impl_wasm_error_code!(BitGoMusigError); /// BIP340-style tagged hash fn tagged_hash(tag: &str, msg: &[u8]) -> [u8; 32] { diff --git a/packages/wasm-utxo/src/fixed_script_wallet/wallet_scripts/checkmultisig.rs b/packages/wasm-utxo/src/fixed_script_wallet/wallet_scripts/checkmultisig.rs index c61d542edd7..0a5763f949b 100644 --- a/packages/wasm-utxo/src/fixed_script_wallet/wallet_scripts/checkmultisig.rs +++ b/packages/wasm-utxo/src/fixed_script_wallet/wallet_scripts/checkmultisig.rs @@ -1,20 +1,22 @@ use crate::bitcoin::blockdata::opcodes::all::OP_CHECKMULTISIG; use crate::bitcoin::blockdata::script::Builder; use crate::bitcoin::{CompressedPublicKey, ScriptBuf}; -use crate::fixed_script_wallet::wallet_keys::PubTriple; +use crate::error::WasmUtxoError; +use crate::fixed_script_wallet::wallet_keys::{require_distinct_pubkeys, PubTriple}; /// Build bare multisig script. Needs to wrapped to be useful as an output script. -pub fn build_multisig_script_2_of_3(keys: &PubTriple) -> ScriptBuf { +pub fn build_multisig_script_2_of_3(keys: &PubTriple) -> Result { + require_distinct_pubkeys(keys)?; let quorum = 2; let total_count = 3; let mut builder = Builder::default().push_int(quorum as i64); for key in keys { builder = builder.push_slice(key.to_bytes()) } - builder + Ok(builder .push_int(total_count as i64) .push_opcode(OP_CHECKMULTISIG) - .into_script() + .into_script()) } pub fn parse_multisig_script_2_of_3(script: &ScriptBuf) -> Result { @@ -71,8 +73,11 @@ pub fn parse_multisig_script_2_of_3(script: &ScriptBuf) -> Result TaprootBuilder { builder } -fn build_p2tr_spend_info(keys: &PubTriple, p2tr_musig2: bool) -> TaprootSpendInfo { +fn build_p2tr_spend_info( + keys: &PubTriple, + p2tr_musig2: bool, +) -> Result { use super::bitgo_musig::key_agg_bitgo_p2tr_legacy; use super::bitgo_musig::key_agg_p2tr_musig2; use crate::bitcoin::secp256k1::Secp256k1; @@ -103,24 +107,26 @@ fn build_p2tr_spend_info(keys: &PubTriple, p2tr_musig2: bool) -> TaprootSpendInf let [user, _backup, bitgo] = *keys; let agg_key_bytes = if p2tr_musig2 { - key_agg_p2tr_musig2(&[user, bitgo]).expect("valid aggregation") + key_agg_p2tr_musig2(&[user, bitgo])? } else { - key_agg_bitgo_p2tr_legacy(&[user, bitgo]).expect("valid aggregation") + key_agg_bitgo_p2tr_legacy(&[user, bitgo])? }; - let internal_key = XOnlyPublicKey::from_slice(&agg_key_bytes).expect("valid xonly key"); + let internal_key = XOnlyPublicKey::from_slice(&agg_key_bytes) + .map_err(|e| WasmUtxoError::new(&format!("Invalid aggregated x-only key: {}", e)))?; build_taproot_builder(keys, p2tr_musig2) .finalize(&secp, internal_key) - .expect("valid taptree") + .map_err(|e| WasmUtxoError::new(&format!("Failed to finalize Taproot tree: {e:?}"))) } /// Build a TapTree for PSBT output from wallet keys pub fn build_tap_tree_for_output( pub_triple: &PubTriple, is_musig2: bool, -) -> miniscript::bitcoin::taproot::TapTree { +) -> Result { + require_distinct_pubkeys(pub_triple)?; miniscript::bitcoin::taproot::TapTree::try_from(build_taproot_builder(pub_triple, is_musig2)) - .expect("valid tap tree") + .map_err(|e| WasmUtxoError::new(&format!("Invalid Taproot tree: {e:?}"))) } /// Create tap key origins for outputs with multiple leaf hashes per key. @@ -131,7 +137,7 @@ pub fn create_tap_bip32_derivation_for_output( index: u32, pub_triple: &PubTriple, is_musig2: bool, -) -> std::collections::BTreeMap< +) -> Result, @@ -140,7 +146,8 @@ pub fn create_tap_bip32_derivation_for_output( miniscript::bitcoin::bip32::DerivationPath, ), ), -> { +>, WasmUtxoError> { + require_distinct_pubkeys(pub_triple)?; use crate::fixed_script_wallet::derivation_path; use miniscript::bitcoin::secp256k1::{PublicKey, Secp256k1}; use miniscript::bitcoin::taproot::{LeafVersion, TapLeafHash}; @@ -183,7 +190,7 @@ pub fn create_tap_bip32_derivation_for_output( map.insert(x_only, (key_leaf_hashes, (xpub.fingerprint(), path))); } - map + Ok(map) } #[derive(Debug)] @@ -192,9 +199,13 @@ pub struct ScriptP2tr { } impl ScriptP2tr { - pub fn new(keys: &PubTriple, p2tr_musig2: bool) -> ScriptP2tr { - let spend_info = build_p2tr_spend_info(keys, p2tr_musig2); - ScriptP2tr { spend_info } + pub fn new( + keys: &PubTriple, + p2tr_musig2: bool, + ) -> Result { + require_distinct_pubkeys(keys)?; + let spend_info = build_p2tr_spend_info(keys, p2tr_musig2)?; + Ok(ScriptP2tr { spend_info }) } pub fn output_script(&self) -> ScriptBuf { @@ -256,13 +267,14 @@ pub struct ScriptP2mr { impl ScriptP2mr { /// Build a P2MR wallet script from a public key triple. - pub fn new(keys: &PubTriple) -> ScriptP2mr { + pub fn new(keys: &PubTriple) -> Result { + require_distinct_pubkeys(keys)?; let tree = build_p2mr_script_tree(keys); let info = build_p2mr_tree(&tree); - ScriptP2mr { + Ok(ScriptP2mr { merkle_root: info.merkle_root, leaves: info.leaves, - } + }) } /// Return the 34-byte P2MR scriptPubKey: `OP_2 OP_PUSHBYTES_32 `. @@ -369,7 +381,7 @@ mod tests { for (i, fixture) in p2mr_fixtures().iter().enumerate() { let triple = pub_triple_from_hex(fixture.pubkeys[0], fixture.pubkeys[1], fixture.pubkeys[2]); - let script = ScriptP2mr::new(&triple); + let script = ScriptP2mr::new(&triple).unwrap(); // Verify merkle root assert_eq!( @@ -482,7 +494,7 @@ mod tests { "028714039c6866c27eb6885ffbb4085964a603140e5a39b0fa29b1d9839212f9a2", "03203ab799ce28e2cca044f594c69275050af4bb0854ad730a8f74622342300e64", ); - let script = ScriptP2mr::new(&triple); + let script = ScriptP2mr::new(&triple).unwrap(); let spk_bytes = script.output_script().to_bytes(); assert_eq!( spk_bytes[0], 0x52, @@ -491,7 +503,7 @@ mod tests { assert_eq!(spk_bytes.len(), 34, "P2MR scriptPubKey must be 34 bytes"); // Compare: P2TR for same keys would start with 0x51 - let p2tr = ScriptP2tr::new(&triple, false); + let p2tr = ScriptP2tr::new(&triple, false).unwrap(); assert_eq!( p2tr.output_script().to_bytes()[0], 0x51, @@ -527,7 +539,7 @@ mod tests { pubkeys.try_into().expect("Failed to convert to array"); // Generate scripts using the from_p2tr method - let spend_info = ScriptP2tr::new(&pub_triple, use_musig2); + let spend_info = ScriptP2tr::new(&pub_triple, use_musig2).unwrap(); let internal_key = spend_info.spend_info.internal_key().serialize(); assert_eq!( @@ -555,6 +567,27 @@ mod tests { test_p2tr_output_scripts_helper("p2tr", false); } + #[test] + fn test_p2tr_aggregation_errors_are_propagated() { + use super::super::bitgo_musig::BitGoMusigError; + + let triple = pub_triple_from_hex( + "02d20a62701c54f6eb3abb9f964b0e29ff90ffa3b4e3fcb73e7c67d4950fa6e3c7", + "028714039c6866c27eb6885ffbb4085964a603140e5a39b0fa29b1d9839212f9a2", + "03203ab799ce28e2cca044f594c69275050af4bb0854ad730a8f74622342300e64", + ); + let duplicate_user_bitgo = [triple[0], triple[1], triple[0]]; + + for musig2 in [false, true] { + assert!(matches!( + build_p2tr_spend_info(&duplicate_user_bitgo, musig2), + Err(WasmUtxoError::BitGoMusig( + BitGoMusigError::InvalidPubkeyCount(_) + )) + )); + } + } + #[test] fn test_p2tr_musig2_output_scripts_from_fixture() { test_p2tr_output_scripts_helper("p2trMusig2", true); diff --git a/packages/wasm-utxo/src/fixed_script_wallet/wallet_scripts/mod.rs b/packages/wasm-utxo/src/fixed_script_wallet/wallet_scripts/mod.rs index 4974e4a184f..f78e8e5222d 100644 --- a/packages/wasm-utxo/src/fixed_script_wallet/wallet_scripts/mod.rs +++ b/packages/wasm-utxo/src/fixed_script_wallet/wallet_scripts/mod.rs @@ -20,7 +20,9 @@ use crate::bitcoin::bip32::{ChildNumber, DerivationPath, Fingerprint}; use crate::bitcoin::secp256k1::PublicKey as Secp256k1PublicKey; use crate::bitcoin::{ScriptBuf, TapLeafHash, XOnlyPublicKey}; use crate::error::WasmUtxoError; -use crate::fixed_script_wallet::wallet_keys::{to_pub_triple, PubTriple, RootWalletKeys}; +use crate::fixed_script_wallet::wallet_keys::{ + require_distinct_pubkeys, to_pub_triple, PubTriple, RootWalletKeys, +}; use crate::Network; use std::collections::BTreeMap; use std::str::FromStr; @@ -48,17 +50,18 @@ impl WalletScripts { script_type: OutputScriptType, script_support: &OutputScriptSupport, ) -> Result { + require_distinct_pubkeys(keys)?; match script_type { OutputScriptType::P2sh => { script_support.assert_legacy()?; - let script = build_multisig_script_2_of_3(keys); + let script = build_multisig_script_2_of_3(keys)?; Ok(WalletScripts::P2sh(ScriptP2sh { redeem_script: script, })) } OutputScriptType::P2shP2wsh => { script_support.assert_segwit()?; - let script = build_multisig_script_2_of_3(keys); + let script = build_multisig_script_2_of_3(keys)?; Ok(WalletScripts::P2shP2wsh(ScriptP2shP2wsh { redeem_script: script.clone().to_p2wsh(), witness_script: script, @@ -66,22 +69,22 @@ impl WalletScripts { } OutputScriptType::P2wsh => { script_support.assert_segwit()?; - let script = build_multisig_script_2_of_3(keys); + let script = build_multisig_script_2_of_3(keys)?; Ok(WalletScripts::P2wsh(ScriptP2wsh { witness_script: script, })) } OutputScriptType::P2trLegacy => { script_support.assert_taproot()?; - Ok(WalletScripts::P2trLegacy(ScriptP2tr::new(keys, false))) + Ok(WalletScripts::P2trLegacy(ScriptP2tr::new(keys, false)?)) } OutputScriptType::P2trMusig2 => { script_support.assert_taproot()?; - Ok(WalletScripts::P2trMusig2(ScriptP2tr::new(keys, true))) + Ok(WalletScripts::P2trMusig2(ScriptP2tr::new(keys, true)?)) } OutputScriptType::P2mr => { script_support.assert_p2mr()?; - Ok(WalletScripts::P2mr(ScriptP2mr::new(keys))) + Ok(WalletScripts::P2mr(ScriptP2mr::new(keys)?)) } } } @@ -528,6 +531,39 @@ mod tests { assert!(WalletScripts::from_wallet_keys(&keys, P2trMusig2, p, &btc_support).is_ok()); } + #[test] + fn duplicate_derived_roles_are_rejected_for_every_script_family() { + use crate::error::WasmUtxoError; + use crate::fixed_script_wallet::WalletKeyError; + + let wallet_keys = get_test_wallet_keys("duplicate-derived-roles"); + let derived = wallet_keys + .derive_path(&chain_index_path(0, 0)) + .unwrap(); + let distinct = to_pub_triple(&derived); + let support = Network::Bitcoin.output_script_support(); + + for &script_type in OutputScriptType::all() { + for (first, second) in [(0, 1), (0, 2), (1, 2)] { + let mut duplicate = distinct; + duplicate[second] = duplicate[first]; + assert!(matches!( + WalletScripts::new(&duplicate, script_type, &support), + Err(WasmUtxoError::WalletKey( + WalletKeyError::DuplicateDerivedKeys { .. } + )) + )); + } + + assert!(matches!( + WalletScripts::new(&[distinct[0]; 3], script_type, &support), + Err(WasmUtxoError::WalletKey( + WalletKeyError::DuplicateDerivedKeys { .. } + )) + )); + } + } + #[test] fn test_output_script_type_from_str() { use OutputScriptType::*; diff --git a/packages/wasm-utxo/src/inspect/psbt.rs b/packages/wasm-utxo/src/inspect/psbt.rs index e13a2ca3a66..77f05daaa68 100644 --- a/packages/wasm-utxo/src/inspect/psbt.rs +++ b/packages/wasm-utxo/src/inspect/psbt.rs @@ -803,7 +803,8 @@ mod ironwood_v6_tests { crate::fixed_script_wallet::bitgo_psbt::ZcashBitGoPsbt, [SecretKeyTriple; 1], ) { - let wallet_keys = RootWalletKeys::new(get_test_wallet_keys(seed)); + let wallet_keys = RootWalletKeys::new(get_test_wallet_keys(seed)) + .expect("test wallet xpubs are distinct"); let mut psbt = BitGoPsbt::new_zcash_v6_at_height( NetEnum::ZcashTestnet, &wallet_keys, diff --git a/packages/wasm-utxo/src/wasm/wallet_keys.rs b/packages/wasm-utxo/src/wasm/wallet_keys.rs index aa552ab0752..2e4b039a59d 100644 --- a/packages/wasm-utxo/src/wasm/wallet_keys.rs +++ b/packages/wasm-utxo/src/wasm/wallet_keys.rs @@ -42,14 +42,7 @@ impl WasmRootWalletKeys { bitgo: &WasmBIP32, ) -> Result { let xpubs = [user.to_xpub()?, backup.to_xpub()?, bitgo.to_xpub()?]; - let inner = RootWalletKeys::new_with_derivation_prefixes( - xpubs, - [ - DerivationPath::from_str("m/0/0").unwrap(), - DerivationPath::from_str("m/0/0").unwrap(), - DerivationPath::from_str("m/0/0").unwrap(), - ], - ); + let inner = RootWalletKeys::new(xpubs)?; Ok(WasmRootWalletKeys { inner }) } @@ -93,7 +86,7 @@ impl WasmRootWalletKeys { .try_into() .map_err(|_| WasmUtxoError::new("Failed to convert derivation paths"))?; - let inner = RootWalletKeys::new_with_derivation_prefixes(xpubs, derivation_paths); + let inner = RootWalletKeys::new_with_derivation_prefixes(xpubs, derivation_paths)?; Ok(WasmRootWalletKeys { inner }) } diff --git a/packages/wasm-utxo/test/fixedScript/walletKeys.ts b/packages/wasm-utxo/test/fixedScript/walletKeys.ts new file mode 100644 index 00000000000..d8a2d8a24e0 --- /dev/null +++ b/packages/wasm-utxo/test/fixedScript/walletKeys.ts @@ -0,0 +1,77 @@ +import assert from "node:assert"; +import * as utxolib from "@bitgo/utxo-lib"; + +import { RootWalletKeys } from "../../js/fixedScriptWallet/RootWalletKeys.js"; +import { outputScript } from "../../js/fixedScriptWallet/address.js"; +import { WasmBIP32, WasmRootWalletKeys } from "../../js/wasm/wasm_utxo.js"; + +type Triple = [T, T, T]; + +function assertDuplicateRootError(fn: () => unknown): void { + assert.throws(fn, (error: unknown) => { + assert.ok(error instanceof Error); + const wasmError = error as Error & { code?: string }; + return ( + wasmError.code === "WalletKeyError.DuplicateRootKeys" && + wasmError.message.includes("Wallet role xpubs must be distinct") + ); + }); +} + +describe("fixed-script wallet role key uniqueness", function () { + const xpubs = utxolib.testutil + .getKeyTriple("duplicate-wallet-role-keys") + .map((key) => key.neutered().toBase58()) as Triple; + const duplicatePairs: Triple[] = [ + [xpubs[0], xpubs[0], xpubs[2]], + [xpubs[0], xpubs[1], xpubs[0]], + [xpubs[0], xpubs[1], xpubs[1]], + ]; + + it("rejects repeated roles in TypeScript constructors", function () { + for (const duplicate of duplicatePairs) { + assertDuplicateRootError(() => RootWalletKeys.fromXpubs(duplicate)); + } + + assertDuplicateRootError(() => + RootWalletKeys.fromXpubs([xpubs[0], xpubs[0], xpubs[0]]), + ); + assertDuplicateRootError(() => + RootWalletKeys.withDerivationPrefixes( + [xpubs[0], xpubs[0], xpubs[2]], + ["m/0/0", "m/1/0", "m/2/0"], + ), + ); + }); + + it("rejects repeated roles in direct WASM constructors", function () { + const wasmXpubs = xpubs.map((xpub) => WasmBIP32.from_xpub(xpub)) as Triple; + + for (const duplicate of duplicatePairs) { + const wasmDuplicate = duplicate.map((xpub) => + WasmBIP32.from_xpub(xpub), + ) as Triple; + assertDuplicateRootError(() => new WasmRootWalletKeys(...wasmDuplicate)); + } + + assertDuplicateRootError(() => + WasmRootWalletKeys.with_derivation_prefixes( + wasmXpubs[0], + wasmXpubs[0], + wasmXpubs[2], + "m/0/0", + "m/1/0", + "m/2/0", + ), + ); + assertDuplicateRootError(() => + new WasmRootWalletKeys(wasmXpubs[0], wasmXpubs[0], wasmXpubs[0]), + ); + + // Failed construction must not poison subsequent operations in the WASM module. + const validWasmKeys = new WasmRootWalletKeys(...wasmXpubs); + assert.ok(validWasmKeys.user_key()); + const validKeys = RootWalletKeys.fromXpubs(xpubs); + assert.ok(outputScript(validKeys, 0, 0, "bitcoin").byteLength > 0); + }); +});