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.

Sign up for GitHub

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

chore(gossipsub): add DEFAULT_MAX_TRANSMIT_SIZE const #5906

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

chore(gossipsub): add DEFAULT_MAX_TRANSMIT_SIZE const #5906

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
240e0a0
feat(gossipsub): create DEFAULT_MAX_TRANSMIT_SIZE and use it in the c…
diegomrsantos Mar 4, 2025
0255c72
fmt
diegomrsantos Mar 4, 2025
1e72f14
make const public
diegomrsantos Mar 4, 2025
3773843
export const
diegomrsantos Mar 4, 2025
f928ca7
add changelog entry
diegomrsantos Mar 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions protocols/gossipsub/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
- Improve `max_messages_per_rpc` consistency by ensuring RPC control messages also adhere to the existing limits.
See [PR 5826](https://github.com/libp2p/rust-libp2p/pull/5826)

- Create `DEFAULT_MAX_TRANSMIT_SIZE` and use it in the code and docs. The main motivation is to fix the incorrect value mentioned in the docs and prevent it from happening again. See [PR 5906](https://github.com/libp2p/rust-libp2p/pull/5906)
Copy link
Member

Choose a reason for hiding this comment

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

this is more like a chore, we don't need to add a CHANGELOG.md entry

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's a failure as well https://github.com/libp2p/rust-libp2p/actions/runs/13655713410/job/38174302038


## 0.48.0

- Allow broadcasting `IDONTWANT` messages when publishing to avoid downloading data that is already available.
Expand Down
6 changes: 4 additions & 2 deletions protocols/gossipsub/src/config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,8 @@ impl Config {
self.check_explicit_peers_ticks
}

/// The maximum byte size for each gossipsub RPC (default is 65536 bytes).
/// The maximum byte size for each gossipsub RPC (default is
/// [`crate::protocol::DEFAULT_MAX_TRANSMIT_SIZE`] bytes).
///
/// This represents the maximum size of the published message. It is additionally wrapped
/// in a protobuf struct, so the actual wire size may be a bit larger. It must be at least
Expand Down Expand Up @@ -619,7 +620,8 @@ impl ConfigBuilder {
self
}

/// The maximum byte size for each gossip (default is 2048 bytes).
/// The maximum byte size for each gossip (default is
/// [`crate::protocol::DEFAULT_MAX_TRANSMIT_SIZE`] bytes).
pub fn max_transmit_size(&mut self, max_transmit_size: usize) -> &mut Self {
self.config.protocol.max_transmit_size = max_transmit_size;
self
Expand Down
2 changes: 2 additions & 0 deletions protocols/gossipsub/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,3 +136,5 @@ pub type Rpc = self::types::Rpc;

pub type IdentTopic = Topic<self::topic::IdentityHash>;
pub type Sha256Topic = Topic<self::topic::Sha256Hash>;

pub use crate::protocol::DEFAULT_MAX_TRANSMIT_SIZE;
Copy link
Member

Choose a reason for hiding this comment

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

do we need it to be pub?

4 changes: 3 additions & 1 deletion protocols/gossipsub/src/protocol.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@ pub(crate) const FLOODSUB_PROTOCOL: ProtocolId = ProtocolId {
kind: PeerKind::Floodsub,
};

pub const DEFAULT_MAX_TRANSMIT_SIZE: usize = 65536;
Copy link
Member

Choose a reason for hiding this comment

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

same as above, do we need this to be pub?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's a failure otherwise https://github.com/libp2p/rust-libp2p/actions/runs/13655153154/job/38172407946

Copy link
Member

Choose a reason for hiding this comment

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

and doesn't pub(crate) work?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's what I had when it failed 1e72f14


/// Implementation of [`InboundUpgrade`] and [`OutboundUpgrade`] for the Gossipsub protocol.
#[derive(Debug, Clone)]
pub struct ProtocolConfig {
Expand All @@ -75,7 +77,7 @@ pub struct ProtocolConfig {
impl Default for ProtocolConfig {
fn default() -> Self {
Self {
max_transmit_size: 65536,
max_transmit_size: DEFAULT_MAX_TRANSMIT_SIZE,
validation_mode: ValidationMode::Strict,
protocol_ids: vec![
GOSSIPSUB_1_2_0_PROTOCOL,
Expand Down
Loading