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.

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

optimistic sync p1 - attest only validated head #3434

Merged
merged 24 commits into from
Dec 15, 2021
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
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: 4 additions & 1 deletion .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@ name: Tests

on: [pull_request, push]

env:
GOERLI_RPC_DEFAULT_URL: https://goerli.infura.io/v3/84842078b09946638c03157f83405213

jobs:
tests-main:
name: Tests
Expand Down Expand Up @@ -53,6 +56,6 @@ jobs:
- name: E2e tests
run: yarn test:e2e
env:
GOERLI_RPC_URL: ${{ secrets.GOERLI_RPC_URL }}
GOERLI_RPC_URL: ${{ secrets.GOERLI_RPC_URL!=0 && secrets.GOERLI_RPC_URL || env.GOERLI_RPC_DEFAULT_URL }}
- name: README check
run: yarn run check-readme
53 changes: 46 additions & 7 deletions packages/fork-choice/src/forkChoice/forkChoice.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {
import {IChainConfig, IChainForkConfig} from "@chainsafe/lodestar-config";

import {computeDeltas} from "../protoArray/computeDeltas";
import {HEX_ZERO_HASH, IVoteTracker, IProtoBlock} from "../protoArray/interface";
import {HEX_ZERO_HASH, IVoteTracker, IProtoBlock, ExecutionStatus} from "../protoArray/interface";
import {ProtoArray} from "../protoArray/protoArray";

import {IForkChoiceMetrics} from "../metrics";
Expand Down Expand Up @@ -296,9 +296,8 @@ export class ForkChoice implements IForkChoice {
}

if (
merge.isMergeStateType(state) &&
merge.isMergeBlockBodyType(block.body) &&
merge.isMergeBlock(state, block.body)
preCachedData?.isMergeBlock ||
(merge.isMergeStateType(state) && merge.isMergeBlockBodyType(block.body) && merge.isMergeBlock(state, block.body))
)
assertValidTerminalPowBlock(this.config, (block as unknown) as merge.BeaconBlock, preCachedData);

Expand Down Expand Up @@ -368,13 +367,20 @@ export class ForkChoice implements IForkChoice {
parentRoot: parentRootHex,
targetRoot: toHexString(targetRoot),
stateRoot: toHexString(block.stateRoot),
executionPayloadBlockHash: merge.isMergeBlockBodyType(block.body)
? toHexString(block.body.executionPayload.blockHash)
: null,

justifiedEpoch: stateJustifiedEpoch,
justifiedRoot: toHexString(state.currentJustifiedCheckpoint.root),
finalizedEpoch: finalizedCheckpoint.epoch,
finalizedRoot: toHexString(state.finalizedCheckpoint.root),

...(merge.isMergeBlockBodyType(block.body) &&
merge.isMergeStateType(state) &&
merge.isExecutionEnabled(state, block.body)
? {
executionPayloadBlockHash: toHexString(block.body.executionPayload.blockHash),
executionStatus: this.getPostMergeExecStatus(preCachedData),
}
: {executionPayloadBlockHash: null, executionStatus: this.getPreMergeExecStatus(preCachedData)}),
});
}

Expand Down Expand Up @@ -629,6 +635,35 @@ export class ForkChoice implements IForkChoice {
return newNode.slot - commonAncestor.slot;
}

/**
* Optimistic sync validate till validated latest hash, invalidate any decendant branch if invalidated branch decendant provided
* TODO: implementation:
* 1. verify is_merge_block if the mergeblock has not yet been validated
* 2. Throw critical error and exit if a block in finalized chain gets invalidated
*/
validateLatestHash(_latestValidHash: RootHex, _invalidBranchDecendantHash: RootHex | null): void {
// Silently ignore for now if all calls were valid
return;
}

private getPreMergeExecStatus(preCachedData?: OnBlockPrecachedData): ExecutionStatus.PreMerge {
const executionStatus = preCachedData?.executionStatus || ExecutionStatus.PreMerge;
if (executionStatus !== ExecutionStatus.PreMerge)
throw Error(`Invalid pre-merge execution status: expected: ${ExecutionStatus.PreMerge}, got ${executionStatus}`);
return executionStatus;
}

private getPostMergeExecStatus(
preCachedData?: OnBlockPrecachedData
): ExecutionStatus.Valid | ExecutionStatus.Syncing {
const executionStatus = preCachedData?.executionStatus || ExecutionStatus.Syncing;
if (executionStatus === ExecutionStatus.PreMerge)
throw Error(
`Invalid post-merge execution status: expected: ${ExecutionStatus.Syncing} or ${ExecutionStatus.Valid} , got ${executionStatus}`
);
return executionStatus;
}

private updateJustified(justifiedCheckpoint: CheckpointWithHex, justifiedBalances: number[]): void {
this.synced = false;
this.justifiedBalances = justifiedBalances;
Expand Down Expand Up @@ -922,6 +957,10 @@ function assertValidTerminalPowBlock(
);
} else {
// If no TERMINAL_BLOCK_HASH override, check ttd

// Delay powBlock checks if the payload execution status is unknown because of syncing response in executePayload call while verifying
if (preCachedData?.executionStatus === ExecutionStatus.Syncing) return;

const {powBlock, powBlockParent} = preCachedData || {};
if (!powBlock) throw Error("onBlock preCachedData must include powBlock");
if (!powBlockParent) throw Error("onBlock preCachedData must include powBlock");
Expand Down
15 changes: 12 additions & 3 deletions packages/fork-choice/src/forkChoice/interface.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import {Epoch, Slot, ValidatorIndex, phase0, allForks, Root, RootHex} from "@chainsafe/lodestar-types";
import {IProtoBlock} from "../protoArray/interface";
import {IProtoBlock, ExecutionStatus} from "../protoArray/interface";
import {CheckpointWithHex} from "./store";

export type CheckpointHex = {
Expand Down Expand Up @@ -132,6 +132,10 @@ export interface IForkChoice {
getBlockSummariesAtSlot(slot: Slot): IProtoBlock[];
/** Returns the distance of common ancestor of nodes to newNode. Returns null if newNode is descendant of prevNode */
getCommonAncestorDistance(prevBlock: IProtoBlock, newBlock: IProtoBlock): number | null;
/**
* Optimistic sync validate till validated latest hash, invalidate any decendant branch if invalidated branch decendant provided
*/
validateLatestHash(latestValidHash: RootHex, invalidBranchDecendantHash: RootHex | null): void;
}

/** Same to the PowBlock but we want RootHex to work with forkchoice conveniently */
Expand All @@ -150,14 +154,19 @@ export type OnBlockPrecachedData = {
* powBlock = getPowBlock((block as merge.BeaconBlock).body.executionPayload.parentHash)
* ```
*/
powBlock?: PowBlockHex;
powBlock?: PowBlockHex | null;
/**
* POW chain block's block parent, from getPowBlock() `eth_getBlockByHash` JSON RPC endpoint
* ```ts
* const powParent = getPowBlock(powBlock.parentHash);
* ```
*/
powBlockParent?: PowBlockHex;
powBlockParent?: PowBlockHex | null;
/**
* Optimistic sync fields
*/
isMergeBlock?: boolean;
executionStatus?: ExecutionStatus;
};

export interface ILatestMessage {
Expand Down
2 changes: 1 addition & 1 deletion packages/fork-choice/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
export {ProtoArray} from "./protoArray/protoArray";
export {IProtoBlock, IProtoNode} from "./protoArray/interface";
export {IProtoBlock, IProtoNode, ExecutionStatus} from "./protoArray/interface";

export {ForkChoice} from "./forkChoice/forkChoice";
export {
Expand Down
25 changes: 15 additions & 10 deletions packages/fork-choice/src/protoArray/interface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,22 @@ export interface IVoteTracker {
nextEpoch: Epoch;
}

export enum ExecutionStatus {
Valid = "Valid",
Syncing = "Syncing",
PreMerge = "PreMerge",
}

type BlockExecution =
| {executionPayloadBlockHash: RootHex; executionStatus: ExecutionStatus.Valid | ExecutionStatus.Syncing}
| {executionPayloadBlockHash: null; executionStatus: ExecutionStatus.PreMerge};
/**
* A block that is to be applied to the fork choice
*
* A simplified version of BeaconBlock
*/
export interface IProtoBlock {

export type IProtoBlock = BlockExecution & {
/**
* The slot is not necessary for ProtoArray,
* it just exists so external components can easily query the block slot.
Expand All @@ -39,25 +49,20 @@ export interface IProtoBlock {
* it also just exists for upstream components (namely attestation verification)
*/
targetRoot: RootHex;
/**
* `executionPayloadBlockHash` is not necessary for ProtoArray either.
* Here to do ExecutionEngine.notify_forkchoice_updated() easier.
* TODO: Check with other teams if this is the optimal strategy
*/
executionPayloadBlockHash: RootHex | null;

justifiedEpoch: Epoch;
justifiedRoot: RootHex;
finalizedEpoch: Epoch;
finalizedRoot: RootHex;
}
};

/**
* A block root with additional metadata required to form a DAG
* with vote weights and best blocks stored as metadata
*/
export interface IProtoNode extends IProtoBlock {
export type IProtoNode = IProtoBlock & {
parent?: number;
weight: number;
bestChild?: number;
bestDescendant?: number;
}
};
2 changes: 1 addition & 1 deletion packages/fork-choice/src/protoArray/protoArray.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ export class ProtoArray {
...block,
// We are using the blockROot as the targetRoot, since it always lies on an epoch boundary
targetRoot: block.blockRoot,
});
} as IProtoBlock);
return protoArray;
}

Expand Down
14 changes: 10 additions & 4 deletions packages/fork-choice/test/unit/forkChoice/forkChoice.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {ForkChoice, IForkChoiceStore, IProtoBlock, ProtoArray} from "../../../src";
import {ForkChoice, IForkChoiceStore, IProtoBlock, ProtoArray, ExecutionStatus} from "../../../src";
import {config} from "@chainsafe/lodestar-config/default";
import {expect} from "chai";
import {fromHexString} from "@chainsafe/ssz";
Expand All @@ -18,12 +18,15 @@ describe("Forkchoice", function () {
stateRoot,
parentRoot,
blockRoot: finalizedRoot,
executionPayloadBlockHash: null,

justifiedEpoch: genesisEpoch,
justifiedRoot: genesisRoot,
finalizedEpoch: genesisEpoch,
finalizedRoot: genesisRoot,
});

executionPayloadBlockHash: null,
executionStatus: ExecutionStatus.PreMerge,
} as Omit<IProtoBlock, "targetRoot">);

// Add block that is a finalized descendant.
const block: IProtoBlock = {
Expand All @@ -32,11 +35,14 @@ describe("Forkchoice", function () {
parentRoot: finalizedRoot,
stateRoot,
targetRoot: finalizedRoot,
executionPayloadBlockHash: null,

justifiedEpoch: genesisEpoch,
justifiedRoot: genesisRoot,
finalizedEpoch: genesisEpoch,
finalizedRoot: genesisRoot,

executionPayloadBlockHash: null,
executionStatus: ExecutionStatus.PreMerge,
};

const fcStore: IForkChoiceStore = {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import {expect} from "chai";
import {ProtoArray} from "../../../src";
import {ProtoArray, ExecutionStatus} from "../../../src";

describe("getCommonAncestor", () => {
const blocks: {slot: number; root: string; parent: string}[] = [
Expand Down Expand Up @@ -29,11 +29,13 @@ describe("getCommonAncestor", () => {
stateRoot: "-",
parentRoot: "-",
blockRoot: "0",
executionPayloadBlockHash: null,

justifiedEpoch: 0,
justifiedRoot: "-",
finalizedEpoch: 0,
finalizedRoot: "-",

...{executionPayloadBlockHash: null, executionStatus: ExecutionStatus.PreMerge},
});

for (const block of blocks) {
Expand All @@ -43,11 +45,13 @@ describe("getCommonAncestor", () => {
parentRoot: block.parent,
stateRoot: "-",
targetRoot: "-",
executionPayloadBlockHash: null,

justifiedEpoch: 0,
justifiedRoot: "-",
finalizedEpoch: 0,
finalizedRoot: "-",

...{executionPayloadBlockHash: null, executionStatus: ExecutionStatus.PreMerge},
});
}

Expand Down
14 changes: 10 additions & 4 deletions packages/fork-choice/test/unit/protoArray/protoArray.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import {RootHex} from "@chainsafe/lodestar-types";
import {expect} from "chai";

import {ProtoArray} from "../../../src";
import {ProtoArray, ExecutionStatus} from "../../../src";

describe("ProtoArray", () => {
it("finalized descendant", () => {
Expand All @@ -19,11 +19,13 @@ describe("ProtoArray", () => {
stateRoot,
parentRoot,
blockRoot: finalizedRoot,
executionPayloadBlockHash: null,

justifiedEpoch: genesisEpoch,
justifiedRoot: stateRoot,
finalizedEpoch: genesisEpoch,
finalizedRoot: stateRoot,

...{executionPayloadBlockHash: null, executionStatus: ExecutionStatus.PreMerge},
});

// Add block that is a finalized descendant.
Expand All @@ -33,11 +35,13 @@ describe("ProtoArray", () => {
parentRoot: finalizedRoot,
stateRoot,
targetRoot: finalizedRoot,
executionPayloadBlockHash: null,

justifiedEpoch: genesisEpoch,
justifiedRoot: stateRoot,
finalizedEpoch: genesisEpoch,
finalizedRoot: stateRoot,

...{executionPayloadBlockHash: null, executionStatus: ExecutionStatus.PreMerge},
});

// Add block that is *not* a finalized descendant.
Expand All @@ -47,11 +51,13 @@ describe("ProtoArray", () => {
parentRoot: unknown,
stateRoot,
targetRoot: finalizedRoot,
executionPayloadBlockHash: null,

justifiedEpoch: genesisEpoch,
justifiedRoot: stateRoot,
finalizedEpoch: genesisEpoch,
finalizedRoot: stateRoot,

...{executionPayloadBlockHash: null, executionStatus: ExecutionStatus.PreMerge},
g11tech marked this conversation as resolved.
Show resolved Hide resolved
});

// ancestorRoot, descendantRoot, isDescendant
Expand Down
Loading