Skip to content

Consensus: Remove calls to error() and FormatStateMessage() #7287

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
laanwj merged 1 commit into from
Feb 1, 2016
Merged

Consensus: Remove calls to error() and FormatStateMessage() #7287

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 32 additions & 51 deletions src/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -814,12 +814,13 @@ bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState &state, const C
bool* pfMissingInputs, bool fOverrideMempoolLimit, bool fRejectAbsurdFee,
std::vector<uint256>& vHashTxnToUncache)
{
const uint256 hash = tx.GetHash();
AssertLockHeld(cs_main);
if (pfMissingInputs)
*pfMissingInputs = false;

if (!CheckTransaction(tx, state))
return false;
return error("%s: CheckTransaction: %s, %s", __func__, hash.ToString(), FormatStateMessage(state));

// Coinbase is only valid in a block, not as a loose transaction
if (tx.IsCoinBase())
Expand All @@ -837,7 +838,6 @@ bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState &state, const C
return state.DoS(0, false, REJECT_NONSTANDARD, "non-final");

// is it already in the memory pool?
uint256 hash = tx.GetHash();
if (pool.exists(hash))
return state.Invalid(false, REJECT_ALREADY_KNOWN, "txn-already-in-mempool");

Expand Down Expand Up @@ -1170,7 +1170,7 @@ bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState &state, const C
// Check against previous transactions
// This is done last to help prevent CPU exhaustion denial-of-service attacks.
if (!CheckInputs(tx, state, view, true, STANDARD_SCRIPT_VERIFY_FLAGS, true))
return false;
return error("%s: CheckInputs: %s, %s", __func__, hash.ToString(), FormatStateMessage(state));

// Check again against just the consensus-critical mandatory script
// verification flags, in case of bugs in the standard flags that cause
Expand Down Expand Up @@ -1964,7 +1964,7 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin

// Check it again in case a previous version let a bad block in
if (!CheckBlock(block, state, !fJustCheck, !fJustCheck))
return false;
return error("%s: Consensus::CheckBlock: %s", __func__, FormatStateMessage(state));

// verify that the view's current state corresponds to the previous block
uint256 hashPrevBlock = pindex->pprev == NULL ? uint256() : pindex->pprev->GetBlockHash();
Expand Down Expand Up @@ -2909,13 +2909,11 @@ bool CheckBlockHeader(const CBlockHeader& block, CValidationState& state, bool f
{
// Check proof of work matches claimed amount
if (fCheckPOW && !CheckProofOfWork(block.GetHash(), block.nBits, Params().GetConsensus()))
return state.DoS(50, error("CheckBlockHeader(): proof of work failed"),
REJECT_INVALID, "high-hash");
return state.DoS(50, false, REJECT_INVALID, "high-hash", false, "proof of work failed");

// Check timestamp
if (block.GetBlockTime() > GetAdjustedTime() + 2 * 60 * 60)
return state.Invalid(error("CheckBlockHeader(): block timestamp too far in the future"),
REJECT_INVALID, "time-too-new");
return state.Invalid(false, REJECT_INVALID, "time-too-new", "block timestamp too far in the future");

return true;
}
Expand All @@ -2937,15 +2935,13 @@ bool CheckBlock(const CBlock& block, CValidationState& state, bool fCheckPOW, bo
bool mutated;
uint256 hashMerkleRoot2 = BlockMerkleRoot(block, &mutated);
if (block.hashMerkleRoot != hashMerkleRoot2)
return state.DoS(100, error("CheckBlock(): hashMerkleRoot mismatch"),
REJECT_INVALID, "bad-txnmrklroot", true);
return state.DoS(100, false, REJECT_INVALID, "bad-txnmrklroot", true, "hashMerkleRoot mismatch");

// Check for merkle tree malleability (CVE-2012-2459): repeating sequences
// of transactions in a block without affecting the merkle root of a block,
// while still invalidating it.
if (mutated)
return state.DoS(100, error("CheckBlock(): duplicate transaction"),
REJECT_INVALID, "bad-txns-duplicate", true);
return state.DoS(100, false, REJECT_INVALID, "bad-txns-duplicate", true, "duplicate transaction");
}

// All potential-corruption validation must be done before we do any
Expand All @@ -2954,33 +2950,28 @@ bool CheckBlock(const CBlock& block, CValidationState& state, bool fCheckPOW, bo

// Size limits
if (block.vtx.empty() || block.vtx.size() > MAX_BLOCK_SIZE || ::GetSerializeSize(block, SER_NETWORK, PROTOCOL_VERSION) > MAX_BLOCK_SIZE)
return state.DoS(100, error("CheckBlock(): size limits failed"),
REJECT_INVALID, "bad-blk-length");
return state.DoS(100, false, REJECT_INVALID, "bad-blk-length", false, "size limits failed");

// First transaction must be coinbase, the rest must not be
if (block.vtx.empty() || !block.vtx[0].IsCoinBase())
return state.DoS(100, error("CheckBlock(): first tx is not coinbase"),
REJECT_INVALID, "bad-cb-missing");
return state.DoS(100, false, REJECT_INVALID, "bad-cb-missing", false, "first tx is not coinbase");
for (unsigned int i = 1; i < block.vtx.size(); i++)
if (block.vtx[i].IsCoinBase())
return state.DoS(100, error("CheckBlock(): more than one coinbase"),
REJECT_INVALID, "bad-cb-multiple");
return state.DoS(100, false, REJECT_INVALID, "bad-cb-multiple", false, "more than one coinbase");

// Check transactions
BOOST_FOREACH(const CTransaction& tx, block.vtx)
if (!CheckTransaction(tx, state))
return error("CheckBlock(): CheckTransaction of %s failed with %s",
tx.GetHash().ToString(),
FormatStateMessage(state));
return state.Invalid(false, state.GetRejectCode(), state.GetRejectReason(),
strprintf("Transaction check failed (tx hash %s) %s", tx.GetHash().ToString(), state.GetDebugMessage()));

unsigned int nSigOps = 0;
BOOST_FOREACH(const CTransaction& tx, block.vtx)
{
nSigOps += GetLegacySigOpCount(tx);
}
if (nSigOps > MAX_BLOCK_SIGOPS)
return state.DoS(100, error("CheckBlock(): out-of-bounds SigOpCount"),
REJECT_INVALID, "bad-blk-sigops");
return state.DoS(100, false, REJECT_INVALID, "bad-blk-sigops", false, "out-of-bounds SigOpCount");

if (fCheckPOW && fCheckMerkleRoot)
block.fChecked = true;
Expand All @@ -3007,28 +2998,17 @@ bool ContextualCheckBlockHeader(const CBlockHeader& block, CValidationState& sta
const Consensus::Params& consensusParams = Params().GetConsensus();
// Check proof of work
if (block.nBits != GetNextWorkRequired(pindexPrev, &block, consensusParams))
return state.DoS(100, error("%s: incorrect proof of work", __func__),
REJECT_INVALID, "bad-diffbits");
return state.DoS(100, false, REJECT_INVALID, "bad-diffbits", false, "incorrect proof of work");

// Check timestamp against prev
if (block.GetBlockTime() <= pindexPrev->GetMedianTimePast())
return state.Invalid(error("%s: block's timestamp is too early", __func__),
REJECT_INVALID, "time-too-old");

// Reject block.nVersion=1 blocks when 95% (75% on testnet) of the network has upgraded:
if (block.nVersion < 2 && IsSuperMajority(2, pindexPrev, consensusParams.nMajorityRejectBlockOutdated, consensusParams))
return state.Invalid(error("%s: rejected nVersion=1 block", __func__),
REJECT_OBSOLETE, "bad-version");
return state.Invalid(false, REJECT_INVALID, "time-too-old", "block's timestamp is too early");

// Reject block.nVersion=2 blocks when 95% (75% on testnet) of the network has upgraded:
if (block.nVersion < 3 && IsSuperMajority(3, pindexPrev, consensusParams.nMajorityRejectBlockOutdated, consensusParams))
return state.Invalid(error("%s: rejected nVersion=2 block", __func__),
REJECT_OBSOLETE, "bad-version");

// Reject block.nVersion=3 blocks when 95% (75% on testnet) of the network has upgraded:
if (block.nVersion < 4 && IsSuperMajority(4, pindexPrev, consensusParams.nMajorityRejectBlockOutdated, consensusParams))
return state.Invalid(error("%s : rejected nVersion=3 block", __func__),
REJECT_OBSOLETE, "bad-version");
// Reject outdated version blocks when 95% (75% on testnet) of the network has upgraded:
for (int32_t version = 2; version < 5; ++version) // check for version 2, 3 and 4 upgrades
if (block.nVersion < version && IsSuperMajority(version, pindexPrev, consensusParams.nMajorityRejectBlockOutdated, consensusParams))
return state.Invalid(false, REJECT_OBSOLETE, strprintf("bad-version(v%d)", version - 1),
strprintf("rejected nVersion=%d block", version - 1));

return true;
}
Expand All @@ -3045,7 +3025,7 @@ bool ContextualCheckBlock(const CBlock& block, CValidationState& state, CBlockIn
? pindexPrev->GetMedianTimePast()
: block.GetBlockTime();
if (!IsFinalTx(tx, nHeight, nLockTimeCutoff)) {
return state.DoS(10, error("%s: contains a non-final transaction", __func__), REJECT_INVALID, "bad-txns-nonfinal");
return state.DoS(10, false, REJECT_INVALID, "bad-txns-nonfinal", false, "non-final transaction");
}
}

Expand All @@ -3056,7 +3036,7 @@ bool ContextualCheckBlock(const CBlock& block, CValidationState& state, CBlockIn
CScript expect = CScript() << nHeight;
if (block.vtx[0].vin[0].scriptSig.size() < expect.size() ||
!std::equal(expect.begin(), expect.end(), block.vtx[0].vin[0].scriptSig.begin())) {
return state.DoS(100, error("%s: block height mismatch in coinbase", __func__), REJECT_INVALID, "bad-cb-height");
return state.DoS(100, false, REJECT_INVALID, "bad-cb-height", false, "block height mismatch in coinbase");
}
}

Expand All @@ -3083,7 +3063,7 @@ static bool AcceptBlockHeader(const CBlockHeader& block, CValidationState& state
}

if (!CheckBlockHeader(block, state))
return false;
return error("%s: Consensus::CheckBlockHeader: %s, %s", __func__, hash.ToString(), FormatStateMessage(state));

// Get prev block index
CBlockIndex* pindexPrev = NULL;
Expand All @@ -3099,7 +3079,7 @@ static bool AcceptBlockHeader(const CBlockHeader& block, CValidationState& state
return error("%s: CheckIndexAgainstCheckpoint(): %s", __func__, state.GetRejectReason().c_str());

if (!ContextualCheckBlockHeader(block, state, pindexPrev))
return false;
return error("%s: Consensus::ContextualCheckBlockHeader: %s, %s", __func__, hash.ToString(), FormatStateMessage(state));
}
if (pindex == NULL)
pindex = AddToBlockIndex(block);
Expand Down Expand Up @@ -3146,7 +3126,7 @@ static bool AcceptBlock(const CBlock& block, CValidationState& state, const CCha
pindex->nStatus |= BLOCK_FAILED_VALID;
setDirtyBlockIndex.insert(pindex);
}
return false;
return error("%s: %s", __func__, FormatStateMessage(state));
}

int nHeight = pindex->nHeight;
Expand Down Expand Up @@ -3197,7 +3177,7 @@ bool ProcessNewBlock(CValidationState& state, const CChainParams& chainparams, c
bool fRequested = MarkBlockAsReceived(pblock->GetHash());
fRequested |= fForceProcessing;
if (!checked) {
return error("%s: CheckBlock FAILED", __func__);
return error("%s: CheckBlock FAILED %s", __func__, FormatStateMessage(state));
}

// Store to disk
Expand Down Expand Up @@ -3231,11 +3211,11 @@ bool TestBlockValidity(CValidationState& state, const CChainParams& chainparams,

// NOTE: CheckBlockHeader is called by CheckBlock
if (!ContextualCheckBlockHeader(block, state, pindexPrev))
return false;
return error("%s: Consensus::ContextualCheckBlockHeader: %s", __func__, FormatStateMessage(state));
if (!CheckBlock(block, state, fCheckPOW, fCheckMerkleRoot))
return false;
return error("%s: Consensus::CheckBlock: %s", __func__, FormatStateMessage(state));
if (!ContextualCheckBlock(block, state, pindexPrev))
return false;
return error("%s: Consensus::ContextualCheckBlock: %s", __func__, FormatStateMessage(state));
if (!ConnectBlock(block, state, &indexDummy, viewNew, true))
return false;
assert(state.IsValid());
Expand Down Expand Up @@ -3565,7 +3545,8 @@ bool CVerifyDB::VerifyDB(const CChainParams& chainparams, CCoinsView *coinsview,
return error("VerifyDB(): *** ReadBlockFromDisk failed at %d, hash=%s", pindex->nHeight, pindex->GetBlockHash().ToString());
Copy link
Member

@laanwj laanwj Jan 29, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're replacing the VerifyDB() with func below, but not here

Copy link
Contributor Author

@jtimon jtimon Jan 29, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes and neither do I in the other 5 occurrences of the string "VerifyDB():" in this function. The goal of this PR is not to fix that. I only do it in the line below because I had to touch that line anyway. But I'd rather not cause more conflicts than necessary.
Of course somebody else can cleanup the whole function (or the whole file) in some other PR (preferably after #7310 ).

Copy link
Member

@laanwj laanwj Jan 29, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, just wondered if there was a reason for doing this change only in one place, but that seems a good reason.

// check level 1: verify block validity
if (nCheckLevel >= 1 && !CheckBlock(block, state))
return error("VerifyDB(): *** found bad block at %d, hash=%s\n", pindex->nHeight, pindex->GetBlockHash().ToString());
return error("%s: *** found bad block at %d, hash=%s (%s)\n", __func__,
pindex->nHeight, pindex->GetBlockHash().ToString(), FormatStateMessage(state));
// check level 2: verify undo validity
if (nCheckLevel >= 2 && pindex) {
CBlockUndo undo;
Expand Down