Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: ignore forkchoice invalidations if latestValidHash not found #6361

Merged
merged 4 commits into from
Jan 27, 2024
Merged

fix: ignore forkchoice invalidations if latestValidHash not found #6361

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
39c1ca9
fix: ignore forkchoice invalidations if latestValidHash not found
g11tech Jan 27, 2024
c3f8120
rename for better understanding
g11tech Jan 27, 2024
2532f6d
update the lvh search start index
g11tech Jan 27, 2024
a30838d
apply feedback
g11tech Jan 27, 2024
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
4 changes: 2 additions & 2 deletions packages/beacon-node/src/chain/blocks/verifyBlocksExecutionPayloads.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ export async function verifyBlockExecutionPayload(
const lvhResponse = {
executionStatus,
latestValidExecHash: execResult.latestValidHash,
invalidateFromBlockHash: toHexString(block.message.parentRoot),
invalidateFromParentBlockHash: toHexString(block.message.parentRoot),
g11tech marked this conversation as resolved.
Show resolved Hide resolved
};
const execError = new BlockError(block, {
code: BlockErrorCode.EXECUTION_ENGINE_ERROR,
Expand Down Expand Up @@ -416,7 +416,7 @@ function getSegmentErrorResponse(
invalidSegmentLVH = {
executionStatus: ExecutionStatus.Invalid,
latestValidExecHash: lvhResponse.latestValidExecHash,
invalidateFromBlockHash: parentBlock.blockRoot,
invalidateFromParentBlockHash: parentBlock.blockRoot,
};
}
}
Expand Down
2 changes: 1 addition & 1 deletion packages/fork-choice/src/protoArray/interface.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ export type LVHValidResponse = {
export type LVHInvalidResponse = {
executionStatus: ExecutionStatus.Invalid;
latestValidExecHash: RootHex | null;
invalidateFromBlockHash: RootHex;
invalidateFromParentBlockHash: RootHex;
};
export type LVHExecResponse = LVHValidResponse | LVHInvalidResponse;

Expand Down
30 changes: 16 additions & 14 deletions packages/fork-choice/src/protoArray/protoArray.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -279,13 +279,13 @@ export class ProtoArray {
// Mark chain ii) as Invalid if LVH is found and non null, else only invalidate invalid_payload
// if its in fcU.
//
const {invalidateFromBlockHash, latestValidExecHash} = execResponse;
const invalidateFromIndex = this.indices.get(invalidateFromBlockHash);
if (invalidateFromIndex === undefined) {
throw Error(`Unable to find invalidateFromBlockHash=${invalidateFromBlockHash} in forkChoice`);
const {invalidateFromParentBlockHash, latestValidExecHash} = execResponse;
const invalidateFromParentIndex = this.indices.get(invalidateFromParentBlockHash);
if (invalidateFromParentIndex === undefined) {
throw Error(`Unable to find invalidateFromParentBlockHash=${invalidateFromParentBlockHash} in forkChoice`);
}
const latestValidHashIndex =
latestValidExecHash !== null ? this.getNodeIndexFromLVH(latestValidExecHash, invalidateFromIndex) : null;
latestValidExecHash !== null ? this.getNodeIndexFromLVH(latestValidExecHash, invalidateFromParentIndex) : null;
if (latestValidHashIndex === null) {
/**
* If the LVH is null or not found, represented with latestValidHashIndex=undefined,
Expand All @@ -299,12 +299,14 @@ export class ProtoArray {
* ii) lazy: that invalidation was result of simple check and the EL just
* responded with a bogus LVH
*
* So we will just invalidate the current payload and let future responses take care
* to be as robust as possible.
* In such case for robustness, lets not process this invalidation into forkchoice
* as it might poision it since the invalidations can't be processed unless latestValidHashIndex
* is known as invalidateFromParentIndex is the parent of the payload being verified which has not
* been imported yet into forkchoice.
g11tech marked this conversation as resolved.
Show resolved Hide resolved
*/
this.invalidateNodeByIndex(invalidateFromIndex);
throw Error(`Unable to find latestValidExecHash=${latestValidExecHash} in the forkchoice`);
} else {
this.propagateInValidExecutionStatusByIndex(invalidateFromIndex, latestValidHashIndex, currentSlot);
this.propagateInValidExecutionStatusByIndex(invalidateFromParentIndex, latestValidHashIndex, currentSlot);
}
}
}
Expand Down Expand Up @@ -333,12 +335,12 @@ export class ProtoArray {
*/

private propagateInValidExecutionStatusByIndex(
invalidateFromIndex: number,
invalidateFromParentIndex: number,
latestValidHashIndex: number,
currentSlot: Slot
): void {
// Pass 1: mark invalidateFromIndex and its parents invalid
let invalidateIndex: number | undefined = invalidateFromIndex;
// Pass 1: mark invalidateFromParentIndex and its parents invalid
let invalidateIndex: number | undefined = invalidateFromParentIndex;
while (invalidateIndex !== undefined && invalidateIndex > latestValidHashIndex) {
const invalidNode = this.invalidateNodeByIndex(invalidateIndex);
invalidateIndex = invalidNode.parent;
Expand Down Expand Up @@ -368,8 +370,8 @@ export class ProtoArray {
});
}

private getNodeIndexFromLVH(latestValidExecHash: RootHex, ancestorOfIndex: number): number | null {
let nodeIndex = this.nodes[ancestorOfIndex].parent;
private getNodeIndexFromLVH(latestValidExecHash: RootHex, ancestorFromIndex: number): number | null {
let nodeIndex: number | undefined = ancestorFromIndex;
while (nodeIndex !== undefined && nodeIndex >= 0) {
const node = this.getNodeFromIndex(nodeIndex);
if (
Expand Down
10 changes: 5 additions & 5 deletions packages/fork-choice/test/unit/protoArray/executionStatusUpdates.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ describe("executionStatus / normal updates", () => {
{
executionStatus: ExecutionStatus.Invalid,
latestValidExecHash: "2C",
invalidateFromBlockHash: "3C",
invalidateFromParentBlockHash: "3C",
},
3
);
Expand Down Expand Up @@ -212,7 +212,7 @@ describe("executionStatus / normal updates", () => {
{
executionStatus: ExecutionStatus.Invalid,
latestValidExecHash: "1A",
invalidateFromBlockHash: "3A",
invalidateFromParentBlockHash: "3A",
},
3
);
Expand Down Expand Up @@ -259,7 +259,7 @@ describe("executionStatus / invalidate all postmerge chain", () => {
{
executionStatus: ExecutionStatus.Invalid,
latestValidExecHash: "0x0000000000000000000000000000000000000000000000000000000000000000",
invalidateFromBlockHash: "3B",
invalidateFromParentBlockHash: "3B",
},
3
);
Expand Down Expand Up @@ -336,7 +336,7 @@ describe("executionStatus / poision forkchoice if we invalidate previous valid",
{
executionStatus: ExecutionStatus.Invalid,
latestValidExecHash: "0x0000000000000000000000000000000000000000000000000000000000000000",
invalidateFromBlockHash: "3A",
invalidateFromParentBlockHash: "3A",
},
3
)
Expand Down Expand Up @@ -373,7 +373,7 @@ describe("executionStatus / poision forkchoice if we validate previous invalid",
{
executionStatus: ExecutionStatus.Invalid,
latestValidExecHash: "0x0000000000000000000000000000000000000000000000000000000000000000",
invalidateFromBlockHash: "3B",
invalidateFromParentBlockHash: "3B",
},
3
);
Expand Down
Loading