refactor(chain): calculate DescriptorId as sha256 hash of spk at index 0

Also update docs to explain how KeychainTxOutIndex handles descriptors that
generate the same spks.

diff --git a/crates/chain/src/descriptor_ext.rs b/crates/chain/src/descriptor_ext.rs
index e42d291bb737acf29cb82fa6126a2eead2810cd5..eac01a684d4d29135378248aadd590846575a472 100644 (file)
--- a/crates/chain/src/descriptor_ext.rs
+++ b/crates/chain/src/descriptor_ext.rs
@@ -1,12 +1,8 @@
-use crate::{
-    alloc::{string::ToString, vec::Vec},
-    miniscript::{Descriptor, DescriptorPublicKey},
-};
+use crate::miniscript::{Descriptor, DescriptorPublicKey};
 use bitcoin::hashes::{hash_newtype, sha256, Hash};
 
 hash_newtype! {
-    /// Represents the ID of a descriptor, defined as the sha256 hash of
-    /// the descriptor string, checksum excluded.
+    /// Represents the unique ID of a descriptor.
     ///
     /// This is useful for having a fixed-length unique representation of a descriptor,
     /// in particular, we use it to persist application state changes related to the
@@ -21,8 +17,8 @@ pub trait DescriptorExt {
     /// Panics if the descriptor wildcard is hardened.
     fn dust_value(&self) -> u64;
 
-    /// Returns the descriptor id, calculated as the sha256 of the descriptor, checksum not
-    /// included.
+    /// Returns the descriptor ID, calculated as the sha256 hash of the spk derived from the
+    /// descriptor at index 0.
     fn descriptor_id(&self) -> DescriptorId;
 }
 
@@ -36,9 +32,7 @@ impl DescriptorExt for Descriptor<DescriptorPublicKey> {
     }
 
     fn descriptor_id(&self) -> DescriptorId {
-        let desc = self.to_string();
-        let desc_without_checksum = desc.split('#').next().expect("Must be here");
-        let descriptor_bytes = <Vec<u8>>::from(desc_without_checksum.as_bytes());
-        DescriptorId(sha256::Hash::hash(&descriptor_bytes))
+        let spk = self.at_derivation_index(0).unwrap().script_pubkey();
+        DescriptorId(sha256::Hash::hash(spk.as_bytes()))
     }
 }

diff --git a/crates/chain/src/keychain/txout_index.rs b/crates/chain/src/keychain/txout_index.rs
index e4fad3d5ad1c2bf186e021f7a25b8094b8e512ba..758f16cbca2313e5a121bfc6412644d8762c4611 100644 (file)
--- a/crates/chain/src/keychain/txout_index.rs
+++ b/crates/chain/src/keychain/txout_index.rs
@@ -352,6 +352,13 @@ impl<K: Clone + Ord + Debug> KeychainTxOutIndex<K> {
     ///
     /// keychain <-> descriptor is a one-to-one mapping that cannot be changed. Attempting to do so
     /// will return a [`InsertDescriptorError<K>`].
+    ///
+    /// `[KeychainTxOutIndex]` will prevent you from inserting two descriptors which derive the same
+    /// script pubkey at index 0, but it's up to you to ensure that descriptors don't collide at
+    /// other indices. If they do nothing catastrophic happens at the `KeychainTxOutIndex` level
+    /// (one keychain just becomes the defacto owner of that spk arbitrarily) but this may have
+    /// subtle implications up the application stack like one UTXO being missing from one keychain
+    /// because it has been assigned to another which produces the same script pubkey.
     pub fn insert_descriptor(
         &mut self,
         keychain: K,

diff --git a/crates/wallet/src/wallet/mod.rs b/crates/wallet/src/wallet/mod.rs
index 400ecde1d00733a0633309ec516e142cddb5f4c7..dcd5123ebe780e21fd8a4bc638fb582651a9bdb8 100644 (file)
--- a/crates/wallet/src/wallet/mod.rs
+++ b/crates/wallet/src/wallet/mod.rs
@@ -91,6 +91,9 @@ const COINBASE_MATURITY: u32 = 100;
 /// [`ChangeSet`]s (see [`take_staged`]). Also see individual functions and example for instructions
 /// on when [`Wallet`] state needs to be persisted.
 ///
+/// The `Wallet` descriptor (external) and change descriptor (internal) must not derive the same
+/// script pubkeys. See [`KeychainTxOutIndex::insert_descriptor()`] for more details.
+///
 /// [`signer`]: crate::signer
 /// [`take_staged`]: Wallet::take_staged
 #[derive(Debug)]