Gossipsub v1.1 #1720

AgeManning · 2020-08-27T08:25:19Z

Description

This PR upgrades the current gossipsub implementation to support the v1.1 spec.

It adds a number of features, bug fixes and performance improvements.

Besides support for all new 1.1 features, other improvements that are of particular note:

Improved duplicate LRU-time cache (this was previously a severe bottleneck for large message throughput topics)
Extended message validation configuration options
Arbitrary topics (users can now implement their own hashing schemes)
Improved message validation handling - Invalid messages are no longer dropped but sent to the behaviour for application-level processing (including scoring)
Support for floodsub, gossipsub v1 and gossipsub v2
Protobuf encoding has been shifted into the behaviour. This has permitted two improvements:
1. Message size verification during publishing (report to the user if the message is too large before attempting to send).
2. Message fragmentation. If an RPC is too large it is fragmented into its sub components and sent in smaller chunks.

Additional Notes

The peer eXchange protocol defined in the v1.1 spec is inactive in its current form. The current implementation permits sending PeerId in PRUNE messages, however a PeerId is not sufficient to form a new connection to a peer. A Signed Address Record is required to safely transmit peer identity information. Once these are confirmed (libp2p/specs#217) a future PR will implement these and make PX usable.

Final Note

This is currently undergoing further testing and an audit and I may modify this slightly as bugs come up :)

mxinden · 2020-11-20T21:05:11Z

As promised I gave this another review, see sigp#93.

Given the size of #1720 I did my review directly inline. A lot of suggestions are just related to styling - I hope delivering my review as a pull request makes accepting those easier. Comments are prefixed with // mxinden:.

Let me know if this format works for you. If not, I can transform this into a traditional Github review.

Again, as #1720 is quite large, I have not yet fully reviewed all changes / additions.

AgeManning · 2020-11-27T07:30:23Z

Thanks max. I think we've addressed all the comments. I've merged into your branch there. Let me know if anything is missed.

If all good, I'll merge that branch down and then merge latest master.

* protocols/gossipsub: Review * Addressing most of the review comments * Address reviewers comments * Appease CI Co-authored-by: blacktemplar <blacktemplar@a1.net> Co-authored-by: Age Manning <Age@AgeManning.com>

jolestar · 2020-12-07T08:13:20Z

It is possible to support a custom peer selector/filter strategy for publishing messages?

AgeManning · 2020-12-08T08:25:47Z

It is possible to support a custom peer selector/filter strategy for publishing messages?

We could add this in, but I'm not sure why you would want to. What is it's intended purpose? Do you want it to apply just to publishing messages or also to forwarding?

Lets say you want to publish to only half the peers in your mesh (based on some filter). Those peers would then forward it to peers in their mesh and the message would propagate regardless (with added latency), including to those you don't directly publish to. This filtering would also affect the scoring that is implemented in 1.1 as some nodes would look like they are censoring.

Maybe if you could detail the reasons why you might want this I could provide more help.

jolestar · 2020-12-09T01:45:02Z

We could add this in, but I'm not sure why you would want to. What is it's intended purpose? Do you want it to apply just to publishing messages or also to forwarding?

Lets say you want to publish to only half the peers in your mesh (based on some filter). Those peers would then forward it to peers in their mesh and the message would propagate regardless (with added latency), including to those you don't directly publish to. This filtering would also affect the scoring that is implemented in 1.1 as some nodes would look like they are censoring.

Maybe if you could detail the reasons why you might want this I could provide more help.

Thanks. My usage scenario is blockchain. I want to publish messages to some peers base on the peer's state, such as peer's type(lighting node/full node), block_height, or total_difficulity. If the Gossipsub does not support a custom strategy, the application needs to implement a custom Gossip protocol, such as subtract(https://github.com/paritytech/substrat) network/network-gossip. But the hardest part of the custom strategy is how to maintain the custom state of the peer. if the custom state can not keep in Gossipsub's peer state, it will very hard to implement a peer filter strategy.

blacktemplar · 2020-12-10T13:17:43Z

We could add this in, but I'm not sure why you would want to. What is it's intended purpose? Do you want it to apply just to publishing messages or also to forwarding?
Lets say you want to publish to only half the peers in your mesh (based on some filter). Those peers would then forward it to peers in their mesh and the message would propagate regardless (with added latency), including to those you don't directly publish to. This filtering would also affect the scoring that is implemented in 1.1 as some nodes would look like they are censoring.
Maybe if you could detail the reasons why you might want this I could provide more help.

Thanks. My usage scenario is blockchain. I want to publish messages to some peers base on the peer's state, such as peer's type(lighting node/full node), block_height, or total_difficulity. If the Gossipsub does not support a custom strategy, the application needs to implement a custom Gossip protocol, such as subtract(https://github.com/paritytech/substrat) network/network-gossip. But the hardest part of the custom strategy is how to maintain the custom state of the peer. if the custom state can not keep in Gossipsub's peer state, it will very hard to implement a peer filter strategy.

I think you should try to solve that with dividing your topics. If you really need dynamic strategies for distributing messages I am not sure if gossipsub is the right way to do it. You would have to think how to handle the different states in your IHAVE messages too, so it becomes very complicated very fast.

* Update rustls requirement from 0.18.0 to 0.19.0 (libp2p#1852) Updates the requirements on [rustls](https://github.com/ctz/rustls) to permit the latest version. - [Release notes](https://github.com/ctz/rustls/releases) - [Changelog](https://github.com/ctz/rustls/blob/main/OLDCHANGES.md) - [Commits](rustls/rustls@v/0.18.0...v/0.19.0) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Prepare libp2p-websocket-0.26.1 * Update top-level libp2p-websocket patch version. * [request-response] Refine success & error reporting for inbound requests. (libp2p#1867) * Refine error reporting for inbound request handling. At the moment one can neither get confirmation when a response has been sent on the underlying transport, nor is one aware of response omissions. The latter was originally intended as a feature for support of one-way protocols, which seems like a bad idea in hindsight. The lack of notification for sent responses may prohibit implementation of some request-response protocols that need to ensure a happens-before relation between sending a response and a subsequent request, besides uses for collecting statistics. Even with these changes, there is no active notification for failed inbound requests as a result of connections unexpectedly closing, as is the case for outbound requests. Instead, for pending inbound requests this scenario can be identified if necessary by the absense of both `InboundFailure` and `ResponseSent` events for a particular previously received request. Interest in this situation is not expected to be common and would otherwise require explicitly tracking all inbound requests in the `RequestResponse` behaviour, which would be a pity. `RequestResponse::send_response` now also synchronously returns an error if the inbound upgrade handling the request has been aborted, due to timeout or closing of the connection, giving more options for graceful error handling for inbound requests. As an aside, the `Throttled` wrapper now no longer emits inbound or outbound error events occurring in the context of sending credit requests or responses. This is in addition to not emitting `ResponseSent` events for ACK responses of credit grants. * Update protocols/request-response/src/lib.rs Co-authored-by: Max Inden <mail@max-inden.de> * Address some minor clippy warnings. (libp2p#1868) * Track pending credit request IDs. In order to avoid emitting events relating to credit grants or acks on the public API. The public API should only emit events relating to the actual requests and responses sent by client code. * Small cleanup * Cleanup * Update versions and changelogs. * Unreleased Co-authored-by: Max Inden <mail@max-inden.de> * core/benches: Add rudimentary benchmark for PeerId::from_bytes and clone (libp2p#1875) * core: Add rudimentary benchmark for PeerId::from_bytes and clone * .github/workflow: Include benchmarks To ensure changes through pull requests won't make benchmarks fail to compile or run, run them as part of CI. * [mdns] Split response packets if necessary. (libp2p#1877) * [mdns] Split response packets. Prevent MDNS response packets becoming too large by creating multi-packet responses. Also skip addresses that don't fit into a TXT record or contain invalid characters. * Update protocols/mdns/src/dns.rs Co-authored-by: Max Inden <mail@max-inden.de> * Refactor response packet construction. * Update mdns changelog. Co-authored-by: Max Inden <mail@max-inden.de> * core/benches: Add PeerId sort_vec benchmark (libp2p#1878) * Prepare v0.32 (libp2p#1879) * [websocket] Update minimum async-tls patch version. (libp2p#1881) * Update minimum async-tls patch version. After the upgrade to rustls 0.19, this is the minimum version required to build. * Prepare libp2p patch. * Update async-tls requirement from 0.10.2 to 0.11.0 (libp2p#1884) * Update async-tls requirement from 0.10.2 to 0.11.0 Updates the requirements on [async-tls](https://github.com/async-std/async-tls) to permit the latest version. - [Release notes](https://github.com/async-std/async-tls/releases) - [Commits](async-rs/async-tls@v0.10.2...v0.11.0) Signed-off-by: dependabot[bot] <support@github.com> * *: Prepare release Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Max Inden <mail@max-inden.de> * rename generic types + add default types + update documentation links * fix broken intra links * fix tests * type annotation in example * fix doc example * Add compression to gossipsub * Add snappy compression to the smoke tests * Stack allocated PeerId (libp2p#1874) * Stack allocate PeerId. * Update stuff. * Upgrade rusttls to fix build. * Remove unnecessary manual implementations. * Remove PeerId::into_bytes. * Remove bytes dependency. * Perform some cleanup. * Use Into<kbucket::Key<K>>. * Update versions and changelogs. * Fix PR link. * Fix benchmarks. Co-authored-by: Roman S. Borschel <roman@parity.io> * Remove copy and adjust validation ordering Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Roman S. Borschel <roman@parity.io> Co-authored-by: Roman Borschel <romanb@users.noreply.github.com> Co-authored-by: Max Inden <mail@max-inden.de> Co-authored-by: blacktemplar <blacktemplar@a1.net> Co-authored-by: David Craven <david@craven.ch>

jolestar · 2020-12-16T02:45:28Z

We could add this in, but I'm not sure why you would want to. What is it's intended purpose? Do you want it to apply just to publishing messages or also to forwarding?
Lets say you want to publish to only half the peers in your mesh (based on some filter). Those peers would then forward it to peers in their mesh and the message would propagate regardless (with added latency), including to those you don't directly publish to. This filtering would also affect the scoring that is implemented in 1.1 as some nodes would look like they are censoring.
Maybe if you could detail the reasons why you might want this I could provide more help.

Thanks. My usage scenario is blockchain. I want to publish messages to some peers base on the peer's state, such as peer's type(lighting node/full node), block_height, or total_difficulity. If the Gossipsub does not support a custom strategy, the application needs to implement a custom Gossip protocol, such as subtract(https://github.com/paritytech/substrat) network/network-gossip. But the hardest part of the custom strategy is how to maintain the custom state of the peer. if the custom state can not keep in Gossipsub's peer state, it will very hard to implement a peer filter strategy.

I think you should try to solve that with dividing your topics. If you really need dynamic strategies for distributing messages I am not sure if gossipsub is the right way to do it. You would have to think how to handle the different states in your IHAVE messages too, so it becomes very complicated very fast.

The gossipsub protocol broadcast message base node's knowledge base, node exchange knowledge by IHAVE message. But IHAVE only support MessageID Vec, I think whether it is possible to support a weighted id, the weight may be message's index or other sortable property, it can help to filter peer and reduce repeat gossip messages.

blacktemplar · 2020-12-16T06:02:59Z

We could add this in, but I'm not sure why you would want to. What is it's intended purpose? Do you want it to apply just to publishing messages or also to forwarding?
Lets say you want to publish to only half the peers in your mesh (based on some filter). Those peers would then forward it to peers in their mesh and the message would propagate regardless (with added latency), including to those you don't directly publish to. This filtering would also affect the scoring that is implemented in 1.1 as some nodes would look like they are censoring.
Maybe if you could detail the reasons why you might want this I could provide more help.

Thanks. My usage scenario is blockchain. I want to publish messages to some peers base on the peer's state, such as peer's type(lighting node/full node), block_height, or total_difficulity. If the Gossipsub does not support a custom strategy, the application needs to implement a custom Gossip protocol, such as subtract(https://github.com/paritytech/substrat) network/network-gossip. But the hardest part of the custom strategy is how to maintain the custom state of the peer. if the custom state can not keep in Gossipsub's peer state, it will very hard to implement a peer filter strategy.

I think you should try to solve that with dividing your topics. If you really need dynamic strategies for distributing messages I am not sure if gossipsub is the right way to do it. You would have to think how to handle the different states in your IHAVE messages too, so it becomes very complicated very fast.

The gossipsub protocol broadcast message base node's knowledge base, node exchange knowledge by IHAVE message. But IHAVE only support MessageID Vec, I think whether it is possible to support a weighted id, the weight may be message's index or other sortable property, it can help to filter peer and reduce repeat gossip messages.

I am not sure if I understand you correctly, but if you propose additions to the gossipsub protocol you are probably better of to propose them in https://github.com/libp2p/specs/.

mxinden

I would be in favor of merging this pull request into master. As far as I can tell, the only outstanding task would be to fix the intra-doc links. What do you think @AgeManning @blacktemplar?

mxinden · 2021-01-05T16:19:12Z

protocols/gossipsub/src/transform.rs

@@ -0,0 +1,58 @@
+//! This trait allows of extended user-level decoding that can apply to message-data before a


Can you add a license header here?

Can do.

I think its mostly stable now. Will fix the doc links.

AgeManning · 2021-01-06T01:05:10Z

Fixed the doclinks and merged latest master.

mxinden · 2021-01-06T07:51:39Z

Great. I will merge later today unless anyone objects till then.

mxinden · 2021-01-07T07:22:07Z

🎉 Thank you for the extensive amount of work you put into this.

diegomrsantos · 2024-04-12T16:16:26Z

protocols/gossipsub/src/peer_score/params.rs

+            gossip_threshold: -10.0,
+            publish_threshold: -50.0,
+            graylist_threshold: -80.0,
+            accept_px_threshold: 10.0,
+            opportunistic_graft_threshold: 20.0,


@AgeManning could you please share how those values were defined?

These essentially can be defined for whatever network you are running gossipsub on. Its been a few years, but I believe these were from the go-implementation at the time, which may have been used in some of the tests in the v1.1 gossipsub paper where they tested a few attack vectors.

These numbers shouldn't be set independently, but based on the maximum and minimum values you can get from the score.

I found this PR that says the values were inspired by LH status-im/nimbus-eth2#2091. These are the values used:

gossipThreshold: -4000, publishThreshold: -8000, graylistThreshold: -16000

They are used here indeed https://gist.github.com/blacktemplar/5c1862cb3f0e32a1a7fb0b25e79e6e2c#file-51000-toml-L33, but LH doesn't use to configure GossipSub, but rather here

rust-libp2p/protocols/gossipsub/src/metrics.rs

Line 76 in 25d1434

let gossip_threshold = -4000.0;

Now I"m confused about what you are asking.

The values in this repository i.e gossip_threshold: -10 is not the same value as what we have chosen in the Ethereum consensus network i.e -4000.

As I mentioned, these values need to be defined for whatever network you are running. For Ethereum's consensus network, we did a bunch of estimations and determined a bunch of numbers. That is entirely independent of the gossipsub protocol, which should run on any network.

If you question is specific to lighthouse or the Ethereum network, then that is a different question and probably should be asked on the lighthouse repo as those values are for a specific network.

You did point out that the metrics value uses the Ethereum consensus values as default. This probably should be using the same defaults as the rust-libp2p repo. At the time, Lighthouse was the only one consuming these metrics and so was built around our scoring values to get sane results in the histograms.

You're right, sorry for the confusion. It's clear now, thanks. I somehow misled myself looking at this PR that introduces params in the lighthouse rust-libp2p fork https://github.com/sigp/rust-libp2p/pull/556/files#diff-9f622a0085d0d96aef9e883b7afdcdd4a652e8801be7cfabf82819f5a74a70e0. Is there a lighthouse PR defining the values for lighthouse?

What specifically are you interested in?

Are you asking how we figured out what values to use in Lighthouse? There is an external document I can share with you if that's the case. The PR to lighthouse will likely just show values being added with no context.

Feel free to DM me in the Lighthouse discord if you like

AgeManning and others added 30 commits May 19, 2020 17:39

Gossipsub message signing

aeba523

Update ipfs-private example

573e4b6

Add optimisation to prevent key calculation each message

d2babd4

Support multiple versions

8353474

Initial porting of peer_scoring from go-libp2p

b7db121

Handle forwarding of signed messages and reviewers comments

ac3c977

Merge latest gossipsub signing

f3ff66b

Progress to porting scoring functions

c8335cc

Merge branch 'master' into gossipsub-signing

e9afd95

Finish first port of score params

984a662

Add defaults to scoring

80ea7df

Generalise topic hashing

58f61b4

Update topic hashing structure

238e305

Update v1.1 tests

fec112a

Add more unit tests

bc09eb7

protocols/gossipsub/src/protocol: Use quickcheck to test signing (#31)

3d1a025

Shift signing into behaviour

31b6849

Merge latest master

a8cec0b

Address reviewers suggestions

2b38112

Send subscriptions to all peers

7d678f5

Update examples

dc050dd

Shift signing option into the behaviour

5bf1cfc

Revert changes to core/

4ffe728

Reviewers suggestion

24b738c

Merge remote-tracking branch 'origin/master' into gossipsub-signing

a2e4bf0

Merge latest master

2f20571

Revert changes to core/

2c50ba2

Switch gossipsub state to use sets instead of vectors

4de878e

Make tests pass

e43f0a0

Some clippy

57f07aa

only respond to IWANT if message got already validated (#96)

5157e70

blacktemplar mentioned this pull request Dec 1, 2020

Out-of-date dependencies sigp/lighthouse#1712

Closed

mxinden and others added 2 commits December 7, 2020 14:41

protocols/gossipsub: Review (#93)

82be02d

* protocols/gossipsub: Review * Addressing most of the review comments * Address reviewers comments * Appease CI Co-authored-by: blacktemplar <blacktemplar@a1.net> Co-authored-by: Age Manning <Age@AgeManning.com>

Merge latest master

ffee445

AgeManning force-pushed the gossipsub-v1.1 branch from d612712 to ffee445 Compare December 7, 2020 04:09

AgeManning added 4 commits December 16, 2020 17:04

Remove feature gating from gossipsub

1b5a35e

Merge latest master

1b19338

Remove compression in favour of a more general data transform

a15b246

Loosen trait bounds on gossipsub

4cca4d3

mxinden approved these changes Jan 5, 2021

View reviewed changes

AgeManning added 2 commits January 6, 2021 11:36

Merge latest master

88ed4e5

Fix doclinks

f47ae78

mxinden merged commit df7e73e into libp2p:master Jan 7, 2021

romanb mentioned this pull request Jan 26, 2021

PubSub topic peer discovery #1559

Closed

mxinden mentioned this pull request Apr 1, 2021

pubsub/gossipsub/README: Mark rust gossipsub v1.0 as complete libp2p/specs#310

Merged

Frederik-Baetens mentioned this pull request Dec 17, 2021

gossipsub peer sharing #2391

Closed

AgeManning deleted the gossipsub-v1.1 branch November 29, 2023 05:11

diegomrsantos reviewed Apr 12, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Gossipsub v1.1 #1720

Gossipsub v1.1 #1720

AgeManning commented Aug 27, 2020

mxinden commented Nov 20, 2020 •

edited

Loading

AgeManning commented Nov 27, 2020

jolestar commented Dec 7, 2020

AgeManning commented Dec 8, 2020

jolestar commented Dec 9, 2020 •

edited

Loading

blacktemplar commented Dec 10, 2020 •

edited

Loading

jolestar commented Dec 16, 2020

blacktemplar commented Dec 16, 2020

mxinden left a comment

mxinden Jan 5, 2021

AgeManning Jan 5, 2021

AgeManning commented Jan 6, 2021

mxinden commented Jan 6, 2021

mxinden commented Jan 7, 2021

diegomrsantos Apr 12, 2024

AgeManning Apr 14, 2024

diegomrsantos Apr 15, 2024

AgeManning Apr 17, 2024

diegomrsantos Apr 17, 2024

AgeManning Apr 17, 2024

		@@ -0,0 +1,58 @@
		//! This trait allows of extended user-level decoding that can apply to message-data before a

Gossipsub v1.1 #1720

Gossipsub v1.1 #1720

Conversation

AgeManning commented Aug 27, 2020

Description

Additional Notes

Final Note

mxinden commented Nov 20, 2020 • edited Loading

AgeManning commented Nov 27, 2020

jolestar commented Dec 7, 2020

AgeManning commented Dec 8, 2020

jolestar commented Dec 9, 2020 • edited Loading

blacktemplar commented Dec 10, 2020 • edited Loading

jolestar commented Dec 16, 2020

blacktemplar commented Dec 16, 2020

mxinden left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AgeManning commented Jan 6, 2021

mxinden commented Jan 6, 2021

mxinden commented Jan 7, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mxinden commented Nov 20, 2020 •

edited

Loading

jolestar commented Dec 9, 2020 •

edited

Loading

blacktemplar commented Dec 10, 2020 •

edited

Loading