-
Notifications
You must be signed in to change notification settings - Fork 65
blockifier_reexecution: refactor get block header #11374
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
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
AvivYossef-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.
@AvivYossef-starkware made 1 comment.
Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @avi-starkware and @noaov1).
crates/blockifier_reexecution/src/state_reader/rpc_state_reader.rs line 257 at r1 (raw file):
})?, ); }
I tested it, and it is unnecessary when using rpc-v0.10, even for old blocks.
Code quote:
if block_header_map.get("l2_gas_price").is_none() {
// In old blocks, the l2_gas_price field is not present.
block_header_map.insert(
"l2_gas_price".to_string(),
to_value(GasPricePerToken {
price_in_wei: 1_u8.into(),
price_in_fri: 1_u8.into(),
})?,
);
}ab01f5b to
29fc954
Compare
e69ee23 to
0bd6454
Compare
avi-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.
@avi-starkware reviewed 1 file and all commit messages, and made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @AvivYossef-starkware and @noaov1).
crates/blockifier_reexecution/src/state_reader/rpc_state_reader.rs line 257 at r1 (raw file):
Previously, AvivYossef-starkware wrote…
I tested it, and it is unnecessary when using rpc-v0.10, even for old blocks.
Are you sure the l2_gas_price is unnecessary for old blocks? Why was it added in the first place if that is the case?
AvivYossef-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.
@AvivYossef-starkware made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @avi-starkware and @noaov1).
crates/blockifier_reexecution/src/state_reader/rpc_state_reader.rs line 257 at r1 (raw file):
Previously, avi-starkware (Avi Cohen) wrote…
Are you sure the
l2_gas_priceis unnecessary for old blocks? Why was it added in the first place if that is the case?
If I remember correctly, we didnt know that newer rpc version would solve it.
Look at blockheader.json
It's an old block and it has the default l2_gas_price
AvivYossef-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.
@AvivYossef-starkware resolved 1 discussion.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @noaov1).
29fc954 to
c95562f
Compare
0bd6454 to
abcff6b
Compare
avi-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.
@avi-starkware made 1 comment.
Reviewable status: 1 of 3 files reviewed, all discussions resolved (waiting on @noaov1).
abcff6b to
1058db4
Compare
AvivYossef-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.
@AvivYossef-starkware reviewed all commit messages.
Reviewable status: 1 of 3 files reviewed, all discussions resolved (waiting on @noaov1).
AvivYossef-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.
@AvivYossef-starkware reviewed 2 files.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @noaov1).

No description provided.