-
Notifications
You must be signed in to change notification settings - Fork 175
✨ Feat: add Thin Waist address validation utilities and integrate into echo example #811
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: add Thin Waist address validation utilities and integrate into echo example #811
Conversation
…tilities - Added `network_discover.py` to demonstrate Thin Waist address handling. - Introduced `address_validation.py` with functions for discovering available network interfaces, expanding wildcard addresses, and determining optimal binding addresses. - Included fallback mechanisms for environments lacking Thin Waist support.
- Replaced hardcoded listen address with `get_optimal_binding_address` for improved flexibility. - Imported address validation utilities in `echo.py` and updated `__init__.py` to include new functions.
- Introduced `test_echo_thin_waist.py` to validate the echo example's output for Thin Waist lines. - Added `test_address_validation.py` to cover functions for available interfaces, optimal binding addresses, and wildcard address expansion. - Included parameterized tests and environment checks for IPv6 support.
Hi @yashksaini-coder, ping me if you need help. |
Right now, I've created a draft PR to introduce the multiadd thin waist address. with integration to examples and tests, could you give me feedback on this approach. I've only done Phase 1 + 2 (partially) based on the issue #804 |
Can you modify
(venv) luca@r17:~/PNL_Launchpad_Curriculum/Libp2p/tmp/yashksaini-coder/py-libp2p$ python examples/echo/echo.py
I am 16Uiu2HAmRxqPcNXm9KAUAVKyQkpXF8c8r7S6jdpGK6K963w9Puwk
Run this from the same folder in another console:
echo-demo -d /ip4/192.168.136.148/tcp/33387/p2p/16Uiu2HAmRxqPcNXm9KAUAVKyQkpXF8c8r7S6jdpGK6K963w9Puwk
Waiting for incoming connections...
luca@r17:~/PNL_Launchpad_Curriculum/Libp2p/js-libp2p-examples/examples/js-libp2p-example-chat$ node src/listener.js
Listener ready, listening on:
/ip4/127.0.0.1/tcp/10333/p2p/12D3KooWCS6QSMEbtt77pWeB618W824aGVxhLhPEPaCUMGMvBhuk
/ip4/192.168.136.148/tcp/10333/p2p/12D3KooWCS6QSMEbtt77pWeB618W824aGVxhLhPEPaCUMGMvBhuk
/ip4/10.149.6.41/tcp/10333/p2p/12D3KooWCS6QSMEbtt77pWeB618W824aGVxhLhPEPaCUMGMvBhuk
echo.py
I am 16Uiu2HAmRxqPcNXm9KAUAVKyQkpXF8c8r7S6jdpGK6K963w9Puwk
Listener ready, listening on:
/ip4/127.0.0.1/tcp/10333/p2p/16Uiu2HAmRxqPcNXm9KAUAVKyQkpXF8c8r7S6jdpGK6K963w9Puwk
/ip4/192.168.136.148/tcp/10333/p2p/16Uiu2HAmRxqPcNXm9KAUAVKyQkpXF8c8r7S6jdpGK6K963w9Puwk
/ip4/10.149.6.41/tcp/10333/p2p/16Uiu2HAmRxqPcNXm9KAUAVKyQkpXF8c8r7S6jdpGK6K963w9Puwk
Run this from the same folder in another console:
echo-demo -d /ip4/192.168.136.148/tcp/33387/p2p/16Uiu2HAmRxqPcNXm9KAUAVKyQkpXF8c8r7S6jdpGK6K963w9Puwk
Waiting for incoming connections...
|
Got it exposing all available interfaces. @acul71 |
@yashksaini-coder Summary: yashksaini-coder's Thin Waist Address Validation PR🎯 Quick AssessmentOverall: Excellent foundation, needs minor refinements for JavaScript parity. Status: Phase 1 ✅ Complete, Phase 2 🔄 Partially Complete ✅ What's Working Well
🔧 Key Issues to Fix1. JavaScript Parity (High Priority)Problem: Echo shows only 1 address instead of multiple interfaces like JS libp2p. Current:
Target (like JS):
Fix: Use 2. Missing Loopback (High Priority)Problem: No 127.0.0.1 in output. Fix: Add loopback to addrs.append(Multiaddr(f"/ip4/127.0.0.1/{protocol}/{port}")) 3. Command Display (Medium Priority)Problem: Fix: Use first expanded address instead of 🚀 Quick Wins
📋 Action ItemsHigh Priority
Medium Priority
🏆 RecommendationApprove with minor revisions. The core implementation is solid - just needs these small tweaks to match JavaScript libp2p behavior. 🧪 Test Status✅ Unit Tests - All Passing
❌ Integration Test - FailingProblem: Test expects:
Current echo.py outputs:
Fix: Update integration test to match actual echo.py output format. 💡 Pro Tips
Bottom Line: Great work! Just need to show all interfaces like JavaScript libp2p does. 🚀 |
@acul71 got it this clears a lot, I will focus on the javascript libp2p interface showcase. |
@yashksaini-coder , @acul71 : Thank you for submitting the PR. Appreciate it. Wish if CI/CD issues could be resolved. |
@yashksaini-coder @seetadev Thin Waist Feature FixesSummaryFixed linting, type checking, and test issues in the Thin Waist address validation feature. Repository & Branch
Fixes Applied1. Linting Issues (E501 - Line too long)
2. Type Checking Issues
3. Test Improvements
Instructions for yashksaini-coderOption 1: Cherry-pick from forkgit remote add acul71 https://github.com/acul71/py-libp2p-fork.git
git fetch acul71
git cherry-pick acul71/feat/804-add-thin-waist-address Option 2: Pull specific commitsgit remote add acul71 https://github.com/acul71/py-libp2p-fork.git
git fetch acul71
git merge acul71/feat/804-add-thin-waist-address VerificationAll tests pass:
Files Modified
|
Yes thanks for the engangement, I have done the previous requests, of modifying the |
… into feat/804-add-thin-waist-address
@seetadev can you please approve the workflow runs, I picked some commit changes of linting issues, and merge them in this PR, needs to check the workflow |
…e address printing
https://github.com/yashksaini-coder/py-libp2p into feat/804-add-thin-waist-address
Hi @yashksaini-coder I can see that you have a lint error just split the long comment in more lines ruff (legacy alias)......................................................Failed
- hook id: ruff
- exit code: 1
tests/examples/test_echo_thin_waist.py:14:89: E501 Line too long (93 > 88)
|
13 | # This test is intentionally lightweight and can be marked as 'integration'.
14 | # It ensures the echo example runs and prints the new Thin Waist lines using Trio primitives.
| ^^^^^ E501
15 |
16 | current_file = Path(__file__)
|
Found 1 error. runned echo-demo, (venv) luca@r17:~/PNL_Launchpad_Curriculum/Libp2p/py-libp2p$ echo-demo
I am 16Uiu2HAmSt1ybASKw5UJzmGTbjjz9MiRNsYVVdxsEbxTfhBiY8Vp
Listener ready, listening on:
/ip4/192.168.1.75/tcp/0/p2p/16Uiu2HAmSt1ybASKw5UJzmGTbjjz9MiRNsYVVdxsEbxTfhBiY8Vp
Run this from the same folder in another console:
echo-demo -d /ip4/192.168.1.75/tcp/44969/p2p/16Uiu2HAmSt1ybASKw5UJzmGTbjjz9MiRNsYVVdxsEbxTfhBiY8Vp
Waiting for incoming connections...
|
While the Thin Waist branch has made significant progress in IPv6 support, approximately 40 files still contain hardcoded IPv4 references that limit full IPv6 functionality. The most critical issues have been addressed in the transport layer, but additional work is needed in:
|
@yashksaini-coder @sumanjeet0012 |
Hi @yashksaini-coder, you should add a doc newsfragment file to complete this PR. File to create:
|
…e news fragment formatting
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.
If you can refactor then for me is ready to be merged
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.
Well done
@seetadev @pacrob PR #811 Review SummaryStatus: ✅ APPROVEDPR: #811 - Feat: add Thin Waist address validation utilities and integrate into echo example 🎯 Key Achievements
🔧 Core ChangesNew Module:
|
aaffe2c
to
4195ee1
Compare
4195ee1
to
6a0a7c2
Compare
@acul71 : Great to hear. I just had a long discussion with @yashksaini-coder. Wonderful progress and great work. Special thanks to you and @sumanjeet0012 for the great support and help. Will wait for a thumbs up from @pacrob and will then merge today. |
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.
Just one comment that can be removed, otherwise looks good! Thanks, @yashksaini-coder!
|
@pacrob : Thank you so much for your feedback. Appreciate it. @yashksaini-coder : Re-running CI/CD pipeline and doing a final review. Will merge the PR soon. Kindly share a detailed blog on how you did this PR. Since this was your first PR in py-libp2p, which is getting merged, we would like you to share your experiences from a technical perspective in detail. This would also help the new contributors to take up projects in py-libp2p and arrive at a good conclusion on them. Keep up the good work :) |
What was wrong?
There was no integration for thin waist address validation.
Issue #804
How was it fixed?
Implements Phase 1 & partial Phase 2 of issue #804 by introducing Thin Waist Address Validation utilities and updating the
echo
example to use them, Using the multiaddr.libp2p/utils/address_validation.py
get_available_interfaces
get_optimal_binding_address
expand_wildcard_address
multiaddr.utils
libp2p/utils/__init__.py
examples/echo/echo.py
:/ip4/0.0.0.0/tcp/{port}
withget_optimal_binding_address
examples/advanced/network_discovery.py
tests/utils/test_address_validation.py
tests/examples/test_echo_thin_waist.py
(light integration)Why
Backward Compatibility
multiaddr.utils.get_thin_waist_addresses
/get_network_addrs
are unavailable, utilities fallback to conservative defaults.To-Do