Skip to content

Conversation

rebroad
Copy link
Contributor

@rebroad rebroad commented Aug 25, 2016

Don't disconnect whitelisted nodes when bloom services are requested.

}
strCommand == NetMsgType::FILTERCLEAR)) {
LOCK(cs_main);
Misbehaving(pfrom->GetId(), 5);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea here is that it will be banned if it continues to connect and request bloom services 20 times... I suspect the misbehave count gets reset upon disconnection though..!

@laanwj
Copy link
Member

laanwj commented Aug 25, 2016

NACK on overloading whitelisting even further. It's already not well-defined nor documented.

@paveljanik
Copy link
Contributor

NACK

I doesn't even compile! It is fWhitelisted and not fWhiteListed.

@rebroad rebroad changed the title Provide bloom services to whitelisted nodes. {WIP} Provide bloom services to whitelisted nodes. Aug 26, 2016
@rebroad
Copy link
Contributor Author

rebroad commented Aug 26, 2016

@laanwj Where would be the best place to document this please? I would certainly be willing to give this a go. My thinking is that whitelisted peers are nodes that we trust to not do anything detrimental or misbehave. They are usually nodes we have control of. Therefore, in this example, we could set up a node that doesn't advertise BLOOM and doesn't provide bloom services to nodes other than our own. This would allow us to configure a SPV node to use our node but not let it be used by other SPV nodes. Many SPV nodes, allow it to be configured which full-node to use and trust, but currently there isn't a way to set up a full-node that only provides SPV services to specific nodes (as far as I know anyway), hence this pull request.

I agree that "Whitelisted" is somewhat vague and generic though but it seemed the closest to what was already apt for the job - and it might still be so, but yes, the documentation would be useful.

Checking through previous commits, e.g. beceac9 (#8078), it does look like Whitelisting feature is already being used to provide BLOOM services to whitelisted nodes and no other nodes, so I don't think this pull req deviates from the current use so far.

Regarding who is most qualified to document what Whitelisting is, I would propose that it is the authors who have used it so far, so this would be @petertodd (#8078) @morcos (#7542) @sipa (#7125 #5157 #4378) @gmaxwell (#7166) @pstratem (#7406 #6374) @jonasschnelli (#6984) @laanwj (#6974) @sdaftuar (#5875) @rubensayshi (#5942) - this is an exhaustive list.

@rebroad rebroad changed the title {WIP} Provide bloom services to whitelisted nodes. Provide bloom services to whitelisted nodes. Aug 26, 2016
@rebroad rebroad force-pushed the WhitelistedBloom branch 3 times, most recently from c3e9c90 to bfc282c Compare September 13, 2016 02:38
@rebroad
Copy link
Contributor Author

rebroad commented Sep 13, 2016

@venzen your message above seemed to get mangled somehow - not sure if I read your message about VERACK - will check the pull request now.

@rebroad
Copy link
Contributor Author

rebroad commented Dec 11, 2016

@paveljanik Apologies for it not compiling. Just to let you know I test everything with "make check" as a minimum now before PR anything, and more often than not have tested things for months beforehand.

@fanquake
Copy link
Member

Closing this. Two NACKS and no other comments. A new PR could be opened to some whitelist documentation.

@fanquake fanquake closed this Jan 21, 2017
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants