-
Notifications
You must be signed in to change notification settings - Fork 182
feat/606-enable-nat-traversal-via-hole-punching #668
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
feat/606-enable-nat-traversal-via-hole-punching #668
Conversation
@Winter-Soren : Hi Soham. The PR is coming along well. Wish to share that I re-ran the CI/CD pipeline and found some build issues. I am adding a discussion page to help you resolve them soon: please visit #714 Let me know if you want a patch or help digging into the interface of |
great work @Winter-Soren , seems a logical implementations. ⚡🚀 |
….com/Winter-Soren/py-libp2p into feat/606-nat-traversal-hole-punching
@Winter-Soren : Thank you for considering my feedback and resolving the CI/CD issues. Appreciate it. Reviewing the code commits and improvements as discussed. Please continue the module integration with circuit relay and overall NAT traversal stack. |
@Winter-Soren : Huge thanks for your excellent work on implementing the hole punching module in py-libp2p! 🎉 This is a major step forward for py-libp2p’s connectivity capabilities and support for modern NAT traversal techniques. We'll be doing a deeper review and testing soon, especially around edge cases where hole punching may fail due to asymmetric NATs or race conditions. @pacrob: Hi Paul. This is indeed an important module addition and will be needed by webrtc and universal connectivity dapp PRs, right away. Wish to have your feedback and pointers. @Winter-Soren : Wish if you could add a discussion page with notes on: Protocol compliance with the spec Error handling and retry logic How this could be extended or generalized in the future (e.g., interop testing, quic integration, etc.) Kindly also include details on test case coverage with focus on circuit relay fallback when hole punching fails (please include tests with VPN and without VPN connection as well). Wish to also share that hole punching does not work with mobile hotspots. Kindly check if we can try testing this module on a linux and a windows device with both wired and wireless connection. CCing @guha-rahul , @lla-dane , @sukhman-sukh, @Nkovaturient, @acul71 and @sumanjeet0012 for their feedback and pointers too. This puts us in a much stronger place to support real-world use cases with relays and NAT traversal in Python. @Winter-Soren : Thanks again for the hard work and for pushing this forward. Looking forward to everyone’s feedback. |
Thank you so much, @seetadev!! |
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.
I don't see that any of the code in this file is actually being hit during testing. My method was to place breakpoints at various places throughout each method, and all tests passed without hitting them. Please let me know if I'm missing something.
…h.Type.CONNECT and HolePunch.Type.SYNC
….com/Winter-Soren/py-libp2p into feat/606-nat-traversal-hole-punching
@Winter-Soren : Wish if you could reply to each of the feedback points shared by @pacrob with appropriate code commit references. We will do a final review once you have responded to each of the points individually. We are pretty close to final review + merge. Appreciate your efforts. |
….com/Winter-Soren/py-libp2p into feat/606-nat-traversal-hole-punching
@Winter-Soren : Hi Soham. The PR is coming along well. Wish to share that I re-ran the CI/CD pipeline and found some build issues. Please resolve them. |
@Winter-Soren : Hi Soham. The PR is coming along well. Wish to share that I re-ran the CI/CD pipeline and found some build issues. Please resolve them. |
…xed method signature inconsistencies
Looking good, @Winter-Soren! Just one question on a test, otherwise good to merge. |
@Winter-Soren , @pacrob : This is a wonderful milestone and achievement. Great efforts and initiative. Thank you so much @pacrob for sharing key improvement points and for your detailed feedback. Appreciate your support. @Winter-Soren : Wish if you could also open a discussion page and share developer documentation, screencasts or demo for enabling our hole punching interop developers to build test plans for interoperability with other libp2p modules. CCing @acul71, who is leading the interop efforts and will coordinate with you on hole punching interop test plans. This will also support our lead webrtc developers (@Nkovaturient and @sukhman-sukh ) integrate hole punching module and circuit relay fall back mechanism in webrtc-direct and webrtc private to private PRs. They have made great progress and the PR is getting close towards completion as shared in the meeting. Also, wish if you could also add a note on test suite developed for unit and integration testing of hole punching module. This will enable us to scale NAT traversal efforts as we build P2P decentralized AI use-cases in the coming month. Thank you once again and great work indeed. |
What was wrong?
Issue #606 requested the implementation of Direct Connection Upgrade through Relay (DCUtR) protocol in py-libp2p. This protocol enables peers that initially connect through a relay to establish direct connections when possible, improving performance and reducing relay load.
Issue #606
How was it fixed?
I implemented the DCUtR protocol according to the libp2p spec The implementation includes:
DCUtRProtocol
class that handles the protocol logic for hole punchingThe implementation allows peers to exchange observed addresses and synchronize connection attempts to punch through NATs. When successful, peers can upgrade from relayed connections to direct connections, improving performance and reducing load on relay nodes.
To-Do
Cute Animal Picture