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

Dev/test option for short BFT block periods #7588

Merged
merged 8 commits into from
Sep 20, 2024

Conversation

matthew1001
Copy link
Contributor

@matthew1001 matthew1001 commented Sep 9, 2024

PR description

This is a test-only option to mine BFT blocks more quickly than once a second.

It is not designed to be used in production and I've added warnings to the logs when it is set.

The reason for adding it is for Web3 frameworks wanting to run tests against a Besu node, possibly as part of a CI/CD pipeline, where lots of transactions & receipts need round-tripping during the test. 1 second block periods mean a test could take a very long time to complete.

I've tested it down to 50ms blocks and a single node is stable enough to mine transactions into blocks at that speed. I've also tested it in conjunction with draft PR #6965 with emptyblockperiodseconds set to 10s (so while the test isn't driving TXN traffic Besu isn't spinning creating huge numbers of empty blocks) and the two work fine together - see the following logs with 300ms blocks:

image

Signed-off-by: Matthew Whitehead <[email protected]>
Signed-off-by: Matthew Whitehead <[email protected]>
@matthew1001 matthew1001 marked this pull request as ready for review September 11, 2024 10:29
Signed-off-by: Matthew Whitehead <[email protected]>
@matthew1001 matthew1001 changed the title Dev mode for short BFT block periods Dev/test option for short BFT block periods Sep 11, 2024
@@ -45,7 +45,12 @@ public RoundTimer(
this.queue = queue;
this.bftExecutors = bftExecutors;
this.currentTimerTask = Optional.empty();
this.baseExpiryMillis = baseExpirySeconds * 1000;
Copy link
Contributor

Choose a reason for hiding this comment

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

If the baseExpiryMillis type was changed to a duration, then we could handle both milliseconds and seconds without any special logic

@matthew1001
Copy link
Contributor Author

Thanks for the comments @jframe, agree with you on all counts. I'll update the PR shortly

@matthew1001
Copy link
Contributor Author

matthew1001 commented Sep 18, 2024

I've done some refactoring to the PR as follows:

  • Removed the env var BESU_X_DEV_BFT_PERIOD_MS and created xblockperiodmilliseconds as a QBFT/IBFT2 genesis config option
  • If xblockperiodmilliseconds is set >0 then blockperiodseconds is ignored
  • Refactored several areas of block and round handling to pass Duration rather than seconds as an int

@matthew1001
Copy link
Contributor Author

Thanks for the comments @jframe See the latest commit & comment which hopefully resolves your comments

Signed-off-by: Matthew Whitehead <[email protected]>
@matthew1001 matthew1001 merged commit 19d3ca8 into hyperledger:main Sep 20, 2024
41 checks passed
Wolmin pushed a commit to lukso-network/network-besu that referenced this pull request Sep 27, 2024
* Dev mode for short BFT block periods

Signed-off-by: Matthew Whitehead <[email protected]>

* Refactoring

Signed-off-by: Matthew Whitehead <[email protected]>

* Fix comment

Signed-off-by: Matthew Whitehead <[email protected]>

* Refactor to make BFT block milliseconds an experimental QBFT config option

Signed-off-by: Matthew Whitehead <[email protected]>

* Update Json BFT config options

Signed-off-by: Matthew Whitehead <[email protected]>

---------

Signed-off-by: Matthew Whitehead <[email protected]>
Signed-off-by: Wolmin <[email protected]>
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.

2 participants