-
Notifications
You must be signed in to change notification settings - Fork 721
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
Remove simple script distinction #4763
Conversation
63921d8
to
060cc49
Compare
6edf77c
to
73bdb37
Compare
go (RequireSignature (PaymentKeyHash kh)) | ||
= Shelley.RequireSignature (Shelley.coerceKeyRole kh) | ||
go (RequireAllOf s) = Shelley.RequireAllOf (map go s) | ||
go (RequireAnyOf s) = Shelley.RequireAnyOf (map go s) | ||
go (RequireMOf m s) = Shelley.RequireMOf m (map go s) | ||
go timelock = error $ "toShelleyMultiSig: " <> show timelock <> | ||
" not supported in MultiSig scripts." |
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'm wondering if it might be better not to error
here.
Using error
from the CLI is okay, but error
would cause an application that uses the API to exit.
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.
So I opted to return an Either
as you suggested for toShelleyMultiSig
because this is a function a consumer of the api is likely to use and potentially shoot themselves in the foot. However toShelleyMultiSig
is called in: toShelleyScript :: ScriptInEra era -> Ledger.Script (ShelleyLedgerEra era)
. I opted to error here instead because this will error
if someone is running a Shelley era network and tries to use a Timelock. I think the odds of this is extremely low and not worth polluting the code further with an Either
. Lmk what you think.
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 let this pass for now given the low probably of it being a problem, but we should also create a ticket to remove calls to error
from cardano-api
as a separate piece of work to be done after the next release.
06238b7
to
0e5d91e
Compare
75d9ed7
to
b8aa9dc
Compare
SimpleScript' Remove type variable from SimpleScript data declaration
b8aa9dc
to
776a29e
Compare
Resolves: #4261
Timelock scripts are a superset of MultiSig scripts in that they can also express validation in terms of before or after a particular slot.
In
cardano-ledger
the serialization of Timelock and MultiSig scripts are identical.Due to these properties, it is not necessary to distinguish between MultiSig and Timelock scripts. We can default to Timelock scripts without issue.