Skip to content
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

hide CostModel constructor #2703

Merged
merged 1 commit into from
Mar 25, 2022
Merged

Conversation

JaredCorduan
Copy link
Contributor

The CostModel constructor should have been hidden when the EvaluationContext was added to it, costModelParamsToCostModel is the appropriate smart constructor that everyone should use. The evaluation
context is completely dependent on the cost model parameters.

Copy link
Collaborator

@lehins lehins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some suggestions, but with respect to improving safety of the code this is great

eras/alonzo/impl/src/Cardano/Ledger/Alonzo/Scripts.hs Outdated Show resolved Hide resolved
eras/alonzo/impl/src/Cardano/Ledger/Alonzo/Scripts.hs Outdated Show resolved Hide resolved
eras/alonzo/impl/src/Cardano/Ledger/Alonzo/Scripts.hs Outdated Show resolved Hide resolved
eras/alonzo/impl/src/Cardano/Ledger/Alonzo/Scripts.hs Outdated Show resolved Hide resolved
eras/alonzo/impl/src/Cardano/Ledger/Alonzo/Scripts.hs Outdated Show resolved Hide resolved
eras/alonzo/impl/src/Cardano/Ledger/Alonzo/Scripts.hs Outdated Show resolved Hide resolved
eras/alonzo/impl/src/Cardano/Ledger/Alonzo/Scripts.hs Outdated Show resolved Hide resolved
eras/alonzo/impl/src/Cardano/Ledger/Alonzo/Scripts.hs Outdated Show resolved Hide resolved
eras/alonzo/impl/src/Cardano/Ledger/Alonzo/Scripts.hs Outdated Show resolved Hide resolved
The CostModel constructor should have been hidden when the
EvaluationContext was added to it, costModelParamsToCostModel is the
appropriate smart constructor that everyone should use. The evaluation
context is completely dependend on the cost model parameters.
@JaredCorduan JaredCorduan force-pushed the jc/hide-cost-model-constructor branch from 4354289 to 60c1c9f Compare March 25, 2022 20:43
@JaredCorduan JaredCorduan merged commit b1ac16b into master Mar 25, 2022
@iohk-bors iohk-bors bot deleted the jc/hide-cost-model-constructor branch March 25, 2022 22:08
JaredCorduan pushed a commit that referenced this pull request Apr 18, 2022
In the Alonzo era, the map of languages to cost models was mistakenly encoded
using an indefinite CBOR map (contrary to cannonical CBOR, as intended) when
computing the script integrity hash.

For this reason, PlutusV1 remains with this encoding.
Future versions of Plutus, starting with PlutusV2 in the Babbage era, will
use the intended definite length encoding.

PR #2703 accidentally reversed this Alonzo bug, this commit restores it.
JaredCorduan pushed a commit that referenced this pull request Apr 18, 2022
In the Alonzo era, the map of languages to cost models was mistakenly encoded
using an indefinite CBOR map (contrary to cannonical CBOR, as intended) when
computing the script integrity hash.

For this reason, PlutusV1 remains with this encoding.
Future versions of Plutus, starting with PlutusV2 in the Babbage era, will
use the intended definite length encoding.

PR #2703 accidentally reversed this Alonzo bug, this commit restores it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants