Skip to content
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

peer: Strictly enforce bloom filter service bit. #768

Merged
merged 2 commits into from
Jul 27, 2017

Conversation

davecgh
Copy link
Member

@davecgh davecgh commented Jul 26, 2017

Upstream commit 8965d88.


This makes the enforcement of the bloom filter service bit much more strict. In particular, it does the following:

  • Moves the enforcement of the bloom filter service bit out of the peer package and into the server so the server can ban as necessary
  • Disconnect peers that send filter commands when the server is configured to disable them regardless of the protocol version
  • Bans peers that are a high enough protocol version that they are supposed to observe the service bit is disabled, but ignore it and send filter commands regardless.

As an added bonus, this fixes the old logic which had a bug in that it was examining the remote peer's supported services in order to choose whether or not to disconnect instead of the local server's supported services.

This makes the enforcement of the bloom filter service bit much more
strict.  In particular, it does the following:

- Moves the enforcement of the bloom filter service bit out of the peer
  package and into the server so the server can ban as necessary
- Disconnect peers that send filter commands when the server is
  configured to disable them regardless of the protocol version
- Bans peers that are a high enough protocol version that they are
  supposed to observe the service bit is disabled, but ignore it and
  send filter commands regardless.

As an added bonus, this fixes the old logic which had a bug in that it
was examining the *remote* peer's supported services in order to choose
whether or not to disconnect instead of the *local* server's supported
services.
// whether or not banning is enabled, it is checked here as well
// to ensure the violation is logged and the peer is
// disconnected regardless.
if sp.ProtocolVersion() >= wire.BIP0111Version &&
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if a whitelisted check should go here... (&& !sp.isWhitelisted)

Copy link
Member Author

Choose a reason for hiding this comment

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

I would prefer that go into a separate PR, if we even want it at all.

Copy link
Member

@dajohi dajohi left a comment

Choose a reason for hiding this comment

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

ok

@davecgh davecgh merged commit 3d5ceb1 into decred:master Jul 27, 2017
@davecgh davecgh deleted the merge_peer_strict_node_bloom branch July 27, 2017 16:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants