From d03713e2e919d9a6ba98d2f6f1c2c38da9556a2f Mon Sep 17 00:00:00 2001 From: AmosOO7 Date: Mon, 11 May 2026 12:07:46 +0100 Subject: [PATCH 1/3] feat(wallet): validate descriptors contain only public keys in wallet_name_from_descriptor - Add check that parsed KeyMap is empty after descriptor parsing - Reject descriptors containing private keys with clear error message - Apply validation to both main and change descriptors - Add test_wallet_name_from_descriptor_public_key_check regression test Resolves TODO comment in wallet_name_from_descriptor function. Ensures wallet names are derived from public information only. --- src/wallet/mod.rs | 90 ++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 82 insertions(+), 8 deletions(-) diff --git a/src/wallet/mod.rs b/src/wallet/mod.rs index 1cdd1cc7b..cefd61313 100644 --- a/src/wallet/mod.rs +++ b/src/wallet/mod.rs @@ -2797,16 +2797,24 @@ where T: IntoWalletDescriptor, { // TODO: check descriptors contains only public keys - let descriptor = descriptor - .into_wallet_descriptor(secp, network_kind)? - .0 - .to_string(); + let (descriptor, keymap) = descriptor + .into_wallet_descriptor(secp, network_kind)?; + if !keymap.is_empty() { + return Err(DescriptorError::Miniscript(miniscript::Error::Unexpected( + "Descriptor must contain only public keys".to_string(), + ))); + } + let descriptor = descriptor.to_string(); let mut wallet_name = descriptor.split_once('#').unwrap().1.to_string(); if let Some(change_descriptor) = change_descriptor { - let change_descriptor = change_descriptor - .into_wallet_descriptor(secp, network_kind)? - .0 - .to_string(); + let (change_descriptor, change_keymap) = change_descriptor + .into_wallet_descriptor(secp, network_kind)?; + if !change_keymap.is_empty() { + return Err(DescriptorError::Miniscript(miniscript::Error::Unexpected( + "Change descriptor must contain only public keys".to_string(), + ))); + } + let change_descriptor = change_descriptor.to_string(); wallet_name.push_str(change_descriptor.split_once('#').unwrap().1); } @@ -3064,4 +3072,70 @@ mod test { let wallet = params.network(Network::Testnet).create_wallet_no_persist(); assert!(wallet.is_err()); } + #[test] + fn test_wallet_name_from_descriptor_public_key_check() { + use miniscript::Error::Unexpected; + + let secp = SecpCtx::new(); + + // Test with a public descriptor + let public_descriptor = "wpkh([9a6a2580/84'/1'/0']tpubDDnGNapGEY6AZAdQbfRJgMg9fvz8pUBrLwvyvUqEgcUfgzM6zc2eVK4vY9x9L5FJWdX8WumXuLEDV5zDZnTfbn87vLe9XceCFwTu9so9Kks/0/*)"; + let result = wallet_name_from_descriptor( + public_descriptor, + None, + NetworkKind::Test, + &secp, + ); + assert!(result.is_ok()); + // Optionally, assert the exact wallet name if known + // assert_eq!(result.unwrap(), "expected_checksum_part"); + + // Test with a private descriptor + let private_descriptor = "wpkh(tprv8ZgxMBicQKsPdWAHbugK2tjtVtRjKGixYVZUdL7xLHMgXZS6BFbFi1UDb1CHT25Z5PU1F9j7wGxwUiRhqz9E3nZRztikGUV6HoRDYcqPhM4/84'/1'/0'/*)"; + let result = wallet_name_from_descriptor( + private_descriptor, + None, + NetworkKind::Test, + &secp, + ); + assert!(matches!( + result, + Err(DescriptorError::Miniscript(Unexpected(..))) + )); + + // Test with change descriptor containing private keys + let public_change = "wpkh([9a6a2580/84'/1'/0']tpubDDnGNapGEY6AZAdQbfRJgMg9fvz8pUBrLwvyvUqEgcUfgzM6zc2eVK4vY9x9L5FJWdX8WumXuLEDV5zDZnTfbn87vLe9XceCFwTu9so9Kks/1/*)"; + let result = wallet_name_from_descriptor( + public_change, + Some(private_descriptor), + NetworkKind::Test, + &secp, + ); + assert!(matches!( + result, + Err(DescriptorError::Miniscript(Unexpected(..))) + )); + // Test with change descriptor containing public keys (should be ok) + let public_change_without_private = "wpkh([9a6a2580/84'/1'/0']tpubDDnGNapGEY6AZAdQbfRJgMg9fvz8pUBrLwvyvUqEgcUfgzM6zc2eVK4vY9x9L5FJWdX8WumXuLEDV5zDZnTfbn87vLe9XceCFwTu9so9Kks/1/*)"; + let result = wallet_name_from_descriptor( + public_change_without_private, + Some(public_descriptor), + NetworkKind::Test, + &secp, + ); + assert!(result.is_ok()); + + // Test with empty descriptor + let public_empty = ""; + let result = wallet_name_from_descriptor( + public_empty, + None, + NetworkKind::Test, + &secp, + ); + assert!(matches!( + result, + Err(DescriptorError::Miniscript(Unexpected(..))) + )); + } } From 64c937ce8b8dda69fb84937b615a1e4352064b43 Mon Sep 17 00:00:00 2001 From: AmosOO7 Date: Mon, 11 May 2026 12:10:41 +0100 Subject: [PATCH 2/3] feat(wallet): validate descriptors contain only public keys in wallet_name_from_descriptor - Add check that parsed KeyMap is empty after descriptor parsing - Reject descriptors containing private keys with clear error message - Apply validation to both main and change descriptors - Add test_wallet_name_from_descriptor_public_key_check regression test Resolves TODO comment in wallet_name_from_descriptor function. Ensures wallet names are derived from public information only. --- src/wallet/mod.rs | 29 +++++++---------------------- 1 file changed, 7 insertions(+), 22 deletions(-) diff --git a/src/wallet/mod.rs b/src/wallet/mod.rs index cefd61313..54a2c8e85 100644 --- a/src/wallet/mod.rs +++ b/src/wallet/mod.rs @@ -2797,8 +2797,7 @@ where T: IntoWalletDescriptor, { // TODO: check descriptors contains only public keys - let (descriptor, keymap) = descriptor - .into_wallet_descriptor(secp, network_kind)?; + let (descriptor, keymap) = descriptor.into_wallet_descriptor(secp, network_kind)?; if !keymap.is_empty() { return Err(DescriptorError::Miniscript(miniscript::Error::Unexpected( "Descriptor must contain only public keys".to_string(), @@ -2807,8 +2806,8 @@ where let descriptor = descriptor.to_string(); let mut wallet_name = descriptor.split_once('#').unwrap().1.to_string(); if let Some(change_descriptor) = change_descriptor { - let (change_descriptor, change_keymap) = change_descriptor - .into_wallet_descriptor(secp, network_kind)?; + let (change_descriptor, change_keymap) = + change_descriptor.into_wallet_descriptor(secp, network_kind)?; if !change_keymap.is_empty() { return Err(DescriptorError::Miniscript(miniscript::Error::Unexpected( "Change descriptor must contain only public keys".to_string(), @@ -3080,24 +3079,15 @@ mod test { // Test with a public descriptor let public_descriptor = "wpkh([9a6a2580/84'/1'/0']tpubDDnGNapGEY6AZAdQbfRJgMg9fvz8pUBrLwvyvUqEgcUfgzM6zc2eVK4vY9x9L5FJWdX8WumXuLEDV5zDZnTfbn87vLe9XceCFwTu9so9Kks/0/*)"; - let result = wallet_name_from_descriptor( - public_descriptor, - None, - NetworkKind::Test, - &secp, - ); + let result = wallet_name_from_descriptor(public_descriptor, None, NetworkKind::Test, &secp); assert!(result.is_ok()); // Optionally, assert the exact wallet name if known // assert_eq!(result.unwrap(), "expected_checksum_part"); // Test with a private descriptor let private_descriptor = "wpkh(tprv8ZgxMBicQKsPdWAHbugK2tjtVtRjKGixYVZUdL7xLHMgXZS6BFbFi1UDb1CHT25Z5PU1F9j7wGxwUiRhqz9E3nZRztikGUV6HoRDYcqPhM4/84'/1'/0'/*)"; - let result = wallet_name_from_descriptor( - private_descriptor, - None, - NetworkKind::Test, - &secp, - ); + let result = + wallet_name_from_descriptor(private_descriptor, None, NetworkKind::Test, &secp); assert!(matches!( result, Err(DescriptorError::Miniscript(Unexpected(..))) @@ -3127,12 +3117,7 @@ mod test { // Test with empty descriptor let public_empty = ""; - let result = wallet_name_from_descriptor( - public_empty, - None, - NetworkKind::Test, - &secp, - ); + let result = wallet_name_from_descriptor(public_empty, None, NetworkKind::Test, &secp); assert!(matches!( result, Err(DescriptorError::Miniscript(Unexpected(..))) From 7aaa1d321a92deb4bdccb9d4a7f72a3c6748fe2d Mon Sep 17 00:00:00 2001 From: AmosOO7 Date: Wed, 13 May 2026 21:15:42 +0100 Subject: [PATCH 3/3] feat(wallet): derive wallet_name_from_descriptor from public descriptor checksums - Update `wallet_name_from_descriptor` to compute descriptor checksum explicitly - Remove reliance on `to_string()` implicitly containing `#checksum` - Clarify that wallet name is defined by public descriptor checksums - Add regression test for equivalent xpub/xprv wallet names --- src/wallet/mod.rs | 94 +++++++++++++++-------------------------------- 1 file changed, 30 insertions(+), 64 deletions(-) diff --git a/src/wallet/mod.rs b/src/wallet/mod.rs index 54a2c8e85..e2dd6b792 100644 --- a/src/wallet/mod.rs +++ b/src/wallet/mod.rs @@ -66,9 +66,9 @@ pub(crate) mod utils; use crate::collections::{BTreeMap, HashMap, HashSet}; use crate::descriptor::{ - check_wallet_descriptor, error::Error as DescriptorError, policy::BuildSatisfaction, - DerivedDescriptor, DescriptorMeta, ExtendedDescriptor, ExtractPolicy, IntoWalletDescriptor, - Policy, XKeyUtils, + check_wallet_descriptor, checksum::calc_checksum, error::Error as DescriptorError, + policy::BuildSatisfaction, DerivedDescriptor, DescriptorMeta, ExtendedDescriptor, + ExtractPolicy, IntoWalletDescriptor, Policy, XKeyUtils, }; use crate::psbt::PsbtUtils; use crate::types::*; @@ -2784,9 +2784,10 @@ impl AsRef> for Wallet { } } -/// Deterministically generate a unique name given the descriptors defining the [`Wallet`]. +/// Generate a wallet name from the checksums of the wallet's public descriptors. /// -/// Compatible with [`wallet_name_from_descriptor`]. +/// The name is derived from the checksums of the provided descriptor(s), ensuring +/// it's based on public information only. pub fn wallet_name_from_descriptor( descriptor: T, change_descriptor: Option, @@ -2796,25 +2797,14 @@ pub fn wallet_name_from_descriptor( where T: IntoWalletDescriptor, { - // TODO: check descriptors contains only public keys - let (descriptor, keymap) = descriptor.into_wallet_descriptor(secp, network_kind)?; - if !keymap.is_empty() { - return Err(DescriptorError::Miniscript(miniscript::Error::Unexpected( - "Descriptor must contain only public keys".to_string(), - ))); - } - let descriptor = descriptor.to_string(); - let mut wallet_name = descriptor.split_once('#').unwrap().1.to_string(); + // Wallet name is defined by the checksums of the wallet's public descriptors. + let (descriptor, _keymap) = descriptor.into_wallet_descriptor(secp, network_kind)?; + let mut wallet_name = calc_checksum(&descriptor.to_string())?; + if let Some(change_descriptor) = change_descriptor { - let (change_descriptor, change_keymap) = + let (change_descriptor, _change_keymap) = change_descriptor.into_wallet_descriptor(secp, network_kind)?; - if !change_keymap.is_empty() { - return Err(DescriptorError::Miniscript(miniscript::Error::Unexpected( - "Change descriptor must contain only public keys".to_string(), - ))); - } - let change_descriptor = change_descriptor.to_string(); - wallet_name.push_str(change_descriptor.split_once('#').unwrap().1); + wallet_name.push_str(&calc_checksum(&change_descriptor.to_string())?); } Ok(wallet_name) @@ -3073,54 +3063,30 @@ mod test { } #[test] fn test_wallet_name_from_descriptor_public_key_check() { - use miniscript::Error::Unexpected; - let secp = SecpCtx::new(); // Test with a public descriptor - let public_descriptor = "wpkh([9a6a2580/84'/1'/0']tpubDDnGNapGEY6AZAdQbfRJgMg9fvz8pUBrLwvyvUqEgcUfgzM6zc2eVK4vY9x9L5FJWdX8WumXuLEDV5zDZnTfbn87vLe9XceCFwTu9so9Kks/0/*)"; - let result = wallet_name_from_descriptor(public_descriptor, None, NetworkKind::Test, &secp); - assert!(result.is_ok()); - // Optionally, assert the exact wallet name if known - // assert_eq!(result.unwrap(), "expected_checksum_part"); - - // Test with a private descriptor - let private_descriptor = "wpkh(tprv8ZgxMBicQKsPdWAHbugK2tjtVtRjKGixYVZUdL7xLHMgXZS6BFbFi1UDb1CHT25Z5PU1F9j7wGxwUiRhqz9E3nZRztikGUV6HoRDYcqPhM4/84'/1'/0'/*)"; - let result = + let public_descriptor = "wpkh([31a30ffd/84'/1'/0']tpubDCG4yNzDpNYw5ZMuR2usfbPKcnaKjFGwgyussBdhjy2mXmLWnzkwUTZBQPrQxPVcfwh6uFPN4Q7Jk2DPRFb2c4xbrStpqCbKzLkGhvcJvSx/1/*)#vn4aqs37"; + let public_result = + wallet_name_from_descriptor(public_descriptor, None, NetworkKind::Test, &secp); + assert!(public_result.is_ok()); + + // Test with equivalent private descriptor (should produce same name) + let private_descriptor = "wpkh(tprv8ZgxMBicQKsPctT28ZYaU77s1UFjHv7o7cafmDntdggZ2dFtNn38RYMzJiDVMBqnqBFDP8rHxsiVRudhyrqi6mgPc4gekgxChgnkTSxHAZ5/84'/1'/0'/1/*)#7z7rgndh"; + let private_result = wallet_name_from_descriptor(private_descriptor, None, NetworkKind::Test, &secp); - assert!(matches!( - result, - Err(DescriptorError::Miniscript(Unexpected(..))) - )); - - // Test with change descriptor containing private keys - let public_change = "wpkh([9a6a2580/84'/1'/0']tpubDDnGNapGEY6AZAdQbfRJgMg9fvz8pUBrLwvyvUqEgcUfgzM6zc2eVK4vY9x9L5FJWdX8WumXuLEDV5zDZnTfbn87vLe9XceCFwTu9so9Kks/1/*)"; - let result = wallet_name_from_descriptor( - public_change, - Some(private_descriptor), + assert!(private_result.is_ok()); + assert_eq!(public_result.unwrap(), private_result.unwrap()); // Same wallet name + + // Test with change descriptor + let change_descriptor = "wpkh([76011771/84'/1'/0']tpubDC3fWoucXCvSyfh6YbyHu1mSQdFjCz5Ejx62eUnRkKdr9bsHGgLEjAaCRNNuaeLjCttfz8sXgshqzawtgWvtozE84rH9BvQn2PUyMCiU1fT/1/*)#jgrerlc3"; + let result_with_change = wallet_name_from_descriptor( + public_descriptor, + Some(change_descriptor), NetworkKind::Test, &secp, ); - assert!(matches!( - result, - Err(DescriptorError::Miniscript(Unexpected(..))) - )); - // Test with change descriptor containing public keys (should be ok) - let public_change_without_private = "wpkh([9a6a2580/84'/1'/0']tpubDDnGNapGEY6AZAdQbfRJgMg9fvz8pUBrLwvyvUqEgcUfgzM6zc2eVK4vY9x9L5FJWdX8WumXuLEDV5zDZnTfbn87vLe9XceCFwTu9so9Kks/1/*)"; - let result = wallet_name_from_descriptor( - public_change_without_private, - Some(public_descriptor), - NetworkKind::Test, - &secp, - ); - assert!(result.is_ok()); - - // Test with empty descriptor - let public_empty = ""; - let result = wallet_name_from_descriptor(public_empty, None, NetworkKind::Test, &secp); - assert!(matches!( - result, - Err(DescriptorError::Miniscript(Unexpected(..))) - )); + assert!(result_with_change.is_ok()); + // Wallet name should be main_checksum + change_checksum } }