-
Notifications
You must be signed in to change notification settings - Fork 156
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
Bring Plutus language version to the type level #3899
Conversation
88ccc10
to
616166e
Compare
33521d9
to
ce6d42b
Compare
ce6d42b
to
7ab1a2b
Compare
libs/cardano-ledger-test/src/Test/Cardano/Ledger/Examples/AlonzoBBODY.hs
Outdated
Show resolved
Hide resolved
c108aaa
to
ef9fc1e
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.
Looks great so far. I'll finish the rest of the review tomorrow. Some of the comments I wrote down for myself as notes, so feel free to ignore them for now.
eras/conway/impl/testlib/Test/Cardano/Ledger/Conway/TreeDiff.hs
Outdated
Show resolved
Hide resolved
libs/cardano-ledger-test/src/Test/Cardano/Ledger/Examples/AlonzoInvalidTxUTXOW.hs
Show resolved
Hide resolved
libs/cardano-ledger-test/src/Test/Cardano/Ledger/Examples/AlonzoInvalidTxUTXOW.hs
Show resolved
Hide resolved
libs/cardano-ledger-test/src/Test/Cardano/Ledger/Examples/AlonzoCollectInputs.hs
Show resolved
Hide resolved
875aab9
to
e34b840
Compare
@Soupstraw I've addressed all of your comments. |
b55cc2e
to
91f4f22
Compare
This is a major change to how we represent Plutus Scripts. This affects how we collect and evaluate plutus scripts. There is now a unified interface for all plutus versions and incorrect usage of an unsupported plutus version in all eras is prevented by the type system. Translation of PlutusContext has also been overhauled. There is still quite a bit duplication happens with TxInfo translation, but this will be taken care of by a subsequent commit.
* `Cardano.Ledger.Alonzo.Plutus.Context`
91f4f22
to
5bb3488
Compare
eras/alonzo/test-suite/src/Test/Cardano/Ledger/Alonzo/Translation/TranslatableGen.hs
Show resolved
Hide resolved
libs/cardano-ledger-api/src/Cardano/Ledger/Api/Scripts/ExUnits.hs
Outdated
Show resolved
Hide resolved
libs/cardano-ledger-api/src/Cardano/Ledger/Api/Scripts/ExUnits.hs
Outdated
Show resolved
Hide resolved
libs/cardano-ledger-api/src/Cardano/Ledger/Api/Scripts/ExUnits.hs
Outdated
Show resolved
Hide resolved
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.
This is a huge improvement. LGTM overall, but I'm sure I didn't catch everything because it's such a massive PR with lots of moving parts.
3494c1e
to
7d28d69
Compare
7d28d69
to
fa4ad77
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.
Epic!
The relationship between everything involved needed to execute the scripts is now so clear, as is, supporting a new Plutus Version, and/or Era.
I'm in an awe of this work! 🙇
@Soupstraw and @teodanciu Thank you for reviewing this PR! I know it was not a small task. |
Description
Fixes #3849 and #3406
Other improvements:
UTxO
the validation functionppViewHashesMatch
now acceptsScriptsProvided
and remove the no longer neededlanguages
function. This approach avoids duplicate computation ofScritpsProvided
, which was done in thelanguages
function.Checklist
.cabal
andCHANGELOG.md
files according to theversioning process.
.cabal
files for all affected packages are updated. If you change the bounds in a cabal file, that package itself must have a version increase. (See RELEASING.md)CHANGELOG.md
for the affected packages. New section is never added with the code changes. (See RELEASING.md)fourmolu
(usescripts/fourmolize.sh
)scripts/cabal-format.sh
)hie.yaml
has been updated (usescripts/gen-hie.sh
)