Haddocks for certain aspects of the HFC status quo #428
Conversation
|
I can split out the first commit if desired |
| data EraTranslation xs = EraTranslation { | ||
| -- | For each pair @(x, y)@ of subsequent eras, describe how to construct | ||
| -- the initial ledger state for @y@ from the last ledger state in @x@. | ||
| -- | ||
| -- When ticking across an era boundary, the HFC will first invoke this and | ||
| -- then tick the resulting ledger state (in @y@) to the requested slot. | ||
| -- | ||
| -- The resulting ledger state must summarize every relevant aspect of what | ||
| -- came before the new era. This is intentionally vague; for example, | ||
| -- ticking in @y@ might work rather differntly than in @x@, and so certain | ||
| -- aspects of the ticking logic of @x@ might need to happen as part of | ||
| -- 'translateLedgerState'. For a concrete example in Cardano, see | ||
| -- 'translateLedgerStateBabbageToConwayWrapper'. | ||
| translateLedgerState :: InPairs (RequiringBoth WrapLedgerConfig (Translate LedgerState)) xs | ||
| -- | For each pair @(x, y)@ of subsequent eras, describe how to construct | ||
| -- the initial chain-dependent state for @y@ from the last chain-dep state | ||
| -- in @x@. | ||
| -- | ||
| -- When ticking across an era boundary, the HFC will first invoke this and | ||
| -- then tick the resulting chain-dep state (in @y@) to the requested slot. | ||
| , translateChainDepState :: InPairs (RequiringBoth WrapConsensusConfig (Translate WrapChainDepState)) xs |
There was a problem hiding this comment.
Do we want to rename these? I am a bit unsure whether sth like constructInitialLedgerState is really more clear, but OTOH, translateLedgerState is maybe too generic?
There was a problem hiding this comment.
I would be in favor of renaming this to something more clear, that conveys not only in what context this will be used (as it is now with translateX) but also what it actually does (as it is in your proposed constructInitialLedgerState).
There was a problem hiding this comment.
Yes, I also think renaming to something much more concrete than "translate" is important.
There was a problem hiding this comment.
I'm Requesting Changes because:
- I don't think we should delay renaming the
translate*functions. - I think we can eliminate some ambiguity in the new Haddock.
- The above renaming reminded me about the
Ouroboros.Consensus.Protocol.Translatemodule; I figure it needs the same treatment you're givingCanHardForkhere.
| -- | ||
| -- with inequality exactly if we ticked across an era boundary. | ||
| -- | ||
| -- == Ticking alone can't reveal era transitions |
There was a problem hiding this comment.
The wording of this subheading seems off to me:
Ticking alone can't reveal era transitions
It seems like there are two things worth explaining/referencing:
-
The transition must have been predicted by
singleEraTransitionat least one safe zone before it happened. -
Ticking is currently unable to change the output of
singleEraTransition.
Right now, the text of this subheading could be interpreted as either of those two things. Is that ambiguity intentional?
Also, the text seems to call the first one a "reflection" of the second one, which I hesitate to agree with.
There was a problem hiding this comment.
Ticking is currently unable to change the output of
singleEraTransition.
This is what I wanted to talk about here in this subsection, but I didn't phrase it like this as you can't apply singleEraTransition to the output of ticking.
I didn't intend to talk about safe zones here (but that could definitely happen in a different subsection, will add that 👍). In my mind, these two topics are orthogonal:
- This subsection is about how era transitions can become known (currently, only via block application, not via ticking).
- Safe zones are about when an era transition can become known.
Also, the text seems to call the first one a "reflection" of the second one, which I hesitate to agree with.
Yeah, I didn't want to imply that, hopefully, separating safe zones to its own section will make that more clear.
...ros-consensus/src/ouroboros-consensus/Ouroboros/Consensus/HardFork/Combinator/Translation.hs
Outdated
Show resolved
Hide resolved
...ros-consensus/src/ouroboros-consensus/Ouroboros/Consensus/HardFork/Combinator/Translation.hs
Outdated
Show resolved
Hide resolved
...ros-consensus/src/ouroboros-consensus/Ouroboros/Consensus/HardFork/Combinator/Translation.hs
Outdated
Show resolved
Hide resolved
| data EraTranslation xs = EraTranslation { | ||
| -- | For each pair @(x, y)@ of subsequent eras, describe how to construct | ||
| -- the initial ledger state for @y@ from the last ledger state in @x@. | ||
| -- | ||
| -- When ticking across an era boundary, the HFC will first invoke this and | ||
| -- then tick the resulting ledger state (in @y@) to the requested slot. | ||
| -- | ||
| -- The resulting ledger state must summarize every relevant aspect of what | ||
| -- came before the new era. This is intentionally vague; for example, | ||
| -- ticking in @y@ might work rather differntly than in @x@, and so certain | ||
| -- aspects of the ticking logic of @x@ might need to happen as part of | ||
| -- 'translateLedgerState'. For a concrete example in Cardano, see | ||
| -- 'translateLedgerStateBabbageToConwayWrapper'. | ||
| translateLedgerState :: InPairs (RequiringBoth WrapLedgerConfig (Translate LedgerState)) xs | ||
| -- | For each pair @(x, y)@ of subsequent eras, describe how to construct | ||
| -- the initial chain-dependent state for @y@ from the last chain-dep state | ||
| -- in @x@. | ||
| -- | ||
| -- When ticking across an era boundary, the HFC will first invoke this and | ||
| -- then tick the resulting chain-dep state (in @y@) to the requested slot. | ||
| , translateChainDepState :: InPairs (RequiringBoth WrapConsensusConfig (Translate WrapChainDepState)) xs |
There was a problem hiding this comment.
Yes, I also think renaming to something much more concrete than "translate" is important.
Co-authored-by: Nicolas Frisby <[email protected]>
Related to #420, in particular #413 and #345