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.

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

More TPraos separation #2518

Merged
merged 3 commits into from
Oct 15, 2021
Merged

More TPraos separation #2518

merged 3 commits into from
Oct 15, 2021

Conversation

JaredCorduan
Copy link
Contributor

@JaredCorduan JaredCorduan commented Oct 13, 2021

This PR completes a few of the tasks remaining for separating TPraos from the ledger code (see #2462)

  • PoolDistr, IndividualPoolStake and ProtVer have been moved to the cardano-ledger-core package.
  • The Wallet API function getLeaderSchedule was moved to the Protocol API module, so that it does not need any TPraos imports. I'm not sure what to do with the Protocol API module, it may need to move to consensus.
  • All the pretty printers have been moved to a new package `cardano-ledger-pretty'. There are a few things worth mentioning:
    • I wanted to keep the Cardano.Ledger.Pretty namespace, so I was not able to deprecate the old module. I think this is okay, I don't think any downstream libraries are using the pretty printers.
    • I added some unidirectional patterns since the instances are no longer couple to the hidden module definitions. I also had to add some COMPLETE pragma to some existing patterns.
  • I removed the TPraos rules (OCERT, OVERLAY and TICKN) from the era mappings.

@JaredCorduan JaredCorduan force-pushed the jc/more-tpraos-separation branch 2 times, most recently from 29a6af2 to 7fd7993 Compare October 13, 2021 01:24
@JaredCorduan JaredCorduan requested a review from nc6 October 13, 2021 01:24
Copy link
Contributor

@nc6 nc6 left a comment

Choose a reason for hiding this comment

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

Minor comments, otherwise looks good!

boundedRationalFromCBOR,
boundedRationalToCBOR,
)
import Cardano.Ledger.BaseTypes (ActiveSlotCoeff, NonNegativeInterval, ProtVer (..), ShelleyBase, UnitInterval, boundedRationalFromCBOR, boundedRationalToCBOR)
Copy link
Contributor

Choose a reason for hiding this comment

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

[minor] long line

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good catch! I would not do that on purpose, I must have done something to trigger omolu.

activeSlotVal,
invalidKey,
)
import Cardano.Ledger.BaseTypes (ActiveSlotCoeff, BoundedRational (..), NonNegativeInterval, ProtVer, UnitInterval, activeSlotVal, invalidKey)
Copy link
Contributor

Choose a reason for hiding this comment

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

[minor] long line

randomnessStabilisationWindow,
securityParameter,
)
import Cardano.Ledger.BaseTypes (BlocksMade, NonNegativeInterval, ProtVer, ShelleyBase, StrictMaybe (..), UnitInterval, activeSlotCoeff, epochInfo, maxLovelaceSupply, randomnessStabilisationWindow, securityParameter)
Copy link
Contributor

Choose a reason for hiding this comment

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

[minor] long line

{-# LANGUAGE UndecidableInstances #-}
{-# OPTIONS_GHC -Wno-redundant-constraints #-}

module Cardano.Ledger.PoolDistr where
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we document this module a bit? Just to provide a general idea of what a PoolDistr is and why it's a fundamental concept.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

absolutely! Thank you for reminding me recently about things that could be documented better, I need to do this more by reflex!

@nc6 nc6 merged commit d0aa86d into master Oct 15, 2021
@iohk-bors iohk-bors bot deleted the jc/more-tpraos-separation branch October 15, 2021 08:46
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.

2 participants