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

fix: autorelay: treat static relays as just another peer source #1875

Merged
merged 3 commits into from
Nov 17, 2022

Conversation

MarcoPolo
Copy link
Contributor

fixes #1782 by making static relays a specific type of peer source. Then re-using all the peersource logic. @marten-seemann was there a reason you didn't do this originally?

Copy link
Contributor

@marten-seemann marten-seemann left a comment

Choose a reason for hiding this comment

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

We probably could be a lot smarter about static relays (for example, doing exponential backoff for the ones we can't connect to), but we can leave that for a future PR.

This logic looks fine, and I'm not sure why I didn't implement it like that back then.

Comment on lines +81 to +83
if len(static) < numPeers {
numPeers = len(static)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

The documentation for WithPeerSource says:

Implementations must send at most numPeers, and close the channel when they don't intend to provide any more peers.

Copy link
Contributor

Choose a reason for hiding this comment

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

We can use WithMaxCandidaters to make sure that the relayFinder is asking us for enough peers. We should probably also add some documentation that there's little point in providing a huge number of static relays.

Copy link
Contributor Author

@MarcoPolo MarcoPolo Nov 11, 2022

Choose a reason for hiding this comment

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

This check is to make sure we send at most numPeers. If we don’t have numPeers we’ll only send what we have. Essentially the min(len(static),numPeers)).

I debated using with WithMaxCandidaters, but your right I’ll add it.

Copy link
Contributor

Choose a reason for hiding this comment

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

For some reason I thought the if was the other way round (> instead of <). Sorry for that!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

autorelay: don't discard static relays on disconnections
2 participants