Consensus Reactor communication specification by lasarojc · Pull Request #74 · cometbft/cometbft

lasarojc · 2023-01-04T19:13:15Z

Contributes to #15

* [cherry-picked] ABCI++: Update new protos to use enum instead of bool (#8158) This pull request updates the new ABCI++ protos to use `enum`s in place of `bool`s. `enums` may be preferred over `bool` because an `enum` can be udpated to include new statuses in the future, whereas a `bool` cannot and is fixed as just `true` or `false` over the whole lifecycle of the API. * Detect and handle UNKNOWN in `ResponseVerifyVoteExtension` Co-authored-by: William Banfield <4561443+williambanfield@users.noreply.github.com>

…-abci++vef

Merge `main` into `feature/abci++vef`

… passed-through to application (#8216) (#98) closes: #7950 Co-authored-by: William Banfield <4561443+williambanfield@users.noreply.github.com>

josef-widder

I left some comments that we can discuss during our next meeting.

spec/consensus/reactor/reactor.md

josef-widder · 2023-01-17T13:41:18Z

spec/consensus/reactor/reactor.md

+
+|Communication with Supersession|
+|-----|
+| If process $p$ broadcast message $m2$, then $p$ does not broadcast any message $m1$, $m2.\text{SSS}(m1)$.[^todo1]


Is this already formalized? Should be m2.\text{SSS}(m1) -> G (bc(p,m2) -> G \neg bc(p,m1))

What does G stand for?

I would phrase this property like the following one, in terms of "not required". In the current shape, not broadcasting $m1$ is a requirement.

spec/consensus/reactor/reactor.md

sergio-mena

Thanks Lásaro for advancing on this. This is an awesome step forward, I'm excited about the prospect of having a complete spec of this part of the system.

In particular, I like that the part that deals with consensus gossip spec, could be reused for non-consensus upper layers almost verbatim.

spec/consensus/reactor/files.md

sergio-mena · 2023-01-23T09:51:00Z

spec/consensus/reactor/files.md

+
+    - Message delivery
+        - `Receive`
+            - Processes messages delivered by the Gossip layer and updates consensus (`State`) and peer information (`PeerState`)


According to what I've seen elsewhere PeerState is used by other reactors? Is that so? If that's the case we need decide what to do with this (not modular!)

sergio-mena · 2023-01-23T09:53:02Z

spec/consensus/reactor/files.md

+        - `Receive`
+            - Processes messages delivered by the Gossip layer and updates consensus (`State`) and peer information (`PeerState`)
+            - Messages are received through multiple channels
+                - `StateChannel`: `NewRoundStepMessage`, `NewValidBlockMessage`, `HasVoteMessage`, `VoteSetMaj23Message`


HasVoteMessage not part of VoteChannel or VotBitsChannel ? (good) reason?

spec/consensus/reactor/reactor.md

sergio-mena · 2023-01-23T18:55:37Z

spec/consensus/reactor/reactor.md

+
+|[REQ-GOSSIP-CONS-SUPERSESSION.2]|
+|----|
+| There exists a constant $c \in Int$ such that, at any point in time, for any process $p$, the subset of messages in bMsgs[p] that have not been superseded is smaller than $c$.


We would need to be careful here, as the 2f+1 precommits allowing a node to decide for every height need to be kept around, and my understanding is they cannot be superseded.

The question is for what these messages are needed? For consensus? Or for some other protocol? I guess bMsgs[p] should just keep the messages that consensus needs, while the 2f+1 precommits could be archived somewhere else? If this is possible, then $c$ could be fine.

These are goo good points. Since precommits are evaluated consensus, I guess they will need to be in bMsgs. But I guess that makes it necessary that $c$ be a function of $f$ or of $n$.

BTW, I've now hidden bMsgs to simplify some definition. bMsgs and dMsgs became witness variables to the GOSSIP layer and CONS expressions cannot be expressed in terms of them.

spec/consensus/reactor/reactor.md

spec/consensus/reactor/implementation.md

sergio-mena · 2023-01-23T19:19:17Z

spec/consensus/reactor/reactor.md

+> **TODO**: Provide an outline
+
+
+# Part 1: Background


To think about: this section does not consider the "dynamic model" that the system is assuming ($p$ and $q$ come and go). Is that a problem?

Good point.
Processes that go are failed ones. Processes that come could be seen as always existing, but slow, but that would complicate the definitions. I will need to think more about them.

…ng some gossip internal state.

lasarojc · 2023-01-27T17:02:46Z

To register some of what we discussed today

Superseding has two goals: allow nodes to forget messages (drop what I know is superseded) and allow nodes to identify what others need (don't forward info that is superseded at the destination).
The first goal seems to require a full node to detect supersession, so we'll consider only full nodes at the gossip level.
Gossip only nodes does not seem to make sense; they must be full nodes at least.
P2P only nodes may make sense, but it is not in the scope here.
The second goal requires knowledge of the destination's state and we need to move forward in formalizing this.
Shared predicates is a good way to represent leaked abstractions for now, but ideally we want a cleaner interface.
The supersession idea seems to make sense in other reactors (committed transactions may invalidate/supersede other transactions; old evidences are superseded by passage of time), but this is something to consider later.

…metbft into lasarojc/spec/gossip

* Renames cmd/tendermint to cmd/cometbft * Replacing TMHOME for CMTHOME. On reading the environment variable we still try TMHOME if CMTHOME is not found. TMHOME is deprecated and will be removed in the future releases. * Include TMHOME deprecation in the changelog * Updating the config * updating uses of .tendermint to .cometbft * Updating the Makefile * Fix test of debug level flags. * Update .changelog/unreleased/breaking-changes/211-deprecate-tmhome.md Co-authored-by: Thane Thomson <connect@thanethomson.com> --------- Co-authored-by: Thane Thomson <connect@thanethomson.com>

* Fix multiversion e2e when `version <= v0.34.24` * Changes we want no matter what we do * Approach 1: With docker-compose up recreating image * Revert "Approach 1: With docker-compose up recreating image" This reverts commit 068f5b78d3b7cf99fb21296707117e4f4e2ef3e0. * Approach 2: With distinct docker-compose service * Fix the case when 'upgrade' is not the last perturbation * Update generator * Simplify template * bump * Update test/e2e/pkg/manifest.go * changelog * doc * fix doc

* add peer gossip sleep * add changelog * Update .changelog/unreleased/bug-fixes/4-busy-loop-send-block-part.md Co-authored-by: Jasmina Malicevic <jasmina.dustinac@gmail.com> * Update .changelog/unreleased/bug-fixes/4-busy-loop-send-block-part.md --------- Co-authored-by: jhernandezb <contact@jhernandez.me> Co-authored-by: Jasmina Malicevic <jasmina.dustinac@gmail.com>

Looking at Dependabot updates like #233, it makes no sense that the code generation check is failing, because running `make mockery` locally on that dependabot branch works just fine. This should hopefully help debug what's going on there. --- #### PR checklist - [x] Tests written/updated, or no tests needed - [x] `CHANGELOG_PENDING.md` updated, or no changelog entry needed - [x] Updated relevant documentation (`docs/`) and code comments, or no documentation updates needed

…a time t from definitions.

…tests and invariant checks on their own modules. All modules on a single file since quint cannot handle multiple files yet.

…c P2P mocking. Removed hack to do named import.

… all tests passing. Reworking the Gossip part

…ional for now) and of fully connected (type issue).

…t there yet

…metbft into lasarojc/spec/gossip

lasarojc · 2023-05-03T11:34:03Z

instead of this large PR, breaking the (updated) changes into a series of smaller PRs

…ft#3017)… (backport cometbft#74) (cometbft#79) * perf: Minor speedup to consenus metrics MarkLateVote (backport cometbft#3017) (cometbft#3026) Minor speedup to metrics MarkLateVote. I saw the excess string allocation calls, so made a quick PR to remove it. This saves between .18s-25s from the consensus mutex across this one hour block sync. (Likely not at all consensus critical) It also appears in receiving votes ![image](https://github.com/cometbft/cometbft/assets/6440154/61875b0d-b419-4900-a74c-76c65087bca4) (Saves ToLower(), .String(), .TrimPrefix() and newObject calls. The new call has comparable complexity to .String()) --- #### PR checklist - [x] 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 cometbft#3017 done by [Mergify](https://mergify.com). --------- Co-authored-by: Dev Ojha <ValarDragon@users.noreply.github.com> Co-authored-by: Anton Kaliaev <anton.kalyaev@gmail.com> (cherry picked from commit 29dd011) * Add changelog (cherry picked from commit 6392e64) # Conflicts: # CHANGELOG.md --------- Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> Co-authored-by: Dev Ojha <dojha@berkeley.edu>

lasarojc changed the title ~~Lasarojc/spec/gossip~~ Consensus Reactor communication specification Jan 4, 2023

lasarojc self-assigned this Jan 5, 2023

lasarojc force-pushed the lasarojc/spec/gossip branch from 1a9007c to b7a7ece Compare January 6, 2023 11:52

lasarojc and others added 6 commits January 11, 2023 08:49

flushing some text

cca3c2e

updating

2321009

Merge remote-tracking branch 'comet/main' into sergio/merge-main-into…

bf96a1a

…-abci++vef

Merge pull request #102 from cometbft/sergio/merge-main-into-abci++vef

8786a7b

Merge `main` into `feature/abci++vef`

mild reorganization

2863828

lasarojc force-pushed the lasarojc/spec/gossip branch from 5a8e35d to 2863828 Compare January 14, 2023 10:52

lasarojc and others added 2 commits January 15, 2023 10:31

mild reorganization

7a55949

[cherry-picked] abci++: only include meaningful header fields in data…

cbcdffb

… passed-through to application (#8216) (#98) closes: #7950 Co-authored-by: William Banfield <4561443+williambanfield@users.noreply.github.com>

josef-widder reviewed Jan 17, 2023

View reviewed changes

mergify bot closed this Jan 18, 2023

mergify bot deleted the lasarojc/spec/gossip branch January 18, 2023 13:26

lasarojc restored the lasarojc/spec/gossip branch January 18, 2023 13:38

lasarojc reopened this Jan 18, 2023

lasarojc added the spec Specification-related label Jan 19, 2023

sergio-mena reviewed Jan 23, 2023

View reviewed changes

lasarojc added 2 commits January 27, 2023 10:20

Addressing some of the reviewers comments. Improving the spec by hidi…

b9893db

…ng some gossip internal state.

Moving intro information to README.md, which should be the entry point.

eb43845

lasarojc and others added 8 commits January 27, 2023 14:18

Merge branch 'main' into lasarojc/spec/gossip

64ebd10

Updating qnt to reflect the latest changes in the english spec

44ebb20

Merge branch 'lasarojc/spec/gossip' of https://github.com/cometbft/co…

05114bf

…metbft into lasarojc/spec/gossip

Renamed prefix in command (#237)

c25484f

lasarojc added 24 commits April 25, 2023 07:54

Moving intro information to README.md, which should be the entry point.

4107cdb

Updating qnt to reflect the latest changes in the english spec

500d66f

WIP: saving changes to switch branches

8f44a31

Correct language

274926e

Provide alternative definition of supersession. Remove references to …

2869074

…a time t from definitions.

Saving some WIP to switch branches

35e58a1

a first pass towards moving to CRDT to represent the state

f8700a4

Rebranding

058516e

switching branches

7971aa3

Rewriting to use CRDT

c93c31b

Rewriting to use CRDT

c6f71e7

updating .qnt

7d0b82e

updating the quint spec

7bc183b

Using modules. Using a moot parameter to do named import. Separating …

7fd6151

…tests and invariant checks on their own modules. All modules on a single file since quint cannot handle multiple files yet.

Exploring modules. Separated P2P tests from GOSSIP Tests to test basi…

249a291

…c P2P mocking. Removed hack to do named import.

playing with modules

0b0d089

Renaming the folder

8292024

WIP: working on proper modularization

616ff14

reverting some of the modularization. Fixed the p2p part of the spec;…

a3dcb46

… all tests passing. Reworking the Gossip part

adding some gossip tests for different topologies

8a4acd6

Working on gossip tests. Failing on ring (because it is single direct…

0c1036b

…ional for now) and of fully connected (type issue).

Making progress in specifying gossip over a mocking P2P layer, but no…

35a6d53

…t there yet

wip: modularizing

176937e

multi-files

05129a1

cason mentioned this pull request May 2, 2023

Discusses the Tendermint algorithm communication in terms of a tuple space cometbft/knowledge-base#2

Merged

Merge branch 'lasarojc/spec/gossip' of https://github.com/cometbft/co…

e03056a

…metbft into lasarojc/spec/gossip

lasarojc closed this May 3, 2023

zrbecker deleted the lasarojc/spec/gossip branch February 7, 2025 17:07

Conversation

lasarojc commented Jan 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

josef-widder left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sergio-mena left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

josef-widder Jan 27, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lasarojc commented Jan 27, 2023

Uh oh!

lasarojc commented May 3, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

19 participants

lasarojc commented Jan 4, 2023 •

edited

Loading

josef-widder Jan 27, 2023 •

edited

Loading