8000 Default ExtraData to "besu vM.m.p" rather than empty by benaadams · Pull Request #8536 · hyperledger/besu · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Default ExtraData to "besu vM.m.p" rather than empty #8536

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
merged 8 commits into from
Apr 11, 2025
Merged

Default ExtraData to "besu vM.m.p" rather than empty #8536

merged 8 commits into from
Apr 11, 2025
+143 −68

Conversation

benaadams
Copy link
Contributor

PR description

Default ExtraData to "besu vM.m.p" rather than empty e.g. "besu v25.4.0"

This is done similarly by Geth, Nethermind and reth; and will be in latest version of Erigon erigontech/erigon#14419

This helps if are any built block issues to know if is old version (with known fixed issues) or new version

@benaadams benaadams force-pushed the default-extra-data branch from 2ab7341 to a259acd Compare April 10, 2025 00:06
fab-10
fab-10 reviewed Apr 10, 2025
View reviewed changes
Copy link
Contributor
@fab-10 fab-10 left a comment

Choose a reason for hiding this comment

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

LGTM, please add a CHANGELOG entry

@benaadams
Default ExtraData to 'besu vM.m.p' rather than empty
93417f7 
Signed-off-by: Ben Adams <thundercat@illyriad.co.uk>
@benaadams
Fix + changelog
f823050 
Signed-off-by: Ben Adams <thundercat@illyriad.co.uk>
@benaadams benaadams force-pushed the default-extra-data branch from efc86d6 to f823050 Compare April 10, 2025 13:22
@benaadams
Copy link
Contributor Author

Added changelog, fixed compilation error

@benaadams benaadams requested a review from fab-10 April 10, 2025 13:23
@benaadams
Copy link
Contributor Author

Nethermind is also adding version to default ExtraData (is currently just Nethermind) NethermindEth/nethermind#8492

fab-10 and others added 2 commits April 10, 2025 20:09
@fab-10
Refactor to use BesuVersionUtils everywhere version info are required
1b30de8 
Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>

# Conflicts:
#	ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/MiningConfiguration.java
@benaadams
Merge pull request #1 from fab-10/besu-version-4-extra-data
b71ff14 
Refactor to use BesuVersionUtils everywhere version info are required
@benaadams
Copy link
Contributor Author

Updated with changes

@fab-10 fab-10 enabled auto-merge (squash) April 10, 2025 18:16
@benaadams
Copy link
Contributor Author

Oh, yea 🫠

So in the Nethermind PR we did it so the main app triggered the version add (or in this case could be non-emptiness); because the ExtraData is included in the blockhash so as not to either rekt all the tests or have to update them every version.

Wouldn't know how to go about that in besu

@benaadams
Copy link
Contributor Author

Wouldn't know how to go about that in besu

And how it would interplay with config overrides

@fab-10
Copy link
Contributor
fab-10 commented Apr 10, 2025

There are some other tests failing because they expected any empty extra data field, I can take care of them tomorrow

macfarla
macfarla approved these changes Apr 11, 2025
View reviewed changes
Copy link
Contributor
@macfarla macfarla left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM but still some test failures to fix

@fab-10
Update and fix tests
0d1b91b 
Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
@fab-10
Copy link
Contributor
fab-10 commented Apr 11, 2025

PR benaadams#2 that should fix the failing tests

auto-merge was automatically disabled April 11, 2025 16:40

Head branch was pushed to by a user without write access

@benaadams
Copy link
Contributor Author

PR benaadams#2 that should fix the failing tests

Merged

@benaadams
Merge branch 'main' into default-extra-data
3a2a322 
Signed-off-by: Ben {chmark} Adams <thundercat@illyriad.co.uk>
@fab-10 fab-10 enabled auto-merge (squash) April 11, 2025 16:42
@fab-10 fab-10 merged commit d611483 into hyperledger:main Apr 11, 2025
43 checks passed
@benaadams benaadams deleted the default-extra-data branch April 11, 2025 18:46
@fab-10 fab-10 mentioned this pull request Apr 16, 2025
Merged
9 tasks
@benaadams
Copy link
Contributor Author

Seen in the wild

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fab-10 fab-10 fab-10 left review comments

@macfarla macfarla macfarla approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@benaadams @fab-10 @macfarla
0