fix: remove TOO_MANY_SKIPPED_SLOTS block error - #9484
Merged
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request removes the TOO_MANY_SKIPPED_SLOTS validation check from gossip block validation, preventing legitimate post-skip blocks from being ignored. Instead, the number of skipped slots is now computed, returned by validateGossipBlock, and tracked via a new histogram metric (lodestar_gossip_block_skipped_slots) and debug logs. Corresponding unit tests and error codes have been cleaned up. There are no review comments to address, and the changes look solid.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
twoeths
marked this pull request as ready for review
June 9, 2026 06:42
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## unstable #9484 +/- ##
============================================
- Coverage 52.59% 52.58% -0.01%
============================================
Files 848 848
Lines 60890 60879 -11
Branches 4485 4483 -2
============================================
- Hits 32023 32014 -9
+ Misses 28805 28803 -2
Partials 62 62 🚀 New features to boost your workflow:
|
Contributor
Performance Report🚀🚀 Significant benchmark improvement detected
Full benchmark results
|
wemeetagain
approved these changes
Jun 9, 2026
Member
|
@lodekeeper can you double check this against the spec, is there such a rule in the spec? (also would we pass gossip validation spec tests when removing this) |
Contributor
|
Spec-checked — short answer: no such rule in spec, and gossip validation pyspec tests would pass. Is there a TOO_MANY_SKIPPED_SLOTS rule in spec? No.
No skip-distance limit anywhere. The diff's own deleted comment ( Would gossip validation spec tests pass? Yes. The pyspec coverage at DoS footnote: the original comment was right that a long parent→block gap forces multiple epoch transitions before sig verification (~750ms each per the chain.ts comment), so this gate was load-shedding. The PR trades that local mitigation for spec compliance and instruments the actual skip distribution via Unrelated benchmark alert: the |
nflaig
reviewed
Jun 9, 2026
| // single bad block can trigger | ||
| // Note: Ensure this check is done before calling chain.regen.getBlockSlotStat as this is the function that does various epoch transitions. | ||
| // Note: This validation check is not part of the spec. | ||
| if (chain.opts.maxSkipSlots != null && parentBlock.slot + chain.opts.maxSkipSlots < blockSlot) { |
Member
There was a problem hiding this comment.
worth pointing out that this is 32 by default, so with slotImportTolerance being 32 by default, this is very unlikely to happen as we won't be subscribed to gossip if our head is 32 slots behind clock slot, although it can eg. for a block on a different branch
Contributor
There was a problem hiding this comment.
Good point — that's another reason the gate was effectively dead code for our own canonical chain: gossip subscription is already gated by slotImportTolerance = SLOTS_PER_EPOCH = 32, so a block where parent.slot + 32 < block.slot under our canonical head would already be filtered before reaching validateGossipBlock. The different-branch case you flagged is the residual scenario — a peer gossips a block whose parent is on a side fork older than head - slotImportTolerance. The new lodestar_gossip_block_skipped_slots histogram should surface that distribution from mainnet; if a non-trivial tail shows up from competing-branch gossip, that data tells us whether a non-gating mitigation is warranted (peer budget, per-shuffle work cap, etc.).
nflaig
approved these changes
Jun 9, 2026
This was referenced Jun 9, 2026
Member
|
🎉 This PR is included in v1.44.0 🎉 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
TOO_MANY_SKIPPED_SLOTblock error but still let the gossip block run through the syncDescription
Closes #9483
AI Assistance Disclosure