diff --git a/src/blockchain/rpc.rs b/src/blockchain/rpc.rs index d6a74d9c..e5940dec 100644 --- a/src/blockchain/rpc.rs +++ b/src/blockchain/rpc.rs @@ -35,7 +35,7 @@ use crate::bitcoin::hashes::hex::ToHex; use crate::bitcoin::{Network, OutPoint, Transaction, TxOut, Txid}; use crate::blockchain::*; use crate::database::{BatchDatabase, BatchOperations, DatabaseUtils}; -use crate::descriptor::get_checksum; +use crate::descriptor::calc_checksum; use crate::error::MissingCachedScripts; use crate::{BlockTime, Error, FeeRate, KeychainKind, LocalUtxo, TransactionDetails}; use bitcoin::Script; @@ -806,7 +806,7 @@ fn is_wallet_descriptor(client: &Client) -> Result { fn descriptor_from_script_pubkey(script: &Script) -> String { let desc = format!("raw({})", script.to_hex()); - format!("{}#{}", desc, get_checksum(&desc).unwrap()) + format!("{}#{}", desc, calc_checksum(&desc).unwrap()) } /// Factory of [`RpcBlockchain`] instances, implements [`BlockchainFactory`] diff --git a/src/descriptor/checksum.rs b/src/descriptor/checksum.rs index 5ed1151b..6be3a244 100644 --- a/src/descriptor/checksum.rs +++ b/src/descriptor/checksum.rs @@ -41,12 +41,24 @@ fn poly_mod(mut c: u64, val: u64) -> u64 { c } -/// Computes the checksum bytes of a descriptor -pub fn get_checksum_bytes(desc: &str) -> Result<[u8; 8], DescriptorError> { +/// Computes the checksum bytes of a descriptor. +/// `exclude_hash = true` ignores all data after the first '#' (inclusive). +pub(crate) fn calc_checksum_bytes_internal( + mut desc: &str, + exclude_hash: bool, +) -> Result<[u8; 8], DescriptorError> { let mut c = 1; let mut cls = 0; let mut clscount = 0; + let mut original_checksum = None; + if exclude_hash { + if let Some(split) = desc.split_once('#') { + desc = split.0; + original_checksum = Some(split.1); + } + } + for ch in desc.as_bytes() { let pos = INPUT_CHARSET .iter() @@ -72,37 +84,96 @@ pub fn get_checksum_bytes(desc: &str) -> Result<[u8; 8], DescriptorError> { checksum[j] = CHECKSUM_CHARSET[((c >> (5 * (7 - j))) & 31) as usize]; } + // if input data already had a checksum, check calculated checksum against original checksum + if let Some(original_checksum) = original_checksum { + if original_checksum.as_bytes() != checksum { + return Err(DescriptorError::InvalidDescriptorChecksum); + } + } + Ok(checksum) } +/// Compute the checksum bytes of a descriptor, excludes any existing checksum in the descriptor string from the calculation +pub fn calc_checksum_bytes(desc: &str) -> Result<[u8; 8], DescriptorError> { + calc_checksum_bytes_internal(desc, true) +} + +/// Compute the checksum of a descriptor, excludes any existing checksum in the descriptor string from the calculation +pub fn calc_checksum(desc: &str) -> Result { + // unsafe is okay here as the checksum only uses bytes in `CHECKSUM_CHARSET` + calc_checksum_bytes_internal(desc, true) + .map(|b| unsafe { String::from_utf8_unchecked(b.to_vec()) }) +} + +// TODO in release 0.25.0, remove get_checksum_bytes and get_checksum +// TODO in release 0.25.0, consolidate calc_checksum_bytes_internal into calc_checksum_bytes + +/// Compute the checksum bytes of a descriptor +#[deprecated( + since = "0.24.0", + note = "Use new `calc_checksum_bytes` function which excludes any existing checksum in the descriptor string before calculating the checksum hash bytes. See https://github.com/bitcoindevkit/bdk/pull/765." +)] +pub fn get_checksum_bytes(desc: &str) -> Result<[u8; 8], DescriptorError> { + calc_checksum_bytes_internal(desc, false) +} + /// Compute the checksum of a descriptor +#[deprecated( + since = "0.24.0", + note = "Use new `calc_checksum` function which excludes any existing checksum in the descriptor string before calculating the checksum hash. See https://github.com/bitcoindevkit/bdk/pull/765." +)] pub fn get_checksum(desc: &str) -> Result { // unsafe is okay here as the checksum only uses bytes in `CHECKSUM_CHARSET` - get_checksum_bytes(desc).map(|b| unsafe { String::from_utf8_unchecked(b.to_vec()) }) + calc_checksum_bytes_internal(desc, false) + .map(|b| unsafe { String::from_utf8_unchecked(b.to_vec()) }) } #[cfg(test)] mod test { use super::*; - use crate::descriptor::get_checksum; + use crate::descriptor::calc_checksum; - // test get_checksum() function; it should return the same value as Bitcoin Core + // test calc_checksum() function; it should return the same value as Bitcoin Core #[test] - fn test_get_checksum() { + fn test_calc_checksum() { let desc = "wpkh(tprv8ZgxMBicQKsPdpkqS7Eair4YxjcuuvDPNYmKX3sCniCf16tHEVrjjiSXEkFRnUH77yXc6ZcwHHcLNfjdi5qUvw3VDfgYiH5mNsj5izuiu2N/1/2/*)"; - assert_eq!(get_checksum(desc).unwrap(), "tqz0nc62"); + assert_eq!(calc_checksum(desc).unwrap(), "tqz0nc62"); let desc = "pkh(tpubD6NzVbkrYhZ4XHndKkuB8FifXm8r5FQHwrN6oZuWCz13qb93rtgKvD4PQsqC4HP4yhV3tA2fqr2RbY5mNXfM7RxXUoeABoDtsFUq2zJq6YK/44'/1'/0'/0/*)"; - assert_eq!(get_checksum(desc).unwrap(), "lasegmfs"); + assert_eq!(calc_checksum(desc).unwrap(), "lasegmfs"); + } + + // test calc_checksum() function; it should return the same value as Bitcoin Core even if the + // descriptor string includes a checksum hash + #[test] + fn test_calc_checksum_with_checksum_hash() { + let desc = "wpkh(tprv8ZgxMBicQKsPdpkqS7Eair4YxjcuuvDPNYmKX3sCniCf16tHEVrjjiSXEkFRnUH77yXc6ZcwHHcLNfjdi5qUvw3VDfgYiH5mNsj5izuiu2N/1/2/*)#tqz0nc62"; + assert_eq!(calc_checksum(desc).unwrap(), "tqz0nc62"); + + let desc = "pkh(tpubD6NzVbkrYhZ4XHndKkuB8FifXm8r5FQHwrN6oZuWCz13qb93rtgKvD4PQsqC4HP4yhV3tA2fqr2RbY5mNXfM7RxXUoeABoDtsFUq2zJq6YK/44'/1'/0'/0/*)#lasegmfs"; + assert_eq!(calc_checksum(desc).unwrap(), "lasegmfs"); + + let desc = "wpkh(tprv8ZgxMBicQKsPdpkqS7Eair4YxjcuuvDPNYmKX3sCniCf16tHEVrjjiSXEkFRnUH77yXc6ZcwHHcLNfjdi5qUvw3VDfgYiH5mNsj5izuiu2N/1/2/*)#tqz0nc26"; + assert!(matches!( + calc_checksum(desc).err(), + Some(DescriptorError::InvalidDescriptorChecksum) + )); + + let desc = "pkh(tpubD6NzVbkrYhZ4XHndKkuB8FifXm8r5FQHwrN6oZuWCz13qb93rtgKvD4PQsqC4HP4yhV3tA2fqr2RbY5mNXfM7RxXUoeABoDtsFUq2zJq6YK/44'/1'/0'/0/*)#lasegmsf"; + assert!(matches!( + calc_checksum(desc).err(), + Some(DescriptorError::InvalidDescriptorChecksum) + )); } #[test] - fn test_get_checksum_invalid_character() { + fn test_calc_checksum_invalid_character() { let sparkle_heart = unsafe { std::str::from_utf8_unchecked(&[240, 159, 146, 150]) }; let invalid_desc = format!("wpkh(tprv8ZgxMBicQKsPdpkqS7Eair4YxjcuuvDPNYmKX3sCniCf16tHEVrjjiSXEkFRnUH77yXc6ZcwHHcL{}fjdi5qUvw3VDfgYiH5mNsj5izuiu2N/1/2/*)", sparkle_heart); assert!(matches!( - get_checksum(&invalid_desc).err(), + calc_checksum(&invalid_desc).err(), Some(DescriptorError::InvalidDescriptorCharacter(invalid_char)) if invalid_char == sparkle_heart.as_bytes()[0] )); } diff --git a/src/descriptor/mod.rs b/src/descriptor/mod.rs index c8f4d29d..1329063a 100644 --- a/src/descriptor/mod.rs +++ b/src/descriptor/mod.rs @@ -37,7 +37,8 @@ pub mod error; pub mod policy; pub mod template; -pub use self::checksum::get_checksum; +pub use self::checksum::calc_checksum; +use self::checksum::calc_checksum_bytes; pub use self::error::Error as DescriptorError; pub use self::policy::Policy; use self::template::DescriptorTemplateOut; @@ -81,19 +82,15 @@ impl IntoWalletDescriptor for &str { secp: &SecpCtx, network: Network, ) -> Result<(ExtendedDescriptor, KeyMap), DescriptorError> { - let descriptor = if self.contains('#') { - let parts: Vec<&str> = self.splitn(2, '#').collect(); - if !get_checksum(parts[0]) - .ok() - .map(|computed| computed == parts[1]) - .unwrap_or(false) - { - return Err(DescriptorError::InvalidDescriptorChecksum); + let descriptor = match self.split_once('#') { + Some((desc, original_checksum)) => { + let checksum = calc_checksum_bytes(desc)?; + if original_checksum.as_bytes() != checksum { + return Err(DescriptorError::InvalidDescriptorChecksum); + } + desc } - - parts[0] - } else { - self + None => self, }; ExtendedDescriptor::parse_descriptor(secp, descriptor)? diff --git a/src/descriptor/policy.rs b/src/descriptor/policy.rs index 413fe6e3..964ec291 100644 --- a/src/descriptor/policy.rs +++ b/src/descriptor/policy.rs @@ -63,7 +63,7 @@ use crate::keys::ExtScriptContext; use crate::wallet::signer::{SignerId, SignersContainer}; use crate::wallet::utils::{After, Older, SecpCtx}; -use super::checksum::get_checksum; +use super::checksum::calc_checksum; use super::error::Error; use super::XKeyUtils; use bitcoin::util::psbt::{Input as PsbtInput, PartiallySignedTransaction as Psbt}; @@ -168,7 +168,7 @@ impl SatisfiableItem { /// Returns a unique id for the [`SatisfiableItem`] pub fn id(&self) -> String { - get_checksum(&serde_json::to_string(self).expect("Failed to serialize a SatisfiableItem")) + calc_checksum(&serde_json::to_string(self).expect("Failed to serialize a SatisfiableItem")) .expect("Failed to compute a SatisfiableItem id") } } diff --git a/src/wallet/mod.rs b/src/wallet/mod.rs index 54d76f13..c400c5a4 100644 --- a/src/wallet/mod.rs +++ b/src/wallet/mod.rs @@ -59,9 +59,10 @@ use utils::{check_nsequence_rbf, After, Older, SecpCtx}; use crate::blockchain::{GetHeight, NoopProgress, Progress, WalletSync}; use crate::database::memory::MemoryDatabase; use crate::database::{AnyDatabase, BatchDatabase, BatchOperations, DatabaseUtils, SyncTime}; +use crate::descriptor::checksum::calc_checksum_bytes_internal; use crate::descriptor::policy::BuildSatisfaction; use crate::descriptor::{ - get_checksum, into_wallet_descriptor_checked, DerivedDescriptor, DescriptorMeta, + calc_checksum, into_wallet_descriptor_checked, DerivedDescriptor, DescriptorMeta, ExtendedDescriptor, ExtractPolicy, IntoWalletDescriptor, Policy, XKeyUtils, }; use crate::error::{Error, MiniscriptPsbtError}; @@ -193,18 +194,20 @@ where let secp = Secp256k1::new(); let (descriptor, keymap) = into_wallet_descriptor_checked(descriptor, &secp, network)?; - database.check_descriptor_checksum( + Self::db_checksum( + &mut database, + &descriptor.to_string(), KeychainKind::External, - get_checksum(&descriptor.to_string())?.as_bytes(), )?; let signers = Arc::new(SignersContainer::build(keymap, &descriptor, &secp)); let (change_descriptor, change_signers) = match change_descriptor { Some(desc) => { let (change_descriptor, change_keymap) = into_wallet_descriptor_checked(desc, &secp, network)?; - database.check_descriptor_checksum( + Self::db_checksum( + &mut database, + &change_descriptor.to_string(), KeychainKind::Internal, - get_checksum(&change_descriptor.to_string())?.as_bytes(), )?; let change_signers = Arc::new(SignersContainer::build( @@ -212,9 +215,6 @@ where &change_descriptor, &secp, )); - // if !parsed.same_structure(descriptor.as_ref()) { - // return Err(Error::DifferentDescriptorStructure); - // } (Some(change_descriptor), change_signers) } @@ -232,6 +232,19 @@ where }) } + /// This checks the checksum within [`BatchDatabase`] twice (if needed). The first time with the + /// actual checksum, and the second time with the checksum of `descriptor+checksum`. The second + /// check is necessary for backwards compatibility of a checksum-inception bug. + fn db_checksum(db: &mut D, desc: &str, kind: KeychainKind) -> Result<(), Error> { + let checksum = calc_checksum_bytes_internal(desc, true)?; + if db.check_descriptor_checksum(kind, checksum).is_ok() { + return Ok(()); + } + + let checksum_inception = calc_checksum_bytes_internal(desc, false)?; + db.check_descriptor_checksum(kind, checksum_inception) + } + /// Get the Bitcoin network the wallet is using. pub fn network(&self) -> Network { self.network @@ -1781,14 +1794,14 @@ where .into_wallet_descriptor(secp, network)? .0 .to_string(); - let mut wallet_name = get_checksum(&descriptor[..descriptor.find('#').unwrap()])?; + let mut wallet_name = calc_checksum(&descriptor[..descriptor.find('#').unwrap()])?; if let Some(change_descriptor) = change_descriptor { let change_descriptor = change_descriptor .into_wallet_descriptor(secp, network)? .0 .to_string(); wallet_name.push_str( - get_checksum(&change_descriptor[..change_descriptor.find('#').unwrap()])?.as_str(), + calc_checksum(&change_descriptor[..change_descriptor.find('#').unwrap()])?.as_str(), ); } @@ -1860,15 +1873,38 @@ pub(crate) mod test { let (wallet, _, _) = get_funded_wallet(get_test_wpkh()); let checksum = wallet.descriptor_checksum(KeychainKind::External); assert_eq!(checksum.len(), 8); + assert_eq!( + calc_checksum(&wallet.descriptor.to_string()).unwrap(), + checksum + ); + } - let raw_descriptor = wallet - .descriptor - .to_string() - .split_once('#') - .unwrap() - .0 - .to_string(); - assert_eq!(get_checksum(&raw_descriptor).unwrap(), checksum); + #[test] + fn test_db_checksum() { + let (wallet, _, _) = get_funded_wallet(get_test_wpkh()); + let desc = wallet.descriptor.to_string(); + + let checksum = calc_checksum_bytes_internal(&desc, true).unwrap(); + let checksum_inception = calc_checksum_bytes_internal(&desc, false).unwrap(); + let checksum_invalid = [b'q'; 8]; + + let mut db = MemoryDatabase::new(); + db.check_descriptor_checksum(KeychainKind::External, checksum) + .expect("failed to save actual checksum"); + Wallet::db_checksum(&mut db, &desc, KeychainKind::External) + .expect("db that uses actual checksum should be supported"); + + let mut db = MemoryDatabase::new(); + db.check_descriptor_checksum(KeychainKind::External, checksum_inception) + .expect("failed to save checksum inception"); + Wallet::db_checksum(&mut db, &desc, KeychainKind::External) + .expect("db that uses checksum inception should be supported"); + + let mut db = MemoryDatabase::new(); + db.check_descriptor_checksum(KeychainKind::External, checksum_invalid) + .expect("failed to save invalid checksum"); + Wallet::db_checksum(&mut db, &desc, KeychainKind::External) + .expect_err("db that uses invalid checksum should fail"); } #[test]