Use newtype instead of GADT for LedgerProtocolParameters

[newhoggy]

Changelog

- description: |
    Use `newtype` instead of `GADT` for `LedgerProtocolParameters`
# 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

A newtype is a thinner wrapper around the base type than a GADT. This will make accessing the contents of the type easier.

There is no need to carry a witness value because the caller can supply it and there is no need to carry a constraint because the caller can use the witness to summon the constraint. In this case shelleyBasedEraConstraints w will do.

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

[carbolymer]

Why do we even need a wrapper for PParams?

[newhoggy]

Convenience. For example other types contain this type and by providing Eq, and Show instances, automatic instance derivation in parent types just works which is not true if you try to use the ledger types directly.

[carbolymer]

But ledger types do have Eq and Show, don't they?

[newhoggy]

For example we embed the type like this:

data TxBodyContent build era =
     TxBodyContent {
       txIns               :: TxIns build era,
       txInsCollateral     :: TxInsCollateral era,
       txInsReference      :: TxInsReference build era,
       txOuts              :: [TxOut CtxTx era],
       txTotalCollateral   :: TxTotalCollateral era,
       txReturnCollateral  :: TxReturnCollateral CtxTx era,
       txFee               :: TxFee era,
       txValidityRange     :: (TxValidityLowerBound era,
                              TxValidityUpperBound era),
       txMetadata          :: TxMetadataInEra era,
       txAuxScripts        :: TxAuxScripts era,
       txExtraKeyWits      :: TxExtraKeyWitnesses era,
       txProtocolParams    :: BuildTxWith build (Maybe (LedgerProtocolParameters era)), -- <------ here
       txWithdrawals       :: TxWithdrawals  build era,
       txCertificates      :: TxCertificates build era,
       txUpdateProposal    :: TxUpdateProposal era,
       txMintValue         :: TxMintValue    build era,
       txScriptValidity    :: TxScriptValidity era,
       txGovernanceActions :: TxGovernanceActions era,
       txVotingProcedures  :: Maybe (Featured ConwayEraOnwards era (VotingProcedures era))
     }
     deriving (Eq, Show) -- <----- automatic derivation here

[newhoggy]

But ledger types do have Eq and Show, don't they?

Yes, but because they are parameterised by ledgerera and they demand additional constraints GHC can't figure out how to find them in automatic derivation.

[carbolymer]

Without newtype you'd just need a standalone deriving clause with a constraint - not much of a convenience gain imo.

[carbolymer]

My proposal: #219

[newhoggy]

See how to call the ledger version of Eq and Show instances you have to use shelleyBasedEraConstraints.

[carbolymer]

You're just trading here EraPParams era with IsShelleyBasedEra era. I'm fine with both, but since we're aiming to reduce amount of newtype wrappers, not using a newtype seems a better option.

[newhoggy]

I'm trading constraints, yes, but automatic instance derivation works with IsShelleyBasedEra era but not with EraPParams ledgerera.

[newhoggy]

For example if I try this:

data TxBodyContent build era =
     TxBodyContent {
       txIns               :: TxIns build era,
       txInsCollateral     :: TxInsCollateral era,
       txInsReference      :: TxInsReference build era,
       txOuts              :: [TxOut CtxTx era],
       txTotalCollateral   :: TxTotalCollateral era,
       txReturnCollateral  :: TxReturnCollateral CtxTx era,
       txFee               :: TxFee era,
       txValidityRange     :: (TxValidityLowerBound era,
                              TxValidityUpperBound era),
       txMetadata          :: TxMetadataInEra era,
       txAuxScripts        :: TxAuxScripts era,
       txExtraKeyWits      :: TxExtraKeyWitnesses era,
       txProtocolParams    :: BuildTxWith build (Maybe (Ledger.PParams (ShelleyLedgerEra era))),
       txWithdrawals       :: TxWithdrawals  build era,
       txCertificates      :: TxCertificates build era,
       txUpdateProposal    :: TxUpdateProposal era,
       txMintValue         :: TxMintValue    build era,
       txScriptValidity    :: TxScriptValidity era,
       txGovernanceActions :: TxGovernanceActions era,
       txVotingProcedures  :: Maybe (Featured ConwayEraOnwards era (VotingProcedures era))
     }
     deriving (Eq, Show)

I get these errors:

internal/Cardano/Api/TxBody.hs:1766:16: error:
    • Could not deduce (Eq
                          (Ledger.PParamsHKD
                             Data.Functor.Identity.Identity (ShelleyLedgerEra era)))
        arising from the 12th field of ‘TxBodyContent’
          (type ‘BuildTxWith
                   build (Maybe (Ledger.PParams (ShelleyLedgerEra era)))’)
      from the context: IsShelleyBasedEra era
        bound by the deriving clause for ‘Eq (TxBodyContent build era)’
        at internal/Cardano/Api/TxBody.hs:1766:16-17
      Possible fix:
        use a standalone 'deriving instance' declaration,
          so you can specify the instance context yourself
    • When deriving the instance for (Eq (TxBodyContent build era))
     |
1766 |      deriving (Eq, Show)
     |                ^^

internal/Cardano/Api/TxBody.hs:1766:20: error:
    • Could not deduce (Show
                          (Ledger.PParamsHKD
                             Data.Functor.Identity.Identity (ShelleyLedgerEra era)))
        arising from the 12th field of ‘TxBodyContent’
          (type ‘BuildTxWith
                   build (Maybe (Ledger.PParams (ShelleyLedgerEra era)))’)
      from the context: IsShelleyBasedEra era
        bound by the deriving clause for ‘Show (TxBodyContent build era)’
        at internal/Cardano/Api/TxBody.hs:1766:20-23
      Possible fix:
        use a standalone 'deriving instance' declaration,
          so you can specify the instance context yourself
    • When deriving the instance for (Show (TxBodyContent build era))
     |
1766 |      deriving (Eq, Show)