Skip to content

Conversation

lehins
Copy link
Collaborator

@lehins lehins commented Aug 1, 2025

Description

This PR moves all Era type class instances into the internal sublibrary of cardano-ledger-core, which ii believe is an improvement in its own right, since we can now see all eras and their expected protocol versions in one place. However, the bigger reason why this change is introduced is to improve how we define constraints for functionality that should be available up to or starting from a specific era. This is achieved by introduction of a type level name for each era, that can safely be used to infer protocol version, without exposing access to the actual era types too early in the hierarchy.

Checklist

  • Commits in meaningful sequence and with useful messages.
  • Tests added or updated when needed.
  • CHANGELOG.md files updated for packages with externally visible changes.
    NOTE: New section is never added with the code changes. (See RELEASING.md).
  • Versions updated in .cabal and CHANGELOG.md files when necessary, according to the
    versioning process.
  • Version bounds in .cabal files updated when necessary.
    NOTE: If bounds change in a cabal file, that package itself must have a version increase. (See RELEASING.md).
  • Code formatted (use scripts/fourmolize.sh).
  • Cabal files formatted (use scripts/cabal-format.sh).
  • CDDL files are up to date (use scripts/gen-cddl.sh)
  • hie.yaml updated (use scripts/gen-hie.sh).
  • Self-reviewed the diff.

@lehins lehins changed the title Re-arrange era definitions Re-arrange Era instances Aug 1, 2025
@lehins lehins force-pushed the lehins/rearrange-era-definitions branch from 2caec03 to 5d41e3d Compare August 1, 2025 16:48
@lehins lehins force-pushed the lehins/rearrange-era-definitions branch 2 times, most recently from 5145894 to 059cd2c Compare September 3, 2025 21:38
@lehins lehins marked this pull request as ready for review September 3, 2025 21:38
@lehins lehins requested a review from a team as a code owner September 3, 2025 21:38
@lehins lehins force-pushed the lehins/rearrange-era-definitions branch from 059cd2c to cd1e383 Compare September 3, 2025 21:46
Copy link
Contributor

@teodanciu teodanciu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I admit the relationship between the new EraHasName and Era is over my head.
But the result is great - and also find it useful that all Era definitions are now in the same place - i seem to be looking at these definitions - small as they are- quite often, its good that they're much easier to find.
Looks good to me

Copy link
Contributor

@aniketd aniketd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just have questions 😅 . Looks great!

@lehins lehins force-pushed the lehins/rearrange-era-definitions branch from 98159fb to dfa8ace Compare September 4, 2025 20:24
@lehins lehins enabled auto-merge September 4, 2025 20:25
@lehins lehins merged commit aa656a5 into master Sep 4, 2025
122 of 123 checks passed
@lehins lehins deleted the lehins/rearrange-era-definitions branch September 4, 2025 22:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants