feat(floodsub): Add option to configure the maximum message length

[romac]

Description

This PR adds a max_message_len option to FloodsubConfig for configuring the maximum message length.

Notes & open questions

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

[romac]

Sorry, did not mean to open that PR already. Will leave as draft until properly tested on our side.

[dariusc93]

LGTM so far. Could you bump the minor version in both Cargo.toml and update the changelog entry?

[romac]

Ready for review. CI failure does not seem related to this PR.

[dariusc93]

Ready for review. CI failure does not seem related to this PR.

Youre right. That is unrelated and would be handled in a different PR in the future.

[jxs]

LGTM thanks! Just one question

[jxs]

this doesn't seem like a breaking change, do we need a minor version bump instead of patch?

[dariusc93]

I would assume that the introduction of a new public field in FloodsubConfig could be considered as one, especially given that it doesnt use the non_exhaustive attribute. I could be wrong though, but would have to check semver on that. If it is the case that it is a breaking change, we could set this PR to draft until the next round of breaking changes.

[romac]

Yes, I unfortunately do not see a way around doing a minor version bump, at least according to cargo-semver-checks:

argo semver-checks --baseline-version 0.45.0 --release-type patch
     Parsing libp2p-floodsub v0.46.0 (current)
      Parsed [   2.067s] (current)
     Parsing libp2p-floodsub v0.45.0 (baseline, cached)
      Parsed [   0.170s] (baseline)
    Checking libp2p-floodsub v0.45.0 -> v0.46.0 (assume patch change)
     Checked [   0.013s] 79 checks: 78 pass, 1 fail, 0 warn, 0 skip

--- failure constructible_struct_adds_field: externally-constructible struct adds field ---

Description:
A pub struct constructible with a struct literal has a new pub field. Existing struct literals must be updated to include the new field.
        ref: https://doc.rust-lang.org/reference/expressions/struct-expr.html
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.34.0/src/lints/constructible_struct_adds_field.ron

Failed in:
  field FloodsubProtocol.max_message_len in /Users/romac/Informal/Code/rust-libp2p/protocols/floodsub/src/protocol.rs:43
  field FloodsubConfig.max_message_len in /Users/romac/Informal/Code/rust-libp2p/protocols/floodsub/src/lib.rs:54
  field FloodsubRpc.max_message_len in /Users/romac/Informal/Code/rust-libp2p/protocols/floodsub/src/protocol.rs:155
  field FloodsubRpc.max_message_len in /Users/romac/Informal/Code/rust-libp2p/protocols/floodsub/src/protocol.rs:155

     Summary semver requires new major version: 1 major and 0 minor checks failed
    Finished [   2.267s] libp2p-floodsub

Note that what cargo-semver-checks calls a major version corresponds to a minor version bump for libp2p-floodsub since the crate is at 0.x.y.

[romac]

By the way, if we are doing a breaking change in libp2p-floodsub, would you accept a PR to align the naming of types with the other crates, as per #2217?

ie. rename FloodsubConfig to just Config, Floodsub to Behavior and FloodsubEvent to Event, etc.

[dariusc93]

By the way, if we are doing a breaking change in libp2p-floodsub, would you accept a PR to align the naming of types with the other crates, as per #2217?

ie. rename FloodsubConfig to just Config, Floodsub to Behavior and FloodsubEvent to Event, etc.

We can do this in a separate PR and just use an alias pointing to the new names with the alias being deprecated :)

[romac]

Cool, will open a PR for this sometime soon then :)

[jxs]

that's the problem, those fields don't need to be public in FloodsubConfig, and probably FloodsubRpc and FloodsubProtocol also don't need to be public. Could you also do it in the subsequent PR @romac?
Thanks!

[mergify]

This pull request has merge conflicts. Could you please resolve them @romac? 🙏