-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
feat(vm): Implement call tracing for fast VM #2905
Conversation
1cc9ec9
to
6a0c824
Compare
6a0c824
to
52a6a72
Compare
9078e12
to
7ef9bd9
Compare
This does not turn enable call tracing in API because of the details of how that happens depend on #2863 so it is better to wait until both PRs are in. |
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.
This is certainly not the domain I'm comfortable with, but I've looked through the diff and it doesn't look suspicious. Not very helpful, but LGTM 😅
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.
Didn't find any serious issues. Two very confusing spots that can probably be fixed with comments.
@@ -191,15 +193,14 @@ impl CallTracer { | |||
.farcall | |||
.parent_gas | |||
.saturating_sub(state.vm_local_state.callstack.current.ergs_remaining as u64); | |||
|
|||
self.save_output_latest(state, memory, ret_opcode, &mut current_call.farcall); | |||
|
|||
// If there is a parent call, push the current call to it | |||
// Otherwise, push the current call to the stack, because it's the top level call |
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.
This comment is outdated.
let output_ptr = FatPointer::from(maybe_output_ptr); | ||
if output_ptr.length == 0 && output_ptr.offset == 0 { | ||
// Trivial pointer, which is formally cannot be dereferenced. This only matters | ||
// when extracting the revert reason; the legacy VM treats the trivial pointer specially. |
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.
Does this case require special handling? I'd imagine that reading a length zero pointer produces an empty array anyway.
Nit: extra is
.
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.
It seems to cause "Unknown revert reason". So length 0, offset 0 is unknown reason but offset 1 is not?
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.
As you've noted, the reason for special handling is purely corresponding to traces produced by the legacy implementation. Subjectively, this special case makes little sense, but it's easy to implement and I assume that compatibility has higher priority.
So length 0, offset 0 is unknown reason but offset 1 is not?
Yep, this is how it works in the legacy VM:
zksync-era/core/lib/multivm/src/tracers/call_tracer/vm_latest/mod.rs
Lines 145 to 153 in 731b824
if !fat_data_pointer.is_trivial() { | |
Some(memory.read_unaligned_bytes( | |
fat_data_pointer.memory_page as usize, | |
fat_data_pointer.start as usize, | |
fat_data_pointer.length as usize, | |
)) | |
} else { | |
None | |
} |
🤖 I have created a release *beep* *boop* --- ## [26.1.0](core-v26.0.0...core-v26.1.0) (2025-01-21) ### Features * update l2 erc20 bridge address in updater as well ([#3500](#3500)) ([fe3c7b2](fe3c7b2)) * **vm:** Implement call tracing for fast VM ([#2905](#2905)) ([731b824](731b824)) ### Bug Fixes * copy special case to fast VM call tracer ([#3509](#3509)) ([995e583](995e583)) * fix execute encoding for transactions ([#3501](#3501)) ([4c381a8](4c381a8)) * **gateway:** erc20 workaround for gateway upgrade ([#3511](#3511)) ([c140ba8](c140ba8)) ### Performance Improvements * optimize get_unsealed_l1_batch_inner ([#3491](#3491)) ([9b121c9](9b121c9)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: zksync-era-bot <zksync-era-bot@users.noreply.github.com>
What ❔
Why ❔
Call tracing is the last large missing part of the fast VM.