From a27a2957ed9afbe5a96caa5f0f4cbec730d27460 Mon Sep 17 00:00:00 2001 From: John Newbery Date: Thu, 24 Oct 2019 11:35:42 -0400 Subject: [validation] Add CValidationState subclasses Split CValidationState into TxValidationState and BlockValidationState to store validation results for transactions and blocks respectively. --- src/bench/block_assemble.cpp | 2 +- src/bench/checkblock.cpp | 2 +- src/bench/duplicate_inputs.cpp | 2 +- src/blockencodings.cpp | 4 +- src/consensus/tx_check.cpp | 20 +-- src/consensus/tx_check.h | 4 +- src/consensus/tx_verify.cpp | 12 +- src/consensus/tx_verify.h | 4 +- src/consensus/validation.h | 129 ++++++++++------- src/init.cpp | 2 +- src/interfaces/chain.h | 1 - src/miner.cpp | 2 +- src/net_processing.cpp | 107 ++++++++------ src/net_processing.h | 2 +- src/node/transaction.cpp | 2 +- src/rpc/blockchain.cpp | 6 +- src/rpc/mining.cpp | 10 +- src/rpc/rawtransaction.cpp | 2 +- src/test/blockfilter_index_tests.cpp | 2 +- src/test/fuzz/transaction.cpp | 2 +- src/test/setup_common.cpp | 2 +- src/test/sighash_tests.cpp | 2 +- src/test/transaction_tests.cpp | 6 +- src/test/txvalidation_tests.cpp | 4 +- src/test/txvalidationcache_tests.cpp | 14 +- src/test/validation_block_tests.cpp | 4 +- src/txmempool.cpp | 4 +- src/util/validation.cpp | 4 +- src/util/validation.h | 6 +- src/validation.cpp | 272 +++++++++++++++++------------------ src/validation.h | 39 ++--- src/validationinterface.cpp | 4 +- src/validationinterface.h | 8 +- 33 files changed, 363 insertions(+), 323 deletions(-) (limited to 'src') diff --git a/src/bench/block_assemble.cpp b/src/bench/block_assemble.cpp index 157f936a95..a3a44e5dbc 100644 --- a/src/bench/block_assemble.cpp +++ b/src/bench/block_assemble.cpp @@ -38,7 +38,7 @@ static void AssembleBlock(benchmark::State& state) LOCK(::cs_main); // Required for ::AcceptToMemoryPool. for (const auto& txr : txs) { - CValidationState state; + TxValidationState state; bool ret{::AcceptToMemoryPool(::mempool, state, txr, nullptr /* pfMissingInputs */, nullptr /* plTxnReplaced */, false /* bypass_limits */, /* nAbsurdFee */ 0)}; assert(ret); } diff --git a/src/bench/checkblock.cpp b/src/bench/checkblock.cpp index 4b13381e16..edf43bd4dc 100644 --- a/src/bench/checkblock.cpp +++ b/src/bench/checkblock.cpp @@ -42,7 +42,7 @@ static void DeserializeAndCheckBlockTest(benchmark::State& state) bool rewound = stream.Rewind(benchmark::data::block413567.size()); assert(rewound); - CValidationState validationState; + BlockValidationState validationState; bool checked = CheckBlock(block, validationState, chainParams->GetConsensus()); assert(checked); } diff --git a/src/bench/duplicate_inputs.cpp b/src/bench/duplicate_inputs.cpp index 6cfa3750d6..a783370b4e 100644 --- a/src/bench/duplicate_inputs.cpp +++ b/src/bench/duplicate_inputs.cpp @@ -54,7 +54,7 @@ static void DuplicateInputs(benchmark::State& state) block.hashMerkleRoot = BlockMerkleRoot(block); while (state.KeepRunning()) { - CValidationState cvstate{}; + BlockValidationState cvstate{}; assert(!CheckBlock(block, cvstate, chainparams.GetConsensus(), false, false)); assert(cvstate.GetRejectReason() == "bad-txns-inputs-duplicate"); } diff --git a/src/blockencodings.cpp b/src/blockencodings.cpp index f0fcf675eb..bf13297582 100644 --- a/src/blockencodings.cpp +++ b/src/blockencodings.cpp @@ -197,13 +197,13 @@ ReadStatus PartiallyDownloadedBlock::FillBlock(CBlock& block, const std::vector< if (vtx_missing.size() != tx_missing_offset) return READ_STATUS_INVALID; - CValidationState state; + BlockValidationState state; if (!CheckBlock(block, state, Params().GetConsensus())) { // TODO: We really want to just check merkle tree manually here, // but that is expensive, and CheckBlock caches a block's // "checked-status" (in the CBlock?). CBlock should be able to // check its own merkle root and cache that check. - if (state.GetReason() == ValidationInvalidReason::BLOCK_MUTATED) + if (state.GetResult() == BlockValidationResult::BLOCK_MUTATED) return READ_STATUS_FAILED; // Possible Short ID collision return READ_STATUS_CHECKBLOCK_FAILED; } diff --git a/src/consensus/tx_check.cpp b/src/consensus/tx_check.cpp index 6793f871cf..a9d5cec4ac 100644 --- a/src/consensus/tx_check.cpp +++ b/src/consensus/tx_check.cpp @@ -7,28 +7,28 @@ #include #include -bool CheckTransaction(const CTransaction& tx, CValidationState& state) +bool CheckTransaction(const CTransaction& tx, TxValidationState& state) { // Basic checks that don't depend on any context if (tx.vin.empty()) - return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-txns-vin-empty"); + return state.Invalid(TxValidationResult::TX_CONSENSUS, false, "bad-txns-vin-empty"); if (tx.vout.empty()) - return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-txns-vout-empty"); + return state.Invalid(TxValidationResult::TX_CONSENSUS, false, "bad-txns-vout-empty"); // Size limits (this doesn't take the witness into account, as that hasn't been checked for malleability) if (::GetSerializeSize(tx, PROTOCOL_VERSION | SERIALIZE_TRANSACTION_NO_WITNESS) * WITNESS_SCALE_FACTOR > MAX_BLOCK_WEIGHT) - return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-txns-oversize"); + return state.Invalid(TxValidationResult::TX_CONSENSUS, false, "bad-txns-oversize"); // Check for negative or overflow output values (see CVE-2010-5139) CAmount nValueOut = 0; for (const auto& txout : tx.vout) { if (txout.nValue < 0) - return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-txns-vout-negative"); + return state.Invalid(TxValidationResult::TX_CONSENSUS, false, "bad-txns-vout-negative"); if (txout.nValue > MAX_MONEY) - return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-txns-vout-toolarge"); + return state.Invalid(TxValidationResult::TX_CONSENSUS, false, "bad-txns-vout-toolarge"); nValueOut += txout.nValue; if (!MoneyRange(nValueOut)) - return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-txns-txouttotal-toolarge"); + return state.Invalid(TxValidationResult::TX_CONSENSUS, false, "bad-txns-txouttotal-toolarge"); } // Check for duplicate inputs (see CVE-2018-17144) @@ -39,19 +39,19 @@ bool CheckTransaction(const CTransaction& tx, CValidationState& state) std::set vInOutPoints; for (const auto& txin : tx.vin) { if (!vInOutPoints.insert(txin.prevout).second) - return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-txns-inputs-duplicate"); + return state.Invalid(TxValidationResult::TX_CONSENSUS, false, "bad-txns-inputs-duplicate"); } if (tx.IsCoinBase()) { if (tx.vin[0].scriptSig.size() < 2 || tx.vin[0].scriptSig.size() > 100) - return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-cb-length"); + return state.Invalid(TxValidationResult::TX_CONSENSUS, false, "bad-cb-length"); } else { for (const auto& txin : tx.vin) if (txin.prevout.IsNull()) - return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-txns-prevout-null"); + return state.Invalid(TxValidationResult::TX_CONSENSUS, false, "bad-txns-prevout-null"); } return true; diff --git a/src/consensus/tx_check.h b/src/consensus/tx_check.h index 6f3f8fe969..b818a284f1 100644 --- a/src/consensus/tx_check.h +++ b/src/consensus/tx_check.h @@ -13,8 +13,8 @@ */ class CTransaction; -class CValidationState; +class TxValidationState; -bool CheckTransaction(const CTransaction& tx, CValidationState& state); +bool CheckTransaction(const CTransaction& tx, TxValidationState& state); #endif // BITCOIN_CONSENSUS_TX_CHECK_H diff --git a/src/consensus/tx_verify.cpp b/src/consensus/tx_verify.cpp index ceeddc3f6d..47242aae93 100644 --- a/src/consensus/tx_verify.cpp +++ b/src/consensus/tx_verify.cpp @@ -156,11 +156,11 @@ int64_t GetTransactionSigOpCost(const CTransaction& tx, const CCoinsViewCache& i return nSigOps; } -bool Consensus::CheckTxInputs(const CTransaction& tx, CValidationState& state, const CCoinsViewCache& inputs, int nSpendHeight, CAmount& txfee) +bool Consensus::CheckTxInputs(const CTransaction& tx, TxValidationState& state, const CCoinsViewCache& inputs, int nSpendHeight, CAmount& txfee) { // are the actual inputs available? if (!inputs.HaveInputs(tx)) { - return state.Invalid(ValidationInvalidReason::TX_MISSING_INPUTS, false, "bad-txns-inputs-missingorspent", + return state.Invalid(TxValidationResult::TX_MISSING_INPUTS, false, "bad-txns-inputs-missingorspent", strprintf("%s: inputs missing/spent", __func__)); } @@ -172,27 +172,27 @@ bool Consensus::CheckTxInputs(const CTransaction& tx, CValidationState& state, c // If prev is coinbase, check that it's matured if (coin.IsCoinBase() && nSpendHeight - coin.nHeight < COINBASE_MATURITY) { - return state.Invalid(ValidationInvalidReason::TX_PREMATURE_SPEND, false, "bad-txns-premature-spend-of-coinbase", + return state.Invalid(TxValidationResult::TX_PREMATURE_SPEND, false, "bad-txns-premature-spend-of-coinbase", strprintf("tried to spend coinbase at depth %d", nSpendHeight - coin.nHeight)); } // Check for negative or overflow input values nValueIn += coin.out.nValue; if (!MoneyRange(coin.out.nValue) || !MoneyRange(nValueIn)) { - return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-txns-inputvalues-outofrange"); + return state.Invalid(TxValidationResult::TX_CONSENSUS, false, "bad-txns-inputvalues-outofrange"); } } const CAmount value_out = tx.GetValueOut(); if (nValueIn < value_out) { - return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-txns-in-belowout", + return state.Invalid(TxValidationResult::TX_CONSENSUS, false, "bad-txns-in-belowout", strprintf("value in (%s) < value out (%s)", FormatMoney(nValueIn), FormatMoney(value_out))); } // Tally transaction fees const CAmount txfee_aux = nValueIn - value_out; if (!MoneyRange(txfee_aux)) { - return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-txns-fee-outofrange"); + return state.Invalid(TxValidationResult::TX_CONSENSUS, false, "bad-txns-fee-outofrange"); } txfee = txfee_aux; diff --git a/src/consensus/tx_verify.h b/src/consensus/tx_verify.h index 3519fc555d..b6599f2878 100644 --- a/src/consensus/tx_verify.h +++ b/src/consensus/tx_verify.h @@ -13,7 +13,7 @@ class CBlockIndex; class CCoinsViewCache; class CTransaction; -class CValidationState; +class TxValidationState; /** Transaction validation functions */ @@ -24,7 +24,7 @@ namespace Consensus { * @param[out] txfee Set to the transaction fee if successful. * Preconditions: tx.IsCoinBase() is false. */ -bool CheckTxInputs(const CTransaction& tx, CValidationState& state, const CCoinsViewCache& inputs, int nSpendHeight, CAmount& txfee); +bool CheckTxInputs(const CTransaction& tx, TxValidationState& state, const CCoinsViewCache& inputs, int nSpendHeight, CAmount& txfee); } // namespace Consensus /** Auxiliary functions for transaction validation (ideally should not be exposed) */ diff --git a/src/consensus/validation.h b/src/consensus/validation.h index 4920cdf881..c285817c0d 100644 --- a/src/consensus/validation.h +++ b/src/consensus/validation.h @@ -12,13 +12,12 @@ #include #include -/** A "reason" why something was invalid, suitable for determining whether the - * provider of the object should be banned/ignored/disconnected/etc. +/** A "reason" why a transaction was invalid, suitable for determining whether the + * provider of the transaction should be banned/ignored/disconnected/etc. */ -enum class ValidationInvalidReason { - // txn and blocks: - NONE, //!< not actually invalid - CONSENSUS, //!< invalid by consensus rules (excluding any below reasons) +enum class TxValidationResult { + TX_RESULT_UNSET, //!< initial value. Tx has not yet been rejected + TX_CONSENSUS, //!< invalid by consensus rules /** * Invalid by a change to consensus rules more recent than SegWit. * Currently unused as there are no such consensus rule changes, and any download @@ -26,18 +25,14 @@ enum class ValidationInvalidReason { * so differentiating between always-invalid and invalid-by-pre-SegWit-soft-fork * is uninteresting. */ - RECENT_CONSENSUS_CHANGE, - // Only blocks (or headers): - CACHED_INVALID, //!< this object was cached as being invalid, but we don't know why - BLOCK_INVALID_HEADER, //!< invalid proof of work or time too old - BLOCK_MUTATED, //!< the block's data didn't match the data committed to by the PoW - BLOCK_MISSING_PREV, //!< We don't have the previous block the checked one is built on - BLOCK_INVALID_PREV, //!< A block this one builds on is invalid - BLOCK_TIME_FUTURE, //!< block timestamp was > 2 hours in the future (or our clock is bad) - BLOCK_CHECKPOINT, //!< the block failed to meet one of our checkpoints - // Only loose txn: + TX_RECENT_CONSENSUS_CHANGE, TX_NOT_STANDARD, //!< didn't meet our local policy rules - TX_MISSING_INPUTS, //!< a transaction was missing some of its inputs + /** + * transaction was missing some of its inputs + * TODO: ATMP uses fMissingInputs and a valid ValidationState to indicate missing inputs. + * Change ATMP to use TX_MISSING_INPUTS. + */ + TX_MISSING_INPUTS, TX_PREMATURE_SPEND, //!< transaction spends a coinbase too early, or violates locktime/sequence locks /** * Transaction might be missing a witness, have a witness prior to SegWit @@ -48,57 +43,55 @@ enum class ValidationInvalidReason { /** * Tx already in mempool or conflicts with a tx in the chain * (if it conflicts with another tx in mempool, we use MEMPOOL_POLICY as it failed to reach the RBF threshold) - * TODO: Currently this is only used if the transaction already exists in the mempool or on chain, - * TODO: ATMP's fMissingInputs and a valid CValidationState being used to indicate missing inputs + * Currently this is only used if the transaction already exists in the mempool or on chain. */ TX_CONFLICT, TX_MEMPOOL_POLICY, //!< violated mempool's fee/size/descendant/RBF/etc limits }; -inline bool IsTransactionReason(ValidationInvalidReason r) -{ - return r == ValidationInvalidReason::NONE || - r == ValidationInvalidReason::CONSENSUS || - r == ValidationInvalidReason::RECENT_CONSENSUS_CHANGE || - r == ValidationInvalidReason::TX_NOT_STANDARD || - r == ValidationInvalidReason::TX_PREMATURE_SPEND || - r == ValidationInvalidReason::TX_MISSING_INPUTS || - r == ValidationInvalidReason::TX_WITNESS_MUTATED || - r == ValidationInvalidReason::TX_CONFLICT || - r == ValidationInvalidReason::TX_MEMPOOL_POLICY; -} +/** A "reason" why a block was invalid, suitable for determining whether the + * provider of the block should be banned/ignored/disconnected/etc. + * These are much more granular than the rejection codes, which may be more + * useful for some other use-cases. + */ +enum class BlockValidationResult { + BLOCK_RESULT_UNSET, //!< initial value. Block has not yet been rejected + BLOCK_CONSENSUS, //!< invalid by consensus rules (excluding any below reasons) + /** + * Invalid by a change to consensus rules more recent than SegWit. + * Currently unused as there are no such consensus rule changes, and any download + * sources realistically need to support SegWit in order to provide useful data, + * so differentiating between always-invalid and invalid-by-pre-SegWit-soft-fork + * is uninteresting. + */ + BLOCK_RECENT_CONSENSUS_CHANGE, + BLOCK_CACHED_INVALID, //!< this block was cached as being invalid and we didn't store the reason why + BLOCK_INVALID_HEADER, //!< invalid proof of work or time too old + BLOCK_MUTATED, //!< the block's data didn't match the data committed to by the PoW + BLOCK_MISSING_PREV, //!< We don't have the previous block the checked one is built on + BLOCK_INVALID_PREV, //!< A block this one builds on is invalid + BLOCK_TIME_FUTURE, //!< block timestamp was > 2 hours in the future (or our clock is bad) + BLOCK_CHECKPOINT, //!< the block failed to meet one of our checkpoints +}; -inline bool IsBlockReason(ValidationInvalidReason r) -{ - return r == ValidationInvalidReason::NONE || - r == ValidationInvalidReason::CONSENSUS || - r == ValidationInvalidReason::RECENT_CONSENSUS_CHANGE || - r == ValidationInvalidReason::CACHED_INVALID || - r == ValidationInvalidReason::BLOCK_INVALID_HEADER || - r == ValidationInvalidReason::BLOCK_MUTATED || - r == ValidationInvalidReason::BLOCK_MISSING_PREV || - r == ValidationInvalidReason::BLOCK_INVALID_PREV || - r == ValidationInvalidReason::BLOCK_TIME_FUTURE || - r == ValidationInvalidReason::BLOCK_CHECKPOINT; -} -/** Capture information about block/transaction validation */ -class CValidationState { + +/** Base class for capturing information about block/transaction validation. This is subclassed + * by TxValidationState and BlockValidationState for validation information on transactions + * and blocks respectively. */ +class ValidationState { private: enum mode_state { MODE_VALID, //!< everything ok MODE_INVALID, //!< network rule violation (DoS value may be set) MODE_ERROR, //!< run-time error } mode; - ValidationInvalidReason m_reason; std::string strRejectReason; std::string strDebugMessage; -public: - CValidationState() : mode(MODE_VALID), m_reason(ValidationInvalidReason::NONE) {} - bool Invalid(ValidationInvalidReason reasonIn, bool ret = false, +protected: + bool Invalid(bool ret = false, const std::string &strRejectReasonIn="", const std::string &strDebugMessageIn="") { - m_reason = reasonIn; strRejectReason = strRejectReasonIn; strDebugMessage = strDebugMessageIn; if (mode == MODE_ERROR) @@ -106,6 +99,11 @@ public: mode = MODE_INVALID; return ret; } +public: + // ValidationState is abstract. Have a pure virtual destructor. + virtual ~ValidationState() = 0; + + ValidationState() : mode(MODE_VALID) {} bool Error(const std::string& strRejectReasonIn) { if (mode == MODE_VALID) strRejectReason = strRejectReasonIn; @@ -121,11 +119,38 @@ public: bool IsError() const { return mode == MODE_ERROR; } - ValidationInvalidReason GetReason() const { return m_reason; } std::string GetRejectReason() const { return strRejectReason; } std::string GetDebugMessage() const { return strDebugMessage; } }; +inline ValidationState::~ValidationState() {}; + +class TxValidationState : public ValidationState { +private: + TxValidationResult m_result; +public: + bool Invalid(TxValidationResult result, bool ret = false, + const std::string &_strRejectReason="", + const std::string &_strDebugMessage="") { + m_result = result; + return ValidationState::Invalid(ret, _strRejectReason, _strDebugMessage); + } + TxValidationResult GetResult() const { return m_result; } +}; + +class BlockValidationState : public ValidationState { +private: + BlockValidationResult m_result; +public: + bool Invalid(BlockValidationResult result, bool ret = false, + const std::string &_strRejectReason="", + const std::string &_strDebugMessage="") { + m_result = result; + return ValidationState::Invalid(ret, _strRejectReason, _strDebugMessage); + } + BlockValidationResult GetResult() const { return m_result; } +}; + // These implement the weight = (stripped_size * 4) + witness_size formula, // using only serialization with and without witness data. As witness_size // is equal to total_size - stripped_size, this formula is identical to: diff --git a/src/init.cpp b/src/init.cpp index 035725b090..01faf8f6c0 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -712,7 +712,7 @@ static void ThreadImport(std::vector vImportFiles) } // scan for better chains in the block chain database, that are not yet connected in the active best chain - CValidationState state; + BlockValidationState state; if (!ActivateBestChain(state, chainparams)) { LogPrintf("Failed to connect best block (%s)\n", FormatStateMessage(state)); StartShutdown(); diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h index 73a78e21fb..d54a14022c 100644 --- a/src/interfaces/chain.h +++ b/src/interfaces/chain.h @@ -18,7 +18,6 @@ class CBlock; class CFeeRate; class CRPCCommand; class CScheduler; -class CValidationState; class Coin; class uint256; enum class RBFTransactionState; diff --git a/src/miner.cpp b/src/miner.cpp index 4f51be8a08..1c9174ee07 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -162,7 +162,7 @@ std::unique_ptr BlockAssembler::CreateNewBlock(const CScript& sc pblock->nNonce = 0; pblocktemplate->vTxSigOpsCost[0] = WITNESS_SCALE_FACTOR * GetLegacySigOpCount(*pblock->vtx[0]); - CValidationState state; + BlockValidationState state; if (!TestBlockValidity(state, chainparams, *pblock, pindexPrev, false, false)) { throw std::runtime_error(strprintf("%s: TestBlockValidity failed: %s", __func__, FormatStateMessage(state))); } diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 4a3076e3c7..38d0461b1f 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -982,14 +982,12 @@ void Misbehaving(NodeId pnode, int howmuch, const std::string& message) EXCLUSIV * banning/disconnecting us. We use this to determine which unaccepted * transactions from a whitelisted peer that we can safely relay. */ -static bool TxRelayMayResultInDisconnect(const CValidationState& state) -{ - assert(IsTransactionReason(state.GetReason())); - return state.GetReason() == ValidationInvalidReason::CONSENSUS; +static bool TxRelayMayResultInDisconnect(const TxValidationState& state) { + return state.GetResult() == TxValidationResult::TX_CONSENSUS; } /** - * Potentially ban a node based on the contents of a CValidationState object + * Potentially ban a node based on the contents of a BlockValidationState object * * @param[in] via_compact_block: this bool is passed in because net_processing should * punish peers differently depending on whether the data was provided in a compact @@ -997,23 +995,21 @@ static bool TxRelayMayResultInDisconnect(const CValidationState& state) * txs, the peer should not be punished. See BIP 152. * * @return Returns true if the peer was punished (probably disconnected) - * - * Changes here may need to be reflected in TxRelayMayResultInDisconnect(). */ -static bool MaybePunishNode(NodeId nodeid, const CValidationState& state, bool via_compact_block, const std::string& message = "") { - switch (state.GetReason()) { - case ValidationInvalidReason::NONE: +static bool MaybePunishNodeForBlock(NodeId nodeid, const BlockValidationState& state, bool via_compact_block, const std::string& message = "") { + switch (state.GetResult()) { + case BlockValidationResult::BLOCK_RESULT_UNSET: break; // The node is providing invalid data: - case ValidationInvalidReason::CONSENSUS: - case ValidationInvalidReason::BLOCK_MUTATED: + case BlockValidationResult::BLOCK_CONSENSUS: + case BlockValidationResult::BLOCK_MUTATED: if (!via_compact_block) { LOCK(cs_main); Misbehaving(nodeid, 100, message); return true; } break; - case ValidationInvalidReason::CACHED_INVALID: + case BlockValidationResult::BLOCK_CACHED_INVALID: { LOCK(cs_main); CNodeState *node_state = State(nodeid); @@ -1029,30 +1025,24 @@ static bool MaybePunishNode(NodeId nodeid, const CValidationState& state, bool v } break; } - case ValidationInvalidReason::BLOCK_INVALID_HEADER: - case ValidationInvalidReason::BLOCK_CHECKPOINT: - case ValidationInvalidReason::BLOCK_INVALID_PREV: + case BlockValidationResult::BLOCK_INVALID_HEADER: + case BlockValidationResult::BLOCK_CHECKPOINT: + case BlockValidationResult::BLOCK_INVALID_PREV: { LOCK(cs_main); Misbehaving(nodeid, 100, message); } return true; // Conflicting (but not necessarily invalid) data or different policy: - case ValidationInvalidReason::BLOCK_MISSING_PREV: + case BlockValidationResult::BLOCK_MISSING_PREV: { // TODO: Handle this much more gracefully (10 DoS points is super arbitrary) LOCK(cs_main); Misbehaving(nodeid, 10, message); } return true; - case ValidationInvalidReason::RECENT_CONSENSUS_CHANGE: - case ValidationInvalidReason::BLOCK_TIME_FUTURE: - case ValidationInvalidReason::TX_NOT_STANDARD: - case ValidationInvalidReason::TX_MISSING_INPUTS: - case ValidationInvalidReason::TX_PREMATURE_SPEND: - case ValidationInvalidReason::TX_WITNESS_MUTATED: - case ValidationInvalidReason::TX_CONFLICT: - case ValidationInvalidReason::TX_MEMPOOL_POLICY: + case BlockValidationResult::BLOCK_RECENT_CONSENSUS_CHANGE: + case BlockValidationResult::BLOCK_TIME_FUTURE: break; } if (message != "") { @@ -1061,6 +1051,39 @@ static bool MaybePunishNode(NodeId nodeid, const CValidationState& state, bool v return false; } +/** + * Potentially ban a node based on the contents of a TxValidationState object + * + * @return Returns true if the peer was punished (probably disconnected) + * + * Changes here may need to be reflected in TxRelayMayResultInDisconnect(). + */ +static bool MaybePunishNodeForTx(NodeId nodeid, const TxValidationState& state, const std::string& message = "") { + switch (state.GetResult()) { + case TxValidationResult::TX_RESULT_UNSET: + break; + // The node is providing invalid data: + case TxValidationResult::TX_CONSENSUS: + { + LOCK(cs_main); + Misbehaving(nodeid, 100, message); + return true; + } + // Conflicting (but not necessarily invalid) data or different policy: + case TxValidationResult::TX_RECENT_CONSENSUS_CHANGE: + case TxValidationResult::TX_NOT_STANDARD: + case TxValidationResult::TX_MISSING_INPUTS: + case TxValidationResult::TX_PREMATURE_SPEND: + case TxValidationResult::TX_WITNESS_MUTATED: + case TxValidationResult::TX_CONFLICT: + case TxValidationResult::TX_MEMPOOL_POLICY: + break; + } + if (message != "") { + LogPrint(BCLog::NET, "peer=%d: %s\n", nodeid, message); + } + return false; +} @@ -1229,7 +1252,7 @@ void PeerLogicValidation::UpdatedBlockTip(const CBlockIndex *pindexNew, const CB * Handle invalid block rejection and consequent peer banning, maintain which * peers announce compact blocks. */ -void PeerLogicValidation::BlockChecked(const CBlock& block, const CValidationState& state) { +void PeerLogicValidation::BlockChecked(const CBlock& block, const BlockValidationState& state) { LOCK(cs_main); const uint256 hash(block.GetHash()); @@ -1240,7 +1263,7 @@ void PeerLogicValidation::BlockChecked(const CBlock& block, const CValidationSta if (state.IsInvalid() && it != mapBlockSource.end() && State(it->second.first)) { - MaybePunishNode(/*nodeid=*/ it->second.first, state, /*via_compact_block=*/ !it->second.second); + MaybePunishNodeForBlock(/*nodeid=*/ it->second.first, state, /*via_compact_block=*/ !it->second.second); } // Check that: // 1. The block is valid @@ -1378,7 +1401,7 @@ void static ProcessGetBlockData(CNode* pfrom, const CChainParams& chainparams, c } } // release cs_main before calling ActivateBestChain if (need_activate_chain) { - CValidationState state; + BlockValidationState state; if (!ActivateBestChain(state, Params(), a_recent_block)) { LogPrint(BCLog::NET, "failed to activate chain (%s)\n", FormatStateMessage(state)); } @@ -1674,11 +1697,11 @@ bool static ProcessHeadersMessage(CNode *pfrom, CConnman *connman, const std::ve } } - CValidationState state; + BlockValidationState state; CBlockHeader first_invalid_header; if (!ProcessNewBlockHeaders(headers, state, chainparams, &pindexLast, &first_invalid_header)) { if (state.IsInvalid()) { - MaybePunishNode(pfrom->GetId(), state, via_compact_block, "invalid header received"); + MaybePunishNodeForBlock(pfrom->GetId(), state, via_compact_block, "invalid header received"); return false; } } @@ -1815,10 +1838,10 @@ void static ProcessOrphanTx(CConnman* connman, std::set& orphan_work_se const CTransaction& orphanTx = *porphanTx; NodeId fromPeer = orphan_it->second.fromPeer; bool fMissingInputs2 = false; - // Use a new CValidationState because orphans come from different peers (and we call - // MaybePunishNode based on the source peer from the orphan map, not based on the peer + // Use a new TxValidationState because orphans come from different peers (and we call + // MaybePunishNodeForTx based on the source peer from the orphan map, not based on the peer // that relayed the previous transaction). - CValidationState orphan_state; + TxValidationState orphan_state; if (setMisbehaving.count(fromPeer)) continue; if (AcceptToMemoryPool(mempool, orphan_state, porphanTx, &fMissingInputs2, &removed_txn, false /* bypass_limits */, 0 /* nAbsurdFee */)) { @@ -1837,7 +1860,7 @@ void static ProcessOrphanTx(CConnman* connman, std::set& orphan_work_se } else if (!fMissingInputs2) { if (orphan_state.IsInvalid()) { // Punish peer that gave us an invalid orphan tx - if (MaybePunishNode(fromPeer, orphan_state, /*via_compact_block*/ false)) { + if (MaybePunishNodeForTx(fromPeer, orphan_state)) { setMisbehaving.insert(fromPeer); } LogPrint(BCLog::MEMPOOL, " invalid orphan tx %s\n", orphanHash.ToString()); @@ -1845,8 +1868,7 @@ void static ProcessOrphanTx(CConnman* connman, std::set& orphan_work_se // Has inputs but not accepted to mempool // Probably non-standard or insufficient fee LogPrint(BCLog::MEMPOOL, " removed orphan tx %s\n", orphanHash.ToString()); - assert(IsTransactionReason(orphan_state.GetReason())); - if (!orphanTx.HasWitness() && orphan_state.GetReason() != ValidationInvalidReason::TX_WITNESS_MUTATED) { + if (!orphanTx.HasWitness() && orphan_state.GetResult() != TxValidationResult::TX_WITNESS_MUTATED) { // Do not use rejection cache for witness transactions or // witness-stripped transactions, as they can have been malleated. // See https://github.com/bitcoin/bitcoin/issues/8279 for details. @@ -2291,7 +2313,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr LOCK(cs_most_recent_block); a_recent_block = most_recent_block; } - CValidationState state; + BlockValidationState state; if (!ActivateBestChain(state, Params(), a_recent_block)) { LogPrint(BCLog::NET, "failed to activate chain (%s)\n", FormatStateMessage(state)); } @@ -2472,7 +2494,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr LOCK2(cs_main, g_cs_orphans); bool fMissingInputs = false; - CValidationState state; + TxValidationState state; CNodeState* nodestate = State(pfrom->GetId()); nodestate->m_tx_download.m_tx_announced.erase(inv.hash); @@ -2537,8 +2559,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr recentRejects->insert(tx.GetHash()); } } else { - assert(IsTransactionReason(state.GetReason())); - if (!tx.HasWitness() && state.GetReason() != ValidationInvalidReason::TX_WITNESS_MUTATED) { + if (!tx.HasWitness() && state.GetResult() != TxValidationResult::TX_WITNESS_MUTATED) { // Do not use rejection cache for witness transactions or // witness-stripped transactions, as they can have been malleated. // See https://github.com/bitcoin/bitcoin/issues/8279 for details. @@ -2593,7 +2614,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr LogPrint(BCLog::MEMPOOLREJ, "%s from peer=%d was not accepted: %s\n", tx.GetHash().ToString(), pfrom->GetId(), FormatStateMessage(state)); - MaybePunishNode(pfrom->GetId(), state, /*via_compact_block*/ false); + MaybePunishNodeForTx(pfrom->GetId(), state); } return true; } @@ -2627,10 +2648,10 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr } const CBlockIndex *pindex = nullptr; - CValidationState state; + BlockValidationState state; if (!ProcessNewBlockHeaders({cmpctblock.header}, state, chainparams, &pindex)) { if (state.IsInvalid()) { - MaybePunishNode(pfrom->GetId(), state, /*via_compact_block*/ true, "invalid header via cmpctblock"); + MaybePunishNodeForBlock(pfrom->GetId(), state, /*via_compact_block*/ true, "invalid header via cmpctblock"); return true; } } diff --git a/src/net_processing.h b/src/net_processing.h index e8bc3580dd..4adb7d3a21 100644 --- a/src/net_processing.h +++ b/src/net_processing.h @@ -40,7 +40,7 @@ public: /** * Overridden from CValidationInterface. */ - void BlockChecked(const CBlock& block, const CValidationState& state) override; + void BlockChecked(const CBlock& block, const BlockValidationState& state) override; /** * Overridden from CValidationInterface. */ diff --git a/src/node/transaction.cpp b/src/node/transaction.cpp index 7783671a6c..4645baa3d7 100644 --- a/src/node/transaction.cpp +++ b/src/node/transaction.cpp @@ -36,7 +36,7 @@ TransactionError BroadcastTransaction(const CTransactionRef tx, std::string& err } if (!mempool.exists(hashTx)) { // Transaction is not already in the mempool. Submit it. - CValidationState state; + TxValidationState state; bool fMissingInputs; if (!AcceptToMemoryPool(mempool, state, std::move(tx), &fMissingInputs, nullptr /* plTxnReplaced */, false /* bypass_limits */, max_tx_fee)) { diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 4bbd4aaf64..c4ce561fe3 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -1470,7 +1470,7 @@ static UniValue preciousblock(const JSONRPCRequest& request) } } - CValidationState state; + BlockValidationState state; PreciousBlock(state, Params(), pblockindex); if (!state.IsValid()) { @@ -1495,7 +1495,7 @@ static UniValue invalidateblock(const JSONRPCRequest& request) }.Check(request); uint256 hash(ParseHashV(request.params[0], "blockhash")); - CValidationState state; + BlockValidationState state; CBlockIndex* pblockindex; { @@ -1545,7 +1545,7 @@ static UniValue reconsiderblock(const JSONRPCRequest& request) ResetBlockFailureFlags(pblockindex); } - CValidationState state; + BlockValidationState state; ActivateBestChain(state, Params()); if (!state.IsValid()) { diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 07c2958635..f294c815f6 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -252,7 +252,7 @@ static UniValue prioritisetransaction(const JSONRPCRequest& request) // NOTE: Assumes a conclusive result; if result is inconclusive, it must be handled by caller -static UniValue BIP22ValidationResult(const CValidationState& state) +static UniValue BIP22ValidationResult(const BlockValidationState& state) { if (state.IsValid()) return NullUniValue; @@ -401,7 +401,7 @@ static UniValue getblocktemplate(const JSONRPCRequest& request) // TestBlockValidity only supports blocks built on the current Tip if (block.hashPrevBlock != pindexPrev->GetBlockHash()) return "inconclusive-not-best-prevblk"; - CValidationState state; + BlockValidationState state; TestBlockValidity(state, Params(), block, pindexPrev, false, true); return BIP22ValidationResult(state); } @@ -668,12 +668,12 @@ class submitblock_StateCatcher : public CValidationInterface public: uint256 hash; bool found; - CValidationState state; + BlockValidationState state; explicit submitblock_StateCatcher(const uint256 &hashIn) : hash(hashIn), found(false), state() {} protected: - void BlockChecked(const CBlock& block, const CValidationState& stateIn) override { + void BlockChecked(const CBlock& block, const BlockValidationState& stateIn) override { if (block.GetHash() != hash) return; found = true; @@ -772,7 +772,7 @@ static UniValue submitheader(const JSONRPCRequest& request) } } - CValidationState state; + BlockValidationState state; ProcessNewBlockHeaders({h}, state, Params(), /* ppindex */ nullptr, /* first_invalid */ nullptr); if (state.IsValid()) return NullUniValue; if (state.IsError()) { diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index cdcf0c9971..68024fbe2f 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -893,7 +893,7 @@ static UniValue testmempoolaccept(const JSONRPCRequest& request) UniValue result_0(UniValue::VOBJ); result_0.pushKV("txid", tx_hash.GetHex()); - CValidationState state; + TxValidationState state; bool missing_inputs; bool test_accept_res; { diff --git a/src/test/blockfilter_index_tests.cpp b/src/test/blockfilter_index_tests.cpp index ba293b7836..2e40ed0dde 100644 --- a/src/test/blockfilter_index_tests.cpp +++ b/src/test/blockfilter_index_tests.cpp @@ -102,7 +102,7 @@ static bool BuildChain(const CBlockIndex* pindex, const CScript& coinbase_script block = std::make_shared(CreateBlock(pindex, no_txns, coinbase_script_pub_key)); CBlockHeader header = block->GetBlockHeader(); - CValidationState state; + BlockValidationState state; if (!ProcessNewBlockHeaders({header}, state, Params(), &pindex, nullptr)) { return false; } diff --git a/src/test/fuzz/transaction.cpp b/src/test/fuzz/transaction.cpp index 383d879040..76b230ef3c 100644 --- a/src/test/fuzz/transaction.cpp +++ b/src/test/fuzz/transaction.cpp @@ -42,7 +42,7 @@ void test_one_input(const std::vector& buffer) return; } - CValidationState state_with_dupe_check; + TxValidationState state_with_dupe_check; (void)CheckTransaction(tx, state_with_dupe_check); const CFeeRate dust_relay_fee{DUST_RELAY_TX_FEE}; diff --git a/src/test/setup_common.cpp b/src/test/setup_common.cpp index bbdf1ef830..d83668460f 100644 --- a/src/test/setup_common.cpp +++ b/src/test/setup_common.cpp @@ -95,7 +95,7 @@ TestingSetup::TestingSetup(const std::string& chainName) : BasicTestingSetup(cha throw std::runtime_error("LoadGenesisBlock failed."); } - CValidationState state; + BlockValidationState state; if (!ActivateBestChain(state, chainparams)) { throw std::runtime_error(strprintf("ActivateBestChain failed. (%s)", FormatStateMessage(state))); } diff --git a/src/test/sighash_tests.cpp b/src/test/sighash_tests.cpp index 15f8db899b..b18f9df72d 100644 --- a/src/test/sighash_tests.cpp +++ b/src/test/sighash_tests.cpp @@ -193,7 +193,7 @@ BOOST_AUTO_TEST_CASE(sighash_from_data) CDataStream stream(ParseHex(raw_tx), SER_NETWORK, PROTOCOL_VERSION); stream >> tx; - CValidationState state; + TxValidationState state; BOOST_CHECK_MESSAGE(CheckTransaction(*tx, state), strTest); BOOST_CHECK(state.IsValid()); diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp index 34192c6b6a..dac46879e8 100644 --- a/src/test/transaction_tests.cpp +++ b/src/test/transaction_tests.cpp @@ -152,7 +152,7 @@ BOOST_AUTO_TEST_CASE(tx_valid) CDataStream stream(ParseHex(transaction), SER_NETWORK, PROTOCOL_VERSION); CTransaction tx(deserialize, stream); - CValidationState state; + TxValidationState state; BOOST_CHECK_MESSAGE(CheckTransaction(tx, state), strTest); BOOST_CHECK(state.IsValid()); @@ -239,7 +239,7 @@ BOOST_AUTO_TEST_CASE(tx_invalid) CDataStream stream(ParseHex(transaction), SER_NETWORK, PROTOCOL_VERSION ); CTransaction tx(deserialize, stream); - CValidationState state; + TxValidationState state; fValid = CheckTransaction(tx, state) && state.IsValid(); PrecomputedTransactionData txdata(tx); @@ -274,7 +274,7 @@ BOOST_AUTO_TEST_CASE(basic_transaction_tests) CDataStream stream(vch, SER_DISK, CLIENT_VERSION); CMutableTransaction tx; stream >> tx; - CValidationState state; + TxValidationState state; BOOST_CHECK_MESSAGE(CheckTransaction(CTransaction(tx), state) && state.IsValid(), "Simple deserialized transaction should be valid."); // Check that duplicate txins fail diff --git a/src/test/txvalidation_tests.cpp b/src/test/txvalidation_tests.cpp index 2356e0ccdc..1f781ca83a 100644 --- a/src/test/txvalidation_tests.cpp +++ b/src/test/txvalidation_tests.cpp @@ -30,7 +30,7 @@ BOOST_FIXTURE_TEST_CASE(tx_mempool_reject_coinbase, TestChain100Setup) BOOST_CHECK(CTransaction(coinbaseTx).IsCoinBase()); - CValidationState state; + TxValidationState state; LOCK(cs_main); @@ -50,7 +50,7 @@ BOOST_FIXTURE_TEST_CASE(tx_mempool_reject_coinbase, TestChain100Setup) // Check that the validation state reflects the unsuccessful attempt. BOOST_CHECK(state.IsInvalid()); BOOST_CHECK_EQUAL(state.GetRejectReason(), "coinbase"); - BOOST_CHECK(state.GetReason() == ValidationInvalidReason::CONSENSUS); + BOOST_CHECK(state.GetResult() == TxValidationResult::TX_CONSENSUS); } BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/txvalidationcache_tests.cpp b/src/test/txvalidationcache_tests.cpp index 193858cca9..ea2ef5546f 100644 --- a/src/test/txvalidationcache_tests.cpp +++ b/src/test/txvalidationcache_tests.cpp @@ -13,7 +13,7 @@ #include -bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsViewCache &inputs, unsigned int flags, bool cacheSigStore, bool cacheFullScriptStore, PrecomputedTransactionData& txdata, std::vector *pvChecks); +bool CheckInputs(const CTransaction& tx, TxValidationState &state, const CCoinsViewCache &inputs, unsigned int flags, bool cacheSigStore, bool cacheFullScriptStore, PrecomputedTransactionData& txdata, std::vector *pvChecks); BOOST_AUTO_TEST_SUITE(tx_validationcache_tests) @@ -22,7 +22,7 @@ ToMemPool(const CMutableTransaction& tx) { LOCK(cs_main); - CValidationState state; + TxValidationState state; return AcceptToMemoryPool(mempool, state, MakeTransactionRef(tx), nullptr /* pfMissingInputs */, nullptr /* plTxnReplaced */, true /* bypass_limits */, 0 /* nAbsurdFee */); } @@ -114,7 +114,7 @@ static void ValidateCheckInputsForAllFlags(const CTransaction &tx, uint32_t fail // If we add many more flags, this loop can get too expensive, but we can // rewrite in the future to randomly pick a set of flags to evaluate. for (uint32_t test_flags=0; test_flags < (1U << 16); test_flags += 1) { - CValidationState state; + TxValidationState state; // Filter out incompatible flag choices if ((test_flags & SCRIPT_VERIFY_CLEANSTACK)) { // CLEANSTACK requires P2SH and WITNESS, see VerifyScript() in @@ -201,7 +201,7 @@ BOOST_FIXTURE_TEST_CASE(checkinputs_test, TestChain100Setup) { LOCK(cs_main); - CValidationState state; + TxValidationState state; PrecomputedTransactionData ptd_spend_tx(spend_tx); BOOST_CHECK(!CheckInputs(CTransaction(spend_tx), state, &::ChainstateActive().CoinsTip(), SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_DERSIG, true, true, ptd_spend_tx, nullptr)); @@ -270,7 +270,7 @@ BOOST_FIXTURE_TEST_CASE(checkinputs_test, TestChain100Setup) // Make it valid, and check again invalid_with_cltv_tx.vin[0].scriptSig = CScript() << vchSig << 100; - CValidationState state; + TxValidationState state; PrecomputedTransactionData txdata(invalid_with_cltv_tx); BOOST_CHECK(CheckInputs(CTransaction(invalid_with_cltv_tx), state, ::ChainstateActive().CoinsTip(), SCRIPT_VERIFY_CHECKLOCKTIMEVERIFY, true, true, txdata, nullptr)); } @@ -298,7 +298,7 @@ BOOST_FIXTURE_TEST_CASE(checkinputs_test, TestChain100Setup) // Make it valid, and check again invalid_with_csv_tx.vin[0].scriptSig = CScript() << vchSig << 100; - CValidationState state; + TxValidationState state; PrecomputedTransactionData txdata(invalid_with_csv_tx); BOOST_CHECK(CheckInputs(CTransaction(invalid_with_csv_tx), state, &::ChainstateActive().CoinsTip(), SCRIPT_VERIFY_CHECKSEQUENCEVERIFY, true, true, txdata, nullptr)); } @@ -359,7 +359,7 @@ BOOST_FIXTURE_TEST_CASE(checkinputs_test, TestChain100Setup) // Invalidate vin[1] tx.vin[1].scriptWitness.SetNull(); - CValidationState state; + TxValidationState state; PrecomputedTransactionData txdata(tx); // This transaction is now invalid under segwit, because of the second input. BOOST_CHECK(!CheckInputs(CTransaction(tx), state, &::ChainstateActive().CoinsTip(), SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_WITNESS, true, true, txdata, nullptr)); diff --git a/src/test/validation_block_tests.cpp b/src/test/validation_block_tests.cpp index b3368d44b6..cc1406c062 100644 --- a/src/test/validation_block_tests.cpp +++ b/src/test/validation_block_tests.cpp @@ -151,7 +151,7 @@ BOOST_AUTO_TEST_CASE(processnewblock_signals_ordering) } bool ignored; - CValidationState state; + BlockValidationState state; std::vector headers; std::transform(blocks.begin(), blocks.end(), std::back_inserter(headers), [](std::shared_ptr b) { return b->GetBlockHeader(); }); @@ -278,7 +278,7 @@ BOOST_AUTO_TEST_CASE(mempool_locks_reorg) // Add the txs to the tx pool { LOCK(cs_main); - CValidationState state; + TxValidationState state; std::list plTxnReplaced; for (const auto& tx : txs) { BOOST_REQUIRE(AcceptToMemoryPool( diff --git a/src/txmempool.cpp b/src/txmempool.cpp index e4c1fd4bc6..64d270f4c6 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -591,9 +591,9 @@ void CTxMemPool::clear() static void CheckInputsAndUpdateCoins(const CTransaction& tx, CCoinsViewCache& mempoolDuplicate, const int64_t spendheight) { - CValidationState state; + TxValidationState dummy_state; // Not used. CheckTxInputs() should always pass CAmount txfee = 0; - bool fCheckResult = tx.IsCoinBase() || Consensus::CheckTxInputs(tx, state, mempoolDuplicate, spendheight, txfee); + bool fCheckResult = tx.IsCoinBase() || Consensus::CheckTxInputs(tx, dummy_state, mempoolDuplicate, spendheight, txfee); assert(fCheckResult); UpdateCoins(tx, mempoolDuplicate, std::numeric_limits::max()); } diff --git a/src/util/validation.cpp b/src/util/validation.cpp index 9a0d889447..bd52f57751 100644 --- a/src/util/validation.cpp +++ b/src/util/validation.cpp @@ -8,8 +8,8 @@ #include #include -/** Convert CValidationState to a human-readable message for logging */ -std::string FormatStateMessage(const CValidationState &state) +/** Convert ValidationState to a human-readable message for logging */ +std::string FormatStateMessage(const ValidationState &state) { return strprintf("%s%s", state.GetRejectReason(), diff --git a/src/util/validation.h b/src/util/validation.h index 32559853ee..da2cf9f102 100644 --- a/src/util/validation.h +++ b/src/util/validation.h @@ -8,10 +8,10 @@ #include -class CValidationState; +class ValidationState; -/** Convert CValidationState to a human-readable message for logging */ -std::string FormatStateMessage(const CValidationState &state); +/** Convert ValidationState to a human-readable message for logging */ +std::string FormatStateMessage(const ValidationState &state); extern const std::string strMessageMagic; diff --git a/src/validation.cpp b/src/validation.cpp index 9301066c6a..4af2e9d3fc 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -181,7 +181,7 @@ std::unique_ptr pblocktree; // See definition for documentation static void FindFilesToPruneManual(std::set& setFilesToPrune, int nManualPruneHeight); static void FindFilesToPrune(std::set& setFilesToPrune, uint64_t nPruneAfterHeight); -bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsViewCache &inputs, unsigned int flags, bool cacheSigStore, bool cacheFullScriptStore, PrecomputedTransactionData& txdata, std::vector *pvChecks = nullptr); +bool CheckInputs(const CTransaction& tx, TxValidationState &state, const CCoinsViewCache &inputs, unsigned int flags, bool cacheSigStore, bool cacheFullScriptStore, PrecomputedTransactionData& txdata, std::vector *pvChecks = nullptr); static FILE* OpenUndoFile(const FlatFilePos &pos, bool fReadOnly = false); static FlatFileSeq BlockFileSeq(); static FlatFileSeq UndoFileSeq(); @@ -363,7 +363,7 @@ static void UpdateMempoolForReorg(DisconnectedBlockTransactions& disconnectpool, auto it = disconnectpool.queuedTx.get().rbegin(); while (it != disconnectpool.queuedTx.get().rend()) { // ignore validation errors in resurrected transactions - CValidationState stateDummy; + TxValidationState stateDummy; if (!fAddToMempool || (*it)->IsCoinBase() || !AcceptToMemoryPool(mempool, stateDummy, *it, nullptr /* pfMissingInputs */, nullptr /* plTxnReplaced */, true /* bypass_limits */, 0 /* nAbsurdFee */)) { @@ -391,7 +391,7 @@ static void UpdateMempoolForReorg(DisconnectedBlockTransactions& disconnectpool, // Used to avoid mempool polluting consensus critical paths if CCoinsViewMempool // were somehow broken and returning the wrong scriptPubKeys -static bool CheckInputsFromMempoolAndCache(const CTransaction& tx, CValidationState& state, const CCoinsViewCache& view, const CTxMemPool& pool, +static bool CheckInputsFromMempoolAndCache(const CTransaction& tx, TxValidationState& state, const CCoinsViewCache& view, const CTxMemPool& pool, unsigned int flags, PrecomputedTransactionData& txdata) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { AssertLockHeld(cs_main); @@ -441,7 +441,7 @@ public: // around easier. struct ATMPArgs { const CChainParams& m_chainparams; - CValidationState &m_state; + TxValidationState &m_state; bool* m_missing_inputs; const int64_t m_accept_time; std::list* m_replaced_transactions; @@ -502,15 +502,15 @@ private: bool Finalize(ATMPArgs& args, Workspace& ws) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_pool.cs); // Compare a package's feerate against minimum allowed. - bool CheckFeeRate(size_t package_size, CAmount package_fee, CValidationState& state) + bool CheckFeeRate(size_t package_size, CAmount package_fee, TxValidationState& state) { CAmount mempoolRejectFee = m_pool.GetMinFee(gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000).GetFee(package_size); if (mempoolRejectFee > 0 && package_fee < mempoolRejectFee) { - return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, "mempool min fee not met", strprintf("%d < %d", package_fee, mempoolRejectFee)); + return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, false, "mempool min fee not met", strprintf("%d < %d", package_fee, mempoolRejectFee)); } if (package_fee < ::minRelayTxFee.GetFee(package_size)) { - return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, "min relay fee not met", strprintf("%d < %d", package_fee, ::minRelayTxFee.GetFee(package_size))); + return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, false, "min relay fee not met", strprintf("%d < %d", package_fee, ::minRelayTxFee.GetFee(package_size))); } return true; } @@ -537,7 +537,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) const uint256& hash = ws.m_hash; // Copy/alias what we need out of args - CValidationState &state = args.m_state; + TxValidationState &state = args.m_state; bool* pfMissingInputs = args.m_missing_inputs; const int64_t nAcceptTime = args.m_accept_time; const bool bypass_limits = args.m_bypass_limits; @@ -563,29 +563,29 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) // Coinbase is only valid in a block, not as a loose transaction if (tx.IsCoinBase()) - return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "coinbase"); + return state.Invalid(TxValidationResult::TX_CONSENSUS, false, "coinbase"); // Rather not work on nonstandard transactions (unless -testnet/-regtest) std::string reason; if (fRequireStandard && !IsStandardTx(tx, reason)) - return state.Invalid(ValidationInvalidReason::TX_NOT_STANDARD, false, reason); + return state.Invalid(TxValidationResult::TX_NOT_STANDARD, false, reason); // Do not work on transactions that are too small. // A transaction with 1 segwit input and 1 P2WPHK output has non-witness size of 82 bytes. // Transactions smaller than this are not relayed to mitigate CVE-2017-12842 by not relaying // 64-byte transactions. if (::GetSerializeSize(tx, PROTOCOL_VERSION | SERIALIZE_TRANSACTION_NO_WITNESS) < MIN_STANDARD_TX_NONWITNESS_SIZE) - return state.Invalid(ValidationInvalidReason::TX_NOT_STANDARD, false, "tx-size-small"); + return state.Invalid(TxValidationResult::TX_NOT_STANDARD, false, "tx-size-small"); // Only accept nLockTime-using transactions that can be mined in the next // block; we don't want our mempool filled up with transactions that can't // be mined yet. if (!CheckFinalTx(tx, STANDARD_LOCKTIME_VERIFY_FLAGS)) - return state.Invalid(ValidationInvalidReason::TX_PREMATURE_SPEND, false, "non-final"); + return state.Invalid(TxValidationResult::TX_PREMATURE_SPEND, false, "non-final"); // is it already in the memory pool? if (m_pool.exists(hash)) { - return state.Invalid(ValidationInvalidReason::TX_CONFLICT, false, "txn-already-in-mempool"); + return state.Invalid(TxValidationResult::TX_CONFLICT, false, "txn-already-in-mempool"); } // Check for conflicts with in-memory transactions @@ -617,7 +617,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) } } if (fReplacementOptOut) { - return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, "txn-mempool-conflict"); + return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, false, "txn-mempool-conflict"); } setConflicts.insert(ptxConflicting->GetHash()); @@ -643,7 +643,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) for (size_t out = 0; out < tx.vout.size(); out++) { // Optimistically just do efficient check of cache for outputs if (coins_cache.HaveCoinInCache(COutPoint(hash, out))) { - return state.Invalid(ValidationInvalidReason::TX_CONFLICT, false, "txn-already-known"); + return state.Invalid(TxValidationResult::TX_CONFLICT, false, "txn-already-known"); } } // Otherwise assume this might be an orphan tx for which we just haven't seen parents yet @@ -668,7 +668,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) // Must keep pool.cs for this unless we change CheckSequenceLocks to take a // CoinsViewCache instead of create its own if (!CheckSequenceLocks(m_pool, tx, STANDARD_LOCKTIME_VERIFY_FLAGS, &lp)) - return state.Invalid(ValidationInvalidReason::TX_PREMATURE_SPEND, false, "non-BIP68-final"); + return state.Invalid(TxValidationResult::TX_PREMATURE_SPEND, false, "non-BIP68-final"); CAmount nFees = 0; if (!Consensus::CheckTxInputs(tx, state, m_view, GetSpendHeight(m_view), nFees)) { @@ -677,11 +677,11 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) // Check for non-standard pay-to-script-hash in inputs if (fRequireStandard && !AreInputsStandard(tx, m_view)) - return state.Invalid(ValidationInvalidReason::TX_NOT_STANDARD, false, "bad-txns-nonstandard-inputs"); + return state.Invalid(TxValidationResult::TX_NOT_STANDARD, false, "bad-txns-nonstandard-inputs"); // Check for non-standard witness in P2WSH if (tx.HasWitness() && fRequireStandard && !IsWitnessStandard(tx, m_view)) - return state.Invalid(ValidationInvalidReason::TX_WITNESS_MUTATED, false, "bad-witness-nonstandard"); + return state.Invalid(TxValidationResult::TX_WITNESS_MUTATED, false, "bad-witness-nonstandard"); int64_t nSigOpsCost = GetTransactionSigOpCost(tx, m_view, STANDARD_SCRIPT_VERIFY_FLAGS); @@ -705,7 +705,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) unsigned int nSize = entry->GetTxSize(); if (nSigOpsCost > MAX_STANDARD_TX_SIGOPS_COST) - return state.Invalid(ValidationInvalidReason::TX_NOT_STANDARD, false, "bad-txns-too-many-sigops", + return state.Invalid(TxValidationResult::TX_NOT_STANDARD, false, "bad-txns-too-many-sigops", strprintf("%d", nSigOpsCost)); // No transactions are allowed below minRelayTxFee except from disconnected @@ -713,7 +713,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) if (!bypass_limits && !CheckFeeRate(nSize, nModifiedFees, state)) return false; if (nAbsurdFee && nFees > nAbsurdFee) - return state.Invalid(ValidationInvalidReason::TX_NOT_STANDARD, false, + return state.Invalid(TxValidationResult::TX_NOT_STANDARD, false, "absurdly-high-fee", strprintf("%d > %d", nFees, nAbsurdFee)); const CTxMemPool::setEntries setIterConflicting = m_pool.GetIterSet(setConflicts); @@ -771,7 +771,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) // this, see https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2018-November/016518.html if (nSize > EXTRA_DESCENDANT_TX_SIZE_LIMIT || !m_pool.CalculateMemPoolAncestors(*entry, setAncestors, 2, m_limit_ancestor_size, m_limit_descendants + 1, m_limit_descendant_size + EXTRA_DESCENDANT_TX_SIZE_LIMIT, dummy_err_string)) { - return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, "too-long-mempool-chain", errString); + return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, false, "too-long-mempool-chain", errString); } } @@ -784,7 +784,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) const uint256 &hashAncestor = ancestorIt->GetTx().GetHash(); if (setConflicts.count(hashAncestor)) { - return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-txns-spends-conflicting-tx", + return state.Invalid(TxValidationResult::TX_CONSENSUS, false, "bad-txns-spends-conflicting-tx", strprintf("%s spends conflicting transaction %s", hash.ToString(), hashAncestor.ToString())); @@ -824,7 +824,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) CFeeRate oldFeeRate(mi->GetModifiedFee(), mi->GetTxSize()); if (newFeeRate <= oldFeeRate) { - return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, "insufficient fee", + return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, false, "insufficient fee", strprintf("rejecting replacement %s; new feerate %s <= old feerate %s", hash.ToString(), newFeeRate.ToString(), @@ -852,7 +852,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) nConflictingSize += it->GetTxSize(); } } else { - return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, "too many potential replacements", + return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, false, "too many potential replacements", strprintf("rejecting replacement %s; too many potential replacements (%d > %d)\n", hash.ToString(), nConflictingCount, @@ -876,7 +876,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) // it's cheaper to just check if the new input refers to a // tx that's in the mempool. if (m_pool.exists(tx.vin[j].prevout.hash)) { - return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, "replacement-adds-unconfirmed", + return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, false, "replacement-adds-unconfirmed", strprintf("replacement %s adds unconfirmed input, idx %d", hash.ToString(), j)); } @@ -888,7 +888,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) // transactions would not be paid for. if (nModifiedFees < nConflictingFees) { - return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, "insufficient fee", + return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, false, "insufficient fee", strprintf("rejecting replacement %s, less fees than conflicting txs; %s < %s", hash.ToString(), FormatMoney(nModifiedFees), FormatMoney(nConflictingFees))); } @@ -898,7 +898,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) CAmount nDeltaFees = nModifiedFees - nConflictingFees; if (nDeltaFees < ::incrementalRelayFee.GetFee(nSize)) { - return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, "insufficient fee", + return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, false, "insufficient fee", strprintf("rejecting replacement %s, not enough additional fees to relay; %s < %s", hash.ToString(), FormatMoney(nDeltaFees), @@ -912,7 +912,7 @@ bool MemPoolAccept::PolicyScriptChecks(ATMPArgs& args, Workspace& ws, Precompute { const CTransaction& tx = *ws.m_ptx; - CValidationState &state = args.m_state; + TxValidationState &state = args.m_state; constexpr unsigned int scriptVerifyFlags = STANDARD_SCRIPT_VERIFY_FLAGS; @@ -922,14 +922,13 @@ bool MemPoolAccept::PolicyScriptChecks(ATMPArgs& args, Workspace& ws, Precompute // SCRIPT_VERIFY_CLEANSTACK requires SCRIPT_VERIFY_WITNESS, so we // need to turn both off, and compare against just turning off CLEANSTACK // to see if the failure is specifically due to witness validation. - CValidationState stateDummy; // Want reported failures to be from first CheckInputs - if (!tx.HasWitness() && CheckInputs(tx, stateDummy, m_view, scriptVerifyFlags & ~(SCRIPT_VERIFY_WITNESS | SCRIPT_VERIFY_CLEANSTACK), true, false, txdata) && - !CheckInputs(tx, stateDummy, m_view, scriptVerifyFlags & ~SCRIPT_VERIFY_CLEANSTACK, true, false, txdata)) { + TxValidationState state_dummy; // Want reported failures to be from first CheckInputs + if (!tx.HasWitness() && CheckInputs(tx, state_dummy, m_view, scriptVerifyFlags & ~(SCRIPT_VERIFY_WITNESS | SCRIPT_VERIFY_CLEANSTACK), true, false, txdata) && + !CheckInputs(tx, state_dummy, m_view, scriptVerifyFlags & ~SCRIPT_VERIFY_CLEANSTACK, true, false, txdata)) { // Only the witness is missing, so the transaction itself may be fine. - state.Invalid(ValidationInvalidReason::TX_WITNESS_MUTATED, false, + state.Invalid(TxValidationResult::TX_WITNESS_MUTATED, false, state.GetRejectReason(), state.GetDebugMessage()); } - assert(IsTransactionReason(state.GetReason())); return false; // state filled in by CheckInputs } @@ -941,7 +940,7 @@ bool MemPoolAccept::ConsensusScriptChecks(ATMPArgs& args, Workspace& ws, Precomp const CTransaction& tx = *ws.m_ptx; const uint256& hash = ws.m_hash; - CValidationState &state = args.m_state; + TxValidationState &state = args.m_state; const CChainParams& chainparams = args.m_chainparams; // Check again against the current block tip's script verification @@ -972,7 +971,7 @@ bool MemPoolAccept::Finalize(ATMPArgs& args, Workspace& ws) { const CTransaction& tx = *ws.m_ptx; const uint256& hash = ws.m_hash; - CValidationState &state = args.m_state; + TxValidationState &state = args.m_state; const bool bypass_limits = args.m_bypass_limits; CTxMemPool::setEntries& allConflicting = ws.m_all_conflicting; @@ -1010,7 +1009,7 @@ bool MemPoolAccept::Finalize(ATMPArgs& args, Workspace& ws) if (!bypass_limits) { LimitMempoolSize(m_pool, gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000, std::chrono::hours{gArgs.GetArg("-mempoolexpiry", DEFAULT_MEMPOOL_EXPIRY)}); if (!m_pool.exists(hash)) - return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, "mempool full"); + return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, false, "mempool full"); } return true; } @@ -1047,7 +1046,7 @@ bool MemPoolAccept::AcceptSingleTransaction(const CTransactionRef& ptx, ATMPArgs } // anon namespace /** (try to) add transaction to memory pool with a specified acceptance time **/ -static bool AcceptToMemoryPoolWithTime(const CChainParams& chainparams, CTxMemPool& pool, CValidationState &state, const CTransactionRef &tx, +static bool AcceptToMemoryPoolWithTime(const CChainParams& chainparams, CTxMemPool& pool, TxValidationState &state, const CTransactionRef &tx, bool* pfMissingInputs, int64_t nAcceptTime, std::list* plTxnReplaced, bool bypass_limits, const CAmount nAbsurdFee, bool test_accept) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { @@ -1064,12 +1063,12 @@ static bool AcceptToMemoryPoolWithTime(const CChainParams& chainparams, CTxMemPo ::ChainstateActive().CoinsTip().Uncache(hashTx); } // After we've (potentially) uncached entries, ensure our coins cache is still within its size limits - CValidationState stateDummy; - ::ChainstateActive().FlushStateToDisk(chainparams, stateDummy, FlushStateMode::PERIODIC); + BlockValidationState state_dummy; + ::ChainstateActive().FlushStateToDisk(chainparams, state_dummy, FlushStateMode::PERIODIC); return res; } -bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransactionRef &tx, +bool AcceptToMemoryPool(CTxMemPool& pool, TxValidationState &state, const CTransactionRef &tx, bool* pfMissingInputs, std::list* plTxnReplaced, bool bypass_limits, const CAmount nAbsurdFee, bool test_accept) { @@ -1419,8 +1418,8 @@ void static InvalidChainFound(CBlockIndex* pindexNew) EXCLUSIVE_LOCKS_REQUIRED(c CheckForkWarningConditions(); } -void CChainState::InvalidBlockFound(CBlockIndex *pindex, const CValidationState &state) { - if (state.GetReason() != ValidationInvalidReason::BLOCK_MUTATED) { +void CChainState::InvalidBlockFound(CBlockIndex *pindex, const BlockValidationState &state) { + if (state.GetResult() != BlockValidationResult::BLOCK_MUTATED) { pindex->nStatus |= BLOCK_FAILED_VALID; m_blockman.m_failed_blocks.insert(pindex); setDirtyBlockIndex.insert(pindex); @@ -1493,7 +1492,7 @@ void InitScriptExecutionCache() { * * Non-static (and re-declared) in src/test/txvalidationcache_tests.cpp */ -bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsViewCache &inputs, unsigned int flags, bool cacheSigStore, bool cacheFullScriptStore, PrecomputedTransactionData& txdata, std::vector *pvChecks) EXCLUSIVE_LOCKS_REQUIRED(cs_main) +bool CheckInputs(const CTransaction& tx, TxValidationState &state, const CCoinsViewCache &inputs, unsigned int flags, bool cacheSigStore, bool cacheFullScriptStore, PrecomputedTransactionData& txdata, std::vector *pvChecks) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { if (tx.IsCoinBase()) return true; @@ -1545,10 +1544,10 @@ bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsVi CScriptCheck check2(coin.out, tx, i, flags & ~STANDARD_NOT_MANDATORY_VERIFY_FLAGS, cacheSigStore, &txdata); if (check2()) - return state.Invalid(ValidationInvalidReason::TX_NOT_STANDARD, false, strprintf("non-mandatory-script-verify-flag (%s)", ScriptErrorString(check.GetScriptError()))); + return state.Invalid(TxValidationResult::TX_NOT_STANDARD, false, strprintf("non-mandatory-script-verify-flag (%s)", ScriptErrorString(check.GetScriptError()))); } // MANDATORY flag failures correspond to - // ValidationInvalidReason::CONSENSUS. Because CONSENSUS + // TxValidationResult::TX_CONSENSUS. Because CONSENSUS // failures are the most serious case of validation // failures, we may need to consider using // RECENT_CONSENSUS_CHANGE for any script failure that @@ -1556,7 +1555,7 @@ bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsVi // support, to avoid splitting the network (but this // depends on the details of how net_processing handles // such errors). - return state.Invalid(ValidationInvalidReason::CONSENSUS, false, strprintf("mandatory-script-verify-flag-failed (%s)", ScriptErrorString(check.GetScriptError()))); + return state.Invalid(TxValidationResult::TX_CONSENSUS, false, strprintf("mandatory-script-verify-flag-failed (%s)", ScriptErrorString(check.GetScriptError()))); } } @@ -1641,7 +1640,7 @@ static bool AbortNode(const std::string& strMessage, const std::string& userMess return false; } -static bool AbortNode(CValidationState& state, const std::string& strMessage, const std::string& userMessage = "", unsigned int prefix = 0) +static bool AbortNode(BlockValidationState& state, const std::string& strMessage, const std::string& userMessage = "", unsigned int prefix = 0) { AbortNode(strMessage, userMessage, prefix); return state.Error(strMessage); @@ -1755,9 +1754,9 @@ void static FlushBlockFile(bool fFinalize = false) } } -static bool FindUndoPos(CValidationState &state, int nFile, FlatFilePos &pos, unsigned int nAddSize); +static bool FindUndoPos(BlockValidationState &state, int nFile, FlatFilePos &pos, unsigned int nAddSize); -static bool WriteUndoDataForBlock(const CBlockUndo& blockundo, CValidationState& state, CBlockIndex* pindex, const CChainParams& chainparams) +static bool WriteUndoDataForBlock(const CBlockUndo& blockundo, BlockValidationState& state, CBlockIndex* pindex, const CChainParams& chainparams) { // Write undo information to disk if (pindex->GetUndoPos().IsNull()) { @@ -1896,7 +1895,7 @@ static int64_t nBlocksTotal = 0; /** Apply the effects of this block (with given index) on the UTXO set represented by coins. * Validity checks that depend on the UTXO set are also done; ConnectBlock() * can fail if those validity checks fail (among other reasons). */ -bool CChainState::ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pindex, +bool CChainState::ConnectBlock(const CBlock& block, BlockValidationState& state, CBlockIndex* pindex, CCoinsViewCache& view, const CChainParams& chainparams, bool fJustCheck) { AssertLockHeld(cs_main); @@ -1918,7 +1917,7 @@ bool CChainState::ConnectBlock(const CBlock& block, CValidationState& state, CBl // re-enforce that rule here (at least until we make it impossible for // GetAdjustedTime() to go backward). if (!CheckBlock(block, state, chainparams.GetConsensus(), !fJustCheck, !fJustCheck)) { - if (state.GetReason() == ValidationInvalidReason::BLOCK_MUTATED) { + if (state.GetResult() == BlockValidationResult::BLOCK_MUTATED) { // We don't write down blocks to disk if they may have been // corrupted, so this should be impossible unless we're having hardware // problems. @@ -2058,7 +2057,7 @@ bool CChainState::ConnectBlock(const CBlock& block, CValidationState& state, CBl for (const auto& tx : block.vtx) { for (size_t o = 0; o < tx->vout.size(); o++) { if (view.HaveCoin(COutPoint(tx->GetHash(), o))) { - return state.Invalid(ValidationInvalidReason::CONSENSUS, error("ConnectBlock(): tried to overwrite transaction"), + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, error("ConnectBlock(): tried to overwrite transaction"), "bad-txns-BIP30"); } } @@ -2097,20 +2096,16 @@ bool CChainState::ConnectBlock(const CBlock& block, CValidationState& state, CBl if (!tx.IsCoinBase()) { CAmount txfee = 0; - if (!Consensus::CheckTxInputs(tx, state, view, pindex->nHeight, txfee)) { - if (!IsBlockReason(state.GetReason())) { - // CheckTxInputs may return MISSING_INPUTS or - // PREMATURE_SPEND but we can't return that, as it's not - // defined for a block, so we reset the reason flag to - // CONSENSUS here. - state.Invalid(ValidationInvalidReason::CONSENSUS, false, - state.GetRejectReason(), state.GetDebugMessage()); - } + TxValidationState tx_state; + if (!Consensus::CheckTxInputs(tx, tx_state, view, pindex->nHeight, txfee)) { + // Any transaction validation failure in ConnectBlock is a block consensus failure + state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, false, + tx_state.GetRejectReason(), tx_state.GetDebugMessage()); return error("%s: Consensus::CheckTxInputs: %s, %s", __func__, tx.GetHash().ToString(), FormatStateMessage(state)); } nFees += txfee; if (!MoneyRange(nFees)) { - return state.Invalid(ValidationInvalidReason::CONSENSUS, error("%s: accumulated fee in the block out of range.", __func__), + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, error("%s: accumulated fee in the block out of range.", __func__), "bad-txns-accumulated-fee-outofrange"); } @@ -2123,7 +2118,7 @@ bool CChainState::ConnectBlock(const CBlock& block, CValidationState& state, CBl } if (!SequenceLocks(tx, nLockTimeFlags, &prevheights, *pindex)) { - return state.Invalid(ValidationInvalidReason::CONSENSUS, error("%s: contains a non-BIP68-final transaction", __func__), + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, error("%s: contains a non-BIP68-final transaction", __func__), "bad-txns-nonfinal"); } } @@ -2134,7 +2129,7 @@ bool CChainState::ConnectBlock(const CBlock& block, CValidationState& state, CBl // * witness (when witness enabled in flags and excludes coinbase) nSigOpsCost += GetTransactionSigOpCost(tx, view, flags); if (nSigOpsCost > MAX_BLOCK_SIGOPS_COST) - return state.Invalid(ValidationInvalidReason::CONSENSUS, error("ConnectBlock(): too many sigops"), + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, error("ConnectBlock(): too many sigops"), "bad-blk-sigops"); txdata.emplace_back(tx); @@ -2142,17 +2137,11 @@ bool CChainState::ConnectBlock(const CBlock& block, CValidationState& state, CBl { std::vector vChecks; bool fCacheResults = fJustCheck; /* Don't cache results if we're actually connecting blocks (still consult the cache, though) */ - if (fScriptChecks && !CheckInputs(tx, state, view, flags, fCacheResults, fCacheResults, txdata[i], nScriptCheckThreads ? &vChecks : nullptr)) { - if (state.GetReason() == ValidationInvalidReason::TX_NOT_STANDARD) { - // CheckInputs may return NOT_STANDARD for extra flags we passed, - // but we can't return that, as it's not defined for a block, so - // we reset the reason flag to CONSENSUS here. - // In the event of a future soft-fork, we may need to - // consider whether rewriting to CONSENSUS or - // RECENT_CONSENSUS_CHANGE would be more appropriate. - state.Invalid(ValidationInvalidReason::CONSENSUS, false, - state.GetRejectReason(), state.GetDebugMessage()); - } + TxValidationState tx_state; + if (fScriptChecks && !CheckInputs(tx, tx_state, view, flags, fCacheResults, fCacheResults, txdata[i], nScriptCheckThreads ? &vChecks : nullptr)) { + // Any transaction validation failure in ConnectBlock is a block consensus failure + state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, false, + tx_state.GetRejectReason(), tx_state.GetDebugMessage()); return error("ConnectBlock(): CheckInputs on %s failed with %s", tx.GetHash().ToString(), FormatStateMessage(state)); } @@ -2170,13 +2159,13 @@ bool CChainState::ConnectBlock(const CBlock& block, CValidationState& state, CBl CAmount blockReward = nFees + GetBlockSubsidy(pindex->nHeight, chainparams.GetConsensus()); if (block.vtx[0]->GetValueOut() > blockReward) - return state.Invalid(ValidationInvalidReason::CONSENSUS, + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, error("ConnectBlock(): coinbase pays too much (actual=%d vs limit=%d)", block.vtx[0]->GetValueOut(), blockReward), "bad-cb-amount"); if (!control.Wait()) - return state.Invalid(ValidationInvalidReason::CONSENSUS, error("%s: CheckQueue failed", __func__), "block-validation-failed"); + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, error("%s: CheckQueue failed", __func__), "block-validation-failed"); int64_t nTime4 = GetTimeMicros(); nTimeVerify += nTime4 - nTime2; LogPrint(BCLog::BENCH, " - Verify %u txins: %.2fms (%.3fms/txin) [%.2fs (%.2fms/blk)]\n", nInputs - 1, MILLI * (nTime4 - nTime2), nInputs <= 1 ? 0 : MILLI * (nTime4 - nTime2) / (nInputs-1), nTimeVerify * MICRO, nTimeVerify * MILLI / nBlocksTotal); @@ -2206,7 +2195,7 @@ bool CChainState::ConnectBlock(const CBlock& block, CValidationState& state, CBl bool CChainState::FlushStateToDisk( const CChainParams& chainparams, - CValidationState &state, + BlockValidationState &state, FlushStateMode mode, int nManualPruneHeight) { @@ -2317,7 +2306,7 @@ bool CChainState::FlushStateToDisk( } void CChainState::ForceFlushStateToDisk() { - CValidationState state; + BlockValidationState state; const CChainParams& chainparams = Params(); if (!this->FlushStateToDisk(chainparams, state, FlushStateMode::ALWAYS)) { LogPrintf("%s: failed to flush state (%s)\n", __func__, FormatStateMessage(state)); @@ -2325,7 +2314,7 @@ void CChainState::ForceFlushStateToDisk() { } void CChainState::PruneAndFlush() { - CValidationState state; + BlockValidationState state; fCheckForPruning = true; const CChainParams& chainparams = Params(); @@ -2411,7 +2400,7 @@ void static UpdateTip(const CBlockIndex* pindexNew, const CChainParams& chainPar * disconnectpool (note that the caller is responsible for mempool consistency * in any case). */ -bool CChainState::DisconnectTip(CValidationState& state, const CChainParams& chainparams, DisconnectedBlockTransactions *disconnectpool) +bool CChainState::DisconnectTip(BlockValidationState& state, const CChainParams& chainparams, DisconnectedBlockTransactions *disconnectpool) { CBlockIndex *pindexDelete = m_chain.Tip(); assert(pindexDelete); @@ -2531,7 +2520,7 @@ public: * * The block is added to connectTrace if connection succeeds. */ -bool CChainState::ConnectTip(CValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexNew, const std::shared_ptr& pblock, ConnectTrace& connectTrace, DisconnectedBlockTransactions &disconnectpool) +bool CChainState::ConnectTip(BlockValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexNew, const std::shared_ptr& pblock, ConnectTrace& connectTrace, DisconnectedBlockTransactions &disconnectpool) { assert(pindexNew->pprev == m_chain.Tip()); // Read block from disk. @@ -2663,7 +2652,7 @@ void CChainState::PruneBlockIndexCandidates() { * * @returns true unless a system error occurred */ -bool CChainState::ActivateBestChainStep(CValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexMostWork, const std::shared_ptr& pblock, bool& fInvalidFound, ConnectTrace& connectTrace) +bool CChainState::ActivateBestChainStep(BlockValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexMostWork, const std::shared_ptr& pblock, bool& fInvalidFound, ConnectTrace& connectTrace) { AssertLockHeld(cs_main); @@ -2710,10 +2699,10 @@ bool CChainState::ActivateBestChainStep(CValidationState& state, const CChainPar if (!ConnectTip(state, chainparams, pindexConnect, pindexConnect == pindexMostWork ? pblock : std::shared_ptr(), connectTrace, disconnectpool)) { if (state.IsInvalid()) { // The block violates a consensus rule. - if (state.GetReason() != ValidationInvalidReason::BLOCK_MUTATED) { + if (state.GetResult() != BlockValidationResult::BLOCK_MUTATED) { InvalidChainFound(vpindexToConnect.front()); } - state = CValidationState(); + state = BlockValidationState(); fInvalidFound = true; fContinue = false; break; @@ -2781,7 +2770,7 @@ static void LimitValidationInterfaceQueue() LOCKS_EXCLUDED(cs_main) { } } -bool CChainState::ActivateBestChain(CValidationState &state, const CChainParams& chainparams, std::shared_ptr pblock) { +bool CChainState::ActivateBestChain(BlockValidationState &state, const CChainParams& chainparams, std::shared_ptr pblock) { // Note that while we're often called here from ProcessNewBlock, this is // far from a guarantee. Things in the P2P/RPC will often end up calling // us in the middle of ProcessNewBlock - do not assume pblock is set @@ -2881,11 +2870,11 @@ bool CChainState::ActivateBestChain(CValidationState &state, const CChainParams& return true; } -bool ActivateBestChain(CValidationState &state, const CChainParams& chainparams, std::shared_ptr pblock) { +bool ActivateBestChain(BlockValidationState &state, const CChainParams& chainparams, std::shared_ptr pblock) { return ::ChainstateActive().ActivateBestChain(state, chainparams, std::move(pblock)); } -bool CChainState::PreciousBlock(CValidationState& state, const CChainParams& params, CBlockIndex *pindex) +bool CChainState::PreciousBlock(BlockValidationState& state, const CChainParams& params, CBlockIndex *pindex) { { LOCK(cs_main); @@ -2913,11 +2902,11 @@ bool CChainState::PreciousBlock(CValidationState& state, const CChainParams& par return ActivateBestChain(state, params, std::shared_ptr()); } -bool PreciousBlock(CValidationState& state, const CChainParams& params, CBlockIndex *pindex) { +bool PreciousBlock(BlockValidationState& state, const CChainParams& params, CBlockIndex *pindex) { return ::ChainstateActive().PreciousBlock(state, params, pindex); } -bool CChainState::InvalidateBlock(CValidationState& state, const CChainParams& chainparams, CBlockIndex *pindex) +bool CChainState::InvalidateBlock(BlockValidationState& state, const CChainParams& chainparams, CBlockIndex *pindex) { CBlockIndex* to_mark_failed = pindex; bool pindex_was_in_chain = false; @@ -3053,7 +3042,7 @@ bool CChainState::InvalidateBlock(CValidationState& state, const CChainParams& c return true; } -bool InvalidateBlock(CValidationState& state, const CChainParams& chainparams, CBlockIndex *pindex) { +bool InvalidateBlock(BlockValidationState& state, const CChainParams& chainparams, CBlockIndex *pindex) { return ::ChainstateActive().InvalidateBlock(state, chainparams, pindex); } @@ -3227,7 +3216,7 @@ static bool FindBlockPos(FlatFilePos &pos, unsigned int nAddSize, unsigned int n return true; } -static bool FindUndoPos(CValidationState &state, int nFile, FlatFilePos &pos, unsigned int nAddSize) +static bool FindUndoPos(BlockValidationState &state, int nFile, FlatFilePos &pos, unsigned int nAddSize) { pos.nFile = nFile; @@ -3249,16 +3238,16 @@ static bool FindUndoPos(CValidationState &state, int nFile, FlatFilePos &pos, un return true; } -static bool CheckBlockHeader(const CBlockHeader& block, CValidationState& state, const Consensus::Params& consensusParams, bool fCheckPOW = true) +static bool CheckBlockHeader(const CBlockHeader& block, BlockValidationState& state, const Consensus::Params& consensusParams, bool fCheckPOW = true) { // Check proof of work matches claimed amount if (fCheckPOW && !CheckProofOfWork(block.GetHash(), block.nBits, consensusParams)) - return state.Invalid(ValidationInvalidReason::BLOCK_INVALID_HEADER, false, "high-hash", "proof of work failed"); + return state.Invalid(BlockValidationResult::BLOCK_INVALID_HEADER, false, "high-hash", "proof of work failed"); return true; } -bool CheckBlock(const CBlock& block, CValidationState& state, const Consensus::Params& consensusParams, bool fCheckPOW, bool fCheckMerkleRoot) +bool CheckBlock(const CBlock& block, BlockValidationState& state, const Consensus::Params& consensusParams, bool fCheckPOW, bool fCheckMerkleRoot) { // These are checks that are independent of context. @@ -3275,13 +3264,13 @@ bool CheckBlock(const CBlock& block, CValidationState& state, const Consensus::P bool mutated; uint256 hashMerkleRoot2 = BlockMerkleRoot(block, &mutated); if (block.hashMerkleRoot != hashMerkleRoot2) - return state.Invalid(ValidationInvalidReason::BLOCK_MUTATED, false, "bad-txnmrklroot", "hashMerkleRoot mismatch"); + return state.Invalid(BlockValidationResult::BLOCK_MUTATED, false, "bad-txnmrklroot", "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.Invalid(ValidationInvalidReason::BLOCK_MUTATED, false, "bad-txns-duplicate", "duplicate transaction"); + return state.Invalid(BlockValidationResult::BLOCK_MUTATED, false, "bad-txns-duplicate", "duplicate transaction"); } // All potential-corruption validation must be done before we do any @@ -3292,28 +3281,34 @@ bool CheckBlock(const CBlock& block, CValidationState& state, const Consensus::P // Size limits if (block.vtx.empty() || block.vtx.size() * WITNESS_SCALE_FACTOR > MAX_BLOCK_WEIGHT || ::GetSerializeSize(block, PROTOCOL_VERSION | SERIALIZE_TRANSACTION_NO_WITNESS) * WITNESS_SCALE_FACTOR > MAX_BLOCK_WEIGHT) - return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-blk-length", "size limits failed"); + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, false, "bad-blk-length", "size limits failed"); // First transaction must be coinbase, the rest must not be if (block.vtx.empty() || !block.vtx[0]->IsCoinBase()) - return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-cb-missing", "first tx is not coinbase"); + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, false, "bad-cb-missing", "first tx is not coinbase"); for (unsigned int i = 1; i < block.vtx.size(); i++) if (block.vtx[i]->IsCoinBase()) - return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-cb-multiple", "more than one coinbase"); + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, false, "bad-cb-multiple", "more than one coinbase"); // Check transactions - for (const auto& tx : block.vtx) - if (!CheckTransaction(*tx, state)) - return state.Invalid(state.GetReason(), false, state.GetRejectReason(), - strprintf("Transaction check failed (tx hash %s) %s", tx->GetHash().ToString(), state.GetDebugMessage())); - + // Must check for duplicate inputs (see CVE-2018-17144) + for (const auto& tx : block.vtx) { + TxValidationState tx_state; + if (!CheckTransaction(*tx, tx_state)) { + // CheckBlock() does context-free validation checks. The only + // possible failures are consensus failures. + assert(tx_state.GetResult() == TxValidationResult::TX_CONSENSUS); + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, false, tx_state.GetRejectReason(), + strprintf("Transaction check failed (tx hash %s) %s", tx->GetHash().ToString(), tx_state.GetDebugMessage())); + } + } unsigned int nSigOps = 0; for (const auto& tx : block.vtx) { nSigOps += GetLegacySigOpCount(*tx); } if (nSigOps * WITNESS_SCALE_FACTOR > MAX_BLOCK_SIGOPS_COST) - return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-blk-sigops", "out-of-bounds SigOpCount"); + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, false, "bad-blk-sigops", "out-of-bounds SigOpCount"); if (fCheckPOW && fCheckMerkleRoot) block.fChecked = true; @@ -3406,7 +3401,7 @@ static CBlockIndex* GetLastCheckpoint(const CCheckpointData& data) EXCLUSIVE_LOC * in ConnectBlock(). * Note that -reindex-chainstate skips the validation that happens here! */ -static bool ContextualCheckBlockHeader(const CBlockHeader& block, CValidationState& state, const CChainParams& params, const CBlockIndex* pindexPrev, int64_t nAdjustedTime) EXCLUSIVE_LOCKS_REQUIRED(cs_main) +static bool ContextualCheckBlockHeader(const CBlockHeader& block, BlockValidationState& state, const CChainParams& params, const CBlockIndex* pindexPrev, int64_t nAdjustedTime) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { assert(pindexPrev != nullptr); const int nHeight = pindexPrev->nHeight + 1; @@ -3414,7 +3409,7 @@ static bool ContextualCheckBlockHeader(const CBlockHeader& block, CValidationSta // Check proof of work const Consensus::Params& consensusParams = params.GetConsensus(); if (block.nBits != GetNextWorkRequired(pindexPrev, &block, consensusParams)) - return state.Invalid(ValidationInvalidReason::BLOCK_INVALID_HEADER, false, "bad-diffbits", "incorrect proof of work"); + return state.Invalid(BlockValidationResult::BLOCK_INVALID_HEADER, false, "bad-diffbits", "incorrect proof of work"); // Check against checkpoints if (fCheckpointsEnabled) { @@ -3423,23 +3418,23 @@ static bool ContextualCheckBlockHeader(const CBlockHeader& block, CValidationSta // g_blockman.m_block_index. CBlockIndex* pcheckpoint = GetLastCheckpoint(params.Checkpoints()); if (pcheckpoint && nHeight < pcheckpoint->nHeight) - return state.Invalid(ValidationInvalidReason::BLOCK_CHECKPOINT, error("%s: forked chain older than last checkpoint (height %d)", __func__, nHeight), "bad-fork-prior-to-checkpoint"); + return state.Invalid(BlockValidationResult::BLOCK_CHECKPOINT, error("%s: forked chain older than last checkpoint (height %d)", __func__, nHeight), "bad-fork-prior-to-checkpoint"); } // Check timestamp against prev if (block.GetBlockTime() <= pindexPrev->GetMedianTimePast()) - return state.Invalid(ValidationInvalidReason::BLOCK_INVALID_HEADER, false, "time-too-old", "block's timestamp is too early"); + return state.Invalid(BlockValidationResult::BLOCK_INVALID_HEADER, false, "time-too-old", "block's timestamp is too early"); // Check timestamp if (block.GetBlockTime() > nAdjustedTime + MAX_FUTURE_BLOCK_TIME) - return state.Invalid(ValidationInvalidReason::BLOCK_TIME_FUTURE, false, "time-too-new", "block timestamp too far in the future"); + return state.Invalid(BlockValidationResult::BLOCK_TIME_FUTURE, false, "time-too-new", "block timestamp too far in the future"); // Reject outdated version blocks when 95% (75% on testnet) of the network has upgraded: // check for version 2, 3 and 4 upgrades if((block.nVersion < 2 && nHeight >= consensusParams.BIP34Height) || (block.nVersion < 3 && nHeight >= consensusParams.BIP66Height) || (block.nVersion < 4 && nHeight >= consensusParams.BIP65Height)) - return state.Invalid(ValidationInvalidReason::BLOCK_INVALID_HEADER, false, strprintf("bad-version(0x%08x)", block.nVersion), + return state.Invalid(BlockValidationResult::BLOCK_INVALID_HEADER, false, strprintf("bad-version(0x%08x)", block.nVersion), strprintf("rejected nVersion=0x%08x block", block.nVersion)); return true; @@ -3451,7 +3446,7 @@ static bool ContextualCheckBlockHeader(const CBlockHeader& block, CValidationSta * in ConnectBlock(). * Note that -reindex-chainstate skips the validation that happens here! */ -static bool ContextualCheckBlock(const CBlock& block, CValidationState& state, const Consensus::Params& consensusParams, const CBlockIndex* pindexPrev) +static bool ContextualCheckBlock(const CBlock& block, BlockValidationState& state, const Consensus::Params& consensusParams, const CBlockIndex* pindexPrev) { const int nHeight = pindexPrev == nullptr ? 0 : pindexPrev->nHeight + 1; @@ -3469,7 +3464,7 @@ static bool ContextualCheckBlock(const CBlock& block, CValidationState& state, c // Check that all transactions are finalized for (const auto& tx : block.vtx) { if (!IsFinalTx(*tx, nHeight, nLockTimeCutoff)) { - return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-txns-nonfinal", "non-final transaction"); + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, false, "bad-txns-nonfinal", "non-final transaction"); } } @@ -3479,7 +3474,7 @@ static bool ContextualCheckBlock(const CBlock& block, CValidationState& state, c 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.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-cb-height", "block height mismatch in coinbase"); + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, false, "bad-cb-height", "block height mismatch in coinbase"); } } @@ -3501,11 +3496,11 @@ static bool ContextualCheckBlock(const CBlock& block, CValidationState& state, c // already does not permit it, it is impossible to trigger in the // witness tree. if (block.vtx[0]->vin[0].scriptWitness.stack.size() != 1 || block.vtx[0]->vin[0].scriptWitness.stack[0].size() != 32) { - return state.Invalid(ValidationInvalidReason::BLOCK_MUTATED, false, "bad-witness-nonce-size", strprintf("%s : invalid witness reserved value size", __func__)); + return state.Invalid(BlockValidationResult::BLOCK_MUTATED, false, "bad-witness-nonce-size", strprintf("%s : invalid witness reserved value size", __func__)); } CHash256().Write(hashWitness.begin(), 32).Write(&block.vtx[0]->vin[0].scriptWitness.stack[0][0], 32).Finalize(hashWitness.begin()); if (memcmp(hashWitness.begin(), &block.vtx[0]->vout[commitpos].scriptPubKey[6], 32)) { - return state.Invalid(ValidationInvalidReason::BLOCK_MUTATED, false, "bad-witness-merkle-match", strprintf("%s : witness merkle commitment mismatch", __func__)); + return state.Invalid(BlockValidationResult::BLOCK_MUTATED, false, "bad-witness-merkle-match", strprintf("%s : witness merkle commitment mismatch", __func__)); } fHaveWitness = true; } @@ -3515,7 +3510,7 @@ static bool ContextualCheckBlock(const CBlock& block, CValidationState& state, c if (!fHaveWitness) { for (const auto& tx : block.vtx) { if (tx->HasWitness()) { - return state.Invalid(ValidationInvalidReason::BLOCK_MUTATED, false, "unexpected-witness", strprintf("%s : unexpected witness data found", __func__)); + return state.Invalid(BlockValidationResult::BLOCK_MUTATED, false, "unexpected-witness", strprintf("%s : unexpected witness data found", __func__)); } } } @@ -3527,13 +3522,13 @@ static bool ContextualCheckBlock(const CBlock& block, CValidationState& state, c // the block hash, so we couldn't mark the block as permanently // failed). if (GetBlockWeight(block) > MAX_BLOCK_WEIGHT) { - return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-blk-weight", strprintf("%s : weight limit failed", __func__)); + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, false, "bad-blk-weight", strprintf("%s : weight limit failed", __func__)); } return true; } -bool BlockManager::AcceptBlockHeader(const CBlockHeader& block, CValidationState& state, const CChainParams& chainparams, CBlockIndex** ppindex) +bool BlockManager::AcceptBlockHeader(const CBlockHeader& block, BlockValidationState& state, const CChainParams& chainparams, CBlockIndex** ppindex) { AssertLockHeld(cs_main); // Check for duplicate @@ -3547,7 +3542,7 @@ bool BlockManager::AcceptBlockHeader(const CBlockHeader& block, CValidationState if (ppindex) *ppindex = pindex; if (pindex->nStatus & BLOCK_FAILED_MASK) - return state.Invalid(ValidationInvalidReason::CACHED_INVALID, error("%s: block %s is marked invalid", __func__, hash.ToString()), "duplicate"); + return state.Invalid(BlockValidationResult::BLOCK_CACHED_INVALID, error("%s: block %s is marked invalid", __func__, hash.ToString()), "duplicate"); return true; } @@ -3558,10 +3553,10 @@ bool BlockManager::AcceptBlockHeader(const CBlockHeader& block, CValidationState CBlockIndex* pindexPrev = nullptr; BlockMap::iterator mi = m_block_index.find(block.hashPrevBlock); if (mi == m_block_index.end()) - return state.Invalid(ValidationInvalidReason::BLOCK_MISSING_PREV, error("%s: prev block not found", __func__), "prev-blk-not-found"); + return state.Invalid(BlockValidationResult::BLOCK_MISSING_PREV, error("%s: prev block not found", __func__), "prev-blk-not-found"); pindexPrev = (*mi).second; if (pindexPrev->nStatus & BLOCK_FAILED_MASK) - return state.Invalid(ValidationInvalidReason::BLOCK_INVALID_PREV, error("%s: prev block invalid", __func__), "bad-prevblk"); + return state.Invalid(BlockValidationResult::BLOCK_INVALID_PREV, error("%s: prev block invalid", __func__), "bad-prevblk"); if (!ContextualCheckBlockHeader(block, state, chainparams, pindexPrev, GetAdjustedTime())) return error("%s: Consensus::ContextualCheckBlockHeader: %s, %s", __func__, hash.ToString(), FormatStateMessage(state)); @@ -3598,7 +3593,7 @@ bool BlockManager::AcceptBlockHeader(const CBlockHeader& block, CValidationState setDirtyBlockIndex.insert(invalid_walk); invalid_walk = invalid_walk->pprev; } - return state.Invalid(ValidationInvalidReason::BLOCK_INVALID_PREV, error("%s: prev block invalid", __func__), "bad-prevblk"); + return state.Invalid(BlockValidationResult::BLOCK_INVALID_PREV, error("%s: prev block invalid", __func__), "bad-prevblk"); } } } @@ -3613,7 +3608,7 @@ bool BlockManager::AcceptBlockHeader(const CBlockHeader& block, CValidationState } // Exposed wrapper for AcceptBlockHeader -bool ProcessNewBlockHeaders(const std::vector& headers, CValidationState& state, const CChainParams& chainparams, const CBlockIndex** ppindex, CBlockHeader *first_invalid) +bool ProcessNewBlockHeaders(const std::vector& headers, BlockValidationState& state, const CChainParams& chainparams, const CBlockIndex** ppindex, CBlockHeader *first_invalid) { if (first_invalid != nullptr) first_invalid->SetNull(); { @@ -3660,7 +3655,7 @@ static FlatFilePos SaveBlockToDisk(const CBlock& block, int nHeight, const CChai } /** Store block on disk. If dbp is non-nullptr, the file is known to already reside on disk */ -bool CChainState::AcceptBlock(const std::shared_ptr& pblock, CValidationState& state, const CChainParams& chainparams, CBlockIndex** ppindex, bool fRequested, const FlatFilePos* dbp, bool* fNewBlock) +bool CChainState::AcceptBlock(const std::shared_ptr& pblock, BlockValidationState& state, const CChainParams& chainparams, CBlockIndex** ppindex, bool fRequested, const FlatFilePos* dbp, bool* fNewBlock) { const CBlock& block = *pblock; @@ -3710,8 +3705,7 @@ bool CChainState::AcceptBlock(const std::shared_ptr& pblock, CVali if (!CheckBlock(block, state, chainparams.GetConsensus()) || !ContextualCheckBlock(block, state, chainparams.GetConsensus(), pindex->pprev)) { - assert(IsBlockReason(state.GetReason())); - if (state.IsInvalid() && state.GetReason() != ValidationInvalidReason::BLOCK_MUTATED) { + if (state.IsInvalid() && state.GetResult() != BlockValidationResult::BLOCK_MUTATED) { pindex->nStatus |= BLOCK_FAILED_VALID; setDirtyBlockIndex.insert(pindex); } @@ -3750,7 +3744,7 @@ bool ProcessNewBlock(const CChainParams& chainparams, const std::shared_ptr& setFilesToPrune, int nManualPr /* This function is called from the RPC code for pruneblockchain */ void PruneBlockFilesManual(int nManualPruneHeight) { - CValidationState state; + BlockValidationState state; const CChainParams& chainparams = Params(); if (!::ChainstateActive().FlushStateToDisk( chainparams, state, FlushStateMode::NONE, nManualPruneHeight)) { @@ -4185,7 +4179,7 @@ bool CVerifyDB::VerifyDB(const CChainParams& chainparams, CCoinsView *coinsview, CBlockIndex* pindex; CBlockIndex* pindexFailure = nullptr; int nGoodTransactions = 0; - CValidationState state; + BlockValidationState state; int reportDone = 0; LogPrintf("[0%%]..."); /* Continued */ for (pindex = ::ChainActive().Tip(); pindex && pindex->pprev; pindex = pindex->pprev) { @@ -4429,7 +4423,7 @@ bool CChainState::RewindBlockIndex(const CChainParams& params) } // nHeight is now the height of the first insufficiently-validated block, or tipheight + 1 - CValidationState state; + BlockValidationState state; // Loop until the tip is below nHeight, or we reach a pruned block. while (!ShutdownRequested()) { { @@ -4497,7 +4491,7 @@ bool RewindBlockIndex(const CChainParams& params) { // FlushStateToDisk can possibly read ::ChainActive(). Be conservative // and skip it here, we're about to -reindex-chainstate anyway, so // it'll get called a bunch real soon. - CValidationState state; + BlockValidationState state; if (!::ChainstateActive().FlushStateToDisk(params, state, FlushStateMode::ALWAYS)) { LogPrintf("RewindBlockIndex: unable to flush state to disk (%s)\n", FormatStateMessage(state)); return false; @@ -4649,7 +4643,7 @@ bool LoadExternalBlockFile(const CChainParams& chainparams, FILE* fileIn, FlatFi // process in case the block isn't known yet CBlockIndex* pindex = LookupBlockIndex(hash); if (!pindex || (pindex->nStatus & BLOCK_HAVE_DATA) == 0) { - CValidationState state; + BlockValidationState state; if (::ChainstateActive().AcceptBlock(pblock, state, chainparams, nullptr, true, dbp, nullptr)) { nLoaded++; } @@ -4663,7 +4657,7 @@ bool LoadExternalBlockFile(const CChainParams& chainparams, FILE* fileIn, FlatFi // Activate the genesis block so normal node progress can continue if (hash == chainparams.GetConsensus().hashGenesisBlock) { - CValidationState state; + BlockValidationState state; if (!ActivateBestChain(state, chainparams)) { break; } @@ -4686,7 +4680,7 @@ bool LoadExternalBlockFile(const CChainParams& chainparams, FILE* fileIn, FlatFi LogPrint(BCLog::REINDEX, "%s: Processing out of order child %s of %s\n", __func__, pblockrecursive->GetHash().ToString(), head.ToString()); LOCK(cs_main); - CValidationState dummy; + BlockValidationState dummy; if (::ChainstateActive().AcceptBlock(pblockrecursive, dummy, chainparams, nullptr, true, &it->second, nullptr)) { nLoaded++; @@ -4963,7 +4957,7 @@ bool LoadMempool(CTxMemPool& pool) if (amountdelta) { pool.PrioritiseTransaction(tx->GetHash(), amountdelta); } - CValidationState state; + TxValidationState state; if (nTime + nExpiryTimeout > nNow) { LOCK(cs_main); AcceptToMemoryPoolWithTime(chainparams, pool, state, tx, nullptr /* pfMissingInputs */, nTime, diff --git a/src/validation.h b/src/validation.h index d17a320a47..f346e5caac 100644 --- a/src/validation.h +++ b/src/validation.h @@ -32,6 +32,7 @@ #include class CChainState; +class BlockValidationState; class CBlockIndex; class CBlockTreeDB; class CBlockUndo; @@ -41,7 +42,7 @@ class CConnman; class CScriptCheck; class CBlockPolicyEstimator; class CTxMemPool; -class CValidationState; +class TxValidationState; struct ChainTxData; struct DisconnectedBlockTransactions; @@ -223,7 +224,7 @@ bool ProcessNewBlock(const CChainParams& chainparams, const std::shared_ptr& block, CValidationState& state, const CChainParams& chainparams, const CBlockIndex** ppindex = nullptr, CBlockHeader* first_invalid = nullptr) LOCKS_EXCLUDED(cs_main); +bool ProcessNewBlockHeaders(const std::vector& block, BlockValidationState& state, const CChainParams& chainparams, const CBlockIndex** ppindex = nullptr, CBlockHeader* first_invalid = nullptr) LOCKS_EXCLUDED(cs_main); /** Open a block file (blk?????.dat) */ FILE* OpenBlockFile(const FlatFilePos &pos, bool fReadOnly = false); @@ -248,7 +249,7 @@ bool GetTransaction(const uint256& hash, CTransactionRef& tx, const Consensus::P * May not be called with cs_main held. May not be called in a * validationinterface callback. */ -bool ActivateBestChain(CValidationState& state, const CChainParams& chainparams, std::shared_ptr pblock = std::shared_ptr()); +bool ActivateBestChain(BlockValidationState& state, const CChainParams& chainparams, std::shared_ptr pblock = std::shared_ptr()); CAmount GetBlockSubsidy(int nHeight, const Consensus::Params& consensusParams); /** Guess verification progress (as a fraction between 0.0=genesis and 1.0=current tip). */ @@ -272,7 +273,7 @@ void PruneBlockFilesManual(int nManualPruneHeight); /** (try to) add transaction to memory pool * plTxnReplaced will be appended to with all transactions replaced from mempool **/ -bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransactionRef &tx, +bool AcceptToMemoryPool(CTxMemPool& pool, TxValidationState &state, const CTransactionRef &tx, bool* pfMissingInputs, std::list* plTxnReplaced, bool bypass_limits, const CAmount nAbsurdFee, bool test_accept=false) EXCLUSIVE_LOCKS_REQUIRED(cs_main); @@ -368,10 +369,10 @@ bool UndoReadFromDisk(CBlockUndo& blockundo, const CBlockIndex* pindex); /** Functions for validating blocks and updating the block tree */ /** Context-independent validity checks */ -bool CheckBlock(const CBlock& block, CValidationState& state, const Consensus::Params& consensusParams, bool fCheckPOW = true, bool fCheckMerkleRoot = true); +bool CheckBlock(const CBlock& block, BlockValidationState& state, const Consensus::Params& consensusParams, bool fCheckPOW = true, bool fCheckMerkleRoot = true); /** Check a block is completely valid from start to finish (only works on top of our current best block) */ -bool TestBlockValidity(CValidationState& state, const CChainParams& chainparams, const CBlock& block, CBlockIndex* pindexPrev, bool fCheckPOW = true, bool fCheckMerkleRoot = true) EXCLUSIVE_LOCKS_REQUIRED(cs_main); +bool TestBlockValidity(BlockValidationState& state, const CChainParams& chainparams, const CBlock& block, CBlockIndex* pindexPrev, bool fCheckPOW = true, bool fCheckMerkleRoot = true) EXCLUSIVE_LOCKS_REQUIRED(cs_main); /** Check whether witness commitments are required for a block, and whether to enforce NULLDUMMY (BIP 147) rules. * Note that transaction witness validation rules are always enforced when P2SH is enforced. */ @@ -488,7 +489,7 @@ public: */ bool AcceptBlockHeader( const CBlockHeader& block, - CValidationState& state, + BlockValidationState& state, const CChainParams& chainparams, CBlockIndex** ppindex) EXCLUSIVE_LOCKS_REQUIRED(cs_main); }; @@ -652,7 +653,7 @@ public: */ bool FlushStateToDisk( const CChainParams& chainparams, - CValidationState &state, + BlockValidationState &state, FlushStateMode mode, int nManualPruneHeight = 0); @@ -678,23 +679,23 @@ public: * @returns true unless a system error occurred */ bool ActivateBestChain( - CValidationState& state, + BlockValidationState& state, const CChainParams& chainparams, std::shared_ptr pblock) LOCKS_EXCLUDED(cs_main); - bool AcceptBlock(const std::shared_ptr& pblock, CValidationState& state, const CChainParams& chainparams, CBlockIndex** ppindex, bool fRequested, const FlatFilePos* dbp, bool* fNewBlock) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + bool AcceptBlock(const std::shared_ptr& pblock, BlockValidationState& state, const CChainParams& chainparams, CBlockIndex** ppindex, bool fRequested, const FlatFilePos* dbp, bool* fNewBlock) EXCLUSIVE_LOCKS_REQUIRED(cs_main); // Block (dis)connection on a given view: DisconnectResult DisconnectBlock(const CBlock& block, const CBlockIndex* pindex, CCoinsViewCache& view); - bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pindex, + bool ConnectBlock(const CBlock& block, BlockValidationState& state, CBlockIndex* pindex, CCoinsViewCache& view, const CChainParams& chainparams, bool fJustCheck = false) EXCLUSIVE_LOCKS_REQUIRED(cs_main); // Apply the effects of a block disconnection on the UTXO set. - bool DisconnectTip(CValidationState& state, const CChainParams& chainparams, DisconnectedBlockTransactions* disconnectpool) EXCLUSIVE_LOCKS_REQUIRED(cs_main, ::mempool.cs); + bool DisconnectTip(BlockValidationState& state, const CChainParams& chainparams, DisconnectedBlockTransactions* disconnectpool) EXCLUSIVE_LOCKS_REQUIRED(cs_main, ::mempool.cs); // Manual block validity manipulation: - bool PreciousBlock(CValidationState& state, const CChainParams& params, CBlockIndex* pindex) LOCKS_EXCLUDED(cs_main); - bool InvalidateBlock(CValidationState& state, const CChainParams& chainparams, CBlockIndex* pindex) LOCKS_EXCLUDED(cs_main); + bool PreciousBlock(BlockValidationState& state, const CChainParams& params, CBlockIndex* pindex) LOCKS_EXCLUDED(cs_main); + bool InvalidateBlock(BlockValidationState& state, const CChainParams& chainparams, CBlockIndex* pindex) LOCKS_EXCLUDED(cs_main); void ResetBlockFailureFlags(CBlockIndex* pindex) EXCLUSIVE_LOCKS_REQUIRED(cs_main); /** Replay blocks that aren't fully applied to the database. */ @@ -720,10 +721,10 @@ public: bool LoadChainTip(const CChainParams& chainparams) EXCLUSIVE_LOCKS_REQUIRED(cs_main); private: - bool ActivateBestChainStep(CValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexMostWork, const std::shared_ptr& pblock, bool& fInvalidFound, ConnectTrace& connectTrace) EXCLUSIVE_LOCKS_REQUIRED(cs_main, ::mempool.cs); - bool ConnectTip(CValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexNew, const std::shared_ptr& pblock, ConnectTrace& connectTrace, DisconnectedBlockTransactions& disconnectpool) EXCLUSIVE_LOCKS_REQUIRED(cs_main, ::mempool.cs); + bool ActivateBestChainStep(BlockValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexMostWork, const std::shared_ptr& pblock, bool& fInvalidFound, ConnectTrace& connectTrace) EXCLUSIVE_LOCKS_REQUIRED(cs_main, ::mempool.cs); + bool ConnectTip(BlockValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexNew, const std::shared_ptr& pblock, ConnectTrace& connectTrace, DisconnectedBlockTransactions& disconnectpool) EXCLUSIVE_LOCKS_REQUIRED(cs_main, ::mempool.cs); - void InvalidBlockFound(CBlockIndex *pindex, const CValidationState &state) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + void InvalidBlockFound(CBlockIndex *pindex, const BlockValidationState &state) EXCLUSIVE_LOCKS_REQUIRED(cs_main); CBlockIndex* FindMostWorkChain() EXCLUSIVE_LOCKS_REQUIRED(cs_main); void ReceivedBlockTransactions(const CBlock& block, CBlockIndex* pindexNew, const FlatFilePos& pos, const Consensus::Params& consensusParams) EXCLUSIVE_LOCKS_REQUIRED(cs_main); @@ -738,10 +739,10 @@ private: * May not be called in a * validationinterface callback. */ -bool PreciousBlock(CValidationState& state, const CChainParams& params, CBlockIndex *pindex) LOCKS_EXCLUDED(cs_main); +bool PreciousBlock(BlockValidationState& state, const CChainParams& params, CBlockIndex *pindex) LOCKS_EXCLUDED(cs_main); /** Mark a block as invalid. */ -bool InvalidateBlock(CValidationState& state, const CChainParams& chainparams, CBlockIndex* pindex) LOCKS_EXCLUDED(cs_main); +bool InvalidateBlock(BlockValidationState& state, const CChainParams& chainparams, CBlockIndex* pindex) LOCKS_EXCLUDED(cs_main); /** Remove invalidity status from a block and its descendants. */ void ResetBlockFailureFlags(CBlockIndex* pindex) EXCLUSIVE_LOCKS_REQUIRED(cs_main); diff --git a/src/validationinterface.cpp b/src/validationinterface.cpp index cf4a529a6d..a46b4003f1 100644 --- a/src/validationinterface.cpp +++ b/src/validationinterface.cpp @@ -32,7 +32,7 @@ struct MainSignalsInstance { boost::signals2::signal &)> BlockDisconnected; boost::signals2::signal TransactionRemovedFromMempool; boost::signals2::signal ChainStateFlushed; - boost::signals2::signal BlockChecked; + boost::signals2::signal BlockChecked; boost::signals2::signal&)> NewPoWValidBlock; // We are not allowed to assume the scheduler only runs in one thread, @@ -168,7 +168,7 @@ void CMainSignals::ChainStateFlushed(const CBlockLocator &locator) { }); } -void CMainSignals::BlockChecked(const CBlock& block, const CValidationState& state) { +void CMainSignals::BlockChecked(const CBlock& block, const BlockValidationState& state) { m_internals->BlockChecked(block, state); } diff --git a/src/validationinterface.h b/src/validationinterface.h index 3ce617b827..dc8425869b 100644 --- a/src/validationinterface.h +++ b/src/validationinterface.h @@ -13,12 +13,12 @@ #include extern CCriticalSection cs_main; +class BlockValidationState; class CBlock; class CBlockIndex; struct CBlockLocator; class CConnman; class CValidationInterface; -class CValidationState; class uint256; class CScheduler; class CTxMemPool; @@ -134,11 +134,11 @@ protected: virtual void ChainStateFlushed(const CBlockLocator &locator) {} /** * Notifies listeners of a block validation result. - * If the provided CValidationState IsValid, the provided block + * If the provided BlockValidationState IsValid, the provided block * is guaranteed to be the current best block at the time the * callback was generated (not necessarily now) */ - virtual void BlockChecked(const CBlock&, const CValidationState&) {} + virtual void BlockChecked(const CBlock&, const BlockValidationState&) {} /** * Notifies listeners that a block which builds directly on our current tip * has been received and connected to the headers tree, though not validated yet */ @@ -180,7 +180,7 @@ public: void BlockConnected(const std::shared_ptr &, const CBlockIndex *pindex, const std::shared_ptr> &); void BlockDisconnected(const std::shared_ptr &); void ChainStateFlushed(const CBlockLocator &); - void BlockChecked(const CBlock&, const CValidationState&); + void BlockChecked(const CBlock&, const BlockValidationState&); void NewPoWValidBlock(const CBlockIndex *, const std::shared_ptr&); }; -- cgit v1.2.3 From 067981e49246822421a7bcc720491427e1dba8a3 Mon Sep 17 00:00:00 2001 From: John Newbery Date: Sun, 28 Apr 2019 12:17:56 -0500 Subject: [validation] Tidy Up ValidationResult class Minor style fixups and comment updates. This is purely a style change. There is no change in behavior. --- src/consensus/validation.h | 63 +++++++++++++++++++++------------------------- 1 file changed, 29 insertions(+), 34 deletions(-) (limited to 'src') diff --git a/src/consensus/validation.h b/src/consensus/validation.h index c285817c0d..34bdf36c5f 100644 --- a/src/consensus/validation.h +++ b/src/consensus/validation.h @@ -85,42 +85,34 @@ private: MODE_VALID, //!< everything ok MODE_INVALID, //!< network rule violation (DoS value may be set) MODE_ERROR, //!< run-time error - } mode; - std::string strRejectReason; - std::string strDebugMessage; + } m_mode; + std::string m_reject_reason; + std::string m_debug_message; protected: - bool Invalid(bool ret = false, - const std::string &strRejectReasonIn="", - const std::string &strDebugMessageIn="") { - strRejectReason = strRejectReasonIn; - strDebugMessage = strDebugMessageIn; - if (mode == MODE_ERROR) - return ret; - mode = MODE_INVALID; - return ret; + void Invalid(const std::string &reject_reason="", + const std::string &debug_message="") + { + m_reject_reason = reject_reason; + m_debug_message = debug_message; + if (m_mode != MODE_ERROR) m_mode = MODE_INVALID; } public: // ValidationState is abstract. Have a pure virtual destructor. virtual ~ValidationState() = 0; - ValidationState() : mode(MODE_VALID) {} - bool Error(const std::string& strRejectReasonIn) { - if (mode == MODE_VALID) - strRejectReason = strRejectReasonIn; - mode = MODE_ERROR; + ValidationState() : m_mode(MODE_VALID) {} + bool Error(const std::string& reject_reason) + { + if (m_mode == MODE_VALID) + m_reject_reason = reject_reason; + m_mode = MODE_ERROR; return false; } - bool IsValid() const { - return mode == MODE_VALID; - } - bool IsInvalid() const { - return mode == MODE_INVALID; - } - bool IsError() const { - return mode == MODE_ERROR; - } - std::string GetRejectReason() const { return strRejectReason; } - std::string GetDebugMessage() const { return strDebugMessage; } + bool IsValid() const { return m_mode == MODE_VALID; } + bool IsInvalid() const { return m_mode == MODE_INVALID; } + bool IsError() const { return m_mode == MODE_ERROR; } + std::string GetRejectReason() const { return m_reject_reason; } + std::string GetDebugMessage() const { return m_debug_message; } }; inline ValidationState::~ValidationState() {}; @@ -130,10 +122,12 @@ private: TxValidationResult m_result; public: bool Invalid(TxValidationResult result, bool ret = false, - const std::string &_strRejectReason="", - const std::string &_strDebugMessage="") { + const std::string &reject_reason="", + const std::string &debug_message="") + { m_result = result; - return ValidationState::Invalid(ret, _strRejectReason, _strDebugMessage); + ValidationState::Invalid(reject_reason, debug_message); + return ret; } TxValidationResult GetResult() const { return m_result; } }; @@ -143,10 +137,11 @@ private: BlockValidationResult m_result; public: bool Invalid(BlockValidationResult result, bool ret = false, - const std::string &_strRejectReason="", - const std::string &_strDebugMessage="") { + const std::string &reject_reason="", + const std::string &debug_message="") { m_result = result; - return ValidationState::Invalid(ret, _strRejectReason, _strDebugMessage); + ValidationState::Invalid(reject_reason, debug_message); + return ret; } BlockValidationResult GetResult() const { return m_result; } }; -- cgit v1.2.3 From 1a37de4b3174d19a6d8691ae07e92b32fdfaef11 Mon Sep 17 00:00:00 2001 From: John Newbery Date: Sun, 28 Apr 2019 15:32:34 -0500 Subject: [validation] Remove error() calls from Invalid() calls This is in preparation for the next commit, which removes the useless `ret` parameter from ValidationState::Invalid(). error() is simply a convenience wrapper that calls LogPrintf and returns false. Call LogPrintf explicitly and substitute the error() call for a false bool literal. --- src/validation.cpp | 63 ++++++++++++++++++++++++++++++++---------------------- 1 file changed, 37 insertions(+), 26 deletions(-) (limited to 'src') diff --git a/src/validation.cpp b/src/validation.cpp index 4af2e9d3fc..f21e44d348 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2057,8 +2057,8 @@ bool CChainState::ConnectBlock(const CBlock& block, BlockValidationState& state, for (const auto& tx : block.vtx) { for (size_t o = 0; o < tx->vout.size(); o++) { if (view.HaveCoin(COutPoint(tx->GetHash(), o))) { - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, error("ConnectBlock(): tried to overwrite transaction"), - "bad-txns-BIP30"); + LogPrintf("ERROR: ConnectBlock(): tried to overwrite transaction\n"); + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, false, "bad-txns-BIP30"); } } } @@ -2105,8 +2105,8 @@ bool CChainState::ConnectBlock(const CBlock& block, BlockValidationState& state, } nFees += txfee; if (!MoneyRange(nFees)) { - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, error("%s: accumulated fee in the block out of range.", __func__), - "bad-txns-accumulated-fee-outofrange"); + LogPrintf("ERROR: %s: accumulated fee in the block out of range.\n", __func__); + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, false, "bad-txns-accumulated-fee-outofrange"); } // Check that transaction is BIP68 final @@ -2118,8 +2118,8 @@ bool CChainState::ConnectBlock(const CBlock& block, BlockValidationState& state, } if (!SequenceLocks(tx, nLockTimeFlags, &prevheights, *pindex)) { - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, error("%s: contains a non-BIP68-final transaction", __func__), - "bad-txns-nonfinal"); + LogPrintf("ERROR: %s: contains a non-BIP68-final transaction\n", __func__); + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, false, "bad-txns-nonfinal"); } } @@ -2128,9 +2128,10 @@ bool CChainState::ConnectBlock(const CBlock& block, BlockValidationState& state, // * p2sh (when P2SH enabled in flags and excludes coinbase) // * witness (when witness enabled in flags and excludes coinbase) nSigOpsCost += GetTransactionSigOpCost(tx, view, flags); - if (nSigOpsCost > MAX_BLOCK_SIGOPS_COST) - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, error("ConnectBlock(): too many sigops"), - "bad-blk-sigops"); + if (nSigOpsCost > MAX_BLOCK_SIGOPS_COST) { + LogPrintf("ERROR: ConnectBlock(): too many sigops\n"); + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, false, "bad-blk-sigops"); + } txdata.emplace_back(tx); if (!tx.IsCoinBase()) @@ -2158,14 +2159,15 @@ bool CChainState::ConnectBlock(const CBlock& block, BlockValidationState& state, LogPrint(BCLog::BENCH, " - Connect %u transactions: %.2fms (%.3fms/tx, %.3fms/txin) [%.2fs (%.2fms/blk)]\n", (unsigned)block.vtx.size(), MILLI * (nTime3 - nTime2), MILLI * (nTime3 - nTime2) / block.vtx.size(), nInputs <= 1 ? 0 : MILLI * (nTime3 - nTime2) / (nInputs-1), nTimeConnect * MICRO, nTimeConnect * MILLI / nBlocksTotal); CAmount blockReward = nFees + GetBlockSubsidy(pindex->nHeight, chainparams.GetConsensus()); - if (block.vtx[0]->GetValueOut() > blockReward) - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, - error("ConnectBlock(): coinbase pays too much (actual=%d vs limit=%d)", - block.vtx[0]->GetValueOut(), blockReward), - "bad-cb-amount"); - - if (!control.Wait()) - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, error("%s: CheckQueue failed", __func__), "block-validation-failed"); + if (block.vtx[0]->GetValueOut() > blockReward) { + LogPrintf("ERROR: ConnectBlock(): coinbase pays too much (actual=%d vs limit=%d)\n", block.vtx[0]->GetValueOut(), blockReward); + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, false, "bad-cb-amount"); + } + + if (!control.Wait()) { + LogPrintf("ERROR: %s: CheckQueue failed\n", __func__); + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, false, "block-validation-failed"); + } int64_t nTime4 = GetTimeMicros(); nTimeVerify += nTime4 - nTime2; LogPrint(BCLog::BENCH, " - Verify %u txins: %.2fms (%.3fms/txin) [%.2fs (%.2fms/blk)]\n", nInputs - 1, MILLI * (nTime4 - nTime2), nInputs <= 1 ? 0 : MILLI * (nTime4 - nTime2) / (nInputs-1), nTimeVerify * MICRO, nTimeVerify * MILLI / nBlocksTotal); @@ -3417,8 +3419,10 @@ static bool ContextualCheckBlockHeader(const CBlockHeader& block, BlockValidatio // GetLastCheckpoint finds the last checkpoint in MapCheckpoints that's in our // g_blockman.m_block_index. CBlockIndex* pcheckpoint = GetLastCheckpoint(params.Checkpoints()); - if (pcheckpoint && nHeight < pcheckpoint->nHeight) - return state.Invalid(BlockValidationResult::BLOCK_CHECKPOINT, error("%s: forked chain older than last checkpoint (height %d)", __func__, nHeight), "bad-fork-prior-to-checkpoint"); + if (pcheckpoint && nHeight < pcheckpoint->nHeight) { + LogPrintf("ERROR: %s: forked chain older than last checkpoint (height %d)\n", __func__, nHeight); + return state.Invalid(BlockValidationResult::BLOCK_CHECKPOINT, false, "bad-fork-prior-to-checkpoint"); + } } // Check timestamp against prev @@ -3541,8 +3545,10 @@ bool BlockManager::AcceptBlockHeader(const CBlockHeader& block, BlockValidationS pindex = miSelf->second; if (ppindex) *ppindex = pindex; - if (pindex->nStatus & BLOCK_FAILED_MASK) - return state.Invalid(BlockValidationResult::BLOCK_CACHED_INVALID, error("%s: block %s is marked invalid", __func__, hash.ToString()), "duplicate"); + if (pindex->nStatus & BLOCK_FAILED_MASK) { + LogPrintf("ERROR: %s: block %s is marked invalid\n", __func__, hash.ToString()); + return state.Invalid(BlockValidationResult::BLOCK_CACHED_INVALID, false, "duplicate"); + } return true; } @@ -3552,11 +3558,15 @@ bool BlockManager::AcceptBlockHeader(const CBlockHeader& block, BlockValidationS // Get prev block index CBlockIndex* pindexPrev = nullptr; BlockMap::iterator mi = m_block_index.find(block.hashPrevBlock); - if (mi == m_block_index.end()) - return state.Invalid(BlockValidationResult::BLOCK_MISSING_PREV, error("%s: prev block not found", __func__), "prev-blk-not-found"); + if (mi == m_block_index.end()) { + LogPrintf("ERROR: %s: prev block not found\n", __func__); + return state.Invalid(BlockValidationResult::BLOCK_MISSING_PREV, false, "prev-blk-not-found"); + } pindexPrev = (*mi).second; - if (pindexPrev->nStatus & BLOCK_FAILED_MASK) - return state.Invalid(BlockValidationResult::BLOCK_INVALID_PREV, error("%s: prev block invalid", __func__), "bad-prevblk"); + if (pindexPrev->nStatus & BLOCK_FAILED_MASK) { + LogPrintf("ERROR: %s: prev block invalid\n", __func__); + return state.Invalid(BlockValidationResult::BLOCK_INVALID_PREV, false, "bad-prevblk"); + } if (!ContextualCheckBlockHeader(block, state, chainparams, pindexPrev, GetAdjustedTime())) return error("%s: Consensus::ContextualCheckBlockHeader: %s, %s", __func__, hash.ToString(), FormatStateMessage(state)); @@ -3593,7 +3603,8 @@ bool BlockManager::AcceptBlockHeader(const CBlockHeader& block, BlockValidationS setDirtyBlockIndex.insert(invalid_walk); invalid_walk = invalid_walk->pprev; } - return state.Invalid(BlockValidationResult::BLOCK_INVALID_PREV, error("%s: prev block invalid", __func__), "bad-prevblk"); + LogPrintf("ERROR: %s: prev block invalid\n", __func__); + return state.Invalid(BlockValidationResult::BLOCK_INVALID_PREV, false, "bad-prevblk"); } } } -- cgit v1.2.3 From 7204c6434b944f6ad51b3c895837729d3aa56eea Mon Sep 17 00:00:00 2001 From: John Newbery Date: Sun, 28 Apr 2019 15:40:01 -0500 Subject: [validation] Remove useless ret parameter from Invalid() ValidationState::Invalid() takes a parameter `ret` which is returned to the caller. All call sites set this to false. Remove the `ret` parameter and just return false always. --- src/consensus/tx_check.cpp | 18 +++---- src/consensus/tx_verify.cpp | 10 ++-- src/consensus/validation.h | 8 ++-- src/validation.cpp | 112 ++++++++++++++++++++++---------------------- 4 files changed, 74 insertions(+), 74 deletions(-) (limited to 'src') diff --git a/src/consensus/tx_check.cpp b/src/consensus/tx_check.cpp index a9d5cec4ac..88bb12c713 100644 --- a/src/consensus/tx_check.cpp +++ b/src/consensus/tx_check.cpp @@ -11,24 +11,24 @@ bool CheckTransaction(const CTransaction& tx, TxValidationState& state) { // Basic checks that don't depend on any context if (tx.vin.empty()) - return state.Invalid(TxValidationResult::TX_CONSENSUS, false, "bad-txns-vin-empty"); + return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-txns-vin-empty"); if (tx.vout.empty()) - return state.Invalid(TxValidationResult::TX_CONSENSUS, false, "bad-txns-vout-empty"); + return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-txns-vout-empty"); // Size limits (this doesn't take the witness into account, as that hasn't been checked for malleability) if (::GetSerializeSize(tx, PROTOCOL_VERSION | SERIALIZE_TRANSACTION_NO_WITNESS) * WITNESS_SCALE_FACTOR > MAX_BLOCK_WEIGHT) - return state.Invalid(TxValidationResult::TX_CONSENSUS, false, "bad-txns-oversize"); + return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-txns-oversize"); // Check for negative or overflow output values (see CVE-2010-5139) CAmount nValueOut = 0; for (const auto& txout : tx.vout) { if (txout.nValue < 0) - return state.Invalid(TxValidationResult::TX_CONSENSUS, false, "bad-txns-vout-negative"); + return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-txns-vout-negative"); if (txout.nValue > MAX_MONEY) - return state.Invalid(TxValidationResult::TX_CONSENSUS, false, "bad-txns-vout-toolarge"); + return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-txns-vout-toolarge"); nValueOut += txout.nValue; if (!MoneyRange(nValueOut)) - return state.Invalid(TxValidationResult::TX_CONSENSUS, false, "bad-txns-txouttotal-toolarge"); + return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-txns-txouttotal-toolarge"); } // Check for duplicate inputs (see CVE-2018-17144) @@ -39,19 +39,19 @@ bool CheckTransaction(const CTransaction& tx, TxValidationState& state) std::set vInOutPoints; for (const auto& txin : tx.vin) { if (!vInOutPoints.insert(txin.prevout).second) - return state.Invalid(TxValidationResult::TX_CONSENSUS, false, "bad-txns-inputs-duplicate"); + return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-txns-inputs-duplicate"); } if (tx.IsCoinBase()) { if (tx.vin[0].scriptSig.size() < 2 || tx.vin[0].scriptSig.size() > 100) - return state.Invalid(TxValidationResult::TX_CONSENSUS, false, "bad-cb-length"); + return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-cb-length"); } else { for (const auto& txin : tx.vin) if (txin.prevout.IsNull()) - return state.Invalid(TxValidationResult::TX_CONSENSUS, false, "bad-txns-prevout-null"); + return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-txns-prevout-null"); } return true; diff --git a/src/consensus/tx_verify.cpp b/src/consensus/tx_verify.cpp index 47242aae93..31bdabea28 100644 --- a/src/consensus/tx_verify.cpp +++ b/src/consensus/tx_verify.cpp @@ -160,7 +160,7 @@ bool Consensus::CheckTxInputs(const CTransaction& tx, TxValidationState& state, { // are the actual inputs available? if (!inputs.HaveInputs(tx)) { - return state.Invalid(TxValidationResult::TX_MISSING_INPUTS, false, "bad-txns-inputs-missingorspent", + return state.Invalid(TxValidationResult::TX_MISSING_INPUTS, "bad-txns-inputs-missingorspent", strprintf("%s: inputs missing/spent", __func__)); } @@ -172,27 +172,27 @@ bool Consensus::CheckTxInputs(const CTransaction& tx, TxValidationState& state, // If prev is coinbase, check that it's matured if (coin.IsCoinBase() && nSpendHeight - coin.nHeight < COINBASE_MATURITY) { - return state.Invalid(TxValidationResult::TX_PREMATURE_SPEND, false, "bad-txns-premature-spend-of-coinbase", + return state.Invalid(TxValidationResult::TX_PREMATURE_SPEND, "bad-txns-premature-spend-of-coinbase", strprintf("tried to spend coinbase at depth %d", nSpendHeight - coin.nHeight)); } // Check for negative or overflow input values nValueIn += coin.out.nValue; if (!MoneyRange(coin.out.nValue) || !MoneyRange(nValueIn)) { - return state.Invalid(TxValidationResult::TX_CONSENSUS, false, "bad-txns-inputvalues-outofrange"); + return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-txns-inputvalues-outofrange"); } } const CAmount value_out = tx.GetValueOut(); if (nValueIn < value_out) { - return state.Invalid(TxValidationResult::TX_CONSENSUS, false, "bad-txns-in-belowout", + return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-txns-in-belowout", strprintf("value in (%s) < value out (%s)", FormatMoney(nValueIn), FormatMoney(value_out))); } // Tally transaction fees const CAmount txfee_aux = nValueIn - value_out; if (!MoneyRange(txfee_aux)) { - return state.Invalid(TxValidationResult::TX_CONSENSUS, false, "bad-txns-fee-outofrange"); + return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-txns-fee-outofrange"); } txfee = txfee_aux; diff --git a/src/consensus/validation.h b/src/consensus/validation.h index 34bdf36c5f..cdc3aecc1e 100644 --- a/src/consensus/validation.h +++ b/src/consensus/validation.h @@ -121,13 +121,13 @@ class TxValidationState : public ValidationState { private: TxValidationResult m_result; public: - bool Invalid(TxValidationResult result, bool ret = false, + bool Invalid(TxValidationResult result, const std::string &reject_reason="", const std::string &debug_message="") { m_result = result; ValidationState::Invalid(reject_reason, debug_message); - return ret; + return false; } TxValidationResult GetResult() const { return m_result; } }; @@ -136,12 +136,12 @@ class BlockValidationState : public ValidationState { private: BlockValidationResult m_result; public: - bool Invalid(BlockValidationResult result, bool ret = false, + bool Invalid(BlockValidationResult result, const std::string &reject_reason="", const std::string &debug_message="") { m_result = result; ValidationState::Invalid(reject_reason, debug_message); - return ret; + return false; } BlockValidationResult GetResult() const { return m_result; } }; diff --git a/src/validation.cpp b/src/validation.cpp index f21e44d348..21f7f9c8db 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -506,11 +506,11 @@ private: { CAmount mempoolRejectFee = m_pool.GetMinFee(gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000).GetFee(package_size); if (mempoolRejectFee > 0 && package_fee < mempoolRejectFee) { - return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, false, "mempool min fee not met", strprintf("%d < %d", package_fee, mempoolRejectFee)); + return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "mempool min fee not met", strprintf("%d < %d", package_fee, mempoolRejectFee)); } if (package_fee < ::minRelayTxFee.GetFee(package_size)) { - return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, false, "min relay fee not met", strprintf("%d < %d", package_fee, ::minRelayTxFee.GetFee(package_size))); + return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "min relay fee not met", strprintf("%d < %d", package_fee, ::minRelayTxFee.GetFee(package_size))); } return true; } @@ -563,29 +563,29 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) // Coinbase is only valid in a block, not as a loose transaction if (tx.IsCoinBase()) - return state.Invalid(TxValidationResult::TX_CONSENSUS, false, "coinbase"); + return state.Invalid(TxValidationResult::TX_CONSENSUS, "coinbase"); // Rather not work on nonstandard transactions (unless -testnet/-regtest) std::string reason; if (fRequireStandard && !IsStandardTx(tx, reason)) - return state.Invalid(TxValidationResult::TX_NOT_STANDARD, false, reason); + return state.Invalid(TxValidationResult::TX_NOT_STANDARD, reason); // Do not work on transactions that are too small. // A transaction with 1 segwit input and 1 P2WPHK output has non-witness size of 82 bytes. // Transactions smaller than this are not relayed to mitigate CVE-2017-12842 by not relaying // 64-byte transactions. if (::GetSerializeSize(tx, PROTOCOL_VERSION | SERIALIZE_TRANSACTION_NO_WITNESS) < MIN_STANDARD_TX_NONWITNESS_SIZE) - return state.Invalid(TxValidationResult::TX_NOT_STANDARD, false, "tx-size-small"); + return state.Invalid(TxValidationResult::TX_NOT_STANDARD, "tx-size-small"); // Only accept nLockTime-using transactions that can be mined in the next // block; we don't want our mempool filled up with transactions that can't // be mined yet. if (!CheckFinalTx(tx, STANDARD_LOCKTIME_VERIFY_FLAGS)) - return state.Invalid(TxValidationResult::TX_PREMATURE_SPEND, false, "non-final"); + return state.Invalid(TxValidationResult::TX_PREMATURE_SPEND, "non-final"); // is it already in the memory pool? if (m_pool.exists(hash)) { - return state.Invalid(TxValidationResult::TX_CONFLICT, false, "txn-already-in-mempool"); + return state.Invalid(TxValidationResult::TX_CONFLICT, "txn-already-in-mempool"); } // Check for conflicts with in-memory transactions @@ -617,7 +617,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) } } if (fReplacementOptOut) { - return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, false, "txn-mempool-conflict"); + return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "txn-mempool-conflict"); } setConflicts.insert(ptxConflicting->GetHash()); @@ -643,7 +643,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) for (size_t out = 0; out < tx.vout.size(); out++) { // Optimistically just do efficient check of cache for outputs if (coins_cache.HaveCoinInCache(COutPoint(hash, out))) { - return state.Invalid(TxValidationResult::TX_CONFLICT, false, "txn-already-known"); + return state.Invalid(TxValidationResult::TX_CONFLICT, "txn-already-known"); } } // Otherwise assume this might be an orphan tx for which we just haven't seen parents yet @@ -668,7 +668,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) // Must keep pool.cs for this unless we change CheckSequenceLocks to take a // CoinsViewCache instead of create its own if (!CheckSequenceLocks(m_pool, tx, STANDARD_LOCKTIME_VERIFY_FLAGS, &lp)) - return state.Invalid(TxValidationResult::TX_PREMATURE_SPEND, false, "non-BIP68-final"); + return state.Invalid(TxValidationResult::TX_PREMATURE_SPEND, "non-BIP68-final"); CAmount nFees = 0; if (!Consensus::CheckTxInputs(tx, state, m_view, GetSpendHeight(m_view), nFees)) { @@ -677,11 +677,11 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) // Check for non-standard pay-to-script-hash in inputs if (fRequireStandard && !AreInputsStandard(tx, m_view)) - return state.Invalid(TxValidationResult::TX_NOT_STANDARD, false, "bad-txns-nonstandard-inputs"); + return state.Invalid(TxValidationResult::TX_NOT_STANDARD, "bad-txns-nonstandard-inputs"); // Check for non-standard witness in P2WSH if (tx.HasWitness() && fRequireStandard && !IsWitnessStandard(tx, m_view)) - return state.Invalid(TxValidationResult::TX_WITNESS_MUTATED, false, "bad-witness-nonstandard"); + return state.Invalid(TxValidationResult::TX_WITNESS_MUTATED, "bad-witness-nonstandard"); int64_t nSigOpsCost = GetTransactionSigOpCost(tx, m_view, STANDARD_SCRIPT_VERIFY_FLAGS); @@ -705,7 +705,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) unsigned int nSize = entry->GetTxSize(); if (nSigOpsCost > MAX_STANDARD_TX_SIGOPS_COST) - return state.Invalid(TxValidationResult::TX_NOT_STANDARD, false, "bad-txns-too-many-sigops", + return state.Invalid(TxValidationResult::TX_NOT_STANDARD, "bad-txns-too-many-sigops", strprintf("%d", nSigOpsCost)); // No transactions are allowed below minRelayTxFee except from disconnected @@ -713,7 +713,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) if (!bypass_limits && !CheckFeeRate(nSize, nModifiedFees, state)) return false; if (nAbsurdFee && nFees > nAbsurdFee) - return state.Invalid(TxValidationResult::TX_NOT_STANDARD, false, + return state.Invalid(TxValidationResult::TX_NOT_STANDARD, "absurdly-high-fee", strprintf("%d > %d", nFees, nAbsurdFee)); const CTxMemPool::setEntries setIterConflicting = m_pool.GetIterSet(setConflicts); @@ -771,7 +771,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) // this, see https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2018-November/016518.html if (nSize > EXTRA_DESCENDANT_TX_SIZE_LIMIT || !m_pool.CalculateMemPoolAncestors(*entry, setAncestors, 2, m_limit_ancestor_size, m_limit_descendants + 1, m_limit_descendant_size + EXTRA_DESCENDANT_TX_SIZE_LIMIT, dummy_err_string)) { - return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, false, "too-long-mempool-chain", errString); + return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "too-long-mempool-chain", errString); } } @@ -784,7 +784,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) const uint256 &hashAncestor = ancestorIt->GetTx().GetHash(); if (setConflicts.count(hashAncestor)) { - return state.Invalid(TxValidationResult::TX_CONSENSUS, false, "bad-txns-spends-conflicting-tx", + return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-txns-spends-conflicting-tx", strprintf("%s spends conflicting transaction %s", hash.ToString(), hashAncestor.ToString())); @@ -824,7 +824,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) CFeeRate oldFeeRate(mi->GetModifiedFee(), mi->GetTxSize()); if (newFeeRate <= oldFeeRate) { - return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, false, "insufficient fee", + return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "insufficient fee", strprintf("rejecting replacement %s; new feerate %s <= old feerate %s", hash.ToString(), newFeeRate.ToString(), @@ -852,7 +852,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) nConflictingSize += it->GetTxSize(); } } else { - return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, false, "too many potential replacements", + return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "too many potential replacements", strprintf("rejecting replacement %s; too many potential replacements (%d > %d)\n", hash.ToString(), nConflictingCount, @@ -876,7 +876,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) // it's cheaper to just check if the new input refers to a // tx that's in the mempool. if (m_pool.exists(tx.vin[j].prevout.hash)) { - return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, false, "replacement-adds-unconfirmed", + return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "replacement-adds-unconfirmed", strprintf("replacement %s adds unconfirmed input, idx %d", hash.ToString(), j)); } @@ -888,7 +888,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) // transactions would not be paid for. if (nModifiedFees < nConflictingFees) { - return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, false, "insufficient fee", + return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "insufficient fee", strprintf("rejecting replacement %s, less fees than conflicting txs; %s < %s", hash.ToString(), FormatMoney(nModifiedFees), FormatMoney(nConflictingFees))); } @@ -898,7 +898,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) CAmount nDeltaFees = nModifiedFees - nConflictingFees; if (nDeltaFees < ::incrementalRelayFee.GetFee(nSize)) { - return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, false, "insufficient fee", + return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "insufficient fee", strprintf("rejecting replacement %s, not enough additional fees to relay; %s < %s", hash.ToString(), FormatMoney(nDeltaFees), @@ -926,7 +926,7 @@ bool MemPoolAccept::PolicyScriptChecks(ATMPArgs& args, Workspace& ws, Precompute if (!tx.HasWitness() && CheckInputs(tx, state_dummy, m_view, scriptVerifyFlags & ~(SCRIPT_VERIFY_WITNESS | SCRIPT_VERIFY_CLEANSTACK), true, false, txdata) && !CheckInputs(tx, state_dummy, m_view, scriptVerifyFlags & ~SCRIPT_VERIFY_CLEANSTACK, true, false, txdata)) { // Only the witness is missing, so the transaction itself may be fine. - state.Invalid(TxValidationResult::TX_WITNESS_MUTATED, false, + state.Invalid(TxValidationResult::TX_WITNESS_MUTATED, state.GetRejectReason(), state.GetDebugMessage()); } return false; // state filled in by CheckInputs @@ -1009,7 +1009,7 @@ bool MemPoolAccept::Finalize(ATMPArgs& args, Workspace& ws) if (!bypass_limits) { LimitMempoolSize(m_pool, gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000, std::chrono::hours{gArgs.GetArg("-mempoolexpiry", DEFAULT_MEMPOOL_EXPIRY)}); if (!m_pool.exists(hash)) - return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, false, "mempool full"); + return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "mempool full"); } return true; } @@ -1544,7 +1544,7 @@ bool CheckInputs(const CTransaction& tx, TxValidationState &state, const CCoinsV CScriptCheck check2(coin.out, tx, i, flags & ~STANDARD_NOT_MANDATORY_VERIFY_FLAGS, cacheSigStore, &txdata); if (check2()) - return state.Invalid(TxValidationResult::TX_NOT_STANDARD, false, strprintf("non-mandatory-script-verify-flag (%s)", ScriptErrorString(check.GetScriptError()))); + return state.Invalid(TxValidationResult::TX_NOT_STANDARD, strprintf("non-mandatory-script-verify-flag (%s)", ScriptErrorString(check.GetScriptError()))); } // MANDATORY flag failures correspond to // TxValidationResult::TX_CONSENSUS. Because CONSENSUS @@ -1555,7 +1555,7 @@ bool CheckInputs(const CTransaction& tx, TxValidationState &state, const CCoinsV // support, to avoid splitting the network (but this // depends on the details of how net_processing handles // such errors). - return state.Invalid(TxValidationResult::TX_CONSENSUS, false, strprintf("mandatory-script-verify-flag-failed (%s)", ScriptErrorString(check.GetScriptError()))); + return state.Invalid(TxValidationResult::TX_CONSENSUS, strprintf("mandatory-script-verify-flag-failed (%s)", ScriptErrorString(check.GetScriptError()))); } } @@ -2058,7 +2058,7 @@ bool CChainState::ConnectBlock(const CBlock& block, BlockValidationState& state, for (size_t o = 0; o < tx->vout.size(); o++) { if (view.HaveCoin(COutPoint(tx->GetHash(), o))) { LogPrintf("ERROR: ConnectBlock(): tried to overwrite transaction\n"); - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, false, "bad-txns-BIP30"); + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-txns-BIP30"); } } } @@ -2099,14 +2099,14 @@ bool CChainState::ConnectBlock(const CBlock& block, BlockValidationState& state, TxValidationState tx_state; if (!Consensus::CheckTxInputs(tx, tx_state, view, pindex->nHeight, txfee)) { // Any transaction validation failure in ConnectBlock is a block consensus failure - state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, false, + state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, tx_state.GetRejectReason(), tx_state.GetDebugMessage()); return error("%s: Consensus::CheckTxInputs: %s, %s", __func__, tx.GetHash().ToString(), FormatStateMessage(state)); } nFees += txfee; if (!MoneyRange(nFees)) { LogPrintf("ERROR: %s: accumulated fee in the block out of range.\n", __func__); - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, false, "bad-txns-accumulated-fee-outofrange"); + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-txns-accumulated-fee-outofrange"); } // Check that transaction is BIP68 final @@ -2119,7 +2119,7 @@ bool CChainState::ConnectBlock(const CBlock& block, BlockValidationState& state, if (!SequenceLocks(tx, nLockTimeFlags, &prevheights, *pindex)) { LogPrintf("ERROR: %s: contains a non-BIP68-final transaction\n", __func__); - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, false, "bad-txns-nonfinal"); + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-txns-nonfinal"); } } @@ -2130,7 +2130,7 @@ bool CChainState::ConnectBlock(const CBlock& block, BlockValidationState& state, nSigOpsCost += GetTransactionSigOpCost(tx, view, flags); if (nSigOpsCost > MAX_BLOCK_SIGOPS_COST) { LogPrintf("ERROR: ConnectBlock(): too many sigops\n"); - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, false, "bad-blk-sigops"); + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-blk-sigops"); } txdata.emplace_back(tx); @@ -2141,7 +2141,7 @@ bool CChainState::ConnectBlock(const CBlock& block, BlockValidationState& state, TxValidationState tx_state; if (fScriptChecks && !CheckInputs(tx, tx_state, view, flags, fCacheResults, fCacheResults, txdata[i], nScriptCheckThreads ? &vChecks : nullptr)) { // Any transaction validation failure in ConnectBlock is a block consensus failure - state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, false, + state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, tx_state.GetRejectReason(), tx_state.GetDebugMessage()); return error("ConnectBlock(): CheckInputs on %s failed with %s", tx.GetHash().ToString(), FormatStateMessage(state)); @@ -2161,12 +2161,12 @@ bool CChainState::ConnectBlock(const CBlock& block, BlockValidationState& state, CAmount blockReward = nFees + GetBlockSubsidy(pindex->nHeight, chainparams.GetConsensus()); if (block.vtx[0]->GetValueOut() > blockReward) { LogPrintf("ERROR: ConnectBlock(): coinbase pays too much (actual=%d vs limit=%d)\n", block.vtx[0]->GetValueOut(), blockReward); - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, false, "bad-cb-amount"); + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cb-amount"); } if (!control.Wait()) { LogPrintf("ERROR: %s: CheckQueue failed\n", __func__); - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, false, "block-validation-failed"); + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "block-validation-failed"); } int64_t nTime4 = GetTimeMicros(); nTimeVerify += nTime4 - nTime2; LogPrint(BCLog::BENCH, " - Verify %u txins: %.2fms (%.3fms/txin) [%.2fs (%.2fms/blk)]\n", nInputs - 1, MILLI * (nTime4 - nTime2), nInputs <= 1 ? 0 : MILLI * (nTime4 - nTime2) / (nInputs-1), nTimeVerify * MICRO, nTimeVerify * MILLI / nBlocksTotal); @@ -3244,7 +3244,7 @@ static bool CheckBlockHeader(const CBlockHeader& block, BlockValidationState& st { // Check proof of work matches claimed amount if (fCheckPOW && !CheckProofOfWork(block.GetHash(), block.nBits, consensusParams)) - return state.Invalid(BlockValidationResult::BLOCK_INVALID_HEADER, false, "high-hash", "proof of work failed"); + return state.Invalid(BlockValidationResult::BLOCK_INVALID_HEADER, "high-hash", "proof of work failed"); return true; } @@ -3266,13 +3266,13 @@ bool CheckBlock(const CBlock& block, BlockValidationState& state, const Consensu bool mutated; uint256 hashMerkleRoot2 = BlockMerkleRoot(block, &mutated); if (block.hashMerkleRoot != hashMerkleRoot2) - return state.Invalid(BlockValidationResult::BLOCK_MUTATED, false, "bad-txnmrklroot", "hashMerkleRoot mismatch"); + return state.Invalid(BlockValidationResult::BLOCK_MUTATED, "bad-txnmrklroot", "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.Invalid(BlockValidationResult::BLOCK_MUTATED, false, "bad-txns-duplicate", "duplicate transaction"); + return state.Invalid(BlockValidationResult::BLOCK_MUTATED, "bad-txns-duplicate", "duplicate transaction"); } // All potential-corruption validation must be done before we do any @@ -3283,14 +3283,14 @@ bool CheckBlock(const CBlock& block, BlockValidationState& state, const Consensu // Size limits if (block.vtx.empty() || block.vtx.size() * WITNESS_SCALE_FACTOR > MAX_BLOCK_WEIGHT || ::GetSerializeSize(block, PROTOCOL_VERSION | SERIALIZE_TRANSACTION_NO_WITNESS) * WITNESS_SCALE_FACTOR > MAX_BLOCK_WEIGHT) - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, false, "bad-blk-length", "size limits failed"); + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-blk-length", "size limits failed"); // First transaction must be coinbase, the rest must not be if (block.vtx.empty() || !block.vtx[0]->IsCoinBase()) - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, false, "bad-cb-missing", "first tx is not coinbase"); + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cb-missing", "first tx is not coinbase"); for (unsigned int i = 1; i < block.vtx.size(); i++) if (block.vtx[i]->IsCoinBase()) - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, false, "bad-cb-multiple", "more than one coinbase"); + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cb-multiple", "more than one coinbase"); // Check transactions // Must check for duplicate inputs (see CVE-2018-17144) @@ -3300,7 +3300,7 @@ bool CheckBlock(const CBlock& block, BlockValidationState& state, const Consensu // CheckBlock() does context-free validation checks. The only // possible failures are consensus failures. assert(tx_state.GetResult() == TxValidationResult::TX_CONSENSUS); - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, false, tx_state.GetRejectReason(), + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, tx_state.GetRejectReason(), strprintf("Transaction check failed (tx hash %s) %s", tx->GetHash().ToString(), tx_state.GetDebugMessage())); } } @@ -3310,7 +3310,7 @@ bool CheckBlock(const CBlock& block, BlockValidationState& state, const Consensu nSigOps += GetLegacySigOpCount(*tx); } if (nSigOps * WITNESS_SCALE_FACTOR > MAX_BLOCK_SIGOPS_COST) - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, false, "bad-blk-sigops", "out-of-bounds SigOpCount"); + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-blk-sigops", "out-of-bounds SigOpCount"); if (fCheckPOW && fCheckMerkleRoot) block.fChecked = true; @@ -3411,7 +3411,7 @@ static bool ContextualCheckBlockHeader(const CBlockHeader& block, BlockValidatio // Check proof of work const Consensus::Params& consensusParams = params.GetConsensus(); if (block.nBits != GetNextWorkRequired(pindexPrev, &block, consensusParams)) - return state.Invalid(BlockValidationResult::BLOCK_INVALID_HEADER, false, "bad-diffbits", "incorrect proof of work"); + return state.Invalid(BlockValidationResult::BLOCK_INVALID_HEADER, "bad-diffbits", "incorrect proof of work"); // Check against checkpoints if (fCheckpointsEnabled) { @@ -3421,24 +3421,24 @@ static bool ContextualCheckBlockHeader(const CBlockHeader& block, BlockValidatio CBlockIndex* pcheckpoint = GetLastCheckpoint(params.Checkpoints()); if (pcheckpoint && nHeight < pcheckpoint->nHeight) { LogPrintf("ERROR: %s: forked chain older than last checkpoint (height %d)\n", __func__, nHeight); - return state.Invalid(BlockValidationResult::BLOCK_CHECKPOINT, false, "bad-fork-prior-to-checkpoint"); + return state.Invalid(BlockValidationResult::BLOCK_CHECKPOINT, "bad-fork-prior-to-checkpoint"); } } // Check timestamp against prev if (block.GetBlockTime() <= pindexPrev->GetMedianTimePast()) - return state.Invalid(BlockValidationResult::BLOCK_INVALID_HEADER, false, "time-too-old", "block's timestamp is too early"); + return state.Invalid(BlockValidationResult::BLOCK_INVALID_HEADER, "time-too-old", "block's timestamp is too early"); // Check timestamp if (block.GetBlockTime() > nAdjustedTime + MAX_FUTURE_BLOCK_TIME) - return state.Invalid(BlockValidationResult::BLOCK_TIME_FUTURE, false, "time-too-new", "block timestamp too far in the future"); + return state.Invalid(BlockValidationResult::BLOCK_TIME_FUTURE, "time-too-new", "block timestamp too far in the future"); // Reject outdated version blocks when 95% (75% on testnet) of the network has upgraded: // check for version 2, 3 and 4 upgrades if((block.nVersion < 2 && nHeight >= consensusParams.BIP34Height) || (block.nVersion < 3 && nHeight >= consensusParams.BIP66Height) || (block.nVersion < 4 && nHeight >= consensusParams.BIP65Height)) - return state.Invalid(BlockValidationResult::BLOCK_INVALID_HEADER, false, strprintf("bad-version(0x%08x)", block.nVersion), + return state.Invalid(BlockValidationResult::BLOCK_INVALID_HEADER, strprintf("bad-version(0x%08x)", block.nVersion), strprintf("rejected nVersion=0x%08x block", block.nVersion)); return true; @@ -3468,7 +3468,7 @@ static bool ContextualCheckBlock(const CBlock& block, BlockValidationState& stat // Check that all transactions are finalized for (const auto& tx : block.vtx) { if (!IsFinalTx(*tx, nHeight, nLockTimeCutoff)) { - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, false, "bad-txns-nonfinal", "non-final transaction"); + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-txns-nonfinal", "non-final transaction"); } } @@ -3478,7 +3478,7 @@ static bool ContextualCheckBlock(const CBlock& block, BlockValidationState& stat 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.Invalid(BlockValidationResult::BLOCK_CONSENSUS, false, "bad-cb-height", "block height mismatch in coinbase"); + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cb-height", "block height mismatch in coinbase"); } } @@ -3500,11 +3500,11 @@ static bool ContextualCheckBlock(const CBlock& block, BlockValidationState& stat // already does not permit it, it is impossible to trigger in the // witness tree. if (block.vtx[0]->vin[0].scriptWitness.stack.size() != 1 || block.vtx[0]->vin[0].scriptWitness.stack[0].size() != 32) { - return state.Invalid(BlockValidationResult::BLOCK_MUTATED, false, "bad-witness-nonce-size", strprintf("%s : invalid witness reserved value size", __func__)); + return state.Invalid(BlockValidationResult::BLOCK_MUTATED, "bad-witness-nonce-size", strprintf("%s : invalid witness reserved value size", __func__)); } CHash256().Write(hashWitness.begin(), 32).Write(&block.vtx[0]->vin[0].scriptWitness.stack[0][0], 32).Finalize(hashWitness.begin()); if (memcmp(hashWitness.begin(), &block.vtx[0]->vout[commitpos].scriptPubKey[6], 32)) { - return state.Invalid(BlockValidationResult::BLOCK_MUTATED, false, "bad-witness-merkle-match", strprintf("%s : witness merkle commitment mismatch", __func__)); + return state.Invalid(BlockValidationResult::BLOCK_MUTATED, "bad-witness-merkle-match", strprintf("%s : witness merkle commitment mismatch", __func__)); } fHaveWitness = true; } @@ -3514,7 +3514,7 @@ static bool ContextualCheckBlock(const CBlock& block, BlockValidationState& stat if (!fHaveWitness) { for (const auto& tx : block.vtx) { if (tx->HasWitness()) { - return state.Invalid(BlockValidationResult::BLOCK_MUTATED, false, "unexpected-witness", strprintf("%s : unexpected witness data found", __func__)); + return state.Invalid(BlockValidationResult::BLOCK_MUTATED, "unexpected-witness", strprintf("%s : unexpected witness data found", __func__)); } } } @@ -3526,7 +3526,7 @@ static bool ContextualCheckBlock(const CBlock& block, BlockValidationState& stat // the block hash, so we couldn't mark the block as permanently // failed). if (GetBlockWeight(block) > MAX_BLOCK_WEIGHT) { - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, false, "bad-blk-weight", strprintf("%s : weight limit failed", __func__)); + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-blk-weight", strprintf("%s : weight limit failed", __func__)); } return true; @@ -3547,7 +3547,7 @@ bool BlockManager::AcceptBlockHeader(const CBlockHeader& block, BlockValidationS *ppindex = pindex; if (pindex->nStatus & BLOCK_FAILED_MASK) { LogPrintf("ERROR: %s: block %s is marked invalid\n", __func__, hash.ToString()); - return state.Invalid(BlockValidationResult::BLOCK_CACHED_INVALID, false, "duplicate"); + return state.Invalid(BlockValidationResult::BLOCK_CACHED_INVALID, "duplicate"); } return true; } @@ -3560,12 +3560,12 @@ bool BlockManager::AcceptBlockHeader(const CBlockHeader& block, BlockValidationS BlockMap::iterator mi = m_block_index.find(block.hashPrevBlock); if (mi == m_block_index.end()) { LogPrintf("ERROR: %s: prev block not found\n", __func__); - return state.Invalid(BlockValidationResult::BLOCK_MISSING_PREV, false, "prev-blk-not-found"); + return state.Invalid(BlockValidationResult::BLOCK_MISSING_PREV, "prev-blk-not-found"); } pindexPrev = (*mi).second; if (pindexPrev->nStatus & BLOCK_FAILED_MASK) { LogPrintf("ERROR: %s: prev block invalid\n", __func__); - return state.Invalid(BlockValidationResult::BLOCK_INVALID_PREV, false, "bad-prevblk"); + return state.Invalid(BlockValidationResult::BLOCK_INVALID_PREV, "bad-prevblk"); } if (!ContextualCheckBlockHeader(block, state, chainparams, pindexPrev, GetAdjustedTime())) return error("%s: Consensus::ContextualCheckBlockHeader: %s, %s", __func__, hash.ToString(), FormatStateMessage(state)); @@ -3604,7 +3604,7 @@ bool BlockManager::AcceptBlockHeader(const CBlockHeader& block, BlockValidationS invalid_walk = invalid_walk->pprev; } LogPrintf("ERROR: %s: prev block invalid\n", __func__); - return state.Invalid(BlockValidationResult::BLOCK_INVALID_PREV, false, "bad-prevblk"); + return state.Invalid(BlockValidationResult::BLOCK_INVALID_PREV, "bad-prevblk"); } } } -- cgit v1.2.3 From c428622a5bb1e37b2e6ab2c52791ac05d9271238 Mon Sep 17 00:00:00 2001 From: John Newbery Date: Sun, 28 Apr 2019 15:46:35 -0500 Subject: [validation] Remove unused first_invalid parameter from ProcessNewBlockHeaders() No callers use the returned value in first_invalid. Remove it from the function signature and don't set it in the function. --- src/net_processing.cpp | 3 +-- src/rpc/mining.cpp | 2 +- src/test/blockfilter_index_tests.cpp | 2 +- src/validation.cpp | 4 +--- src/validation.h | 3 +-- 5 files changed, 5 insertions(+), 9 deletions(-) (limited to 'src') diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 38d0461b1f..6c9cf1ccb6 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1698,8 +1698,7 @@ bool static ProcessHeadersMessage(CNode *pfrom, CConnman *connman, const std::ve } BlockValidationState state; - CBlockHeader first_invalid_header; - if (!ProcessNewBlockHeaders(headers, state, chainparams, &pindexLast, &first_invalid_header)) { + if (!ProcessNewBlockHeaders(headers, state, chainparams, &pindexLast)) { if (state.IsInvalid()) { MaybePunishNodeForBlock(pfrom->GetId(), state, via_compact_block, "invalid header received"); return false; diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index f294c815f6..78e80195be 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -773,7 +773,7 @@ static UniValue submitheader(const JSONRPCRequest& request) } BlockValidationState state; - ProcessNewBlockHeaders({h}, state, Params(), /* ppindex */ nullptr, /* first_invalid */ nullptr); + ProcessNewBlockHeaders({h}, state, Params()); if (state.IsValid()) return NullUniValue; if (state.IsError()) { throw JSONRPCError(RPC_VERIFY_ERROR, FormatStateMessage(state)); diff --git a/src/test/blockfilter_index_tests.cpp b/src/test/blockfilter_index_tests.cpp index 2e40ed0dde..4a15bf0c77 100644 --- a/src/test/blockfilter_index_tests.cpp +++ b/src/test/blockfilter_index_tests.cpp @@ -103,7 +103,7 @@ static bool BuildChain(const CBlockIndex* pindex, const CScript& coinbase_script CBlockHeader header = block->GetBlockHeader(); BlockValidationState state; - if (!ProcessNewBlockHeaders({header}, state, Params(), &pindex, nullptr)) { + if (!ProcessNewBlockHeaders({header}, state, Params(), &pindex)) { return false; } } diff --git a/src/validation.cpp b/src/validation.cpp index 21f7f9c8db..eaf1cfe23d 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -3619,9 +3619,8 @@ bool BlockManager::AcceptBlockHeader(const CBlockHeader& block, BlockValidationS } // Exposed wrapper for AcceptBlockHeader -bool ProcessNewBlockHeaders(const std::vector& headers, BlockValidationState& state, const CChainParams& chainparams, const CBlockIndex** ppindex, CBlockHeader *first_invalid) +bool ProcessNewBlockHeaders(const std::vector& headers, BlockValidationState& state, const CChainParams& chainparams, const CBlockIndex** ppindex) { - if (first_invalid != nullptr) first_invalid->SetNull(); { LOCK(cs_main); for (const CBlockHeader& header : headers) { @@ -3630,7 +3629,6 @@ bool ProcessNewBlockHeaders(const std::vector& headers, BlockValid ::ChainstateActive().CheckBlockIndex(chainparams.GetConsensus()); if (!accepted) { - if (first_invalid) *first_invalid = header; return false; } if (ppindex) { diff --git a/src/validation.h b/src/validation.h index f346e5caac..a8d7e76a31 100644 --- a/src/validation.h +++ b/src/validation.h @@ -222,9 +222,8 @@ bool ProcessNewBlock(const CChainParams& chainparams, const std::shared_ptr& block, BlockValidationState& state, const CChainParams& chainparams, const CBlockIndex** ppindex = nullptr, CBlockHeader* first_invalid = nullptr) LOCKS_EXCLUDED(cs_main); +bool ProcessNewBlockHeaders(const std::vector& block, BlockValidationState& state, const CChainParams& chainparams, const CBlockIndex** ppindex = nullptr) LOCKS_EXCLUDED(cs_main); /** Open a block file (blk?????.dat) */ FILE* OpenBlockFile(const FlatFilePos &pos, bool fReadOnly = false); -- cgit v1.2.3 From 3004d5a12d09d94bfc4dee2a8e8f2291996a4aaf Mon Sep 17 00:00:00 2001 From: John Newbery Date: Sun, 28 Apr 2019 16:26:31 -0500 Subject: [validation] Remove fMissingInputs from AcceptToMemoryPool() Handle this failure in the same way as all other failures: call Invalid() with the reasons for the failure. --- src/bench/block_assemble.cpp | 2 +- src/consensus/validation.h | 7 +------ src/net_processing.cpp | 10 ++++------ src/node/transaction.cpp | 12 +++++------- src/rpc/rawtransaction.cpp | 11 ++++++----- src/test/txvalidation_tests.cpp | 1 - src/test/txvalidationcache_tests.cpp | 2 +- src/test/validation_block_tests.cpp | 1 - src/validation.cpp | 23 +++++++---------------- src/validation.h | 2 +- 10 files changed, 26 insertions(+), 45 deletions(-) (limited to 'src') diff --git a/src/bench/block_assemble.cpp b/src/bench/block_assemble.cpp index a3a44e5dbc..2f47398d99 100644 --- a/src/bench/block_assemble.cpp +++ b/src/bench/block_assemble.cpp @@ -39,7 +39,7 @@ static void AssembleBlock(benchmark::State& state) for (const auto& txr : txs) { TxValidationState state; - bool ret{::AcceptToMemoryPool(::mempool, state, txr, nullptr /* pfMissingInputs */, nullptr /* plTxnReplaced */, false /* bypass_limits */, /* nAbsurdFee */ 0)}; + bool ret{::AcceptToMemoryPool(::mempool, state, txr, nullptr /* plTxnReplaced */, false /* bypass_limits */, /* nAbsurdFee */ 0)}; assert(ret); } } diff --git a/src/consensus/validation.h b/src/consensus/validation.h index cdc3aecc1e..e602b9d5f3 100644 --- a/src/consensus/validation.h +++ b/src/consensus/validation.h @@ -27,12 +27,7 @@ enum class TxValidationResult { */ TX_RECENT_CONSENSUS_CHANGE, TX_NOT_STANDARD, //!< didn't meet our local policy rules - /** - * transaction was missing some of its inputs - * TODO: ATMP uses fMissingInputs and a valid ValidationState to indicate missing inputs. - * Change ATMP to use TX_MISSING_INPUTS. - */ - TX_MISSING_INPUTS, + TX_MISSING_INPUTS, //!< transaction was missing some of its inputs TX_PREMATURE_SPEND, //!< transaction spends a coinbase too early, or violates locktime/sequence locks /** * Transaction might be missing a witness, have a witness prior to SegWit diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 6c9cf1ccb6..8b1f18398d 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1836,14 +1836,13 @@ void static ProcessOrphanTx(CConnman* connman, std::set& orphan_work_se const CTransactionRef porphanTx = orphan_it->second.tx; const CTransaction& orphanTx = *porphanTx; NodeId fromPeer = orphan_it->second.fromPeer; - bool fMissingInputs2 = false; // Use a new TxValidationState because orphans come from different peers (and we call // MaybePunishNodeForTx based on the source peer from the orphan map, not based on the peer // that relayed the previous transaction). TxValidationState orphan_state; if (setMisbehaving.count(fromPeer)) continue; - if (AcceptToMemoryPool(mempool, orphan_state, porphanTx, &fMissingInputs2, &removed_txn, false /* bypass_limits */, 0 /* nAbsurdFee */)) { + if (AcceptToMemoryPool(mempool, orphan_state, porphanTx, &removed_txn, false /* bypass_limits */, 0 /* nAbsurdFee */)) { LogPrint(BCLog::MEMPOOL, " accepted orphan tx %s\n", orphanHash.ToString()); RelayTransaction(orphanHash, *connman); for (unsigned int i = 0; i < orphanTx.vout.size(); i++) { @@ -1856,7 +1855,7 @@ void static ProcessOrphanTx(CConnman* connman, std::set& orphan_work_se } EraseOrphanTx(orphanHash); done = true; - } else if (!fMissingInputs2) { + } else if (orphan_state.GetResult() != TxValidationResult::TX_MISSING_INPUTS) { if (orphan_state.IsInvalid()) { // Punish peer that gave us an invalid orphan tx if (MaybePunishNodeForTx(fromPeer, orphan_state)) { @@ -2492,7 +2491,6 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr LOCK2(cs_main, g_cs_orphans); - bool fMissingInputs = false; TxValidationState state; CNodeState* nodestate = State(pfrom->GetId()); @@ -2503,7 +2501,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr std::list lRemovedTxn; if (!AlreadyHave(inv) && - AcceptToMemoryPool(mempool, state, ptx, &fMissingInputs, &lRemovedTxn, false /* bypass_limits */, 0 /* nAbsurdFee */)) { + AcceptToMemoryPool(mempool, state, ptx, &lRemovedTxn, false /* bypass_limits */, 0 /* nAbsurdFee */)) { mempool.check(&::ChainstateActive().CoinsTip()); RelayTransaction(tx.GetHash(), *connman); for (unsigned int i = 0; i < tx.vout.size(); i++) { @@ -2525,7 +2523,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr // Recursively process any orphan transactions that depended on this one ProcessOrphanTx(connman, pfrom->orphan_work_set, lRemovedTxn); } - else if (fMissingInputs) + else if (state.GetResult() == TxValidationResult::TX_MISSING_INPUTS) { bool fRejectedParents = false; // It may be the case that the orphans parents have all been rejected for (const CTxIn& txin : tx.vin) { diff --git a/src/node/transaction.cpp b/src/node/transaction.cpp index 4645baa3d7..8989a77e57 100644 --- a/src/node/transaction.cpp +++ b/src/node/transaction.cpp @@ -37,17 +37,15 @@ TransactionError BroadcastTransaction(const CTransactionRef tx, std::string& err if (!mempool.exists(hashTx)) { // Transaction is not already in the mempool. Submit it. TxValidationState state; - bool fMissingInputs; - if (!AcceptToMemoryPool(mempool, state, std::move(tx), &fMissingInputs, + if (!AcceptToMemoryPool(mempool, state, std::move(tx), nullptr /* plTxnReplaced */, false /* bypass_limits */, max_tx_fee)) { + err_string = FormatStateMessage(state); if (state.IsInvalid()) { - err_string = FormatStateMessage(state); - return TransactionError::MEMPOOL_REJECTED; - } else { - if (fMissingInputs) { + if (state.GetResult() == TxValidationResult::TX_MISSING_INPUTS) { return TransactionError::MISSING_INPUTS; } - err_string = FormatStateMessage(state); + return TransactionError::MEMPOOL_REJECTED; + } else { return TransactionError::MEMPOOL_ERROR; } } diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 68024fbe2f..48b26d2a6f 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -894,19 +894,20 @@ static UniValue testmempoolaccept(const JSONRPCRequest& request) result_0.pushKV("txid", tx_hash.GetHex()); TxValidationState state; - bool missing_inputs; bool test_accept_res; { LOCK(cs_main); - test_accept_res = AcceptToMemoryPool(mempool, state, std::move(tx), &missing_inputs, + test_accept_res = AcceptToMemoryPool(mempool, state, std::move(tx), nullptr /* plTxnReplaced */, false /* bypass_limits */, max_raw_tx_fee, /* test_accept */ true); } result_0.pushKV("allowed", test_accept_res); if (!test_accept_res) { if (state.IsInvalid()) { - result_0.pushKV("reject-reason", strprintf("%s", state.GetRejectReason())); - } else if (missing_inputs) { - result_0.pushKV("reject-reason", "missing-inputs"); + if (state.GetResult() == TxValidationResult::TX_MISSING_INPUTS) { + result_0.pushKV("reject-reason", "missing-inputs"); + } else { + result_0.pushKV("reject-reason", strprintf("%s", state.GetRejectReason())); + } } else { result_0.pushKV("reject-reason", state.GetRejectReason()); } diff --git a/src/test/txvalidation_tests.cpp b/src/test/txvalidation_tests.cpp index 1f781ca83a..391ebfadfb 100644 --- a/src/test/txvalidation_tests.cpp +++ b/src/test/txvalidation_tests.cpp @@ -39,7 +39,6 @@ BOOST_FIXTURE_TEST_CASE(tx_mempool_reject_coinbase, TestChain100Setup) BOOST_CHECK_EQUAL( false, AcceptToMemoryPool(mempool, state, MakeTransactionRef(coinbaseTx), - nullptr /* pfMissingInputs */, nullptr /* plTxnReplaced */, true /* bypass_limits */, 0 /* nAbsurdFee */)); diff --git a/src/test/txvalidationcache_tests.cpp b/src/test/txvalidationcache_tests.cpp index ea2ef5546f..144230b114 100644 --- a/src/test/txvalidationcache_tests.cpp +++ b/src/test/txvalidationcache_tests.cpp @@ -23,7 +23,7 @@ ToMemPool(const CMutableTransaction& tx) LOCK(cs_main); TxValidationState state; - return AcceptToMemoryPool(mempool, state, MakeTransactionRef(tx), nullptr /* pfMissingInputs */, + return AcceptToMemoryPool(mempool, state, MakeTransactionRef(tx), nullptr /* plTxnReplaced */, true /* bypass_limits */, 0 /* nAbsurdFee */); } diff --git a/src/test/validation_block_tests.cpp b/src/test/validation_block_tests.cpp index cc1406c062..aca9f475ac 100644 --- a/src/test/validation_block_tests.cpp +++ b/src/test/validation_block_tests.cpp @@ -285,7 +285,6 @@ BOOST_AUTO_TEST_CASE(mempool_locks_reorg) ::mempool, state, tx, - /* pfMissingInputs */ &ignored, &plTxnReplaced, /* bypass_limits */ false, /* nAbsurdFee */ 0)); diff --git a/src/validation.cpp b/src/validation.cpp index eaf1cfe23d..ca6d2176b3 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -365,7 +365,7 @@ static void UpdateMempoolForReorg(DisconnectedBlockTransactions& disconnectpool, // ignore validation errors in resurrected transactions TxValidationState stateDummy; if (!fAddToMempool || (*it)->IsCoinBase() || - !AcceptToMemoryPool(mempool, stateDummy, *it, nullptr /* pfMissingInputs */, + !AcceptToMemoryPool(mempool, stateDummy, *it, nullptr /* plTxnReplaced */, true /* bypass_limits */, 0 /* nAbsurdFee */)) { // If the transaction doesn't make it in to the mempool, remove any // transactions that depend on it (which would now be orphans). @@ -442,7 +442,6 @@ public: struct ATMPArgs { const CChainParams& m_chainparams; TxValidationState &m_state; - bool* m_missing_inputs; const int64_t m_accept_time; std::list* m_replaced_transactions; const bool m_bypass_limits; @@ -538,7 +537,6 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) // Copy/alias what we need out of args TxValidationState &state = args.m_state; - bool* pfMissingInputs = args.m_missing_inputs; const int64_t nAcceptTime = args.m_accept_time; const bool bypass_limits = args.m_bypass_limits; const CAmount& nAbsurdFee = args.m_absurd_fee; @@ -554,10 +552,6 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) CAmount& nConflictingFees = ws.m_conflicting_fees; size_t& nConflictingSize = ws.m_conflicting_size; - if (pfMissingInputs) { - *pfMissingInputs = false; - } - if (!CheckTransaction(tx, state)) return false; // state filled in by CheckTransaction @@ -647,10 +641,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) } } // Otherwise assume this might be an orphan tx for which we just haven't seen parents yet - if (pfMissingInputs) { - *pfMissingInputs = true; - } - return false; // fMissingInputs and !state.IsInvalid() is used to detect this condition, don't set state.Invalid() + return state.Invalid(TxValidationResult::TX_MISSING_INPUTS, "bad-txns-inputs-missingorspent"); } } @@ -1047,11 +1038,11 @@ bool MemPoolAccept::AcceptSingleTransaction(const CTransactionRef& ptx, ATMPArgs /** (try to) add transaction to memory pool with a specified acceptance time **/ static bool AcceptToMemoryPoolWithTime(const CChainParams& chainparams, CTxMemPool& pool, TxValidationState &state, const CTransactionRef &tx, - bool* pfMissingInputs, int64_t nAcceptTime, std::list* plTxnReplaced, + int64_t nAcceptTime, std::list* plTxnReplaced, bool bypass_limits, const CAmount nAbsurdFee, bool test_accept) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { std::vector coins_to_uncache; - MemPoolAccept::ATMPArgs args { chainparams, state, pfMissingInputs, nAcceptTime, plTxnReplaced, bypass_limits, nAbsurdFee, coins_to_uncache, test_accept }; + MemPoolAccept::ATMPArgs args { chainparams, state, nAcceptTime, plTxnReplaced, bypass_limits, nAbsurdFee, coins_to_uncache, test_accept }; bool res = MemPoolAccept(pool).AcceptSingleTransaction(tx, args); if (!res) { // Remove coins that were not present in the coins cache before calling ATMPW; @@ -1069,11 +1060,11 @@ static bool AcceptToMemoryPoolWithTime(const CChainParams& chainparams, CTxMemPo } bool AcceptToMemoryPool(CTxMemPool& pool, TxValidationState &state, const CTransactionRef &tx, - bool* pfMissingInputs, std::list* plTxnReplaced, + std::list* plTxnReplaced, bool bypass_limits, const CAmount nAbsurdFee, bool test_accept) { const CChainParams& chainparams = Params(); - return AcceptToMemoryPoolWithTime(chainparams, pool, state, tx, pfMissingInputs, GetTime(), plTxnReplaced, bypass_limits, nAbsurdFee, test_accept); + return AcceptToMemoryPoolWithTime(chainparams, pool, state, tx, GetTime(), plTxnReplaced, bypass_limits, nAbsurdFee, test_accept); } /** @@ -4969,7 +4960,7 @@ bool LoadMempool(CTxMemPool& pool) TxValidationState state; if (nTime + nExpiryTimeout > nNow) { LOCK(cs_main); - AcceptToMemoryPoolWithTime(chainparams, pool, state, tx, nullptr /* pfMissingInputs */, nTime, + AcceptToMemoryPoolWithTime(chainparams, pool, state, tx, nTime, nullptr /* plTxnReplaced */, false /* bypass_limits */, 0 /* nAbsurdFee */, false /* test_accept */); if (state.IsValid()) { diff --git a/src/validation.h b/src/validation.h index a8d7e76a31..7f9582adfd 100644 --- a/src/validation.h +++ b/src/validation.h @@ -273,7 +273,7 @@ void PruneBlockFilesManual(int nManualPruneHeight); /** (try to) add transaction to memory pool * plTxnReplaced will be appended to with all transactions replaced from mempool **/ bool AcceptToMemoryPool(CTxMemPool& pool, TxValidationState &state, const CTransactionRef &tx, - bool* pfMissingInputs, std::list* plTxnReplaced, + std::list* plTxnReplaced, bool bypass_limits, const CAmount nAbsurdFee, bool test_accept=false) EXCLUSIVE_LOCKS_REQUIRED(cs_main); /** Get the BIP9 state for a given deployment at the current tip. */ -- cgit v1.2.3