Skip to content

Conversation

@nimrod-starkware
Copy link
Contributor

No description provided.

@reviewable-StarkWare
Copy link

This change is Reviewable

Copy link
Contributor Author

nimrod-starkware commented Nov 5, 2025

This stack of pull requests is managed by Graphite. Learn more about stacking.

@nimrod-starkware nimrod-starkware force-pushed the nimrod/BHM/move_hash_output branch from e5d852e to 9df4271 Compare November 5, 2025 13:43
@github-actions
Copy link

github-actions bot commented Nov 5, 2025

Benchmark movements: tree_computation_flow performance improved 😺 tree_computation_flow time: [13.792 ms 13.949 ms 14.113 ms] change: [-6.0530% -4.6126% -3.0766%] (p = 0.00 < 0.05) Performance has improved. Found 2 outliers among 100 measurements (2.00%) 2 (2.00%) high mild

Copy link
Contributor

@yoavGrs yoavGrs left a comment

Choose a reason for hiding this comment

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

@yoavGrs reviewed 51 of 51 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @nimrod-starkware)


crates/starknet_api/src/hash.rs line 72 at r1 (raw file):

}

impl HashOutput {

Single impl

Suggestion:

impl HashOutput {
    pub const ROOT_OF_EMPTY_TREE: Self = Self(Felt::ZERO);

crates/starknet_api/src/hash.rs line 73 at r1 (raw file):

impl HashOutput {
    pub fn from_hex(hex_string: &str) -> Result<Self, FromStrError> {

Consider moving impl_from_hex_for_felt_wrapper to here.

Code quote:

pub fn from_hex(hex_string: &str) -> Result<Self, FromStrError>

@nimrod-starkware nimrod-starkware force-pushed the nimrod/BHM/move_hash_output branch from 62f4f28 to a6e83f3 Compare November 10, 2025 15:41
Copy link
Contributor Author

@nimrod-starkware nimrod-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: all files reviewed, 2 unresolved discussions (waiting on @yoavGrs)


crates/starknet_api/src/hash.rs line 72 at r1 (raw file):

Previously, yoavGrs wrote…

Single impl

Done.


crates/starknet_api/src/hash.rs line 73 at r1 (raw file):

Previously, yoavGrs wrote…

Consider moving impl_from_hex_for_felt_wrapper to here.

Added a todo.

@nimrod-starkware nimrod-starkware force-pushed the nimrod/BHM/move_hash_output branch from 618a1ab to bda5575 Compare November 12, 2025 08:19
Copy link
Contributor

@yoavGrs yoavGrs left a comment

Choose a reason for hiding this comment

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

@yoavGrs reviewed 10 of 10 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @nimrod-starkware)

@nimrod-starkware nimrod-starkware added this pull request to the merge queue Nov 12, 2025
Merged via the queue into main-v0.14.1-committer with commit 583b5fe Nov 12, 2025
30 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Nov 14, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants