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

Implement eth/67 sub protocol (EIP-4938) #4646

Merged

Conversation

Gabriel-Trintinalia
Copy link
Contributor

@Gabriel-Trintinalia Gabriel-Trintinalia commented Nov 10, 2022

Signed-off-by: Gabriel Trintinalia gabriel.trintinalia@gmail.com

PR description

eth/67 (EIP-4938, March 2022)
Version 67 removed the GetNodeData and NodeData messages.

Besu:

  • Supports multiple versions of a wire protocol.
  • Does not break older clients, since they can keep using protocol version eth/66.
  • For fast sync, eth/66 will be used

Fixed Issue(s)

fixes #4596

Documentation

  • I thought about documentation and added the doc-change-required label to this PR if
    updates are required.

Changelog

Signed-off-by: Gabriel Trintinalia <gabriel.trintinalia@gmail.com>
Signed-off-by: Gabriel Trintinalia <gabriel.trintinalia@gmail.com>
Signed-off-by: Gabriel Trintinalia <gabriel.trintinalia@gmail.com>
… mocked

Signed-off-by: Gabriel Trintinalia <gabriel.trintinalia@gmail.com>
Signed-off-by: Gabriel Trintinalia <gabriel.trintinalia@gmail.com>
@Gabriel-Trintinalia Gabriel-Trintinalia added the TeamRevenant GH issues worked on by Revenant Team label Nov 14, 2022
Signed-off-by: Gabriel Trintinalia <gabriel.trintinalia@gmail.com>
Signed-off-by: Gabriel Trintinalia <gabriel.trintinalia@gmail.com>
@Gabriel-Trintinalia Gabriel-Trintinalia changed the title WIP: Implement eth/67 sub protocol Implement eth/67 sub protocol (EIP-4938) Nov 15, 2022
Signed-off-by: Gabriel Trintinalia <gabriel.trintinalia@gmail.com>
@Gabriel-Trintinalia Gabriel-Trintinalia marked this pull request as ready for review November 15, 2022 03:24
Copy link
Contributor

@macfarla macfarla left a comment

Choose a reason for hiding this comment

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

looks nice and simple! Did you test it out on a node running fast sync?

// Fast sync depends on GetNodeData and NodeData
// Do not add eth/67 if fast sync is enabled
// see https://eips.ethereum.org/EIPS/eip-4938
if (!Objects.equals(SyncMode.FAST, synchronizerConfiguration.getSyncMode())) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Curious to know why you are using Object.equals instead of just doing SyncMode.FAST == synchronizerConfiguration.getSyncMode()

Signed-off-by: Gabriel-Trintinalia <gabriel.trintinalia@gmail.com>
@Gabriel-Trintinalia Gabriel-Trintinalia merged commit 7c35be3 into hyperledger:main Nov 17, 2022
gfukushima added a commit to gfukushima/besu that referenced this pull request Nov 29, 2022
@Gabriel-Trintinalia Gabriel-Trintinalia deleted the 4596-eth67-protocol branch December 12, 2022 02:24
macfarla pushed a commit to jflo/besu that referenced this pull request Jan 10, 2023
Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>
eum602 pushed a commit to lacchain/besu that referenced this pull request Nov 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mainnet TeamRevenant GH issues worked on by Revenant Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement eth/67 sub protocol (EIP-4938)
4 participants