diff --git a/doc/release-notes-19219.md b/doc/release-notes-19219.md new file mode 100644 index 000000000000..b5ee885ddc3a --- /dev/null +++ b/doc/release-notes-19219.md @@ -0,0 +1,23 @@ +#### Changes regarding misbehaving peers + +Peers that misbehave (e.g. send us invalid blocks) are now referred to as +discouraged nodes in log output, as they're not (and weren't) strictly banned: +incoming connections are still allowed from them, but they're preferred for +eviction. + +Furthermore, a few additional changes are introduced to how discouraged +addresses are treated: + +- Discouraging an address does not time out automatically after 24 hours + (or the `-bantime` setting). Depending on traffic from other peers, + discouragement may time out at an indeterminate time. + +- Discouragement is not persisted over restarts. + +- There is no method to list discouraged addresses. They are not returned by + the `listbanned` RPC. That RPC also no longer reports the `ban_reason` + field, as `"manually added"` is the only remaining option. + +- Discouragement cannot be removed with the `setban remove` RPC command. + If you need to remove a discouragement, you can remove all discouragements by + stop-starting your node. diff --git a/src/addrdb.h b/src/addrdb.h index 5a44993f8f26..e1352645eef8 100644 --- a/src/addrdb.h +++ b/src/addrdb.h @@ -17,13 +17,6 @@ class CSubNet; class CAddrMan; class CDataStream; -typedef enum BanReason -{ - BanReasonUnknown = 0, - BanReasonNodeMisbehaving = 1, - BanReasonManuallyAdded = 2 -} BanReason; - class CBanEntry { public: @@ -31,7 +24,6 @@ class CBanEntry int nVersion; int64_t nCreateTime; int64_t nBanUntil; - uint8_t banReason; CBanEntry() { @@ -44,31 +36,17 @@ class CBanEntry nCreateTime = nCreateTimeIn; } - explicit CBanEntry(int64_t n_create_time_in, BanReason ban_reason_in) : CBanEntry(n_create_time_in) + SERIALIZE_METHODS(CBanEntry, obj) { - banReason = ban_reason_in; + uint8_t ban_reason = 2; //! For backward compatibility + READWRITE(obj.nVersion, obj.nCreateTime, obj.nBanUntil, ban_reason); } - SERIALIZE_METHODS(CBanEntry, obj) { READWRITE(obj.nVersion, obj.nCreateTime, obj.nBanUntil, obj.banReason); } - void SetNull() { nVersion = CBanEntry::CURRENT_VERSION; nCreateTime = 0; nBanUntil = 0; - banReason = BanReasonUnknown; - } - - std::string banReasonToString() const - { - switch (banReason) { - case BanReasonNodeMisbehaving: - return "node misbehaving"; - case BanReasonManuallyAdded: - return "manually added"; - default: - return "unknown"; - } } }; diff --git a/src/banman.cpp b/src/banman.cpp index a9ff1e39aa2f..48973e9c4a97 100644 --- a/src/banman.cpp +++ b/src/banman.cpp @@ -68,28 +68,13 @@ void BanMan::ClearBanned() if (m_client_interface) m_client_interface->BannedListChanged(); } -int BanMan::IsBannedLevel(CNetAddr net_addr) +bool BanMan::IsDiscouraged(const CNetAddr& net_addr) { - // Returns the most severe level of banning that applies to this address. - // 0 - Not banned - // 1 - Automatic misbehavior ban - // 2 - Any other ban - int level = 0; - auto current_time = GetTime(); LOCK(m_cs_banned); - for (const auto& it : m_banned) { - CSubNet sub_net = it.first; - CBanEntry ban_entry = it.second; - - if (current_time < ban_entry.nBanUntil && sub_net.Match(net_addr)) { - if (ban_entry.banReason != BanReasonNodeMisbehaving) return 2; - level = 1; - } - } - return level; + return m_discouraged.contains(net_addr.GetAddrBytes()); } -bool BanMan::IsBanned(CNetAddr net_addr) +bool BanMan::IsBanned(const CNetAddr& net_addr) { auto current_time = GetTime(); LOCK(m_cs_banned); @@ -104,7 +89,7 @@ bool BanMan::IsBanned(CNetAddr net_addr) return false; } -bool BanMan::IsBanned(CSubNet sub_net) +bool BanMan::IsBanned(const CSubNet& sub_net) { auto current_time = GetTime(); LOCK(m_cs_banned); @@ -118,15 +103,21 @@ bool BanMan::IsBanned(CSubNet sub_net) return false; } -void BanMan::Ban(const CNetAddr& net_addr, const BanReason& ban_reason, int64_t ban_time_offset, bool since_unix_epoch) +void BanMan::Ban(const CNetAddr& net_addr, int64_t ban_time_offset, bool since_unix_epoch) { CSubNet sub_net(net_addr); - Ban(sub_net, ban_reason, ban_time_offset, since_unix_epoch); + Ban(sub_net, ban_time_offset, since_unix_epoch); +} + +void BanMan::Discourage(const CNetAddr& net_addr) +{ + LOCK(m_cs_banned); + m_discouraged.insert(net_addr.GetAddrBytes()); } -void BanMan::Ban(const CSubNet& sub_net, const BanReason& ban_reason, int64_t ban_time_offset, bool since_unix_epoch) +void BanMan::Ban(const CSubNet& sub_net, int64_t ban_time_offset, bool since_unix_epoch) { - CBanEntry ban_entry(GetTime(), ban_reason); + CBanEntry ban_entry(GetTime()); int64_t normalized_ban_time_offset = ban_time_offset; bool normalized_since_unix_epoch = since_unix_epoch; @@ -146,8 +137,8 @@ void BanMan::Ban(const CSubNet& sub_net, const BanReason& ban_reason, int64_t ba } if (m_client_interface) m_client_interface->BannedListChanged(); - //store banlist to disk immediately if user requested ban - if (ban_reason == BanReasonManuallyAdded) DumpBanlist(); + //store banlist to disk immediately + DumpBanlist(); } bool BanMan::Unban(const CNetAddr& net_addr) diff --git a/src/banman.h b/src/banman.h index 7943f666e864..f8eb7a5a0b79 100644 --- a/src/banman.h +++ b/src/banman.h @@ -9,6 +9,7 @@ #include #include +#include #include #include // For banmap_t #include @@ -20,32 +21,55 @@ class CClientUIInterface; class CNetAddr; class CSubNet; -// Denial-of-service detection/prevention -// The idea is to detect peers that are behaving -// badly and disconnect/ban them, but do it in a -// one-coding-mistake-won't-shatter-the-entire-network -// way. -// IMPORTANT: There should be nothing I can give a -// node that it will forward on that will make that -// node's peers drop it. If there is, an attacker -// can isolate a node and/or try to split the network. -// Dropping a node for sending stuff that is invalid -// now but might be valid in a later version is also -// dangerous, because it can cause a network split -// between nodes running old code and nodes running -// new code. +// Banman manages two related but distinct concepts: +// +// 1. Banning. This is configured manually by the user, through the setban RPC. +// If an address or subnet is banned, we never accept incoming connections from +// it and never create outgoing connections to it. We won't gossip its address +// to other peers in addr messages. Banned addresses and subnets are stored to +// banlist.dat on shutdown and reloaded on startup. Banning can be used to +// prevent connections with spy nodes or other griefers. +// +// 2. Discouragement. If a peer misbehaves enough (see Misbehaving() in +// net_processing.cpp), we'll mark that address as discouraged. We still allow +// incoming connections from them, but they're preferred for eviction when +// we receive new incoming connections. We never make outgoing connections to +// them, and do not gossip their address to other peers. This is implemented as +// a bloom filter. We can (probabilistically) test for membership, but can't +// list all discouraged addresses or unmark them as discouraged. Discouragement +// can prevent our limited connection slots being used up by incompatible +// or broken peers. +// +// Neither banning nor discouragement are protections against denial-of-service +// attacks, since if an attacker has a way to waste our resources and we +// disconnect from them and ban that address, it's trivial for them to +// reconnect from another IP address. +// +// Attempting to automatically disconnect or ban any class of peer carries the +// risk of splitting the network. For example, if we banned/disconnected for a +// transaction that fails a policy check and a future version changes the +// policy check so the transaction is accepted, then that transaction could +// cause the network to split between old nodes and new nodes. class BanMan { public: ~BanMan(); BanMan(fs::path ban_file, CClientUIInterface* client_interface, int64_t default_ban_time); - void Ban(const CNetAddr& net_addr, const BanReason& ban_reason, int64_t ban_time_offset = 0, bool since_unix_epoch = false); - void Ban(const CSubNet& sub_net, const BanReason& ban_reason, int64_t ban_time_offset = 0, bool since_unix_epoch = false); + void Ban(const CNetAddr& net_addr, int64_t ban_time_offset = 0, bool since_unix_epoch = false); + void Ban(const CSubNet& sub_net, int64_t ban_time_offset = 0, bool since_unix_epoch = false); + void Discourage(const CNetAddr& net_addr); void ClearBanned(); - int IsBannedLevel(CNetAddr net_addr); - bool IsBanned(CNetAddr net_addr); - bool IsBanned(CSubNet sub_net); + + //! Return whether net_addr is banned + bool IsBanned(const CNetAddr& net_addr); + + //! Return whether sub_net is exactly banned + bool IsBanned(const CSubNet& sub_net); + + //! Return whether net_addr is discouraged. + bool IsDiscouraged(const CNetAddr& net_addr); + bool Unban(const CNetAddr& net_addr); bool Unban(const CSubNet& sub_net); void GetBanned(banmap_t& banmap); @@ -65,6 +89,7 @@ class BanMan CClientUIInterface* m_client_interface = nullptr; CBanDB m_ban_db; const int64_t m_default_ban_time; + CRollingBloomFilter m_discouraged GUARDED_BY(m_cs_banned) {50000, 0.000001}; }; #endif diff --git a/src/blockencodings.cpp b/src/blockencodings.cpp index 9e62a989b8ec..5653685f9e3d 100644 --- a/src/blockencodings.cpp +++ b/src/blockencodings.cpp @@ -204,7 +204,7 @@ ReadStatus PartiallyDownloadedBlock::FillBlock(CBlock& block, const std::vector< // but that is expensive, and CheckBlock caches a block's // "checked-status" (in the CBlock?). CBlock should be able to // check its own merkle root and cache that check. - if (state.CorruptionPossible()) + if (state.GetReason() == ValidationInvalidReason::BLOCK_MUTATED) return READ_STATUS_FAILED; // Possible Short ID collision return READ_STATUS_CHECKBLOCK_FAILED; } diff --git a/src/consensus/tx_check.cpp b/src/consensus/tx_check.cpp index cf57522a70ba..f7799a48e2bf 100644 --- a/src/consensus/tx_check.cpp +++ b/src/consensus/tx_check.cpp @@ -19,25 +19,25 @@ bool CheckTransaction(const CTransaction& tx, CValidationState& state) // Basic checks that don't depend on any context if (!allowEmptyTxInOut && 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 (!allowEmptyTxInOut && 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 if (::GetSerializeSize(tx, PROTOCOL_VERSION) > MAX_LEGACY_BLOCK_SIZE) - return state.DoS(100, false, REJECT_INVALID, "bad-txns-oversize"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-txns-oversize"); if (tx.vExtraPayload.size() > MAX_TX_EXTRA_PAYLOAD) - return state.DoS(100, false, REJECT_INVALID, "bad-txns-payload-oversize"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-txns-payload-oversize"); // Check for negative or overflow output values (see CVE-2010-5139) CAmount nValueOut = 0; for (const auto& txout : tx.vout) { if (txout.nValue < 0) - return state.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 (see CVE-2018-17144) @@ -48,7 +48,7 @@ bool CheckTransaction(const CTransaction& tx, CValidationState& state) std::set vInOutPoints; 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()) { @@ -58,11 +58,11 @@ bool CheckTransaction(const CTransaction& tx, CValidationState& state) minCbSize = 1; } if (tx.vin[0].scriptSig.size() < minCbSize || 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; diff --git a/src/consensus/tx_verify.cpp b/src/consensus/tx_verify.cpp index 23e29f5aeeb2..d283d3209982 100644 --- a/src/consensus/tx_verify.cpp +++ b/src/consensus/tx_verify.cpp @@ -162,8 +162,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, - strprintf("%s: inputs missing/spent", __func__)); + return state.Invalid(ValidationInvalidReason::TX_MISSING_INPUTS, false, REJECT_INVALID, "bad-txns-inputs-missingorspent", strprintf("%s: inputs missing/spent", __func__)); } CAmount nValueIn = 0; @@ -174,28 +173,25 @@ 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", - strprintf("tried to spend coinbase at depth %d", nSpendHeight - coin.nHeight)); + 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, - strprintf("value in (%s) < value out (%s)", FormatMoney(nValueIn), FormatMoney(value_out))); + 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 2b222f202659..5120c90e4992 100644 --- a/src/consensus/validation.h +++ b/src/consensus/validation.h @@ -18,6 +18,72 @@ 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 some major deployment. + */ + 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 + BLOCK_CHAINLOCK, //!< the block conflicts with the ChainLock + // 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 + TX_BAD_SPECIAL, //!< special transaction violates some rules that are not enough for insta-ban + /** + * Tx already in mempool or conflicts with a tx in the chain + * 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_CONFLICT_LOCK, //!< conflicts with InstantSend lock + TX_MEMPOOL_POLICY, //!< violated mempool's fee/size/descendant/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_BAD_SPECIAL || + r == ValidationInvalidReason::TX_CONFLICT || + r == ValidationInvalidReason::TX_CONFLICT_LOCK || + r == ValidationInvalidReason::TX_MEMPOOL_POLICY; +} + +inline bool IsBlockReason(ValidationInvalidReason r) +{ + return r == ValidationInvalidReason::NONE || + r == ValidationInvalidReason::CONSENSUS || + r == ValidationInvalidReason::RECENT_CONSENSUS_CHANGE || + r == ValidationInvalidReason::CACHED_INVALID || + r == ValidationInvalidReason::BLOCK_INVALID_HEADER || + r == ValidationInvalidReason::BLOCK_MUTATED || + r == ValidationInvalidReason::BLOCK_MISSING_PREV || + r == ValidationInvalidReason::BLOCK_INVALID_PREV || + r == ValidationInvalidReason::BLOCK_TIME_FUTURE || + r == ValidationInvalidReason::BLOCK_CHECKPOINT || + r == ValidationInvalidReason::BLOCK_CHAINLOCK; +} + /** Capture information about block/transaction validation */ class CValidationState { private: @@ -26,32 +92,24 @@ class CValidationState { 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; @@ -67,16 +125,7 @@ class CValidationState { 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; - } + 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/evo/cbtx.cpp b/src/evo/cbtx.cpp index a40fef95ecdc..8f7dfc9ca1e5 100644 --- a/src/evo/cbtx.cpp +++ b/src/evo/cbtx.cpp @@ -17,30 +17,30 @@ bool CheckCbTx(const CTransaction& tx, const CBlockIndex* pindexPrev, CValidationState& state) { if (tx.nType != TRANSACTION_COINBASE) { - return state.DoS(100, false, REJECT_INVALID, "bad-cbtx-type"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-cbtx-type"); } if (!tx.IsCoinBase()) { - return state.DoS(100, false, REJECT_INVALID, "bad-cbtx-invalid"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-cbtx-invalid"); } CCbTx cbTx; if (!GetTxPayload(tx, cbTx)) { - return state.DoS(100, false, REJECT_INVALID, "bad-cbtx-payload"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-cbtx-payload"); } if (cbTx.nVersion == 0 || cbTx.nVersion > CCbTx::CURRENT_VERSION) { - return state.DoS(100, false, REJECT_INVALID, "bad-cbtx-version"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-cbtx-version"); } if (pindexPrev && pindexPrev->nHeight + 1 != cbTx.nHeight) { - return state.DoS(100, false, REJECT_INVALID, "bad-cbtx-height"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-cbtx-height"); } if (pindexPrev) { bool fDIP0008Active = pindexPrev->nHeight >= Params().GetConsensus().DIP0008Height; if (fDIP0008Active && cbTx.nVersion < 2) { - return state.DoS(100, false, REJECT_INVALID, "bad-cbtx-version"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-cbtx-version"); } } @@ -60,7 +60,7 @@ bool CheckCbTxMerkleRoots(const CBlock& block, const CBlockIndex* pindex, CValid CCbTx cbTx; if (!GetTxPayload(*block.vtx[0], cbTx)) { - return state.DoS(100, false, REJECT_INVALID, "bad-cbtx-payload"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-cbtx-payload"); } int64_t nTime2 = GetTimeMicros(); nTimePayload += nTime2 - nTime1; @@ -76,7 +76,7 @@ bool CheckCbTxMerkleRoots(const CBlock& block, const CBlockIndex* pindex, CValid return false; } if (calculatedMerkleRoot != cbTx.merkleRootMNList) { - return state.DoS(100, false, REJECT_INVALID, "bad-cbtx-mnmerkleroot"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-cbtx-mnmerkleroot"); } int64_t nTime3 = GetTimeMicros(); nTimeMerkleMNL += nTime3 - nTime2; @@ -88,7 +88,7 @@ bool CheckCbTxMerkleRoots(const CBlock& block, const CBlockIndex* pindex, CValid return false; } if (calculatedMerkleRoot != cbTx.merkleRootQuorums) { - return state.DoS(100, false, REJECT_INVALID, "bad-cbtx-quorummerkleroot"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-cbtx-quorummerkleroot"); } } @@ -132,7 +132,7 @@ bool CalcCbTxMerkleRootMNList(const CBlock& block, const CBlockIndex* pindexPrev if (sml.mnList == smlCached.mnList) { merkleRootRet = merkleRootCached; if (mutatedCached) { - return state.DoS(100, false, REJECT_INVALID, "mutated-cached-calc-cb-mnmerkleroot"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "mutated-cached-calc-cb-mnmerkleroot"); } return true; } @@ -148,13 +148,13 @@ bool CalcCbTxMerkleRootMNList(const CBlock& block, const CBlockIndex* pindexPrev mutatedCached = mutated; if (mutated) { - return state.DoS(100, false, REJECT_INVALID, "mutated-calc-cb-mnmerkleroot"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "mutated-calc-cb-mnmerkleroot"); } return true; } catch (const std::exception& e) { LogPrintf("%s -- failed: %s\n", __func__, e.what()); - return state.DoS(100, false, REJECT_INVALID, "failed-calc-cb-mnmerkleroot"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "failed-calc-cb-mnmerkleroot"); } } @@ -182,7 +182,7 @@ bool CalcCbTxMerkleRootQuorums(const CBlock& block, const CBlockIndex* pindexPre for (const auto& p2 : p.second) { uint256 minedBlockHash; llmq::CFinalCommitmentPtr qc = llmq::quorumBlockProcessor->GetMinedCommitment(p.first, p2->GetBlockHash(), minedBlockHash); - if (qc == nullptr) return state.DoS(100, false, REJECT_INVALID, "commitment-not-found"); + if (qc == nullptr) return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "commitment-not-found"); if (llmq::CLLMQUtils::IsQuorumRotationEnabled(qc->llmqType, pindexPrev)) { auto& qi = qcIndexedHashes[p.first]; qi.insert(std::make_pair(qc->quorumIndex, ::SerializeHash(*qc))); @@ -205,7 +205,7 @@ bool CalcCbTxMerkleRootQuorums(const CBlock& block, const CBlockIndex* pindexPre if (tx->nVersion == 3 && tx->nType == TRANSACTION_QUORUM_COMMITMENT) { llmq::CFinalCommitmentTxPayload qc; if (!GetTxPayload(*tx, qc)) { - return state.DoS(100, false, REJECT_INVALID, "bad-qc-payload-calc-cbtx-quorummerkleroot"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-qc-payload-calc-cbtx-quorummerkleroot"); } if (qc.commitment.IsNull()) { continue; @@ -227,7 +227,7 @@ bool CalcCbTxMerkleRootQuorums(const CBlock& block, const CBlockIndex* pindexPre v.emplace_back(qcHash); hashCount++; if (v.size() > uint64_t(llmq_params.signingActiveQuorumCount)) { - return state.DoS(100, false, REJECT_INVALID, "excess-quorums-calc-cbtx-quorummerkleroot"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "excess-quorums-calc-cbtx-quorummerkleroot"); } } } @@ -261,7 +261,7 @@ bool CalcCbTxMerkleRootQuorums(const CBlock& block, const CBlockIndex* pindexPre LogPrint(BCLog::BENCHMARK, " - ComputeMerkleRoot: %.2fms [%.2fs]\n", 0.001 * (nTime5 - nTime4), nTimeMerkle * 0.000001); if (mutated) { - return state.DoS(100, false, REJECT_INVALID, "mutated-calc-cbtx-quorummerkleroot"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "mutated-calc-cbtx-quorummerkleroot"); } return true; diff --git a/src/evo/deterministicmns.cpp b/src/evo/deterministicmns.cpp index 8ce9c5b16f1f..06c548dde80b 100644 --- a/src/evo/deterministicmns.cpp +++ b/src/evo/deterministicmns.cpp @@ -575,7 +575,7 @@ bool CDeterministicMNManager::ProcessBlock(const CBlock& block, const CBlockInde mnListDiffsCache.emplace(pindex->GetBlockHash(), diff); } catch (const std::exception& e) { LogPrintf("CDeterministicMNManager::%s -- internal error: %s\n", __func__, e.what()); - return _state.DoS(100, false, REJECT_INVALID, "failed-dmn-block"); + return _state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "failed-dmn-block"); } // Don't hold cs while calling signals @@ -588,7 +588,7 @@ bool CDeterministicMNManager::ProcessBlock(const CBlock& block, const CBlockInde if (!consensusParams.DIP0003EnforcementHash.IsNull() && consensusParams.DIP0003EnforcementHash != pindex->GetBlockHash()) { LogPrintf("CDeterministicMNManager::%s -- DIP3 enforcement block has wrong hash: hash=%s, expected=%s, nHeight=%d\n", __func__, pindex->GetBlockHash().ToString(), consensusParams.DIP0003EnforcementHash.ToString(), nHeight); - return _state.DoS(100, false, REJECT_INVALID, "bad-dip3-enf-block"); + return _state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-dip3-enf-block"); } LogPrintf("CDeterministicMNManager::%s -- DIP3 is enforced now. nHeight=%d\n", __func__, nHeight); } @@ -685,7 +685,7 @@ bool CDeterministicMNManager::BuildNewListFromBlock(const CBlock& block, const C if (tx.nType == TRANSACTION_PROVIDER_REGISTER) { CProRegTx proTx; if (!GetTxPayload(tx, proTx)) { - return _state.DoS(100, false, REJECT_INVALID, "bad-protx-payload"); + return _state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-protx-payload"); } auto dmn = std::make_shared(newList.GetTotalRegisteredCount()); @@ -702,7 +702,7 @@ bool CDeterministicMNManager::BuildNewListFromBlock(const CBlock& block, const C if (!proTx.collateralOutpoint.hash.IsNull() && (!view.GetCoin(dmn->collateralOutpoint, coin) || coin.IsSpent() || coin.out.nValue != 1000 * COIN)) { // should actually never get to this point as CheckProRegTx should have handled this case. // We do this additional check nevertheless to be 100% sure - return _state.DoS(100, false, REJECT_INVALID, "bad-protx-collateral"); + return _state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-protx-collateral"); } auto replacedDmn = newList.GetMNByCollateral(dmn->collateralOutpoint); @@ -718,10 +718,10 @@ bool CDeterministicMNManager::BuildNewListFromBlock(const CBlock& block, const C } if (newList.HasUniqueProperty(proTx.addr)) { - return _state.DoS(100, false, REJECT_DUPLICATE, "bad-protx-dup-addr"); + return _state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_DUPLICATE, "bad-protx-dup-addr"); } if (newList.HasUniqueProperty(proTx.keyIDOwner) || newList.HasUniqueProperty(proTx.pubKeyOperator)) { - return _state.DoS(100, false, REJECT_DUPLICATE, "bad-protx-dup-key"); + return _state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_DUPLICATE, "bad-protx-dup-key"); } dmn->nOperatorReward = proTx.nOperatorReward; @@ -743,16 +743,16 @@ bool CDeterministicMNManager::BuildNewListFromBlock(const CBlock& block, const C } else if (tx.nType == TRANSACTION_PROVIDER_UPDATE_SERVICE) { CProUpServTx proTx; if (!GetTxPayload(tx, proTx)) { - return _state.DoS(100, false, REJECT_INVALID, "bad-protx-payload"); + return _state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-protx-payload"); } if (newList.HasUniqueProperty(proTx.addr) && newList.GetUniquePropertyMN(proTx.addr)->proTxHash != proTx.proTxHash) { - return _state.DoS(100, false, REJECT_DUPLICATE, "bad-protx-dup-addr"); + return _state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_DUPLICATE, "bad-protx-dup-addr"); } CDeterministicMNCPtr dmn = newList.GetMN(proTx.proTxHash); if (!dmn) { - return _state.DoS(100, false, REJECT_INVALID, "bad-protx-hash"); + return _state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-protx-hash"); } auto newState = std::make_shared(*dmn->pdmnState); newState->addr = proTx.addr; @@ -777,12 +777,12 @@ bool CDeterministicMNManager::BuildNewListFromBlock(const CBlock& block, const C } else if (tx.nType == TRANSACTION_PROVIDER_UPDATE_REGISTRAR) { CProUpRegTx proTx; if (!GetTxPayload(tx, proTx)) { - return _state.DoS(100, false, REJECT_INVALID, "bad-protx-payload"); + return _state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-protx-payload"); } CDeterministicMNCPtr dmn = newList.GetMN(proTx.proTxHash); if (!dmn) { - return _state.DoS(100, false, REJECT_INVALID, "bad-protx-hash"); + return _state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-protx-hash"); } auto newState = std::make_shared(*dmn->pdmnState); if (newState->pubKeyOperator.Get() != proTx.pubKeyOperator) { @@ -803,12 +803,12 @@ bool CDeterministicMNManager::BuildNewListFromBlock(const CBlock& block, const C } else if (tx.nType == TRANSACTION_PROVIDER_UPDATE_REVOKE) { CProUpRevTx proTx; if (!GetTxPayload(tx, proTx)) { - return _state.DoS(100, false, REJECT_INVALID, "bad-protx-payload"); + return _state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-protx-payload"); } CDeterministicMNCPtr dmn = newList.GetMN(proTx.proTxHash); if (!dmn) { - return _state.DoS(100, false, REJECT_INVALID, "bad-protx-hash"); + return _state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-protx-hash"); } auto newState = std::make_shared(*dmn->pdmnState); newState->ResetOperatorFields(); @@ -824,7 +824,7 @@ bool CDeterministicMNManager::BuildNewListFromBlock(const CBlock& block, const C } else if (tx.nType == TRANSACTION_QUORUM_COMMITMENT) { llmq::CFinalCommitmentTxPayload qc; if (!GetTxPayload(tx, qc)) { - return _state.DoS(100, false, REJECT_INVALID, "bad-qc-payload"); + return _state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-qc-payload"); } if (!qc.commitment.IsNull()) { const auto& llmq_params = llmq::GetLLMQParams(qc.commitment.llmqType); @@ -833,7 +833,7 @@ bool CDeterministicMNManager::BuildNewListFromBlock(const CBlock& block, const C auto pQuorumBaseBlockIndex = pindexPrev->GetAncestor(quorumHeight); if (!pQuorumBaseBlockIndex || pQuorumBaseBlockIndex->GetBlockHash() != qc.commitment.quorumHash) { // we should actually never get into this case as validation should have caught it...but let's be sure - return _state.DoS(100, false, REJECT_INVALID, "bad-qc-quorum-hash"); + return _state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-qc-quorum-hash"); } HandleQuorumCommitment(qc.commitment, pQuorumBaseBlockIndex, newList, debugLogs); @@ -1187,23 +1187,23 @@ template static bool CheckService(const ProTx& proTx, CValidationState& state) { if (!proTx.addr.IsValid()) { - return state.DoS(10, false, REJECT_INVALID, "bad-protx-ipaddr"); + return state.Invalid(ValidationInvalidReason::TX_BAD_SPECIAL, false, REJECT_INVALID, "bad-protx-ipaddr"); } if (Params().RequireRoutableExternalIP() && !proTx.addr.IsRoutable()) { - return state.DoS(10, false, REJECT_INVALID, "bad-protx-ipaddr"); + return state.Invalid(ValidationInvalidReason::TX_BAD_SPECIAL, false, REJECT_INVALID, "bad-protx-ipaddr"); } static int mainnetDefaultPort = CreateChainParams(CBaseChainParams::MAIN)->GetDefaultPort(); if (Params().NetworkIDString() == CBaseChainParams::MAIN) { if (proTx.addr.GetPort() != mainnetDefaultPort) { - return state.DoS(10, false, REJECT_INVALID, "bad-protx-ipaddr-port"); + return state.Invalid(ValidationInvalidReason::TX_BAD_SPECIAL, false, REJECT_INVALID, "bad-protx-ipaddr-port"); } } else if (proTx.addr.GetPort() == mainnetDefaultPort) { - return state.DoS(10, false, REJECT_INVALID, "bad-protx-ipaddr-port"); + return state.Invalid(ValidationInvalidReason::TX_BAD_SPECIAL, false, REJECT_INVALID, "bad-protx-ipaddr-port"); } if (!proTx.addr.IsIPv4()) { - return state.DoS(10, false, REJECT_INVALID, "bad-protx-ipaddr"); + return state.Invalid(ValidationInvalidReason::TX_BAD_SPECIAL, false, REJECT_INVALID, "bad-protx-ipaddr"); } return true; @@ -1214,7 +1214,7 @@ static bool CheckHashSig(const ProTx& proTx, const CKeyID& keyID, CValidationSta { std::string strError; if (!CHashSigner::VerifyHash(::SerializeHash(proTx), keyID, proTx.vchSig, strError)) { - return state.DoS(100, false, REJECT_INVALID, "bad-protx-sig", false, strError); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-protx-sig"); } return true; } @@ -1224,7 +1224,7 @@ static bool CheckStringSig(const ProTx& proTx, const CKeyID& keyID, CValidationS { std::string strError; if (!CMessageSigner::VerifyMessage(keyID, proTx.vchSig, proTx.MakeSignString(), strError)) { - return state.DoS(100, false, REJECT_INVALID, "bad-protx-sig", false, strError); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-protx-sig"); } return true; } @@ -1233,7 +1233,7 @@ template static bool CheckHashSig(const ProTx& proTx, const CBLSPublicKey& pubKey, CValidationState& state) { if (!proTx.sig.VerifyInsecure(pubKey, ::SerializeHash(proTx))) { - return state.DoS(100, false, REJECT_INVALID, "bad-protx-sig", false); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-protx-sig"); } return true; } @@ -1241,16 +1241,16 @@ static bool CheckHashSig(const ProTx& proTx, const CBLSPublicKey& pubKey, CValid bool CheckProRegTx(const CTransaction& tx, const CBlockIndex* pindexPrev, CValidationState& state, const CCoinsViewCache& view, bool check_sigs) { if (tx.nType != TRANSACTION_PROVIDER_REGISTER) { - return state.DoS(100, false, REJECT_INVALID, "bad-protx-type"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-protx-type"); } CProRegTx ptx; if (!GetTxPayload(tx, ptx)) { - return state.DoS(100, false, REJECT_INVALID, "bad-protx-payload"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-protx-payload"); } if (auto maybe_err = ptx.IsTriviallyValid(); maybe_err.did_err) { - return state.DoS(maybe_err.ban_amount, false, REJECT_INVALID, std::string(maybe_err.error_str)); + return state.Invalid(maybe_err.reason, false, REJECT_INVALID, std::string(maybe_err.error_str)); } // It's allowed to set addr to 0, which will put the MN into PoSe-banned state and require a ProUpServTx to be issues later @@ -1267,31 +1267,31 @@ bool CheckProRegTx(const CTransaction& tx, const CBlockIndex* pindexPrev, CValid if (!ptx.collateralOutpoint.hash.IsNull()) { Coin coin; if (!view.GetCoin(ptx.collateralOutpoint, coin) || coin.IsSpent() || coin.out.nValue != 1000 * COIN) { - return state.DoS(10, false, REJECT_INVALID, "bad-protx-collateral"); + return state.Invalid(ValidationInvalidReason::TX_BAD_SPECIAL, false, REJECT_INVALID, "bad-protx-collateral"); } if (!ExtractDestination(coin.out.scriptPubKey, collateralTxDest)) { - return state.DoS(10, false, REJECT_INVALID, "bad-protx-collateral-dest"); + return state.Invalid(ValidationInvalidReason::TX_BAD_SPECIAL, false, REJECT_INVALID, "bad-protx-collateral-dest"); } // Extract key from collateral. This only works for P2PK and P2PKH collaterals and will fail for P2SH. // Issuer of this ProRegTx must prove ownership with this key by signing the ProRegTx keyForPayloadSig = boost::get(&collateralTxDest); if (!keyForPayloadSig) { - return state.DoS(10, false, REJECT_INVALID, "bad-protx-collateral-pkh"); + return state.Invalid(ValidationInvalidReason::TX_BAD_SPECIAL, false, REJECT_INVALID, "bad-protx-collateral-pkh"); } collateralOutpoint = ptx.collateralOutpoint; } else { if (ptx.collateralOutpoint.n >= tx.vout.size()) { - return state.DoS(10, false, REJECT_INVALID, "bad-protx-collateral-index"); + return state.Invalid(ValidationInvalidReason::TX_BAD_SPECIAL, false, REJECT_INVALID, "bad-protx-collateral-index"); } if (tx.vout[ptx.collateralOutpoint.n].nValue != 1000 * COIN) { - return state.DoS(10, false, REJECT_INVALID, "bad-protx-collateral"); + return state.Invalid(ValidationInvalidReason::TX_BAD_SPECIAL, false, REJECT_INVALID, "bad-protx-collateral"); } if (!ExtractDestination(tx.vout[ptx.collateralOutpoint.n].scriptPubKey, collateralTxDest)) { - return state.DoS(10, false, REJECT_INVALID, "bad-protx-collateral-dest"); + return state.Invalid(ValidationInvalidReason::TX_BAD_SPECIAL, false, REJECT_INVALID, "bad-protx-collateral-dest"); } collateralOutpoint = COutPoint(tx.GetHash(), ptx.collateralOutpoint.n); @@ -1300,7 +1300,7 @@ bool CheckProRegTx(const CTransaction& tx, const CBlockIndex* pindexPrev, CValid // don't allow reuse of collateral key for other keys (don't allow people to put the collateral key onto an online server) // this check applies to internal and external collateral, but internal collaterals are not necessarily a P2PKH if (collateralTxDest == CTxDestination(ptx.keyIDOwner) || collateralTxDest == CTxDestination(ptx.keyIDVoting)) { - return state.DoS(10, false, REJECT_INVALID, "bad-protx-collateral-reuse"); + return state.Invalid(ValidationInvalidReason::TX_BAD_SPECIAL, false, REJECT_INVALID, "bad-protx-collateral-reuse"); } if (pindexPrev) { @@ -1308,23 +1308,23 @@ bool CheckProRegTx(const CTransaction& tx, const CBlockIndex* pindexPrev, CValid // only allow reusing of addresses when it's for the same collateral (which replaces the old MN) if (mnList.HasUniqueProperty(ptx.addr) && mnList.GetUniquePropertyMN(ptx.addr)->collateralOutpoint != collateralOutpoint) { - return state.DoS(10, false, REJECT_DUPLICATE, "bad-protx-dup-addr"); + return state.Invalid(ValidationInvalidReason::TX_BAD_SPECIAL, false, REJECT_DUPLICATE, "bad-protx-dup-addr"); } // never allow duplicate keys, even if this ProTx would replace an existing MN if (mnList.HasUniqueProperty(ptx.keyIDOwner) || mnList.HasUniqueProperty(ptx.pubKeyOperator)) { - return state.DoS(10, false, REJECT_DUPLICATE, "bad-protx-dup-key"); + return state.Invalid(ValidationInvalidReason::TX_BAD_SPECIAL, false, REJECT_DUPLICATE, "bad-protx-dup-key"); } if (!deterministicMNManager->IsDIP3Enforced(pindexPrev->nHeight)) { if (ptx.keyIDOwner != ptx.keyIDVoting) { - return state.DoS(10, false, REJECT_INVALID, "bad-protx-key-not-same"); + return state.Invalid(ValidationInvalidReason::TX_BAD_SPECIAL, false, REJECT_INVALID, "bad-protx-key-not-same"); } } } if (auto maybe_err = CheckInputsHash(tx, ptx); maybe_err.did_err) { - return state.DoS(maybe_err.ban_amount, false, REJECT_INVALID, std::string(maybe_err.error_str)); + return state.Invalid(maybe_err.reason, false, REJECT_INVALID, std::string(maybe_err.error_str)); } if (keyForPayloadSig) { @@ -1336,7 +1336,7 @@ bool CheckProRegTx(const CTransaction& tx, const CBlockIndex* pindexPrev, CValid } else { // collateral is part of this ProRegTx, so we know the collateral is owned by the issuer if (!ptx.vchSig.empty()) { - return state.DoS(100, false, REJECT_INVALID, "bad-protx-sig"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-protx-sig"); } } @@ -1346,16 +1346,16 @@ bool CheckProRegTx(const CTransaction& tx, const CBlockIndex* pindexPrev, CValid bool CheckProUpServTx(const CTransaction& tx, const CBlockIndex* pindexPrev, CValidationState& state, bool check_sigs) { if (tx.nType != TRANSACTION_PROVIDER_UPDATE_SERVICE) { - return state.DoS(100, false, REJECT_INVALID, "bad-protx-type"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-protx-type"); } CProUpServTx ptx; if (!GetTxPayload(tx, ptx)) { - return state.DoS(100, false, REJECT_INVALID, "bad-protx-payload"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-protx-payload"); } if (auto maybe_err = ptx.IsTriviallyValid(); maybe_err.did_err) { - return state.DoS(maybe_err.ban_amount, false, REJECT_INVALID, std::string(maybe_err.error_str)); + return state.Invalid(maybe_err.reason, false, REJECT_INVALID, std::string(maybe_err.error_str)); } if (!CheckService(ptx, state)) { @@ -1367,27 +1367,27 @@ bool CheckProUpServTx(const CTransaction& tx, const CBlockIndex* pindexPrev, CVa auto mnList = deterministicMNManager->GetListForBlock(pindexPrev); auto mn = mnList.GetMN(ptx.proTxHash); if (!mn) { - return state.DoS(100, false, REJECT_INVALID, "bad-protx-hash"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-protx-hash"); } // don't allow updating to addresses already used by other MNs if (mnList.HasUniqueProperty(ptx.addr) && mnList.GetUniquePropertyMN(ptx.addr)->proTxHash != ptx.proTxHash) { - return state.DoS(10, false, REJECT_DUPLICATE, "bad-protx-dup-addr"); + return state.Invalid(ValidationInvalidReason::TX_BAD_SPECIAL, false, REJECT_DUPLICATE, "bad-protx-dup-addr"); } if (ptx.scriptOperatorPayout != CScript()) { if (mn->nOperatorReward == 0) { // don't allow setting operator reward payee in case no operatorReward was set - return state.DoS(10, false, REJECT_INVALID, "bad-protx-operator-payee"); + return state.Invalid(ValidationInvalidReason::TX_BAD_SPECIAL, false, REJECT_INVALID, "bad-protx-operator-payee"); } if (!ptx.scriptOperatorPayout.IsPayToPublicKeyHash() && !ptx.scriptOperatorPayout.IsPayToScriptHash()) { - return state.DoS(10, false, REJECT_INVALID, "bad-protx-operator-payee"); + return state.Invalid(ValidationInvalidReason::TX_BAD_SPECIAL, false, REJECT_INVALID, "bad-protx-operator-payee"); } } // we can only check the signature if pindexPrev != nullptr and the MN is known if (auto maybe_err = CheckInputsHash(tx, ptx); maybe_err.did_err) { - return state.DoS(maybe_err.ban_amount, false, REJECT_INVALID, std::string(maybe_err.error_str)); + return state.Invalid(maybe_err.reason, false, REJECT_INVALID, std::string(maybe_err.error_str)); } if (check_sigs && !CheckHashSig(ptx, mn->pdmnState->pubKeyOperator.Get(), state)) { // pass the state returned by the function above @@ -1401,66 +1401,66 @@ bool CheckProUpServTx(const CTransaction& tx, const CBlockIndex* pindexPrev, CVa bool CheckProUpRegTx(const CTransaction& tx, const CBlockIndex* pindexPrev, CValidationState& state, const CCoinsViewCache& view, bool check_sigs) { if (tx.nType != TRANSACTION_PROVIDER_UPDATE_REGISTRAR) { - return state.DoS(100, false, REJECT_INVALID, "bad-protx-type"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-protx-type"); } CProUpRegTx ptx; if (!GetTxPayload(tx, ptx)) { - return state.DoS(100, false, REJECT_INVALID, "bad-protx-payload"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-protx-payload"); } if (auto maybe_err = ptx.IsTriviallyValid(); maybe_err.did_err) { - return state.DoS(maybe_err.ban_amount, false, REJECT_INVALID, std::string(maybe_err.error_str)); + return state.Invalid(maybe_err.reason, false, REJECT_INVALID, std::string(maybe_err.error_str)); } CTxDestination payoutDest; if (!ExtractDestination(ptx.scriptPayout, payoutDest)) { // should not happen as we checked script types before - return state.DoS(10, false, REJECT_INVALID, "bad-protx-payee-dest"); + return state.Invalid(ValidationInvalidReason::TX_BAD_SPECIAL, false, REJECT_INVALID, "bad-protx-payee-dest"); } if (pindexPrev) { auto mnList = deterministicMNManager->GetListForBlock(pindexPrev); auto dmn = mnList.GetMN(ptx.proTxHash); if (!dmn) { - return state.DoS(100, false, REJECT_INVALID, "bad-protx-hash"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-protx-hash"); } // don't allow reuse of payee key for other keys (don't allow people to put the payee key onto an online server) if (payoutDest == CTxDestination(dmn->pdmnState->keyIDOwner) || payoutDest == CTxDestination(ptx.keyIDVoting)) { - return state.DoS(10, false, REJECT_INVALID, "bad-protx-payee-reuse"); + return state.Invalid(ValidationInvalidReason::TX_BAD_SPECIAL, false, REJECT_INVALID, "bad-protx-payee-reuse"); } Coin coin; if (!view.GetCoin(dmn->collateralOutpoint, coin) || coin.IsSpent()) { // this should never happen (there would be no dmn otherwise) - return state.DoS(100, false, REJECT_INVALID, "bad-protx-collateral"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-protx-collateral"); } // don't allow reuse of collateral key for other keys (don't allow people to put the collateral key onto an online server) CTxDestination collateralTxDest; if (!ExtractDestination(coin.out.scriptPubKey, collateralTxDest)) { - return state.DoS(100, false, REJECT_INVALID, "bad-protx-collateral-dest"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-protx-collateral-dest"); } if (collateralTxDest == CTxDestination(dmn->pdmnState->keyIDOwner) || collateralTxDest == CTxDestination(ptx.keyIDVoting)) { - return state.DoS(10, false, REJECT_INVALID, "bad-protx-collateral-reuse"); + return state.Invalid(ValidationInvalidReason::TX_BAD_SPECIAL, false, REJECT_INVALID, "bad-protx-collateral-reuse"); } if (mnList.HasUniqueProperty(ptx.pubKeyOperator)) { auto otherDmn = mnList.GetUniquePropertyMN(ptx.pubKeyOperator); if (ptx.proTxHash != otherDmn->proTxHash) { - return state.DoS(10, false, REJECT_DUPLICATE, "bad-protx-dup-key"); + return state.Invalid(ValidationInvalidReason::TX_BAD_SPECIAL, false, REJECT_DUPLICATE, "bad-protx-dup-key"); } } if (!deterministicMNManager->IsDIP3Enforced(pindexPrev->nHeight)) { if (dmn->pdmnState->keyIDOwner != ptx.keyIDVoting) { - return state.DoS(10, false, REJECT_INVALID, "bad-protx-key-not-same"); + return state.Invalid(ValidationInvalidReason::TX_BAD_SPECIAL, false, REJECT_INVALID, "bad-protx-key-not-same"); } } if (auto maybe_err = CheckInputsHash(tx, ptx); maybe_err.did_err) { - return state.DoS(maybe_err.ban_amount, false, REJECT_INVALID, std::string(maybe_err.error_str)); + return state.Invalid(maybe_err.reason, false, REJECT_INVALID, std::string(maybe_err.error_str)); } if (check_sigs && !CheckHashSig(ptx, dmn->pdmnState->keyIDOwner, state)) { // pass the state returned by the function above @@ -1474,26 +1474,26 @@ bool CheckProUpRegTx(const CTransaction& tx, const CBlockIndex* pindexPrev, CVal bool CheckProUpRevTx(const CTransaction& tx, const CBlockIndex* pindexPrev, CValidationState& state, bool check_sigs) { if (tx.nType != TRANSACTION_PROVIDER_UPDATE_REVOKE) { - return state.DoS(100, false, REJECT_INVALID, "bad-protx-type"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-protx-type"); } CProUpRevTx ptx; if (!GetTxPayload(tx, ptx)) { - return state.DoS(100, false, REJECT_INVALID, "bad-protx-payload"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-protx-payload"); } if (auto maybe_err = ptx.IsTriviallyValid(); maybe_err.did_err) { - return state.DoS(maybe_err.ban_amount, false, REJECT_INVALID, std::string(maybe_err.error_str)); + return state.Invalid(maybe_err.reason, false, REJECT_INVALID, std::string(maybe_err.error_str)); } if (pindexPrev) { auto mnList = deterministicMNManager->GetListForBlock(pindexPrev); auto dmn = mnList.GetMN(ptx.proTxHash); if (!dmn) - return state.DoS(100, false, REJECT_INVALID, "bad-protx-hash"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-protx-hash"); if (auto maybe_err = CheckInputsHash(tx, ptx); maybe_err.did_err) { - return state.DoS(maybe_err.ban_amount, false, REJECT_INVALID, std::string(maybe_err.error_str)); + return state.Invalid(maybe_err.reason, false, REJECT_INVALID, std::string(maybe_err.error_str)); } if (check_sigs && !CheckHashSig(ptx, dmn->pdmnState->pubKeyOperator.Get(), state)) { // pass the state returned by the function above diff --git a/src/evo/mnhftx.cpp b/src/evo/mnhftx.cpp index f753ae5d840a..8c2446595a0e 100644 --- a/src/evo/mnhftx.cpp +++ b/src/evo/mnhftx.cpp @@ -34,29 +34,29 @@ bool CheckMNHFTx(const CTransaction& tx, const CBlockIndex* pindexPrev, CValidat { MNHFTxPayload mnhfTx; if (!GetTxPayload(tx, mnhfTx)) { - return state.DoS(100, false, REJECT_INVALID, "bad-mnhf-payload"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-mnhf-payload"); } if (mnhfTx.nVersion == 0 || mnhfTx.nVersion > MNHFTxPayload::CURRENT_VERSION) { - return state.DoS(100, false, REJECT_INVALID, "bad-mnhf-version"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-mnhf-version"); } const CBlockIndex* pindexQuorum = LookupBlockIndex(mnhfTx.signal.quorumHash); if (!pindexQuorum) { - return state.DoS(100, false, REJECT_INVALID, "bad-mnhf-quorum-hash"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-mnhf-quorum-hash"); } if (pindexQuorum != pindexPrev->GetAncestor(pindexQuorum->nHeight)) { // not part of active chain - return state.DoS(100, false, REJECT_INVALID, "bad-mnhf-quorum-hash"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-mnhf-quorum-hash"); } if (!Params().HasLLMQ(Params().GetConsensus().llmqTypeMnhf)) { - return state.DoS(100, false, REJECT_INVALID, "bad-mnhf-type"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-mnhf-type"); } if (!mnhfTx.signal.Verify(pindexQuorum)) { - return state.DoS(100, false, REJECT_INVALID, "bad-mnhf-invalid"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-mnhf-invalid"); } return true; diff --git a/src/evo/providertx.cpp b/src/evo/providertx.cpp index 426f1711dc23..83e8622da6fe 100644 --- a/src/evo/providertx.cpp +++ b/src/evo/providertx.cpp @@ -12,34 +12,34 @@ maybe_error CProRegTx::IsTriviallyValid() const { if (nVersion == 0 || nVersion > CProRegTx::CURRENT_VERSION) { - return {100, "bad-protx-version"}; + return {ValidationInvalidReason::CONSENSUS, "bad-protx-version"}; } if (nType != 0) { - return {100, "bad-protx-type"}; + return {ValidationInvalidReason::CONSENSUS, "bad-protx-type"}; } if (nMode != 0) { - return {100, "bad-protx-mode"}; + return {ValidationInvalidReason::CONSENSUS, "bad-protx-mode"}; } if (keyIDOwner.IsNull() || !pubKeyOperator.IsValid() || keyIDVoting.IsNull()) { - return {10, "bad-protx-key-null"}; + return {ValidationInvalidReason::TX_BAD_SPECIAL, "bad-protx-key-null"}; } if (!scriptPayout.IsPayToPublicKeyHash() && !scriptPayout.IsPayToScriptHash()) { - return {10, "bad-protx-payee"}; + return {ValidationInvalidReason::TX_BAD_SPECIAL, "bad-protx-payee"}; } CTxDestination payoutDest; if (!ExtractDestination(scriptPayout, payoutDest)) { // should not happen as we checked script types before - return {10, "bad-protx-payee-dest"}; + return {ValidationInvalidReason::TX_BAD_SPECIAL, "bad-protx-payee-dest"}; } // don't allow reuse of payout key for other keys (don't allow people to put the payee key onto an online server) if (payoutDest == CTxDestination(keyIDOwner) || payoutDest == CTxDestination(keyIDVoting)) { - return {10, "bad-protx-payee-reuse"}; + return {ValidationInvalidReason::TX_BAD_SPECIAL, "bad-protx-payee-reuse"}; } if (nOperatorReward > 10000) { - return {10, "bad-protx-operator-reward"}; + return {ValidationInvalidReason::TX_BAD_SPECIAL, "bad-protx-operator-reward"}; } return {}; @@ -85,7 +85,7 @@ std::string CProRegTx::ToString() const maybe_error CProUpServTx::IsTriviallyValid() const { if (nVersion == 0 || nVersion > CProUpServTx::CURRENT_VERSION) { - return {100, "bad-protx-version"}; + return {ValidationInvalidReason::CONSENSUS, "bad-protx-version"}; } return {}; @@ -106,17 +106,17 @@ std::string CProUpServTx::ToString() const maybe_error CProUpRegTx::IsTriviallyValid() const { if (nVersion == 0 || nVersion > CProUpRegTx::CURRENT_VERSION) { - return {100, "bad-protx-version"}; + return {ValidationInvalidReason::CONSENSUS, "bad-protx-version"}; } if (nMode != 0) { - return {100, "bad-protx-mode"}; + return {ValidationInvalidReason::CONSENSUS, "bad-protx-mode"}; } if (!pubKeyOperator.IsValid() || keyIDVoting.IsNull()) { - return {10, "bad-protx-key-null"}; + return {ValidationInvalidReason::TX_BAD_SPECIAL, "bad-protx-key-null"}; } if (!scriptPayout.IsPayToPublicKeyHash() && !scriptPayout.IsPayToScriptHash()) { - return {10, "bad-protx-payee"}; + return {ValidationInvalidReason::TX_BAD_SPECIAL, "bad-protx-payee"}; } return {}; } @@ -136,13 +136,13 @@ std::string CProUpRegTx::ToString() const maybe_error CProUpRevTx::IsTriviallyValid() const { if (nVersion == 0 || nVersion > CProUpRevTx::CURRENT_VERSION) { - return {100, "bad-protx-version"}; + return {ValidationInvalidReason::CONSENSUS, "bad-protx-version"}; } // nReason < CProUpRevTx::REASON_NOT_SPECIFIED is always `false` since // nReason is unsigned and CProUpRevTx::REASON_NOT_SPECIFIED == 0 if (nReason > CProUpRevTx::REASON_LAST) { - return {100, "bad-protx-reason"}; + return {ValidationInvalidReason::CONSENSUS, "bad-protx-reason"}; } return {}; } diff --git a/src/evo/providertx.h b/src/evo/providertx.h index 515609d7e83a..e905bc166fb8 100644 --- a/src/evo/providertx.h +++ b/src/evo/providertx.h @@ -9,6 +9,7 @@ #include #include +#include #include #include #include @@ -20,11 +21,11 @@ class CValidationState; struct maybe_error{ bool did_err{false}; - int ban_amount{0}; + ValidationInvalidReason reason{ValidationInvalidReason::CONSENSUS}; std::string_view error_str; constexpr maybe_error() = default; - constexpr maybe_error(int amount, std::string_view err): did_err(true), ban_amount(amount), error_str(err) {}; + constexpr maybe_error(ValidationInvalidReason reasonIn, std::string_view err): did_err(true), reason(reasonIn), error_str(err) {}; }; class CProRegTx @@ -235,7 +236,7 @@ template static maybe_error CheckInputsHash(const CTransaction& tx, const ProTx& proTx) { if (uint256 inputsHash = CalcTxInputsHash(tx); inputsHash != proTx.inputsHash) { - return {100, "bad-protx-inputs-hash"}; + return {ValidationInvalidReason::CONSENSUS, "bad-protx-inputs-hash"}; } return {}; diff --git a/src/evo/specialtxman.cpp b/src/evo/specialtxman.cpp index 63e8694500c4..b291db80f892 100644 --- a/src/evo/specialtxman.cpp +++ b/src/evo/specialtxman.cpp @@ -24,7 +24,7 @@ bool CheckSpecialTx(const CTransaction& tx, const CBlockIndex* pindexPrev, CVali return true; if (pindexPrev && pindexPrev->nHeight + 1 < Params().GetConsensus().DIP0003Height) { - return state.DoS(10, false, REJECT_INVALID, "bad-tx-type"); + return state.Invalid(ValidationInvalidReason::TX_BAD_SPECIAL, false, REJECT_INVALID, "bad-tx-type"); } try { @@ -46,10 +46,10 @@ bool CheckSpecialTx(const CTransaction& tx, const CBlockIndex* pindexPrev, CVali } } catch (const std::exception& e) { LogPrintf("%s -- failed: %s\n", __func__, e.what()); - return state.DoS(100, false, REJECT_INVALID, "failed-check-special-tx"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "failed-check-special-tx"); } - return state.DoS(10, false, REJECT_INVALID, "bad-tx-type-check"); + return state.Invalid(ValidationInvalidReason::TX_BAD_SPECIAL, false, REJECT_INVALID, "bad-tx-type-check"); } bool ProcessSpecialTx(const CTransaction& tx, const CBlockIndex* pindex, CValidationState& state) @@ -72,7 +72,7 @@ bool ProcessSpecialTx(const CTransaction& tx, const CBlockIndex* pindex, CValida return true; // handled per block } - return state.DoS(100, false, REJECT_INVALID, "bad-tx-type-proc"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-tx-type-proc"); } bool UndoSpecialTx(const CTransaction& tx, const CBlockIndex* pindex) @@ -153,7 +153,7 @@ bool ProcessSpecialTxsInBlock(const CBlock& block, const CBlockIndex* pindex, CV LogPrint(BCLog::BENCHMARK, " - CheckCbTxMerkleRoots: %.2fms [%.2fs]\n", 0.001 * (nTime5 - nTime4), nTimeMerkle * 0.000001); } catch (const std::exception& e) { LogPrintf("%s -- failed: %s\n", __func__, e.what()); - return state.DoS(100, false, REJECT_INVALID, "failed-procspectxsinblock"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "failed-procspectxsinblock"); } return true; diff --git a/src/init.cpp b/src/init.cpp index a94f03f6e9c9..b07e158c8017 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -544,8 +544,8 @@ void SetupServerArgs(NodeContext& node) argsman.AddArg("-asmap=", strprintf("Specify asn mapping used for bucketing of the peers (default: %s). Relative paths will be prefixed by the net-specific datadir location.", DEFAULT_ASMAP_FILENAME), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-addnode=", "Add a node to connect to and attempt to keep the connection open (see the `addnode` RPC command help for more info). This option can be specified multiple times to add multiple nodes.", ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::CONNECTION); argsman.AddArg("-allowprivatenet", strprintf("Allow RFC1918 addresses to be relayed and connected to (default: %u)", DEFAULT_ALLOWPRIVATENET), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); - argsman.AddArg("-banscore=", strprintf("Threshold for disconnecting misbehaving peers (default: %u)", DEFAULT_BANSCORE_THRESHOLD), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); - argsman.AddArg("-bantime=", strprintf("Number of seconds to keep misbehaving peers from reconnecting (default: %u)", DEFAULT_MISBEHAVING_BANTIME), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); + argsman.AddArg("-banscore=", strprintf("Threshold for disconnecting and discouraging misbehaving peers (default: %u)", DEFAULT_BANSCORE_THRESHOLD), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); + argsman.AddArg("-bantime=", strprintf("Default duration (in seconds) of manually configured bans (default: %u)", DEFAULT_MISBEHAVING_BANTIME), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-bind=", "Bind to given address and always listen on it. Use [host]:port notation for IPv6", ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::CONNECTION); argsman.AddArg("-connect=", "Connect only to the specified node; -noconnect disables automatic connections (the rules for this peer are the same as for -addnode). This option can be specified multiple times to connect to multiple nodes.", ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::CONNECTION); argsman.AddArg("-discover", "Discover own IP addresses (default: 1 when listening and no -externalip or -proxy)", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); diff --git a/src/interfaces/node.cpp b/src/interfaces/node.cpp index b1db4d297ff2..8f8356a62e60 100644 --- a/src/interfaces/node.cpp +++ b/src/interfaces/node.cpp @@ -257,10 +257,10 @@ class NodeImpl : public Node } return false; } - bool ban(const CNetAddr& net_addr, BanReason reason, int64_t ban_time_offset) override + bool ban(const CNetAddr& net_addr, int64_t ban_time_offset) override { if (m_context->banman) { - m_context->banman->Ban(net_addr, reason, ban_time_offset); + m_context->banman->Ban(net_addr, ban_time_offset); return true; } return false; diff --git a/src/interfaces/node.h b/src/interfaces/node.h index 1e928bbbc1b4..ef4947ab335b 100644 --- a/src/interfaces/node.h +++ b/src/interfaces/node.h @@ -195,7 +195,7 @@ class Node virtual bool getBanned(banmap_t& banmap) = 0; //! Ban node. - virtual bool ban(const CNetAddr& net_addr, BanReason reason, int64_t ban_time_offset) = 0; + virtual bool ban(const CNetAddr& net_addr, int64_t ban_time_offset) = 0; //! Unban node. virtual bool unban(const CSubNet& ip) = 0; diff --git a/src/llmq/blockprocessor.cpp b/src/llmq/blockprocessor.cpp index e176343b16b7..2f8cc245c814 100644 --- a/src/llmq/blockprocessor.cpp +++ b/src/llmq/blockprocessor.cpp @@ -153,11 +153,11 @@ bool CQuorumBlockProcessor::ProcessBlock(const CBlock& block, const CBlockIndex* const auto numCommitmentsInNewBlock = qcs.count(params.type); if (!isCommitmentRequired && numCommitmentsInNewBlock > 0) { - return state.DoS(100, false, REJECT_INVALID, "bad-qc-not-allowed"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-qc-not-allowed"); } if (isCommitmentRequired && numCommitmentsInNewBlock == 0) { - return state.DoS(100, false, REJECT_INVALID, "bad-qc-missing"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-qc-missing"); } if (llmq::CLLMQUtils::IsQuorumRotationEnabled(params.type, pindex)) { LogPrintf("[ProcessBlock] h[%d] isCommitmentRequired[%d] numCommitmentsInNewBlock[%d]\n", pindex->nHeight, isCommitmentRequired, numCommitmentsInNewBlock); @@ -210,31 +210,31 @@ bool CQuorumBlockProcessor::ProcessCommitment(int nHeight, const uint256& blockH if (quorumHash.IsNull()) { LogPrint(BCLog::LLMQ, "CQuorumBlockProcessor::%s height=%d, type=%d, quorumIndex=%d, quorumHash=%s, signers=%s, validMembers=%d, quorumPublicKey=%s quorumHash is null.\n", __func__, nHeight, uint8_t(qc.llmqType), qc.quorumIndex, quorumHash.ToString(), qc.CountSigners(), qc.CountValidMembers(), qc.quorumPublicKey.ToString()); - return state.DoS(100, false, REJECT_INVALID, "bad-qc-block"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-qc-block"); } if (quorumHash != qc.quorumHash) { LogPrint(BCLog::LLMQ, "CQuorumBlockProcessor::%s height=%d, type=%d, quorumIndex=%d, quorumHash=%s, qc.quorumHash=%s signers=%s, validMembers=%d, quorumPublicKey=%s non equal quorumHash.\n", __func__, nHeight, uint8_t(qc.llmqType), qc.quorumIndex, quorumHash.ToString(), qc.quorumHash.ToString(), qc.CountSigners(), qc.CountValidMembers(), qc.quorumPublicKey.ToString()); - return state.DoS(100, false, REJECT_INVALID, "bad-qc-block"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-qc-block"); } if (qc.IsNull()) { if (!qc.VerifyNull()) { LogPrint(BCLog::LLMQ, "CQuorumBlockProcessor::%s height=%d, type=%d, quorumIndex=%d, quorumHash=%s, signers=%s, validMembers=%dqc verifynull failed.\n", __func__, nHeight, uint8_t(qc.llmqType), qc.quorumIndex, quorumHash.ToString(), qc.CountSigners(), qc.CountValidMembers()); - return state.DoS(100, false, REJECT_INVALID, "bad-qc-invalid-null"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-qc-invalid-null"); } return true; } if (HasMinedCommitment(llmq_params.type, quorumHash)) { // should not happen as it's already handled in ProcessBlock - return state.DoS(100, false, REJECT_INVALID, "bad-qc-dup"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-qc-dup"); } if (!IsMiningPhase(llmq_params, nHeight)) { // should not happen as it's already handled in ProcessBlock - return state.DoS(100, false, REJECT_INVALID, "bad-qc-height"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-qc-height"); } const auto* pQuorumBaseBlockIndex = LookupBlockIndex(qc.quorumHash); @@ -242,7 +242,7 @@ bool CQuorumBlockProcessor::ProcessCommitment(int nHeight, const uint256& blockH if (!qc.Verify(pQuorumBaseBlockIndex, fBLSChecks)) { LogPrint(BCLog::LLMQ, "CQuorumBlockProcessor::%s height=%d, type=%d, quorumIndex=%d, quorumHash=%s, signers=%s, validMembers=%d, quorumPublicKey=%s qc verify failed.\n", __func__, nHeight, uint8_t(qc.llmqType), qc.quorumIndex, quorumHash.ToString(), qc.CountSigners(), qc.CountValidMembers(), qc.quorumPublicKey.ToString()); - return state.DoS(100, false, REJECT_INVALID, "bad-qc-invalid"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-qc-invalid"); } if (fJustCheck) { @@ -385,13 +385,13 @@ bool CQuorumBlockProcessor::GetCommitmentsFromBlock(const CBlock& block, const C if (!GetTxPayload(*tx, qc)) { // should not happen as it was verified before processing the block LogPrint(BCLog::LLMQ, "CQuorumBlockProcessor::%s height=%d GetTxPayload fails\n", __func__, pindex->nHeight); - return state.DoS(100, false, REJECT_INVALID, "bad-qc-payload"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-qc-payload"); } // only allow one commitment per type and per block (This was changed with rotation) if (!CLLMQUtils::IsQuorumRotationEnabled(qc.commitment.llmqType, pindex)) { if (ret.count(qc.commitment.llmqType) != 0) { - return state.DoS(100, false, REJECT_INVALID, "bad-qc-dup"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-qc-dup"); } } @@ -400,7 +400,7 @@ bool CQuorumBlockProcessor::GetCommitmentsFromBlock(const CBlock& block, const C } if (pindex->nHeight < consensus.DIP0003Height && !ret.empty()) { - return state.DoS(100, false, REJECT_INVALID, "bad-qc-premature"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-qc-premature"); } return true; diff --git a/src/llmq/commitment.cpp b/src/llmq/commitment.cpp index 14683ae887ec..838ea875f21a 100644 --- a/src/llmq/commitment.cpp +++ b/src/llmq/commitment.cpp @@ -165,7 +165,7 @@ bool CheckLLMQCommitment(const CTransaction& tx, const CBlockIndex* pindexPrev, CFinalCommitmentTxPayload qcTx; if (!GetTxPayload(tx, qcTx)) { LogPrintfFinalCommitment("h[%d] GetTxPayload failed\n", pindexPrev->nHeight); - return state.DoS(100, false, REJECT_INVALID, "bad-qc-payload"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-qc-payload"); } const auto& llmq_params = GetLLMQParams(qcTx.commitment.llmqType); std::stringstream ss; @@ -176,40 +176,40 @@ bool CheckLLMQCommitment(const CTransaction& tx, const CBlockIndex* pindexPrev, if (qcTx.nVersion == 0 || qcTx.nVersion > CFinalCommitmentTxPayload::CURRENT_VERSION) { LogPrintfFinalCommitment("h[%d] invalid qcTx.nVersion[%d]\n", pindexPrev->nHeight, qcTx.nVersion); - return state.DoS(100, false, REJECT_INVALID, "bad-qc-version"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-qc-version"); } if (qcTx.nHeight != uint32_t(pindexPrev->nHeight + 1)) { LogPrintfFinalCommitment("h[%d] invalid qcTx.nHeight[%d]\n", pindexPrev->nHeight, qcTx.nHeight); - return state.DoS(100, false, REJECT_INVALID, "bad-qc-height"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-qc-height"); } const CBlockIndex* pQuorumBaseBlockIndex = WITH_LOCK(cs_main, return LookupBlockIndex(qcTx.commitment.quorumHash)); if (!pQuorumBaseBlockIndex) { - return state.DoS(100, false, REJECT_INVALID, "bad-qc-quorum-hash"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-qc-quorum-hash"); } if (pQuorumBaseBlockIndex != pindexPrev->GetAncestor(pQuorumBaseBlockIndex->nHeight)) { // not part of active chain - return state.DoS(100, false, REJECT_INVALID, "bad-qc-quorum-hash"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-qc-quorum-hash"); } if (!Params().HasLLMQ(qcTx.commitment.llmqType)) { - return state.DoS(100, false, REJECT_INVALID, "bad-qc-type"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-qc-type"); } if (qcTx.commitment.IsNull()) { if (!qcTx.commitment.VerifyNull()) { LogPrintfFinalCommitment("h[%d] invalid qcTx.commitment[%s] VerifyNull failed\n", pindexPrev->nHeight, qcTx.commitment.quorumHash.ToString()); - return state.DoS(100, false, REJECT_INVALID, "bad-qc-invalid-null"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-qc-invalid-null"); } return true; } if (!qcTx.commitment.Verify(pQuorumBaseBlockIndex, false)) { LogPrintfFinalCommitment("h[%d] invalid qcTx.commitment[%s] Verify failed\n", pindexPrev->nHeight, qcTx.commitment.quorumHash.ToString()); - return state.DoS(100, false, REJECT_INVALID, "bad-qc-invalid"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-qc-invalid"); } LogPrintfFinalCommitment("h[%d] CheckLLMQCommitment VALID\n", pindexPrev->nHeight); diff --git a/src/net.cpp b/src/net.cpp index e849c7404832..139b780518f6 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -1095,17 +1095,24 @@ void CConnman::AcceptConnection(const ListenSocket& hListenSocket) { // on all platforms. Set it again here just to be sure. SetSocketNoDelay(hSocket); - int bannedlevel = m_banman ? m_banman->IsBannedLevel(addr) : 0; - - // Don't accept connections from banned peers, but if our inbound slots aren't almost full, accept - // if the only banning reason was an automatic misbehavior ban. - if (!NetPermissions::HasFlag(permissionFlags, NetPermissionFlags::PF_NOBAN) && bannedlevel > ((nInbound + 1 < nMaxInbound) ? 1 : 0)) + // Don't accept connections from banned peers. + bool banned = m_banman->IsBanned(addr); + if (!NetPermissions::HasFlag(permissionFlags, NetPermissionFlags::PF_NOBAN) && banned) { LogPrint(BCLog::NET, "%s (banned)\n", strDropped); CloseSocket(hSocket); return; } + // Only accept connections from discouraged peers if our inbound slots aren't (almost) full. + bool discouraged = m_banman->IsDiscouraged(addr); + if (!NetPermissions::HasFlag(permissionFlags, NetPermissionFlags::PF_NOBAN) && nInbound + 1 >= nMaxInbound && discouraged) + { + LogPrint(BCLog::NET, "connection from %s dropped (discouraged)\n", addr.ToString()); + CloseSocket(hSocket); + return; + } + // Evict connections until we are below nMaxInbound. In case eviction protection resulted in nodes to not be evicted // before, they might get evicted in batches now (after the protection timeout). // We don't evict verified MN connections and also don't take them into account when checking limits. We can do this @@ -1142,7 +1149,7 @@ void CConnman::AcceptConnection(const ListenSocket& hListenSocket) { pnode->m_permissionFlags = permissionFlags; // If this flag is present, the user probably expect that RPC and QT report it as whitelisted (backward compatibility) pnode->m_legacyWhitelisted = legacyWhitelisted; - pnode->m_prefer_evict = bannedlevel > 0; + pnode->m_prefer_evict = discouraged; m_msgproc->InitializeNode(pnode); if (fLogIPs) { @@ -2527,8 +2534,8 @@ void CConnman::OpenNetworkConnection(const CAddress& addrConnect, bool fCountFai return; } if (!pszDest) { - // banned or exact match? - if ((m_banman && m_banman->IsBanned(addrConnect)) || FindNode(addrConnect.ToStringIPPort())) + // banned, discouraged or exact match? + if ((m_banman && (m_banman->IsDiscouraged(addrConnect) || m_banman->IsBanned(addrConnect))) || FindNode(addrConnect.ToStringIPPort())) return; // local and not a connection to itself? bool fAllowLocal = Params().AllowMultiplePorts() && addrConnect.GetPort() != GetListenPort(); diff --git a/src/net_permissions.h b/src/net_permissions.h index 789c417808d8..e8dac927a96f 100644 --- a/src/net_permissions.h +++ b/src/net_permissions.h @@ -21,7 +21,7 @@ enum NetPermissionFlags // Always relay transactions from this peer, even if already in mempool or rejected from policy // Keep parameter interaction: forcerelay implies relay PF_FORCERELAY = (1U << 2) | PF_RELAY, - // Can't be banned for misbehavior + // Can't be banned/disconnected/discouraged for misbehavior PF_NOBAN = (1U << 4), // Can query the mempool PF_MEMPOOL = (1U << 5), diff --git a/src/net_processing.cpp b/src/net_processing.cpp index ad4e66ffcd8e..5d442c825bbb 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -266,8 +266,8 @@ struct CNodeState { bool fCurrentlyConnected; //! Accumulated misbehaviour score for this peer. int nMisbehavior; - //! Whether this peer should be disconnected and banned (unless whitelisted). - bool fShouldBan; + //! Whether this peer should be disconnected and marked as discouraged (unless whitelisted with noban). + bool m_should_discourage; //! String name of this peer (debugging/logging purposes). const std::string name; //! List of asynchronously-determined block rejections to notify this peer about. @@ -402,10 +402,19 @@ struct CNodeState { ObjectDownloadState m_object_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; + m_should_discourage = false; pindexBestKnownBlock = nullptr; hashLastUnknownBlock.SetNull(); pindexLastCommonBlock = nullptr; @@ -899,7 +908,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()); @@ -1112,7 +1121,7 @@ unsigned int LimitOrphanTxSize(unsigned int nMaxOrphansSize) void static ProcessOrphanTx(CConnman* connman, CTxMemPool& mempool, std::set& orphan_work_set) EXCLUSIVE_LOCKS_REQUIRED(cs_main, g_cs_orphans); /** - * Mark a misbehaving peer to be banned depending upon the value of `-banscore`. + * Increment peer's misbehavior score. If the new value surpasses banscore (specified on startup or by default), mark node to be discouraged, meaning the peer might be disconnected & added to the discouragement filter. */ void Misbehaving(NodeId pnode, int howmuch, const std::string& message) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { @@ -1128,8 +1137,8 @@ void Misbehaving(NodeId pnode, int howmuch, const std::string& message) EXCLUSIV std::string message_prefixed = message.empty() ? "" : (": " + message); if (state->nMisbehavior >= banscore && state->nMisbehavior - howmuch < banscore) { - LogPrint(BCLog::NET, "%s: %s peer=%d (%d -> %d) BAN THRESHOLD EXCEEDED%s\n", __func__, state->name, pnode, state->nMisbehavior-howmuch, state->nMisbehavior, message_prefixed); - state->fShouldBan = true; + LogPrint(BCLog::NET, "%s: %s peer=%d (%d -> %d) DISCOURAGE THRESHOLD EXCEEDED%s\n", __func__, state->name, pnode, state->nMisbehavior-howmuch, state->nMisbehavior, message_prefixed); + state->m_should_discourage = true; statsClient.inc("misbehavior.banned", 1.0f); } else { LogPrint(BCLog::NET, "%s: %s peer=%d (%d -> %d)%s\n", __func__, state->name, pnode, state->nMisbehavior-howmuch, state->nMisbehavior, message_prefixed); @@ -1142,12 +1151,100 @@ bool IsBanned(NodeId pnode) CNodeState *state = State(pnode); if (state == nullptr) return false; - if (state->fShouldBan) { + if (state->m_should_discourage) { return true; } return false; } +/** + * Returns true if the given validation state result may result in a peer + * banning/disconnecting us. We use this to determine which unaccepted + * transactions from a whitelisted peer that we can safely relay. + */ +static bool TxRelayMayResultInDisconnect(const CValidationState& state) +{ + assert(IsTransactionReason(state.GetReason())); + return state.GetReason() == ValidationInvalidReason::CONSENSUS; +} + +/** + * Potentially mark a node discouraged based on the contents of a CValidationState object + * + * @param[in] via_compact_block: this bool is passed in because net_processing should + * punish peers differently depending on whether the data was provided in a compact + * block message or not. If the compact block had a valid header, but contained invalid + * txs, the peer should not be punished. See BIP 152. + * + * @return Returns true if the peer was punished (probably disconnected) + * + * Changes here may need to be reflected in TxRelayMayResultInDisconnect(). + */ +static bool MaybePunishNode(NodeId nodeid, const CValidationState& state, bool via_compact_block, const std::string& message = "") { + switch (state.GetReason()) { + case ValidationInvalidReason::NONE: + break; + // The node is providing invalid data: + case ValidationInvalidReason::CONSENSUS: + case ValidationInvalidReason::BLOCK_MUTATED: + if (!via_compact_block) { + LOCK(cs_main); + Misbehaving(nodeid, 100, message); + return true; + } + break; + case ValidationInvalidReason::CACHED_INVALID: + { + LOCK(cs_main); + CNodeState *node_state = State(nodeid); + if (node_state == nullptr) { + break; + } + + // Discourage 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: + case ValidationInvalidReason::BLOCK_CHAINLOCK: + case ValidationInvalidReason::TX_BAD_SPECIAL: + case ValidationInvalidReason::TX_CONFLICT_LOCK: + { + // TODO: Handle this much more gracefully (10 DoS points is super arbitrary) + LOCK(cs_main); + Misbehaving(nodeid, 10, message); + } + return true; + case ValidationInvalidReason::RECENT_CONSENSUS_CHANGE: + case ValidationInvalidReason::BLOCK_TIME_FUTURE: + case ValidationInvalidReason::TX_NOT_STANDARD: + case ValidationInvalidReason::TX_MISSING_INPUTS: + case ValidationInvalidReason::TX_PREMATURE_SPEND: + case ValidationInvalidReason::TX_CONFLICT: + case ValidationInvalidReason::TX_MEMPOOL_POLICY: + break; + } + if (message != "") { + LogPrint(BCLog::NET, "peer=%d: %s\n", nodeid, message); + } + return false; +} + + + @@ -1328,7 +1425,7 @@ void PeerLogicValidation::UpdatedBlockTip(const CBlockIndex *pindexNew, const CB } /** - * Handle invalid block rejection and consequent peer banning, maintain which + * Handle invalid block rejection and consequent peer discouragement, maintain which * peers announce compact blocks. */ void PeerLogicValidation::BlockChecked(const CBlock& block, const CValidationState& state) { @@ -1337,14 +1434,12 @@ void PeerLogicValidation::BlockChecked(const CBlock& block, const CValidationSta const uint256 hash(block.GetHash()); std::map >::iterator it = mapBlockSource.find(hash); - int nDoS = 0; - if (state.IsInvalid(nDoS)) { + if (state.IsInvalid()) { // Don't send reject message with code 0 or an internal reject code. if (it != mapBlockSource.end() && State(it->second.first) && state.GetRejectCode() > 0 && state.GetRejectCode() < REJECT_INTERNAL) { CBlockReject reject = {(unsigned char)state.GetRejectCode(), state.GetRejectReason().substr(0, MAX_REJECT_MESSAGE_LENGTH), hash}; State(it->second.first)->rejects.push_back(reject); - if (nDoS > 0 && it->second.second) - Misbehaving(it->second.first, nDoS); + MaybePunishNode(/*nodeid=*/ it->second.first, state, /*via_compact_block=*/ !it->second.second); } } // Check that: @@ -1878,7 +1973,7 @@ inline void static SendBlockTransactions(const CBlock& block, const BlockTransac connman->PushMessage(pfrom, msgMaker.Make(NetMsgType::BLOCKTXN, resp)); } -bool static ProcessHeadersMessage(CNode *pfrom, CConnman *connman, ChainstateManager& chainman, CTxMemPool& mempool, const std::vector& headers, const CChainParams& chainparams, bool punish_duplicate_invalid) +bool static ProcessHeadersMessage(CNode *pfrom, CConnman *connman, ChainstateManager& chainman, CTxMemPool& mempool, const std::vector& headers, const CChainParams& chainparams, bool via_compact_block) { const CNetMsgMaker msgMaker(pfrom->GetSendVersion()); size_t nCount = headers.size(); @@ -1942,48 +2037,8 @@ bool static ProcessHeadersMessage(CNode *pfrom, CConnman *connman, ChainstateMan CValidationState state; CBlockHeader first_invalid_header; if (!chainman.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; } } @@ -2119,13 +2174,13 @@ void static ProcessOrphanTx(CConnman* connman, CTxMemPool& mempool, std::setsecond.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 /* pfMissingInputs */, + if (AcceptToMemoryPool(mempool, orphan_state, porphanTx, &fMissingInputs2 /* pfMissingInputs */, false /* bypass_limits */, 0 /* nAbsurdFee */)) { LogPrint(BCLog::MEMPOOL, " accepted orphan tx %s\n", orphanHash.ToString()); RelayTransaction(orphanTx.GetHash(), *connman); @@ -2140,20 +2195,19 @@ void static ProcessOrphanTx(CConnman* connman, CTxMemPool& mempool, std::set 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 (!stateDummy.CorruptionPossible()) { - assert(recentRejects); - recentRejects->insert(orphanHash); - } + assert(IsTransactionReason(orphan_state.GetReason())); + assert(recentRejects); + recentRejects->insert(orphanHash); EraseOrphanTx(orphanHash); done = true; } @@ -2838,7 +2892,8 @@ bool ProcessMessage(CNode* pfrom, const std::string& msg_type, CDataStream& vRec if (addr.nTime <= 100000000 || addr.nTime > nNow + 10 * 60) addr.nTime = nNow - 5 * 24 * 60 * 60; pfrom->AddAddressKnown(addr); - if (banman->IsBanned(addr)) continue; // Do not process banned addresses beyond remembering we received them + if (banman->IsDiscouraged(addr)) continue; // Do not process banned/discouraged addresses beyond remembering we received them + if (banman->IsBanned(addr)) continue; bool fReachable = IsReachable(addr); if (addr.nTime > nSince && !pfrom->fGetAddr && vAddr.size() <= 10 && addr.IsRoutable()) { @@ -3329,12 +3384,11 @@ bool ProcessMessage(CNode* pfrom, const std::string& msg_type, CDataStream& vRec llmq::quorumInstantSendManager->TransactionRemovedFromMempool(ptx); } } else { - if (!state.CorruptionPossible()) { - assert(recentRejects); - recentRejects->insert(tx.GetHash()); - if (RecursiveDynamicUsage(*ptx) < 100000) { - AddToCompactExtraTransactions(ptx); - } + assert(IsTransactionReason(state.GetReason())); + assert(recentRejects); + recentRejects->insert(tx.GetHash()); + if (RecursiveDynamicUsage(*ptx) < 100000) { + AddToCompactExtraTransactions(ptx); } if (pfrom->HasPermission(PF_FORCERELAY)) { @@ -3343,15 +3397,13 @@ bool ProcessMessage(CNode* pfrom, const std::string& msg_type, CDataStream& vRec // 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.GetHash(), *connman); - } else { - LogPrintf("Not relaying invalid transaction %s from whitelisted peer=%d (%s)\n", tx.GetHash().ToString(), pfrom->GetId(), FormatStateMessage(state)); } } llmq::quorumInstantSendManager->TransactionRemovedFromMempool(ptx); @@ -3374,8 +3426,7 @@ bool ProcessMessage(CNode* pfrom, const std::string& msg_type, CDataStream& vRec // 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(), @@ -3384,9 +3435,7 @@ bool ProcessMessage(CNode* pfrom, const std::string& msg_type, CDataStream& vRec connman->PushMessage(pfrom, msgMaker.Make(NetMsgType::REJECT, msg_type, (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; } @@ -3422,14 +3471,8 @@ bool ProcessMessage(CNode* pfrom, const std::string& msg_type, CDataStream& vRec const CBlockIndex *pindex = nullptr; CValidationState state; if (!chainman.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", 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; } } @@ -3572,8 +3615,8 @@ bool ProcessMessage(CNode* pfrom, const std::string& msg_type, CDataStream& vRec // relayed before full validation (see BIP 152), so we don't want to disconnect // 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, chainman, mempool, {cmpctblock.header}, chainparams, /*punish_duplicate_invalid=*/false); + // will be detected and the peer will be disconnected/discouraged. + return ProcessHeadersMessage(pfrom, connman, chainman, mempool, {cmpctblock.header}, chainparams, /*punish_duplicate_invalid=*/true); } if (fBlockReconstructed) { @@ -3658,7 +3701,7 @@ bool ProcessMessage(CNode* pfrom, const std::string& msg_type, CDataStream& vRec // 3. the block is otherwise invalid (eg invalid coinbase, // block is too big, too many legacy sigops, etc). // So if CheckBlock failed, #3 is the only possibility. - // Under BIP 152, we don't DoS-ban unless proof of work is + // Under BIP 152, we don't discourage the peer unless proof of work is // invalid (we don't require all the stateless checks to have // been run). This is handled below, so just treat this as // though the block was successfully read, and rely on the @@ -3726,12 +3769,7 @@ bool ProcessMessage(CNode* pfrom, const std::string& msg_type, CDataStream& vRec } } - // 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, chainman, mempool, headers, chainparams, should_punish); + return ProcessHeadersMessage(pfrom, connman, chainman, mempool, headers, chainparams, /*via_compact_block=*/false); } if (msg_type == NetMsgType::BLOCK) @@ -3796,7 +3834,7 @@ bool ProcessMessage(CNode* pfrom, const std::string& msg_type, CDataStream& vRec std::vector vAddr = connman->GetAddresses(); FastRandomContext insecure_rand; for (const CAddress &addr : vAddr) { - if (!banman->IsBanned(addr)) { + if (!banman->IsDiscouraged(addr) && !banman->IsBanned(addr)) { pfrom->PushAddress(addr, insecure_rand); } } @@ -4089,7 +4127,7 @@ bool ProcessMessage(CNode* pfrom, const std::string& msg_type, CDataStream& vRec return true; } -bool PeerLogicValidation::SendRejectsAndCheckIfBanned(CNode* pnode, bool enable_bip61) EXCLUSIVE_LOCKS_REQUIRED(cs_main) +bool PeerLogicValidation::MaybeDiscourageAndDisconnect(CNode* pnode, bool enable_bip61) { AssertLockHeld(cs_main); CNodeState &state = *State(pnode->GetId()); @@ -4101,20 +4139,21 @@ bool PeerLogicValidation::SendRejectsAndCheckIfBanned(CNode* pnode, bool enable_ } state.rejects.clear(); - if (state.fShouldBan) { - state.fShouldBan = false; - if (pnode->HasPermission(PF_NOBAN)) + if (state.m_should_discourage) { + state.m_should_discourage = false; + if (pnode->HasPermission(PF_NOBAN)) { LogPrintf("Warning: not punishing whitelisted peer %s!\n", pnode->GetLogString()); - else if (pnode->m_manual_connection) - LogPrintf("Warning: not punishing manually-connected peer %s!\n", pnode->GetLogString()); - else if (pnode->addr.IsLocal()) { - // Disconnect but don't ban _this_ local node - LogPrintf("Warning: disconnecting but not banning local peer %s!\n", pnode->GetLogString()); + } else if (pnode->m_manual_connection) { + LogPrintf("Warning: not punishing manually-connected peer %s!\n", pnode->addr.ToString()); + } else if (pnode->addr.IsLocal()) { + // Disconnect but don't discourage this local node + LogPrintf("Warning: disconnecting but not discouraging local peer %s!\n", pnode->GetLogString()); pnode->fDisconnect = true; } else { - // Disconnect and ban all nodes sharing the address + // Disconnect and discourage all nodes sharing the address + LogPrintf("Disconnecting and discouraging peer %s!\n", pnode->addr.ToString()); if (m_banman) { - m_banman->Ban(pnode->addr, BanReasonNodeMisbehaving); + m_banman->Discourage(pnode->addr); } connman->DisconnectNode(pnode->addr); } @@ -4239,7 +4278,7 @@ bool PeerLogicValidation::ProcessMessages(CNode* pfrom, std::atomic& inter } LOCK(cs_main); - SendRejectsAndCheckIfBanned(pfrom, m_enable_bip61); + MaybeDiscourageAndDisconnect(pfrom, m_enable_bip61); return fMoreWork; } @@ -4443,7 +4482,8 @@ bool PeerLogicValidation::SendMessages(CNode* pto) if (!lockMain) return true; - if (SendRejectsAndCheckIfBanned(pto, m_enable_bip61)) return true; + if (MaybeDiscourageAndDisconnect(pto, m_enable_bip61)) return true; + CNodeState &state = *State(pto->GetId()); // Address refresh broadcast diff --git a/src/net_processing.h b/src/net_processing.h index c50400041182..aecd9c0dccea 100644 --- a/src/net_processing.h +++ b/src/net_processing.h @@ -32,7 +32,7 @@ class PeerLogicValidation final : public CValidationInterface, public NetEventsI ChainstateManager& m_chainman; CTxMemPool& m_mempool; - bool SendRejectsAndCheckIfBanned(CNode* pnode, bool enable_bip61) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + bool MaybeDiscourageAndDisconnect(CNode* pnode, bool enable_bip61) EXCLUSIVE_LOCKS_REQUIRED(cs_main); public: PeerLogicValidation(CConnman* connmanIn, BanMan* banman, CScheduler &scheduler, ChainstateManager& chainman, CTxMemPool& pool, bool enable_bip61); diff --git a/src/qt/rpcconsole.cpp b/src/qt/rpcconsole.cpp index d13489212c6b..aef53f0236a4 100644 --- a/src/qt/rpcconsole.cpp +++ b/src/qt/rpcconsole.cpp @@ -1360,7 +1360,7 @@ void RPCConsole::banSelectedNode(int bantime) // Find possible nodes, ban it and clear the selected node const CNodeCombinedStats *stats = clientModel->getPeerTableModel()->getNodeStats(detailNodeRow); if (stats) { - m_node.ban(stats->nodeStats.addr, BanReasonManuallyAdded, bantime); + m_node.ban(stats->nodeStats.addr, bantime); m_node.disconnect(stats->nodeStats.addr); } } diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index 4567ae401ff6..848cc68b519b 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -641,12 +641,12 @@ static UniValue setban(const JSONRPCRequest& request) absolute = true; if (isSubnet) { - node.banman->Ban(subNet, BanReasonManuallyAdded, banTime, absolute); + node.banman->Ban(subNet, banTime, absolute); if (node.connman) { node.connman->DisconnectNode(subNet); } } else { - node.banman->Ban(netAddr, BanReasonManuallyAdded, banTime, absolute); + node.banman->Ban(netAddr, banTime, absolute); if (node.connman) { node.connman->DisconnectNode(netAddr); } @@ -655,7 +655,7 @@ static UniValue setban(const JSONRPCRequest& request) else if(strCommand == "remove") { if (!( isSubnet ? node.banman->Unban(subNet) : node.banman->Unban(netAddr) )) { - throw JSONRPCError(RPC_CLIENT_INVALID_IP_OR_SUBNET, "Error: Unban failed. Requested address/subnet was not previously banned."); + throw JSONRPCError(RPC_CLIENT_INVALID_IP_OR_SUBNET, "Error: Unban failed. Requested address/subnet was not previously manually banned."); } } return NullUniValue; @@ -664,7 +664,7 @@ static UniValue setban(const JSONRPCRequest& request) static UniValue listbanned(const JSONRPCRequest& request) { RPCHelpMan{"listbanned", - "\nList all banned IPs/Subnets.\n", + "\nList all manually banned IPs/Subnets.\n", {}, RPCResult{RPCResult::Type::ARR, "", "", { @@ -673,7 +673,6 @@ static UniValue listbanned(const JSONRPCRequest& request) {RPCResult::Type::STR, "address", ""}, {RPCResult::Type::NUM_TIME, "banned_until", ""}, {RPCResult::Type::NUM_TIME, "ban_created", ""}, - {RPCResult::Type::STR, "ban_reason", ""}, }}, }}, RPCExamples{ @@ -698,7 +697,6 @@ static UniValue listbanned(const JSONRPCRequest& request) rec.pushKV("address", entry.first.ToString()); rec.pushKV("banned_until", banEntry.nBanUntil); rec.pushKV("ban_created", banEntry.nCreateTime); - rec.pushKV("ban_reason", banEntry.banReasonToString()); bannedAddresses.push_back(rec); } diff --git a/src/test/denialofservice_tests.cpp b/src/test/denialofservice_tests.cpp index aa98834f5968..8f663e79d312 100644 --- a/src/test/denialofservice_tests.cpp +++ b/src/test/denialofservice_tests.cpp @@ -238,8 +238,8 @@ BOOST_AUTO_TEST_CASE(DoS_banning) LOCK(dummyNode1.cs_sendProcessing); BOOST_CHECK(peerLogic->SendMessages(&dummyNode1)); } - BOOST_CHECK(banman->IsBanned(addr1)); - BOOST_CHECK(!banman->IsBanned(ip(0xa0b0c001|0x0000ff00))); // Different IP, not banned + BOOST_CHECK(banman->IsDiscouraged(addr1)); + BOOST_CHECK(!banman->IsDiscouraged(ip(0xa0b0c001|0x0000ff00))); // Different IP, not banned CAddress addr2(ip(0xa0b0c002), NODE_NONE); CNode dummyNode2(id++, NODE_NETWORK, 0, INVALID_SOCKET, addr2, 1, 1, CAddress(), "", true); @@ -255,8 +255,8 @@ BOOST_AUTO_TEST_CASE(DoS_banning) LOCK(dummyNode2.cs_sendProcessing); BOOST_CHECK(peerLogic->SendMessages(&dummyNode2)); } - BOOST_CHECK(!banman->IsBanned(addr2)); // 2 not banned yet... - BOOST_CHECK(banman->IsBanned(addr1)); // ... but 1 still should be + BOOST_CHECK(!banman->IsDiscouraged(addr2)); // 2 not banned yet... + BOOST_CHECK(banman->IsDiscouraged(addr1)); // ... but 1 still should be { LOCK(cs_main); Misbehaving(dummyNode2.GetId(), 50); @@ -265,7 +265,7 @@ BOOST_AUTO_TEST_CASE(DoS_banning) LOCK(dummyNode2.cs_sendProcessing); BOOST_CHECK(peerLogic->SendMessages(&dummyNode2)); } - BOOST_CHECK(banman->IsBanned(addr2)); + BOOST_CHECK(banman->IsDiscouraged(addr2)); bool dummy; peerLogic->FinalizeNode(dummyNode1.GetId(), dummy); @@ -294,7 +294,7 @@ BOOST_AUTO_TEST_CASE(DoS_banscore) LOCK2(cs_main, dummyNode1.cs_sendProcessing); BOOST_CHECK(peerLogic->SendMessages(&dummyNode1)); } - BOOST_CHECK(!banman->IsBanned(addr1)); + BOOST_CHECK(!banman->IsDiscouraged(addr1)); { LOCK(cs_main); Misbehaving(dummyNode1.GetId(), 10); @@ -303,7 +303,7 @@ BOOST_AUTO_TEST_CASE(DoS_banscore) LOCK2(cs_main, dummyNode1.cs_sendProcessing); BOOST_CHECK(peerLogic->SendMessages(&dummyNode1)); } - BOOST_CHECK(!banman->IsBanned(addr1)); + BOOST_CHECK(!banman->IsDiscouraged(addr1)); { LOCK(cs_main); Misbehaving(dummyNode1.GetId(), 1); @@ -312,7 +312,7 @@ BOOST_AUTO_TEST_CASE(DoS_banscore) LOCK2(cs_main, dummyNode1.cs_sendProcessing); BOOST_CHECK(peerLogic->SendMessages(&dummyNode1)); } - BOOST_CHECK(banman->IsBanned(addr1)); + BOOST_CHECK(banman->IsDiscouraged(addr1)); gArgs.ForceSetArg("-banscore", std::to_string(DEFAULT_BANSCORE_THRESHOLD)); bool dummy; @@ -344,13 +344,7 @@ BOOST_AUTO_TEST_CASE(DoS_bantime) LOCK(dummyNode.cs_sendProcessing); BOOST_CHECK(peerLogic->SendMessages(&dummyNode)); } - BOOST_CHECK(banman->IsBanned(addr)); - - SetMockTime(nStartTime+60*60); - BOOST_CHECK(banman->IsBanned(addr)); - - SetMockTime(nStartTime+60*60*24+1); - BOOST_CHECK(!banman->IsBanned(addr)); + BOOST_CHECK(banman->IsDiscouraged(addr)); bool dummy; peerLogic->FinalizeNode(dummyNode.GetId(), dummy); diff --git a/src/test/specialtx_tests.cpp b/src/test/specialtx_tests.cpp index 250a7491deeb..11f330c5eb00 100644 --- a/src/test/specialtx_tests.cpp +++ b/src/test/specialtx_tests.cpp @@ -21,11 +21,11 @@ bool VerifyMNHFTx(const CTransaction& tx, CValidationState& state) { MNHFTxPayload mnhfTx; if (!GetTxPayload(tx, mnhfTx)) { - return state.DoS(100, false, REJECT_INVALID, "bad-mnhf-payload"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-mnhf-payload"); } if (mnhfTx.nVersion == 0 || mnhfTx.nVersion > MNHFTxPayload::CURRENT_VERSION) { - return state.DoS(100, false, REJECT_INVALID, "bad-mnhf-version"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-mnhf-version"); } return true; diff --git a/src/test/txvalidation_tests.cpp b/src/test/txvalidation_tests.cpp index 30d611be6e7c..7c597fe2b62c 100644 --- a/src/test/txvalidation_tests.cpp +++ b/src/test/txvalidation_tests.cpp @@ -49,10 +49,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 6f85e024ebc6..600522923129 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -372,18 +372,18 @@ bool ContextualCheckTransaction(const CTransaction& tx, CValidationState &state, tx.nType != TRANSACTION_COINBASE && tx.nType != TRANSACTION_QUORUM_COMMITMENT && tx.nType != TRANSACTION_MNHF_SIGNAL) { - return state.DoS(100, false, REJECT_INVALID, "bad-txns-type"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-txns-type"); } if (tx.IsCoinBase() && tx.nType != TRANSACTION_COINBASE) - return state.DoS(100, false, REJECT_INVALID, "bad-txns-cb-type"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-txns-cb-type"); } else if (tx.nType != TRANSACTION_NORMAL) { - return state.DoS(100, false, REJECT_INVALID, "bad-txns-type"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-txns-type"); } } // Size limits if (fDIP0001Active_context && ::GetSerializeSize(tx, PROTOCOL_VERSION) > MAX_STANDARD_TX_SIZE) - return state.DoS(100, false, REJECT_INVALID, "bad-txns-oversize"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-txns-oversize"); return true; } @@ -534,35 +534,35 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool if (tx.nVersion == 3 && tx.nType == TRANSACTION_QUORUM_COMMITMENT) { // quorum commitment is not allowed outside of blocks - return state.DoS(100, false, REJECT_INVALID, "qc-not-allowed"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "qc-not-allowed"); } // 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 empty scriptSig input and 1 P2SH output has size of 83 bytes. // Transactions smaller than this are not relayed to mitigate CVE-2017-12842 by not relaying // 64-byte transactions. if (::GetSerializeSize(tx, PROTOCOL_VERSION) < MIN_STANDARD_TX_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)) { statsClient.inc("transactions.duplicate", 1.0f); - return state.Invalid(false, REJECT_DUPLICATE, "txn-already-in-mempool"); + return state.Invalid(ValidationInvalidReason::TX_CONFLICT, false, REJECT_DUPLICATE, "txn-already-in-mempool"); } llmq::CInstantSendLockPtr conflictLock = llmq::quorumInstantSendManager->GetConflictingLock(tx); @@ -572,9 +572,7 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool if (txConflict) { GetMainSignals().NotifyInstantSendDoubleSpendAttempt(ptx, txConflict); } - return state.DoS(10, error("AcceptToMemoryPool : Transaction %s conflicts with locked TX %s", - hash.ToString(), conflictLock->txid.ToString()), - REJECT_INVALID, "tx-txlock-conflict"); + return state.Invalid(ValidationInvalidReason::TX_CONFLICT_LOCK, error("AcceptToMemoryPool : Transaction %s conflicts with locked TX %s", hash.ToString(), conflictLock->txid.ToString()), REJECT_INVALID, "tx-txlock-conflict"); } if (llmq::quorumInstantSendManager->IsWaitingForTx(hash)) { @@ -588,7 +586,7 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool if (ptxConflicting) { // Transaction conflicts with mempool and RBF doesn't exist in Dash - return state.Invalid(false, REJECT_DUPLICATE, "txn-mempool-conflict"); + return state.Invalid(ValidationInvalidReason::TX_CONFLICT, false, REJECT_DUPLICATE, "txn-mempool-conflict"); } } } @@ -616,7 +614,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 (coins_cache.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 @@ -639,7 +637,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)) { @@ -648,7 +646,7 @@ 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"); unsigned int nSigOps = GetTransactionSigOpCount(tx, view, STANDARD_SCRIPT_VERIFY_FLAGS); @@ -671,27 +669,22 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool fSpendsCoinbase, nSigOps, 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 (nSigOps > MAX_STANDARD_TX_SIGOPS) - 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", nSigOps)); 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)); @@ -718,7 +711,7 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool // this, see https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2018-November/016518.html if (nSize > EXTRA_DESCENDANT_TX_SIZE_LIMIT || !pool.CalculateMemPoolAncestors(entry, setAncestors, 2, nLimitAncestorSize, nLimitDescendants + 1, nLimitDescendantSize + EXTRA_DESCENDANT_TX_SIZE_LIMIT, dummy_err_string)) { - 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); } } @@ -730,7 +723,7 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool return false; if (pool.existsProviderTxConflict(tx)) { - return state.DoS(0, false, REJECT_DUPLICATE, "protx-dup"); + return state.Invalid(ValidationInvalidReason::TX_CONFLICT, false, REJECT_DUPLICATE, "protx-dup"); } constexpr unsigned int scriptVerifyFlags = STANDARD_SCRIPT_VERIFY_FLAGS; @@ -738,8 +731,10 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool // Check against previous transactions // This is done last to help prevent CPU exhaustion denial-of-service attacks. PrecomputedTransactionData txdata; - if (!CheckInputs(tx, state, view, true, scriptVerifyFlags, true, false, txdata)) + if (!CheckInputs(tx, state, view, true, scriptVerifyFlags, true, false, txdata)) { + assert(IsTransactionReason(state.GetReason())); return false; // state filled in by CheckInputs + } // Check again against the current block tip's script verification // flags to cache our script execution flags. This is, of course, @@ -796,7 +791,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"); } } @@ -1323,7 +1318,7 @@ void static ConflictingChainFound(CBlockIndex* pindexNew) EXCLUSIVE_LOCKS_REQUIR void CChainState::InvalidBlockFound(CBlockIndex *pindex, const CValidationState &state) { statsClient.inc("warnings.InvalidBlockFound", 1.0f); - if (!state.CorruptionPossible()) { + if (state.GetReason() != ValidationInvalidReason::BLOCK_MUTATED) { pindex->nStatus |= BLOCK_FAILED_VALID; m_blockman.m_failed_blocks.insert(pindex); setDirtyBlockIndex.insert(pindex); @@ -1398,6 +1393,9 @@ void InitScriptExecutionCache() { * which are matched. This is useful for checking blocks where we will likely never need the cache * entry again. * + * Note that we may set state.reason to NOT_STANDARD for extra soft-fork flags in flags, block-checking + * callers should probably reset it to CONSENSUS in such cases. + * * Non-static (and re-declared) in src/test/txvalidationcache_tests.cpp */ bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsViewCache &inputs, bool fScriptChecks, unsigned int flags, bool cacheSigStore, bool cacheFullScriptStore, PrecomputedTransactionData& txdata, std::vector *pvChecks) EXCLUSIVE_LOCKS_REQUIRED(cs_main) @@ -1459,22 +1457,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) | SCRIPT_ENABLE_DIP0020_OPCODES, 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()))); } } @@ -1982,7 +1984,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. @@ -1992,7 +1994,7 @@ bool CChainState::ConnectBlock(const CBlock& block, CValidationState& state, CBl } if (pindex->pprev && pindex->phashBlock && llmq::chainLocksHandler->HasConflictingChainLock(pindex->nHeight, pindex->GetBlockHash())) { - return state.DoS(10, error("%s: conflicting with chainlock", __func__), REJECT_INVALID, "bad-chainlock"); + return state.Invalid(ValidationInvalidReason::BLOCK_CHAINLOCK, error("%s: conflicting with chainlock", __func__), REJECT_INVALID, "bad-chainlock"); } // verify that the view's current state corresponds to the previous block @@ -2076,8 +2078,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"), - REJECT_INVALID, "bad-txns-BIP30"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, error("ConnectBlock(): tried to overwrite transaction"), REJECT_INVALID, "bad-txns-BIP30"); } } } @@ -2088,10 +2089,9 @@ bool CChainState::ConnectBlock(const CBlock& block, CValidationState& state, CBl // make sure old budget is the real one if (pindex->nHeight == chainparams.GetConsensus().nSuperblockStartBlock && chainparams.GetConsensus().nSuperblockStartHash != uint256() && - block.GetHash() != chainparams.GetConsensus().nSuperblockStartHash) - return state.DoS(100, error("ConnectBlock(): invalid superblock start"), - REJECT_INVALID, "bad-sb-start"); - + block.GetHash() != chainparams.GetConsensus().nSuperblockStartHash) { + return state.Invalid(ValidationInvalidReason::CONSENSUS, error("ConnectBlock(): invalid superblock start"), REJECT_INVALID, "bad-sb-start"); + } /// END DASH // Start enforcing BIP68 (sequence locks) and BIP112 (CHECKSEQUENCEVERIFY) using versionbits logic. @@ -2147,12 +2147,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__), - REJECT_INVALID, "bad-txns-accumulated-fee-outofrange"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, error("%s: accumulated fee in the block out of range.", __func__), REJECT_INVALID, "bad-txns-accumulated-fee-outofrange"); } // Check that transaction is BIP68 final @@ -2164,8 +2171,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__), - REJECT_INVALID, "bad-txns-nonfinal"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, error("%s: contains a non-BIP68-final transaction", __func__), REJECT_INVALID, "bad-txns-nonfinal"); } if (fAddressIndex || fSpentIndex) @@ -2214,9 +2220,9 @@ bool CChainState::ConnectBlock(const CBlock& block, CValidationState& state, CBl // * legacy (always) // * p2sh (when P2SH enabled in flags and excludes coinbase) nSigOps += GetTransactionSigOpCount(tx, view, flags); - if (nSigOps > MaxBlockSigOps(fDIP0001Active_context)) - return state.DoS(100, error("ConnectBlock(): too many sigops"), - REJECT_INVALID, "bad-blk-sigops"); + if (nSigOps > MaxBlockSigOps(fDIP0001Active_context)) { + return state.Invalid(ValidationInvalidReason::CONSENSUS, error("ConnectBlock(): too many sigops"), REJECT_INVALID, "bad-blk-sigops"); + } if (!tx.IsCoinBase()) { @@ -2224,6 +2230,16 @@ bool CChainState::ConnectBlock(const CBlock& block, CValidationState& state, CBl std::vector vChecks; bool fCacheResults = fJustCheck; /* Don't cache results if we're actually connecting blocks (still consult the cache, though) */ if (fScriptChecks && !CheckInputs(tx, state, view, fScriptChecks, flags, fCacheResults, fCacheResults, txsdata[i], g_parallel_script_checks ? &vChecks : nullptr)) { + if (state.GetReason() == ValidationInvalidReason::TX_NOT_STANDARD) { + // CheckInputs may return NOT_STANDARD for extra flags we passed, + // but we can't return that, as it's not defined for a block, so + // we reset the reason flag to CONSENSUS here. + // In the event of a future soft-fork, we may need to + // consider whether rewriting to CONSENSUS or + // RECENT_CONSENSUS_CHANGE would be more appropriate. + state.Invalid(ValidationInvalidReason::CONSENSUS, false, + state.GetRejectCode(), state.GetRejectReason(), state.GetDebugMessage()); + } return error("ConnectBlock(): CheckInputs on %s failed with %s", tx.GetHash().ToString(), FormatStateMessage(state)); } @@ -2273,7 +2289,7 @@ bool CChainState::ConnectBlock(const CBlock& block, CValidationState& state, CBl LogPrint(BCLog::BENCHMARK, " - Connect %u transactions: %.2fms (%.3fms/tx, %.3fms/txin) [%.2fs (%.2fms/blk)]\n", (unsigned)block.vtx.size(), MILLI * (nTime3 - nTime2), MILLI * (nTime3 - nTime2) / block.vtx.size(), nInputs <= 1 ? 0 : MILLI * (nTime3 - nTime2) / (nInputs-1), nTimeConnect * MICRO, nTimeConnect * MILLI / nBlocksTotal); 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::BENCHMARK, " - 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); @@ -2301,8 +2317,7 @@ bool CChainState::ConnectBlock(const CBlock& block, CValidationState& state, CBl } else { // The node which relayed this should switch to correct chain. // TODO: relay instantsend data/proof. - return state.DoS(10, error("ConnectBlock(DASH): transaction %s conflicts with transaction lock %s", - tx->GetHash().ToString(), conflictLock->txid.ToString()), REJECT_INVALID, "conflict-tx-lock"); + return state.Invalid(ValidationInvalidReason::TX_CONFLICT_LOCK, error("ConnectBlock(DASH): transaction %s conflicts with transaction lock %s", tx->GetHash().ToString(), conflictLock->txid.ToString()), REJECT_INVALID, "conflict-tx-lock"); } } } @@ -2321,15 +2336,16 @@ bool CChainState::ConnectBlock(const CBlock& block, CValidationState& state, CBl LogPrint(BCLog::BENCHMARK, " - GetBlockSubsidy: %.2fms [%.2fs (%.2fms/blk)]\n", MILLI * (nTime5_2 - nTime5_1), nTimeSubsidy * MICRO, nTimeSubsidy * MILLI / nBlocksTotal); if (!IsBlockValueValid(block, pindex->nHeight, blockReward, strError)) { - return state.DoS(0, error("ConnectBlock(DASH): %s", strError), REJECT_INVALID, "bad-cb-amount"); + // NOTE: Do not punish, the node might be missing governance data + return state.Invalid(ValidationInvalidReason::NONE, error("ConnectBlock(DASH): %s", strError), REJECT_INVALID, "bad-cb-amount"); } int64_t nTime5_3 = GetTimeMicros(); nTimeValueValid += nTime5_3 - nTime5_2; LogPrint(BCLog::BENCHMARK, " - IsBlockValueValid: %.2fms [%.2fs (%.2fms/blk)]\n", MILLI * (nTime5_3 - nTime5_2), nTimeValueValid * MICRO, nTimeValueValid * MILLI / nBlocksTotal); if (!IsBlockPayeeValid(*block.vtx[0], pindex->nHeight, blockReward)) { - return state.DoS(0, error("ConnectBlock(DASH): couldn't find masternode or superblock payments"), - REJECT_INVALID, "bad-cb-payee"); + // NOTE: Do not punish, the node might be missing governance data + return state.Invalid(ValidationInvalidReason::NONE, error("ConnectBlock(DASH): couldn't find masternode or superblock payments"), REJECT_INVALID, "bad-cb-payee"); } int64_t nTime5_4 = GetTimeMicros(); nTimePayeeValid += nTime5_4 - nTime5_3; @@ -2973,7 +2989,7 @@ bool CChainState::ActivateBestChainStep(CValidationState& state, const CChainPar if (!ConnectTip(state, chainparams, pindexConnect, pindexConnect == pindexMostWork ? pblock : std::shared_ptr(), connectTrace, disconnectpool)) { if (state.IsInvalid()) { // The block violates a consensus rule. - if (!state.CorruptionPossible()) { + if (state.GetReason() != ValidationInvalidReason::BLOCK_MUTATED) { InvalidChainFound(vpindexToConnect.front()); } state = CValidationState(); @@ -3688,14 +3704,13 @@ 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"); // Check DevNet if (!consensusParams.hashDevnetGenesisBlock.IsNull() && block.hashPrevBlock == consensusParams.hashGenesisBlock && block.GetHash() != consensusParams.hashDevnetGenesisBlock) { - return state.DoS(100, error("CheckBlockHeader(): wrong devnet genesis"), - REJECT_INVALID, "devnet-genesis"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, error("CheckBlockHeader(): wrong devnet genesis"), REJECT_INVALID, "devnet-genesis"); } return true; @@ -3720,13 +3735,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 @@ -3735,19 +3750,19 @@ bool CheckBlock(const CBlock& block, CValidationState& state, const Consensus::P // Size limits (relaxed) if (block.vtx.empty() || block.vtx.size() > MaxBlockSize() || ::GetSerializeSize(block, PROTOCOL_VERSION) > MaxBlockSize()) - 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)) - return state.Invalid(false, state.GetRejectCode(), state.GetRejectReason(), + return state.Invalid(state.GetReason(), false, state.GetRejectCode(), state.GetRejectReason(), strprintf("Transaction check failed (tx hash %s) %s", tx->GetHash().ToString(), state.GetDebugMessage())); unsigned int nSigOps = 0; @@ -3757,7 +3772,7 @@ bool CheckBlock(const CBlock& block, CValidationState& state, const Consensus::P } // sigops limits (relaxed) if (nSigOps > MaxBlockSigOps()) - 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; @@ -3808,11 +3823,11 @@ static bool ContextualCheckBlockHeader(const CBlockHeader& block, CValidationSta double n2 = ConvertBitsToDouble(nBitsNext); if (abs(n1-n2) > n1*0.5) - return state.DoS(100, error("%s : incorrect proof of work (DGW pre-fork) - %f %f %f at %d", __func__, abs(n1-n2), n1, n2, nHeight), + return state.Invalid(ValidationInvalidReason::BLOCK_INVALID_HEADER, error("%s : incorrect proof of work (DGW pre-fork) - %f %f %f at %d", __func__, abs(n1-n2), n1, n2, nHeight), REJECT_INVALID, "bad-diffbits"); } else { if (block.nBits != GetNextWorkRequired(pindexPrev, &block, consensusParams)) - return state.DoS(100, false, REJECT_INVALID, "bad-diffbits", false, strprintf("incorrect proof of work at %d", nHeight)); + return state.Invalid(ValidationInvalidReason::BLOCK_INVALID_HEADER, false, REJECT_INVALID, "bad-diffbits", strprintf("incorrect proof of work at %d", nHeight)); } // Check against checkpoints @@ -3822,23 +3837,22 @@ static bool ContextualCheckBlockHeader(const CBlockHeader& block, CValidationSta // BlockIndex(). CBlockIndex* pcheckpoint = GetLastCheckpoint(params.Checkpoints()); if (pcheckpoint && nHeight < pcheckpoint->nHeight) - return state.DoS(100, error("%s: forked chain older than last checkpoint (height %d)", __func__, nHeight), REJECT_CHECKPOINT, "bad-fork-prior-to-checkpoint"); + return state.Invalid(ValidationInvalidReason::BLOCK_CHECKPOINT, error("%s: forked chain older than last checkpoint (height %d)", __func__, nHeight), REJECT_CHECKPOINT, "bad-fork-prior-to-checkpoint"); } // Check timestamp against prev if (block.GetBlockTime() <= pindexPrev->GetMedianTimePast()) - return state.Invalid(false, REJECT_INVALID, "time-too-old", strprintf("block's timestamp is too early %d %d", block.GetBlockTime(), pindexPrev->GetMedianTimePast())); + return state.Invalid(ValidationInvalidReason::BLOCK_INVALID_HEADER, false, REJECT_INVALID, "time-too-old", strprintf("block's timestamp is too early %d %d", block.GetBlockTime(), pindexPrev->GetMedianTimePast())); // Check timestamp if (block.GetBlockTime() > nAdjustedTime + MAX_FUTURE_BLOCK_TIME) - return state.Invalid(false, REJECT_INVALID, "time-too-new", strprintf("block timestamp too far in the future %d %d", block.GetBlockTime(), nAdjustedTime + 2 * 60 * 60)); + return state.Invalid(ValidationInvalidReason::BLOCK_TIME_FUTURE, false, REJECT_INVALID, "time-too-new", strprintf("block timestamp too far in the future %d %d", block.GetBlockTime(), nAdjustedTime + 2 * 60 * 60)); // 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), - strprintf("rejected nVersion=0x%08x block", block.nVersion)); + return state.Invalid(ValidationInvalidReason::BLOCK_INVALID_HEADER, false, REJECT_OBSOLETE, strprintf("bad-version(0x%08x)", block.nVersion), strprintf("rejected nVersion=0x%08x block", block.nVersion)); return true; } @@ -3871,14 +3885,14 @@ static bool ContextualCheckBlock(const CBlock& block, CValidationState& state, c // Size limits unsigned int nMaxBlockSize = MaxBlockSize(fDIP0001Active_context); if (block.vtx.empty() || block.vtx.size() > nMaxBlockSize || ::GetSerializeSize(block, PROTOCOL_VERSION) > nMaxBlockSize) - 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"); // Check that all transactions are finalized and not over-sized // Also count sigops unsigned int nSigOps = 0; 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"); } if (!ContextualCheckTransaction(*tx, state, consensusParams, pindexPrev)) { return false; @@ -3888,7 +3902,7 @@ static bool ContextualCheckBlock(const CBlock& block, CValidationState& state, c // Check sigops if (nSigOps > MaxBlockSigOps(fDIP0001Active_context)) - 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"); // Enforce rule that the coinbase starts with serialized block height // After DIP3/DIP4 activation, we don't enforce the height in the input script anymore. @@ -3898,13 +3912,13 @@ 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"); } } if (fDIP0003Active_context) { if (block.vtx[0]->nType != TRANSACTION_COINBASE) { - return state.DoS(100, false, REJECT_INVALID, "bad-cb-type", false, "coinbase is not a CbTx"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-cb-type", "coinbase is not a CbTx"); } } @@ -3927,9 +3941,9 @@ bool BlockManager::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"); if (pindex->nStatus & BLOCK_CONFLICT_CHAINLOCK) - return state.Invalid(error("%s: block %s is marked conflicting", __func__, hash.ToString()), 0, "duplicate"); + return state.Invalid(ValidationInvalidReason::BLOCK_CHAINLOCK, error("%s: block %s is marked conflicting", __func__, hash.ToString()), 0, "duplicate"); return true; } @@ -3940,16 +3954,16 @@ bool BlockManager::AcceptBlockHeader(const CBlockHeader& block, CValidationState CBlockIndex* pindexPrev = nullptr; BlockMap::iterator mi = m_block_index.find(block.hashPrevBlock); if (mi == m_block_index.end()) - return state.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; assert(pindexPrev); 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 (pindexPrev->nStatus & BLOCK_CONFLICT_CHAINLOCK) // it's ok-ish, the other node is probably missing the latest chainlock - return state.DoS(10, error("%s: prev block %s conflicts with chainlock", __func__, block.hashPrevBlock.ToString()), REJECT_INVALID, "bad-prevblk-chainlock"); + return state.Invalid(ValidationInvalidReason::BLOCK_CHAINLOCK, error("%s: prev block %s conflicts with chainlock", __func__, block.hashPrevBlock.ToString()), REJECT_INVALID, "bad-prevblk-chainlock"); if (!ContextualCheckBlockHeader(block, state, chainparams, pindexPrev, GetAdjustedTime())) return error("%s: Consensus::ContextualCheckBlockHeader: %s, %s", __func__, hash.ToString(), FormatStateMessage(state)); @@ -3987,7 +4001,7 @@ bool BlockManager::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"); } } } @@ -3996,7 +4010,7 @@ bool BlockManager::AcceptBlockHeader(const CBlockHeader& block, CValidationState if (pindex == nullptr) { AddToBlockIndex(block, BLOCK_CONFLICT_CHAINLOCK); } - return state.DoS(10, error("%s: header %s conflicts with chainlock", __func__, hash.ToString()), REJECT_INVALID, "bad-chainlock"); + return state.Invalid(ValidationInvalidReason::BLOCK_CHAINLOCK, error("%s: header %s conflicts with chainlock", __func__, hash.ToString()), REJECT_INVALID, "bad-chainlock"); } } if (pindex == nullptr) @@ -4113,7 +4127,8 @@ bool CChainState::AcceptBlock(const std::shared_ptr& 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); } @@ -4194,7 +4209,7 @@ bool TestBlockValidity(CValidationState& state, const CChainParams& chainparams, uint256 hash = block.GetHash(); if (llmq::chainLocksHandler->HasConflictingChainLock(pindexPrev->nHeight + 1, hash)) { - return state.DoS(10, error("%s: conflicting with chainlock", __func__), REJECT_INVALID, "bad-chainlock"); + return state.Invalid(ValidationInvalidReason::BLOCK_INVALID_PREV, error("%s: conflicting with chainlock", __func__), REJECT_INVALID, "bad-chainlock"); } CCoinsViewCache viewNew(&::ChainstateActive().CoinsTip()); diff --git a/test/functional/data/invalid_txs.py b/test/functional/data/invalid_txs.py index 7d106eabd407..1ab72f5d02a7 100644 --- a/test/functional/data/invalid_txs.py +++ b/test/functional/data/invalid_txs.py @@ -58,7 +58,7 @@ def get_tx(self, *args, **kwargs): 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 @@ def get_tx(self): 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 cc854cad7ca1..6fd8f1b55927 100755 --- a/test/functional/feature_block.py +++ b/test/functional/feature_block.py @@ -301,7 +301,7 @@ def run_test(self): self.log.info("Reject a block spending an immature coinbase.") self.move_tip(15) b20 = self.next_block(20, spend=out[7]) - self.send_blocks([b20], success=False, reject_reason='bad-txns-premature-spend-of-coinbase') + self.send_blocks([b20], success=False, reject_reason='bad-txns-premature-spend-of-coinbase', reconnect=True) # Attempt to spend a coinbase at depth too low (on a fork this time) # genesis -> b1 (0) -> b2 (1) -> b5 (2) -> b6 (3) @@ -314,7 +314,7 @@ def run_test(self): self.send_blocks([b21], False) b22 = self.next_block(22, spend=out[5]) - self.send_blocks([b22], success=False, reject_reason='bad-txns-premature-spend-of-coinbase') + self.send_blocks([b22], success=False, reject_reason='bad-txns-premature-spend-of-coinbase', reconnect=True) # Create a block on either side of MAX_BLOCK_SIZE and make sure its accepted/rejected # genesis -> b1 (0) -> b2 (1) -> b5 (2) -> b6 (3) @@ -636,7 +636,7 @@ def run_test(self): while b47.sha256 <= target: b47.nNonce += 1 b47.rehash() - self.send_blocks([b47], False, force_send=True, reject_reason='high-hash') + self.send_blocks([b47], False, force_send=True, reject_reason='high-hash', reconnect=True) self.log.info("Reject a block with a timestamp >2 hours in the future") self.move_tip(44) @@ -687,7 +687,7 @@ def run_test(self): b54 = self.next_block(54, spend=out[15]) b54.nTime = b35.nTime - 1 b54.solve() - self.send_blocks([b54], False, force_send=True, reject_reason='time-too-old') + self.send_blocks([b54], False, force_send=True, reject_reason='time-too-old', reconnect=True) # valid timestamp self.move_tip(53) @@ -857,7 +857,7 @@ def run_test(self): assert tx.vin[0].nSequence < 0xffffffff tx.calc_sha256() b62 = self.update_block(62, [tx]) - self.send_blocks([b62], success=False, reject_reason='bad-txns-nonfinal') + self.send_blocks([b62], success=False, reject_reason='bad-txns-nonfinal', reconnect=True) # Test a non-final coinbase is also rejected # @@ -871,7 +871,7 @@ def run_test(self): b63.vtx[0].vin[0].nSequence = 0xDEADBEEF b63.vtx[0].rehash() b63 = self.update_block(63, []) - self.send_blocks([b63], success=False, reject_reason='bad-txns-nonfinal') + self.send_blocks([b63], success=False, reject_reason='bad-txns-nonfinal', reconnect=True) # This checks that a block with a bloated VARINT between the block_header and the array of tx such that # the block is > MAX_BLOCK_SIZE with the bloated varint, but <= MAX_BLOCK_SIZE without the bloated varint, @@ -1285,7 +1285,7 @@ def run_test(self): self.log.info("Reject a block with an invalid block header version") b_v1 = self.next_block('b_v1', version=1) - self.send_blocks([b_v1], success=False, force_send=True, reject_reason='bad-version(0x00000001)') + self.send_blocks([b_v1], success=False, force_send=True, reject_reason='bad-version(0x00000001)', reconnect=True) self.move_tip(chain1_tip + 2) b_cb34 = self.next_block('b_cb34', version=4) diff --git a/test/lint/lint-circular-dependencies.sh b/test/lint/lint-circular-dependencies.sh index 6d4576d88118..c97bffc741c6 100755 --- a/test/lint/lint-circular-dependencies.sh +++ b/test/lint/lint-circular-dependencies.sh @@ -85,6 +85,7 @@ EXPECTED_CIRCULAR_DEPENDENCIES=( "policy/policy -> policy/settings -> policy/policy" "evo/specialtxman -> validation -> evo/specialtxman" "bloom -> llmq/commitment -> llmq/utils -> net -> bloom" + "banman -> bloom -> llmq/commitment -> llmq/utils -> net -> banman" "evo/simplifiedmns -> llmq/blockprocessor -> net_processing -> llmq/snapshot -> evo/simplifiedmns" "llmq/blockprocessor -> net_processing -> llmq/snapshot -> llmq/blockprocessor"