From 038d8044fd50e41887407f2245d4d7cc5c0db089 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kittywhiskers@users.noreply.github.com> Date: Fri, 2 Dec 2022 15:43:01 +0530 Subject: [PATCH] merge bitcoin#15437: Remove BIP61 reject messages --- doc/bips.md | 2 +- doc/man/dash-qt.1 | 4 - doc/man/dashd.1 | 4 - doc/release-notes-4892.md | 34 ++++++++ src/coinjoin/client.cpp | 6 +- src/coinjoin/client.h | 4 +- src/init.cpp | 5 +- src/net_processing.cpp | 90 +++------------------- src/net_processing.h | 9 +-- src/protocol.cpp | 2 - src/protocol.h | 7 -- src/test/denialofservice_tests.cpp | 10 +-- src/test/fuzz/process_message.cpp | 4 +- src/test/util/setup_common.cpp | 2 +- src/validation.h | 2 - test/functional/feature_cltv.py | 4 - test/functional/feature_dersig.py | 15 +--- test/functional/test_framework/messages.py | 33 -------- test/functional/test_framework/mininode.py | 2 - 19 files changed, 64 insertions(+), 175 deletions(-) create mode 100644 doc/release-notes-4892.md diff --git a/doc/bips.md b/doc/bips.md index 3f2d83213a..50e9cb7c3f 100644 --- a/doc/bips.md +++ b/doc/bips.md @@ -15,7 +15,7 @@ BIPs that are implemented by Dash Core (up-to-date up to **v18.0**): * [`BIP 35`](https://github.com/bitcoin/bips/blob/master/bip-0035.mediawiki): The 'mempool' protocol message (and the protocol version bump to 60002) has been implemented since **v0.7.0** ([PR #1641](https://github.com/bitcoin/bitcoin/pull/1641)). As of **v0.13.0**, this is only available for `NODE_BLOOM` (BIP 111) peers. * [`BIP 37`](https://github.com/bitcoin/bips/blob/master/bip-0037.mediawiki): The bloom filtering for transaction relaying, partial Merkle trees for blocks, and the protocol version bump to 70001 (enabling low-bandwidth SPV clients) has been implemented since **v0.8.0** ([PR #1795](https://github.com/bitcoin/bitcoin/pull/1795)). * [`BIP 42`](https://github.com/bitcoin/bips/blob/master/bip-0042.mediawiki): The bug that would have caused the subsidy schedule to resume after block 13440000 was fixed in **v0.9.2** ([PR #3842](https://github.com/bitcoin/bitcoin/pull/3842)). -* [`BIP 61`](https://github.com/bitcoin/bips/blob/master/bip-0061.mediawiki): The 'reject' protocol message (and the protocol version bump to 70002) was added in **v0.9.0** ([PR #3185](https://github.com/bitcoin/bitcoin/pull/3185)). Starting *v0.16.0*, whether to send reject messages can be configured with the `-enablebip61` option. +* [`BIP 61`](https://github.com/bitcoin/bips/blob/master/bip-0061.mediawiki): The 'reject' protocol message (and the protocol version bump to 70002) was added in **v0.9.0** ([PR #3185](https://github.com/bitcoin/bitcoin/pull/3185)). Starting *v0.16.0*, whether to send reject messages can be configured with the `-enablebip61` option. Support was removed in **v0.20.0** ([PR #15437](https://github.com/bitcoin/bitcoin/pull/15437)). * [`BIP 65`](https://github.com/bitcoin/bips/blob/master/bip-0065.mediawiki): The CHECKLOCKTIMEVERIFY softfork was merged in **v0.12.0** ([PR #6351](https://github.com/bitcoin/bitcoin/pull/6351)), and backported to **v0.11.2** and **v0.10.4**. Mempool-only CLTV was added in [PR #6124](https://github.com/bitcoin/bitcoin/pull/6124). * [`BIP 66`](https://github.com/bitcoin/bips/blob/master/bip-0066.mediawiki): The strict DER rules and associated version 3 blocks have been implemented since **v0.10.0** ([PR #5713](https://github.com/bitcoin/bitcoin/pull/5713)). * [`BIP 68`](https://github.com/bitcoin/bips/blob/master/bip-0068.mediawiki): Sequence locks have been implemented as of **v0.12.1** ([PR #7184](https://github.com/bitcoin/bitcoin/pull/7184)), and have been activated since *block 419328*. diff --git a/doc/man/dash-qt.1 b/doc/man/dash-qt.1 index b231ebb7bb..c5991786e3 100644 --- a/doc/man/dash-qt.1 +++ b/doc/man/dash-qt.1 @@ -199,10 +199,6 @@ Allow DNS lookups for \fB\-addnode\fR, \fB\-seednode\fR and \fB\-connect\fR (def Query for peer addresses via DNS lookup, if low on addresses (default: 1 unless \fB\-connect\fR used) .HP -\fB\-enablebip61\fR -.IP -Send reject messages per BIP61 (default: 1) -.HP \fB\-externalip=\fR .IP Specify your own public address diff --git a/doc/man/dashd.1 b/doc/man/dashd.1 index 067a3de68f..48fd4703c0 100644 --- a/doc/man/dashd.1 +++ b/doc/man/dashd.1 @@ -199,10 +199,6 @@ Allow DNS lookups for \fB\-addnode\fR, \fB\-seednode\fR and \fB\-connect\fR (def Query for peer addresses via DNS lookup, if low on addresses (default: 1 unless \fB\-connect\fR used) .HP -\fB\-enablebip61\fR -.IP -Send reject messages per BIP61 (default: 1) -.HP \fB\-externalip=\fR .IP Specify your own public address diff --git a/doc/release-notes-4892.md b/doc/release-notes-4892.md new file mode 100644 index 0000000000..00cb35cf06 --- /dev/null +++ b/doc/release-notes-4892.md @@ -0,0 +1,34 @@ +P2P and network changes +----------------------- + +#### Removal of reject network messages from Dash Core (BIP61) + +The command line option to enable BIP61 (`-enablebip61`) has been removed. + +This feature has been disabled by default since Dash Core version 0.19.0. +Nodes on the network can not generally be trusted to send valid ("reject") +messages, so this should only ever be used when connected to a trusted node. +Please use the recommended alternatives if you rely on this deprecated feature: + +* Testing or debugging of implementations of the Dash P2P network protocol + should be done by inspecting the log messages that are produced by a recent + version of Dash Core. Dash Core logs debug messages + (`-debug=`) to a stream (`-printtoconsole`) or to a file + (`-debuglogfile=`). + +* Testing the validity of a block can be achieved by specific RPCs: + - `submitblock` + - `getblocktemplate` with `'mode'` set to `'proposal'` for blocks with + potentially invalid POW + +* Testing the validity of a transaction can be achieved by specific RPCs: + - `sendrawtransaction` + - `testmempoolaccept` + +* Wallets should not use the absence of "reject" messages to indicate a + transaction has propagated the network, nor should wallets use "reject" + messages to set transaction fees. Wallets should rather use fee estimation + to determine transaction fees and set replace-by-fee if desired. Thus, they + could wait until the transaction has confirmed (taking into account the fee + target they set (compare the RPC `estimatesmartfee`)) or listen for the + transaction announcement by other network peers to check for propagation. diff --git a/src/coinjoin/client.cpp b/src/coinjoin/client.cpp index 604d2dcac4..f5c271333b 100644 --- a/src/coinjoin/client.cpp +++ b/src/coinjoin/client.cpp @@ -113,7 +113,7 @@ void CCoinJoinClientQueueManager::ProcessDSQueue(const CNode& peer, CDataStream& } } -void CCoinJoinClientManager::ProcessMessage(CNode& peer, std::string_view msg_type, CDataStream& vRecv, CConnman& connman, bool enable_bip61) +void CCoinJoinClientManager::ProcessMessage(CNode& peer, std::string_view msg_type, CDataStream& vRecv, CConnman& connman) { if (fMasternodeMode) return; if (!CCoinJoinClientOptions::IsEnabled()) return; @@ -132,12 +132,12 @@ void CCoinJoinClientManager::ProcessMessage(CNode& peer, std::string_view msg_ty AssertLockNotHeld(cs_deqsessions); LOCK(cs_deqsessions); for (auto& session : deqSessions) { - session.ProcessMessage(peer, msg_type, vRecv, connman, enable_bip61); + session.ProcessMessage(peer, msg_type, vRecv, connman); } } } -void CCoinJoinClientSession::ProcessMessage(CNode& peer, std::string_view msg_type, CDataStream& vRecv, CConnman& connman, bool enable_bip61) +void CCoinJoinClientSession::ProcessMessage(CNode& peer, std::string_view msg_type, CDataStream& vRecv, CConnman& connman) { if (fMasternodeMode) return; if (!CCoinJoinClientOptions::IsEnabled()) return; diff --git a/src/coinjoin/client.h b/src/coinjoin/client.h index 9da375096d..2f49c01757 100644 --- a/src/coinjoin/client.h +++ b/src/coinjoin/client.h @@ -123,7 +123,7 @@ public: { } - void ProcessMessage(CNode& peer, std::string_view msg_type, CDataStream& vRecv, CConnman& connman, bool enable_bip61); + void ProcessMessage(CNode& peer, std::string_view msg_type, CDataStream& vRecv, CConnman& connman); void UnlockCoins(); @@ -201,7 +201,7 @@ public: explicit CCoinJoinClientManager(CWallet& wallet) : mixingWallet(wallet) {} - void ProcessMessage(CNode& peer, std::string_view msg_type, CDataStream& vRecv, CConnman& connman, bool enable_bip61) LOCKS_EXCLUDED(cs_deqsessions); + void ProcessMessage(CNode& peer, std::string_view msg_type, CDataStream& vRecv, CConnman& connman) LOCKS_EXCLUDED(cs_deqsessions); bool StartMixing(); void StopMixing(); diff --git a/src/init.cpp b/src/init.cpp index b776d76e7c..0d4dfcd5a1 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -572,7 +572,6 @@ void SetupServerArgs(NodeContext& node) argsman.AddArg("-discover", "Discover own IP addresses (default: 1 when listening and no -externalip or -proxy)", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-dns", strprintf("Allow DNS lookups for -addnode, -seednode and -connect (default: %u)", DEFAULT_NAME_LOOKUP), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-dnsseed", "Query for peer addresses via DNS lookup, if low on addresses (default: 1 unless -connect used)", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); - argsman.AddArg("-enablebip61", strprintf("Send reject messages per BIP61 (default: %u)", DEFAULT_ENABLE_BIP61), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-externalip=", "Specify your own public address", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-forcednsseed", strprintf("Always query for peer addresses via DNS lookup (default: %u)", DEFAULT_FORCEDNSSEED), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-listen", "Accept connections from outside (default: 1 if no -proxy or -connect)", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); @@ -1786,8 +1785,8 @@ bool AppInitMain(const CoreContext& context, NodeContext& node, interfaces::Bloc ChainstateManager& chainman = *Assert(node.chainman); node.peer_logic.reset(new PeerLogicValidation( - node.connman.get(), node.banman.get(), *node.scheduler, chainman, *node.mempool, node.llmq_ctx, args.GetBoolArg("-enablebip61", DEFAULT_ENABLE_BIP61)) - ); + node.connman.get(), node.banman.get(), *node.scheduler, chainman, *node.mempool, node.llmq_ctx + )); RegisterValidationInterface(node.peer_logic.get()); ::governance = std::make_unique(); diff --git a/src/net_processing.cpp b/src/net_processing.cpp index cbdeebfd36..71a8460ca5 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -258,12 +258,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 @@ -281,8 +275,6 @@ struct CNodeState { 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. - 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. @@ -1262,8 +1254,8 @@ static bool BlockRequestAllowed(const CBlockIndex* pindex, const Consensus::Para } PeerLogicValidation::PeerLogicValidation(CConnman* connmanIn, BanMan* banman, CScheduler &scheduler, ChainstateManager& chainman, CTxMemPool& pool, - std::unique_ptr& llmq_ctx, bool enable_bip61) : - connman(connmanIn), m_banman(banman), m_chainman(chainman), m_mempool(pool), m_llmq_ctx(llmq_ctx), m_stale_tip_check_time(0), m_enable_bip61(enable_bip61) + std::unique_ptr& llmq_ctx) : + connman(connmanIn), m_banman(banman), m_chainman(chainman), m_mempool(pool), m_llmq_ctx(llmq_ctx), m_stale_tip_check_time(0) { // Initialize global variables that cannot be constructed at startup. recentRejects.reset(new CRollingBloomFilter(120000, 0.000001)); @@ -1465,8 +1457,6 @@ void PeerLogicValidation::BlockChecked(const CBlock& block, const CValidationSta 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); MaybePunishNode(/*nodeid=*/ it->second.first, state, /*via_compact_block=*/ !it->second.second); } } @@ -2540,7 +2530,7 @@ std::pair static ValidateDSTX(CCoinJoinBroadca bool ProcessMessage(CNode* pfrom, const std::string& msg_type, CDataStream& vRecv, int64_t nTimeReceived, const CChainParams& chainparams, ChainstateManager& chainman, CTxMemPool& mempool, - LLMQContext& llmq_ctx, CConnman* connman, BanMan* banman, const std::atomic& interruptMsgProc, bool enable_bip61) + LLMQContext& llmq_ctx, CConnman* connman, BanMan* banman, const std::atomic& interruptMsgProc) { LogPrint(BCLog::NET, "received: %s (%u bytes) peer=%d\n", SanitizeString(msg_type), vRecv.size(), pfrom->GetId()); statsClient.inc("message.received." + SanitizeString(msg_type), 1.0f); @@ -2560,48 +2550,10 @@ bool ProcessMessage(CNode* pfrom, const std::string& msg_type, CDataStream& vRec return false; } - if (msg_type == NetMsgType::REJECT) - { - std::string strMsg; unsigned char ccode; std::string strReason; - uint256 hash; - try { - vRecv >> LIMITED_STRING(strMsg, CMessageHeader::COMMAND_SIZE) >> ccode >> LIMITED_STRING(strReason, MAX_REJECT_MESSAGE_LENGTH); - if (strMsg == NetMsgType::BLOCK || strMsg == NetMsgType::TX) { - vRecv >> hash; - } - } 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"); - } - - if (strMsg == NetMsgType::BLOCK) { - // The node requested a block from us and then rejected it, which indicates that it's most likely running - // on rules which are incompatible to ours. Better to ban him after some time as it might otherwise keep - // asking for the same block (if -addnode/-connect was used on the other side). - LOCK(cs_main); - Misbehaving(pfrom->GetId(), 1); - } - - if (LogAcceptCategory(BCLog::NET)) { - std::ostringstream ss; - ss << strMsg << " code " << itostr(ccode) << ": " << strReason; - - if (strMsg == NetMsgType::BLOCK || strMsg == NetMsgType::TX) { - ss << ": hash " << hash.ToString(); - } - LogPrint(BCLog::NET, "Reject %s\n", SanitizeString(ss.str())); - } - statsClient.inc("message.sent.reject_" + strMsg + "_" + RejectCodeToString(ccode), 1.0f); - return true; - } - if (msg_type == 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, msg_type, REJECT_DUPLICATE, std::string("Duplicate version message"))); - } LOCK(cs_main); Misbehaving(pfrom->GetId(), 1); return false; @@ -2629,10 +2581,6 @@ bool ProcessMessage(CNode* pfrom, const std::string& msg_type, CDataStream& vRec 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, msg_type, REJECT_NONSTANDARD, - strprintf("Expected to offer services %08x", GetDesirableServiceFlags(nServices)))); - } pfrom->fDisconnect = true; return false; } @@ -2640,10 +2588,6 @@ bool ProcessMessage(CNode* pfrom, const std::string& msg_type, CDataStream& vRec 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, msg_type, REJECT_OBSOLETE, - strprintf("Version must be %d or greater", MIN_PEER_PROTO_VERSION))); - } pfrom->fDisconnect = true; return false; } @@ -3473,10 +3417,6 @@ bool ProcessMessage(CNode* pfrom, const std::string& msg_type, CDataStream& vRec 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, msg_type, (unsigned char)state.GetRejectCode(), - state.GetRejectReason().substr(0, MAX_REJECT_MESSAGE_LENGTH), inv.hash)); - } MaybePunishNode(pfrom->GetId(), state, /*via_compact_block*/ false); llmq_ctx.isman->TransactionRemovedFromMempool(ptx); } @@ -3651,7 +3591,7 @@ bool ProcessMessage(CNode* pfrom, const std::string& msg_type, CDataStream& vRec } // cs_main if (fProcessBLOCKTXN) - return ProcessMessage(pfrom, NetMsgType::BLOCKTXN, blockTxnMsg, nTimeReceived, chainparams, chainman, mempool, llmq_ctx, connman, banman, interruptMsgProc, enable_bip61); + return ProcessMessage(pfrom, NetMsgType::BLOCKTXN, blockTxnMsg, nTimeReceived, chainparams, chainman, mempool, llmq_ctx, connman, banman, interruptMsgProc); if (fRevertToHeaderProcessing) { // Headers received from HB compact block peers are permitted to be @@ -4146,7 +4086,7 @@ bool ProcessMessage(CNode* pfrom, const std::string& msg_type, CDataStream& vRec #ifdef ENABLE_WALLET coinJoinClientQueueManager->ProcessMessage(*pfrom, msg_type, vRecv); for (auto& pair : coinJoinClientManagers) { - pair.second->ProcessMessage(*pfrom, msg_type, vRecv, *connman, enable_bip61); + pair.second->ProcessMessage(*pfrom, msg_type, vRecv, *connman); } #endif // ENABLE_WALLET coinJoinServer->ProcessMessage(*pfrom, msg_type, vRecv); @@ -4170,18 +4110,11 @@ bool ProcessMessage(CNode* pfrom, const std::string& msg_type, CDataStream& vRec return true; } -bool PeerLogicValidation::MaybeDiscourageAndDisconnect(CNode* pnode, bool enable_bip61) +bool PeerLogicValidation::MaybeDiscourageAndDisconnect(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.m_should_discourage) { state.m_should_discourage = false; if (pnode->HasPermission(PF_NOBAN)) { @@ -4253,7 +4186,7 @@ bool PeerLogicValidation::ProcessMessages(CNode* pfrom, std::atomic& inter bool fRet = false; try { - fRet = ProcessMessage(pfrom, msg_type, msg.m_recv, msg.m_time, chainparams, m_chainman, m_mempool, *m_llmq_ctx, connman, m_banman, interruptMsgProc, m_enable_bip61); + fRet = ProcessMessage(pfrom, msg_type, msg.m_recv, msg.m_time, chainparams, m_chainman, m_mempool, *m_llmq_ctx, connman, m_banman, interruptMsgProc); if (interruptMsgProc) return false; if (!pfrom->vRecvGetData.empty()) @@ -4261,9 +4194,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, msg_type, 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(msg_type), nMessageSize, e.what()); @@ -4291,7 +4221,7 @@ bool PeerLogicValidation::ProcessMessages(CNode* pfrom, std::atomic& inter } LOCK(cs_main); - MaybeDiscourageAndDisconnect(pfrom, m_enable_bip61); + MaybeDiscourageAndDisconnect(pfrom); return fMoreWork; } @@ -4493,11 +4423,11 @@ bool PeerLogicValidation::SendMessages(CNode* pto) connman->PushMessage(pto, msgMaker.Make(NetMsgType::PING, nonce)); } - TRY_LOCK(cs_main, lockMain); // Acquire cs_main for IsInitialBlockDownload() and CNodeState() + TRY_LOCK(cs_main, lockMain); if (!lockMain) return true; - if (MaybeDiscourageAndDisconnect(pto, m_enable_bip61)) return true; + if (MaybeDiscourageAndDisconnect(pto)) return true; CNodeState &state = *State(pto->GetId()); diff --git a/src/net_processing.h b/src/net_processing.h index 2fa1e430af..4bb55deda4 100644 --- a/src/net_processing.h +++ b/src/net_processing.h @@ -21,8 +21,6 @@ extern CCriticalSection cs_main; static const unsigned int DEFAULT_MAX_ORPHAN_TRANSACTIONS_SIZE = 10; // this allows around 100 TXs of max size (and many more of normal size) /** 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 = true; static const bool DEFAULT_PEERBLOOMFILTERS = true; static const bool DEFAULT_PEERBLOCKFILTERS = false; @@ -34,10 +32,10 @@ private: CTxMemPool& m_mempool; std::unique_ptr& m_llmq_ctx; - bool MaybeDiscourageAndDisconnect(CNode* pnode, bool enable_bip61) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + bool MaybeDiscourageAndDisconnect(CNode* pnode) EXCLUSIVE_LOCKS_REQUIRED(cs_main); public: PeerLogicValidation(CConnman* connmanIn, BanMan* banman, CScheduler &scheduler, ChainstateManager& chainman, CTxMemPool& pool, - std::unique_ptr& llmq_ctx, bool enable_bip61); + std::unique_ptr& llmq_ctx); /** * Overridden from CValidationInterface. @@ -87,9 +85,6 @@ public: 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 d2f69ba034..173c183f33 100644 --- a/src/protocol.cpp +++ b/src/protocol.cpp @@ -35,7 +35,6 @@ MAKE_MSG(NOTFOUND, "notfound"); MAKE_MSG(FILTERLOAD, "filterload"); MAKE_MSG(FILTERADD, "filteradd"); MAKE_MSG(FILTERCLEAR, "filterclear"); -MAKE_MSG(REJECT, "reject"); MAKE_MSG(SENDHEADERS, "sendheaders"); MAKE_MSG(SENDCMPCT, "sendcmpct"); MAKE_MSG(CMPCTBLOCK, "cmpctblock"); @@ -117,7 +116,6 @@ const static std::string allNetMessageTypes[] = { NetMsgType::FILTERLOAD, NetMsgType::FILTERADD, NetMsgType::FILTERCLEAR, - NetMsgType::REJECT, NetMsgType::SENDHEADERS, NetMsgType::SENDCMPCT, NetMsgType::CMPCTBLOCK, diff --git a/src/protocol.h b/src/protocol.h index e23afc579f..8b0c25db42 100644 --- a/src/protocol.h +++ b/src/protocol.h @@ -179,13 +179,6 @@ extern const char *FILTERADD; * 70011 as described by BIP111. */ 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/test/denialofservice_tests.cpp b/src/test/denialofservice_tests.cpp index 767a073ee1..0d414107a7 100644 --- a/src/test/denialofservice_tests.cpp +++ b/src/test/denialofservice_tests.cpp @@ -81,7 +81,7 @@ BOOST_AUTO_TEST_CASE(outbound_slow_chain_eviction) { auto connman = std::make_unique(0x1337, 0x1337); auto peerLogic = std::make_unique( - connman.get(), nullptr, *m_node.scheduler, *m_node.chainman, *m_node.mempool, m_node.llmq_ctx, false + connman.get(), nullptr, *m_node.scheduler, *m_node.chainman, *m_node.mempool, m_node.llmq_ctx ); // Mock an outbound peer @@ -154,7 +154,7 @@ BOOST_AUTO_TEST_CASE(stale_tip_peer_management) { auto connman = std::make_unique(0x1337, 0x1337); auto peerLogic = std::make_unique( - connman.get(), nullptr, *m_node.scheduler, *m_node.chainman, *m_node.mempool, m_node.llmq_ctx, false + connman.get(), nullptr, *m_node.scheduler, *m_node.chainman, *m_node.mempool, m_node.llmq_ctx ); const Consensus::Params& consensusParams = Params().GetConsensus(); @@ -229,7 +229,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(), *m_node.scheduler, *m_node.chainman, *m_node.mempool, m_node.llmq_ctx, false + connman.get(), banman.get(), *m_node.scheduler, *m_node.chainman, *m_node.mempool, m_node.llmq_ctx ); banman->ClearBanned(); @@ -286,7 +286,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(), *m_node.scheduler, *m_node.chainman, *m_node.mempool, m_node.llmq_ctx, false + connman.get(), banman.get(), *m_node.scheduler, *m_node.chainman, *m_node.mempool, m_node.llmq_ctx ); banman->ClearBanned(); @@ -335,7 +335,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(), *m_node.scheduler, *m_node.chainman, *m_node.mempool, m_node.llmq_ctx, false + connman.get(), banman.get(), *m_node.scheduler, *m_node.chainman, *m_node.mempool, m_node.llmq_ctx ); banman->ClearBanned(); diff --git a/src/test/fuzz/process_message.cpp b/src/test/fuzz/process_message.cpp index 8060f1b5a1..828b71f4dd 100644 --- a/src/test/fuzz/process_message.cpp +++ b/src/test/fuzz/process_message.cpp @@ -36,7 +36,7 @@ #include #include -bool ProcessMessage(CNode* pfrom, const std::string& msg_type, CDataStream& vRecv, int64_t nTimeReceived, const CChainParams& chainparams, ChainstateManager& chainman, CTxMemPool& mempool, LLMQContext& llmq_ctx, CConnman* connman, BanMan* banman, const std::atomic& interruptMsgProc, bool enable_bip61); +bool ProcessMessage(CNode* pfrom, const std::string& msg_type, CDataStream& vRecv, int64_t nTimeReceived, const CChainParams& chainparams, ChainstateManager& chainman, CTxMemPool& mempool, LLMQContext& llmq_ctx, CConnman* connman, BanMan* banman, const std::atomic& interruptMsgProc); namespace { const TestingSetup* g_setup; @@ -72,7 +72,7 @@ void fuzz_target(const std::vector& buffer, const std::string& LIMIT_TO p2p_node.SetSendVersion(PROTOCOL_VERSION); g_setup->m_node.peer_logic->InitializeNode(&p2p_node); try { - (void)ProcessMessage(&p2p_node, random_message_type, random_bytes_data_stream, GetTimeMillis(), Params(), *g_setup->m_node.chainman, *g_setup->m_node.mempool, *g_setup->m_node.llmq_ctx, g_setup->m_node.connman.get(), g_setup->m_node.banman.get(), std::atomic{false}, true); + (void)ProcessMessage(&p2p_node, random_message_type, random_bytes_data_stream, GetTimeMillis(), Params(), *g_setup->m_node.chainman, *g_setup->m_node.mempool, *g_setup->m_node.llmq_ctx, g_setup->m_node.connman.get(), g_setup->m_node.banman.get(), std::atomic{false}); } catch (const std::ios_base::failure& e) { } SyncWithValidationInterfaceQueue(); diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 913cbc81e4..ec623ed29e 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -237,7 +237,7 @@ TestingSetup::TestingSetup(const std::string& chainName, const std::vector(GetDataDir() / "banlist.dat", nullptr, DEFAULT_MISBEHAVING_BANTIME); m_node.peer_logic = std::make_unique( - m_node.connman.get(), m_node.banman.get(), *m_node.scheduler, *m_node.chainman, *m_node.mempool, m_node.llmq_ctx, false + m_node.connman.get(), m_node.banman.get(), *m_node.scheduler, *m_node.chainman, *m_node.mempool, m_node.llmq_ctx ); { CConnman::Options options; diff --git a/src/validation.h b/src/validation.h index 9eb91b7af6..1c5fba5951 100644 --- a/src/validation.h +++ b/src/validation.h @@ -85,8 +85,6 @@ static const int DEFAULT_SCRIPTCHECK_THREADS = 0; /** Number of headers sent in one getheaders result. We rely on the assumption that if a peer sends * less than this number, we reached its tip. Changing this value is a protocol upgrade. */ static const unsigned int MAX_HEADERS_RESULTS = 2000; -/** Maximum length of reject messages. */ -static const unsigned int MAX_REJECT_MESSAGE_LENGTH = 111; static const int64_t DEFAULT_MAX_TIP_AGE = 6 * 60 * 60; // ~144 blocks behind -> 2 x fork detection time, was 24 * 60 * 60 in bitcoin diff --git a/test/functional/feature_cltv.py b/test/functional/feature_cltv.py index a1bcc3b1aa..6e5e16e673 100755 --- a/test/functional/feature_cltv.py +++ b/test/functional/feature_cltv.py @@ -23,10 +23,6 @@ from io import BytesIO CLTV_HEIGHT = 1351 -# Reject codes that we might receive in this test -REJECT_INVALID = 16 -# REJECT_OBSOLETE = 17 -REJECT_NONSTANDARD = 64 def cltv_invalidate(tx): '''Modify the signature in vin 0 of the tx to fail CLTV diff --git a/test/functional/feature_dersig.py b/test/functional/feature_dersig.py index ab47d73865..e09219f245 100755 --- a/test/functional/feature_dersig.py +++ b/test/functional/feature_dersig.py @@ -9,21 +9,16 @@ Test that the DERSIG soft-fork activates at (regtest) height 1251. 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 BitcoinTestFramework from test_framework.util import ( assert_equal, assert_raises_rpc_error, - wait_until, ) DERSIG_HEIGHT = 1251 -# Reject codes that we might receive in this test -REJECT_INVALID = 16 -# REJECT_OBSOLETE = 17 -REJECT_NONSTANDARD = 64 # A canonical signature consists of: # <30> <02> <02> @@ -45,7 +40,7 @@ def unDERify(tx): class BIP66Test(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 1 - self.extra_args = [['-whitelist=noban@127.0.0.1', '-dip3params=9000:9000', '-par=1', '-enablebip61']] # Use only one script thread to get the exact reject reason for testing + self.extra_args = [['-whitelist=noban@127.0.0.1', '-dip3params=9000:9000', '-par=1']] # Use only one script thread to get the exact reject reason for testing self.setup_clean_chain = True self.rpc_timeout = 240 @@ -130,12 +125,6 @@ class BIP66Test(BitcoinTestFramework): 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") block.vtx[1] = create_transaction(self.nodes[0], self.coinbase_txids[1], self.nodeaddress, amount=1.0) block.hashMerkleRoot = block.calc_merkle_root() diff --git a/test/functional/test_framework/messages.py b/test/functional/test_framework/messages.py index 6e65a051b7..b614cce8df 100755 --- a/test/functional/test_framework/messages.py +++ b/test/functional/test_framework/messages.py @@ -1798,39 +1798,6 @@ class msg_headers2: return "msg_headers2(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("