Gossipsub default values: a few questions from the Forest team

[johannamoran]

Hi @jxs @elenaf9

Thanks so much for doing the recent rust release.

As per my TG message, I'm putting everything in one place so you and the relevant Forest team @LesnyRumcajs @hanabi1224 can converge. The Forest team have a few questions in relation to below. They are trying to understand the rationale behind the differences. There may be perfectly valid protocol, or implementation-specific reasons for these values, so they'd really appreciate your perspective before drawing any conclusions.

[The Ask to the Forest team] To move this forward @jxs has asked if @LesnyRumcajs could kindly open issues for each of these points below to move forward.

@seetadev cc as FYI

Thanks so much to everyone here!

Johanna

—-

Notes from the Forest team

• a possible mismatch between the default configured value in config.rs and the corresponding value in protocol.rs;

• max_control_message_size being substantially smaller in Rust-libp2p than the corresponding go-libp2p-pubsub default;

• a few additional potential discrepancies documented in the Forest issue tracker.

To give you a heads up specifically see below:

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

Line 555 in 050fbb7

max_publish_messages: 5000,

Vs.

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

Line 102 in 050fbb7

max_publish_messages: 500,

seems the defaults mismatch, is there a reason for this?

—-

the default for max_control_message_size is 32x smaller than the one in

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

Line 103 in 050fbb7

max_control_message_size: 16384, // 16KB

vs.

https://github.com/libp2p/go-libp2p-pubsub/blob/892d7aaf6b8952c1ebfb540901b9a5656615d949/pubsub.go#L33

Is this intended?

—-

Finally...

Additional (potential) mismatches ChainSafe/forest#7616
worth understanding why certain values were chosen and why they differ between implementations

[LesnyRumcajs]

Hey, thanks for tagging.

Just to clarify, I don't see a reason to open separate issues to track the mismatches from Forest's side. These are not issues, just observations that warrant some research and a broader discussion between, e.g., rust-libp2p and go-libp2p maintainers.

Forest has been working fine with the defaults, seldom doing any overrides. I just think it might be worth converging to shared defaults to improve consumers' experience, especially those that use several *-libp2p implementations due. I don't have the expertise to weigh whether changing the defaults to those of go-libp2p or the other way around would improve things on any front (but it might be breaking change for some).

Also, please note @hanabi1224 no longer works in Forest.

[jxs]

Hi, and sorry for the delay answering this.
the max_publish_messages oversight has been corrected thanks, see #6669
wrt to the other values they were discussed before setting on them an we prefer to be conservative on that field, nonetheless those values should all be configurable so feel free to unify/set them according to your needs :)

Cheers