From 079e87a0b3b3a5381bb8210aba1be983e21d99a3 Mon Sep 17 00:00:00 2001 From: Nico Flaig Date: Thu, 18 Jul 2024 21:40:06 +0100 Subject: [PATCH 1/2] fix: return finalized as false if called with genesis slot or epoch --- packages/beacon-node/src/chain/chain.ts | 25 +++++++++++++++++++------ packages/cli/test/sim/endpoints.test.ts | 10 ++++++++++ 2 files changed, 29 insertions(+), 6 deletions(-) diff --git a/packages/beacon-node/src/chain/chain.ts b/packages/beacon-node/src/chain/chain.ts index 61f044d4f895..f73f83197643 100644 --- a/packages/beacon-node/src/chain/chain.ts +++ b/packages/beacon-node/src/chain/chain.ts @@ -36,7 +36,7 @@ import { import {CheckpointWithHex, ExecutionStatus, IForkChoice, ProtoBlock, UpdateHeadOpt} from "@lodestar/fork-choice"; import {ProcessShutdownCallback} from "@lodestar/validator"; import {Logger, gweiToWei, isErrorAborted, pruneSetToMax, sleep, toHex} from "@lodestar/utils"; -import {ForkSeq, SLOTS_PER_EPOCH} from "@lodestar/params"; +import {ForkSeq, GENESIS_SLOT, SLOTS_PER_EPOCH} from "@lodestar/params"; import {GENESIS_EPOCH, ZERO_HASH} from "../constants/index.js"; import {IBeaconDb} from "../db/index.js"; @@ -430,7 +430,11 @@ export class BeaconChain implements IBeaconChain { {dontTransferCache: true}, RegenCaller.restApi ); - return {state, executionOptimistic: isOptimisticBlock(block), finalized: slot === finalizedBlock.slot}; + return { + state, + executionOptimistic: isOptimisticBlock(block), + finalized: slot !== GENESIS_SLOT && slot === finalizedBlock.slot, + }; } else { // Just check if state is already in the cache. If it's not dialed to the correct slot, // do not bother in advancing the state. restApiCanTriggerRegen == false means do no work @@ -440,7 +444,13 @@ export class BeaconChain implements IBeaconChain { } const state = this.regen.getStateSync(block.stateRoot); - return state && {state, executionOptimistic: isOptimisticBlock(block), finalized: slot === finalizedBlock.slot}; + return ( + state && { + state, + executionOptimistic: isOptimisticBlock(block), + finalized: slot !== GENESIS_SLOT && slot === finalizedBlock.slot, + } + ); } } else { // request for finalized state @@ -458,10 +468,11 @@ export class BeaconChain implements IBeaconChain { if (opts?.allowRegen) { const state = await this.regen.getState(stateRoot, RegenCaller.restApi); const block = this.forkChoice.getBlock(state.latestBlockHeader.hashTreeRoot()); + const stateEpoch = state.epochCtx.epoch; return { state, executionOptimistic: block != null && isOptimisticBlock(block), - finalized: state.epochCtx.epoch <= this.forkChoice.getFinalizedCheckpoint().epoch, + finalized: stateEpoch !== GENESIS_EPOCH && stateEpoch <= this.forkChoice.getFinalizedCheckpoint().epoch, }; } @@ -473,10 +484,11 @@ export class BeaconChain implements IBeaconChain { const cachedStateCtx = this.regen.getStateSync(stateRoot); if (cachedStateCtx) { const block = this.forkChoice.getBlock(cachedStateCtx.latestBlockHeader.hashTreeRoot()); + const stateEpoch = cachedStateCtx.epochCtx.epoch; return { state: cachedStateCtx, executionOptimistic: block != null && isOptimisticBlock(block), - finalized: cachedStateCtx.epochCtx.epoch <= this.forkChoice.getFinalizedCheckpoint().epoch, + finalized: stateEpoch !== GENESIS_EPOCH && stateEpoch <= this.forkChoice.getFinalizedCheckpoint().epoch, }; } @@ -491,10 +503,11 @@ export class BeaconChain implements IBeaconChain { const cachedStateCtx = this.regen.getCheckpointStateSync(checkpoint); if (cachedStateCtx) { const block = this.forkChoice.getBlock(cachedStateCtx.latestBlockHeader.hashTreeRoot()); + const stateEpoch = cachedStateCtx.epochCtx.epoch; return { state: cachedStateCtx, executionOptimistic: block != null && isOptimisticBlock(block), - finalized: cachedStateCtx.epochCtx.epoch <= this.forkChoice.getFinalizedCheckpoint().epoch, + finalized: stateEpoch !== GENESIS_EPOCH && stateEpoch <= this.forkChoice.getFinalizedCheckpoint().epoch, }; } diff --git a/packages/cli/test/sim/endpoints.test.ts b/packages/cli/test/sim/endpoints.test.ts index a40a18e379eb..9997778f802a 100644 --- a/packages/cli/test/sim/endpoints.test.ts +++ b/packages/cli/test/sim/endpoints.test.ts @@ -3,6 +3,7 @@ import path from "node:path"; import assert from "node:assert"; import {toHexString} from "@chainsafe/ssz"; import {routes} from "@lodestar/api"; +import {GENESIS_SLOT} from "@lodestar/params"; import {Simulation} from "../utils/crucible/simulation.js"; import {BeaconClient, ExecutionClient} from "../utils/crucible/interfaces.js"; import {defineSimTestConfig, logFilesDir} from "../utils/crucible/utils/index.js"; @@ -105,6 +106,15 @@ await env.tracker.assert( } ); +await env.tracker.assert( + "should return 'finalized' as 'false' when getStateValidator is called with genesis slot", + async () => { + const res = await node.api.beacon.getStateValidator({stateId: GENESIS_SLOT, validatorId: 0}); + + assert.equal(res.meta().finalized, false); + } +); + await env.tracker.assert("BN Not Synced", async () => { const expectedSyncStatus: routes.node.SyncingStatus = { headSlot: "2", From d7534fcd4e52eb2eba6d0e9b31fd30aa6fdba6cb Mon Sep 17 00:00:00 2001 From: Nico Flaig Date: Mon, 22 Jul 2024 15:52:13 +0100 Subject: [PATCH 2/2] Update genesis epoch / slot checks --- packages/beacon-node/src/chain/chain.ts | 16 ++++++++-------- packages/cli/test/sim/endpoints.test.ts | 10 ---------- 2 files changed, 8 insertions(+), 18 deletions(-) diff --git a/packages/beacon-node/src/chain/chain.ts b/packages/beacon-node/src/chain/chain.ts index f73f83197643..081de6f85063 100644 --- a/packages/beacon-node/src/chain/chain.ts +++ b/packages/beacon-node/src/chain/chain.ts @@ -433,7 +433,7 @@ export class BeaconChain implements IBeaconChain { return { state, executionOptimistic: isOptimisticBlock(block), - finalized: slot !== GENESIS_SLOT && slot === finalizedBlock.slot, + finalized: slot === finalizedBlock.slot && finalizedBlock.slot !== GENESIS_SLOT, }; } else { // Just check if state is already in the cache. If it's not dialed to the correct slot, @@ -448,7 +448,7 @@ export class BeaconChain implements IBeaconChain { state && { state, executionOptimistic: isOptimisticBlock(block), - finalized: slot !== GENESIS_SLOT && slot === finalizedBlock.slot, + finalized: slot === finalizedBlock.slot && finalizedBlock.slot !== GENESIS_SLOT, } ); } @@ -468,11 +468,11 @@ export class BeaconChain implements IBeaconChain { if (opts?.allowRegen) { const state = await this.regen.getState(stateRoot, RegenCaller.restApi); const block = this.forkChoice.getBlock(state.latestBlockHeader.hashTreeRoot()); - const stateEpoch = state.epochCtx.epoch; + const finalizedEpoch = this.forkChoice.getFinalizedCheckpoint().epoch; return { state, executionOptimistic: block != null && isOptimisticBlock(block), - finalized: stateEpoch !== GENESIS_EPOCH && stateEpoch <= this.forkChoice.getFinalizedCheckpoint().epoch, + finalized: state.epochCtx.epoch <= finalizedEpoch && finalizedEpoch !== GENESIS_EPOCH, }; } @@ -484,11 +484,11 @@ export class BeaconChain implements IBeaconChain { const cachedStateCtx = this.regen.getStateSync(stateRoot); if (cachedStateCtx) { const block = this.forkChoice.getBlock(cachedStateCtx.latestBlockHeader.hashTreeRoot()); - const stateEpoch = cachedStateCtx.epochCtx.epoch; + const finalizedEpoch = this.forkChoice.getFinalizedCheckpoint().epoch; return { state: cachedStateCtx, executionOptimistic: block != null && isOptimisticBlock(block), - finalized: stateEpoch !== GENESIS_EPOCH && stateEpoch <= this.forkChoice.getFinalizedCheckpoint().epoch, + finalized: cachedStateCtx.epochCtx.epoch <= finalizedEpoch && finalizedEpoch !== GENESIS_EPOCH, }; } @@ -503,11 +503,11 @@ export class BeaconChain implements IBeaconChain { const cachedStateCtx = this.regen.getCheckpointStateSync(checkpoint); if (cachedStateCtx) { const block = this.forkChoice.getBlock(cachedStateCtx.latestBlockHeader.hashTreeRoot()); - const stateEpoch = cachedStateCtx.epochCtx.epoch; + const finalizedEpoch = this.forkChoice.getFinalizedCheckpoint().epoch; return { state: cachedStateCtx, executionOptimistic: block != null && isOptimisticBlock(block), - finalized: stateEpoch !== GENESIS_EPOCH && stateEpoch <= this.forkChoice.getFinalizedCheckpoint().epoch, + finalized: cachedStateCtx.epochCtx.epoch <= finalizedEpoch && finalizedEpoch !== GENESIS_EPOCH, }; } diff --git a/packages/cli/test/sim/endpoints.test.ts b/packages/cli/test/sim/endpoints.test.ts index 9997778f802a..a40a18e379eb 100644 --- a/packages/cli/test/sim/endpoints.test.ts +++ b/packages/cli/test/sim/endpoints.test.ts @@ -3,7 +3,6 @@ import path from "node:path"; import assert from "node:assert"; import {toHexString} from "@chainsafe/ssz"; import {routes} from "@lodestar/api"; -import {GENESIS_SLOT} from "@lodestar/params"; import {Simulation} from "../utils/crucible/simulation.js"; import {BeaconClient, ExecutionClient} from "../utils/crucible/interfaces.js"; import {defineSimTestConfig, logFilesDir} from "../utils/crucible/utils/index.js"; @@ -106,15 +105,6 @@ await env.tracker.assert( } ); -await env.tracker.assert( - "should return 'finalized' as 'false' when getStateValidator is called with genesis slot", - async () => { - const res = await node.api.beacon.getStateValidator({stateId: GENESIS_SLOT, validatorId: 0}); - - assert.equal(res.meta().finalized, false); - } -); - await env.tracker.assert("BN Not Synced", async () => { const expectedSyncStatus: routes.node.SyncingStatus = { headSlot: "2",