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.

diff --git a/src/descriptor/checksum.rs b/src/descriptor/checksum.rs
index 8dfdac49b44608b91c85e6cf5bc51fcf5174e458..9eedb45b7181966e7107deabf2d7e75fcb4561f3 100644 (file)
--- a/src/descriptor/checksum.rs
+++ b/src/descriptor/checksum.rs
@@ -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 let Some(original_checksum) = original_checksum {
-        if original_checksum.as_bytes() != &checksum {
+        if original_checksum.as_bytes() != checksum {
             return Err(DescriptorError::InvalidDescriptorChecksum);
         }
     }

diff --git a/src/descriptor/mod.rs b/src/descriptor/mod.rs
index 7c51d27fc912b4eca5919b07d8b38315da135513..4955f8ff8bd9bf26ebfc082404401f4f42a9ea1b 100644 (file)
--- a/src/descriptor/mod.rs
+++ b/src/descriptor/mod.rs
@@ -88,7 +88,7 @@ impl IntoWalletDescriptor for &str {
         let descriptor = match self.split_once('#') {
             Some((desc, original_checksum)) => {
                 let checksum = get_checksum_bytes(desc, false)?;
-                if original_checksum.as_bytes() != &checksum {
+                if original_checksum.as_bytes() != checksum {
                     return Err(DescriptorError::InvalidDescriptorChecksum);
                 }
                 desc

diff --git a/src/wallet/mod.rs b/src/wallet/mod.rs
index 776e1740a1afab9ae0c160a974ca5f67afbff9f6..737afe8da5063d3d825417c7f6512aa1884b8486 100644 (file)
--- a/src/wallet/mod.rs
+++ b/src/wallet/mod.rs
@@ -64,6 +64,7 @@ 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::derived::AsDerived;
 use crate::descriptor::policy::BuildSatisfaction;
 use crate::descriptor::{
@@ -203,18 +204,21 @@ 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(
@@ -222,9 +226,6 @@ where
                     &change_descriptor,
                     &secp,
                 ));
-                // if !parsed.same_structure(descriptor.as_ref()) {
-                //     return Err(Error::DifferentDescriptorStructure);
-                // }
 
                 (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.
     pub fn 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]
     fn test_get_funded_wallet_balance() {
         let (wallet, _, _) = get_funded_wallet(get_test_wpkh());