From e2e2f4c856363bbb0e3b5ba4df225f3754c3db39 Mon Sep 17 00:00:00 2001 From: Russell Yanofsky Date: Thu, 16 Feb 2017 10:49:03 -0500 Subject: [PATCH] Return errors from importmulti if complete rescans are not successful --- src/validation.cpp | 7 +++- src/validation.h | 10 +++++- src/wallet/rpcdump.cpp | 25 +++++++++++-- src/wallet/test/wallet_tests.cpp | 60 ++++++++++++++++++++++++++++++++ src/wallet/wallet.cpp | 23 +++++++----- src/wallet/wallet.h | 2 +- 6 files changed, 114 insertions(+), 13 deletions(-) diff --git a/src/validation.cpp b/src/validation.cpp index 4ce0723b2..dbb42ab8c 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -3331,7 +3331,7 @@ void PruneOneBlockFile(const int fileNumber) } -void UnlinkPrunedFiles(std::set& setFilesToPrune) +void UnlinkPrunedFiles(const std::set& setFilesToPrune) { for (std::set::iterator it = setFilesToPrune.begin(); it != setFilesToPrune.end(); ++it) { CDiskBlockPos pos(*it, 0); @@ -4163,6 +4163,11 @@ std::string CBlockFileInfo::ToString() const return strprintf("CBlockFileInfo(blocks=%u, size=%u, heights=%u...%u, time=%s...%s)", nBlocks, nSize, nHeightFirst, nHeightLast, DateTimeStrFormat("%Y-%m-%d", nTimeFirst), DateTimeStrFormat("%Y-%m-%d", nTimeLast)); } +CBlockFileInfo* GetBlockFileInfo(size_t n) +{ + return &vinfoBlockFile.at(n); +} + ThresholdState VersionBitsTipState(const Consensus::Params& params, Consensus::DeploymentPos pos) { LOCK(cs_main); diff --git a/src/validation.h b/src/validation.h index 6fcbb1c10..9c606f241 100644 --- a/src/validation.h +++ b/src/validation.h @@ -299,10 +299,15 @@ double GuessVerificationProgress(const ChainTxData& data, CBlockIndex* pindex); */ void FindFilesToPrune(std::set& setFilesToPrune, uint64_t nPruneAfterHeight); +/** + * Mark one block file as pruned. + */ +void PruneOneBlockFile(const int fileNumber); + /** * Actually unlink the specified files */ -void UnlinkPrunedFiles(std::set& setFilesToPrune); +void UnlinkPrunedFiles(const std::set& setFilesToPrune); /** Create a new block index entry for a given block hash */ CBlockIndex * InsertBlockIndex(uint256 hash); @@ -562,6 +567,9 @@ static const unsigned int REJECT_ALREADY_KNOWN = 0x101; /** Transaction conflicts with a transaction already known */ static const unsigned int REJECT_CONFLICT = 0x102; +/** Get block file info entry for one block file */ +CBlockFileInfo* GetBlockFileInfo(size_t n); + /** Dump the mempool to disk. */ void DumpMempool(); diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 30f241467..68049dcef 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -1074,11 +1074,32 @@ UniValue importmulti(const JSONRPCRequest& mainRequest) if (fRescan && fRunScan && requests.size() && nLowestTimestamp <= chainActive.Tip()->GetBlockTimeMax()) { CBlockIndex* pindex = nLowestTimestamp > minimumTimestamp ? chainActive.FindEarliestAtLeast(std::max(nLowestTimestamp - 7200, 0)) : chainActive.Genesis(); - + CBlockIndex* scannedRange = nullptr; if (pindex) { - pwalletMain->ScanForWalletTransactions(pindex, true); + scannedRange = pwalletMain->ScanForWalletTransactions(pindex, true); pwalletMain->ReacceptWalletTransactions(); } + + if (!scannedRange || scannedRange->nHeight > pindex->nHeight) { + std::vector results = response.getValues(); + response.clear(); + response.setArray(); + size_t i = 0; + for (const UniValue& request : requests.getValues()) { + // If key creation date is within the successfully scanned + // range, or if the import result already has an error set, let + // the result stand unmodified. Otherwise replace the result + // with an error message. + if (GetImportTimestamp(request, now) - 7200 >= scannedRange->GetBlockTimeMax() || results.at(i).exists("error")) { + response.push_back(results.at(i)); + } else { + UniValue result = UniValue(UniValue::VOBJ); + result.pushKV("success", UniValue(false)); + result.pushKV("error", JSONRPCError(RPC_MISC_ERROR, strprintf("Failed to rescan before time %d, transactions may be missing.", scannedRange->GetBlockTimeMax()))); + response.push_back(std::move(result)); + } + } + } } return response; diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index ca086c86a..d32e8ba06 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -9,10 +9,16 @@ #include #include +#include "rpc/server.h" +#include "test/test_bitcoin.h" +#include "validation.h" #include "wallet/test/wallet_test_fixture.h" #include #include +#include + +extern UniValue importmulti(const JSONRPCRequest& request); // how many times to run all the tests to have a chance to catch errors that only show up with particular random shuffles #define RUN_TESTS 100 @@ -355,4 +361,58 @@ BOOST_AUTO_TEST_CASE(ApproximateBestSubset) empty_wallet(); } +BOOST_FIXTURE_TEST_CASE(rescan, TestChain100Setup) +{ + LOCK(cs_main); + + // 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(); + + // Verify ScanForWalletTransactions picks up transactions in both the old + // and new block files. + { + CWallet wallet; + LOCK(wallet.cs_wallet); + wallet.AddKeyPubKey(coinbaseKey, coinbaseKey.GetPubKey()); + BOOST_CHECK_EQUAL(oldTip, wallet.ScanForWalletTransactions(oldTip)); + BOOST_CHECK_EQUAL(wallet.GetImmatureBalance(), 100 * COIN); + } + + // Prune the older block file. + PruneOneBlockFile(oldTip->GetBlockPos().nFile); + UnlinkPrunedFiles({oldTip->GetBlockPos().nFile}); + + // Verify ScanForWalletTransactions only picks transactions in the new block + // file. + { + CWallet wallet; + LOCK(wallet.cs_wallet); + wallet.AddKeyPubKey(coinbaseKey, coinbaseKey.GetPubKey()); + BOOST_CHECK_EQUAL(newTip, wallet.ScanForWalletTransactions(oldTip)); + BOOST_CHECK_EQUAL(wallet.GetImmatureBalance(), 50 * COIN); + } + + { + CWallet wallet; + ::pwalletMain = &wallet; + UniValue key; + key.setObject(); + key.pushKV("scriptPubKey", HexStr(GetScriptForRawPubKey(coinbaseKey.GetPubKey()))); + key.pushKV("timestamp", 0); + key.pushKV("internal", UniValue(true)); + UniValue keys; + keys.setArray(); + keys.push_back(key); + JSONRPCRequest request; + request.params.setArray(); + request.params.push_back(keys); + + UniValue response = importmulti(request); + BOOST_CHECK_EQUAL(response.write(), strprintf("[{\"success\":false,\"error\":{\"code\":-1,\"message\":\"Failed to rescan before time %d, transactions may be missing.\"}}]", newTip->GetBlockTimeMax())); + } +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index f8f5a9306..1b59c3cd8 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1545,10 +1545,14 @@ void CWalletTx::GetAccountAmounts(const string& strAccount, CAmount& nReceived, * Scan the block chain (starting in pindexStart) for transactions * from or to us. If fUpdate is true, found transactions that already * exist in the wallet will be updated. + * + * Returns pointer to the first block in the last contiguous range that was + * successfully scanned. + * */ -int CWallet::ScanForWalletTransactions(CBlockIndex* pindexStart, bool fUpdate) +CBlockIndex* CWallet::ScanForWalletTransactions(CBlockIndex* pindexStart, bool fUpdate) { - int ret = 0; + CBlockIndex* ret = nullptr; int64_t nNow = GetTime(); const CChainParams& chainParams = Params(); @@ -1570,12 +1574,15 @@ int CWallet::ScanForWalletTransactions(CBlockIndex* pindexStart, bool fUpdate) ShowProgress(_("Rescanning..."), std::max(1, std::min(99, (int)((GuessVerificationProgress(chainParams.TxData(), pindex) - dProgressStart) / (dProgressTip - dProgressStart) * 100)))); CBlock block; - ReadBlockFromDisk(block, pindex, Params().GetConsensus()); - int posInBlock; - for (posInBlock = 0; posInBlock < (int)block.vtx.size(); posInBlock++) - { - if (AddToWalletIfInvolvingMe(*block.vtx[posInBlock], pindex, posInBlock, fUpdate)) - ret++; + if (ReadBlockFromDisk(block, pindex, Params().GetConsensus())) { + for (size_t posInBlock = 0; posInBlock < block.vtx.size(); ++posInBlock) { + AddToWalletIfInvolvingMe(*block.vtx[posInBlock], pindex, posInBlock, fUpdate); + } + if (!ret) { + ret = pindex; + } + } else { + ret = nullptr; } pindex = chainActive.Next(pindex); if (GetTime() >= nNow + 60) { diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index a7b15a844..98e4fb87b 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -788,7 +788,7 @@ public: bool LoadToWallet(const CWalletTx& wtxIn); void SyncTransaction(const CTransaction& tx, const CBlockIndex *pindex, int posInBlock) override; bool AddToWalletIfInvolvingMe(const CTransaction& tx, const CBlockIndex* pIndex, int posInBlock, bool fUpdate); - int ScanForWalletTransactions(CBlockIndex* pindexStart, bool fUpdate = false); + CBlockIndex* ScanForWalletTransactions(CBlockIndex* pindexStart, bool fUpdate = false); void ReacceptWalletTransactions(); void ResendWalletTransactions(int64_t nBestBlockTime, CConnman* connman) override; std::vector ResendWalletTransactionsBefore(int64_t nTime, CConnman* connman);