Fix how descriptor checksums are calculated

src/descriptor/checksum.rs

@@ -41,12 +41,21 @@ 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 fn get_checksum_bytes(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,13 +81,20 @@ 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 of a descriptor
 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).map(|b| unsafe { String::from_utf8_unchecked(b.to_vec()) })
+    get_checksum_bytes(desc, true).map(|b| unsafe { String::from_utf8_unchecked(b.to_vec()) })
 }
 
 #[cfg(test)]

src/descriptor/mod.rs

@@ -40,6 +40,7 @@ pub mod policy;
 pub mod template;
 
 pub use self::checksum::get_checksum;
+use self::checksum::get_checksum_bytes;
 pub use self::derived::{AsDerived, DerivedDescriptorKey};
 pub use self::error::Error as DescriptorError;
 pub use self::policy::Policy;
@@ -84,19 +85,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 = get_checksum_bytes(desc, false)?;
+                if original_checksum.as_bytes() != checksum {
+                    return Err(DescriptorError::InvalidDescriptorChecksum);
+                }
+                desc
             }
-
-            parts[0]
-        } else {
-            self
+            None => self,
         };
 
         ExtendedDescriptor::parse_descriptor(secp, descriptor)?

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,28 +204,28 @@ 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(
                     change_keymap,
                     &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
@@ -1943,15 +1957,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!(
+            get_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 = 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]