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

feat: remove unfinalized pubkey cache #7230

Open
wants to merge 3 commits into
base: unstable
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
22 changes: 0 additions & 22 deletions packages/beacon-node/src/chain/chain.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1047,9 +1047,6 @@ export class BeaconChain implements IBeaconChain {
metrics.forkChoice.balancesLength.set(forkChoiceMetrics.balancesLength);
metrics.forkChoice.nodes.set(forkChoiceMetrics.nodes);
metrics.forkChoice.indices.set(forkChoiceMetrics.indices);

const headState = this.getHeadState();
metrics.headState.unfinalizedPubkeyCacheSize.set(headState.epochCtx.unfinalizedPubkey2index.size);
}

private onClockSlot(slot: Slot): void {
Expand Down Expand Up @@ -1139,27 +1136,8 @@ export class BeaconChain implements IBeaconChain {
this.opPool.pruneAll(headBlock, headState);
}

const cpEpoch = cp.epoch;

if (headState === null) {
this.logger.verbose("Head state is null");
} else if (cpEpoch >= this.config.ELECTRA_FORK_EPOCH) {
// Get the validator.length from the state at cpEpoch
// We are confident the last element in the list is from headEpoch
// Thus we query from the end of the list. (cpEpoch - headEpoch - 1) is negative number
const pivotValidatorIndex = headState.epochCtx.getValidatorCountAtEpoch(cpEpoch);

if (pivotValidatorIndex !== undefined) {
// Note EIP-6914 will break this logic
const newFinalizedValidators = headState.epochCtx.unfinalizedPubkey2index.filter(
(index, _pubkey) => index < pivotValidatorIndex
);

// Populate finalized pubkey cache and remove unfinalized pubkey cache
if (!newFinalizedValidators.isEmpty()) {
this.regen.updateUnfinalizedPubkeys(newFinalizedValidators);
}
}
}

// TODO-Electra: Deprecating eth1Data poll requires a check on a finalized checkpoint state.
Expand Down
50 changes: 1 addition & 49 deletions packages/beacon-node/src/chain/regen/queued.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import {routes} from "@lodestar/api";
import {IForkChoice, ProtoBlock} from "@lodestar/fork-choice";
import {CachedBeaconStateAllForks, UnfinalizedPubkeyIndexMap, computeEpochAtSlot} from "@lodestar/state-transition";
import {CachedBeaconStateAllForks, computeEpochAtSlot} from "@lodestar/state-transition";
import {BeaconBlock, Epoch, RootHex, Slot, phase0} from "@lodestar/types";
import {Logger, toRootHex} from "@lodestar/utils";
import {Metrics} from "../../metrics/index.js";
Expand Down Expand Up @@ -206,54 +206,6 @@ export class QueuedStateRegenerator implements IStateRegenerator {
return this.checkpointStateCache.updatePreComputedCheckpoint(rootHex, epoch);
}

/**
* Remove `validators` from all unfinalized cache's epochCtx.UnfinalizedPubkey2Index,
* and add them to epochCtx.pubkey2index and epochCtx.index2pubkey
*/
updateUnfinalizedPubkeys(validators: UnfinalizedPubkeyIndexMap): void {
let numStatesUpdated = 0;
const states = this.blockStateCache.getStates();
const cpStates = this.checkpointStateCache.getStates();

// Add finalized pubkeys to all states.
const addTimer = this.metrics?.regenFnAddPubkeyTime.startTimer();

// We only need to add pubkeys to any one of the states since the finalized caches is shared globally across all states
const firstState = (states.next().value ?? cpStates.next().value) as CachedBeaconStateAllForks | undefined;

if (firstState !== undefined) {
firstState.epochCtx.addFinalizedPubkeys(validators, this.metrics?.epochCache ?? undefined);
} else {
this.logger.warn("Attempt to delete finalized pubkey from unfinalized pubkey cache. But no state is available");
}

addTimer?.();

// Delete finalized pubkeys from unfinalized pubkey cache for all states
const deleteTimer = this.metrics?.regenFnDeletePubkeyTime.startTimer();
const pubkeysToDelete = Array.from(validators.keys());

for (const s of states) {
s.epochCtx.deleteUnfinalizedPubkeys(pubkeysToDelete);
numStatesUpdated++;
}

for (const s of cpStates) {
s.epochCtx.deleteUnfinalizedPubkeys(pubkeysToDelete);
numStatesUpdated++;
}

// Since first state is consumed from the iterator. Will need to perform delete explicitly
if (firstState !== undefined) {
firstState?.epochCtx.deleteUnfinalizedPubkeys(pubkeysToDelete);
numStatesUpdated++;
}

deleteTimer?.();

this.metrics?.regenFnNumStatesUpdated.observe(numStatesUpdated);
}

/**
* Get the state to run with `block`.
* - State after `block.parentRoot` dialed forward to block.slot
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ export class BlockStateCacheImpl implements BlockStateCache {
this.maxStates = maxStates;
this.cache = new MapTracker(metrics?.stateCache);
if (metrics) {
this.metrics = {...metrics.stateCache, ...metrics.epochCache};
this.metrics = metrics.stateCache;
metrics.stateCache.size.addCollect(() => metrics.stateCache.size.set(this.cache.size));
}
}
Expand Down
7 changes: 0 additions & 7 deletions packages/beacon-node/src/metrics/metrics/beacon.ts
Original file line number Diff line number Diff line change
Expand Up @@ -124,13 +124,6 @@ export function createBeaconMetrics(register: RegistryMetricCreator) {
}),
},

headState: {
unfinalizedPubkeyCacheSize: register.gauge({
name: "beacon_head_state_unfinalized_pubkey_cache_size",
help: "Current size of the unfinalizedPubkey2Index cache in the head state",
}),
},

parentBlockDistance: register.histogram({
name: "beacon_imported_block_parent_distance",
help: "Histogram of distance to parent block of valid imported blocks",
Expand Down
11 changes: 0 additions & 11 deletions packages/beacon-node/src/metrics/metrics/lodestar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -378,17 +378,6 @@ export function createLodestarMetrics(
help: "Total count state.validators nodesPopulated is false on stfn for post state",
}),

epochCache: {
finalizedPubkeyDuplicateInsert: register.gauge({
name: "lodestar_epoch_cache_finalized_pubkey_duplicate_insert_total",
help: "Total count of duplicate insert of finalized pubkeys",
}),
newUnFinalizedPubkey: register.gauge({
name: "lodestar_epoch_cache_new_unfinalized_pubkey_total",
help: "Total count of unfinalized pubkeys added",
}),
},

// BLS verifier thread pool and queue

bls: {
Expand Down
54 changes: 0 additions & 54 deletions packages/beacon-node/test/memory/unfinalizedPubkey2Index.ts

This file was deleted.

This file was deleted.

31 changes: 3 additions & 28 deletions packages/beacon-node/test/sim/electra-interop.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -375,17 +375,8 @@ describe("executionEngine / ExecutionEngineHttp", () => {
if (headState.validators.length !== 33 || headState.balances.length !== 33) {
throw Error("New validator is not reflected in the beacon state at slot 5");
}
if (epochCtx.index2pubkey.length !== 32 || epochCtx.pubkey2index.size !== 32) {
throw Error("Finalized cache is modified.");
}
if (epochCtx.unfinalizedPubkey2index.size !== 1) {
throw Error(
`Unfinalized cache is missing the expected validator. Size: ${epochCtx.unfinalizedPubkey2index.size}`
);
}
// validator count at epoch 1 should be empty at this point since no epoch transition has happened.
if (epochCtx.getValidatorCountAtEpoch(1) !== undefined) {
throw Error("Historical validator lengths is modified");
if (epochCtx.index2pubkey.length !== 33 || epochCtx.pubkey2index.size !== 33) {
throw Error("Pubkey cache is not updated");
}

await new Promise<void>((resolve, _reject) => {
Expand All @@ -412,23 +403,7 @@ describe("executionEngine / ExecutionEngineHttp", () => {
throw Error("New validator is not reflected in the beacon state.");
}
if (epochCtx.index2pubkey.length !== 33 || epochCtx.pubkey2index.size !== 33) {
throw Error("New validator is not in finalized cache");
}
if (!epochCtx.unfinalizedPubkey2index.isEmpty()) {
throw Error("Unfinalized cache still contains new validator");
}
// After 4 epochs, headState's finalized cp epoch should be 2
// epochCtx should only have validator count for epoch 3 and 4.
if (epochCtx.getValidatorCountAtEpoch(4) === undefined || epochCtx.getValidatorCountAtEpoch(3) === undefined) {
throw Error("Missing historical validator length for epoch 3 or 4");
}

if (epochCtx.getValidatorCountAtEpoch(4) !== 33 || epochCtx.getValidatorCountAtEpoch(3) !== 33) {
throw Error("Incorrect historical validator length for epoch 3 or 4");
}

if (epochCtx.getValidatorCountAtEpoch(2) !== undefined || epochCtx.getValidatorCountAtEpoch(1) !== undefined) {
throw Error("Historical validator length for epoch 1 or 2 is not dropped properly");
throw Error("New validator is not in pubkey cache");
}

if (headState.depositRequestsStartIndex === UNSET_DEPOSIT_REQUESTS_START_INDEX) {
Expand Down
3 changes: 1 addition & 2 deletions packages/state-transition/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,7 @@
"@lodestar/params": "^1.23.0",
"@lodestar/types": "^1.23.0",
"@lodestar/utils": "^1.23.0",
"bigint-buffer": "^1.1.5",
"immutable": "^4.3.2"
"bigint-buffer": "^1.1.5"
},
"keywords": [
"ethereum",
Expand Down
Loading
Loading