Simplify DisconnectBlock arguments/return value

DisconnectBlock currently has a complicated interface:

  Situation       Return value
                  pfClean != nullptr   pfClean == nullptr

  All good:       true                 true
  Failure:        false                false
  Unclean rewind: true                 false
                  with *pfClean=false

Change this to return a tristate enum instead. As an added bonus,
remove the ValidationState& argument which was unused.
This commit is contained in:
Pieter Wuille 2017-04-28 16:08:39 -07:00
parent 4c924011f5
commit db994b2de9

View File

@ -1526,28 +1526,36 @@ bool ApplyTxInUndo(const CTxInUndo& undo, CCoinsViewCache& view, const COutPoint
return fClean; return fClean;
} }
enum DisconnectResult
{
DISCONNECT_OK, // All good.
DISCONNECT_UNCLEAN, // Rolled back, but UTXO set was inconsistent with block.
DISCONNECT_FAILED // Something else went wrong.
};
/** Undo the effects of this block (with given index) on the UTXO set represented by 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 * When UNCLEAN or FAILED is returned, view is left in an indeterminate state. */
* will be true if no problems were found. Otherwise, the return value will be false in case static DisconnectResult DisconnectBlock(const CBlock& block, const CBlockIndex* pindex, CCoinsViewCache& view)
* of problems. Note that in any case, coins may be modified. */
static bool DisconnectBlock(const CBlock& block, CValidationState& state, const CBlockIndex* pindex, CCoinsViewCache& view, bool* pfClean = NULL)
{ {
assert(pindex->GetBlockHash() == view.GetBestBlock()); assert(pindex->GetBlockHash() == view.GetBestBlock());
if (pfClean)
*pfClean = false;
bool fClean = true; bool fClean = true;
CBlockUndo blockUndo; CBlockUndo blockUndo;
CDiskBlockPos pos = pindex->GetUndoPos(); CDiskBlockPos pos = pindex->GetUndoPos();
if (pos.IsNull()) if (pos.IsNull()) {
return error("DisconnectBlock(): no undo data available"); error("DisconnectBlock(): no undo data available");
if (!UndoReadFromDisk(blockUndo, pos, pindex->pprev->GetBlockHash())) return DISCONNECT_FAILED;
return error("DisconnectBlock(): failure reading undo data"); }
if (!UndoReadFromDisk(blockUndo, pos, pindex->pprev->GetBlockHash())) {
error("DisconnectBlock(): failure reading undo data");
return DISCONNECT_FAILED;
}
if (blockUndo.vtxundo.size() + 1 != block.vtx.size()) if (blockUndo.vtxundo.size() + 1 != block.vtx.size()) {
return error("DisconnectBlock(): block and undo data inconsistent"); error("DisconnectBlock(): block and undo data inconsistent");
return DISCONNECT_FAILED;
}
// undo transactions in reverse order // undo transactions in reverse order
for (int i = block.vtx.size() - 1; i >= 0; i--) { for (int i = block.vtx.size() - 1; i >= 0; i--) {
@ -1576,8 +1584,10 @@ static bool DisconnectBlock(const CBlock& block, CValidationState& state, const
// restore inputs // restore inputs
if (i > 0) { // not coinbases if (i > 0) { // not coinbases
const CTxUndo &txundo = blockUndo.vtxundo[i-1]; const CTxUndo &txundo = blockUndo.vtxundo[i-1];
if (txundo.vprevout.size() != tx.vin.size()) if (txundo.vprevout.size() != tx.vin.size()) {
return error("DisconnectBlock(): transaction and undo data inconsistent"); error("DisconnectBlock(): transaction and undo data inconsistent");
return DISCONNECT_FAILED;
}
for (unsigned int j = tx.vin.size(); j-- > 0;) { for (unsigned int j = tx.vin.size(); j-- > 0;) {
const COutPoint &out = tx.vin[j].prevout; const COutPoint &out = tx.vin[j].prevout;
const CTxInUndo &undo = txundo.vprevout[j]; const CTxInUndo &undo = txundo.vprevout[j];
@ -1590,12 +1600,7 @@ static bool DisconnectBlock(const CBlock& block, CValidationState& state, const
// move best block pointer to prevout block // move best block pointer to prevout block
view.SetBestBlock(pindex->pprev->GetBlockHash()); view.SetBestBlock(pindex->pprev->GetBlockHash());
if (pfClean) { return fClean ? DISCONNECT_OK : DISCONNECT_UNCLEAN;
*pfClean = fClean;
return true;
}
return fClean;
} }
void static FlushBlockFile(bool fFinalize = false) void static FlushBlockFile(bool fFinalize = false)
@ -2131,7 +2136,7 @@ bool static DisconnectTip(CValidationState& state, const CChainParams& chainpara
int64_t nStart = GetTimeMicros(); int64_t nStart = GetTimeMicros();
{ {
CCoinsViewCache view(pcoinsTip); CCoinsViewCache view(pcoinsTip);
if (!DisconnectBlock(block, state, pindexDelete, view)) if (DisconnectBlock(block, pindexDelete, view) != DISCONNECT_OK)
return error("DisconnectTip(): DisconnectBlock %s failed", pindexDelete->GetBlockHash().ToString()); return error("DisconnectTip(): DisconnectBlock %s failed", pindexDelete->GetBlockHash().ToString());
bool flushed = view.Flush(); bool flushed = view.Flush();
assert(flushed); assert(flushed);
@ -3656,15 +3661,17 @@ bool CVerifyDB::VerifyDB(const CChainParams& chainparams, CCoinsView *coinsview,
} }
// check level 3: check for inconsistencies during memory-only disconnect of tip blocks // check level 3: check for inconsistencies during memory-only disconnect of tip blocks
if (nCheckLevel >= 3 && pindex == pindexState && (coins.DynamicMemoryUsage() + pcoinsTip->DynamicMemoryUsage()) <= nCoinCacheUsage) { if (nCheckLevel >= 3 && pindex == pindexState && (coins.DynamicMemoryUsage() + pcoinsTip->DynamicMemoryUsage()) <= nCoinCacheUsage) {
bool fClean = true; DisconnectResult res = DisconnectBlock(block, pindex, coins);
if (!DisconnectBlock(block, state, pindex, coins, &fClean)) if (res == DISCONNECT_FAILED) {
return error("VerifyDB(): *** irrecoverable inconsistency in block data at %d, hash=%s", pindex->nHeight, pindex->GetBlockHash().ToString()); return error("VerifyDB(): *** irrecoverable inconsistency in block data at %d, hash=%s", pindex->nHeight, pindex->GetBlockHash().ToString());
}
pindexState = pindex->pprev; pindexState = pindex->pprev;
if (!fClean) { if (res == DISCONNECT_UNCLEAN) {
nGoodTransactions = 0; nGoodTransactions = 0;
pindexFailure = pindex; pindexFailure = pindex;
} else } else {
nGoodTransactions += block.vtx.size(); nGoodTransactions += block.vtx.size();
}
} }
if (ShutdownRequested()) if (ShutdownRequested())
return true; return true;