From 55ecff7a5d7d8a43f2d0e62b9f3d4e3cd0400f4f Mon Sep 17 00:00:00 2001 From: Tuyen Nguyen Date: Thu, 9 Feb 2023 14:03:54 +0700 Subject: [PATCH 01/13] Fix forkchoice bugs and clean up --- .../fork-choice/src/forkChoice/forkChoice.ts | 113 +----------------- packages/fork-choice/src/forkChoice/store.ts | 3 - .../fork-choice/src/protoArray/protoArray.ts | 78 ++++++++++-- .../test/perf/forkChoice/forkChoice.test.ts | 4 - .../test/unit/forkChoice/forkChoice.test.ts | 4 - .../protoArray/executionStatusUpdates.test.ts | 9 +- packages/state-transition/src/util/balance.ts | 4 +- 7 files changed, 77 insertions(+), 138 deletions(-) diff --git a/packages/fork-choice/src/forkChoice/forkChoice.ts b/packages/fork-choice/src/forkChoice/forkChoice.ts index bea7de77c06b..94903468e358 100644 --- a/packages/fork-choice/src/forkChoice/forkChoice.ts +++ b/packages/fork-choice/src/forkChoice/forkChoice.ts @@ -1,11 +1,6 @@ import {toHexString} from "@chainsafe/ssz"; import {fromHex} from "@lodestar/utils"; -import { - SAFE_SLOTS_TO_UPDATE_JUSTIFIED, - SLOTS_PER_HISTORICAL_ROOT, - SLOTS_PER_EPOCH, - INTERVALS_PER_SLOT, -} from "@lodestar/params"; +import {SLOTS_PER_HISTORICAL_ROOT, SLOTS_PER_EPOCH, INTERVALS_PER_SLOT} from "@lodestar/params"; import {bellatrix, Slot, ValidatorIndex, phase0, allForks, ssz, RootHex, Epoch, Root} from "@lodestar/types"; import { computeSlotsSinceEpochStart, @@ -131,31 +126,7 @@ export class ForkChoice implements IForkChoice { * https://github.com/ethereum/consensus-specs/blob/v1.1.10/specs/phase0/fork-choice.md#get_ancestor */ getAncestor(blockRoot: RootHex, ancestorSlot: Slot): RootHex { - const block = this.protoArray.getBlock(blockRoot); - if (!block) { - throw new ForkChoiceError({ - code: ForkChoiceErrorCode.MISSING_PROTO_ARRAY_BLOCK, - root: blockRoot, - }); - } - - if (block.slot > ancestorSlot) { - // Search for a slot that is lte the target slot. - // We check for lower slots to account for skip slots. - for (const node of this.protoArray.iterateAncestorNodes(blockRoot)) { - if (node.slot <= ancestorSlot) { - return node.blockRoot; - } - } - throw new ForkChoiceError({ - code: ForkChoiceErrorCode.UNKNOWN_ANCESTOR, - descendantRoot: blockRoot, - ancestorSlot, - }); - } else { - // Root is older or equal than queried slot, thus a skip slot. Return most recent root prior to slot. - return blockRoot; - } + return this.protoArray.getAncestor(blockRoot, ancestorSlot); } /** @@ -284,10 +255,6 @@ export class ForkChoice implements IForkChoice { return this.fcStore.justified.checkpoint; } - getBestJustifiedCheckpoint(): CheckpointWithHex { - return this.fcStore.bestJustified.checkpoint; - } - /** * Add `block` to the fork choice DAG. * @@ -884,56 +851,6 @@ export class ForkChoice implements IForkChoice { return executionStatus; } - /** - * Returns `true` if the given `store` should be updated to set - * `state.current_justified_checkpoint` its `justified_checkpoint`. - * - * ## Specification - * - * Is equivalent to: - * - * https://github.com/ethereum/consensus-specs/blob/v1.1.10/specs/phase0/fork-choice.md#should_update_justified_checkpoint - */ - private shouldUpdateJustifiedCheckpoint(newJustifiedCheckpoint: CheckpointWithHex, stateSlot: Slot): boolean { - // To address the bouncing attack, only update conflicting justified checkpoints in the first 1/3 of the epoch. - // Otherwise, delay consideration until the next epoch boundary with bestJustifiedCheckpoint - // See https://ethresear.ch/t/prevention-of-bouncing-attack-on-ffg/6114 for more detailed analysis and discussion. - if (computeSlotsSinceEpochStart(this.fcStore.currentSlot) < SAFE_SLOTS_TO_UPDATE_JUSTIFIED) { - return true; - } - - const justifiedSlot = computeStartSlotAtEpoch(this.fcStore.justified.checkpoint.epoch); - - // This sanity check is not in the spec, but the invariant is implied - if (justifiedSlot >= stateSlot) { - throw new ForkChoiceError({ - code: ForkChoiceErrorCode.ATTEMPT_TO_REVERT_JUSTIFICATION, - store: justifiedSlot, - state: stateSlot, - }); - } - - // at regular sync time we don't want to wait for clock time next epoch to update bestJustifiedCheckpoint - if (computeEpochAtSlot(stateSlot) < computeEpochAtSlot(this.fcStore.currentSlot)) { - return true; - } - - // We know that the slot for `new_justified_checkpoint.root` is not greater than - // `state.slot`, since a state cannot justify its own slot. - // - // We know that `new_justified_checkpoint.root` is an ancestor of `state`, since a `state` - // only ever justifies ancestors. - // - // A prior `if` statement protects against a justified_slot that is greater than - // `state.slot` - const justifiedAncestor = this.getAncestor(toHexString(newJustifiedCheckpoint.root), justifiedSlot); - if (justifiedAncestor !== this.fcStore.justified.checkpoint.rootHex) { - return false; - } - - return true; - } - /** * Why `getJustifiedBalances` getter? * - updateCheckpoints() is called in both on_block and on_tick. @@ -951,7 +868,6 @@ export class ForkChoice implements IForkChoice { * * **`on_tick`** * May need the justified balances of: - * - bestJustified: Already available in `CheckpointHexWithBalance` * - unrealizedJustified: Already available in `CheckpointHexWithBalance` * Since this balances are already available the getter is just `() => balances`, without cache interaction */ @@ -963,20 +879,13 @@ export class ForkChoice implements IForkChoice { ): void { // Update justified checkpoint. if (justifiedCheckpoint.epoch > this.fcStore.justified.checkpoint.epoch) { - if (justifiedCheckpoint.epoch > this.fcStore.bestJustified.checkpoint.epoch) { - this.fcStore.bestJustified = {checkpoint: justifiedCheckpoint, balances: getJustifiedBalances()}; - } - - if (this.shouldUpdateJustifiedCheckpoint(justifiedCheckpoint, stateSlot)) { - this.fcStore.justified = {checkpoint: justifiedCheckpoint, balances: getJustifiedBalances()}; - this.justifiedProposerBoostScore = null; - } + this.fcStore.justified = {checkpoint: justifiedCheckpoint, balances: getJustifiedBalances()}; + this.justifiedProposerBoostScore = null; } // Update finalized checkpoint. if (finalizedCheckpoint.epoch > this.fcStore.finalizedCheckpoint.epoch) { this.fcStore.finalizedCheckpoint = finalizedCheckpoint; - this.fcStore.justified = {checkpoint: justifiedCheckpoint, balances: getJustifiedBalances()}; this.justifiedProposerBoostScore = null; } } @@ -1209,20 +1118,6 @@ export class ForkChoice implements IForkChoice { return; } - // Reason: A better justifiedCheckpoint from a block is only updated immediately if in the first 1/3 of the epoch - // This addresses a bouncing attack, see https://ethresear.ch/t/prevention-of-bouncing-attack-on-ffg/6114 - if (this.fcStore.bestJustified.checkpoint.epoch > this.fcStore.justified.checkpoint.epoch) { - // TODO: Is this check necessary? It checks that bestJustifiedCheckpoint is still descendant of finalized - // From https://github.com/ChainSafe/lodestar/commit/6a0745e9db27dfce67b6e6c25bba452283dbbea9# - const finalizedSlot = computeStartSlotAtEpoch(this.fcStore.finalizedCheckpoint.epoch); - const ancestorAtFinalizedSlot = this.getAncestor(this.fcStore.bestJustified.checkpoint.rootHex, finalizedSlot); - if (ancestorAtFinalizedSlot === this.fcStore.finalizedCheckpoint.rootHex) { - // Provide pre-computed balances for bestJustified, will never trigger .justifiedBalancesGetter() - this.fcStore.justified = this.fcStore.bestJustified; - this.justifiedProposerBoostScore = null; - } - } - // Update store.justified_checkpoint if a better unrealized justified checkpoint is known this.updateCheckpoints( time, diff --git a/packages/fork-choice/src/forkChoice/store.ts b/packages/fork-choice/src/forkChoice/store.ts index 16666c352360..e70aca3ebd16 100644 --- a/packages/fork-choice/src/forkChoice/store.ts +++ b/packages/fork-choice/src/forkChoice/store.ts @@ -38,7 +38,6 @@ export type JustifiedBalancesGetter = ( export interface IForkChoiceStore { currentSlot: Slot; justified: CheckpointHexWithBalance; - bestJustified: CheckpointHexWithBalance; unrealizedJustified: CheckpointHexWithBalance; finalizedCheckpoint: CheckpointWithHex; unrealizedFinalizedCheckpoint: CheckpointWithHex; @@ -51,7 +50,6 @@ export interface IForkChoiceStore { */ export class ForkChoiceStore implements IForkChoiceStore { private _justified: CheckpointHexWithBalance; - bestJustified: CheckpointHexWithBalance; unrealizedJustified: CheckpointHexWithBalance; private _finalizedCheckpoint: CheckpointWithHex; unrealizedFinalizedCheckpoint: CheckpointWithHex; @@ -73,7 +71,6 @@ export class ForkChoiceStore implements IForkChoiceStore { balances: justifiedBalances, }; this._justified = justified; - this.bestJustified = justified; this.unrealizedJustified = justified; this._finalizedCheckpoint = toCheckpointWithHex(finalizedCheckpoint); this.unrealizedFinalizedCheckpoint = this._finalizedCheckpoint; diff --git a/packages/fork-choice/src/protoArray/protoArray.ts b/packages/fork-choice/src/protoArray/protoArray.ts index 999b39349d02..28088bf4367c 100644 --- a/packages/fork-choice/src/protoArray/protoArray.ts +++ b/packages/fork-choice/src/protoArray/protoArray.ts @@ -1,9 +1,10 @@ import {Epoch, RootHex, Slot} from "@lodestar/types"; -import {computeEpochAtSlot} from "@lodestar/state-transition"; +import {computeEpochAtSlot, computeStartSlotAtEpoch} from "@lodestar/state-transition"; import {GENESIS_EPOCH} from "@lodestar/params"; import {toHexString} from "@chainsafe/ssz"; import {ForkChoiceOpts} from "../forkChoice/forkChoice.js"; +import {ForkChoiceError, ForkChoiceErrorCode} from "../forkChoice/errors.js"; import {ProtoBlock, ProtoNode, HEX_ZERO_HASH, ExecutionStatus, LVHExecResponse} from "./interface.js"; import {ProtoArrayError, ProtoArrayErrorCode, LVHExecError, LVHExecErrorCode} from "./errors.js"; @@ -743,21 +744,72 @@ export class ProtoArray { const isFromPrevEpoch = computeEpochAtSlot(node.slot) < currentEpoch; const nodeJustifiedEpoch = isFromPrevEpoch ? node.unrealizedJustifiedEpoch : node.justifiedEpoch; const nodeJustifiedRoot = isFromPrevEpoch ? node.unrealizedJustifiedRoot : node.justifiedRoot; - const nodeFinalizedEpoch = isFromPrevEpoch ? node.unrealizedFinalizedEpoch : node.finalizedEpoch; - const nodeFinalizedRoot = isFromPrevEpoch ? node.unrealizedFinalizedRoot : node.finalizedRoot; - // If previous epoch is justified, pull up all tips to at least the previous epoch - if (this.countUnrealizedFull && currentEpoch > GENESIS_EPOCH && this.justifiedEpoch === previousEpoch) { - return node.unrealizedJustifiedEpoch >= previousEpoch; - // If previous epoch is not justified, pull up only tips from past epochs up to the current epoch - } else { - const correctJustified = - (nodeJustifiedEpoch === this.justifiedEpoch && nodeJustifiedRoot === this.justifiedRoot) || - this.justifiedEpoch === 0; + // The voting source should be at the same height as the store's justified checkpoint + let correctJustified = + (nodeJustifiedEpoch === this.justifiedEpoch && nodeJustifiedRoot === this.justifiedRoot) || + this.justifiedEpoch === 0; + + // If this is a pulled-up block from the current epoch, also check that + // the unrealized justification is higher than the store's justified checkpoint, and + // the voting source is not more than two epochs ago. + if ( + !correctJustified && + this.countUnrealizedFull && + currentEpoch > GENESIS_EPOCH && + this.justifiedEpoch === previousEpoch + ) { + correctJustified = node.unrealizedJustifiedEpoch >= previousEpoch && nodeJustifiedEpoch + 2 >= currentEpoch; + } + + try { + const finalizedSlot = computeStartSlotAtEpoch(this.finalizedEpoch); const correctFinalized = - (nodeFinalizedEpoch === this.finalizedEpoch && nodeFinalizedRoot === this.finalizedRoot) || - this.finalizedEpoch === 0; + this.finalizedRoot === this.getAncestor(node.blockRoot, finalizedSlot) || this.finalizedEpoch === 0; return correctJustified && correctFinalized; + } catch (_) { + // getAncestor may throw here, just return false + return false; + } + } + + /** + * Returns the block root of an ancestor of `blockRoot` at the given `slot`. + * (Note: `slot` refers to the block that is *returned*, not the one that is supplied.) + * + * NOTE: May be expensive: potentially walks through the entire fork of head to finalized block + * + * ### Specification + * + * Equivalent to: + * + * https://github.com/ethereum/consensus-specs/blob/v1.1.10/specs/phase0/fork-choice.md#get_ancestor + */ + getAncestor(blockRoot: RootHex, ancestorSlot: Slot): RootHex { + const block = this.getBlock(blockRoot); + if (!block) { + throw new ForkChoiceError({ + code: ForkChoiceErrorCode.MISSING_PROTO_ARRAY_BLOCK, + root: blockRoot, + }); + } + + if (block.slot > ancestorSlot) { + // Search for a slot that is lte the target slot. + // We check for lower slots to account for skip slots. + for (const node of this.iterateAncestorNodes(blockRoot)) { + if (node.slot <= ancestorSlot) { + return node.blockRoot; + } + } + throw new ForkChoiceError({ + code: ForkChoiceErrorCode.UNKNOWN_ANCESTOR, + descendantRoot: blockRoot, + ancestorSlot, + }); + } else { + // Root is older or equal than queried slot, thus a skip slot. Return most recent root prior to slot. + return blockRoot; } } diff --git a/packages/fork-choice/test/perf/forkChoice/forkChoice.test.ts b/packages/fork-choice/test/perf/forkChoice/forkChoice.test.ts index 0932cd5a2788..aa1bb495fcb7 100644 --- a/packages/fork-choice/test/perf/forkChoice/forkChoice.test.ts +++ b/packages/fork-choice/test/perf/forkChoice/forkChoice.test.ts @@ -47,10 +47,6 @@ describe("ForkChoice", () => { checkpoint: {epoch: genesisEpoch, root: fromHexString(finalizedRoot), rootHex: finalizedRoot}, balances, }, - bestJustified: { - checkpoint: {epoch: genesisEpoch, root: fromHexString(finalizedRoot), rootHex: finalizedRoot}, - balances, - }, unrealizedJustified: { checkpoint: {epoch: genesisEpoch, root: fromHexString(finalizedRoot), rootHex: finalizedRoot}, balances, diff --git a/packages/fork-choice/test/unit/forkChoice/forkChoice.test.ts b/packages/fork-choice/test/unit/forkChoice/forkChoice.test.ts index 2782755f2661..966d3a27556c 100644 --- a/packages/fork-choice/test/unit/forkChoice/forkChoice.test.ts +++ b/packages/fork-choice/test/unit/forkChoice/forkChoice.test.ts @@ -52,10 +52,6 @@ describe("Forkchoice", function () { checkpoint: {epoch: genesisEpoch, root: fromHexString(finalizedRoot), rootHex: finalizedRoot}, balances: new Uint8Array([32]), }, - bestJustified: { - checkpoint: {epoch: genesisEpoch, root: fromHexString(finalizedRoot), rootHex: finalizedRoot}, - balances: new Uint8Array([32]), - }, unrealizedJustified: { checkpoint: {epoch: genesisEpoch, root: fromHexString(finalizedRoot), rootHex: finalizedRoot}, balances: new Uint8Array([32]), diff --git a/packages/fork-choice/test/unit/protoArray/executionStatusUpdates.test.ts b/packages/fork-choice/test/unit/protoArray/executionStatusUpdates.test.ts index 76905700c1b5..49cedf5823a3 100644 --- a/packages/fork-choice/test/unit/protoArray/executionStatusUpdates.test.ts +++ b/packages/fork-choice/test/unit/protoArray/executionStatusUpdates.test.ts @@ -47,7 +47,8 @@ const expectedPreValidationFC1: TestCase[] = [ ["3A", undefined, undefined, ExecutionStatus.Syncing], ["2B", "3B", "3B", ExecutionStatus.Syncing], ["3B", undefined, undefined, ExecutionStatus.Syncing], - ["2C", "3C", "3C", ExecutionStatus.Syncing], + // this branch is not a child of finalized node + ["2C", undefined, undefined, ExecutionStatus.Syncing], ["3C", undefined, undefined, ExecutionStatus.Syncing], ]; const expectedPreValidationFC: ValidationTestCase[] = toFcTestCase(expectedPreValidationFC1); @@ -267,7 +268,7 @@ describe("executionStatus / invalidate all postmerge chain", () => { ["3A", undefined, undefined, ExecutionStatus.Invalid], ["2B", undefined, undefined, ExecutionStatus.Invalid], ["3B", undefined, undefined, ExecutionStatus.Invalid], - ["2C", "3C", "3C", ExecutionStatus.Syncing], + ["2C", undefined, undefined, ExecutionStatus.Syncing], ["3C", undefined, undefined, ExecutionStatus.Syncing], ]) ); @@ -318,7 +319,7 @@ describe("executionStatus / poision forkchoice if we invalidate previous valid", ["3A", undefined, undefined, ExecutionStatus.Syncing], ["2B", "3B", "3B", ExecutionStatus.Valid], ["3B", undefined, undefined, ExecutionStatus.Valid], - ["2C", "3C", "3C", ExecutionStatus.Syncing], + ["2C", undefined, undefined, ExecutionStatus.Syncing], ["3C", undefined, undefined, ExecutionStatus.Syncing], ]) ); @@ -381,7 +382,7 @@ describe("executionStatus / poision forkchoice if we validate previous invalid", ["3A", undefined, undefined, ExecutionStatus.Invalid], ["2B", undefined, undefined, ExecutionStatus.Invalid], ["3B", undefined, undefined, ExecutionStatus.Invalid], - ["2C", "3C", "3C", ExecutionStatus.Syncing], + ["2C", undefined, undefined, ExecutionStatus.Syncing], ["3C", undefined, undefined, ExecutionStatus.Syncing], ]) ); diff --git a/packages/state-transition/src/util/balance.ts b/packages/state-transition/src/util/balance.ts index 2af26deab35b..989d54e7a163 100644 --- a/packages/state-transition/src/util/balance.ts +++ b/packages/state-transition/src/util/balance.ts @@ -63,9 +63,11 @@ export function getEffectiveBalanceIncrementsZeroInactive( validatorCount ); + const validators = justifiedState.validators.getAllReadonly(); + let j = 0; for (let i = 0; i < validatorCount; i++) { - if (i === activeIndices[j]) { + if (i === activeIndices[j] && !validators[i].slashed) { // active validator j++; } else { From 8532b878b03ff267dd540fe076b4e0e8f107d0bd Mon Sep 17 00:00:00 2001 From: Tuyen Nguyen Date: Thu, 9 Feb 2023 17:03:17 +0700 Subject: [PATCH 02/13] Fix justified balances --- packages/beacon-node/src/chain/forkChoice/index.ts | 3 ++- packages/state-transition/src/util/balance.ts | 7 +++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/packages/beacon-node/src/chain/forkChoice/index.ts b/packages/beacon-node/src/chain/forkChoice/index.ts index 652793da815d..6611f8070e64 100644 --- a/packages/beacon-node/src/chain/forkChoice/index.ts +++ b/packages/beacon-node/src/chain/forkChoice/index.ts @@ -85,7 +85,8 @@ export function initializeForkChoice( } : {executionPayloadBlockHash: null, executionStatus: ExecutionStatus.PreMerge}), }, - currentSlot + currentSlot, + opts ), opts diff --git a/packages/state-transition/src/util/balance.ts b/packages/state-transition/src/util/balance.ts index 989d54e7a163..e305c745ab72 100644 --- a/packages/state-transition/src/util/balance.ts +++ b/packages/state-transition/src/util/balance.ts @@ -64,12 +64,15 @@ export function getEffectiveBalanceIncrementsZeroInactive( ); const validators = justifiedState.validators.getAllReadonly(); - let j = 0; for (let i = 0; i < validatorCount; i++) { - if (i === activeIndices[j] && !validators[i].slashed) { + if (i === activeIndices[j]) { // active validator j++; + if (validators[i].slashed) { + // slashed validator + effectiveBalanceIncrementsZeroInactive[i] = 0; + } } else { // inactive validator effectiveBalanceIncrementsZeroInactive[i] = 0; From 6c9213ae10a26fe03c39fa279e3afacafa255fa4 Mon Sep 17 00:00:00 2001 From: Tuyen Nguyen Date: Fri, 10 Feb 2023 08:41:33 +0700 Subject: [PATCH 03/13] Refactor variable name in nodeIsViableForHead --- packages/fork-choice/src/protoArray/protoArray.ts | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/packages/fork-choice/src/protoArray/protoArray.ts b/packages/fork-choice/src/protoArray/protoArray.ts index 28088bf4367c..059699326c9b 100644 --- a/packages/fork-choice/src/protoArray/protoArray.ts +++ b/packages/fork-choice/src/protoArray/protoArray.ts @@ -742,13 +742,10 @@ export class ProtoArray { // If block is from a previous epoch, filter using unrealized justification & finalization information // If block is from the current epoch, filter using the head state's justification & finalization information const isFromPrevEpoch = computeEpochAtSlot(node.slot) < currentEpoch; - const nodeJustifiedEpoch = isFromPrevEpoch ? node.unrealizedJustifiedEpoch : node.justifiedEpoch; - const nodeJustifiedRoot = isFromPrevEpoch ? node.unrealizedJustifiedRoot : node.justifiedRoot; + const votingSourceEpoch = isFromPrevEpoch ? node.unrealizedJustifiedEpoch : node.justifiedEpoch; // The voting source should be at the same height as the store's justified checkpoint - let correctJustified = - (nodeJustifiedEpoch === this.justifiedEpoch && nodeJustifiedRoot === this.justifiedRoot) || - this.justifiedEpoch === 0; + let correctJustified = votingSourceEpoch === this.justifiedEpoch || this.justifiedEpoch === 0; // If this is a pulled-up block from the current epoch, also check that // the unrealized justification is higher than the store's justified checkpoint, and @@ -759,7 +756,7 @@ export class ProtoArray { currentEpoch > GENESIS_EPOCH && this.justifiedEpoch === previousEpoch ) { - correctJustified = node.unrealizedJustifiedEpoch >= previousEpoch && nodeJustifiedEpoch + 2 >= currentEpoch; + correctJustified = node.unrealizedJustifiedEpoch >= previousEpoch && votingSourceEpoch + 2 >= currentEpoch; } try { From 341928e909ae240927820e156f57733c6048363f Mon Sep 17 00:00:00 2001 From: Tuyen Nguyen Date: Fri, 10 Feb 2023 16:02:39 +0700 Subject: [PATCH 04/13] Remove countUnrealizedFull flag --- .../beacon-node/src/chain/forkChoice/index.ts | 3 +- packages/beacon-node/src/chain/options.ts | 1 - .../opPools/aggregatedAttestationPool.test.ts | 4 -- .../src/options/beaconNodeOptions/chain.ts | 10 ---- .../unit/options/beaconNodeOptions.test.ts | 2 - .../fork-choice/src/forkChoice/forkChoice.ts | 1 - .../fork-choice/src/protoArray/protoArray.ts | 60 +++++++------------ 7 files changed, 24 insertions(+), 57 deletions(-) diff --git a/packages/beacon-node/src/chain/forkChoice/index.ts b/packages/beacon-node/src/chain/forkChoice/index.ts index 6611f8070e64..652793da815d 100644 --- a/packages/beacon-node/src/chain/forkChoice/index.ts +++ b/packages/beacon-node/src/chain/forkChoice/index.ts @@ -85,8 +85,7 @@ export function initializeForkChoice( } : {executionPayloadBlockHash: null, executionStatus: ExecutionStatus.PreMerge}), }, - currentSlot, - opts + currentSlot ), opts diff --git a/packages/beacon-node/src/chain/options.ts b/packages/beacon-node/src/chain/options.ts index 4ea7613573b8..0024244f0787 100644 --- a/packages/beacon-node/src/chain/options.ts +++ b/packages/beacon-node/src/chain/options.ts @@ -52,7 +52,6 @@ export const defaultChainOptions: IChainOptions = { disableBlsBatchVerify: false, proposerBoostEnabled: true, computeUnrealized: true, - countUnrealizedFull: false, safeSlotsToImportOptimistically: SAFE_SLOTS_TO_IMPORT_OPTIMISTICALLY, suggestedFeeRecipient: defaultValidatorOptions.suggestedFeeRecipient, assertCorrectProgressiveBalances: false, diff --git a/packages/beacon-node/test/perf/chain/opPools/aggregatedAttestationPool.test.ts b/packages/beacon-node/test/perf/chain/opPools/aggregatedAttestationPool.test.ts index 440ba3bd30b5..e4f202684a29 100644 --- a/packages/beacon-node/test/perf/chain/opPools/aggregatedAttestationPool.test.ts +++ b/packages/beacon-node/test/perf/chain/opPools/aggregatedAttestationPool.test.ts @@ -107,10 +107,6 @@ describe("getAttestationsForBlock", () => { checkpoint: {...justifiedCheckpoint, rootHex: toHexString(justifiedCheckpoint.root)}, balances: originalState.epochCtx.effectiveBalanceIncrements, }, - bestJustified: { - checkpoint: {...justifiedCheckpoint, rootHex: toHexString(justifiedCheckpoint.root)}, - balances: originalState.epochCtx.effectiveBalanceIncrements, - }, unrealizedJustified: { checkpoint: {...justifiedCheckpoint, rootHex: toHexString(justifiedCheckpoint.root)}, balances: originalState.epochCtx.effectiveBalanceIncrements, diff --git a/packages/cli/src/options/beaconNodeOptions/chain.ts b/packages/cli/src/options/beaconNodeOptions/chain.ts index 308dfa70ca30..1a1b18e811ba 100644 --- a/packages/cli/src/options/beaconNodeOptions/chain.ts +++ b/packages/cli/src/options/beaconNodeOptions/chain.ts @@ -13,7 +13,6 @@ export type ChainArgs = { "chain.proposerBoostEnabled": boolean; "chain.disableImportExecutionFcU": boolean; "chain.computeUnrealized": boolean; - "chain.countUnrealizedFull": boolean; "chain.assertCorrectProgressiveBalances": boolean; "chain.maxSkipSlots": number; "safe-slots-to-import-optimistically": number; @@ -32,7 +31,6 @@ export function parseArgs(args: ChainArgs): IBeaconNodeOptions["chain"] { proposerBoostEnabled: args["chain.proposerBoostEnabled"], disableImportExecutionFcU: args["chain.disableImportExecutionFcU"], computeUnrealized: args["chain.computeUnrealized"], - countUnrealizedFull: args["chain.countUnrealizedFull"], assertCorrectProgressiveBalances: args["chain.assertCorrectProgressiveBalances"], maxSkipSlots: args["chain.maxSkipSlots"], safeSlotsToImportOptimistically: args["safe-slots-to-import-optimistically"], @@ -105,14 +103,6 @@ Will double processing times. Use only for debugging purposes.", group: "chain", }, - "chain.countUnrealizedFull": { - hidden: true, - type: "boolean", - description: "Compute unrealized checkpoints and fully use it", - defaultDescription: String(defaultOptions.chain.computeUnrealized), - group: "chain", - }, - "chain.maxSkipSlots": { hidden: true, type: "number", diff --git a/packages/cli/test/unit/options/beaconNodeOptions.test.ts b/packages/cli/test/unit/options/beaconNodeOptions.test.ts index d8a9bb438304..44946258f76e 100644 --- a/packages/cli/test/unit/options/beaconNodeOptions.test.ts +++ b/packages/cli/test/unit/options/beaconNodeOptions.test.ts @@ -24,7 +24,6 @@ describe("options / beaconNodeOptions", () => { "chain.proposerBoostEnabled": false, "chain.disableImportExecutionFcU": false, "chain.computeUnrealized": true, - "chain.countUnrealizedFull": true, suggestedFeeRecipient: "0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", "chain.assertCorrectProgressiveBalances": true, "chain.maxSkipSlots": 100, @@ -111,7 +110,6 @@ describe("options / beaconNodeOptions", () => { proposerBoostEnabled: false, disableImportExecutionFcU: false, computeUnrealized: true, - countUnrealizedFull: true, safeSlotsToImportOptimistically: 256, suggestedFeeRecipient: "0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", assertCorrectProgressiveBalances: true, diff --git a/packages/fork-choice/src/forkChoice/forkChoice.ts b/packages/fork-choice/src/forkChoice/forkChoice.ts index 94903468e358..825231adf28b 100644 --- a/packages/fork-choice/src/forkChoice/forkChoice.ts +++ b/packages/fork-choice/src/forkChoice/forkChoice.ts @@ -45,7 +45,6 @@ import {IForkChoiceStore, CheckpointWithHex, toCheckpointWithHex, JustifiedBalan export type ForkChoiceOpts = { proposerBoostEnabled?: boolean; computeUnrealized?: boolean; - countUnrealizedFull?: boolean; }; /** diff --git a/packages/fork-choice/src/protoArray/protoArray.ts b/packages/fork-choice/src/protoArray/protoArray.ts index 059699326c9b..745959c42435 100644 --- a/packages/fork-choice/src/protoArray/protoArray.ts +++ b/packages/fork-choice/src/protoArray/protoArray.ts @@ -3,7 +3,6 @@ import {computeEpochAtSlot, computeStartSlotAtEpoch} from "@lodestar/state-trans import {GENESIS_EPOCH} from "@lodestar/params"; import {toHexString} from "@chainsafe/ssz"; -import {ForkChoiceOpts} from "../forkChoice/forkChoice.js"; import {ForkChoiceError, ForkChoiceErrorCode} from "../forkChoice/errors.js"; import {ProtoBlock, ProtoNode, HEX_ZERO_HASH, ExecutionStatus, LVHExecResponse} from "./interface.js"; import {ProtoArrayError, ProtoArrayErrorCode, LVHExecError, LVHExecErrorCode} from "./errors.js"; @@ -26,43 +25,35 @@ export class ProtoArray { lvhError?: LVHExecError; private previousProposerBoost: ProposerBoost | null = null; - private countUnrealizedFull = false; - - constructor( - { - pruneThreshold, - justifiedEpoch, - justifiedRoot, - finalizedEpoch, - finalizedRoot, - }: { - pruneThreshold: number; - justifiedEpoch: Epoch; - justifiedRoot: RootHex; - finalizedEpoch: Epoch; - finalizedRoot: RootHex; - }, - opts?: ForkChoiceOpts - ) { + + constructor({ + pruneThreshold, + justifiedEpoch, + justifiedRoot, + finalizedEpoch, + finalizedRoot, + }: { + pruneThreshold: number; + justifiedEpoch: Epoch; + justifiedRoot: RootHex; + finalizedEpoch: Epoch; + finalizedRoot: RootHex; + }) { this.pruneThreshold = pruneThreshold; this.justifiedEpoch = justifiedEpoch; this.justifiedRoot = justifiedRoot; this.finalizedEpoch = finalizedEpoch; this.finalizedRoot = finalizedRoot; - this.countUnrealizedFull = opts?.countUnrealizedFull ?? false; } - static initialize(block: Omit, currentSlot: Slot, opts?: ForkChoiceOpts): ProtoArray { - const protoArray = new ProtoArray( - { - pruneThreshold: DEFAULT_PRUNE_THRESHOLD, - justifiedEpoch: block.justifiedEpoch, - justifiedRoot: block.justifiedRoot, - finalizedEpoch: block.finalizedEpoch, - finalizedRoot: block.finalizedRoot, - }, - opts - ); + static initialize(block: Omit, currentSlot: Slot): ProtoArray { + const protoArray = new ProtoArray({ + pruneThreshold: DEFAULT_PRUNE_THRESHOLD, + justifiedEpoch: block.justifiedEpoch, + justifiedRoot: block.justifiedRoot, + finalizedEpoch: block.finalizedEpoch, + finalizedRoot: block.finalizedRoot, + }); protoArray.onBlock( { ...block, @@ -750,12 +741,7 @@ export class ProtoArray { // If this is a pulled-up block from the current epoch, also check that // the unrealized justification is higher than the store's justified checkpoint, and // the voting source is not more than two epochs ago. - if ( - !correctJustified && - this.countUnrealizedFull && - currentEpoch > GENESIS_EPOCH && - this.justifiedEpoch === previousEpoch - ) { + if (!correctJustified && currentEpoch > GENESIS_EPOCH && this.justifiedEpoch === previousEpoch) { correctJustified = node.unrealizedJustifiedEpoch >= previousEpoch && votingSourceEpoch + 2 >= currentEpoch; } From 8de4916a3e8ea55ebb4317c521741ba538fbfd5b Mon Sep 17 00:00:00 2001 From: Tuyen Nguyen Date: Mon, 13 Feb 2023 09:46:00 +0700 Subject: [PATCH 05/13] Add getAncestorOrNull() --- .../fork-choice/src/protoArray/protoArray.ts | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/packages/fork-choice/src/protoArray/protoArray.ts b/packages/fork-choice/src/protoArray/protoArray.ts index 745959c42435..e552971ac447 100644 --- a/packages/fork-choice/src/protoArray/protoArray.ts +++ b/packages/fork-choice/src/protoArray/protoArray.ts @@ -745,14 +745,20 @@ export class ProtoArray { correctJustified = node.unrealizedJustifiedEpoch >= previousEpoch && votingSourceEpoch + 2 >= currentEpoch; } + const finalizedSlot = computeStartSlotAtEpoch(this.finalizedEpoch); + const correctFinalized = + this.finalizedRoot === this.getAncestorOrNull(node.blockRoot, finalizedSlot) || this.finalizedEpoch === 0; + return correctJustified && correctFinalized; + } + + /** + * Same to getAncestor but it may return null instead of throwing error + */ + getAncestorOrNull(blockRoot: RootHex, ancestorSlot: Slot): RootHex | null { try { - const finalizedSlot = computeStartSlotAtEpoch(this.finalizedEpoch); - const correctFinalized = - this.finalizedRoot === this.getAncestor(node.blockRoot, finalizedSlot) || this.finalizedEpoch === 0; - return correctJustified && correctFinalized; + return this.getAncestor(blockRoot, ancestorSlot); } catch (_) { - // getAncestor may throw here, just return false - return false; + return null; } } From 10d30e0554a8e41eafb21eed4d18548688bfca8c Mon Sep 17 00:00:00 2001 From: Tuyen Nguyen Date: Tue, 14 Mar 2023 13:55:17 +0700 Subject: [PATCH 06/13] Remove SAFE_SLOTS_TO_UPDATE_JUSTIFIED --- packages/params/src/index.ts | 1 - packages/params/src/interface.ts | 3 --- packages/params/src/presets/mainnet.ts | 5 ----- packages/params/src/presets/minimal.ts | 5 ----- packages/validator/src/util/params.ts | 3 --- packages/validator/test/unit/utils/interopConfigs.ts | 4 ---- 6 files changed, 21 deletions(-) diff --git a/packages/params/src/index.ts b/packages/params/src/index.ts index 75c3dbccaa8c..7e008389bb4d 100644 --- a/packages/params/src/index.ts +++ b/packages/params/src/index.ts @@ -50,7 +50,6 @@ export const { HYSTERESIS_QUOTIENT, HYSTERESIS_DOWNWARD_MULTIPLIER, HYSTERESIS_UPWARD_MULTIPLIER, - SAFE_SLOTS_TO_UPDATE_JUSTIFIED, MIN_DEPOSIT_AMOUNT, MAX_EFFECTIVE_BALANCE, EFFECTIVE_BALANCE_INCREMENT, diff --git a/packages/params/src/interface.ts b/packages/params/src/interface.ts index 2d4f60f0e6e4..41628fd2cf72 100644 --- a/packages/params/src/interface.ts +++ b/packages/params/src/interface.ts @@ -11,9 +11,6 @@ export interface BeaconPreset { HYSTERESIS_DOWNWARD_MULTIPLIER: number; HYSTERESIS_UPWARD_MULTIPLIER: number; - // Fork choice - SAFE_SLOTS_TO_UPDATE_JUSTIFIED: number; - // Gwei Values MIN_DEPOSIT_AMOUNT: number; MAX_EFFECTIVE_BALANCE: number; diff --git a/packages/params/src/presets/mainnet.ts b/packages/params/src/presets/mainnet.ts index 6c09ae2b5f50..5026858bd934 100644 --- a/packages/params/src/presets/mainnet.ts +++ b/packages/params/src/presets/mainnet.ts @@ -19,11 +19,6 @@ export const mainnetPreset: BeaconPreset = { // 5 (plus 1.25) HYSTERESIS_UPWARD_MULTIPLIER: 5, - // Fork Choice - // --------------------------------------------------------------- - // 2**3 (= 8) - SAFE_SLOTS_TO_UPDATE_JUSTIFIED: 8, - // Gwei values // --------------------------------------------------------------- // 2**0 * 10**9 (= 1,000,000,000) Gwei diff --git a/packages/params/src/presets/minimal.ts b/packages/params/src/presets/minimal.ts index 5e4aca671137..ce2e66b23f99 100644 --- a/packages/params/src/presets/minimal.ts +++ b/packages/params/src/presets/minimal.ts @@ -19,11 +19,6 @@ export const minimalPreset: BeaconPreset = { // 5 (plus 1.25) HYSTERESIS_UPWARD_MULTIPLIER: 5, - // Fork Choice - // --------------------------------------------------------------- - // 2**1 (= 1) - SAFE_SLOTS_TO_UPDATE_JUSTIFIED: 2, - // Gwei values // --------------------------------------------------------------- // 2**0 * 10**9 (= 1,000,000,000) Gwei diff --git a/packages/validator/src/util/params.ts b/packages/validator/src/util/params.ts index 731da00595f7..511e16815f2b 100644 --- a/packages/validator/src/util/params.ts +++ b/packages/validator/src/util/params.ts @@ -145,9 +145,6 @@ function getSpecCriticalParams(localConfig: ChainConfig): Record Date: Tue, 14 Mar 2023 15:25:35 +0700 Subject: [PATCH 07/13] Fix unrealized checkpoint reused from parent --- .../fork-choice/src/forkChoice/forkChoice.ts | 38 +++++++++---------- 1 file changed, 18 insertions(+), 20 deletions(-) diff --git a/packages/fork-choice/src/forkChoice/forkChoice.ts b/packages/fork-choice/src/forkChoice/forkChoice.ts index 825231adf28b..0f7ce701ed4e 100644 --- a/packages/fork-choice/src/forkChoice/forkChoice.ts +++ b/packages/fork-choice/src/forkChoice/forkChoice.ts @@ -360,20 +360,19 @@ export class ForkChoice implements IForkChoice { // 1. Its prudent to fail fast and not try importing a block in forkChoice. // 2. Also the data to run such a validation is readily available there. - const currentJustifiedCheckpoint = toCheckpointWithHex(state.currentJustifiedCheckpoint); - const stateJustifiedEpoch = currentJustifiedCheckpoint.epoch; - const justifiedCheckpoint = toCheckpointWithHex(state.currentJustifiedCheckpoint); const finalizedCheckpoint = toCheckpointWithHex(state.finalizedCheckpoint); + const stateJustifiedEpoch = justifiedCheckpoint.epoch; // Justified balances for `justifiedCheckpoint` are new to the fork-choice. Compute them on demand only if // the justified checkpoint changes - this.updateCheckpoints(state.slot, justifiedCheckpoint, finalizedCheckpoint, () => + this.updateCheckpoints(justifiedCheckpoint, finalizedCheckpoint, () => this.fcStore.justifiedBalancesGetter(justifiedCheckpoint, state) ); const blockEpoch = computeEpochAtSlot(slot); + // same logic to compute_pulled_up_tip because a lot of varible could be reused here // If the parent checkpoints are already at the same epoch as the block being imported, // it's impossible for the unrealized checkpoints to differ from the parent's. This // holds true because: @@ -388,7 +387,10 @@ export class ForkChoice implements IForkChoice { let unrealizedJustifiedCheckpoint: CheckpointWithHex; let unrealizedFinalizedCheckpoint: CheckpointWithHex; if (this.opts?.computeUnrealized) { - if (parentBlock.unrealizedJustifiedEpoch === blockEpoch && parentBlock.unrealizedFinalizedEpoch >= blockEpoch) { + if ( + parentBlock.unrealizedJustifiedEpoch === blockEpoch && + parentBlock.unrealizedFinalizedEpoch + 1 >= blockEpoch + ) { // reuse from parent, happens at 1/3 last blocks of epoch as monitored in mainnet unrealizedJustifiedCheckpoint = { epoch: parentBlock.unrealizedJustifiedEpoch, @@ -425,13 +427,7 @@ export class ForkChoice implements IForkChoice { // If block is from past epochs, try to update store's justified & finalized checkpoints right away if (blockEpoch < computeEpochAtSlot(currentSlot)) { - // Compute justified balances for unrealizedJustifiedCheckpoint on demand - if (unrealizedJustifiedCheckpoint === undefined) { - throw Error(); - } - this.updateCheckpoints(state.slot, unrealizedJustifiedCheckpoint, unrealizedFinalizedCheckpoint, () => - this.fcStore.justifiedBalancesGetter(unrealizedJustifiedCheckpoint as CheckpointWithHex, state) - ); + this.pullUpStoreCheckpoints(); } const targetSlot = computeStartSlotAtEpoch(blockEpoch); @@ -850,6 +846,15 @@ export class ForkChoice implements IForkChoice { return executionStatus; } + private pullUpStoreCheckpoints(): void { + this.updateCheckpoints( + this.fcStore.unrealizedJustified.checkpoint, + this.fcStore.unrealizedFinalizedCheckpoint, + // Provide pre-computed balances for unrealizedJustified, will never trigger .justifiedBalancesGetter() + () => this.fcStore.unrealizedJustified.balances + ); + } + /** * Why `getJustifiedBalances` getter? * - updateCheckpoints() is called in both on_block and on_tick. @@ -871,7 +876,6 @@ export class ForkChoice implements IForkChoice { * Since this balances are already available the getter is just `() => balances`, without cache interaction */ private updateCheckpoints( - stateSlot: Slot, justifiedCheckpoint: CheckpointWithHex, finalizedCheckpoint: CheckpointWithHex, getJustifiedBalances: () => JustifiedBalances @@ -1118,13 +1122,7 @@ export class ForkChoice implements IForkChoice { } // Update store.justified_checkpoint if a better unrealized justified checkpoint is known - this.updateCheckpoints( - time, - this.fcStore.unrealizedJustified.checkpoint, - this.fcStore.unrealizedFinalizedCheckpoint, - // Provide pre-computed balances for unrealizedJustified, will never trigger .justifiedBalancesGetter() - () => this.fcStore.unrealizedJustified.balances - ); + this.pullUpStoreCheckpoints(); } } From 6cdce83b0fe5e9fc4fe76c8d7d4b3d2cb6a1e037 Mon Sep 17 00:00:00 2001 From: Tuyen Nguyen Date: Tue, 14 Mar 2023 15:35:40 +0700 Subject: [PATCH 08/13] Remove best_justified_checkpoint step from forkchoice spec test --- packages/beacon-node/test/spec/presets/fork_choice.ts | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/packages/beacon-node/test/spec/presets/fork_choice.ts b/packages/beacon-node/test/spec/presets/fork_choice.ts index 35b35d8b7a0a..8a6eedc4f760 100644 --- a/packages/beacon-node/test/spec/presets/fork_choice.ts +++ b/packages/beacon-node/test/spec/presets/fork_choice.ts @@ -83,7 +83,7 @@ export const forkChoiceTest = (opts: {onlyPredefinedResponses: boolean}): TestRu // we don't use these in fork choice spec tests disablePrepareNextSlot: true, assertCorrectProgressiveBalances, - computeUnrealized: false, + computeUnrealized: true, }, { config: createBeaconConfig(config, state.genesisValidatorsRoot), @@ -246,14 +246,6 @@ export const forkChoiceTest = (opts: {onlyPredefinedResponses: boolean}): TestRu `Invalid finalized checkpoint at step ${i}` ); } - if (step.checks.best_justified_checkpoint) { - expect( - toSpecTestCheckpoint((chain.forkChoice as ForkChoice).getBestJustifiedCheckpoint()) - ).to.be.deep.equal( - step.checks.best_justified_checkpoint, - `Invalid best justified checkpoint at step ${i}` - ); - } } // None of the above @@ -398,7 +390,6 @@ type Checks = { time?: bigint; justified_checkpoint?: SpecTestCheckpoint; finalized_checkpoint?: SpecTestCheckpoint; - best_justified_checkpoint?: SpecTestCheckpoint; proposer_boost_root?: RootHex; }; }; From 9ab95e0a56e10673f2d35f5f5ea6aed37605220c Mon Sep 17 00:00:00 2001 From: Tuyen Nguyen Date: Sat, 18 Mar 2023 06:27:16 +0700 Subject: [PATCH 09/13] Reenable forkchoice tests --- packages/beacon-node/test/spec/presets/index.test.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/packages/beacon-node/test/spec/presets/index.test.ts b/packages/beacon-node/test/spec/presets/index.test.ts index 7aa8398c0111..db7afce6fc3e 100644 --- a/packages/beacon-node/test/spec/presets/index.test.ts +++ b/packages/beacon-node/test/spec/presets/index.test.ts @@ -32,9 +32,6 @@ import {transition} from "./transition.js"; const skipOpts: SkipOpts = { // To be enabled in decouple blobs PR: https://github.com/ChainSafe/lodestar/pull/5181 skippedForks: ["deneb"], - // To be enabled with the fork choice safe slots to justified removal PR - // https://github.com/ChainSafe/lodestar/pull/5126 - skippedRunners: ["fork_choice"], // TODO: capella // BeaconBlockBody proof in lightclient is the new addition in v1.3.0-rc.2-hotfix // Skip them for now to enable subsequently From 8476e1e9908db02d35e9a57c600871729c6c7de9 Mon Sep 17 00:00:00 2001 From: Tuyen Nguyen Date: Sat, 18 Mar 2023 13:47:44 +0700 Subject: [PATCH 10/13] Revert forkchoice unit test modification --- .../test/unit/protoArray/executionStatusUpdates.test.ts | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/packages/fork-choice/test/unit/protoArray/executionStatusUpdates.test.ts b/packages/fork-choice/test/unit/protoArray/executionStatusUpdates.test.ts index 49cedf5823a3..76905700c1b5 100644 --- a/packages/fork-choice/test/unit/protoArray/executionStatusUpdates.test.ts +++ b/packages/fork-choice/test/unit/protoArray/executionStatusUpdates.test.ts @@ -47,8 +47,7 @@ const expectedPreValidationFC1: TestCase[] = [ ["3A", undefined, undefined, ExecutionStatus.Syncing], ["2B", "3B", "3B", ExecutionStatus.Syncing], ["3B", undefined, undefined, ExecutionStatus.Syncing], - // this branch is not a child of finalized node - ["2C", undefined, undefined, ExecutionStatus.Syncing], + ["2C", "3C", "3C", ExecutionStatus.Syncing], ["3C", undefined, undefined, ExecutionStatus.Syncing], ]; const expectedPreValidationFC: ValidationTestCase[] = toFcTestCase(expectedPreValidationFC1); @@ -268,7 +267,7 @@ describe("executionStatus / invalidate all postmerge chain", () => { ["3A", undefined, undefined, ExecutionStatus.Invalid], ["2B", undefined, undefined, ExecutionStatus.Invalid], ["3B", undefined, undefined, ExecutionStatus.Invalid], - ["2C", undefined, undefined, ExecutionStatus.Syncing], + ["2C", "3C", "3C", ExecutionStatus.Syncing], ["3C", undefined, undefined, ExecutionStatus.Syncing], ]) ); @@ -319,7 +318,7 @@ describe("executionStatus / poision forkchoice if we invalidate previous valid", ["3A", undefined, undefined, ExecutionStatus.Syncing], ["2B", "3B", "3B", ExecutionStatus.Valid], ["3B", undefined, undefined, ExecutionStatus.Valid], - ["2C", undefined, undefined, ExecutionStatus.Syncing], + ["2C", "3C", "3C", ExecutionStatus.Syncing], ["3C", undefined, undefined, ExecutionStatus.Syncing], ]) ); @@ -382,7 +381,7 @@ describe("executionStatus / poision forkchoice if we validate previous invalid", ["3A", undefined, undefined, ExecutionStatus.Invalid], ["2B", undefined, undefined, ExecutionStatus.Invalid], ["3B", undefined, undefined, ExecutionStatus.Invalid], - ["2C", undefined, undefined, ExecutionStatus.Syncing], + ["2C", "3C", "3C", ExecutionStatus.Syncing], ["3C", undefined, undefined, ExecutionStatus.Syncing], ]) ); From 8b0db5b51e47ccd1c73114c1a841d5426c081af1 Mon Sep 17 00:00:00 2001 From: Tuyen Nguyen Date: Sat, 18 Mar 2023 14:43:13 +0700 Subject: [PATCH 11/13] Update specConfigCommit --- packages/params/test/e2e/ensure-config-is-synced.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/params/test/e2e/ensure-config-is-synced.test.ts b/packages/params/test/e2e/ensure-config-is-synced.test.ts index 216d90f32f3c..52865a7d3d12 100644 --- a/packages/params/test/e2e/ensure-config-is-synced.test.ts +++ b/packages/params/test/e2e/ensure-config-is-synced.test.ts @@ -8,7 +8,7 @@ import {loadConfigYaml} from "../yaml.js"; // Not e2e, but slow. Run with e2e tests /** https://github.com/ethereum/consensus-specs/releases */ -const specConfigCommit = "v1.3.0-rc.2"; +const specConfigCommit = "v1.3.0-rc.4"; describe("Ensure config is synced", function () { this.timeout(60 * 1000); From 5318ea724104dcb17af3f5b359cc0bb9062f5b5a Mon Sep 17 00:00:00 2001 From: Tuyen Nguyen Date: Sat, 18 Mar 2023 15:14:32 +0700 Subject: [PATCH 12/13] Fix e2e in params --- packages/params/test/e2e/ensure-config-is-synced.test.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/params/test/e2e/ensure-config-is-synced.test.ts b/packages/params/test/e2e/ensure-config-is-synced.test.ts index 52865a7d3d12..3e969d6e4f06 100644 --- a/packages/params/test/e2e/ensure-config-is-synced.test.ts +++ b/packages/params/test/e2e/ensure-config-is-synced.test.ts @@ -37,10 +37,7 @@ async function downloadRemoteConfig(preset: "mainnet" | "minimal", commit: strin const downloadedParams = await Promise.all( Object.values(ForkName).map((forkName) => axios({ - url: `https://raw.githubusercontent.com/ethereum/consensus-specs/${commit}/presets/${preset}/${ - // TODO eip4844: clean this up when specs are released with deneb - forkName === "deneb" ? "eip4844" : forkName - }.yaml`, + url: `https://raw.githubusercontent.com/ethereum/consensus-specs/${commit}/presets/${preset}/${forkName}.yaml`, timeout: 30 * 1000, }).then((response) => loadConfigYaml(response.data)) ) From 5050722970b8b26bea65e588cd7c7943b6324c7e Mon Sep 17 00:00:00 2001 From: Tuyen Nguyen Date: Sun, 19 Mar 2023 06:32:58 +0700 Subject: [PATCH 13/13] computeUnrealized: use default chain option --- packages/beacon-node/test/spec/presets/fork_choice.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/beacon-node/test/spec/presets/fork_choice.ts b/packages/beacon-node/test/spec/presets/fork_choice.ts index 8a6eedc4f760..02d3ea55fd9a 100644 --- a/packages/beacon-node/test/spec/presets/fork_choice.ts +++ b/packages/beacon-node/test/spec/presets/fork_choice.ts @@ -83,7 +83,6 @@ export const forkChoiceTest = (opts: {onlyPredefinedResponses: boolean}): TestRu // we don't use these in fork choice spec tests disablePrepareNextSlot: true, assertCorrectProgressiveBalances, - computeUnrealized: true, }, { config: createBeaconConfig(config, state.genesisValidatorsRoot),