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/beacon-node/test/spec/presets/fork_choice.ts b/packages/beacon-node/test/spec/presets/fork_choice.ts index 35b35d8b7a0a..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: false, }, { config: createBeaconConfig(config, state.genesisValidatorsRoot), @@ -246,14 +245,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 +389,6 @@ type Checks = { time?: bigint; justified_checkpoint?: SpecTestCheckpoint; finalized_checkpoint?: SpecTestCheckpoint; - best_justified_checkpoint?: SpecTestCheckpoint; proposer_boost_root?: RootHex; }; }; 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 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 bea7de77c06b..0f7ce701ed4e 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, @@ -50,7 +45,6 @@ import {IForkChoiceStore, CheckpointWithHex, toCheckpointWithHex, JustifiedBalan export type ForkChoiceOpts = { proposerBoostEnabled?: boolean; computeUnrealized?: boolean; - countUnrealizedFull?: boolean; }; /** @@ -131,31 +125,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 +254,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. * @@ -394,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: @@ -422,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, @@ -459,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); @@ -884,54 +846,13 @@ 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; + 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 + ); } /** @@ -951,32 +872,23 @@ 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 */ private updateCheckpoints( - stateSlot: Slot, justifiedCheckpoint: CheckpointWithHex, finalizedCheckpoint: CheckpointWithHex, getJustifiedBalances: () => JustifiedBalances ): 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,28 +1121,8 @@ 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, - this.fcStore.unrealizedJustified.checkpoint, - this.fcStore.unrealizedFinalizedCheckpoint, - // Provide pre-computed balances for unrealizedJustified, will never trigger .justifiedBalancesGetter() - () => this.fcStore.unrealizedJustified.balances - ); + this.pullUpStoreCheckpoints(); } } 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..e552971ac447 100644 --- a/packages/fork-choice/src/protoArray/protoArray.ts +++ b/packages/fork-choice/src/protoArray/protoArray.ts @@ -1,9 +1,9 @@ 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"; @@ -25,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, @@ -741,23 +733,72 @@ 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 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 + const votingSourceEpoch = isFromPrevEpoch ? node.unrealizedJustifiedEpoch : node.justifiedEpoch; + + // The voting source should be at the same height as the store's justified checkpoint + 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 + // the voting source is not more than two epochs ago. + if (!correctJustified && currentEpoch > GENESIS_EPOCH && this.justifiedEpoch === previousEpoch) { + 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 { + return this.getAncestor(blockRoot, ancestorSlot); + } catch (_) { + return null; + } + } + + /** + * 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 { - const correctJustified = - (nodeJustifiedEpoch === this.justifiedEpoch && nodeJustifiedRoot === this.justifiedRoot) || - this.justifiedEpoch === 0; - const correctFinalized = - (nodeFinalizedEpoch === this.finalizedEpoch && nodeFinalizedRoot === this.finalizedRoot) || - this.finalizedEpoch === 0; - return correctJustified && correctFinalized; + // 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/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/params/test/e2e/ensure-config-is-synced.test.ts b/packages/params/test/e2e/ensure-config-is-synced.test.ts index 216d90f32f3c..3e969d6e4f06 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); @@ -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://github.com/raw/ethereum/consensus-specs/${commit}/presets/${preset}/${ - // TODO eip4844: clean this up when specs are released with deneb - forkName === "deneb" ? "eip4844" : forkName - }.yaml`, + url: `https://github.com/raw/ethereum/consensus-specs/${commit}/presets/${preset}/${forkName}.yaml`, timeout: 30 * 1000, }).then((response) => loadConfigYaml(response.data)) ) diff --git a/packages/state-transition/src/util/balance.ts b/packages/state-transition/src/util/balance.ts index 2af26deab35b..e305c745ab72 100644 --- a/packages/state-transition/src/util/balance.ts +++ b/packages/state-transition/src/util/balance.ts @@ -63,11 +63,16 @@ export function getEffectiveBalanceIncrementsZeroInactive( validatorCount ); + const validators = justifiedState.validators.getAllReadonly(); let j = 0; for (let i = 0; i < validatorCount; i++) { if (i === activeIndices[j]) { // active validator j++; + if (validators[i].slashed) { + // slashed validator + effectiveBalanceIncrementsZeroInactive[i] = 0; + } } else { // inactive validator effectiveBalanceIncrementsZeroInactive[i] = 0; 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