-
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
Add PParamUpdates to the plutus context, by transforming them to Plutus Data #3999
Conversation
60d5784
to
749fa0a
Compare
Still a work in progress as it doesn't do anything yet with the plutus context |
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.
There are some issues in the PR, but the overall direction is great!
The only thing that is missing now is the actual translation to PlutusV3 context and roundtrip tests that should have the reusable functionality placed in cardano-ledger-core:testlib:Test.Cardano.Ledger.Plutus.ToPlutusData
and the actual Conway tests should go into cardano-ledger-conway:tests:Test.Cardano.Ledger.Conway.Plutus.ToPlutusData
libs/cardano-ledger-core/src/Cardano/Ledger/Plutus/ToPlutusData.hs
Outdated
Show resolved
Hide resolved
libs/cardano-ledger-core/src/Cardano/Ledger/Plutus/ToPlutusData.hs
Outdated
Show resolved
Hide resolved
libs/cardano-ledger-core/src/Cardano/Ledger/Plutus/ToPlutusData.hs
Outdated
Show resolved
Hide resolved
libs/cardano-ledger-core/src/Cardano/Ledger/Plutus/ToPlutusData.hs
Outdated
Show resolved
Hide resolved
libs/cardano-ledger-core/src/Cardano/Ledger/Plutus/ToPlutusData.hs
Outdated
Show resolved
Hide resolved
libs/cardano-ledger-core/src/Cardano/Ledger/Plutus/ToPlutusData.hs
Outdated
Show resolved
Hide resolved
libs/cardano-ledger-core/src/Cardano/Ledger/Plutus/ToPlutusData.hs
Outdated
Show resolved
Hide resolved
libs/cardano-ledger-core/src/Cardano/Ledger/Plutus/ToPlutusData.hs
Outdated
Show resolved
Hide resolved
libs/cardano-ledger-core/src/Cardano/Ledger/Plutus/ToPlutusData.hs
Outdated
Show resolved
Hide resolved
d99889f
to
1ea5e01
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.
This looks great. Semantically that is exactly what we need.
Just a few comments on shuffling things around for consistency with the rest of the codebase
libs/cardano-ledger-core/testlib/Test/Cardano/Ledger/Plutus/ToPlutusData.hs
Outdated
Show resolved
Hide resolved
libs/cardano-ledger-core/testlib/Test/Cardano/Ledger/Plutus/ToPlutusData.hs
Outdated
Show resolved
Hide resolved
libs/cardano-ledger-core/testlib/Test/Cardano/Ledger/Plutus/ToPlutusData.hs
Outdated
Show resolved
Hide resolved
eras/conway/impl/testlib/Test/Cardano/Ledger/Conway/Plutus/ToPlutusData.hs
Outdated
Show resolved
Hide resolved
d144ec8
to
7a4041a
Compare
5ad5f34
to
356ae86
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.
@TimSheard There was a few things still needed to be rearranged in this PR, but because we will try to make a release today and this functionality is needed for the next release I took upon myself to finish this up. Check out the last few commits to see what I've done.
356ae86
to
0296139
Compare
* Add `Cardano.Ledger.Core.Plutus.ToPlutusData` * Add ToPlutusData class * Add ToPlutusData instances in `Cardano.Ledger.Conway.PParams` * Use table driven approach to make instance for PParamsUpdate * Property tests added * Add module `Cardano.Ledger.Conway.Plutus.Context` * Add class `ToPlutusData (PParamsUpdate era) => ConwayEraPlutusTxInfo (l :: Language) era` * Complete module `Cardano.Ledger.Conway.TxInfo` using `ConwayEraPlutusTxInfo` instance
This change is needed for consistency with how the rest of the translation works and to avoid constructing unnecessary `Proxy`, since `Plutus a` is already used as a proxy.
This is necessary to remove such instance for PlutusV1, PlutusV2 and the future languages past PlutusV3. Make `EraPlutusTxInfo` usperclass of `ConwayEraPlutusTxInfo` for consistent hierarchy of classes
0296139
to
6901667
Compare
Add PParamUpdates to the plutus context, by transforming them to Plutus Data
Adds a new class ToPlutusData
Uses table driven approach to make instance for PParamsUpdate
resolves #3994
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
)