Replies: 1 comment
-
This is a great breakdown, @acul71, thanks! I agree with your recommendation. |
Beta Was this translation helpful? Give feedback.
0 replies
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
-
TODO Analysis: BasicHost.get_addrs() p2p Postfix Issue
Issue Location
File:
libp2p/host/basic_host.py:140
Method:
get_addrs()
TODO Comment:
# TODO: We don't need "/p2p/{peer_id}" postfix actually.
Current Status Analysis
Current Implementation
The
get_addrs()
method inBasicHost
currently:self.get_id()
f"/p2p/{self.get_id()!s}"
self._network.listeners.values()
transport.get_addrs()
addr.encapsulate(p2p_part)
What the TODO Suggests
The TODO suggests that the
/p2p/{peer_id}
postfix should not be added to the addresses returned byget_addrs()
.What Must Be Done to Fix This Issue
1. Remove the p2p encapsulation
The fix would be to simply return the raw transport addresses without adding the
/p2p/{peer_id}
postfix:2. Impact Analysis Required
a) Interface Compliance
IHost
interface inabc.py
definesget_addrs()
to returnlist[Multiaddr]
PeerData.get_addrs()
return raw addresses without p2p postfixb) Usage Patterns
identify
protocol useshost.get_addrs()
to share listening addressestests/utils/interop/utils.py:32
)c) Transport Layer Behavior
TCPListener
) return raw addresses like/ip4/127.0.0.1/tcp/8000
3. Recommended Approach
Option A: Remove p2p postfix (as TODO suggests)
Pros:
Cons:
Option B: Keep current behavior but add a new method
Pros:
Cons:
Implementation:
Option C: Make it configurable
Pros:
Cons:
Implementation:
4. Required Changes if Implementing Option A
get_addrs()
get_p2p_addrs()
if p2p addresses are still needed5. Testing Strategy
You would need to:
6. Files That Would Need Updates
Core Implementation
libp2p/host/basic_host.py
- Main implementationTests (if breaking change)
tests/core/host/test_*.py
- Host-related teststests/core/identity/identify/test_*.py
- Identify protocol teststests/core/examples/test_examples.py
- Example teststests/utils/interop/utils.py
- Interop utilitiesExamples
examples/*/
- All example files that useget_addrs()
Documentation
docs/
- Update any documentation referencing the behaviorConclusion
The TODO suggests this is a known issue that needs addressing, but it requires careful consideration of the impact on the entire codebase and ecosystem. The choice between the three options depends on:
Recommendation: Start with Option B (add new method) to maintain backward compatibility while providing the cleaner API, then consider deprecating the old behavior in a future major version.
Beta Was this translation helpful? Give feedback.
All reactions