-
Notifications
You must be signed in to change notification settings - Fork 65
blockifier_reexecution: move execute single tx from utils #10707
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
blockifier_reexecution: move execute single tx from utils #10707
Conversation
0709751 to
956123f
Compare
4d8d3cf to
1a77fe4
Compare
meship-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.
@meship-starkware reviewed 3 of 3 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @AvivYossef-starkware)
crates/blockifier_reexecution/src/state_reader/test_state_reader.rs line 480 at r1 (raw file):
/// fetch block context. Does not assert correctness, only prints the execution result. pub fn execute_single_transaction(self, tx_input: TransactionInput) -> ReexecutionResult<()> { let chain_id = self.next_block_state_reader.chain_id.clone();
Consider moving it into the second arm of the match, as we only use it there.
Code quote:
let chain_id = self.next_block_state_reader.chain_id.clone();crates/blockifier_reexecution/src/state_reader/test_state_reader.rs line 520 at r1 (raw file):
execution_results.first().expect("Expected exactly one execution result, but got none"); println!("Execution result: {:?}", res);
Do we want this print here? Wouldn't it be better to log it?
Code quote:
println!("Execution result: {:?}", res);
meship-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.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @AvivYossef-starkware)
crates/blockifier_reexecution/src/state_reader/test_state_reader.rs line 520 at r1 (raw file):
Previously, meship-starkware (Meshi Peled) wrote…
Do we want this print here? Wouldn't it be better to log it?
I see that we print in most places in the re-execution crate so I changed it to a quetion rether than blocking commant
1a77fe4 to
c3ca891
Compare
956123f to
9ee95c4
Compare
meship-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.
@meship-starkware reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @AvivYossef-starkware)
c3ca891 to
796e19e
Compare
9ee95c4 to
884ff2d
Compare
Merge activity
|
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.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @meship-starkware)
crates/blockifier_reexecution/src/state_reader/test_state_reader.rs line 480 at r1 (raw file):
Previously, meship-starkware (Meshi Peled) wrote…
Consider moving it into the second arm of the match, as we only use it there.
Done
crates/blockifier_reexecution/src/state_reader/test_state_reader.rs line 520 at r1 (raw file):
Previously, meship-starkware (Meshi Peled) wrote…
I see that we print in most places in the re-execution crate so I changed it to a quetion rether than blocking commant
We use it as a CLI tool, not as a library, so we want to print the result.
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 1 of 1 files at r2.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @AvivYossef-starkware)

No description provided.