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

Jump to bottom

Replace PeerListAck with GetPeerList #2580

Merged
merged 100 commits into from Jan 14, 2024
Merged

Replace PeerListAck with GetPeerList #2580

merged 100 commits into from Jan 14, 2024

Conversation

StephenButtolph
Copy link
Contributor

@StephenButtolph StephenButtolph commented Jan 3, 2024 •
edited

Why this should be merged

In spite of the PeerListAck optimization, a significant portion of network bandwidth is used by PeerList gossip. Additionally, the propagation speed of validator IP changes is exceedingly slow.

This PR replaces the entire PeerList gossip mechanism with a pull gossip based approach. This aligns the gossip mechanism with the gossip mechanism used for consensus and transaction gossip.

How this works

  • Removes handling of PeerListAck messages
  • Removes the gossip tracker
  • Implements a validator tracker
  • Adds a bloom filter to the Handshake message
  • Adds a GetPeerList message
  • Periodically refreshes the validator bloom filter
  • Periodically sends a GetPeerList message

How this was tested

  • CI
  • Fuji
  • Additional unit tests
  • Local network with legacy peerlist gossip disabled

@StephenButtolph StephenButtolph added the networking This involves networking label Jan 3, 2024
@StephenButtolph StephenButtolph self-assigned this Jan 3, 2024
@StephenButtolph StephenButtolph changed the base branch from dev to optimize-bloom-filter January 5, 2024 20:50
@StephenButtolph StephenButtolph changed the base branch from dev to dynamic-staking-port-fix January 13, 2024 00:23
dhrubabasu
dhrubabasu reviewed Jan 13, 2024
View reviewed changes
proto/p2p/p2p.proto Outdated Show resolved Hide resolved
dhrubabasu
dhrubabasu reviewed Jan 13, 2024
View reviewed changes
network/README.md Outdated Show resolved Hide resolved
Base automatically changed from dynamic-staking-port-fix to dev January 13, 2024 22:41
@StephenButtolph StephenButtolph merged commit 864dbec into dev Jan 14, 2024
17 checks passed
@StephenButtolph StephenButtolph deleted the ip-pull-gossip branch January 14, 2024 07:03
marun pushed a commit that referenced this pull request Jan 17, 2024
@StephenButtolph @marun
Replace PeerListAck with GetPeerList (#2580)
35068ab
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dhrubabasu dhrubabasu dhrubabasu left review comments

@joshua-kim joshua-kim joshua-kim left review comments

@patrick-ogrady patrick-ogrady patrick-ogrady approved these changes

@gyuho gyuho Awaiting requested review from gyuho gyuho is a code owner

@danlaine danlaine Awaiting requested review from danlaine danlaine is a code owner

@abi87 abi87 Awaiting requested review from abi87

@marun marun Awaiting requested review from marun

Assignees

@StephenButtolph StephenButtolph

Labels
networking This involves networking
Projects
Platform Engineering Group
Archived in project
Milestone
v1.10.18
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@StephenButtolph @patrick-ogrady @dhrubabasu @joshua-kim