-
Notifications
You must be signed in to change notification settings - Fork 190
eg/870-add-circuit-relay-dcutr-autonat-demonstration #975
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
base: main
Are you sure you want to change the base?
eg/870-add-circuit-relay-dcutr-autonat-demonstration #975
Conversation
|
@Winter-Soren : Appreciate your great efforts, Soham. Re-ran the CI/CD pipeline. There are a couple of issues, which need resolution. We will need a proper screencast demonstrating the example along with proper documentation to enable hole punching interop testing as the next step. CCing @acul71, who could help you on the hole punching interop testing front using the example as a starting point. |
|
@Winter-Soren : Great progress, Soham. Appreciate your support and efforts. Wish if you could resolve the CI/CD issues. Looking forward to seeing this PR landed before the upcoming release. |
- Fix AutoNATService type mismatch by casting IHost to BasicHost - Add type: ignore comments for raw_conn attribute access (implementation-specific) - Fix stream handler type mismatch in listener.py with proper casting - Fix all line length violations (E501) by breaking long lines - Remove unused variable listener_peer_info - Fix docstring format (D301) by using raw string for backslashes - Fix DCUtR test failures by restoring cache trust in _have_direct_connection - Add examples.nat to documentation toctree to fix build warning All linting, type checking, tests, and documentation builds now pass.
|
Hello @Winter-Soren and @seetadev I've fixed all the linting, type checking, test failures, and documentation issues identified in the review. Here's a summary of the changes: Fixed Issues1. Type Checking ErrorsAutoNATService type mismatch: # examples/nat/dialer.py and listener.py
from typing import cast
from libp2p.host.basic_host import BasicHost
# new_host() returns IHost but always returns BasicHost or RoutedHost
autonat_service = AutoNATService(cast(BasicHost, host))Missing raw_conn attribute: # Added type: ignore comments for implementation-specific attributes
conn = stream.muxed_conn.raw_conn # type: ignore[attr-defined]Stream handler type mismatch: # examples/nat/listener.py
from libp2p.network.stream.net_stream import NetStream
# Wrap bound method to match expected function signature
host.set_stream_handler(
AUTONAT_PROTOCOL_ID,
lambda stream: autonat_service.handle_stream(cast(NetStream, stream)),
)2. Linting Errors
3. Test FailuresDCUtR test failures fixed: Fix in async def _have_direct_connection(self, peer_id: ID) -> bool:
"""
Check if we already have a direct connection to a peer.
"""
# Check our direct connections cache first
# Trust the cache for fast path - this allows early return when we know
# we have a direct connection. Verification happens when connections are
# established (in _dial_peer) to ensure cache accuracy.
if peer_id in self._direct_connections:
return True
# Check if the peer is connected and verify it's direct
return await self._verify_direct_connection(peer_id)This restores the original behavior where the cache is trusted for the fast path, while verification still happens when connections are established to ensure cache accuracy. 4. DocumentationAdded Test Results✅ All tests passing: 1806 passed, 13 skipped QuestionThe two previously failing DCUtR tests ( Question: Is this the expected behavior? The verification logic in |
|
@Winter-Soren , @acul71 : Great, this PR is indeed ready for review + merge. Will do a final review before merging. |
pacrob
left a comment
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, just please add a newsfragment
What was wrong?
Demonstrating an example of NAT traversal using Circuit Relay v2, DCUtR hole punching, and AutoNAT was missing from the codebase.
Issue #870
How was it fixed?
Implemented a comprehensive NAT traversal example that demonstrates the complete flow:
The implementation includes three components:
relay.py: Publicly accessible relay node with resource managementlistener.py: NAT'd peer that accepts connections via relay and supports direct upgradesdialer.py: NAT'd peer that connects through relay and attempts hole punchinggraph TD subgraph "Dialer (Behind NAT)" A[Dialer Node] A1[AutoNAT] A2[Circuit Relay CLIENT Role] A3[DCUtR Protocol] end subgraph "Public Relay" B[Relay Node] B1[Circuit Relay HOP Role] B2[Resource Management] end subgraph "Listener (Behind NAT)" C[Listener Node] C1[AutoNAT] C2[Circuit Relay STOP Role] C3[DCUtR Protocol] end A -->|"1. Connect via relay"| B B -->|"2. Relay connection"| C A -->|"3. DCUtR hole punching"| C %% Component connections A --- A1 A --- A2 A --- A3 B --- B1 B --- B2 C --- C1 C --- C2 C --- C3 class A,C natNode class B publicNodeTo-Do