In 217efa59c734d9ff40ccbb8ad51ae6c75c66dbbb "wallet/tests: pin ScanForWalletTransactions behaviour"
Nit, you use the same WITH_LOCK approach in other tests too?
<details>
<summary>see diff and other potential deduplications</summary>
diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp
index fd2dfc238c..eaace67d8e 100644
--- a/src/wallet/test/wallet_tests.cpp
+++ b/src/wallet/test/wallet_tests.cpp
@@ -76,6 +76,49 @@ static void AddKey(CWallet& wallet, const CKey& key)
Assert(wallet.AddWalletDescriptor(w_desc, provider, "", false));
}
+struct ActiveChainInfo {
+ uint256 genesis_hash;
+ int tip_height;
+ uint256 tip_hash;
+};
+
+struct BoundedScanInfo {
+ ActiveChainInfo chain_info;
+ int max_height;
+ uint256 max_hash;
+};
+
+static ActiveChainInfo GetActiveChainInfo(ChainstateManager& chainman)
+{
+ return WITH_LOCK(chainman.GetMutex(), return (ActiveChainInfo{
+ chainman.ActiveChain().Genesis()->GetBlockHash(),
+ chainman.ActiveChain().Height(),
+ chainman.ActiveChain().Tip()->GetBlockHash()}));
+}
+
+static BoundedScanInfo GetBoundedScanInfo(ChainstateManager& chainman, int blocks_before_tip)
+{
+ return WITH_LOCK(chainman.GetMutex(),
+ const int max_height{chainman.ActiveChain().Height() - blocks_before_tip};
+ return (BoundedScanInfo{
+ {chainman.ActiveChain().Genesis()->GetBlockHash(),
+ chainman.ActiveChain().Height(),
+ chainman.ActiveChain().Tip()->GetBlockHash()},
+ max_height,
+ chainman.ActiveChain()[max_height]->GetBlockHash()});
+ );
+}
+
+static void SetupDescriptorWalletWithKey(CWallet& wallet, const CKey& key, int block_height, const uint256& block_hash)
+{
+ {
+ LOCK(wallet.cs_wallet);
+ wallet.SetWalletFlag(WALLET_FLAG_DESCRIPTORS);
+ wallet.SetLastBlockProcessed(block_height, block_hash);
+ }
+ AddKey(wallet, key);
+}
+
BOOST_FIXTURE_TEST_CASE(update_non_range_descriptor, TestingSetup)
{
CWallet wallet(m_node.chain.get(), "", CreateMockableWalletDatabase());
@@ -362,55 +405,36 @@ BOOST_FIXTURE_TEST_CASE(wallet_rescan_reserver, TestingSetup)
BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions_bounded, TestChain100Setup)
{
- uint256 genesis_hash, max_hash, tip_hash;
- int max_height, tip_height;
- {
- LOCK(Assert(m_node.chainman)->GetMutex());
- genesis_hash = m_node.chainman->ActiveChain().Genesis()->GetBlockHash();
- tip_height = m_node.chainman->ActiveChain().Height();
- tip_hash = m_node.chainman->ActiveChain().Tip()->GetBlockHash();
- max_height = tip_height - 2;
- max_hash = m_node.chainman->ActiveChain()[max_height]->GetBlockHash();
- }
+ const auto scan{GetBoundedScanInfo(*Assert(m_node.chainman), /*blocks_before_tip=*/2)};
// A scan with max_height set stops exactly at max_height and does not
// sync any blocks beyond it.
{
CWallet wallet(m_node.chain.get(), "", CreateMockableWalletDatabase());
- {
- LOCK(wallet.cs_wallet);
- wallet.SetWalletFlag(WALLET_FLAG_DESCRIPTORS);
- wallet.SetLastBlockProcessed(tip_height, tip_hash);
- }
- AddKey(wallet, coinbaseKey);
+ SetupDescriptorWalletWithKey(wallet, coinbaseKey, scan.chain_info.tip_height, scan.chain_info.tip_hash);
WalletRescanReserver reserver(wallet);
reserver.reserve();
- CWallet::ScanResult result = wallet.ScanForWalletTransactions(genesis_hash, /*start_height=*/0, max_height, reserver, /*save_progress=*/false);
+ CWallet::ScanResult result = wallet.ScanForWalletTransactions(scan.chain_info.genesis_hash, /*start_height=*/0, scan.max_height, reserver, /*save_progress=*/false);
BOOST_CHECK_EQUAL(result.status, CWallet::ScanResult::SUCCESS);
BOOST_CHECK(result.last_failed_block.IsNull());
- BOOST_CHECK_EQUAL(result.last_scanned_block, max_hash);
- BOOST_CHECK_EQUAL(*result.last_scanned_height, max_height);
+ BOOST_CHECK_EQUAL(result.last_scanned_block, scan.max_hash);
+ BOOST_CHECK_EQUAL(*result.last_scanned_height, scan.max_height);
// One coinbase per block from height 1 through max_height.
- BOOST_CHECK_EQUAL(WITH_LOCK(wallet.cs_wallet, return wallet.mapWallet.size()), static_cast<size_t>(max_height));
+ BOOST_CHECK_EQUAL(WITH_LOCK(wallet.cs_wallet, return wallet.mapWallet.size()), static_cast<size_t>(scan.max_height));
}
// A single-block range (start == max_height == tip) scans exactly that
// block.
{
CWallet wallet(m_node.chain.get(), "", CreateMockableWalletDatabase());
- {
- LOCK(wallet.cs_wallet);
- wallet.SetWalletFlag(WALLET_FLAG_DESCRIPTORS);
- wallet.SetLastBlockProcessed(tip_height, tip_hash);
- }
- AddKey(wallet, coinbaseKey);
+ SetupDescriptorWalletWithKey(wallet, coinbaseKey, scan.chain_info.tip_height, scan.chain_info.tip_hash);
WalletRescanReserver reserver(wallet);
reserver.reserve();
- CWallet::ScanResult result = wallet.ScanForWalletTransactions(tip_hash, tip_height, tip_height, reserver, /*save_progress=*/false);
+ CWallet::ScanResult result = wallet.ScanForWalletTransactions(scan.chain_info.tip_hash, scan.chain_info.tip_height, scan.chain_info.tip_height, reserver, /*save_progress=*/false);
BOOST_CHECK_EQUAL(result.status, CWallet::ScanResult::SUCCESS);
BOOST_CHECK(result.last_failed_block.IsNull());
- BOOST_CHECK_EQUAL(result.last_scanned_block, tip_hash);
- BOOST_CHECK_EQUAL(*result.last_scanned_height, tip_height);
+ BOOST_CHECK_EQUAL(result.last_scanned_block, scan.chain_info.tip_hash);
+ BOOST_CHECK_EQUAL(*result.last_scanned_height, scan.chain_info.tip_height);
BOOST_CHECK_EQUAL(WITH_LOCK(wallet.cs_wallet, return wallet.mapWallet.size()), 1U);
}
}
@@ -418,15 +442,8 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions_bounded, TestChain100Setup)
BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions_tip_extension, TestChain100Setup)
{
CWallet wallet(m_node.chain.get(), "", CreateMockableWalletDatabase());
- uint256 genesis_hash;
- {
- LOCK(wallet.cs_wallet);
- LOCK(Assert(m_node.chainman)->GetMutex());
- wallet.SetWalletFlag(WALLET_FLAG_DESCRIPTORS);
- wallet.SetLastBlockProcessed(m_node.chainman->ActiveChain().Height(), m_node.chainman->ActiveChain().Tip()->GetBlockHash());
- genesis_hash = m_node.chainman->ActiveChain().Genesis()->GetBlockHash();
- }
- AddKey(wallet, coinbaseKey);
+ const auto chain_info{GetActiveChainInfo(*Assert(m_node.chainman))};
+ SetupDescriptorWalletWithKey(wallet, coinbaseKey, chain_info.tip_height, chain_info.tip_hash);
// Connect a block while the scan is running (the handler fires on the
// scanning thread as the scan starts) and advance the wallet's tip, as
@@ -437,17 +454,15 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions_tip_extension, TestChain100
auto handler = wallet.ShowProgress.connect([&](const std::string&, int progress) {
if (progress != 0 || new_tip_height != 0) return;
CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey()));
- LOCK(wallet.cs_wallet);
- LOCK(Assert(m_node.chainman)->GetMutex());
- const CBlockIndex* new_tip = m_node.chainman->ActiveChain().Tip();
- new_tip_hash = new_tip->GetBlockHash();
- new_tip_height = new_tip->nHeight;
- wallet.SetLastBlockProcessed(new_tip_height, new_tip_hash);
+ const auto new_tip{GetActiveChainInfo(*Assert(m_node.chainman))};
+ new_tip_hash = new_tip.tip_hash;
+ new_tip_height = new_tip.tip_height;
+ WITH_LOCK(wallet.cs_wallet, wallet.SetLastBlockProcessed(new_tip_height, new_tip_hash));
});
WalletRescanReserver reserver(wallet);
reserver.reserve();
- CWallet::ScanResult result = wallet.ScanForWalletTransactions(genesis_hash, /*start_height=*/0, /*max_height=*/{}, reserver, /*save_progress=*/false);
+ CWallet::ScanResult result = wallet.ScanForWalletTransactions(chain_info.genesis_hash, /*start_height=*/0, /*max_height=*/{}, reserver, /*save_progress=*/false);
handler.disconnect();
BOOST_CHECK_EQUAL(result.status, CWallet::ScanResult::SUCCESS);
BOOST_CHECK_EQUAL(result.last_scanned_block, new_tip_hash);
@@ -457,18 +472,9 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions_tip_extension, TestChain100
BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions_no_progress_saved, TestChain100Setup)
{
CWallet wallet(m_node.chain.get(), "", CreateMockableWalletDatabase());
- uint256 genesis_hash, tip_hash;
- int max_height;
- {
- LOCK(wallet.cs_wallet);
- LOCK(Assert(m_node.chainman)->GetMutex());
- wallet.SetWalletFlag(WALLET_FLAG_DESCRIPTORS);
- tip_hash = m_node.chainman->ActiveChain().Tip()->GetBlockHash();
- wallet.SetLastBlockProcessed(m_node.chainman->ActiveChain().Height(), tip_hash);
- genesis_hash = m_node.chainman->ActiveChain().Genesis()->GetBlockHash();
- max_height = m_node.chainman->ActiveChain().Height() - 2;
- }
- AddKey(wallet, coinbaseKey);
+ const auto chain_info{GetActiveChainInfo(*Assert(m_node.chainman))};
+ const int max_height{chain_info.tip_height - 2};
+ SetupDescriptorWalletWithKey(wallet, coinbaseKey, chain_info.tip_height, chain_info.tip_hash);
WalletRescanReserver reserver(wallet);
// Advance the clock on every call so that every scanned block would be
@@ -477,7 +483,7 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions_no_progress_saved, TestChai
reserver.setNow([&] { fake_time += 60s; return fake_time; });
reserver.reserve();
- CWallet::ScanResult result = wallet.ScanForWalletTransactions(genesis_hash, /*start_height=*/0, max_height, reserver, /*save_progress=*/false);
+ CWallet::ScanResult result = wallet.ScanForWalletTransactions(chain_info.genesis_hash, /*start_height=*/0, max_height, reserver, /*save_progress=*/false);
BOOST_CHECK_EQUAL(result.status, CWallet::ScanResult::SUCCESS);
// With save_progress=false the scan must not touch the wallet's best
@@ -486,7 +492,7 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions_no_progress_saved, TestChai
CBlockLocator locator;
BOOST_CHECK(WalletBatch{wallet.GetDatabase()}.ReadBestBlock(locator));
BOOST_CHECK(!locator.IsNull());
- BOOST_CHECK_EQUAL(locator.vHave.front(), tip_hash);
+ BOOST_CHECK_EQUAL(locator.vHave.front(), chain_info.tip_hash);
}
BOOST_FIXTURE_TEST_CASE(rescan_from_time, TestChain100Setup)
@@ -499,21 +505,15 @@ BOOST_FIXTURE_TEST_CASE(rescan_from_time, TestChain100Setup)
// Prune the older block file.
int file_number;
- {
- LOCK(cs_main);
+ WITH_LOCK(cs_main,
file_number = old_tip->GetBlockPos().nFile;
Assert(m_node.chainman)->m_blockman.PruneOneBlockFile(file_number);
- }
+ );
m_node.chainman->m_blockman.UnlinkPrunedFiles({file_number});
CWallet wallet(m_node.chain.get(), "", CreateMockableWalletDatabase());
- {
- LOCK(wallet.cs_wallet);
- LOCK(Assert(m_node.chainman)->GetMutex());
- wallet.SetWalletFlag(WALLET_FLAG_DESCRIPTORS);
- wallet.SetLastBlockProcessed(m_node.chainman->ActiveChain().Height(), m_node.chainman->ActiveChain().Tip()->GetBlockHash());
- }
- AddKey(wallet, coinbaseKey);
+ const auto chain_info{GetActiveChainInfo(*Assert(m_node.chainman))};
+ SetupDescriptorWalletWithKey(wallet, coinbaseKey, chain_info.tip_height, chain_info.tip_hash);
WalletRescanReserver reserver(wallet);
reserver.reserve();
@@ -540,27 +540,17 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions_missing_filter, TestChain10
{
CWallet wallet(m_node.chain.get(), "", CreateMockableWalletDatabase());
- uint256 genesis_hash, tip_hash;
- int tip_height;
- {
- LOCK(wallet.cs_wallet);
- LOCK(Assert(m_node.chainman)->GetMutex());
- wallet.SetWalletFlag(WALLET_FLAG_DESCRIPTORS);
- genesis_hash = m_node.chainman->ActiveChain().Genesis()->GetBlockHash();
- tip_height = m_node.chainman->ActiveChain().Height();
- tip_hash = m_node.chainman->ActiveChain().Tip()->GetBlockHash();
- wallet.SetLastBlockProcessed(tip_height, tip_hash);
- }
- AddKey(wallet, coinbaseKey);
+ const auto chain_info{GetActiveChainInfo(*Assert(m_node.chainman))};
+ SetupDescriptorWalletWithKey(wallet, coinbaseKey, chain_info.tip_height, chain_info.tip_hash);
WalletRescanReserver reserver(wallet);
reserver.reserve();
- CWallet::ScanResult result = wallet.ScanForWalletTransactions(genesis_hash, /*start_height=*/0, /*max_height=*/{}, reserver, /*save_progress=*/false);
+ CWallet::ScanResult result = wallet.ScanForWalletTransactions(chain_info.genesis_hash, /*start_height=*/0, /*max_height=*/{}, reserver, /*save_progress=*/false);
BOOST_CHECK_EQUAL(result.status, CWallet::ScanResult::SUCCESS);
BOOST_CHECK(result.last_failed_block.IsNull());
- BOOST_CHECK_EQUAL(result.last_scanned_block, tip_hash);
- BOOST_CHECK_EQUAL(*result.last_scanned_height, tip_height);
+ BOOST_CHECK_EQUAL(result.last_scanned_block, chain_info.tip_hash);
+ BOOST_CHECK_EQUAL(*result.last_scanned_height, chain_info.tip_height);
// One coinbase per block from height 1 through the tip.
- BOOST_CHECK_EQUAL(WITH_LOCK(wallet.cs_wallet, return wallet.mapWallet.size()), static_cast<size_t>(tip_height));
+ BOOST_CHECK_EQUAL(WITH_LOCK(wallet.cs_wallet, return wallet.mapWallet.size()), static_cast<size_t>(chain_info.tip_height));
}
filter_index.Stop();
@@ -588,21 +578,15 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions_attach_chain, TestChain100S
CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey()));
}
- int tip_height;
- uint256 tip_hash;
- {
- LOCK(Assert(m_node.chainman)->GetMutex());
- tip_height = m_node.chainman->ActiveChain().Height();
- tip_hash = m_node.chainman->ActiveChain().Tip()->GetBlockHash();
- }
+ const auto chain_info{GetActiveChainInfo(*Assert(m_node.chainman))};
// Loading the wallet must rescan the extension from the recorded best
// block and find its coinbases.
wallet = TestLoadWallet(context);
{
LOCK(wallet->cs_wallet);
- BOOST_CHECK_EQUAL(wallet->GetLastBlockHeight(), tip_height);
- BOOST_CHECK_EQUAL(wallet->GetLastBlockHash(), tip_hash);
+ BOOST_CHECK_EQUAL(wallet->GetLastBlockHeight(), chain_info.tip_height);
+ BOOST_CHECK_EQUAL(wallet->GetLastBlockHash(), chain_info.tip_hash);
// The extension's coinbases plus the one of the recorded best block:
// the load rescan starts mid-chain, at that block inclusive.
BOOST_CHECK_EQUAL(wallet->mapWallet.size(), static_cast<size_t>(NEW_BLOCKS + 1));
</details>
