Skip to content

Drop redundant ProtocolParametersUpdate conversion in shelleyToBabbage update#1373

Merged
Jimbo4350 merged 1 commit into
masterfrom
jordan/migrate-EraBasedProtocolParametersUpdate
May 13, 2026
Merged

Drop redundant ProtocolParametersUpdate conversion in shelleyToBabbage update#1373
Jimbo4350 merged 1 commit into
masterfrom
jordan/migrate-EraBasedProtocolParametersUpdate

Conversation

@Jimbo4350
Copy link
Copy Markdown
Contributor

@Jimbo4350 Jimbo4350 commented May 5, 2026

Changelog

- description: |
    Drop the redundant `createEraBasedProtocolParamUpdate` /
    `fromLedgerPParamsUpdate` round-trip in
    `shelleyToBabbageProtocolParametersUpdate`; pass
    `EraBasedProtocolParametersUpdate era` straight to
    `makeShelleyUpdateProposal`.
# uncomment types applicable to the change:
  type:
   - refactoring    # QoL changes
# uncomment at least one main project this PR is associated with
  projects:
   - cardano-cli

Context

makeShelleyUpdateProposal (cardano-api ^>=11.1, already required on master) accepts EraBasedProtocolParametersUpdate era directly. The existing code in shelleyToBabbageProtocolParametersUpdate constructed that value, converted it to the deprecated ProtocolParametersUpdate via createEraBasedProtocolParamUpdate / fromLedgerPParamsUpdate, then passed the result to makeShelleyUpdateProposal — an unnecessary round-trip through the old API.

No behaviour change: the function still produces the same UpdateProposal era text envelope.

How to trust this PR

  • cabal build cardano-cli -j4 builds cleanly.
  • Test.Cli.Compatible.Transaction.Build exercises compatible <era> governance action create-protocol-parameters-update for shelley/allegra/mary/alonzo/babbage and diffs each against a reference produced via the same code path — passing means the produced text envelope is byte-identical to before.

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

Comment thread cardano-cli/src/Cardano/CLI/Compatible/Json/Friendly.hs Fixed
@Jimbo4350 Jimbo4350 force-pushed the jordan/migrate-EraBasedProtocolParametersUpdate branch from 7fa5ac0 to 69c471d Compare May 5, 2026 20:39
@Jimbo4350 Jimbo4350 marked this pull request as ready for review May 11, 2026 20:26
Copilot AI review requested due to automatic review settings May 11, 2026 20:26
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates cardano-cli to match cardano-api’s removal of ProtocolParametersUpdate, switching rendering and governance flows to the era-indexed replacement while keeping the existing “friendly” JSON/YAML output shape for protocol parameter updates.

Changes:

  • Bump cardano-api dependency and adjust code to use era-indexed protocol parameter update types.
  • Rewrite friendlyProtocolParametersUpdate to render from ledger PParamsUpdate produced from EraBasedProtocolParametersUpdate.
  • Add a golden test + fixtures to pin the YAML output shape for an Alonzo update proposal embedded in a transaction.

Reviewed changes

Copilot reviewed 8 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
flake.lock Updates pinned CHaP revision/hash to align with dependency changes.
cabal.project Advances CHaP index-state to pick up the newer package set.
cardano-cli/cardano-cli.cabal Bumps cardano-api bound and adds cardano-ledger-binary dependency for version rendering.
cardano-cli/src/Cardano/CLI/Read.hs Adds shelleyToBabbageEraConstraints to satisfy new era-indexed UpdateProposal envelope constraints.
cardano-cli/src/Cardano/CLI/Compatible/Json/Friendly.hs Refactors protocol-parameter-update rendering to dispatch by era and emit the legacy flat key set.
cardano-cli/src/Cardano/CLI/Compatible/Governance/Run.hs Removes a redundant conversion round-trip when building Shelley-to-Babbage update proposals.
cardano-cli/test/cardano-cli-golden/Test/Golden/TxView.hs Adds a new golden test that generates and views an Alonzo tx with an update proposal and compares YAML output.
cardano-cli/test/cardano-cli-golden/files/input/genesis1.vkey Adds a genesis verification key fixture used by the new golden test.
cardano-cli/test/cardano-cli-golden/files/golden/alonzo/transaction-view-update-proposal.yaml Adds the expected YAML output for the new golden test.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

protocolVersionPair ppu =
[ ( \(Ledger.ProtVer major minor) ->
"protocol version" .= (textShow (getVersion major :: Word) <> "." <> textShow minor)
)
@Jimbo4350 Jimbo4350 force-pushed the jordan/migrate-EraBasedProtocolParametersUpdate branch from 0268b1d to f029bcb Compare May 12, 2026 20:41
`makeShelleyUpdateProposal` (cardano-api ^>=11.1) accepts
`EraBasedProtocolParametersUpdate era` directly. The existing code in
`shelleyToBabbageProtocolParametersUpdate` constructed that value,
converted it to the deprecated `ProtocolParametersUpdate` via
`createEraBasedProtocolParamUpdate` / `fromLedgerPParamsUpdate`, and
passed the result to `makeShelleyUpdateProposal` — an unnecessary
round-trip through the old API.

Drop the conversion; pass `eraBasedPParams` directly. No behaviour
change.
@Jimbo4350 Jimbo4350 force-pushed the jordan/migrate-EraBasedProtocolParametersUpdate branch from f029bcb to 30a1ce9 Compare May 12, 2026 21:04
@Jimbo4350 Jimbo4350 changed the title Migrate to EraBasedProtocolParametersUpdate Drop redundant PParamsUpdate round-trip in update proposal flow May 12, 2026
@Jimbo4350 Jimbo4350 changed the title Drop redundant PParamsUpdate round-trip in update proposal flow Drop redundant ProtocolParametersUpdate conversion in shelleyToBabbage update May 12, 2026
Copy link
Copy Markdown
Contributor

@palas palas left a comment

Choose a reason for hiding this comment

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

👍

@Jimbo4350 Jimbo4350 enabled auto-merge May 13, 2026 13:39
@Jimbo4350 Jimbo4350 added this pull request to the merge queue May 13, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks May 13, 2026
@Jimbo4350 Jimbo4350 added this pull request to the merge queue May 13, 2026
Merged via the queue into master with commit 9ca9c9b May 13, 2026
29 of 30 checks passed
@Jimbo4350 Jimbo4350 deleted the jordan/migrate-EraBasedProtocolParametersUpdate branch May 13, 2026 16:24
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.

5 participants