Merge bitcoin/bitcoin#26761: wallet: fully migrate address book entries for watchonly/solvable wallets

730e14a317 test: wallet: check that labels are migrated to watchonly wallet (Sebastian Falbesoner)
d5f4ae7fac wallet: fully migrate address book entries for watchonly/solvable wallets (Sebastian Falbesoner)

Pull request description:

  Currently `migratewallet` migrates the address book (i.e. labels and purposes) for watchonly and solvable wallets only in RAM, but doesn't persist them on disk. Fix this by adding another loop for both of the special wallet types after which writes the corresponding NAME and PURPOSE entries to the database in a single batch. Also adds a corresponding test that checks if labels were migrated correctly for a watchonly wallet.

ACKs for top commit:
  achow101:
    ACK 730e14a317
  furszy:
    code ACK 730e14a3, left a non-blocking nit.
  aureleoules:
    ACK 730e14a317

Tree-SHA512: 159487e11e858924ef762e0190ccaea185bdff239e3d2280c8d63c4ac2649ec71714dc4d53dec644f03488f91c3b4bbbbf3434dad23bc0fcecb6657f353ea766
This commit is contained in:
Andrew Chow 2023-01-05 12:12:16 -05:00
commit b4fb0a3255
No known key found for this signature in database
GPG key ID: 17565732E08E5E41
2 changed files with 25 additions and 1 deletions

View file

@ -4001,6 +4001,23 @@ bool CWallet::ApplyMigrationData(MigrationData& data, bilingual_str& error)
} }
} }
} }
// Persist added address book entries (labels, purpose) for watchonly and solvable wallets
auto persist_address_book = [](const CWallet& wallet) {
LOCK(wallet.cs_wallet);
WalletBatch batch{wallet.GetDatabase()};
for (const auto& [destination, addr_book_data] : wallet.m_address_book) {
auto address{EncodeDestination(destination)};
auto purpose{addr_book_data.purpose};
auto label{addr_book_data.GetLabel()};
// don't bother writing default values (unknown purpose, empty label)
if (purpose != "unknown") batch.WritePurpose(address, purpose);
if (!label.empty()) batch.WriteName(address, label);
}
};
if (data.watchonly_wallet) persist_address_book(*data.watchonly_wallet);
if (data.solvable_wallet) persist_address_book(*data.solvable_wallet);
// Remove the things to delete // Remove the things to delete
if (dests_to_delete.size() > 0) { if (dests_to_delete.size() > 0) {
for (const auto& dest : dests_to_delete) { for (const auto& dest : dests_to_delete) {

View file

@ -258,7 +258,7 @@ class WalletMigrationTest(BitcoinTestFramework):
self.log.info("Test migration of a wallet with watchonly imports") self.log.info("Test migration of a wallet with watchonly imports")
imports0 = self.create_legacy_wallet("imports0") imports0 = self.create_legacy_wallet("imports0")
# Exteranl address label # External address label
imports0.setlabel(default.getnewaddress(), "external") imports0.setlabel(default.getnewaddress(), "external")
# Normal non-watchonly tx # Normal non-watchonly tx
@ -311,6 +311,13 @@ class WalletMigrationTest(BitcoinTestFramework):
assert_raises_rpc_error(-5, "Invalid or non-wallet transaction id", watchonly.gettransaction, received_txid) assert_raises_rpc_error(-5, "Invalid or non-wallet transaction id", watchonly.gettransaction, received_txid)
assert_equal(len(watchonly.listtransactions(include_watchonly=True)), 3) assert_equal(len(watchonly.listtransactions(include_watchonly=True)), 3)
# Check that labels were migrated and persisted to watchonly wallet
self.nodes[0].unloadwallet("imports0_watchonly")
self.nodes[0].loadwallet("imports0_watchonly")
labels = watchonly.listlabels()
assert "external" in labels
assert "imported" in labels
def test_no_privkeys(self): def test_no_privkeys(self):
default = self.nodes[0].get_wallet_rpc(self.default_wallet_name) default = self.nodes[0].get_wallet_rpc(self.default_wallet_name)