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

Question: Extending Gossipsub protocol list #408

Closed
richard-ramos opened this issue Mar 23, 2021 · 11 comments · Fixed by #413
Closed

Question: Extending Gossipsub protocol list #408

richard-ramos opened this issue Mar 23, 2021 · 11 comments · Fixed by #413
Assignees
Labels
help wanted Seeking public contribution on this issue status/ready Ready to be worked

Comments

@richard-ramos
Copy link
Contributor

Is it possible to extend the list of protocols returned in:

func (gs *GossipSubRouter) Protocols() []protocol.ID {
return []protocol.ID{GossipSubID_v11, GossipSubID_v10, FloodSubID}
}

I'd like to include an additional ID besides GossipSubID_v11, GossipSubID_v10, FloodSubID, yet I find it difficult to extend GossipSubRouter to include my custom ID.

Floodsub supports this feature with NewFoodsubWithProtocols

func NewFloodsubWithProtocols(ctx context.Context, h host.Host, ps []protocol.ID, opts ...Option) (*PubSub, error) {
rt := &FloodSubRouter{
protocols: ps,
}
return NewPubSub(ctx, h, rt, opts...)
}

Is adding a similar functionality something viable? I could create a PR that adds a NewGossipsubWithProtocols(ctx context.Context, h host.Host, ps []protocol.ID, opts ...Option) (*PubSub, error)

@vyzo
Copy link
Collaborator

vyzo commented Mar 23, 2021

In principle we could, but there is some impedance mismatch in that the router uses the protocol ID to enable certain features.
For example, it checks for the floodsub ID to determine whether to just treat those peers as simple floodsub peers.
It also checks for the GSv1.1 ID to check whether to enable PX.

@richard-ramos
Copy link
Contributor Author

Here's the code for my previous idea:
master...richard-ramos:master

There is some impedance mismatch in that the router uses the protocol ID to enable certain features.

Perhaps this coud be solved by always return GossipSubID_v11, GossipSubID_v10, FloodSubID, yet still allowing to add custom protocols? Something like this code:

func (gs *GossipSubRouter) Protocols() []protocol.ID {
	ps := []protocol.ID{GossipSubID_v11, GossipSubID_v10, FloodSubID}
	return append(ps, gs.protocols...)
}

If having this logic is not desirable, would you mind sharing some tips on how could I achieve something similar with the current implementation of GossipSub?

@vyzo
Copy link
Collaborator

vyzo commented Mar 23, 2021 via email

@richard-ramos
Copy link
Contributor Author

I'm attempting to implement this spec: https://specs.vac.dev/specs/waku/v2/waku-relay
The TLDR is:

  • It's a thin layer over GossipSub
  • Has the following protocol identifier: /vac/waku/relay/2.0.0-beta2
  • Uses StrictNoSign

I tried to implement it without forking go-libp2p-pubsub, but found it very difficult to do so, hence why attempted to implement it here using the solution that I described earlier: https://github.com/status-im/go-waku/blob/master/waku/v2/protocol/waku_relay.go#L29-L43

@vyzo
Copy link
Collaborator

vyzo commented Mar 23, 2021 via email

@richard-ramos
Copy link
Contributor Author

Cool! I will start working on it!
Do you have specific requirements on how should this feature check function work?

Thanks!

@vyzo
Copy link
Collaborator

vyzo commented Mar 23, 2021 via email

@BigLep
Copy link

BigLep commented Mar 29, 2021

@richard-ramos : are you planning to open a PR here?

@BigLep BigLep added help wanted Seeking public contribution on this issue status/ready Ready to be worked labels Mar 29, 2021
@richard-ramos
Copy link
Contributor Author

Hello @BigLep, I'm not able to work on a PR for this for the time being!

@vyzo
Copy link
Collaborator

vyzo commented Apr 2, 2021

@richard-ramos

Implemented support for custom gossipsub protocols in #413.

To use it, just instantiate your gossipsub using the WithGossipSubProtocols option; it takes a list of protocols and a feature test function.

So you shouldn't have to maintain a fork anymore and miss out on all the bug fixes and new features.

@vyzo vyzo closed this as completed in #413 Apr 2, 2021
@richard-ramos
Copy link
Contributor Author

Awesome! thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Seeking public contribution on this issue status/ready Ready to be worked
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants