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 Consensys/quorum#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: #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 #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 #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 #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 created a Draft PR, please let me know what are the next steps...

#6944

Cheers.

[macfarla]

current PR #6965

[matthew1001]

Experimental support has been delivered under #6965

[MCrypto]

testing with release 24.10.0 and genesis configs:

    		"qbft": {
		      "blockperiodseconds": 5,
		      "emptyblockperiodseconds": 30,
		      "epochlength": 30000,
		      "requesttimeoutseconds": 3
		},

a block is generated every 5 seconds with no regard to the emptyblock time?!

I was testing on a local vm that I just fired up from scratch

[siladu]

@MCrypto It is an early access/experimental feature so the "x" is in front, i.e. xemptyblockperiodseconds.

We don't normally document early access features, but I think we should make an exception for this one as likely to be widely used

[MCrypto]

@MCrypto It is an early access/experimental feature so the "x" is in front, i.e. xemptyblockperiodseconds.

We don't normally document early access features, but I think we should make an exception for this one as likely to be widely used

Thank you so much, we have been waiting for this feature for our private blockchain implementation before we launch our mainnet! We are still on our testnet, so hopefully things will be stable enough by the time we come to our launch.

Thanks to everyone who worked on this!!

[matthew1001]

@MCrypto you might need requesttimeoutseconds to be >= xemptyblockperiodseconds as well