foster/bitcoin-bitcoin-core
0
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-02-08 10:31:50 -05:00
Code Issues Activity

Merge bitcoin/bitcoin#28610: wallet: Migrate entire address book entries to watchonly and solvables too

Browse source 
406b71abcb wallet: Migrate entire address book entries (Andrew Chow)

Pull request description:

  Not all of the data in an address book entry was being copied to the watchonly and solvables wallets. This includes information such as whether the address was previously spent, and any receive requests that may exist. A test has been added to check that the previously spent information is copied, although it passes without the changes in this PR since this information is also regenerated when a transaction is loaded/added into a wallet.

ACKs for top commit:
  ryanofsky:
    Code review ACK 406b71abcb. Just suggested change since last review
  furszy:
    Code review ACK 406b71ab

Tree-SHA512: 13de42b16a1d8524fe0555764744139566b2e7d29741ceffc1158a905dd537136b762330568b3b5cac28cbee1bfd363a20de97d0a6c5296738cb3aa99133945b
This commit is contained in:
fanquake 2024-01-08 14:41:02 +00:00
commit c2d04f1319
No known key found for this signature in database
GPG key ID: 2EEB9F5CC09526C1
2 changed files with 69 additions and 27 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

40
src/wallet/wallet.cpp
View file

@ -3989,12 +3989,8 @@ 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;
}
@ -4002,12 +3998,8 @@ bool CWallet::ApplyMigrationData(MigrationData& data, bilingual_str& error)
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;
}
@ -4027,21 +4019,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;
}
}
}
@ -4052,10 +4036,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);

56
test/functional/wallet_migration.py
View file

@ -877,6 +877,61 @@ class WalletMigrationTest(BitcoinTestFramework):
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 @@ class WalletMigrationTest(BitcoinTestFramework):
self.test_conflict_txs()
self.test_hybrid_pubkey()
self.test_failed_migration_cleanup()
self.test_avoidreuse()
if __name__ == '__main__':
WalletMigrationTest().main()