Add change field to z_listreceivedbyaddress for transparent addrs.

This updates the `IsChange` method to check the HD keypath associated
with the key for whether the address was generated as internal
or external.

qa/rpc-tests/wallet_listreceived.py

@@ -477,7 +477,7 @@ class ListReceivedTest (BitcoinTestFramework):
             'txid': txid_taddr,
             'amount': Decimal('0.2'),
             'amountZat': 20000000,
-            'change': None,
+            'change': False,
             'memo': None,
             'confirmations': 0,
         } in outputs)

src/wallet/rpcwallet.cpp

@@ -3443,7 +3443,7 @@ UniValue z_listreceivedbyaddress(const UniValue& params, bool fHelp)
             "  \"jsindex\" (sprout) : n,       (numeric) the joinsplit index\n"
             "  \"jsoutindex\" (sprout) : n,    (numeric) the output index of the joinsplit\n"
             "  \"outindex\" (transparent, sapling, orchard) : n, (numeric) the output index for transparent and Sapling outputs, or the action index for Orchard\n"
-            "  \"change\": true|false,         (boolean) true if the address that received the note is also one of the sending addresses\n"
+            "  \"change\": true|false,         (boolean) true if the output was received to a change address\n"
             "}\n"
             "\nExamples:\n"
             + HelpExampleCli("z_listreceivedbyaddress", "\"ztfaW34Gj9FrnGUEf833ywDVL62NWXBM81u6EQnM6VR45eYnXhwztecW1SjxA7JrmAXKJhxhj3vDNEpVCQoSvVoSpmbhtjf\"")
@@ -3498,6 +3498,7 @@ UniValue z_listreceivedbyaddress(const UniValue& params, bool fHelp)
                     obj.pushKV("amountZat", txout.nValue);
                     obj.pushKV("outindex", int(i));
                     obj.pushKV("confirmations", nDepth);
+                    obj.pushKV("change", pwalletMain->IsChange(txout));
                     result.push_back(obj);
                 }
             }

src/wallet/wallet.cpp

@@ -1,5 +1,6 @@
 // Copyright (c) 2009-2010 Satoshi Nakamoto
 // Copyright (c) 2009-2014 The Bitcoin Core developers
+// Copyright (c) 2016-2022 The Zcash developers
 // Distributed under the MIT software license, see the accompanying
 // file COPYING or https://www.opensource.org/licenses/mit-license.php .
 
@@ -3008,24 +3009,37 @@ CAmount CWallet::GetCredit(const CTxOut& txout, const isminefilter& filter) cons
 
 bool CWallet::IsChange(const CTxOut& txout) const
 {
-    // TODO: fix handling of 'change' outputs. The assumption is that any
-    // payment to a script that is ours, but is not in the address book
-    // is change. That assumption is likely to break when we implement multisignature
-    // wallets that return change back into a multi-signature-protected address;
-    // a better way of identifying which outputs are 'the send' and which are
-    // 'the change' will need to be implemented (maybe extend CWalletTx to remember
-    // which output, if any, was change).
-    if (::IsMine(*this, txout.scriptPubKey))
-    {
-        CTxDestination address;
-        if (!ExtractDestination(txout.scriptPubKey, address))
-            return true;
+    // Addresses must be ours to be change
+    if (!::IsMine(*this, txout.scriptPubKey))
+        return false;
 
-        LOCK(cs_wallet);
-        if (!mapAddressBook.count(address))
-            return true;
-    }
-    return false;
+    // Only p2pkh addresses are used for change
+    CTxDestination address;
+    if (!ExtractDestination(txout.scriptPubKey, address))
+        return true;
+
+    LOCK(cs_wallet);
+    // Any payment to a script that is in the address book is not change.
+    if (mapAddressBook.count(address))
+        return false;
+
+    // We look to key metadata to determine whether the address was generated
+    // using an internal key path. This could fail to identify some legacy
+    // change addresses as change outputs.
+    return std::visit(match {
+        [&](const CKeyID& key) {
+            auto keyMetaIt = mapKeyMetadata.find(key);
+            return
+                // If we don't have key metadata, it's a legacy address that is not
+                // in the address book, so we judge it to be change.
+                keyMetaIt == mapKeyMetadata.end() ||
+                keyMetaIt->second.hdKeypath == "" ||
+                // If we do have non-null key metadata, we inspect the metadata to
+                // make our determination
+                IsInternalKeyPath(44, BIP44CoinType(), keyMetaIt->second.hdKeypath);
+        },
+        [&](const auto& other) { return false; }
+    }, address);
 }
 
 CAmount CWallet::GetChange(const CTxOut& txout) const

src/zcash/address/zip32.cpp

@@ -285,4 +285,14 @@ std::optional<unsigned long> ParseHDKeypathAccount(uint32_t purpose, uint32_t co
     }
 }
 
+bool IsInternalKeyPath(uint32_t purpose, uint32_t coinType, const std::string& keyPath) {
+    std::regex pattern("m/" + std::to_string(purpose)  + "'/" + std::to_string(coinType) + "'/[0-9]+'/([01])/.*");
+    std::smatch matches;
+    if (std::regex_match(keyPath, matches, pattern)) {
+        return stoul(matches[1]) == 1;
+    } else {
+        return false;
+    }
+}
+
 };

src/zcash/address/zip32.h

@@ -287,6 +287,11 @@ std::optional<unsigned long> ParseHDKeypathAccount(
         uint32_t coinType,
         const std::string& keyPath);
 
+bool IsInternalKeyPath(
+        uint32_t purpose,
+        uint32_t coinType,
+        const std::string& keyPath);
+
 } //namespace libzcash
 
 #endif // ZCASH_ZCASH_ADDRESS_ZIP32_H