Repository navigation
Introduce the Praos2 protocol: ie Praos plus the Leios overlay - #2386
Conversation
ch1bo
left a comment
ch1bo
left a comment
There was a problem hiding this comment.
I only request changes because I want this PR to be done on top of #2354, so we don't do things double (fixing CDDL, passing era into mkHeader, drop seedInitialStakeSnapshots, ...)
The other comments are only Should's.
In any case: we must resolve this today and move on
jasagredo
left a comment
jasagredo
left a comment
There was a problem hiding this comment.
I also don't enjoy Praos2, I preferred PraosLeios or PraosWithLeios or Leios.
jasagredo
left a comment
jasagredo
left a comment
There was a problem hiding this comment.
Discussed both PRs with Nick on a call and I am approving the combination of both. Thanks!
ba337d7 to
fe6b559
Compare
This commit is best reviewed with `git diff -w`, to suppress the whitespace changes. This incurs some duplication (notably signatures), but it's worth it. - The duplication is not overly burdensome to compensate for by factoring the Praos impl so its types and functions can be reused as much as possible. For Leios, at least, that's easy since Leios only adds a few independent fields to the header semantics. - Duplicating instances between Praos and PraosWithLeios instead of having them share some instances (ala a shared-head `BasePraos leiosFlag` protocol type) has a couple benefits. First, most code outside of the PolyPraos functions are either monomorphic or reuses the existing parameterizations over `proto`, which is already ubiquitous. Second, it avoids _implicit_ reuse of today's Praos's rules for Leios, which makes _accidental_ reuse less likely---compare to type class method defaults. We do _not_ duplicate more than we need to, though. In particular, many of Praos's data types and some of its classes gain a `proto` parameter, which is used so that the single data type definition can be reused for Praos with and without Leios. The benefit is that there is just one constructor/field per Praos concept, regardless of whether Leios is enabled. This is not a fully modular design: subsequent additional extensions will also need to add to these same definitions (eg adding fields for Ouroboros Phalanx). That is intentional. This code does not need to be classically extensible, since there is, unfortunately, no such thing as an extensible security proof. Our protocol changes are well studied before implemented, and have never happened concurrently. In other words: it's a very important benefit that there is _one definition_ to look at in order to see everything all of the Praos extensions _cumulatively_ do. The type-level DSL used to isolate extension components is simple and legible; see the `LeiosOnly` data family.
This prepares for the PolyPraos definitions to be defined before Praos.
This commit is merely reorg.
Co-authored-by: Sebastian Nagel <sebastian.nagel@ncoding.at>
fe6b559 to
ca727c6
Compare
|
nfrisby
changed the title
nfrisby
changed the title
|
I've resolved all comments, somewhat unilaterally. We didn't reach unanimity be we did reach plurality :/ |
|
@ch1bo said
I just looked through I had misunderstood the scope of your PR; didn't realize there were parts unrelated to the divergent approach---I should have developed on top, I agree. The main benefit of having not developed on top of another PR is that I was able to minimize the first commit's diff against |
| transLeiosLS :: | ||
| LedgerState (ShelleyBlock (Praos c) ConwayEra) mk -> | ||
| LedgerState (ShelleyBlock (Praos2 c) ConwayEra) mk | ||
| transLeiosLS (ShelleyLedgerState wo nes st tb) = |
There was a problem hiding this comment.
Nitpick: Naming is off here
The protocols have no Leios in their name, so why should this be called transLeiosLS?
|
I did a pass through PR #2354, since I interrupted its in-progress review. I replied to the open threads there. I don't think any (yet?)deserve follow-up work to this merged PR. |
3 participants
This is an alternative to PR #2354
The key differences are that we don't define the new protocol by composing various Praos pieces with their various Leios addenda. Instead, we have one holistic definition of "PolyPraos" and then the Praos and Praos2 (ie Praos plus the Leios extension) variants are defined in terms of that one. Notably, and by design, Praos2 is essentially PolyPraos: PolyPraos will always be the richest/"latest and greatest"/fully-extended Praos variant running on Cardano. But it's defined in such a way that the previous Praos variants (starting after TPraos) can also be defined in terms of it.
The first commit is best read using
diff -w(or the equivalent setting the PR's settings cog): it most directly shows that the old Praos methods have been floated out to become PolyPraos definitions. The subsequent commits then merely reorg around that idea.