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

fix(gossipsub): forward to floodsub peers #5908

Merged
merged 5 commits into from
Mar 7, 2025
Merged

Conversation

turuslan
Copy link
Contributor

@turuslan turuslan commented Mar 4, 2025

Description

messages weren't forwarded to floodsub peers

Notes & open questions

rust-libp2p gossipsub forward_msg sends message to explicit and mesh peers only,
which doesn't include floodsub peers.
go-libp2p-pubsub gossipsub forwards messages to explicit and mesh peers too,
as well as floodsub peers (code)
(call stack: PubSub.handleIncomingRPC, PubSub.pushMsg, PubSub.publishMessage, GossipSubRouter.Publish).

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

messages weren't forwarded to floodsub peers
Copy link
Member

@jxs jxs left a comment

Choose a reason for hiding this comment

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

Hi, and thanks for this!
Spec wise this makes sense to me, but I think we can make it more optimal, with your added code we are iterating the connected_peers again after getting the ones from the explicit_peers, can we do the opposite instead? I.e iterate over the connected_peers and check if the explicit_peers contains them?

@turuslan turuslan requested a review from jxs March 5, 2025 05:03
Copy link
Member

@jxs jxs left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks!
Can you update the CHANGELOG.md and Cargo.toml files?

@mergify mergify bot merged commit 9caa7f5 into libp2p:master Mar 7, 2025
71 checks passed
@turuslan turuslan deleted the patch-2 branch March 7, 2025 07:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants