Skip to content

Commit

Permalink
Deprecate backward compatible get_checksum_bytes, get_checksum functions
Browse files Browse the repository at this point in the history
Rename replacement functions calc_checksum_bytes and calc_checksum
  • Loading branch information
notmandatory committed Oct 24, 2022
1 parent e2a4a58 commit 60057a7
Show file tree
Hide file tree
Showing 5 changed files with 68 additions and 25 deletions.
4 changes: 2 additions & 2 deletions src/blockchain/rpc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -801,7 +801,7 @@ fn is_wallet_descriptor(client: &Client) -> Result<bool, Error> {

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`]
Expand Down
61 changes: 52 additions & 9 deletions src/descriptor/checksum.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<String, DescriptorError> {
// 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<String, DescriptorError> {
// 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]
));
}
Expand Down
6 changes: 3 additions & 3 deletions src/descriptor/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
Expand Down
4 changes: 2 additions & 2 deletions src/descriptor/policy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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")
}
}
Expand Down
18 changes: 9 additions & 9 deletions src/wallet/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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(),
);
}

Expand Down Expand Up @@ -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
);
}
Expand All @@ -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();
Expand Down

0 comments on commit 60057a7

Please sign in to comment.