Skip to content

clients/erigon: change p2p version priorities to fix several Erigon devp2p tests #776

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

Closed

Conversation

cryptoscruffy
Copy link

Use 68 version as default. This fixes 5 failed devp2p tests for Erigon

@holiman holiman changed the title Change p2p versions priorities to fix several Erigon devp2p tests clients/erigon: change p2p version priorities to fix several Erigon devp2p tests May 5, 2023
@holiman
Copy link
Collaborator

holiman commented May 5, 2023

Hm... so is the end-user supposed to know this, and set those flags explicitly?

@fjl
Copy link
Collaborator

fjl commented May 5, 2023

Yeah, I think the same thing, why are these flags not default? cc. @yperbasis

@cryptoscruffy
Copy link
Author

cryptoscruffy commented May 5, 2023

As far as I know end users use --p2p.protocol setting often enough, but I don't sure it is a correct way to resolve the issue. I've tried to copy accepted #766 PR, reject my PR please if you think it is wrong approach.

@fjl
Copy link
Collaborator

fjl commented May 5, 2023

@cryptoscruffy Your PR is totally fine, we are just wondering in general why erigon requires this flag to activate the protocol versions. We are not expecting an answer from you about this.

@cryptoscruffy
Copy link
Author

Looks like 67 version is default now just because it was not updated in Erigon, so the correct way to fix this issue is to update Erigon's defaults. I'll try to make PR to Erigon's repo instead of this one.

yperbasis added a commit to erigontech/erigon that referenced this pull request May 8, 2023
Set `DefaultConfig.ProtocolVersion` to [68, 67, 66] instead of [67, 68].
See ethereum/hive#776
racytech pushed a commit to racytech/hive that referenced this pull request Apr 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants