From 60057a7bf78f01bfb1149251f384de6432f94d59 Mon Sep 17 00:00:00 2001 From: Steve Myers Date: Mon, 24 Oct 2022 12:05:49 -0500 Subject: [PATCH] Deprecate backward compatible get_checksum_bytes, get_checksum functions Rename replacement functions calc_checksum_bytes and calc_checksum --- src/blockchain/rpc.rs | 4 +-- src/descriptor/checksum.rs | 61 ++++++++++++++++++++++++++++++++------ src/descriptor/mod.rs | 6 ++-- src/descriptor/policy.rs | 4 +-- src/wallet/mod.rs | 18 +++++------ 5 files changed, 68 insertions(+), 25 deletions(-) diff --git a/src/blockchain/rpc.rs b/src/blockchain/rpc.rs index b2c64ba5..b4a003c4 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; @@ -801,7 +801,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 9eedb45b..de2b0681 100644 --- a/src/descriptor/checksum.rs +++ b/src/descriptor/checksum.rs @@ -43,7 +43,10 @@ fn poly_mod(mut c: u64, val: u64) -> u64 { /// Computes the checksum bytes of a descriptor. /// `exclude_hash = true` ignores all data after the first '#' (inclusive). -pub fn get_checksum_bytes(mut desc: &str, exclude_hash: bool) -> Result<[u8; 8], DescriptorError> { +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; @@ -91,34 +94,74 @@ pub fn get_checksum_bytes(mut desc: &str, exclude_hash: bool) -> Result<[u8; 8], Ok(checksum) } +/// Compute the checksum bytes of a descriptor, any existing checksum hash will be excluded 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, any existing checksum hash will be excluded 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 hash 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 hash 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, true).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"); } #[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 4955f8ff..3b6e7d35 100644 --- a/src/descriptor/mod.rs +++ b/src/descriptor/mod.rs @@ -39,8 +39,8 @@ pub mod error; pub mod policy; pub mod template; -pub use self::checksum::get_checksum; -use self::checksum::get_checksum_bytes; +pub use self::checksum::calc_checksum; +use self::checksum::calc_checksum_bytes; pub use self::derived::{AsDerived, DerivedDescriptorKey}; pub use self::error::Error as DescriptorError; pub use self::policy::Policy; @@ -87,7 +87,7 @@ impl IntoWalletDescriptor for &str { ) -> Result<(ExtendedDescriptor, KeyMap), DescriptorError> { let descriptor = match self.split_once('#') { Some((desc, original_checksum)) => { - let checksum = get_checksum_bytes(desc, false)?; + let checksum = calc_checksum_bytes(desc)?; if original_checksum.as_bytes() != checksum { return Err(DescriptorError::InvalidDescriptorChecksum); } diff --git a/src/descriptor/policy.rs b/src/descriptor/policy.rs index 215078b6..8f64a5d2 100644 --- a/src/descriptor/policy.rs +++ b/src/descriptor/policy.rs @@ -60,7 +60,7 @@ use crate::keys::ExtScriptContext; use crate::wallet::signer::{SignerId, SignersContainer}; use crate::wallet::utils::{self, 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}; @@ -165,7 +165,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 737afe8d..8c52e82c 100644 --- a/src/wallet/mod.rs +++ b/src/wallet/mod.rs @@ -64,11 +64,11 @@ use utils::{check_nlocktime, 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::get_checksum_bytes; +use crate::descriptor::checksum::calc_checksum_bytes_internal; use crate::descriptor::derived::AsDerived; use crate::descriptor::policy::BuildSatisfaction; use crate::descriptor::{ - get_checksum, into_wallet_descriptor_checked, DerivedDescriptor, DerivedDescriptorMeta, + calc_checksum, into_wallet_descriptor_checked, DerivedDescriptor, DerivedDescriptorMeta, DescriptorMeta, DescriptorScripts, ExtendedDescriptor, ExtractPolicy, IntoWalletDescriptor, Policy, XKeyUtils, }; @@ -248,12 +248,12 @@ where /// 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 = get_checksum_bytes(desc, true)?; + let checksum = calc_checksum_bytes_internal(desc, true)?; if db.check_descriptor_checksum(kind, checksum).is_ok() { return Ok(()); } - let checksum_inception = get_checksum_bytes(desc, false)?; + let checksum_inception = calc_checksum_bytes_internal(desc, false)?; db.check_descriptor_checksum(kind, checksum_inception) } @@ -1878,14 +1878,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(), ); } @@ -1958,7 +1958,7 @@ pub(crate) mod test { let checksum = wallet.descriptor_checksum(KeychainKind::External); assert_eq!(checksum.len(), 8); assert_eq!( - get_checksum(&wallet.descriptor.to_string()).unwrap(), + calc_checksum(&wallet.descriptor.to_string()).unwrap(), checksum ); } @@ -1968,8 +1968,8 @@ pub(crate) mod test { let (wallet, _, _) = get_funded_wallet(get_test_wpkh()); let desc = wallet.descriptor.to_string(); - let checksum = get_checksum_bytes(&desc, true).unwrap(); - let checksum_inception = get_checksum_bytes(&desc, false).unwrap(); + 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();