Skip to content

Remove peer sampling code #7768

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
Merged
mergify merged 4 commits into from
Jul 23, 2025
Merged

Remove peer sampling code #7768

mergify merged 4 commits into from
Jul 23, 2025

Conversation

@jimmygchen
Copy link
Member

@jimmygchen jimmygchen commented Jul 22, 2025

Issue Addressed

Peer sampling has been completely removed from the spec. This PR removes our partial implementation from the codebase.
ethereum/consensus-specs#4393

@jimmygchen jimmygchen changed the title Remove peer sampling code. Remove peer sampling code Jul 22, 2025
@jimmygchen jimmygchen added ready-for-review The code is ready for review code-quality das Data Availability Sampling labels Jul 22, 2025
@jimmygchen jimmygchen marked this pull request as ready for review July 22, 2025 04:30
@jimmygchen jimmygchen requested a review from jxs as a code owner July 22, 2025 04:30
pawanjay176
pawanjay176 approved these changes Jul 22, 2025
View reviewed changes
Copy link
Member

@pawanjay176 pawanjay176 left a comment

Choose a reason for hiding this comment

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

LGTM, love removing code.
Just want to run a sync test to make sure nothing depended on sampling

@jimmygchen
Copy link
Member Author

jimmygchen commented Jul 23, 2025

LGTM, love removing code. Just want to run a sync test to make sure nothing depended on sampling

Good idea, triggered one now

@jimmygchen
Copy link
Member Author

jimmygchen commented Jul 23, 2025

@pawanjay176 sync tests are working - except for Fulu backfill for superdnodes which still needs retry implemented in your PR. I'll go ahead and merge this one.

@jimmygchen jimmygchen added ready-for-merge This PR is ready to merge. and removed ready-for-review The code is ready for review labels Jul 23, 2025
@mergify
Copy link

mergify bot commented Jul 23, 2025

This pull request has been removed from the queue for the following reason: checks failed.

The merge conditions cannot be satisfied due to failing checks:

You may have to fix your CI before adding the pull request to the queue again.
If you update this pull request, to fix the CI, it will automatically be requeued once the queue conditions match again.
If you think this was a flaky issue instead, you can requeue the pull request, without updating it, by posting a @mergifyio requeue comment.

@mergify mergify bot merged commit 4daa015 into sigp:unstable Jul 23, 2025
46 of 49 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pawanjay176 pawanjay176 pawanjay176 approved these changes

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

Assignees

No one assigned

Labels

code-quality das Data Availability Sampling ready-for-merge This PR is ready to merge. syncing

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@jimmygchen @pawanjay176