Merge #14679: importmulti: Don't add internal addresses to address book

7afddfa8cefd01249ad59cf2370e7cec90b34f6f importmulti: Don't add internal addresses to address book (Gregory Sanders)

Pull request description:

  Currently anything imported with `internal` will not be treated as change since checking the address book is a primary test of this.

  Added basic tests of all combinations of arguments and change identification.

  Resolves https://github.com/bitcoin/bitcoin/issues/14662

Tree-SHA512: a1f08dc624a3fadee93cc5392d50c4796b0c5eedf38e295382f71570f2066d9e978ed6e3962084b902989863fe1273a8642d8fdb094a266d69de10622a4176b0

2 files changed, 10 insertions, 6 deletions

diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp
index a2b7adc5e6..00a9a6ec8b 100644
--- a/src/wallet/rpcdump.cpp
+++ b/src/wallet/rpcdump.cpp
@@ -1009,8 +1009,9 @@ static UniValue ProcessImport(CWallet * const pwallet, const UniValue& data, con
             throw JSONRPCError(RPC_WALLET_ERROR, "Error adding scriptPubKey script to wallet");
         }
 
-        // add to address book or update label
-        if (IsValidDestination(scriptpubkey_dest)) {
+        // if not internal add to address book or update label
+        if (!internal) {
+            assert(IsValidDestination(scriptpubkey_dest));
             pwallet->SetAddressBook(scriptpubkey_dest, label, "receive");
         }
 
@@ -1102,7 +1103,7 @@ UniValue importmulti(const JSONRPCRequest& mainRequest)
             "      \"witnessscript\": \"<script>\"                           , (string, optional) Allowed only if the scriptPubKey is a P2SH-P2WSH or P2WSH address/scriptPubKey\n"
             "      \"pubkeys\": [\"<pubKey>\", ... ]                         , (array, optional) Array of strings giving pubkeys that must occur in the output or redeemscript\n"
             "      \"keys\": [\"<key>\", ... ]                               , (array, optional) Array of strings giving private keys whose corresponding public keys must occur in the output or redeemscript\n"
-            "      \"internal\": <true>                                    , (boolean, optional, default: false) Stating whether matching outputs should be treated as not incoming payments\n"
+            "      \"internal\": <true>                                    , (boolean, optional, default: false) Stating whether matching outputs should be treated as not incoming payments aka change\n"
             "      \"watchonly\": <true>                                   , (boolean, optional, default: false) Stating whether matching outputs should be considered watched even when they're not spendable, only allowed if keys are empty\n"
             "      \"label\": <label>                                      , (string, optional, default: '') Label to assign to the address, only allowed with internal=false\n"
             "    }\n"
diff --git a/test/functional/wallet_importmulti.py b/test/functional/wallet_importmulti.py
index 9ba6860306..f2855b6e89 100755
--- a/test/functional/wallet_importmulti.py
+++ b/test/functional/wallet_importmulti.py
@@ -54,7 +54,7 @@ class ImportMultiTest(BitcoinTestFramework):
 
         # RPC importmulti -----------------------------------------------
 
-        # Bitcoin Address
+        # Bitcoin Address (implicit non-internal)
         self.log.info("Should import an address")
         address = self.nodes[0].getaddressinfo(self.nodes[0].getnewaddress())
         result = self.nodes[1].importmulti([{
@@ -68,6 +68,7 @@ class ImportMultiTest(BitcoinTestFramework):
         assert_equal(address_assert['iswatchonly'], True)
         assert_equal(address_assert['ismine'], False)
         assert_equal(address_assert['timestamp'], timestamp)
+        assert_equal(address_assert['ischange'], False)
         watchonly_address = address['address']
         watchonly_timestamp = timestamp
 
@@ -95,6 +96,7 @@ class ImportMultiTest(BitcoinTestFramework):
         assert_equal(address_assert['iswatchonly'], True)
         assert_equal(address_assert['ismine'], False)
         assert_equal(address_assert['timestamp'], timestamp)
+        assert_equal(address_assert['ischange'], True)
 
         # ScriptPubKey + internal + label
         self.log.info("Should not allow a label to be specified when internal is true")
@@ -126,7 +128,7 @@ class ImportMultiTest(BitcoinTestFramework):
         assert_equal('timestamp' in address_assert, False)
 
 
-        # Address + Public key + !Internal
+        # Address + Public key + !Internal(explicit)
         self.log.info("Should import an address with public key")
         address = self.nodes[0].getaddressinfo(self.nodes[0].getnewaddress())
         result = self.nodes[1].importmulti([{
@@ -134,7 +136,8 @@ class ImportMultiTest(BitcoinTestFramework):
                 "address": address['address']
             },
             "timestamp": "now",
-            "pubkeys": [ address['pubkey'] ]
+            "pubkeys": [ address['pubkey'] ],
+            "internal": False
         }])
         assert_equal(result[0]['success'], True)
         address_assert = self.nodes[1].getaddressinfo(address['address'])