8000 PBTS should be enabled by default in consensus test units · Issue #2324 · cometbft/cometbft · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

PBTS should be enabled by default in consensus test units #2324

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.

Sign up for GitHub

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

Jump to bottom
Closed
Tracked by #2063
cason opened this issue Feb 13, 2024 · 0 comments · Fixed by #2329
Closed
Tracked by #2063

PBTS should be enabled by default in consensus test units #2324

cason opened this issue Feb 13, 2024 · 0 comments · Fixed by #2329
Assignees
@cason
Labels
pbts testing related to unit testing in general

Comments

@cason
Copy link
Contributor
cason commented Feb 13, 2024 •
edited
Loading

Enabling PBTS in existing chains or via genesis is performed via consensus parameters (see #2197), as chains have to define safe and good values for the synchronous parameters.

However, in internal test units we should only run consensus with PBTS enabled, as it will become the de facto default for computing timestamps from v1.x.

Regading e2e tests, the default behavior will be defined by #2284.
@cason cason added e2e Related to our end-to-end tests pbts testing related to unit testing in general labels Feb 13, 2024
@cason cason added this to CometBFT Feb 13, 2024
@github-project-automation github-project-automation bot moved this to Todo in CometBFT Feb 13, 2024
@cason cason mentioned this issue Feb 13, 2024
Closed
30 tasks
@lasarojc lasarojc mentioned this issue Feb 13, 2024
Closed
@cason cason mentioned this issue Feb 13, 2024
Merged
4 tasks
@cason cason linked a pull request Feb 13, 2024 that will close this issue
test(consensus): PBTS should be enabled by default in test units #2329
Merged
4 tasks
@cason cason self-assigned this Feb 13, 2024
@sergio-mena sergio-mena mentioned this issue Feb 19, 2024
Closed
14 tasks
This was referenced Feb 19, 2024
Merged
Closed
@cason cason removed the e2e Related to our end-to-end tests label Feb 20, 2024
cason added a commit that referenced this issue Feb 26, 2024
@cason
test(consensus): PBTS should be enabled by default in test units (#2329)
0911607 
Addresses #2324.

PBTS is enabled by setting `ConsensusParams.Feature.PbtsEnableHeight` to
a height > 0. To be enabled from startup, this value should be set to
`1`.

The default values for consensus parameters have both the `Feature`
entries set `0`, meaning that both PBTS and vote extensions are
disabled. While this setup can be reasonable for in production networks,
it does not allow test units to explore these features that are expected
to be enabled in new versions.

This PR ensures that PBTS is enabled in all test units. As a side
effect, vote extensions are also enabled in all unit tests (except when
the test checks the behavior without these features).

---

#### 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
@cason cason closed this as completed Feb 26, 2024
@github-project-automation github-project-automation bot moved this from Todo to Done in CometBFT Feb 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@cason cason

Labels
pbts testing related to unit testing in general
Projects
No open projects
CometBFT
Status: Done
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

test(consensus): PBTS should be enabled by default in test units cometbft/cometbft
1 participant
@cason
0