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/bogotafrom
jochem-brouwer:pr/no-reset-reanchor-start-block
Draft

jochem-brouwer wants to merge 8 commits into
ethereum:forks/bogotafrom
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-->

@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 94.98%. Comparing base (64cbead) to head (e63c81d).

Additional details and impacted files
@@              Coverage Diff              @@
##           forks/bogota    #3507   +/-   ##
=============================================
  Coverage         94.98%   94.98%           
=============================================
  Files               673      673           
  Lines             40831    40831           
  Branches           3747     3747           
=============================================
  Hits              38785    38785           
  Misses             1429     1429           
  Partials            617      617           
Flag Coverage Δ
unittests 94.98% <ø> (ø)

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.

LouisTsai-Csie and others added 8 commits October 9, 2026 17:47
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`.
@fselmo
fselmo force-pushed the pr/no-reset-reanchor-start-block branch from 4dd4b6a to e63c81d Compare October 9, 2026 23:53
@fselmo
fselmo changed the base branch from forks/amsterdam to forks/bogota October 9, 2026 23:53
@fselmo

fselmo commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

forks/amsterdam is now frozen, so I moved this PR to forks/bogota and rebased it onto the new base. The rebase applied without conflicts. If you have local commits, please rebase them onto this branch before pushing again.

This branch has not been deployed

No deployments
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.

5 participants