Remove the era case combinators in favour of inEonForEra - #1326
Merged
Jimbo4350 merged 5 commits intoSep 4, 2026
Merged
Conversation
Jimbo4350
requested review from
CarlosLopezDeLara,
carbolymer,
disassembler,
erikd and
palas
as code owners
September 3, 2026 15:28
Jimbo4350
force-pushed
the
jordan/remove-case-shelley-era-only-or-allegra-era-onwards
branch
from
September 3, 2026 15:30
32a8d98 to
80f0897
Compare
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The refactor cleanly removes an internal-only combinator, updates all call sites, and preserves the Shelley vs. Allegra-onwards partitioning without introducing API or behavioral changes.
Pull request overview
This PR refactors internal era-case handling by removing the internal caseShelleyEraOnlyOrAllegraEraOnwards combinator from Cardano.Api.Era.Internal.Case and rewriting its former call sites to use inEonForShelleyBasedEra (when no Shelley witness is needed) or direct ShelleyBasedEra constructor matching (when a Shelley witness is required). This keeps behavior the same while reducing bespoke combinators and simplifying call-site intent.
Changes:
- Removed
caseShelleyEraOnlyOrAllegraEraOnwardsfromCardano.Api.Era.Internal.Case. - Updated
fromLedgerTxValidityLowerBoundandfromLedgerTxAuxiliaryDatato useinEonForShelleyBasedErawith a Shelley default value. - Reworked
invalidHereAfterTxBodyLto match directly onShelleyBasedEraconstructors, using a localAllegraEraOnwardshelper to avoid repeating the Allegra-onwards lens chain.
File summaries
| File | Description |
|---|---|
cardano-api/src/Cardano/Api/Tx/Internal/Body/Lens.hs |
Replaces the removed combinator usage with explicit ShelleyBasedEra matching and a local helper for the Allegra-onwards lens path. |
cardano-api/src/Cardano/Api/Tx/Internal/Body.hs |
Switches two call sites to inEonForShelleyBasedEra to select a Shelley default vs. Allegra-onwards witness-based logic. |
cardano-api/src/Cardano/Api/Era/Internal/Case.hs |
Removes the internal caseShelleyEraOnlyOrAllegraEraOnwards function and its exports/imports. |
.changes/remove-case-shelley-era-only-or-allegra-era-onwards.yml |
Adds a refactoring changelog fragment describing the internal removal and call-site rewrites. |
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Jimbo4350
force-pushed
the
jordan/remove-case-shelley-era-only-or-allegra-era-onwards
branch
from
September 3, 2026 15:39
80f0897 to
ac923b4
Compare
Jimbo4350
changed the title
Jimbo4350
force-pushed
the
jordan/remove-case-shelley-era-only-or-allegra-era-onwards
branch
from
September 3, 2026 15:50
ac923b4 to
6eb957a
Compare
Jimbo4350
changed the title
Jimbo4350
force-pushed
the
jordan/remove-case-shelley-era-only-or-allegra-era-onwards
branch
from
September 3, 2026 15:58
6eb957a to
b1c2cf1
Compare
Jimbo4350
changed the title
4 tasks
Jimbo4350
force-pushed
the
jordan/remove-case-shelley-era-only-or-allegra-era-onwards
branch
from
September 3, 2026 18:15
b1c2cf1 to
9316e8a
Compare
palas
approved these changes
Sep 4, 2026
palas
left a comment
palas
left a comment
Contributor
There was a problem hiding this comment.
All good except for the shrinking Era issue
| ) | ||
| ( \w -> | ||
| obtainCommonConstraints (convert w) $ | ||
| obtainCommonConstraints w $ |
Contributor
There was a problem hiding this comment.
Same problem as here in all this file:
Because of the use of inEonForShelleyBasedEra, when Conway goes out of Era, then it will lose the conway related commands (like drep), and we won't get any compilation error that warns about it here.
So maybe we should do something like postConway below.
Jimbo4350
force-pushed
the
jordan/remove-case-shelley-era-only-or-allegra-era-onwards
branch
3 times, most recently
from
September 4, 2026 15:21
12f2690 to
26fadf5
Compare
Two of the three call sites discarded the `ShelleyEraOnly` witness entirely, so they are expressed directly with `inEonForShelleyBasedEra` and an `AllegraEraOnwards` default. `invalidHereAfterTxBodyL` needs the `ShelleyEraOnly` witness in the Shelley branch to reach `ttlAsInvalidHereAfterTxBodyL`, which `inEonForShelleyBasedEra` cannot supply, so it now matches on the `ShelleyBasedEra` constructors directly.
It had no call sites left, and its own comment marked it for deletion once `build-raw --byron-era` was deprecated in cardano-cli. Callers needing the same split can use `inEonForEra` with `ShelleyBasedEra`, which is what the `Cardano.Api.Network.IPC` haddock example now shows.
…wards Sixteen of the nineteen call sites ignored one of the two witnesses, so they are expressed with `inEonForShelleyBasedEra` and a default for the eras outside the eon. Unlike the case combinator, `inEonForShelleyBasedEra` hands the callback a witness but no constraints, so `maybeFromLedgerTxUpdateProposal` now calls `shelleyToBabbageEraConstraints` itself. In `toConsensusQueryShelleyBased` the eleven Conway-onwards queries go through a local `conwayOnwards` witness which matches on `ShelleyBasedEra` exhaustively instead of using an eon, so adding or retiring an era is a compile error at that match rather than a silent fall through to the unsupported branch. The witness is `ConwayEraOnwards` and the constraints come from `conwayEraOnwardsConstraints`, so these queries no longer depend on the experimental `Era`, whose constructors only cover the currently supported eras. `nextEpochEligibleLeadershipSlots` needs a visible type application because neither branch mentions the witness.
The three remaining call sites are certificate generators whose pre-Conway branch needs `ShelleyEraTxCert` and whose Conway branch needs `ConwayEraTxCert`. `inEonForShelleyBasedEra` supplies a witness but no constraints, and its default argument has no witness at all, so these match on the `ShelleyBasedEra` constructors and obtain the constraints from the witness in each branch. That empties `Cardano.Api.Era.Internal.Case`, so the module goes too.
Jimbo4350
force-pushed
the
jordan/remove-case-shelley-era-only-or-allegra-era-onwards
branch
from
September 4, 2026 17:13
26fadf5 to
df6b4ac
Compare
Jimbo4350
deleted the
jordan/remove-case-shelley-era-only-or-allegra-era-onwards
branch
September 4, 2026 19:27
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
Removes all three era case combinators and the
Cardano.Api.Era.Internal.Casemodule they lived in. Call sites useinEonForEra/inEonForShelleyBasedEra, or match on the era constructors where both branches need era constraints.caseShelleyEraOnlyOrAllegraEraOnwards— internal only, never re-exported. Two of three call sites ignored theShelleyEraOnlywitness; the third,invalidHereAfterTxBodyL, needs it forttlAsInvalidHereAfterTxBodyLand now matches on constructors.caseByronOrShelleyBasedEra— no call sites left, and its own comment marked it for deletion oncebuild-raw --byron-erawas deprecated in cardano-cli. TheCardano.Api.Network.IPChaddock example now showsinEonForErainstead.caseShelleyToBabbageOrConwayEraOnwards— 16 of 19 call sites ignored one of the two witnesses. IntoConsensusQueryShelleyBasedthe eon is the experimentalEra, which is whatobtainCommonConstraintswants, so 11convertcalls go away too. The last three are certificate generators inTest.Gen.Cardano.Api.TypedneedingShelleyEraTxCertpre-Conway andConwayEraTxCertafter; they match on constructors.Note that
inEonForShelleyBasedErahands the callback a witness but no constraints, unlike the case combinators' rank-2 arguments — so a few sites now callshelleyToBabbageEraConstraints/obtainCommonConstraintsexplicitly.Downstream: cardano-cli has 5 uses of
caseShelleyToBabbageOrConwayEraOnwards(Compatible/Transaction/Run.hs,Compatible/Transaction/Option.hs,Compatible/Governance/Option.hs) that will need the same treatment. All butpRefScriptFpdiscard a witness, so they map straight ontoinEonForShelleyBasedEra.How to trust this PR
No behaviour change: each branch of the removed combinators maps one-to-one onto the new code, and the era partitions are unchanged.
cabal build all -j4 --enable-tests cabal test cardano-api-test cardano-api-golden -j4Locally: clean build under
-Werror, 259/259 and 125/125 tests pass.cardano-rpc-testfails to build onmasteralready (Not in scope: T.show,Test/Cardano/Rpc/Pagination.hs:89) — untouched here.Checklist
.changes/