-
Notifications
You must be signed in to change notification settings - Fork 720
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
Integrate o-network changes for Genesis support / big ledger peer snapshots #5787
base: master
Are you sure you want to change the base?
Conversation
b281d8f
to
7a66082
Compare
cardano-node/src/Cardano/Node/Run.hs
Outdated
(\io_m_lps -> do | ||
m_lps <- io_m_lps |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not camelcase?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I though io_m_lps was a little clearer than ioMLps, or do you have something else in mind?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Having to prefix the variables with the types is usually a sign that you're working against the language. Here it is because you're trying to shoehorn your function into MaybeT
which doesn't work here very well and forces you to work with variables with type IO (Maybe a)
outside of MaybeT
. Why not write this function like this:
updateLedgerPeerSnapshot :: Tracer IO (StartupTrace blk)
-> STM IO (Maybe PeerSnapshotFile)
-> (Maybe LedgerPeerSnapshot -> STM IO ())
-> IO (Maybe LedgerPeerSnapshot)
updateLedgerPeerSnapshot startupTracer readLedgerPeerPath writeVar = do
mPeerSnapshotFile <- atomically readLedgerPeerPath
mLedgerPeerSnapshot <- forM mPeerSnapshotFile $ \f -> do
lps@(LedgerPeerSnapshot (wOrigin, _)) <- readPeerSnapshotFile f
lps <$ traceWith startupTracer (LedgerPeerSnapshotLoaded wOrigin)
atomically . writeVar $ mLedgerPeerSnapshot
pure mLedgerPeerSnapshot
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that's most intuitive.
Maybe I don't have to bump the protocol version at all in this patch based on review comment in related PR on consensus side |
7a66082
to
c10e12e
Compare
This PR is stale because it has been open 45 days with no activity. |
c10e12e
to
6b9616c
Compare
6b9616c
to
70b3121
Compare
c427e76
to
982cd1d
Compare
7050b6a
to
1286ce8
Compare
Give more context for incompatibility between bootstrap peers and new Genesis syncing mechanism.
1286ce8
to
8797a16
Compare
Description
This change introduces support for big ledger peers in the node. A new optional entry in network topology JSON parser is added that is intended to point to a path containing a serialized snapshot of big ledger peers taken from some slot a priori. When present, this file is decoded at node startup, or when a SIGHUP is triggered, and made available to the diffusion layer via reading from a TVar.
Checklist
See Runnings tests for more details
CHANGELOG.md
for affected package.cabal
files are updatedhlint
. See.github/workflows/check-hlint.yml
to get thehlint
versionstylish-haskell
. See.github/workflows/stylish-haskell.yml
to get thestylish-haskell
versionghc-8.10.7
andghc-9.2.7
Note on CI
If your PR is from a fork, the necessary CI jobs won't trigger automatically for security reasons.
You will need to get someone with write privileges. Please contact IOG node developers to do this
for you.