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.

Sign up for GitHub

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

Jump to bottom

Only call regen.getBlockSlotState() if necessary #5401

Merged
merged 1 commit into from
Apr 23, 2023
+35 −20
Merged

Only call regen.getBlockSlotState() if necessary #5401

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
16 changes: 3 additions & 13 deletions packages/beacon-node/src/api/impl/validator/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,6 @@ export function getValidatorApi({
const headState = chain.getHeadState();
const headSlot = headState.slot;
const attEpoch = computeEpochAtSlot(slot);
const headEpoch = computeEpochAtSlot(headSlot);
const headBlockRootHex = chain.forkChoice.getHead().blockRoot;
const headBlockRoot = fromHexString(headBlockRootHex);

Expand All @@ -304,16 +303,7 @@ export function getValidatorApi({

// To get the correct source we must get a state in the same epoch as the attestation's epoch.
// An epoch transition may change state.currentJustifiedCheckpoint
const attEpochState =
attEpoch <= headEpoch
? headState
: // Will advance the state to the correct next epoch if necessary
await chain.regen.getBlockSlotState(
headBlockRootHex,
slot,
{dontTransferCache: true},
RegenCaller.produceAttestationData
);
const attEpochState = await chain.getHeadStateAtEpoch(attEpoch, RegenCaller.produceAttestationData);

return {
data: {
Expand Down Expand Up @@ -367,7 +357,7 @@ export function getValidatorApi({
await waitForNextClosestEpoch();

const head = chain.forkChoice.getHead();
const state = await chain.getHeadStateAtCurrentEpoch();
const state = await chain.getHeadStateAtCurrentEpoch(RegenCaller.getDuties);

const stateEpoch = state.epochCtx.epoch;
let indexes: ValidatorIndex[] = [];
Expand Down Expand Up @@ -423,7 +413,7 @@ export function getValidatorApi({
}

const head = chain.forkChoice.getHead();
const state = await chain.getHeadStateAtCurrentEpoch();
const state = await chain.getHeadStateAtCurrentEpoch(RegenCaller.getDuties);

// TODO: Determine what the current epoch would be if we fast-forward our system clock by
// `MAXIMUM_GOSSIP_CLOCK_DISPARITY`.
Expand Down
19 changes: 15 additions & 4 deletions packages/beacon-node/src/chain/chain.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -333,11 +333,22 @@ export class BeaconChain implements IBeaconChain {
return headState;
}

async getHeadStateAtCurrentEpoch(): Promise<CachedBeaconStateAllForks> {
const currentEpochStartSlot = computeStartSlotAtEpoch(this.clock.currentEpoch);
async getHeadStateAtCurrentEpoch(regenCaller: RegenCaller): Promise<CachedBeaconStateAllForks> {
return this.getHeadStateAtEpoch(this.clock.currentEpoch, regenCaller);
}

async getHeadStateAtEpoch(epoch: Epoch, regenCaller: RegenCaller): Promise<CachedBeaconStateAllForks> {
// using getHeadState() means we'll use checkpointStateCache if it's available
const headState = this.getHeadState();
// head state is in the same epoch, or we pulled up head state already from past epoch
if (epoch <= computeEpochAtSlot(headState.slot)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tuyennhv shouldn't this be a strict equals check? I noticed this method always returns the same state if a previous epoch is passed in

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nflaig no this should be "<=", the function means to get head state dialing to an epoch. If provided epoch < head state epoch just return head state

also if we let it pass through the below regen.getBlockSlotState call, if provided slot < head state slot it'll throw error

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes sense, I was trying to figure out what would be required to get proposer duties of a finalized epoch. The right method to use in that case would be getStateBySlot I'd assume.

lodestar/packages/beacon-node/src/chain/chain.ts

Line 362 in f77b774

async getStateBySlot(

You also noted in #5846 is that Lodestar does not persists finalized states very frequently which would be an issue in that case. We could solve this issue if we add --chain.archiveStateEpochFrequency 1 flag but this significantly increases storage requirements (but that might be fine for someone that really needs that data).

// should go to this most of the time
return headState;
}
// only use regen queue if necessary, it'll cache in checkpointStateCache if regen gets through epoch transition
const head = this.forkChoice.getHead();
const bestSlot = currentEpochStartSlot > head.slot ? currentEpochStartSlot : head.slot;
return this.regen.getBlockSlotState(head.blockRoot, bestSlot, {dontTransferCache: true}, RegenCaller.getDuties);
const startSlot = computeStartSlotAtEpoch(epoch);
return this.regen.getBlockSlotState(head.blockRoot, startSlot, {dontTransferCache: true}, regenCaller);
}

async getCanonicalBlockAtSlot(slot: Slot): Promise<allForks.SignedBeaconBlock | null> {
Expand Down
5 changes: 3 additions & 2 deletions packages/beacon-node/src/chain/interface.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {IExecutionEngine, IExecutionBuilder} from "../execution/index.js";
import {Metrics} from "../metrics/metrics.js";
import {BeaconClock} from "./clock/interface.js";
import {ChainEventEmitter} from "./emitter.js";
import {IStateRegenerator} from "./regen/index.js";
import {IStateRegenerator, RegenCaller} from "./regen/index.js";
import {StateContextCache, CheckpointStateCache} from "./stateCache/index.js";
import {IBlsVerifier} from "./bls/index.js";
import {
Expand Down Expand Up @@ -105,7 +105,8 @@ export interface IBeaconChain {
validatorSeenAtEpoch(index: ValidatorIndex, epoch: Epoch): boolean;

getHeadState(): CachedBeaconStateAllForks;
getHeadStateAtCurrentEpoch(): Promise<CachedBeaconStateAllForks>;
getHeadStateAtCurrentEpoch(regenCaller: RegenCaller): Promise<CachedBeaconStateAllForks>;
getHeadStateAtEpoch(epoch: Epoch, regenCaller: RegenCaller): Promise<CachedBeaconStateAllForks>;

/**
* Since we can have multiple parallel chains,
Expand Down
1 change: 1 addition & 0 deletions packages/beacon-node/src/chain/regen/interface.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ export enum RegenCaller {
processBlocksInEpoch = "processBlocksInEpoch",
validateGossipAggregateAndProof = "validateGossipAggregateAndProof",
validateGossipAttestation = "validateGossipAttestation",
validateGossipVoluntaryExit = "validateGossipVoluntaryExit",
onForkChoiceFinalized = "onForkChoiceFinalized",
}

Expand Down
6 changes: 6 additions & 0 deletions packages/beacon-node/src/chain/regen/queued.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,12 @@ export class QueuedStateRegenerator implements IStateRegenerator {
return this.jobQueue.push({key: "getCheckpointState", args: [cp, opts, rCaller]});
}

/**
* Get state of provided `blockRoot` and dial forward to `slot`
* Use this api with care because we don't want the queue to be busy
* For the context, gossip block validation uses this api so we want it to be as fast as possible
* @returns
*/
async getBlockSlotState(
blockRoot: RootHex,
slot: Slot,
Expand Down
3 changes: 2 additions & 1 deletion packages/beacon-node/src/chain/validation/voluntaryExit.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import {phase0} from "@lodestar/types";
import {isValidVoluntaryExit, getVoluntaryExitSignatureSet} from "@lodestar/state-transition";
import {IBeaconChain} from "..";
import {VoluntaryExitError, VoluntaryExitErrorCode, GossipAction} from "../errors/index.js";
import {RegenCaller} from "../regen/index.js";

export async function validateGossipVoluntaryExit(
chain: IBeaconChain,
Expand All @@ -22,7 +23,7 @@ export async function validateGossipVoluntaryExit(
// The voluntaryExit.epoch must be in the past but the validator's status may change in recent epochs.
// We dial the head state to the current epoch to get the current status of the validator. This is
// relevant on periods of many skipped slots.
const state = await chain.getHeadStateAtCurrentEpoch();
const state = await chain.getHeadStateAtCurrentEpoch(RegenCaller.validateGossipVoluntaryExit);

// [REJECT] All of the conditions within process_voluntary_exit pass validation.
// verifySignature = false, verified in batch below
Expand Down
5 changes: 5 additions & 0 deletions packages/beacon-node/test/utils/mocks/chain/chain.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,7 @@ export class MockBeaconChain implements IBeaconChain {
this.pubkey2index = new PubkeyIndexMap();
this.index2pubkey = [];
}

executionBuilder?: IExecutionBuilder | undefined;

validatorSeenAtEpoch(): boolean {
Expand All @@ -181,6 +182,10 @@ export class MockBeaconChain implements IBeaconChain {
return this.state;
}

async getHeadStateAtEpoch(): Promise<CachedBeaconStateAllForks> {
return this.state;
}

async getCanonicalBlockAtSlot(): Promise<allForks.SignedBeaconBlock> {
throw Error("Not implemented");
}
Expand Down