Skip to content

Fix duplicate FCR run in stale GU test - #5499

Merged
mkalinin merged 2 commits into
ethereum:masterfrom
syjn99:syjn99/fcr-fix-restart-gu-stale
Aug 4, 2026
Merged

Fix duplicate FCR run in stale GU test#5499
mkalinin merged 2 commits into
ethereum:masterfrom
syjn99:syjn99/fcr-fix-restart-gu-stale

Conversation

@syjn99

@syjn99 syjn99 commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Share similar problem (FCR should run once per slot) with #5498, but this PR actually deduplicates FCR run unlike #5498.

  • test_fcr_no_restart_if_head_gu_is_stale

This test runs FCR twice when slot number is 24. Line 459-463 advances the store and updates FCR variables until slot 24 (3 * S). After that, the store never advances, but at line 483 it runs FCR update again which makes previous_slot_head == current_slot_head because:

def update_fast_confirmation_variables(fcr_store: FastConfirmationStore) -> None:
    # Update prev and curr slot head
    store = fcr_store.store
    fcr_store.previous_slot_head = fcr_store.current_slot_head
    fcr_store.current_slot_head = get_head(store).root
    
    # omit after...

which is not a state we want:

  Slot 23 FCR: previous=A, current=B
  Slot 24 FCR: previous=B, current=HEAD
  Slot 24 again: previous=HEAD, current=HEAD

Related to #5400 as this test was added there.

@github-actions github-actions Bot added the testing CI, actions, tests, testing infra label Jul 30, 2026
@jihoonsong
jihoonsong requested a review from mkalinin August 1, 2026 17:26

@mkalinin mkalinin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Good catch!

@mkalinin
mkalinin merged commit 95bdc67 into ethereum:master Aug 4, 2026
16 checks passed
matthewkeil pushed a commit to ChainSafe/lodestar that referenced this pull request Aug 10, 2026
**Motivation**

Update spec tests to v1.7.0-alpha.13 FCR fixes proposed from #9690
([consensus-specs#5489](ethereum/consensus-specs#5489),
[consensus-specs#5490](ethereum/consensus-specs#5490))
and complete the missing FCR assertions.

**Description**

- Unskip `is_one_confirmed_fails_large_validator_slashed`, fixed
upstream by
[consensus-specs#5490](ethereum/consensus-specs#5490)
- Enable the gloas `fast_confirmation` suite — gloas alpha.13 state
transition and `get_safe_execution_block_hash` landed via #9393
- Assert `safe_execution_block_hash`
([consensus-specs#5449](ethereum/consensus-specs#5449))
and the `FastConfirmationStore` variables, exposed via
`IForkChoice.getFastConfirmationStore()` (debug API endpoint in a
follow-up PR)
- Skip vectors blocked on upstream vector artifacts, documented inline;
the schedule-related ones are already fixed upstream
([consensus-specs#5498](ethereum/consensus-specs#5498),
[consensus-specs#5499](ethereum/consensus-specs#5499))
and unskip on the next spec-tests release

`fast_confirmation` (minimal): 1416 passed / 20 skipped / 0 failed. Full
spec suites green: minimal 70,899 passed, mainnet 13,586 passed, 0
failures.

Refs #9690

**AI Assistance Disclosure**

- [x] I have read the [contributor
guidelines](https://github.com/ChainSafe/lodestar/blob/unstable/CONTRIBUTING.md#ai-assistance-notice)
and disclosed my usage of AI below.

Implemented and verified with AI assistance (Claude Code); all changes
reviewed and tests executed locally.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
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