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

filter unworkable peers in queries + enhanced logging #363

Merged
merged 16 commits into from
Jun 26, 2019

Conversation

raulk
Copy link
Member

@raulk raulk commented Jun 26, 2019

The critical contribution of this PR is preventing adding useless peers to our queries. The definition of useless is:

  • peers with no addresses.
  • peers with relay addresses (therefore unupgraded DHT clients).
  • peers with no publicly routable addresses (unless we're connected to the peer via LAN).

Secondly it also adds outbound filtering so that upgraded nodes will contextualise the peers they send back, based on whether the connection with the requester is via LAN or not.

@raulk raulk requested a review from whyrusleeping June 26, 2019 11:45
@raulk raulk changed the base branch from feat/routing-table-filter to stabilize June 26, 2019 11:56
@raulk raulk changed the title initial stab at filtering peers in queries filter unworkable peers in queries Jun 26, 2019
@raulk raulk marked this pull request as ready for review June 26, 2019 19:04
@raulk raulk changed the title filter unworkable peers in queries filter unworkable peers in queries + enhanced logging Jun 26, 2019
@raulk raulk merged commit 8fe679a into stabilize Jun 26, 2019
@raulk raulk deleted the feat/query-filters branch June 26, 2019 19:12
@Stebalien
Copy link
Member

We need to put/get to a consistent set of peers. This current patch doesn't do that. It may work in practice because, unless we're entirely offline, we're only likely to find at most one "closest peer" on the local network (out of K=20) but it's still incorrect and this at least needs to be documented somewhere.

We could also:

  1. Have a disconnected/connected "switch": when we add the first non-local peer to our routing table, we remove all unroutable peers, when we run out of routable peers, we rebuild our routing table from unroutable peers. This won't break local peer routing as we return the target of FindPeer even if it isn't in our routing table.
  2. Make sure to return at least 20 global peers if we have them, even if we have 20 closer local peers. Also make sure to put to 20 routable peers. This solution is closest to the existing solution but really hard to get right.
  3. Run two DHTs (overhead should be minimal as the "local" DHT should be small). This is probably the most elegant solution.

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.

3 participants