Skip to content

PBTS: Block time specification, replacing BFTTime#8600

Closed
cason wants to merge 37 commits intomasterfrom
main/pbts
Closed

PBTS: Block time specification, replacing BFTTime#8600
cason wants to merge 37 commits intomasterfrom
main/pbts

Conversation

@cason
Copy link

@cason cason commented May 24, 2022

This PR updates the specification of PBTS, considering its implementation and availability in v0.36.

No actual technical content is included, but a general re-organization of the documentation of PBTS. In addition, the TLA+ specification was re-organized and some documentation for running the model checker was introduced.

Closes #8591.

Copy link

@angbrav angbrav left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review

Great work! It is the first time I am reviewing this spec, so I went through everything (not only the changes introduced in this PR). I have opened issue #8628 with my feedback, to avoid polluting this PR with comments unrelated to the PR itself.

Copy link
Contributor

@josef-widder josef-widder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The log for the experiments and the readme that explains how to run them looks great, Thanks @Kukovec !

@github-actions
Copy link

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale for use by stalebot label Jun 12, 2022
@github-actions github-actions bot closed this Jun 17, 2022
@cason cason reopened this Jun 20, 2022
@cason cason self-assigned this Jun 20, 2022
@cason cason marked this pull request as draft June 20, 2022 09:06
@cason cason added this to the v0.36 milestone Jun 20, 2022
@cason
Copy link
Author

cason commented Jun 20, 2022

I moved this PR to the draft state because:

  1. It includes several files it should not, outside the spec directory.
  2. I need to address the comments from Manuel (@angbrav)
  3. We need to address the crash-recovery behavior of the solution (Chain halts when validator with >1/3 voting power is unable to sign replayed prevote #8739)

I assume this PR is from v0.36 milestone, so I will complete it by then.

@github-actions github-actions bot removed the stale for use by stalebot label Jun 21, 2022
@github-actions
Copy link

github-actions bot commented Jul 1, 2022

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale for use by stalebot label Jul 1, 2022
@github-actions github-actions bot closed this Jul 6, 2022
@cason
Copy link
Author

cason commented Jul 7, 2022

I moved this PR to the draft state because:

1. It includes several files it should not, outside the `spec` directory.

2. I need to address the comments from Manuel (@angbrav)

3. We need to address the crash-recovery behavior of the solution ([Chain halts when validator with >1/3 voting power is unable to sign replayed prevote #8739](https://github.com/tendermint/tendermint/issues/8739))

I assume this PR is from v0.36 milestone, so I will complete it by then.

I will let this PR be closed and start a new one.

@cason cason mentioned this pull request Jul 7, 2022
@tychoish tychoish deleted the main/pbts branch July 25, 2022 18:22
cason pushed a commit to cometbft/cometbft that referenced this pull request Jan 11, 2024
   * Originally part of tendermint/tendermint#8600
   * Ignoring large and temporary intermediate/ directories
   * Duplicated files were removed
cason pushed a commit to cometbft/cometbft that referenced this pull request Jan 24, 2024
Addresses #2019.

[Spec entry point
(rendered)](https://github.com/cometbft/cometbft/blob/cason/pbts-review/spec/consensus/proposer-based-timestamp/README.md)

For some reason, Git is not tracking the renamed filed
`pbts-algorithm*.md` properly. A better diff as follows:

git diff --word-diff
main:spec/consensus/proposer-based-timestamp/pbts-algorithm_002_draft.md
cason/pbts-review:spec/consensus/proposer-based-timestamp/pbts-algorithm.md

Relevant changes are accompanied by comments from the author.

### TLA+ verification

A second contribution of this PR is to add the most recent TLA+
specification of consensus with PBTS, plus scripts, configurations, and
the results of performed experiments. This was a manual backport of the
content of the legacy `main-pbts` branch, as in
tendermint/tendermint#8600.

#### 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

---------

Co-authored-by: glnro <8335464+glnro@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

stale for use by stalebot

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update Block Time documentation in Spec

5 participants