Skip to content

Conversation

torrpriius
Copy link

@torrpriius torrpriius commented Aug 21, 2025

Make the Identify allowlist configurable instead of hard-coding protocol IDs.

  • Add identify::Metrics::new_with_allowed_protocols(...) to accept extra StreamProtocols.
  • Add libp2p_metrics::Metrics::new_with_identify_allowed_protocols(...) to forward user-provided protocols.
  • Defaults remain unchanged; apps can pass custom IDs when using ConfigBuilder::protocol_id.

Usage

use libp2p_metrics::{Metrics, Registry};
use libp2p_swarm::StreamProtocol;

let extra = [StreamProtocol::new("/myappsub/1.2.3")];

let mut registry = Registry::default();
let metrics = Metrics::new_with_identify_allowed_protocols(&mut registry, extra);

Notes & open questions

  • Happy to adjust naming or expose a builder if maintainers prefer a different API.

Change checklist

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • A changelog entry has been made in the appropriate crates

@torrpriius torrpriius changed the title Add Debug for OneShotHandler, Gossipsub allowlist, and timeout doc chore: Add Debug for OneShotHandler, Gossipsub allowlist, and timeout doc Aug 21, 2025
@elenaf9 elenaf9 added the kind/generated This issue might have been automatically generated label Aug 21, 2025
@torrpriius torrpriius changed the title chore: Add Debug for OneShotHandler, Gossipsub allowlist, and timeout doc chore: Implement OneShotHandler Debug, update metrics allowlist and docs Aug 21, 2025
Copy link

It seems this issue might have been automatically generated. To help us address it effectively, please provide additional details.

We value the use of LLMs for code generation and welcome your contributions but please ensure your submission is of such quality that a maintainer will spend less time reviewing it than implementing it themselves. Verify the code functions correctly and meets our standards. If your change requires tests, kindly include them and ensure they pass.

If no further information is provided, the issue will be automatically closed in 7 days. Thank you for your understanding and for aiding us in maintaining quality contributions!

@github-actions github-actions bot added the need/author-input Needs input from the original author label Aug 22, 2025
@torrpriius
Copy link
Author

@elenaf9

Thanks for the feedback!

  • I addressed the Identify allowlist note by making it configurable rather than hardcoding protocol IDs. This keeps defaults intact while allowing applications to opt-in their custom protocol IDs (e.g., when using ConfigBuilder::protocol_id).

  • To keep this PR focused and reduce surface area, I dropped the Debug implementation for OneShotHandler. If there is interest, I can follow up with a small change (or guard behind cfg(test)/a feature) to aid test-only usage.

What changed concretely

  • Identify metrics allowlist:
    • Kept a stable base set of protocols and added a simple API to pass additional protocol IDs.
    • No behavior changes by default. Applications that customize protocol IDs can now pass theirs.
  • OneShotHandler:
    • Removed the Debug impl to align with the handler’s typical usage within the swarm and to keep the PR scoped.
  • Docs:
    • TransportTimeout example now uses ListenerId::next() and is marked no_run; doctest passes.

How to pass custom protocols (example)

use libp2p_metrics::{Metrics, Registry};
use libp2p_swarm::StreamProtocol;

// e.g., when an app uses a custom pubsub protocol ID via ConfigBuilder::protocol_id
let custom_gossipsub = StreamProtocol::new("/myappsub/1.2.3");

// You can pass multiple protocols (meshsub/floodsub/customs) as needed.
let extra = [
    custom_gossipsub,
    // StreamProtocol::new("/meshsub/1.2.0"),
    // StreamProtocol::new("/floodsub/1.0.0"),
];

let mut registry = Registry::default();
let metrics = Metrics::new_with_identify_allowed_protocols(&mut registry, extra);

Defaults remain unchanged when using:

let metrics = Metrics::new(&mut registry);

If anything else would make this more useful for users (naming, docs, or making a builder), happy to adjust.

@torrpriius torrpriius changed the title chore: Implement OneShotHandler Debug, update metrics allowlist and docs chore(metrics, core): parameterize Identify allowlist; add TransportTimeout doc Aug 22, 2025
@elenaf9
Copy link
Member

elenaf9 commented Aug 22, 2025

  • I addressed the Identify allowlist note by making it configurable rather than hardcoding protocol IDs. This keeps defaults intact while allowing applications to opt-in their custom protocol IDs (e.g., when using ConfigBuilder::protocol_id).

This is not pushed yet, I think? I can't see it in the diff.

@torrpriius torrpriius changed the title chore(metrics, core): parameterize Identify allowlist; add TransportTimeout doc chore(metrics,core): make Identify allowlist configurable Aug 25, 2025
@torrpriius
Copy link
Author

@elenaf9
Thanks for pointing it out — I hadn’t included the Identify parameterization earlier. It’s now pushed.

  • Identify allowlist is configurable (no hard-coded Gossipsub IDs).
  • Added identify::Metrics::new_with_allowed_protocols(...).
  • Added libp2p_metrics::Metrics::new_with_identify_allowed_protocols(...).
  • Defaults remain unchanged; applications can pass custom StreamProtocol when using ConfigBuilder::protocol_id.

@torrpriius torrpriius changed the title chore(metrics,core): make Identify allowlist configurable chore(metrics): make Identify allowlist configurable Aug 25, 2025
@torrpriius
Copy link
Author

@dariusc93
Thanks for the suggestion — done.

I moved the TransportTimeout doc example into a separate PR as requested: #6152. This PR (#6147) now contains only the Identify allowlist parameterization.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/generated This issue might have been automatically generated need/author-input Needs input from the original author
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants