-
Notifications
You must be signed in to change notification settings - Fork 37.8k
De-neuter NODE_BLOOM #7708
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
De-neuter NODE_BLOOM #7708
Conversation
See also #6579 for discussions. utACK. |
@@ -4377,7 +4377,7 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, | |||
if (pfrom->nVersion >= NO_BLOOM_VERSION) { | |||
Misbehaving(pfrom->GetId(), 100); | |||
return false; | |||
} else if (GetBoolArg("-enforcenodebloom", DEFAULT_ENFORCENODEBLOOM)) { | |||
} else { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DEFAULT_ENFORCENODEBLOOM constant in main.h needs to go too :)
Has someone checked what impact this would have now? (Wouldn't preclude a merge in master, since there is still a fair amount of time to respond before release-- but it would be good to know). |
@gmaxwell Very little, I expect. the intersection of nodes that::
Is likely to be very, very small. This is just continuing the planning, in 0.12 NODE_BLOOM was introduced but not yet enforced (by default) for old versions, and the plan is that in 0.13 it will be enforced for all versions. What kind of measurement do you think should change that planning? In no case this is going to be backported to 0.12. |
Added a counter to my SPV stats node to see how many SPV nodes have not adapted the new rule. |
Ah. Indeed, I was forgetting that with peerbloomfilters defaulting to true this would have no effect. Still, interesting to know if there has been any progress or not. |
d2aee0b
to
c90036f
Compare
@laanwj nit picked |
ACK c90036f |
c90036f Always disconnect old nodes which request filtered connections. (Patrick Strateman)
@pstratem What is the impact of not disconnecting these nodes? |
@rebroad: check https://github.com/bitcoin/bips/blob/master/bip-0111.mediawiki. Not all nodes want to provide the BLOOM FILTER service. SPV bloom filtering will consume lots of CPU and disk usage. The produced and consumed data will very likely be worthless for the p2p health after generating/transmitting (where a BLOCK or TX structure will be relay further by other full nodes to other full nodes). BIP-0111 will help SPV clients to detect, which peers do and don't support the NODE_BLOOM service. Otherwise they need to try and disconnect in case they won't. |
And not disconnecting them when they request a bloom filter which we don't
provide would make them unnecessarily stall. By disconnecting you give them
an opportunity to find a better peer.
|
@jonasnick @sipa thanks for explanaing |
c90036f Always disconnect old nodes which request filtered connections. (Patrick Strateman)
Disconnect nodes which request bloom filtering in violation of the protocol.