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

test(starknet_os): test hint strings are unique #3700

Merged

Conversation

dorimedini-starkware
Copy link
Collaborator

No description provided.

@reviewable-StarkWare
Copy link

This change is Reviewable

Copy link
Collaborator Author

dorimedini-starkware commented Jan 26, 2025

Copy link

Benchmark movements:
tree_computation_flow performance improved 😺
tree_computation_flow time: [34.804 ms 34.825 ms 34.847 ms]
change: [-5.0579% -3.3956% -1.8657%] (p = 0.00 < 0.05)
Performance has improved.
Found 6 outliers among 100 measurements (6.00%)
4 (4.00%) high mild
2 (2.00%) high severe

@dorimedini-starkware dorimedini-starkware force-pushed the 01-26-feat_starknet_os_integrate_transaction_hash_hints branch from 51d9645 to a810643 Compare January 28, 2025 10:07
@dorimedini-starkware dorimedini-starkware force-pushed the 01-26-test_starknet_os_test_hint_strings_are_unique branch 2 times, most recently from 16892db to 8afa869 Compare January 28, 2025 15:33
Copy link
Collaborator Author

@dorimedini-starkware dorimedini-starkware 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: 3 of 5 files reviewed, all discussions resolved (waiting on @amosStarkware and @Yoni-Starkware)


crates/starknet_os/src/hints/enum_definition_test.rs line 13 at r3 (raw file):

Previously, amosStarkware wrote…

OK
consider adding a comment for this

added explicit check (so we don't rely on the match)

@dorimedini-starkware dorimedini-starkware force-pushed the 01-26-feat_starknet_os_integrate_transaction_hash_hints branch from 9668484 to ddeabca Compare January 28, 2025 16:36
@dorimedini-starkware dorimedini-starkware force-pushed the 01-26-test_starknet_os_test_hint_strings_are_unique branch from 8afa869 to 27c728c Compare January 28, 2025 16:36
@dorimedini-starkware dorimedini-starkware force-pushed the 01-26-feat_starknet_os_integrate_transaction_hash_hints branch from ddeabca to dce34f4 Compare January 28, 2025 20:55
@dorimedini-starkware dorimedini-starkware force-pushed the 01-26-test_starknet_os_test_hint_strings_are_unique branch from 27c728c to bb5523e Compare January 28, 2025 20:56
@dorimedini-starkware dorimedini-starkware force-pushed the 01-26-feat_starknet_os_integrate_transaction_hash_hints branch from dce34f4 to 30d2866 Compare January 29, 2025 07:43
@dorimedini-starkware dorimedini-starkware force-pushed the 01-26-test_starknet_os_test_hint_strings_are_unique branch from bb5523e to 3972cb7 Compare January 29, 2025 07:43
@dorimedini-starkware dorimedini-starkware force-pushed the 01-26-feat_starknet_os_integrate_transaction_hash_hints branch from 30d2866 to 85924a0 Compare January 29, 2025 08:13
@dorimedini-starkware dorimedini-starkware force-pushed the 01-26-test_starknet_os_test_hint_strings_are_unique branch from 3972cb7 to 82642cf Compare January 29, 2025 08:13
Copy link
Collaborator

@Yoni-Starkware Yoni-Starkware left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 2 of 5 files at r1, 1 of 1 files at r2, 2 of 2 files at r6, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @dorimedini-starkware)

@dorimedini-starkware dorimedini-starkware force-pushed the 01-26-feat_starknet_os_integrate_transaction_hash_hints branch from 85924a0 to 871192f Compare January 29, 2025 09:23
@dorimedini-starkware dorimedini-starkware force-pushed the 01-26-test_starknet_os_test_hint_strings_are_unique branch from 82642cf to 1ad40d0 Compare January 29, 2025 09:23
Copy link
Collaborator Author

@dorimedini-starkware dorimedini-starkware left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r7, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @dorimedini-starkware)

Copy link
Collaborator Author

@dorimedini-starkware dorimedini-starkware left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r8, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @dorimedini-starkware)

@dorimedini-starkware dorimedini-starkware force-pushed the 01-26-feat_starknet_os_integrate_transaction_hash_hints branch from a48c0ea to 46d1b35 Compare January 29, 2025 15:01
@dorimedini-starkware dorimedini-starkware force-pushed the 01-26-test_starknet_os_test_hint_strings_are_unique branch from 88f5536 to ccda0fc Compare January 29, 2025 15:01
@dafnamatsry dafnamatsry changed the base branch from 01-26-feat_starknet_os_integrate_transaction_hash_hints to main January 29, 2025 15:21
@dorimedini-starkware dorimedini-starkware added this pull request to the merge queue Jan 29, 2025
Merged via the queue into main with commit db7f1f3 Jan 29, 2025
37 of 39 checks passed
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