mirror of
https://github.com/bitcoin/bitcoin.git
synced 2026-09-11 21:20:39 +02:00
fb26bc9cfe
ec2adf3c51test: Check miniscript descriptor h and apostrophe equivalence (w0xlt)a2d001b57ctest: Enforce descriptor reimport is an update (Ava Chow)e2b2f1c5c6descriptor: Rename DescriptorID to CompatDescriptorHash (Ava Chow)6ad31c062ctest: Add 31.0 to wallet backwards compatibility test (Ava Chow)2a6c53371bwallet, spkm: Treat Descriptor ID as an opaque SPKM ID (Ava Chow)62e826fa76wallet: Update WalletDescriptor from another one instead of overwriting (Ava Chow)1113f7590ewallet, export: Include descriptor cache when exporting descriptors (Ava Chow)9fc7b2618bspkm: Remove DescriptorSPKM constructor that doesn't take a descriptor (Ava Chow)770ff64bd7test: Add v30.2 and Miniscript to wallet backwards compatibility test (Ava Chow)35d6a60dbfdescriptor: Add ToCanonicalString (Ava Chow)1d87af26cedescriptors: Remove default StringType from PubkeyProvider::ToString() (Ava Chow)1c7f9aaf75miniscript: Don't use StringType::COMPAT (Ava Chow) Pull request description: Since keys in Miniscript expressions were not correctly handling `StringType::COMPAT` when generating the Descriptor ID, in order to keep compatibility with previous versions, we need to continue to handle that enum incorrectly when computing the ID. Given that this it the second time that we have had this issue, this PR also drops the concept of Descriptor ID being something that we can validate. Instead, the ID read in from the database is treated as an opaque blob that is used only to tie together the records related to a particular SPKM. It is instead treated as a ScriptPubKeyMan ID and users of it must be retrieving the ID from somewhere rather than computing it from a descriptor. The check of comparing the read ID to the computed ID is removed so that all previously created wallets can be read. To clarify that the ID is not actually an ID, the function `DescriptorID` is renamed to `CompatDescriptorHash` and it is still used to generate the SPKM ID that is written to the database. The ID was additionally being used to determine whether a descriptor is equal to another descriptor. This was used only by `importdescriptors` and `createwalletdescriptor`. These uses have been changed to do a string comparison rather than computing a hash and comparing the hashes. This removes the need to rely on `CompatDescriptorHash`. The only caveat is that previously the hash was being used to do a map lookup in `m_spk_managers`, but this is now changed to use `std::find_if`. The lookup complexity changes from logarithmic to linear, which may be really bad for wallets with a lot of descriptors, e.g. migrated formerly non-HD wallets. I think in general though, the tradeoff is okay, and neither of these functions purport to be performant, especially as `importdescriptors` may also do a rescan which can take a long time. However, if that is a concern, an additional map of `CompatDescriptorHash` to DescriptorSPKM can be added. Lastly, the wallet backwards compatibility test is updated to have 30.2 and 31.0 nodes, and a wallet with miniscript expressions. This exercises both creating wallets in previous versions and making sure they load in master, and making new wallets on master and checking whether they load, depending on the version. Fixes #35432 ACKs for top commit: pseudoramdom: ACKec2adf3c51davidgumberg: crACKec2adf3c51w0xlt: ACKec2adf3c51Tree-SHA512: a32995c171b829a874cfd1bb03adde46fd8737322d5c44bc2ff27eff1ea8742c16c7ea1bb6fdc0fb2b89d0f11919850383799c3af126e7f55fe0878a8f1a7024