wallet: Migrate entire address book entries

Author: achow101

src/wallet/wallet.cpp

@@ -3960,25 +3960,17 @@ bool CWallet::ApplyMigrationData(MigrationData& data, bilingual_str& error)
                 if (data.watchonly_wallet) {
                     LOCK(data.watchonly_wallet->cs_wallet);
                     if (data.watchonly_wallet->IsMine(addr_pair.first)) {
-                        // Add to the watchonly. Preserve the labels, purpose, and change-ness
-                        std::string label = addr_pair.second.GetLabel();
-                        data.watchonly_wallet->m_address_book[addr_pair.first].purpose = addr_pair.second.purpose;
-                        if (!addr_pair.second.IsChange()) {
-                            data.watchonly_wallet->m_address_book[addr_pair.first].SetLabel(label);
-                        }
+                        // Add to the watchonly. Copy the entire address book entry
+                        data.watchonly_wallet->m_address_book[addr_pair.first] = addr_pair.second;
                         dests_to_delete.push_back(addr_pair.first);
                         continue;
                     }
                 }
                 if (data.solvable_wallet) {
                     LOCK(data.solvable_wallet->cs_wallet);
                     if (data.solvable_wallet->IsMine(addr_pair.first)) {
-                        // Add to the solvable. Preserve the labels, purpose, and change-ness
-                        std::string label = addr_pair.second.GetLabel();
-                        data.solvable_wallet->m_address_book[addr_pair.first].purpose = addr_pair.second.purpose;
-                        if (!addr_pair.second.IsChange()) {
-                            data.solvable_wallet->m_address_book[addr_pair.first].SetLabel(label);
-                        }
+                        // Add to the solvable. Copy the entire address book entry
+                        data.solvable_wallet->m_address_book[addr_pair.first] = addr_pair.second;
                         dests_to_delete.push_back(addr_pair.first);
                         continue;
                     }
@@ -3998,21 +3990,13 @@ bool CWallet::ApplyMigrationData(MigrationData& data, bilingual_str& error)
             // Labels for everything else ("send") should be cloned to all
             if (data.watchonly_wallet) {
                 LOCK(data.watchonly_wallet->cs_wallet);
-                // Add to the watchonly. Preserve the labels, purpose, and change-ness
-                std::string label = addr_pair.second.GetLabel();
-                data.watchonly_wallet->m_address_book[addr_pair.first].purpose = addr_pair.second.purpose;
-                if (!addr_pair.second.IsChange()) {
-                    data.watchonly_wallet->m_address_book[addr_pair.first].SetLabel(label);
-                }
+                // Add to the watchonly. Copy the entire address book entry
+                data.watchonly_wallet->m_address_book[addr_pair.first] = addr_pair.second;
             }
             if (data.solvable_wallet) {
                 LOCK(data.solvable_wallet->cs_wallet);
-                // Add to the solvable. Preserve the labels, purpose, and change-ness
-                std::string label = addr_pair.second.GetLabel();
-                data.solvable_wallet->m_address_book[addr_pair.first].purpose = addr_pair.second.purpose;
-                if (!addr_pair.second.IsChange()) {
-                    data.solvable_wallet->m_address_book[addr_pair.first].SetLabel(label);
-                }
+                // Add to the solvable. Copy the entire address book entry
+                data.solvable_wallet->m_address_book[addr_pair.first] = addr_pair.second;
             }
         }
     }
@@ -4023,10 +4007,12 @@ bool CWallet::ApplyMigrationData(MigrationData& data, bilingual_str& error)
         WalletBatch batch{wallet.GetDatabase()};
         for (const auto& [destination, addr_book_data] : wallet.m_address_book) {
             auto address{EncodeDestination(destination)};
-            std::optional<std::string> label = addr_book_data.IsChange() ? std::nullopt : std::make_optional(addr_book_data.GetLabel());
-            // don't bother writing default values (unknown purpose)
             if (addr_book_data.purpose) batch.WritePurpose(address, PurposeToString(*addr_book_data.purpose));
-            if (label) batch.WriteName(address, *label);
+            if (addr_book_data.label) batch.WriteName(address, *addr_book_data.label);
+            for (const auto& [id, request] : addr_book_data.receive_requests) {
+                batch.WriteAddressReceiveRequest(destination, id, request);
+            }
+            if (addr_book_data.previously_spent) batch.WriteAddressPreviouslySpent(destination, true);
         }
     };
     if (data.watchonly_wallet) persist_address_book(*data.watchonly_wallet);

test/functional/wallet_migration.py

@@ -877,6 +877,61 @@ def test_failed_migration_cleanup(self):
             assert_equal(magic, BTREE_MAGIC)
 
 
+    def test_avoidreuse(self):
+        self.log.info("Test that avoidreuse persists after migration")
+        def_wallet = self.nodes[0].get_wallet_rpc(self.default_wallet_name)
+
+        wallet = self.create_legacy_wallet("avoidreuse")
+        wallet.setwalletflag("avoid_reuse", True)
+
+        # Import a pubkey to the test wallet and send some funds to it
+        reused_imported_addr = def_wallet.getnewaddress()
+        wallet.importpubkey(def_wallet.getaddressinfo(reused_imported_addr)["pubkey"])
+        imported_utxos = self.create_outpoints(def_wallet, outputs=[{reused_imported_addr: 2}])
+        def_wallet.lockunspent(False, imported_utxos)
+
+        # Send funds to the test wallet
+        reused_addr = wallet.getnewaddress()
+        def_wallet.sendtoaddress(reused_addr, 2)
+
+        self.generate(self.nodes[0], 1)
+
+        # Send funds from the test wallet with both its own and the imported
+        wallet.sendall([def_wallet.getnewaddress()])
+        def_wallet.sendall(recipients=[def_wallet.getnewaddress()], inputs=imported_utxos)
+        self.generate(self.nodes[0], 1)
+        balances = wallet.getbalances()
+        assert_equal(balances["mine"]["trusted"], 0)
+        assert_equal(balances["watchonly"]["trusted"], 0)
+
+        # Reuse the addresses
+        def_wallet.sendtoaddress(reused_addr, 1)
+        def_wallet.sendtoaddress(reused_imported_addr, 1)
+        self.generate(self.nodes[0], 1)
+        balances = wallet.getbalances()
+        assert_equal(balances["mine"]["used"], 1)
+        # Reused watchonly will not show up in balances
+        assert_equal(balances["watchonly"]["trusted"], 0)
+        assert_equal(balances["watchonly"]["untrusted_pending"], 0)
+        assert_equal(balances["watchonly"]["immature"], 0)
+
+        utxos = wallet.listunspent()
+        assert_equal(len(utxos), 2)
+        for utxo in utxos:
+            assert_equal(utxo["reused"], True)
+
+        # Migrate
+        migrate_res = wallet.migratewallet()
+        watchonly_wallet = self.nodes[0].get_wallet_rpc(migrate_res["watchonly_name"])
+
+        # One utxo in each wallet, marked used
+        utxos = wallet.listunspent()
+        assert_equal(len(utxos), 1)
+        assert_equal(utxos[0]["reused"], True)
+        watchonly_utxos = watchonly_wallet.listunspent()
+        assert_equal(len(watchonly_utxos), 1)
+        assert_equal(watchonly_utxos[0]["reused"], True)
+
     def run_test(self):
         self.generate(self.nodes[0], 101)
 
@@ -896,6 +951,7 @@ def run_test(self):
         self.test_conflict_txs()
         self.test_hybrid_pubkey()
         self.test_failed_migration_cleanup()
+        self.test_avoidreuse()
 
 if __name__ == '__main__':
     WalletMigrationTest().main()