Era handling

[Jimbo4350]

Changelog

- description: |
    Implement Era GADT and UseEra class as an alternative to the existing era handling code
# uncomment types applicable to the change:
  type:
  - feature        # introduces a new feature
  # - breaking       # the API has changed in a breaking way
  - compatible     # the API has changed but is non-breaking
  # - optimisation   # measurable performance improvements
  # - improvement    # QoL changes e.g. refactoring
  # - bugfix         # fixes a defect
  # - test           # fixes/modifies tests
  # - maintenance    # not directly related to the code
  # - release        # related to a new release preparation
  # - documentation  # change in code docs, haddocks...

Context

Additional context for the PR goes here. If the PR fixes a particular issue please provide a link to the issue.

How to trust this PR

Highlight important bits of the PR that will make the review faster. If there are commands the reviewer can run to observe the new behavior, describe them.

Checklist

• Commit sequence broadly makes sense and commits have useful messages
• New tests are added if needed and existing tests are updated. See Running tests for more details
• Self-reviewed the diff

[Jimbo4350]

This and protocolVersionToSbe are temporary and allows us to change cardano-api bit by bit.

[carbolymer]

Will this be our new protocol version thing, or is this planned to be correlated somehow with node-to-client, or shelley node-to-client protocol version?

Should we be naming it protocol version? It's not really a communication protocol version, maybe we should go with era number or something like that?

[Jimbo4350]

It corresponds to protocol versions defined in ouroboros-consensus: https://github.com/input-output-hk/ouroboros-consensus/blob/4322b57bf683cd7ac28bc3e3b9fef3fc30fbc0f2/ouroboros-consensus-cardano/src/shelley/Ouroboros/Consensus/Shelley/Node/Praos.hs#L120

and https://github.com/input-output-hk/ouroboros-consensus/blob/4322b57bf683cd7ac28bc3e3b9fef3fc30fbc0f2/ouroboros-consensus-cardano/src/unstable-cardano-tools/Cardano/Node/Protocol/Cardano.hs#L229

Ideally they would export these as standalone values and we can just use them.

Ledger has min and max supported versions:

-- | Minimum supported version
type MinVersion = 0

-- | Maximum supported version. This is the protocol version of the next upcoming era
type MaxVersion = 10

but they extend over what we really want to support.

[Jimbo4350]

is this planned to be correlated somehow with node-to-client, or shelley node-to-client protocol version?

It's worth exploring this (albeit not right now). It would be useful to have a single type class that gives you these values along with GHC complaining about any unmatched cases.

[carbolymer]

I think this is a good example of my doubts with using "current" and "experimental" as mutable pointers to eras. When we transition between eras by switching eras in SomeProtocolVersion definition, this place will change its behaviour without triggering compilation error.

An ideal solution in my opinion would be to get value-level version and rely on it in the check.

[Jimbo4350]

This is a good point. Looking into this.

[Jimbo4350]

#402 (comment)

[carbolymer]

I like the idea of using type-level naturals, which gives us Ord and Enum.

[carbolymer]

Can you try to force compilation error due to version mismatch, I'm curious how it would look like?

[Jimbo4350]

Just pushed a commit that lets you play with a toy example.

[carbolymer]

with:

  Foo
    :: SupportedProtocolVersionRange PostConwayEra
    => SomeProtocolVersion PostConwayEra

this:

err :: ()
err = (undefined :: SomeProtocolVersion v -> ()) Foo

gives:

internal/Cardano/Api/Protocol/Version.hs:136:50: error: [GHC-64725]
    • Cannot satisfy: PostConwayEra <= MaxSupportedVersion
    • In the first argument of ‘undefined ::
                                  SomeProtocolVersion v -> ()’, namely
        ‘Foo’
      In the expression: (undefined :: SomeProtocolVersion v -> ()) Foo
      In an equation for ‘err’:
          err = (undefined :: SomeProtocolVersion v -> ()) Foo
    |
136 | err = (undefined :: SomeProtocolVersion v -> ()) Foo
    |                

[Jimbo4350]

Yep this is working as intended. PostConwayEra exceeds our upper bound of BabbageEra. We would need to adjust MaxSupportedVersion to include PostConwayEra which is what we would do if we wanted to support PostConwayEra.

[carlhammann]

I agree with everything @carbolymer said. In particular

• I think that the type-level naturals are a really nice idea, and
• I worry like @carbolymer : We'll definitely want the compiler to guide us on a version bump.

With regard to the second point, a few unordered thoughts.

• We might have several types like SomeProtocolVersion, one for each "generation" of supported and experimental version. However, I fear you'll dislike this option, because it reintroduces a proliferation of "eons", which I understand is what you're trying to get rid of.

• A wild idea would be something like

  data WithProtocolVersions (baseversion :: Nat) (t :: Nat -> k)  where
    CurrentVersion :: t baseversion -> WithProtocolVersion baseversion t
    ExperimentalVersion :: t (baseversion + 1) -> WithProtocolVersion baseversion t

  with t :: Nat -> Type type families for everything that we want to depend on versions. Then, we could write something like

  validateTxBodyContent'
    :: WithProtocolVersions BabbageEra (TxBodyContent BuildTx)
    -> Either TxBodyError ()

[Jimbo4350]

We'll definitely want the compiler to guide us on a version bump.

Only the type synonyms will change. Have a look at my most recent commit (the commit message aswell) and play with it to see how GHC complains when the type synonyms are adjusted in the value constructors of SomeProtocolVersion.

• We might have several types like SomeProtocolVersion, one for each "generation" of supported and experimental version.

I believe my most recent commit also deals with this concern unless I'm misunderstanding.

[smelc]

I agree that using type-level naturals seems quite natural and smooth 👍

But I'm wondering how this interacts with code that is going to support older versions than what is in SupportedProtocolVersionRange. Would we also introduce type AlonzoEra = 7 :: Nat and friends? Or would the "old" stuff not be updated with this new system (like the legacy part of cardano-cli?)

[Jimbo4350]

I agree that using type-level naturals seems quite natural and smooth 👍

But I'm wondering how this interacts with code that is going to support older versions than what is in SupportedProtocolVersionRange. Would we also introduce type AlonzoEra = 7 :: Nat and friends? Or would the "old" stuff not be updated with this new system (like the legacy part of cardano-cli?)

So we are operating on the assumption that consumers of cardano-api and cli are only interested in the latest era. Because of this we would not introduce Nats for anything prior to BabbageEra.

[ch1bo]

I like this usage as I will be only required to pick the era once and it stays out of the way when using multiple functions together.

However the signature of classExample with the UseProtocolVersion version is not very clear to me as a user. Especially as I would be picking an Era when using it. As a user I do care about which Era I pick when, e.g. creating a transaction, however I do not really care which protocol version that is. So having a classExample :: forall era. ... would be more clear to me.

One use case we have upcoming is that we want to deliberately stay with BabbageEra when creating transactions, but be able to read BabbageEra or ConwayEra transactions (the latter is not so important as we should be able to deserialize a BabbageEra tx from ConwayEra blocks).

[Jimbo4350]

So there is a relationship between the protocol version and the era but the average user likely does not need to know/care about this. What about a rename to something like:

class UseEra era where 
  useEra :: Era era
  
 data Era era where
  CurrentEra
    :: SupportedEras BabbageEra
    => Era BabbageEra
  UpcomingEra
    :: SupportedEras ConwayEra
    => Era ConwayEra

Users don't need to be aware that a particular protocol version (as a type level Nat) corresponds to an era. They will use the type synonyms.

[Jimbo4350]

One use case we have upcoming is that we want to deliberately stay with BabbageEra when creating transactions, but be able to read BabbageEra or ConwayEra transactions (the latter is not so important as we should be able to deserialize a BabbageEra tx from ConwayEra blocks).

You're interested in this deserialisation for the purposes of experimenting the the ConwayEra correct? After the hardfork, our plan would be to drop support for BabbageEra transactions. However our intention is to support to serialization of Conway txs while mainnet is still Babbage so users/we can experiment.

It would be useful if you could point to your code where you currently using this functionality.

[ch1bo]

You're interested in this deserialisation for the purposes of experimenting the the ConwayEra correct?

Not exactly. We are interested in deserializing Babbage transactions from Conway blocks as our codebase is not fully parameterized on the era, but uses one hard-coded era (currently Babbage) in both: creating and observing transactions. That means, that when building a hydra-node version, we need to pick an era we do talk/understand. We do want to build a hydra-node that works in Babbage and Conway so we would want to build/inspect Babbage transactions (and also expose a Babbage ledger on L2).. until we drop support for Babbage and switch to full Conway. Hope that makes sense? NB: Making our codebase parameterized by era will require us to still decide which era we want to produce transactions for in a given build. Unless we make it configurable (a) or run-time switching (b) .. which we would like to avoid due to unnecessariy complexity in usage (a) or code (b).

It would be useful if you could point to your code where you currently using this functionality.

We are currently only getting started on that, so nothing to point to yet.

[ch1bo]

Regarding the snippet: classExample :: UseEra era => [(TxIn, BuildTxWith BuildTx (UpdatedWitness era))] -> Either TxBodyError () would make more sense to me as a user 👍
Would that mean I use it as classExample @CurrentEra [] and for any given version of cardano-api that would be either Babbage or Conway?

[Jimbo4350]

Your use case makes sense. What I'm proposing will give access to both Babbage and Conway transaction serialization and those will be available until after mainnet has hard forked to Conway. So I think this will be fine.

Would that mean I use it as classExample @CurrentEra [] and for any given version of cardano-api that would be either Babbage or Conway?

It would still be classExample @BabbageEra []. I think we would want some breakage when mainnet is in the ConwayEra and we need to deprecate BabbageEra. Users should be forced to acknowledge the change (a deprecation pragma?). However it would be a simple type synonym change rather than having to update the CurrentEra value constructor. I want to avoid a never ending enumeration of value constructors and instead swap out type synonyms.

[Jimbo4350]

I need to expose the Era constructors

[carbolymer]

But Era line is not exporting them.

Do we really need to export them? I find them to be footguns. It's fine to use them internally for eras infrastructure code, but it seems dangerous to use constructors directly in feature toggling code.

[Jimbo4350]

I envision two interfaces: the typeclass UseEra and the GADT Era. So our functions will either require satisfaction of the constraint or choosing one of the GADT constructors.

[carbolymer]

How about changing Era definition a bit to:

Suggested change

	-- | The era currently active on Cardano's mainnet.
	CurrentEra
	:: SupportedProtocolVersionRange BabbageEra
	=> Era BabbageEra
	-- | The era planned for the next hardfork on Cardano's mainnet.
	UpcomingEra
	:: SupportedProtocolVersionRange ConwayEra
	=> Era ConwayEra
	BabbageEra
	:: SupportedProtocolVersionRange BabbageEra
	=> Era BabbageEra
	ConwayEra
	:: SupportedProtocolVersionRange ConwayEra
	=> Era ConwayEra
	-- | The era currently active on Cardano's mainnet.
	pattern CurrentEra :: Era BabbageEra
	pattern CurrentEra = BabbageEra
	-- | The era planned for the next hardfork on Cardano's mainnet.
	pattern UpcomingEra :: Era ConwayEra
	pattern UpcomingEra = ConwayEra
	{-# COMPLETE CurrentEra, UpcomingEra #-}

and exporting constructors, but not patterns. This could be useful when we would like to rely on era-dependent behaviour, but also have compile-time checks for eras.

[Jimbo4350]

We already have compile time checks for eras via SupportedProtocolVersionRange. The issue with your approach is we now have to be continuously adding and removing data constructors. The patterns seem like an extra unnecessary step because it amounts to the same thing: the user would only experience the change of the type synonym in the pattern definition.

[carbolymer]

The patterns seem like an extra unnecessary step because it amounts to the same thing: the user would only experience the change of the type synonym in the pattern definition.

I would say it's a different thing. My concern is that if an user decides to write a function like:

doThing :: Era era -> ()
doThing = \case
  CurrentEra -> enableFeature
  UpcomingEra -> disableFeature

he won't get any type errors when we switch current eras, but the application will change its behaviour unnoticed.
My point is to not export type constructors representing CurrentEra and UpcomingEra, to prevent pattern matching on eras.

[Jimbo4350]

I see what you're saying. Let me think about this further. As you suggested pattern synonyms with concrete types is likely the way to go.

[Jimbo4350]

#414

[carbolymer]

Those constraints seem redundant. The should be easily recoverable wherever needed.

Suggested change

	:: SupportedProtocolVersionRange BabbageEra
	=> Era BabbageEra
	-- | The era planned for the next hardfork on Cardano's mainnet.
	UpcomingEra
	:: SupportedProtocolVersionRange ConwayEra
	=> Era ConwayEra
	:: Era BabbageEra
	-- | The era planned for the next hardfork on Cardano's mainnet.
	UpcomingEra
	:: Era ConwayEra

[Jimbo4350]

It's not. How else would we force a compiler error in the type synonym if we don't enforce a range of protocol versions?

[carbolymer]

So it's purpose is to trigger a compiler error in case there are constructors, which are outside of the supported version range. Do I understand it correctly?

[Jimbo4350]

So when we modify SupportedProtocolVersionRange's range, users will receive a compilation error saying the current era they are instantiating to is not supported. They will have to update their type synonyms.

[carbolymer]

This type family is fine, but I'm not sure about naming. Here Era means our old Era, but in this module we're using Era also as a name for the new type. This may be a bit confusing for library users, which era type (new version or old) is used in the name. Maybe we should use data Version instead of data Era?

In general, that would be only an issue for the transition period, before the codebase abandons eons usage.

[Jimbo4350]

This is confusing but it will be internal. I'll add a comment. We can come back to the name change.

[Jimbo4350]

I've changed it to VersionToSbe. It's not great but this interface hinges on the fact that protocol versions map to specific eras.