feat(config)!: merge timeout_prevote and timeout_precommit#2895
feat(config)!: merge timeout_prevote and timeout_precommit#2895
timeout_prevote and timeout_precommit#2895Conversation
`timeout_prevote_delta` and `timeout_precommit_delta` into `timeout_round` and `timeout_round_delta` accordingly Closes #2893
| This has important liveness implications and should be avoided. | ||
| #### Warning | ||
|
|
||
| Setting `timeout_round` to `0s` means that the validator will not wait for |
There was a problem hiding this comment.
We need a good way to minimize disruption for operators upgrading from current config (and not thinking of adapting this, so the new field will have value 0).
An idea can be that the new timeout, when 0, means some sensible default value (e.g. our default value: 1s).
Anyway, disabling these timeouts never made any sense to me in production. And if need to disable it in text, we can just set it to time.Hour.
Also, this design would enable us to (later on) add those timeouts as ConsensusParams:
- If the value is 0, the node uses the consensus param value
- But if the operator wants to override the consensus param value, they can by setting a value != 0 in the
config.toml
Let me know what you think
There was a problem hiding this comment.
While this would work in this case, it won't in the case of skip_timeout_commit. If the user had skip_timeout_commit=true and timeout_commit=1s, in v1 if he/she doesn't do anything, the node will be running with 1s timeout commit.
What if instead we version config and write a CLI tool for upgrading it? If the manual cmd is not desirable, then we can attempt upgrade upon a startup. How does that sound?
There was a problem hiding this comment.
Fair enough, but it risks being quite some effort. Maybe, a middle ground would be:
- We version the config as you say
- We include all changes to the config in a paragraph in
UPGRADING.md - Upon start-up, if the version received is
- In the future, we panic saying this is an old version of CometBFT
- In the past, we warn the user that they may have some fields that became obsolete
- we include a link to the paragraph in
UPGRADING.mdto explain what to change - alternatively, for safely, we panic and ask the user to
- bump the version manually
- while doing that, to pay attention at the config fields that changed
- we include a link to the paragraph in
If we decide to go the config tool path (I think it's overkill for the changes we have ATM), there is already one, called Confix, developed by the previous Tendermint Core team, so we should probably reuse it. It's on v0.36.x and master. AFAICT the SDK folks are also using it.
There was a problem hiding this comment.
Anyway, this discussion should block this PR (although the value 0 to vote when upgrading isn't solved)
There was a problem hiding this comment.
@melekes FYI, I'm adding a point in the community call, to see what users think
There was a problem hiding this comment.
I will address this in a separate PR.
`timeout_prevote_delta` and `timeout_precommit_delta` into `timeout_vote` and `timeout_vote_delta` accordingly Closes #2893 <!-- Please add a reference to the issue that this PR addresses and indicate which files are most critical to review. If it fully addresses a particular issue, please include "Closes #XXX" (where "XXX" is the issue number). If this PR is non-trivial/large/complex, please ensure that you have either created an issue that the team's had a chance to respond to, or had some discussion with the team prior to submitting substantial pull requests. The team can be reached via GitHub Discussions or the Cosmos Network Discord server in the #cometbft channel. GitHub Discussions is preferred over Discord as it allows us to keep track of conversations topically. https://github.com/cometbft/cometbft/discussions If the work in this PR is not aligned with the team's current priorities, please be advised that it may take some time before it is merged - especially if it has not yet been discussed with the team. See the project board for the team's current priorities: https://github.com/orgs/cometbft/projects/1 --> --- #### PR checklist - [ ] ~~Tests written/updated~~ - [x] 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 (cherry picked from commit 19dc85e)
…ort #2895) (#3014) `timeout_prevote_delta` and `timeout_precommit_delta` into `timeout_vote` and `timeout_vote_delta` accordingly Closes #2893 --- #### PR checklist - [ ] ~~Tests written/updated~~ - [x] 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 <hr>This is an automatic backport of pull request #2895 done by [Mergify](https://mergify.com). Co-authored-by: Anton Kaliaev <anton.kalyaev@gmail.com>
…` (backport #2895) (#3014)" This reverts commit 54a086b. <!-- Please add a reference to the issue that this PR addresses and indicate which files are most critical to review. If it fully addresses a particular issue, please include "Closes #XXX" (where "XXX" is the issue number). If this PR is non-trivial/large/complex, please ensure that you have either created an issue that the team's had a chance to respond to, or had some discussion with the team prior to submitting substantial pull requests. The team can be reached via GitHub Discussions or the Cosmos Network Discord server in the #cometbft channel. GitHub Discussions is preferred over Discord as it allows us to keep track of conversations topically. https://github.com/cometbft/cometbft/discussions If the work in this PR is not aligned with the team's current priorities, please be advised that it may take some time before it is merged - especially if it has not yet been discussed with the team. See the project board for the team's current priorities: https://github.com/orgs/cometbft/projects/1 --> --- #### 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 [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec
timeout_prevote_deltaandtimeout_precommit_deltainto
timeout_voteandtimeout_vote_deltaaccordinglyCloses #2893
PR checklist
Tests written/updated.changelog(we use unclog to manage our changelog)docs/orspec/) and code comments