Add NODE_BLOOM service bit and bump protocol version

petertodd commented at 2:44 pm on August 15, 2013: contributor

Lets nodes advertise that they offer bloom filter support explicitly. The protocol version bump allows SPV nodes to assume that NODE_BLOOM is set if NODE_NETWORK is set for pre-70002 nodes.

Also adds an undocumented option to turn bloom filter support off and immediately kick peers that attempt to use bloom filters for testing purposes. In addition a number of DoS attacks are made significantly easier by bloom support, so having an option makes it easy to take immediate steps in the event of an attack.

petertodd commented at 3:32 pm on August 15, 2013: contributor

The alert tests are protocol version specific; what would be best way to work around this? I’d say just set PROTOCOL_VERSION for testing, but it’s a #define rather than a mutable variable.

mikehearn commented at 4:43 pm on August 15, 2013: contributor

What DoS attacks? Why not post them to the security list so we can look at how to fix them (ignoring for now the fact that bitcoin is full of DoS attacks so this would not make much difference).

Anyway, I think this pull request is a bad idea. I do not wish to have to support this in bitcoinj as it would complicate things significantly to have to search out nodes that properly support bloom filtering. Basically any feature in Bitcoin can be DoS attacked, that’s not a good reason to just switch things off.

petertodd commented at 5:09 pm on August 15, 2013: contributor

For instance this one posted to the email list: http://www.mail-archive.com/bitcoin-development@lists.sourceforge.net/msg02490.html All of the devs, including you, are aware of a second and more serious non-public vulnerability.

bitcoinj added a protocol version test to find bloom capable peers in revision 2c44a4fad7faccfe9b1392c67f60d21b25703bde; add a second test to check for the NODE_BLOOM bit. In the future DNS seeds can be updated to let you ask for peers with specific service bits set, but there’s no rush as bloom filtering is left on by default for now.

gmaxwell commented at 6:17 pm on August 15, 2013: contributor

I asked Peter to open this pull.

Service bitting bloom makes sense to me— it increases nodes control over the granularity of the services they are providing the public, should it be needed— I have no clue why I didn’t ask for this to begin with. The primary risk I see with this is that it will potentially result in increased levels lazy alternative node implementers not implementing the bloom server side functionality, but at the same time at least those nodes will be identifiable and avoidable (as opposed to them simply not implementing and making your connecting life hard). So I do not believe it would be useful to protocol-version-suicide-pact the bloom functionality.

If we’re concerned that ignorant node operators may turn off bloom when they don’t need to, we could remove the hidden switch, but I’m not especially concerned about that as bloom saves a lot of bandwidth when not being abused.

Separately, this is what litecoin has implemented since they had the benefit of later knowledge when they did their protocol update that added bloom support. So anything that supports litecoin would have the service bit testing in any case.

petertodd commented at 6:34 pm on August 15, 2013: contributor

Keep in mind that if an alternative node implementation doesn’t want to put in the relatively small amount of effort to implement bloom filters, what makes you think they are going to put in the much larger amount of effort required to really get Bitcoin semantics exactly right regarding convergence? Given that SPV nodes are relatively dependent on their peers - especially if users are accepting zero-conf transactions in any way - it’s reasonable to only want to connect to peers running the satoshi codebase rather than some other implementation.

mikehearn commented at 7:30 pm on August 15, 2013: contributor

So the argument is that because you can ask a peer to use less bandwidth, that makes dos attacks worse?

You can’t fix a denial of service by denying service. That’s backwards, especially as I don’t recall the last time I saw a DoS attacker who actually paid for his own bandwidth. The only way to fix it is to figure out who should be getting service and ensuring they get it first. So why don’t you fix it by allowing nodes to prioritise inbound connections?

petertodd commented at 8:42 pm on August 15, 2013: contributor

@mikehearn As you know the DoS attacks identified are not attacks on bandwidth, but rather other resources. Those attacks are significantly easier to pull off if the attacker can reduce the amount of data the targets send back to them; bloom filters do exactly that by reducing the INV messages the attacker receives, allowing one computer to attack a very large number of nodes with very little bandwidth consumption.

It would be helpful if you responded on the email list with what types of prioritization you would consider most useful for SPV clients - @johndillon attempted to start that discussion but I haven’t seen you reply. Disabling bloom filters is a form of prioritization in of itself of course, as SPV nodes do not contribute back to the network by relaying data. It is much better for them to have temporary issues connecting in an attack than for relaying in general to fail, something that could lead to a dangerous fork due to convergence failure.

Regardless as @gmaxwell pointed out aside from attacks, the protocol should allow for nodes to advertise whether or not they support bloom filters as a matter of good protocol design - it is after all a service. It’s telling that when I mentioned that NODE_BLOOM didn’t exist, @gmaxwell was surprised.

jgarzik commented at 0:15 am on August 16, 2013: contributor

ACK NODE_BLOOM and ACK the entire patch, though myself, I probably wouldn’t have added an option to disable bloom filters in the reference impl

A minor BIP would be nice (like to document every protocol change in a BIP, even though this is terribly minor)

petertodd commented at 1:29 am on August 16, 2013: contributor

@jgarzik Good idea re: BIP.

wtogami commented at 3:57 am on August 16, 2013: contributor

Note: When you bump the protocol version it breaks the alert tests. Your options are then to either generate new signed test data every time the protocol version is bumped, or rather fix the alert tests so the protocol version does not matter.

https://github.com/litecoin-project/litecoin/commit/dbc5f6d7f4ef7ba38157e12b6d9c52ff99aba695 Litecoin chose the latter option as the alert notify codepath is still sufficiently tested without hard-coded protocol versions within signed test data. We feel it is better to minimize the risk of exposing the alert key, and as a matter of policy it seems ideal to allow protocol bumping without the centralized approval of the alert key signer.

gavinandresen commented at 4:14 am on August 16, 2013: contributor

I’m curious to see what @SergioDemianLerner thinks of this patch.

My concern is that if there IS a valid DoS attack on bloom-filter nodes (the “attack”, as I understand it, is supposed to cause excessive disk seeking looking for transactions, yes? “meh” – worst case is “node gets slow” if it isn’t running with big disk cache buffers or from an SSD), then adding a NODE_BLOOM bit will just make it easier for attackers to find them and attack them.

luke-jr commented at 6:18 am on August 16, 2013: member

We only have 64 service bits. It would make sense to only use them when nodes benefit from not implementing features. On the other hand, having a separate service bit means notes can support bloom filters, but not NODE_NETWORK…

petertodd commented at 8:01 am on August 16, 2013: contributor

0uint64 nServices;

We’ve got 64 of them - I don’t think we’re about to run out…

petertodd commented at 8:09 am on August 16, 2013: contributor

@gavinandresen “node gets slow” is a potentially serious problem. “making it easier for attackers to find them”: Serving SPV nodes is far less important to the health of the network than ensuring relaying works and consensus is maintained.

Next time I’d suggest letting people finish exploring the issue before you assume revealing the exploit publicly won’t do any harm.

mikehearn commented at 10:40 am on August 16, 2013: contributor

Can we cut it out with “revealing the exploit” stuff? It’s trivial to find DoS attacks on bitcoind. I did it for fun last time this came up and it took about 45 minutes. I was going to post about it (it’s a memory bloat issue) and then got distracted with the RNG stuff.

Why would we make bloom filtering optional? There’s no benefit to be had from that at this point. If somebody wants to reimplement the protocol for some reason, they can reimplement that too. No big deal. Clients that rely on it are not dramatically less important than other nodes. They’re called “end users” and they’re extremely important!

You can force nodes to do arbitrary amounts of disk seeking with or without Bloom filtering. Just ask for random blocks in the chain. Asking them to filter the blocks adds some CPU load, but that just multiplies resource exhaustion dimensions from 1 to 2. Recall that most average hard disks can only manage about 100 seeks per second. Connect 100 times, request one random block per second per connection and all the seeks are gone. Bandwidth usage is hardly a problem. Nodes don’t remember what data elements they already served so you can just do that forever, but even if they did you could just reconnect every so often to make them forget.

I think I’ve explained how to implement a working anti-DoS strategy several times by now. If I didn’t reply to John it’s only because this issue keeps coming up and I feel like I’m repeating myself.

First step - make nodes understand their own resource limits. This is useful anyway because an operator may not want bitcoind to eat all available machine resources, and currently it will just if it legitimately gets busy.

Second step - implement handlers that run when the node is running out of resources, that perform load shedding. For example, throwing out low priority transactions in the mempool, disconnecting peers that score badly (idle for long periods of time, use lots more memory than would be expected, etc). Coming up with good scoring functions is a big part of the art of DoS defence. This is where you’d score transactions before signature checking as well.

Third step - once you force attackers to basically act just like regular network nodes, you can introduce an optional cookie mechanism so if there are suddenly thousands of clients that appear to be well behaved, clients that have a long history get priority over clients that appeared 5 minutes ago. This also helps in the case where Bitcoin “suffers” a sudden spike in popularity. Established users take priority over new users.

Final step - optimise everything so the amount of load you can handle before falling over gets pushed higher and higher.

The wrong strategy is the one being pursued here:

Find a feature that uses resources
Panic and disable it
GOTO 1

The last time this happened I described the strategy as a “death spiral” and I wasn’t joking.

petertodd commented at 11:49 am on August 16, 2013: contributor

Mike, enough with the overheated rhetoric.

Occasionally nodes will have reasons not to offer the bloom filter service, while still having block data. (NODE_NETWORK) Right now the protocol doesn’t give any way to say that, adding the NODE_BLOOM service bit lets you do that.

Of course, if you are going to have NODE_BLOOM, it’s useful to be able to disable the bloom service for testing SPV clients, hence the undocumented command line flag. The code implementing the feature also shows what should happen if a node doesn’t support bloom filters: kick peers requiring that feature so they won’t waste their bandwidth. It’d also be useful as a temporary emergency measure if a DoS attack is launched, but in the meantime we don’t have any reason to expect users to use the flag. It is unfortunate that Gavin’s revealed a particularly effective one, but that’s life. In any case, finding DoS attacks may be trivial for you, but our attackers don’t seem to find them on their own and seem to only launch them after they have been revealed publicly prior to a patch.

If you want to talk about priority schemes, move it to the email list and reply to @johndillon’s thread rather than cluttering up this pull-req with off-topic discussion. Your ideas only work against some types of attacks in any case.

petertodd commented at 11:52 am on August 16, 2013: contributor

@wtogami I suggested awhile back to make it easy to set the alert keys locally for testing - maybe that’s the way to handle testing them? You’d still have to regenerate the test cases every time the protocol version was changed, but that could be turned into a simple script using a known privkey.

Might be a better idea in general: because the key is what’s non-standard, you can have test cases that use times that are standard without risking re-use of the test alerts on the network.

SergioDemianLerner commented at 4:07 pm on August 16, 2013: contributor

I don’t have access to the DoS report of issue it’s been discussed, but I can imagine.

Any tool (or NODE_ bit) that gives the users the ability to mitigate an eventual DoS attack would be welcomed by the community. Bitcoin network protocol is not “fair”, peers do not send the exact amount of useful information they receive. This is a true altruistic network, so I think people won’t use these bits to discriminate peers.

SergioDemianLerner commented at 4:15 pm on August 16, 2013: contributor

Regarding using the bit NODE_BLOOM maliciously to detect “vulnerable” nodes, I don’t think this ease much an attack, since bloom enabled nodes can been detected by many different ways indirectly. (e.g. connecting to the victim with two peers, and testing if txs are filtered or not).

petertodd commented at 4:18 pm on August 16, 2013: contributor

Another way of looking at it is Gavin is worried about NODE_BLOOM making it easier to detect “vulnerable” nodes, and his solution is to make all nodes vulnerable.

wtogami commented at 11:58 am on August 17, 2013: contributor

ACK to NODE_BLOOM and this particular patch. We will however need a solution for the broken alert tests, preferably one that does not require the alert key signer to have de facto centralized control over any protocol version bumps.

gmaxwell commented at 12:06 pm on August 17, 2013: contributor

@wtogami change your test to run with a throwaway key which is only used in testing mode if thats your concern.

wtogami commented at 12:08 pm on August 17, 2013: contributor

@gmaxwell Does that mean you suggest that be added as a second commit to this PR?

gmaxwell commented at 12:26 pm on August 17, 2013: contributor

@wtogami No, I don’t care, updating the protocol version is infrequent enough that we can just fix the test after doing so. “Defacto centeral control” is not a concern there, if you are having trouble getting the test updated you just make the change I suggested, remove the test, or remove the alert key entirely.

sipa commented at 3:46 pm on August 25, 2013: member

@gavinandresen I don’t really follow your reasoning; assuming an attack exists (ignoring whether it does or not for now), then forcing everyone to be vulnerable is certainly worse than making it optional and advertizing it (perhaps combining it with stronger anti-DoS measures, knowing that it may reduce availability of other offered services).

I do agree with @mikehearn that a decent resource-limitating implementation is necessary as a generic solution against DoS attacks. I do believe that’s orthogonal to the approach taken by this change, though.

The services offered by P2P nodes to the network (for free!) are quite distinct, and have different use cases, different clients, and different attack models:

(unfiltered) old blocks are only necessary for full nodes that are synchronizing. The only thing that matters is bandwidth really (latency is only important in so far that extreme degradation would increase sybil-vulnerability).
(unfiltered) recent blocks are only necessary for full nodes keeping up with the chain, and mostly need low latency to keep the convergence speed of the network fast. Bandwidth is only important when it starts influencing latency.
(filtered) blocks are only necessary for SPV nodes, and the same old/new distinction exists (old blocks need bandwidth, new blocks need latency).
lone transactions are only necessary for distribution to miners and (to the extent possible) aim of the network to prevent 0-conf double spending. Until the payment protocol takes off, it also matters for distribution to receivers, but this isn’t necessary IMHO. I believe all this is mostly best-effort in any case, and secondary to the other services offered.

In my opinion, these are sufficiently independent from each other that they should be easily isolatable. We’re relying on charity of those running full nodes to provide these services, and which of these they consider important for the survival of the network may differ. Additionally, not all of these are necessarily present to the same extent in the network - there may be more demand for some than for others. This all speaks in favor of having separate services bits for them. Maybe at some point more specialized and separately-maintained software for each exists, though that’s probably not for soon.

On the other hand, requiring full node implementations to support SPV functionality probably benefits the popularity of Bitcoin as a payment system, and may improve its usefulness to the economy. “Surviving” up to the point where we’re worried about certain attacks may be more important than dealing with those attacks in the first place. Still, ultimately this is about whether alternate full node implementations are allowed to not implement SPV services, and they may have reasons not to (perhaps because they offer a competing lightweight client model). If the choice is between them not implementing Bloom filtering (and ignore requests, or disconnect in case of such requests), and them being able to advertize not supporting it in the first place, I certainly choose the latter.

I lean towards ACK, but this discussion probably belongs elsewhere (it’s not entirely specific to the reference client), and certainly warrants a BIP. The implementation looks good in any case.

petertodd commented at 4:12 pm on August 26, 2013: contributor

A good example where the very different requirements of the different types of nodes matters is for anyone with a lot of bandwidth available to them, but not a lot of disk io or memory, a common scenario for servers in datacenters.

The most efficient way to push blocks over the wire for such a server would be to use sendfile() to do a direct copy from the blockchain data on disk to the network interface - behind the scenes Linux will use direct zero-copy DMA transfers of the data from the disk interface to the network interface, or system memory to the network interface, using almost no CPU in the process. On the other hand that means the blocks have to be sent unfiltered, which is fine if my peer needs the whole block. This arrangement uses the resources I have available most efficiently for the sake of the network.

But if I start accepting SPV clients I can’t use sendfile() anymore, and even worse is that because I’m serving those SPV clients filtered blocks I’m using up far more disk IO than network bandwidth, when what I have available to me is the opposite.

Interestingly the opposite case is common too: lots of home users running nodes have plenty of disk IO available, especially if they have SSD’s, but very poor upload capacity. It would probably make sense for full nodes to prefer to connect to nodes without NODE_BLOOM set, especially if they are syncing a lot of archival history, leaving capacity on the NODE_BLOOM capable nodes for SPV clients that need it.

petertodd commented at 4:46 pm on August 26, 2013: contributor

Also, here is my draft BIP: http://www.mail-archive.com/bitcoin-development@lists.sourceforge.net/msg02751.html

gavinandresen commented at 2:16 am on October 21, 2013: contributor

Needs rebase and a test plan. But I still think this is a bad idea….

petertodd commented at 3:02 am on October 21, 2013: contributor

@gavinandresen rebased

Current test plan and results has been as follows:

Bloom filters enabled, NODE_BLOOM set, protocol version bump (default settings)

Peers connect normally, no observed changes: PASS Android Bitcoin Wallet connects successfully: PASS Android Bitcoin Wallet connects as only peer: PASS

Bloom filters disabled, NODE_BLOOM unset (-bloomfilters=0)

Non-bloom-using nodes connect normally: PASS Bloom-using nodes kicked: PASS No bloom-using nodes seen in getpeerinfo (which would indicate they don’t give up): PASS Android Bitcoin wallet w/ !NODE_BLOOM peer set as trusted peer and with DNS peer discovery enabled: PASS (fails to connect to the peer, but behaves normally otherwise) Android Bitcoin wallet w/ !NODE_BLOOM peer as only peer: PASS, although the wallet code never gives up, connecting multiple times a second. But that’s a bug in the Android wallet that should be fixed.

DNS seeds @sipa implementation https://github.com/sipa/bitcoin-seeder ignores NODE_BLOOM: PASS @TheBlueMatt’s implementation https://github.com/TheBlueMatt/dnsseed-bitcoinj ignores NODE_BLOOM: PASS

Note that I only checked that the source code for both tests for NODE_NETWORK with an and, and thus will ignore other service bits being set. We can modify the dns seeds to filter requested service bits later if required; useful later for other things like the proposed NODE_ARCHIVAL_BLOCKCHAIN_DATA-type stuff.

petertodd commented at 3:05 am on October 21, 2013: contributor

Note that the alert tests need to be fixed because the protocol version was incremented, as discussed above; if disabled manually all other tests run fine.

wtogami commented at 10:32 am on October 21, 2013: contributor

https://code.google.com/p/bitcoinj/source/detail?r=2c44a4fad7faccfe9b1392c67f60d21b25703bde The current Android Wallet logic is to determine if a peer does bloom only from the protocol version. bitcoinj would need to learn how to to read service bits.

petertodd commented at 10:36 am on October 21, 2013: contributor

@wtogami Sure, but that’s not something that prevents merging this patch; it’s compatible with older SPV implementations. More to the point, those implementations aren’t going to change until this patch is merged.

wtogami commented at 10:38 am on October 21, 2013: contributor

Sorry, I didn’t mean that is a blocker for this patch, it was meant as a FYI.

mikehearn commented at 1:23 pm on October 21, 2013: contributor

Um, old SPV implementations are not compatible with this patch - it is inherently impossible for them to be so. A node that opts out of a previously mandatory feature will still be expected to provide it by old software. If an old client connects to a new server that has the feature disabled, it will hang as it waits for a response to a message that the remote server has ignored.

This patch is just all kinds of bad news. Please stop it. People who want to serve the chain but not do computational work on behalf of clients can just seed the chain torrent instead.

petertodd commented at 1:35 pm on October 21, 2013: contributor

@mikehearn Read the patch prior to commenting about it; it automatically kicks older peers so they won’t waste their bandwidth and my draft BIP says that behavior is a must.

Also, the requirements of serving archival and bloomfilter-using nodes are very different and can productively be optimized differently: #2900 (comment)

mikehearn commented at 1:41 pm on October 21, 2013: contributor

Yes, I read that comment. That’s why I suggested serving the chain torrent if you have lots of bandwidth and not much CPU. Or heck, just serve snapshots of the chain via HTTP. Then you have a piece of software and protocol actually designed for serving large files, instead of bitcoin+p2p protocol which simply isn’t.

petertodd commented at 1:49 pm on October 21, 2013: contributor

So you agree that it’s backwards compatible with older SPV implementations? Any other issues?

I don’t see why we want to depend on a clunky system of requiring separate manual torrent/HTTP downloads, especially given ideas like partial UTXO mode to transparently bring nodes from SPV security to full over time in the background.

mikehearn commented at 2:03 pm on October 21, 2013: contributor

Your patch is “backwards compatible” as long as most nodes don’t use it, which is not the meaning most people would associate with the term. Worse, you implemented this by DoS banning the IP address, which fails to take into account that many users (especially on mobile) sit behind giant NAT boxes. A single user who didn’t upgrade could get their entire local cell tower or even city banned from every node with your patch activated, even for clients that are upgraded.

You could make it backwards compatible by not having such nodes take part in the regular P2P network at all and provide a separate DNS seed for them, so old clients would never see them at all, but then if you’re going to have a separate P2P network why not use a P2P system that’s actually designed for file distribution, like BitTorrent?

Ignoring for now that Bitcoin-Qt doesn’t have any SPV support and nobody is working on any, fancy automatic migration of SPV nodes to full nodes doesn’t make sense from a user experience perspective - the requirements are just so different, such arguments don’t persuade me.

As it is, the maintainer of the serving side says this patch is a bad idea, the maintainer of the client side says this is a bad idea, so what do you hope to achieve by keeping this issue open? It gets quite frustrating when the people who are actually maintaining the code bases that are affected by this patch both say “this is a bad idea” and you just plough on regardless.

petertodd commented at 2:28 pm on October 21, 2013: contributor

Ah good, finally you looked at the code. Anyone have any comments re: the DoS behavior? I could potentially change it to simply close the associated connection.

Service bits are meant to advertise services, and performing computations on behalf of clients who don’t serve other clients sounds very much like a service to me. That said I’d be very surprised to see many nodes operators disabling bloom filters as the command-line switch to do so is undocumented, so we’ve got plenty of time to upgrade DNS seeds to let nodes pick service bits they want, or just require NODE_BLOOM to be set if protocol version >=70002. (DNS seeds are just for bootstrapping after all and nodes should use them infrequently; no sense putting too much effort into them vs. decentralized peer discovery)

I’m sure more people would run Bitcoin-QT full-nodes if it was useful immediately, albeit with reduced SPV security. We could certainly use more full nodes. You like to say how businesses will “obviously” run full nodes - maybe we can make it easier for them by getting their nodes up and running as fast as possible. An attractive alternative is to just use a central service immediately rather than waiting after all.

Note the positive comments and ACKS from gmaxwell, sipa, jgarzik, and wtogami - the reference client is maintained by a group of people, not just one guy who calls the shots.

TheBlueMatt commented at 2:31 pm on October 21, 2013: member

I think it may make more sense to start advertising NODE_BLOOM before we add a (even undocumented) option to disable it. That could ease the transition significantly

petertodd commented at 2:53 pm on October 21, 2013: contributor

@TheBlueMatt If the sticking point for people is the fact that there is that option, I’ll remove it.

wtogami commented at 6:39 pm on October 21, 2013: contributor

our patch is “backwards compatible” as long as most nodes don’t use it

NODE_BLOOM enabled-by-default with 0.9 combined with always-enabled with 0.8.x means most nodes will use it.

I think it may make more sense to start advertising NODE_BLOOM before we add a (even undocumented) option to disable it. That could ease the transition significantly

Please. No. Removing the option would mean other clients can entirely opt out of paying attention to the service bit, thereby rendering it pointless. The option being widely deployed at least makes it very easy to tell the world how to defend against a particular type of problem if it were to happen, which may be enough to discourage that problem from happening as they would know the network can bounce back very quickly without any software update. @petertodd In retrospect, the 24 hour ban was too heavy handed. Mike is right about it being too easy to ban an entire IP.

gavinandresen commented at 10:32 pm on October 21, 2013: contributor

The “sticking point” for me is practical: every service bit adds complication– another possible configuration that should be tested, but probably won’t be (which makes attacks more likely).

I have seen zero evidence that requiring that every NODE_NETWORK node support bloom filters causes anything more than temporary denial-of-service problems for under-powered full nodes, and I see large benefits to requiring that all full nodes support SPV clients.

TheBlueMatt commented at 1:53 am on October 22, 2013: member

On Mon, 2013-10-21 at 11:39 -0700, Warren Togami wrote:

0    I think it may make more sense to start advertising NODE_BLOOM
1    before we add a (even undocumented) option to disable it. That
2    could ease the transition significantly
Please. No. Removing the option would mean other clients can entirely opt out of paying attention to the service bit, thereby rendering it pointless. The option being widely deployed at least makes it very easy to tell the world how to defend against a particular type of problem if it were to happen, which may be enough to discourage that problem from happening as they would know the network can bounce back very quickly without any software update.

Umm…what? people will refuse to implement a new standard because its not actively used but will break their apps in the future if they dont? Somehow I don’t see why this would be true.

petertodd commented at 6:15 am on October 26, 2013: contributor

Updated to use CloseSocketDisconnect() rather than a 24-hour DoS-ban.

Tested with my Android wallet, which happily successfully connects then is disconnected about five times a second forever…

Off-topic here, but @schildbach I’d suggest some kind of back-off algorithm, or at least waiting a second or two. It even does that on a cellular data connection, which could get rather expensive.

schildbach commented at 7:23 am on October 26, 2013: contributor

@petertodd Yes, that’s a well known regression in bitcoinj: http://code.google.com/p/bitcoinj/issues/detail?id=296

laanwj added the label P2P on May 9, 2014

laanwj commented at 1:50 pm on May 28, 2014: member

There has been no activity in this issue for more than half a year. What’s the way forward here?

It is clear that next time that a service is introduced on the P2P network we have to be more careful, and add a services bit (or another way) to advertise the service instead of making it mandatory at a protocol version. This makes it possible for the node owner to choose whether to run the service or not, and to disable it if problems arise.

However for Bloom filtering I think this ship has sailed. All SPV clients rely on this functionality to be available with NODE_NETWORK, and it seems unreasonable to break this expectation in a new protocol version.

leofidus commented at 4:11 pm on May 28, 2014: none

Both Litecoin and Dogecoin as well as their forks have deployed this patch without problems. From what I’ve seen an heard, their SPV clients are not acutally looking for the service bit, since this patch is designed to be backwards compatible and there are only few nodes which disable bloom filters. Instead they are disconnected when trying to set the bloom filter and choose another node instead.

I think giving node operators the option to influence cpu/bandwith tradeoffs based on their needs is a good idea, and experience from altcoins seems to suggest that SPV clients are not negatively impacted by this change in a notable way.

mikehearn commented at 4:22 pm on May 28, 2014: contributor

Arguing for a patch on the grounds that not enough people use it to be harmful is not a great start. But I think we went around this six months ago and the conclusion was not to apply - the pull req should just have been closed back then.

The right way to solve the problem (is it a problem yet?) of nodes running out of CPU is to teach them how to prioritise within their capabilities, not expect node operators to learn complicated administration tasks. If bitcoind were routinely saturating my cheap little VPS’ CPU this sort of quick hack would be a lot more compelling, but there’s no sign of such problems. The top complaint at the moment is bandwidth not CPU.

laanwj commented at 4:46 pm on May 28, 2014: member

Right, I run various nodes on cheap ARM boxes and have never seen significant CPU usage after the initial block sync (and from that, ECDSA verification is by far the most expensive operation). Bloom filtering is really efficient. If anyone can show with benchmarks/profiles that serving SPV nodes filtered blocks is actually a problem in practice, I’ll gladly reconsider my opinion.

Bandwidth is another matter, but disabling bloom certainly won’t help with that.

jgarzik commented at 5:36 pm on May 28, 2014: contributor

AFAIK, this is not about CPU usage or network bandwidth at all, but about the ability of remote attackers to DoS a node’s disk bandwidth. The capability bit provides a way to opt out of one DoS attack vector.

Do we need this? My feeling is “NAK” but it is really a protocol change, and thus requires a BIP and some email list discussion (the latter of which has already happened, IIRC).

Even if we all agreed this is ACK-worthy, it would still need a BIP, as this is a network-wide behavior change.

wtogami commented at 6:25 pm on May 28, 2014: contributor

Wouldn’t it be a good idea to have for full node implementations like btcd which do not support bloom?

laanwj commented at 7:19 pm on May 28, 2014: member

This discussion has been going on for a long time. A draft BIP was written, as well as discussion on the mailing list: http://sourceforge.net/p/bitcoin/mailman/message/31298904/ .

If this is to be merged it at least needs a rebase.

In any case there needs to be a decision, this has dragged on for too long, and no one seems to care much either way. If we don’t absolutely need it it’s IMO better to not do it.

petertodd commented at 7:37 pm on May 28, 2014: contributor

I’ll be submitting the BIP to the bitcoin/bips repo once I get back next week. @wtogami Exactly. Obelisk nodes don’t support bloom filters either; they will in the future support the more scalable and private prefix filters. Eventually we’ll likely wind up with NODE_PREFIX in addition to NODE_BLOOM.

mikehearn commented at 7:58 pm on May 28, 2014: contributor

@laanwj I think @gavinandresen’s announcement has left the chain of command a little unclear, but I guess if you want a decision, it’s up to you to make one. Asking for one just makes us all restate our positions again, it doesn’t lead to a decision :)

Add nLocalServices info to RPC getinfo c50f2b4de8

Add NODE_BLOOM service bit and bump protocol version

Lets nodes advertise that they offer bloom filter support explicitly.
The protocol version bump allows SPV nodes to assume that NODE_BLOOM is
set if NODE_NETWORK is set for pre-70003 nodes.

Also adds an undocumented option to turn bloom filter support off for
testing purposes. Nodes attempting to use bloom filters are immediately
dropped so as to not waste their bandwidth.

29834c0a07

BitcoinPullTester commented at 7:43 am on June 6, 2014: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/29834c0a077ddce249a8822fe2549e3dc5eb08c1 for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.

gavinandresen commented at 1:44 pm on June 12, 2014: contributor

Closing, no consensus this is a good idea.

gavinandresen closed this on Jun 12, 2014

DrahtBot locked this on Sep 8, 2021

Add NODE_BLOOM service bit and bump protocol version #2900