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

Make peer-info protocol optional. #1698

Merged
merged 1 commit into from
Jul 26, 2023
Merged

Make peer-info protocol optional. #1698

merged 1 commit into from
Jul 26, 2023

Conversation

shamil-gadelshin
Copy link
Member

This PR sets peer-info protocol optional for bootstrap-node. We opted for a simpler design of peer-info protocol and didn't implement requesting cuckoo-filter from farmers. We push cuckoo-filter unconditionally to new connections and it's not suitable for bootstrap nodes. Bootstrap-node will receive thousands of cuckoo-filters from bootstrapping farmers which is unnecessary and could lead to a choke point.

Code contributor checklist:

@shamil-gadelshin shamil-gadelshin added the networking Subspace networking (DSN) label Jul 26, 2023
@shamil-gadelshin shamil-gadelshin self-assigned this Jul 26, 2023
Copy link
Member

@nazar-pc nazar-pc left a comment

Choose a reason for hiding this comment

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

In this case should we probably rename PeerInfo into FarmerInfo and simplify it to only work for farmers? We can always introduce a new protocol later.

@shamil-gadelshin
Copy link
Member Author

In this case should we probably rename PeerInfo into FarmerInfo and simplify it to only work for farmers? We can always introduce a new protocol later.

I understand your viewpoint, however, I'd like to keep it at least until #1681
I'd like to try using peer-info to clean up unnecessary records from Kademlia DHT.

The proper solution of the original problem would be to reconsider peer-info design and implement three protocol phases (send, receive, send/receive extra-data) instead of two phases like now. And this PR is a shortcut.

@shamil-gadelshin shamil-gadelshin merged commit 146c92a into main Jul 26, 2023
@shamil-gadelshin shamil-gadelshin deleted the peer-info-upgrade branch July 26, 2023 15:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
networking Subspace networking (DSN)
Projects
Development

Successfully merging this pull request may close these issues.

2 participants