Skip to content

Tracking: Transport, Relay, and Discovery Enhancements #996

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open

Tracking: Transport, Relay, and Discovery Enhancements #996

wants to merge 16 commits into from

Conversation

@codemaestro64
Copy link
Contributor

@codemaestro64 codemaestro64 commented Oct 16, 2025 •
edited
Loading

What was wrong?

Issue #691

The libp2p stack lacked robust relay support, secure resource validation, and dynamic peer discovery, impacting reliability in NATed or firewalled environments.

Description

This PR enhances the py-libp2p networking layer with improvements to relay functionality and security, advancing the library toward production-grade reliability for decentralized applications.

TODOs

  • Implement voucher and signature verification in resources.py
  • Implement initial relay selection logic in transport.py
  • Implement DHT-based peer discovery
  • Implement more sophisticated relay selection
  • Implement reservation storage and refresh mechanism
  • Add relay multiaddrs to peerstore
  • Implement proper multiaddr parsing and handling for relayed connections
  • Implement run() method in CircuitV2Listener

closes #691

@codemaestro64 codemaestro64 force-pushed the trd-enhancements branch 2 times, most recently from 71ea513 to 271cdd6 Compare October 20, 2025 15:48
@codemaestro64 codemaestro64 marked this pull request as ready for review October 20, 2025 16:59
@seetadev
Copy link
Contributor

seetadev commented Oct 20, 2025

@codemaestro64 : Thank you so much for opening the PR for completing all the required enhancements and todos in circuit relay. Appreciate your efforts.

Updated the branch and re-run CI/CD pipeline. You will get immediate results on CI/CD test cases.

@seetadev
Copy link
Contributor

seetadev commented Oct 27, 2025

@codemaestro64 : Appreciate your efforts.

Wish to invite @acul71 , @lla-dane, @Winter-Soren for sharing feedback points on the PR.

@Winter-Soren
Copy link
Contributor

Winter-Soren commented Oct 30, 2025

Hey @codemaestro64,
I have reviewed your PR.
delivers production‑grade rigor: secure reservation validation, resilient relay discovery/selection, durable reservation management, and correct circuit multiaddr behavior. The protocol/service lifecycle is implemented cleanly with timeouts and explicit error signaling. Tests cover the core and many edge conditions. This is a well‑structured, merge‑ready upgrade that improves reliability and parity with other libp2p stacks.

CC'ing @seetadev @acul71

pacrob
pacrob reviewed Nov 2, 2025
View reviewed changes
libp2p/relay/circuit_v2/transport.py Outdated
# Prefer stored /p2p-circuit addrs from peerstore
# Try first to read addresses from peerstore
peer_store = self.host.get_peerstore()
stored_addrs = peer_store.addrs(peer_info.peer_id)
Copy link
Member

@pacrob pacrob Nov 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

peer_store.addrs will raise a PeerStoreError if the peer we're dialing isn't already in the peerstore, making dialing new peers impossible. Maybe wrap in a try/except or similar?

@codemaestro64
add doc
2b1ddba 
fix signed_envelope error

fix tests

fix lint issue
@codemaestro64
fix lint issues
968dfff
@seetadev
Copy link
Contributor

seetadev commented Nov 11, 2025

@codemaestro64 : Great work. Appreciate the progress.

Please reply to @pacrob so that he can review and mark the feedback as resolved.

There is one CI/CD failure, which can be ignored for now (not related to the PR):

=========================== short test summary info ============================
FAILED tests/core/kad_dht/test_kad_dht.py::test_put_and_get_value - Assertion...
============ 1 failed, 1551 passed, 5 warnings in 553.16s (0:09:13) ============
py311-core: exit 1 (554.53 seconds) /home/runner/work/py-libp2p/py-libp2p> pytest -n auto --timeout=1200 tests/core pid=2376
py311-core: FAIL code 1 (581.97=setup[27.44]+cmd[554.53] seconds)
evaluation failed :( (582.54 seconds)
Error: Process completed with exit code 1.

@seetadev
Copy link
Contributor

seetadev commented Nov 14, 2025

@codemaestro64 : Wonderful efforts indeed. Appreciate your contribution.

Wish if you could mark the todos at #691 completed in your PR. We are reviewing the PR in details, once again. It is indeed heading towards final review + merge. Great progress.

On the same note, wish if you could open up a discussion page sharing more details on the implementation steps and test suite.

CCing @pacrob, @Winter-Soren, who are reviewing this PR. Looking forward to their pointers and feedback.

pacrob
pacrob reviewed Nov 18, 2025
View reviewed changes
pyproject.toml Outdated
Comment on lines 288 to 290
[tool.ruff.per-file-ignores]
"tests/*.py" = ["F821"] # undefined name (e.g., pytest fixtures like mocker)

Copy link
Member

@pacrob pacrob Nov 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not getting an error with this. Is it necessary?

Copy link
Contributor Author

@codemaestro64 codemaestro64 Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No longer needed. Removed

pacrob
pacrob reviewed Nov 18, 2025
View reviewed changes
libp2p/relay/circuit_v2/transport.py Outdated
async def handle_incoming_connection(
self,
stream: INetStream,
remote_peer_id: ID,
Copy link
Member

@pacrob pacrob Nov 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This arg is never used in the function.

Copy link
Contributor Author

@codemaestro64 codemaestro64 Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My bad. It's been removed

pacrob
pacrob reviewed Nov 19, 2025
View reviewed changes
libp2p/relay/circuit_v2/protocol.py
Comment on lines 601 to 602
voucher=b"", # We don't use vouchers yet
signature=b"", # We don't use signatures yet
Copy link
Member

@pacrob pacrob Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should these be updated? It looks like we are checking against these values in RelayResourceManager->verify_reservation, but they are always empty strings here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pacrob pacrob pacrob left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Tracking: Transport, Relay, and Discovery Enhancements

4 participants

@codemaestro64 @seetadev @Winter-Soren @pacrob