diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp index 1f2680fa0a6c..c5922b0e7a30 100644 --- a/src/node/blockstorage.cpp +++ b/src/node/blockstorage.cpp @@ -574,16 +574,19 @@ bool UndoReadFromDisk(CBlockUndo& blockundo, const CBlockIndex* pindex) return true; } -void BlockManager::FlushUndoFile(int block_file, bool finalize) +bool BlockManager::FlushUndoFile(int block_file, bool finalize) { FlatFilePos undo_pos_old(block_file, m_blockfile_info[block_file].nUndoSize); if (!UndoFileSeq().Flush(undo_pos_old, finalize)) { AbortNode("Flushing undo file to disk failed. This is likely the result of an I/O error."); + return false; } + return true; } -void BlockManager::FlushBlockFile(bool fFinalize, bool finalize_undo) +bool BlockManager::FlushBlockFile(bool fFinalize, bool finalize_undo) { + bool success = true; LOCK(cs_LastBlockFile); if (m_blockfile_info.size() < 1) { @@ -591,17 +594,23 @@ void BlockManager::FlushBlockFile(bool fFinalize, bool finalize_undo) // chainstate init, when we call ChainstateManager::MaybeRebalanceCaches() (which // then calls FlushStateToDisk()), resulting in a call to this function before we // have populated `m_blockfile_info` via LoadBlockIndexDB(). - return; + return true; } assert(static_cast(m_blockfile_info.size()) > m_last_blockfile); FlatFilePos block_pos_old(m_last_blockfile, m_blockfile_info[m_last_blockfile].nSize); if (!BlockFileSeq().Flush(block_pos_old, fFinalize)) { AbortNode("Flushing block file to disk failed. This is likely the result of an I/O error."); + success = false; } // we do not always flush the undo file, as the chain tip may be lagging behind the incoming blocks, // e.g. during IBD or a sync after a node going offline - if (!fFinalize || finalize_undo) FlushUndoFile(m_last_blockfile, finalize_undo); + if (!fFinalize || finalize_undo) { + if (!FlushUndoFile(m_last_blockfile, finalize_undo)) { + success = false; + } + } + return success; } uint64_t BlockManager::CalculateCurrentUsage() @@ -696,7 +705,19 @@ bool BlockManager::FindBlockPos(FlatFilePos& pos, unsigned int nAddSize, unsigne if (!fKnown) { LogPrint(BCLog::BLOCKSTORAGE, "Leaving block file %i: %s\n", m_last_blockfile, m_blockfile_info[m_last_blockfile].ToString()); } - FlushBlockFile(!fKnown, finalize_undo); + + // Do not propagate the return code. The flush concerns a previous block + // and undo file that has already been written to. If a flush fails + // here, and we crash, there is no expected additional block data + // inconsistency arising from the flush failure here. However, the undo + // data may be inconsistent after a crash if the flush is called during + // a reindex. A flush error might also leave some of the data files + // untrimmed. + if (!FlushBlockFile(!fKnown, finalize_undo)) { + LogPrintLevel(BCLog::BLOCKSTORAGE, BCLog::Level::Warning, /* Continued */ + "Failed to flush previous block file %05i (finalize=%i, finalize_undo=%i) before opening new block file %05i\n", + m_last_blockfile, !fKnown, finalize_undo, nFile); + } m_last_blockfile = nFile; m_undo_height_in_last_blockfile = 0; // No undo data yet in the new file, so reset our undo-height tracking. } @@ -789,7 +810,14 @@ bool BlockManager::WriteUndoDataForBlock(const CBlockUndo& blockundo, BlockValid // with the block writes (usually when a synced up node is getting newly mined blocks) -- this case is caught in // the FindBlockPos function if (_pos.nFile < m_last_blockfile && static_cast(block.nHeight) == m_blockfile_info[_pos.nFile].nHeightLast) { - FlushUndoFile(_pos.nFile, true); + // Do not propagate the return code, a failed flush here should not + // be an indication for a failed write. If it were propagated here, + // the caller would assume the undo data not to be written, when in + // fact it is. Note though, that a failed flush might leave the data + // file untrimmed. + if (!FlushUndoFile(_pos.nFile, true)) { + LogPrintLevel(BCLog::BLOCKSTORAGE, BCLog::Level::Warning, "Failed to flush undo file %05i\n", _pos.nFile); + } } else if (_pos.nFile == m_last_blockfile && static_cast(block.nHeight) > m_undo_height_in_last_blockfile) { m_undo_height_in_last_blockfile = block.nHeight; } diff --git a/src/node/blockstorage.h b/src/node/blockstorage.h index 048139db25c4..7d03bb542fda 100644 --- a/src/node/blockstorage.h +++ b/src/node/blockstorage.h @@ -98,9 +98,14 @@ class BlockManager */ bool LoadBlockIndex() EXCLUSIVE_LOCKS_REQUIRED(cs_main); - void FlushBlockFile(bool fFinalize = false, bool finalize_undo = false); - void FlushUndoFile(int block_file, bool finalize = false); - bool FindBlockPos(FlatFilePos& pos, unsigned int nAddSize, unsigned int nHeight, uint64_t nTime, bool fKnown); + + /** Return false if block file or undo file flushing fails. */ + [[nodiscard]] bool FlushBlockFile(bool fFinalize = false, bool finalize_undo = false); + + /** Return false if undo file flushing fails. */ + [[nodiscard]] bool FlushUndoFile(int block_file, bool finalize = false); + + [[nodiscard]] bool FindBlockPos(FlatFilePos& pos, unsigned int nAddSize, unsigned int nHeight, uint64_t nTime, bool fKnown); bool FindUndoPos(BlockValidationState& state, int nFile, FlatFilePos& pos, unsigned int nAddSize); /* Calculate the block/rev files to delete based on height specified by user with RPC command pruneblockchain */ diff --git a/src/validation.cpp b/src/validation.cpp index d9e8fd69c5e4..b00a5114c9fb 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -3020,7 +3020,11 @@ bool Chainstate::FlushStateToDisk( LOG_TIME_MILLIS_WITH_CATEGORY("write block and undo data to disk", BCLog::BENCHMARK); // First make sure all block and undo data is flushed to disk. - m_blockman.FlushBlockFile(); + // TODO: Handle return error, or add detailed comment why it is + // safe to not return an error upon failure. + if (!m_blockman.FlushBlockFile()) { + LogPrintLevel(BCLog::VALIDATION, BCLog::Level::Warning, "%s: Failed to flush block file.\n", __func__); + } } // Then update all block file information (which may refer to block and undo files). diff --git a/src/wallet/bdb.cpp b/src/wallet/bdb.cpp index 54fdbce21990..a0104f24e820 100644 --- a/src/wallet/bdb.cpp +++ b/src/wallet/bdb.cpp @@ -14,6 +14,7 @@ #include +#include #include // Windows may not define S_IRUSR or S_IWUSR. We define both @@ -25,12 +26,10 @@ #endif #endif +static_assert(BDB_DB_FILE_ID_LEN == DB_FILE_ID_LEN, "DB_FILE_ID_LEN should be 20."); + namespace wallet { namespace { -Span SpanFromDbt(const BerkeleyBatch::SafeDbt& dbt) -{ - return {reinterpret_cast(dbt.get_data()), dbt.get_size()}; -} //! Make sure database has a unique fileid within the environment. If it //! doesn't, throw an error. BDB caches do not work properly when more than one @@ -232,17 +231,37 @@ BerkeleyEnvironment::BerkeleyEnvironment() : m_use_shared_memory(false) fMockDb = true; } -BerkeleyBatch::SafeDbt::SafeDbt() +/** RAII class that automatically cleanses its data on destruction */ +class SafeDbt final +{ + Dbt m_dbt; + +public: + // construct Dbt with internally-managed data + SafeDbt(); + // construct Dbt with provided data + SafeDbt(void* data, size_t size); + ~SafeDbt(); + + // delegate to Dbt + const void* get_data() const; + uint32_t get_size() const; + + // conversion operator to access the underlying Dbt + operator Dbt*(); +}; + +SafeDbt::SafeDbt() { m_dbt.set_flags(DB_DBT_MALLOC); } -BerkeleyBatch::SafeDbt::SafeDbt(void* data, size_t size) +SafeDbt::SafeDbt(void* data, size_t size) : m_dbt(data, size) { } -BerkeleyBatch::SafeDbt::~SafeDbt() +SafeDbt::~SafeDbt() { if (m_dbt.get_data() != nullptr) { // Clear memory, e.g. in case it was a private key @@ -256,21 +275,33 @@ BerkeleyBatch::SafeDbt::~SafeDbt() } } -const void* BerkeleyBatch::SafeDbt::get_data() const +const void* SafeDbt::get_data() const { return m_dbt.get_data(); } -uint32_t BerkeleyBatch::SafeDbt::get_size() const +uint32_t SafeDbt::get_size() const { return m_dbt.get_size(); } -BerkeleyBatch::SafeDbt::operator Dbt*() +SafeDbt::operator Dbt*() { return &m_dbt; } +static Span SpanFromDbt(const SafeDbt& dbt) +{ + return {reinterpret_cast(dbt.get_data()), dbt.get_size()}; +} + +BerkeleyDatabase::BerkeleyDatabase(std::shared_ptr env, fs::path filename, const DatabaseOptions& options) : + WalletDatabase(), env(std::move(env)), m_filename(std::move(filename)), m_max_log_mb(options.max_log_mb) +{ + auto inserted = this->env->m_databases.emplace(m_filename, std::ref(*this)); + assert(inserted.second); +} + bool BerkeleyDatabase::Verify(bilingual_str& errorStr) { fs::path walletDir = env->Directory(); @@ -459,6 +490,15 @@ void BerkeleyEnvironment::ReloadDbEnv() Open(open_err); } +DbTxn* BerkeleyEnvironment::TxnBegin(int flags) +{ + DbTxn* ptxn = nullptr; + int ret = dbenv->txn_begin(nullptr, &ptxn, flags); + if (!ptxn || ret != 0) + return nullptr; + return ptxn; +} + bool BerkeleyDatabase::Rewrite(const char* pszSkip) { while (true) { @@ -715,7 +755,7 @@ bool BerkeleyBatch::TxnBegin() { if (!pdb || activeTxn) return false; - DbTxn* ptxn = env->TxnBegin(); + DbTxn* ptxn = env->TxnBegin(DB_TXN_WRITE_NOSYNC); if (!ptxn) return false; activeTxn = ptxn; diff --git a/src/wallet/bdb.h b/src/wallet/bdb.h index 9389e70f629b..b141fd46efa4 100644 --- a/src/wallet/bdb.h +++ b/src/wallet/bdb.h @@ -21,11 +21,18 @@ struct bilingual_str; -#include +class DbEnv; +class DbTxn; +class Db; +class Dbc; + +// This constant was introduced in BDB 4.0.14 and has never changed, but there +// is a belt-and-suspenders check in the cpp file just in case. +#define BDB_DB_FILE_ID_LEN 20 /* Unique file ID length. */ namespace wallet { struct WalletDatabaseFileId { - uint8_t value[DB_FILE_ID_LEN]; + uint8_t value[BDB_DB_FILE_ID_LEN]; bool operator==(const WalletDatabaseFileId& rhs) const; }; @@ -64,14 +71,7 @@ class BerkeleyEnvironment void CloseDb(const fs::path& filename); void ReloadDbEnv(); - DbTxn* TxnBegin(int flags = DB_TXN_WRITE_NOSYNC) - { - DbTxn* ptxn = nullptr; - int ret = dbenv->txn_begin(nullptr, &ptxn, flags); - if (!ptxn || ret != 0) - return nullptr; - return ptxn; - } + DbTxn* TxnBegin(int flags); }; /** Get BerkeleyEnvironment given a directory path. */ @@ -88,12 +88,7 @@ class BerkeleyDatabase : public WalletDatabase BerkeleyDatabase() = delete; /** Create DB handle to real database */ - BerkeleyDatabase(std::shared_ptr env, fs::path filename, const DatabaseOptions& options) : - WalletDatabase(), env(std::move(env)), m_filename(std::move(filename)), m_max_log_mb(options.max_log_mb) - { - auto inserted = this->env->m_databases.emplace(m_filename, std::ref(*this)); - assert(inserted.second); - } + BerkeleyDatabase(std::shared_ptr env, fs::path filename, const DatabaseOptions& options); ~BerkeleyDatabase() override; @@ -161,27 +156,6 @@ class BerkeleyDatabase : public WalletDatabase /** RAII class that provides access to a Berkeley database */ class BerkeleyBatch : public DatabaseBatch { -public: - /** RAII class that automatically cleanses its data on destruction */ - class SafeDbt final - { - Dbt m_dbt; - - public: - // construct Dbt with internally-managed data - SafeDbt(); - // construct Dbt with provided data - SafeDbt(void* data, size_t size); - ~SafeDbt(); - - // delegate to Dbt - const void* get_data() const; - uint32_t get_size() const; - - // conversion operator to access the underlying Dbt - operator Dbt*(); - }; - private: bool ReadKey(CDataStream&& key, CDataStream& value) override; bool WriteKey(CDataStream&& key, CDataStream&& value, bool overwrite = true) override; diff --git a/src/wallet/salvage.cpp b/src/wallet/salvage.cpp index fdae721b7667..ff8bb918a41c 100644 --- a/src/wallet/salvage.cpp +++ b/src/wallet/salvage.cpp @@ -11,6 +11,8 @@ #include #include +#include + namespace wallet { /* End of headers, beginning of key/value data */ static const char *HEADER_END = "HEADER=END"; @@ -134,7 +136,7 @@ bool RecoverDatabaseFile(const ArgsManager& args, const fs::path& file_path, bil return false; } - DbTxn* ptxn = env->TxnBegin(); + DbTxn* ptxn = env->TxnBegin(DB_TXN_WRITE_NOSYNC); CWallet dummyWallet(/*chain=*/nullptr, /*coinjoin_loader=*/nullptr, "", gArgs, CreateDummyWalletDatabase()); dummyWallet.SetupLegacyScriptPubKeyMan(); for (KeyValPair& row : salvagedData) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 0ae127473cd2..d09e660287ad 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -4803,7 +4803,7 @@ bool CWallet::ApplyMigrationData(MigrationData& data, bilingual_str& error) return false; } } else { - // Labels for everything else (send) should be cloned to all + // 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 @@ -4815,7 +4815,6 @@ bool CWallet::ApplyMigrationData(MigrationData& data, bilingual_str& error) if (!addr_pair.second.IsChange()) { data.watchonly_wallet->m_address_book[addr_pair.first].SetLabel(label); } - continue; } if (data.solvable_wallet) { LOCK(data.solvable_wallet->cs_wallet); @@ -4828,7 +4827,6 @@ bool CWallet::ApplyMigrationData(MigrationData& data, bilingual_str& error) if (!addr_pair.second.IsChange()) { data.solvable_wallet->m_address_book[addr_pair.first].SetLabel(label); } - continue; } } } @@ -4840,10 +4838,10 @@ bool CWallet::ApplyMigrationData(MigrationData& data, bilingual_str& error) 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) + std::optional label = addr_book_data.IsChange() ? std::nullopt : std::make_optional(addr_book_data.GetLabel()); + // don't bother writing default values (unknown purpose) if (purpose != "unknown") batch.WritePurpose(address, purpose); - if (!label.empty()) batch.WriteName(address, label); + if (label) batch.WriteName(address, *label); } }; if (data.watchonly_wallet) persist_address_book(*data.watchonly_wallet); diff --git a/test/functional/wallet_fundrawtransaction.py b/test/functional/wallet_fundrawtransaction.py index f1e6749dccec..e8a8571f860a 100755 --- a/test/functional/wallet_fundrawtransaction.py +++ b/test/functional/wallet_fundrawtransaction.py @@ -20,6 +20,7 @@ assert_raises_rpc_error, count_bytes, find_vout_for_address, + get_fee, satoshi_round, ) from test_framework.wallet_util import bytes_to_wif @@ -547,11 +548,22 @@ def test_spend_2of2(self): def test_locked_wallet(self): self.log.info("Test fundrawtxn with locked wallet and hardened derivation") - self.nodes[1].encryptwallet("test") + df_wallet = self.nodes[1].get_wallet_rpc(self.default_wallet_name) + self.nodes[1].createwallet(wallet_name="locked_wallet", descriptors=self.options.descriptors) + wallet = self.nodes[1].get_wallet_rpc("locked_wallet") + # This test is not meant to exercise fee estimation. Making sure all txs are sent at a consistent fee rate. + wallet.settxfee(self.min_relay_tx_fee) + + # Add some balance to the wallet (this will be reverted at the end of the test) + df_wallet.sendall(recipients=[wallet.getnewaddress()]) + self.generate(self.nodes[1], 1) + + # Encrypt wallet and import descriptors + wallet.encryptwallet("test") if self.options.descriptors: - self.nodes[1].walletpassphrase('test', 999000) - self.nodes[1].importdescriptors([{ + wallet.walletpassphrase('test', 999000) + wallet.importdescriptors([{ 'desc': descsum_create('pkh(tprv8ZgxMBicQKsPdYeeZbPSKd2KYLmeVKtcFA7kqCxDvDR13MQ6us8HopUR2wLcS2ZKPhLyKsqpDL2FtL73LMHcgoCL7DXsciA8eX8nbjCR2eG/0h/*h)'), 'timestamp': 'now', 'active': True @@ -562,48 +574,58 @@ def test_locked_wallet(self): 'active': True, 'internal': True }]) - self.nodes[1].walletlock() + wallet.walletlock() # Drain the keypool. - self.nodes[1].getnewaddress() + wallet.getnewaddress() + + # Choose input + inputs = wallet.listunspent() + + # Deduce exact fee to produce a changeless transaction + tx_size = 192 # Total tx size: 192 bytes, p2pkh -> p2pkh. Input 148 bytes + rest of tx is 44 bytes. + value = inputs[0]["amount"] - get_fee(tx_size, self.min_relay_tx_fee) - # Choose 2 inputs - # bnb doesn't work same way as in bitcoin, so, `value` is also calculated by different way - inputs = self.nodes[1].listunspent() - value = sum(inp["amount"] for inp in inputs) - Decimal("0.00002200") outputs = {self.nodes[0].getnewaddress():value} - rawtx = self.nodes[1].createrawtransaction(inputs, outputs) + rawtx = wallet.createrawtransaction(inputs, outputs) # fund a transaction that does not require a new key for the change output - self.nodes[1].fundrawtransaction(rawtx) + funded_tx = wallet.fundrawtransaction(rawtx) + assert_equal(funded_tx["changepos"], -1) # fund a transaction that requires a new key for the change output # creating the key must be impossible because the wallet is locked outputs = {self.nodes[0].getnewaddress():value - Decimal("0.1")} - rawtx = self.nodes[1].createrawtransaction(inputs, outputs) - assert_raises_rpc_error(-4, "Transaction needs a change address, but we can't generate it.", self.nodes[1].fundrawtransaction, rawtx) + rawtx = wallet.createrawtransaction(inputs, outputs) + assert_raises_rpc_error(-4, "Transaction needs a change address, but we can't generate it.", wallet.fundrawtransaction, rawtx) # Refill the keypool. - self.nodes[1].walletpassphrase("test", 999000) - self.nodes[1].walletlock() + wallet.walletpassphrase("test", 999000) + wallet.walletlock() - assert_raises_rpc_error(-13, "walletpassphrase", self.nodes[1].sendtoaddress, self.nodes[0].getnewaddress(), 12) + assert_raises_rpc_error(-13, "walletpassphrase", wallet.sendtoaddress, self.nodes[0].getnewaddress(), 12) oldBalance = self.nodes[0].getbalance() inputs = [] outputs = {self.nodes[0].getnewaddress():11} - rawtx = self.nodes[1].createrawtransaction(inputs, outputs) - fundedTx = self.nodes[1].fundrawtransaction(rawtx) + rawtx = wallet.createrawtransaction(inputs, outputs) + fundedTx = wallet.fundrawtransaction(rawtx) + assert fundedTx["changepos"] != -1 # Now we need to unlock. - self.nodes[1].walletpassphrase("test", 999000) - signedTx = self.nodes[1].signrawtransactionwithwallet(fundedTx['hex']) - self.nodes[1].sendrawtransaction(signedTx['hex']) + wallet.walletpassphrase("test", 999000) + signedTx = wallet.signrawtransactionwithwallet(fundedTx['hex']) + wallet.sendrawtransaction(signedTx['hex']) self.generate(self.nodes[1], 1) # Make sure funds are received at node1. assert_equal(oldBalance+Decimal('511.0000000'), self.nodes[0].getbalance()) + # Restore pre-test wallet state + wallet.sendall(recipients=[df_wallet.getnewaddress(), df_wallet.getnewaddress(), df_wallet.getnewaddress()]) + wallet.unloadwallet() + self.generate(self.nodes[1], 1) + def test_many_inputs_fee(self): """Multiple (~19) inputs tx test | Compare fee.""" self.log.info("Test fundrawtxn fee with many inputs") diff --git a/test/functional/wallet_migration.py b/test/functional/wallet_migration.py index 98da6442a467..2036bb1a789d 100755 --- a/test/functional/wallet_migration.py +++ b/test/functional/wallet_migration.py @@ -69,6 +69,15 @@ def assert_list_txs_equal(self, received_list_txs, expected_list_txs): del d["parent_descs"] assert_equal(received_list_txs, expected_list_txs) + def check_address(self, wallet, addr, is_mine, is_change, label): + addr_info = wallet.getaddressinfo(addr) + assert_equal(addr_info['ismine'], is_mine) + assert_equal(addr_info['ischange'], is_change) + if label is not None: + assert_equal(addr_info['labels'], [label]), + else: + assert_equal(addr_info['labels'], []), + def test_basic(self): default = self.nodes[0].get_wallet_rpc(self.default_wallet_name) @@ -650,6 +659,132 @@ def test_hybrid_pubkey(self): wallet.unloadwallet() + def test_addressbook(self): + df_wallet = self.nodes[0].get_wallet_rpc(self.default_wallet_name) + + self.log.info("Test migration of address book data") + wallet = self.create_legacy_wallet("legacy_addrbook") + df_wallet.sendtoaddress(wallet.getnewaddress(), 3) + + # Import watch-only script to create a watch-only wallet after migration + watch_addr = df_wallet.getnewaddress() + wallet.importaddress(watch_addr) + df_wallet.sendtoaddress(watch_addr, 2) + + # Import solvable script + multi_addr1 = wallet.getnewaddress() + multi_addr2 = wallet.getnewaddress() + multi_addr3 = df_wallet.getnewaddress() + wallet.importpubkey(df_wallet.getaddressinfo(multi_addr3)["pubkey"]) + ms_addr_info = wallet.addmultisigaddress(2, [multi_addr1, multi_addr2, multi_addr3]) + + self.generate(self.nodes[0], 1) + + # Test vectors + addr_external = { + "addr": df_wallet.getnewaddress(), + "is_mine": False, + "is_change": False, + "label": "" + } + addr_external_with_label = { + "addr": df_wallet.getnewaddress(), + "is_mine": False, + "is_change": False, + "label": "external" + } + addr_internal = { + "addr": wallet.getnewaddress(), + "is_mine": True, + "is_change": False, + "label": "" + } + addr_internal_with_label = { + "addr": wallet.getnewaddress(), + "is_mine": True, + "is_change": False, + "label": "internal" + } + change_address = { + "addr": wallet.getrawchangeaddress(), + "is_mine": True, + "is_change": True, + "label": None + } + watch_only_addr = { + "addr": watch_addr, + "is_mine": False, + "is_change": False, + "label": "imported" + } + ms_addr = { + "addr": ms_addr_info['address'], + "is_mine": False, + "is_change": False, + "label": "multisig" + } + + # To store the change address in the addressbook need to send coins to it + wallet.send(outputs=[{wallet.getnewaddress(): 2}], options={"change_address": change_address['addr']}) + self.generate(self.nodes[0], 1) + + # Util wrapper func for 'addr_info' + def check(info, node): + self.check_address(node, info['addr'], info['is_mine'], info['is_change'], info["label"]) + + # Pre-migration: set label and perform initial checks + for addr_info in [addr_external, addr_external_with_label, addr_internal, addr_internal_with_label, change_address, watch_only_addr, ms_addr]: + if not addr_info['is_change']: + wallet.setlabel(addr_info['addr'], addr_info["label"]) + check(addr_info, wallet) + + # Migrate wallet + info_migration = wallet.migratewallet() + wallet_wo = self.nodes[0].get_wallet_rpc(info_migration["watchonly_name"]) + wallet_solvables = self.nodes[0].get_wallet_rpc(info_migration["solvables_name"]) + + ######################### + # Post migration checks # + ######################### + + # First check the main wallet + for addr_info in [addr_external, addr_external_with_label, addr_internal, addr_internal_with_label, change_address, ms_addr]: + check(addr_info, wallet) + + # Watch-only wallet will contain the watch-only entry (with 'is_mine=True') and all external addresses ('send') + self.check_address(wallet_wo, watch_only_addr['addr'], is_mine=True, is_change=watch_only_addr['is_change'], label=watch_only_addr["label"]) + for addr_info in [addr_external, addr_external_with_label, ms_addr]: + check(addr_info, wallet_wo) + + # Solvables wallet will contain the multisig entry (with 'is_mine=True') and all external addresses ('send') + self.check_address(wallet_solvables, ms_addr['addr'], is_mine=True, is_change=ms_addr['is_change'], label=ms_addr["label"]) + for addr_info in [addr_external, addr_external_with_label]: + check(addr_info, wallet_solvables) + + ######################################################################################## + # Now restart migrated wallets and verify that the addressbook entries are still there # + ######################################################################################## + + # First the main wallet + self.nodes[0].unloadwallet("legacy_addrbook") + self.nodes[0].loadwallet("legacy_addrbook") + for addr_info in [addr_external, addr_external_with_label, addr_internal, addr_internal_with_label, change_address, ms_addr]: + check(addr_info, wallet) + + # Watch-only wallet + self.nodes[0].unloadwallet(info_migration["watchonly_name"]) + self.nodes[0].loadwallet(info_migration["watchonly_name"]) + self.check_address(wallet_wo, watch_only_addr['addr'], is_mine=True, is_change=watch_only_addr['is_change'], label=watch_only_addr["label"]) + for addr_info in [addr_external, addr_external_with_label, ms_addr]: + check(addr_info, wallet_wo) + + # Solvables wallet + self.nodes[0].unloadwallet(info_migration["solvables_name"]) + self.nodes[0].loadwallet(info_migration["solvables_name"]) + self.check_address(wallet_solvables, ms_addr['addr'], is_mine=True, is_change=ms_addr['is_change'], label=ms_addr["label"]) + for addr_info in [addr_external, addr_external_with_label]: + check(addr_info, wallet_solvables) + def run_test(self): self.generate(self.nodes[0], 101) @@ -667,6 +802,7 @@ def run_test(self): self.test_direct_file() self.test_migrate_raw_p2sh() self.test_hybrid_pubkey() + self.test_addressbook() if __name__ == '__main__': WalletMigrationTest().main()