-
Notifications
You must be signed in to change notification settings - Fork 21.2k
eth/tracers: fix testcase 7702_delegate #32349
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
Conversation
eca22b3
to
a9e3685
Compare
d2a8282
to
a9e3685
Compare
…iguration. remove changes from util.go
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.
Wish I knew what the test is actually doing so that I can verify the updated expected result.
The changes to util.go
were unecessary. We can just modify the test case to use a premerged genesis configuration.
The logic in With your changes, new test cases will need to handle this and update the However, with my original commit, they can simply ignore this and use a configuration closer to mainnet. |
How do you exactly reproduce this error? The test passes for me locally. I can also verify that a delegation is in fact invoked (from 0x17816E9A858b161c3E37016D139cf618056CaCD4 to 0xb684710e6d5914aD6e64493dE2a3c424CC43e970) and the prestate result includes the delegated code. |
Weird. I thought I had witnessed it failing locally, but indeed it does not.. |
The root cause is that |
Please share which is the delegate contract that is missing. I saw one delegation and that was included.
Where is it set to false? |
this PR have 3 commits now, I recommend only keep the first commit, check the first commit, you can see
then IsPrague will be set to false! so when IsPrague is false, the test case to test 7702-tx is meaning less. to be simple, can you just run the case |
@allformless I can confirm it now thank you. The reason for my confusion was that the tracer was in prague because it used chainConfig.IsPrague logic. However as you pointed out its the EVM that's not in prague. Please revert the last 2 commits. But instead of changing toBlockContext logic in the test, let's set the testcase's genesis.Config.TerminalTotalDifficulty to 0. |
This is exactly what my last two commits do. It's weird that this test-case is meant to test 7702 behavior, yet passes regardless of whether 7702 is enabled. That seems to indicate that the test is not sufficient. |
Ok I see pardon I was confused by the change in fork blocks. Yeah I ran it and it seems to work. I think the fix is good.
What was happening is: the tracer was correctly capturing the delegation because that happens right at the start of the tx (in fact tx is sent directly to a delegated account). However the EVM fails to correctly perform the delegation because it thinks it's still not in Prague so tx stops abruptly. Now with these changes it will finish processing the tx and so it captures more prestate. We just need to fix the merge conflict. |
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.
LGTM
case
7702_delegate
is used to test the trace result of processing a 7702 txbut
IsPrague
is not enabled when running, so the code0xef0100b684710e6d5914ad6e64493de2a3c424cc43e970
is run, and view '0xef' as an invalid opcode.this PR is to fix it.