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.

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

eth/downloader: purge pre-merge sync code #29281

Merged
merged 5 commits into from
Apr 30, 2024

Conversation

jwasinger
Copy link
Contributor

@jwasinger jwasinger commented Mar 18, 2024

This PR removes pre-merge sync logic from the downloader. Now-irrelevant tests are removed and others have been updated. Fork tests are currently failing due to the issue identified/addressed in #29358

Copy link
Contributor

@holiman holiman left a comment

Choose a reason for hiding this comment

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

Looking good!

jwasinger

This comment was marked as duplicate.

func TestFailedSyncProgress68Snap(t *testing.T) { testFailedSyncProgress(t, eth.ETH68, SnapSync) }
func TestFailedSyncProgress68Light(t *testing.T) { testFailedSyncProgress(t, eth.ETH68, LightSync) }

func testFailedSyncProgress(t *testing.T, protocol uint, mode SyncMode) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please restore this too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure how to adapt/replace this test case. Because the head is trusted, a faulty chain/peer cannot cause the sync cycle to fail.

@fjl fjl removed the status:triage label Apr 9, 2024
@jwasinger jwasinger force-pushed the downloader-post-merge-purge branch 2 times, most recently from 9173271 to 218008b Compare April 23, 2024 02:15
@jwasinger
Copy link
Contributor Author

Whoops. There was one failing test that I had forgotten to fix. I've done that and rebased this. Should be okay to merge now.

@karalabe karalabe added this to the 1.14.1 milestone Apr 23, 2024
@jwasinger jwasinger force-pushed the downloader-post-merge-purge branch from 9a30230 to 1d6956a Compare April 30, 2024 12:33
@holiman holiman merged commit 45baf21 into ethereum:master Apr 30, 2024
2 of 3 checks passed
jorgemmsilva pushed a commit to iotaledger/go-ethereum that referenced this pull request Jun 17, 2024
This PR removes pre-merge sync logic from the downloader. Now-irrelevant tests are removed and others have been updated.
stwiname pushed a commit to subquery/data-node-go-ethereum that referenced this pull request Sep 9, 2024
This PR removes pre-merge sync logic from the downloader. Now-irrelevant tests are removed and others have been updated.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants