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 peers using non-std ports #17

Merged
merged 1 commit into from
Jun 7, 2022
Merged

Conversation

conradoplg
Copy link
Contributor

@conradoplg conradoplg commented Jun 2, 2022

We list all crawled IPs, but some of them use non-standard ports. But clients have no way to know what are the port of the peers, so we must only list ones using default ports.

I did the simplest thing that worked which is filtering at the last possible moment, when the seeder is queried. Since the current implementation is not particularly optimized (it iterates all the peers and shuffles them), this seems OK for now.

Closes ZcashFoundation/zebra#4547

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.

Looks good!

The port in the config is a string, which is more error-prone. But I don't want to change that existing code now. I'd rather we focus on replacing the seeder long-term.

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.

Can we also filter Flux/ZelCash peers as soon as we receive their addresses?

Flux didn't change their network magic, so our seeders will end up mapping their entire network as soon as a single Flux peer is in our peer set. (And they are already in Zcash node peer sets.)

But we can drop peer addresses on ports 16125 and 26125 as soon as they are received. That way, our seeders never make connections to them.

@conradoplg
Copy link
Contributor Author

Can we also filter Flux/ZelCash peers as soon as we receive their addresses?

Flux didn't change their network magic, so our seeders will end up mapping their entire network as soon as a single Flux peer is in our peer set. (And they are already in Zcash node peer sets.)

But we can drop peer addresses on ports 16125 and 26125 as soon as they are received. That way, our seeders never make connections to them.

I ended up doing that in the next PR #18 for simplicity, let me know if that seems OK

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.

Thanks, looks good!

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.

Check if DNS seeder are working correctly and fix if required
2 participants