From 09f59a5c01ff869f42d587cabab088db9a2cfd00 Mon Sep 17 00:00:00 2001 From: Bushstar Date: Mon, 11 Dec 2023 11:54:20 +0000 Subject: [PATCH] Remove BIP61 --- src/consensus/tx_check.cpp | 20 +- src/consensus/tx_verify.cpp | 18 +- src/consensus/validation.h | 20 +- src/init.cpp | 3 +- src/net_processing.cpp | 103 ++------- src/net_processing.h | 9 +- src/protocol.cpp | 2 - src/protocol.h | 7 - src/rpc/rawtransaction.cpp | 2 +- src/spv/bitcoin/BRPeer.cpp | 47 ---- src/test/denialofservice_tests.cpp | 10 +- src/util/validation.cpp | 5 +- src/validation.cpp | 243 ++++++--------------- src/validation.h | 10 - test/functional/feature_bip68_sequence.py | 2 +- test/functional/feature_cltv.py | 6 +- test/functional/feature_dersig.py | 28 +-- test/functional/feature_nulldummy.py | 4 +- test/functional/feature_segwit.py | 8 +- test/functional/mempool_accept.py | 36 +-- test/functional/rpc_rawtransaction.py | 2 +- test/functional/test_framework/messages.py | 39 ---- test/functional/test_framework/mininode.py | 2 - 23 files changed, 152 insertions(+), 474 deletions(-) diff --git a/src/consensus/tx_check.cpp b/src/consensus/tx_check.cpp index db2970f3e7..7b920b8c81 100644 --- a/src/consensus/tx_check.cpp +++ b/src/consensus/tx_check.cpp @@ -17,24 +17,24 @@ bool CheckTransaction(const CTransaction& tx, CValidationState &state, bool fChe /// @note we don't check minted token's outputs nor auth here! // Basic checks that don't depend on any context if (tx.vin.empty()) - return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-txns-vin-empty"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-txns-vin-empty"); if (tx.vout.empty()) - return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-txns-vout-empty"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-txns-vout-empty"); // Size limits (this doesn't take the witness into account, as that hasn't been checked for malleability) if (::GetSerializeSize(tx, PROTOCOL_VERSION | SERIALIZE_TRANSACTION_NO_WITNESS) * WITNESS_SCALE_FACTOR > MAX_BLOCK_WEIGHT) - return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-txns-oversize"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-txns-oversize"); // Check for negative or overflow output values CAmount nValueOut = 0; for (const auto& txout : tx.vout) { if (txout.nValue < 0) - return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-txns-vout-negative"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-txns-vout-negative"); if (txout.nValue > MAX_MONEY) - return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-txns-vout-toolarge"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-txns-vout-toolarge"); nValueOut += txout.nValue; if (!MoneyRange(nValueOut)) - return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-txns-txouttotal-toolarge"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-txns-txouttotal-toolarge"); } auto isEVMTx = IsEVMTx(tx); @@ -45,7 +45,7 @@ bool CheckTransaction(const CTransaction& tx, CValidationState &state, bool fChe for (const auto& txin : tx.vin) { if (!vInOutPoints.insert(txin.prevout).second) - return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-txns-inputs-duplicate"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-txns-inputs-duplicate"); } } @@ -55,18 +55,18 @@ bool CheckTransaction(const CTransaction& tx, CValidationState &state, bool fChe if (IsAnchorRewardTx(tx, dummy) || IsAnchorRewardTxPlus(tx, dummy) || IsTokenSplitTx(tx, dummy)) return true; if (tx.vin[0].scriptSig.size() < 2 || (tx.vin[0].scriptSig.size() > 100)) - return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-cb-length"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-cb-length"); } else if (isEVMTx) { if (tx.vin[0].scriptSig.size() != 1 || tx.vin[1].scriptSig.size() != 1) - return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-evm-length"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-evm-length"); } else { for (const auto& txin : tx.vin) if (txin.prevout.IsNull()) - return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-txns-prevout-null"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-txns-prevout-null"); } return true; diff --git a/src/consensus/tx_verify.cpp b/src/consensus/tx_verify.cpp index e903698e1a..11c580392f 100644 --- a/src/consensus/tx_verify.cpp +++ b/src/consensus/tx_verify.cpp @@ -170,7 +170,7 @@ bool Consensus::CheckTxInputs(const CTransaction& tx, CValidationState& state, c // are the actual inputs available? if (!inputs.HaveInputs(tx)) { - return state.Invalid(ValidationInvalidReason::TX_MISSING_INPUTS, false, REJECT_INVALID, "bad-txns-inputs-missingorspent", + return state.Invalid(ValidationInvalidReason::TX_MISSING_INPUTS, false, "bad-txns-inputs-missingorspent", strprintf("%s: inputs missing/spent", __func__)); } @@ -194,7 +194,7 @@ bool Consensus::CheckTxInputs(const CTransaction& tx, CValidationState& state, c }; auto res = ApplyCustomTx(blockCtx, txCtx, &canSpend); if (!res.ok && (res.code & CustomTxErrCodes::Fatal)) { - return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-txns-customtx", res.msg); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-txns-customtx", res.msg); } } @@ -206,18 +206,18 @@ bool Consensus::CheckTxInputs(const CTransaction& tx, CValidationState& state, c // If prev is coinbase, check that it's matured if (coin.IsCoinBase() && nSpendHeight - coin.nHeight < COINBASE_MATURITY) { - return state.Invalid(ValidationInvalidReason::TX_PREMATURE_SPEND, false, REJECT_INVALID, "bad-txns-premature-spend-of-coinbase", + return state.Invalid(ValidationInvalidReason::TX_PREMATURE_SPEND, false, "bad-txns-premature-spend-of-coinbase", strprintf("tried to spend coinbase at depth %d", nSpendHeight - coin.nHeight)); } // Check for negative or overflow input values nValuesIn[coin.out.nTokenId] += coin.out.nValue; if (!MoneyRange(coin.out.nValue) || !MoneyRange(nValuesIn[coin.out.nTokenId])) { - return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-txns-inputvalues-outofrange"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-txns-inputvalues-outofrange"); } /// @todo tokens: later match the range with TotalSupply if (canSpend != prevout.hash && prevout.n == 1 && !mnview.CanSpend(prevout.hash, nSpendHeight)) { - return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-txns-collateral-locked", + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-txns-collateral-locked", strprintf("tried to spend locked collateral for %s", prevout.hash.ToString())); /// @todo may be somehow place the height of unlocking? } } @@ -227,27 +227,27 @@ bool Consensus::CheckTxInputs(const CTransaction& tx, CValidationState& state, c // special (old) case for 'DFI'. Do not "optimize" due to tests compatibility if (nValuesIn[DCT_ID{0}] < non_minted_values_out[DCT_ID{0}]) { - return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-txns-in-belowout", + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-txns-in-belowout", strprintf("value in (%s) < value out (%s)", FormatMoney(nValuesIn[DCT_ID{0}]), FormatMoney(non_minted_values_out[DCT_ID{0}]))); } // Tally transaction fees const CAmount txfee_aux = nValuesIn[DCT_ID{0}] - non_minted_values_out[DCT_ID{0}]; if (!MoneyRange(txfee_aux)) { - return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-txns-fee-outofrange"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-txns-fee-outofrange"); } txfee = txfee_aux; // after fee calc it is guaranteed that both values[0] exists (even if zero) if (tx.nVersion < CTransaction::TOKENS_MIN_VERSION && (nValuesIn.size() > 1 || non_minted_values_out.size() > 1)) { - return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-txns-tokens-in-old-version-tx"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-txns-tokens-in-old-version-tx"); } for (auto const & kv : non_minted_values_out) { DCT_ID const & tokenId = kv.first; if (nValuesIn[tokenId] < kv.second) { - return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-txns-minttokens-in-belowout", + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-txns-minttokens-in-belowout", strprintf("token (%s) value in (%s) < value out (%s)", tokenId.ToString(), FormatMoney(nValuesIn[tokenId]), FormatMoney(kv.second))); } diff --git a/src/consensus/validation.h b/src/consensus/validation.h index 93b204bf6f..219e6c7db2 100644 --- a/src/consensus/validation.h +++ b/src/consensus/validation.h @@ -12,21 +12,8 @@ #include #include -/** "reject" message codes */ -static const unsigned char REJECT_MALFORMED = 0x01; -static const unsigned char REJECT_INVALID = 0x10; -static const unsigned char REJECT_OBSOLETE = 0x11; -static const unsigned char REJECT_DUPLICATE = 0x12; -static const unsigned char REJECT_NONSTANDARD = 0x40; -// static const unsigned char REJECT_DUST = 0x41; // part of BIP 61 -static const unsigned char REJECT_INSUFFICIENTFEE = 0x42; -static const unsigned char REJECT_CHECKPOINT = 0x43; -static const unsigned char REJECT_CUSTOMTX = 0x44; - /** A "reason" why something was invalid, suitable for determining whether the * provider of the object should be banned/ignored/disconnected/etc. - * These are much more granular than the rejection codes, which may be more - * useful for some other use-cases. */ enum class ValidationInvalidReason { // txn and blocks: @@ -105,15 +92,13 @@ class CValidationState { } mode; ValidationInvalidReason m_reason; std::string strRejectReason; - unsigned int chRejectCode; std::string strDebugMessage; public: - CValidationState() : mode(MODE_VALID), m_reason(ValidationInvalidReason::NONE), chRejectCode(0) {} + CValidationState() : mode(MODE_VALID), m_reason(ValidationInvalidReason::NONE) {} bool Invalid(ValidationInvalidReason reasonIn, bool ret = false, - unsigned int chRejectCodeIn=0, const std::string &strRejectReasonIn="", + const std::string &strRejectReasonIn="", const std::string &strDebugMessageIn="") { m_reason = reasonIn; - chRejectCode = chRejectCodeIn; strRejectReason = strRejectReasonIn; strDebugMessage = strDebugMessageIn; if (mode == MODE_ERROR) @@ -137,7 +122,6 @@ class CValidationState { return mode == MODE_ERROR; } 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/init.cpp b/src/init.cpp index 4fb992c024..ab33581936 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -469,7 +469,6 @@ void SetupServerArgs() gArgs.AddArg("-discover", "Discover own IP addresses (default: 1 when listening and no -externalip or -proxy)", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); gArgs.AddArg("-dns", strprintf("Allow DNS lookups for -addnode, -seednode and -connect (default: %u)", DEFAULT_NAME_LOOKUP), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); gArgs.AddArg("-dnsseed", "Query for peer addresses via DNS lookup, if low on addresses (default: 1 unless -connect used)", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); - gArgs.AddArg("-enablebip61", strprintf("Send reject messages per BIP61 (default: %u)", DEFAULT_ENABLE_BIP61), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); gArgs.AddArg("-externalip=", "Specify your own public address", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); gArgs.AddArg("-forcednsseed", strprintf("Always query for peer addresses via DNS lookup (default: %u)", DEFAULT_FORCEDNSSEED), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); gArgs.AddArg("-listen", "Accept connections from outside (default: 1 if no -proxy or -connect)", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); @@ -1447,7 +1446,7 @@ bool SetupNetwork() { assert(!g_connman); g_connman = std::unique_ptr(new CConnman(GetRand(std::numeric_limits::max()), GetRand(std::numeric_limits::max()))); - peerLogic.reset(new PeerLogicValidation(g_connman.get(), g_banman.get(), scheduler, gArgs.GetBoolArg("-enablebip61", DEFAULT_ENABLE_BIP61))); + peerLogic.reset(new PeerLogicValidation(g_connman.get(), g_banman.get(), scheduler)); RegisterValidationInterface(peerLogic.get()); // sanitize comments per BIP-0014, format user agent and check total size diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 696a48647d..63109ee648 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -125,8 +125,8 @@ namespace { int nSyncStarted GUARDED_BY(cs_main) = 0; /** - * Sources of received blocks, saved to be able to send them reject - * messages or ban them when processing happens afterwards. + * Sources of received blocks, saved to be able punish them when processing + * happens afterwards. * Set mapBlockSource[hash].second to false if the node should not be * punished if the block is invalid. */ @@ -201,12 +201,6 @@ namespace { } // namespace namespace { -struct CBlockReject { - unsigned char chRejectCode; - std::string strRejectReason; - uint256 hashBlock; -}; - /** * Maintain validation-specific state about nodes, protected by cs_main, instead * by CNode's own locks. This simplifies asynchronous operation, where @@ -224,8 +218,6 @@ struct CNodeState { bool fShouldBan; //! String name of this peer (debugging/logging purposes). const std::string name; - //! List of asynchronously-determined block rejections to notify this peer about. - std::vector rejects; //! The best known block we know this peer has announced. const CBlockIndex *pindexBestKnownBlock; //! The hash of the last unknown block this peer has announced. @@ -1110,8 +1102,8 @@ static bool BlockRequestAllowed(const CBlockIndex* pindex, const Consensus::Para (GetBlockProofEquivalentTime(*pindexBestHeader, *pindex, *pindexBestHeader, consensusParams) < STALE_RELAY_AGE_LIMIT); } -PeerLogicValidation::PeerLogicValidation(CConnman* connmanIn, BanMan* banman, CScheduler &scheduler, bool enable_bip61) - : connman(connmanIn), m_banman(banman), m_stale_tip_check_time(0), m_enable_bip61(enable_bip61) { +PeerLogicValidation::PeerLogicValidation(CConnman* connmanIn, BanMan* banman, CScheduler &scheduler) + : connman(connmanIn), m_banman(banman), m_stale_tip_check_time(0) { // Initialize global variables that cannot be constructed at startup. recentRejects.reset(new CRollingBloomFilter(120000, 0.000001)); @@ -1258,13 +1250,12 @@ void PeerLogicValidation::BlockChecked(const CBlock& block, const CValidationSta const uint256 hash(block.GetHash()); std::map>::iterator it = mapBlockSource.find(hash); - 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 the block failed validation, we know where it came from and we're still connected + // to that peer, maybe punish. + if (state.IsInvalid() && + it != mapBlockSource.end() && + State(it->second.first)) { MaybePunishNode(/*nodeid=*/ it->second.first, state, /*via_compact_block=*/ !it->second.second); - } } // Check that: // 1. The block is valid @@ -1952,7 +1943,7 @@ void static ProcessOrphanTx(CConnman* connman, std::set& orphan_work_se } } -bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStream& vRecv, int64_t nTimeReceived, const CChainParams& chainparams, CConnman* connman, const std::atomic& interruptMsgProc, bool enable_bip61) +bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStream& vRecv, int64_t nTimeReceived, const CChainParams& chainparams, CConnman* connman, const std::atomic& interruptMsgProc) { LogPrint(BCLog::NET, "received: %s (%u bytes) peer=%d\n", SanitizeString(strCommand), vRecv.size(), pfrom->GetId()); if (gArgs.IsArgSet("-dropmessagestest") && GetRand(gArgs.GetArg("-dropmessagestest", 0)) == 0) @@ -1976,38 +1967,10 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr } } - if (strCommand == NetMsgType::REJECT) - { - if (LogAcceptCategory(BCLog::NET)) { - try { - std::string strMsg; unsigned char ccode; std::string strReason; - vRecv >> LIMITED_STRING(strMsg, CMessageHeader::COMMAND_SIZE) >> ccode >> LIMITED_STRING(strReason, MAX_REJECT_MESSAGE_LENGTH); - - std::ostringstream ss; - ss << strMsg << " code " << itostr(ccode) << ": " << strReason; - - if (strMsg == NetMsgType::BLOCK || strMsg == NetMsgType::TX) - { - uint256 hash; - vRecv >> hash; - ss << ": hash " << hash.ToString(); - } - LogPrint(BCLog::NET, "Reject %s\n", SanitizeString(ss.str())); - } catch (const std::ios_base::failure&) { - // Avoid feedback loops by preventing reject messages from triggering a new reject message. - LogPrint(BCLog::NET, "Unparseable reject message received\n"); - } - } - return true; - } - if (strCommand == NetMsgType::VERSION) { // Each connection can only send one version message if (pfrom->nVersion != 0) { - if (enable_bip61) { - connman->PushMessage(pfrom, CNetMsgMaker(INIT_PROTO_VERSION).Make(NetMsgType::REJECT, strCommand, REJECT_DUPLICATE, std::string("Duplicate version message"))); - } LOCK(cs_main); Misbehaving(pfrom->GetId(), 1); return false; @@ -2035,10 +1998,6 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr if (!pfrom->fInbound && !pfrom->fFeeler && !pfrom->m_manual_connection && !HasAllDesirableServiceFlags(nServices)) { LogPrint(BCLog::NET, "peer=%d does not offer the expected services (%08x offered, %08x expected); disconnecting\n", pfrom->GetId(), nServices, GetDesirableServiceFlags(nServices)); - if (enable_bip61) { - connman->PushMessage(pfrom, CNetMsgMaker(INIT_PROTO_VERSION).Make(NetMsgType::REJECT, strCommand, REJECT_NONSTANDARD, - strprintf("Expected to offer services %08x", GetDesirableServiceFlags(nServices)))); - } pfrom->fDisconnect = true; return false; } @@ -2046,10 +2005,6 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr if (nVersion < MIN_PEER_PROTO_VERSION) { // disconnect from peers older than this proto version LogPrint(BCLog::NET, "peer=%d using obsolete version %i; disconnecting\n", pfrom->GetId(), nVersion); - if (enable_bip61) { - connman->PushMessage(pfrom, CNetMsgMaker(INIT_PROTO_VERSION).Make(NetMsgType::REJECT, strCommand, REJECT_OBSOLETE, - strprintf("Version must be %d or greater", MIN_PEER_PROTO_VERSION))); - } pfrom->fDisconnect = true; return false; } @@ -2879,10 +2834,6 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr LogPrint(BCLog::MEMPOOLREJ, "%s from peer=%d was not accepted: %s\n", tx.GetHash().ToString(), pfrom->GetId(), FormatStateMessage(state)); - if (enable_bip61 && state.GetRejectCode() > 0 && state.GetRejectCode() < REJECT_INTERNAL) { // Never send AcceptToMemoryPool's internal codes over P2P - connman->PushMessage(pfrom, msgMaker.Make(NetMsgType::REJECT, strCommand, (unsigned char)state.GetRejectCode(), - state.GetRejectReason().substr(0, MAX_REJECT_MESSAGE_LENGTH), inv.hash)); - } MaybePunishNode(pfrom->GetId(), state, /*via_compact_block*/ false); } return true; @@ -3062,7 +3013,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr } // cs_main if (fProcessBLOCKTXN) - return ProcessMessage(pfrom, NetMsgType::BLOCKTXN, blockTxnMsg, nTimeReceived, chainparams, connman, interruptMsgProc, enable_bip61); + return ProcessMessage(pfrom, NetMsgType::BLOCKTXN, blockTxnMsg, nTimeReceived, chainparams, connman, interruptMsgProc); if (fRevertToHeaderProcessing) { // Headers received from HB compact block peers are permitted to be @@ -3165,11 +3116,12 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr // been run). This is handled below, so just treat this as // though the block was successfully read, and rely on the // handling in ProcessNewBlock to ensure the block index is - // updated, reject messages go out, etc. + // updated, etc. MarkBlockAsReceived(resp.blockhash); // it is now an empty pointer fBlockRead = true; - // mapBlockSource is only used for sending reject messages and DoS scores, - // so the race between here and cs_main in ProcessNewBlock is fine. + // mapBlockSource is used for potentially punishing peers and + // updating which peers send us compact blocks, so the race + // between here and cs_main in ProcessNewBlock is fine. // BIP 152 permits peers to relay compact blocks after validating // the header only; we should not punish peers if the block turns // out to be invalid. @@ -3241,8 +3193,9 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr // Also always process if we requested the block explicitly, as we may // need it even though it is not a candidate for a new best tip. forceProcessing |= MarkBlockAsReceived(hash); - // mapBlockSource is only used for sending reject messages and DoS scores, - // so the race between here and cs_main in ProcessNewBlock is fine. + // mapBlockSource is only used for punishing peers and setting + // which peers send us compact blocks, so the race between here and + // cs_main in ProcessNewBlock is fine. mapBlockSource.emplace(hash, std::make_pair(pfrom->GetId(), true)); } bool fNewBlock = false; @@ -3485,18 +3438,11 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr return true; } -bool PeerLogicValidation::SendRejectsAndCheckIfBanned(CNode* pnode, bool enable_bip61) EXCLUSIVE_LOCKS_REQUIRED(cs_main) +bool PeerLogicValidation::CheckIfBanned(CNode* pnode) { AssertLockHeld(cs_main); CNodeState &state = *State(pnode->GetId()); - if (enable_bip61) { - for (const CBlockReject& reject : state.rejects) { - connman->PushMessage(pnode, CNetMsgMaker(INIT_PROTO_VERSION).Make(NetMsgType::REJECT, std::string(NetMsgType::BLOCK), reject.chRejectCode, reject.strRejectReason, reject.hashBlock)); - } - } - state.rejects.clear(); - if (state.fShouldBan) { state.fShouldBan = false; if (pnode->HasPermission(PF_NOBAN)) @@ -3604,7 +3550,7 @@ bool PeerLogicValidation::ProcessMessages(CNode* pfrom, std::atomic& inter bool fRet = false; try { - fRet = ProcessMessage(pfrom, strCommand, vRecv, msg.nTime, chainparams, connman, interruptMsgProc, m_enable_bip61); + fRet = ProcessMessage(pfrom, strCommand, vRecv, msg.nTime, chainparams, connman, interruptMsgProc); if (interruptMsgProc) return false; if (!pfrom->vRecvGetData.empty()) @@ -3612,9 +3558,6 @@ bool PeerLogicValidation::ProcessMessages(CNode* pfrom, std::atomic& inter } catch (const std::ios_base::failure& e) { - if (m_enable_bip61) { - connman->PushMessage(pfrom, CNetMsgMaker(INIT_PROTO_VERSION).Make(NetMsgType::REJECT, strCommand, REJECT_MALFORMED, std::string("error parsing message"))); - } if (strstr(e.what(), "end of data")) { // Allow exceptions from under-length message on vRecv LogPrint(BCLog::NET, "%s(%s, %u bytes): Exception '%s' caught, normally caused by a message being shorter than its stated length\n", __func__, SanitizeString(strCommand), nMessageSize, e.what()); @@ -3645,7 +3588,7 @@ bool PeerLogicValidation::ProcessMessages(CNode* pfrom, std::atomic& inter } LOCK(cs_main); - SendRejectsAndCheckIfBanned(pfrom, m_enable_bip61); + CheckIfBanned(pfrom); return fMoreWork; } @@ -3842,11 +3785,11 @@ bool PeerLogicValidation::SendMessages(CNode* pto) } } - TRY_LOCK(cs_main, lockMain); // Acquire cs_main for IsInitialBlockDownload() and CNodeState() + TRY_LOCK(cs_main, lockMain); if (!lockMain) return true; - if (SendRejectsAndCheckIfBanned(pto, m_enable_bip61)) return true; + if (CheckIfBanned(pto)) return true; CNodeState &state = *State(pto->GetId()); // Address refresh broadcast diff --git a/src/net_processing.h b/src/net_processing.h index dc69b38e0d..a3a4e2a845 100644 --- a/src/net_processing.h +++ b/src/net_processing.h @@ -17,8 +17,6 @@ extern CCriticalSection cs_main; static const unsigned int DEFAULT_MAX_ORPHAN_TRANSACTIONS = 100; /** Default number of orphan+recently-replaced txn to keep around for block reconstruction */ static const unsigned int DEFAULT_BLOCK_RECONSTRUCTION_EXTRA_TXN = 100; -/** Default for BIP61 (sending reject messages) */ -static constexpr bool DEFAULT_ENABLE_BIP61{false}; static const bool DEFAULT_PEERBLOOMFILTERS = false; /** The maximum rate of address records we're willing to process on average. Can be bypassed using @@ -42,9 +40,9 @@ class PeerLogicValidation final : public CValidationInterface, public NetEventsI CConnman* const connman; BanMan* const m_banman; - bool SendRejectsAndCheckIfBanned(CNode* pnode, bool enable_bip61) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + bool CheckIfBanned(CNode* pnode) EXCLUSIVE_LOCKS_REQUIRED(cs_main); public: - PeerLogicValidation(CConnman* connman, BanMan* banman, CScheduler &scheduler, bool enable_bip61); + PeerLogicValidation(CConnman* connman, BanMan* banman, CScheduler &scheduler); /** * Overridden from CValidationInterface. @@ -91,9 +89,6 @@ class PeerLogicValidation final : public CValidationInterface, public NetEventsI private: int64_t m_stale_tip_check_time; //!< Next time to check for stale tip - - /** Enable BIP61 (sending reject messages) */ - const bool m_enable_bip61; }; struct CNodeStateStats { diff --git a/src/protocol.cpp b/src/protocol.cpp index da24c190b7..e39e59a810 100644 --- a/src/protocol.cpp +++ b/src/protocol.cpp @@ -35,7 +35,6 @@ const char *NOTFOUND="notfound"; const char *FILTERLOAD="filterload"; const char *FILTERADD="filteradd"; const char *FILTERCLEAR="filterclear"; -const char *REJECT="reject"; const char *SENDHEADERS="sendheaders"; const char *FEEFILTER="feefilter"; const char *SENDCMPCT="sendcmpct"; @@ -70,7 +69,6 @@ const static std::string allNetMessageTypes[] = { NetMsgType::FILTERLOAD, NetMsgType::FILTERADD, NetMsgType::FILTERCLEAR, - NetMsgType::REJECT, NetMsgType::SENDHEADERS, NetMsgType::FEEFILTER, NetMsgType::SENDCMPCT, diff --git a/src/protocol.h b/src/protocol.h index d403aac8d2..a68e0076e8 100644 --- a/src/protocol.h +++ b/src/protocol.h @@ -197,13 +197,6 @@ extern const char *FILTERADD; * @see https://bitcoin.org/en/developer-reference#filterclear */ extern const char *FILTERCLEAR; -/** - * The reject message informs the receiving node that one of its previous - * messages has been rejected. - * @since protocol version 70002 as described by BIP61. - * @see https://bitcoin.org/en/developer-reference#reject - */ -extern const char *REJECT; /** * Indicates that a node prefers to receive new block announcements via a * "headers" message rather than an "inv". diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 4316fbfd35..247cfda006 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -900,7 +900,7 @@ static UniValue testmempoolaccept(const JSONRPCRequest& request) result_0.pushKV("allowed", test_accept_res); if (!test_accept_res) { if (state.IsInvalid()) { - result_0.pushKV("reject-reason", strprintf("%i: %s", state.GetRejectCode(), state.GetRejectReason())); + result_0.pushKV("reject-reason", strprintf("%s", state.GetRejectReason())); } else if (missing_inputs) { result_0.pushKV("reject-reason", "missing-inputs"); } else { diff --git a/src/spv/bitcoin/BRPeer.cpp b/src/spv/bitcoin/BRPeer.cpp index 36e8c519dc..e1774133d7 100644 --- a/src/spv/bitcoin/BRPeer.cpp +++ b/src/spv/bitcoin/BRPeer.cpp @@ -754,52 +754,6 @@ static int _BRPeerAcceptMerkleblockMessage(BRPeer *peer, const uint8_t *msg, siz return r; } -// described in BIP61: https://github.com/bitcoin/bips/blob/master/bip-0061.mediawiki -static int _BRPeerAcceptRejectMessage(BRPeer *peer, const uint8_t *msg, size_t msgLen) -{ - BRPeerContext *ctx = (BRPeerContext *)peer; - size_t off = 0, strLen = (size_t)BRVarInt(msg, msgLen, &off); - int r = 1; - - if (off + strLen + sizeof(uint8_t) > msgLen) { - peer_log(peer, "malformed reject message, length is %zu, should be >= %zu", msgLen, - off + strLen + sizeof(uint8_t)); - r = 0; - } - else { - uint8_t code; - size_t len = 0, hashLen = 0; - - std::string type((const char *)&msg[off], strLen); - off += strLen; - code = msg[off++]; - strLen = (size_t)BRVarInt(&msg[off], (off <= msgLen ? msgLen - off : 0), &len); - off += len; - if (type == NetMsgType::TX) hashLen = sizeof(UInt256); - - if (off + strLen + hashLen > msgLen) { - peer_log(peer, "malformed reject message, length is %zu, should be >= %zu", msgLen, off + strLen + hashLen); - r = 0; - } - else { - UInt256 txHash = UINT256_ZERO; - - std::string reason((const char *)&msg[off], strLen); - off += strLen; - if (hashLen == sizeof(UInt256)) txHash = UInt256Get(&msg[off]); - off += hashLen; - - if (! UInt256IsZero(txHash)) { - peer_log(peer, "rejected %s code: 0x%x reason: \"%s\" txid: %s", type.c_str(), code, reason.c_str(), u256hex(txHash).c_str()); - if (ctx->rejectedTx) ctx->rejectedTx(ctx->info, txHash, code); - } - else peer_log(peer, "rejected %s code: 0x%x reason: \"%s\"", type.c_str(), code, reason.c_str()); - } - } - - return r; -} - // BIP133: https://github.com/bitcoin/bips/blob/master/bip-0133.mediawiki static int _BRPeerAcceptFeeFilterMessage(BRPeer *peer, const uint8_t *msg, size_t msgLen) { @@ -846,7 +800,6 @@ static int _BRPeerAcceptMessage(BRPeer *peer, const uint8_t *msg, size_t msgLen, else if (strncmp(NetMsgType::PING, type, 12) == 0) r = _BRPeerAcceptPingMessage(peer, msg, msgLen); else if (strncmp(NetMsgType::PONG, type, 12) == 0) r = _BRPeerAcceptPongMessage(peer, msg, msgLen); else if (strncmp(NetMsgType::MERKLEBLOCK, type, 12) == 0) r = _BRPeerAcceptMerkleblockMessage(peer, msg, msgLen); - else if (strncmp(NetMsgType::REJECT, type, 12) == 0) r = _BRPeerAcceptRejectMessage(peer, msg, msgLen); else if (strncmp(NetMsgType::FEEFILTER, type, 12) == 0) r = _BRPeerAcceptFeeFilterMessage(peer, msg, msgLen); else peer_log(peer, "dropping %s, length %zu, not implemented", type, msgLen); diff --git a/src/test/denialofservice_tests.cpp b/src/test/denialofservice_tests.cpp index 40a2b65b7c..396c30538a 100644 --- a/src/test/denialofservice_tests.cpp +++ b/src/test/denialofservice_tests.cpp @@ -77,7 +77,7 @@ BOOST_FIXTURE_TEST_SUITE(denialofservice_tests, TestingSetup) BOOST_AUTO_TEST_CASE(outbound_slow_chain_eviction) { auto connman = std::make_unique(0x1337, 0x1337); - auto peerLogic = std::make_unique(connman.get(), nullptr, scheduler, false); + auto peerLogic = std::make_unique(connman.get(), nullptr, scheduler); // Mock an outbound peer CAddress addr1(ip(0xa0b0c001), NODE_NONE); @@ -136,7 +136,7 @@ BOOST_AUTO_TEST_CASE(outbound_slow_chain_eviction) //BOOST_AUTO_TEST_CASE(stale_tip_peer_management) //{ // auto connman = MakeUnique(0x1337, 0x1337); -// auto peerLogic = MakeUnique(connman.get(), nullptr, scheduler, false); +// auto peerLogic = MakeUnique(connman.get(), nullptr, scheduler); // const Consensus::Params& consensusParams = Params().GetConsensus(); // constexpr int nMaxOutbound = 8; @@ -209,7 +209,7 @@ BOOST_AUTO_TEST_CASE(DoS_banning) { auto banman = std::make_unique(GetDataDir() / "banlist.dat", nullptr, DEFAULT_MISBEHAVING_BANTIME); auto connman = std::make_unique(0x1337, 0x1337); - auto peerLogic = std::make_unique(connman.get(), banman.get(), scheduler, false); + auto peerLogic = std::make_unique(connman.get(), banman.get(), scheduler); banman->ClearBanned(); CAddress addr1(ip(0xa0b0c001), NODE_NONE); @@ -264,7 +264,7 @@ BOOST_AUTO_TEST_CASE(DoS_banscore) { auto banman = std::make_unique(GetDataDir() / "banlist.dat", nullptr, DEFAULT_MISBEHAVING_BANTIME); auto connman = std::make_unique(0x1337, 0x1337); - auto peerLogic = std::make_unique(connman.get(), banman.get(), scheduler, false); + auto peerLogic = std::make_unique(connman.get(), banman.get(), scheduler); banman->ClearBanned(); gArgs.ForceSetArg("-banscore", "111"); // because 11 is my favorite number @@ -311,7 +311,7 @@ BOOST_AUTO_TEST_CASE(DoS_bantime) { auto banman = std::make_unique(GetDataDir() / "banlist.dat", nullptr, DEFAULT_MISBEHAVING_BANTIME); auto connman = std::make_unique(0x1337, 0x1337); - auto peerLogic = std::make_unique(connman.get(), banman.get(), scheduler, false); + auto peerLogic = std::make_unique(connman.get(), banman.get(), scheduler); banman->ClearBanned(); int64_t nStartTime = GetTime(); diff --git a/src/util/validation.cpp b/src/util/validation.cpp index 798cd5ba3e..164e1c3d11 100644 --- a/src/util/validation.cpp +++ b/src/util/validation.cpp @@ -11,10 +11,9 @@ /** Convert CValidationState to a human-readable message for logging */ std::string FormatStateMessage(const CValidationState &state) { - return strprintf("%s%s (code %i)", + return strprintf("%s%s", state.GetRejectReason(), - state.GetDebugMessage().empty() ? "" : ", "+state.GetDebugMessage(), - state.GetRejectCode()); + state.GetDebugMessage().empty() ? "" : ", "+state.GetDebugMessage()); } const std::string strMessageMagic = "Defi Signed Message:\n"; diff --git a/src/validation.cpp b/src/validation.cpp index 83a15677ee..0153a53806 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -571,33 +571,33 @@ static bool AcceptToMemoryPoolWorker(const CChainParams &chainparams, // Coinbase is only valid in a block, not as a loose transaction if (tx.IsCoinBase()) { - return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "coinbase"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "coinbase"); } // Rather not work on nonstandard transactions (unless -testnet/-regtest) std::string reason; const auto isStandard{IsStandardTx(tx, reason)}; if (reason == "eth-scriptpubkey" || (fRequireStandard && !isStandard)) { - return state.Invalid(ValidationInvalidReason::TX_NOT_STANDARD, false, REJECT_NONSTANDARD, reason); + return state.Invalid(ValidationInvalidReason::TX_NOT_STANDARD, false, reason); } // Do not work on transactions that are too small. // A transaction with 1 segwit input and 1 P2WPHK output has non-witness size of 82 bytes. // Transactions smaller than this are not relayed to reduce unnecessary malloc overhead. if (::GetSerializeSize(tx, PROTOCOL_VERSION | SERIALIZE_TRANSACTION_NO_WITNESS) < MIN_STANDARD_TX_NONWITNESS_SIZE) { - return state.Invalid(ValidationInvalidReason::TX_NOT_STANDARD, false, REJECT_NONSTANDARD, "tx-size-small"); + return state.Invalid(ValidationInvalidReason::TX_NOT_STANDARD, false, "tx-size-small"); } // Only accept nLockTime-using transactions that can be mined in the next // block; we don't want our mempool filled up with transactions that can't // be mined yet. if (!CheckFinalTx(tx, STANDARD_LOCKTIME_VERIFY_FLAGS)) { - return state.Invalid(ValidationInvalidReason::TX_PREMATURE_SPEND, false, REJECT_NONSTANDARD, "non-final"); + return state.Invalid(ValidationInvalidReason::TX_PREMATURE_SPEND, false, "non-final"); } // is it already in the memory pool? if (pool.exists(hash)) { - return state.Invalid(ValidationInvalidReason::TX_CONFLICT, false, REJECT_DUPLICATE, "txn-already-in-mempool"); + return state.Invalid(ValidationInvalidReason::TX_CONFLICT, false, "txn-already-in-mempool"); } auto isEVMTx = IsEVMTx(tx); @@ -629,10 +629,7 @@ static bool AcceptToMemoryPoolWorker(const CChainParams &chainparams, } } if (fReplacementOptOut) { - return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, - false, - REJECT_DUPLICATE, - "txn-mempool-conflict"); + return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, "txn-mempool-conflict"); } setConflicts.insert(ptxConflicting->GetHash()); @@ -665,8 +662,7 @@ static bool AcceptToMemoryPoolWorker(const CChainParams &chainparams, for (size_t out = 0; out < tx.vout.size(); out++) { // Optimistically just do efficient check of cache for outputs if (coins_cache.HaveCoinInCache(COutPoint(hash, out))) { - return state.Invalid( - ValidationInvalidReason::TX_CONFLICT, false, REJECT_DUPLICATE, "txn-already-known"); + return state.Invalid(ValidationInvalidReason::TX_CONFLICT, false, "txn-already-known"); } } // Otherwise assume this might be an orphan tx for which we just haven't seen parents yet @@ -682,7 +678,6 @@ static bool AcceptToMemoryPoolWorker(const CChainParams &chainparams, if (txin.prevout.n == 1 && IsMempooledCustomTxCreate(pool, txin.prevout.hash)) { return state.Invalid(ValidationInvalidReason::CONSENSUS, false, - REJECT_INVALID, "collateral-locked-in-mempool", strprintf("tried to spend collateral of non-created mn or token %s, cheater?", txin.prevout.hash.ToString())); @@ -710,15 +705,13 @@ static bool AcceptToMemoryPoolWorker(const CChainParams &chainparams, if (nAbsurdFee && nFees > nAbsurdFee) { return state.Invalid(ValidationInvalidReason::TX_NOT_STANDARD, false, - REJECT_HIGHFEE, "absurdly-high-fee", strprintf("%d > %d", nFees, nAbsurdFee)); } // let make sure we have needed coins if (!isEVMTx && view.GetValueIn(tx) < nFees) { - return state.Invalid( - ValidationInvalidReason::TX_MEMPOOL_POLICY, false, REJECT_INVALID, "bad-txns-inputs-below-tx-fee"); + return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, "bad-txns-inputs-below-tx-fee"); } const auto &consensus = chainparams.GetConsensus(); @@ -741,7 +734,7 @@ static bool AcceptToMemoryPoolWorker(const CChainParams &chainparams, auto res = ApplyCustomTx(blockCtx, txCtx); if (!res.ok || (res.code & CustomTxErrCodes::Fatal)) { - return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, REJECT_INVALID, res.msg); + return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, res.msg); } // we have all inputs cached now, so switch back to dummy, so we don't need to keep lock on mempool @@ -753,20 +746,17 @@ static bool AcceptToMemoryPoolWorker(const CChainParams &chainparams, // Must keep pool.cs for this unless we change CheckSequenceLocks to take a // CoinsViewCache instead of create its own if (!isEVMTx && !CheckSequenceLocks(pool, tx, STANDARD_LOCKTIME_VERIFY_FLAGS, &lp)) { - return state.Invalid( - ValidationInvalidReason::TX_PREMATURE_SPEND, false, REJECT_NONSTANDARD, "non-BIP68-final"); + return state.Invalid(ValidationInvalidReason::TX_PREMATURE_SPEND, false, "non-BIP68-final"); } // Check for non-standard pay-to-script-hash in inputs if (fRequireStandard && !isEVMTx && !AreInputsStandard(tx, view)) { - return state.Invalid( - ValidationInvalidReason::TX_NOT_STANDARD, false, REJECT_NONSTANDARD, "bad-txns-nonstandard-inputs"); + return state.Invalid(ValidationInvalidReason::TX_NOT_STANDARD, false, "bad-txns-nonstandard-inputs"); } // Check for non-standard witness in P2WSH if (tx.HasWitness() && fRequireStandard && !IsWitnessStandard(tx, view)) { - return state.Invalid( - ValidationInvalidReason::TX_WITNESS_MUTATED, false, REJECT_NONSTANDARD, "bad-witness-nonstandard"); + return state.Invalid(ValidationInvalidReason::TX_WITNESS_MUTATED, false, "bad-witness-nonstandard"); } int64_t nSigOpsCost = GetTransactionSigOpCost(tx, view, STANDARD_SCRIPT_VERIFY_FLAGS); @@ -792,7 +782,6 @@ static bool AcceptToMemoryPoolWorker(const CChainParams &chainparams, if (nSigOpsCost > MAX_STANDARD_TX_SIGOPS_COST) { return state.Invalid(ValidationInvalidReason::TX_NOT_STANDARD, false, - REJECT_NONSTANDARD, "bad-txns-too-many-sigops", strprintf("%d", nSigOpsCost)); } @@ -802,7 +791,6 @@ static bool AcceptToMemoryPoolWorker(const CChainParams &chainparams, if (!bypass_limits && mempoolRejectFee > 0 && nModifiedFees < mempoolRejectFee) { return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, - REJECT_INSUFFICIENTFEE, "mempool min fee not met", strprintf("%d < %d", nModifiedFees, mempoolRejectFee)); } @@ -811,7 +799,6 @@ static bool AcceptToMemoryPoolWorker(const CChainParams &chainparams, if (!isEVMTx && !bypass_limits && 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))); } @@ -851,11 +838,8 @@ static bool AcceptToMemoryPoolWorker(const CChainParams &chainparams, nLimitDescendants + 1, nLimitDescendantSize + EXTRA_DESCENDANT_TX_SIZE_LIMIT, dummy_err_string)) { - return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, - false, - REJECT_NONSTANDARD, - "too-long-mempool-chain", - errString); + return state.Invalid( + ValidationInvalidReason::TX_MEMPOOL_POLICY, false, "too-long-mempool-chain", errString); } } @@ -869,7 +853,6 @@ static bool AcceptToMemoryPoolWorker(const CChainParams &chainparams, return state.Invalid( ValidationInvalidReason::CONSENSUS, false, - REJECT_INVALID, "bad-txns-spends-conflicting-tx", strprintf("%s spends conflicting transaction %s", hash.ToString(), hashAncestor.ToString())); } @@ -910,7 +893,6 @@ static bool AcceptToMemoryPoolWorker(const CChainParams &chainparams, if (newFeeRate <= oldFeeRate) { return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, - REJECT_INSUFFICIENTFEE, "insufficient fee", strprintf("rejecting replacement %s; new feerate %s <= old feerate %s", hash.ToString(), @@ -940,7 +922,6 @@ static bool AcceptToMemoryPoolWorker(const CChainParams &chainparams, } else { return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, - REJECT_NONSTANDARD, "too many potential replacements", strprintf("rejecting replacement %s; too many potential replacements (%d > %d)\n", hash.ToString(), @@ -961,7 +942,6 @@ static bool AcceptToMemoryPoolWorker(const CChainParams &chainparams, return state.Invalid( ValidationInvalidReason::TX_MEMPOOL_POLICY, false, - REJECT_NONSTANDARD, "replacement-adds-unconfirmed", strprintf("replacement %s adds unconfirmed input, idx %d", hash.ToString(), j)); } @@ -974,7 +954,6 @@ static bool AcceptToMemoryPoolWorker(const CChainParams &chainparams, if (nModifiedFees < nConflictingFees) { return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, - REJECT_INSUFFICIENTFEE, "insufficient fee", strprintf("rejecting replacement %s, less fees than conflicting txs; %s < %s", hash.ToString(), @@ -988,7 +967,6 @@ static bool AcceptToMemoryPoolWorker(const CChainParams &chainparams, if (nDeltaFees < ::incrementalRelayFee.GetFee(nSize)) { return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, - REJECT_INSUFFICIENTFEE, "insufficient fee", strprintf("rejecting replacement %s, not enough additional fees to relay; %s < %s", hash.ToString(), @@ -1021,7 +999,6 @@ static bool AcceptToMemoryPoolWorker(const CChainParams &chainparams, // Only the witness is missing, so the transaction itself may be fine. state.Invalid(ValidationInvalidReason::TX_WITNESS_MUTATED, false, - state.GetRejectCode(), state.GetRejectReason(), state.GetDebugMessage()); } @@ -1069,7 +1046,7 @@ static bool AcceptToMemoryPoolWorker(const CChainParams &chainparams, if (!r) { LogPrint(BCLog::MEMPOOL, "Failed to parse EVM tx metadata\n"); return state.Invalid( - ValidationInvalidReason::TX_NOT_STANDARD, false, REJECT_INVALID, "failed-to-parse-evm-tx-metadata"); + ValidationInvalidReason::TX_NOT_STANDARD, false, "failed-to-parse-evm-tx-metadata"); } std::string rawEVMTx; @@ -1093,8 +1070,7 @@ static bool AcceptToMemoryPoolWorker(const CChainParams &chainparams, result, rawEVMTx, static_cast(reinterpret_cast(&mnview))); if (!result.ok) { LogPrint(BCLog::MEMPOOL, "EVM tx failed to get sender info %s\n", result.reason.c_str()); - return state.Invalid( - ValidationInvalidReason::TX_NOT_STANDARD, false, REJECT_INVALID, "evm-sender-info"); + return state.Invalid(ValidationInvalidReason::TX_NOT_STANDARD, false, "evm-sender-info"); } EvmAddressWithNonce evmAddrAndNonce{txResult.nonce, txResult.address.c_str()}; @@ -1112,14 +1088,11 @@ static bool AcceptToMemoryPoolWorker(const CChainParams &chainparams, LogPrint(BCLog::MEMPOOL, "Too many replace-by-fee EVM tx from the same sender in mempool. Limit %d.\n", MEMPOOL_MAX_ETH_RBF); - return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, - false, - REJECT_INVALID, - "too-many-evm-rbf-txs-by-sender"); + return state.Invalid( + ValidationInvalidReason::TX_MEMPOOL_POLICY, false, "too-many-evm-rbf-txs-by-sender"); } else { LogPrint(BCLog::MEMPOOL, "EVM tx rejected due to same or lower fee as existing mempool entry\n"); - return state.Invalid( - ValidationInvalidReason::TX_MEMPOOL_POLICY, false, REJECT_INVALID, "evm-low-fee"); + return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, "evm-low-fee"); } } @@ -1128,8 +1101,7 @@ static bool AcceptToMemoryPoolWorker(const CChainParams &chainparams, LogPrint(BCLog::MEMPOOL, "Too many EVM tx from the same sender in mempool. Limit %d.\n", MEMPOOL_MAX_ETH_TXS); - return state.Invalid( - ValidationInvalidReason::TX_MEMPOOL_POLICY, false, REJECT_INVALID, "too-many-evm-txs-by-sender"); + return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, "too-many-evm-txs-by-sender"); } else { ethSender = txResultSender; } @@ -1178,8 +1150,7 @@ static bool AcceptToMemoryPoolWorker(const CChainParams &chainparams, gArgs.GetArg("-mempoolexpiry", DEFAULT_MEMPOOL_DVM_EXPIRY) * 60 * 60, gArgs.GetArg("-mempoolexpiryevm", DEFAULT_MEMPOOL_EVM_EXPIRY) * 60 * 60); if (!pool.exists(hash)) { - return state.Invalid( - ValidationInvalidReason::TX_MEMPOOL_POLICY, false, REJECT_INSUFFICIENTFEE, "mempool full"); + return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, "mempool full"); } } } @@ -1763,7 +1734,6 @@ bool CheckInputs(const CTransaction &tx, if (!CheckBurnSpend(tx, inputs)) { return (state.Invalid(ValidationInvalidReason::CONSENSUS, error("CheckBurnSpend: Trying to spend burnt outputs"), - REJECT_INVALID, "burnt-output")); } @@ -1833,7 +1803,6 @@ bool CheckInputs(const CTransaction &tx, if (check2()) { return state.Invalid(ValidationInvalidReason::TX_NOT_STANDARD, false, - REJECT_NONSTANDARD, strprintf("non-mandatory-script-verify-flag (%s)", ScriptErrorString(check.GetScriptError()))); } @@ -1849,7 +1818,6 @@ bool CheckInputs(const CTransaction &tx, // such errors). return state.Invalid(ValidationInvalidReason::CONSENSUS, false, - REJECT_INVALID, strprintf("mandatory-script-verify-flag-failed (%s)", ScriptErrorString(check.GetScriptError()))); } @@ -2631,10 +2599,8 @@ bool StopOrInterruptConnect(const CBlockIndex *pIndex, CValidationState &state) if (pIndex->nHeight == fStopBlockHeight) { StartShutdown(); } - state.Invalid(ValidationInvalidReason::CONSENSUS, - error("%s: user interrupt", __func__), - REJECT_INVALID, - "user-interrupt-request"); + state.Invalid( + ValidationInvalidReason::CONSENSUS, error("%s: user interrupt", __func__), "user-interrupt-request"); return true; } @@ -2801,7 +2767,6 @@ bool CChainState::ConnectBlock(const CBlock &block, nodeId->ToString(), nodePtr->mintedBlocks + 1, block.mintedBlocks), - REJECT_INVALID, "bad-minted-blocks"); } uint256 stakeModifierPrevBlock = pindex->pprev == nullptr ? uint256() : pindex->pprev->stakeModifier; @@ -2813,7 +2778,6 @@ bool CChainState::ConnectBlock(const CBlock &block, __func__, block.stakeModifier.ToString(), pos::ComputeStakeModifier(stakeModifierPrevBlock, nodePtr->operatorAuthAddress).ToString()), - REJECT_INVALID, "bad-minted-blocks"); } } @@ -2946,7 +2910,6 @@ bool CChainState::ConnectBlock(const CBlock &block, if (view.HaveCoin(COutPoint(tx->GetHash(), o))) { return state.Invalid(ValidationInvalidReason::CONSENSUS, error("%s: tried to overwrite transaction", __func__), - REJECT_INVALID, "bad-txns-BIP30"); } } @@ -3008,7 +2971,6 @@ bool CChainState::ConnectBlock(const CBlock &block, if (!xvmRes) { return state.Invalid(ValidationInvalidReason::CONSENSUS, error("%s: Failed to process XVM in coinbase", __func__), - REJECT_INVALID, "bad-xvm-coinbase"); } blockCtx.SetEVMTemplate( @@ -3020,7 +2982,6 @@ bool CChainState::ConnectBlock(const CBlock &block, if (!evmTemplate) { return state.Invalid(ValidationInvalidReason::CONSENSUS, error("%s: Failed to create block template", __func__), - REJECT_INVALID, "bad-evm-template"); } XResultThrowOnErr(evm_try_unsafe_update_state_in_template(result, evmTemplate->GetTemplate())); @@ -3096,11 +3057,8 @@ bool CChainState::ConnectBlock(const CBlock &block, // 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()); + state.Invalid( + ValidationInvalidReason::CONSENSUS, false, state.GetRejectReason(), state.GetDebugMessage()); } return error("%s: Consensus::CheckTxInputs: %s, %s", __func__, @@ -3111,7 +3069,6 @@ bool CChainState::ConnectBlock(const CBlock &block, if (!MoneyRange(nFees)) { return state.Invalid(ValidationInvalidReason::CONSENSUS, error("%s: accumulated fee in the block out of range.", __func__), - REJECT_INVALID, "bad-txns-accumulated-fee-outofrange"); } @@ -3126,7 +3083,6 @@ bool CChainState::ConnectBlock(const CBlock &block, if (!SequenceLocks(tx, nLockTimeFlags, &prevheights, *pindex)) { return state.Invalid(ValidationInvalidReason::CONSENSUS, error("%s: contains a non-BIP68-final transaction", __func__), - REJECT_INVALID, "bad-txns-nonfinal"); } } @@ -3137,10 +3093,8 @@ bool CChainState::ConnectBlock(const CBlock &block, // * witness (when witness enabled in flags and excludes coinbase) nSigOpsCost += GetTransactionSigOpCost(tx, view, flags); if (nSigOpsCost > MAX_BLOCK_SIGOPS_COST) { - return state.Invalid(ValidationInvalidReason::CONSENSUS, - error("%s: too many sigops", __func__), - REJECT_INVALID, - "bad-blk-sigops"); + return state.Invalid( + ValidationInvalidReason::CONSENSUS, error("%s: too many sigops", __func__), "bad-blk-sigops"); } txdata.emplace_back(tx); @@ -3164,11 +3118,8 @@ bool CChainState::ConnectBlock(const CBlock &block, // 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()); + state.Invalid( + ValidationInvalidReason::CONSENSUS, false, state.GetRejectReason(), state.GetDebugMessage()); } return error("%s: CheckInputs on %s failed with %s", __func__, @@ -3193,7 +3144,6 @@ bool CChainState::ConnectBlock(const CBlock &block, return state.Invalid( ValidationInvalidReason::CONSENSUS, error("%s: ApplyCustomTx on %s failed with %s", __func__, tx.GetHash().ToString(), res.msg), - REJECT_CUSTOMTX, "bad-custom-tx"); } else { // we will never fail, but skip, unless transaction mints UTXOs @@ -3222,10 +3172,8 @@ bool CChainState::ConnectBlock(const CBlock &block, } ResVal res = ApplyAnchorRewardTxPlus(mnview, tx, pindex->nHeight, metadata, consensus); if (!res.ok) { - return state.Invalid(ValidationInvalidReason::CONSENSUS, - error("%s: %s", __func__, res.msg), - REJECT_INVALID, - res.dbgMsg); + return state.Invalid( + ValidationInvalidReason::CONSENSUS, error("%s: %s", __func__, res.msg), res.dbgMsg); } rewardedAnchors = true; if (!fJustCheck) { @@ -3250,10 +3198,8 @@ bool CChainState::ConnectBlock(const CBlock &block, metadata, consensus); if (!res.ok) { - return state.Invalid(ValidationInvalidReason::CONSENSUS, - error("%s: %s", __func__, res.msg), - REJECT_INVALID, - res.dbgMsg); + return state.Invalid( + ValidationInvalidReason::CONSENSUS, error("%s: %s", __func__, res.msg), res.dbgMsg); } rewardedAnchors = true; if (!fJustCheck) { @@ -3305,15 +3251,12 @@ bool CChainState::ConnectBlock(const CBlock &block, // check main coinbase Res res = ApplyGeneralCoinbaseTx(accountsView, *block.vtx[0], pindex->nHeight, nFees, consensus); if (!res.ok) { - return state.Invalid( - ValidationInvalidReason::CONSENSUS, error("%s: %s", __func__, res.msg), REJECT_INVALID, res.dbgMsg); + return state.Invalid(ValidationInvalidReason::CONSENSUS, error("%s: %s", __func__, res.msg), res.dbgMsg); } if (!control.Wait()) { - return state.Invalid(ValidationInvalidReason::CONSENSUS, - error("%s: CheckQueue failed", __func__), - REJECT_INVALID, - "block-validation-failed"); + return state.Invalid( + ValidationInvalidReason::CONSENSUS, error("%s: CheckQueue failed", __func__), "block-validation-failed"); } int64_t nTime4 = GetTimeMicros(); @@ -3346,7 +3289,6 @@ bool CChainState::ConnectBlock(const CBlock &block, if (!GetCreationTransactions(block, id, multiplier, tokenCreationTx, poolCreationTx)) { return state.Invalid(ValidationInvalidReason::CONSENSUS, error("%s: coinbase missing split token creation TX", __func__), - REJECT_INVALID, "bad-cb-token-split"); } @@ -3378,7 +3320,6 @@ bool CChainState::ConnectBlock(const CBlock &block, if (poolsToMigrate.size() != poolCreationTx.size()) { return state.Invalid(ValidationInvalidReason::CONSENSUS, error("%s: coinbase missing split pool creation TX", __func__), - REJECT_INVALID, "bad-cb-pool-split"); } @@ -3402,22 +3343,16 @@ bool CChainState::ConnectBlock(const CBlock &block, bool mutated; uint256 hashMerkleRoot2 = BlockMerkleRoot(block, &mutated); if (block.hashMerkleRoot != Hash2(hashMerkleRoot2, accountsView.MerkleRoot())) { - return state.Invalid(ValidationInvalidReason::BLOCK_MUTATED, - false, - REJECT_INVALID, - "bad-txnmrklroot", - "hashMerkleRoot mismatch"); + return state.Invalid( + ValidationInvalidReason::BLOCK_MUTATED, false, "bad-txnmrklroot", "hashMerkleRoot mismatch"); } // Check for merkle tree malleability (CVE-2012-2459): repeating sequences // of transactions in a block without affecting the merkle root of a block, // while still invalidating it. if (mutated) { - return state.Invalid(ValidationInvalidReason::BLOCK_MUTATED, - false, - REJECT_INVALID, - "bad-txns-duplicate", - "duplicate transaction"); + return state.Invalid( + ValidationInvalidReason::BLOCK_MUTATED, false, "bad-txns-duplicate", "duplicate transaction"); } } @@ -3427,8 +3362,7 @@ bool CChainState::ConnectBlock(const CBlock &block, // Execute EVM Queue res = ProcessDeFiEventFallible(block, pindex, mnview, chainparams, evmTemplate, isEvmEnabledForBlock); if (!res.ok) { - return state.Invalid( - ValidationInvalidReason::CONSENSUS, error("%s: %s", __func__, res.msg), REJECT_INVALID, res.dbgMsg); + return state.Invalid(ValidationInvalidReason::CONSENSUS, error("%s: %s", __func__, res.msg), res.dbgMsg); } if (!WriteUndoDataForBlock(blockundo, state, pindex, chainparams)) { @@ -4234,7 +4168,7 @@ bool CChainState::ActivateBestChainStep(CValidationState &state, if (state.GetRejectReason() == "high-hash" || (pindexConnect == pindexMostWork && pindexConnect->nHeight >= chainparams.GetConsensus().DF13FortCanningParkHeight && - state.GetRejectCode() == REJECT_CUSTOMTX)) { + state.GetDebugMessage() == "bad-custom-tx")) { UpdateMempoolForReorg(disconnectpool, false); return false; } @@ -4257,7 +4191,7 @@ bool CChainState::ActivateBestChainStep(CValidationState &state, } if (pindexConnect == pindexMostWork && (pindexConnect->nHeight < chainparams.GetConsensus().DF8EunosHeight || - state.GetRejectCode() == REJECT_CUSTOMTX)) { + state.GetDebugMessage() == "bad-custom-tx")) { // NOTE: Invalidate blocks back to last checkpoint auto &checkpoints = chainparams.Checkpoints().mapCheckpoints; // calculate the latest suitable checkpoint block height @@ -4533,7 +4467,6 @@ bool CChainState::InvalidateBlock(CValidationState &state, const CChainParams &c if (pcheckpoint && pindex->nHeight <= pcheckpoint->nHeight) { return state.Invalid(ValidationInvalidReason::BLOCK_CHECKPOINT, error("Cannot invalidate block prior last checkpoint height %d", pcheckpoint->nHeight), - REJECT_CHECKPOINT, ""); } @@ -4880,7 +4813,7 @@ bool CheckBlock(const CBlock &block, if (!fIsFakeNet && fCheckPOS && !pos::ContextualCheckProofOfStake(block, consensusParams, pcustomcsview.get(), ctxState, height)) { return state.Invalid( - ValidationInvalidReason::BLOCK_INVALID_HEADER, false, REJECT_INVALID, "high-hash", "proof of stake failed"); + ValidationInvalidReason::BLOCK_INVALID_HEADER, false, "high-hash", "proof of stake failed"); } // Check the merkle root. @@ -4890,22 +4823,16 @@ bool CheckBlock(const CBlock &block, bool mutated; uint256 hashMerkleRoot2 = BlockMerkleRoot(block, &mutated); if (block.hashMerkleRoot != hashMerkleRoot2) { - return state.Invalid(ValidationInvalidReason::BLOCK_MUTATED, - false, - REJECT_INVALID, - "bad-txnmrklroot", - "hashMerkleRoot mismatch"); + return state.Invalid( + ValidationInvalidReason::BLOCK_MUTATED, false, "bad-txnmrklroot", "hashMerkleRoot mismatch"); } // Check for merkle tree malleability (CVE-2012-2459): repeating sequences // of transactions in a block without affecting the merkle root of a block, // while still invalidating it. if (mutated) { - return state.Invalid(ValidationInvalidReason::BLOCK_MUTATED, - false, - REJECT_INVALID, - "bad-txns-duplicate", - "duplicate transaction"); + return state.Invalid( + ValidationInvalidReason::BLOCK_MUTATED, false, "bad-txns-duplicate", "duplicate transaction"); } } @@ -4919,14 +4846,12 @@ bool CheckBlock(const CBlock &block, if (block.vtx.empty() || block.vtx.size() * WITNESS_SCALE_FACTOR > MAX_BLOCK_WEIGHT || ::GetSerializeSize(block, PROTOCOL_VERSION | SERIALIZE_TRANSACTION_NO_WITNESS) * WITNESS_SCALE_FACTOR > MAX_BLOCK_WEIGHT) { - return state.Invalid( - ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-blk-length", "size limits failed"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-blk-length", "size limits failed"); } // First transaction must be coinbase, the rest must not be if (block.vtx.empty() || !block.vtx[0]->IsCoinBase()) { - return state.Invalid( - ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-cb-missing", "first tx is not coinbase"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-cb-missing", "first tx is not coinbase"); } // skip this validation if it is Genesis (due to mn creation txs) @@ -4937,11 +4862,8 @@ bool CheckBlock(const CBlock &block, !IsAnchorRewardTx(*block.vtx[i], dummy, height >= consensusParams.DF11FortCanningHeight) && !IsAnchorRewardTxPlus(*block.vtx[i], dummy, height >= consensusParams.DF11FortCanningHeight) && !IsTokenSplitTx(*block.vtx[i], dummy, height >= consensusParams.DF16FortCanningCrunchHeight)) { - return state.Invalid(ValidationInvalidReason::CONSENSUS, - false, - REJECT_INVALID, - "bad-cb-multiple", - "more than one coinbase"); + return state.Invalid( + ValidationInvalidReason::CONSENSUS, false, "bad-cb-multiple", "more than one coinbase"); } } } @@ -4954,7 +4876,6 @@ bool CheckBlock(const CBlock &block, return state.Invalid( state.GetReason(), false, - state.GetRejectCode(), state.GetRejectReason(), strprintf( "Transaction check failed (tx hash %s) %s", tx->GetHash().ToString(), state.GetDebugMessage())); @@ -4979,11 +4900,8 @@ bool CheckBlock(const CBlock &block, } if (block.vtx[0]->vout[0].scriptPubKey != GetScriptForDestination(destination)) { - return state.Invalid(ValidationInvalidReason::BLOCK_INVALID_HEADER, - false, - REJECT_INVALID, - "bad-rewardaddress", - "proof of stake failed"); + return state.Invalid( + ValidationInvalidReason::BLOCK_INVALID_HEADER, false, "bad-rewardaddress", "proof of stake failed"); } } } @@ -4993,8 +4911,7 @@ bool CheckBlock(const CBlock &block, nSigOps += GetLegacySigOpCount(*tx); } if (nSigOps * WITNESS_SCALE_FACTOR > MAX_BLOCK_SIGOPS_COST) { - return state.Invalid( - ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-blk-sigops", "out-of-bounds SigOpCount"); + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-blk-sigops", "out-of-bounds SigOpCount"); } if (fCheckPOS && fCheckMerkleRoot) { @@ -5090,7 +5007,6 @@ static bool ContextualCheckBlockHeader(const CBlockHeader &block, static_cast(nHeight) != block.deprecatedHeight) { return state.Invalid(ValidationInvalidReason::BLOCK_INVALID_HEADER, false, - REJECT_INVALID, "incorrect-height", "incorrect height set in block header"); } @@ -5098,11 +5014,8 @@ static bool ContextualCheckBlockHeader(const CBlockHeader &block, // Check proof of work const Consensus::Params &consensusParams = params.GetConsensus(); if (block.nBits != pos::GetNextWorkRequired(pindexPrev, block.nTime, consensusParams)) { - return state.Invalid(ValidationInvalidReason::BLOCK_INVALID_HEADER, - false, - REJECT_INVALID, - "bad-diffbits", - "incorrect proof of work"); + return state.Invalid( + ValidationInvalidReason::BLOCK_INVALID_HEADER, false, "bad-diffbits", "incorrect proof of work"); } // Check against checkpoints @@ -5113,7 +5026,6 @@ static bool ContextualCheckBlockHeader(const CBlockHeader &block, if (pcheckpoint && nHeight <= pcheckpoint->nHeight) { 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"); } @@ -5121,7 +5033,6 @@ static bool ContextualCheckBlockHeader(const CBlockHeader &block, if (block.GetBlockTime() <= pindexPrev->GetMedianTimePast()) { return state.Invalid(ValidationInvalidReason::BLOCK_INVALID_HEADER, false, - REJECT_INVALID, "time-too-old", strprintf("block's timestamp is too early. Block time: %d Min time: %d", block.GetBlockTime(), @@ -5133,7 +5044,6 @@ static bool ContextualCheckBlockHeader(const CBlockHeader &block, if (block.GetBlockTime() > GetTime() + MAX_FUTURE_BLOCK_TIME_EUNOSPAYA) { return state.Invalid(ValidationInvalidReason::BLOCK_TIME_FUTURE, false, - REJECT_INVALID, "time-too-new", strprintf("block timestamp too far in the future. Block time: %d Max time: %d", block.GetBlockTime(), @@ -5142,18 +5052,14 @@ static bool ContextualCheckBlockHeader(const CBlockHeader &block, } if (block.GetBlockTime() > nAdjustedTime + MAX_FUTURE_BLOCK_TIME) { - return state.Invalid(ValidationInvalidReason::BLOCK_TIME_FUTURE, - false, - REJECT_INVALID, - "time-too-new", - "block timestamp too far in the future"); + return state.Invalid( + ValidationInvalidReason::BLOCK_TIME_FUTURE, false, "time-too-new", "block timestamp too far in the future"); } if (nHeight >= consensusParams.DF7DakotaCrescentHeight) { if (block.GetBlockTime() > GetTime() + MAX_FUTURE_BLOCK_TIME_DAKOTACRESCENT) { return state.Invalid(ValidationInvalidReason::BLOCK_TIME_FUTURE, false, - REJECT_INVALID, "time-too-new", strprintf("block timestamp too far in the future. Block time: %d Max time: %d", block.GetBlockTime(), @@ -5168,7 +5074,6 @@ static bool ContextualCheckBlockHeader(const CBlockHeader &block, (block.nVersion < 4 && nHeight >= consensusParams.BIP65Height)) { 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)); } @@ -5201,11 +5106,8 @@ static bool ContextualCheckBlock(const CBlock &block, // Check that all transactions are finalized for (const auto &tx : block.vtx) { if (!IsFinalTx(*tx, nHeight, nLockTimeCutoff)) { - return state.Invalid(ValidationInvalidReason::CONSENSUS, - false, - REJECT_INVALID, - "bad-txns-nonfinal", - "non-final transaction"); + return state.Invalid( + ValidationInvalidReason::CONSENSUS, false, "bad-txns-nonfinal", "non-final transaction"); } } @@ -5214,11 +5116,8 @@ static bool ContextualCheckBlock(const CBlock &block, CScript expect = CScript() << nHeight; if (block.vtx[0]->vin[0].scriptSig.size() < expect.size() || !std::equal(expect.begin(), expect.end(), block.vtx[0]->vin[0].scriptSig.begin())) { - return state.Invalid(ValidationInvalidReason::CONSENSUS, - false, - REJECT_INVALID, - "bad-cb-height", - "block height mismatch in coinbase"); + return state.Invalid( + ValidationInvalidReason::CONSENSUS, false, "bad-cb-height", "block height mismatch in coinbase"); } } @@ -5245,7 +5144,6 @@ static bool ContextualCheckBlock(const CBlock &block, block.vtx[0]->vin[0].scriptWitness.stack[0].size() != 32) { return state.Invalid(ValidationInvalidReason::BLOCK_MUTATED, false, - REJECT_INVALID, "bad-witness-nonce-size", strprintf("%s : invalid witness reserved value size", __func__)); } @@ -5256,7 +5154,6 @@ static bool ContextualCheckBlock(const CBlock &block, if (memcmp(hashWitness.begin(), &block.vtx[0]->vout[commitpos].scriptPubKey[6], 32)) { return state.Invalid(ValidationInvalidReason::BLOCK_MUTATED, false, - REJECT_INVALID, "bad-witness-merkle-match", strprintf("%s : witness merkle commitment mismatch", __func__)); } @@ -5271,7 +5168,6 @@ static bool ContextualCheckBlock(const CBlock &block, if (tx->HasWitness()) { return state.Invalid(ValidationInvalidReason::BLOCK_MUTATED, false, - REJECT_INVALID, "unexpected-witness", strprintf("%s : unexpected witness data found", __func__)); } @@ -5287,7 +5183,6 @@ static bool ContextualCheckBlock(const CBlock &block, if (GetBlockWeight(block) > MAX_BLOCK_WEIGHT) { return state.Invalid(ValidationInvalidReason::CONSENSUS, false, - REJECT_INVALID, "bad-blk-weight", strprintf("%s : weight limit failed", __func__)); } @@ -5314,7 +5209,6 @@ bool BlockManager::AcceptBlockHeader(const CBlockHeader &block, if (pindex->nStatus & BLOCK_FAILED_MASK) { return state.Invalid(ValidationInvalidReason::CACHED_INVALID, error("%s: block %s is marked invalid", __func__, hash.ToString()), - 0, "duplicate"); } return true; @@ -5325,7 +5219,6 @@ bool BlockManager::AcceptBlockHeader(const CBlockHeader &block, error("%s: Consensus::CheckHeaderSignature: block %s: bad-pos-header-signature", __func__, hash.ToString()), - REJECT_INVALID, "bad-pos-header-signature"); } @@ -5335,15 +5228,12 @@ bool BlockManager::AcceptBlockHeader(const CBlockHeader &block, if (mi == m_block_index.end()) { return state.Invalid(ValidationInvalidReason::BLOCK_MISSING_PREV, error("%s: prev block not found", __func__), - 0, "prev-blk-not-found"); } pindexPrev = (*mi).second; if (pindexPrev->nStatus & BLOCK_FAILED_MASK) { - return state.Invalid(ValidationInvalidReason::BLOCK_INVALID_PREV, - error("%s: prev block invalid", __func__), - REJECT_INVALID, - "bad-prevblk"); + return state.Invalid( + ValidationInvalidReason::BLOCK_INVALID_PREV, error("%s: prev block invalid", __func__), "bad-prevblk"); } if (!ContextualCheckBlockHeader(block, state, chainparams, pindexPrev, GetAdjustedTime())) { return error("%s: Consensus::ContextualCheckBlockHeader: %s, %s", @@ -5356,7 +5246,6 @@ bool BlockManager::AcceptBlockHeader(const CBlockHeader &block, if (!fIsFakeNet && !pos::CheckStakeModifier(pindexPrev, block)) { return state.Invalid(ValidationInvalidReason::BLOCK_INVALID_HEADER, error("%s: block %s: bad PoS stake modifier", __func__, hash.ToString()), - REJECT_INVALID, "bad-stakemodifier"); } @@ -5395,7 +5284,6 @@ bool BlockManager::AcceptBlockHeader(const CBlockHeader &block, } return state.Invalid(ValidationInvalidReason::BLOCK_INVALID_PREV, error("%s: prev block invalid", __func__), - REJECT_INVALID, "bad-prevblk"); } } @@ -5744,7 +5632,6 @@ bool ProcessNewBlock(const CChainParams &chainparams, ret = false; state.Invalid(ValidationInvalidReason::BLOCK_MISSING_PREV, error("%s: prev block not found", __func__), - 0, "prev-blk-not-found"); } diff --git a/src/validation.h b/src/validation.h index d758407000..c968d59849 100644 --- a/src/validation.h +++ b/src/validation.h @@ -116,8 +116,6 @@ static const unsigned int BLOCK_DOWNLOAD_WINDOW = 1024; static const unsigned int DATABASE_WRITE_INTERVAL = 60 * 60; /** Time to wait (in seconds) between flushing chainstate to disk. */ static const unsigned int DATABASE_FLUSH_INTERVAL = 24 * 60 * 60; -/** Maximum length of reject messages. */ -static const unsigned int MAX_REJECT_MESSAGE_LENGTH = 111; /** Block download timeout base, expressed in millionths of the block interval (i.e. 10 min) */ static const int64_t BLOCK_DOWNLOAD_TIMEOUT_BASE = 1000000; /** Additional block download timeout per parallel downloading peer (i.e. 5 min) */ @@ -902,14 +900,6 @@ extern VersionBitsCache versionbitscache; */ int32_t ComputeBlockVersion(const CBlockIndex *pindexPrev, const Consensus::Params ¶ms); -/** Reject codes greater or equal to this can be returned by AcceptToMemPool - * for transactions, to signal internal conditions. They cannot and should not - * be sent over the P2P network. - */ -static const unsigned int REJECT_INTERNAL = 0x100; -/** Too high fee. Can not be triggered by P2P transactions */ -static const unsigned int REJECT_HIGHFEE = 0x100; - /** Get block file info entry for one block file */ CBlockFileInfo *GetBlockFileInfo(size_t n); diff --git a/test/functional/feature_bip68_sequence.py b/test/functional/feature_bip68_sequence.py index 23e7c3b599..c6a02adcd7 100755 --- a/test/functional/feature_bip68_sequence.py +++ b/test/functional/feature_bip68_sequence.py @@ -36,7 +36,7 @@ SEQUENCE_LOCKTIME_MASK = 0x0000FFFF # RPC error for non-BIP68 final transactions -NOT_FINAL_ERROR = "non-BIP68-final (code 64)" +NOT_FINAL_ERROR = "non-BIP68-final" class BIP68Test(DefiTestFramework): diff --git a/test/functional/feature_cltv.py b/test/functional/feature_cltv.py index c51372da03..4b9a84e7ae 100755 --- a/test/functional/feature_cltv.py +++ b/test/functional/feature_cltv.py @@ -28,10 +28,6 @@ CLTV_HEIGHT = 1351 -# Reject codes that we might receive in this test -REJECT_INVALID = 16 -REJECT_NONSTANDARD = 64 - def cltv_invalidate(tx): """Modify the signature in vin 0 of the tx to fail CLTV @@ -145,7 +141,7 @@ def run_test(self): { "txid": spendtx.hash, "allowed": False, - "reject-reason": "64: non-mandatory-script-verify-flag (Negative locktime)", + "reject-reason": "non-mandatory-script-verify-flag (Negative locktime)", } ], self.nodes[0].testmempoolaccept( diff --git a/test/functional/feature_dersig.py b/test/functional/feature_dersig.py index 467a0b7b87..17354a4b06 100755 --- a/test/functional/feature_dersig.py +++ b/test/functional/feature_dersig.py @@ -9,20 +9,15 @@ from test_framework.blocktools import create_coinbase, create_block, create_transaction from test_framework.messages import msg_block -from test_framework.mininode import mininode_lock, P2PInterface +from test_framework.mininode import P2PInterface from test_framework.script import CScript from test_framework.test_framework import DefiTestFramework from test_framework.util import ( assert_equal, - wait_until, ) DERSIG_HEIGHT = 1251 -# Reject codes that we might receive in this test -REJECT_INVALID = 16 -REJECT_NONSTANDARD = 64 - # A canonical signature consists of: # <30> <02> <02> @@ -45,8 +40,8 @@ class BIP66Test(DefiTestFramework): def set_test_params(self): self.num_nodes = 1 self.extra_args = [ - ["-whitelist=127.0.0.1", "-par=1", "-enablebip61"] - ] # Use only one script thread to get the exact reject reason for testing + ["-whitelist=127.0.0.1", "-par=1"] + ] # Use only one script thread to get the exact log msg for testing self.setup_clean_chain = True self.rpc_timeout = 120 @@ -120,7 +115,7 @@ def run_test(self): { "txid": spendtx.hash, "allowed": False, - "reject-reason": "64: non-mandatory-script-verify-flag (Non-canonical DER signature)", + "reject-reason": "non-mandatory-script-verify-flag (Non-canonical DER signature)", } ], self.nodes[0].testmempoolaccept( @@ -145,21 +140,6 @@ def run_test(self): assert_equal(int(self.nodes[0].getbestblockhash(), 16), tip) self.nodes[0].p2p.sync_with_ping() - wait_until( - lambda: "reject" in self.nodes[0].p2p.last_message.keys(), - lock=mininode_lock, - ) - with mininode_lock: - assert self.nodes[0].p2p.last_message["reject"].code in [ - REJECT_INVALID, - REJECT_NONSTANDARD, - ] - assert_equal(self.nodes[0].p2p.last_message["reject"].data, block.sha256) - assert ( - b"Non-canonical DER signature" - in self.nodes[0].p2p.last_message["reject"].reason - ) - self.log.info( "Test that a version 3 block with a DERSIG-compliant transaction is accepted" ) diff --git a/test/functional/feature_nulldummy.py b/test/functional/feature_nulldummy.py index 6045833329..f58b1d7449 100755 --- a/test/functional/feature_nulldummy.py +++ b/test/functional/feature_nulldummy.py @@ -23,7 +23,9 @@ from test_framework.test_framework import DefiTestFramework from test_framework.util import assert_equal, assert_raises_rpc_error -NULLDUMMY_ERROR = "non-mandatory-script-verify-flag (Dummy CHECKMULTISIG argument must be zero) (code 64)" +NULLDUMMY_ERROR = ( + "non-mandatory-script-verify-flag (Dummy CHECKMULTISIG argument must be zero)" +) def trueDummy(tx): diff --git a/test/functional/feature_segwit.py b/test/functional/feature_segwit.py index 45411cfe01..cec82b4d11 100755 --- a/test/functional/feature_segwit.py +++ b/test/functional/feature_segwit.py @@ -342,26 +342,26 @@ def run_test(self): ) self.fail_accept( self.nodes[2], - "non-mandatory-script-verify-flag (Witness program hash mismatch) (code 64)", + "non-mandatory-script-verify-flag (Witness program hash mismatch)", wit_ids[NODE_2][WIT_V0][2], sign=False, ) self.fail_accept( self.nodes[2], - "non-mandatory-script-verify-flag (Witness program was passed an empty witness) (code 64)", + "non-mandatory-script-verify-flag (Witness program was passed an empty witness)", wit_ids[NODE_2][WIT_V1][2], sign=False, ) self.fail_accept( self.nodes[2], - "non-mandatory-script-verify-flag (Witness program hash mismatch) (code 64)", + "non-mandatory-script-verify-flag (Witness program hash mismatch)", p2sh_ids[NODE_2][WIT_V0][2], sign=False, redeem_script=witness_script(False, self.pubkey[2]), ) self.fail_accept( self.nodes[2], - "non-mandatory-script-verify-flag (Witness program was passed an empty witness) (code 64)", + "non-mandatory-script-verify-flag (Witness program was passed an empty witness)", p2sh_ids[NODE_2][WIT_V1][2], sign=False, redeem_script=witness_script(True, self.pubkey[2]), diff --git a/test/functional/mempool_accept.py b/test/functional/mempool_accept.py index 12ecccbffc..5eda85d08a 100755 --- a/test/functional/mempool_accept.py +++ b/test/functional/mempool_accept.py @@ -92,7 +92,7 @@ def run_test(self): { "txid": txid_in_block, "allowed": False, - "reject-reason": "18: txn-already-known", + "reject-reason": "txn-already-known", } ], rawtxs=[raw_tx_in_block], @@ -148,7 +148,7 @@ def run_test(self): { "txid": txid_0, "allowed": False, - "reject-reason": "18: txn-already-in-mempool", + "reject-reason": "txn-already-in-mempool", } ], rawtxs=[raw_tx_0], @@ -178,7 +178,7 @@ def run_test(self): { "txid": tx.rehash(), "allowed": False, - "reject-reason": "18: txn-mempool-conflict", + "reject-reason": "txn-mempool-conflict", } ], rawtxs=[tx.serialize().hex()], @@ -262,7 +262,7 @@ def run_test(self): { "txid": tx.rehash(), "allowed": False, - "reject-reason": "16: bad-txns-vout-empty", + "reject-reason": "bad-txns-vout-empty", } ], rawtxs=[tx.serialize().hex()], @@ -280,7 +280,7 @@ def run_test(self): { "txid": tx.rehash(), "allowed": False, - "reject-reason": "16: bad-txns-oversize", + "reject-reason": "bad-txns-oversize", } ], rawtxs=[tx.serialize().hex()], @@ -294,7 +294,7 @@ def run_test(self): { "txid": tx.rehash(), "allowed": False, - "reject-reason": "16: bad-txns-vout-negative", + "reject-reason": "bad-txns-vout-negative", } ], rawtxs=[tx.serialize().hex()], @@ -308,7 +308,7 @@ def run_test(self): { "txid": tx.rehash(), "allowed": False, - "reject-reason": "16: bad-txns-vout-toolarge", + "reject-reason": "bad-txns-vout-toolarge", } ], rawtxs=[tx.serialize().hex()], @@ -323,7 +323,7 @@ def run_test(self): { "txid": tx.rehash(), "allowed": False, - "reject-reason": "16: bad-txns-txouttotal-toolarge", + "reject-reason": "bad-txns-txouttotal-toolarge", } ], rawtxs=[tx.serialize().hex()], @@ -337,7 +337,7 @@ def run_test(self): { "txid": tx.rehash(), "allowed": False, - "reject-reason": "16: bad-txns-inputs-duplicate", + "reject-reason": "bad-txns-inputs-duplicate", } ], rawtxs=[tx.serialize().hex()], @@ -351,7 +351,7 @@ def run_test(self): tx.deserialize(BytesIO(hex_str_to_bytes(raw_tx_coinbase_spent))) self.check_mempool_result( result_expected=[ - {"txid": tx.rehash(), "allowed": False, "reject-reason": "16: coinbase"} + {"txid": tx.rehash(), "allowed": False, "reject-reason": "coinbase"} ], rawtxs=[tx.serialize().hex()], ) @@ -361,7 +361,7 @@ def run_test(self): tx.nVersion = 3 # A version currently non-standard self.check_mempool_result( result_expected=[ - {"txid": tx.rehash(), "allowed": False, "reject-reason": "64: version"} + {"txid": tx.rehash(), "allowed": False, "reject-reason": "version"} ], rawtxs=[tx.serialize().hex()], ) @@ -372,7 +372,7 @@ def run_test(self): { "txid": tx.rehash(), "allowed": False, - "reject-reason": "64: scriptpubkey", + "reject-reason": "scriptpubkey", } ], rawtxs=[tx.serialize().hex()], @@ -384,7 +384,7 @@ def run_test(self): { "txid": tx.rehash(), "allowed": False, - "reject-reason": "64: scriptsig-not-pushonly", + "reject-reason": "scriptsig-not-pushonly", } ], rawtxs=[tx.serialize().hex()], @@ -399,7 +399,7 @@ def run_test(self): tx.vout = [output_p2sh_burn] * num_scripts self.check_mempool_result( result_expected=[ - {"txid": tx.rehash(), "allowed": False, "reject-reason": "64: tx-size"} + {"txid": tx.rehash(), "allowed": False, "reject-reason": "tx-size"} ], rawtxs=[tx.serialize().hex()], ) @@ -410,7 +410,7 @@ def run_test(self): ].nValue -= 1 # Make output smaller, such that it is dust for our policy self.check_mempool_result( result_expected=[ - {"txid": tx.rehash(), "allowed": False, "reject-reason": "64: dust"} + {"txid": tx.rehash(), "allowed": False, "reject-reason": "dust"} ], rawtxs=[tx.serialize().hex()], ) @@ -422,7 +422,7 @@ def run_test(self): { "txid": tx.rehash(), "allowed": False, - "reject-reason": "64: multi-op-return", + "reject-reason": "multi-op-return", } ], rawtxs=[tx.serialize().hex()], @@ -437,7 +437,7 @@ def run_test(self): { "txid": tx.rehash(), "allowed": False, - "reject-reason": "64: non-final", + "reject-reason": "non-final", } ], rawtxs=[tx.serialize().hex()], @@ -454,7 +454,7 @@ def run_test(self): { "txid": tx.rehash(), "allowed": False, - "reject-reason": "64: non-BIP68-final", + "reject-reason": "non-BIP68-final", } ], rawtxs=[tx.serialize().hex()], diff --git a/test/functional/rpc_rawtransaction.py b/test/functional/rpc_rawtransaction.py index f13b5a8f4d..29808073b7 100755 --- a/test/functional/rpc_rawtransaction.py +++ b/test/functional/rpc_rawtransaction.py @@ -823,7 +823,7 @@ def run_test(self): # Thus, testmempoolaccept should reject testres = self.nodes[2].testmempoolaccept([rawTxSigned["hex"]], 0.00001000)[0] assert_equal(testres["allowed"], False) - assert_equal(testres["reject-reason"], "256: absurdly-high-fee") + assert_equal(testres["reject-reason"], "absurdly-high-fee") # and sendrawtransaction should throw assert_raises_rpc_error( -26, diff --git a/test/functional/test_framework/messages.py b/test/functional/test_framework/messages.py index 8b887e83b4..7a7ea59902 100755 --- a/test/functional/test_framework/messages.py +++ b/test/functional/test_framework/messages.py @@ -1552,45 +1552,6 @@ def __repr__(self): return "msg_headers(headers=%s)" % repr(self.headers) -class msg_reject: - __slots__ = ("code", "data", "message", "reason") - command = b"reject" - REJECT_MALFORMED = 1 - - def __init__(self): - self.message = b"" - self.code = 0 - self.reason = b"" - self.data = 0 - - def deserialize(self, f): - self.message = deser_string(f) - self.code = struct.unpack("