-
Notifications
You must be signed in to change notification settings - Fork 37.8k
Provide bloom services to whitelisted nodes. #8587
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
Conversation
} | ||
strCommand == NetMsgType::FILTERCLEAR)) { | ||
LOCK(cs_main); | ||
Misbehaving(pfrom->GetId(), 5); |
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.
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..!
NACK on overloading whitelisting even further. It's already not well-defined nor documented. |
NACK I doesn't even compile! It is |
6ab8fd4
to
cd5cda9
Compare
@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. |
c3e9c90
to
bfc282c
Compare
@venzen your message above seemed to get mangled somehow - not sure if I read your message about VERACK - will check the pull request now. |
bfc282c
to
86574e5
Compare
@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. |
Closing this. Two NACKS and no other comments. A new PR could be opened to some whitelist documentation. |
Don't disconnect whitelisted nodes when bloom services are requested.