Skip to content
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.

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

p2p/reactors: some additional topics for discussion (WIP) #8

Merged
merged 9 commits into from
Jun 12, 2023

Conversation

cason
Copy link
Contributor

@cason cason commented May 26, 2023

Comment on lines 21 to 23
> I think we can come up with a cleaner interface for this information flow,
> ideally having all consensus-specific logic within consensus. I think we should
> look into this more closely.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I understand now it is less about consensus per se, but just communicates the height. I guess the question we should ask is "How shall we make a node's local height, as well as the heights of the peers available to the reactors?"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could abstract this information as a logical clock. In the case of cometbft, logical time is measured in heights, but other algorithms can measure it in "real time", epochs, ...

p2p/reactors/README.md Show resolved Hide resolved
Copy link
Contributor

@josef-widder josef-widder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As this is mainly to keep track of things we found, I think we can merge, and discuss with the team later. @sergio-mena said that at some point he would like to see what potential work is out there, so this would be a good starting point.


func (sw *Switch) MarkPeerAsGood(peer Peer)

However, there is no method in the [API to Reactors][pr-851] to mark a peer as
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of marking as bad as good, we could use a point system, where points are normalized to give a rating between 0 and 1.
what reactors do based on this rating, e.g. setting a minimum rate of 0.8 to be considered good, is up to them.
this is more or less the idea of the accrual failure detector.

@cason cason marked this pull request as ready for review June 12, 2023 08:37
@cason cason merged commit 8a23602 into main Jun 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants