Skip to content

Conversation

@dhvogel
Copy link
Contributor

@dhvogel dhvogel commented Oct 4, 2025

PR description

This PR fixes #9270, allowing for customizable ethstats report intervals, which are needed by other blockchains like Linea to keep pace with faster block times than Ethereum Mainnet (see #9269).

Fixed Issue(s)

Fixes #9270

Thanks for sending a pull request! Have you done the following?

  • Checked out our contribution guidelines?
  • Considered documentation and added the doc-change-required label to this PR if updates are required. (I think probably not, because the CLI contains documentation, but let me know if otherwise)
  • Considered the changelog and included an update if required.
  • For database changes (e.g. KeyValueSegmentIdentifier) considered compatibility and performed forwards and backwards compatibility tests

Locally, you can run these tests to catch failures early:

  • spotless: ./gradlew spotlessApply
  • unit tests: ./gradlew build
  • acceptance tests: ./gradlew acceptanceTest
  • integration tests: ./gradlew integrationTest
  • reference tests: ./gradlew ethereum:referenceTests:referenceTests
  • hive tests: Engine or other RPCs modified?

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 good, just a suggestion re type of CLI param - you'll see elsewhere in the codebase Boxed types are used rather than primitives for CLI options

@dhvogel
Copy link
Contributor Author

dhvogel commented Oct 6, 2025

Ok, I've made the change from primitive to boxed type.

@macfarla macfarla assigned macfarla and unassigned dhvogel Oct 7, 2025
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.

spotless check is failing

@dhvogel
Copy link
Contributor Author

dhvogel commented Oct 7, 2025

I ran ./gradlew :app:spotlessApply and added the changes. Sorry about that.

@macfarla
Copy link
Contributor

macfarla commented Oct 7, 2025

@dhvogel spotless is still failing

@macfarla macfarla marked this pull request as draft October 7, 2025 01:21
@macfarla macfarla assigned dhvogel and unassigned macfarla Oct 7, 2025
@dhvogel
Copy link
Contributor Author

dhvogel commented Oct 7, 2025

./gradlew spotlessApply caught some more formatting errors in addition to ./gradlew :app:spotlessApply

./gradlew :ethereum:ethstats:spotlessJavaCheck now passing

@macfarla macfarla assigned macfarla and unassigned dhvogel Oct 7, 2025
@dhvogel dhvogel marked this pull request as ready for review October 7, 2025 14:37
@dhvogel dhvogel marked this pull request as draft October 7, 2025 20:00
@dhvogel
Copy link
Contributor Author

dhvogel commented Oct 7, 2025

running ./gradlew integrationTest has two failures, QbftContractAcceptanceTest > shouldMineOnMultipleNodesEvenWhenClusterContainsNonValidator() and BftMiningAcceptanceTest_Part3 > shouldStillMineWhenANonProposerNodeFailsAndHasSufficientValidators. I don't know much about these tests but they seem like timing errors that are unrelated to the change

@dhvogel dhvogel force-pushed the main branch 2 times, most recently from 834002d to f01699a Compare October 7, 2025 21:24
Signed-off-by: Dan Vogel <dhvogel2468@gmail.com>
@dhvogel dhvogel marked this pull request as ready for review October 7, 2025 21:27
@macfarla macfarla merged commit 826585b into hyperledger:main Oct 8, 2025
55 of 65 checks passed
@macfarla macfarla added the doc-change-required Indicates an issue or PR that requires doc to be updated label Oct 8, 2025
jflo pushed a commit to jflo/besu that referenced this pull request Oct 13, 2025
Signed-off-by: Dan Vogel <dhvogel2468@gmail.com>
Signed-off-by: jflo <justin+github@florentine.us>
@alexandratran alexandratran removed the doc-change-required Indicates an issue or PR that requires doc to be updated label Oct 15, 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.

Add CLI option to customise ethstats reporting interval

4 participants