consensus: bugfix in HFC.reconstructSummary

[nfrisby]

One commit PR, see its message.

[nfrisby]

This PR is draft because I think the new code below is correct, but I'm in the process of convincing myself (and en passant writing missing documentation). In particular, I need to understand exactly when TransitionKnown, TransitionUnknown, and in particular TransitionImpossible both can and cannot arise.

This code dates back to Edsko's original PR #2034 two years ago, and this code unfortunately didn't get any discussion that archived on that PR (Thomas and Edsko would review on Meet and write-down TODOs -- it's possible they talked about it without writing a comment on the PR).

The other aspect of due diligence is to catalog the downstream consequences of this change. Which is unfortunately kind of broad -- for those with access, you can see the use-def tree here https://input-output.atlassian.net/browse/CAD-4296?focusedCommentId=91616

[nfrisby]

This go (ExactlyCons params ExactlyNil) (TZ Current{..}) case is the buggy one. Just because it's the last era the code knows about doesn't mean there won't be another era. My intuition is that it should instead be treated just like the go (ExactlyCons params (ExactlyCons nextParams _)) (TZ Current{..}) case below.

By removing this case, we will be changing many kinds of downstream behaviors: eg local clients can send the GetInterpreter query (aka QueryEraHistory in cardano-node repo) in order to be able to do their own slot<->time translations. Because of this bug, such requests while in the last known era currently have no time horizon (they'll happily translate slot<->time a million years in the future), but as of the bugfix they response to that query (and everything else downstream of reconstructSummary) will have the intended finite horizon (mostly "one stability window").

In particular, it's possible some SPO has written their own client and is unfortunately depending on this buggy unlimited translation range during Alonzo.

[nfrisby]

cc @kevinhammond

[nfrisby]

I tried to fix the branch name, and GitHub closed the PR. I'm restoring and reopening :(

[nfrisby]

At this point, these four commits are my attempt to refactor/bugfix what was there, comprehensively. The last two commits are mainly driven by trying to preserve the ability to have an unbounded final era. That was a feature of the original design, and so I've tried here to retain it, but I'm not 100% sure we actually need it. So maybe we could replace those last two commits with something simpler. Even so, I think those last two commits demonstrate some key learnings.

In particular, these two occurrences of TransitionImpossible are the only two that I don't know see a way to simply eliminate and both rely on the notion that a "degenerate, single era" use of the HFC is to be unbounded. I think that notion assumes that the single block type alone, would also be unbounded. Which clearly wasn't the case with Byron, but our "lesson learned" is to just use the HFC, even with a single block (but not in a degenerate way), even when you're chain is just starting and you only have the one era. In other words, make an open-world assumption that you'll have another era (and thus a bounded forecast range) even when you don't initially plan on having more eras. We had to do some hacky stuff to make up for the fact that Byron was not future-proof in that way.

https://github.com/input-output-hk/ouroboros-network/blob/0a3a61ae80244dde943843fd39ef17cd85260980/ouroboros-consensus/src/Ouroboros/Consensus/HardFork/Combinator/Embed/Unary.hs#L75-L81

https://github.com/input-output-hk/ouroboros-network/blob/0a3a61ae80244dde943843fd39ef17cd85260980/ouroboros-consensus/src/Ouroboros/Consensus/HardFork/Combinator/Embed/Unary.hs#L340-L353

https://github.com/input-output-hk/ouroboros-network/blob/0a3a61ae80244dde943843fd39ef17cd85260980/ouroboros-consensus/src/Ouroboros/Consensus/HardFork/Combinator/Embed/Unary.hs#L564-L570

I think there are two primary paths forward:

• never have unbounded forecast ranges, even for a "degenerate" HFC application

• add a new first parameter to the HardForkBlock that indicates whether the era list argument is open or closed (approx. final, a la Java's keyword) and probably add a matching related index to TransitionInfo etc

[nfrisby]

never have unbounded forecast ranges, even for a "degenerate" HFC application

I chatted with Duncan on Slack, and he said something similar and a bit stronger:

HardForkBlock [blk] being indistinguishable from blk was a useful property. Perhaps that property can be preserved by saying that even for a single non-HFC block type that there's a limited time horizon.
I think it's a much more reasonable assumption to make to assume that there could be change in future than to assume there will never be change.

And no, I also cannot think of when we'd need an unbounded epoch info.
Well, ok there are some client use cases where it's useful to be able to make that assumption locally (and understand and accept the consequences), but I can't think of places within the node we want that.

I agree, if it all, it should be opt-in (via eg IntersectMBO/cardano-base#277).

[nfrisby]

Closed -- see PR #3754 instead.