Skip to content

Refactor/quic: Add capability check for transport #1214

Open
ChayanDass wants to merge 3 commits intolibp2p:mainfrom
ChayanDass:refactor/quic
Open

Refactor/quic: Add capability check for transport #1214
ChayanDass wants to merge 3 commits intolibp2p:mainfrom
ChayanDass:refactor/quic

Conversation

@ChayanDass
Copy link
Copy Markdown
Contributor

@ChayanDass ChayanDass commented Feb 13, 2026

What was wrong?

  • Issue: #395 (Restructuring py-libp2p: modularity, interfaces, and developer experience); discussion in #1204.
  • The core (swarm, basic_host) depended on concrete QUIC types (isinstance(QUICTransport), isinstance(QUICConnection)), which:
    • Blocked other “secure+muxed” or “muxed-only” transports (e.g. WebRTC, Iroh) from plugging in without special-casing.
    • Didn’t support the desired model of separate capabilities (e.g. “secure only”, “muxed only”, or both).

How was it fixed?

  • Capability-based checks: Introduced libp2p/transport/capabilities.py with:
    • Transport: provides_secure_connection, provides_muxed_connection (and helper transport_provides_secure_muxed).
    • MuxedConn: muxed_conn_has_resource_scope, muxed_conn_has_establishment_wait, muxed_conn_has_negotiation_semaphore, muxed_conn_get_establishment_waiter, muxed_conn_is_established.
  • QUIC now declares both flags (provides_secure_connection=True, provides_muxed_connection=True) instead of being detected by type.
  • Swarm uses these helpers to decide when to skip security and/or muxer upgrade:
    • Muxed from dial (secure or not) → use as-is.
    • Secure but not muxed from dial → muxer-only upgrade.
    • Neither → full security then muxer (unchanged).
  • BasicHost uses capability helpers (e.g. muxed_conn_has_negotiation_semaphore) instead of isinstance(..., QUICConnection).
  • Tests: Added tests/core/transport/test_capabilities.py (edge cases for all capability helpers) and swarm tests that assert TCP has no capability flags and QUIC has both.

To-Do

  • Clean up commit history
  • Add or update documentation related to these changes
  • Add entry to the release notes

Cute Animal Picture

Cute animal

@ChayanDass ChayanDass changed the title Refactor/quic Refactor/quic: Add capability check for transport Feb 13, 2026
@ChayanDass
Copy link
Copy Markdown
Contributor Author

ChayanDass commented Feb 13, 2026

@seetadev , @acul71 , @sumanjeet0012 .. could you give it a review?
Thank you!

@acul71
Copy link
Copy Markdown
Contributor

acul71 commented Feb 17, 2026

@seetadev , @acul71 , @sumanjeet0012 .. could you give it a review? Thank you!

Quick auto review here:

AI Pull Request Review: #1214 — Refactor/quic: Add capability check for transport

PR: libp2p/py-libp2p#1214
Branch: refactor/quic → main
Author: ChayanDass

1. Summary of Changes

This PR refactors the core (swarm, basic_host) to use capability-based checks instead of concrete QUIC types (isinstance(QUICTransport), isinstance(QUICConnection)). It addresses issue #395 (Restructuring py-libp2p: modularity, interfaces, and developer experience) and aligns with discussion #1204, which advocates for capability-based protocol requirements and decoupling the core from specific transport implementations.

What changed:

  • New module: libp2p/transport/capabilities.py — helpers for transport capabilities (provides_secure_connection, provides_muxed_connection, transport_provides_secure_muxed) and muxed-connection capabilities (muxed_conn_has_resource_scope, muxed_conn_has_establishment_wait, muxed_conn_has_negotiation_semaphore, muxed_conn_get_establishment_waiter, muxed_conn_is_established).
  • libp2p/abc.py: Docstring update on ITransport describing optional capability attributes and reference to libp2p.transport.capabilities.
  • libp2p/host/basic_host.py: Replaced QUICConnection import and _is_quic_muxer() with muxed_conn_has_negotiation_semaphore; registry stats branch now uses hasattr(muxed_conn, "_transport") and non-None check instead of connection_type == "QUICConnection"; comments generalized (no QUIC-specific wording).
  • libp2p/network/swarm.py: Replaced all isinstance(..., QUICTransport) / isinstance(..., QUICConnection) with capability helpers; added _upgrade_outbound_secure_conn() for the “secure but not muxed” path; dial/listen/add_conn/new_stream use transport_provides_* and muxed-conn helpers.
  • libp2p/transport/quic/transport.py: QUIC transport now sets class attributes provides_secure_connection = True and provides_muxed_connection = True.
  • Tests: New tests/core/transport/test_capabilities.py (edge cases for all capability helpers); new swarm tests in tests/core/network/test_swarm.py asserting TCP has no capability flags and QUIC has both.

Issues addressed: #395 (modularity/interfaces). Discussion #1204 is referenced for design context.

Modules affected: libp2p/abc.py, libp2p/host/basic_host.py, libp2p/network/swarm.py, libp2p/transport/capabilities.py (new), libp2p/transport/quic/transport.py, and the above test files. No breaking API changes; behavior for existing TCP/QUIC usage is preserved.

2. Branch Sync Status and Merge Conflicts

Branch Sync Status

  • Status: ℹ️ Ahead of origin/main
  • Details: Branch is 0 commits behind, 4 commits ahead of origin/main. No rebase required for sync; the 4 commits are the PR’s own changes.

Merge Conflict Analysis

  • Conflicts Detected: ✅ No conflicts
  • Details: Test merge with origin/main completed successfully (“Already up to date”). The PR branch can be merged cleanly into origin/main.

3. Strengths

  • Clear alignment with Restructuring py-libp2p to be more modular and developer-friendly #395 and [Issue #395] Restructuring py-libp2p: modularity, interfaces, and developer experience #1204: Moves from type-based branching to capability-based checks, enabling future “secure-only” or “muxed-only” transports (e.g. WebRTC, Iroh) without further special-casing in the core.
  • Focused capability module: capabilities.py has a single responsibility, good module docstring, and consistent helper naming. Use of getattr(..., False) and bool() avoids reliance on strict types and handles missing attributes safely.
  • Swarm upgrade logic: The three paths (muxed from dial → use as-is; secure but not muxed → muxer only; neither → full upgrade) are explicit and easier to extend.
  • Test coverage: test_capabilities.py covers missing attributes, explicit True/False, truthy/falsy values, and real TCP/QUIC instances for all helpers. Swarm tests lock in that TCP has no flags and QUIC has both.
  • BasicHost decoupling: Removal of QUICConnection import and use of muxed_conn_has_negotiation_semaphore keeps the host independent of QUIC.
  • Logging: QUIC-specific log message replaced with a generic “successfully opened muxed connection to peer %s”, and debug logs use capability helpers where relevant.

4. Issues Found

Critical

None.

Major

Minor

  • File: libp2p/transport/capabilities.py

  • Line(s): 21

  • Issue: Orphan comment # can skip security and/or muxer upgrade when not needed. sits between imports and the constants; it duplicates the idea already in the module docstring.

  • Suggestion: Remove the comment or attach it to the constants (e.g. “# Attribute names for transports that can skip …”) if you want to keep a short inline note.

  • File: libp2p/network/swarm.py

  • Line(s): ~499–533 (_upgrade_outbound_secure_conn)

  • Issue: The block that opens a resource-manager scope and sets _resource_scope on muxed_conn mirrors logic in upgrade_outbound_raw_conn; there is some duplication and broad except Exception: pass.

  • Suggestion: Consider extracting shared “open scope and attach to muxed_conn” logic into a small helper to reduce duplication and make exception handling explicit (e.g. log or re-raise where appropriate).

5. Security Review

  • No new external input surfaces; capability flags are class attributes on transports and attribute checks on connection objects.
  • No change to cryptographic usage or key handling.
  • No new subprocess/temp file usage.
  • Logging changes are neutral (less QUIC-specific, no additional sensitive data).

Conclusion: No security concerns identified; Security impact: None.

6. Documentation and Examples

  • libp2p/abc.py: ITransport docstring updated to describe optional capability attributes and point to libp2p.transport.capabilities — good.
  • libp2p/transport/capabilities.py: Module and function docstrings are clear and describe when the swarm skips upgrades.
  • To-Do in PR: Author lists “Add or update documentation related to these changes”. No user-facing docs or examples were added in this PR. For a refactor that doesn’t change user API, current docstrings may be enough; a short note in release notes (via newsfragment) would complete the picture once the fragment is added.

7. Newsfragment Requirement

⚠️ BLOCKER

8. Tests and Validation

Linting (make lint)

  • Result: Passed (exit code 0).
  • Output: All hooks passed (check yaml, check toml, fix end of files, trim trailing whitespace, pyupgrade, ruff, ruff format, mdformat, mypy, pyrefly, RST check). No errors or warnings to report.

Type Checking (make typecheck)

  • Result: Passed (exit code 0).
  • Output: mypy and pyrefly typecheck passed. No type errors or warnings to report.

Test Execution (make test)

  • Result: Passed (exit code 0).
  • Summary: 2204 passed, 16 skipped, 0 failed, 0 errored (≈91.7 s).
  • Relevant tests: New capability tests and new swarm capability tests are included; no failures or skips related to this PR. No additional warnings or failures documented.

Documentation Build (make linux-docs)

9. Recommendations for Improvement

  1. Add the required newsfragment 395.internal.rst (or 395.feature.rst) as in Section 7.
  2. Remove or clarify the orphan comment in libp2p/transport/capabilities.py (line 21).
  3. Optional: Extract shared resource-scope handling between _upgrade_outbound_secure_conn and upgrade_outbound_raw_conn to reduce duplication and clarify exception handling.
  4. Optional: In the PR’s To-Do, “Clean up commit history” and “Add or update documentation” can be ticked after adding the newsfragment and any small doc updates you choose.

10. Questions for the Author

  1. Do you prefer 395.internal.rst or 395.feature.rst for the newsfragment, given that this is a refactor enabling future transports rather than a user-facing feature?
  2. For the “secure but not muxed” path, are there any current or planned transports that will use it (e.g. a TLS-only transport returning ISecureConn), or is this primarily for future use?
  3. Would you consider adding a one-line note in the release notes or a short “Transport capabilities” subsection in the docs to explain how transport authors can set provides_secure_connection / provides_muxed_connection?

11. Overall Assessment

@acul71
Copy link
Copy Markdown
Contributor

acul71 commented Feb 24, 2026

@ChayanDass What's the status of this PR? Do you need assistance ?

@ChayanDass
Copy link
Copy Markdown
Contributor Author

ChayanDass commented Feb 24, 2026

Sorry for the delay! My exams are going on , so i will fix those by Sunday 😞

@ChayanDass
feat(transport): capability-based dispatch for QUIC; drop example, ad…
4b2dcc5 
…d transport tests

- Add libp2p.capabilities with transport_provides_secure_and_muxed,
  connection_provides_muxed, connection_needs_establishment_wait
- QUIC transport/connection set provides_secure=True, provides_muxed=True
- Swarm and BasicHost use capability checks instead of isinstance(QUICTransport/
  QUICConnection); INFO logs when upgrade is skipped
- QUIC listener: fix getsockname() for IPv6 (use sockname[0:2])
- Add tests/core/transport/test_capabilities.py (TCP + helpers)
- Add test_quic_transport_capability_flags in quic/test_transport.py
- Newsfragment 395.feature.rst

Signed-off-by: Chayan Das <01chayandas@gmail.com>
@ChayanDass
Copy link
Copy Markdown
Contributor Author

ChayanDass commented Feb 28, 2026

hey i updated the code...
And I have added some more tests for capabilities .... PTAL

@yashksaini-coder
Copy link
Copy Markdown
Contributor

yashksaini-coder commented Mar 1, 2026

@ChayanDass Reviewed the PR, the failing docs ci check, was caused by ITransport in the docutils. It has been resolved, and now ready for the final review and merge, @acul71

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

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ChayanDass @acul71 @yashksaini-coder