Skip to content

Instantly share code, notes, and snippets.

@jamesob
Created April 3, 2019 15:02
Show Gist options
  • Save jamesob/26a9a2fcdad52097ea4e2d891adb2f99 to your computer and use it in GitHub Desktop.
Save jamesob/26a9a2fcdad52097ea4e2d891adb2f99 to your computer and use it in GitHub Desktop.
15141.sdaftuar.rewrite_dos_interface_be: daf23bf -> daf674e

To recreate:

git remote add jamesob https://github.com/jamesob/bitcoin.git
git fetch jamesob

# Base branch:
# d5dbb45bdf - Merge #15314: [Doc] update release notes for changes up to cb35f1d (8 weeks ago)
#
git diff d5dbb45bdf jamesob:ackr/15141.1.sdaftuar.rewrite_dos_interface_be > 1.daf23bf-base.diff

# Base branch:
# 2c364fde42 - (upstream/master) Merge #14853: depends: latest RapidCheck (24 hours ago)
#
git diff 2c364fde42 jamesob:ackr/15141.2.sdaftuar.rewrite_dos_interface_be > 2.daf674e-base.diff

diff 1.daf23bf-base.diff 2.daf674e-base.diff
259c259
< index 5927a14a6e..72cd346bdb 100644
---
> index 0654a96e26..d8cb564378 100644
262c262
< @@ -349,7 +349,16 @@ struct CNodeState {
---
> @@ -352,7 +352,16 @@ struct CNodeState {
280c280
< @@ -745,7 +754,7 @@ void PeerLogicValidation::InitializeNode(CNode *pnode) {
---
> @@ -748,7 +757,7 @@ void PeerLogicValidation::InitializeNode(CNode *pnode) {
289c289
< @@ -947,6 +956,80 @@ void Misbehaving(NodeId pnode, int howmuch, const std::string& message) EXCLUSIV
---
> @@ -960,6 +969,80 @@ void Misbehaving(NodeId pnode, int howmuch, const std::string& message) EXCLUSIV
370c370
< @@ -1122,14 +1205,12 @@ void PeerLogicValidation::BlockChecked(const CBlock& block, const CValidationSta
---
> @@ -1135,14 +1218,12 @@ void PeerLogicValidation::BlockChecked(const CBlock& block, const CValidationSta
387c387
< @@ -1479,7 +1560,7 @@ inline void static SendBlockTransactions(const CBlock& block, const BlockTransac
---
> @@ -1492,7 +1573,7 @@ inline void static SendBlockTransactions(const CBlock& block, const BlockTransac
396c396
< @@ -1541,48 +1622,8 @@ bool static ProcessHeadersMessage(CNode *pfrom, CConnman *connman, const std::ve
---
> @@ -1554,48 +1635,8 @@ bool static ProcessHeadersMessage(CNode *pfrom, CConnman *connman, const std::ve
447,494c447,490
< @@ -2380,15 +2421,14 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
< const uint256& orphanHash = orphanTx.GetHash();
< NodeId fromPeer = (*mi)->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, &lRemovedTxn, false /* bypass_limits */, 0 /* nAbsurdFee */)) {
< + if (AcceptToMemoryPool(mempool, orphan_state, porphanTx, &fMissingInputs2, &lRemovedTxn, 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++) {
< @@ -2398,19 +2438,20 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
< }
< 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());
< vEraseQueue.push_back(orphanHash);
< - 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.
< @@ -2458,7 +2499,8 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
---
> @@ -1730,13 +1771,13 @@ void static ProcessOrphanTx(CConnman* connman, std::set<uint256>& 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++) {
> @@ -1750,17 +1791,18 @@ void static ProcessOrphanTx(CConnman* connman, std::set<uint256>& 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.
> @@ -2478,7 +2520,8 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
504c500
< @@ -2477,15 +2519,13 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
---
> @@ -2497,15 +2540,13 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
525c521
< @@ -2510,8 +2550,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
---
> @@ -2530,8 +2571,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
535c531
< @@ -2520,9 +2559,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
---
> @@ -2540,9 +2580,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
546c542
< @@ -2552,14 +2589,8 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
---
> @@ -2578,14 +2616,8 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
563c559
< @@ -2709,7 +2740,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
---
> @@ -2735,7 +2767,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
572c568
< @@ -2840,12 +2871,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
---
> @@ -2878,12 +2910,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
585c581
< if (strCommand == NetMsgType::BLOCK && !fImporting && !fReindex) // Ignore blocks received while importing
---
> if (strCommand == NetMsgType::BLOCK)
603c599
< index dbdc1afb35..ee58e7edc8 100644
---
> index ae3985485e..f6c8d42ccb 100644
606c602
< @@ -582,28 +582,28 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool
---
> @@ -587,28 +587,28 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool
640c636
< @@ -639,7 +639,7 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool
---
> @@ -644,7 +644,7 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool
649c645
< @@ -665,7 +665,7 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool
---
> @@ -670,7 +670,7 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool
658c654
< @@ -688,7 +688,7 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool
---
> @@ -693,7 +693,7 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool
667c663
< @@ -697,11 +697,11 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool
---
> @@ -702,11 +702,11 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool
681c677
< @@ -724,27 +724,22 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool
---
> @@ -729,27 +729,22 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool
713c709
< @@ -756,7 +751,7 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool
---
> @@ -761,7 +756,7 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool
722c718
< @@ -768,8 +763,7 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool
---
> @@ -773,8 +768,7 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool
732c728
< @@ -811,8 +805,7 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool
---
> @@ -816,8 +810,7 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool
742c738
< @@ -840,8 +833,7 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool
---
> @@ -845,8 +838,7 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool
752c748
< @@ -860,8 +852,7 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool
---
> @@ -865,8 +857,7 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool
762c758
< @@ -873,8 +864,7 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool
---
> @@ -878,8 +869,7 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool
772c768
< @@ -884,8 +874,7 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool
---
> @@ -889,8 +879,7 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool
782c778
< @@ -906,8 +895,10 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool
---
> @@ -911,8 +900,10 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool
794c790
< @@ -964,7 +955,7 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool
---
> @@ -969,7 +960,7 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool
803c799
< @@ -1297,7 +1288,7 @@ void static InvalidChainFound(CBlockIndex* pindexNew) EXCLUSIVE_LOCKS_REQUIRED(c
---
> @@ -1302,7 +1293,7 @@ void static InvalidChainFound(CBlockIndex* pindexNew) EXCLUSIVE_LOCKS_REQUIRED(c
812c808
< @@ -1365,6 +1356,9 @@ void InitScriptExecutionCache() {
---
> @@ -1370,6 +1361,9 @@ void InitScriptExecutionCache() {
822c818
< @@ -1420,22 +1414,26 @@ bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsVi
---
> @@ -1425,22 +1419,26 @@ bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsVi
861c857
< @@ -1812,7 +1810,7 @@ bool CChainState::ConnectBlock(const CBlock& block, CValidationState& state, CBl
---
> @@ -1800,7 +1798,7 @@ bool CChainState::ConnectBlock(const CBlock& block, CValidationState& state, CBl
870c866
< @@ -1947,7 +1945,7 @@ bool CChainState::ConnectBlock(const CBlock& block, CValidationState& state, CBl
---
> @@ -1935,7 +1933,7 @@ bool CChainState::ConnectBlock(const CBlock& block, CValidationState& state, CBl
879c875
< @@ -1987,11 +1985,19 @@ bool CChainState::ConnectBlock(const CBlock& block, CValidationState& state, CBl
---
> @@ -1975,11 +1973,19 @@ bool CChainState::ConnectBlock(const CBlock& block, CValidationState& state, CBl
900c896
< @@ -2004,7 +2010,7 @@ bool CChainState::ConnectBlock(const CBlock& block, CValidationState& state, CBl
---
> @@ -1992,7 +1998,7 @@ bool CChainState::ConnectBlock(const CBlock& block, CValidationState& state, CBl
909c905
< @@ -2015,7 +2021,7 @@ bool CChainState::ConnectBlock(const CBlock& block, CValidationState& state, CBl
---
> @@ -2003,7 +2009,7 @@ bool CChainState::ConnectBlock(const CBlock& block, CValidationState& state, CBl
918c914
< @@ -2023,9 +2029,21 @@ bool CChainState::ConnectBlock(const CBlock& block, CValidationState& state, CBl
---
> @@ -2011,9 +2017,21 @@ bool CChainState::ConnectBlock(const CBlock& block, CValidationState& state, CBl
941c937
< @@ -2040,13 +2058,13 @@ bool CChainState::ConnectBlock(const CBlock& block, CValidationState& state, CBl
---
> @@ -2028,13 +2046,13 @@ bool CChainState::ConnectBlock(const CBlock& block, CValidationState& state, CBl
957c953
< @@ -2578,7 +2596,7 @@ bool CChainState::ActivateBestChainStep(CValidationState& state, const CChainPar
---
> @@ -2562,7 +2580,7 @@ bool CChainState::ActivateBestChainStep(CValidationState& state, const CChainPar
966c962
< @@ -3066,7 +3084,7 @@ static bool CheckBlockHeader(const CBlockHeader& block, CValidationState& state,
---
> @@ -3060,7 +3078,7 @@ static bool CheckBlockHeader(const CBlockHeader& block, CValidationState& state,
975c971
< @@ -3088,13 +3106,13 @@ bool CheckBlock(const CBlock& block, CValidationState& state, const Consensus::P
---
> @@ -3082,13 +3100,13 @@ bool CheckBlock(const CBlock& block, CValidationState& state, const Consensus::P
991c987
< @@ -3105,20 +3123,22 @@ bool CheckBlock(const CBlock& block, CValidationState& state, const Consensus::P
---
> @@ -3099,20 +3117,22 @@ bool CheckBlock(const CBlock& block, CValidationState& state, const Consensus::P
1021c1017
< @@ -3126,7 +3146,7 @@ bool CheckBlock(const CBlock& block, CValidationState& state, const Consensus::P
---
> @@ -3120,7 +3140,7 @@ bool CheckBlock(const CBlock& block, CValidationState& state, const Consensus::P
1030c1026
< @@ -3219,7 +3239,7 @@ static bool ContextualCheckBlockHeader(const CBlockHeader& block, CValidationSta
---
> @@ -3213,7 +3233,7 @@ static bool ContextualCheckBlockHeader(const CBlockHeader& block, CValidationSta
1039c1035
< @@ -3228,23 +3248,23 @@ static bool ContextualCheckBlockHeader(const CBlockHeader& block, CValidationSta
---
> @@ -3222,23 +3242,23 @@ static bool ContextualCheckBlockHeader(const CBlockHeader& block, CValidationSta
1067c1063
< @@ -3274,7 +3294,7 @@ static bool ContextualCheckBlock(const CBlock& block, CValidationState& state, c
---
> @@ -3268,7 +3288,7 @@ static bool ContextualCheckBlock(const CBlock& block, CValidationState& state, c
1076c1072
< @@ -3284,7 +3304,7 @@ static bool ContextualCheckBlock(const CBlock& block, CValidationState& state, c
---
> @@ -3278,7 +3298,7 @@ static bool ContextualCheckBlock(const CBlock& block, CValidationState& state, c
1085c1081
< @@ -3306,11 +3326,11 @@ static bool ContextualCheckBlock(const CBlock& block, CValidationState& state, c
---
> @@ -3300,11 +3320,11 @@ static bool ContextualCheckBlock(const CBlock& block, CValidationState& state, c
1099c1095
< @@ -3320,7 +3340,7 @@ static bool ContextualCheckBlock(const CBlock& block, CValidationState& state, c
---
> @@ -3314,7 +3334,7 @@ static bool ContextualCheckBlock(const CBlock& block, CValidationState& state, c
1108c1104
< @@ -3332,7 +3352,7 @@ static bool ContextualCheckBlock(const CBlock& block, CValidationState& state, c
---
> @@ -3326,7 +3346,7 @@ static bool ContextualCheckBlock(const CBlock& block, CValidationState& state, c
1117c1113
< @@ -3352,7 +3372,7 @@ bool CChainState::AcceptBlockHeader(const CBlockHeader& block, CValidationState&
---
> @@ -3346,7 +3366,7 @@ bool CChainState::AcceptBlockHeader(const CBlockHeader& block, CValidationState&
1126c1122
< @@ -3363,10 +3383,10 @@ bool CChainState::AcceptBlockHeader(const CBlockHeader& block, CValidationState&
---
> @@ -3357,10 +3377,10 @@ bool CChainState::AcceptBlockHeader(const CBlockHeader& block, CValidationState&
1139c1135
< @@ -3403,7 +3423,7 @@ bool CChainState::AcceptBlockHeader(const CBlockHeader& block, CValidationState&
---
> @@ -3397,7 +3417,7 @@ bool CChainState::AcceptBlockHeader(const CBlockHeader& block, CValidationState&
1148c1144
< @@ -3507,7 +3527,8 @@ bool CChainState::AcceptBlock(const std::shared_ptr<const CBlock>& pblock, CVali
---
> @@ -3501,7 +3521,8 @@ bool CChainState::AcceptBlock(const std::shared_ptr<const CBlock>& pblock, CVali
1181c1177
< index 5253ff7aaa..9df597e8e6 100755
---
> index faf7f20257..9f8fdb5ac3 100755
1221c1217
< assert(tx.vin[0].nSequence < 0xffffffff)
---
> assert tx.vin[0].nSequence < 0xffffffff
diff --git a/src/blockencodings.cpp b/src/blockencodings.cpp
index 10f51931f0..f0fcf675eb 100644
--- a/src/blockencodings.cpp
+++ b/src/blockencodings.cpp
@@ -203,7 +203,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_verify.cpp b/src/consensus/tx_verify.cpp
index 0a7eacfb91..6ecaa8c821 100644
--- a/src/consensus/tx_verify.cpp
+++ b/src/consensus/tx_verify.cpp
@@ -160,24 +160,24 @@ 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 nValueOut = 0;
for (const auto& txout : tx.vout)
{
if (txout.nValue < 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 > 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");
nValueOut += txout.nValue;
if (!MoneyRange(nValueOut))
- 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
@@ -186,20 +186,20 @@ 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");
}
else
{
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;
@@ -209,7 +209,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__));
}
@@ -221,28 +221,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(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));
}
// Check for negative or overflow input values
nValueIn += coin.out.nValue;
if (!MoneyRange(coin.out.nValue) || !MoneyRange(nValueIn)) {
- return state.DoS(100, false, REJECT_INVALID, "bad-txns-inputvalues-outofrange");
+ return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-txns-inputvalues-outofrange");
}
}
const CAmount value_out = tx.GetValueOut();
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");
}
txfee = txfee_aux;
diff --git a/src/consensus/validation.h b/src/consensus/validation.h
index f2e2c3585a..62890e2b26 100644
--- a/src/consensus/validation.h
+++ b/src/consensus/validation.h
@@ -22,6 +22,78 @@ static const unsigned char REJECT_NONSTANDARD = 0x40;
static const unsigned char REJECT_INSUFFICIENTFEE = 0x42;
static const unsigned char REJECT_CHECKPOINT = 0x43;
+/** 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_BAD_TIME, //!< 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_BAD_TIME ||
+ r == ValidationInvalidReason::BLOCK_CHECKPOINT;
+}
+
/** Capture information about block/transaction validation */
class CValidationState {
private:
@@ -30,32 +102,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;
@@ -71,19 +135,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 5927a14a6e..72cd346bdb 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -349,7 +349,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;
@@ -745,7 +754,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());
@@ -947,6 +956,80 @@ 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;
+}
+
+//! 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_BAD_TIME:
+ 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;
+}
+
@@ -1122,14 +1205,12 @@ void PeerLogicValidation::BlockChecked(const CBlock& block, const CValidationSta
const uint256 hash(block.GetHash());
std::map<uint256, std::pair<NodeId, bool>>::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:
@@ -1479,7 +1560,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<CBlockHeader>& headers, const CChainParams& chainparams, bool punish_duplicate_invalid)
+bool static ProcessHeadersMessage(CNode *pfrom, CConnman *connman, const std::vector<CBlockHeader>& headers, const CChainParams& chainparams, bool via_compact_block)
{
const CNetMsgMaker msgMaker(pfrom->GetSendVersion());
size_t nCount = headers.size();
@@ -1541,48 +1622,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;
}
}
@@ -2380,15 +2421,14 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
const uint256& orphanHash = orphanTx.GetHash();
NodeId fromPeer = (*mi)->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, &lRemovedTxn, false /* bypass_limits */, 0 /* nAbsurdFee */)) {
+ if (AcceptToMemoryPool(mempool, orphan_state, porphanTx, &fMissingInputs2, &lRemovedTxn, 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++) {
@@ -2398,19 +2438,20 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
}
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());
vEraseQueue.push_back(orphanHash);
- 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.
@@ -2458,7 +2499,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.
@@ -2477,15 +2519,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));
}
}
}
@@ -2510,8 +2550,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(),
@@ -2520,9 +2559,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;
}
@@ -2552,14 +2589,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;
}
}
@@ -2709,7 +2740,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) {
@@ -2840,12 +2871,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 && !fImporting && !fReindex) // Ignore blocks received while importing
diff --git a/src/test/txvalidation_tests.cpp b/src/test/txvalidation_tests.cpp
index c2777cd6d1..aa30129361 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 dbdc1afb35..ee58e7edc8 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -582,28 +582,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
@@ -639,7 +639,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());
@@ -665,7 +665,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
@@ -688,7 +688,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");
CAmount nFees = 0;
if (!Consensus::CheckTxInputs(tx, state, view, GetSpendHeight(view), nFees)) {
@@ -697,11 +697,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);
@@ -724,27 +724,22 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool
fSpendsCoinbase, nSigOpsCost, lp);
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));
@@ -756,7 +751,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
@@ -768,8 +763,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()));
@@ -811,8 +805,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(),
@@ -840,8 +833,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,
@@ -860,8 +852,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));
}
@@ -873,8 +864,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)));
}
@@ -884,8 +874,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),
@@ -906,8 +895,10 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool
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.SetCorruptionPossible();
+ state.Invalid(ValidationInvalidReason::TX_WITNESS_MUTATED, false,
+ state.GetRejectCode(), state.GetRejectReason(), state.GetDebugMessage());
}
+ assert(IsTransactionReason(state.GetReason()));
return false; // state filled in by CheckInputs
}
@@ -964,7 +955,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");
}
}
@@ -1297,7 +1288,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);
@@ -1365,6 +1356,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<CScriptCheck> *pvChecks) EXCLUSIVE_LOCKS_REQUIRED(cs_main)
@@ -1420,22 +1414,26 @@ 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(check.GetScriptError())));
+ return state.Invalid(ValidationInvalidReason::TX_NOT_STANDARD, false, REJECT_NONSTANDARD, strprintf("non-mandatory-script-verify-flag (%s)", ScriptErrorString(check.GetScriptError())));
}
- // 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(check.GetScriptError())));
+ // 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(check.GetScriptError())));
}
}
@@ -1812,7 +1810,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.
@@ -1947,7 +1945,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");
}
}
@@ -1987,11 +1985,19 @@ bool CChainState::ConnectBlock(const CBlock& block, CValidationState& state, CBl
{
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.GetRejectCode(), state.GetRejectReason(), state.GetDebugMessage());
+ }
return error("%s: Consensus::CheckTxInputs: %s, %s", __func__, tx.GetHash().ToString(), FormatStateMessage(state));
}
nFees += txfee;
if (!MoneyRange(nFees)) {
- return state.DoS(100, error("%s: accumulated fee in the block out of range.", __func__),
+ return state.Invalid(ValidationInvalidReason::CONSENSUS, error("%s: accumulated fee in the block out of range.", __func__),
REJECT_INVALID, "bad-txns-accumulated-fee-outofrange");
}
@@ -2004,7 +2010,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");
}
}
@@ -2015,7 +2021,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);
@@ -2023,9 +2029,21 @@ bool CChainState::ConnectBlock(const CBlock& block, CValidationState& state, CBl
{
std::vector<CScriptCheck> 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.
+ // (note that this may not be the case until we add additional
+ // soft-fork flags to our script flags, in which case we need to
+ // be careful to differentiate RECENT_CONSENSUS_CHANGE and
+ // CONSENSUS)
+ 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);
}
@@ -2040,13 +2058,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.DoS(100,
+ return state.Invalid(ValidationInvalidReason::CONSENSUS,
error("ConnectBlock(): coinbase pays too much (actual=%d vs limit=%d)",
block.vtx[0]->GetValueOut(), blockReward),
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);
@@ -2578,7 +2596,7 @@ bool CChainState::ActivateBestChainStep(CValidationState& state, const CChainPar
if (!ConnectTip(state, chainparams, pindexConnect, pindexConnect == pindexMostWork ? pblock : std::shared_ptr<const CBlock>(), 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();
@@ -3066,7 +3084,7 @@ static bool CheckBlockHeader(const CBlockHeader& block, CValidationState& state,
{
// Check proof of work matches claimed amount
if (fCheckPOW && !CheckProofOfWork(block.GetHash(), block.nBits, consensusParams))
- return state.DoS(50, false, REJECT_INVALID, "high-hash", false, "proof of work failed");
+ return state.Invalid(ValidationInvalidReason::BLOCK_INVALID_HEADER, false, REJECT_INVALID, "high-hash", "proof of work failed");
return true;
}
@@ -3088,13 +3106,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
@@ -3105,20 +3123,22 @@ 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(),
- strprintf("Transaction check failed (tx hash %s) %s", tx->GetHash().ToString(), state.GetDebugMessage()));
+ for (const auto& tx : block.vtx) {
+ if (!CheckTransaction(*tx, state, true)) {
+ LogPrintf("Transaction check failed (tx hash %s) %s\n", tx->GetHash().ToString(), state.GetDebugMessage());
+ return false;
+ }
+ }
unsigned int nSigOps = 0;
for (const auto& tx : block.vtx)
@@ -3126,7 +3146,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;
@@ -3219,7 +3239,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.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) {
@@ -3228,23 +3248,23 @@ static bool ContextualCheckBlockHeader(const CBlockHeader& block, CValidationSta
// MapBlockIndex.
CBlockIndex* pcheckpoint = Checkpoints::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 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_BAD_TIME, 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::CONSENSUS, false, REJECT_OBSOLETE, strprintf("bad-version(0x%08x)", block.nVersion),
strprintf("rejected nVersion=0x%08x block", block.nVersion));
return true;
@@ -3274,7 +3294,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");
}
}
@@ -3284,7 +3304,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");
}
}
@@ -3306,11 +3326,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.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]->vin[0].scriptWitness.stack[0][0], 32).Finalize(hashWitness.begin());
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", __func__));
+ return state.Invalid(ValidationInvalidReason::BLOCK_MUTATED, false, REJECT_INVALID, "bad-witness-merkle-match", strprintf("%s : witness merkle commitment mismatch", __func__));
}
fHaveWitness = true;
}
@@ -3320,7 +3340,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__));
}
}
}
@@ -3332,7 +3352,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;
@@ -3352,7 +3372,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;
}
@@ -3363,10 +3383,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));
@@ -3403,7 +3423,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");
}
}
}
@@ -3507,7 +3527,8 @@ bool CChainState::AcceptBlock(const std::shared_ptr<const CBlock>& pblock, CVali
if (!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 02deae92f3..f0991a284b 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
def get_tx(self):
tx = CTransaction()
diff --git a/test/functional/feature_block.py b/test/functional/feature_block.py
index 5253ff7aaa..9df597e8e6 100755
--- a/test/functional/feature_block.py
+++ b/test/functional/feature_block.py
@@ -295,7 +295,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.sync_blocks([b20], success=False, reject_reason='bad-txns-premature-spend-of-coinbase')
+ self.sync_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)
@@ -308,7 +308,7 @@ class FullBlockTest(BitcoinTestFramework):
self.sync_blocks([b21], False)
b22 = self.next_block(22, spend=out[5])
- self.sync_blocks([b22], success=False, reject_reason='bad-txns-premature-spend-of-coinbase')
+ self.sync_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)
@@ -630,7 +630,7 @@ class FullBlockTest(BitcoinTestFramework):
while b47.sha256 < target:
b47.nNonce += 1
b47.rehash()
- self.sync_blocks([b47], False, force_send=True, reject_reason='high-hash')
+ self.sync_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)
@@ -681,7 +681,7 @@ class FullBlockTest(BitcoinTestFramework):
b54 = self.next_block(54, spend=out[15])
b54.nTime = b35.nTime - 1
b54.solve()
- self.sync_blocks([b54], False, force_send=True, reject_reason='time-too-old')
+ self.sync_blocks([b54], False, force_send=True, reject_reason='time-too-old', reconnect=True)
# valid timestamp
self.move_tip(53)
@@ -827,7 +827,7 @@ class FullBlockTest(BitcoinTestFramework):
assert(tx.vin[0].nSequence < 0xffffffff)
tx.calc_sha256()
b62 = self.update_block(62, [tx])
- self.sync_blocks([b62], success=False, reject_reason='bad-txns-nonfinal')
+ self.sync_blocks([b62], success=False, reject_reason='bad-txns-nonfinal', reconnect=True)
# Test a non-final coinbase is also rejected
#
@@ -841,7 +841,7 @@ class FullBlockTest(BitcoinTestFramework):
b63.vtx[0].vin[0].nSequence = 0xDEADBEEF
b63.vtx[0].rehash()
b63 = self.update_block(63, [])
- self.sync_blocks([b63], success=False, reject_reason='bad-txns-nonfinal')
+ self.sync_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,
@@ -1255,7 +1255,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.sync_blocks([b_v1], success=False, force_send=True, reject_reason='bad-version(0x00000001)')
+ self.sync_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)
diff --git a/src/blockencodings.cpp b/src/blockencodings.cpp
index 10f51931f0..f0fcf675eb 100644
--- a/src/blockencodings.cpp
+++ b/src/blockencodings.cpp
@@ -203,7 +203,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_verify.cpp b/src/consensus/tx_verify.cpp
index 0a7eacfb91..6ecaa8c821 100644
--- a/src/consensus/tx_verify.cpp
+++ b/src/consensus/tx_verify.cpp
@@ -160,24 +160,24 @@ 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 nValueOut = 0;
for (const auto& txout : tx.vout)
{
if (txout.nValue < 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 > 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");
nValueOut += txout.nValue;
if (!MoneyRange(nValueOut))
- 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
@@ -186,20 +186,20 @@ 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");
}
else
{
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;
@@ -209,7 +209,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__));
}
@@ -221,28 +221,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(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));
}
// Check for negative or overflow input values
nValueIn += coin.out.nValue;
if (!MoneyRange(coin.out.nValue) || !MoneyRange(nValueIn)) {
- return state.DoS(100, false, REJECT_INVALID, "bad-txns-inputvalues-outofrange");
+ return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-txns-inputvalues-outofrange");
}
}
const CAmount value_out = tx.GetValueOut();
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");
}
txfee = txfee_aux;
diff --git a/src/consensus/validation.h b/src/consensus/validation.h
index f2e2c3585a..62890e2b26 100644
--- a/src/consensus/validation.h
+++ b/src/consensus/validation.h
@@ -22,6 +22,78 @@ static const unsigned char REJECT_NONSTANDARD = 0x40;
static const unsigned char REJECT_INSUFFICIENTFEE = 0x42;
static const unsigned char REJECT_CHECKPOINT = 0x43;
+/** 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_BAD_TIME, //!< 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_BAD_TIME ||
+ r == ValidationInvalidReason::BLOCK_CHECKPOINT;
+}
+
/** Capture information about block/transaction validation */
class CValidationState {
private:
@@ -30,32 +102,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;
@@ -71,19 +135,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 0654a96e26..d8cb564378 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -352,7 +352,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;
@@ -748,7 +757,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());
@@ -960,6 +969,80 @@ 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;
+}
+
+//! 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_BAD_TIME:
+ 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;
+}
+
@@ -1135,14 +1218,12 @@ void PeerLogicValidation::BlockChecked(const CBlock& block, const CValidationSta
const uint256 hash(block.GetHash());
std::map<uint256, std::pair<NodeId, bool>>::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:
@@ -1492,7 +1573,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<CBlockHeader>& headers, const CChainParams& chainparams, bool punish_duplicate_invalid)
+bool static ProcessHeadersMessage(CNode *pfrom, CConnman *connman, const std::vector<CBlockHeader>& headers, const CChainParams& chainparams, bool via_compact_block)
{
const CNetMsgMaker msgMaker(pfrom->GetSendVersion());
size_t nCount = headers.size();
@@ -1554,48 +1635,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;
}
}
@@ -1730,13 +1771,13 @@ void static ProcessOrphanTx(CConnman* connman, std::set<uint256>& 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++) {
@@ -1750,17 +1791,18 @@ void static ProcessOrphanTx(CConnman* connman, std::set<uint256>& 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.
@@ -2478,7 +2520,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.
@@ -2497,15 +2540,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));
}
}
}
@@ -2530,8 +2571,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(),
@@ -2540,9 +2580,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;
}
@@ -2578,14 +2616,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;
}
}
@@ -2735,7 +2767,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) {
@@ -2878,12 +2910,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 c2777cd6d1..aa30129361 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 ae3985485e..f6c8d42ccb 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -587,28 +587,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
@@ -644,7 +644,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());
@@ -670,7 +670,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
@@ -693,7 +693,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");
CAmount nFees = 0;
if (!Consensus::CheckTxInputs(tx, state, view, GetSpendHeight(view), nFees)) {
@@ -702,11 +702,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);
@@ -729,27 +729,22 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool
fSpendsCoinbase, nSigOpsCost, lp);
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));
@@ -761,7 +756,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
@@ -773,8 +768,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()));
@@ -816,8 +810,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(),
@@ -845,8 +838,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,
@@ -865,8 +857,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));
}
@@ -878,8 +869,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)));
}
@@ -889,8 +879,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),
@@ -911,8 +900,10 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool
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.SetCorruptionPossible();
+ state.Invalid(ValidationInvalidReason::TX_WITNESS_MUTATED, false,
+ state.GetRejectCode(), state.GetRejectReason(), state.GetDebugMessage());
}
+ assert(IsTransactionReason(state.GetReason()));
return false; // state filled in by CheckInputs
}
@@ -969,7 +960,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");
}
}
@@ -1302,7 +1293,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);
@@ -1370,6 +1361,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<CScriptCheck> *pvChecks) EXCLUSIVE_LOCKS_REQUIRED(cs_main)
@@ -1425,22 +1419,26 @@ 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(check.GetScriptError())));
+ return state.Invalid(ValidationInvalidReason::TX_NOT_STANDARD, false, REJECT_NONSTANDARD, strprintf("non-mandatory-script-verify-flag (%s)", ScriptErrorString(check.GetScriptError())));
}
- // 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(check.GetScriptError())));
+ // 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(check.GetScriptError())));
}
}
@@ -1800,7 +1798,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.
@@ -1935,7 +1933,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");
}
}
@@ -1975,11 +1973,19 @@ bool CChainState::ConnectBlock(const CBlock& block, CValidationState& state, CBl
{
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.GetRejectCode(), state.GetRejectReason(), state.GetDebugMessage());
+ }
return error("%s: Consensus::CheckTxInputs: %s, %s", __func__, tx.GetHash().ToString(), FormatStateMessage(state));
}
nFees += txfee;
if (!MoneyRange(nFees)) {
- return state.DoS(100, error("%s: accumulated fee in the block out of range.", __func__),
+ return state.Invalid(ValidationInvalidReason::CONSENSUS, error("%s: accumulated fee in the block out of range.", __func__),
REJECT_INVALID, "bad-txns-accumulated-fee-outofrange");
}
@@ -1992,7 +1998,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");
}
}
@@ -2003,7 +2009,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);
@@ -2011,9 +2017,21 @@ bool CChainState::ConnectBlock(const CBlock& block, CValidationState& state, CBl
{
std::vector<CScriptCheck> 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.
+ // (note that this may not be the case until we add additional
+ // soft-fork flags to our script flags, in which case we need to
+ // be careful to differentiate RECENT_CONSENSUS_CHANGE and
+ // CONSENSUS)
+ 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);
}
@@ -2028,13 +2046,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.DoS(100,
+ return state.Invalid(ValidationInvalidReason::CONSENSUS,
error("ConnectBlock(): coinbase pays too much (actual=%d vs limit=%d)",
block.vtx[0]->GetValueOut(), blockReward),
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);
@@ -2562,7 +2580,7 @@ bool CChainState::ActivateBestChainStep(CValidationState& state, const CChainPar
if (!ConnectTip(state, chainparams, pindexConnect, pindexConnect == pindexMostWork ? pblock : std::shared_ptr<const CBlock>(), 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();
@@ -3060,7 +3078,7 @@ static bool CheckBlockHeader(const CBlockHeader& block, CValidationState& state,
{
// Check proof of work matches claimed amount
if (fCheckPOW && !CheckProofOfWork(block.GetHash(), block.nBits, consensusParams))
- return state.DoS(50, false, REJECT_INVALID, "high-hash", false, "proof of work failed");
+ return state.Invalid(ValidationInvalidReason::BLOCK_INVALID_HEADER, false, REJECT_INVALID, "high-hash", "proof of work failed");
return true;
}
@@ -3082,13 +3100,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
@@ -3099,20 +3117,22 @@ 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(),
- strprintf("Transaction check failed (tx hash %s) %s", tx->GetHash().ToString(), state.GetDebugMessage()));
+ for (const auto& tx : block.vtx) {
+ if (!CheckTransaction(*tx, state, true)) {
+ LogPrintf("Transaction check failed (tx hash %s) %s\n", tx->GetHash().ToString(), state.GetDebugMessage());
+ return false;
+ }
+ }
unsigned int nSigOps = 0;
for (const auto& tx : block.vtx)
@@ -3120,7 +3140,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;
@@ -3213,7 +3233,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.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) {
@@ -3222,23 +3242,23 @@ static bool ContextualCheckBlockHeader(const CBlockHeader& block, CValidationSta
// MapBlockIndex.
CBlockIndex* pcheckpoint = Checkpoints::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 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_BAD_TIME, 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::CONSENSUS, false, REJECT_OBSOLETE, strprintf("bad-version(0x%08x)", block.nVersion),
strprintf("rejected nVersion=0x%08x block", block.nVersion));
return true;
@@ -3268,7 +3288,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");
}
}
@@ -3278,7 +3298,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");
}
}
@@ -3300,11 +3320,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.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]->vin[0].scriptWitness.stack[0][0], 32).Finalize(hashWitness.begin());
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", __func__));
+ return state.Invalid(ValidationInvalidReason::BLOCK_MUTATED, false, REJECT_INVALID, "bad-witness-merkle-match", strprintf("%s : witness merkle commitment mismatch", __func__));
}
fHaveWitness = true;
}
@@ -3314,7 +3334,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__));
}
}
}
@@ -3326,7 +3346,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;
@@ -3346,7 +3366,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;
}
@@ -3357,10 +3377,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));
@@ -3397,7 +3417,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");
}
}
}
@@ -3501,7 +3521,8 @@ bool CChainState::AcceptBlock(const std::shared_ptr<const CBlock>& pblock, CVali
if (!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 02deae92f3..f0991a284b 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
def get_tx(self):
tx = CTransaction()
diff --git a/test/functional/feature_block.py b/test/functional/feature_block.py
index faf7f20257..9f8fdb5ac3 100755
--- a/test/functional/feature_block.py
+++ b/test/functional/feature_block.py
@@ -295,7 +295,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.sync_blocks([b20], success=False, reject_reason='bad-txns-premature-spend-of-coinbase')
+ self.sync_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)
@@ -308,7 +308,7 @@ class FullBlockTest(BitcoinTestFramework):
self.sync_blocks([b21], False)
b22 = self.next_block(22, spend=out[5])
- self.sync_blocks([b22], success=False, reject_reason='bad-txns-premature-spend-of-coinbase')
+ self.sync_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)
@@ -630,7 +630,7 @@ class FullBlockTest(BitcoinTestFramework):
while b47.sha256 < target:
b47.nNonce += 1
b47.rehash()
- self.sync_blocks([b47], False, force_send=True, reject_reason='high-hash')
+ self.sync_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)
@@ -681,7 +681,7 @@ class FullBlockTest(BitcoinTestFramework):
b54 = self.next_block(54, spend=out[15])
b54.nTime = b35.nTime - 1
b54.solve()
- self.sync_blocks([b54], False, force_send=True, reject_reason='time-too-old')
+ self.sync_blocks([b54], False, force_send=True, reject_reason='time-too-old', reconnect=True)
# valid timestamp
self.move_tip(53)
@@ -827,7 +827,7 @@ class FullBlockTest(BitcoinTestFramework):
assert tx.vin[0].nSequence < 0xffffffff
tx.calc_sha256()
b62 = self.update_block(62, [tx])
- self.sync_blocks([b62], success=False, reject_reason='bad-txns-nonfinal')
+ self.sync_blocks([b62], success=False, reject_reason='bad-txns-nonfinal', reconnect=True)
# Test a non-final coinbase is also rejected
#
@@ -841,7 +841,7 @@ class FullBlockTest(BitcoinTestFramework):
b63.vtx[0].vin[0].nSequence = 0xDEADBEEF
b63.vtx[0].rehash()
b63 = self.update_block(63, [])
- self.sync_blocks([b63], success=False, reject_reason='bad-txns-nonfinal')
+ self.sync_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,
@@ -1255,7 +1255,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.sync_blocks([b_v1], success=False, force_send=True, reject_reason='bad-version(0x00000001)')
+ self.sync_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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment