From 10df6fb3dfa0a86ae96a9cca1a368cf51fc6eebb Mon Sep 17 00:00:00 2001 From: Sean Bowe Date: Mon, 13 Jun 2016 11:45:41 -0600 Subject: [PATCH 1/7] Rename `CheckInputs` to `ContextualCheckInputs` since it relies on a global variable and assumes calling conditions. --- src/main.cpp | 8 ++++---- src/main.h | 4 ++-- src/miner.cpp | 2 +- src/txmempool.cpp | 4 ++-- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index 572883f61..a16d59f8f 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -1219,7 +1219,7 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransa // 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)) + if (!ContextualCheckInputs(tx, state, view, true, STANDARD_SCRIPT_VERIFY_FLAGS, true)) { return error("AcceptToMemoryPool: ConnectInputs failed %s", hash.ToString()); } @@ -1233,7 +1233,7 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransa // There is a similar check in CreateNewBlock() to prevent creating // invalid blocks, however allowing such transactions into the mempool // can be exploited as a DoS attack. - if (!CheckInputs(tx, state, view, true, MANDATORY_SCRIPT_VERIFY_FLAGS, true)) + if (!ContextualCheckInputs(tx, state, view, true, MANDATORY_SCRIPT_VERIFY_FLAGS, true)) { return error("AcceptToMemoryPool: BUG! PLEASE REPORT THIS! ConnectInputs failed against MANDATORY but not STANDARD flags %s", hash.ToString()); } @@ -1604,7 +1604,7 @@ bool CScriptCheck::operator()() { return true; } -bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsViewCache &inputs, bool fScriptChecks, unsigned int flags, bool cacheStore, std::vector *pvChecks) +bool ContextualCheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsViewCache &inputs, bool fScriptChecks, unsigned int flags, bool cacheStore, std::vector *pvChecks) { if (!tx.IsCoinBase()) { @@ -2128,7 +2128,7 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin nFees += view.GetValueIn(tx)-tx.GetValueOut(); std::vector vChecks; - if (!CheckInputs(tx, state, view, fScriptChecks, flags, false, nScriptCheckThreads ? &vChecks : NULL)) + if (!ContextualCheckInputs(tx, state, view, fScriptChecks, flags, false, nScriptCheckThreads ? &vChecks : NULL)) return false; control.Add(vChecks); } diff --git a/src/main.h b/src/main.h index ef601e69f..9357ce9f5 100644 --- a/src/main.h +++ b/src/main.h @@ -317,8 +317,8 @@ unsigned int GetP2SHSigOpCount(const CTransaction& tx, const CCoinsViewCache& ma * This does not modify the UTXO set. If pvChecks is not NULL, script checks are pushed onto it * instead of being performed inline. */ -bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsViewCache &view, bool fScriptChecks, - unsigned int flags, bool cacheStore, std::vector *pvChecks = NULL); +bool ContextualCheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsViewCache &view, bool fScriptChecks, + unsigned int flags, bool cacheStore, std::vector *pvChecks = NULL); /** Apply the effects of this transaction on the UTXO set represented by view */ void UpdateCoins(const CTransaction& tx, CValidationState &state, CCoinsViewCache &inputs, int nHeight); diff --git a/src/miner.cpp b/src/miner.cpp index 614d97efe..475e90039 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -282,7 +282,7 @@ CBlockTemplate* CreateNewBlock(const CScript& scriptPubKeyIn) // policy here, but we still have to ensure that the block we // create only contains transactions that are valid in new blocks. CValidationState state; - if (!CheckInputs(tx, state, view, true, MANDATORY_SCRIPT_VERIFY_FLAGS, true)) + if (!ContextualCheckInputs(tx, state, view, true, MANDATORY_SCRIPT_VERIFY_FLAGS, true)) continue; UpdateCoins(tx, state, view, nHeight); diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 0a202229a..e8ed8f82f 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -327,7 +327,7 @@ void CTxMemPool::check(const CCoinsViewCache *pcoins) const waitingOnDependants.push_back(&it->second); else { CValidationState state; - assert(CheckInputs(tx, state, mempoolDuplicate, false, 0, false, NULL)); + assert(ContextualCheckInputs(tx, state, mempoolDuplicate, false, 0, false, NULL)); UpdateCoins(tx, state, mempoolDuplicate, 1000000); } } @@ -341,7 +341,7 @@ void CTxMemPool::check(const CCoinsViewCache *pcoins) const stepsSinceLastRemove++; assert(stepsSinceLastRemove < waitingOnDependants.size()); } else { - assert(CheckInputs(entry->GetTx(), state, mempoolDuplicate, false, 0, false, NULL)); + assert(ContextualCheckInputs(entry->GetTx(), state, mempoolDuplicate, false, 0, false, NULL)); UpdateCoins(entry->GetTx(), state, mempoolDuplicate, 1000000); stepsSinceLastRemove = 0; } From 2c901fd87d62d1d393af65dd1a461d30554e2955 Mon Sep 17 00:00:00 2001 From: Sean Bowe Date: Mon, 13 Jun 2016 11:52:46 -0600 Subject: [PATCH 2/7] Refactor contextual and noncontextual input checks. --- src/main.cpp | 41 +++++++++++++++++++++++++++++++---------- src/main.h | 3 +++ 2 files changed, 34 insertions(+), 10 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index a16d59f8f..09c357b01 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -1604,7 +1604,7 @@ bool CScriptCheck::operator()() { return true; } -bool ContextualCheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsViewCache &inputs, bool fScriptChecks, unsigned int flags, bool cacheStore, std::vector *pvChecks) +bool NonContextualCheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsViewCache &inputs, bool fScriptChecks, unsigned int flags, bool cacheStore, std::vector *pvChecks) { if (!tx.IsCoinBase()) { @@ -1620,10 +1620,6 @@ bool ContextualCheckInputs(const CTransaction& tx, CValidationState &state, cons if (!inputs.HavePourRequirements(tx)) return state.Invalid(error("CheckInputs(): %s pour requirements not met", tx.GetHash().ToString())); - // While checking, GetBestBlock() refers to the parent block. - // This is also true for mempool checks. - CBlockIndex *pindexPrev = mapBlockIndex.find(inputs.GetBestBlock())->second; - int nSpendHeight = pindexPrev->nHeight + 1; CAmount nValueIn = 0; CAmount nFees = 0; for (unsigned int i = 0; i < tx.vin.size(); i++) @@ -1632,12 +1628,8 @@ bool ContextualCheckInputs(const CTransaction& tx, CValidationState &state, cons const CCoins *coins = inputs.AccessCoins(prevout.hash); assert(coins); - // If prev is coinbase, check that it's matured if (coins->IsCoinBase()) { - if (nSpendHeight - coins->nHeight < COINBASE_MATURITY) - return state.Invalid( - error("CheckInputs(): tried to spend coinbase at depth %d", nSpendHeight - coins->nHeight), - REJECT_INVALID, "bad-txns-premature-spend-of-coinbase"); + } // Check for negative or overflow input values @@ -1715,6 +1707,35 @@ bool ContextualCheckInputs(const CTransaction& tx, CValidationState &state, cons return true; } +bool ContextualCheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsViewCache &inputs, bool fScriptChecks, unsigned int flags, bool cacheStore, std::vector *pvChecks) +{ + if (!tx.IsCoinBase()) + { + // While checking, GetBestBlock() refers to the parent block. + // This is also true for mempool checks. + CBlockIndex *pindexPrev = mapBlockIndex.find(inputs.GetBestBlock())->second; + int nSpendHeight = pindexPrev->nHeight + 1; + for (unsigned int i = 0; i < tx.vin.size(); i++) + { + const COutPoint &prevout = tx.vin[i].prevout; + const CCoins *coins = inputs.AccessCoins(prevout.hash); + assert(coins); + + // If prev is coinbase, check that it's matured + if (coins->IsCoinBase()) { + if (nSpendHeight - coins->nHeight < COINBASE_MATURITY) + return state.Invalid( + error("CheckInputs(): tried to spend coinbase at depth %d", nSpendHeight - coins->nHeight), + REJECT_INVALID, "bad-txns-premature-spend-of-coinbase"); + } + } + } + + return NonContextualCheckInputs( + tx, state, inputs, fScriptChecks, flags, cacheStore, pvChecks + ); +} + namespace { bool UndoWriteToDisk(const CBlockUndo& blockundo, CDiskBlockPos& pos, const uint256& hashBlock, const CMessageHeader::MessageStartChars& messageStart) diff --git a/src/main.h b/src/main.h index 9357ce9f5..f1627e621 100644 --- a/src/main.h +++ b/src/main.h @@ -320,6 +320,9 @@ unsigned int GetP2SHSigOpCount(const CTransaction& tx, const CCoinsViewCache& ma bool ContextualCheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsViewCache &view, bool fScriptChecks, unsigned int flags, bool cacheStore, std::vector *pvChecks = NULL); +bool NonContextualCheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsViewCache &view, bool fScriptChecks, + unsigned int flags, bool cacheStore, std::vector *pvChecks = NULL); + /** Apply the effects of this transaction on the UTXO set represented by view */ void UpdateCoins(const CTransaction& tx, CValidationState &state, CCoinsViewCache &inputs, int nHeight); From 1d38795f507e98391bcc62cc39f1fc712c730b53 Mon Sep 17 00:00:00 2001 From: Sean Bowe Date: Mon, 13 Jun 2016 11:58:10 -0600 Subject: [PATCH 3/7] Prevent coinbases from being spent to transparent outputs. --- src/main.cpp | 7 +++++- src/test/coins_tests.cpp | 47 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 1 deletion(-) diff --git a/src/main.cpp b/src/main.cpp index 09c357b01..72b3cabe0 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -1629,7 +1629,12 @@ bool NonContextualCheckInputs(const CTransaction& tx, CValidationState &state, c assert(coins); if (coins->IsCoinBase()) { - + // Ensure that coinbases cannot be spent to transparent outputs + if (!tx.vout.empty()) { + return state.Invalid( + error("CheckInputs(): tried to spend coinbase with transparent outputs"), + REJECT_INVALID, "bad-txns-coinbase-spend-has-transparent-outputs"); + } } // Check for negative or overflow input values diff --git a/src/test/coins_tests.cpp b/src/test/coins_tests.cpp index ce9b7eeb2..9edf432ea 100644 --- a/src/test/coins_tests.cpp +++ b/src/test/coins_tests.cpp @@ -6,6 +6,9 @@ #include "random.h" #include "uint256.h" #include "test/test_bitcoin.h" +#include "consensus/validation.h" +#include "main.h" +#include "undo.h" #include #include @@ -475,4 +478,48 @@ BOOST_AUTO_TEST_CASE(coins_cache_simulation_test) BOOST_CHECK(missed_an_entry); } +BOOST_AUTO_TEST_CASE(coins_coinbase_spends) +{ + CCoinsViewTest base; + CCoinsViewCacheTest cache(&base); + + // Create coinbase transaction + CMutableTransaction mtx; + mtx.vin.resize(1); + mtx.vin[0].scriptSig = CScript() << OP_1; + mtx.vin[0].nSequence = 0; + mtx.vout.resize(1); + mtx.vout[0].nValue = 500; + mtx.vout[0].scriptPubKey = CScript() << OP_1; + + CTransaction tx(mtx); + + BOOST_CHECK(tx.IsCoinBase()); + + CValidationState state; + UpdateCoins(tx, state, cache, 100); + + // Create coinbase spend + CMutableTransaction mtx2; + mtx2.vin.resize(1); + mtx2.vin[0].prevout = COutPoint(tx.GetHash(), 0); + mtx2.vin[0].scriptSig = CScript() << OP_1; + mtx2.vin[0].nSequence = 0; + + { + CTransaction tx2(mtx2); + BOOST_CHECK(NonContextualCheckInputs(tx2, state, cache, false, SCRIPT_VERIFY_NONE, false, NULL)); + } + + mtx2.vout.resize(1); + mtx2.vout[0].nValue = 500; + mtx2.vout[0].scriptPubKey = CScript() << OP_1; + + { + CTransaction tx2(mtx2); + BOOST_CHECK(!NonContextualCheckInputs(tx2, state, cache, false, SCRIPT_VERIFY_NONE, false, NULL)); + BOOST_CHECK(state.GetRejectReason() == "bad-txns-coinbase-spend-has-transparent-outputs"); + } +} + BOOST_AUTO_TEST_SUITE_END() From c0dde76d8a7330368d940656b8cf3f7f84f32f74 Mon Sep 17 00:00:00 2001 From: Sean Bowe Date: Mon, 13 Jun 2016 12:23:55 -0600 Subject: [PATCH 4/7] Disable coinbase-must-be-protected rule on regtest. --- src/chainparams.cpp | 2 ++ src/consensus/params.h | 3 +++ src/main.cpp | 15 ++++++++------- src/main.h | 6 ++++-- src/miner.cpp | 2 +- src/test/coins_tests.cpp | 4 ++-- src/txmempool.cpp | 4 ++-- 7 files changed, 22 insertions(+), 14 deletions(-) diff --git a/src/chainparams.cpp b/src/chainparams.cpp index 7d303c1b6..cb75808ff 100644 --- a/src/chainparams.cpp +++ b/src/chainparams.cpp @@ -31,6 +31,7 @@ class CMainParams : public CChainParams { public: CMainParams() { strNetworkID = "main"; + consensus.coinbaseMustBeProtected = true; consensus.nSubsidySlowStartInterval = 20000; consensus.nSubsidyHalvingInterval = 840000; consensus.nMajorityEnforceBlockUpgrade = 750; @@ -210,6 +211,7 @@ class CRegTestParams : public CTestNetParams { public: CRegTestParams() { strNetworkID = "regtest"; + consensus.coinbaseMustBeProtected = false; consensus.nSubsidySlowStartInterval = 0; consensus.nSubsidyHalvingInterval = 150; consensus.nMajorityEnforceBlockUpgrade = 750; diff --git a/src/consensus/params.h b/src/consensus/params.h index 1dea7c47b..8d2cb3af8 100644 --- a/src/consensus/params.h +++ b/src/consensus/params.h @@ -14,6 +14,9 @@ namespace Consensus { */ struct Params { uint256 hashGenesisBlock; + + bool coinbaseMustBeProtected; + /** Needs to evenly divide MAX_SUBSIDY to avoid rounding errors. */ int nSubsidySlowStartInterval; /** diff --git a/src/main.cpp b/src/main.cpp index 72b3cabe0..bda5d307e 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -1219,7 +1219,7 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransa // Check against previous transactions // This is done last to help prevent CPU exhaustion denial-of-service attacks. - if (!ContextualCheckInputs(tx, state, view, true, STANDARD_SCRIPT_VERIFY_FLAGS, true)) + if (!ContextualCheckInputs(tx, state, view, true, STANDARD_SCRIPT_VERIFY_FLAGS, true, Params().GetConsensus())) { return error("AcceptToMemoryPool: ConnectInputs failed %s", hash.ToString()); } @@ -1233,7 +1233,7 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransa // There is a similar check in CreateNewBlock() to prevent creating // invalid blocks, however allowing such transactions into the mempool // can be exploited as a DoS attack. - if (!ContextualCheckInputs(tx, state, view, true, MANDATORY_SCRIPT_VERIFY_FLAGS, true)) + if (!ContextualCheckInputs(tx, state, view, true, MANDATORY_SCRIPT_VERIFY_FLAGS, true, Params().GetConsensus())) { return error("AcceptToMemoryPool: BUG! PLEASE REPORT THIS! ConnectInputs failed against MANDATORY but not STANDARD flags %s", hash.ToString()); } @@ -1604,7 +1604,7 @@ bool CScriptCheck::operator()() { return true; } -bool NonContextualCheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsViewCache &inputs, bool fScriptChecks, unsigned int flags, bool cacheStore, std::vector *pvChecks) +bool NonContextualCheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsViewCache &inputs, bool fScriptChecks, unsigned int flags, bool cacheStore, const Consensus::Params& consensusParams, std::vector *pvChecks) { if (!tx.IsCoinBase()) { @@ -1630,7 +1630,8 @@ bool NonContextualCheckInputs(const CTransaction& tx, CValidationState &state, c if (coins->IsCoinBase()) { // Ensure that coinbases cannot be spent to transparent outputs - if (!tx.vout.empty()) { + // Disabled on regtest + if (consensusParams.coinbaseMustBeProtected && !tx.vout.empty()) { return state.Invalid( error("CheckInputs(): tried to spend coinbase with transparent outputs"), REJECT_INVALID, "bad-txns-coinbase-spend-has-transparent-outputs"); @@ -1712,7 +1713,7 @@ bool NonContextualCheckInputs(const CTransaction& tx, CValidationState &state, c return true; } -bool ContextualCheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsViewCache &inputs, bool fScriptChecks, unsigned int flags, bool cacheStore, std::vector *pvChecks) +bool ContextualCheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsViewCache &inputs, bool fScriptChecks, unsigned int flags, bool cacheStore, const Consensus::Params& consensusParams, std::vector *pvChecks) { if (!tx.IsCoinBase()) { @@ -1737,7 +1738,7 @@ bool ContextualCheckInputs(const CTransaction& tx, CValidationState &state, cons } return NonContextualCheckInputs( - tx, state, inputs, fScriptChecks, flags, cacheStore, pvChecks + tx, state, inputs, fScriptChecks, flags, cacheStore, consensusParams, pvChecks ); } @@ -2154,7 +2155,7 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin nFees += view.GetValueIn(tx)-tx.GetValueOut(); std::vector vChecks; - if (!ContextualCheckInputs(tx, state, view, fScriptChecks, flags, false, nScriptCheckThreads ? &vChecks : NULL)) + if (!ContextualCheckInputs(tx, state, view, fScriptChecks, flags, false, chainparams.GetConsensus(), nScriptCheckThreads ? &vChecks : NULL)) return false; control.Add(vChecks); } diff --git a/src/main.h b/src/main.h index f1627e621..df5085a96 100644 --- a/src/main.h +++ b/src/main.h @@ -318,10 +318,12 @@ unsigned int GetP2SHSigOpCount(const CTransaction& tx, const CCoinsViewCache& ma * instead of being performed inline. */ bool ContextualCheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsViewCache &view, bool fScriptChecks, - unsigned int flags, bool cacheStore, std::vector *pvChecks = NULL); + unsigned int flags, bool cacheStore, const Consensus::Params& consensusParams, + std::vector *pvChecks = NULL); bool NonContextualCheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsViewCache &view, bool fScriptChecks, - unsigned int flags, bool cacheStore, std::vector *pvChecks = NULL); + unsigned int flags, bool cacheStore, const Consensus::Params& consensusParams, + std::vector *pvChecks = NULL); /** Apply the effects of this transaction on the UTXO set represented by view */ void UpdateCoins(const CTransaction& tx, CValidationState &state, CCoinsViewCache &inputs, int nHeight); diff --git a/src/miner.cpp b/src/miner.cpp index 475e90039..2f9caf02f 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -282,7 +282,7 @@ CBlockTemplate* CreateNewBlock(const CScript& scriptPubKeyIn) // policy here, but we still have to ensure that the block we // create only contains transactions that are valid in new blocks. CValidationState state; - if (!ContextualCheckInputs(tx, state, view, true, MANDATORY_SCRIPT_VERIFY_FLAGS, true)) + if (!ContextualCheckInputs(tx, state, view, true, MANDATORY_SCRIPT_VERIFY_FLAGS, true, Params().GetConsensus())) continue; UpdateCoins(tx, state, view, nHeight); diff --git a/src/test/coins_tests.cpp b/src/test/coins_tests.cpp index 9edf432ea..8b0f7dd79 100644 --- a/src/test/coins_tests.cpp +++ b/src/test/coins_tests.cpp @@ -508,7 +508,7 @@ BOOST_AUTO_TEST_CASE(coins_coinbase_spends) { CTransaction tx2(mtx2); - BOOST_CHECK(NonContextualCheckInputs(tx2, state, cache, false, SCRIPT_VERIFY_NONE, false, NULL)); + BOOST_CHECK(NonContextualCheckInputs(tx2, state, cache, false, SCRIPT_VERIFY_NONE, false, Params().GetConsensus())); } mtx2.vout.resize(1); @@ -517,7 +517,7 @@ BOOST_AUTO_TEST_CASE(coins_coinbase_spends) { CTransaction tx2(mtx2); - BOOST_CHECK(!NonContextualCheckInputs(tx2, state, cache, false, SCRIPT_VERIFY_NONE, false, NULL)); + BOOST_CHECK(!NonContextualCheckInputs(tx2, state, cache, false, SCRIPT_VERIFY_NONE, false, Params().GetConsensus())); BOOST_CHECK(state.GetRejectReason() == "bad-txns-coinbase-spend-has-transparent-outputs"); } } diff --git a/src/txmempool.cpp b/src/txmempool.cpp index e8ed8f82f..669e35e3b 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -327,7 +327,7 @@ void CTxMemPool::check(const CCoinsViewCache *pcoins) const waitingOnDependants.push_back(&it->second); else { CValidationState state; - assert(ContextualCheckInputs(tx, state, mempoolDuplicate, false, 0, false, NULL)); + assert(ContextualCheckInputs(tx, state, mempoolDuplicate, false, 0, false, Params().GetConsensus(), NULL)); UpdateCoins(tx, state, mempoolDuplicate, 1000000); } } @@ -341,7 +341,7 @@ void CTxMemPool::check(const CCoinsViewCache *pcoins) const stepsSinceLastRemove++; assert(stepsSinceLastRemove < waitingOnDependants.size()); } else { - assert(ContextualCheckInputs(entry->GetTx(), state, mempoolDuplicate, false, 0, false, NULL)); + assert(ContextualCheckInputs(entry->GetTx(), state, mempoolDuplicate, false, 0, false, Params().GetConsensus(), NULL)); UpdateCoins(entry->GetTx(), state, mempoolDuplicate, 1000000); stepsSinceLastRemove = 0; } From 89f3cd11c76c99444b0b3d7e58bd79b6429f13ee Mon Sep 17 00:00:00 2001 From: Sean Bowe Date: Tue, 14 Jun 2016 12:41:32 -0600 Subject: [PATCH 5/7] Ensure NonContextualCheckInputs runs before routines in ContextualCheckInputs. --- src/main.cpp | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index bda5d307e..c6f310410 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -1715,6 +1715,10 @@ bool NonContextualCheckInputs(const CTransaction& tx, CValidationState &state, c bool ContextualCheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsViewCache &inputs, bool fScriptChecks, unsigned int flags, bool cacheStore, const Consensus::Params& consensusParams, std::vector *pvChecks) { + if (!NonContextualCheckInputs(tx, state, inputs, fScriptChecks, flags, cacheStore, consensusParams, pvChecks)) { + return false; + } + if (!tx.IsCoinBase()) { // While checking, GetBestBlock() refers to the parent block. @@ -1725,21 +1729,22 @@ bool ContextualCheckInputs(const CTransaction& tx, CValidationState &state, cons { const COutPoint &prevout = tx.vin[i].prevout; const CCoins *coins = inputs.AccessCoins(prevout.hash); + // Assertion is okay because NonContextualCheckInputs ensures the inputs + // are available. assert(coins); // If prev is coinbase, check that it's matured if (coins->IsCoinBase()) { - if (nSpendHeight - coins->nHeight < COINBASE_MATURITY) + if (nSpendHeight - coins->nHeight < COINBASE_MATURITY) { return state.Invalid( error("CheckInputs(): tried to spend coinbase at depth %d", nSpendHeight - coins->nHeight), REJECT_INVALID, "bad-txns-premature-spend-of-coinbase"); + } } } } - return NonContextualCheckInputs( - tx, state, inputs, fScriptChecks, flags, cacheStore, consensusParams, pvChecks - ); + return true; } namespace { From a180d0a6c687c48eb4865bae9fde40e62e16ac9b Mon Sep 17 00:00:00 2001 From: Sean Bowe Date: Tue, 14 Jun 2016 15:16:34 -0600 Subject: [PATCH 6/7] Rename to `fCoinbaseMustBeProtected`. --- src/chainparams.cpp | 4 ++-- src/consensus/params.h | 2 +- src/main.cpp | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/chainparams.cpp b/src/chainparams.cpp index cb75808ff..68dbe994f 100644 --- a/src/chainparams.cpp +++ b/src/chainparams.cpp @@ -31,7 +31,7 @@ class CMainParams : public CChainParams { public: CMainParams() { strNetworkID = "main"; - consensus.coinbaseMustBeProtected = true; + consensus.fCoinbaseMustBeProtected = true; consensus.nSubsidySlowStartInterval = 20000; consensus.nSubsidyHalvingInterval = 840000; consensus.nMajorityEnforceBlockUpgrade = 750; @@ -211,7 +211,7 @@ class CRegTestParams : public CTestNetParams { public: CRegTestParams() { strNetworkID = "regtest"; - consensus.coinbaseMustBeProtected = false; + consensus.fCoinbaseMustBeProtected = false; consensus.nSubsidySlowStartInterval = 0; consensus.nSubsidyHalvingInterval = 150; consensus.nMajorityEnforceBlockUpgrade = 750; diff --git a/src/consensus/params.h b/src/consensus/params.h index 8d2cb3af8..07e945256 100644 --- a/src/consensus/params.h +++ b/src/consensus/params.h @@ -15,7 +15,7 @@ namespace Consensus { struct Params { uint256 hashGenesisBlock; - bool coinbaseMustBeProtected; + bool fCoinbaseMustBeProtected; /** Needs to evenly divide MAX_SUBSIDY to avoid rounding errors. */ int nSubsidySlowStartInterval; diff --git a/src/main.cpp b/src/main.cpp index c6f310410..ebedca949 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -1631,7 +1631,7 @@ bool NonContextualCheckInputs(const CTransaction& tx, CValidationState &state, c if (coins->IsCoinBase()) { // Ensure that coinbases cannot be spent to transparent outputs // Disabled on regtest - if (consensusParams.coinbaseMustBeProtected && !tx.vout.empty()) { + if (consensusParams.fCoinbaseMustBeProtected && !tx.vout.empty()) { return state.Invalid( error("CheckInputs(): tried to spend coinbase with transparent outputs"), REJECT_INVALID, "bad-txns-coinbase-spend-has-transparent-outputs"); From d212ba320b5efdf1ae9d90ab6d4fdf0fd5229dc6 Mon Sep 17 00:00:00 2001 From: Sean Bowe Date: Tue, 14 Jun 2016 15:18:52 -0600 Subject: [PATCH 7/7] Disable enforced coinbase protection in miner_tests. --- src/main.cpp | 5 ++++- src/main.h | 3 +++ src/test/miner_tests.cpp | 2 ++ 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/main.cpp b/src/main.cpp index ebedca949..ff129ae3f 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -62,6 +62,7 @@ bool fPruneMode = false; bool fIsBareMultisigStd = true; bool fCheckBlockIndex = false; bool fCheckpointsEnabled = true; +bool fCoinbaseEnforcedProtectionEnabled = true; size_t nCoinCacheUsage = 5000 * 300; uint64_t nPruneTarget = 0; bool fAlerts = DEFAULT_ALERTS; @@ -1631,7 +1632,9 @@ bool NonContextualCheckInputs(const CTransaction& tx, CValidationState &state, c if (coins->IsCoinBase()) { // Ensure that coinbases cannot be spent to transparent outputs // Disabled on regtest - if (consensusParams.fCoinbaseMustBeProtected && !tx.vout.empty()) { + if (fCoinbaseEnforcedProtectionEnabled && + consensusParams.fCoinbaseMustBeProtected && + !tx.vout.empty()) { return state.Invalid( error("CheckInputs(): tried to spend coinbase with transparent outputs"), REJECT_INVALID, "bad-txns-coinbase-spend-has-transparent-outputs"); diff --git a/src/main.h b/src/main.h index df5085a96..088c58be5 100644 --- a/src/main.h +++ b/src/main.h @@ -116,6 +116,9 @@ extern bool fTxIndex; extern bool fIsBareMultisigStd; extern bool fCheckBlockIndex; extern bool fCheckpointsEnabled; +// TODO: remove this flag by structuring our code such that +// it is unneeded for testing +extern bool fCoinbaseEnforcedProtectionEnabled; extern size_t nCoinCacheUsage; extern CFeeRate minRelayTxFee; extern bool fAlerts; diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp index a657fe72e..a10abacd1 100644 --- a/src/test/miner_tests.cpp +++ b/src/test/miner_tests.cpp @@ -147,6 +147,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) LOCK(cs_main); fCheckpointsEnabled = false; + fCoinbaseEnforcedProtectionEnabled = false; // We can't make transactions until we have inputs // Therefore, load 100 blocks :) @@ -417,6 +418,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) delete tx; fCheckpointsEnabled = true; + fCoinbaseEnforcedProtectionEnabled = true; } BOOST_AUTO_TEST_SUITE_END()