Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Expose ledger types PParams and PParamsUpdate with support code #191

Closed
wants to merge 3 commits into from
Closed

Expose ledger types PParams and PParamsUpdate with support code #191

wants to merge 3 commits into from

Conversation

newhoggy
Copy link
Collaborator

@newhoggy newhoggy commented Aug 15, 2023 •
edited
Loading

Changelog

- description: |
    Expose ledger types `PParams` and `PParamsUpdate` with support code
# uncomment types applicable to the change:
  type:
  # - feature        # introduces a new feature
  # - breaking       # the API has changed in a breaking way
  - compatible     # the API has changed but is non-breaking
  # - optimisation   # measurable performance improvements
  # - improvement    # QoL changes e.g. refactoring
  # - bugfix         # fixes a defect
  # - test           # fixes/modifies tests
  # - maintenance    # not directly related to the code
  # - release        # related to a new release preparation
  # - documentation  # change in code docs, haddocks...

Context

This a a simplified version of #182 with all the refactoring, newtype wrapper and aeson instances removed. The removed commits were mainly to facilitate the custom aeson instances which during discussions were determined to not be needed.

Checklist

  • Commit sequence broadly makes sense and commits have useful messages
  • The change log section in the PR description has been filled in
  • New tests are added if needed and existing tests are updated. These may include:
    • golden tests
    • property tests
    • round trip tests
    • integration tests
      See Running tests for more details
  • The version bounds in .cabal files are updated
  • CI passes. See note on CI. The following CI checks are required:
    • Code is linted with hlint. See .github/workflows/check-hlint.yml to get the hlint version
    • Code is formatted with stylish-haskell. See .github/workflows/stylish-haskell.yml to get the stylish-haskell version
    • Code builds on Linux, MacOS and Windows for ghc-8.10.7 and ghc-9.2.7
  • The changelog section in the PR is updated to describe the change
  • Self-reviewed the diff

@newhoggy newhoggy changed the title Features for pparams simplified Expose ledger types PParams and PParamsUpdate with support code Aug 15, 2023
@newhoggy newhoggy marked this pull request as ready for review August 15, 2023 08:01
@newhoggy newhoggy force-pushed the newhoggy/features-for-pparams-2 branch from 36b6726 to 4d0abff Compare August 15, 2023 08:04
Jimbo4350
Jimbo4350 requested changes Aug 15, 2023
View reviewed changes
cardano-api/internal/Cardano/Api/Domain/ProtocolParametersUpdate.hs Outdated
protocolUpdateMinUTxOValue w = shelleyToAllegraEraConstraints w Ledger.ppuMinUTxOValueL

-- | The deposit required to register a stake address.
protocolUpdateStakeAddressDeposit :: ShelleyBasedEra era -> Lens' (Ledger.PParamsUpdate (ShelleyLedgerEra era)) (Ledger.StrictMaybe Ledger.Coin)
Copy link
Contributor

Choose a reason for hiding this comment

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

Where are these being used?

cardano-api/internal/Cardano/Api/Domain/ProtocolParameters.hs

-- | The fraction of the fee pot each epoch that goes to the treasury.
-- This is the \"tau\" incentives parameter from the design document.
protocolParamTreasuryCutL :: ShelleyBasedEra era -> Lens' (Ledger.PParams (ShelleyLedgerEra era)) Ledger.UnitInterval
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here, where are they being used?

cardano-api/internal/Cardano/Api/Eras/Core.hs Outdated
-- ----------------------------------------------------------------------------
-- ToCardanoEra

class ToCardanoEra (feature :: Type -> Type) where
Copy link
Contributor

Choose a reason for hiding this comment

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

And this? Where is this being used?

@newhoggy
Copy link
Collaborator Author

These are intended to be used by the CLI.

@newhoggy newhoggy force-pushed the newhoggy/features-for-pparams-2 branch from e6770b8 to c806e12 Compare August 31, 2023 13:20
@newhoggy newhoggy force-pushed the newhoggy/features-for-pparams-2 branch from c806e12 to d23eead Compare September 18, 2023 12:02
@github-actions GitHub Actions
Copy link

This PR is stale because it has been open 45 days with no activity.

@github-actions github-actions bot added the Stale label Nov 20, 2023
@github-actions GitHub Actions
Copy link

This issue was closed because it has been stalled for 60 days with no activity.

@github-actions github-actions bot closed this Jan 20, 2024
newhoggy added a commit that referenced this pull request Mar 11, 2024
@newhoggy
Merge pull request #191 from input-output-hk/newhoggy/governance-drep…
dce9584 
…-command-group

Governance `drep` and `vote` command groups
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Jimbo4350 Jimbo4350 Jimbo4350 requested changes

@carbolymer carbolymer carbolymer approved these changes

@dcoutts dcoutts Awaiting requested review from dcoutts

@erikd erikd Awaiting requested review from erikd

@disassembler disassembler Awaiting requested review from disassembler

@CarlosLopezDeLara CarlosLopezDeLara Awaiting requested review from CarlosLopezDeLara

Assignees
No one assigned
Labels
Stale
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@newhoggy @carbolymer @Jimbo4350