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

eth/tracers/logger: support tracing system calls #30923

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

fjl
Copy link
Contributor

@fjl fjl commented Dec 17, 2024

The struct logger relies on the OnTxStart hook to store the execution context. In order to support tracing system calls, the logger also needs to listen to the OnSystemCallStartV2 hook.

@fjl fjl requested a review from s1na as a code owner December 17, 2024 11:30
@s1na
Copy link
Contributor

s1na commented Dec 17, 2024

Question how do you activate the struct logger on a block level? is it through the blocktest command?

@fjl
Copy link
Contributor Author

fjl commented Dec 17, 2024

Yeah I hit this issue during debugging, just setting the tracer in VMConfig around a system call.

s1na
s1na previously approved these changes Dec 17, 2024
Copy link
Contributor

@s1na s1na left a comment

Choose a reason for hiding this comment

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

LGTM

@fjl
Copy link
Contributor Author

fjl commented Dec 17, 2024

Sorry you have to approve again, I pushed something.

@s1na
Copy link
Contributor

s1na commented Dec 17, 2024

Ah hold on for the json logger we are omitting the system call traces. I'm wondering if it can lead to inconsistent results for evm blocktest based on which format is selected (struct vs json).

@fjl
Copy link
Contributor Author

fjl commented Dec 17, 2024

I don't think it will make a difference there. Right now, when tracing a system call with the struct logger, it just panics with nil pointer dereference. The PR fixes it to not crash and print the logs instead.

@s1na s1na self-assigned this Dec 17, 2024
@s1na
Copy link
Contributor

s1na commented Dec 19, 2024

Running the test fixtures/blockchain_tests/cancun/eip4788_beacon_root/beacon_root_contract/tx_to_beacon_root_contract.json with evm blocktest and comparing it between different formats I can see that indeed the struct and md loggers now different the system call opcodes but json logger doesn't. IMO we should skip the opcode traces in the other 2 formats or add a way in them to distinguish system call traces from those of normal txes.

$ ./evm  blocktest --trace --trace.format "struct" ...

CALLER          pc=00000000 gas=30000000 cost=2

PUSH20          pc=00000001 gas=29999998 cost=3
Stack:
00000000  0xfffffffffffffffffffffffffffffffffffffffe

EQ              pc=00000022 gas=29999995 cost=3
Stack:
00000000  0xfffffffffffffffffffffffffffffffffffffffe
00000001  0xfffffffffffffffffffffffffffffffffffffffe

PUSH1           pc=00000023 gas=29999992 cost=3
Stack:
00000000  0x1

JUMPI           pc=00000025 gas=29999989 cost=10
Stack:
00000000  0x4d
00000001  0x1

JUMPDEST        pc=00000077 gas=29999979 cost=1

PUSH3           pc=00000078 gas=29999978 cost=3

TIMESTAMP       pc=00000082 gas=29999975 cost=2
Stack:
00000000  0x1fff

MOD             pc=00000083 gas=29999973 cost=5
Stack:
00000000  0xc
00000001  0x1fff

TIMESTAMP       pc=00000084 gas=29999968 cost=2
Stack:
00000000  0xc

Vs

$ ./evm  blocktest --trace --trace.format "json" 

{"pc":0,"op":51,"gas":"0xeefac","gasCost":"0x2","memSize":0,"stack":[],"depth":1,"refund":0,"opName":"CALLER"}
{"pc":1,"op":115,"gas":"0xeefaa","gasCost":"0x3","memSize":0,"stack":["0xa94f5374fce5edbc8e2a8697c15331677e6ebf0b"],"depth":1,"refund":0,"opName":"PUSH20"}
{"pc":22,"op":20,"gas":"0xeefa7","gasCost":"0x3","memSize":0,"stack":["0xa94f5374fce5edbc8e2a8697c15331677e6ebf0b","0xfffffffffffffffffffffffffffffffffffffffe"],"depth":1,"refund":0,"opName":"EQ"}
{"pc":23,"op":96,"gas":"0xeefa4","gasCost":"0x3","memSize":0,"stack":["0x0"],"depth":1,"refund":0,"opName":"PUSH1"}
{"pc":25,"op":87,"gas":"0xeefa1","gasCost":"0xa","memSize":0,"stack":["0x0","0x4d"],"depth":1,"refund":0,"opName":"JUMPI"}
{"pc":26,"op":96,"gas":"0xeef97","gasCost":"0x3","memSize":0,"stack":[],"depth":1,"refund":0,"opName":"PUSH1"}
{"pc":28,"op":54,"gas":"0xeef94","gasCost":"0x2","memSize":0,"stack":["0x20"],"depth":1,"refund":0,"opName":"CALLDATASIZE"}
{"pc":29,"op":20,"gas":"0xeef92","gasCost":"0x3","memSize":0,"stack":["0x20","0x20"],"depth":1,"refund":0,"opName":"EQ"}

If you squint hard you can see the difference from pc: 25 onwards. I will go ahead and apply the same behaviour in those tracers.

@fjl
Copy link
Contributor Author

fjl commented Dec 19, 2024

I still don't understand how my change causes this difference

@s1na
Copy link
Contributor

s1na commented Dec 19, 2024

I hate it that I can't the cool trick from the json logger on these 2.

I still don't understand how my change causes this difference

Basically in the json logger the tracing gets disabled during system calls.

Copy link
Contributor

@s1na s1na left a comment

Choose a reason for hiding this comment

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

LGTM

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.

2 participants