types: add default values for the synchrony parameters#7788
Merged
mergify[bot] merged 7 commits intomasterfrom Feb 19, 2022
Merged
types: add default values for the synchrony parameters#7788mergify[bot] merged 7 commits intomasterfrom
mergify[bot] merged 7 commits intomasterfrom
Conversation
creachadair
approved these changes
Feb 8, 2022
tychoish
approved these changes
Feb 9, 2022
30 tasks
This was referenced Oct 4, 2024
github-merge-queue bot
pushed a commit
to cometbft/cometbft
that referenced
this pull request
Oct 7, 2024
…values (#4249) Closes #4246 - `Precision` was set to `505ms`, because it should be greater than 0.5s, as in tendermint/tendermint#7724 - `MessageDelay` was increased to `15s`, following the measurements reported in tendermint/tendermint#7788, and considering some outputs of `e2e` tests --- #### 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
github-merge-queue bot
pushed a commit
to cometbft/cometbft
that referenced
this pull request
Oct 7, 2024
…values (#4249) Closes #4246 - `Precision` was set to `505ms`, because it should be greater than 0.5s, as in tendermint/tendermint#7724 - `MessageDelay` was increased to `15s`, following the measurements reported in tendermint/tendermint#7788, and considering some outputs of `e2e` tests --- #### 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
mergify bot
pushed a commit
to cometbft/cometbft
that referenced
this pull request
Oct 7, 2024
…values (#4249) Closes #4246 - `Precision` was set to `505ms`, because it should be greater than 0.5s, as in tendermint/tendermint#7724 - `MessageDelay` was increased to `15s`, following the measurements reported in tendermint/tendermint#7788, and considering some outputs of `e2e` tests --- #### 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 (cherry picked from commit bbb86ce)
3 tasks
cason
pushed a commit
to cometbft/cometbft
that referenced
this pull request
Oct 7, 2024
…values (backport #4249) (#4278) Closes #4246 - `Precision` was set to `505ms`, because it should be greater than 0.5s, as in tendermint/tendermint#7724 - `MessageDelay` was increased to `15s`, following the measurements reported in tendermint/tendermint#7788, and considering some outputs of `e2e` tests --- #### 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 <hr>This is an automatic backport of pull request #4249 done by [Mergify](https://mergify.com). Co-authored-by: Daniel <daniel.cason@informal.systems>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This pull request adds a default set of values to the new Synchrony parameters. These values were chosen by after observation of three live networks: Emoney, Osmosis, and the Cosmos Hub.
For the default Precision value,
505mswas selected. The reasoning for this is summarized in #7724For each observed chain, an experimental Message Delay was collected over a 24 hour period and an average over this period was calculated using this data. Values over 10s were considered outliers and treated separately for the average since the majority of observations were far below 10s. The message delay was calculated both for the quorum and the 'full' prevote. Description of the technique for collecting the experimental values can found in #7202. This value is calculated only using timestamps given by processes on the network, so large variation in values is almost certainly due to clock skew among the validator set.
12sis proposed for the default MessageDelay value. This value would easily accomodates all non-outlier values, allowing even E-money's 4.25s value to be valid. This would also allow some validators with skewed clocks to still participate without allowing for huge variation in the timestamps produced by the network. Additionally, for the currently listed use-cases of PBTS, such as unbonding period, and light client trust period, the current bounds for these are in weeks. Adding a few seconds of tolerance by default is therefore unlikely to have serious side-effects.Data
Cosmos Hub
Observation Period: 2022-02-03 20:22-2022-02-04 20:22
Avg Full Prevote Message Delay: 1.27s
Outliers: 11s,13s,50s,106s,144s
Total Outlier Heights: 86
Avg Quorum Prevote Message Delay: .77s
Outliers: 10s,14s,107s,144s
Total Outlier Heights: 617
Total heights: 11528
Osmosis
Observation Period: 2022-01-29 20:26-2022-01-28 20:26
Avg Quorum Prevote Message Delay: .46s
Outliers: 21s,50s
Total Outlier Heights: 26
NOTE: During the observation period, a 'full' prevote was not observed.
Total heights: 13983
E-Money
Observation Period: 2022-02-07 04:29-2022-02-08 04:29
Avg Full Prevote Message Delay: 4.25s
Outliers: 12s,15s,39s
Total Outlier Heights: 128
Avg Quorum Prevote Message Delay: .20s
Outliers: 28s
Total Outlier Heights: 15
Total heights: 3791