descriptor wallet: Cache last hardened xpub and use in normalized descriptors

achow101 commented at 10:53 pm on March 1, 2021: member

Currently fetching a normalized descriptor requires the wallet to be unlocked as it needs the private keys to derive the last hardened xpub. This is not very user friendly as normalized descriptors shouldn’t require and don’t involve the private keys except for derivation. We solve this problem by caching the last hardened xpub (which has to be derived at some point when generating the address pool).

However the last hardened xpub was not already being cached. We only cached the immediate parent xpub and derived child keys. For example, with a descriptor derivation path of /84'/0'/0'/0/*, the parent xpub that is cached is m/84'/0'/0'/0, and the child keys of m/84'/0'/0'/0/i (note that child keys would not be cached in this case). This parent xpub is not suitable for the normalized descriptor form as we want the key at m/84'/0'/0'. So this PR adds another field to DescriptorCache to cache the last hardened xpub so that we can use them for normalized descriptors.

Since DescriptorCache is changing, existing descriptor wallets need to be upgraded to use this new cache. The upgrade will occur in the background either at loading time (if the wallet is not encrypted) or at unlocking time in the same manner that UpgradeKeyMetadata operates. It will use a new wallet flag WALLET_FLAG_LAST_HARDENED_XPUB_CACHED to indicate whether the descriptor wallet has the last hardened xpub cache.

Lastly listdescriptors will not require the wallet to be locked and getaddressinfo’s parent_desc will always be output (assuming the upgrade has occurred).

DrahtBot added the label RPC/REST/ZMQ on Mar 1, 2021

DrahtBot added the label Wallet on Mar 1, 2021

in src/script/descriptor.cpp:414 in c096de4098 outdated

410@@ -410,7 +411,7 @@ class BIP32PubkeyProvider final : public PubkeyProvider
411         }
412         return true;
413     }
414-    bool ToNormalizedString(const SigningProvider& arg, std::string& out, bool priv) const override
415+    bool ToNormalizedString(const SigningProvider& arg, std::string& out, bool priv, std::optional<DescriptorCache> cache) const override

sipa commented at 3:03 am on March 2, 2021:

Passing a copy of the entire cache seems overkill here, and especially without std::move’s you’re going to be making copies in every step in the call graph.

The language purist version would be std::optional<std::reference_wrapper<const DescriptorCache>> to achieve something like an optional reference to a const cache. But just const DescriptorCache* works just as well…

sipa commented at 3:06 am on March 2, 2021:

Argh, I meant to click comment, not approve…

achow101 commented at 6:06 pm on March 2, 2021:

Done

sipa approved

DrahtBot commented at 10:11 am on March 2, 2021: member

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#21500 by S3RK
#20096 by achow101

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

achow101 force-pushed on Mar 2, 2021

S3RK commented at 7:55 am on March 9, 2021: member

Started reviewing this

DrahtBot added the label Needs rebase on Mar 10, 2021

in src/script/descriptor.cpp:437 in 0ab914a06b outdated

436-        CExtKey xprv;
437-        if (!GetExtKey(arg, xprv)) return false;
438+        // Get the path to the last hardened stup
439         KeyOriginInfo origin;
440         int k = 0;
441         for (; k <= i; ++k) {

S3RK commented at 8:58 am on March 10, 2021:

nit: I think it would be nice to simply this function by extracting the code for splitting KeyPath. We can introduce a member function like:

0std::pair<KeyPath, KeyPath> KeyPath::split_unhardened() const;

or

0KeyPath KeyPath::split_unhardened();

achow101 commented at 6:14 pm on March 14, 2021:

I don’t think that’s really necessary since we only do the split in this one spot.

S3RK commented at 8:25 am on March 18, 2021:

Feel free to leave it as it. But just to clarify my point. It’s not about code deduplication, it’s about readability and comprehension. There are 3 loops dedicated to splitting derivation path and I think they distract from the main purpose of this function.

achow101 force-pushed on Mar 10, 2021

DrahtBot removed the label Needs rebase on Mar 10, 2021

in src/script/descriptor.h:113 in 690160aac9 outdated

109@@ -110,7 +110,7 @@ struct Descriptor {
110     virtual bool ToPrivateString(const SigningProvider& provider, std::string& out) const = 0;
111 
112     /** Convert the descriptor to a normalized string. Normalized descriptors have the xpub at the last hardened step. This fails if the provided provider does not have the private keys to derive that xpub. */
113-    virtual bool ToNormalizedString(const SigningProvider& provider, std::string& out, bool priv) const = 0;
114+    virtual bool ToNormalizedString(const SigningProvider& provider, std::string& out, bool priv, const DescriptorCache* cache = nullptr) const = 0;

S3RK commented at 7:55 am on March 11, 2021:

Why not pass it as const reference? It can help to remove some of the checks and we can always relax the requirements and make it optional later if needed.

achow101 commented at 6:07 pm on March 14, 2021:

Not every caller of this will necessarily have a cache to pass.

S3RK commented at 7:42 am on March 15, 2021:

Ok. I don’t have strong feelings about it, but right now the only caller besides the unit tests is DescriptorScriptPubKeyMan. Though I can imagine other possibilities in the future I’d still go with more restrictive signature as it’s easier to relax the requirements rather than the other way around.

in src/wallet/scriptpubkeyman.cpp:2321 in 8c5d41f40c outdated

2317+    // Cache the last hardened xpubs
2318+    WalletBatch batch(m_storage.GetDatabase());
2319+    uint256 id = GetID();
2320+    for (const auto& lh_xpub_pair : temp_cache.GetCachedLastHardenedExtPubKeys()) {
2321+        CExtPubKey xpub;
2322+        if (m_wallet_descriptor.cache.GetCachedLastHardenedExtPubKey(lh_xpub_pair.first, xpub)) {

S3RK commented at 4:57 pm on March 14, 2021:

I can’t see how this ever evaluates to true since we exit early if the cache exists.

achow101 commented at 6:10 pm on March 14, 2021:

I don’t follow.

This is checking to see if elements in the temp cache are in the stored cache;

S3RK commented at 7:27 am on March 15, 2021:

But the stored cache is always empty, no? I’m looking at the line 2303

achow101 commented at 3:40 pm on March 15, 2021:

Ah, I see. There shouldn’t be anything in the stored cache, however I am doing this as a belt-and-suspenders check.

S3RK commented at 8:40 am on March 18, 2021:

I find it confusing when we check same conditions multiple times in different places because it’s not clear what is the responsibility/assumptions of each class/function. It also makes it harder to modify the code. DescriptorScriptPubKeyMan and CWallet are strongly coupled anyway.

Maybe we can just remove the check on the top of the function? Even in the worst case nothing will go wrong 1) If wallet is locked then expand will fail 2) if the cache exists already we will just regenerate it.

achow101 commented at 5:16 pm on March 18, 2021:

This is largely just code duplicated from TopUp. The intention was to refactor them to avoid the duplication, but I never got around to doing that.

The check at the top of the function is important to avoid unnecessary extra computation which slows down wallet loading and unlocking.

in src/wallet/scriptpubkeyman.cpp:2297 in 8c5d41f40c outdated

2308+    FlatSigningProvider provider;
2309+    provider.keys = GetKeys();
2310+    FlatSigningProvider out_keys;
2311+    std::vector<CScript> scripts_temp;
2312+    DescriptorCache temp_cache;
2313+    if (!m_wallet_descriptor.descriptor->Expand(0, provider, scripts_temp, out_keys, &temp_cache)){

S3RK commented at 5:20 pm on March 14, 2021:

Why do we need a temp cache?

achow101 commented at 6:12 pm on March 14, 2021:

To avoid modifying the wallet state if something goes wrong during expansion. It is possible that the cache could be modified, but something else causes expansion to fail.

S3RK commented at 8:07 am on March 18, 2021:

I don’t understand the real life scenario. From what I see Expand fails if we can’t get a pub key. For example we tried to derive hardened derivation from the cache or the cache itself was inconsistent. This seems to indicate some logic error in the code itself or a corrupted state. Is my understanding correct or do I miss something?

achow101 commented at 5:24 pm on March 18, 2021:

Consider multi(2,xpub.../0/*,xpub.../0h/*). The first xpub is entirely unhardened derivation. So we will have entries added to the cache when it is expanded. When ExpandHelper moves onto the second xpub, it finds it is entirely hardened derivation. If the keys are unavailable (e.g. they’re encrypted), expand now fails here. However the cache now has entries for the first xpub but no entries for the second. While this is not a dangerous state to be in, the in memory state will not be accurately reflected on disk and that is generally something that I think we should avoid. Thus the temp cache is used.

S3RK commented at 5:31 pm on March 14, 2021: member

Wow, it took more than expected to grasp what’s going on. I think I still need more time, but I have a few questions/suggestions now already.

in src/wallet/scriptpubkeyman.cpp:1830 in e2519b0f6c outdated

1823+                if (xpub != lh_xpub_pair.second) {
1824+                    throw std::runtime_error(std::string(__func__) + ": New cached last hardened xpub does not match already cached last hardened xpub");
1825+                }
1826+                continue;
1827+            }
1828+            if (!batch.WriteDescriptorLastHardenedCache(lh_xpub_pair.second, id, lh_xpub_pair.first)) {

S3RK commented at 8:42 am on March 17, 2021:

If this is a new wallet, at what time do we set the WALLET_FLAG_LAST_HARDENED_XPUB_CACHED flag?

achow101 commented at 5:12 pm on March 18, 2021:

LoadWallet, which is called as part of wallet creation, will always call CWallet::UpgradeDescriptorCache. Since a newly created wallet is never encrypted at that stage, and has no ScriptPubKeyMans, UpgradeDescriptorCache will just set the flag.

S3RK commented at 8:58 am on March 18, 2021: member

Code review ACK 1cb501721ecbd2e3a86f16f4ed9db4cf8ac6ba3f I don’t see any logical errors in the code, it does what it supposed to do.

With that said, I don’t like the direction in which DescriptorImpl::ToStringHelper and DescriptorScriptPubKeyMan::TopUp is going. The code becomes more and more complicated and there are untapped opportunities for simplification.

I believe refactoring should go together with feature implementation because of the following reasons:

just a pure refactoring PR by itself will get much less attention and therefore lower chances to get merged
this is not an urgent feature and it can wait for a refactoring to be implemented

I’m happy to propose, discuss and implement some refactoring ideas if you think it will benefit the project.

DrahtBot added the label Needs rebase on Apr 20, 2021

achow101 force-pushed on Apr 20, 2021

DrahtBot removed the label Needs rebase on Apr 20, 2021

S3RK commented at 7:04 am on April 21, 2021: member

reACK ec22064. Just rebased on top of latest master

My comment above is still relevant though.

achow101 commented at 4:31 pm on April 21, 2021: member

With that said, I don’t like the direction in which DescriptorImpl::ToStringHelper and DescriptorScriptPubKeyMan::TopUp is going. The code becomes more and more complicated and there are untapped opportunities for simplification.

What direction do you think it should be going in?

S3RK commented at 7:30 am on April 22, 2021: member

What direction do you think it should be going in?

Thanks for asking. I think we need to try to simplify the code for readability and comprehension. Some rough ideas:

for ToStringHelper we can a) avoid multiple flags controlling the control flow b) shift non-general code to child classes (addr, raw, multisig).

To be more specific for this PR: I think it’s better to have ToNormalizedString reimplement some logic and not call ToStringHelper at all. Both functions would be easier to reason about. We also don’t need to support normalized private descriptors case, no code actually uses this.

for DescriptorScriptPubKeyMan::TopUp we can keep the code on the same/constant level of abstraction by better encapsulating cache structure. One option is to introduce something like DescriptorCache::CopyFrom(DescriptorCache) and WalletBatch::WriteCache(DescriptorCache).

achow101 commented at 7:26 pm on April 22, 2021: member

To be more specific for this PR: I think it’s better to have ToNormalizedString reimplement some logic and not call ToStringHelper at all. Both functions would be easier to reason about. We also don’t need to support normalized private descriptors case, no code actually uses this.

I kind of disagree. All of the To*String functions do roughly the same thing. The construction of all of the non-key related parts are all the same and it doesn’t make sense to me to be duplicating that. The only parts that are different are the key things, which is why ToStringHelper has options in order to determine which key function to use. I think that instead of using bools, it would be better to use an enum, so I have implemented it that way.

for DescriptorScriptPubKeyMan::TopUp we can keep the code on the same/constant level of abstraction by better encapsulating cache structure. One option is to introduce something like DescriptorCache::CopyFrom(DescriptorCache) and WalletBatch::WriteCache(DescriptorCache).

I’ve added DescriptorCache::MergeAndDiff which merges another DescriptorCache with the current DescriptorCache and returns another DescriptorCache containing a copy of the new items. I’ve also added WalletBatch::WriteDescriptorCache which takes a DescriptorCache and writes all of the items. This cleans up TopUp and deduplicates some code.

achow101 force-pushed on Apr 22, 2021

S3RK commented at 7:00 am on April 26, 2021: member

utACK e668b9e. Thanks for incorporating my suggestions.

meshcollider added this to the milestone 22.0 on Jun 3, 2021

in src/script/descriptor.cpp:293 in e668b9e2bb outdated

285@@ -287,9 +286,6 @@ class BIP32PubkeyProvider final : public PubkeyProvider
286     CExtPubKey m_root_extkey;
287     KeyPath m_path;
288     DeriveType m_derive;
289-    // Cache of the parent of the final derived pubkeys.
290-    // Primarily useful for situations when no read_cache is provided
291-    CExtPubKey m_cached_xpub;

jonatack commented at 3:06 pm on June 24, 2021:

0cc90f1e3dbccd5940b05b633260fdcd8dac5581 essentially reverts 09e25071f40 IIUC. Perhaps re-add const to the four GetPubKey() member functions in the commit and mention in the commit message that it reverts 09e25071f40

0-    virtual bool GetPubKey(int pos, const SigningProvider& arg, CPubKey& key, KeyOriginInfo& info, const DescriptorCache* read_cache = nullptr, DescriptorCache* write_cache = nullptr) = 0;
1+    virtual bool GetPubKey(int pos, const SigningProvider& arg, CPubKey& key, KeyOriginInfo& info, const DescriptorCache* read_cache = nullptr, DescriptorCache* write_cache = nullptr) const = 0;

jonatack commented at 4:25 pm on June 24, 2021:

Apart from this, ACK up to 8fac319f6d180fa5c597ea08beb7827331e7d9ec so far (the first three commits).

achow101 commented at 5:47 pm on June 24, 2021:

I’ve made this commit into a pure revert with an extra comment saying why.

achow101 force-pushed on Jun 24, 2021

Revert "Cache parent xpub inside of BIP32PubkeyProvider"

This reverts commit 09e25071f40c564af08a1386c39c4f2d8eb484b6.

The changes made in this commit have turned out to be unnecessary and
confusing, so it is being reverted.

976b53b085

Refactor Cache merging and writing

Instead of having a large blob of cache merging code in TopUp, refactor
this into DescriptorCache so that it can merge and provide a diff
(another DescriptorCache containing just the items that were added).
Then TopUp can just write everything that was in the diff.

0b4c8ef75c

Move DescriptorCache writing to WalletBatch

Instead of adhoc writing of the items in DescriptorCache, move it all
into WalletBatch.

cacc391098

descriptors: Cache last hardened xpub

Cache the last hardenex xpub in the DescriptorCache

d87b544b83

wallet: Store last hardened xpub cache 432ba9e543

wallet: Upgrade existing descriptor caches

Add functions to upgrade existing descriptor caches to support the use
of last hardened xpub caching.

74fede3b8b

Remove priv option for ToNormalizedString 75530c93a8

achow101 force-pushed on Jun 24, 2021

Change DescriptorImpl::ToStringHelper to use an enum

Instead of having multiple, possibly conflicting, bools controlling the
flow of ToStringHelper, use an enum.

7a26ff10c2

Pass in DescriptorCache to ToNormalizedString

Use the descriptor xpub cache in ToNormalizedString so that the wallet
does not need to be unlocked in order to get the normalized descriptor.

3280704886

wallet, rpc: listdescriptors does not need unlocked

With the last hardened xpub cache, we don't neeed to have the wallet be
unlocked for listdescriptors.

e6cf0ed92d

achow101 force-pushed on Jun 24, 2021

in src/script/descriptor.cpp:357 in e6cf0ed92d

356-            parent_extkey = final_extkey = m_cached_xpub;
357-            if (m_derive == DeriveType::UNHARDENED) der = parent_extkey.Derive(final_extkey, pos);
358         } else if (IsHardened()) {
359             CExtKey xprv;
360-            if (!GetDerivedExtKey(arg, xprv)) return false;
361+            CExtKey lh_xprv;

jonatack commented at 12:56 pm on June 26, 2021:

d87b544b consider naming s/lh_xprv/last_hardened_xprv/ as “lh” is a common abbreviation for left-hand and last_hardened_extkey is a localvar too

in src/script/descriptor.cpp:1494 in e6cf0ed92d

1489+            diff.CacheDerivedExtPubKey(derived_xpub_map_pair.first, derived_xpub_pair.first, derived_xpub_pair.second);
1490+        }
1491+    }
1492+    for (const auto& lh_xpub_pair : other.GetCachedLastHardenedExtPubKeys()) {
1493+        CExtPubKey xpub;
1494+        if (GetCachedLastHardenedExtPubKey(lh_xpub_pair.first, xpub)) {

jonatack commented at 1:07 pm on June 26, 2021:

d87b544b could make this easier to quickly understand

0        if (GetCachedLastHardenedExtPubKey(/* key_exp_pos */ lh_xpub_pair.first, xpub)) {

in src/script/descriptor.cpp:1500 in e6cf0ed92d

1495+            if (xpub != lh_xpub_pair.second) {
1496+                throw std::runtime_error(std::string(__func__) + ": New cached last hardened xpub does not match already cached last hardened xpub");
1497+            }
1498+            continue;
1499+        }
1500+        CacheLastHardenedExtPubKey(lh_xpub_pair.first, lh_xpub_pair.second);

jonatack commented at 1:10 pm on June 26, 2021:

d87b544b It would be nice to make the code easier to quickly understand (using a struct instead of pairs would make the code more self-documenting but is outside the scope here)

0-        CacheLastHardenedExtPubKey(lh_xpub_pair.first, lh_xpub_pair.second);
1-        diff.CacheLastHardenedExtPubKey(lh_xpub_pair.first, lh_xpub_pair.second);
2+        CacheLastHardenedExtPubKey(
3+            /* key_exp_pos */ last_hardened_xpub_pair.first,
4+            /* xpub */ last_hardened_xpub_pair.second);
5+        diff.CacheLastHardenedExtPubKey(
6+            /* key_exp_pos */ last_hardened_xpub_pair.first,
7+            /* xpub */ last_hardened_xpub_pair.second);

in src/wallet/walletdb.cpp:55 in e6cf0ed92d

51@@ -52,6 +52,7 @@ const std::string TX{"tx"};
52 const std::string VERSION{"version"};
53 const std::string WALLETDESCRIPTOR{"walletdescriptor"};
54 const std::string WALLETDESCRIPTORCACHE{"walletdescriptorcache"};
55+const std::string WALLETDESCRIPTORLHCACHE{"walletdescriptorlhcache"};

jonatack commented at 1:27 pm on June 26, 2021:

432ba9e this naming isn’t very readable; s/LH/LASTHARDENED/ would be nice and per developer-notes.md, “constant names are all uppercase, and use _ to separate words”

0const std::string WALLET_DESCRIPTOR_LAST_HARDENED_CACHE{"walletdescriptorlhcache"};

in src/script/descriptor.cpp:309 in e6cf0ed92d

305+    bool GetDerivedExtKey(const SigningProvider& arg, CExtKey& xprv, CExtKey& last_hardened) const
306     {
307         if (!GetExtKey(arg, xprv)) return false;
308         for (auto entry : m_path) {
309             xprv.Derive(xprv, entry);
310+            if (entry >> 31) {

jonatack commented at 1:33 pm on June 26, 2021:

d87b544 it would be nice to hoist these 31 values to a well-named constant

in src/wallet/walletdb.cpp:274 in e6cf0ed92d

269+                return false;
270+            }
271+        }
272+    }
273+    for (const auto& lh_xpub_pair : cache.GetCachedLastHardenedExtPubKeys()) {
274+        if (!WriteDescriptorLastHardenedCache(lh_xpub_pair.second, desc_id, lh_xpub_pair.first)) {

jonatack commented at 1:38 pm on June 26, 2021:

432ba9e readability suggestion

0-    for (const auto& lh_xpub_pair : cache.GetCachedLastHardenedExtPubKeys()) {
1-        if (!WriteDescriptorLastHardenedCache(lh_xpub_pair.second, desc_id, lh_xpub_pair.first)) {
2+    for (const auto& last_hardened_xpub_pair : cache.GetCachedLastHardenedExtPubKeys()) {
3+        if (!WriteDescriptorLastHardenedCache(/* xpub */ last_hardened_xpub_pair.second, desc_id, /* key_exp_index */ lh_xpub_pair.first)) {

in src/wallet/scriptpubkeyman.cpp:2282 in e6cf0ed92d

2280+}
2281+
2282+void DescriptorScriptPubKeyMan::UpgradeDescriptorCache()
2283+{
2284+    LOCK(cs_desc_man);
2285+    if (m_storage.IsLocked() || m_storage.IsWalletFlagSet(WALLET_FLAG_LAST_HARDENED_XPUB_CACHED)) {

jonatack commented at 2:31 pm on June 26, 2021:

74fede3 It seems like IsLocked() is already checked by the callers CWallet::UpgradeDescriptorCache and CWallet::Unlock IIUC, but belt-and-suspenders, I suppose.

in src/script/descriptor.cpp:451 in e6cf0ed92d

456+        // Get the fingerprint
457+        CKeyID id = m_root_extkey.pubkey.GetID();
458+        std::copy(id.begin(), id.begin() + 4, origin.fingerprint);
459+
460+        CExtPubKey xpub;
461+        CExtKey lh_xprv;

jonatack commented at 2:57 pm on June 26, 2021:

3280704 naming style, suggest last_hardened_xprv, as this looks like “left-hand xprv”

in src/script/descriptor.cpp:457 in e6cf0ed92d

462+        // If we have the cache, just get the parent xpub
463+        if (cache != nullptr) {
464+            cache->GetCachedLastHardenedExtPubKey(m_expr_index, xpub);
465+        }
466+        if (!xpub.pubkey.IsValid()) {
467+            // Cache miss, or nor cache, or need privkey

jonatack commented at 2:59 pm on June 26, 2021:

3280704

0            // Cache miss, or no cache, or need privkey

jonatack commented at 3:40 pm on June 26, 2021: member

Semi ACK e6cf0ed92de31a5ac35a271b0da8f0a8364d1175 reviewed, debug-built and ran unit tests and some of the descriptor functional tests at each commit. I’m not very familiar with this code and it could be clearer to the uninitiated IMHO, so I’m not confident enough to give a full ACK. Various minor suggestions follow, most of them for readability, feel free to pick and choose.

fjahr commented at 10:48 pm on June 27, 2021: member

tACK e6cf0ed92de31a5ac35a271b0da8f0a8364d1175

Reviewed code, light manual testing with restarts to ensure persistence works (could have been covered by a test case as well).

nit: in d87b544b834077f102724415e0fada6ee8b2def2 there is a typo in commit description: “hardenex”

achow101 commented at 5:51 pm on June 29, 2021: member

I will implement the suggestions if I have to retouch.

S3RK commented at 7:23 am on June 30, 2021: member

reACK e6cf0ed

Changes from last review: 1) PubkeyProvider::GetPubKey became const 2) necessary changes for taproot descriptors

Happy to reACK again if you want to incorporate jonatack’s readability suggestions

fanquake commented at 8:15 am on June 30, 2021: member

@meshcollider

meshcollider commented at 9:58 pm on June 30, 2021: contributor

Code review + functional test run ACK e6cf0ed92de31a5ac35a271b0da8f0a8364d1175

meshcollider referenced this in commit 722776c0fd on Jun 30, 2021

meshcollider commented at 9:59 pm on June 30, 2021: contributor

Nits can be left for follow-up if needed, lets just get this in

DrahtBot commented at 10:12 pm on June 30, 2021: member

🐙 This pull request conflicts with the target branch and needs rebase.

Want to unsubscribe from rebase notifications on this pull request? Just convert this pull request to a “draft”.

DrahtBot added the label Needs rebase on Jun 30, 2021

achow101 commented at 10:31 pm on June 30, 2021: member

This was merged, but github isn’t detecting it.

jonatack commented at 10:34 pm on June 30, 2021: member

There have been several of these, I think this is the third one in as many days.

achow101 closed this on Jun 30, 2021

sidhujag referenced this in commit 46eee0f6de on Jul 1, 2021

gwillen referenced this in commit 8a158e3e93 on Jun 1, 2022

DrahtBot locked this on Aug 18, 2022

descriptor wallet: Cache last hardened xpub and use in normalized descriptors #21329

Conflicts