Skip to content

fix(fill): re-anchor start_block under --no-reset-between-tests - #3507

Draft
jochem-brouwer wants to merge 8 commits into
ethereum:forks/amsterdamfrom
jochem-brouwer:pr/no-reset-reanchor-start-block
Draft

fix(fill): re-anchor start_block under --no-reset-between-tests#3507
jochem-brouwer wants to merge 8 commits into
ethereum:forks/amsterdamfrom
jochem-brouwer:pr/no-reset-reanchor-start-block

Conversation

@jochem-brouwer

Copy link
Copy Markdown
Member

Description

Problem

--no-reset-between-tests keeps the chain moving between fills, but the
session anchor never moves with it.

_session_pre_run captures start_block once, from latest, after the
global setup:

@pytest.fixture(scope="session", autouse=True)
def _session_pre_run(...):
    ...
    # 4. Capture start block (head after global setup).
    start_block = eth_rpc.get_block_by_number("latest")
    client_backend.start_block = start_block

That is the only production write to client_backend.start_block. Every
test then chains its first block from it, in make_stateful_fixture.

Normally the invariant holds because _reset_chain_between_tests rewinds
the chain back to that block after each test. Under
--no-reset-between-tests the rewind is skipped and nothing takes its
place, so the anchor goes stale as soon as the first test builds a block.

The nonce side of the flag was already adapted — worker_key reads at
latest under the flag, with a comment explaining the inversion. The
anchor side was missed.

Related Issues or PRs

Stacked on #3304, which introduces the --no-reset-between-tests flag this fixes. Until that PR merges, the diff here includes its commits; the change in this PR is the single commit on top, by @skylenet. See also jochem-brouwer#6

Checklist

  • Ran fast static checks to avoid CI fails, see Code Standards & Verifying Changes: just static
  • PR title has the form <type>(<area>): <title>, where <type> and <area> come from an appropriate C-<type>, respectively A-<area>, label. The title should match the target squash commit message.

Cute Animal Picture

Put a link to a cute animal picture inside the parenthesis-->

LouisTsai-Csie and others added 8 commits August 24, 2026 20:57
The session captures start_block once, at the head that follows the
global setup. `_reset_chain_between_tests` rewinds to that block after
every test, which keeps the anchor correct for the next fill.

`--no-reset-between-tests` skips the rewind, but nothing advances the
anchor in its place. The first test moves the head, and every later test
still chains its first block from the stale anchor. The client then
rejects the parent ("parentHash is not current head"), or, on a long run
against a non-archive client, reports "historical state ... is not
available" once that block's state is pruned.

Read the head before each test and re-anchor on it. Consumers are not
affected: they route pre-run setup by directory (`pre_run/*.json`,
applied once per session), not by a fixture's `start_block_hash`.
@jochem-brouwer jochem-brouwer added the backport benchmarks/amsterdam On merge, bot cherry-picks the squash commit to `benchmarks/amsterdam` and opens a PR. Squash only. label Sep 2, 2026
@jochem-brouwer

Copy link
Copy Markdown
Member Author

This builds on top of #3304 so that should first be merged.

@codecov

codecov Bot commented Sep 2, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.90%. Comparing base (786c0d4) to head (4dd4b6a).
⚠️ Report is 34 commits behind head on forks/amsterdam.

Additional details and impacted files
@@                 Coverage Diff                 @@
##           forks/amsterdam    #3507      +/-   ##
===================================================
+ Coverage            93.54%   93.90%   +0.35%     
===================================================
  Files                  624      624              
  Lines                37074    36905     -169     
  Branches              3394     3326      -68     
===================================================
- Hits                 34681    34655      -26     
+ Misses                1644     1569      -75     
+ Partials               749      681      -68     
Flag Coverage Δ
unittests 93.90% <ø> (+0.35%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@danceratopz

Copy link
Copy Markdown
Member

This builds on top of #3304 so that should first be merged.

What do you think about adding that to the top of the PR description and moving this to draft?

@jochem-brouwer
jochem-brouwer marked this pull request as draft September 3, 2026 09:08
@jochem-brouwer

jochem-brouwer commented Sep 3, 2026

Copy link
Copy Markdown
Member Author

@danceratopz that PR (#3304) adds the --no-rest-between-tests flag which we slightly alter here.

I have drafted this PR because from https://github.com/jochem-brouwer/execution-specs/pull/6/commits we need to add two more commits 3d072ce and e269bb4. The latter somehow raises a merge conflict if I cherry-pick it. Will look into it.

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

Labels

backport benchmarks/amsterdam On merge, bot cherry-picks the squash commit to `benchmarks/amsterdam` and opens a PR. Squash only.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants