-
Notifications
You must be signed in to change notification settings - Fork 65
starknet_api: define partial block hash #12450
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
base: main-v0.14.2
Are you sure you want to change the base?
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
nimrod-starkware
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nimrod-starkware made 1 comment.
Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @ArielElp).
crates/starknet_api/src/block_hash/block_hash_calculator.rs line 260 at r1 (raw file):
)?; Ok(PartialBlockHash(block_hash.0)) }
consider this
IMO it's cleaner
Suggestion:
/// Hash of [`PartialBlockHashComponents`] only (no state root or parent hash).
#[derive(Clone, Copy, Debug, Default, PartialEq, Eq, Hash, Serialize, Deserialize)]
pub struct PartialBlockHash(pub StarkHash);
impl PartialBlockHash {
const GLOBAL_ROOT_FOR_PARTIAL_BLOCK_HASH = ..
const PARENT_HASH_FOR_PARTIAL_BLOCK_HASH = ..
/// Hash of [`PartialBlockHashComponents`].
/// Uses the same formula as [`calculate_block_hash`] with the constants above for state root and
/// parent hash.
pub fn from_partial_components(
partial_block_hash_components: &PartialBlockHashComponents,
) -> StarknetApiResult<Self> {
let block_hash = calculate_block_hash(
partial_block_hash_components,
Self::GLOBAL_ROOT_FOR_PARTIAL_BLOCK_HASH,
Self::PARENT_HASH_FOR_PARTIAL_BLOCK_HASH,
)?;
Ok(Self(block_hash.0))
}71a6a74 to
9d1db12
Compare
ArielElp
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ArielElp made 1 comment.
Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @nimrod-starkware).
crates/starknet_api/src/block_hash/block_hash_calculator.rs line 260 at r1 (raw file):
Previously, nimrod-starkware wrote…
consider this
IMO it's cleaner
Done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
nimrod-starkware
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nimrod-starkware made 3 comments and resolved 1 discussion.
Reviewable status: 0 of 1 files reviewed, 4 unresolved discussions (waiting on @ArielElp).
crates/starknet_api/src/block_hash/block_hash_calculator.rs line 179 at r2 (raw file):
/// Hash of [`PartialBlockHashComponents`]. /// Uses the same formula as [`calculate_block_hash`] with the fixed constants above for the /// state root and parent hash..
Suggestion:
state root and parent hash.crates/starknet_api/src/block_hash/block_hash_calculator.rs line 180 at r2 (raw file):
/// Uses the same formula as [`calculate_block_hash`] with the fixed constants above for the /// state root and parent hash.. fn from_partial_block_hash_components(
Suggestion:
pub fn from_partial_block_hash_components(crates/starknet_api/src/block_hash/block_hash_calculator.rs line 188 at r2 (raw file):
Self::PARENT_HASH_FOR_PARTIAL_BLOCK_HASH, )?; Ok(PartialBlockHash(block_hash.0))
Suggestion:
Self9d1db12 to
fc53903
Compare
ArielElp
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ArielElp made 3 comments.
Reviewable status: 0 of 1 files reviewed, 3 unresolved discussions (waiting on @nimrod-starkware).
crates/starknet_api/src/block_hash/block_hash_calculator.rs line 179 at r2 (raw file):
/// Hash of [`PartialBlockHashComponents`]. /// Uses the same formula as [`calculate_block_hash`] with the fixed constants above for the /// state root and parent hash..
Done.
crates/starknet_api/src/block_hash/block_hash_calculator.rs line 180 at r2 (raw file):
/// Uses the same formula as [`calculate_block_hash`] with the fixed constants above for the /// state root and parent hash.. fn from_partial_block_hash_components(
Done.
crates/starknet_api/src/block_hash/block_hash_calculator.rs line 188 at r2 (raw file):
Self::PARENT_HASH_FOR_PARTIAL_BLOCK_HASH, )?; Ok(PartialBlockHash(block_hash.0))
Done.
fc53903 to
e529641
Compare
nimrod-starkware
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nimrod-starkware reviewed 1 file and all commit messages, made 1 comment, and resolved 3 discussions.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @ArielElp).

Note
Low Risk
Additive API change that reuses existing hashing logic with fixed constants; low risk aside from potential downstream assumptions about hash semantics/usage.
Overview
Adds a new
PartialBlockHashwrapper type representing the hash ofPartialBlockHashComponentsonly (excluding state root and parent hash).PartialBlockHash::from_partial_block_hash_componentsreuses the existingcalculate_block_hashformula by fixingstate_rootandparent_hashto zero constants, and introduces the neededStarkHashimport/serialization traits for the new type.Written by Cursor Bugbot for commit e529641. This will update automatically on new commits. Configure here.