Skip to content

Commit

Permalink
Add stop_block out arg to ScanForWalletTransactions
Browse files Browse the repository at this point in the history 
Accurately reports the last block successfully scanned, replacing a return of
the chain tip, which represented possibly inaccurated data in a race condition.
  • Loading branch information
@Empact
Empact committed Nov 13, 2018
1 parent 3002d6c commit bd3b036
Show file tree
Hide file tree
Showing 5 changed files with 32 additions and 24 deletions.
7 changes: 4 additions & 3 deletions src/qt/test/wallettests.cpp
View file
Expand Up @@ -147,11 +147,12 @@ void TestGUI()
WalletRescanReserver reserver(wallet.get());
reserver.reserve();
const CBlockIndex* const null_block = nullptr;
const CBlockIndex* stop_block;
const CBlockIndex *stop_block, *failed_block;
QCOMPARE(
wallet->ScanForWalletTransactions(chainActive.Genesis(), nullptr, reserver, stop_block, true /* fUpdate */),
wallet->ScanForWalletTransactions(chainActive.Genesis(), nullptr, reserver, failed_block, stop_block, true /* fUpdate */),
CWallet::ScanResult::SUCCESS);
QCOMPARE(stop_block, null_block);
QCOMPARE(stop_block, chainActive.Tip());
QCOMPARE(failed_block, null_block);
}
wallet->SetBroadcastTransactions(true);

Expand Down
7 changes: 3 additions & 4 deletions src/wallet/rpcwallet.cpp
View file
Expand Up @@ -3332,13 +3332,12 @@ UniValue rescanblockchain(const JSONRPCRequest& request)
}
}

const CBlockIndex* stopBlock;
const CBlockIndex *failed_block, *stopBlock;
CWallet::ScanResult result =
pwallet->ScanForWalletTransactions(pindexStart, pindexStop, reserver, stopBlock, true);
pwallet->ScanForWalletTransactions(pindexStart, pindexStop, reserver, failed_block, stopBlock, true);
switch (result) {
case CWallet::ScanResult::SUCCESS:
stopBlock = pindexStop ? pindexStop : pChainTip;
break;
break; // stopBlock set by ScanForWalletTransactions
case CWallet::ScanResult::FAILURE:
throw JSONRPCError(RPC_MISC_ERROR, "Rescan failed. Potentially corrupted data files.");
case CWallet::ScanResult::USER_ABORT:
Expand Down
21 changes: 12 additions & 9 deletions src/wallet/test/wallet_tests.cpp
View file
Expand Up @@ -53,9 +53,10 @@ BOOST_FIXTURE_TEST_CASE(rescan, TestChain100Setup)
AddKey(wallet, coinbaseKey);
WalletRescanReserver reserver(&wallet);
reserver.reserve();
const CBlockIndex* stop_block;
BOOST_CHECK_EQUAL(wallet.ScanForWalletTransactions(oldTip, nullptr, reserver, stop_block), CWallet::ScanResult::SUCCESS);
BOOST_CHECK_EQUAL(stop_block, null_block);
const CBlockIndex *stop_block, *failed_block;
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);
BOOST_CHECK_EQUAL(wallet.GetImmatureBalance(), 100 * COIN);
}

Expand All @@ -70,9 +71,10 @@ BOOST_FIXTURE_TEST_CASE(rescan, TestChain100Setup)
AddKey(wallet, coinbaseKey);
WalletRescanReserver reserver(&wallet);
reserver.reserve();
const CBlockIndex* stop_block;
BOOST_CHECK_EQUAL(wallet.ScanForWalletTransactions(oldTip, nullptr, reserver, stop_block), CWallet::ScanResult::FAILURE);
BOOST_CHECK_EQUAL(oldTip, stop_block);
const CBlockIndex *stop_block, *failed_block;
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);
}

Expand Down Expand Up @@ -291,9 +293,10 @@ class ListCoinsTestingSetup : public TestChain100Setup
WalletRescanReserver reserver(wallet.get());
reserver.reserve();
const CBlockIndex* const null_block = nullptr;
const CBlockIndex* stop_block;
BOOST_CHECK_EQUAL(wallet->ScanForWalletTransactions(chainActive.Genesis(), nullptr, reserver, stop_block), CWallet::ScanResult::SUCCESS);
BOOST_CHECK_EQUAL(stop_block, null_block);
const CBlockIndex *stop_block, *failed_block;
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);
}

~ListCoinsTestingSetup()
Expand Down
19 changes: 12 additions & 7 deletions src/wallet/wallet.cpp
View file
Expand Up @@ -1613,9 +1613,9 @@ int64_t CWallet::RescanFromTime(int64_t startTime, const WalletRescanReserver& r
}

if (startBlock) {
const CBlockIndex* failedBlock;
const CBlockIndex *failedBlock, *stop_block;
// TODO: this should take into account failure by ScanResult::USER_ABORT
if (ScanResult::FAILURE == ScanForWalletTransactions(startBlock, nullptr, reserver, failedBlock, update)) {
if (ScanResult::FAILURE == ScanForWalletTransactions(startBlock, nullptr, reserver, failedBlock, stop_block, update)) {
return failedBlock->GetBlockTimeMax() + TIMESTAMP_WINDOW + 1;
}
}
Expand All @@ -1628,8 +1628,10 @@ int64_t CWallet::RescanFromTime(int64_t startTime, const WalletRescanReserver& r
* exist in the wallet will be updated.
*
* @param[in] pindexStop if not a nullptr, the scan will stop at this block-index
* @param[out] failed_block if FAILURE is returned, will be set to the most
* recent block that could not be scanned, otherwise nullptr
* @param[out] failed_block if FAILURE is returned, the most recent block
* that could not be scanned, otherwise nullptr
* @param[out] stop_block the most recent block that could be scanned,
* otherwise nullptr if no block could be scanned
*
* @return ScanResult indicating success or failure of the scan. SUCCESS if
* scan was successful. FAILURE if a complete rescan was not possible (due to
Expand All @@ -1640,7 +1642,7 @@ int64_t CWallet::RescanFromTime(int64_t startTime, const WalletRescanReserver& r
* the main chain after to the addition of any new keys you want to detect
* transactions for.
*/
CWallet::ScanResult CWallet::ScanForWalletTransactions(const CBlockIndex* const pindexStart, const CBlockIndex* const pindexStop, const WalletRescanReserver& reserver, const CBlockIndex*& failed_block, bool fUpdate)
CWallet::ScanResult CWallet::ScanForWalletTransactions(const CBlockIndex* const pindexStart, const CBlockIndex* const pindexStop, const WalletRescanReserver& reserver, const CBlockIndex*& failed_block, const CBlockIndex*& stop_block, bool fUpdate)
{
int64_t nNow = GetTime();
const CChainParams& chainParams = Params();
Expand Down Expand Up @@ -1694,7 +1696,10 @@ CWallet::ScanResult CWallet::ScanForWalletTransactions(const CBlockIndex* const
for (size_t posInBlock = 0; posInBlock < block.vtx.size(); ++posInBlock) {
SyncTransaction(block.vtx[posInBlock], pindex, posInBlock, fUpdate);
}
// scan succeeded, record block as most recent successfully scanned
stop_block = pindex;
} else {
// could not scan block, keep scanning but record this block as the most recent failure
failed_block = pindex;
}
if (pindex == pindexStop) {
Expand Down Expand Up @@ -4174,8 +4179,8 @@ std::shared_ptr<CWallet> CWallet::CreateWalletFromFile(interfaces::Chain& chain,
nStart = GetTimeMillis();
{
WalletRescanReserver reserver(walletInstance.get());
const CBlockIndex* stop_block;
if (!reserver.reserve() || (ScanResult::SUCCESS != walletInstance->ScanForWalletTransactions(pindexRescan, nullptr, reserver, stop_block, true))) {
const CBlockIndex *stop_block, *failed_block;
if (!reserver.reserve() || (ScanResult::SUCCESS != walletInstance->ScanForWalletTransactions(pindexRescan, nullptr, reserver, failed_block, stop_block, true))) {
InitError(_("Failed to rescan the wallet during initialization"));
return nullptr;
}
Expand Down
2 changes: 1 addition & 1 deletion src/wallet/wallet.h
View file
Expand Up @@ -902,7 +902,7 @@ class CWallet final : public CCryptoKeyStore, public CValidationInterface
FAILURE,
USER_ABORT
};
ScanResult ScanForWalletTransactions(const CBlockIndex* const pindexStart, const CBlockIndex* const pindexStop, const WalletRescanReserver& reserver, const CBlockIndex*& failed_block, bool fUpdate = false);
ScanResult ScanForWalletTransactions(const CBlockIndex* const pindexStart, const CBlockIndex* const pindexStop, const WalletRescanReserver& reserver, const CBlockIndex*& failed_block, const CBlockIndex*& stop_block, bool fUpdate = false);
void TransactionRemovedFromMempool(const CTransactionRef &ptx) override;
void ReacceptWalletTransactions();
void ResendWalletTransactions(int64_t nBestBlockTime, CConnman* connman) override EXCLUSIVE_LOCKS_REQUIRED(cs_main);
Expand Down

0 comments on commit bd3b036

Please sign in to comment.