Merge #14957: wallet: Initialize stop_block in CWallet::ScanForWalletTransactions

8b9171c wallet: Initialize stop_block to nullptr in CWallet::ScanForWalletTransactions (Ben Woosley)

Pull request description:

  Previously the argument would be untouched if the first block scan failed. This
  makes the behavior predictable, and consistent with the documentation.

Tree-SHA512: 3efadf9fd5e25ecd9450f32545f58e61a123ad883e921ef427b13e4782ffdd8ffe905c9ad3edc7e8f9e4953342cd72247bb4cc9eeaf9e5fd04291ac5c1bb5eec

Author: meshcollider

src/wallet/test/wallet_tests.cpp

@@ -34,7 +34,7 @@ static void AddKey(CWallet& wallet, const CKey& key)
     wallet.AddKeyPubKey(key, key.GetPubKey());
 }
 
-BOOST_FIXTURE_TEST_CASE(rescan, TestChain100Setup)
+BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup)
 {
     auto chain = interfaces::MakeChain();
 
@@ -47,14 +47,27 @@ BOOST_FIXTURE_TEST_CASE(rescan, TestChain100Setup)
 
     auto locked_chain = chain->lock();
 
+    // Verify ScanForWalletTransactions accomodates a null start block.
+    {
+        CWallet wallet(*chain, WalletLocation(), WalletDatabase::CreateDummy());
+        AddKey(wallet, coinbaseKey);
+        WalletRescanReserver reserver(&wallet);
+        reserver.reserve();
+        const CBlockIndex *stop_block = null_block + 1, *failed_block = null_block + 1;
+        BOOST_CHECK_EQUAL(wallet.ScanForWalletTransactions(nullptr, nullptr, reserver, failed_block, stop_block), CWallet::ScanResult::SUCCESS);
+        BOOST_CHECK_EQUAL(failed_block, null_block);
+        BOOST_CHECK_EQUAL(stop_block, null_block);
+        BOOST_CHECK_EQUAL(wallet.GetImmatureBalance(), 0);
+    }
+
     // Verify ScanForWalletTransactions picks up transactions in both the old
     // and new block files.
     {
         CWallet wallet(*chain, WalletLocation(), WalletDatabase::CreateDummy());
         AddKey(wallet, coinbaseKey);
         WalletRescanReserver reserver(&wallet);
         reserver.reserve();
-        const CBlockIndex *stop_block, *failed_block;
+        const CBlockIndex *stop_block = null_block + 1, *failed_block = null_block + 1;
         BOOST_CHECK_EQUAL(wallet.ScanForWalletTransactions(oldTip, nullptr, reserver, failed_block, stop_block), CWallet::ScanResult::SUCCESS);
         BOOST_CHECK_EQUAL(failed_block, null_block);
         BOOST_CHECK_EQUAL(stop_block, newTip);
@@ -72,13 +85,47 @@ BOOST_FIXTURE_TEST_CASE(rescan, TestChain100Setup)
         AddKey(wallet, coinbaseKey);
         WalletRescanReserver reserver(&wallet);
         reserver.reserve();
-        const CBlockIndex *stop_block, *failed_block;
+        const CBlockIndex *stop_block = null_block + 1, *failed_block = null_block + 1;
         BOOST_CHECK_EQUAL(wallet.ScanForWalletTransactions(oldTip, nullptr, reserver, failed_block, stop_block), CWallet::ScanResult::FAILURE);
         BOOST_CHECK_EQUAL(failed_block, oldTip);
         BOOST_CHECK_EQUAL(stop_block, newTip);
         BOOST_CHECK_EQUAL(wallet.GetImmatureBalance(), 50 * COIN);
     }
 
+    // Prune the remaining block file.
+    PruneOneBlockFile(newTip->GetBlockPos().nFile);
+    UnlinkPrunedFiles({newTip->GetBlockPos().nFile});
+
+    // Verify ScanForWalletTransactions scans no blocks.
+    {
+        CWallet wallet(*chain, WalletLocation(), WalletDatabase::CreateDummy());
+        AddKey(wallet, coinbaseKey);
+        WalletRescanReserver reserver(&wallet);
+        reserver.reserve();
+        const CBlockIndex *stop_block = null_block + 1, *failed_block = null_block + 1;
+        BOOST_CHECK_EQUAL(wallet.ScanForWalletTransactions(oldTip, nullptr, reserver, failed_block, stop_block), CWallet::ScanResult::FAILURE);
+        BOOST_CHECK_EQUAL(failed_block, newTip);
+        BOOST_CHECK_EQUAL(stop_block, null_block);
+        BOOST_CHECK_EQUAL(wallet.GetImmatureBalance(), 0);
+    }
+}
+
+BOOST_FIXTURE_TEST_CASE(importmulti_rescan, TestChain100Setup)
+{
+    auto chain = interfaces::MakeChain();
+
+    // Cap last block file size, and mine new block in a new block file.
+    CBlockIndex* oldTip = chainActive.Tip();
+    GetBlockFileInfo(oldTip->GetBlockPos().nFile)->nSize = MAX_BLOCKFILE_SIZE;
+    CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey()));
+    CBlockIndex* newTip = chainActive.Tip();
+
+    auto locked_chain = chain->lock();
+
+    // Prune the older block file.
+    PruneOneBlockFile(oldTip->GetBlockPos().nFile);
+    UnlinkPrunedFiles({oldTip->GetBlockPos().nFile});
+
     // Verify importmulti RPC returns failure for a key whose creation time is
     // before the missing block, and success for a key whose creation time is
     // after.
@@ -294,7 +341,7 @@ class ListCoinsTestingSetup : public TestChain100Setup
         WalletRescanReserver reserver(wallet.get());
         reserver.reserve();
         const CBlockIndex* const null_block = nullptr;
-        const CBlockIndex *stop_block, *failed_block;
+        const CBlockIndex *stop_block = null_block + 1, *failed_block = null_block + 1;
         BOOST_CHECK_EQUAL(wallet->ScanForWalletTransactions(chainActive.Genesis(), nullptr, reserver, failed_block, stop_block), CWallet::ScanResult::SUCCESS);
         BOOST_CHECK_EQUAL(stop_block, chainActive.Tip());
         BOOST_CHECK_EQUAL(failed_block, null_block);

src/wallet/wallet.cpp

@@ -1601,6 +1601,7 @@ CWallet::ScanResult CWallet::ScanForWalletTransactions(const CBlockIndex* const
 
     const CBlockIndex* pindex = pindexStart;
     failed_block = nullptr;
+    stop_block = nullptr;
 
     if (pindex) WalletLogPrintf("Rescan started from block %d...\n", pindex->nHeight);