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
8 changes: 6 additions & 2 deletions packages/beacon-node/src/chain/forkChoice/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,10 +105,14 @@ export function initializeForkChoiceFromFinalizedState(
const isForkPostGloas = computeEpochAtSlot(state.slot) >= config.GLOAS_FORK_EPOCH;

// Determine justified checkpoint payload status
const justifiedPayloadStatus = getCheckpointPayloadStatus(config, state, justifiedCheckpoint.epoch);
const justifiedPayloadStatus = isForkPostGloas
? PayloadStatus.PENDING
: getCheckpointPayloadStatus(config, state, justifiedCheckpoint.epoch);

// Determine finalized checkpoint payload status
const finalizedPayloadStatus = getCheckpointPayloadStatus(config, state, finalizedCheckpoint.epoch);
const finalizedPayloadStatus = isForkPostGloas
? PayloadStatus.PENDING
: getCheckpointPayloadStatus(config, state, finalizedCheckpoint.epoch);
Comment on lines +108 to +115
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.

medium

There's some code duplication here for determining justifiedPayloadStatus and finalizedPayloadStatus. You can introduce a helper function to centralize the logic, which improves maintainability by avoiding repetition of the isForkPostGloas check.

  const getPayloadStatus = (epoch: number): PayloadStatus => {
    if (isForkPostGloas) {
      return PayloadStatus.PENDING;
    }
    return getCheckpointPayloadStatus(config, state, epoch);
  };

  // Determine justified checkpoint payload status
  const justifiedPayloadStatus = getPayloadStatus(justifiedCheckpoint.epoch);

  // Determine finalized checkpoint payload status
  const finalizedPayloadStatus = getPayloadStatus(finalizedCheckpoint.epoch);


return new forkchoiceConstructor(
config,
Expand Down
110 changes: 103 additions & 7 deletions packages/beacon-node/test/spec/presets/fork_choice.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,13 @@ import {expect} from "vitest";
import {toHexString} from "@chainsafe/ssz";
import {createBeaconConfig} from "@lodestar/config";
import {getConfig} from "@lodestar/config/test-utils";
import {CheckpointWithHex, ForkChoice} from "@lodestar/fork-choice";
import {CheckpointWithHex, ExecutionStatus, ForkChoice} from "@lodestar/fork-choice";
import {testLogger} from "@lodestar/logger/test-utils";
import {
ACTIVE_PRESET,
ForkPostDeneb,
ForkPostFulu,
ForkPostGloas,
ForkPreDeneb,
ForkPreFulu,
ForkPreGloas,
Expand All @@ -22,6 +23,7 @@ import {
createCachedBeaconState,
createPubkeyCache,
isExecutionStateType,
isGloasStateType,
signedBlockToSignedHeader,
syncPubkeys,
} from "@lodestar/state-transition";
Expand All @@ -33,13 +35,15 @@ import {
SignedBeaconBlock,
deneb,
fulu,
gloas,
ssz,
sszTypesFor,
} from "@lodestar/types";
import {bnToNum, fromHex, toHex} from "@lodestar/utils";
import {
BlockInputBlobs,
BlockInputColumns,
BlockInputNoData,
BlockInputPreData,
BlockInputSource,
} from "../../../src/chain/blocks/blockInput/index.js";
Expand All @@ -65,6 +69,7 @@ const ANCHOR_BLOCK_FILE_NAME = "anchor_block";
const BLOCK_FILE_NAME = "^(block)_([0-9a-zA-Z]+)$";
const BLOBS_FILE_NAME = "^(blobs)_([0-9a-zA-Z]+)$";
const COLUMN_FILE_NAME = "^(column)_([0-9a-zA-Z]+)$";
const EXECUTION_PAYLOAD_ENVELOPE_FILE_NAME = "^(execution_payload_envelope)_([0-9a-zA-Z]+)$";
const ATTESTATION_FILE_NAME = "^(attestation)_([0-9a-zA-Z])+$";
const ATTESTER_SLASHING_FILE_NAME = "^(attester_slashing)_([0-9a-zA-Z])+$";

Expand All @@ -85,9 +90,11 @@ const forkChoiceTest =
const clock = new ClockStopped(currentSlot);
const executionEngineBackend = new ExecutionEngineMockBackend({
onlyPredefinedResponses: opts.onlyPredefinedResponses,
genesisBlockHash: isExecutionStateType(anchorState)
? toHexString(anchorState.latestExecutionPayloadHeader.blockHash)
: ZERO_HASH_HEX,
genesisBlockHash: isGloasStateType(anchorState)
? toHexString(anchorState.latestBlockHash)
: isExecutionStateType(anchorState)
? toHexString(anchorState.latestExecutionPayloadHeader.blockHash)
: ZERO_HASH_HEX,
});

const controller = new AbortController();
Expand Down Expand Up @@ -235,7 +242,19 @@ const forkChoiceTest =
let blockImport;
const forkSeq = config.getForkSeq(slot);

if (forkSeq >= ForkSeq.fulu) {
if (forkSeq >= ForkSeq.gloas) {
// Gloas (ePBS) blocks don't carry blobs/columns directly on the block body.
// Blob KZG commitments are nested inside signedExecutionPayloadBid.
// Use BlockInputNoData since DA is handled separately via execution payload envelopes.
blockImport = BlockInputNoData.createFromBlock({
forkName: fork,
block: signedBlock as SignedBeaconBlock<ForkPostGloas>,
blockRootHex,
source: BlockInputSource.gossip,
seenTimestampSec: 0,
daOutOfRange: false,
});
} else if (forkSeq >= ForkSeq.fulu) {
if (columns === undefined) {
columns = [];
}
Expand Down Expand Up @@ -348,6 +367,42 @@ const forkChoiceTest =
}
}

// execution_payload step for Gloas (ePBS) tests
else if (isExecutionPayload(step)) {
const isValid = Boolean(step.valid ?? true);
logger.debug(`Step ${i}/${stepsLen} execution_payload`, {
envelope: step.execution_payload,
valid: isValid,
});
const envelope = testcase.executionPayloadEnvelopes.get(step.execution_payload);
if (!envelope) throw Error(`No execution payload envelope ${step.execution_payload}`);

try {
const beaconBlockRoot = toHex(envelope.message.beaconBlockRoot);
const blockHash = toHex(envelope.message.payload.blockHash);
const blockNumber = envelope.message.payload.blockNumber;
const stateRoot = toHex(envelope.message.stateRoot);

// Add predefined VALID status for the payload's block hash so the EL mock accepts it
executionEngineBackend.addPredefinedPayloadStatus(blockHash, {
status: ExecutionPayloadStatus.VALID,
latestValidHash: null,
validationError: null,
});

(chain.forkChoice as ForkChoice).onExecutionPayload(
beaconBlockRoot,
blockHash,
blockNumber,
stateRoot,
ExecutionStatus.Valid
);
if (!isValid) throw Error("Expect error since this is a negative test");
} catch (e) {
if (isValid || (e as Error).message === "Expect error since this is a negative test") throw e;
}
}

// Optional step for optimistic sync tests.
else if (isOnPayloadInfoStep(step)) {
logger.debug(`Step ${i}/${stepsLen} payload_status`, {blockHash: step.block_hash});
Expand Down Expand Up @@ -415,6 +470,16 @@ const forkChoiceTest =
`Invalid proposer head at step ${i}`
);
}
if (step.checks.head_payload_status !== undefined) {
// Map our PayloadStatus enum to spec's numbering:
// Spec: EMPTY=0, FULL=1, PENDING=2
// Ours: PENDING=0, EMPTY=1, FULL=2
const payloadStatusToSpec: Record<number, number> = {0: 2, 1: 0, 2: 1};
expect(payloadStatusToSpec[head.payloadStatus]).toEqualWithMessage(
bnToNum(step.checks.head_payload_status),
`Invalid head payload status at step ${i}`
);
}
if (step.checks.should_override_forkchoice_update) {
const currentSlot = Math.floor(tickTime / (config.SLOT_DURATION_MS / 1000));
const result = chain.forkChoice.shouldOverrideForkChoiceUpdate(
Expand Down Expand Up @@ -453,6 +518,7 @@ const forkChoiceTest =
[BLOCK_FILE_NAME]: ssz[fork].SignedBeaconBlock,
[BLOBS_FILE_NAME]: ssz.deneb.Blobs,
[COLUMN_FILE_NAME]: ssz.fulu.DataColumnSidecar,
[EXECUTION_PAYLOAD_ENVELOPE_FILE_NAME]: ssz.gloas.SignedExecutionPayloadEnvelope,
[ATTESTATION_FILE_NAME]: sszTypesFor(fork).Attestation,
[ATTESTER_SLASHING_FILE_NAME]: sszTypesFor(fork).AttesterSlashing,
},
Expand All @@ -461,6 +527,7 @@ const forkChoiceTest =
const blocks = new Map<string, SignedBeaconBlock>();
const blobs = new Map<string, deneb.Blobs>();
const columns = new Map<string, fulu.DataColumnSidecar>();
const executionPayloadEnvelopes = new Map<string, gloas.SignedExecutionPayloadEnvelope>();
const attestations = new Map<string, Attestation>();
const attesterSlashings = new Map<string, AttesterSlashing>();
for (const key in t) {
Expand All @@ -478,6 +545,10 @@ const forkChoiceTest =
if (columnMatch) {
columns.set(key, t[key]);
}
const envelopeMatch = key.match(EXECUTION_PAYLOAD_ENVELOPE_FILE_NAME);
if (envelopeMatch) {
executionPayloadEnvelopes.set(key, t[key]);
}
const attMatch = key.match(ATTESTATION_FILE_NAME);
if (attMatch) {
attestations.set(key, t[key]);
Expand All @@ -495,6 +566,7 @@ const forkChoiceTest =
blocks,
blobs,
columns,
executionPayloadEnvelopes,
attestations,
attesterSlashings,
};
Expand All @@ -515,7 +587,18 @@ const forkChoiceTest =
// and these tests are failing until we update our implementation.
name.includes("voting_source_beyond_two_epoch") ||
name.includes("justified_update_always_if_better") ||
name.includes("justified_update_not_realized_finality"),
name.includes("justified_update_not_realized_finality") ||
// TODO GLOAS: Requires should_apply_proposer_boost (gloas/fork-choice.md#new-should_apply_proposer_boost)
// which conditionally suppresses proposer boost when the parent is weak and from the previous slot.
// Pre-Gloas forks always apply boost; Gloas adds is_head_weak + equivocation checks.
(name.includes("gloas") &&
name.includes("include_votes_another_empty_chain_with_enough_ffg_votes_previous_epoch")) ||
// TODO GLOAS: These two tests are affected by the wrong proposer boost cutoff time from the
// consensus-specs and thus have wrong expectation of proposer boost. Our implementation
// should pass these two tests after https://github.com/ethereum/consensus-specs/pull/5095
// is included the spec release.
(name.includes("gloas/fork_choice/on_block") &&
(name.endsWith("proposer_boost") || name.endsWith("proposer_boost_is_first_block"))),
},
};
};
Expand All @@ -527,7 +610,7 @@ function toSpecTestCheckpoint(checkpoint: CheckpointWithHex): SpecTestCheckpoint
};
}

type Step = OnTick | OnAttestation | OnAttesterSlashing | OnBlock | OnPayloadInfo | Checks;
type Step = OnTick | OnAttestation | OnAttesterSlashing | OnBlock | OnExecutionPayloadEnvelope | OnPayloadInfo | Checks;

type SpecTestCheckpoint = {epoch: bigint; root: string};

Expand Down Expand Up @@ -568,6 +651,13 @@ type OnBlock = {
valid?: number;
};

type OnExecutionPayloadEnvelope = {
/** the name of the execution_payload_envelope file */
execution_payload: string;
/** optional, default to `true`. */
valid?: number;
};

type OnPayloadInfo = {
/** Encoded 32-byte value of payload's block hash. */
block_hash: string;
Expand All @@ -591,6 +681,7 @@ type Checks = {
justified_checkpoint?: SpecTestCheckpoint;
finalized_checkpoint?: SpecTestCheckpoint;
proposer_boost_root?: RootHex;
head_payload_status?: bigint;
get_proposer_head?: string;
should_override_forkchoice_update?: {
validator_is_connected: boolean;
Expand All @@ -610,6 +701,7 @@ type ForkChoiceTestCase = {
blocks: Map<string, SignedBeaconBlock>;
blobs: Map<string, deneb.Blobs>;
columns: Map<string, fulu.DataColumnSidecar>;
executionPayloadEnvelopes: Map<string, gloas.SignedExecutionPayloadEnvelope>;
attestations: Map<string, Attestation>;
attesterSlashings: Map<string, AttesterSlashing>;
};
Expand All @@ -630,6 +722,10 @@ function isBlock(step: Step): step is OnBlock {
return typeof (step as OnBlock).block === "string";
}

function isExecutionPayload(step: Step): step is OnExecutionPayloadEnvelope {
return typeof (step as OnExecutionPayloadEnvelope).execution_payload === "string";
}

function isOnPayloadInfoStep(step: Step): step is OnPayloadInfo {
return typeof (step as OnPayloadInfo).block_hash === "string";
}
Expand Down
1 change: 0 additions & 1 deletion packages/beacon-node/test/spec/utils/specTestIterator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@ export const defaultSkipOpts: SkipOpts = {
/^electra\/light_client\/single_merkle_proof\/BeaconBlockBody.*/,
/^fulu\/light_client\/single_merkle_proof\/BeaconBlockBody.*/,
/^.+\/light_client\/data_collection\/.*/,
/^gloas\/fork_choice\/.*$/,
/^gloas\/ssz_static\/ForkChoiceNode.*$/,
],
skippedTests: [
Expand Down
5 changes: 1 addition & 4 deletions packages/fork-choice/src/forkChoice/forkChoice.ts
Original file line number Diff line number Diff line change
Expand Up @@ -641,10 +641,7 @@ export class ForkChoice implements IForkChoice {
// Check block is a descendant of the finalized block at the checkpoint finalized slot.
const blockAncestorNode = this.getAncestor(parentRootHex, finalizedSlot);
const fcStoreFinalized = this.fcStore.finalizedCheckpoint;
if (
blockAncestorNode.blockRoot !== fcStoreFinalized.rootHex ||
blockAncestorNode.payloadStatus !== fcStoreFinalized.payloadStatus
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.

We remove the payload status comparison because we don't know the payload status of the finalized checkpoint (both before and after deferring payload processing).

We should just compare the root only.

) {
if (blockAncestorNode.blockRoot !== fcStoreFinalized.rootHex) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Restore payload-status check for finalized descendants

The finalized-descendant gate in onBlock() now only compares blockRoot and ignores payloadStatus. In post-Gloas fork choice, the same beacon root can have distinct EMPTY/FULL variants, so this change allows a block that extends the wrong finalized variant (e.g., EMPTY branch when store finalized checkpoint is FULL) to pass NOT_FINALIZED_DESCENDANT validation. That weakens finality safety by admitting blocks from a non-finalized payload branch whenever roots match.

Useful? React with 👍 / 👎.

throw new ForkChoiceError({
code: ForkChoiceErrorCode.INVALID_BLOCK,
err: {
Expand Down
16 changes: 16 additions & 0 deletions packages/fork-choice/src/protoArray/protoArray.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,22 @@ export class ProtoArray {
// Spec: https://github.com/ethereum/consensus-specs/blob/v1.7.0-alpha.4/specs/gloas/fork-choice.md#modified-get_forkchoice_store
if (protoArray.ptcVotes.has(block.blockRoot)) {
protoArray.ptcVotes.set(block.blockRoot, BitArray.fromBoolArray(Array.from({length: PTC_SIZE}, () => true)));

// Anchor block must have FULL variant per spec get_forkchoice_store:
// payload_states = {anchor_root: anchor_state.copy()}
// This means the anchor's "payload" is considered received (the anchor state IS the post-payload state).
// Without FULL, blocks extending FULL from the anchor would be orphaned.
if (block.executionPayloadBlockHash !== null) {
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.

this is likely a bug in the spec

Image

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.

we can merge for the sake of passing spec tests
but need to leave a TODO GLOAS to remove later

protoArray.onExecutionPayload(
block.blockRoot,
currentSlot,
block.executionPayloadBlockHash,
(block as {executionPayloadNumber?: number}).executionPayloadNumber ?? 0,
block.stateRoot,
null,
ExecutionStatus.Valid
);
}
}

return protoArray;
Expand Down
12 changes: 10 additions & 2 deletions packages/state-transition/src/stateView/beaconStateView.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,12 @@ import {getBlockRootAtSlot} from "../util/blockRoot.js";
import {computeAnchorCheckpoint} from "../util/computeAnchorCheckpoint.js";
import {computeEpochAtSlot, computeStartSlotAtEpoch} from "../util/epoch.js";
import {EpochShuffling} from "../util/epochShuffling.js";
import {isExecutionEnabled, isExecutionStateType, isMergeTransitionComplete} from "../util/execution.js";
import {
isExecutionEnabled,
isExecutionStateType,
isGloasStateType,
isMergeTransitionComplete,
} from "../util/execution.js";
import {canBuilderCoverBid} from "../util/gloas.js";
import {loadState} from "../util/loadState/loadState.js";
import {getRandaoMix} from "../util/seed.js";
Expand Down Expand Up @@ -589,7 +594,10 @@ export class BeaconStateView implements IBeaconStateViewLatestFork {
}

get isMergeTransitionComplete(): boolean {
return isExecutionStateType(this.cachedState) && isMergeTransitionComplete(this.cachedState);
return (
(isExecutionStateType(this.cachedState) || isGloasStateType(this.cachedState)) &&
isMergeTransitionComplete(this.cachedState)
);
}

// Block production
Expand Down
12 changes: 11 additions & 1 deletion packages/state-transition/src/util/execution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
BeaconStateBellatrix,
BeaconStateCapella,
BeaconStateExecutions,
BeaconStateGloas,
CachedBeaconStateAllForks,
CachedBeaconStateExecutions,
} from "../types.js";
Expand Down Expand Up @@ -46,7 +47,11 @@ export function isExecutionEnabled(state: BeaconStateExecutions, block: BeaconBl
* Merge is complete when the state includes execution layer data:
* state.latestExecutionPayloadHeader NOT EMPTY or state is post-capella
*/
export function isMergeTransitionComplete(state: BeaconStateExecutions): boolean {
export function isMergeTransitionComplete(state: BeaconStateExecutions | BeaconStateGloas): boolean {
if (isGloasStateType(state)) {
return true;
}

if (isCapellaStateType(state)) {
// All networks have completed the merge transition before capella
return true;
Expand All @@ -71,6 +76,11 @@ export function isCapellaStateType(state: BeaconStateAllForks): state is BeaconS
);
}

/** Type guard for gloas.BeaconState */
export function isGloasStateType(state: BeaconStateAllForks): state is BeaconStateGloas {
return (state as BeaconStateGloas).latestBlockHash !== undefined;
}

/** Type guard for bellatrix.CachedBeaconState */
export function isExecutionCachedStateType(state: CachedBeaconStateAllForks): state is CachedBeaconStateExecutions {
return (state as CachedBeaconStateExecutions).latestExecutionPayloadHeader !== undefined;
Expand Down
Loading