QBFT no empty block

[jframe]

Background and specification document: https://docs.google.com/document/d/1UjefKBgnFnBqQGYZSfVr7LZxVOgrDjeO0uZVHtKSRQA/edit?usp=sharing

The QBFT consensus protocol creates new blocks on a regular basis with the time interval between blocks being a configurable parameter. This is also true in the case that none of the validating nodes has received any transaction not already included in the blockchain. Under these circumstances, since no new transaction is being received, the QBFT protocol ends up producing a continuous sequence of blocks void of any transaction, i.e. empty blocks.

In cases where the transaction volume is quite low, this is less than ideal as empty blocks end up occupying a lot of the node storage space compared to the space occupied by actual transactions.

The approach will be need to be co-ordinated with the implementation in GoQuoum as well so that the protocol remains compatible.

Tasks (TODO)

• Make empty period configurable?
• Add emptyBlockTimer logic in QbftBlockHeightManager.handleBlockTimerExpiry?
  • Decided to add a check after the block is (potentially wastefully) created block.getBody().getTransactions().isEmpty() instead of attempting to query the txpool directly.
  • Will probably need to do something in handleRoundExpiry as well as blockTimerExpiry.
• May not need to update validation rules?
• PEEPS ~* Timestamp-dependent smart contract test?~
• Testnet?
• Next quarterly release?

[antonydenyer]

GoQuorum implementation https://github.com/ConsenSys/quorum/pull/1433

[sammyp42]

Any update on when this might get implemented in Besu?

[owyah]

Please solve this one, it would help many people.

[yokawaiik]

It's a really necessary feature and it will helps to save memory!

[siladu]

WIP: https://github.com/hyperledger/besu/pull/4634

[non-fungible-nelson]

@siladu - status on PR ?

[siladu]

@siladu - status on PR ?

@non-fungible-nelson We had to pause it to prioritise Withdrawals/Shanghai

[thiagodeev]

@siladu Is there any prevision about that?

[matthew1001]

Discussed with @siladu and I'm going to work on progressing the existing PR to see how far this is from being closeable.

[MCrypto]

are there any updates on this as it's now implemented for Clique https://github.com/hyperledger/besu/pull/6082 ?

[matthew1001]

I've not moved it along much as I've been focusing on other bug fixes & features that have taken priority. Working on https://github.com/hyperledger/besu/issues/5446 has helped get me into the BFT code more than I have before, so this might be a natural thing for me to pick up again once that PR is over the line but happy if someone else wants to pick up the empty block work in the mean time.

[matthew1001]

For now I'll remove my name from it, but will add it back once I'm actively working on it again.

[amsmota]

Hi @matthew1001, I actually spent some time last weekend looking at some relevant parts of the code, in a discussion on the besu channel I said l may be able to help but then I didn't knew this issue was already created. So if I can help in any way I'll have some time to do it both off and on my working hours... 😃

[matthew1001]

I haven't spent time on this feature recently so if you're interested in contributing then that would be great. You could start by looking at https://github.com/hyperledger/besu/pull/4634 which was a draft, WIP PR by @siladu. I'm not expecting to have time to work on it in the near future.

[amsmota]

Hi, yes I downloaded @siladu code, unfortunately I have to work with it on a very low end personal laptop but I'll do my best 😃.

Thank you guys...

[amsmota]

Guys, I do now have an initial working version (untested and very basic) that I plan to put on a draft PR for review ASAIC, I'm actually eager to do it but even if I started doing this personally now I learned that my employer is actually a Hyperleger Foundation contributor, so now I'm doing it under their umbrella. That means I have to wait for their approvals which I expect will be early next week... 😀

It's based on the previous work by @siladu, so my kudos to him...

[siladu]

@amsmota I've updated the description with some more background details and the specification of the proposed solution. This is what both my implementation and the GoQuorum one are based on.

Note, this could be implemented for IBFT2 as well, but for now I would recommend only attempting to update QBFT. If someone wants to extend this to IBFT2 they can do that themselves or make a case for it. The scope of this issue was only intended to be QBFT and better to keep the changes as small as possible.

[amsmota]

Hi, thanks for that. I'm still waiting for the authorization of my firm to be able to push what I have, it was working quite fine untilnow that I started to implement the support for transitions. It's quite complex because the blockTimer is started before the block is created so by that time the timer doesn't know if the block is empty or not. Yesterday I think I achieve that but couldn't test it yet. But I had to comment out the TimestampMoreRecentThanParent because of that too.

Just to clarify the emptyBlockPeriodSeconds is already implemented and working in the Fork and Options, including the Mutable ones, it's just that for now I can't test the empty/non-empty scenarios but because of code unrelated issues...

I'll let you know how it goes...

[amsmota]

Well, I think this is almost done, I tested today the blockperiod/emptyblockperiod before and after a transition and it looks fine, but my tests were not very reliable due to the environment I'm using. I did quite a few changes, the most noticeable in the BlockTimer, I kind of moved the responsability of calculating those values from the QbftBlockHeightManager to it.

Anyway I got the approval from my company so I'll be able to create PR for review soon...

Cheers.

[amsmota]

Guys, I just pushed my changes to our internal fork of Besu, so one last approval and I'll be able to push it to the main repo. One word of caution, I can't guarantee everything is OK, I had to do all the development on top of 23.10.2 due to several restrictions and copy&paste the changes to the main branch as of today. In a few hours I will be able to build and test this version.

The better part of the code was based on @siladu code, I've tried to limit my changes to the minimum possible and by adding code, not rewrite existing one, wherever possible. One big exception was commenting out the TimestampMoreRecentThanParent validation rule. The core functionality was done on QbftBlockHeightManager and the BlockTimer. There is some code duplication there, it can be easily changed but I didn't want to change the previous implementation and it actually looks more clear that way.

And this is still a DRAFT version for review, I still have to do the unit tests and the like...

I hope I can push as soon as possible, but as a overview below are the classes the were changed. Note that whenever possible I changed the Bft classses, meaning it can probably be used by the Ibft code as well.

- besu/src/main/java/org/hyperledger/besu/controller/QbftBesuControllerBuilder.java
- config/src/main/java/org/hyperledger/besu/config/BftConfigOptions.java
- config/src/main/java/org/hyperledger/besu/config/BftFork.java
- config/src/main/java/org/hyperledger/besu/config/JsonBftConfigOptions.java
- consensus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/BlockTimer.java
- consensus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/MutableBftConfigOptions.java
- consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/QbftBlockHeaderValidationRulesetFactory.ja
- consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/QbftForksSchedulesFactory.java
- consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/statemachine/QbftBlockHeightManager.java
- consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/statemachine/QbftRound.java

Any question or suggestion please let me know.

Cheers.

[non-fungible-nelson]

Could you open a PR on the main repo so we can more easily review your code and test it against our test suite?

[amsmota]

Hi, sorry for the delay, I'm still waiting for my manager's approval, which will be no later than this Friday. I'll do it right after it.

Cheers.

[MASDXI]

Hi, sorry for the delay, I'm still waiting for my manager's approval, which will be no later than this Friday. I'll do it right after it.

Cheers.

Any update, I have looked into this issue for a while.

[amsmota]

I'm still waiting for my manager approval on our own PR, there was a internal peer-review that is done by now, I hope it'll be approved soon so I can do the PR to the Hyperledger Besu for review too... I'll post here as soon as I do it...

On Tue, Mar 19, 2024, 02:57 MASDXI @.***> wrote:

Hi, sorry for the delay, I'm still waiting for my manager's approval, which will be no later than this Friday. I'll do it right after it.

Cheers.

Any update, I have looked into this issue for a while.

— Reply to this email directly, view it on GitHub https://github.com/hyperledger/besu/issues/3810#issuecomment-2005655463, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB3UF3LPKGKCO2RCP5CLSGLYY6SQHAVCNFSM5VPXNU2KU5DIOJSWCZC7NNSXTN2JONZXKZKDN5WW2ZLOOQ5TEMBQGU3DKNJUGYZQ . You are receiving this because you were mentioned.Message ID: @.***>

[amsmota]

Guys, I'm completely blocked here, I need to run Besu with my code thru a Jenkins pipeline and all the checks it contains, this is a mandatory step for me. The build runs fine on my local computer, but in a java-gradle-build in the pipeline it ends with this strange error

FAILURE: Build failed with an exception.

• What went wrong: Gradle build daemon disappeared unexpectedly (it may have been killed or may have crashed)
• Exception is: org.gradle.launcher.daemon.client.DaemonDisappearedException: Gradle build daemon disappeared unexpectedly (it may have been killed or may have crashed)

Googling for it almost all replies say it has to do with memory available, I did tweaked that and other gradle setting (daemon on/off, parallel on/of) but the best I got was instead of havin3 of those FAILURE errors I'm now getting only one.

Anybody have a clue of what's happening here? My last config was

org.gradle.parallel=false 
org.gradle.daemon=true 
org.gradle.caching=true 
org.gradle.jvmargs=-Xmx8G -Xms4G -XX:MaxMetaspaceSize=2048m -Dfile.encoding=UTF-8

Thanks all.

[MASDXI]

Guys, I'm completely blocked here, I need to run Besu with my code thru a Jenkins pipeline and all the checks it contains, this is a mandatory step for me. The build runs fine on my local computer, but in a java-gradle-build in the pipeline it ends with this strange error

FAILURE: Build failed with an exception.

• What went wrong: Gradle build daemon disappeared unexpectedly (it may have been killed or may have crashed)
• Exception is: org.gradle.launcher.daemon.client.DaemonDisappearedException: Gradle build daemon disappeared unexpectedly (it may have been killed or may have crashed)

Googling for it almost all replies say it has to do with memory available, I did tweaked that and other gradle setting (daemon on/off, parallel on/of) but the best I got was instead of havin3 of those FAILURE errors I'm now getting only one.

Anybody have a clue of what's happening here? My last config was

org.gradle.parallel=false 
org.gradle.daemon=true 
org.gradle.caching=true 
org.gradle.jvmargs=-Xmx8G -Xms4G -XX:MaxMetaspaceSize=2048m -Dfile.encoding=UTF-8

Thanks all.

Have you tired set caching to false?

[amsmota]

I think I did but just in case I'll try again...

Cheers.

Have you tired set caching to false?

[amsmota]

I think I did but just in case I'll try again...

Cheers.

Have you tired set caching to false?

Same thing...

[macfarla]

@MASDXI are you still having this issue?

[amsmota]

@MASDXI are you still having this issue?

I do, even if in another place in the flow... Anyway, this is not a blocker anymore, I expect that later today or tomorrow I can do a merge in our own github fork of Besu, and after some tests I'll create the draft PR for review. I'm planning to work over the weekend if needed.

Cheers.