Skip to content

Conversation

@teodanciu
Copy link
Contributor

@teodanciu teodanciu commented Feb 6, 2026

Description

Some PParams are doing unnecessary conversions via lenses. This is an attempt to simplify and match were possible with the type, while maintaining the safety of the current version.

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.

, ppUpdate = Nothing
}

conwayPPCollateralPercentage :: PParam ConwayEra
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lehins - what do you think of this approach?

The downside is of course that we'd have to duplicate the definition for Dijkstra and future eras, which kinda defeats the purpose of PParam mechanism.

Another way I was thinking of is to allow specifying something like setFromCBOR in PParam, in order to decouple the lens from the decoder, but this would also lead to different definitions of the PParam.

The root of the difficulty is the fact that hkdCollateralPercentageL is defined in AlonzoEraPParams -
I experimented with turning it off after Babbage (with AtMostEra) and adding a new field to ConwayEraPParams, but the problem with this is reusing the validations in Conway+ eras.

I'm leaning towards this approach I drafted here, but I'm curious what you think.

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.

1 participant