Skip to content
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

Override protocol id prefix on GossipsubConfigBuilder #2709

Closed
bernardoaraujor opened this issue Jun 15, 2022 · 2 comments
Closed

Override protocol id prefix on GossipsubConfigBuilder #2709

bernardoaraujor opened this issue Jun 15, 2022 · 2 comments

Comments

@bernardoaraujor
Copy link
Contributor

bernardoaraujor commented Jun 15, 2022

Description

I need to be able to set the entire GossipSub's ProtocolId, not only the prefix.

Motivation

I'm working on a Rust implementation of Waku, an evolution of Ethereum Whisper: https://github.com/bernardoaraujor/waku-rs

At this point, I have a minimal working implementation of 11/WAKU2-RELAY, 13/WAKU2-STORE and 19/WAKU2-LIGHTPUSH.

When trying to connect it to the Nim reference implementation without success, I noticed the following messages on the Nim logs:

DBG 2022-06-15 18:23:17.558-03:00 no handlers                                topics="libp2p multistream" tid=77736 file=multistream.nim:173 conn=12D*PH8vPd:62aa4dc5649138700bc0af39 protocol=//vac/waku/relay/2.0.0/1.1.0
DBG 2022-06-15 18:23:17.558-03:00 Could not establish send connection        topics="libp2p pubsubpeer" tid=77736 file=pubsubpeer.nim:205 msg="Unable to select sub-protocol @[\"/vac/waku/relay/2.0.0\", \"/vac/waku/relay/2.0.0\"]

where protocol=//vac/waku/relay/2.0.0/1.1.0 is clearly not what I intended with RELAY_PROTOCOL_ID.

It turns out that's a result of GossipsubConfigBuilder::default().protocol_id_prefix(RELAY_PROTOCOL_ID), which uses RELAY_PROTOCOL_ID as a mere prefix for ProtocolId, adding extra / and /1.1.0.

Requirements

A way to set ProtocolId on GossipSub without prefix logic.

Open questions

  • What is the feasibility of this feature?
  • Are there any reasons not to have this feature on GossipSub?

Are you planning to do it yourself in a pull request?

Maybe. I would probably need some guidance.

@bernardoaraujor bernardoaraujor changed the title Override protocol prefix on GossipsubConfigBuilder Override protocol id prefix on GossipsubConfigBuilder Jun 15, 2022
@mxinden
Copy link
Member

mxinden commented Jun 20, 2022

Hi @bernardoaraujor,

Thanks for the detailed feature request.

Are there any reasons not to have this feature on GossipSub?

I don't see a reason why not to support your use-case.

It turns out that's a result of GossipsubConfigBuilder::default().protocol_id_prefix(RELAY_PROTOCOL_ID), which uses RELAY_PROTOCOL_ID as a mere prefix for ProtocolId, adding extra / and /1.1.0.

How about adding an additional method e.g. GossipsubConfigBuilder::protocol_id to be able to set the whole id?

Maybe. I would probably need some guidance.

Great. I am happy to help.

@mxinden
Copy link
Member

mxinden commented Jul 2, 2022

Closing here with #2718 merged.

@mxinden mxinden closed this as completed Jul 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants