Merge bitcoin#21500: wallet, rpc: add an option to list private descr…

…iptors

bb822a7 wallet, rpc: add listdescriptors private option (S3RK)

Pull request description:

  Rationale: make it possible to backup your wallet with `listdescriptors` command

  * The default behaviour is still to show public version
  * For private version only the root xprv is returned

  Example use-case:
  ```
  > bitcoin-cli -regtest -named createwallet wallet_name=old descriptors=true
  > bitcoin-cli -regtest -rpcwallet=old listdescriptors true | jq '.descriptors' > descriptors.txt

  > bitcoin-cli -regtest -named createwallet wallet_name=new descriptors=true blank=true
  > bitcoin-cli -regtest -rpcwallet=new importdescriptors "$(cat descriptors.txt)"
  ```

  In case of watch-only wallet without private keys there will be following output:
  ```
  error code: -4
  error message:
  Can't get descriptor string.
  ```

ACKs for top commit:
  achow101:
    re-ACK bb822a7
  Rspigler:
    tACK bb822a7
  jonatack:
    ACK bb822a7 per `git diff 2854ddc bb822a7`
  prayank23:
    tACK bitcoin@bb822a7
  meshcollider:
    Code review ACK bb822a7

Tree-SHA512: f6dddc72a74e5667071ccd77f8dce578382e8e29e7ed6a0834ac2e114a6d3918b59c2f194f4079b3259e13d9ba3b4f405619940c3ecb7a1a0344615aed47c43d

src/rpc/client.cpp

@@ -160,6 +160,7 @@ static const CRPCConvertParam vRPCConvertParams[] =
     { "importmulti", 0, "requests" },
     { "importmulti", 1, "options" },
     { "importdescriptors", 0, "requests" },
+    { "listdescriptors", 0, "private" },
     { "verifychain", 0, "checklevel" },
     { "verifychain", 1, "nblocks" },
     { "getblockstats", 0, "hash_or_height" },

src/wallet/rpcdump.cpp

@@ -1942,8 +1942,10 @@ RPCHelpMan listdescriptors()
 {
     return RPCHelpMan{
         "listdescriptors",
-        "\nList descriptors imported into a descriptor-enabled wallet.",
-        {},
+        "\nList descriptors imported into a descriptor-enabled wallet.\n",
+        {
+            {"private", RPCArg::Type::BOOL, RPCArg::Default{false}, "Show private descriptors."}
+        },
         RPCResult{RPCResult::Type::OBJ, "", "", {
             {RPCResult::Type::STR, "wallet_name", "Name of wallet this operation was performed on"},
             {RPCResult::Type::ARR, "descriptors", "Array of descriptor objects",
@@ -1963,6 +1965,7 @@ RPCHelpMan listdescriptors()
         }},
         RPCExamples{
             HelpExampleCli("listdescriptors", "") + HelpExampleRpc("listdescriptors", "")
+            + HelpExampleCli("listdescriptors", "true") + HelpExampleRpc("listdescriptors", "true")
         },
         [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
 {
@@ -1973,6 +1976,11 @@ RPCHelpMan listdescriptors()
         throw JSONRPCError(RPC_WALLET_ERROR, "listdescriptors is not available for non-descriptor wallets");
     }
 
+    const bool priv = !request.params[0].isNull() && request.params[0].get_bool();
+    if (priv) {
+        EnsureWalletIsUnlocked(*wallet);
+    }
+
     LOCK(wallet->cs_wallet);
 
     UniValue descriptors(UniValue::VARR);
@@ -1986,8 +1994,9 @@ RPCHelpMan listdescriptors()
         LOCK(desc_spk_man->cs_desc_man);
         const auto& wallet_descriptor = desc_spk_man->GetWalletDescriptor();
         std::string descriptor;
-        if (!desc_spk_man->GetDescriptorString(descriptor)) {
-            throw JSONRPCError(RPC_WALLET_ERROR, "Can't get normalized descriptor string.");
+
+        if (!desc_spk_man->GetDescriptorString(descriptor, priv)) {
+            throw JSONRPCError(RPC_WALLET_ERROR, "Can't get descriptor string.");
         }
         spk.pushKV("desc", descriptor);
         spk.pushKV("timestamp", wallet_descriptor.creation_time);

src/wallet/rpcwallet.cpp

@@ -4008,7 +4008,7 @@ RPCHelpMan getaddressinfo()
     DescriptorScriptPubKeyMan* desc_spk_man = dynamic_cast<DescriptorScriptPubKeyMan*>(pwallet->GetScriptPubKeyMan(scriptPubKey));
     if (desc_spk_man) {
         std::string desc_str;
-        if (desc_spk_man->GetDescriptorString(desc_str)) {
+        if (desc_spk_man->GetDescriptorString(desc_str, /* priv */ false)) {
             ret.pushKV("parent_desc", desc_str);
         }
     }

src/wallet/scriptpubkeyman.cpp

@@ -2378,13 +2378,20 @@ const std::vector<CScript> DescriptorScriptPubKeyMan::GetScriptPubKeys() const
     return script_pub_keys;
 }
 
-bool DescriptorScriptPubKeyMan::GetDescriptorString(std::string& out) const
+bool DescriptorScriptPubKeyMan::GetDescriptorString(std::string& out, const bool priv) const
 {
     LOCK(cs_desc_man);
 
     FlatSigningProvider provider;
     provider.keys = GetKeys();
 
+    if (priv) {
+        // For the private version, always return the master key to avoid
+        // exposing child private keys. The risk implications of exposing child
+        // private keys together with the parent xpub may be non-obvious for users.
+        return m_wallet_descriptor.descriptor->ToPrivateString(provider, out);
+    }
+
     return m_wallet_descriptor.descriptor->ToNormalizedString(provider, out, &m_wallet_descriptor.cache);
 }
 

src/wallet/scriptpubkeyman.h

@@ -606,7 +606,7 @@ class DescriptorScriptPubKeyMan : public ScriptPubKeyMan
     const WalletDescriptor GetWalletDescriptor() const EXCLUSIVE_LOCKS_REQUIRED(cs_desc_man);
     const std::vector<CScript> GetScriptPubKeys() const;
 
-    bool GetDescriptorString(std::string& out) const;
+    bool GetDescriptorString(std::string& out, const bool priv) const;
 
     void UpgradeDescriptorCache();
 };

test/functional/wallet_listdescriptors.py

@@ -72,11 +72,39 @@ def run_test(self):
             ],
         }
         assert_equal(expected, wallet.listdescriptors())
+        assert_equal(expected, wallet.listdescriptors(False))
+
+        self.log.info('Test list private descriptors')
+        expected_private = {
+            'wallet_name': 'w2',
+            'descriptors': [
+                {'desc': descsum_create('wpkh(' + xprv + hardened_path + '/0/*)'),
+                 'timestamp': 1296688602,
+                 'active': False,
+                 'range': [0, 0],
+                 'next': 0},
+            ],
+        }
+        assert_equal(expected_private, wallet.listdescriptors(True))
 
         self.log.info("Test listdescriptors with encrypted wallet")
         wallet.encryptwallet("pass")
         assert_equal(expected, wallet.listdescriptors())
 
+        self.log.info('Test list private descriptors with encrypted wallet')
+        assert_raises_rpc_error(-13, 'Please enter the wallet passphrase with walletpassphrase first.', wallet.listdescriptors, True)
+        wallet.walletpassphrase(passphrase="pass", timeout=1000000)
+        assert_equal(expected_private, wallet.listdescriptors(True))
+
+        self.log.info('Test list private descriptors with watch-only wallet')
+        node.createwallet(wallet_name='watch-only', descriptors=True, disable_private_keys=True)
+        watch_only_wallet = node.get_wallet_rpc('watch-only')
+        watch_only_wallet.importdescriptors([{
+            'desc': descsum_create('wpkh(' + xpub_acc + ')'),
+            'timestamp': 1296688602,
+        }])
+        assert_raises_rpc_error(-4, 'Can\'t get descriptor string', watch_only_wallet.listdescriptors, True)
+
         self.log.info('Test non-active non-range combo descriptor')
         node.createwallet(wallet_name='w4', blank=True, descriptors=True)
         wallet = node.get_wallet_rpc('w4')