Skip to content

Conversation

@AgeManning
Copy link
Contributor

Description

I noticed two issues during testing.

  1. We allow our mesh to grow greater than mesh_n_high, intentionally
  2. There is a potential underflow in the heartbeat that can cause a panic

For the first 1. This looks like its intentional but I can't recall why we would have added it. I think its counter-intuitive to allow our mesh to grow larger than the specified parameter. I suspect we added it to prevent our mesh from being filled with inbound peers and potentially being eclipsed. I suspect the best approach here is to remove inbound peers in the mesh maintenance rather than exceeding the mesh_n_high configuration.

For 2. There is an underflow which this PR prevents. It can be triggered for low mesh_n_high values, i.e 0. This shouldn't be a concern for regular users, but we shouldn't have code that can panic based on user configuration.

@AgeManning AgeManning changed the title Correct underflow and prevent mesh exceeding mesh_n_high fix: Correct underflow and prevent mesh exceeding mesh_n_high Oct 17, 2025
Copy link
Member

@jxs jxs left a comment

Choose a reason for hiding this comment

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

hey man thanks for this! Can we split these two into two different PR's and add CHANGELOG.md entries for each?

@jxs jxs force-pushed the underflow-gossipsub branch from fd7922c to a41be07 Compare October 17, 2025 19:36
@jxs jxs force-pushed the underflow-gossipsub branch from a41be07 to 3581061 Compare October 17, 2025 19:38
@jxs jxs changed the title fix: Correct underflow and prevent mesh exceeding mesh_n_high fix(gossipsub): Fix underflow when shuffling peers after prunning. Oct 17, 2025
@jxs jxs added the send-it label Oct 17, 2025
@mergify mergify bot added the queued label Oct 17, 2025
@mergify mergify bot merged commit a753ab0 into libp2p:master Oct 17, 2025
71 checks passed
@mergify mergify bot removed the queued label Oct 17, 2025
mergify bot pushed a commit that referenced this pull request Oct 24, 2025
Split off from #6183, to quote:
>I noticed two issues during testing.
>
>We allow our mesh to grow greater than mesh_n_high, intentionally
>This looks like its intentional but I can't recall why we would have added it. I think its counter-intuitive to allow our mesh to grow larger than the specified parameter. I suspect we added it to prevent our mesh from being filled with inbound peers and potentially being eclipsed. I suspect the best approach here is to remove inbound peers in the mesh maintenance rather than exceeding the mesh_n_high configuration.

Pull-Request: #6184.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants