Skip to content

Commit e2e2f4c

Browse files
ryanofskyryanofsky
committed
Return errors from importmulti if complete rescans are not successful
1 parent 9828f9a commit e2e2f4c

File tree

6 files changed

+114
-13
lines changed

6 files changed

+114
-13
lines changed

‎src/validation.cpp‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3331,7 +3331,7 @@ void PruneOneBlockFile(const int fileNumber)
33313331
}
33323332

33333333

3334-
void UnlinkPrunedFiles(std::set<int>& setFilesToPrune)
3334+
void UnlinkPrunedFiles(const std::set<int>& setFilesToPrune)
33353335
{
33363336
for (std::set<int>::iterator it = setFilesToPrune.begin(); it != setFilesToPrune.end(); ++it) {
33373337
CDiskBlockPos pos(*it, 0);
@@ -4163,6 +4163,11 @@ std::string CBlockFileInfo::ToString() const
41634163
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));
41644164
}
41654165

4166+
CBlockFileInfo* GetBlockFileInfo(size_t n)
4167+
{
4168+
return &vinfoBlockFile.at(n);
4169+
}
4170+
41664171
ThresholdState VersionBitsTipState(const Consensus::Params& params, Consensus::DeploymentPos pos)
41674172
{
41684173
LOCK(cs_main);

‎src/validation.h‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -299,10 +299,15 @@ double GuessVerificationProgress(const ChainTxData& data, CBlockIndex* pindex);
299299
*/
300300
void FindFilesToPrune(std::set<int>& setFilesToPrune, uint64_t nPruneAfterHeight);
301301

302+
/**
303+
* Mark one block file as pruned.
304+
*/
305+
void PruneOneBlockFile(const int fileNumber);
306+
302307
/**
303308
* Actually unlink the specified files
304309
*/
305-
void UnlinkPrunedFiles(std::set<int>& setFilesToPrune);
310+
void UnlinkPrunedFiles(const std::set<int>& setFilesToPrune);
306311

307312
/** Create a new block index entry for a given block hash */
308313
CBlockIndex * InsertBlockIndex(uint256 hash);
@@ -562,6 +567,9 @@ static const unsigned int REJECT_ALREADY_KNOWN = 0x101;
562567
/** Transaction conflicts with a transaction already known */
563568
static const unsigned int REJECT_CONFLICT = 0x102;
564569

570+
/** Get block file info entry for one block file */
571+
CBlockFileInfo* GetBlockFileInfo(size_t n);
572+
565573
/** Dump the mempool to disk. */
566574
void DumpMempool();
567575

‎src/wallet/rpcdump.cpp‎

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1074,11 +1074,32 @@ UniValue importmulti(const JSONRPCRequest& mainRequest)
10741074

10751075
if (fRescan && fRunScan && requests.size() && nLowestTimestamp <= chainActive.Tip()->GetBlockTimeMax()) {
10761076
CBlockIndex* pindex = nLowestTimestamp > minimumTimestamp ? chainActive.FindEarliestAtLeast(std::max<int64_t>(nLowestTimestamp - 7200, 0)) : chainActive.Genesis();
1077-
1077+
CBlockIndex* scannedRange = nullptr;
10781078
if (pindex) {
1079-
pwalletMain->ScanForWalletTransactions(pindex, true);
1079+
scannedRange = pwalletMain->ScanForWalletTransactions(pindex, true);
10801080
pwalletMain->ReacceptWalletTransactions();
10811081
}
1082+
1083+
if (!scannedRange || scannedRange->nHeight > pindex->nHeight) {
1084+
std::vector<UniValue> results = response.getValues();
1085+
response.clear();
1086+
response.setArray();
1087+
size_t i = 0;
1088+
for (const UniValue& request : requests.getValues()) {
1089+
// If key creation date is within the successfully scanned
1090+
// range, or if the import result already has an error set, let
1091+
// the result stand unmodified. Otherwise replace the result
1092+
// with an error message.
1093+
if (GetImportTimestamp(request, now) - 7200 >= scannedRange->GetBlockTimeMax() || results.at(i).exists("error")) {
1094+
response.push_back(results.at(i));
1095+
} else {
1096+
UniValue result = UniValue(UniValue::VOBJ);
1097+
result.pushKV("success", UniValue(false));
1098+
result.pushKV("error", JSONRPCError(RPC_MISC_ERROR, strprintf("Failed to rescan before time %d, transactions may be missing.", scannedRange->GetBlockTimeMax())));
1099+
response.push_back(std::move(result));
1100+
}
1101+
}
1102+
}
10821103
}
10831104

10841105
return response;

‎src/wallet/test/wallet_tests.cpp‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,16 @@
99
#include <utility>
1010
#include <vector>
1111

12+
#include "rpc/server.h"
13+
#include "test/test_bitcoin.h"
14+
#include "validation.h"
1215
#include "wallet/test/wallet_test_fixture.h"
1316

1417
#include <boost/foreach.hpp>
1518
#include <boost/test/unit_test.hpp>
19+
#include <univalue.h>
20+
21+
extern UniValue importmulti(const JSONRPCRequest& request);
1622

1723
// how many times to run all the tests to have a chance to catch errors that only show up with particular random shuffles
1824
#define RUN_TESTS 100
@@ -355,4 +361,58 @@ BOOST_AUTO_TEST_CASE(ApproximateBestSubset)
355361
empty_wallet();
356362
}
357363

364+
BOOST_FIXTURE_TEST_CASE(rescan, TestChain100Setup)
365+
{
366+
LOCK(cs_main);
367+
368+
// Cap last block file size, and mine new block in a new block file.
369+
CBlockIndex* oldTip = chainActive.Tip();
370+
GetBlockFileInfo(oldTip->GetBlockPos().nFile)->nSize = MAX_BLOCKFILE_SIZE;
371+
CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey()));
372+
CBlockIndex* newTip = chainActive.Tip();
373+
374+
// Verify ScanForWalletTransactions picks up transactions in both the old
375+
// and new block files.
376+
{
377+
CWallet wallet;
378+
LOCK(wallet.cs_wallet);
379+
wallet.AddKeyPubKey(coinbaseKey, coinbaseKey.GetPubKey());
380+
BOOST_CHECK_EQUAL(oldTip, wallet.ScanForWalletTransactions(oldTip));
381+
BOOST_CHECK_EQUAL(wallet.GetImmatureBalance(), 100 * COIN);
382+
}
383+
384+
// Prune the older block file.
385+
PruneOneBlockFile(oldTip->GetBlockPos().nFile);
386+
UnlinkPrunedFiles({oldTip->GetBlockPos().nFile});
387+
388+
// Verify ScanForWalletTransactions only picks transactions in the new block
389+
// file.
390+
{
391+
CWallet wallet;
392+
LOCK(wallet.cs_wallet);
393+
wallet.AddKeyPubKey(coinbaseKey, coinbaseKey.GetPubKey());
394+
BOOST_CHECK_EQUAL(newTip, wallet.ScanForWalletTransactions(oldTip));
395+
BOOST_CHECK_EQUAL(wallet.GetImmatureBalance(), 50 * COIN);
396+
}
397+
398+
{
399+
CWallet wallet;
400+
::pwalletMain = &wallet;
401+
UniValue key;
402+
key.setObject();
403+
key.pushKV("scriptPubKey", HexStr(GetScriptForRawPubKey(coinbaseKey.GetPubKey())));
404+
key.pushKV("timestamp", 0);
405+
key.pushKV("internal", UniValue(true));
406+
UniValue keys;
407+
keys.setArray();
408+
keys.push_back(key);
409+
JSONRPCRequest request;
410+
request.params.setArray();
411+
request.params.push_back(keys);
412+
413+
UniValue response = importmulti(request);
414+
BOOST_CHECK_EQUAL(response.write(), strprintf("[{\"success\":false,\"error\":{\"code\":-1,\"message\":\"Failed to rescan before time %d, transactions may be missing.\"}}]", newTip->GetBlockTimeMax()));
415+
}
416+
}
417+
358418
BOOST_AUTO_TEST_SUITE_END()

‎src/wallet/wallet.cpp‎

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1545,10 +1545,14 @@ void CWalletTx::GetAccountAmounts(const string& strAccount, CAmount& nReceived,
15451545
* Scan the block chain (starting in pindexStart) for transactions
15461546
* from or to us. If fUpdate is true, found transactions that already
15471547
* exist in the wallet will be updated.
1548+
*
1549+
* Returns pointer to the first block in the last contiguous range that was
1550+
* successfully scanned.
1551+
*
15481552
*/
1549-
int CWallet::ScanForWalletTransactions(CBlockIndex* pindexStart, bool fUpdate)
1553+
CBlockIndex* CWallet::ScanForWalletTransactions(CBlockIndex* pindexStart, bool fUpdate)
15501554
{
1551-
int ret = 0;
1555+
CBlockIndex* ret = nullptr;
15521556
int64_t nNow = GetTime();
15531557
const CChainParams& chainParams = Params();
15541558

@@ -1570,12 +1574,15 @@ int CWallet::ScanForWalletTransactions(CBlockIndex* pindexStart, bool fUpdate)
15701574
ShowProgress(_("Rescanning..."), std::max(1, std::min(99, (int)((GuessVerificationProgress(chainParams.TxData(), pindex) - dProgressStart) / (dProgressTip - dProgressStart) * 100))));
15711575

15721576
CBlock block;
1573-
ReadBlockFromDisk(block, pindex, Params().GetConsensus());
1574-
int posInBlock;
1575-
for (posInBlock = 0; posInBlock < (int)block.vtx.size(); posInBlock++)
1576-
{
1577-
if (AddToWalletIfInvolvingMe(*block.vtx[posInBlock], pindex, posInBlock, fUpdate))
1578-
ret++;
1577+
if (ReadBlockFromDisk(block, pindex, Params().GetConsensus())) {
1578+
for (size_t posInBlock = 0; posInBlock < block.vtx.size(); ++posInBlock) {
1579+
AddToWalletIfInvolvingMe(*block.vtx[posInBlock], pindex, posInBlock, fUpdate);
1580+
}
1581+
if (!ret) {
1582+
ret = pindex;
1583+
}
1584+
} else {
1585+
ret = nullptr;
15791586
}
15801587
pindex = chainActive.Next(pindex);
15811588
if (GetTime() >= nNow + 60) {

‎src/wallet/wallet.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -788,7 +788,7 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface
788788
bool LoadToWallet(const CWalletTx& wtxIn);
789789
void SyncTransaction(const CTransaction& tx, const CBlockIndex *pindex, int posInBlock) override;
790790
bool AddToWalletIfInvolvingMe(const CTransaction& tx, const CBlockIndex* pIndex, int posInBlock, bool fUpdate);
791-
int ScanForWalletTransactions(CBlockIndex* pindexStart, bool fUpdate = false);
791+
CBlockIndex* ScanForWalletTransactions(CBlockIndex* pindexStart, bool fUpdate = false);
792792
void ReacceptWalletTransactions();
793793
void ResendWalletTransactions(int64_t nBestBlockTime, CConnman* connman) override;
794794
std::vector<uint256> ResendWalletTransactionsBefore(int64_t nTime, CConnman* connman);

0 commit comments

Comments
 (0)