Skip to content

Add initial fork choice tests for Gloas#4940

Merged
jtraglia merged 4 commits intoethereum:masterfrom
brech1:feat/fork-choice-tests
Mar 13, 2026
Merged

Add initial fork choice tests for Gloas#4940
jtraglia merged 4 commits intoethereum:masterfrom
brech1:feat/fork-choice-tests

Conversation

@brech1
Copy link
Member

@brech1 brech1 commented Feb 18, 2026

Description

Adds initial fork choice tests for Gloas. Covers store initialization and the on_execution_payload handler that transitions blocks from EMPTY to FULL when a builder reveals a payload.

Checklist

  • make lint
  • make reftests fork=gloas runner=fork_choice
  • make test fork=gloas k=test_genesis

Relations

@github-actions github-actions bot added the testing CI, actions, tests, testing infra label Feb 18, 2026
@brech1 brech1 force-pushed the feat/fork-choice-tests branch from 33d57ca to 00235a3 Compare February 19, 2026 00:42
@jtraglia
Copy link
Member

Hey @brech1, nice thanks!

I want to point out that the make test command you mentioned in the PR description will not run these tests; the k flag collects all tests which contain the input string & "fork_choice" is not in "test_genesis" or "test_on_execution_payload". You could run make test fork=gloas k=test_genesis for example, or make reftests fork=gloas runner=fork_choice to run generate all fork-choice tests.

Also, I believe we'll need to update this test case to not run in Gloas+:

@with_altair_and_later
@spec_state_test
def test_genesis(spec, state):

On the other hand, if it's possible to add Gloas support to the function above, instead of defining a new test_genesis test function, I think that would be preferable.

@brech1 brech1 force-pushed the feat/fork-choice-tests branch from a067284 to e3841e7 Compare February 20, 2026 02:22
@brech1
Copy link
Member Author

brech1 commented Feb 20, 2026

I didn't realize k parameter was fn name matching, my bad. I ran both suggested commands and received correct outputs.

I was also not aware that it was ok/preferred to modify tests from older version specs on the go, adjusting them to newer params/cfg, instead of duplicating them/generating new ones on their respective fork directory.

Thanks a lot for the review!

@brech1 brech1 force-pushed the feat/fork-choice-tests branch 3 times, most recently from 09b429a to c1d663c Compare February 25, 2026 22:02
@jtraglia
Copy link
Member

Hey @brech1 we recently merged this PR which is why the CI check is failing.

You'll need to replace SECONDS_PER_SLOT with SLOT_DURATION_MS and divide by 1000 there.

@brech1 brech1 force-pushed the feat/fork-choice-tests branch from edd5f0c to ab3dc4b Compare March 12, 2026 14:03
Copy link
Member

@jtraglia jtraglia left a comment

Choose a reason for hiding this comment

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

LGTM, let's merge this. If for whatever reason there is an issue, clients can disable the test & we'll fix it in the next release.

@jtraglia jtraglia merged commit 4b6f527 into ethereum:master Mar 13, 2026
16 checks passed
@brech1 brech1 deleted the feat/fork-choice-tests branch March 13, 2026 13:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

testing CI, actions, tests, testing infra

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants