diff --git a/src/blockencodings.cpp b/src/blockencodings.cpp index 4c95d730e6..10650ed4bf 100644 --- a/src/blockencodings.cpp +++ b/src/blockencodings.cpp @@ -209,7 +209,7 @@ ReadStatus PartiallyDownloadedBlock::FillBlock(CBlock& block, const std::vector< // 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.CorruptionPossible()) + if (state.GetReason() == ValidationInvalidReason::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 54d656518b..09e3710e3c 100644 --- a/src/consensus/tx_check.cpp +++ b/src/consensus/tx_check.cpp @@ -11,28 +11,28 @@ bool CheckTransaction(const CTransaction& tx, CValidationState &state, bool fChe { // Basic checks that don't depend on any context if (tx.vin.empty()) - return state.DoS(10, false, REJECT_INVALID, "bad-txns-vin-empty"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-txns-vin-empty"); if (tx.vout.empty()) - return state.DoS(10, false, REJECT_INVALID, "bad-txns-vout-empty"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "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.DoS(100, false, REJECT_INVALID, "bad-txns-oversize"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-txns-oversize"); // Check for negative or overflow output values CAmount nValueOutExplicit = 0; for (const auto& txout : tx.vout) { if (!txout.nValue.IsValid()) - return state.DoS(100, false, REJECT_INVALID, "bad-txns-vout-amount-invalid"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-txns-vout-amount-invalid"); if (!txout.nValue.IsExplicit()) continue; if (txout.nValue.GetAmount() < 0) - return state.DoS(100, false, REJECT_INVALID, "bad-txns-vout-negative"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-txns-vout-negative"); if (txout.nValue.GetAmount() > MAX_MONEY) - return state.DoS(100, false, REJECT_INVALID, "bad-txns-vout-toolarge"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-txns-vout-toolarge"); nValueOutExplicit += txout.nValue.GetAmount(); if (!MoneyRange(nValueOutExplicit)) - return state.DoS(100, false, REJECT_INVALID, "bad-txns-txouttotal-toolarge"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-txns-txouttotal-toolarge"); } // Check for duplicate inputs - note that this check is slow so we skip it in CheckBlock @@ -41,18 +41,18 @@ bool CheckTransaction(const CTransaction& tx, CValidationState &state, bool fChe for (const auto& txin : tx.vin) { if (!vInOutPoints.insert(txin.prevout).second) - return state.DoS(100, false, REJECT_INVALID, "bad-txns-inputs-duplicate"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-txns-inputs-duplicate"); } } if (tx.IsCoinBase()) { if (tx.vin[0].scriptSig.size() < 2 || tx.vin[0].scriptSig.size() > 100) - return state.DoS(100, false, REJECT_INVALID, "bad-cb-length"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-cb-length"); for (unsigned int i = 0; i < tx.vout.size(); i++) { if (tx.vout[i].IsFee()) { - return state.DoS(100, false, REJECT_INVALID, "bad-cb-fee"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-cb-fee"); } } } @@ -60,7 +60,7 @@ bool CheckTransaction(const CTransaction& tx, CValidationState &state, bool fChe { for (const auto& txin : tx.vin) if (txin.prevout.IsNull()) - return state.DoS(10, false, REJECT_INVALID, "bad-txns-prevout-null"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-txns-prevout-null"); } return true; diff --git a/src/consensus/tx_verify.cpp b/src/consensus/tx_verify.cpp index fa22ec7ff7..948a7378d4 100644 --- a/src/consensus/tx_verify.cpp +++ b/src/consensus/tx_verify.cpp @@ -182,7 +182,7 @@ bool Consensus::CheckTxInputs(const CTransaction& tx, CValidationState& state, c { // are the actual inputs available? if (!inputs.HaveInputs(tx)) { - return state.DoS(100, false, REJECT_INVALID, "bad-txns-inputs-missingorspent", false, + return state.Invalid(ValidationInvalidReason::TX_MISSING_INPUTS, false, REJECT_INVALID, "bad-txns-inputs-missingorspent", strprintf("%s: inputs missing/spent", __func__)); } @@ -194,14 +194,14 @@ bool Consensus::CheckTxInputs(const CTransaction& tx, CValidationState& state, c // Check existence and validity of pegin witness std::string err; if (tx.witness.vtxinwit.size() <= i || !IsValidPeginWitness(tx.witness.vtxinwit[i].m_pegin_witness, prevout, err, true)) { - return state.DoS(0, false, REJECT_PEGIN, "bad-pegin-witness", false, err); + return state.Invalid(ValidationInvalidReason::TX_WITNESS_MUTATED, false, REJECT_PEGIN, "bad-pegin-witness", err); } std::pair pegin = std::make_pair(uint256(tx.witness.vtxinwit[i].m_pegin_witness.stack[2]), prevout); if (inputs.IsPeginSpent(pegin)) { - return state.Invalid(false, REJECT_INVALID, "bad-txns-double-pegin", strprintf("Double-pegin of %s:%d", prevout.hash.ToString(), prevout.n)); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-txns-double-pegin", strprintf("Double-pegin of %s:%d", prevout.hash.ToString(), prevout.n)); } if (setPeginsSpent.count(pegin)) { - return state.DoS(100, false, REJECT_INVALID, "bad-txns-double-pegin-in-obj", false, + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-txns-double-pegin-in-obj", strprintf("Double-pegin of %s:%d in single tx/block", prevout.hash.ToString(), prevout.n)); } setPeginsSpent.insert(pegin); @@ -211,7 +211,7 @@ bool Consensus::CheckTxInputs(const CTransaction& tx, CValidationState& state, c const CTxOut& out = spent_inputs.back(); nValueIn += out.nValue.GetAmount(); // Non-explicit already filtered by IsValidPeginWitness if (!MoneyRange(out.nValue.GetAmount())) { - return state.DoS(100, false, REJECT_INVALID, "bad-txns-pegin-inputvalue-outofrange"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-txns-inputvalues-outofrange"); } } else { const Coin& coin = inputs.AccessCoin(prevout); @@ -219,8 +219,7 @@ 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(false, - REJECT_INVALID, "bad-txns-premature-spend-of-coinbase", + return state.Invalid(ValidationInvalidReason::TX_PREMATURE_SPEND, false, REJECT_INVALID, "bad-txns-premature-spend-of-coinbase", strprintf("tried to spend coinbase at depth %d", nSpendHeight - coin.nHeight)); } spent_inputs.push_back(coin.out); @@ -233,25 +232,25 @@ bool Consensus::CheckTxInputs(const CTransaction& tx, CValidationState& state, c if (g_con_elementsmode) { // Tally transaction fees if (!HasValidFee(tx)) { - return state.DoS(100, false, REJECT_INVALID, "bad-txns-fee-outofrange"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-txns-fee-outofrange"); } // Verify that amounts add up. if (fScriptChecks && !VerifyAmounts(spent_inputs, tx, pvChecks, cacheStore)) { - return state.DoS(100, false, REJECT_INVALID, "bad-txns-in-ne-out", false, "value in != value out"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-txns-in-ne-out", "value in != value out"); } fee_map += GetFeeMap(tx); } else { const CAmount value_out = tx.GetValueOutMap()[CAsset()]; if (nValueIn < value_out) { - return state.DoS(100, false, REJECT_INVALID, "bad-txns-in-belowout", false, + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "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.DoS(100, false, REJECT_INVALID, "bad-txns-fee-outofrange"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-txns-fee-outofrange"); } fee_map[CAsset()] += txfee_aux; diff --git a/src/consensus/validation.h b/src/consensus/validation.h index 70d9fd2a07..2ed4e636f1 100644 --- a/src/consensus/validation.h +++ b/src/consensus/validation.h @@ -24,6 +24,78 @@ static const unsigned char REJECT_CHECKPOINT = 0x43; // ELEMENTS: static const unsigned char REJECT_PEGIN = 0x44; +/** A "reason" why something was invalid, suitable for determining whether the + * provider of the object 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 ValidationInvalidReason { + // txn and blocks: + NONE, //!< not actually invalid + 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. + */ + 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_NOT_STANDARD, //!< didn't meet our local policy rules + TX_MISSING_INPUTS, //!< a 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 + * activation, or witness may have been malleated (which includes + * non-standard witnesses). + */ + TX_WITNESS_MUTATED, + /** + * 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 + */ + 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; +} + +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 { private: @@ -32,32 +104,24 @@ private: MODE_INVALID, //!< network rule violation (DoS value may be set) MODE_ERROR, //!< run-time error } mode; - int nDoS; + ValidationInvalidReason m_reason; std::string strRejectReason; unsigned int chRejectCode; - bool corruptionPossible; std::string strDebugMessage; public: - CValidationState() : mode(MODE_VALID), nDoS(0), chRejectCode(0), corruptionPossible(false) {} - bool DoS(int level, bool ret = false, - unsigned int chRejectCodeIn=0, const std::string &strRejectReasonIn="", - bool corruptionIn=false, - const std::string &strDebugMessageIn="") { + CValidationState() : mode(MODE_VALID), m_reason(ValidationInvalidReason::NONE), chRejectCode(0) {} + bool Invalid(ValidationInvalidReason reasonIn, bool ret = false, + unsigned int chRejectCodeIn=0, const std::string &strRejectReasonIn="", + const std::string &strDebugMessageIn="") { + m_reason = reasonIn; chRejectCode = chRejectCodeIn; strRejectReason = strRejectReasonIn; - corruptionPossible = corruptionIn; strDebugMessage = strDebugMessageIn; if (mode == MODE_ERROR) return ret; - nDoS += level; mode = MODE_INVALID; return ret; } - bool Invalid(bool ret = false, - unsigned int _chRejectCode=0, const std::string &_strRejectReason="", - const std::string &_strDebugMessage="") { - return DoS(0, ret, _chRejectCode, _strRejectReason, false, _strDebugMessage); - } bool Error(const std::string& strRejectReasonIn) { if (mode == MODE_VALID) strRejectReason = strRejectReasonIn; @@ -73,19 +137,7 @@ public: bool IsError() const { return mode == MODE_ERROR; } - bool IsInvalid(int &nDoSOut) const { - if (IsInvalid()) { - nDoSOut = nDoS; - return true; - } - return false; - } - bool CorruptionPossible() const { - return corruptionPossible; - } - void SetCorruptionPossible() { - corruptionPossible = true; - } + ValidationInvalidReason GetReason() const { return m_reason; } unsigned int GetRejectCode() const { return chRejectCode; } std::string GetRejectReason() const { return strRejectReason; } std::string GetDebugMessage() const { return strDebugMessage; } diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 74e33189dc..71ebd72b83 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -351,7 +351,16 @@ struct CNodeState { TxDownloadState m_tx_download; - CNodeState(CAddress addrIn, std::string addrNameIn) : address(addrIn), name(addrNameIn) { + //! Whether this peer is an inbound connection + bool m_is_inbound; + + //! Whether this peer is a manual connection + bool m_is_manual_connection; + + CNodeState(CAddress addrIn, std::string addrNameIn, bool is_inbound, bool is_manual) : + address(addrIn), name(std::move(addrNameIn)), m_is_inbound(is_inbound), + m_is_manual_connection (is_manual) + { fCurrentlyConnected = false; nMisbehavior = 0; fShouldBan = false; @@ -747,7 +756,7 @@ void PeerLogicValidation::InitializeNode(CNode *pnode) { NodeId nodeid = pnode->GetId(); { LOCK(cs_main); - mapNodeState.emplace_hint(mapNodeState.end(), std::piecewise_construct, std::forward_as_tuple(nodeid), std::forward_as_tuple(addr, std::move(addrName))); + mapNodeState.emplace_hint(mapNodeState.end(), std::piecewise_construct, std::forward_as_tuple(nodeid), std::forward_as_tuple(addr, std::move(addrName), pnode->fInbound, pnode->m_manual_connection)); } if(!pnode->fInbound) PushNodeVersion(pnode, connman, GetTime()); @@ -959,6 +968,90 @@ void Misbehaving(NodeId pnode, int howmuch, const std::string& message) EXCLUSIV LogPrint(BCLog::NET, "%s: %s peer=%d (%d -> %d)%s\n", __func__, state->name, pnode, state->nMisbehavior-howmuch, state->nMisbehavior, message_prefixed); } +/** + * Returns true if the given validation state result may result in a peer + * 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; +} + +/** + * Potentially ban a node based on the contents of a CValidationState 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 + * block message or not. If the compact block had a valid header, but contained invalid + * 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: + break; + // The node is providing invalid data: + case ValidationInvalidReason::CONSENSUS: + case ValidationInvalidReason::BLOCK_MUTATED: + if (!via_compact_block) { + LOCK(cs_main); + Misbehaving(nodeid, 100, message); + return true; + } + break; + case ValidationInvalidReason::CACHED_INVALID: + { + LOCK(cs_main); + CNodeState *node_state = State(nodeid); + if (node_state == nullptr) { + break; + } + + // Ban outbound (but not inbound) peers if on an invalid chain. + // Exempt HB compact block peers and manual connections. + if (!via_compact_block && !node_state->m_is_inbound && !node_state->m_is_manual_connection) { + Misbehaving(nodeid, 100, message); + return true; + } + break; + } + case ValidationInvalidReason::BLOCK_INVALID_HEADER: + case ValidationInvalidReason::BLOCK_CHECKPOINT: + case ValidationInvalidReason::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: + { + // 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: + break; + } + if (message != "") { + LogPrint(BCLog::NET, "peer=%d: %s\n", nodeid, message); + } + return false; +} + @@ -1132,14 +1225,12 @@ void PeerLogicValidation::BlockChecked(const CBlock& block, const CValidationSta const uint256 hash(block.GetHash()); std::map>::iterator it = mapBlockSource.find(hash); - int nDoS = 0; - if (state.IsInvalid(nDoS)) { + if (state.IsInvalid()) { // Don't send reject message with code 0 or an internal reject code. if (it != mapBlockSource.end() && State(it->second.first) && state.GetRejectCode() > 0 && state.GetRejectCode() < REJECT_INTERNAL) { CBlockReject reject = {(unsigned char)state.GetRejectCode(), state.GetRejectReason().substr(0, MAX_REJECT_MESSAGE_LENGTH), hash}; State(it->second.first)->rejects.push_back(reject); - if (nDoS > 0 && it->second.second) - Misbehaving(it->second.first, nDoS); + MaybePunishNode(/*nodeid=*/ it->second.first, state, /*via_compact_block=*/ !it->second.second); } } // Check that: @@ -1489,7 +1580,7 @@ inline void static SendBlockTransactions(const CBlock& block, const BlockTransac connman->PushMessage(pfrom, msgMaker.Make(nSendFlags, NetMsgType::BLOCKTXN, resp)); } -bool static ProcessHeadersMessage(CNode *pfrom, CConnman *connman, const std::vector& headers, const CChainParams& chainparams, bool punish_duplicate_invalid) +bool static ProcessHeadersMessage(CNode *pfrom, CConnman *connman, const std::vector& headers, const CChainParams& chainparams, bool via_compact_block) { const CNetMsgMaker msgMaker(pfrom->GetSendVersion()); size_t nCount = headers.size(); @@ -1551,48 +1642,8 @@ bool static ProcessHeadersMessage(CNode *pfrom, CConnman *connman, const std::ve CValidationState state; CBlockHeader first_invalid_header; if (!ProcessNewBlockHeaders(headers, state, chainparams, &pindexLast, &first_invalid_header)) { - int nDoS; - if (state.IsInvalid(nDoS)) { - LOCK(cs_main); - if (nDoS > 0) { - Misbehaving(pfrom->GetId(), nDoS, "invalid header received"); - } else { - LogPrint(BCLog::NET, "peer=%d: invalid header received\n", pfrom->GetId()); - } - if (punish_duplicate_invalid && LookupBlockIndex(first_invalid_header.GetHash())) { - // Goal: don't allow outbound peers to use up our outbound - // connection slots if they are on incompatible chains. - // - // We ask the caller to set punish_invalid appropriately based - // on the peer and the method of header delivery (compact - // blocks are allowed to be invalid in some circumstances, - // under BIP 152). - // Here, we try to detect the narrow situation that we have a - // valid block header (ie it was valid at the time the header - // was received, and hence stored in mapBlockIndex) but know the - // block is invalid, and that a peer has announced that same - // block as being on its active chain. - // Disconnect the peer in such a situation. - // - // Note: if the header that is invalid was not accepted to our - // mapBlockIndex at all, that may also be grounds for - // disconnecting the peer, as the chain they are on is likely - // to be incompatible. However, there is a circumstance where - // that does not hold: if the header's timestamp is more than - // 2 hours ahead of our current time. In that case, the header - // may become valid in the future, and we don't want to - // disconnect a peer merely for serving us one too-far-ahead - // block header, to prevent an attacker from splitting the - // network by mining a block right at the 2 hour boundary. - // - // TODO: update the DoS logic (or, rather, rewrite the - // DoS-interface between validation and net_processing) so that - // the interface is cleaner, and so that we disconnect on all the - // reasons that a peer's headers chain is incompatible - // with ours (eg block->nVersion softforks, MTP violations, - // etc), and not just the duplicate-invalid case. - pfrom->fDisconnect = true; - } + if (state.IsInvalid()) { + MaybePunishNode(pfrom->GetId(), state, via_compact_block, "invalid header received"); return false; } } @@ -1727,13 +1778,13 @@ 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 dummy CValidationState so someone can't setup nodes to counter-DoS based on orphan - // resolution (that is, feeding people an invalid transaction based on LegitTxX in order to get - // anyone relaying LegitTxX banned) - CValidationState stateDummy; + // 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 + // that relayed the previous transaction). + CValidationState orphan_state; if (setMisbehaving.count(fromPeer)) continue; - if (AcceptToMemoryPool(mempool, stateDummy, porphanTx, &fMissingInputs2, &removed_txn, false /* bypass_limits */, 0 /* nAbsurdFee */)) { + if (AcceptToMemoryPool(mempool, orphan_state, porphanTx, &fMissingInputs2, &removed_txn, false /* bypass_limits */, 0 /* nAbsurdFee */)) { LogPrint(BCLog::MEMPOOL, " accepted orphan tx %s\n", orphanHash.ToString()); RelayTransaction(orphanTx, connman); for (unsigned int i = 0; i < orphanTx.vout.size(); i++) { @@ -1747,17 +1798,18 @@ void static ProcessOrphanTx(CConnman* connman, std::set& orphan_work_se EraseOrphanTx(orphanHash); done = true; } else if (!fMissingInputs2) { - int nDos = 0; - if (stateDummy.IsInvalid(nDos) && nDos > 0) { + if (orphan_state.IsInvalid()) { // Punish peer that gave us an invalid orphan tx - Misbehaving(fromPeer, nDos); - setMisbehaving.insert(fromPeer); + if (MaybePunishNode(fromPeer, orphan_state, /*via_compact_block*/ false)) { + setMisbehaving.insert(fromPeer); + } LogPrint(BCLog::MEMPOOL, " invalid orphan tx %s\n", orphanHash.ToString()); } // Has inputs but not accepted to mempool // Probably non-standard or insufficient fee LogPrint(BCLog::MEMPOOL, " removed orphan tx %s\n", orphanHash.ToString()); - if (!orphanTx.HasWitness() && !stateDummy.CorruptionPossible()) { + assert(IsTransactionReason(orphan_state.GetReason())); + if (!orphanTx.HasWitness() && orphan_state.GetReason() != ValidationInvalidReason::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. @@ -2474,7 +2526,8 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr recentRejects->insert(tx.GetHash()); } } else { - if (!tx.HasWitness() && !state.CorruptionPossible()) { + assert(IsTransactionReason(state.GetReason())); + if (!tx.HasWitness() && state.GetReason() != ValidationInvalidReason::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. @@ -2493,15 +2546,13 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr // to policy, allowing the node to function as a gateway for // nodes hidden behind it. // - // Never relay transactions that we would assign a non-zero DoS - // score for, as we expect peers to do the same with us in that - // case. - int nDoS = 0; - if (!state.IsInvalid(nDoS) || nDoS == 0) { + // Never relay transactions that might result in being + // disconnected (or banned). + if (state.IsInvalid() && TxRelayMayResultInDisconnect(state)) { + LogPrintf("Not relaying invalid transaction %s from whitelisted peer=%d (%s)\n", tx.GetHash().ToString(), pfrom->GetId(), FormatStateMessage(state)); + } else { LogPrintf("Force relaying tx %s from whitelisted peer=%d\n", tx.GetHash().ToString(), pfrom->GetId()); RelayTransaction(tx, connman); - } else { - LogPrintf("Not relaying invalid transaction %s from whitelisted peer=%d (%s)\n", tx.GetHash().ToString(), pfrom->GetId(), FormatStateMessage(state)); } } } @@ -2526,8 +2577,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr // peer simply for relaying a tx that our recentRejects has caught, // regardless of false positives. - int nDoS = 0; - if (state.IsInvalid(nDoS)) + if (state.IsInvalid()) { LogPrint(BCLog::MEMPOOLREJ, "%s from peer=%d was not accepted: %s\n", tx.GetHash().ToString(), pfrom->GetId(), @@ -2536,9 +2586,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr connman->PushMessage(pfrom, msgMaker.Make(NetMsgType::REJECT, strCommand, (unsigned char)state.GetRejectCode(), state.GetRejectReason().substr(0, MAX_REJECT_MESSAGE_LENGTH), inv.hash)); } - if (nDoS > 0) { - Misbehaving(pfrom->GetId(), nDoS); - } + MaybePunishNode(pfrom->GetId(), state, /*via_compact_block*/ false); } return true; } @@ -2574,14 +2622,8 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr const CBlockIndex *pindex = nullptr; CValidationState state; if (!ProcessNewBlockHeaders({cmpctblock.header}, state, chainparams, &pindex)) { - int nDoS; - if (state.IsInvalid(nDoS)) { - if (nDoS > 0) { - LOCK(cs_main); - Misbehaving(pfrom->GetId(), nDoS, strprintf("Peer %d sent us invalid header via cmpctblock\n", pfrom->GetId())); - } else { - LogPrint(BCLog::NET, "Peer %d sent us invalid header via cmpctblock\n", pfrom->GetId()); - } + if (state.IsInvalid()) { + MaybePunishNode(pfrom->GetId(), state, /*via_compact_block*/ true, "invalid header via cmpctblock"); return true; } } @@ -2731,7 +2773,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr // the peer if the header turns out to be for an invalid block. // Note that if a peer tries to build on an invalid chain, that // will be detected and the peer will be banned. - return ProcessHeadersMessage(pfrom, connman, {cmpctblock.header}, chainparams, /*punish_duplicate_invalid=*/false); + return ProcessHeadersMessage(pfrom, connman, {cmpctblock.header}, chainparams, /*via_compact_block=*/true); } if (fBlockReconstructed) { @@ -2874,12 +2916,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr ReadCompactSize(vRecv); // ignore tx count; assume it is 0. } - // Headers received via a HEADERS message should be valid, and reflect - // the chain the peer is on. If we receive a known-invalid header, - // disconnect the peer if it is using one of our outbound connection - // slots. - bool should_punish = !pfrom->fInbound && !pfrom->m_manual_connection; - return ProcessHeadersMessage(pfrom, connman, headers, chainparams, should_punish); + return ProcessHeadersMessage(pfrom, connman, headers, chainparams, /*via_compact_block=*/false); } if (strCommand == NetMsgType::BLOCK) diff --git a/src/test/txvalidation_tests.cpp b/src/test/txvalidation_tests.cpp index 331c340b74..26ae7be202 100644 --- a/src/test/txvalidation_tests.cpp +++ b/src/test/txvalidation_tests.cpp @@ -52,10 +52,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"); - - int nDoS; - BOOST_CHECK_EQUAL(state.IsInvalid(nDoS), true); - BOOST_CHECK_EQUAL(nDoS, 100); + BOOST_CHECK(state.GetReason() == ValidationInvalidReason::CONSENSUS); } BOOST_AUTO_TEST_SUITE_END() diff --git a/src/validation.cpp b/src/validation.cpp index c5aded55a3..faac60cdbf 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -602,28 +602,28 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool // Coinbase is only valid in a block, not as a loose transaction if (tx.IsCoinBase()) - return state.DoS(100, false, REJECT_INVALID, "coinbase"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "coinbase"); // Rather not work on nonstandard transactions (unless -testnet/-regtest) std::string reason; if (fRequireStandard && !IsStandardTx(tx, reason)) - return state.DoS(0, false, REJECT_NONSTANDARD, reason); + return state.Invalid(ValidationInvalidReason::TX_NOT_STANDARD, false, REJECT_NONSTANDARD, 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 reduce unnecessary malloc overhead. if (::GetSerializeSize(tx, PROTOCOL_VERSION | SERIALIZE_TRANSACTION_NO_WITNESS) < MIN_STANDARD_TX_NONWITNESS_SIZE) - return state.DoS(0, false, REJECT_NONSTANDARD, "tx-size-small"); + return state.Invalid(ValidationInvalidReason::TX_NOT_STANDARD, false, REJECT_NONSTANDARD, "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.DoS(0, false, REJECT_NONSTANDARD, "non-final"); + return state.Invalid(ValidationInvalidReason::TX_PREMATURE_SPEND, false, REJECT_NONSTANDARD, "non-final"); // is it already in the memory pool? if (pool.exists(hash)) { - return state.Invalid(false, REJECT_DUPLICATE, "txn-already-in-mempool"); + return state.Invalid(ValidationInvalidReason::TX_CONFLICT, false, REJECT_DUPLICATE, "txn-already-in-mempool"); } // Check for conflicts with in-memory transactions @@ -663,7 +663,7 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool } } if (fReplacementOptOut) { - return state.Invalid(false, REJECT_DUPLICATE, "txn-mempool-conflict"); + return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, REJECT_DUPLICATE, "txn-mempool-conflict"); } setConflicts.insert(ptxConflicting->GetHash()); @@ -687,7 +687,7 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool // Note that the witness vector must be size 0 or len(vin) if (!tx.witness.vtxinwit.empty() && !tx.witness.vtxinwit[input_index].m_pegin_witness.IsNull()) { - return state.Invalid(false, REJECT_PEGIN, "extra-pegin-witness"); + return state.Invalid(ValidationInvalidReason::TX_WITNESS_MUTATED, false, REJECT_PEGIN, "extra-pegin-witness"); } } } @@ -706,13 +706,13 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool tx.witness.vtxinwit[i].m_pegin_witness.stack.size() < 6 || uint256(tx.witness.vtxinwit[i].m_pegin_witness.stack[2]).IsNull() || tx.vin[i].prevout.hash.IsNull()) { - return state.Invalid(false, REJECT_INVALID, "pegin-no-witness"); + return state.Invalid(ValidationInvalidReason::TX_WITNESS_MUTATED, false, REJECT_INVALID, "pegin-no-witness"); } std::pair pegin = std::make_pair(uint256(tx.witness.vtxinwit[i].m_pegin_witness.stack[2]), tx.vin[i].prevout); // This assumes non-null prevout and genesis block hash if (view.IsPeginSpent(pegin)) { - return state.Invalid(false, REJECT_INVALID, "pegin-already-claimed"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "pegin-already-claimed"); } continue; } @@ -729,7 +729,7 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool for (size_t out = 0; out < tx.vout.size(); out++) { // Optimistically just do efficient check of cache for outputs if (pcoinsTip->HaveCoinInCache(COutPoint(hash, out))) { - return state.Invalid(false, REJECT_DUPLICATE, "txn-already-known"); + return state.Invalid(ValidationInvalidReason::TX_CONFLICT, false, REJECT_DUPLICATE, "txn-already-known"); } } // Otherwise assume this might be an orphan tx for which we just haven't seen parents yet @@ -752,7 +752,7 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool // Must keep pool.cs for this unless we change CheckSequenceLocks to take a // CoinsViewCache instead of create its own if (!CheckSequenceLocks(pool, tx, STANDARD_LOCKTIME_VERIFY_FLAGS, &lp)) - return state.DoS(0, false, REJECT_NONSTANDARD, "non-BIP68-final"); + return state.Invalid(ValidationInvalidReason::TX_PREMATURE_SPEND, false, REJECT_NONSTANDARD, "non-BIP68-final"); CAmountMap fee_map; if (!Consensus::CheckTxInputs(tx, state, view, GetSpendHeight(view), fee_map, setPeginsSpent, NULL, true, true)) { @@ -761,11 +761,11 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool // Check for non-standard pay-to-script-hash in inputs if (fRequireStandard && !AreInputsStandard(tx, view)) - return state.Invalid(false, REJECT_NONSTANDARD, "bad-txns-nonstandard-inputs"); + return state.Invalid(ValidationInvalidReason::TX_NOT_STANDARD, false, REJECT_NONSTANDARD, "bad-txns-nonstandard-inputs"); // Check for non-standard witness in P2WSH if (tx.HasWitness() && fRequireStandard && !IsWitnessStandard(tx, view)) - return state.DoS(0, false, REJECT_NONSTANDARD, "bad-witness-nonstandard", true); + return state.Invalid(ValidationInvalidReason::TX_WITNESS_MUTATED, false, REJECT_NONSTANDARD, "bad-witness-nonstandard"); int64_t nSigOpsCost = GetTransactionSigOpCost(tx, view, STANDARD_SCRIPT_VERIFY_FLAGS); @@ -795,27 +795,22 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool fSpendsCoinbase, nSigOpsCost, lp, setPeginsSpent); unsigned int nSize = entry.GetTxSize(); - // Check that the transaction doesn't have an excessive number of - // sigops, making it impossible to mine. Since the coinbase transaction - // itself can contain sigops MAX_STANDARD_TX_SIGOPS is less than - // MAX_BLOCK_SIGOPS; we still consider this an invalid rather than - // merely non-standard transaction. if (nSigOpsCost > MAX_STANDARD_TX_SIGOPS_COST) - return state.DoS(0, false, REJECT_NONSTANDARD, "bad-txns-too-many-sigops", false, + return state.Invalid(ValidationInvalidReason::TX_NOT_STANDARD, false, REJECT_NONSTANDARD, "bad-txns-too-many-sigops", strprintf("%d", nSigOpsCost)); CAmount mempoolRejectFee = pool.GetMinFee(gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000).GetFee(nSize); if (!bypass_limits && mempoolRejectFee > 0 && nModifiedFees < mempoolRejectFee) { - return state.DoS(0, false, REJECT_INSUFFICIENTFEE, "mempool min fee not met", false, strprintf("%d < %d", nModifiedFees, mempoolRejectFee)); + return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, REJECT_INSUFFICIENTFEE, "mempool min fee not met", strprintf("%d < %d", nModifiedFees, mempoolRejectFee)); } // No transactions are allowed below minRelayTxFee except from disconnected blocks if (!bypass_limits && nModifiedFees < ::minRelayTxFee.GetFee(nSize)) { - return state.DoS(0, false, REJECT_INSUFFICIENTFEE, "min relay fee not met", false, strprintf("%d < %d", nModifiedFees, ::minRelayTxFee.GetFee(nSize))); + return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, REJECT_INSUFFICIENTFEE, "min relay fee not met", strprintf("%d < %d", nModifiedFees, ::minRelayTxFee.GetFee(nSize))); } if (nAbsurdFee && nFees > nAbsurdFee) - return state.Invalid(false, + return state.Invalid(ValidationInvalidReason::TX_NOT_STANDARD, false, REJECT_HIGHFEE, "absurdly-high-fee", strprintf("%d > %d", nFees, nAbsurdFee)); @@ -827,7 +822,7 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool size_t nLimitDescendantSize = gArgs.GetArg("-limitdescendantsize", DEFAULT_DESCENDANT_SIZE_LIMIT)*1000; std::string errString; if (!pool.CalculateMemPoolAncestors(entry, setAncestors, nLimitAncestors, nLimitAncestorSize, nLimitDescendants, nLimitDescendantSize, errString)) { - return state.DoS(0, false, REJECT_NONSTANDARD, "too-long-mempool-chain", false, errString); + return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, REJECT_NONSTANDARD, "too-long-mempool-chain", errString); } // A transaction that spends outputs that would be replaced by it is invalid. Now @@ -839,8 +834,7 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool const uint256 &hashAncestor = ancestorIt->GetTx().GetHash(); if (setConflicts.count(hashAncestor)) { - return state.DoS(10, false, - REJECT_INVALID, "bad-txns-spends-conflicting-tx", false, + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-txns-spends-conflicting-tx", strprintf("%s spends conflicting transaction %s", hash.ToString(), hashAncestor.ToString())); @@ -882,8 +876,7 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool CFeeRate oldFeeRate(mi->GetModifiedFee(), mi->GetTxSize()); if (newFeeRate <= oldFeeRate) { - return state.DoS(0, false, - REJECT_INSUFFICIENTFEE, "insufficient fee", false, + return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, REJECT_INSUFFICIENTFEE, "insufficient fee", strprintf("rejecting replacement %s; new feerate %s <= old feerate %s", hash.ToString(), newFeeRate.ToString(), @@ -911,8 +904,7 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool nConflictingSize += it->GetTxSize(); } } else { - return state.DoS(0, false, - REJECT_NONSTANDARD, "too many potential replacements", false, + return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, REJECT_NONSTANDARD, "too many potential replacements", strprintf("rejecting replacement %s; too many potential replacements (%d > %d)\n", hash.ToString(), nConflictingCount, @@ -931,8 +923,7 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool // it's cheaper to just check if the new input refers to a // tx that's in the mempool. if (pool.exists(tx.vin[j].prevout.hash)) { - return state.DoS(0, false, - REJECT_NONSTANDARD, "replacement-adds-unconfirmed", false, + return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, REJECT_NONSTANDARD, "replacement-adds-unconfirmed", strprintf("replacement %s adds unconfirmed input, idx %d", hash.ToString(), j)); } @@ -944,8 +935,7 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool // transactions would not be paid for. if (nModifiedFees < nConflictingFees) { - return state.DoS(0, false, - REJECT_INSUFFICIENTFEE, "insufficient fee", false, + return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, REJECT_INSUFFICIENTFEE, "insufficient fee", strprintf("rejecting replacement %s, less fees than conflicting txs; %s < %s", hash.ToString(), FormatMoney(nModifiedFees), FormatMoney(nConflictingFees))); } @@ -955,8 +945,7 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool CAmount nDeltaFees = nModifiedFees - nConflictingFees; if (nDeltaFees < ::incrementalRelayFee.GetFee(nSize)) { - return state.DoS(0, false, - REJECT_INSUFFICIENTFEE, "insufficient fee", false, + return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, REJECT_INSUFFICIENTFEE, "insufficient fee", strprintf("rejecting replacement %s, not enough additional fees to relay; %s < %s", hash.ToString(), FormatMoney(nDeltaFees), @@ -974,12 +963,13 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool // 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, view, true, scriptVerifyFlags & ~(SCRIPT_VERIFY_WITNESS | SCRIPT_VERIFY_CLEANSTACK), true, false, txdata)) { - if (!CheckInputs(tx, stateDummy, view, true, scriptVerifyFlags & ~SCRIPT_VERIFY_CLEANSTACK, true, false, txdata)) { - // Only the witness is missing, so the transaction itself may be fine. - state.SetCorruptionPossible(); - } + if (!tx.HasWitness() && CheckInputs(tx, stateDummy, view, true, scriptVerifyFlags & ~(SCRIPT_VERIFY_WITNESS | SCRIPT_VERIFY_CLEANSTACK), true, false, txdata) && + !CheckInputs(tx, stateDummy, view, true, 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.GetRejectCode(), state.GetRejectReason(), state.GetDebugMessage()); } + assert(IsTransactionReason(state.GetReason())); return false; // state filled in by CheckInputs } @@ -1036,7 +1026,7 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool if (!bypass_limits) { LimitMempoolSize(pool, gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000, gArgs.GetArg("-mempoolexpiry", DEFAULT_MEMPOOL_EXPIRY) * 60 * 60); if (!pool.exists(hash)) - return state.DoS(0, false, REJECT_INSUFFICIENTFEE, "mempool full"); + return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, REJECT_INSUFFICIENTFEE, "mempool full"); } } @@ -1377,7 +1367,7 @@ void static InvalidChainFound(CBlockIndex* pindexNew) EXCLUSIVE_LOCKS_REQUIRED(c } void CChainState::InvalidBlockFound(CBlockIndex *pindex, const CValidationState &state) { - if (!state.CorruptionPossible()) { + if (state.GetReason() != ValidationInvalidReason::BLOCK_MUTATED) { pindex->nStatus |= BLOCK_FAILED_VALID; m_failed_blocks.insert(pindex); setDirtyBlockIndex.insert(pindex); @@ -1454,6 +1444,9 @@ void InitScriptExecutionCache() { * which are matched. This is useful for checking blocks where we will likely never need the cache * entry again. * + * Note that we may set state.reason to NOT_STANDARD for extra soft-fork flags in flags, block-checking + * callers should probably reset it to CONSENSUS in such cases. + * * Non-static (and re-declared) in src/test/txvalidationcache_tests.cpp */ bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsViewCache &inputs, bool fScriptChecks, unsigned int flags, bool cacheSigStore, bool cacheFullScriptStore, PrecomputedTransactionData& txdata, std::vector *pvChecks) EXCLUSIVE_LOCKS_REQUIRED(cs_main) @@ -1516,23 +1509,27 @@ bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsVi // Check whether the failure was caused by a // non-mandatory script verification check, such as // non-standard DER encodings or non-null dummy - // arguments; if so, don't trigger DoS protection to - // avoid splitting the network between upgraded and - // non-upgraded nodes. + // arguments; if so, ensure we return NOT_STANDARD + // instead of CONSENSUS to avoid downstream users + // splitting the network between upgraded and + // non-upgraded nodes by banning CONSENSUS-failing + // data providers. CScriptCheck check2(coin.out, tx, i, flags & ~STANDARD_NOT_MANDATORY_VERIFY_FLAGS, cacheSigStore, &txdata); if (check2()) { - return state.Invalid(false, REJECT_NONSTANDARD, strprintf("non-mandatory-script-verify-flag (%s)", ScriptErrorString(serror))); + return state.Invalid(ValidationInvalidReason::TX_NOT_STANDARD, false, REJECT_NONSTANDARD, strprintf("non-mandatory-script-verify-flag (%s)", ScriptErrorString(serror))); } } - // Failures of other flags indicate a transaction that is - // invalid in new blocks, e.g. an invalid P2SH. We DoS ban - // such nodes as they are not following the protocol. That - // said during an upgrade careful thought should be taken - // as to the correct behavior - we may want to continue - // peering with non-upgraded nodes even after soft-fork - // super-majority signaling has occurred. - return state.DoS(100,false, REJECT_INVALID, strprintf("mandatory-script-verify-flag-failed (%s)", ScriptErrorString(serror))); + // MANDATORY flag failures correspond to + // ValidationInvalidReason::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 + // could be due to non-upgraded nodes which we may want to + // 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, REJECT_INVALID, strprintf("mandatory-script-verify-flag-failed (%s)", ScriptErrorString(serror))); } } @@ -1943,7 +1940,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.CorruptionPossible()) { + if (state.GetReason() == ValidationInvalidReason::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. @@ -1960,7 +1957,7 @@ bool CChainState::ConnectBlock(const CBlock& block, CValidationState& state, CBl for (auto& txout : block.vtx[0]->vout) { bool mustPay = !txout.nValue.IsExplicit() || txout.nValue.GetAmount() != 0; if (mustPay && txout.scriptPubKey != mandatory_coinbase_destination) { - return state.DoS(100, error("ConnectBlock(): Coinbase outputs didn't match required scriptPubKey"), + return state.Invalid(ValidationInvalidReason::CONSENSUS, error("ConnectBlock(): Coinbase outputs didn't match required scriptPubKey"), REJECT_INVALID, "bad-coinbase-txos"); } } @@ -2079,7 +2076,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.DoS(100, error("ConnectBlock(): tried to overwrite transaction"), + return state.Invalid(ValidationInvalidReason::CONSENSUS, error("ConnectBlock(): tried to overwrite transaction"), REJECT_INVALID, "bad-txns-BIP30"); } } @@ -2127,12 +2124,20 @@ bool CChainState::ConnectBlock(const CBlock& block, CValidationState& state, CBl if (!Consensus::CheckTxInputs(tx, state, view, pindex->nHeight, fee_map, setPeginsSpent == NULL ? setPeginsSpentDummy : *setPeginsSpent, nScriptCheckThreads ? &vChecks : NULL, fCacheResults, fScriptChecks)) { + 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.GetRejectCode(), state.GetRejectReason(), state.GetDebugMessage()); + } return error("%s: Consensus::CheckTxInputs: %s, %s", __func__, tx.GetHash().ToString(), FormatStateMessage(state)); } control.Add(vChecks); if (!MoneyRange(fee_map)) - return state.DoS(100, error("ConnectBlock(): total block reward overflowed"), REJECT_INVALID, "bad-blockreward-outofrange"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, error("ConnectBlock(): total block reward overflowed"), REJECT_INVALID, "bad-blockreward-outofrange"); // Check that transaction is BIP68 final // BIP68 lock checks (as opposed to nLockTime checks) must @@ -2147,7 +2152,7 @@ bool CChainState::ConnectBlock(const CBlock& block, CValidationState& state, CBl } if (!SequenceLocks(tx, nLockTimeFlags, &prevheights, *pindex)) { - return state.DoS(100, error("%s: contains a non-BIP68-final transaction", __func__), + return state.Invalid(ValidationInvalidReason::CONSENSUS, error("%s: contains a non-BIP68-final transaction", __func__), REJECT_INVALID, "bad-txns-nonfinal"); } } @@ -2158,7 +2163,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.DoS(100, error("ConnectBlock(): too many sigops"), + return state.Invalid(ValidationInvalidReason::CONSENSUS, error("ConnectBlock(): too many sigops"), REJECT_INVALID, "bad-blk-sigops"); txdata.emplace_back(tx); @@ -2166,9 +2171,20 @@ 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 (!CheckInputs(tx, state, view, fScriptChecks, flags, fCacheResults, fCacheResults, txdata[i], nScriptCheckThreads ? &vChecks : nullptr)) + if (!CheckInputs(tx, state, view, fScriptChecks, 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.GetRejectCode(), state.GetRejectReason(), state.GetDebugMessage()); + } return error("ConnectBlock(): CheckInputs on %s failed with %s", tx.GetHash().ToString(), FormatStateMessage(state)); + } control.Add(vChecks); } @@ -2185,14 +2201,14 @@ bool CChainState::ConnectBlock(const CBlock& block, CValidationState& state, CBl CAmountMap block_reward = fee_map; block_reward[consensusParams.subsidy_asset] += GetBlockSubsidy(pindex->nHeight, consensusParams); if (!MoneyRange(block_reward)) - return state.DoS(100, error("ConnectBlock(): total block reward overflowed"), REJECT_INVALID, "bad-blockreward-outofrange"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, error("ConnectBlock(): total block reward overflowed"), REJECT_INVALID, "bad-blockreward-outofrange"); if (!VerifyCoinbaseAmount(*(block.vtx[0]), block_reward)) { - return state.DoS(100, error("ConnectBlock(): coinbase pays too much (limit=%d)", + return state.Invalid(ValidationInvalidReason::CONSENSUS, error("ConnectBlock(): coinbase pays too much (limit=%d)", block_reward[consensusParams.subsidy_asset]), REJECT_INVALID, "bad-cb-amount"); } if (!control.Wait()) - return state.DoS(100, error("%s: CheckQueue failed", __func__), REJECT_INVALID, "block-validation-failed"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, error("%s: CheckQueue failed", __func__), REJECT_INVALID, "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); @@ -2765,7 +2781,7 @@ 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.CorruptionPossible()) { + if (state.GetReason() != ValidationInvalidReason::BLOCK_MUTATED) { InvalidChainFound(vpindexToConnect.front()); } state = CValidationState(); @@ -3264,7 +3280,7 @@ static bool CheckBlockHeader(const CBlockHeader& block, CValidationState& state, // Check proof of work matches claimed amount if (fCheckPOW && block.GetHash() != consensusParams.hashGenesisBlock && !CheckProof(block, consensusParams)) { - return state.DoS(50, false, REJECT_INVALID, g_signed_blocks ? "block-proof-invalid" : "high-hash", false, "proof of work failed"); + return state.Invalid(ValidationInvalidReason::BLOCK_INVALID_HEADER, false, REJECT_INVALID, g_signed_blocks ? "block-proof-invalid" : "high-hash", "proof of work failed"); } return true; } @@ -3286,13 +3302,13 @@ bool CheckBlock(const CBlock& block, CValidationState& state, const Consensus::P bool mutated; uint256 hashMerkleRoot2 = BlockMerkleRoot(block, &mutated); if (block.hashMerkleRoot != hashMerkleRoot2) - return state.DoS(100, false, REJECT_INVALID, "bad-txnmrklroot", true, "hashMerkleRoot mismatch"); + return state.Invalid(ValidationInvalidReason::BLOCK_MUTATED, false, REJECT_INVALID, "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.DoS(100, false, REJECT_INVALID, "bad-txns-duplicate", true, "duplicate transaction"); + return state.Invalid(ValidationInvalidReason::BLOCK_MUTATED, false, REJECT_INVALID, "bad-txns-duplicate", "duplicate transaction"); } // All potential-corruption validation must be done before we do any @@ -3303,19 +3319,19 @@ 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.DoS(100, false, REJECT_INVALID, "bad-blk-length", false, "size limits failed"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "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.DoS(100, false, REJECT_INVALID, "bad-cb-missing", false, "first tx is not coinbase"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "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.DoS(100, false, REJECT_INVALID, "bad-cb-multiple", false, "more than one coinbase"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-cb-multiple", "more than one coinbase"); // Check transactions for (const auto& tx : block.vtx) if (!CheckTransaction(*tx, state, true)) - return state.Invalid(false, state.GetRejectCode(), state.GetRejectReason(), + return state.Invalid(state.GetReason(), false, state.GetRejectCode(), state.GetRejectReason(), strprintf("Transaction check failed (tx hash %s) %s", tx->GetHash().ToString(), state.GetDebugMessage())); unsigned int nSigOps = 0; @@ -3324,7 +3340,7 @@ bool CheckBlock(const CBlock& block, CValidationState& state, const Consensus::P nSigOps += GetLegacySigOpCount(*tx); } if (nSigOps * WITNESS_SCALE_FACTOR > MAX_BLOCK_SIGOPS_COST) - return state.DoS(100, false, REJECT_INVALID, "bad-blk-sigops", false, "out-of-bounds SigOpCount"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-blk-sigops", "out-of-bounds SigOpCount"); if (fCheckPOW && fCheckMerkleRoot) block.fChecked = true; @@ -3513,7 +3529,7 @@ static bool ContextualCheckBlockHeader(const CBlockHeader& block, CValidationSta // Check proof of work if necessary const Consensus::Params& consensusParams = params.GetConsensus(); if (!CheckChallenge(block, *pindexPrev, consensusParams)) - return state.DoS(100, false, REJECT_INVALID, "bad-diffbits", false, "incorrect proof of work"); + return state.Invalid(ValidationInvalidReason::BLOCK_INVALID_HEADER, false, REJECT_INVALID, "bad-diffbits", "incorrect proof of work"); // Check against checkpoints if (fCheckpointsEnabled) { @@ -3522,28 +3538,28 @@ static bool ContextualCheckBlockHeader(const CBlockHeader& block, CValidationSta // MapBlockIndex. CBlockIndex* pcheckpoint = GetLastCheckpoint(params.Checkpoints()); if (pcheckpoint && nHeight < pcheckpoint->nHeight) - return state.DoS(100, error("%s: forked chain older than last checkpoint (height %d)", __func__, nHeight), REJECT_CHECKPOINT, "bad-fork-prior-to-checkpoint"); + return state.Invalid(ValidationInvalidReason::BLOCK_CHECKPOINT, error("%s: forked chain older than last checkpoint (height %d)", __func__, nHeight), REJECT_CHECKPOINT, "bad-fork-prior-to-checkpoint"); } // Check timestamp against prev if (block.GetBlockTime() <= pindexPrev->GetMedianTimePast()) - return state.Invalid(false, REJECT_INVALID, "time-too-old", "block's timestamp is too early"); + return state.Invalid(ValidationInvalidReason::BLOCK_INVALID_HEADER, false, REJECT_INVALID, "time-too-old", "block's timestamp is too early"); // Check height in header against prev if (g_con_blockheightinheader && (uint32_t)nHeight != block.block_height) - return state.Invalid(error("%s: block height in header is incorrect", __func__), + return state.Invalid(ValidationInvalidReason::BLOCK_INVALID_HEADER, error("%s: block height in header is incorrect", __func__), REJECT_INVALID, "bad-header-height"); // Check timestamp if (block.GetBlockTime() > nAdjustedTime + MAX_FUTURE_BLOCK_TIME) - return state.Invalid(false, REJECT_INVALID, "time-too-new", "block timestamp too far in the future"); + return state.Invalid(ValidationInvalidReason::BLOCK_TIME_FUTURE, false, REJECT_INVALID, "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(false, REJECT_OBSOLETE, strprintf("bad-version(0x%08x)", block.nVersion), + return state.Invalid(ValidationInvalidReason::BLOCK_INVALID_HEADER, false, REJECT_OBSOLETE, strprintf("bad-version(0x%08x)", block.nVersion), strprintf("rejected nVersion=0x%08x block", block.nVersion)); return true; @@ -3573,7 +3589,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.DoS(10, false, REJECT_INVALID, "bad-txns-nonfinal", false, "non-final transaction"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-txns-nonfinal", "non-final transaction"); } } @@ -3583,7 +3599,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.DoS(100, false, REJECT_INVALID, "bad-cb-height", false, "block height mismatch in coinbase"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-cb-height", "block height mismatch in coinbase"); } } @@ -3593,7 +3609,7 @@ static bool ContextualCheckBlock(const CBlock& block, CValidationState& state, c if (!inwit.vchIssuanceAmountRangeproof.empty() || !inwit.vchInflationKeysRangeproof.empty() || !inwit.m_pegin_witness.IsNull()) { - return state.DoS(100, false, REJECT_INVALID, "bad-cb-witness", true, "Coinbase has invalid input witness data."); + return state.Invalid(ValidationInvalidReason::BLOCK_MUTATED, false, REJECT_INVALID, "bad-cb-witness", "Coinbase has invalid input witness data."); } } @@ -3617,12 +3633,12 @@ static bool ContextualCheckBlock(const CBlock& block, CValidationState& state, c if ((block.vtx[0]->witness.vtxinwit.empty()) || (block.vtx[0]->witness.vtxinwit[0].scriptWitness.stack.size() != 1) || (block.vtx[0]->witness.vtxinwit[0].scriptWitness.stack[0].size() != 32)) { - return state.DoS(100, false, REJECT_INVALID, "bad-witness-nonce-size", true, strprintf("%s : invalid witness reserved value size", __func__)); + return state.Invalid(ValidationInvalidReason::BLOCK_MUTATED, false, REJECT_INVALID, "bad-witness-nonce-size", strprintf("%s : invalid witness reserved value size", __func__)); } CHash256().Write(hashWitness.begin(), 32).Write(&block.vtx[0]->witness.vtxinwit[0].scriptWitness.stack[0][0], 32).Finalize(hashWitness.begin()); uint256 committedWitness(std::vector(&block.vtx[0]->vout[commitpos].scriptPubKey[6], &block.vtx[0]->vout[commitpos].scriptPubKey[6+32])); if (memcmp(hashWitness.begin(), &block.vtx[0]->vout[commitpos].scriptPubKey[6], 32)) { - return state.DoS(100, false, REJECT_INVALID, "bad-witness-merkle-match", true, strprintf("%s : witness merkle commitment mismatch: calculated: %s found commitment: %s", __func__, hashWitness.GetHex(), committedWitness.GetHex())); + return state.Invalid(ValidationInvalidReason::BLOCK_MUTATED, false, REJECT_INVALID, "bad-witness-merkle-match", strprintf("%s : witness merkle commitment mismatch", __func__)); } fHaveWitness = true; } @@ -3632,7 +3648,7 @@ static bool ContextualCheckBlock(const CBlock& block, CValidationState& state, c if (!fHaveWitness) { for (const auto& tx : block.vtx) { if (tx->HasWitness()) { - return state.DoS(100, false, REJECT_INVALID, "unexpected-witness", true, strprintf("%s : unexpected witness data found", __func__)); + return state.Invalid(ValidationInvalidReason::BLOCK_MUTATED, false, REJECT_INVALID, "unexpected-witness", strprintf("%s : unexpected witness data found", __func__)); } } } @@ -3644,7 +3660,7 @@ 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.DoS(100, false, REJECT_INVALID, "bad-blk-weight", false, strprintf("%s : weight limit failed", __func__)); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-blk-weight", strprintf("%s : weight limit failed", __func__)); } return true; @@ -3664,7 +3680,7 @@ bool CChainState::AcceptBlockHeader(const CBlockHeader& block, CValidationState& if (ppindex) *ppindex = pindex; if (pindex->nStatus & BLOCK_FAILED_MASK) - return state.Invalid(error("%s: block %s is marked invalid", __func__, hash.ToString()), 0, "duplicate"); + return state.Invalid(ValidationInvalidReason::CACHED_INVALID, error("%s: block %s is marked invalid", __func__, hash.ToString()), 0, "duplicate"); return true; } @@ -3675,10 +3691,10 @@ bool CChainState::AcceptBlockHeader(const CBlockHeader& block, CValidationState& CBlockIndex* pindexPrev = nullptr; BlockMap::iterator mi = mapBlockIndex.find(block.hashPrevBlock); if (mi == mapBlockIndex.end()) - return state.DoS(10, error("%s: prev block not found", __func__), 0, "prev-blk-not-found"); + return state.Invalid(ValidationInvalidReason::BLOCK_MISSING_PREV, error("%s: prev block not found", __func__), 0, "prev-blk-not-found"); pindexPrev = (*mi).second; if (pindexPrev->nStatus & BLOCK_FAILED_MASK) - return state.DoS(100, error("%s: prev block invalid", __func__), REJECT_INVALID, "bad-prevblk"); + return state.Invalid(ValidationInvalidReason::BLOCK_INVALID_PREV, error("%s: prev block invalid", __func__), REJECT_INVALID, "bad-prevblk"); if (!ContextualCheckBlockHeader(block, state, chainparams, pindexPrev, GetAdjustedTime())) return error("%s: Consensus::ContextualCheckBlockHeader: %s, %s", __func__, hash.ToString(), FormatStateMessage(state)); @@ -3715,7 +3731,7 @@ bool CChainState::AcceptBlockHeader(const CBlockHeader& block, CValidationState& setDirtyBlockIndex.insert(invalid_walk); invalid_walk = invalid_walk->pprev; } - return state.DoS(100, error("%s: prev block invalid", __func__), REJECT_INVALID, "bad-prevblk"); + return state.Invalid(ValidationInvalidReason::BLOCK_INVALID_PREV, error("%s: prev block invalid", __func__), REJECT_INVALID, "bad-prevblk"); } } } @@ -3820,7 +3836,8 @@ bool CChainState::AcceptBlock(const std::shared_ptr& pblock, CVali if (chainparams.GetConsensus().hashGenesisBlock != block.GetHash() && (!CheckBlock(block, state, chainparams.GetConsensus()) || !ContextualCheckBlock(block, state, chainparams.GetConsensus(), pindex->pprev))) { - if (state.IsInvalid() && !state.CorruptionPossible()) { + assert(IsBlockReason(state.GetReason())); + if (state.IsInvalid() && state.GetReason() != ValidationInvalidReason::BLOCK_MUTATED) { pindex->nStatus |= BLOCK_FAILED_VALID; setDirtyBlockIndex.insert(pindex); } diff --git a/test/functional/data/invalid_txs.py b/test/functional/data/invalid_txs.py index aa740b1b8a..c0422ba0af 100644 --- a/test/functional/data/invalid_txs.py +++ b/test/functional/data/invalid_txs.py @@ -58,7 +58,7 @@ class BadTxTemplate: class OutputMissing(BadTxTemplate): reject_reason = "bad-txns-vout-empty" - expect_disconnect = False + expect_disconnect = True def get_tx(self): tx = CTransaction() @@ -69,7 +69,7 @@ class OutputMissing(BadTxTemplate): class InputMissing(BadTxTemplate): reject_reason = "bad-txns-vin-empty" - expect_disconnect = False + expect_disconnect = True # We use a blank transaction here to make sure # it is interpreted as a non-witness transaction. diff --git a/test/functional/feature_block.py b/test/functional/feature_block.py index 713b7e6bf6..e230542b55 100755 --- a/test/functional/feature_block.py +++ b/test/functional/feature_block.py @@ -280,7 +280,7 @@ class FullBlockTest(BitcoinTestFramework): self.log.info("Reject a block spending an immature coinbase.") self.move_tip(15) b20 = self.next_block(20, spend=out[7]) - self.send_blocks([b20], success=False, reject_reason='bad-txns-premature-spend-of-coinbase') + self.send_blocks([b20], success=False, reject_reason='bad-txns-premature-spend-of-coinbase', reconnect=True) # Attempt to spend a coinbase at depth too low (on a fork this time) # genesis -> b1 (0) -> b2 (1) -> b5 (2) -> b6 (3) @@ -293,7 +293,7 @@ class FullBlockTest(BitcoinTestFramework): self.send_blocks([b21], False) b22 = self.next_block(22, spend=out[5]) - self.send_blocks([b22], success=False, reject_reason='bad-txns-premature-spend-of-coinbase') + self.send_blocks([b22], success=False, reject_reason='bad-txns-premature-spend-of-coinbase', reconnect=True) # Create a block on either side of MAX_BLOCK_BASE_SIZE and make sure its accepted/rejected # genesis -> b1 (0) -> b2 (1) -> b5 (2) -> b6 (3) @@ -624,7 +624,7 @@ class FullBlockTest(BitcoinTestFramework): #while b47.sha256 < target: # b47.nNonce += 1 # b47.rehash() - #self.send_blocks([b47], False, force_send=True, reject_reason='high-hash') + #self.send_blocks([b47], False, force_send=True, reject_reason='high-hash', reconnect=True) self.log.info("Reject a block with a timestamp >2 hours in the future") self.move_tip(44) @@ -676,7 +676,7 @@ class FullBlockTest(BitcoinTestFramework): b54 = self.next_block(54, spend=out[15]) b54.nTime = b35.nTime - 1 b54.solve() - self.send_blocks([b54], False, force_send=True, reject_reason='time-too-old') + self.send_blocks([b54], False, force_send=True, reject_reason='time-too-old', reconnect=True) # valid timestamp self.move_tip(53) @@ -822,7 +822,7 @@ class FullBlockTest(BitcoinTestFramework): assert tx.vin[0].nSequence < 0xffffffff tx.calc_sha256() b62 = self.update_block(62, [tx]) - self.send_blocks([b62], success=False, reject_reason='bad-txns-nonfinal') + self.send_blocks([b62], success=False, reject_reason='bad-txns-nonfinal', reconnect=True) # Test a non-final coinbase is also rejected # @@ -836,7 +836,7 @@ class FullBlockTest(BitcoinTestFramework): b63.vtx[0].vin[0].nSequence = 0xDEADBEEF b63.vtx[0].rehash() b63 = self.update_block(63, []) - self.send_blocks([b63], success=False, reject_reason='bad-txns-nonfinal') + self.send_blocks([b63], success=False, reject_reason='bad-txns-nonfinal', reconnect=True) # This checks that a block with a bloated VARINT between the block_header and the array of tx such that # the block is > MAX_BLOCK_BASE_SIZE with the bloated varint, but <= MAX_BLOCK_BASE_SIZE without the bloated varint, @@ -1264,7 +1264,7 @@ class FullBlockTest(BitcoinTestFramework): self.log.info("Reject a block with an invalid block header version") b_v1 = self.next_block('b_v1', version=1) - self.send_blocks([b_v1], success=False, force_send=True, reject_reason='bad-version(0x00000001)') + self.send_blocks([b_v1], success=False, force_send=True, reject_reason='bad-version(0x00000001)', reconnect=True) self.move_tip(chain1_tip + 2) b_cb34 = self.next_block('b_cb34', version=4)