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

cmd/devp2p/internal/ethtest: remove TD from status validation #31137

Merged
merged 1 commit into from
Feb 7, 2025

Conversation

marcindsobczak
Copy link
Contributor

After recent changes in Geth (removing TD):
39638c8#diff-d70a44d4b7a0e84fe9dcca25d368f626ae6c9bc0b8fe9690074ba92d298bcc0d

Non-Geth clients are failing many devp2p tests with an error:
peering failed: status exchange failed: wrong TD in status: have 1 want 0

Right now only Geth is passing it - all other clients are affected by this change.
I think there should be no validation of TD when checking Status message in hive tests. Now Geth has 0 (and hive tests requires 0) and all other clients have actual TD. And on real networks there is no validation of TD when peering

@fjl fjl changed the title ethtest: remove TD from status validation cmd/devp2p/internal/ethtest: remove TD from status validation Feb 6, 2025
@fjl fjl added this to the 1.15.1 milestone Feb 6, 2025
@LukaszRozmej
Copy link

Maybe there should be validation that this field exists, but not it's value?

@rjl493456442
Copy link
Member

@LukaszRozmej RLP-decoding will fail if this field is not present.

@rjl493456442 rjl493456442 merged commit d11e9c0 into ethereum:master Feb 7, 2025
4 checks passed
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.

4 participants