-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Gossipsub Protocol #898
Gossipsub Protocol #898
Conversation
…de and modifies GossipsubRpc
- Adds the log crate and implements logging macros - Specifies versions for external crates
- Adds basic documentation, overview and examples to the gossipsub crate.
This commit also adds the inject_connected test.
- Modifies `handle_received_subscriptions` to take a reference of subscriptions - Adds `test_subscribe` - Adds `test_handle_received_subscriptions` - Adds tests for the filter in `get_random_peers`
Just updated sigp#22, sorry for the delay. |
Ok. Stable-futures and latest master have been merged in. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I only gave a brief looked, and I'm tempted to trust @mxinden's review when it comes to the logic.
Applied @tomaka's suggestions and re-merged master. |
I have another pull request following up on the inbound substream error handling discussion: sigp#24. I think the discussion is worth having. I don't think it should block this pull request. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I only gave a brief looked, and I'm tempted to trust @mxinden's review when it comes to the logic.
I would interpret this as an approval @tomaka? Anyone else wanting to give this a review?
I have a couple of smaller comments left, but I think this is in a great merge-able state.
Thanks @AgeManning for all the hard work!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 for merging! I haven't actually looked at the logic, but the general code organization and clean-ness looks code.
@AgeManning Are we good for merging this? |
Yep I think so. I've not updated the code base I'm using to stable futures yet, so haven't tested this version at any scale. If this sounds good to you guys, merge away :) |
Is there already an issue tracking this somewhere? I couldn't find anything but would be interested in following it's progress and to coordinate potential contributions :) |
@thomaseizinger |
This is an implementation of the gossipsub protocol as in the libp2p-specs.
This is feature-complete, except for the control-message piggy-backing and peer-tagging, which still need to be implemented.
Currently this implementation is not backwards compatible with floodsub nodes (see #880).
Further testing and debugging will be done over the next few weeks.