-
Notifications
You must be signed in to change notification settings - Fork 23
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
Update experimental api and propagate #604
Conversation
b0c6dd0
to
d943abd
Compare
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.
Blocking to have more time to review. I'd like to to play with it a bit.
da2fbe0
to
e242232
Compare
data BabbageEra | ||
|
||
data ConwayEra |
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.
Why not reuse uninhabited types from Cardano.Api.Eras.Core
? They're uninhabited types as well these, so they're not very useful on their own nor they are related to each other.
We could reexport them here for convenience.
Or the other way around, define them here, and reexport in old Api (might require moving to another module).
I think reusing type level tags will be much cleaner than introducing additional type families.
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.
I don't want to do this to avoid confusion with the api we are deprecating. I want to keep the types separate.
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.
Same concern as @carbolymer 👍
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.
I think reusing type level tags will be much cleaner than introducing additional type families.
How will this avoid introducing additional type families?
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.
Note that my comment was unrelated to the need/avoidance for additional type families. I'm concerned by symbols having the exact same name, that are going to be disambiguated with different imports; and I suspect this will confuse both us and users (even more for the latter).
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.
Thinking about it some more I agree. We can re-use the types.
type family AvailableErasToSbe era = (r :: Type) | r -> era where | ||
AvailableErasToSbe BabbageEra = Api.BabbageEra | ||
AvailableErasToSbe ConwayEra = Api.ConwayEra |
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.
The type on the right hand side isn't really ShelleyBasedEra
but just type level tag. I'd drop the Sbe
part. For example:
type family AvailableErasToSbe era = (r :: Type) | r -> era where | |
AvailableErasToSbe BabbageEra = Api.BabbageEra | |
AvailableErasToSbe ConwayEra = Api.ConwayEra | |
type family FromExperimentalApiEra era = (r :: Type) | r -> era where | |
FromExperimentalApiEra BabbageEra = Api.BabbageEra | |
FromExperimentalApiEra ConwayEra = Api.ConwayEra |
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.
type family ToConstrainedLedgerEra era = (r :: Type) | r -> era where | ||
ToConstrainedLedgerEra BabbageEra = Ledger.Babbage | ||
ToConstrainedLedgerEra ConwayEra = Ledger.Conway |
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.
I don't understand the naming here. Maybe something more obvious:
type family ToConstrainedLedgerEra era = (r :: Type) | r -> era where | |
ToConstrainedLedgerEra BabbageEra = Ledger.Babbage | |
ToConstrainedLedgerEra ConwayEra = Ledger.Conway | |
type family LedgerEra era = (r :: Type) | r -> era where | |
LedgerEra BabbageEra = Ledger.Babbage | |
LedgerEra ConwayEra = Ledger.Conway |
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.
e242232
to
5f4f93a
Compare
db22ace
to
2fe093e
Compare
This module exposes a new api for accessible cardano eras. It focuses on the current mainnet era and the upcoming era if there is one. It differs from the previous api in that it does not concern itself with any era prior to what is currently on mainnet.
Deprecated Shelley era to Alonzo era in makeTransactionBodyAutoBalance
This module introduces the UnsignedTx type. This is meant to replace cardano-api's TxBody type UnsignedTx type has the benefit of being a thin wrapper around the ledger's Tx type which allows us to easily leverage the lenses and type classes exposed by ledger Moving to this type also reduces the maintenance burden of cardano-api as we will eventually deprecate the TxBody type
2fe093e
to
09b9d3c
Compare
Changelog
Context
Additional context for the PR goes here. If the PR fixes a particular issue please provide a link to the issue.
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