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.

Sign up for GitHub

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

Jump to bottom

Add the pre-Genesis State Machine for bootstrap peers #808

Merged
merged 7 commits into from
Feb 22, 2024
Merged

Add the pre-Genesis State Machine for bootstrap peers #808

merged 7 commits into from
Feb 22, 2024

Conversation

nfrisby
Copy link
Contributor

@nfrisby nfrisby commented Dec 2, 2023 •
edited by amesgen
Loading

The Genesis SoW will deliver a proper GSM. In the interim, this PR defines a GSM that is suitable for use with the bootstrap-peers intermediate solution (see IntersectMBO/ouroboros-network#4555).

The relevant high-level specification is rendered here https://ouroboros-consensus.cardano.intersectmbo.org/docs/for-developers/BootstrapPeersIER

@nfrisby nfrisby force-pushed the nfrisby/bootstrap-gsm branch 2 times, most recently from 53a2c5d to 7ec551c Compare December 4, 2023 15:34
@nfrisby nfrisby requested review from coot and bolt12 December 6, 2023 16:58
@nfrisby
Copy link
Contributor Author

nfrisby commented Dec 6, 2023 •
edited
Loading

In Esgen's December absence, @coot and @bolt12 have agreed to review this. We might later decide to also bring in other Consensus members.

Edit: Esgen is back online

@nfrisby nfrisby force-pushed the nfrisby/bootstrap-gsm branch from 59b82d0 to d697c12 Compare January 8, 2024 16:20
@nfrisby
Copy link
Contributor Author

nfrisby commented Jan 8, 2024

I squashed and rebased to origin/main. I'll consider later whether to split the commit up again.

@nfrisby nfrisby self-assigned this Jan 9, 2024
@nfrisby nfrisby force-pushed the nfrisby/bootstrap-gsm branch 2 times, most recently from 6410c06 to ff123a1 Compare January 17, 2024 14:48
amesgen
amesgen reviewed Jan 24, 2024
View reviewed changes
...boros-consensus/src/ouroboros-consensus/Ouroboros/Consensus/MiniProtocol/ChainSync/Client.hs Outdated Show resolved Hide resolved
...boros-consensus/src/ouroboros-consensus/Ouroboros/Consensus/MiniProtocol/ChainSync/Client.hs Outdated Show resolved Hide resolved
...boros-consensus/src/ouroboros-consensus/Ouroboros/Consensus/MiniProtocol/ChainSync/Client.hs Outdated Show resolved Hide resolved
...boros-consensus/src/ouroboros-consensus/Ouroboros/Consensus/MiniProtocol/ChainSync/Client.hs Outdated Show resolved Hide resolved
...boros-consensus/src/ouroboros-consensus/Ouroboros/Consensus/MiniProtocol/ChainSync/Client.hs Show resolved Hide resolved
ouroboros-consensus-diffusion/src/ouroboros-consensus-diffusion/Ouroboros/Consensus/Node/GSM.hs Outdated Show resolved Hide resolved
ouroboros-consensus-diffusion/src/ouroboros-consensus-diffusion/Ouroboros/Consensus/Node/GSM.hs Outdated Show resolved Hide resolved
...boros-consensus/src/ouroboros-consensus/Ouroboros/Consensus/MiniProtocol/ChainSync/Client.hs Show resolved Hide resolved
ouroboros-consensus-diffusion/src/ouroboros-consensus-diffusion/Ouroboros/Consensus/Node/GSM.hs Outdated Show resolved Hide resolved
ouroboros-consensus-diffusion/src/ouroboros-consensus-diffusion/Ouroboros/Consensus/Node/GSM.hs Outdated Show resolved Hide resolved
@dnadales dnadales added this to the Q1 2024 milestone Jan 25, 2024
@nfrisby nfrisby force-pushed the nfrisby/bootstrap-gsm branch 3 times, most recently from 7ca9b27 to c53bf74 Compare January 26, 2024 22:10
@coot
Copy link
Contributor

coot commented Jan 29, 2024

We're merging IntersectMBO/ouroboros-network#4555.

@nfrisby nfrisby force-pushed the nfrisby/bootstrap-gsm branch from c53bf74 to 4de0c49 Compare January 31, 2024 23:49
@nfrisby nfrisby force-pushed the nfrisby/bootstrap-gsm branch 2 times, most recently from d3f7b12 to 858c8d7 Compare February 2, 2024 15:01
@bolt12 bolt12 mentioned this pull request Feb 5, 2024
Closed
@nfrisby nfrisby force-pushed the nfrisby/bootstrap-gsm branch 4 times, most recently from 3612792 to b3feba5 Compare February 9, 2024 18:53
@bolt12 bolt12 linked an issue Feb 20, 2024 that may be closed by this pull request
[FEAT] - Implement relationship between ChurnModeBulkSync and LedgerStateJudgement #958
Closed
@nfrisby nfrisby mentioned this pull request Feb 20, 2024
Closed
@nfrisby
Copy link
Contributor Author

nfrisby commented Feb 20, 2024 •
edited
Loading

Tech debt that will arise from merging this PR:

  • s-r-p stanzas (see Add the pre-Genesis State Machine for bootstrap peers #808 (comment))
  • move the anti-thundering herd delay into the Diffusion Layer, between the two states CaughtUp and OnlyBootstrap
  • regression tests (code review and ad-hoc manual testing has already evidenced that they're currently correct):
    • realMarkerFileView regression tests
    • initializationLedgerJudgement regression tests
    • realDurationUntilTooOld regression tests
    • The ChainSync client should signal idle exactly when and only when the MsgAwaitReply is processed.
    • The ChainSync client subsequently should signal no-longer-idle exactly when and only when the next message is received.
  • switch to quickcheck-dynamic and/or upstream the io-sim-based QSM module
  • use ArbitraryLedgerStateJudgement from ouroboros-network:sim-tests-lib

Edit: migrated to #964

@nfrisby nfrisby force-pushed the nfrisby/bootstrap-gsm branch 2 times, most recently from 09cda31 to 20768c9 Compare February 20, 2024 20:46
@nfrisby nfrisby marked this pull request as ready for review February 20, 2024 20:52
@nfrisby nfrisby requested a review from a team as a code owner February 20, 2024 20:52
@nfrisby nfrisby marked this pull request as draft February 20, 2024 20:52
@nfrisby
Copy link
Contributor Author

nfrisby commented Feb 20, 2024

This is ready for review, except I need to rename the DONOTMERGE commit, since we opted to go with the s-r-ps for now.

@nfrisby nfrisby marked this pull request as ready for review February 20, 2024 20:53
amesgen
amesgen approved these changes Feb 21, 2024
View reviewed changes
Copy link
Member

@amesgen amesgen left a comment

Choose a reason for hiding this comment

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

LGTM, also confirming that #808 (comment) looks good 👍

cabal.project Show resolved Hide resolved
ouroboros-consensus-diffusion/src/ouroboros-consensus-diffusion/Ouroboros/Consensus/Node.hs Outdated Show resolved Hide resolved
@nfrisby nfrisby force-pushed the nfrisby/bootstrap-gsm branch from 20768c9 to 5f49366 Compare February 21, 2024 23:58
@nfrisby nfrisby force-pushed the nfrisby/bootstrap-gsm branch from 5f49366 to 00ff278 Compare February 21, 2024 23:59
@nfrisby nfrisby enabled auto-merge February 21, 2024 23:59
@nfrisby nfrisby mentioned this pull request Feb 22, 2024
Open
10 tasks
@nfrisby nfrisby added this pull request to the merge queue Feb 22, 2024
Merged via the queue into main with commit ca23fa5 Feb 22, 2024
15 checks passed
@nfrisby nfrisby deleted the nfrisby/bootstrap-gsm branch February 22, 2024 00:45
This was referenced Feb 26, 2024
Closed
Closed
@amesgen amesgen mentioned this pull request Jun 24, 2024
Closed
@amesgen amesgen mentioned this pull request Jul 4, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bolt12 bolt12 bolt12 left review comments

@amesgen amesgen amesgen approved these changes

@coot coot Awaiting requested review from coot

Assignees

@nfrisby nfrisby

Labels
None yet
Projects
Consensus Team Backlog
Status: ✅ Done
Milestone
Q1 2024
Development

Successfully merging this pull request may close these issues.

[FEAT] - Implement relationship between ChurnModeBulkSync and LedgerStateJudgement
5 participants
@nfrisby @coot @amesgen @bolt12 @dnadales