Skip to content

fix(tests): EIP-7702: use penultimate block number instead of fixing block 0 #1390

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

sonhv0212
Copy link
Contributor

πŸ—’οΈ Description

In the test case that uses setcode tx to call the history contract, the block number is fixed 0, which leads to failure when running in execute mode because block number 0 is usually out of range for history contract queries.

This PR uses:

  • execute mode: penultimate block number queried from node instead of 0
  • fill mode: use block number 0, same as before

@sonhv0212
Copy link
Contributor Author

cc @marioevz , seems that you're the author of this test case

@spencer-tb spencer-tb requested a review from marioevz April 7, 2025 05:08
@@ -2656,7 +2656,8 @@ def test_set_code_to_system_contract(
caller_payload = consolidation_request.calldata
call_value = consolidation_request.value
case Address(0x0000F90827F1C53A10CB7A02335B175320002935): # EIP-2935
caller_payload = Hash(0)
# Use the penultimate block to ensure it is saved in the history contract
caller_payload = Hash(max(pre.get_block_number() - 1, 0))
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't it be in env?

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 think it should be in env too, but I can't find how to get block number from node without using _eth_rpc of Alloc, can you suggest a better solution?

Copy link
Member

Choose a reason for hiding this comment

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

I agree that this should be in the env, and I think this requires a bit more complex solution.

We probably need to take the same approach as pre where it's a global fixture generated by the filler/execute plugin:

In the filler plugin, the fixture result would always be an instance of Environment with the default values.

In the execute plugin, the fixture is also an instance of Environment but the values are populated using the return self._eth_rpc... methods.

@spencer-tb spencer-tb added fork:prague Prague hardfork type:chore Type: Chore scope:execute Scope: Changes to the execute command labels Apr 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fork:prague Prague hardfork scope:execute Scope: Changes to the execute command type:chore Type: Chore
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants