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

feat(consensus): introduce predictable block times #2280

Conversation

melekes
Copy link

@melekes melekes commented Dec 17, 2024

based on #2221

Assumptions

  • PBTS is used, which can't be attacked by 1/3+ of malicious validators (time warp attack).
  • InitialTime (genesis time) is persisted across restarts to preserve the desired block cadence.

Algorithm

The algorithm is as follows (assuming the target block cadence (configurable) is 2s):

targetBlockTime = InitialTime + 2s * blockHeight

if actualBlockTime >= targetBlockTime {
  ABCI.FinalizeBlockResponse.NextBlockDelay = 0
} else {
  ABCI.FinalizeBlockResponse.NextBlockDelay = targetBlockTime - actualBlockTime
}

Questions

  1. Can I use CommitMultiStore to store InitialTime? CometBFT only submits genesis time if the app height is 0. If it's greater than 0, we must restore it from somewhere between restarts.
  2. Are we okay with exposing a CLI flag to control block time? Could it be useful for testing?
  3. Do we need the ability to change the desired block cadence (e.g, 2s)?

Uses the latest `FinalizeBlockResponse#NextBlockDelay` ABCI field to
control block times. The algorithm employs the genesis time, which it
gets from `InitChainRequest`, to calculate the target block time for N:

```
targetBlockTime := genesisTime + target block time * N block height
if N block time >= targetBlockTime {
  delay := 0
} else {
  delay :=  targetBlockTime - N block time
}
```
@calbera
Copy link
Contributor

calbera commented Dec 17, 2024

Followup to questions:

  1. Don't think it hurts, generally safer to have it stored on disk vs in memory.
  2. I think it would be helpful to be configurable for testing.
  3. At some point we'd have to enforce this in the binary so all nodes use the same value?

@abi87 abi87 deleted the branch berachain:replace-berachain-cosmos-sdk-point-local-repo-2 December 20, 2024 10:21
@abi87 abi87 closed this Dec 20, 2024
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.

3 participants