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

Adding some instances needed for consensus. #2283

Merged
merged 1 commit into from
May 13, 2021

Conversation

JaredCorduan
Copy link
Contributor

These instances necessary for integration with consensus because the era's translation context is stored in the LedgerConfig.

@JaredCorduan JaredCorduan requested a review from nfrisby May 13, 2021 16:41
Copy link
Contributor

@nfrisby nfrisby left a comment

Choose a reason for hiding this comment

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

Thanks, Jared. LGTM

@@ -73,6 +73,8 @@ instance CryptoClass.Crypto c => UsesTxOut (ShelleyEra c) where
instance CryptoClass.Crypto c => UsesPParams (ShelleyEra c) where
mergePPUpdates _ = updatePParams

type instance E.TranslationContext (ShelleyEra c) = ()
Copy link
Contributor

@nfrisby nfrisby May 13, 2021

Choose a reason for hiding this comment

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

This is somewhat questionable, since we never actually use TranslationContext for ShelleyEra. But that's also the reason that it's harmless. And it currently simplifies Consensus. Also, it's arguably more consistent: every era within the Shelley mega-era now has a TranslationContext instance. (Which is trivial prior to Alonzo.)

@JaredCorduan JaredCorduan merged commit 7540043 into master May 13, 2021
@iohk-bors iohk-bors bot deleted the jc/some-instances-for-consensus branch May 13, 2021 17:47
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