Skip to content
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

fix(tracing): fix the parity vmTrace stack pushes #5561 #5563

Merged
merged 5 commits into from
Nov 25, 2023

Conversation

gbrew
Copy link
Contributor

@gbrew gbrew commented Nov 24, 2023

Fix #5561

Change tracing config to keep track of No stack, only pushed items, or whole stack.

Fix the parity tracer to just use the pushed items directly.

@gbrew gbrew requested a review from mattsse as a code owner November 24, 2023 22:35
/// For reference: <https://github.com/ledgerwatch/erigon/blob/9b74cf0384385817459f88250d1d9c459a18eab1/turbo/jsonrpc/trace_adhoc.go#L451>
pub(crate) fn stack_push_count(step_op: OpCode) -> usize {
let step_op = step_op.get();
if (opcode::PUSH0..=opcode::PUSH32).contains(&step_op) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this can be a single match expression just like the Go switch reference

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this can be a single match expression just like the Go switch reference

Good call, just pushed that change.

Copy link
Collaborator

@mattsse mattsse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is great, tysm!

I only replaces the == with is_ functions

@mattsse mattsse added this pull request to the merge queue Nov 25, 2023
Merged via the queue into paradigmxyz:main with commit 4679bce Nov 25, 2023
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

parity vmTrace stack pushes are incorrect
3 participants