Handle wrong fork/finalization on hello - #598
Merged
Merged
Conversation
twoeths
changed the title
mpetrunic
suggested changes
Dec 29, 2019
twoeths
changed the title
mpetrunic
reviewed
Jan 3, 2020
| return true; | ||
| } | ||
| if (state.finalizedCheckpoint.epoch >= request.finalizedEpoch && | ||
| !request.finalizedRoot.equals(getBlockRoot(this.config, state, request.finalizedEpoch))) { |
Member
There was a problem hiding this comment.
if I'm not mistaken getBlockRoot only contains root for recent slots. I think it's safer to fetch block from database by request.finalizedEpoch start slot.
You should try to avoid fetching entire state here as that call isn't cheap, especially considering that this method will be called on each peer connect.
Member
Author
There was a problem hiding this comment.
I replaced getBlockRoot by getting block root from database as you said.
Regarding the state, I still need it for other calculations.
mpetrunic
previously requested changes
Jan 3, 2020
| } | ||
| // TODO handle incorrect forkVersion or disjoint finalizedCheckpoint | ||
| if (await this.shouldDisconnectOnHello(peerInfo, request)) { | ||
| this.network.reqResp.goodbye(peerInfo, 0n); |
Member
Author
There was a problem hiding this comment.
I correct the one in stop() too
mpetrunic
reviewed
Jan 7, 2020
| this.network.reqResp.removeListener("request", this.onRequest); | ||
| await Promise.all( | ||
| this.network.getPeers().map((peerInfo) => | ||
| this.network.reqResp.goodbye(peerInfo, 0n))); |
Member
There was a problem hiding this comment.
I would extract reason codes to enum
| return true; | ||
| } | ||
| const startSlot = computeStartSlotOfEpoch(this.config, request.finalizedEpoch); | ||
| const startBlock = await this.db.block.get(startSlot); |
Member
There was a problem hiding this comment.
If that block is finalized it will be in this.db.blockArchive
twoeths
force-pushed
the
tuyen/handle-status-message
branch
from
January 8, 2020 01:47
073121b to
6270e00
Compare
nflaig
pushed a commit
that referenced
this pull request
Jun 9, 2026
- Wires the new `fast_confirmation` Server-Sent Event from beacon-APIs PR [#598](ethereum/beacon-APIs#598). The event fires once per slot whenever the Fast Confirmation Rule executes and carries `{block, slot}`, where `slot` is the slot of the confirmed beacon block. - Crosses the fork-choice ↔ beacon-node boundary via a new optional `onFastConfirmation` callback on `ForkChoiceStore`, mirroring the existing `onJustified` / `onFinalized` plumbing. The emit is invoked from `ForkChoice.runFastConfirmation()` after the rule succeeds. - Removes the now-redundant Lodestar-namespace endpoint `GET /eth/v1/lodestar/fast_confirmation_info` (and its `getConfirmedBlock` helper) — the standard SSE event supersedes it, and the head/checkpoint fields it bundled are already available via standard beacon-API endpoints. This PR is aligned with the changes proposed in ethereum/beacon-APIs#616 ### Architecture ``` Chain.onClockSlot → forkChoice.updateTime └── (per tick) runFastConfirmation └── fcStore.notifyFastConfirmation({block, slot}) └── ChainEventEmitter.emit(EventType.fastConfirmation, {block, slot}) └── SSE subscribers via /eth/v1/events?topics=fast_confirmation ``` `ApiEvents` in `ChainEventEmitter` is derived from `routes.events.EventType`, so adding the new variant flows through automatically — no per-event boilerplate in the chain or events API layers. ### Edge cases | Scenario | Behavior | |---|---| | `--chain.fastConfirmation` disabled (default) | No emit (FCR doesn't run) | | FCR rule throws | No emit; existing warn-and-continue catch is unchanged | | Confirmed root not in `protoArray` (defensive) | Warn log with `slot`+`confirmedRoot`, skip emit | | `updateTime` advances multiple slots | One emit per tick |
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
3 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.
Goal