De-neuter NODE_BLOOM #6641

TheBlueMatt · 2015-09-06T06:37:12Z

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.

laanwj · 2015-09-08T15:35:51Z

Assigned 'future' milestone accordingly.

dcousens · 2015-09-09T02:09:51Z

@TheBlueMatt can this be re-based?

Diapolo · 2015-09-09T15:09:45Z

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

TheBlueMatt · 2015-09-09T20:48:28Z

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).

maflcko · 2015-09-22T12:11:00Z

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

TheBlueMatt · 2015-09-25T19:58:22Z

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

maflcko · 2015-09-25T20:02:02Z

doc/bips.md

@@ -16,4 +16,4 @@ BIPs that are implemented by Bitcoin Core (up-to-date up to **v0.12.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)).
 * [`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 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)).
-* [`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)).
+* [`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).


Nit: Missing closing bracket. ')'

Fixed.

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

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

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

BIP 61: The 'reject' protocol message (and the protocol version bump to 70002) was added in v0.9.0 (PR #3185).

BIP 66: The strict DER rules and associated version 3 blocks have been implemented since v0.10.0 (PR #5713).

BIP 70 71 72: Payment Protocol support has been available in Bitcoin Core GUI since v0.9.0 (PR #5216).
-* BIP 111: NODE_BLOOM service bit added, but only enforced for peer versions >=70011 as of v0.12.0 (PR #6579).
+* BIP 111: NODE_BLOOM service bit added, and enforced for all peer versions as of v0.13.0 (PR #6579 and PR #6641.

Nit: Missing closing bracket. ')'

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

laanwj · 2015-11-13T10:17:33Z

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)

jameshilliard · 2015-11-13T10:35:13Z

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).

jonasschnelli · 2015-11-25T09:27:09Z

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.

dcousens · 2015-11-25T09:56:52Z

Concept ACK

TheBlueMatt · 2015-11-26T02:18:21Z

@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.

maflcko · 2015-11-27T07:26:10Z

src/main.cpp

    {
        if (pfrom->nVersion >= NO_BLOOM_VERSION)
            Misbehaving(pfrom->GetId(), 100);
-        //TODO: Enable this after reasonable network upgrade


Needs rebase.

TheBlueMatt · 2015-12-01T06:18:26Z

Rebased.

laanwj · 2015-12-03T11:59:12Z

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

laanwj · 2016-02-02T14:39:05Z

Needs rebase.

laanwj · 2016-03-17T16:11:28Z

Still needs rebase

pstratem · 2016-03-18T00:24:09Z

@laanwj #7708

jonasschnelli · 2016-03-18T07:15:57Z

Closing in favor of #7708

laanwj added this to the Future milestone Sep 8, 2015

laanwj added the P2P label Sep 8, 2015

TheBlueMatt force-pushed the bloom-disable branch from 5331b8c to 2d3704f Compare September 9, 2015 20:45

maflcko mentioned this pull request Sep 20, 2015

[doc] Update doc/bips.md #6705

Closed

TheBlueMatt force-pushed the bloom-disable branch from 2d3704f to 91b2858 Compare September 25, 2015 19:58

maflcko reviewed Sep 25, 2015
View reviewed changes

TheBlueMatt force-pushed the bloom-disable branch from 91b2858 to 17144a1 Compare September 25, 2015 20:16

maflcko mentioned this pull request Nov 24, 2015

[Net]Add -enforcenodebloom option #7087

Merged

maflcko reviewed Nov 27, 2015
View reviewed changes

TheBlueMatt added 2 commits November 30, 2015 23:59

Always disconnect old nodes which request filtered connections.

03cef3f

Update doc/bips.md for full NODE_BLOOM enforcement

632454f

TheBlueMatt force-pushed the bloom-disable branch from 17144a1 to 632454f Compare December 1, 2015 06:16

laanwj modified the milestones: 0.13.0, Future Dec 3, 2015

laanwj mentioned this pull request Feb 11, 2016

Common argument defaults for NODE_BLOOM stuff and -wallet #7509

Merged

jonasschnelli closed this Mar 18, 2016

bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

De-neuter NODE_BLOOM #6641

De-neuter NODE_BLOOM #6641

TheBlueMatt commented Sep 6, 2015

laanwj commented Sep 8, 2015

dcousens commented Sep 9, 2015

Diapolo commented Sep 9, 2015

TheBlueMatt commented Sep 9, 2015

maflcko commented Sep 22, 2015

TheBlueMatt commented Sep 25, 2015

maflcko Sep 25, 2015

TheBlueMatt Sep 25, 2015

laanwj commented Nov 13, 2015

jameshilliard commented Nov 13, 2015

jonasschnelli commented Nov 25, 2015

dcousens commented Nov 25, 2015

TheBlueMatt commented Nov 26, 2015

maflcko Nov 27, 2015

TheBlueMatt commented Dec 1, 2015

laanwj commented Dec 3, 2015

laanwj commented Feb 2, 2016

laanwj commented Mar 17, 2016

pstratem commented Mar 18, 2016

jonasschnelli commented Mar 18, 2016

De-neuter NODE_BLOOM #6641

De-neuter NODE_BLOOM #6641

Conversation

TheBlueMatt commented Sep 6, 2015

laanwj commented Sep 8, 2015

dcousens commented Sep 9, 2015

Diapolo commented Sep 9, 2015

TheBlueMatt commented Sep 9, 2015

maflcko commented Sep 22, 2015

TheBlueMatt commented Sep 25, 2015

maflcko Sep 25, 2015

Choose a reason for hiding this comment

TheBlueMatt Sep 25, 2015

Choose a reason for hiding this comment

laanwj commented Nov 13, 2015

jameshilliard commented Nov 13, 2015

jonasschnelli commented Nov 25, 2015

dcousens commented Nov 25, 2015

TheBlueMatt commented Nov 26, 2015

maflcko Nov 27, 2015

Choose a reason for hiding this comment

TheBlueMatt commented Dec 1, 2015

laanwj commented Dec 3, 2015

laanwj commented Feb 2, 2016

laanwj commented Mar 17, 2016

pstratem commented Mar 18, 2016

jonasschnelli commented Mar 18, 2016