Make CreateNewBlock more robust to coding errors

If a coding bug of some sort let a bad transaction enter the memory pool,
CreateNewBlock would throw a runtime_error which would cause bitcoind to
crash.

It is better for miners if CreateNewBlock is robust against programming errors,
and this pull request makes it more robust. If an invalid block is created,
all of the transactions that went into the block are removed from the memory pool
and then CreateNewBlock is tried again.
This commit is contained in:
Gavin Andresen 2016-03-01 12:45:43 -05:00
parent aefab22579
commit 1c06a9b03c
No known key found for this signature in database
GPG key ID: 7588242FBE38D3A8
2 changed files with 42 additions and 10 deletions

View file

@ -111,6 +111,7 @@ CBlockTemplate* CreateNewBlock(const CChainParams& chainparams, const CScript& s
unsigned int nBlockSigOps = 100;
int lastFewTxs = 0;
CAmount nFees = 0;
bool fCreatedValidBlock = false;
{
LOCK2(cs_main, mempool.cs);
@ -303,11 +304,31 @@ CBlockTemplate* CreateNewBlock(const CChainParams& chainparams, const CScript& s
pblocktemplate->vTxSigOps[0] = GetLegacySigOpCount(pblock->vtx[0]);
CValidationState state;
if (!TestBlockValidity(state, chainparams, *pblock, pindexPrev, false, false)) {
throw std::runtime_error(strprintf("%s: TestBlockValidity failed: %s", __func__, FormatStateMessage(state)));
fCreatedValidBlock = TestBlockValidity(state, chainparams, *pblock, pindexPrev, false, false);
if (!fCreatedValidBlock) {
if (pblock->vtx.size() <= 1) {
// This should REALLY never happen! Empty block that is invalid.
throw std::runtime_error(strprintf("%s: TestBlockValidity failed: %s",
__func__, FormatStateMessage(state)));
}
// This should also never happen... but if an invalid transaction somehow entered
// the mempool due to a bug, remove all the transactions in the block
// and try again (it is not worth trying to figure out which transaction(s)
// are causing the block to be invalid).
LogPrintf("%s: TestBlockValidity failed: %s, retrying with smaller mempool",
__func__, FormatStateMessage(state));
std::list<CTransaction> unused;
BOOST_REVERSE_FOREACH(const CTransaction& tx, pblock->vtx) {
mempool.remove(tx, unused, true);
}
}
}
if (!fCreatedValidBlock) {
pblocktemplate.reset();
return CreateNewBlock(chainparams, scriptPubKeyIn); // recurse with smaller mempool
}
return pblocktemplate.release();
}

View file

@ -124,7 +124,10 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity)
mempool.addUnchecked(hash, entry.Fee(1000000).Time(GetTime()).SpendsCoinbase(spendsCoinbase).FromTx(tx));
tx.vin[0].prevout.hash = hash;
}
BOOST_CHECK_THROW(CreateNewBlock(chainparams, scriptPubKey), std::runtime_error);
BOOST_CHECK(pblocktemplate = CreateNewBlock(chainparams, scriptPubKey));
BOOST_CHECK(!mempool.exists(hash));
BOOST_CHECK_EQUAL(pblocktemplate->block.vtx.size(), 1);
delete pblocktemplate;
mempool.clear();
tx.vin[0].prevout.hash = txFirst[0]->GetHash();
@ -139,6 +142,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity)
tx.vin[0].prevout.hash = hash;
}
BOOST_CHECK(pblocktemplate = CreateNewBlock(chainparams, scriptPubKey));
BOOST_CHECK(pblocktemplate->block.vtx.size() > 1);
delete pblocktemplate;
mempool.clear();
@ -163,10 +167,12 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity)
delete pblocktemplate;
mempool.clear();
// orphan in mempool, template creation fails
// orphan in mempool not mined
hash = tx.GetHash();
mempool.addUnchecked(hash, entry.Fee(1000000).Time(GetTime()).FromTx(tx));
BOOST_CHECK_THROW(CreateNewBlock(chainparams, scriptPubKey), std::runtime_error);
BOOST_CHECK(pblocktemplate = CreateNewBlock(chainparams, scriptPubKey));
BOOST_CHECK_EQUAL(pblocktemplate->block.vtx.size(), 1);
delete pblocktemplate;
mempool.clear();
// child with higher priority than parent
@ -195,10 +201,12 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity)
hash = tx.GetHash();
// give it a fee so it'll get mined
mempool.addUnchecked(hash, entry.Fee(100000).Time(GetTime()).SpendsCoinbase(false).FromTx(tx));
BOOST_CHECK_THROW(CreateNewBlock(chainparams, scriptPubKey), std::runtime_error);
BOOST_CHECK(pblocktemplate = CreateNewBlock(chainparams, scriptPubKey));
BOOST_CHECK_EQUAL(pblocktemplate->block.vtx.size(), 1);
delete pblocktemplate;
mempool.clear();
// invalid (pre-p2sh) txn in mempool, template creation fails
// invalid (pre-p2sh) txn in mempool, don't mine
tx.vin[0].prevout.hash = txFirst[0]->GetHash();
tx.vin[0].prevout.n = 0;
tx.vin[0].scriptSig = CScript() << OP_1;
@ -212,10 +220,11 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity)
tx.vout[0].nValue -= 1000000;
hash = tx.GetHash();
mempool.addUnchecked(hash, entry.Fee(1000000).Time(GetTime()).SpendsCoinbase(false).FromTx(tx));
BOOST_CHECK_THROW(CreateNewBlock(chainparams, scriptPubKey), std::runtime_error);
BOOST_CHECK(pblocktemplate = CreateNewBlock(chainparams, scriptPubKey));
BOOST_CHECK_EQUAL(pblocktemplate->block.vtx.size(), 1); // Just coinbase
mempool.clear();
// double spend txn pair in mempool, template creation fails
// double spend txn pair in mempool, don't mine
tx.vin[0].prevout.hash = txFirst[0]->GetHash();
tx.vin[0].scriptSig = CScript() << OP_1;
tx.vout[0].nValue = 4900000000LL;
@ -225,7 +234,9 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity)
tx.vout[0].scriptPubKey = CScript() << OP_2;
hash = tx.GetHash();
mempool.addUnchecked(hash, entry.Fee(100000000L).Time(GetTime()).SpendsCoinbase(true).FromTx(tx));
BOOST_CHECK_THROW(CreateNewBlock(chainparams, scriptPubKey), std::runtime_error);
BOOST_CHECK(pblocktemplate = CreateNewBlock(chainparams, scriptPubKey));
BOOST_CHECK_EQUAL(pblocktemplate->block.vtx.size(), 1); // Just coinbase
delete pblocktemplate;
mempool.clear();
// subsidy changing