LBRYCommunity/lbrycrd
1
0
Fork 0
Code Issues 29 Pull requests 1 Projects Releases 32 Packages Wiki Activity

Fix importmulti failure to return rescan errors

Browse source 
An off-by-one-block bug in importmulti rescan logic could cause it to return
success in an edge case even when a rescan was not successful. The case where
this would happen is if there were multiple blocks in a row with the same
GetBlockTimeMax() value, and the last block was scanned successfully, but one
or more of the earlier blocks was not readable.
This commit is contained in:
Russell Yanofsky 2017-05-15 09:11:03 -04:00
parent 41987aa92f
commit 4d2d6045a4
3 changed files with 34 additions and 29 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

21
src/wallet/rpcdump.cpp
View file

@ -1121,13 +1121,13 @@ UniValue importmulti(const JSONRPCRequest& mainRequest)
if (fRescan && fRunScan && requests.size()) {
CBlockIndex* pindex = nLowestTimestamp > minimumTimestamp ? chainActive.FindEarliestAtLeast(std::max<int64_t>(nLowestTimestamp - TIMESTAMP_WINDOW, 0)) : chainActive.Genesis();
CBlockIndex* scannedRange = nullptr;
CBlockIndex* scanFailed = nullptr;
if (pindex) {
scannedRange = pwallet->ScanForWalletTransactions(pindex, true);
scanFailed = pwallet->ScanForWalletTransactions(pindex, true);
pwallet->ReacceptWalletTransactions();
}
if (!scannedRange || scannedRange->nHeight > pindex->nHeight) {
if (scanFailed) {
std::vector<UniValue> results = response.getValues();
response.clear();
response.setArray();
@ -1137,12 +1137,23 @@ UniValue importmulti(const JSONRPCRequest& mainRequest)
// 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) - TIMESTAMP_WINDOW >= scannedRange->GetBlockTimeMax() || results.at(i).exists("error")) {
if (GetImportTimestamp(request, now) - TIMESTAMP_WINDOW > scanFailed->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())));
result.pushKV(
"error",
JSONRPCError(
RPC_MISC_ERROR,
strprintf("Rescan failed for key with creation timestamp %d. There was an error reading a "
"block from time %d, which is after or within %d seconds of key creation, and "
"could contain transactions pertaining to the key. As a result, transactions "
"and coins using this key may not appear in the wallet. This error could be "
"caused by pruning or data corruption (see bitcoind log for details) and could "
"be dealt with by downloading and rescanning the relevant blocks (see -reindex "
"and -rescan options).",
GetImportTimestamp(request, now), scanFailed->GetBlockTimeMax(), TIMESTAMP_WINDOW)));
response.push_back(std::move(result));
}
++i;

28
src/wallet/test/wallet_tests.cpp
View file

@ -364,6 +364,7 @@ BOOST_FIXTURE_TEST_CASE(rescan, TestChain100Setup)
LOCK(cs_main);
// Cap last block file size, and mine new block in a new block file.
CBlockIndex* const nullBlock = nullptr;
CBlockIndex* oldTip = chainActive.Tip();
GetBlockFileInfo(oldTip->GetBlockPos().nFile)->nSize = MAX_BLOCKFILE_SIZE;
CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey()));
@ -375,7 +376,7 @@ BOOST_FIXTURE_TEST_CASE(rescan, TestChain100Setup)
CWallet wallet;
LOCK(wallet.cs_wallet);
wallet.AddKeyPubKey(coinbaseKey, coinbaseKey.GetPubKey());
BOOST_CHECK_EQUAL(oldTip, wallet.ScanForWalletTransactions(oldTip));
BOOST_CHECK_EQUAL(nullBlock, wallet.ScanForWalletTransactions(oldTip));
BOOST_CHECK_EQUAL(wallet.GetImmatureBalance(), 100 * COIN);
}
@ -389,7 +390,7 @@ BOOST_FIXTURE_TEST_CASE(rescan, TestChain100Setup)
CWallet wallet;
LOCK(wallet.cs_wallet);
wallet.AddKeyPubKey(coinbaseKey, coinbaseKey.GetPubKey());
BOOST_CHECK_EQUAL(newTip, wallet.ScanForWalletTransactions(oldTip));
BOOST_CHECK_EQUAL(oldTip, wallet.ScanForWalletTransactions(oldTip));
BOOST_CHECK_EQUAL(wallet.GetImmatureBalance(), 50 * COIN);
}
@ -413,7 +414,7 @@ BOOST_FIXTURE_TEST_CASE(rescan, TestChain100Setup)
CKey futureKey;
futureKey.MakeNewKey(true);
key.pushKV("scriptPubKey", HexStr(GetScriptForRawPubKey(futureKey.GetPubKey())));
key.pushKV("timestamp", newTip->GetBlockTimeMax() + TIMESTAMP_WINDOW);
key.pushKV("timestamp", newTip->GetBlockTimeMax() + TIMESTAMP_WINDOW + 1);
key.pushKV("internal", UniValue(true));
keys.push_back(key);
JSONRPCRequest request;
@ -421,20 +422,17 @@ BOOST_FIXTURE_TEST_CASE(rescan, TestChain100Setup)
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.\"}},{\"success\":true}]", newTip->GetBlockTimeMax()));
BOOST_CHECK_EQUAL(response.write(),
strprintf("[{\"success\":false,\"error\":{\"code\":-1,\"message\":\"Rescan failed for key with creation "
"timestamp %d. There was an error reading a block from time %d, which is after or within %d "
"seconds of key creation, and could contain transactions pertaining to the key. As a result, "
"transactions and coins using this key may not appear in the wallet. This error could be caused "
"by pruning or data corruption (see bitcoind log for details) and could be dealt with by "
"downloading and rescanning the relevant blocks (see -reindex and -rescan "
"options).\"}},{\"success\":true}]",
0, oldTip->GetBlockTimeMax(), TIMESTAMP_WINDOW));
::pwalletMain = backup;
}
// Verify ScanForWalletTransactions does not return null when the scan is
// elided due to the nTimeFirstKey optimization.
{
CWallet wallet;
{
LOCK(wallet.cs_wallet);
wallet.UpdateTimeFirstKey(newTip->GetBlockTime() + 7200 + 1);
}
BOOST_CHECK_EQUAL(newTip, wallet.ScanForWalletTransactions(newTip));
}
}
// Check that GetImmatureCredit() returns a newly calculated value instead of

14
src/wallet/wallet.cpp
View file

@ -1456,10 +1456,9 @@ void CWalletTx::GetAmounts(std::list<COutputEntry>& listReceived,
* 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 or elided (elided if pIndexStart points at a block
* before CWallet::nTimeFirstKey). Returns null if there is no such range, or
* the range doesn't include chainActive.Tip().
* Returns null if scan was successful. Otherwise, if a complete rescan was not
* possible (due to pruning or corruption), returns pointer to the most recent
* block that could not be scanned.
*/
CBlockIndex* CWallet::ScanForWalletTransactions(CBlockIndex* pindexStart, bool fUpdate)
{
@ -1467,7 +1466,7 @@ CBlockIndex* CWallet::ScanForWalletTransactions(CBlockIndex* pindexStart, bool f
const CChainParams& chainParams = Params();
CBlockIndex* pindex = pindexStart;
CBlockIndex* ret = pindexStart;
CBlockIndex* ret = nullptr;
{
LOCK2(cs_main, cs_wallet);
fAbortRescan = false;
@ -1495,11 +1494,8 @@ CBlockIndex* CWallet::ScanForWalletTransactions(CBlockIndex* pindexStart, bool f
for (size_t posInBlock = 0; posInBlock < block.vtx.size(); ++posInBlock) {
AddToWalletIfInvolvingMe(block.vtx[posInBlock], pindex, posInBlock, fUpdate);
}
if (!ret) {
ret = pindex;
}
} else {
ret = nullptr;
ret = pindex;
}
pindex = chainActive.Next(pindex);
}