Delete `ConsensusMode` type #342

newhoggy · 2023-10-25T11:56:19Z

Changelog

- description: |
    Delete `ByronMode` and `ShelleyMode`
    Delete `anyEraInModeToAnyEra`
    Delete `AnyEraInMode`
    Modify to use `CardanoMode` only:
      - `executeQueryAnyMode`
      - `connectToLocalNode`
      - `connectToLocalNodeWithVersion
      - `mkLocalNodeClientParams`
      - `queryNodeLocalState`
      - `submitTxToNodeLocal`
      - `queryTxMonitoringLocal`
      - `getLocalChainTip`
      - `executeLocalStateQueryExpr`
    Deparameterise `LocalNodeClientProtocolsInMode`
    Modify `LocalNodeClientProtocols` to only work in `CardanoMode`.
    Rename `LocalNodeClientProtocolsInMode` to `LocalNodeClientProtocolsInIO`
    Rename `TxValidationErrorInMode` to `TxValidationErrorInCardanoMode`
    Deparameterise `TxValidationErrorInMode`
    Modify to use `CardanoMode` only
    - `determineEra`
    Deparameterise `LocalNodeConnectInfo`
    Deparameterise `LocalTxMonitoringResult`
    Deparameterise `LocalTxMonitoringQuery`
    Deparameterise `TxInMode`
    Modify to use `CardanoMode` only
    - `fromConsensusBlock`
    - `toConsensusBlock`
    - `queryExpr`
    - `getProgress`
    - `getSlotForRelativeTime`
    - `toLedgerEpochInfo`
    - `slotToEpoch`
    - `queryCurrentEpochState`
    - `queryEpoch`
    - `queryDebugLedgerState`
    - `queryGenesisParameters`
    - `queryPoolDistribution`
    - `queryPoolState`
    - `queryProtocolParameters`
    - `queryConstitutionHash`
    - `queryProtocolParametersUpdate`
    - `queryProtocolState`
    - `queryStakeAddresses`
    - `queryStakeDelegDeposits`
    - `queryStakeDistribution`
    - `queryStakePoolParameters`
    - `queryStakePools`
    - `queryStakeSnapshot`
    - `queryUtxo`
    - `queryConstitution`
    - `queryGovState`
    - `queryDRepState`
    - `queryDRepStakeDistribution`
    - `queryCommitteeMembersState`
    Deparameterise `BlockInMode`
    Deparameterise `TxIdInMode`
    Remove `EraInMode` argument in `QueryInEra` constructor
    Delete `ConsensusBlockForMode`
    Modify `TxInMode` to carry `CardanoEra` instead of `EraInMode`
    Deparameterise `BlockInMode`
    Deparameterise `TxIdInMode`
    Deparameterise `EraHistory`
    Modify to not take `EraInMode` argument
    - `queryCurrentEpochState`
    - `queryEpoch`
    - `queryDebugLedgerState`
    - `queryGenesisParameters`
    - `queryPoolDistribution`
    - `queryPoolState`
    - `queryProtocolParameters`
    - `queryConstitutionHash`
    - `queryProtocolParametersUpdate`
    - `queryProtocolState`
    - `queryStakeAddresses`
    - `queryStakeDelegDeposits`
    - `queryStakeDistribution`
    - `queryStakePoolParameters`
    - `queryStakePools`
    - `queryStakeSnapshot`
    - `queryUtxo`
    - `queryConstitution`
    - `queryGovState`
    - `queryDRepState`
    - `queryDRepStakeDistribution`
    - `queryCommitteeMembersState`
    Modify `TxInMode` constructors to remove `EraInMode` arguments from constructors
    Remove `EraInMode` from `TxValidationErrorInCardanoMode` constructors
    Delete `EraInMode`, `eraInModeToEra` and `toEraInMode`
    Remove `ConsensusMode` argument from:
    - `fromConsensusBlock`
    - `fromConsensusTip`
    - `determineEra`
    Delete `determineEraExpr`.  Use `queryCurrentEra` instead.
    Deparameterise `QueryInMode`
    Delete `AnyConsensusModeParams`
    Deparametrise `ConsensusModeParams`
    Remove `ConsensusMode` argument from `EraHistory` constructor
    Delete `EraConsensusModeMismatch` constructor
    Delete `localConsensusMode`, `AnyConsensusMode` and `renderMode`
    Delete `EraConsensusModeMismatch`
    Delete `CardanoMode` and `ConsensusMode`.
# 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

We neither use not need ConsensusMode anymore.

This PR makes all the necessary changes to remove ConsensusMode.

Type names and function names retain the word Mode in them because that can be postponed to a future PR.

Refactoring to simplify code enabled by the removal can also be postponed to a future PR.

How to trust this PR

Highlight important bits of the PR that will make the review faster. If there are commands the reviewer can run to observe the new behavior, describe them.

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

cardano-api/internal/Cardano/Api/LedgerState.hs

cardano-api/cardano-api.cabal

Jimbo4350 · 2023-10-27T12:52:55Z

cardano-api/internal/Cardano/Api/IPC.hs

-     , localTxMonitoringClient
-         :: Maybe (LocalTxMonitorClient txid tx slot m ())
-     }
+data LocalNodeClientProtocols point tip slot m = LocalNodeClientProtocols


This should remain generic because we are now forcing consumers of cardano-api to use our types for block, tx, txid, txerr and query. We can instantiate the types we want with type LocalNodeClientProtocolsInIO.

If these need to be parameterised, then we need to keep these types:

data BlockInMode mode where BlockInMode :: CardanoEra era -> Block era -> EraInMode era mode -> BlockInMode mode

data TxInMode mode where TxInMode :: Tx era -> EraInMode era mode -> TxInMode mode TxInByronSpecial :: Consensus.GenTx Consensus.ByronBlock -> EraInMode ByronEra mode -> TxInMode mode

data TxIdInMode mode where TxIdInMode :: TxId -> EraInMode era mode -> TxIdInMode mode

data TxValidationErrorInMode mode where TxValidationErrorInMode :: TxValidationError era -> EraInMode era mode -> TxValidationErrorInMode mode TxValidationEraMismatch :: EraMismatch -> TxValidationErrorInMode mode

data QueryInMode mode result where QueryCurrentEra :: QueryInMode mode AnyCardanoEra QueryInEra :: EraInMode era mode -> QueryInEra era result -> QueryInMode mode (Either EraMismatch result) QueryEraHistory :: QueryInMode mode (EraHistory mode) QuerySystemStart :: QueryInMode mode SystemStart QueryChainBlockNo :: QueryInMode mode (WithOrigin BlockNo) QueryChainPoint :: ConsensusMode mode -> QueryInMode mode ChainPoint

We can still convert EraInMode era mode to CardanoEra era so the mode type parameter disappears however.

It's not clear what the benefit of keeping these type parameters are if we only fix it to one type. Is there a known use case?

The deleted parameters of LocalNodeClientProtocols have been restored.

Jimbo4350

Looking good. One significant comment: https://github.com/input-output-hk/cardano-api/pull/342/files#r1374534246

cardano-api/internal/Cardano/Api/IPC.hs

Jimbo4350 · 2023-10-27T12:59:00Z

cardano-api/internal/Cardano/Api/InMode.hs

-    tx' = Consensus.ByronTx (Consensus.byronIdTx tx) tx
-
-toConsensusGenTx (TxInMode (ByronTx tx) ByronEraInCardanoMode) =
+data TxInMode where


Not necessary for this PR but we need to think of a better name.

Yep. I kept the name so the diff is easier to see. A separate PR can rename the type.

Jimbo4350 · 2023-10-27T12:59:57Z

cardano-api/internal/Cardano/Api/InMode.hs

+  -- actually transactions. This covers: update proposals, votes and
+  -- delegation certs.
+  --
+  TxInByronSpecial


We may be able to get away with using data Tx era directly and removing TxInMode. If we can convert Byron.ATxAux ByteString -> Consensus.GenTx Consensus.ByronBlock then we should be able to. Not necessary for this PR.

Jimbo4350 · 2023-10-27T13:30:46Z

cardano-api/internal/Cardano/Api/Modes.hs

-
-- | The Shelley-only consensus mode consists of only the Shelley era.
--
-- This was used for the early Shelley testnets prior to the use of the


Nice, so my memory served me correctly.

Comments addresed

Jimbo4350

Almost there! I spotted an error.

Jimbo4350 · 2023-10-31T13:21:31Z

cardano-api/internal/Cardano/Api/IPC.hs

-     , localTxMonitoringClient
-         :: Maybe (LocalTxMonitorClient txid tx slot m ())
-     }
+  LocalNodeClientProtocols


Jimbo4350 · 2023-10-31T13:21:44Z

cardano-api/internal/Cardano/Api/IPC.hs

-consensusModeOnly ShelleyModeParams{} = ShelleyMode
-consensusModeOnly CardanoModeParams{} = CardanoMode
-
+type LocalNodeClientProtocolsInMode =


Jimbo4350 · 2023-10-31T13:24:34Z

cardano-api/internal/Cardano/Api/InMode.hs

+  Consensus.HardForkGenTx (Consensus.OneEraGenTx (S (Z tx'))) ->
+    let Consensus.ShelleyTx _txid shelleyEraTx = tx'
+    in TxInMode ShelleyEra (ShelleyTx ShelleyBasedEraShelley shelleyEraTx)
+  Consensus.HardForkGenTx (Consensus.OneEraGenTx (S (S (Z tx')))) ->


Not necessary for this PR but in consensus there are patterns we can use:

pattern TagByron x = Z x pattern TagShelley x = S (Z x) pattern TagAllegra x = S (S (Z x)) pattern TagMary x = S (S (S (Z x))) pattern TagAlonzo x = S (S (S (S (Z x)))) pattern TagBabbage x = S (S (S (S (S (Z x))))) pattern TagConway x = S (S (S (S (S (S (Z x))))))

cardano-api/internal/Cardano/Api/InMode.hs

Jimbo4350 · 2023-10-31T13:39:40Z

cardano-api/src/Cardano/Api.hs

    ConsensusBlockForEra,
-    EraInMode(..),


Jimbo4350 · 2023-10-31T13:39:50Z

cardano-api/src/Cardano/Api/Byron.hs

@@ -47,10 +47,6 @@ module Cardano.Api.Byron

    -- ** Low level protocol interaction with a Cardano node
    LocalNodeConnectInfo(LocalNodeConnectInfo),
-    ByronMode,


…type

* convLocalChainSyncClient * convLocalChainSyncClientPipelined * convLocalNodeClientProtocols * convLocalTxMonitoringClient * fromConsensusGenTx * fromConsensusTip

…cutionUnits

…urate

github-advanced-security bot found potential problems Oct 26, 2023

View reviewed changes

newhoggy force-pushed the newhoggy/delete-ConsensusMode-type branch from a23b983 to 8490c1c Compare October 27, 2023 09:24

newhoggy marked this pull request as ready for review October 27, 2023 09:30

newhoggy requested review from dcoutts, Jimbo4350, erikd, carbolymer, CarlosLopezDeLara and disassembler as code owners October 27, 2023 09:30

newhoggy force-pushed the newhoggy/delete-ConsensusMode-type branch from e8a4f9c to 6ab9918 Compare October 27, 2023 10:52

newhoggy changed the title ~~WIP Delete ConsensusMode type~~ Delete ConsensusMode type Oct 27, 2023

newhoggy force-pushed the newhoggy/delete-ConsensusMode-type branch 2 times, most recently from 2d87f7d to e15de45 Compare October 27, 2023 11:48

carbolymer approved these changes Oct 27, 2023

View reviewed changes

cardano-api/cardano-api.cabal Show resolved Hide resolved

newhoggy added this pull request to the merge queue Oct 27, 2023

newhoggy removed this pull request from the merge queue due to a manual request Oct 27, 2023

Jimbo4350 reviewed Oct 27, 2023

View reviewed changes

Jimbo4350 previously requested changes Oct 27, 2023

View reviewed changes

newhoggy added the do-not-merge label Oct 27, 2023

newhoggy force-pushed the newhoggy/delete-ConsensusMode-type branch 6 times, most recently from 6f81c9a to 7072909 Compare October 31, 2023 10:04

newhoggy requested a review from Jimbo4350 October 31, 2023 10:07

Jimbo4350 reviewed Oct 31, 2023

View reviewed changes

newhoggy added 2 commits November 1, 2023 21:45

Delete ShelleyMode

c936d54

Delete ByronMode

12abf7e

newhoggy added 23 commits November 1, 2023 21:46

Deparameterise TxIdInMode

d0848d1

Deparameterise EraHistory

b3b6bc0

Delete EraInMode constructor argument in QueryInEra constructor

f0916a4

Delete ConsensusBlockForMode type family

26cd166

Replace EraInMode era CardanoMode in TxInMode with CardanoEra

98db317

Remove EraInMode era CardanoMode from TxValidationErrorInCardanoMode …

4748a57

…type

Delete EraInMode, eraInModeToEra and toEraInMode

a957b69

Inline fromConsensusPointInMode

2e98f75

Inline toConsensusPointInMode

713b8e4

Remove ConsensusMode argument from fromConsensusBlock

7b6e6a3

Remove ConsensusMode argument from:

9551d05

* convLocalChainSyncClient * convLocalChainSyncClientPipelined * convLocalNodeClientProtocols * convLocalTxMonitoringClient * fromConsensusGenTx * fromConsensusTip

Delete ConsensusModeParams argument from ConsensusModeParams

aa6123c

Delete determineEraExpr

918bcc2

Delete obsolete TODO

7577421

Deparameterise QueryInMode

c0191e7

Deparameterise ConsensusModeParams

beefcbf

Remove ConsensusMode constructor argument from EraHistory constructor

104369f

Remove ConsensusMode argument from fromConsensusEraIndex function

d7c7535

Delete localConsensusMode and consensusModeOnly functions

aa1eb05

Delete AnyConsensusMode and renderMode

43952f1

Delete ConsensusMode and CardanoMode

0a7466e

Add eon arguments to makeSignedTransaction and evaluateTransactionExe…

aa74887

…cutionUnits

Disable incomplete-patterns warning on ghc-8.10.7 because it is inacc…

b155e2d

…urate

newhoggy force-pushed the newhoggy/delete-ConsensusMode-type branch from 7072909 to b155e2d Compare November 1, 2023 10:53

Jimbo4350 approved these changes Nov 1, 2023

View reviewed changes

newhoggy added this pull request to the merge queue Nov 2, 2023

newhoggy removed the do-not-merge label Nov 2, 2023

Merged via the queue into main with commit 561bec4 Nov 2, 2023

newhoggy deleted the newhoggy/delete-ConsensusMode-type branch November 2, 2023 09:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Delete `ConsensusMode` type #342

Delete `ConsensusMode` type #342

newhoggy commented Oct 25, 2023 •

edited

Loading

Jimbo4350 Oct 27, 2023 •

edited

Loading

newhoggy Oct 30, 2023

newhoggy Oct 31, 2023

Jimbo4350 left a comment

Jimbo4350 Oct 27, 2023

newhoggy Oct 30, 2023

Jimbo4350 Oct 27, 2023

Jimbo4350 Oct 27, 2023

Jimbo4350 left a comment

Jimbo4350 Oct 31, 2023

Jimbo4350 Oct 31, 2023

Jimbo4350 Oct 31, 2023

Jimbo4350 Oct 31, 2023

Jimbo4350 Oct 31, 2023

Delete ConsensusMode type #342

Delete ConsensusMode type #342

Conversation

newhoggy commented Oct 25, 2023 • edited Loading

Changelog

Context

How to trust this PR

Checklist

Jimbo4350 Oct 27, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Jimbo4350 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Jimbo4350 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Delete `ConsensusMode` type #342

Delete `ConsensusMode` type #342

newhoggy commented Oct 25, 2023 •

edited

Loading

Jimbo4350 Oct 27, 2023 •

edited

Loading