-
Notifications
You must be signed in to change notification settings - Fork 186
feat: Implement Rendezvous module in py-libp2p #916
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
Conversation
…ction and instantiate RendezvousDiscovery directly
…istration refresh management
…d timeout handling for stream operations
…ry service with nursery support
… logger calls for better log management
… detailed debug messages
Great work @sumanjeet0012 🎉 This is a very important addition to py-libp2p, and I really appreciate the level of depth and completeness you’ve brought into the Rendezvous implementation. Thank you for keeping a note of each of these important aspects/ features:
Wish if you could add a small document/discussion page along with a screencast recording/demo for the implementation. We will play it during the upcoming maintainer's meeting. This closes a long-standing gap between py-libp2p and other implementations like Go/Rust, and brings us much closer to feature parity in peer discovery. The namespace-based discovery patterns also give developers a lot of flexibility beyond just bootstrapping or DHT-based methods. Thanks again for pushing this forward — it’s a big step for the project. CCing @pacrob, @lla-dane and @acul71 for their feedback and review. We plan to scale this module in dapps built using universal connectivity dapp. Reviewing it in detail. Looking forward to getting this merged soon. |
- Add rendezvous.proto to Makefile PB variable for protobuf generation - Fix type annotations in errors.py and messages.py for mypy compatibility - Changes ensure proper protobuf file generation and type checking
@sumanjeet0012 (venv) luca@r17:~/PNL_Launchpad_Curriculum/Libp2p/py-libp2p$ make typecheck
pre-commit run mypy-local --all-files && pre-commit run pyrefly-local --all-files
run mypy with all dev dependencies present...............................Failed
- hook id: mypy-local
- exit code: 1
libp2p/discovery/rendezvous/pb/rendezvous_pb2.py:17: error: Call to untyped function "Default" in typed context [no-untyped-call]
libp2p/discovery/rendezvous/messages.py:9: error: Module "libp2p.discovery.rendezvous.pb.rendezvous_pb2" has no attribute "Message" [attr-defined]
libp2p/discovery/rendezvous/errors.py:5: error: Module "libp2p.discovery.rendezvous.pb.rendezvous_pb2" has no attribute "Message" [attr-defined]
libp2p/discovery/rendezvous/service.py:26: error: Module "libp2p.discovery.rendezvous.pb.rendezvous_pb2" has no attribute "Message" [attr-defined]
libp2p/discovery/rendezvous/client.py:33: error: Module "libp2p.discovery.rendezvous.pb.rendezvous_pb2" has no attribute "Message" [attr-defined]
Found 5 errors in 5 files (checked 200 source files)
make: *** [Makefile:46: typecheck] Error 1 Fix here: sumanjeet0012#1 |
Fix build system integration and type annotations for rendezvous module
@acul71 I have merged your PR. Could you please verify if everything is functioning as expected? |
Hi @sumanjeet0012 "protobuf>=4.25.0,<5.0.0", Hi @pacrob can you help understand why this had happened ? Problem: After merging the PR, we encountered protobuf version compatibility issues causing import errors: ImportError: cannot import name 'runtime_version' from 'google.protobuf' Root Cause: Version mismatch between:
Solution: Upgraded protobuf Python package from Results:
Note: The Everything is now working as expected! 🎉 |
…s client and discovery initialization
…tion in RendezvousService
@seetadev A detailed discussion, along with a short demonstration video of Rendezvous Peer Discovery, is available here: GitHub Discussion #955. |
@sumanjeet0012 : Thank you Sumanjeet. Appreciate your efforts. Reviewing the screencast and the discussion page too. We will wait for feedback from @pacrob. Once the feedback points are marked resolved, we'll also do a final review together. Once I get a thumbs up from @pacrob, we will merge the PR. Will merge this before the upcoming release of py-libp2p. Looking forward to seeing all the peer discovery methods. |
@pacrob I have removed the class Registration and added the peer Id validation in _handle_register. |
…ed for OK status and raising appropriate exceptions for other statuses
@pacrob Ready for final review. |
register.ns = self.namespace | ||
register.peer.id = self.peer_id.to_bytes() | ||
register.peer.addrs.extend(self.addrs) | ||
register.ttl = self.ttl |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Non-blocking question: This will return the original ttl set, even if it's about to expire. Would it be helpful to peers if we proivded remaining ttl instead of the original value?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, we should send the remaining TTL instead of initial TTL value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! One question, but not blocking merge.
@sumanjeet0012 : Thank you for your continued efforts on implementing Rendezvous module in py-libp2p. Appreciate it. @pacrob : +1, thank you so much for your review and guidance. Appreciate it. I spent sometime trying to understand where Rendezvous peer discovery module would be useful. Wish to recommend @sumanjeet0012 to share some test cases around it. We are very near to merging this PR. @sumanjeet0012 , @pacrob : Please visit #965 |
…-time TTL handling.
…ts and improve error handling.
@sumanjeet0012 : Great to hear. Appreciate your efforts. In reference to kad-dht test failure, it is not just your PR but a couple of others too. Documented it at #949 . Wish if you could test locally and share review at #963. @yashksaini-coder is making good progress on the PR. Will ask him to add you as a collaborator. |
@seetadev Could you please confirm if there’s anything pending in this PR? If not, we can proceed with merging it. |
@sumanjeet0012 : Hi Sumanjeet. Did a final review. This PR is good to merge. Appreciate your great efforts. Let us know try developing some examples using Rendezvous module in py-libp2p. |
What was wrong?
Fixes #898
The py-libp2p library lacked a Rendezvous protocol implementation for peer discovery. This missing functionality created a gap between py-libp2p and other libp2p implementations (Go, Rust) that already support rendezvous-based peer discovery through namespaces.
How was it fixed?
1. Core Module Implementation
libp2p/discovery/rendezvous/
directory with complete protocol implementation:service.py
: Rendezvous service handling peer registrations and discoveryclient.py
: Low-level protocol client with trio structured concurrencydiscovery.py
: High-level discovery interface with caching and auto-refreshconfig.py
: Configuration constants and defaultserrors.py
: Protocol-specific exception classesmessages.py
: Protocol buffer message utilitiespb/rendezvous_pb2.py
: Protocol buffer definitions2. Key Features
3. Protocol Compliance
4. Examples and Integration
examples/rendezvous/
--mode server/client/discover
)5. Documentation and Testing
docs/examples.rendezvous.rst
: Complete user guide with tutorials and real log outputsdocs/libp2p.discovery.rendezvous.rst
: Full API documentation with cross-referencesBenefits
To-Do