Skip to content

Add random execution_payload_header to Bellatrix state in tests#4047

Closed
KatyaRyazantseva wants to merge 10 commits intoethereum:devfrom
KatyaRyazantseva:random-execution-payload
Closed

Add random execution_payload_header to Bellatrix state in tests#4047
KatyaRyazantseva wants to merge 10 commits intoethereum:devfrom
KatyaRyazantseva:random-execution-payload

Conversation

@KatyaRyazantseva
Copy link
Copy Markdown
Contributor

This PR adds random execution_payload_header to random Bellatrix state in tests

Comment thread tests/core/pyspec/eth2spec/test/helpers/execution_payload.py Outdated
@jtraglia
Copy link
Copy Markdown
Member

jtraglia commented Dec 13, 2024

Hey Katya. I've pulled some updates to the Makefile into your branch. Please run git pull, then make clean (just this once, because of the Makefile changes), and then make lint to see some linter issues that need to be fixed.

Comment thread tests/core/pyspec/eth2spec/test/helpers/execution_payload.py Outdated
Comment thread tests/core/pyspec/eth2spec/test/helpers/execution_payload.py Outdated
@KatyaRyazantseva KatyaRyazantseva marked this pull request as ready for review December 19, 2024 18:21


def build_randomized_execution_payload(spec, state, rng):
from eth2spec.test.helpers.random import exit_random_validators
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@jtraglia I have a circular import error when I put the import at the beginning. Is there a way to avoid it here?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hey @KatyaRyazantseva. We shouldn't actually need this import. exit_random_validators mutates the input state and I don't believe we should do that. We can simply rely on get_expected_withdrawals for withdrawals. Given a random state for Capella+, there should (in theory) be some exited validators, which would cause there to be withdrawals.

Also, when building the randomized execution payload header, we can/should re-use the fields from the random execution payload we just built. Then all we need to do is fill in transactions_root and withdrawals_root which is relatively easy. I've pushed a commit for this.

One more thing, I haven't thoroughly checked these randomized payload to ensure they make sense. But this should be enough to unblock you.

@jtraglia
Copy link
Copy Markdown
Member

Hey @KatyaRyazantseva I'm going to close this PR. If you decide to pick this up again in the future, please reopen.

@jtraglia jtraglia closed this Feb 26, 2025
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