-
Notifications
You must be signed in to change notification settings - Fork 27
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
refactor(blockifier): native syscall handler new
method params
#1380
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1380 +/- ##
===========================================
+ Coverage 40.10% 67.34% +27.23%
===========================================
Files 26 102 +76
Lines 1895 13690 +11795
Branches 1895 13690 +11795
===========================================
+ Hits 760 9219 +8459
- Misses 1100 4070 +2970
- Partials 35 401 +366 ☔ View full report in Codecov by Sentry. |
3e9210b
to
edde72b
Compare
c782ef6
to
fe9fe6d
Compare
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.
Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @rodrigo-pino)
crates/blockifier/src/execution/native/entry_point_execution.rs
line 29 at r1 (raw file):
let syscall_handler: NativeSyscallHandler<'_> = NativeSyscallHandler::new(&call, state, resources, context);
Suggestion:
NativeSyscallHandler::new(call, state, resources, context);
crates/blockifier/src/execution/native/entry_point_execution.rs
line 32 at r1 (raw file):
run_native_executor(&contract_class.executor, function_id, call, syscall_handler) }
Suggestion:
run_native_executor(&contract_class.executor, function_id, syscall_handler)
}
crates/blockifier/src/execution/native/entry_point_execution.rs
line 38 at r1 (raw file):
function_id: FunctionId, call: CallEntryPoint, mut syscall_handler: NativeSyscallHandler<'_>,
Can you use the call
in the syscall handler? (see my suggestion below)
Suggestion:
fn run_native_executor(
native_executor: &AotNativeExecutor,
function_id: FunctionId,
mut syscall_handler: NativeSyscallHandler<'_>,
crates/blockifier/src/execution/native/entry_point_execution.rs
line 65 at r1 (raw file):
call: CallEntryPoint, call_result: ContractExecutionResult, syscall_handler: NativeSyscallHandler<'_>,
Suggestion:
fn create_callinfo(
call_result: ContractExecutionResult,
syscall_handler: NativeSyscallHandler<'_>,
crates/blockifier/src/execution/native/syscall_handler.rs
line 34 at r1 (raw file):
pub caller_address: ContractAddress, pub contract_address: ContractAddress, pub entry_point_selector: Felt,
Suggestion:
pub context: &'state mut EntryPointExecutionContext,
pub call: CallEntryPoint
fe9fe6d
to
8be2077
Compare
67c4e9c
to
76f983f
Compare
8cfeaa7
to
576ccaf
Compare
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: 0 of 2 files reviewed, 5 unresolved discussions (waiting on @noaov1)
crates/blockifier/src/execution/native/entry_point_execution.rs
line 29 at r1 (raw file):
let syscall_handler: NativeSyscallHandler<'_> = NativeSyscallHandler::new(&call, state, resources, context);
Done.
crates/blockifier/src/execution/native/entry_point_execution.rs
line 32 at r1 (raw file):
run_native_executor(&contract_class.executor, function_id, call, syscall_handler) }
Done.
crates/blockifier/src/execution/native/entry_point_execution.rs
line 38 at r1 (raw file):
Previously, noaov1 (Noa Oved) wrote…
Can you use the
call
in the syscall handler? (see my suggestion below)
Done.
crates/blockifier/src/execution/native/entry_point_execution.rs
line 65 at r1 (raw file):
call: CallEntryPoint, call_result: ContractExecutionResult, syscall_handler: NativeSyscallHandler<'_>,
Done.
crates/blockifier/src/execution/native/syscall_handler.rs
line 34 at r1 (raw file):
pub caller_address: ContractAddress, pub contract_address: ContractAddress, pub entry_point_selector: Felt,
Done.
Artifacts upload triggered. View details here |
576ccaf
to
e203e9f
Compare
Artifacts upload triggered. View details here |
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.
Reviewed 1 of 2 files at r3, 1 of 1 files at r6, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @rodrigo-pino)
This change is