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

Add translation context to translate from Byron to Shelley #3164

Merged
merged 14 commits into from
Jan 12, 2023

Conversation

bartfrenk
Copy link
Contributor

@bartfrenk bartfrenk commented Nov 22, 2022

This PR does two things:

  1. It changes the TranslationContext for the ShelleyEra to allow for translating from Byron into the first Shelley era.
  2. It removes the TranslateEra instances for ShelleyGenesis. Done in Mempool: Bound ex units of chosen transactions when forging new block ouroboros-network#3224

Closes IntersectMBO/ouroboros-consensus#409. In short, it is required for removing the entire ShelleyGenesis value from the ShelleyLedgerConfig in ouroboros-consensus. Closes IntersectMBO/ouroboros-consensus#409.

There is a PR in ouroboros-consensus: IntersectMBO/ouroboros-network#4091 which require the (backported) changes from this PR.

@bartfrenk bartfrenk marked this pull request as draft November 22, 2022 14:34
@bartfrenk bartfrenk force-pushed the bartfrenk/3976/translation-context-shelley branch 5 times, most recently from 58725b7 to bb4bd92 Compare November 28, 2022 09:06
@bartfrenk bartfrenk marked this pull request as ready for review November 28, 2022 09:06
Copy link
Contributor

@JaredCorduan JaredCorduan left a comment

Choose a reason for hiding this comment

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

This looks great to me, thank you @bartfrenk ! Could you add something to the CHANGELOG about this change? thanks!

@bartfrenk
Copy link
Contributor Author

@bartfrenk bartfrenk closed this Nov 29, 2022
@bartfrenk bartfrenk reopened this Nov 29, 2022
@bartfrenk bartfrenk force-pushed the bartfrenk/3976/translation-context-shelley branch 3 times, most recently from 6afd52c to e450dba Compare December 1, 2022 13:33
@JaredCorduan
Copy link
Contributor

@bartfrenk thanks again! Should I merge this as soon an CI clears? (I don't plan on letting the darwin failure stop us from merging). I'll probably squash all the commits (they started out as logical units, but appeasing CI caused it to be a bit scattered :) )

@bartfrenk bartfrenk force-pushed the bartfrenk/3976/translation-context-shelley branch 3 times, most recently from 0b8bb44 to 13a02b0 Compare December 6, 2022 13:30
@bartfrenk bartfrenk force-pushed the bartfrenk/3976/translation-context-shelley branch 2 times, most recently from 844103a to eb54442 Compare December 22, 2022 09:13
@bartfrenk bartfrenk force-pushed the bartfrenk/3976/translation-context-shelley branch from eb54442 to 1e3c099 Compare January 10, 2023 12:02
Copy link
Collaborator

@lehins lehins left a comment

Choose a reason for hiding this comment

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

Ledger has changed since this PR was originally opened.

In #3224 we switched parameterization of ShelleyGenesis from era to crypto. Same thing is needed for FromByronTransitionContext, since that type is only valid for ShelleyEra. Also TranslateEra instances has been removed, also for the reason listed above.

CHANGELOG.md Outdated Show resolved Hide resolved
eras/shelley/impl/src/Cardano/Ledger/Shelley/Genesis.hs Outdated Show resolved Hide resolved
eras/shelley/impl/src/Cardano/Ledger/Shelley/Genesis.hs Outdated Show resolved Hide resolved
eras/shelley/impl/src/Cardano/Ledger/Shelley/Genesis.hs Outdated Show resolved Hide resolved
eras/shelley/impl/src/Cardano/Ledger/Shelley/Genesis.hs Outdated Show resolved Hide resolved
bartfrenk and others added 6 commits January 12, 2023 10:13
Co-authored-by: Alexey Kuleshevich <alexey.kuleshevich@iohk.io>
Co-authored-by: Alexey Kuleshevich <alexey.kuleshevich@iohk.io>
Co-authored-by: Alexey Kuleshevich <alexey.kuleshevich@iohk.io>
@bartfrenk bartfrenk force-pushed the bartfrenk/3976/translation-context-shelley branch from a323075 to 919d8a7 Compare January 12, 2023 09:14
Copy link
Collaborator

@lehins lehins left a comment

Choose a reason for hiding this comment

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

Thank you. Looks great!

@lehins lehins merged commit 24d7a68 into master Jan 12, 2023
@iohk-bors iohk-bors bot deleted the bartfrenk/3976/translation-context-shelley branch January 12, 2023 11:24
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.

Remove ShelleyGenesis from ShelleyLedgerConfig
3 participants