consensus: proposer-based timestamps (PBTS) #1731
Labels
consensus
enhancement
New feature or request
major-priority
A major, long-running priority for the team
pbts
tracking
A complex issue broken down into sub-problems
Milestone
Comments
|
I think the first step here is to migrate the new PBTS spec to |
We are not fully deprecating BFT Time, so I think this description must be preserved. Maybe updated, maybe merged with PBTS as the two ways of producing timestamps. |
3 tasks
cason
added a commit
that referenced
this issue
Jan 24, 2024
Part of #1731. The original PR precedes tendermint/tendermint#8488, a PR that adopted `metricsgen` for metric generation. This PR was forward ported to this repository as in tendermint/tendermint#9178. Some changes were added to the forward-ported PR to match the behaviour and description of the added metric with this follow-up change. --- #### PR checklist - [ ] Tests written/updated - [ ] Changelog entry added in `.changelog` (we use [unclog](https://github.com/informalsystems/unclog) to manage our changelog) - [ ] Updated relevant documentation (`docs/` or `spec/`) and code comments --------- Co-authored-by: William Banfield <4561443+williambanfield@users.noreply.github.com> Co-authored-by: Sergio Mena <sergio@informal.systems>
3 tasks
This was referenced Jan 25, 2024
cason
added a commit
that referenced
this issue
Jan 31, 2024
Part of #1731. Forward ported changes from PRs: - tendermint/tendermint#8058 - tendermint/tendermint#8129 - tendermint/tendermint#8454 Plus, updated metrics list on `docs/core/metrics.md`, with PBTS and also other added metrics. --- #### PR checklist - [ ] ~Tests written/updated~ - [ ] ~Changelog entry added in `.changelog` (we use [unclog](https://github.com/informalsystems/unclog) to manage our changelog)~ - [x] Updated relevant documentation (`docs/` or `spec/`) and code comments - [x] Title follows the [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec --------- Co-authored-by: William Banfield <4561443+williambanfield@users.noreply.github.com>
4 tasks
cason
added a commit
that referenced
this issue
Feb 12, 2024
ADR 071, from Tendermint Core repository was renamed to ADR 112 and updated to match the current implementation of PBTS ( #1731). Rendered [ADR 112](https://github.com/cometbft/cometbft/blob/cason/pbts-adr/docs/references/architecture/adr-112-proposer-based-timestamps.md). --- Since the file was copied, renamed, then adapted, Git lost track of the actual changes, that can be manually checked using: git diff cason/pbts-adr:docs/architecture/adr-112-proposer-based-timestamps.md feature/pbts:docs/architecture/tendermint-core/adr-071-proposer-based-timestamps.md **Important:** there are missing parts in this ADR, that should be addressed before merging it. It is open for discussion, as it is expected for an ADR. --- #### PR checklist - [ ] Tests written/updated - [ ] Changelog entry added in `.changelog` (we use [unclog](https://github.com/informalsystems/unclog) to manage our changelog) - [x] Updated relevant documentation (`docs/` or `spec/`) and code comments - [x] Title follows the [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec --------- Co-authored-by: Sergio Mena <sergio@informal.systems> Co-authored-by: lasaro <lasaro@informal.systems>
4 tasks
4 tasks
cason
added a commit
that referenced
this issue
Mar 1, 2024
…2489) Contributes to #1731 --- #### PR checklist - [x] Tests written/updated - [ ] Changelog entry added in `.changelog` (we use [unclog](https://github.com/informalsystems/unclog) to manage our changelog) - [ ] Updated relevant documentation (`docs/` or `spec/`) and code comments - [x] Title follows the [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec --------- Co-authored-by: lasaro <lasaro@informal.systems>
This was referenced Mar 1, 2024
sergio-mena
added a commit
that referenced
this issue
Mar 2, 2024
Contributes to #1731 After some troubleshooting, I realized that, for two `// nolint` added at the beginning of files in #2494, the `gofumpt` and the `golangci-lint-full` have contradictory rules. So the first one removes the space between `//` and `nolint` and the second adds it back 🤯. The problem has been to _find_ this, as, at the end no files where modified whereas both linters were saying they were modifying some files. To work around this problem, I changed the way to fix the original lint issues. --- #### PR checklist - [ ] Tests written/updated - [ ] Changelog entry added in `.changelog` (we use [unclog](https://github.com/informalsystems/unclog) to manage our changelog) - [ ] Updated relevant documentation (`docs/` or `spec/`) and code comments - [ ] Title follows the [Convent 6D47 ional Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec
|
There are two action points, but referring to the analysis of the impact of Byzantine nodes in the produced timestamps, that were removed from this tracking issue and will be addressed, on demand, as backlog items: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
Feature Request
Summary
Use proposer-based timestamps instead of calculating the median time of vote timestamps.
Detailed description of previous efforts
Context
This collapsed section describes the previous work on the adoption of proposer-based timestamps (PBTS) for the blocks produced in CometBFT networks and points out relevant aspects to be considered when porting this work to the current code base.
Part of the references included in this section are linked in the PBTS project board in the old Tendermint Core repository: Proposer-Based Timestamps Initial Implementation (view).
Rationale
The adoption of proposer-based timestamps (PBTS) was originally proposed in 2018 (tendermint/tendermint#2840, tendermint/tendermint#1615) to replace the BFT Time algorithm (tendermint/tendermint#2013) that is currently used to compute the timestamp of new blocks. In short, the BFT Time algorithm sets as timestamp of a block (stored in its
Timefield) the voting-power-weighted median of thePrecommitvote messages (stored at the block'sLastCommitfield) that decided the previous block.The main problems with BFT Time are the following:
The consensus algorithm adopted by CometBFT needs to be adapted for the use of proposer-based timestamps. Throughout 2021, two proposals of changes in the consensus algorithm were specified (tendermint/spec#261 and tendermint/spec#369). In addition, the architectural changes required for the adoption of PBTS were discussed in ADR 071, originally built from the first proposal (tendermint/tendermint#6799), then adapted for the second proposal (tendermint/tendermint#7764).
Spec
At the moment, the PBTS spec on
mainrefers to the first proposal of changes in the consensus algorithm. The second and latest proposal of changes is currently available onmaster(tendermint/tendermint#8096).There is some pending work, especially regarding the formal verification of the spec using TLA+ that needs to be retrieved and added to the spec (maybe ported to Quint?), as described in tendermint/tendermint#8953. In particular, the
main/pbtsbranch referred on that issue is not available anymore. A possible source for this content is PR tendermint/tendermint#8600 and the involved authors.Implementation
The second and latest version of the PBTS specification was implemented on
master. This is the branch from which the release branchesv0.35.xandv0.36.xwere cut and later retracted. As a result, porting the PBTS implementation tomain, which includes all changes introduced for releasesv0.37.xandv0.38.xmay not be a trivial task.The tracking issue for the PBTS implementation in
masteris tendermint/tendermint#6942.The implementation has some changes to the consensus protocol as prerequisites, not necessarily related to PBTS:
Moreover, the validation of block timestamps on a node in recovery mode is a missing aspect of the implementation:
Block Validation in Full Nodes
A further implementation aspect, not discussed in this previous effort, is the backwards compatibility in full nodes. The PBTS implementation has removed the logic adopted by BFT Time algorithm for producing and validating block timestamps (tendermint/tendermint#7382, tendermint/tendermint#7563). We need to adapt blocksync to accept both mechanisms for producing block timestamps.
Documentation and configuration
The adoption of PBTS requires the configuration of synchronous parameters (clock precision and end-to-end delay for proposal messages) employed to determine whether the timestamp of a block is valid (time-validity). The adoption of too restrictive or tight synchronous parameters may hinder liveness, as blocks produced by honest validators could be systematically rejected, and impact performance. For this reason, the introduction of PBTS should be accompanied by a comprehensive documentation of how to choose the synchronous parameters and how to diagnose and solve PBTS-associated issues:
It is worth mentioning that the time-validity of a proposed block is only verified for a limited number of rounds, currently hard-coded to 10 (tendermint/tendermint#7739). This decision was taken in order to ensure consensus liveness under the adoption of inappropriate synchronous parameters. It is worth considering having this limit configurable.
Another possibility to consider is to allow users or the application to disable at all the time-validity checks of a block. This possibility is mentioned in the original issue proposing PBTS (tendermint/tendermint#2840) but has not been implemented.
Experiments
The implementation was tested using the
e2etests and evaluated in small-scale testnets (tendermint/tendermint#7757). This effort has to be extended using the QA infrastructure to test the solution in more realistic testnets (Digital Ocean).The experimental efforts included the definition of metrics from which to derive safe and realistic values for the synchronous parameters (tendermint/tendermint#7202), adopted as their default values (tendermint/tendermint#7788). Those experiments should be re-evaluated under the light of the new use cases and features introduced from when they were performed.
Teams interested
Execution steps:
Important: all development related to PBTS will be done on feature branch
feature/pbts. Which will be merged back tomain(via a merge commit) when the team agrees that:mainThen, we will need to backport the branch onto branch
v1.x. But it should not be difficult, asmainandv1.xhave diverged very little.Implementation
master)wb/proposer-based-timestamps#2003main)main)Proposalwhen the proposed block received a Polka? #2119POLRoundvsPolRound)cs.LockedRound == -1check for testing timelyUpstream Dependencies
tendermintrepoSpec
mainrefers to the first proposal. But we implemented the second proposal, living inmaster(PBTS: system model made more precise tendermint/tendermint#8096)mainthe new version of the specification #1973tendermintrepo (optional):Verification
Documentation
docs/architecture/tendermint-core/adr-071-proposer-based-timestamps.mdCommitSigandVotestructs changes (block format breaking changes) are not considered in this versionExperiments
e2e: support PBTS enable param #2227clock_skewto skew the clocks of certain nodes #2453e2emanifest? (not sure it's needed)SynchronyParams.Precision > 500msFinal Steps
feature/pbtsintomain#2516feature/pbtsto be backported tov1.x#2542The text was updated successfully, but these errors were encountered: