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
12 changes: 11 additions & 1 deletion packages/beacon-node/src/chain/blocks/importBlock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,13 +116,23 @@ export async function importBlock(
}
executionStatus = parentBlock.executionStatus;
}

let expectedProposerIndex: number | null = null;
const headState = this.getHeadState();
if (headState.epoch === blockEpoch) {
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 also have BeaconStateView.getBeaconProposer() but it only query from current epoch
looks like a good chance to clean that up
ideally we should be able to have this logic once

expectedProposerIndex = headState.currentProposers[blockSlot % SLOTS_PER_EPOCH];
} else if (headState.epoch + 1 === blockEpoch) {
expectedProposerIndex = headState.nextProposers[blockSlot % SLOTS_PER_EPOCH];
}
Comment on lines +120 to +126
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

The logic for determining the expected proposer index from the head state is duplicated in packages/beacon-node/test/spec/utils/gossipValidation.ts. Consider encapsulating this logic into a shared helper function or using the existing state.getBeaconProposer(slot) method with appropriate error handling. This would improve maintainability and ensure consistency across the codebase when resolving proposers from a given state.


const blockSummary = this.forkChoice.onBlock(
block.message,
postState,
blockDelaySec,
currentSlot,
executionStatus,
dataAvailabilityStatus
dataAvailabilityStatus,
expectedProposerIndex
);

// This adds the state necessary to process the next block
Expand Down
5 changes: 0 additions & 5 deletions packages/beacon-node/test/spec/presets/fork_choice.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -622,11 +622,6 @@ const forkChoiceTest =
// integrated
shouldSkip: (_testcase, name, _index) =>
name.includes("invalid_incorrect_proof") ||
// TODO GLOAS: Proposer boost specs have been changed retroactively in v1.7.0-alpha.1,
// 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") ||
// TODO GLOAS: These tests will be unskipped by https://github.com/ChainSafe/lodestar/pull/9233
(name.includes("gloas") &&
(name.includes("simple_attempted_reorg_without_enough_ffg_votes") ||
Expand Down
19 changes: 16 additions & 3 deletions packages/beacon-node/test/spec/utils/gossipValidation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {chainConfigFromJson, chainConfigTypes, createBeaconConfig} from "@lodest
import {getConfig} from "@lodestar/config/test-utils";
import {ExecutionStatus} from "@lodestar/fork-choice";
import {testLogger} from "@lodestar/logger/test-utils";
import {ForkName} from "@lodestar/params";
import {ForkName, SLOTS_PER_EPOCH} from "@lodestar/params";
import {
BeaconStateAllForks,
BeaconStateView,
Expand Down Expand Up @@ -486,7 +486,8 @@ export async function runGossipValidationTest(
0,
slot,
ExecutionStatus.Valid,
getDataAvailabilityStatusForFork(fork)
getDataAvailabilityStatusForFork(fork),
getHeadProposerIndex(chain.getHeadState(), slot)
);
blockStatesByRoot.set(blockRootHex, postState);
continue;
Expand All @@ -501,7 +502,8 @@ export async function runGossipValidationTest(
0,
slot,
ExecutionStatus.Syncing,
getDataAvailabilityStatusForFork(fork)
getDataAvailabilityStatusForFork(fork),
getHeadProposerIndex(chain.getHeadState(), slot)
);
blockStatesByRoot.set(blockRootHex, postState);
invalidateImportedBlock(chain, blockRootHex, parentRootHex);
Expand Down Expand Up @@ -727,6 +729,17 @@ async function validateMessageForTopic(
}
}

function getHeadProposerIndex(headState: IBeaconStateView, slot: number): number | null {
const slotEpoch = computeEpochAtSlot(slot);
if (headState.epoch === slotEpoch) {
return headState.currentProposers[slot % SLOTS_PER_EPOCH];
}
if (headState.epoch + 1 === slotEpoch) {
return headState.nextProposers[slot % SLOTS_PER_EPOCH];
}
return null;
}

function rejectOnInvalidSerializedBytes<T>(fn: () => T): T {
try {
return fn();
Expand Down
106 changes: 87 additions & 19 deletions packages/beacon-node/test/unit/chain/forkChoice/forkChoice.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,15 +106,17 @@ describe("LodestarForkChoice", () => {
blockDelaySec,
currentSlot,
executionStatus,
dataAvailabilityStatus
dataAvailabilityStatus,
targetBlock.message.proposerIndex
);
forkChoice.onBlock(
orphanedBlock.message,
orphanedState,
blockDelaySec,
currentSlot,
executionStatus,
dataAvailabilityStatus
dataAvailabilityStatus,
orphanedBlock.message.proposerIndex
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.

seems misleading because this requires an "expected proposer index" in terms of head view, may pass null instead?
same to others

);
let head = forkChoice.getHead();
expect(head.slot).toBe(orphanedBlock.message.slot);
Expand All @@ -124,7 +126,8 @@ describe("LodestarForkChoice", () => {
blockDelaySec,
currentSlot,
executionStatus,
dataAvailabilityStatus
dataAvailabilityStatus,
parentBlock.message.proposerIndex
);
// tie break condition causes head to be orphaned block (based on hex root comparison)
head = forkChoice.getHead();
Expand All @@ -135,7 +138,8 @@ describe("LodestarForkChoice", () => {
blockDelaySec,
currentSlot,
executionStatus,
dataAvailabilityStatus
dataAvailabilityStatus,
childBlock.message.proposerIndex
);
head = forkChoice.getHead();
// without vote, head gets stuck at orphaned block
Expand Down Expand Up @@ -195,19 +199,75 @@ describe("LodestarForkChoice", () => {
const currentSlot = 128;
forkChoice.updateTime(currentSlot);

forkChoice.onBlock(block08.message, state08, blockDelaySec, currentSlot, executionStatus, dataAvailabilityStatus);
forkChoice.onBlock(block12.message, state12, blockDelaySec, currentSlot, executionStatus, dataAvailabilityStatus);
forkChoice.onBlock(block16.message, state16, blockDelaySec, currentSlot, executionStatus, dataAvailabilityStatus);
forkChoice.onBlock(block20.message, state20, blockDelaySec, currentSlot, executionStatus, dataAvailabilityStatus);
forkChoice.onBlock(block24.message, state24, blockDelaySec, currentSlot, executionStatus, dataAvailabilityStatus);
forkChoice.onBlock(block28.message, state28, blockDelaySec, currentSlot, executionStatus, dataAvailabilityStatus);
forkChoice.onBlock(
block08.message,
state08,
blockDelaySec,
currentSlot,
executionStatus,
dataAvailabilityStatus,
block08.message.proposerIndex
);
forkChoice.onBlock(
block12.message,
state12,
blockDelaySec,
currentSlot,
executionStatus,
dataAvailabilityStatus,
block12.message.proposerIndex
);
forkChoice.onBlock(
block16.message,
state16,
blockDelaySec,
currentSlot,
executionStatus,
dataAvailabilityStatus,
block16.message.proposerIndex
);
forkChoice.onBlock(
block20.message,
state20,
blockDelaySec,
currentSlot,
executionStatus,
dataAvailabilityStatus,
block20.message.proposerIndex
);
forkChoice.onBlock(
block24.message,
state24,
blockDelaySec,
currentSlot,
executionStatus,
dataAvailabilityStatus,
block24.message.proposerIndex
);
forkChoice.onBlock(
block28.message,
state28,
blockDelaySec,
currentSlot,
executionStatus,
dataAvailabilityStatus,
block28.message.proposerIndex
);
expect(forkChoice.getAllAncestorBlocks(hashBlock(block16.message), PayloadStatus.FULL)).toHaveLength(4);
expect(forkChoice.getAllAncestorBlocks(hashBlock(block24.message), PayloadStatus.FULL)).toHaveLength(6);
expect(forkChoice.getBlockHexDefaultStatus(hashBlock(block08.message))).not.toBeNull();
expect(forkChoice.getBlockHexDefaultStatus(hashBlock(block12.message))).not.toBeNull();
expect(forkChoice.hasBlockHex(hashBlock(block08.message))).toBe(true);
expect(forkChoice.hasBlockHex(hashBlock(block12.message))).toBe(true);
forkChoice.onBlock(block32.message, state32, blockDelaySec, currentSlot, executionStatus, dataAvailabilityStatus);
forkChoice.onBlock(
block32.message,
state32,
blockDelaySec,
currentSlot,
executionStatus,
dataAvailabilityStatus,
block32.message.proposerIndex
);
forkChoice.prune(hashBlock(block16.message));
expect(forkChoice.getAllAncestorBlocks(hashBlock(block16.message), PayloadStatus.FULL).length).toBeWithMessage(
1,
Expand Down Expand Up @@ -243,31 +303,35 @@ describe("LodestarForkChoice", () => {
blockDelaySec,
currentSlot,
executionStatus,
dataAvailabilityStatus
dataAvailabilityStatus,
targetBlock.message.proposerIndex
);
forkChoice.onBlock(
orphanedBlock.message,
orphanedState,
blockDelaySec,
currentSlot,
executionStatus,
dataAvailabilityStatus
dataAvailabilityStatus,
orphanedBlock.message.proposerIndex
);
forkChoice.onBlock(
parentBlock.message,
parentState,
blockDelaySec,
currentSlot,
executionStatus,
dataAvailabilityStatus
dataAvailabilityStatus,
parentBlock.message.proposerIndex
);
forkChoice.onBlock(
childBlock.message,
childState,
blockDelaySec,
currentSlot,
executionStatus,
dataAvailabilityStatus
dataAvailabilityStatus,
childBlock.message.proposerIndex
);
const childBlockRoot = toHexString(ssz.phase0.BeaconBlock.hashTreeRoot(childBlock.message));
// the old way to get non canonical blocks
Expand Down Expand Up @@ -314,7 +378,8 @@ describe("LodestarForkChoice", () => {
blockDelaySec,
blockW.message.slot,
executionStatus,
dataAvailabilityStatus
dataAvailabilityStatus,
blockW.message.proposerIndex
);

// X
Expand All @@ -326,7 +391,8 @@ describe("LodestarForkChoice", () => {
blockDelaySec,
blockX.message.slot,
executionStatus,
dataAvailabilityStatus
dataAvailabilityStatus,
blockX.message.proposerIndex
);

// Y, same epoch to X
Expand All @@ -338,7 +404,8 @@ describe("LodestarForkChoice", () => {
blockDelaySec,
blockY.message.slot,
executionStatus,
dataAvailabilityStatus
dataAvailabilityStatus,
blockY.message.proposerIndex
);

// Y and Z are candidates for new head, make more attestations on Y
Expand Down Expand Up @@ -379,7 +446,8 @@ describe("LodestarForkChoice", () => {
blockDelaySec,
blockZ.message.slot,
executionStatus,
dataAvailabilityStatus
dataAvailabilityStatus,
blockZ.message.proposerIndex
);

const head = forkChoice.updateHead();
Expand Down
10 changes: 8 additions & 2 deletions packages/fork-choice/src/forkChoice/forkChoice.ts
Original file line number Diff line number Diff line change
Expand Up @@ -596,7 +596,11 @@ export class ForkChoice implements IForkChoice {
blockDelaySec: number,
currentSlot: Slot,
executionStatus: BlockExecutionStatus,
dataAvailabilityStatus: DataAvailabilityStatus
dataAvailabilityStatus: DataAvailabilityStatus,
// The expected proposer index on the canonical chain we are following.
// Calculated by our head state. We use it as part of the proposer
// boost decision making. No boost will be set if this is null.
expectedProposerIndex: ValidatorIndex | null
): ProtoBlock {
const {parentRoot, slot} = block;
const parentRootHex = toRootHex(parentRoot);
Expand Down Expand Up @@ -669,7 +673,9 @@ export class ForkChoice implements IForkChoice {
this.opts?.proposerBoost &&
isTimely &&
// only boost the first block we see
this.proposerBoostRoot === null
this.proposerBoostRoot === null &&
expectedProposerIndex !== null &&
block.proposerIndex === expectedProposerIndex
) {
this.proposerBoostRoot = blockRootHex;
}
Expand Down
14 changes: 12 additions & 2 deletions packages/fork-choice/src/forkChoice/interface.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,14 @@
import {DataAvailabilityStatus, EffectiveBalanceIncrements, IBeaconStateView} from "@lodestar/state-transition";
import {AttesterSlashing, BeaconBlock, Epoch, IndexedAttestation, Root, RootHex, Slot} from "@lodestar/types";
import {
AttesterSlashing,
BeaconBlock,
Epoch,
IndexedAttestation,
Root,
RootHex,
Slot,
ValidatorIndex,
} from "@lodestar/types";
import {
BlockExecutionStatus,
LVHExecResponse,
Expand Down Expand Up @@ -145,7 +154,8 @@ export interface IForkChoice {
blockDelaySec: number,
currentSlot: Slot,
executionStatus: BlockExecutionStatus,
dataAvailabilityStatus: DataAvailabilityStatus
dataAvailabilityStatus: DataAvailabilityStatus,
expectedProposerIndex: ValidatorIndex | null
): ProtoBlock;
/**
* Register `attestation` with the fork choice DAG so that it may influence future calls to `getHead`.
Expand Down
Loading