Ensure backward compatibility of the "checksum inception" bug

`Wallet` stores the descriptors' checksum in the database for safety.
Previously, the checksum used was a checksum of a descriptor that
already had a checksum.

This PR allows for backward-compatibility of databases created with this
bug.
This commit is contained in:
志宇 2022-09-29 14:24:28 +08:00
parent fd34956c29
commit e2a4a5884b
No known key found for this signature in database
GPG Key ID: F6345C9837C2BDE8
3 changed files with 51 additions and 9 deletions

View File

@ -83,7 +83,7 @@ pub fn get_checksum_bytes(mut desc: &str, exclude_hash: bool) -> Result<[u8; 8],
// if input data already had a checksum, check calculated checksum against original checksum // if input data already had a checksum, check calculated checksum against original checksum
if let Some(original_checksum) = original_checksum { if let Some(original_checksum) = original_checksum {
if original_checksum.as_bytes() != &checksum { if original_checksum.as_bytes() != checksum {
return Err(DescriptorError::InvalidDescriptorChecksum); return Err(DescriptorError::InvalidDescriptorChecksum);
} }
} }

View File

@ -88,7 +88,7 @@ impl IntoWalletDescriptor for &str {
let descriptor = match self.split_once('#') { let descriptor = match self.split_once('#') {
Some((desc, original_checksum)) => { Some((desc, original_checksum)) => {
let checksum = get_checksum_bytes(desc, false)?; let checksum = get_checksum_bytes(desc, false)?;
if original_checksum.as_bytes() != &checksum { if original_checksum.as_bytes() != checksum {
return Err(DescriptorError::InvalidDescriptorChecksum); return Err(DescriptorError::InvalidDescriptorChecksum);
} }
desc desc

View File

@ -64,6 +64,7 @@ use utils::{check_nlocktime, check_nsequence_rbf, After, Older, SecpCtx};
use crate::blockchain::{GetHeight, NoopProgress, Progress, WalletSync}; use crate::blockchain::{GetHeight, NoopProgress, Progress, WalletSync};
use crate::database::memory::MemoryDatabase; use crate::database::memory::MemoryDatabase;
use crate::database::{AnyDatabase, BatchDatabase, BatchOperations, DatabaseUtils, SyncTime}; use crate::database::{AnyDatabase, BatchDatabase, BatchOperations, DatabaseUtils, SyncTime};
use crate::descriptor::checksum::get_checksum_bytes;
use crate::descriptor::derived::AsDerived; use crate::descriptor::derived::AsDerived;
use crate::descriptor::policy::BuildSatisfaction; use crate::descriptor::policy::BuildSatisfaction;
use crate::descriptor::{ use crate::descriptor::{
@ -203,18 +204,21 @@ where
let secp = Secp256k1::new(); let secp = Secp256k1::new();
let (descriptor, keymap) = into_wallet_descriptor_checked(descriptor, &secp, network)?; let (descriptor, keymap) = into_wallet_descriptor_checked(descriptor, &secp, network)?;
database.check_descriptor_checksum( Self::db_checksum(
&mut database,
&descriptor.to_string(),
KeychainKind::External, KeychainKind::External,
get_checksum(&descriptor.to_string())?.as_bytes(),
)?; )?;
let signers = Arc::new(SignersContainer::build(keymap, &descriptor, &secp)); let signers = Arc::new(SignersContainer::build(keymap, &descriptor, &secp));
let (change_descriptor, change_signers) = match change_descriptor { let (change_descriptor, change_signers) = match change_descriptor {
Some(desc) => { Some(desc) => {
let (change_descriptor, change_keymap) = let (change_descriptor, change_keymap) =
into_wallet_descriptor_checked(desc, &secp, network)?; into_wallet_descriptor_checked(desc, &secp, network)?;
database.check_descriptor_checksum( Self::db_checksum(
&mut database,
&change_descriptor.to_string(),
KeychainKind::Internal, KeychainKind::Internal,
get_checksum(&change_descriptor.to_string())?.as_bytes(),
)?; )?;
let change_signers = Arc::new(SignersContainer::build( let change_signers = Arc::new(SignersContainer::build(
@ -222,9 +226,6 @@ where
&change_descriptor, &change_descriptor,
&secp, &secp,
)); ));
// if !parsed.same_structure(descriptor.as_ref()) {
// return Err(Error::DifferentDescriptorStructure);
// }
(Some(change_descriptor), change_signers) (Some(change_descriptor), change_signers)
} }
@ -243,6 +244,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 = get_checksum_bytes(desc, true)?;
if db.check_descriptor_checksum(kind, checksum).is_ok() {
return Ok(());
}
let checksum_inception = get_checksum_bytes(desc, false)?;
db.check_descriptor_checksum(kind, checksum_inception)
}
/// Get the Bitcoin network the wallet is using. /// Get the Bitcoin network the wallet is using.
pub fn network(&self) -> Network { pub fn network(&self) -> Network {
self.network self.network
@ -1949,6 +1963,34 @@ pub(crate) mod test {
); );
} }
#[test]
fn test_db_checksum() {
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_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] #[test]
fn test_get_funded_wallet_balance() { fn test_get_funded_wallet_balance() {
let (wallet, _, _) = get_funded_wallet(get_test_wpkh()); let (wallet, _, _) = get_funded_wallet(get_test_wpkh());