Merge bitcoin#15024: Allow specific private keys to be derived from d…

…escriptor

53b7de6 Add test for dumping the private key imported from descriptor (MeshCollider)
2857bc4 Extend importmulti descriptor tests (MeshCollider)
81a884b Import private keys from descriptor with importmulti if provided (MeshCollider)
a4d1bd1 Add private key derivation functions to descriptors (MeshCollider)

Pull request description:

  ~This is based on bitcoin#14491, review the last 3 commits only.~

  Currently, descriptors have an Expand() function which returns public keys and scripts for a specific index of a ranged descriptor. But the private key for a specific index is not given. This allows private keys for specific indices to be derived. This also allows those keys to be imported through the `importmulti` RPC rather than having to provide them separately.

ACKs for commit 53b7de:
  achow101:
    ACK 53b7de6

Tree-SHA512: c060bc01358a1adc76d3d470fefc2bdd39c837027f452e9bc4bd2e726097e1ece4af9d5627efd942a5f8819271e15ba54f010b169b50a9435a1f0f40fd1cebf3

src/script/descriptor.cpp

@@ -164,6 +164,9 @@ struct PubkeyProvider
 
     /** Get the descriptor string form including private data (if available in arg). */
     virtual bool ToPrivateString(const SigningProvider& arg, std::string& out) const = 0;
+
+    /** Derive a private key, if private data is available in arg. */
+    virtual bool GetPrivKey(int pos, const SigningProvider& arg, CKey& key) const = 0;
 };
 
 class OriginPubkeyProvider final : public PubkeyProvider
@@ -195,6 +198,10 @@ class OriginPubkeyProvider final : public PubkeyProvider
         ret = "[" + OriginString() + "]" + std::move(sub);
         return true;
     }
+    bool GetPrivKey(int pos, const SigningProvider& arg, CKey& key) const override
+    {
+        return m_provider->GetPrivKey(pos, arg, key);
+    }
 };
 
 /** An object representing a parsed constant public key in a descriptor. */
@@ -222,6 +229,10 @@ class ConstPubkeyProvider final : public PubkeyProvider
         ret = EncodeSecret(key);
         return true;
     }
+    bool GetPrivKey(int pos, const SigningProvider& arg, CKey& key) const override
+    {
+        return arg.GetKey(m_pubkey.GetID(), key);
+    }
 };
 
 enum class DeriveType {
@@ -266,14 +277,9 @@ class BIP32PubkeyProvider final : public PubkeyProvider
     {
         if (key) {
             if (IsHardened()) {
-                CExtKey extkey;
-                if (!GetExtKey(arg, extkey)) return false;
-                for (auto entry : m_path) {
-                    extkey.Derive(extkey, entry);
-                }
-                if (m_derive == DeriveType::UNHARDENED) extkey.Derive(extkey, pos);
-                if (m_derive == DeriveType::HARDENED) extkey.Derive(extkey, pos | 0x80000000UL);
-                *key = extkey.Neuter().pubkey;
+                CKey priv_key;
+                if (!GetPrivKey(pos, arg, priv_key)) return false;
+                *key = priv_key.GetPubKey();
             } else {
                 // TODO: optimize by caching
                 CExtPubKey extkey = m_extkey;
@@ -312,6 +318,18 @@ class BIP32PubkeyProvider final : public PubkeyProvider
         }
         return true;
     }
+    bool GetPrivKey(int pos, const SigningProvider& arg, CKey& key) const override
+    {
+        CExtKey extkey;
+        if (!GetExtKey(arg, extkey)) return false;
+        for (auto entry : m_path) {
+            extkey.Derive(extkey, entry);
+        }
+        if (m_derive == DeriveType::UNHARDENED) extkey.Derive(extkey, pos);
+        if (m_derive == DeriveType::HARDENED) extkey.Derive(extkey, pos | 0x80000000UL);
+        key = extkey.key;
+        return true;
+    }
 };
 
 /** Base class for all Descriptor implementations. */
@@ -462,6 +480,20 @@ class DescriptorImpl : public Descriptor
         Span<const unsigned char> span = MakeSpan(cache);
         return ExpandHelper(pos, DUMMY_SIGNING_PROVIDER, &span, output_scripts, out, nullptr) && span.size() == 0;
     }
+
+    void ExpandPrivate(int pos, const SigningProvider& provider, FlatSigningProvider& out) const final
+    {
+        for (const auto& p : m_pubkey_args) {
+            CKey key;
+            if (!p->GetPrivKey(pos, provider, key)) continue;
+            out.keys.emplace(key.GetPubKey().GetID(), key);
+        }
+        if (m_script_arg) {
+            FlatSigningProvider subprovider;
+            m_script_arg->ExpandPrivate(pos, provider, subprovider);
+            out = Merge(out, subprovider);
+        }
+    }
 };
 
 /** Construct a vector with one element, which is moved into it. */

src/script/descriptor.h

@@ -60,6 +60,14 @@ struct Descriptor {
      * out: scripts and public keys necessary for solving the expanded scriptPubKeys will be put here (may be equal to provider).
      */
     virtual bool ExpandFromCache(int pos, const std::vector<unsigned char>& cache, std::vector<CScript>& output_scripts, FlatSigningProvider& out) const = 0;
+
+    /** Expand the private key for a descriptor at a specified position, if possible.
+     *
+     * pos: the position at which to expand the descriptor. If IsRange() is false, this is ignored.
+     * provider: the provider to query for the private keys.
+     * out: any private keys available for the specified pos will be placed here.
+     */
+    virtual void ExpandPrivate(int pos, const SigningProvider& provider, FlatSigningProvider& out) const = 0;
 };
 
 /** Parse a descriptor string. Included private keys are put in out.

src/wallet/rpcdump.cpp

@@ -1165,8 +1165,7 @@ static UniValue ProcessImportDescriptor(ImportData& import_data, std::map<CKeyID
 
     const UniValue& priv_keys = data.exists("keys") ? data["keys"].get_array() : UniValue();
 
-    // Expand all descriptors to get public keys and scripts.
-    // TODO: get private keys from descriptors too
+    // Expand all descriptors to get public keys and scripts, and private keys if available.
     for (int i = range_start; i <= range_end; ++i) {
         FlatSigningProvider out_keys;
         std::vector<CScript> scripts_temp;
@@ -1180,7 +1179,10 @@ static UniValue ProcessImportDescriptor(ImportData& import_data, std::map<CKeyID
             import_data.import_scripts.emplace(x.second);
         }
 
+        parsed_desc->ExpandPrivate(i, keys, out_keys);
+
         std::copy(out_keys.pubkeys.begin(), out_keys.pubkeys.end(), std::inserter(pubkey_map, pubkey_map.end()));
+        std::copy(out_keys.keys.begin(), out_keys.keys.end(), std::inserter(privkey_map, privkey_map.end()));
         import_data.key_origins.insert(out_keys.origins.begin(), out_keys.origins.end());
     }
 

test/functional/wallet_importmulti.py

@@ -571,6 +571,7 @@ def run_test(self):
         # Test ranged descriptor fails if range is not specified
         xpriv = "tprv8ZgxMBicQKsPeuVhWwi6wuMQGfPKi9Li5GtX35jVNknACgqe3CY4g5xgkfDDJcmtF7o1QnxWDRYw4H5P26PXq7sbcUkEqeR4fg3Kxp2tigg"
         addresses = ["2N7yv4p8G8yEaPddJxY41kPihnWvs39qCMf", "2MsHxyb2JS3pAySeNUsJ7mNnurtpeenDzLA"] # hdkeypath=m/0'/0'/0' and 1'
+        addresses += ["bcrt1qrd3n235cj2czsfmsuvqqpr3lu6lg0ju7scl8gn", "bcrt1qfqeppuvj0ww98r6qghmdkj70tv8qpchehegrg8"] # wpkh subscripts corresponding to the above addresses
         desc = "sh(wpkh(" + xpriv + "/0'/0'/*'" + "))"
         self.log.info("Ranged descriptor import should fail without a specified range")
         self.test_importmulti({"desc": descsum_create(desc),
@@ -579,17 +580,17 @@ def run_test(self):
                               error_code=-8,
                               error_message='Descriptor is ranged, please specify the range')
 
-        # Test importing of a ranged descriptor without keys
+        # Test importing of a ranged descriptor with xpriv
         self.log.info("Should import the ranged descriptor with specified range as solvable")
         self.test_importmulti({"desc": descsum_create(desc),
                                "timestamp": "now",
                                "range": 1},
-                              success=True,
-                              warnings=["Some private keys are missing, outputs will be considered watchonly. If this is intentional, specify the watchonly flag."])
+                              success=True)
         for address in addresses:
             test_address(self.nodes[1],
-                         key.p2sh_p2wpkh_addr,
-                         solvable=True)
+                         address,
+                         solvable=True,
+                         ismine=True)
 
         self.test_importmulti({"desc": descsum_create(desc), "timestamp": "now", "range": -1},
                               success=False, error_code=-8, error_message='End of range is too high')
@@ -606,6 +607,23 @@ def run_test(self):
         self.test_importmulti({"desc": descsum_create(desc), "timestamp": "now", "range": [0, 1000001]},
                               success=False, error_code=-8, error_message='Range is too large')
 
+        # Test importing a descriptor containing a WIF private key
+        wif_priv = "cTe1f5rdT8A8DFgVWTjyPwACsDPJM9ff4QngFxUixCSvvbg1x6sh"
+        address = "2MuhcG52uHPknxDgmGPsV18jSHFBnnRgjPg"
+        desc = "sh(wpkh(" + wif_priv + "))"
+        self.log.info("Should import a descriptor with a WIF private key as spendable")
+        self.test_importmulti({"desc": descsum_create(desc),
+                               "timestamp": "now"},
+                              success=True)
+        test_address(self.nodes[1],
+                     address,
+                     solvable=True,
+                     ismine=True)
+
+        # dump the private key to ensure it matches what was imported
+        privkey = self.nodes[1].dumpprivkey(address)
+        assert_equal(privkey, wif_priv)
+
         # Test importing of a P2PKH address via descriptor
         key = get_key(self.nodes[0])
         self.log.info("Should import a p2pkh address from descriptor")