Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Coin database checks #2145

Merged
merged 3 commits into from

4 participants

@sipa
Owner

-checklevel gets a new meaning:
0: verify blocks can be read from disk (like before)
1: verify (contextless) block validity (like before)
2: verify undo data can be read and matches checksums
3: verify coin database is consistent with the last few blocks (close to level 6 before)
4: verify all validity rules of the last few blocks (including signature checks)

Level 3 is the new default, as it's reasonably fast. As levels 3 and 4 are implemented using an in-memory rollback of the database, they are limited to as many blocks as possible without exceeding the limits set by -dbcache. The default of -dbcache=25 allows for some 150-200 blocks to be rolled back.

In case an error is found, the application quits with a message instructing the user to restart with -reindex. Better instructions, and automatic recovery (when possible) or automatic reindexing are left as future work.

In addition, this also changes the on-disk format of undo data (adding a checksum), as the correctness of the coindb checks depends on having intact undo data.

sipa added some commits
@sipa sipa Make DisconnectBlock fault-tolerant 2cbd71d
@sipa sipa Add checksums to undo data
This should be compatible with older code that didn't write checksums.
8539361
@BitcoinPullTester

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/24e864547189c6b9347740f50d5fed656fc48816 for binaries and test log.

@BitcoinPullTester

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/aa38398d4bd32c6b90175d08b65ecb324701a368 for binaries and test log.

@jgarzik
Owner

ACK

@gmaxwell
Owner

Some problems with making reindex automatic is that reindex is quite burdensome and intrusive so you may want to control when its run, and the other problem is that making it automatic may hide hardware or software errors that can also cause silent failures. A failure of this test is something which should never happen, even if the user is randomly powering off their machine.

@gmaxwell
Owner

So, I see that setting dbcache really large doesn't check more than 2500 blocks by default. Thats good— wouldn't want a long startup time. But setting dbcache to 0 results in it checking very few. I'm not sure that this is a good idea.

I'm also seeing some odd memory usage behavior. Starting with -dbcache=4000 -checkblocks=0 results in an RES of 2.3 GiB once the check is finished.

I also found
2013-01-03 20:18:52 Verifying last 215017 blocks at level 2
2013-01-03 20:23:16 No coin database inconsistencies in last 31137 blocks (6770582 transactions)

a little odd, since that it's actually doing is checking 215017 at level 0/1 and checking 31137 at 2.

Should the undo data also get checked at level 1? Obviously if it's broken it will break the level 2 tests, but since we do the level 2 tests on fewer block, some specific undo tests might make sense? (e.g. we could count the txins in the block and the undo data)

@sipa
Owner

@gmaxwell there is a trivial undo test possible: verify their checksums. However, how meaningful is it to check undo data without checking the coin state - there is no scenario in which you need undo data but not coin state.

In way, the system is backwards. The coin state is by far the most important thing to verify, but unfortunately it is also almost the most expensive, and it already requires consistent blocks and undo data.

Regarding only checking very few with low dbcache... what would you suggest?

Also, the calculation for memory sizes based on transaction counts is only very approximately and spread over several types of caches (leveldb blktree, leveldb coindb, coins view cache). Doing the rollback pulls the transaction data in the global cache, and then changes to it in a memory-only cache on top of it, that is discarded. The transactions themself remain in the global cache though, afterwards.

@sipa
Owner

@Diapolo such a flag would be possible, but why add it? The next startup the database will most likely still be inconsistent, so it would be detected again (and if it isn't, maybe it was something temporary...).

For GUI users, I would like to see something like "Your database is corrupted. Do you want to rebuild it now, or exit?". For bitcoind I think the right approach is indeed quitting immediately with an error message instructing the user to restart with -reindex, so they can run the rebuild on their own terms (or perhaps copy a database from another instance or backup they have running).

@sipa
Owner

@gmaxwell Suggestions for better reporting are welcome :)

@gmaxwell
Owner

The reason I suggested a basic check of undo data is simply because unless dbcache is very large we test far fewer blocks at level 2 and there is currently no way to do even basic checking of all the undo data. This means that if dbcache is set small, we may not discover corrupted undo data which could be trivially discovered, until a reorg.

For small dbcache values— we could give it a minimum value... or could make it always some reasonable number even if we go over our coincachesize limit?

I really wish there were a way to randomly test the coins database as I expect the leveldb disk layout probably ends up with old and new data being exposed to different errors. Oh well.

In any case, tested with fuzzed undo (with and without undo checksums) and coins data. It's quite sensitive to errors.

After running it some with a corrupted leveldb and corrupted undos, after restoring the non-corrupted files, I'm getting

2013-01-04 07:09:47 ERROR: DisconnectBlock() : block and undo data inconsistent
2013-01-04 07:09:47 ERROR: VerifyDB() : *** irrecoverable inconsistency in block data

So either I screwed up, or the corruption was somehow propagated to other files. I note that many of the error messages are repeated, so I can't easily tell which test failed. If giving them distinct messages is too much work, perhaps just slipping in FILE and LINE would be helpful. (though this isn't an issue in the codebase unique to the checking pull)

@sipa sipa New database check routine
-checklevel gets a new meaning:
0: verify blocks can be read from disk (like before)
1: verify (contextless) block validity (like before)
2: verify undo files can be read and have good checksums
3: verify coin database is consistent with the last few blocks
   (close to level 6 before)
4: verify all validity rules of the last few blocks

Level 3 is the new default, as it's reasonably fast. As level 3 and
4 are implemented using an in-memory rollback of the database, they
are limited to as many blocks as possible without exceeding the
limits set by -dbcache. The default of -dbcache=25 allows for some
150-200 blocks to be rolled back.

In case an error is found, the application quits with a message
instructing the user to restart with -reindex. Better instructions,
and automatic recovery (when possible) or automatic reindexing are
left as future work.
1f355b6
@sipa
Owner

@gmaxwell I've added a new level in between 1 and 2 (which verifies undo data). I've also changed the heuristic for determining how far to roll back a bit - it now aims for using +- 5-10 MB extra for validation, even with very small dbcache.

@BitcoinPullTester

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/1f355b66cd90f1bd96a862604a7216e8ba56f853 for binaries and test log.

@gmaxwell
Owner

ACK ACK ACK

@sipa
Owner

Well the reported messages are still confusing. It's not actually checking 2500 blocks at level 3...

@gmaxwell
Owner

Because this increases our sensitivity to corruption so much, I'd rather pull now and get the messages just right in another pull.

@gmaxwell gmaxwell merged commit 1f4b80a into bitcoin:master
@sipa sipa deleted the sipa:checkcoins branch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jan 3, 2013
  1. @sipa
  2. @sipa

    Add checksums to undo data

    sipa authored
    This should be compatible with older code that didn't write checksums.
Commits on Jan 4, 2013
  1. @sipa

    New database check routine

    sipa authored
    -checklevel gets a new meaning:
    0: verify blocks can be read from disk (like before)
    1: verify (contextless) block validity (like before)
    2: verify undo files can be read and have good checksums
    3: verify coin database is consistent with the last few blocks
       (close to level 6 before)
    4: verify all validity rules of the last few blocks
    
    Level 3 is the new default, as it's reasonably fast. As level 3 and
    4 are implemented using an in-memory rollback of the database, they
    are limited to as many blocks as possible without exceeding the
    limits set by -dbcache. The default of -dbcache=25 allows for some
    150-200 blocks to be rolled back.
    
    In case an error is found, the application quits with a message
    instructing the user to restart with -reindex. Better instructions,
    and automatic recovery (when possible) or automatic reindexing are
    left as future work.
This page is out of date. Refresh to see the latest.
Showing with 146 additions and 42 deletions.
  1. +5 −2 src/init.cpp
  2. +89 −37 src/main.cpp
  3. +52 −3 src/main.h
View
7 src/init.cpp
@@ -300,7 +300,7 @@ std::string HelpMessage()
" -rescan " + _("Rescan the block chain for missing wallet transactions") + "\n" +
" -salvagewallet " + _("Attempt to recover private keys from a corrupt wallet.dat") + "\n" +
" -checkblocks=<n> " + _("How many blocks to check at startup (default: 2500, 0 = all)") + "\n" +
- " -checklevel=<n> " + _("How thorough the block verification is (0-6, default: 1)") + "\n" +
+ " -checklevel=<n> " + _("How thorough the block verification is (0-4, default: 3)") + "\n" +
" -loadblock=<file> " + _("Imports blocks from external blk000??.dat file") + "\n" +
" -reindex " + _("Rebuild blockchain index from current blk000??.dat files") + "\n" +
@@ -752,7 +752,10 @@ bool AppInit2()
pblocktree->WriteReindexing(true);
if (!LoadBlockIndex())
- return InitError(_("Error loading blkindex.dat"));
+ return InitError(_("Error loading block/coin databases"));
+
+ if (!VerifyDB())
+ return InitError(_("Corrupted database detected. Please restart the client with -reindex."));
// as LoadBlockIndex can take several minutes, it's possible the user
// requested to kill bitcoin-qt during the last operation. If so, exit.
View
126 src/main.cpp
@@ -1464,23 +1464,24 @@ bool CTransaction::ClientCheckInputs() const
-bool CBlock::DisconnectBlock(CBlockIndex *pindex, CCoinsViewCache &view)
+bool CBlock::DisconnectBlock(CBlockIndex *pindex, CCoinsViewCache &view, bool *pfClean)
{
assert(pindex == view.GetBestBlock());
+ if (pfClean)
+ *pfClean = false;
+
+ bool fClean = true;
+
CBlockUndo blockUndo;
- {
- CDiskBlockPos pos = pindex->GetUndoPos();
- if (pos.IsNull())
- return error("DisconnectBlock() : no undo data available");
- FILE *file = OpenUndoFile(pos, true);
- if (file == NULL)
- return error("DisconnectBlock() : undo file not available");
- CAutoFile fileUndo(file, SER_DISK, CLIENT_VERSION);
- fileUndo >> blockUndo;
- }
+ CDiskBlockPos pos = pindex->GetUndoPos();
+ if (pos.IsNull())
+ return error("DisconnectBlock() : no undo data available");
+ if (!blockUndo.ReadFromDisk(pos, pindex->pprev->GetBlockHash()))
+ return error("DisconnectBlock() : failure reading undo data");
- assert(blockUndo.vtxundo.size() + 1 == vtx.size());
+ if (blockUndo.vtxundo.size() + 1 != vtx.size())
+ return error("DisconnectBlock() : block and undo data inconsistent");
// undo transactions in reverse order
for (int i = vtx.size() - 1; i >= 0; i--) {
@@ -1488,13 +1489,15 @@ bool CBlock::DisconnectBlock(CBlockIndex *pindex, CCoinsViewCache &view)
uint256 hash = tx.GetHash();
// check that all outputs are available
- if (!view.HaveCoins(hash))
- return error("DisconnectBlock() : outputs still spent? database corrupted");
+ if (!view.HaveCoins(hash)) {
+ fClean = fClean && error("DisconnectBlock() : outputs still spent? database corrupted");
+ view.SetCoins(hash, CCoins());
+ }
CCoins &outs = view.GetCoins(hash);
CCoins outsBlock = CCoins(tx, pindex->nHeight);
if (outs != outsBlock)
- return error("DisconnectBlock() : added transaction mismatch? database corrupted");
+ fClean = fClean && error("DisconnectBlock() : added transaction mismatch? database corrupted");
// remove outputs
outs = CCoins();
@@ -1502,24 +1505,27 @@ bool CBlock::DisconnectBlock(CBlockIndex *pindex, CCoinsViewCache &view)
// restore inputs
if (i > 0) { // not coinbases
const CTxUndo &txundo = blockUndo.vtxundo[i-1];
- assert(txundo.vprevout.size() == tx.vin.size());
+ if (txundo.vprevout.size() != tx.vin.size())
+ return error("DisconnectBlock() : transaction and undo data inconsistent");
for (unsigned int j = tx.vin.size(); j-- > 0;) {
const COutPoint &out = tx.vin[j].prevout;
const CTxInUndo &undo = txundo.vprevout[j];
CCoins coins;
view.GetCoins(out.hash, coins); // this can fail if the prevout was already entirely spent
- if (coins.IsPruned()) {
- if (undo.nHeight == 0)
- return error("DisconnectBlock() : undo data doesn't contain tx metadata? database corrupted");
+ if (undo.nHeight != 0) {
+ // undo data contains height: this is the last output of the prevout tx being spent
+ if (!coins.IsPruned())
+ fClean = fClean && error("DisconnectBlock() : undo data overwriting existing transaction");
+ coins = CCoins();
coins.fCoinBase = undo.fCoinBase;
coins.nHeight = undo.nHeight;
coins.nVersion = undo.nVersion;
} else {
- if (undo.nHeight != 0)
- return error("DisconnectBlock() : undo data contains unneeded tx metadata? database corrupted");
+ if (coins.IsPruned())
+ fClean = fClean && error("DisconnectBlock() : undo data adding output to missing transaction");
}
if (coins.IsAvailable(out.n))
- return error("DisconnectBlock() : prevout output not spent? database corrupted");
+ fClean = fClean && error("DisconnectBlock() : undo data overwriting existing output");
if (coins.vout.size() < out.n+1)
coins.vout.resize(out.n+1);
coins.vout[out.n] = undo.txout;
@@ -1532,7 +1538,12 @@ bool CBlock::DisconnectBlock(CBlockIndex *pindex, CCoinsViewCache &view)
// move best block pointer to prevout block
view.SetBestBlock(pindex->pprev);
- return true;
+ if (pfClean) {
+ *pfClean = fClean;
+ return true;
+ } else {
+ return fClean;
+ }
}
void static FlushBlockFile()
@@ -1651,9 +1662,9 @@ bool CBlock::ConnectBlock(CBlockIndex* pindex, CCoinsViewCache &view, bool fJust
{
if (pindex->GetUndoPos().IsNull()) {
CDiskBlockPos pos;
- if (!FindUndoPos(pindex->nFile, pos, ::GetSerializeSize(blockundo, SER_DISK, CLIENT_VERSION) + 8))
+ if (!FindUndoPos(pindex->nFile, pos, ::GetSerializeSize(blockundo, SER_DISK, CLIENT_VERSION) + 40))
return error("ConnectBlock() : FindUndoPos failed");
- if (!blockundo.WriteToDisk(pos))
+ if (!blockundo.WriteToDisk(pos, pindex->pprev->GetBlockHash()))
return error("ConnectBlock() : CBlockUndo::WriteToDisk failed");
// update nUndoPos in block index
@@ -2377,36 +2388,77 @@ bool static LoadBlockIndexDB()
BlockHashStr(hashBestChain).c_str(), nBestHeight,
DateTimeStrFormat("%Y-%m-%d %H:%M:%S", pindexBest->GetBlockTime()).c_str());
+ return true;
+}
+
+bool VerifyDB() {
+ if (pindexBest == NULL || pindexBest->pprev == NULL)
+ return true;
+
// Verify blocks in the best chain
- int nCheckLevel = GetArg("-checklevel", 1);
+ int nCheckLevel = GetArg("-checklevel", 3);
int nCheckDepth = GetArg( "-checkblocks", 2500);
if (nCheckDepth == 0)
nCheckDepth = 1000000000; // suffices until the year 19000
if (nCheckDepth > nBestHeight)
nCheckDepth = nBestHeight;
+ nCheckLevel = std::max(0, std::min(4, nCheckLevel));
printf("Verifying last %i blocks at level %i\n", nCheckDepth, nCheckLevel);
- CBlockIndex* pindexFork = NULL;
+ CCoinsViewCache coins(*pcoinsTip, true);
+ CBlockIndex* pindexState = pindexBest;
+ CBlockIndex* pindexFailure = NULL;
+ int nGoodTransactions = 0;
for (CBlockIndex* pindex = pindexBest; pindex && pindex->pprev; pindex = pindex->pprev)
{
if (fRequestShutdown || pindex->nHeight < nBestHeight-nCheckDepth)
break;
CBlock block;
+ // check level 0: read from disk
if (!block.ReadFromDisk(pindex))
- return error("LoadBlockIndex() : block.ReadFromDisk failed");
+ return error("VerifyDB() : *** block.ReadFromDisk failed at %d, hash=%s", pindex->nHeight, pindex->GetBlockHash().ToString().c_str());
// check level 1: verify block validity
- if (nCheckLevel>0 && !block.CheckBlock())
- {
- printf("LoadBlockIndex() : *** found bad block at %d, hash=%s\n", pindex->nHeight, pindex->GetBlockHash().ToString().c_str());
- pindexFork = pindex->pprev;
+ if (nCheckLevel >= 1 && !block.CheckBlock())
+ return error("VerifyDB() : *** found bad block at %d, hash=%s\n", pindex->nHeight, pindex->GetBlockHash().ToString().c_str());
+ // check level 2: verify undo validity
+ if (nCheckLevel >= 2 && pindex) {
+ CBlockUndo undo;
+ CDiskBlockPos pos = pindex->GetUndoPos();
+ if (!pos.IsNull()) {
+ if (!undo.ReadFromDisk(pos, pindex->pprev->GetBlockHash()))
+ return error("VerifyDB() : *** found bad undo data at %d, hash=%s\n", pindex->nHeight, pindex->GetBlockHash().ToString().c_str());
+ }
+ }
+ // check level 3: check for inconsistencies during memory-only disconnect of tip blocks
+ if (nCheckLevel >= 3 && pindex == pindexState && (coins.GetCacheSize() + pcoinsTip->GetCacheSize()) <= 2*nCoinCacheSize + 32000) {
+ bool fClean = true;
+ if (!block.DisconnectBlock(pindex, coins, &fClean))
+ return error("VerifyDB() : *** irrecoverable inconsistency in block data at %d, hash=%s", pindex->nHeight, pindex->GetBlockHash().ToString().c_str());
+ pindexState = pindex->pprev;
+ if (!fClean) {
+ nGoodTransactions = 0;
+ pindexFailure = pindex;
+ } else
+ nGoodTransactions += block.vtx.size();
}
- // TODO: stronger verifications
}
- if (pindexFork && !fRequestShutdown)
- {
- // TODO: reorg back
- return error("LoadBlockIndex(): chain database corrupted");
+ if (pindexFailure)
+ return error("VerifyDB() : *** coin database inconsistencies found (last %i blocks, %i good transactions before that)\n", pindexBest->nHeight - pindexFailure->nHeight + 1, nGoodTransactions);
+
+ // check level 4: try reconnecting blocks
+ if (nCheckLevel >= 4) {
+ CBlockIndex *pindex = pindexState;
+ while (pindex != pindexBest && !fRequestShutdown) {
+ pindex = pindex->pnext;
+ CBlock block;
+ if (!block.ReadFromDisk(pindex))
+ return error("VerifyDB() : *** block.ReadFromDisk failed at %d, hash=%s", pindex->nHeight, pindex->GetBlockHash().ToString().c_str());
+ if (!block.ConnectBlock(pindex, coins))
+ return error("VerifyDB() : *** found unconnectable block at %d, hash=%s", pindex->nHeight, pindex->GetBlockHash().ToString().c_str());
+ }
}
+ printf("No coin database inconsistencies in last %i blocks (%i transactions)\n", pindexBest->nHeight - pindexState->nHeight, nGoodTransactions);
+
return true;
}
View
55 src/main.h
@@ -126,6 +126,8 @@ FILE* OpenUndoFile(const CDiskBlockPos &pos, bool fReadOnly = false);
bool LoadExternalBlockFile(FILE* fileIn, CDiskBlockPos *dbp = NULL);
/** Load the block tree and coins database from disk */
bool LoadBlockIndex();
+/** Verify consistency of the block and coin databases */
+bool VerifyDB();
/** Print the loaded block tree */
void PrintBlockTree();
/** Find a block by height in the currently-connected chain */
@@ -746,7 +748,7 @@ class CBlockUndo
READWRITE(vtxundo);
)
- bool WriteToDisk(CDiskBlockPos &pos)
+ bool WriteToDisk(CDiskBlockPos &pos, const uint256 &hashBlock)
{
// Open history file to append
CAutoFile fileout = CAutoFile(OpenUndoFile(pos), SER_DISK, CLIENT_VERSION);
@@ -764,6 +766,12 @@ class CBlockUndo
pos.nPos = (unsigned int)fileOutPos;
fileout << *this;
+ // calculate & write checksum
+ CHashWriter hasher(SER_GETHASH, PROTOCOL_VERSION);
+ hasher << hashBlock;
+ hasher << *this;
+ fileout << hasher.GetHash();
+
// Flush stdio buffers and commit to disk before returning
fflush(fileout);
if (!IsInitialBlockDownload())
@@ -771,6 +779,44 @@ class CBlockUndo
return true;
}
+
+ bool ReadFromDisk(const CDiskBlockPos &pos, const uint256 &hashBlock)
+ {
+ // Open history file to read
+ CAutoFile filein = CAutoFile(OpenUndoFile(pos, true), SER_DISK, CLIENT_VERSION);
+ if (!filein)
+ return error("CBlockUndo::ReadFromDisk() : OpenBlockFile failed");
+
+ // Read block
+ uint256 hashChecksum;
+ try {
+ filein >> *this;
+ }
+ catch (std::exception &e) {
+ return error("%s() : deserialize or I/O error", __PRETTY_FUNCTION__);
+ }
+
+ // for compatibility with pre-release code that didn't write checksums to undo data
+ // TODO: replace by a simply 'filein >> hashChecksum' in the above try block
+ try {
+ filein >> hashChecksum;
+ } catch (std::exception &e) {
+ hashChecksum = 0;
+ }
+ uint32_t hashInit = hashChecksum.Get64(0) & 0xFFFFFFFFUL;
+ if (hashChecksum == 0 || memcmp(&hashInit, pchMessageStart, 4) == 0)
+ return true;
+
+ // Verify checksum
+ CHashWriter hasher(SER_GETHASH, PROTOCOL_VERSION);
+ hasher << hashBlock;
+ hasher << *this;
+ if (hashChecksum != hasher.GetHash())
+ return error("CBlockUndo::ReadFromDisk() : checksum mismatch");
+
+ return true;
+ }
+
};
/** pruned version of CTransaction: only retains metadata and unspent transaction outputs
@@ -1305,8 +1351,11 @@ class CBlock : public CBlockHeader
}
- // Undo the effects of this block (with given index) on the UTXO set represented by coins
- bool DisconnectBlock(CBlockIndex *pindex, CCoinsViewCache &coins);
+ /** Undo the effects of this block (with given index) on the UTXO set represented by coins.
+ * In case pfClean is provided, operation will try to be tolerant about errors, and *pfClean
+ * will be true if no problems were found. Otherwise, the return value will be false in case
+ * of problems. Note that in any case, coins may be modified. */
+ bool DisconnectBlock(CBlockIndex *pindex, CCoinsViewCache &coins, bool *pfClean = NULL);
// Apply the effects of this block (with given index) on the UTXO set represented by coins
bool ConnectBlock(CBlockIndex *pindex, CCoinsViewCache &coins, bool fJustCheck=false);
Something went wrong with that request. Please try again.