Skip to content

Commit fa25f43

Browse files
author
MarcoFalke
committed
p2p: Remove BIP61 reject messages
1 parent ddc4e3c commit fa25f43

15 files changed

+57
-152
lines changed

Diff for: ‎doc/bips.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ BIPs that are implemented by Bitcoin Core (up-to-date up to **v0.19.0**):
1515
* [`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.
1616
* [`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)). Disabled by default since **v0.19.0**, can be enabled by the `-peerbloomfilters` option.
1717
* [`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)).
18-
* [`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.17.0**, whether to send reject messages can be configured with the `-enablebip61` option, and support is deprecated as of **v0.18.0**.
18+
* [`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.17.0**, whether to send reject messages can be configured with the `-enablebip61` option, and support is deprecated (disabled by default) as of **v0.18.0**. Support was removed in **v0.20.0** ([PR #15437](https://github.com/bitcoin/bitcoin/pull/15437)).
1919
* [`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).
2020
* [`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)).
2121
* [`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 for: ‎doc/man/bitcoin-qt.1

-4
Original file line numberDiff line numberDiff line change
@@ -179,10 +179,6 @@ Allow DNS lookups for \fB\-addnode\fR, \fB\-seednode\fR and \fB\-connect\fR (def
179179
Query for peer addresses via DNS lookup, if low on addresses (default: 1
180180
unless \fB\-connect\fR used)
181181
.HP
182-
\fB\-enablebip61\fR
183-
.IP
184-
Send reject messages per BIP61 (default: 0)
185-
.HP
186182
\fB\-externalip=\fR<ip>
187183
.IP
188184
Specify your own public address

Diff for: ‎doc/man/bitcoind.1

-4
Original file line numberDiff line numberDiff line change
@@ -179,10 +179,6 @@ Allow DNS lookups for \fB\-addnode\fR, \fB\-seednode\fR and \fB\-connect\fR (def
179179
Query for peer addresses via DNS lookup, if low on addresses (default: 1
180180
unless \fB\-connect\fR used)
181181
.HP
182-
\fB\-enablebip61\fR
183-
.IP
184-
Send reject messages per BIP61 (default: 0)
185-
.HP
186182
\fB\-externalip=\fR<ip>
187183
.IP
188184
Specify your own public address

Diff for: ‎doc/release-notes-15437.md

+34
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
P2P and network changes
2+
-----------------------
3+
4+
#### Removal of reject network messages from Bitcoin Core (BIP61)
5+
6+
The command line option to enable BIP61 (`-enablebip61`) has been removed.
7+
8+
This feature has been disabled by default since Bitcoin Core version 0.18.0.
9+
Nodes on the network can not generally be trusted to send valid ("reject")
10+
messages, so this should only ever be used when connected to a trusted node.
11+
Please use the recommended alternatives if you rely on this deprecated feature:
12+
13+
* Testing or debugging of implementations of the Bitcoin P2P network protocol
14+
should be done by inspecting the log messages that are produced by a recent
15+
version of Bitcoin Core. Bitcoin Core logs debug messages
16+
(`-debug=<category>`) to a stream (`-printtoconsole`) or to a file
17+
(`-debuglogfile=<debug.log>`).
18+
19+
* Testing the validity of a block can be achieved by specific RPCs:
20+
- `submitblock`
21+
- `getblocktemplate` with `'mode'` set to `'proposal'` for blocks with
22+
potentially invalid POW
23+
24+
* Testing the validity of a transaction can be achieved by specific RPCs:
25+
- `sendrawtransaction`
26+
- `testmempoolaccept`
27+
28+
* Wallets should not use the absence of "reject" messages to indicate a
29+
transaction has propagated the network, nor should wallets use "reject"
30+
messages to set transaction fees. Wallets should rather use fee estimation
31+
to determine transaction fees and set replace-by-fee if desired. Thus, they
32+
could wait until the transaction has confirmed (taking into account the fee
33+
target they set (compare the RPC `estimatesmartfee`)) or listen for the
34+
transaction announcement by other network peers to check for propagation.

Diff for: ‎src/init.cpp

+1-2
Original file line numberDiff line numberDiff line change
@@ -407,7 +407,6 @@ void SetupServerArgs()
407407
gArgs.AddArg("-discover", "Discover own IP addresses (default: 1 when listening and no -externalip or -proxy)", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
408408
gArgs.AddArg("-dns", strprintf("Allow DNS lookups for -addnode, -seednode and -connect (default: %u)", DEFAULT_NAME_LOOKUP), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
409409
gArgs.AddArg("-dnsseed", "Query for peer addresses via DNS lookup, if low on addresses (default: 1 unless -connect used)", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
410-
gArgs.AddArg("-enablebip61", strprintf("Send reject messages per BIP61 (default: %u)", DEFAULT_ENABLE_BIP61), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
411410
gArgs.AddArg("-externalip=<ip>", "Specify your own public address", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
412411
gArgs.AddArg("-forcednsseed", strprintf("Always query for peer addresses via DNS lookup (default: %u)", DEFAULT_FORCEDNSSEED), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
413412
gArgs.AddArg("-listen", "Accept connections from outside (default: 1 if no -proxy or -connect)", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
@@ -1320,7 +1319,7 @@ bool AppInitMain(InitInterfaces& interfaces)
13201319
assert(!g_connman);
13211320
g_connman = std::unique_ptr<CConnman>(new CConnman(GetRand(std::numeric_limits<uint64_t>::max()), GetRand(std::numeric_limits<uint64_t>::max())));
13221321

1323-
peerLogic.reset(new PeerLogicValidation(g_connman.get(), g_banman.get(), scheduler, gArgs.GetBoolArg("-enablebip61", DEFAULT_ENABLE_BIP61)));
1322+
peerLogic.reset(new PeerLogicValidation(g_connman.get(), g_banman.get(), scheduler));
13241323
RegisterValidationInterface(peerLogic.get());
13251324

13261325
// sanitize comments per BIP-0014, format user agent and check total size

Diff for: ‎src/net_processing.cpp

+11-69
Original file line numberDiff line numberDiff line change
@@ -193,12 +193,6 @@ namespace {
193193
} // namespace
194194

195195
namespace {
196-
struct CBlockReject {
197-
unsigned char chRejectCode;
198-
std::string strRejectReason;
199-
uint256 hashBlock;
200-
};
201-
202196
/**
203197
* Maintain validation-specific state about nodes, protected by cs_main, instead
204198
* by CNode's own locks. This simplifies asynchronous operation, where
@@ -216,8 +210,6 @@ struct CNodeState {
216210
bool fShouldBan;
217211
//! String name of this peer (debugging/logging purposes).
218212
const std::string name;
219-
//! List of asynchronously-determined block rejections to notify this peer about.
220-
std::vector<CBlockReject> rejects;
221213
//! The best known block we know this peer has announced.
222214
const CBlockIndex *pindexBestKnownBlock;
223215
//! The hash of the last unknown block this peer has announced.
@@ -1093,8 +1085,9 @@ static bool BlockRequestAllowed(const CBlockIndex* pindex, const Consensus::Para
10931085
(GetBlockProofEquivalentTime(*pindexBestHeader, *pindex, *pindexBestHeader, consensusParams) < STALE_RELAY_AGE_LIMIT);
10941086
}
10951087

1096-
PeerLogicValidation::PeerLogicValidation(CConnman* connmanIn, BanMan* banman, CScheduler &scheduler, bool enable_bip61)
1097-
: connman(connmanIn), m_banman(banman), m_stale_tip_check_time(0), m_enable_bip61(enable_bip61) {
1088+
PeerLogicValidation::PeerLogicValidation(CConnman* connmanIn, BanMan* banman, CScheduler& scheduler)
1089+
: connman(connmanIn), m_banman(banman), m_stale_tip_check_time(0)
1090+
{
10981091
// Initialize global variables that cannot be constructed at startup.
10991092
recentRejects.reset(new CRollingBloomFilter(120000, 0.000001));
11001093

@@ -1244,8 +1237,6 @@ void PeerLogicValidation::BlockChecked(const CBlock& block, const CValidationSta
12441237
if (state.IsInvalid()) {
12451238
// Don't send reject message with code 0 or an internal reject code.
12461239
if (it != mapBlockSource.end() && State(it->second.first) && state.GetRejectCode() > 0 && state.GetRejectCode() < REJECT_INTERNAL) {
1247-
CBlockReject reject = {(unsigned char)state.GetRejectCode(), state.GetRejectReason().substr(0, MAX_REJECT_MESSAGE_LENGTH), hash};
1248-
State(it->second.first)->rejects.push_back(reject);
12491240
MaybePunishNode(/*nodeid=*/ it->second.first, state, /*via_compact_block=*/ !it->second.second);
12501241
}
12511242
}
@@ -1859,7 +1850,7 @@ void static ProcessOrphanTx(CConnman* connman, std::set<uint256>& orphan_work_se
18591850
}
18601851
}
18611852

1862-
bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStream& vRecv, int64_t nTimeReceived, const CChainParams& chainparams, CConnman* connman, const std::atomic<bool>& interruptMsgProc, bool enable_bip61)
1853+
bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStream& vRecv, int64_t nTimeReceived, const CChainParams& chainparams, CConnman* connman, const std::atomic<bool>& interruptMsgProc)
18631854
{
18641855
LogPrint(BCLog::NET, "received: %s (%u bytes) peer=%d\n", SanitizeString(strCommand), vRecv.size(), pfrom->GetId());
18651856
if (gArgs.IsArgSet("-dropmessagestest") && GetRand(gArgs.GetArg("-dropmessagestest", 0)) == 0)
@@ -1883,38 +1874,10 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
18831874
}
18841875
}
18851876

1886-
if (strCommand == NetMsgType::REJECT)
1887-
{
1888-
if (LogAcceptCategory(BCLog::NET)) {
1889-
try {
1890-
std::string strMsg; unsigned char ccode; std::string strReason;
1891-
vRecv >> LIMITED_STRING(strMsg, CMessageHeader::COMMAND_SIZE) >> ccode >> LIMITED_STRING(strReason, MAX_REJECT_MESSAGE_LENGTH);
1892-
1893-
std::ostringstream ss;
1894-
ss << strMsg << " code " << itostr(ccode) << ": " << strReason;
1895-
1896-
if (strMsg == NetMsgType::BLOCK || strMsg == NetMsgType::TX)
1897-
{
1898-
uint256 hash;
1899-
vRecv >> hash;
1900-
ss << ": hash " << hash.ToString();
1901-
}
1902-
LogPrint(BCLog::NET, "Reject %s\n", SanitizeString(ss.str()));
1903-
} catch (const std::ios_base::failure&) {
1904-
// Avoid feedback loops by preventing reject messages from triggering a new reject message.
1905-
LogPrint(BCLog::NET, "Unparseable reject message received\n");
1906-
}
1907-
}
1908-
return true;
1909-
}
1910-
19111877
if (strCommand == NetMsgType::VERSION) {
19121878
// Each connection can only send one version message
19131879
if (pfrom->nVersion != 0)
19141880
{
1915-
if (enable_bip61) {
1916-
connman->PushMessage(pfrom, CNetMsgMaker(INIT_PROTO_VERSION).Make(NetMsgType::REJECT, strCommand, REJECT_DUPLICATE, std::string("Duplicate version message")));
1917-
}
19181881
LOCK(cs_main);
19191882
Misbehaving(pfrom->GetId(), 1);
19201883
return false;
@@ -1942,21 +1905,13 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
19421905
if (!pfrom->fInbound && !pfrom->fFeeler && !pfrom->m_manual_connection && !HasAllDesirableServiceFlags(nServices))
19431906
{
19441907
LogPrint(BCLog::NET, "peer=%d does not offer the expected services (%08x offered, %08x expected); disconnecting\n", pfrom->GetId(), nServices, GetDesirableServiceFlags(nServices));
1945-
if (enable_bip61) {
1946-
connman->PushMessage(pfrom, CNetMsgMaker(INIT_PROTO_VERSION).Make(NetMsgType::REJECT, strCommand, REJECT_NONSTANDARD,
1947-
strprintf("Expected to offer services %08x", GetDesirableServiceFlags(nServices))));
1948-
}
19491908
pfrom->fDisconnect = true;
19501909
return false;
19511910
}
19521911

19531912
if (nVersion < MIN_PEER_PROTO_VERSION) {
19541913
// disconnect from peers older than this proto version
19551914
LogPrint(BCLog::NET, "peer=%d using obsolete version %i; disconnecting\n", pfrom->GetId(), nVersion);
1956-
if (enable_bip61) {
1957-
connman->PushMessage(pfrom, CNetMsgMaker(INIT_PROTO_VERSION).Make(NetMsgType::REJECT, strCommand, REJECT_OBSOLETE,
1958-
strprintf("Version must be %d or greater", MIN_PEER_PROTO_VERSION)));
1959-
}
19601915
pfrom->fDisconnect = true;
19611916
return false;
19621917
}
@@ -2628,10 +2583,6 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
26282583
LogPrint(BCLog::MEMPOOLREJ, "%s from peer=%d was not accepted: %s\n", tx.GetHash().ToString(),
26292584
pfrom->GetId(),
26302585
FormatStateMessage(state));
2631-
if (enable_bip61 && state.GetRejectCode() > 0 && state.GetRejectCode() < REJECT_INTERNAL) { // Never send AcceptToMemoryPool's internal codes over P2P
2632-
connman->PushMessage(pfrom, msgMaker.Make(NetMsgType::REJECT, strCommand, (unsigned char)state.GetRejectCode(),
2633-
state.GetRejectReason().substr(0, MAX_REJECT_MESSAGE_LENGTH), inv.hash));
2634-
}
26352586
MaybePunishNode(pfrom->GetId(), state, /*via_compact_block*/ false);
26362587
}
26372588
return true;
@@ -2811,7 +2762,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
28112762
} // cs_main
28122763

28132764
if (fProcessBLOCKTXN)
2814-
return ProcessMessage(pfrom, NetMsgType::BLOCKTXN, blockTxnMsg, nTimeReceived, chainparams, connman, interruptMsgProc, enable_bip61);
2765+
return ProcessMessage(pfrom, NetMsgType::BLOCKTXN, blockTxnMsg, nTimeReceived, chainparams, connman, interruptMsgProc);
28152766

28162767
if (fRevertToHeaderProcessing) {
28172768
// Headers received from HB compact block peers are permitted to be
@@ -3238,18 +3189,11 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
32383189
return true;
32393190
}
32403191

3241-
bool PeerLogicValidation::SendRejectsAndCheckIfBanned(CNode* pnode, bool enable_bip61) EXCLUSIVE_LOCKS_REQUIRED(cs_main)
3192+
bool PeerLogicValidation::CheckIfBanned(CNode* pnode)
32423193
{
32433194
AssertLockHeld(cs_main);
32443195
CNodeState &state = *State(pnode->GetId());
32453196

3246-
if (enable_bip61) {
3247-
for (const CBlockReject& reject : state.rejects) {
3248-
connman->PushMessage(pnode, CNetMsgMaker(INIT_PROTO_VERSION).Make(NetMsgType::REJECT, std::string(NetMsgType::BLOCK), reject.chRejectCode, reject.strRejectReason, reject.hashBlock));
3249-
}
3250-
}
3251-
state.rejects.clear();
3252-
32533197
if (state.fShouldBan) {
32543198
state.fShouldBan = false;
32553199
if (pnode->HasPermission(PF_NOBAN))
@@ -3358,17 +3302,14 @@ bool PeerLogicValidation::ProcessMessages(CNode* pfrom, std::atomic<bool>& inter
33583302
bool fRet = false;
33593303
try
33603304
{
3361-
fRet = ProcessMessage(pfrom, strCommand, vRecv, msg.nTime, chainparams, connman, interruptMsgProc, m_enable_bip61);
3305+
fRet = ProcessMessage(pfrom, strCommand, vRecv, msg.nTime, chainparams, connman, interruptMsgProc);
33623306
if (interruptMsgProc)
33633307
return false;
33643308
if (!pfrom->vRecvGetData.empty())
33653309
fMoreWork = true;
33663310
}
33673311
catch (const std::ios_base::failure& e)
33683312
{
3369-
if (m_enable_bip61) {
3370-
connman->PushMessage(pfrom, CNetMsgMaker(INIT_PROTO_VERSION).Make(NetMsgType::REJECT, strCommand, REJECT_MALFORMED, std::string("error parsing message")));
3371-
}
33723313
if (strstr(e.what(), "end of data")) {
33733314
// Allow exceptions from under-length message on vRecv
33743315
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());
@@ -3399,7 +3340,7 @@ bool PeerLogicValidation::ProcessMessages(CNode* pfrom, std::atomic<bool>& inter
33993340
}
34003341

34013342
LOCK(cs_main);
3402-
SendRejectsAndCheckIfBanned(pfrom, m_enable_bip61);
3343+
CheckIfBanned(pfrom);
34033344

34043345
return fMoreWork;
34053346
}
@@ -3598,11 +3539,12 @@ bool PeerLogicValidation::SendMessages(CNode* pto)
35983539
}
35993540
}
36003541

3601-
TRY_LOCK(cs_main, lockMain); // Acquire cs_main for IsInitialBlockDownload() and CNodeState()
3542+
TRY_LOCK(cs_main, lockMain);
36023543
if (!lockMain)
36033544
return true;
36043545

3605-
if (SendRejectsAndCheckIfBanned(pto, m_enable_bip61)) return true;
3546+
if (CheckIfBanned(pto)) return true;
3547+
36063548
CNodeState &state = *State(pto->GetId());
36073549

36083550
// Address refresh broadcast

Diff for: ‎src/net_processing.h

+3-7
Original file line numberDiff line numberDiff line change
@@ -17,18 +17,17 @@ extern CCriticalSection cs_main;
1717
static const unsigned int DEFAULT_MAX_ORPHAN_TRANSACTIONS = 100;
1818
/** Default number of orphan+recently-replaced txn to keep around for block reconstruction */
1919
static const unsigned int DEFAULT_BLOCK_RECONSTRUCTION_EXTRA_TXN = 100;
20-
/** Default for BIP61 (sending reject messages) */
21-
static constexpr bool DEFAULT_ENABLE_BIP61{false};
2220
static const bool DEFAULT_PEERBLOOMFILTERS = false;
2321

2422
class PeerLogicValidation final : public CValidationInterface, public NetEventsInterface {
2523
private:
2624
CConnman* const connman;
2725
BanMan* const m_banman;
2826

29-
bool SendRejectsAndCheckIfBanned(CNode* pnode, bool enable_bip61) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
27+
bool CheckIfBanned(CNode* pnode) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
28+
3029
public:
31-
PeerLogicValidation(CConnman* connman, BanMan* banman, CScheduler &scheduler, bool enable_bip61);
30+
PeerLogicValidation(CConnman* connman, BanMan* banman, CScheduler& scheduler);
3231

3332
/**
3433
* Overridden from CValidationInterface.
@@ -75,9 +74,6 @@ class PeerLogicValidation final : public CValidationInterface, public NetEventsI
7574

7675
private:
7776
int64_t m_stale_tip_check_time; //!< Next time to check for stale tip
78-
79-
/** Enable BIP61 (sending reject messages) */
80-
const bool m_enable_bip61;
8177
};
8278

8379
struct CNodeStateStats {

Diff for: ‎src/protocol.cpp

-2
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@ const char *NOTFOUND="notfound";
3434
const char *FILTERLOAD="filterload";
3535
const char *FILTERADD="filteradd";
3636
const char *FILTERCLEAR="filterclear";
37-
const char *REJECT="reject";
3837
const char *SENDHEADERS="sendheaders";
3938
const char *FEEFILTER="feefilter";
4039
const char *SENDCMPCT="sendcmpct";
@@ -66,7 +65,6 @@ const static std::string allNetMessageTypes[] = {
6665
NetMsgType::FILTERLOAD,
6766
NetMsgType::FILTERADD,
6867
NetMsgType::FILTERCLEAR,
69-
NetMsgType::REJECT,
7068
NetMsgType::SENDHEADERS,
7169
NetMsgType::FEEFILTER,
7270
NetMsgType::SENDCMPCT,

Diff for: ‎src/protocol.h

-7
Original file line numberDiff line numberDiff line change
@@ -192,13 +192,6 @@ extern const char *FILTERADD;
192192
* @see https://bitcoin.org/en/developer-reference#filterclear
193193
*/
194194
extern const char *FILTERCLEAR;
195-
/**
196-
* The reject message informs the receiving node that one of its previous
197-
* messages has been rejected.
198-
* @since protocol version 70002 as described by BIP61.
199-
* @see https://bitcoin.org/en/developer-reference#reject
200-
*/
201-
extern const char *REJECT;
202195
/**
203196
* Indicates that a node prefers to receive new block announcements via a
204197
* "headers" message rather than an "inv".

0 commit comments

Comments
 (0)