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

Dict hints #455

Merged
merged 9 commits into from
Jan 7, 2025
Merged

Dict hints #455

merged 9 commits into from
Jan 7, 2025

Conversation

ftheirs
Copy link
Collaborator

@ftheirs ftheirs commented Dec 27, 2024

Implement dictionary hints related to compression process.
Add constants.rs file to define constants that are needed across several hints

KZG tests still require extra hints.
Check orphan hints is failing because it detects two extra hints that will be covered in compress PR (COMPRESS and SET_DECOMPRESSED_DST)

Issue Number: N/A

Type

  • feature
  • bugfix
  • dev (no functional changes, no API changes)
  • fmt (formatting, renaming)
  • build
  • docs
  • testing

Description

Breaking changes?

  • yes
  • no

@ftheirs ftheirs changed the base branch from main to os_v0_13_3 December 31, 2024 13:20
@ftheirs ftheirs marked this pull request as ready for review December 31, 2024 13:23
@ftheirs ftheirs requested a review from whichqua January 2, 2025 15:20

let bucket_index = get_maybe_relocatable_from_var_name(vars::ids::BUCKET_INDEX, vm, ids_data, ap_tracking)?;

let prev_offset = dict_tracker.get(&bucket_index).unwrap().clone();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Handling this unwrap is the only thing I can thing of.

Copy link
Collaborator

@whichqua whichqua left a comment

Choose a reason for hiding this comment

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

Should be good!

@ftheirs ftheirs requested a review from GMKrieger January 6, 2025 12:21
Copy link
Collaborator

@notlesh notlesh left a comment

Choose a reason for hiding this comment

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

The code looks good to me, but I would suggest one thing:

In cairo-lang this all comes from a file called compression.cairo, and we have tried to keep our file structure similar to theirs. So, my suggestions:

  • Rename dict.rs -> compression.rs
  • Drop constants.rs and include its contents in compression.rs

@ftheirs ftheirs merged commit 801812b into os_v0_13_3 Jan 7, 2025
3 of 5 checks passed
@ftheirs ftheirs deleted the gm/dict_hints branch January 7, 2025 13:00
@ftheirs ftheirs mentioned this pull request Jan 17, 2025
9 tasks
whichqua added a commit that referenced this pull request Jan 23, 2025
* Ft/os upgrade (#453)

* update cairo-lang to v0.13.3

* expect Starknet v0.13.3 version

* set_state_updates_start & set_compressed_start

* update deserialize OS output

* hint implementation: set_n_updates_small

* disable test until we get a reference PIE with the new OS version

* set full_output for integration tests

* deserialize output fix tests

* update sequencer address

* fix CI: bump cairo-lang to v0.13.3

* clippy

* bump cairo-lang

* fix tests

* fix use_kzg_da global variable

* add hint test

* fix is_s_updates_small

* add tests for set_compressed_start and set_state_updates_start

* fix StarknetGeneralConfig

* add test with reference PIEs

* log2 hint (#454)

* feat: add the log2_ceil hint

* simplify log2 function

* clean up unimplemented.rs

---------

Co-authored-by: whichqua <[email protected]>

* Dict hints (#455)

* feat: add dict manager and get offset hints for v0.13.3

* use TOTAL_N_BUCKETS from constants

* handle error

* rename dict to compression

* compression constants

* change unimplemented.rs description

* clippy

---------

Co-authored-by: whichqua <[email protected]>

* Compression hint (#464)

* compression hint

* set_decompressed_dst hint

* use log2 from math.rs

* add test get_n_elms_per_felt

* fix: handle `full_output` compression correctly (#461)

* fix: handle kzg compression correctly

* lint: clippy and fmt

* fix: replace unwrap with expect for get_index

* add: docs and tests for the compression module

* remove unused constant

---------

Co-authored-by: Geoffrey Mureithi <[email protected]>

* feat: decompress the output to state diff (#466)

* feat: decompress the output to state diff

* chore: replace unwraps with expect

* fix: minor replacements of if with match

---------

Co-authored-by: whichqua <[email protected]>
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.

3 participants