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

Inform Consensus Layer whether a bootstrap peer should be considered remote #4815

Closed
nfrisby opened this issue Feb 26, 2024 · 6 comments · Fixed by #4846
Closed

Inform Consensus Layer whether a bootstrap peer should be considered remote #4815

nfrisby opened this issue Feb 26, 2024 · 6 comments · Fixed by #4846
Assignees
Labels
consensus issues related to ouroboros-consensus Genesis

Comments

@nfrisby
Copy link
Contributor

nfrisby commented Feb 26, 2024

@karknu had raised a concern about the Bootstrap State Machine (ie IntersectMBO/ouroboros-consensus#808) that would be most naturally mitigated by the Diffusion Layer annotating new peers with whether or not that peer should be considered remote.

EG SPOs separately configure the "local trusted" portion of the topology from the "bootstrap peers source" of the topology. If that distinction would be preserved deep enough into the code, then the Bootstrap State Machine would simply refuse to transition from TooOld to YoungEnough while it only has local peers.

@nfrisby nfrisby added consensus issues related to ouroboros-consensus Genesis labels Feb 26, 2024
@coot
Copy link
Contributor

coot commented Feb 26, 2024

To address this we need to extend ExpandedInitiatorContext which is used by node-to-node clients.

@karknu
Copy link
Contributor

karknu commented Feb 27, 2024

simply refuse to transition from TooOld to YoungEnough while it only has local peers

For relays this is exactly what we want.
For nodes with some trusted local peers and no bootstrap peers, such as BPs, the transition should happen when only talking to trusted local peers.

@bolt12
Copy link
Contributor

bolt12 commented Mar 19, 2024

Am I right understanding that by adding a eicIsLocalRootPeer :: Bool field in ExpandedInitiatorContext is information enough for consensus ?

@coot
Copy link
Contributor

coot commented Mar 20, 2024

I realised that Consensus might actually be interested in a signal from the outbound governor which summarises all connections. If we just expose IsLocalRoot in ExpandedInitiatorContext then on the consensus side one will need to build such a signal oneself, despite it being available from the outbound-governor.

This would be handy in other areas as well (e.g. churn). So here's another proposal. The outbound-governor should expose STM EffectiveTargets EffectiveCounters where effective targets counters is a record containing number of hot & warm: local / ledger / big ledger / bootstrap peers currently used by the outbound governor. This would work for us in churn and summarising it to onlyLocalOutboundConnections :: EffectiveCounters -> Bool.

@bolt12
Copy link
Contributor

bolt12 commented Mar 20, 2024

I don't see how exposing targets would work for consensus, since they have to know which peers are local and which peers are not, right?

@coot
Copy link
Contributor

coot commented Mar 20, 2024

I used wrong name, it's about counters not targets.

@coot coot self-assigned this Apr 8, 2024
@bolt12 bolt12 moved this from In Progress to Done in Ouroboros Network May 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
consensus issues related to ouroboros-consensus Genesis
Projects
Status: Done
4 participants