v0.38.x: backport of new content on spec/p2p by cason · Pull Request #1005 · cometbft/cometbft

cason · 2023-06-20T14:14:02Z

This PR backports 3 PRs merged into the main branch: #714, #851, and #966.

Note to reviewers: there is no new content added here, this is just a manual backport of multiple PRs already merged into main.

Addresses #598

thanethomson · 2023-06-20T17:13:12Z

p2p/README.md

+- [Connection](https://github.com/cometbft/cometbft/blob/v0.38.x/spec/p2p/legacy-docs/connection.md) for details on how connections and multiplexing work
+- [Peer](https://github.com/cometbft/cometbft/blob/v0.38.x/spec/p2p/legacy-docs/node.md) for details on peer ID, handshakes, and peer exchange
+- [Node](https://github.com/cometbft/cometbft/blob/v0.38.x/spec/p2p/legacy-docs/node.md) for details about different types of nodes and how they should work
+- [Config](https://github.com/cometbft/cometbft/blob/v0.38.x/spec/p2p/legacy-docs/config.md) for details on some config option


Same as #1004 (comment)

Removed full links in 5877700

lasarojc · 2023-06-22T12:23:38Z

spec/p2p/implementation/README.md

+documentation also applies to the releases `v0.37.*` and `v0.38.*` [^v35].
+
+[^v35]: The releases `v0.35.*` and `v0.36.*`, which included a major
+  refactoring of the p2p layer implementation, were [discontinued][v35postmorten].


"discontinued" gives the impression that they didn't receive updates. I suggest a stronger wording here.

Suggested change

refactoring of the p2p layer implementation, were [discontinued][v35postmorten].

refactoring of the p2p layer implementation, were [retired][v35postmorten].

lasarojc · 2023-06-22T12:27:34Z

spec/p2p/implementation/switch.md


- In the [setup](https://github.com/cometbft/cometbft/blob/29c5a062d23aaef653f11195db55c45cd9e02715/node/node.go#L985) of a node, to establish connections with every configured
-  persistent peer
+- In the [setup](https://github.com/cometbft/cometbft/blob/v0.34.x/node/node.go#L987)


v0.34.x could still be updated and these links be broken. What is the guideline here? Should we incur in the risk of broken links or point to specific patch versions?

So, the implementation/README.md specifies that this documentation is based on v0.34.x branch. I've tried to check whether replacing links to the main branch could to the work, but it doesn't. So I would rather maintain references to v0.34.x here, until we find a way to describe the implementation on all branches consistently.

lasarojc

Good stuff. A couple of minor comments, which I should have made on the main branch. LGTM.

cason · 2023-06-22T14:27:56Z

Good stuff. A couple of minor comments, which I should have made on the main branch. LGTM.

I also realized, from your reviews and from mine, that there are some small changes to make, but I would rather open another PR against main with them, then to backport this new PR to other branches.

sergio-mena

Didn't read through the text. I started but realized I've already reviewed this (it's a backport -- let me know if there's some text not present in main that I should read)

cason · 2023-06-23T07:41:50Z

@sergio-mena, sorry, I have updated the description. The changes are really minimal:

$ git checkout cason/p2p-backport-to-0.38
$ git diff --stat main -- spec/p2p/ spec/abci/ p2p/README.md
 p2p/README.md                              |  8 ++++----
 spec/abci/README.md                        |  2 +-
 spec/abci/abci++_app_requirements.md       | 35 +++++++++++------------------------
 spec/abci/abci++_client_server.md          |  2 +-
 spec/abci/abci++_methods.md                | 12 ++++--------
 spec/p2p/legacy-docs/messages/consensus.md |  2 +-
 spec/p2p/legacy-docs/messages/mempool.md   |  2 +-
 7 files changed, 23 insertions(+), 40 deletions(-)

cason · 2023-06-23T07:46:09Z

The other backport of this same content, but to v0.37.x: #1004

cason · 2023-06-27T08:02:11Z

Any blocker for merging this PR?

spec/p2p/legacy-docs/messages/consensus.md

sergio-mena · 2023-06-29T13:31:20Z

I have updated the description. The changes are really minimal

I've reviewed those changes locally and LGTM, except for a minor comment that I just added.

For me, it looks good to go

* spec/p2p: starting the specification of a Reactor * spec/p2p: generic service modelled in Quint * spec/p2p: minimal/initial reactor model in Quint * spec/p2p: P2P -> reactor interaction grammar * spec/p2p: routines modeled in reactor.qnt * spec/p2p: reactor.Receive) modeled in Quint * spec/p2p: reactor types moved to the beginning * spec/p2p: reactor Quint state encapsulated * spec/p2p: reactor with actions and defintions * spec/p2p: reactor SetSwitch() method modeled * spec/p2p: reactor Service interface modeled * spec/p2p: reactor setup action added * spec/p2p: reactor Quint spec cleanup * spec/p2p: reactor InitPeer spec updated * spec/p2p: reactor Quint simulation support * spec/p2p: reactor Quint spec refactored * spec/p2p: reactor grammar added to a rephrased README * spec/p2p: README structure reorganized * spec/p2p: reactor registration, overview * spec/p2p: reactor implements a service (start/stop) * spec/p2p: reactor peer management, overview * spec/p2p: reactor receive message method documented * spec/p2p: renamed to registration/register reactor * spec/p2p: reactor grammar refactoring, part I * spec/p2p: reactor grammar refactoring, part II * spec/p2p: reactor grammar refactoring, part III * spec/p2p: reactor grammar refactoring, part IV * spec/p2p: removing Quint model used as an example * spec/p2p: reactor grammar refactoring, part V * spec/p2p: reactor grammar refactoring, part VI * spec/p2p: reactor grammar refactoring, part VII * Apply suggestions from code review Co-authored-by: Josef Widder <44643235+josef-widder@users.noreply.github.com> * spec/p2p: reactor Quint model, some comments added * Update spec/p2p/reactor/reactor.qnt Co-authored-by: Lasaro <lasaro@informal.systems> * spec/p2p: syntax HL for Quint files Co-authored-by: Josef Widder <44643235+josef-widder@users.noreply.github.com> * spec/p2p: suggestions from code review Co-authored-by: Sergio Mena <sergio@informal.systems> * spec/p2p: reactor Quint model, channel ID as str * spec/p2p: applying suggestions from code review * spec/p2p: API methods first, then actions on Quint * spec/p2p: reactor Quint model, minor code cleanup * spec/p2p: reactor README intro slightly rephrased * spec/p2p: reactor grammar intro slightly rephrased * spec/p2p: some reactor "should" replaced by "must" * spec/p2p: Quint model reference on README updated * spec/p2p: minor fixes on reactor's README --------- Co-authored-by: Daniel Cason <cason@lausanne.local> Co-authored-by: Josef Widder <44643235+josef-widder@users.noreply.github.com> Co-authored-by: Lasaro <lasaro@informal.systems> Co-authored-by: Sergio Mena <sergio@informal.systems>

* spec/p2p: registered flag in Quint reactor * spec/p2p: NewReactor method in Quint reactor * spec/p2p: reactor is registered with a name * spec/p2p: reactor actions receive ReactorState * spec/p2p: Quint model supports multiple reactors * spec/p2p: Quint reactor/service state simplified * spec/p2p: Quint reactor has channels' list as field * spec/p2p: Quint updateReactorTo action helper * spec/p2p: reactors are assigned to channel IDs * spec/p2p: reactor API specification, small fixes * spec/p2p: API p2p layer provides to reactors * spec/p2p: reactor API content to reactor.md file * spec/p2p: new README for reactors with a diagram * spec/p2p: API for reactors, introduction text * spec/p2p: links between the two API documentations * spec/p2p: suggestions from code review Co-authored-by: Sergio Mena <sergio@informal.systems> * spec/p2p: Apply suggestions from code review Co-authored-by: Josef Widder <44643235+josef-widder@users.noreply.github.com> Co-authored-by: Lasaro <lasaro@informal.systems> * spec/p2p: reactor documentation intro, small fix * spec/p2p: a paragraph about concurrency and reactors * spec/p2p: intro to the reactor grammar, minor add * spec/p2p: notes about switch broadcast method * spec/p2p: note on NumPeers() switch method * spec/p2p: reactor veting peers, some rephrasing * spec/p2p: link to discussion on peer quality * spec/p2p: link to stop peer discussion on other repo * spec/p2p: link to state sharing between reactors added * spec/p2p: link to reactor API split added --------- Co-authored-by: Sergio Mena <sergio@informal.systems> Co-authored-by: Josef Widder <44643235+josef-widder@users.noreply.github.com> Co-authored-by: Lasaro <lasaro@informal.systems>

* spec/p2p: moving legacy documents to legacy/ dir * spec/p2p: using capitalized names for README.md * spec/p2p: intro to p2p spec, moved from other README * spec/p2p: v0.34.x documentation, README shortened * spec/p2p: minor changes on READMEs * spec/p2p: moving messages/ content to legacy/ dir * spec/p2p: dir for docs of the p2p implementation * spec/p2p: renamed reactor dirs to reactor-api * spec/p2p: legacy content to legacy-docs/ dir * spec/p2p: fixes on implementation/README.md file * spec/p2p: moving images to a single subdir * spec/p2p: fixing Markdown links * spec/p2p: fixing Markdown links in all repo * spec/p2p: table of contents for the p2p docs * spec/p2p: applying suggestions from Josef Co-authored-by: Josef Widder <44643235+josef-widder@users.noreply.github.com> --------- Co-authored-by: Josef Widder <44643235+josef-widder@users.noreply.github.com>

cason changed the title ~~v0.38.x: backport of new content on spec/p2p #1004~~ v0.38.x: backport of new content on spec/p2p Jun 20, 2023

cason mentioned this pull request Jun 20, 2023

spec/p2p: new structure for the p2p specification #966

Merged

cason force-pushed the cason/p2p-backport-to-0.38 branch from 76b090c to 65d273c Compare June 20, 2023 15:03

cason marked this pull request as ready for review June 20, 2023 15:04

cason requested review from a team as code owners June 20, 2023 15:04

cason self-assigned this Jun 20, 2023

cason added p2p spec Specification-related labels Jun 20, 2023

thanethomson reviewed Jun 20, 2023

View reviewed changes

lasarojc reviewed Jun 22, 2023

View reviewed changes

lasarojc approved these changes Jun 22, 2023

View reviewed changes

sergio-mena approved these changes Jun 22, 2023

View reviewed changes

cason force-pushed the cason/p2p-backport-to-0.38 branch from 5877700 to 067fd57 Compare June 23, 2023 07:46

sergio-mena reviewed Jun 29, 2023

View reviewed changes

spec/p2p/legacy-docs/messages/consensus.md Outdated Show resolved Hide resolved

sergio-mena approved these changes Jun 29, 2023

View reviewed changes

Daniel and others added 5 commits June 29, 2023 15:47

spec/p2p: removing full links to the p2p spec

43245fb

spec/p2p: fix link typo identified by reviewer

1ebb801

cason force-pushed the cason/p2p-backport-to-0.38 branch from 71fcfe1 to 1ebb801 Compare June 29, 2023 13:47

cason merged commit 88f3c75 into v0.38.x Jun 29, 2023

cason deleted the cason/p2p-backport-to-0.38 branch June 29, 2023 14:24

faddat mentioned this pull request Oct 15, 2024

chore: use latest cometbft-db in v0.38.x #4296

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

v0.38.x: backport of new content on spec/p2p#1005

v0.38.x: backport of new content on spec/p2p#1005
cason merged 5 commits intov0.38.xfrom
cason/p2p-backport-to-0.38

cason commented Jun 20, 2023 •

edited

Loading

Uh oh!

thanethomson Jun 20, 2023

Uh oh!

cason Jun 22, 2023

Uh oh!

lasarojc Jun 22, 2023

Uh oh!

lasarojc Jun 22, 2023

Uh oh!

cason Jun 22, 2023

Uh oh!

lasarojc left a comment •

edited

Loading

Uh oh!

cason commented Jun 22, 2023

Uh oh!

sergio-mena left a comment

Uh oh!

cason commented Jun 23, 2023

Uh oh!

cason commented Jun 23, 2023

Uh oh!

cason commented Jun 27, 2023

Uh oh!

Uh oh!

sergio-mena commented Jun 29, 2023 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

	refactoring of the p2p layer implementation, were [discontinued][v35postmorten].
	refactoring of the p2p layer implementation, were [retired][v35postmorten].

Conversation

cason commented Jun 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thanethomson Jun 20, 2023

Choose a reason for hiding this comment

Uh oh!

cason Jun 22, 2023

Choose a reason for hiding this comment

Uh oh!

lasarojc Jun 22, 2023

Choose a reason for hiding this comment

Uh oh!

lasarojc Jun 22, 2023

Choose a reason for hiding this comment

Uh oh!

cason Jun 22, 2023

Choose a reason for hiding this comment

Uh oh!

lasarojc left a comment • 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!

cason commented Jun 22, 2023

Uh oh!

sergio-mena left a comment

Choose a reason for hiding this comment

Uh oh!

cason commented Jun 23, 2023

Uh oh!

cason commented Jun 23, 2023

Uh oh!

cason commented Jun 27, 2023

Uh oh!

Uh oh!

sergio-mena commented Jun 29, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

cason commented Jun 20, 2023 •

edited

Loading

lasarojc left a comment •

edited

Loading

sergio-mena commented Jun 29, 2023 •

edited

Loading