-
Notifications
You must be signed in to change notification settings - Fork 65
apollo_batcher: use genesis block hash for hash of block 0 #11660
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
apollo_batcher: use genesis block hash for hash of block 0 #11660
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
e0a0fc4 to
4cb0f64
Compare
dorimedini-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.
@dorimedini-starkware reviewed 4 files and all commit messages, and made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @amosStarkware and @yoavGrs).
crates/starknet_api/src/block.rs line 281 at r2 (raw file):
impl BlockHash { // TODO(Amos): Use this anywhere the genesis block hash is used. pub const GENESIS: Self = Self(StarkHash::ZERO);
genesis block is block zero, which has a "real" hash; StarkHash::ZERO is the "hash of it's parent"
Suggestion:
pub const GENESIS_PARENT_HASH: Self = Self(StarkHash::ZERO);
amosStarkware
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.
@amosStarkware made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @dorimedini-starkware and @yoavGrs).
crates/starknet_api/src/block.rs line 281 at r2 (raw file):
Previously, dorimedini-starkware wrote…
genesis block is block zero, which has a "real" hash;
StarkHash::ZEROis the "hash of it's parent"
are you sure?
here GENESIS_HASH is the hash of block -1
dorimedini-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.
@dorimedini-starkware made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @amosStarkware and @yoavGrs).
crates/starknet_api/src/block.rs line 281 at r2 (raw file):
Previously, amosStarkware wrote…
are you sure?
here GENESIS_HASH is the hash of block -1
hmmm... seems misnamed there, IMO.
see here: "genesis hash" is the parent hash of the "first block hash".
genesis block is block zero.
I would rename here, maybe ask whoever wrote the original GENESIS_HASH constant if that' what they meant?
(also may be worth replacing all usages of the existing GENESIS_HASH with your new const
dorimedini-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.
@dorimedini-starkware made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @amosStarkware and @yoavGrs).
crates/starknet_api/src/block.rs line 281 at r2 (raw file):
Previously, dorimedini-starkware wrote…
hmmm... seems misnamed there, IMO.
see here: "genesis hash" is the parent hash of the "first block hash".
genesis block is block zero.
I would rename here, maybe ask whoever wrote the originalGENESIS_HASHconstant if that' what they meant?
(also may be worth replacing all usages of the existingGENESIS_HASHwith your new const
(not in this PR of course)
4cb0f64 to
f9a81fb
Compare
amosStarkware
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.
@amosStarkware made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @dorimedini-starkware and @yoavGrs).
crates/starknet_api/src/block.rs line 281 at r2 (raw file):
Previously, dorimedini-starkware wrote…
(not in this PR of course)
Done.
dorimedini-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.
@dorimedini-starkware reviewed 2 files and all commit messages, and resolved 1 discussion.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @yoavGrs).
d6cde97

No description provided.