-
Notifications
You must be signed in to change notification settings - Fork 30
Add ipcidr protocol in reference with go-multiaddr #95
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
- Added the codec utility functions in codecs/ipcidr.py - Added the general codec internal tests in test_protocols.py
Now will write the ipcidr + multiaddr tests as written in go implementation. |
@lla-dane : Great, thanks Abhinav. Lets try and getting key protocols in the upcoming multiaddr release. We plan for another release 4 weeks from now. |
- Included the ipcidr addresses in the test-cases - Now all the test cases are running for ipcidr addrs also
"/ip4/192.0.2.0/ipcidr/24", | ||
"/ip6/::1", | ||
"/ip6/2601:9:4f81:9700:803e:ca65:66e8:c21", | ||
"/ip6/2001:db8::/ipcidr/32", |
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.
- Added /ipcidr/24 (missing IP address) to valid tests - Added /ip4/1.2.3.0/ipcidr/128 (invalid IPv4 CIDR) to valid tests - Added /ip6/::/ipcidr/255 (invalid IPv6 CIDR) to valid tests - These cases are valid in multiaddr format (correct behavior) - Added news fragment for ipcidr feature documentation - All tests now pass (265 passed, 1 skipped)
- Removed newsfragments/95-feature.ipcidr.md - Keep only the test case additions from Go implementation
- Removed /ipcidr/24, /ip4/1.2.3.0/ipcidr/128, /ip6/::/ipcidr/255 test cases - These cases are valid multiaddr format but don't serve the same purpose as Go tests - Go tests validate MultiaddrToIPNet failures, Python doesn't have equivalent function - All tests still pass (262 passed, 1 skipped)
Thanks, @lla-dane , just add a newfragment doc and I'll merge this PR PR #95 Review: Add ipcidr protocol support to py-multiaddrSummary✅ APPROVED WITH CHANGES - Implementation complete and working correctly. Implementation Status
Test Results
Required ChangesAdd news fragment file: Final Status✅ APPROVED WITH CHANGES - Add documentation file before merging. |
@acul71: Added the newsfragment file |
@seetadev I think we should merge this now rather than waiting for tests. Since the This approach would allow us to:
The protocol addition looks solid based on the Go implementation reference, and having it available will make the py-libp2p testing much more effective. We can always iterate on any issues that come up during integration. What do you think about merging this first and then doing the testing in the context of py-libp2p? Use Cases for ipcidr Protocol in py-libp2p1. Eclipse Attack PreventionAllow trusted peers to connect even when under attack by specifying trusted IP ranges. 2. Network Access ControlRestrict connections to specific IP ranges for security purposes. 3. Resource ManagementSet connection limits per network prefix to prevent resource exhaustion. 4. Peer AuthenticationCombine IP ranges with specific peer IDs for enhanced security. Benefits of Early Integration
Related DiscussionFor more details on ipcidr protocol usage across libp2p implementations, see: ipcidr Protocol Usage in libp2p Discussion #976 |
@acul71 : +1. We will merge it and try the addition in multiple PRs at this juncture. Appreciate your feedback and pointers. @lla-dane : Thank you Abhinav. Will discuss with you the remaining protocols and their additions this week. We wish to reach parity with go-libp2p in this month (if possible). |
Tracks: multiformats/multiaddr#181
Adds the ipcidr protocol in py-multiaddr in reference with go-implementation
@seetadev @acul71