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

version the new cost model deserializers #3274

Merged
merged 1 commit into from
Jan 31, 2023

Conversation

JaredCorduan
Copy link
Contributor

@JaredCorduan JaredCorduan commented Jan 30, 2023

Description

Enforce that the cost model deserializers prior to version 9 use a list of the expected length. This is required for full backwards compatibility (otherwise the current deserializers in ledger are more flexible than those currently used by the node).

Note that the conway CDDL file has been changed to reflect this change (and thus the CDDL tests as well).

This is the last changed needed to resolve #2902. In order to resolve #2902, we still need to support deserializing cost models for future version of Plutus (we do now support adding new builtins).

Checklist

  • Commit sequence broadly makes sense
  • Commits have useful messages
  • New tests are added if needed and existing tests are updated
  • Any changes are noted in the changelog
  • Code is formatted with fourmolu (which can be run with scripts/fourmolize.sh
  • Self-reviewed the diff

@JaredCorduan JaredCorduan force-pushed the jc/version-new-cost-model-deserializers branch from 2764c16 to 31b9e5b Compare January 31, 2023 13:37
Copy link
Contributor

@teodanciu teodanciu left a comment

Choose a reason for hiding this comment

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

Looks good to me

Copy link
Contributor

@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.

I am still a little fuzzy on how #2902 will work, but as far as this PR is concerned it looks good

Enforce that the cost model deserializers prior to version 9 use a list
of the expected length.
@JaredCorduan JaredCorduan force-pushed the jc/version-new-cost-model-deserializers branch from 31b9e5b to a1c06fd Compare January 31, 2023 18:51
@JaredCorduan JaredCorduan enabled auto-merge (rebase) January 31, 2023 19:12
@JaredCorduan JaredCorduan merged commit 6d2ee7b into master Jan 31, 2023
@iohk-bors iohk-bors bot deleted the jc/version-new-cost-model-deserializers branch January 31, 2023 20:52
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.

Problematic cost model serialization
3 participants