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

Limit the number of initial peers #2913

Merged
merged 7 commits into from
Oct 21, 2021
Merged

Conversation

oxarbitrage
Copy link
Contributor

Motivation

We want to limit the number of initial peer attempts by config. Close #2900

Solution

Given the hashset of peers and a config parameter acting as the limit:

  • convert the hashset to a vec.
  • get a rand chunk from it using the standard library.
  • convert back to hashset and use this from now on.

Review

Anyone can review. This is one of the security issues from created by @teor2345 for the beta.

teor2345
teor2345 previously approved these changes Oct 21, 2021
Copy link
Contributor

@teor2345 teor2345 left a comment

Choose a reason for hiding this comment

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

This looks good.

If you can think of an easy way to test it, we could add tests. But I can't think of any quick tests, and the code is simple enough, so we can merge without tests.

Let's merge after #2912 merges, because there are more tickets depending on #2912.

@teor2345 teor2345 self-assigned this Oct 21, 2021
Also fix a comment, and send the whole zebra_network::Config to a method.
This way, we can never get the unused peers out.
This reverts commit 81ede59.

Actually, this doesn't work, because we want those extra peers.
@teor2345 teor2345 enabled auto-merge (squash) October 21, 2021 22:35
Copy link
Contributor

@teor2345 teor2345 left a comment

Choose a reason for hiding this comment

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

I resolved conflicts and moved some more code out of zebra_network::initialize

@teor2345 teor2345 merged commit 2de93bb into ZcashFoundation:main Oct 21, 2021
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.

Security: Limit the number of initial connection attempts to seed peers
3 participants