diff --git a/src/chainparams.cpp b/src/chainparams.cpp index f9729e485..1e45243ae 100644 --- a/src/chainparams.cpp +++ b/src/chainparams.cpp @@ -31,6 +31,7 @@ class CMainParams : public CChainParams { public: CMainParams() { strNetworkID = "main"; + consensus.fCoinbaseMustBeProtected = true; consensus.nSubsidySlowStartInterval = 20000; consensus.nSubsidyHalvingInterval = 840000; consensus.nMajorityEnforceBlockUpgrade = 750; @@ -210,6 +211,7 @@ class CRegTestParams : public CTestNetParams { public: CRegTestParams() { strNetworkID = "regtest"; + 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 1dea7c47b..07e945256 100644 --- a/src/consensus/params.h +++ b/src/consensus/params.h @@ -14,6 +14,9 @@ namespace Consensus { */ struct Params { uint256 hashGenesisBlock; + + 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 17a929891..ef3d8a8bf 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; @@ -1219,7 +1220,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, Params().GetConsensus())) { return error("AcceptToMemoryPool: ConnectInputs failed %s", hash.ToString()); } @@ -1233,7 +1234,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, Params().GetConsensus())) { return error("AcceptToMemoryPool: BUG! PLEASE REPORT THIS! ConnectInputs failed against MANDATORY but not STANDARD flags %s", hash.ToString()); } @@ -1604,7 +1605,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 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()) { @@ -1620,10 +1621,6 @@ bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsVi 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 +1629,16 @@ bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsVi 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) + // Ensure that coinbases cannot be spent to transparent outputs + // Disabled on regtest + if (fCoinbaseEnforcedProtectionEnabled && + consensusParams.fCoinbaseMustBeProtected && + !tx.vout.empty()) { return state.Invalid( - error("CheckInputs(): tried to spend coinbase at depth %d", nSpendHeight - coins->nHeight), - REJECT_INVALID, "bad-txns-premature-spend-of-coinbase"); + 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 @@ -1715,6 +1716,40 @@ bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsVi return true; } +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. + // 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); + // 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) { + return state.Invalid( + error("CheckInputs(): tried to spend coinbase at depth %d", nSpendHeight - coins->nHeight), + REJECT_INVALID, "bad-txns-premature-spend-of-coinbase"); + } + } + } + } + + return true; +} + namespace { bool UndoWriteToDisk(const CBlockUndo& blockundo, CDiskBlockPos& pos, const uint256& hashBlock, const CMessageHeader::MessageStartChars& messageStart) @@ -2106,7 +2141,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, chainparams.GetConsensus(), nScriptCheckThreads ? &vChecks : NULL)) return false; control.Add(vChecks); } diff --git a/src/main.h b/src/main.h index ef601e69f..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; @@ -317,8 +320,13 @@ 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, 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, 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 614d97efe..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 (!CheckInputs(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 ce9b7eeb2..8b0f7dd79 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, Params().GetConsensus())); + } + + 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, Params().GetConsensus())); + BOOST_CHECK(state.GetRejectReason() == "bad-txns-coinbase-spend-has-transparent-outputs"); + } +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp index 48a8108dc..15d1224cb 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 :) @@ -415,6 +416,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) delete tx; fCheckpointsEnabled = true; + fCoinbaseEnforcedProtectionEnabled = true; } BOOST_AUTO_TEST_SUITE_END() diff --git a/src/txmempool.cpp b/src/txmempool.cpp index edffcec78..c3f6c829f 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -341,7 +341,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, Params().GetConsensus(), NULL)); UpdateCoins(tx, state, mempoolDuplicate, 1000000); } } @@ -355,7 +355,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, Params().GetConsensus(), NULL)); UpdateCoins(entry->GetTx(), state, mempoolDuplicate, 1000000); stepsSinceLastRemove = 0; }