Replace caseShelleyToBabbageOrConwayEraOnwards with inEonForShelleyBasedEra - #1438
Merged
Conversation
Jimbo4350
requested review from
carbolymer,
disassembler,
kevinhammond,
newhoggy and
palas
as code owners
September 3, 2026 16:19
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The changes are a mechanical refactor that appears to preserve the same era gating and parser behavior while improving forward compatibility with upcoming cardano-api changes.
Pull request overview
This PR refactors cardano-cli to replace uses of cardano-api’s soon-to-be-removed era case combinator (caseShelleyToBabbageOrConwayEraOnwards) with inEonForShelleyBasedEra (or explicit ShelleyBasedEra matching where both branches need distinct witnesses), preserving the existing era-partitioning logic.
Changes:
- Migrates compatible transaction and governance option parsers to
inEonForShelleyBasedEra(including targeted@ConwayEraOnwardsselection where the eon would otherwise be ambiguous). - Simplifies the governance parser by removing an unnecessary
convertin theobtainCommonConstraintspath. - Rewrites
pUpdateProtocolParametersCmdto explicitly branch onShelleyBasedEraconstructors to retain access to both required witnesses.
File summaries
| File | Description |
|---|---|
| cardano-cli/src/Cardano/CLI/Compatible/Transaction/Run.hs | Replaces the legacy era case combinator with inEonForShelleyBasedEra for protocol-update/vote parsing. |
| cardano-cli/src/Cardano/CLI/Compatible/Transaction/Option.hs | Uses inEonForShelleyBasedEra @ConwayEraOnwards for reference-script and vote-file parsers; adds TypeApplications. |
| cardano-cli/src/Cardano/CLI/Compatible/Governance/Option.hs | Switches governance command gating to inEonForShelleyBasedEra, removes redundant convert, and explicitly matches ShelleyBasedEra for protocol-parameter update command parsing. |
| .changes/replace-era-case-combinators.yml | Adds a changelog fragment documenting the refactor. |
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/replace-era-case-combinators
branch
5 times, most recently
from
September 3, 2026 20:11
283480b to
1b26bde
Compare
palas
approved these changes
Sep 3, 2026
palas
left a comment
palas
left a comment
Contributor
There was a problem hiding this comment.
I am pretty confident this is a pure refactoring (from the point of view of the client). Just a minor comment/suggestion
| , pGovernanceGenesisKeyDelegationCertificate | ||
| , fmap CreateCompatibleProtocolParametersUpdateCmd <$> pGovernanceActionCmds sbe | ||
| ] | ||
| [ inEonForShelleyBasedEra |
Contributor
There was a problem hiding this comment.
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.
4 tasks
…sedEra `caseShelleyToBabbageOrConwayEraOnwards` hands both branches an era witness, which most call sites did not need. Where a branch ignored its witness, this replaces the combinator with `inEonForShelleyBasedEra`. `pUpdateProtocolParametersCmd` looked like it needed both witnesses, but the pre-Conway one was not load-bearing: it only fed a `ShelleyToBabbageEra` into the `eon` field of `UpdateProtocolParametersPreConway`, and nothing ever read that field — the only consumer, `shelleyToBabbageProtocolParametersUpdate`, bound it as `_stB`. The field is dropped, which removes the seven-way match on the `ShelleyBasedEra` constructors that existed only to conjure the witness. The Conway branch keeps its `ConwayEraOnwards`, since `pUpdateProtocolParametersPostConway` needs the `Exp.IsEra` constraint and neither `ShelleyBasedEra` nor `conwayEraOnwardsConstraints` can supply it. With the pre-Conway witness gone the branches are identical apart from which of the two optional payloads they populate, so the shared command scaffolding moves into `mkCmd`. Also folds away the no-op `forShelleyBasedEraMaybeEon` lookup in `pGovernanceActionProtocolParametersUpdateCmd`, and drops the `DataKinds`, `GADTs` and `ScopedTypeVariables` pragmas, which the module no longer needs. No change to the executable's interface: `create-protocol-parameters-update` is still registered for all five pre-Conway eras and for Conway onwards, with identical help output.
…tructors inEonForShelleyBasedEra infers its eon from the branch that consumes the witness. In pCompatibleGovernanceCmds that was the experimental Era, whose membership shrinks as eras are retired: once Conway leaves it, Conway would silently take the pre-Conway branch and lose its governance commands, with no compilation error. Match on the ShelleyBasedEra constructors instead. The branches refine era to a concrete one, so IsEra resolves directly and no experimental witness needs to be threaded through this compatible module. pUpdateProtocolParametersCmd gets the same treatment, dropping its ConwayEraOnwards witness and the convert round trip to the experimental Era.
Jimbo4350
force-pushed
the
jordan/replace-era-case-combinators
branch
from
September 4, 2026 16:58
e102781 to
8f890fb
Compare
Replaces the shared mkCmd helper, which forced both branches to pass a 'pure Nothing' for the payload they do not populate.
Jimbo4350
force-pushed
the
jordan/replace-era-case-combinators
branch
from
September 4, 2026 17:15
8fc3adf to
1722e58
Compare
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
IntersectMBO/cardano-api#1326 removes the era case combinators, including
caseShelleyToBabbageOrConwayEraOnwards. This migrates the call sites here ahead of that.Most sites ignored one or both of the witnesses that combinator hands out, so they become
inEonForShelleyBasedEra.pUpdateProtocolParametersCmdwas the awkward one: it matched on all sevenShelleyBasedEraconstructors, but only to build aShelleyToBabbageErafor theeonfield ofUpdateProtocolParametersPreConway— a field nothing ever read. Dropping the field removes the match. Its two branches then differ only in which optional payload they fill, so the shared scaffolding moves intomkCmd.inEonForShelleyBasedErais already in the released cardano-api, so this can merge before or after the cardano-api PR.How to trust this PR
No change to the CLI's interface — same commands, same help output, which the golden tests cover.
cabal build cardano-cli -j4 cabal test cardano-cli-golden cardano-cli-test -j4Locally: clean build,
cardano-cli-golden815/815,cardano-cli-test73/73, no newhlinthints.Checklist
.changes/