-
Notifications
You must be signed in to change notification settings - Fork 166
Add ability to inject any cost models via AlonzoGenesis
#5379
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
Conversation
31af8b6 to
262c0b3
Compare
d9f0fd1 to
91bd434
Compare
412397f to
6f33fbc
Compare
lehins
left a comment
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 close to what we need, but with current PR extraConfig is completely unused. In other words, it is not enough to add a field to the Genesis file, we need to get it to inject into the ledger state. For this you need to modify this function that will take this cost models from extra config and inject it into the ledger state, potentially overriding a PlutusV1 cost model if one is also supplied through "extraConfig": https://github.com/IntersectMBO/cardano-ledger/blob/d120d9613810c0dc9e22fe2d1c104d8240aeb163/eras/alonzo/impl/src/Cardano/Ledger/Alonzo/Transition.hs#L30C3-L30C22
So, this function should be used for injecting, so that only the supplied cost models gets overridden:
cardano-ledger/libs/cardano-ledger-core/src/Cardano/Ledger/Plutus/CostModels.hs
Lines 390 to 400 in d120d96
| -- | Updates the first @`CostModels`@ with the second one, so that only the cost models | |
| -- that are present in the second one get updated while all the others stay | |
| -- unchanged. Language specific errors and unknown cost models are removed, whenever a | |
| -- valid `CostModel` for the language is supplied in the update. | |
| updateCostModels :: | |
| -- | Old CostModels that will be overwritten | |
| CostModels -> | |
| -- | New CostModels that will overwrite | |
| CostModels -> | |
| CostModels | |
| updateCostModels (CostModels oldValid oldUnk) (CostModels modValid modUnk) = |
One last thing, but extremely important one, we are not allowed to change Genesis files after the era has been hard forked into.
bd913d7 to
e419167
Compare
Lucsanszky
left a comment
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 addressed the suggestions, so it is now actually overriding the cost models by using updateCostModels in Alonzo injectIntoTestState and also keeps the JSON and binary representations intact while making the Haskell representation more restrictive (only PlutusV1 cost models are allowed outside the extra config). Hope this is more inline with what we need!
e419167 to
b081d49
Compare
b081d49 to
1cf7578
Compare
lehins
left a comment
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 good. Some changes are still needed though.
3ce4054 to
c6148bc
Compare
lehins
left a comment
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.
Perfect! Thank you!
Only allow `PlutusV1` when parsing cost models in `AlonzoGenesis` and `UpgradeAlonzoPParams`.
* Override cost models when injecting into test state (if extra config cost models are provided) * Preserve `AlonzoGenesis` JSON and binary representations but change the Haskell side representation
41ce2a0 to
3e1ff5f
Compare
Description
Resolves #5342
Checklist
CHANGELOG.mdfiles updated for packages with externally visible changes.NOTE: New section is never added with the code changes. (See RELEASING.md).
.cabalandCHANGELOG.mdfiles when necessary, according to theversioning process.
.cabalfiles updated when necessary.NOTE: If bounds change in a cabal file, that package itself must have a version increase. (See RELEASING.md).
scripts/fourmolize.sh).scripts/cabal-format.sh).scripts/gen-cddl.sh)hie.yamlupdated (usescripts/gen-hie.sh).