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

Update DicManager + add FinalizeDict hint #5628

Closed

Conversation

fmoletta
Copy link

@fmoletta fmoletta commented May 23, 2024

This PR does the following:

  • Update how the DictManger handles dictionaries to use temporary segments. This includes the changes to the DictManger that were merged into cairo-vm's Cairo1HintProcessor but are yet to be included here. They were introduced by PR Added segment arena cairo1 runner validation. lambdaclass/cairo-vm#1721. It also includes the fixes to dictionary relocation added by PR Move dict finalization to FinalizeDict Hint lambdaclass/cairo-vm#1774
  • Add and implement hint FinalizeDict. This hint is in charge of finalizing and relocating the current dictionary after it has been squashed.
  • Modify the casm generated by the felt252_dict_squash libfunc to add a call to FinalizeDict hint right after the squashing has taken place.

This changes ensure that there are no differences in behaviour between the hint processors for cairo 1 in cairo and cairo-vm crates, and fixes temporary segment relocation for dictionaries (that is currently bugged in cairo-vm).

Note: The pythonic hints still use the old DictManager implementation (without temporary segments), so the FinalizeDict is a NoOp in that context. I think the pythonic hints along with the pythonic DictManager for cairo 1 could also be updated in the future to match this new behaviour.


This change is Reviewable

@fmoletta fmoletta marked this pull request as ready for review May 24, 2024 15:36
Copy link
Collaborator

@orizi orizi left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 4 files reviewed, 2 unresolved discussions (waiting on @fmoletta)


crates/cairo-lang-runner/src/casm_run/dict_manager.rs line 79 at r1 (raw file):

    /// Relocates the dictionary if the previous dictionary was also relocated, if not, assigns a temporary next_start to aid in a future relocation.
    /// Relocates the next dictionary if it was already finalized but not relocated.
    pub fn finalize_segment(

in any case - this entire solution doesn't work - the fix was done in my PR:


crates/cairo-lang-sierra-to-casm/src/invocations/felt252_dict.rs line 115 at r1 (raw file):

                final_squashed_dict_start,
                final_squashed_dict_end) = call DestructDict;
            hint FinalizeDict {

not allowed adding hints in existing libfuncs - this would mean that classes previously declared would not work.

you can add it in CoreHint::GetSegmentArenaIndex handilng.

@fmoletta
Copy link
Author

Closed in favour of lambdaclass/cairo-vm#1776

@fmoletta fmoletta closed this May 30, 2024
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