Skip to content

Fix Flaky Merge Test#12046

Merged
svlachakis merged 1 commit into
masterfrom
fix-flaky-cannot-produce-bad-blocks
Jun 18, 2026
Merged

Fix Flaky Merge Test#12046
svlachakis merged 1 commit into
masterfrom
fix-flaky-cannot-produce-bad-blocks

Conversation

@svlachakis

Copy link
Copy Markdown
Contributor

Test-only fix for a flaky CI test (Cannot_produce_bad_blocks). It was occasionally failing with a NullReferenceException because it asked for the second produced block before block production had finished, getting back null. The fix waits for the block to be produced (and null-checks it) before using it - same pattern the test already uses for the first block.

@claude

claude Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Claude finished @svlachakis's task in 1m 16s —— View job


PR Review: Fix Flaky Merge Test

  • Gathering context
  • Reviewing code changes
  • Posting findings

Summary

This is a clean, minimal fix for a real race condition. The change mirrors the existing pattern already used for the first block in the same test and is correct.

Analysis

Root cause of the flakiness — the original code called engine_getPayloadV1 immediately after engine_forkchoiceUpdatedV1 without waiting for block improvement to complete, so Data could be null if production hadn't finished yet.

The fix subscribes to the BlockImproved event before triggering the FCU (line 666), then awaits after (line 671). This is the correct ordering — subscribing after the triggering action would still have a race. Both the subscription-before and await-after steps match exactly what the test already does for the first block (lines 630 → 642).

The added null-check with a per-iteration message (Assert.That(getSecondBlockPayload, Is.Not.Null, $"iteration {iteration}")) is a useful diagnostic improvement on top of the race fix.

Findings

LowsecondNewPayload is typed string? but assigned from .PayloadId! (null-forgiving), so the declared nullable type and the usage with ! on line 673 are slightly inconsistent. This is pre-existing style in the test and not introduced by this PR, so no action needed.

No correctness, security, or performance issues. The fix is sound.

Verdict: Approved — mergeable.

@svlachakis svlachakis merged commit 2f7d61f into master Jun 18, 2026
560 checks passed
@svlachakis svlachakis deleted the fix-flaky-cannot-produce-bad-blocks branch June 18, 2026 06:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants