Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Merge #16152: Disable bloom filtering by default.
bead32e Add release notes for DEFAULT_BLOOM change (Matt Corallo)
f27309f Move DEFAULT_PEERBLOOMFILTERS from validation.h to net_processing.h (Matt Corallo)
5efcb77 Disable bloom filtering by default. (Matt Corallo)

Pull request description:

  BIP 37 bloom filters have been well-known to be a significant DoS
  target for some time. However, in order to provide continuity for
  SPV clients relying on it, the NODE_BLOOM service flag was added,
  and left as a default, to ensure sufficient nodes exist with such a
  flag.

  NODE_BLOOM is, at this point, well-established and, as long as
  there exist 0.18 nodes with default config (which I'd anticipate
  will be true for many years), will be available from some peers. By
  that time, the continued slowdown of BIP 37-based filtering will
  likely have rendered it useless (though this is already largely the
  case). Further, BIP 37 was deliberately never updated to support
  witness-based filtering as newer wallets are expected to migrate to
  some yet-to-be-network-exposed filters.

ACKs for top commit:
  jnewbery:
    ACK bead32e
  kallewoof:
    ACK bead32e

Tree-SHA512: ecd901898e8efe1a7c82b471af0acc2373c2282ac633eb58d9aae7c35deda1999d0f79fb0485e6cecbda7246aeda00206cd82c7fa36866e2ac64705ba93f9390
  • Loading branch information
fanquake committed Jul 19, 2019
2 parents 89d7229 + bead32e commit 59ce537
Show file tree
Hide file tree
Showing 5 changed files with 12 additions and 6 deletions.
7 changes: 7 additions & 0 deletions doc/release-notes/release-notes-16152.md
@@ -0,0 +1,7 @@
P2P Changes
-----------
- The default value for the -peerbloomfilters configuration option (and, thus, NODE_BLOOM support) has been changed to false.
This resolves well-known DoS vectors in Bitcoin Core, especially for nodes with spinning disks. It is not anticipated that
this will result in a significant lack of availability of NODE_BLOOM-enabled nodes in the coming years, however, clients
which rely on the availability of NODE_BLOOM-supporting nodes on the P2P network should consider the process of migrating
to a more modern (and less trustful and privacy-violating) alternative over the coming years.
1 change: 1 addition & 0 deletions src/net_processing.h
Expand Up @@ -19,6 +19,7 @@ static const unsigned int DEFAULT_MAX_ORPHAN_TRANSACTIONS = 100;
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;

class PeerLogicValidation final : public CValidationInterface, public NetEventsInterface {
private:
Expand Down
2 changes: 0 additions & 2 deletions src/validation.h
Expand Up @@ -126,8 +126,6 @@ static const unsigned int MAX_BLOCKS_TO_ANNOUNCE = 8;
/** Maximum number of unconnecting headers announcements before DoS score */
static const int MAX_UNCONNECTING_HEADERS = 10;

static const bool DEFAULT_PEERBLOOMFILTERS = true;

/** Default for -stopatheight */
static const int DEFAULT_STOPATHEIGHT = 0;

Expand Down
6 changes: 3 additions & 3 deletions test/functional/p2p_node_network_limited.py
Expand Up @@ -8,7 +8,7 @@
and that it responds to getdata requests for blocks correctly:
- send a block within 288 + 2 of the tip
- disconnect peers who request blocks older than that."""
from test_framework.messages import CInv, msg_getdata, msg_verack, NODE_BLOOM, NODE_NETWORK_LIMITED, NODE_WITNESS
from test_framework.messages import CInv, msg_getdata, msg_verack, NODE_NETWORK_LIMITED, NODE_WITNESS
from test_framework.mininode import P2PInterface, mininode_lock
from test_framework.test_framework import BitcoinTestFramework
from test_framework.util import (
Expand Down Expand Up @@ -55,7 +55,7 @@ def setup_network(self):
def run_test(self):
node = self.nodes[0].add_p2p_connection(P2PIgnoreInv())

expected_services = NODE_BLOOM | NODE_WITNESS | NODE_NETWORK_LIMITED
expected_services = NODE_WITNESS | NODE_NETWORK_LIMITED

self.log.info("Check that node has signalled expected services.")
assert_equal(node.nServices, expected_services)
Expand Down Expand Up @@ -83,7 +83,7 @@ def run_test(self):

node1.wait_for_addr()
#must relay address with NODE_NETWORK_LIMITED
assert_equal(node1.firstAddrnServices, 1036)
assert_equal(node1.firstAddrnServices, expected_services)

self.nodes[0].disconnect_p2ps()
node1.wait_for_disconnect()
Expand Down
2 changes: 1 addition & 1 deletion test/functional/test_framework/messages.py
Expand Up @@ -44,7 +44,7 @@

NODE_NETWORK = (1 << 0)
# NODE_GETUTXO = (1 << 1)
NODE_BLOOM = (1 << 2)
# NODE_BLOOM = (1 << 2)
NODE_WITNESS = (1 << 3)
NODE_NETWORK_LIMITED = (1 << 10)

Expand Down

0 comments on commit 59ce537

Please sign in to comment.