search

Consensys / quorum

A permissioned implementation of Ethereum supporting data privacy
https://www.goquorum.com/
GNU Lesser General Public License v3.0
4.68k stars 1.29k forks source link

istanbul: Consider using minblocktime and maxblocktime instead of the current blockperiod and blockpausetime #226

Closed stevenroose closed 2 years ago

stevenroose commented 6 years ago

Currently Istanbul BFT has a --blockperiod and a --blockpausetime parameter. The block period is the time interval at which to make blocks. A proposer will wait for this amount of time, regardless of the amount of transactions it gets. After the block period, it bundles them and emits the block. Whenever one of those blocks has 0 transactions, all validators (including the next proposer) are expectign the next block to only appear after the block pause time. They effectively jointly take a break triggered by an empty block.

The problem is when a transaction arrives within this "pause". The proposer ignores the transaction until the pause is over. Only after the pause, it will make a new block and emit it. This effectively makes the block pause time useless (it delays transactions) and requires a very low block period. Moreover, the interpretation of those values is quite ambiguous.

An alternative would be to have two other variables: --minblocktime and --maxblocktime. They can be interpreted pretty literally, which makes them easy to understand.

After receiving a block, validators will not accept any new block within the minblocktime. Only after that time, the proposer will create a new block and emit it. Except when it has no transactions; in that case, it will wait. When it receives a transaction while waiting, it should make a block immediately and emit it. If it does not receive any transactions before maxblocktime, it creates an empty block.

In this scenario, the validators that are not the proposer will only elect a new proposer when the proposer does not send a block before maxblocktime.

I think the old istanbul.requesttimeout value can thus be dropped in favor of maxblocktime as well.

kubasiemion commented 6 years ago

Yes exactly. The "stop the world" delay in engine.go Seal() is a bit heavy-handed. e.g. the select{} block there could unblock on new transactions in txpool...

joelburget commented 6 years ago

@yutelin @markya0616 thoughts?

yutelin commented 6 years ago

@joelburget Yeah, I think this is a nice feature to have. Would love to gather more opinions from the community though. What's your own thoughts @joelburget ?

ricardolyn commented 3 years ago

@hmoniz from your perspective, should this be closed?

hmoniz commented 3 years ago

The underlying issue is relevant. There's no fundamental reason to have a wait time between blocks. That wait time is simply a counterproductive artifact from proof-of-work. The solution, however, should be removing this wait time altogether and "mine" blocks on-demand: as long as there are transactions in the pool, a proposer generates a block and initiates a consensus instance to decide on that block; if there are no transactions in the pool, then there's no need to execute any consensus instance.

baptiste-b-pegasys commented 2 years ago

we are adding feature around this for QBFT where you will be able to mine empty blocks less frequently, the implementation is different of this current suggestion, however the result is similar. https://github.com/ConsenSys/quorum/pull/1433