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

vm: missing beaconroot account verkle fix #3421

Merged
merged 6 commits into from
May 14, 2024

Conversation

gabrocheleau
Copy link
Contributor

@gabrocheleau gabrocheleau commented May 14, 2024

This PR addresses an issue where the account at the beaconRootAddress would be missing from the verkle witness, leading to an error since the statelessStateManager throws whenever there is an account missing in the witness. With this PR, we are fixing this in the same way that we are addressing the same issue that appeared with EIP-2935.

We are discussing with the verkle folks whether or not this should be included in the witness. We (Gajinder and I) are thinking that it should, as it would be nice for a stateless client not to depend on "local state" or "special cases handling" to execute a block. Eventually, the verkle behavior could change so that this becomes unnecessary.

This also updates the verkle test data to include a bigger, more complex verkle block, and fixes the eip-6800 (verkle) test by actually running the block rather than the individual transactions.

g11tech
g11tech previously approved these changes May 14, 2024
Copy link
Contributor

@g11tech g11tech left a comment

Choose a reason for hiding this comment

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

lgtm

@holgerd77
Copy link
Member

Ok, so this one is "on hold" then until things are settled? Will give this a "Blocked" label for now due to this assumption, feel free to remove for any reason.

@gabrocheleau
Copy link
Contributor Author

gabrocheleau commented May 14, 2024

Ok, so this one is "on hold" then until things are settled? Will give this a "Blocked" label for now due to this assumption, feel free to remove for any reason.

Just need to figure out why the postState access fails, otherwise this should be OK.

Copy link
Contributor

@g11tech g11tech 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants