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

Make block_number configurable in genesis header again #2044

Open
fselmo opened this issue Nov 18, 2021 · 4 comments
Open

Make block_number configurable in genesis header again #2044

fselmo opened this issue Nov 18, 2021 · 4 comments

Comments

@fselmo
Copy link
Collaborator

fselmo commented Nov 18, 2021

What is wrong?

  • After London changes, certain genesis header params became unconfigurable. Introduce back the ability to configure block_number. See eth-tester issue #225.

How can it be fixed

  • fill_header_params_from_parent() needs the ability to pass in a block_number kwarg to be configurable again. This call is made from the create_header_from_parent() method in the header classes.

  • block_number should be a parameter in the fill_header_params_from_parent() method that can be configured but should still keep the current default to GENESIS_BLOCK_NUMBER if the parent is None or to the parent block_number + 1 if the parent exists.

At a quick glance I believe this is the only change necessary. Testing should be added as well.

@catoenm
Copy link

catoenm commented May 23, 2022

Only question here is whether or not we should be passing the parent_hash too when block_number is specified. Is this correct?

@fselmo
Copy link
Collaborator Author

fselmo commented May 23, 2022

@catoenm I'd be curious to hear your use-case for this change. It would be strange to me to provide a block_number and have a parent OR not assume the parent is the genesis header. And in that case we don't need to provide the parent hash.

In other words... if the header to be configured has a parent, then the block_number shouldn't be configurable; it should be parent.block_number + 1. If the header to be configured does not have a parent, then it should be assumed to be genesis.

In this sense something like this might make the most sense ? :

    if parent is None:
        parent_hash = GENESIS_PARENT_HASH
        block_number = block_number if block_number else GENESIS_BLOCK_NUMBER
        if state_root is None:
            state_root = BLANK_ROOT_HASH
    else:
        parent_hash = parent.hash

        if block_number:
            raise PyEVMError("block_number cannot be configured if a parent header exists.")
        block_number = BlockNumber(parent.block_number + 1)

        if state_root is None:
            state_root = parent.state_root

Let me know if I'm missing anything. @Pet3ris I'd be curious on your input here and use-case as well.


edit: This validation also makes sense to me considering the original issue and the validation already in place, as outlined in this comment: ethereum/eth-tester#225 (comment)

@catoenm
Copy link

catoenm commented May 23, 2022

This makes a lot of sense. I've updated #2063 accordingly.

@Pet3ris
Copy link

Pet3ris commented May 24, 2022

@fselmo Thanks a lot for looking into this! I'm using py-evm as the underlying VM for a debugger. In order to handle forking, I have to override the block number to match the block number on the forked chain. I was using genesis params to do so :).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants