[refactor] Add useful-for-dos "reason" field to CValidationState

This is a first step towards cleaning up our DoS interface - make
validation return *why* something is invalid, and let net_processing
figure out what that implies in terms of banning/disconnection/etc.

Behavior change: peers will now be banned for providing blocks
with premature coinbase spends.

Co-authored-by: Anthony Towns <[email protected]>
                Suhas Daftuar <[email protected]>

Author: TheBlueMatt

src/consensus/tx_check.cpp

@@ -11,24 +11,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(100, false, REJECT_INVALID, "bad-txns-vin-empty");
+        return state.DoS(100, ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-txns-vin-empty");
     if (tx.vout.empty())
-        return state.DoS(100, false, REJECT_INVALID, "bad-txns-vout-empty");
+        return state.DoS(100, 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.DoS(100, 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.DoS(100, 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.DoS(100, 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.DoS(100, 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
@@ -37,20 +37,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.DoS(100, 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.DoS(100, ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-cb-length");
     }
     else
     {
         for (const auto& txin : tx.vin)
             if (txin.prevout.IsNull())
-                return state.DoS(100, false, REJECT_INVALID, "bad-txns-prevout-null");
+                return state.DoS(100, ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-txns-prevout-null");
     }
 
     return true;

src/consensus/tx_verify.cpp

@@ -160,7 +160,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.DoS(0, ValidationInvalidReason::TX_MISSING_INPUTS, false, REJECT_INVALID, "bad-txns-inputs-missingorspent", false,
                          strprintf("%s: inputs missing/spent", __func__));
     }
 
@@ -172,28 +172,28 @@ 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.DoS(0, false,
+            return state.DoS(0, ValidationInvalidReason::TX_MISSING_INPUTS, false,
                 REJECT_INVALID, "bad-txns-premature-spend-of-coinbase", false,
                 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.DoS(100, 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.DoS(100, ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-txns-in-belowout", false,
             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.DoS(100, ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-txns-fee-outofrange");
     }
 
     txfee = txfee_aux;

src/consensus/validation.h

@@ -22,6 +22,50 @@ 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_TIME_FUTURE,          //!< block timestamp was > 2 hours in the future (or our clock is bad)
+    BLOCK_CHECKPOINT,        //!< the block failed to meet one of our checkpoints
+    // Only loose txn:
+    TX_NOT_STANDARD,          //!< didn't meet our local policy rules
+    TX_MISSING_INPUTS,        //!< a transaction was missing some of its inputs (or its inputs were spent at < coinbase maturity height)
+    /**
+     * 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
+};
+
 /** Capture information about block/transaction validation */
 class CValidationState {
 private:
@@ -30,31 +74,35 @@ class CValidationState {
         MODE_INVALID, //!< network rule violation (DoS value may be set)
         MODE_ERROR,   //!< run-time error
     } mode;
+    ValidationInvalidReason m_reason;
     int nDoS;
     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,
+    CValidationState() : mode(MODE_VALID), m_reason(ValidationInvalidReason::NONE), nDoS(0), chRejectCode(0), corruptionPossible(false) {}
+    bool DoS(int level, ValidationInvalidReason reasonIn, bool ret = false,
              unsigned int chRejectCodeIn=0, const std::string &strRejectReasonIn="",
              bool corruptionIn=false,
              const std::string &strDebugMessageIn="") {
+        m_reason = reasonIn;
         chRejectCode = chRejectCodeIn;
         strRejectReason = strRejectReasonIn;
         corruptionPossible = corruptionIn;
         strDebugMessage = strDebugMessageIn;
+        nDoS += level;
+        assert(nDoS == GetDoSForReason());
+        assert(corruptionPossible == (m_reason == ValidationInvalidReason::BLOCK_MUTATED || m_reason == ValidationInvalidReason::TX_WITNESS_MUTATED));
         if (mode == MODE_ERROR)
             return ret;
-        nDoS += level;
         mode = MODE_INVALID;
         return ret;
     }
-    bool Invalid(bool ret = false,
+    bool Invalid(ValidationInvalidReason _reason, bool ret = false,
                  unsigned int _chRejectCode=0, const std::string &_strRejectReason="",
                  const std::string &_strDebugMessage="") {
-        return DoS(0, ret, _chRejectCode, _strRejectReason, false, _strDebugMessage);
+        return DoS(0, _reason, ret, _chRejectCode, _strRejectReason, false, _strDebugMessage);
     }
     bool Error(const std::string& strRejectReasonIn) {
         if (mode == MODE_VALID)
@@ -72,12 +120,39 @@ class CValidationState {
         return mode == MODE_ERROR;
     }
     bool CorruptionPossible() const {
+        assert(corruptionPossible == (m_reason == ValidationInvalidReason::BLOCK_MUTATED || m_reason == ValidationInvalidReason::TX_WITNESS_MUTATED));
         return corruptionPossible;
     }
     void SetCorruptionPossible() {
         corruptionPossible = true;
+        assert(corruptionPossible == (m_reason == ValidationInvalidReason::BLOCK_MUTATED || m_reason == ValidationInvalidReason::TX_WITNESS_MUTATED));
     }
     int GetDoS(void) const { return nDoS; }
+    int GetDoSForReason() const {
+        switch (m_reason) {
+        case ValidationInvalidReason::NONE:
+            return 0;
+        case ValidationInvalidReason::CONSENSUS:
+        case ValidationInvalidReason::BLOCK_MUTATED:
+        case ValidationInvalidReason::BLOCK_INVALID_HEADER:
+        case ValidationInvalidReason::BLOCK_CHECKPOINT:
+        case ValidationInvalidReason::BLOCK_INVALID_PREV:
+            return 100;
+        case ValidationInvalidReason::BLOCK_MISSING_PREV:
+            return 10;
+        case ValidationInvalidReason::CACHED_INVALID:
+        case ValidationInvalidReason::RECENT_CONSENSUS_CHANGE:
+        case ValidationInvalidReason::BLOCK_TIME_FUTURE:
+        case ValidationInvalidReason::TX_NOT_STANDARD:
+        case ValidationInvalidReason::TX_MISSING_INPUTS:
+        case ValidationInvalidReason::TX_WITNESS_MUTATED:
+        case ValidationInvalidReason::TX_CONFLICT:
+        case ValidationInvalidReason::TX_MEMPOOL_POLICY:
+            return 0;
+        }
+        return 0;
+    }
+    ValidationInvalidReason GetReason() const { return m_reason; }
     unsigned int GetRejectCode() const { return chRejectCode; }
     std::string GetRejectReason() const { return strRejectReason; }
     std::string GetDebugMessage() const { return strDebugMessage; }

src/net_processing.cpp

@@ -961,6 +961,7 @@ void Misbehaving(NodeId pnode, int howmuch, const std::string& message) EXCLUSIV
 
 static bool TxRelayMayResultInDisconnect(const CValidationState& state)
 {
+    assert(state.GetDoS() == state.GetDoSForReason());
     return (state.GetDoS() > 0);
 }
 
@@ -975,6 +976,7 @@ static bool TxRelayMayResultInDisconnect(const CValidationState& state)
  * txs, the peer should not be punished. See BIP 152.
  */
 static bool MaybePunishNode(NodeId nodeid, const CValidationState& state, bool via_compact_block, const std::string& message = "") {
+    assert(state.GetDoS() == state.GetDoSForReason());
     int nDoS = state.GetDoS();
     if (nDoS > 0 && !via_compact_block) {
          LOCK(cs_main);

src/test/txvalidation_tests.cpp

@@ -52,6 +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");
+    BOOST_CHECK(state.GetReason() == ValidationInvalidReason::CONSENSUS);
 }
 
 BOOST_AUTO_TEST_SUITE_END()