Merge #15437: p2p: Remove BIP61 reject messages

fa25f43ac5 p2p: Remove BIP61 reject messages (MarcoFalke)

Pull request description:

  Reject messages (BIP 61) appear in the following settings:

  * Parsing of reject messages (in case `-debug=net` is set, off by default). This has only been used for a single `LogPrint` call for several releases now. Such logging is completely meaningless to us and should thus be removed.

  * The sending of reject messages (in case `-enablebip61` is set, off by default). This can be used to debug a node that is under our control. Instead of hacking this debugging into the p2p protocol, it could be more easily achieved by parsing the debug log. (Use `-printtoconsole` to have it as stream, or read from the `debug.log` file like our python function `assert_debug_log` in the test framework does)

  Having to maintain all of this logic and code to accommodate debugging, which can be achieved by other means a lot easier, is a burden. It makes review on net processing changes a lot harder, since the reject message logic has to be carried around without introducing any errors or DOS vectors.

ACKs for top commit:
  jnewbery:
    utACK fa25f43ac5
  laanwj:
    I'm still not 100% convinced that I like getting rid of BIP61 conceptually, but apparently everyone wants it, code review ACK fa25f43ac5.
  ryanofsky:
    Code review ACK fa25f43ac5

Tree-SHA512: daf55254202925e56be3d6cfb3c1c804e7a82cecb1dd1e5bd7b472bae989fd68ac4f21ec53fc46751353056fd645f7f877bebcb0b40920257991423a3d99e0be

doc/bips.md

@@ -15,7 +15,7 @@ BIPs that are implemented by Bitcoin Core (up-to-date up to **v0.19.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)). Disabled by default since **v0.19.0**, can be enabled by the `-peerbloomfilters` option.
 * [`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.17.0**, whether to send reject messages can be configured with the `-enablebip61` option, and support is deprecated as of **v0.18.0**.
+* [`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)).
 * [`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*.

doc/man/bitcoin-qt.1

@@ -179,10 +179,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: 0)
-.HP
 \fB\-externalip=\fR<ip>
 .IP
 Specify your own public address

doc/man/bitcoind.1

@@ -179,10 +179,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: 0)
-.HP
 \fB\-externalip=\fR<ip>
 .IP
 Specify your own public address

doc/release-notes-15437.md

@@ -0,0 +1,34 @@
+P2P and network changes
+-----------------------
+
+#### Removal of reject network messages from Bitcoin Core (BIP61)
+
+The command line option to enable BIP61 (`-enablebip61`) has been removed.
+
+This feature has been disabled by default since Bitcoin Core version 0.18.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 Bitcoin P2P network protocol
+  should be done by inspecting the log messages that are produced by a recent
+  version of Bitcoin Core. Bitcoin Core logs debug messages
+  (`-debug=<category>`) to a stream (`-printtoconsole`) or to a file
+  (`-debuglogfile=<debug.log>`).
+
+* 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.