De-neuter NODE_BLOOM #6641

pull TheBlueMatt wants to merge 2 commits into bitcoin:master from TheBlueMatt:bloom-disable changing 3 files +2 −4
  1. TheBlueMatt commented at 6:37 AM on September 6, 2015: contributor

    I forget things easily, so I'm gonna get a few months ahead of myself and PR this now...to be merged a full release cycle after #6579.

  2. laanwj added this to the milestone Future on Sep 8, 2015
  3. laanwj commented at 3:35 PM on September 8, 2015: member

    Assigned 'future' milestone accordingly.

  4. laanwj added the label P2P on Sep 8, 2015
  5. dcousens commented at 2:09 AM on September 9, 2015: contributor

    @TheBlueMatt can this be re-based?

  6. Diapolo commented at 3:09 PM on September 9, 2015: none

    Yeah a rebase would make it clear that the base pull has been merged.

  7. TheBlueMatt force-pushed on Sep 9, 2015
  8. TheBlueMatt commented at 8:48 PM on September 9, 2015: contributor

    Rebased.

    On 09/09/15 15:09, P. Kaufmann wrote:

    Yeah a rebase would make it clear that the base pull has been merged.

    — Reply to this email directly or view it on GitHub #6641 (comment).

  9. MarcoFalke cross-referenced this on Sep 20, 2015 from issue [doc] Update doc/bips.md by MarcoFalke
  10. MarcoFalke commented at 12:11 PM on September 22, 2015: member

    Note for future: Don't forget to adjust https://github.com/bitcoin/bitcoin/blob/e59d2a80f9167031521d882394a08b02fa9d0343/doc/bips.md

  11. TheBlueMatt force-pushed on Sep 25, 2015
  12. TheBlueMatt commented at 7:58 PM on September 25, 2015: contributor

    Updated to include doc/bips.md update (needed to rebase to do so)

  13. in doc/bips.md:None in 91b2858cf3 outdated 
      15 | @@ -16,4 +16,4 @@ BIPs that are implemented by Bitcoin Core (up-to-date up to **v0.12.0**):
  16 |  * [`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)).
  17 |  * [`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)).
  18 |  * [`BIP 70`](https://github.com/bitcoin/bips/blob/master/bip-0070.mediawiki) [`71`](https://github.com/bitcoin/bips/blob/master/bip-0071.mediawiki) [`72`](https://github.com/bitcoin/bips/blob/master/bip-0072.mediawiki): Payment Protocol support has been available in Bitcoin Core GUI since **v0.9.0** ([PR #5216](https://github.com/bitcoin/bitcoin/pull/5216)).
  19 | -* [`BIP 111`](https://github.com/bitcoin/bips/blob/master/bip-0111.mediawiki): `NODE_BLOOM` service bit added, but only enforced for peer versions `>=70011` as of **v0.12.0** ([PR #6579](https://github.com/bitcoin/bitcoin/pull/6579)).
  20 | +* [`BIP 111`](https://github.com/bitcoin/bips/blob/master/bip-0111.mediawiki): `NODE_BLOOM` service bit added, and enforced for all peer versions as of **v0.13.0** ([PR #6579](https://github.com/bitcoin/bitcoin/pull/6579) and [PR #6641](https://github.com/bitcoin/bitcoin/pull/6641).
    MarcoFalke commented at 8:02 PM on September 25, 2015:

    Nit: Missing closing bracket. ')'

    TheBlueMatt commented at 8:16 PM on September 25, 2015:

    Fixed.

    On 09/25/15 20:02, MarcoFalke wrote:

    In doc/bips.md #6641 (review):

    @@ -16,4 +16,4 @@ BIPs that are implemented by Bitcoin Core (up-to-date up to v0.12.0):

    Nit: Missing closing bracket. ')'

    — Reply to this email directly or view it on GitHub https://github.com/bitcoin/bitcoin/pull/6641/files#r40469809.

  14. TheBlueMatt force-pushed on Sep 25, 2015
  15. laanwj commented at 10:17 AM on November 13, 2015: member

    There was discussion about this in #bitcoin today: some prefer this to be an option that would already be present in 0.12 but default to off, then the option removed in 0.13. (not necessarily saying this is a good idea, just thought I'd remark it)

  16. jameshilliard commented at 10:35 AM on November 13, 2015: contributor

    some prefer this to be an option that would already be present in 0.12 but default to off

    This would certainly be useful for mining pools and other high-availability services that can't afford to waste resources serving SPV clients. I don't see any significant usability issues with allowing this right away since SPV clients will just look for more nodes if they get disconnected(the percentage of nodes using this option is likely to be quite small due to most using default settings). This appears to be a better solution than the current method of white-listing nodes by subver(which is in use by pools such as f2pool).

  17. MarcoFalke cross-referenced this on Nov 24, 2015 from issue [Net]Add -enforcenodebloom option by pstratem
  18. jonasschnelli commented at 9:27 AM on November 25, 2015: contributor

    Concept ACK. Although there is one problem: Misbehaving(pfrom->GetId(), 100) results in a ban, fDisconnect=true allows a node to reconnect. My tests have shown that SPV clients tend to directly reconnect, call filter commands, get disconnected, a.s.o. Which happens in connect/disconnect loops multiple times per second. Not sure what is best here, maybe it's reasonable to ban the node for serval minutes after not respecting the NODE_BLOOM bit.

  19. dcousens commented at 9:56 AM on November 25, 2015: contributor

    Concept ACK

  20. TheBlueMatt commented at 2:18 AM on November 26, 2015: contributor

    @jonasschnelli I think fDisconnect is the only reasonable behavior we can do (unless we implement a temporary ban, but that wouldnt add much given that either way it results in disconnect after only one or two roundtrips). We certainly cant completely ban given that we really want the "wait, why is my node not syncing? Oh, there's an update to my software" flow to work.

  21. Always disconnect old nodes which request filtered connections. 03cef3fb8f
  22. Update doc/bips.md for full NODE_BLOOM enforcement 632454f3c5
  23. in src/main.cpp:None in 17144a14e7 outdated 
    4630 | -              pfrom->nVersion >= NO_BLOOM_VERSION)
4631 | +               strCommand == "filterclear"))
4632 |      {
4633 |          if (pfrom->nVersion >= NO_BLOOM_VERSION)
4634 |              Misbehaving(pfrom->GetId(), 100);
4635 | -        //TODO: Enable this after reasonable network upgrade
    MarcoFalke commented at 7:26 AM on November 27, 2015:

    Needs rebase.

  24. TheBlueMatt force-pushed on Dec 1, 2015
  25. TheBlueMatt commented at 6:18 AM on December 1, 2015: contributor

    Rebased.

  26. laanwj added this to the milestone 0.13.0 on Dec 3, 2015
  27. laanwj removed this from the milestone Future on Dec 3, 2015
  28. laanwj commented at 11:59 AM on December 3, 2015: member

    Moved milestone from 'future' to '0.13', we can always decide to postpone further if SPV client implementations aren't ready by then.

  29. laanwj commented at 2:39 PM on February 2, 2016: member

    Needs rebase.

  30. laanwj cross-referenced this on Feb 11, 2016 from issue Common argument defaults for NODE_BLOOM stuff and -wallet by luke-jr
  31. laanwj commented at 4:11 PM on March 17, 2016: member

    Still needs rebase

  32. pstratem commented at 12:24 AM on March 18, 2016: contributor

    @laanwj #7708

  33. jonasschnelli commented at 7:15 AM on March 18, 2016: contributor

    Closing in favor of #7708

  34. jonasschnelli closed this on Mar 18, 2016
  35. bitcoin locked this on Sep 8, 2021
Contributors
TheBlueMattlaanwjdcousensDiapoloMarcoFalkejameshilliardjonasschnellipstratem
Labels
P2P

Milestone
0.13.0

Linked (view graph)
#3185 Reject p2p message#5216 [Qt] payment request work - part 1#5713 Implement BIP66#6579 Add NODE_BLOOM service bit and bump protocol version#6705 [doc] Update doc/bips.md#7087 [Net]Add -enforcenodebloom option#7509 Common argument defaults for NODE_BLOOM stuff and -wallet#7708 De-neuter NODE_BLOOM
github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-05-20 06:55 UTC