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

fix: revert breaking change on public input serialization #1790

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

odesenfans
Copy link
Contributor

@odesenfans odesenfans commented Jun 17, 2024

Context: integration of the OS + bootloader with the Stone prover.

Problem: The 0x prefix was removed from public memory hex values in fe72ee0. This is incompatible with the Stone prover and causes the following exception:

terminate called after throwing an instance of 'starkware::StarkwareException'
what():  /app/src/starkware/utils/to_from_string.cc:60: String ("40780017fff7fff") does not start with '0x'.

Solution: revert the change + fix the serialization/deserialization test.

Checklist

  • Linked to Github Issue
  • Unit tests added
  • Integration tests added.
  • This change requires new documentation.
    • Documentation has been added/updated.
    • CHANGELOG has been updated.

Problem: The 0x prefix was removed from public memory hex values in
fe72ee0. This is incompatible with the
Stone prover and causes an exception.

Solution: revert the change.
Copy link

codecov bot commented Jun 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.79%. Comparing base (5d11811) to head (fdf3fee).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1790   +/-   ##
=======================================
  Coverage   94.79%   94.79%           
=======================================
  Files         102      102           
  Lines       40141    40142    +1     
=======================================
+ Hits        38050    38051    +1     
  Misses       2091     2091           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@fmoletta fmoletta left a comment

Choose a reason for hiding this comment

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

This is currently working fine, the public memory values are being serialized as hex values prefixed by "0x" in the air public input file output. You can also see that introducing this change makes comparisons against the python vm fail in the CI

@pefontana
Copy link
Collaborator

Hi @odesenfans !
Can yoy solve some merge conflicts here so we can merge it!
The Serialization part we already added here #1799

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.

4 participants