Merge #20266: wallet: fix change detection of imported internal descriptors

bd93fc9 Fix change detection of imported internal descriptors (Andrew Chow)

Pull request description:

  Import internal descriptors were having address book entries added which meant they would be detected as non-change. Fix this and add a test for it.

ACKs for top commit:
  laanwj:
    Code review ACK bd93fc9
  meshcollider:
    utACK bd93fc9
  promag:
    Code review ACK bd93fc9.

Tree-SHA512: 8fa9e364be317627ec171eedffdb505976c0e7f1e55bc7e8cfdffa3aeea5db24d231f55166602cd0e97a5ba621acc871de0a765c75d0c65678f83e93c3b657c5

Author: laanwj

src/wallet/rpcdump.cpp

@@ -1559,7 +1559,7 @@ static UniValue ProcessDescriptorImport(CWallet * const pwallet, const UniValue&
         }
 
         // Add descriptor to the wallet
-        auto spk_manager = pwallet->AddWalletDescriptor(w_desc, keys, label);
+        auto spk_manager = pwallet->AddWalletDescriptor(w_desc, keys, label, internal);
         if (spk_manager == nullptr) {
             throw JSONRPCError(RPC_WALLET_ERROR, strprintf("Could not add descriptor '%s'", descriptor));
         }

src/wallet/wallet.cpp

@@ -4511,7 +4511,7 @@ DescriptorScriptPubKeyMan* CWallet::GetDescriptorScriptPubKeyMan(const WalletDes
     return nullptr;
 }
 
-ScriptPubKeyMan* CWallet::AddWalletDescriptor(WalletDescriptor& desc, const FlatSigningProvider& signing_provider, const std::string& label)
+ScriptPubKeyMan* CWallet::AddWalletDescriptor(WalletDescriptor& desc, const FlatSigningProvider& signing_provider, const std::string& label, bool internal)
 {
     if (!IsWalletFlagSet(WALLET_FLAG_DESCRIPTORS)) {
         WalletLogPrintf("Cannot add WalletDescriptor to a non-descriptor wallet\n");
@@ -4568,7 +4568,7 @@ ScriptPubKeyMan* CWallet::AddWalletDescriptor(WalletDescriptor& desc, const Flat
         }
 
         CTxDestination dest;
-        if (ExtractDestination(script_pub_keys.at(0), dest)) {
+        if (!internal && ExtractDestination(script_pub_keys.at(0), dest)) {
             SetAddressBook(dest, label, "receive");
         }
     }

src/wallet/wallet.h

@@ -1280,7 +1280,7 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati
     DescriptorScriptPubKeyMan* GetDescriptorScriptPubKeyMan(const WalletDescriptor& desc) const;
 
     //! Add a descriptor to the wallet, return a ScriptPubKeyMan & associated output type
-    ScriptPubKeyMan* AddWalletDescriptor(WalletDescriptor& desc, const FlatSigningProvider& signing_provider, const std::string& label);
+    ScriptPubKeyMan* AddWalletDescriptor(WalletDescriptor& desc, const FlatSigningProvider& signing_provider, const std::string& label, bool internal);
 };
 
 /**

test/functional/wallet_importdescriptors.py

@@ -15,6 +15,7 @@
 - `test_address()` is called to call getaddressinfo for an address on node1
   and test the values returned."""
 
+from test_framework.address import key_to_p2pkh
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.descriptors import descsum_create
 from test_framework.util import (
@@ -107,6 +108,17 @@ def run_test(self):
                              error_code=-8,
                              error_message="Internal addresses should not have a label")
 
+        self.log.info("Internal addresses should be detected as such")
+        key = get_generate_key()
+        addr = key_to_p2pkh(key.pubkey)
+        self.test_importdesc({"desc": descsum_create("pkh(" + key.pubkey + ")"),
+                              "timestamp": "now",
+                              "internal": True},
+                             success=True)
+        info = w1.getaddressinfo(addr)
+        assert_equal(info["ismine"], True)
+        assert_equal(info["ischange"], True)
+
         # # Test importing of a P2SH-P2WPKH descriptor
         key = get_generate_key()
         self.log.info("Should not import a p2sh-p2wpkh descriptor without checksum")