Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions packages/beacon-node/src/api/impl/validator/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ import {
SyncCommitteeErrorCode,
} from "../../../chain/errors/index.js";
import {ChainEvent, CommonBlockBody} from "../../../chain/index.js";
import {PREPARE_NEXT_SLOT_BPS} from "../../../chain/prepareNextSlot.js";
import {getPrepareNextSlotMs} from "../../../chain/prepareNextSlot.js";
import {BlockType, ProduceFullDeneb, ProduceFullGloas} from "../../../chain/produceBlock/index.js";
import {RegenCaller} from "../../../chain/regen/index.js";
import {CheckpointHexPayload} from "../../../chain/stateCache/types.js";
Expand Down Expand Up @@ -1102,8 +1102,7 @@ export function getValidatorApi(
const head = chain.forkChoice.getHead();
let state: IBeaconStateView | undefined = undefined;
const startSlot = computeStartSlotAtEpoch(epoch);
const prepareNextSlotLookAheadMs =
config.SLOT_DURATION_MS - config.getSlotComponentDurationMs(PREPARE_NEXT_SLOT_BPS);
const prepareNextSlotLookAheadMs = config.SLOT_DURATION_MS - getPrepareNextSlotMs(config, startSlot);
const toNextEpochMs = msToNextEpoch();
// validators may request next epoch's duties when it's close to next epoch
// this is to avoid missed block proposal due to 0 epoch look ahead
Expand Down
4 changes: 2 additions & 2 deletions packages/beacon-node/src/chain/chain.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1080,12 +1080,12 @@ export class BeaconChain implements IBeaconChain {
};
}

recomputeForkChoiceHead(caller: ForkchoiceCaller): ProtoBlock {
recomputeForkChoiceHead(caller: ForkchoiceCaller, slot?: Slot): ProtoBlock {
this.metrics?.forkChoice.requests.inc();
const timer = this.metrics?.forkChoice.findHead.startTimer({caller});

try {
return this.forkChoice.updateAndGetHead({mode: UpdateHeadOpt.GetCanonicalHead}).head;
return this.forkChoice.updateAndGetHead({mode: UpdateHeadOpt.GetCanonicalHead, slot}).head;
} catch (e) {
this.metrics?.forkChoice.errors.inc({entrypoint: UpdateHeadOpt.GetCanonicalHead});
throw e;
Expand Down
3 changes: 2 additions & 1 deletion packages/beacon-node/src/chain/interface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,8 @@ export interface IBeaconChain {

getStatus(): Status;

recomputeForkChoiceHead(caller: ForkchoiceCaller): ProtoBlock;
/** @param slot - If provided, overrides fcStore.currentSlot for Gloas FULL vs EMPTY tie-breaker logic */
recomputeForkChoiceHead(caller: ForkchoiceCaller, slot?: Slot): ProtoBlock;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Discussed this offline with @nflaig

would prefer an interface like so:

recomputeForkChoiceHead(
  caller: ForkchoiceCaller,
  opts?: {prepareNextSlot?: boolean}
): ProtoBlock;

// Call site
const headBlock = this.chain.recomputeForkChoiceHead(
  ForkchoiceCaller.prepareNextSlot, {prepareNextSlot: true}
);

Pass optional opts to forkChoice.updateHead and do the const currentSlot = opts?.prepareNextSlot ? store.currentSlot + 1 : store.curentSlot; there

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

just realized we already have ForkchoiceCaller.prepareNextSlot, so the boolean here seems redundant


/** When proposerBoostReorg is enabled, this is called at slot n-1 to predict the head block to build on if we are proposing at slot n */
predictProposerHead(slot: Slot): ProtoBlock;
Expand Down
26 changes: 19 additions & 7 deletions packages/beacon-node/src/chain/prepareNextSlot.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,22 @@ import {IBeaconChain} from "./interface.js";
import {getPayloadAttributesForSSE, prepareExecutionPayload} from "./produceBlock/produceBlockBody.js";
import {RegenCaller} from "./regen/index.js";

// TODO GLOAS: re-evaluate this timing
/* With 12s slot times, this scheduler will run 4s before the start of each slot (`12 - 0.6667 * 12 = 4`). */
export const PREPARE_NEXT_SLOT_BPS = 6667;
/* Pre-Gloas: prepare at 6667 BPS (~8s into a 12s slot), leaving 4s for preparation. */
const PREPARE_NEXT_SLOT_BPS = 6667;

/**
* Gloas+: prepare at 8333 BPS (~10s into a 12s slot), leaving 2s for preparation.
* This is after PAYLOAD_ATTESTATION_DUE_BPS (7500 / 9s) so that fork choice
* incorporates PTC votes (payload PRESENT/ABSENT) before computing the head
* and preparing the execution payload for the next slot.
* TODO GLOAS: re-check before Gloas mainnet
*/
const PREPARE_NEXT_SLOT_BPS_GLOAS = 8333;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

after my spec changes we can keep previous PREPARE_NEXT_SLOT_BPS but the fcu() call needs to be delayed or we need to reach ptc threshold

something like this could work

await Promise.race([
      sleep(SOME_BPS_VALUE - this.clock.msFromSlot(slot), signal),
      this.emitter.waitForPtcThreshold(slot),
    ]);


export function getPrepareNextSlotMs(config: ChainForkConfig, slot: Slot): number {
const bps = ForkSeq[config.getForkName(slot)] >= ForkSeq.gloas ? PREPARE_NEXT_SLOT_BPS_GLOAS : PREPARE_NEXT_SLOT_BPS;
return config.getSlotComponentDurationMs(bps);
}

/* We don't want to do more epoch transition than this */
const PREPARE_EPOCH_LIMIT = 1;
Expand Down Expand Up @@ -73,12 +86,11 @@ export class PrepareNextSlotScheduler {
}

try {
// At PREPARE_NEXT_SLOT_BPS (~67%) of the current slot we prepare payload for the next slot
// or precompute epoch transition
await sleep(this.config.getSlotComponentDurationMs(PREPARE_NEXT_SLOT_BPS), this.signal);
// Wait until the appropriate point in the slot to prepare payload or precompute epoch transition
await sleep(getPrepareNextSlotMs(this.config, prepareSlot), this.signal);

// calling updateHead() here before we produce a block to reduce reorg possibility
const headBlock = this.chain.recomputeForkChoiceHead(ForkchoiceCaller.prepareNextSlot);
const headBlock = this.chain.recomputeForkChoiceHead(ForkchoiceCaller.prepareNextSlot, prepareSlot);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Avoid caching future-slot head during prepareNextSlot

Passing prepareSlot into recomputeForkChoiceHead makes fork choice recompute and cache this.head using next-slot semantics while the node is still in the current slot. After this runs (~67% into slot N), any code that reads the cached head via forkChoice.getHead() before the next slot tick (for example late validator/API flows) can observe a head selected with slot N+1 FULL-vs-EMPTY tie-breaker instead of slot N rules, which can produce premature head-dependent outputs.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

after 67% of the slot n I don't see any flows want to use head
if there is in the future, it should compute head again and cache it, especially when there are more ptc messages received
so caching here is not an issue

we can make an ephemeral head in this case, but it will make the current design more complex
the above mechanism should work

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

doesn't this cause the head to be recomputed so any caller to forkChoice.getHead() in slot N would get the head of the next slot N+1?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

doesn't this cause the head to be recomputed so any caller to forkChoice.getHead() in slot N would get the head of the next slot N+1?

yes. From getting both EMPTY + FULL till the next slot, we need to fairly choose one of them, recompute head and cache. My interpretation would be: when preparing for the next slot, if we have both EMPTY and FULL variant, we need to get through tie-breaker logic to get the winner

const {slot: headSlot, blockRoot: headRoot} = headBlock;

// PS: previously this was comparing slots, but that gave no leway on the skipped
Expand Down
14 changes: 10 additions & 4 deletions packages/fork-choice/src/forkChoice/forkChoice.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,8 @@ export enum UpdateHeadOpt {
}

export type UpdateAndGetHeadOpt =
| {mode: UpdateHeadOpt.GetCanonicalHead}
// When slot is provided, it overrides fcStore.currentSlot for Gloas FULL vs EMPTY tie-breaker logic
| {mode: UpdateHeadOpt.GetCanonicalHead; slot?: Slot}
| {mode: UpdateHeadOpt.GetProposerHead; secFromSlot: number; slot: Slot}
| {mode: UpdateHeadOpt.GetPredictedProposerHead; secFromSlot: number; slot: Slot};

Expand Down Expand Up @@ -222,7 +223,8 @@ export class ForkChoice implements IForkChoice {
} {
const {mode} = opt;

const canonicalHeadBlock = mode === UpdateHeadOpt.GetPredictedProposerHead ? this.getHead() : this.updateHead();
const canonicalHeadBlock =
mode === UpdateHeadOpt.GetPredictedProposerHead ? this.getHead() : this.updateHead(opt.slot);
switch (mode) {
case UpdateHeadOpt.GetPredictedProposerHead:
return {head: this.predictProposerHead(canonicalHeadBlock, opt.secFromSlot, opt.slot)};
Expand Down Expand Up @@ -458,8 +460,10 @@ export class ForkChoice implements IForkChoice {
* Is equivalent to:
*
* https://github.com/ethereum/consensus-specs/blob/v1.1.10/specs/phase0/fork-choice.md#get_head
*
* @param slot - If provided, overrides fcStore.currentSlot for Gloas FULL vs EMPTY tie-breaker logic
*/
updateHead(): ProtoBlock {
updateHead(slot?: Slot): ProtoBlock {
// balances is not changed but votes are changed

// NOTE: In current Lodestar metrics, 100% of forkChoiceRequests this.synced = false.
Expand Down Expand Up @@ -516,7 +520,9 @@ export class ForkChoice implements IForkChoice {
this.justifiedProposerBoostScore = proposerBoostScore;
}

const currentSlot = this.fcStore.currentSlot;
// When preparing for the next slot, pass slot as currentSlot + 1 to choose FULL vs EMPTY
// This is important for Gloas tie-breaker logic
const currentSlot = slot ?? this.fcStore.currentSlot;
Comment thread
twoeths marked this conversation as resolved.
this.protoArray.applyScoreChanges({
deltas,
proposerBoost,
Expand Down
Loading