Merge bitcoin/bitcoin#22929: wallet: Automatically add receiving destinations to the address book

3d71d16d1e test: listtranscations with externally generated addresses (S3RK)
d04566415e Add to spends only transcations from me (S3RK)
9f3a622b1c Automatically add labels to detected receiving addresses (S3RK)
c1b99c088c Return used destinations from ScriptPubKeyMan::MarkUnusedAddresses (S3RK)
03840c2064 Add CWallet::IsInternalScriptPubKeyMan (S3RK)
456e350926 wallet: resolve ambiguity of two ScriptPubKey managers providing same script (S3RK)

Pull request description:

  This PR fixes certain use-cases when **send-to-self** transactions are missing from `listtransactions` output.

  1. When a receiving address is generated externally to the wallet
  (e.g. same wallet running on two nodes, or by 3rd party from xpub)
  2. When restoring backup with lost metadata, but keypool gap is not exceeded yet

  When the block is connected or tx added to mempool we already mark used keys. This PR extends this logic to determine whether the destination is a receiving one and if yes add it to the address book with empty label.

  Works both for legacy and descriptors wallets.
  - For legacy it uses the internal flag from the keypool entry. Caveat: because we don't know which script type would be used we add all possible destinations for such keys.
  - For descriptor wallets it uses internal flag for the script pub key manager. Caveat: it only works for active descriptors.

  fixes #19856
  fixes #20293

ACKs for top commit:
  laanwj:
    Code review ACK 3d71d16d1e

Tree-SHA512: 03fafd5548ead0c4ffe9ebcc9eb2849f1d2fa7270fda4166419b86877d4e57dcf04460e465fbb9c90b42031f3c05d1b83f1b67a9f82c2a42980825ed1e7b52e6

src/wallet/rpcdump.cpp

@@ -1808,11 +1808,10 @@ RPCHelpMan listdescriptors()
         }
         spk.pushKV("desc", descriptor);
         spk.pushKV("timestamp", wallet_descriptor.creation_time);
-        const bool active = active_spk_mans.count(desc_spk_man) != 0;
+        spk.pushKV("active", active_spk_mans.count(desc_spk_man) != 0);
-        spk.pushKV("active", active);
+        const auto internal = wallet->IsInternalScriptPubKeyMan(desc_spk_man);
-        const auto& type = wallet_descriptor.descriptor->GetOutputType();
+        if (internal.has_value()) {
-        if (active && type) {
+            spk.pushKV("internal", *internal);
-            spk.pushKV("internal", wallet->GetScriptPubKeyMan(*type, true) == desc_spk_man);
         }
         if (wallet_descriptor.descriptor->IsRange()) {
             UniValue range(UniValue::VARR);

src/wallet/rpcwallet.cpp

@@ -3940,8 +3940,12 @@ RPCHelpMan getaddressinfo()
         ret.pushKV("solvable", false);
     }
 
+    const auto& spk_mans = pwallet->GetScriptPubKeyMans(scriptPubKey);
+    // In most cases there is only one matching ScriptPubKey manager and we can't resolve ambiguity in a better way
+    ScriptPubKeyMan* spk_man{nullptr};
+    if (spk_mans.size()) spk_man = *spk_mans.begin();
 
-    DescriptorScriptPubKeyMan* desc_spk_man = dynamic_cast<DescriptorScriptPubKeyMan*>(pwallet->GetScriptPubKeyMan(scriptPubKey));
+    DescriptorScriptPubKeyMan* desc_spk_man = dynamic_cast<DescriptorScriptPubKeyMan*>(spk_man);
     if (desc_spk_man) {
         std::string desc_str;
         if (desc_spk_man->GetDescriptorString(desc_str, /* priv */ false)) {
@@ -3956,7 +3960,6 @@ RPCHelpMan getaddressinfo()
 
     ret.pushKV("ischange", ScriptIsChange(*pwallet, scriptPubKey));
 
-    ScriptPubKeyMan* spk_man = pwallet->GetScriptPubKeyMan(scriptPubKey);
     if (spk_man) {
         if (const std::unique_ptr<CKeyMetadata> meta = spk_man->GetMetadata(dest)) {
             ret.pushKV("timestamp", meta->nCreateTime);

src/wallet/scriptpubkeyman.cpp

@@ -354,15 +354,22 @@ bool LegacyScriptPubKeyMan::TopUpInactiveHDChain(const CKeyID seed_id, int64_t i
     return true;
 }
 
-void LegacyScriptPubKeyMan::MarkUnusedAddresses(const CScript& script)
+std::vector<WalletDestination> LegacyScriptPubKeyMan::MarkUnusedAddresses(const CScript& script)
 {
     LOCK(cs_KeyStore);
+    std::vector<WalletDestination> result;
     // extract addresses and check if they match with an unused keypool key
     for (const auto& keyid : GetAffectedKeys(script, *this)) {
         std::map<CKeyID, int64_t>::const_iterator mi = m_pool_key_to_index.find(keyid);
         if (mi != m_pool_key_to_index.end()) {
             WalletLogPrintf("%s: Detected a used keypool key, mark all keypool keys up to this key as used\n", __func__);
-            MarkReserveKeysAsUsed(mi->second);
+            for (const auto& keypool : MarkReserveKeysAsUsed(mi->second)) {
+                // derive all possible destinations as any of them could have been used
+                for (const auto& type : LEGACY_OUTPUT_TYPES) {
+                    const auto& dest = GetDestinationForKey(keypool.vchPubKey, type);
+                    result.push_back({dest, keypool.fInternal});
+                }
+            }
 
             if (!TopUp()) {
                 WalletLogPrintf("%s: Topping up keypool failed (locked wallet)\n", __func__);
@@ -384,6 +391,8 @@ void LegacyScriptPubKeyMan::MarkUnusedAddresses(const CScript& script)
             }
         }
     }
+
+    return result;
 }
 
 void LegacyScriptPubKeyMan::UpgradeKeyMetadata()
@@ -1427,7 +1436,7 @@ void LegacyScriptPubKeyMan::LearnAllRelatedScripts(const CPubKey& key)
     LearnRelatedScripts(key, OutputType::P2SH_SEGWIT);
 }
 
-void LegacyScriptPubKeyMan::MarkReserveKeysAsUsed(int64_t keypool_id)
+std::vector<CKeyPool> LegacyScriptPubKeyMan::MarkReserveKeysAsUsed(int64_t keypool_id)
 {
     AssertLockHeld(cs_KeyStore);
     bool internal = setInternalKeyPool.count(keypool_id);
@@ -1435,6 +1444,7 @@ void LegacyScriptPubKeyMan::MarkReserveKeysAsUsed(int64_t keypool_id)
     std::set<int64_t> *setKeyPool = internal ? &setInternalKeyPool : (set_pre_split_keypool.empty() ? &setExternalKeyPool : &set_pre_split_keypool);
     auto it = setKeyPool->begin();
 
+    std::vector<CKeyPool> result;
     WalletBatch batch(m_storage.GetDatabase());
     while (it != std::end(*setKeyPool)) {
         const int64_t& index = *(it);
@@ -1448,7 +1458,10 @@ void LegacyScriptPubKeyMan::MarkReserveKeysAsUsed(int64_t keypool_id)
         batch.ErasePool(index);
         WalletLogPrintf("keypool index %d removed\n", index);
         it = setKeyPool->erase(it);
+        result.push_back(std::move(keypool));
     }
+
+    return result;
 }
 
 std::vector<CKeyID> GetAffectedKeys(const CScript& spk, const SigningProvider& provider)
@@ -1820,19 +1833,32 @@ bool DescriptorScriptPubKeyMan::TopUp(unsigned int size)
     return true;
 }
 
-void DescriptorScriptPubKeyMan::MarkUnusedAddresses(const CScript& script)
+std::vector<WalletDestination> DescriptorScriptPubKeyMan::MarkUnusedAddresses(const CScript& script)
 {
     LOCK(cs_desc_man);
+    std::vector<WalletDestination> result;
     if (IsMine(script)) {
         int32_t index = m_map_script_pub_keys[script];
         if (index >= m_wallet_descriptor.next_index) {
             WalletLogPrintf("%s: Detected a used keypool item at index %d, mark all keypool items up to this item as used\n", __func__, index);
-            m_wallet_descriptor.next_index = index + 1;
+            auto out_keys = std::make_unique<FlatSigningProvider>();
+            std::vector<CScript> scripts_temp;
+            while (index >= m_wallet_descriptor.next_index) {
+                if (!m_wallet_descriptor.descriptor->ExpandFromCache(m_wallet_descriptor.next_index, m_wallet_descriptor.cache, scripts_temp, *out_keys)) {
+                    throw std::runtime_error(std::string(__func__) + ": Unable to expand descriptor from cache");
+                }
+                CTxDestination dest;
+                ExtractDestination(scripts_temp[0], dest);
+                result.push_back({dest, std::nullopt});
+                m_wallet_descriptor.next_index++;
+            }
         }
         if (!TopUp()) {
             WalletLogPrintf("%s: Topping up keypool failed (locked wallet)\n", __func__);
         }
     }
+
+    return result;
 }
 
 void DescriptorScriptPubKeyMan::AddDescriptorKey(const CKey& key, const CPubKey &pubkey)

src/wallet/scriptpubkeyman.h

@@ -149,6 +149,12 @@ public:
     }
 };
 
+struct WalletDestination
+{
+    CTxDestination dest;
+    std::optional<bool> internal;
+};
+
 /*
  * A class implementing ScriptPubKeyMan manages some (or all) scriptPubKeys used in a wallet.
  * It contains the scripts and keys related to the scriptPubKeys it manages.
@@ -181,8 +187,14 @@ public:
       */
     virtual bool TopUp(unsigned int size = 0) { return false; }
 
-    //! Mark unused addresses as being used
+    /** Mark unused addresses as being used
-    virtual void MarkUnusedAddresses(const CScript& script) {}
+     * Affects all keys up to and including the one determined by provided script.
+     *
+     * @param script determines the last key to mark as used
+     *
+     * @return All of the addresses affected
+     */
+    virtual std::vector<WalletDestination> MarkUnusedAddresses(const CScript& script) { return {}; }
 
     /** Sets up the key generation stuff, i.e. generates new HD seeds and sets them as active.
       * Returns false if already setup or setup fails, true if setup is successful
@@ -357,7 +369,7 @@ public:
 
     bool TopUp(unsigned int size = 0) override;
 
-    void MarkUnusedAddresses(const CScript& script) override;
+    std::vector<WalletDestination> MarkUnusedAddresses(const CScript& script) override;
 
     //! Upgrade stored CKeyMetadata objects to store key origin info as KeyOriginInfo
     void UpgradeKeyMetadata();
@@ -482,9 +494,13 @@ public:
     void LearnAllRelatedScripts(const CPubKey& key);
 
     /**
-     * Marks all keys in the keypool up to and including reserve_key as used.
+     * Marks all keys in the keypool up to and including the provided key as used.
+     *
+     * @param keypool_id determines the last key to mark as used
+     *
+     * @return All affected keys
      */
-    void MarkReserveKeysAsUsed(int64_t keypool_id) EXCLUSIVE_LOCKS_REQUIRED(cs_KeyStore);
+    std::vector<CKeyPool> MarkReserveKeysAsUsed(int64_t keypool_id) EXCLUSIVE_LOCKS_REQUIRED(cs_KeyStore);
     const std::map<CKeyID, int64_t>& GetAllReserveKeys() const { return m_pool_key_to_index; }
 
     std::set<CKeyID> GetKeys() const override;
@@ -564,7 +580,7 @@ public:
     // (with or without private keys), the "keypool" is a single xpub.
     bool TopUp(unsigned int size = 0) override;
 
-    void MarkUnusedAddresses(const CScript& script) override;
+    std::vector<WalletDestination> MarkUnusedAddresses(const CScript& script) override;
 
     bool IsHDEnabled() const override;
 

src/wallet/test/wallet_tests.cpp

@@ -816,30 +816,35 @@ BOOST_FIXTURE_TEST_CASE(ZapSelectTx, TestChain100Setup)
     context.args = &gArgs;
     context.chain = m_node.chain.get();
     auto wallet = TestLoadWallet(context);
-    CKey key;
+    AddKey(*wallet, coinbaseKey);
-    key.MakeNewKey(true);
-    AddKey(*wallet, key);
 
-    std::string error;
+    // rescan to ensure coinbase transactions from test fixture are picked up by the wallet
+    {
+        WalletRescanReserver reserver(*wallet);
+        reserver.reserve();
+        wallet->ScanForWalletTransactions(m_node.chain->getBlockHash(0), 0, /* max height= */ {}, reserver, /* update= */ true);
+    }
+    // create one more block to get the first block coinbase to maturity
     m_coinbase_txns.push_back(CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())).vtx[0]);
-    auto block_tx = TestSimpleSpend(*m_coinbase_txns[0], 0, coinbaseKey, GetScriptForRawPubKey(key.GetPubKey()));
+    // spend first coinbase tx
-    CreateAndProcessBlock({block_tx}, GetScriptForRawPubKey(coinbaseKey.GetPubKey()));
+    auto spend_tx = TestSimpleSpend(*m_coinbase_txns[0], 0, coinbaseKey, GetScriptForRawPubKey(coinbaseKey.GetPubKey()));
+    CreateAndProcessBlock({spend_tx}, GetScriptForRawPubKey(coinbaseKey.GetPubKey()));
 
     SyncWithValidationInterfaceQueue();
 
     {
-        auto block_hash = block_tx.GetHash();
+        auto spend_tx_hash = spend_tx.GetHash();
         auto prev_hash = m_coinbase_txns[0]->GetHash();
 
         LOCK(wallet->cs_wallet);
         BOOST_CHECK(wallet->HasWalletSpend(prev_hash));
-        BOOST_CHECK_EQUAL(wallet->mapWallet.count(block_hash), 1u);
+        BOOST_CHECK_EQUAL(wallet->mapWallet.count(spend_tx_hash), 1u);
 
-        std::vector<uint256> vHashIn{ block_hash }, vHashOut;
+        std::vector<uint256> vHashIn{spend_tx_hash}, vHashOut;
         BOOST_CHECK_EQUAL(wallet->ZapSelectTx(vHashIn, vHashOut), DBErrors::LOAD_OK);
 
         BOOST_CHECK(!wallet->HasWalletSpend(prev_hash));
-        BOOST_CHECK_EQUAL(wallet->mapWallet.count(block_hash), 0u);
+        BOOST_CHECK_EQUAL(wallet->mapWallet.count(spend_tx_hash), 0u);
     }
 
     TestUnloadWallet(std::move(wallet));

src/wallet/wallet.cpp

@@ -919,7 +919,9 @@ CWalletTx* CWallet::AddToWallet(CTransactionRef tx, const TxState& state, const
         wtx.nOrderPos = IncOrderPosNext(&batch);
         wtx.m_it_wtxOrdered = wtxOrdered.insert(std::make_pair(wtx.nOrderPos, &wtx));
         wtx.nTimeSmart = ComputeTimeSmart(wtx, rescanning_old_block);
-        AddToSpends(hash, &batch);
+        if (IsFromMe(*tx.get())) {
+            AddToSpends(hash);
+        }
     }
 
     if (!fInsertedNew)
@@ -1061,8 +1063,23 @@ bool CWallet::AddToWalletIfInvolvingMe(const CTransactionRef& ptx, const SyncTxS
 
             // loop though all outputs
             for (const CTxOut& txout: tx.vout) {
-                for (const auto& spk_man_pair : m_spk_managers) {
+                for (const auto& spk_man : GetScriptPubKeyMans(txout.scriptPubKey)) {
-                    spk_man_pair.second->MarkUnusedAddresses(txout.scriptPubKey);
+                    for (auto &dest : spk_man->MarkUnusedAddresses(txout.scriptPubKey)) {
+                        // If internal flag is not defined try to infer it from the ScriptPubKeyMan
+                        if (!dest.internal.has_value()) {
+                            dest.internal = IsInternalScriptPubKeyMan(spk_man);
+                        }
+
+                        // skip if can't determine whether it's a receiving address or not
+                        if (!dest.internal.has_value()) continue;
+
+                        // If this is a receiving address and it's not in the address book yet
+                        // (e.g. it wasn't generated on this node or we're restoring from backup)
+                        // add it to the address book for proper transaction accounting
+                        if (!*dest.internal && !FindAddressBookEntry(dest.dest, /* allow_change= */ false)) {
+                            SetAddressBook(dest.dest, "", "receive");
+                        }
+                    }
                 }
             }
 
@@ -2253,16 +2270,15 @@ void ReserveDestination::ReturnDestination()
 bool CWallet::DisplayAddress(const CTxDestination& dest)
 {
     CScript scriptPubKey = GetScriptForDestination(dest);
-    const auto spk_man = GetScriptPubKeyMan(scriptPubKey);
+    for (const auto& spk_man : GetScriptPubKeyMans(scriptPubKey)) {
-    if (spk_man == nullptr) {
+        auto signer_spk_man = dynamic_cast<ExternalSignerScriptPubKeyMan *>(spk_man);
-        return false;
-    }
-    auto signer_spk_man = dynamic_cast<ExternalSignerScriptPubKeyMan*>(spk_man);
         if (signer_spk_man == nullptr) {
-        return false;
+            continue;
         }
         ExternalSigner signer = ExternalSignerScriptPubKeyMan::GetExternalSigner();
         return signer_spk_man->DisplayAddress(scriptPubKey, signer);
+    }
+    return false;
 }
 
 bool CWallet::LockCoin(const COutPoint& output, WalletBatch* batch)
@@ -3050,9 +3066,10 @@ ScriptPubKeyMan* CWallet::GetScriptPubKeyMan(const OutputType& type, bool intern
     return it->second;
 }
 
-std::set<ScriptPubKeyMan*> CWallet::GetScriptPubKeyMans(const CScript& script, SignatureData& sigdata) const
+std::set<ScriptPubKeyMan*> CWallet::GetScriptPubKeyMans(const CScript& script) const
 {
     std::set<ScriptPubKeyMan*> spk_mans;
+    SignatureData sigdata;
     for (const auto& spk_man_pair : m_spk_managers) {
         if (spk_man_pair.second->CanProvide(script, sigdata)) {
             spk_mans.insert(spk_man_pair.second.get());
@@ -3061,17 +3078,6 @@ std::set<ScriptPubKeyMan*> CWallet::GetScriptPubKeyMans(const CScript& script, S
     return spk_mans;
 }
 
-ScriptPubKeyMan* CWallet::GetScriptPubKeyMan(const CScript& script) const
-{
-    SignatureData sigdata;
-    for (const auto& spk_man_pair : m_spk_managers) {
-        if (spk_man_pair.second->CanProvide(script, sigdata)) {
-            return spk_man_pair.second.get();
-        }
-    }
-    return nullptr;
-}
-
 ScriptPubKeyMan* CWallet::GetScriptPubKeyMan(const uint256& id) const
 {
     if (m_spk_managers.count(id) > 0) {
@@ -3287,6 +3293,30 @@ DescriptorScriptPubKeyMan* CWallet::GetDescriptorScriptPubKeyMan(const WalletDes
     return nullptr;
 }
 
+std::optional<bool> CWallet::IsInternalScriptPubKeyMan(ScriptPubKeyMan* spk_man) const
+{
+    // Legacy script pubkey man can't be either external or internal
+    if (IsLegacy()) {
+        return std::nullopt;
+    }
+
+    // only active ScriptPubKeyMan can be internal
+    if (!GetActiveScriptPubKeyMans().count(spk_man)) {
+        return std::nullopt;
+    }
+
+    const auto desc_spk_man = dynamic_cast<DescriptorScriptPubKeyMan*>(spk_man);
+    if (!desc_spk_man) {
+        throw std::runtime_error(std::string(__func__) + ": unexpected ScriptPubKeyMan type.");
+    }
+
+    LOCK(desc_spk_man->cs_desc_man);
+    const auto& type = desc_spk_man->GetWalletDescriptor().descriptor->GetOutputType();
+    assert(type.has_value());
+
+    return GetScriptPubKeyMan(*type, /* internal= */ true) == desc_spk_man;
+}
+
 ScriptPubKeyMan* CWallet::AddWalletDescriptor(WalletDescriptor& desc, const FlatSigningProvider& signing_provider, const std::string& label, bool internal)
 {
     AssertLockHeld(cs_wallet);

src/wallet/wallet.h

@@ -809,14 +809,11 @@ public:
     //! Get the ScriptPubKeyMan for the given OutputType and internal/external chain.
     ScriptPubKeyMan* GetScriptPubKeyMan(const OutputType& type, bool internal) const;
 
-    //! Get the ScriptPubKeyMan for a script
+    //! Get all the ScriptPubKeyMans for a script
-    ScriptPubKeyMan* GetScriptPubKeyMan(const CScript& script) const;
+    std::set<ScriptPubKeyMan*> GetScriptPubKeyMans(const CScript& script) const;
     //! Get the ScriptPubKeyMan by id
     ScriptPubKeyMan* GetScriptPubKeyMan(const uint256& id) const;
 
-    //! Get all of the ScriptPubKeyMans for a script given additional information in sigdata (populated by e.g. a psbt)
-    std::set<ScriptPubKeyMan*> GetScriptPubKeyMans(const CScript& script, SignatureData& sigdata) const;
-
     //! Get the SigningProvider for a script
     std::unique_ptr<SigningProvider> GetSolvingProvider(const CScript& script) const;
     std::unique_ptr<SigningProvider> GetSolvingProvider(const CScript& script, SignatureData& sigdata) const;
@@ -882,6 +879,11 @@ public:
     //! Return the DescriptorScriptPubKeyMan for a WalletDescriptor if it is already in the wallet
     DescriptorScriptPubKeyMan* GetDescriptorScriptPubKeyMan(const WalletDescriptor& desc) const;
 
+    //! Returns whether the provided ScriptPubKeyMan is internal
+    //! @param[in] spk_man The ScriptPubKeyMan to test
+    //! @return contains value only for active DescriptorScriptPubKeyMan, otherwise undefined
+    std::optional<bool> IsInternalScriptPubKeyMan(ScriptPubKeyMan* spk_man) 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, bool internal) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
 };

test/functional/wallet_listtransactions.py

@@ -3,6 +3,10 @@
 # Distributed under the MIT software license, see the accompanying
 # file COPYING or http://www.opensource.org/licenses/mit-license.php.
 """Test the listtransactions API."""
+
+import shutil
+import os
+
 from decimal import Decimal
 
 from test_framework.messages import (
@@ -17,7 +21,7 @@ from test_framework.util import (
 
 class ListTransactionsTest(BitcoinTestFramework):
     def set_test_params(self):
-        self.num_nodes = 2
+        self.num_nodes = 3
         # This test isn't testing txn relay/timing, so set whitelist on the
         # peers for instant txn relay. This speeds up the test run time 2-3x.
         self.extra_args = [["-whitelist=noban@127.0.0.1"]] * self.num_nodes
@@ -102,7 +106,7 @@ class ListTransactionsTest(BitcoinTestFramework):
                                 {"txid": txid, "label": "watchonly"})
 
         self.run_rbf_opt_in_test()
-
+        self.run_externally_generated_address_test()
 
     def run_rbf_opt_in_test(self):
         """Test the opt-in-rbf flag for sent and received transactions."""
@@ -217,5 +221,63 @@ class ListTransactionsTest(BitcoinTestFramework):
         assert_equal(self.nodes[0].gettransaction(txid_3b)["bip125-replaceable"], "no")
         assert_equal(self.nodes[0].gettransaction(txid_4)["bip125-replaceable"], "unknown")
 
+    def run_externally_generated_address_test(self):
+        """Test behavior when receiving address is not in the address book."""
+
+        self.log.info("Setup the same wallet on two nodes")
+        # refill keypool otherwise the second node wouldn't recognize addresses generated on the first nodes
+        self.nodes[0].keypoolrefill(1000)
+        self.stop_nodes()
+        wallet0 = os.path.join(self.nodes[0].datadir, self.chain, self.default_wallet_name, "wallet.dat")
+        wallet2 = os.path.join(self.nodes[2].datadir, self.chain, self.default_wallet_name, "wallet.dat")
+        shutil.copyfile(wallet0, wallet2)
+        self.start_nodes()
+        # reconnect nodes
+        self.connect_nodes(0, 1)
+        self.connect_nodes(1, 2)
+        self.connect_nodes(2, 0)
+
+        addr1 = self.nodes[0].getnewaddress("pizza1", 'legacy')
+        addr2 = self.nodes[0].getnewaddress("pizza2", 'p2sh-segwit')
+        addr3 = self.nodes[0].getnewaddress("pizza3", 'bech32')
+
+        self.log.info("Send to externally generated addresses")
+        # send to an address beyond the next to be generated to test the keypool gap
+        self.nodes[1].sendtoaddress(addr3, "0.001")
+        self.nodes[1].generate(1)
+        self.sync_all()
+
+        # send to an address that is already marked as used due to the keypool gap mechanics
+        self.nodes[1].sendtoaddress(addr2, "0.001")
+        self.nodes[1].generate(1)
+        self.sync_all()
+
+        # send to self transaction
+        self.nodes[0].sendtoaddress(addr1, "0.001")
+        self.nodes[0].generate(1)
+        self.sync_all()
+
+        self.log.info("Verify listtransactions is the same regardless of where the address was generated")
+        transactions0 = self.nodes[0].listtransactions()
+        transactions2 = self.nodes[2].listtransactions()
+
+        # normalize results: remove fields that normally could differ and sort
+        def normalize_list(txs):
+            for tx in txs:
+                tx.pop('label', None)
+                tx.pop('time', None)
+                tx.pop('timereceived', None)
+            txs.sort(key=lambda x: x['txid'])
+
+        normalize_list(transactions0)
+        normalize_list(transactions2)
+        assert_equal(transactions0, transactions2)
+
+        self.log.info("Verify labels are persistent on the node generated the addresses")
+        assert_equal(['pizza1'], self.nodes[0].getaddressinfo(addr1)['labels'])
+        assert_equal(['pizza2'], self.nodes[0].getaddressinfo(addr2)['labels'])
+        assert_equal(['pizza3'], self.nodes[0].getaddressinfo(addr3)['labels'])
+
+
 if __name__ == '__main__':
     ListTransactionsTest().main()