-
Notifications
You must be signed in to change notification settings - Fork 720
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
API changes for the Allegra & Mary eras #2110
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
So rename ByronEra -> ByronEra, Shelley to ShelleyEra etc This first step adds the new names, and keeps deprecated aliases for the previous names. The purpose of this renaming is because we want to clarify that these type tags are really used to indicate an era. Where we are tempted to use them for something other than eras then we need new names. For example we have Byron and Shelley address types, which are not quite the same as the Byron and Shelley era, since both address types are available in the Shelley and all subsequent eras. So we will want to introduce new names such as ByronAddr/ShelleyAddr and use those rather than ByronEra/ShelleyEra for types of addresses.
dcoutts
requested review from
erikd,
Jimbo4350,
mrBliss and
newhoggy
as code owners
November 20, 2020 01:33
Jimbo4350
reviewed
Nov 20, 2020
|
||
pattern AsByron :: AsType ByronEra | ||
pattern AsByron = AsByronEra |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice
Jimbo4350
approved these changes
Nov 20, 2020
Add a CardanoEra GADT to give a value-level representation of all eras and a IsCardanoEra class of era types that support this representation. This lets us pattern match on ears specifically so that we can handle things non-uniformly when we need to. The type class makes it convenient to pass the era. Add a similar trick for Shelley-based eras, with a ShelleyBasedEra GADT and a IsShelleyBasedEra class. This is the same thing but for when we need to handle only the Shelley-based eras in a mostly-uniform way. In addition to CardanoEra which treats each era equally, we provide the same info but factorised into the Byron era and the Shelley-based eras. This makes it easier to handle the Byron era specially, but handle all Shelley eras in a uniform way.
It has become clear that the way we have handled the variation in addresses does not scale to more eras and it needed a rethink. Up to now we paramaterised the address type by the era in which it is valid. With just the Byron and Shelley eras that worked, though is not especially elegant. The problem is that with more Shelley-based eras we have many eras in which we support both Byron and Shelley addresses. Previously where we needed to deal with either Byron or Shelley addresses we slightly abuse things by useing the type of addresses in the Shelley era, since that supports both address types. But this becomes quite ugly once there are many such eras. Fundamentally we have two different dimensions: address type and era. Up to now we have smudged these together and just about got away with it because there were only two of each. Now there are four eras we cannot get away with it. So the solution is to properly introduce a separate dimension of address type: Byron address vs Shelley address. The Address type can be paramaterised by this distinction. For the first time this actually lets us talk about a type of Shelley addresses, without also having Byron addresses as an alternative. We then add a new monomorphic type of AddressAny which can be either of the two Address types. This is useful as a target for parsing or deserialising where we need to handle either address type without reference to an era in which they might be used. Finally, we still need addresses in the context of an era in which they are valid. So we have a new type AddressInEra which is paramaterised by the era. The usual GADT trick is used to manage which address types are valid in which eras. We make use of the new Shelley-based era functionality to factorise this conveniently so we only need to deal with the distinction between Byron vs Shelley-based, without being forced to distinguish unnecessarily between the different Shelley-based eras which are uniform in their treatment of addresses. This patch does not yet update the CLI.
dcoutts
force-pushed
the
dcoutts/api-allegra-mary
branch
from
November 20, 2020 09:27
555b1c3
to
bd7eecc
Compare
Fixed review commend and hlint warning. |
bors merge |
iohk-bors bot
added a commit
that referenced
this pull request
Nov 20, 2020
2110: API changes for the Allegra & Mary eras r=dcoutts a=dcoutts Various refactoring needed to support the addition of two new Shelley-based eras Rename the Byron, Shelley etc types to be clearly eras (i.e. rename ByronEra -> ByronEra, Shelley to ShelleyEra etc) New functionality for generic handling of eras, and Shelley-based eras in particular. This makes it easier to write code that treats eras mostly uniformly when possible and non-uniformly when necessary. It also lets us distinguish Byron vs Shelley-based eras without unnecessarily having to distinguish between the different Shelley-based eras. This is useful as the biggest differences are between Byron and later eras. It is very often possible to treat all the Shelley-based eras uniformly. Change how we handle addresses to properly distinguish the address type from the era in which the address is valid. This would have been useful already but it is really needed once we have multiple Shelley-based eras. Co-authored-by: Duncan Coutts <duncan@well-typed.com>
Build failed: |
bors r+ |
Build succeeded: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Various refactoring needed to support the addition of two new Shelley-based eras
Rename the Byron, Shelley etc types to be clearly eras
(i.e. rename ByronEra -> ByronEra, Shelley to ShelleyEra etc)
New functionality for generic handling of eras, and Shelley-based eras in particular.
This makes it easier to write code that treats eras mostly uniformly when possible
and non-uniformly when necessary. It also lets us distinguish Byron vs Shelley-based
eras without unnecessarily having to distinguish between the different Shelley-based
eras. This is useful as the biggest differences are between Byron and later eras. It is
very often possible to treat all the Shelley-based eras uniformly.
Change how we handle addresses to properly distinguish the address type from the
era in which the address is valid. This would have been useful already but it is really
needed once we have multiple Shelley-based eras.