From 4ed80d5102d93bae94d3fa592938e7652a2ea53e Mon Sep 17 00:00:00 2001 From: matthewkeil Date: Tue, 20 Feb 2024 13:39:00 +0800 Subject: [PATCH 01/23] feat: move epoch off of EpochShuffling and implement on EpochCache --- .../src/chain/blocks/importBlock.ts | 2 +- packages/beacon-node/src/chain/chain.ts | 6 +- .../beacon-node/src/chain/shufflingCache.ts | 6 +- .../test/unit/chain/shufflingCache.test.ts | 2 +- .../stateCache/fifoBlockStateCache.test.ts | 7 +-- .../stateCache/stateContextCache.test.ts | 7 +-- ...hufflingForAttestationVerification.test.ts | 6 +- .../test/utils/validationData/attestation.ts | 6 +- .../src/block/processAttestationPhase0.ts | 2 +- .../state-transition/src/cache/epochCache.ts | 57 ++++++++++++------- .../src/cache/epochTransitionCache.ts | 2 +- .../src/epoch/processPendingAttestations.ts | 2 +- .../src/util/epochShuffling.ts | 6 -- packages/state-transition/src/util/seed.ts | 7 ++- .../test/perf/util/shufflings.test.ts | 9 ++- .../test/unit/util/balance.test.ts | 2 +- 16 files changed, 71 insertions(+), 58 deletions(-) diff --git a/packages/beacon-node/src/chain/blocks/importBlock.ts b/packages/beacon-node/src/chain/blocks/importBlock.ts index d82448a4e932..9c9a2e811deb 100644 --- a/packages/beacon-node/src/chain/blocks/importBlock.ts +++ b/packages/beacon-node/src/chain/blocks/importBlock.ts @@ -351,7 +351,7 @@ export async function importBlock( if (parentEpoch < blockEpoch) { // current epoch and previous epoch are likely cached in previous states - this.shufflingCache.processState(postState, postState.epochCtx.nextShuffling.epoch); + this.shufflingCache.processState(postState, postState.epochCtx.nextEpoch); this.logger.verbose("Processed shuffling for next epoch", {parentEpoch, blockEpoch, slot: blockSlot}); } diff --git a/packages/beacon-node/src/chain/chain.ts b/packages/beacon-node/src/chain/chain.ts index 08743165cd05..abbea14e6098 100644 --- a/packages/beacon-node/src/chain/chain.ts +++ b/packages/beacon-node/src/chain/chain.ts @@ -231,9 +231,9 @@ export class BeaconChain implements IBeaconChain { pubkey2index: new PubkeyIndexMap(), index2pubkey: [], }); - this.shufflingCache.processState(cachedState, cachedState.epochCtx.previousShuffling.epoch); - this.shufflingCache.processState(cachedState, cachedState.epochCtx.currentShuffling.epoch); - this.shufflingCache.processState(cachedState, cachedState.epochCtx.nextShuffling.epoch); + this.shufflingCache.processState(cachedState, cachedState.epochCtx.previousEpoch); + this.shufflingCache.processState(cachedState, cachedState.epochCtx.epoch); + this.shufflingCache.processState(cachedState, cachedState.epochCtx.nextEpoch); // Persist single global instance of state caches this.pubkey2index = cachedState.epochCtx.pubkey2index; diff --git a/packages/beacon-node/src/chain/shufflingCache.ts b/packages/beacon-node/src/chain/shufflingCache.ts index 23177142d846..751b92b1f2cf 100644 --- a/packages/beacon-node/src/chain/shufflingCache.ts +++ b/packages/beacon-node/src/chain/shufflingCache.ts @@ -78,13 +78,13 @@ export class ShufflingCache { const decisionBlockHex = getDecisionBlock(state, shufflingEpoch); let shuffling: EpochShuffling; switch (shufflingEpoch) { - case state.epochCtx.nextShuffling.epoch: + case state.epochCtx.nextEpoch: shuffling = state.epochCtx.nextShuffling; break; - case state.epochCtx.currentShuffling.epoch: + case state.epochCtx.epoch: shuffling = state.epochCtx.currentShuffling; break; - case state.epochCtx.previousShuffling.epoch: + case state.epochCtx.previousEpoch: shuffling = state.epochCtx.previousShuffling; break; default: diff --git a/packages/beacon-node/test/unit/chain/shufflingCache.test.ts b/packages/beacon-node/test/unit/chain/shufflingCache.test.ts index 6295a993c072..60cfb8a8691a 100644 --- a/packages/beacon-node/test/unit/chain/shufflingCache.test.ts +++ b/packages/beacon-node/test/unit/chain/shufflingCache.test.ts @@ -9,7 +9,7 @@ describe("ShufflingCache", function () { const vc = 64; const stateSlot = 100; const state = generateTestCachedBeaconStateOnlyValidators({vc, slot: stateSlot}); - const currentEpoch = state.epochCtx.currentShuffling.epoch; + const currentEpoch = state.epochCtx.epoch; let shufflingCache: ShufflingCache; beforeEach(() => { diff --git a/packages/beacon-node/test/unit/chain/stateCache/fifoBlockStateCache.test.ts b/packages/beacon-node/test/unit/chain/stateCache/fifoBlockStateCache.test.ts index 994cf3f7c085..1f4f407a4be4 100644 --- a/packages/beacon-node/test/unit/chain/stateCache/fifoBlockStateCache.test.ts +++ b/packages/beacon-node/test/unit/chain/stateCache/fifoBlockStateCache.test.ts @@ -9,7 +9,6 @@ import {generateCachedState} from "../../../utils/state.js"; describe("FIFOBlockStateCache", function () { let cache: FIFOBlockStateCache; const shuffling: EpochShuffling = { - epoch: 0, activeIndices: new Uint32Array(), shuffling: new Uint32Array(), committees: [], @@ -18,15 +17,15 @@ describe("FIFOBlockStateCache", function () { const state1 = generateCachedState({slot: 0}); const key1 = toHexString(state1.hashTreeRoot()); - state1.epochCtx.currentShuffling = {...shuffling, epoch: 0}; + state1.epochCtx.currentShuffling = {...shuffling}; const state2 = generateCachedState({slot: 1 * SLOTS_PER_EPOCH}); const key2 = toHexString(state2.hashTreeRoot()); - state2.epochCtx.currentShuffling = {...shuffling, epoch: 1}; + state2.epochCtx.currentShuffling = {...shuffling}; const state3 = generateCachedState({slot: 2 * SLOTS_PER_EPOCH}); const key3 = toHexString(state3.hashTreeRoot()); - state3.epochCtx.currentShuffling = {...shuffling, epoch: 2}; + state3.epochCtx.currentShuffling = {...shuffling}; beforeEach(function () { // max 2 items diff --git a/packages/beacon-node/test/unit/chain/stateCache/stateContextCache.test.ts b/packages/beacon-node/test/unit/chain/stateCache/stateContextCache.test.ts index cca4d7ea7734..04476f30f054 100644 --- a/packages/beacon-node/test/unit/chain/stateCache/stateContextCache.test.ts +++ b/packages/beacon-node/test/unit/chain/stateCache/stateContextCache.test.ts @@ -11,7 +11,6 @@ describe("StateContextCache", function () { let cache: StateContextCache; let key1: Root, key2: Root; const shuffling: EpochShuffling = { - epoch: 0, activeIndices: new Uint32Array(), shuffling: new Uint32Array(), committees: [], @@ -23,18 +22,18 @@ describe("StateContextCache", function () { cache = new StateContextCache({maxStates: 2}); const state1 = generateCachedState({slot: 0}); key1 = state1.hashTreeRoot(); - state1.epochCtx.currentShuffling = {...shuffling, epoch: 0}; + state1.epochCtx.currentShuffling = {...shuffling}; cache.add(state1); const state2 = generateCachedState({slot: 1 * SLOTS_PER_EPOCH}); key2 = state2.hashTreeRoot(); - state2.epochCtx.currentShuffling = {...shuffling, epoch: 1}; + state2.epochCtx.currentShuffling = {...shuffling}; cache.add(state2); }); it("should prune", function () { expect(cache.size).toBe(2); const state3 = generateCachedState({slot: 2 * SLOTS_PER_EPOCH}); - state3.epochCtx.currentShuffling = {...shuffling, epoch: 2}; + state3.epochCtx.currentShuffling = {...shuffling}; cache.add(state3); expect(cache.size).toBe(3); diff --git a/packages/beacon-node/test/unit/chain/validation/attestation/getShufflingForAttestationVerification.test.ts b/packages/beacon-node/test/unit/chain/validation/attestation/getShufflingForAttestationVerification.test.ts index a0eb147db8e8..9f81c93cfbfc 100644 --- a/packages/beacon-node/test/unit/chain/validation/attestation/getShufflingForAttestationVerification.test.ts +++ b/packages/beacon-node/test/unit/chain/validation/attestation/getShufflingForAttestationVerification.test.ts @@ -45,7 +45,7 @@ describe("getShufflingForAttestationVerification", () => { throw new Error("Unexpected input"); } }); - const expectedShuffling = {epoch: attEpoch} as EpochShuffling; + const expectedShuffling = {epoch: attEpoch} as unknown as EpochShuffling; shufflingCacheStub.get.mockImplementationOnce((epoch, root) => { if (epoch === attEpoch && root === previousDependentRoot) { return Promise.resolve(expectedShuffling); @@ -77,7 +77,7 @@ describe("getShufflingForAttestationVerification", () => { throw new Error("Unexpected input"); } }); - const expectedShuffling = {epoch: attEpoch} as EpochShuffling; + const expectedShuffling = {epoch: attEpoch} as unknown as EpochShuffling; shufflingCacheStub.get.mockImplementationOnce((epoch, root) => { if (epoch === attEpoch && root === currentDependentRoot) { return Promise.resolve(expectedShuffling); @@ -101,7 +101,7 @@ describe("getShufflingForAttestationVerification", () => { stateRoot: ZERO_HASH_HEX, blockRoot, } as Partial; - const expectedShuffling = {epoch: attEpoch} as EpochShuffling; + const expectedShuffling = {epoch: attEpoch} as unknown as EpochShuffling; let callCount = 0; shufflingCacheStub.get.mockImplementationOnce((epoch, root) => { if (epoch === attEpoch && root === blockRoot) { diff --git a/packages/beacon-node/test/utils/validationData/attestation.ts b/packages/beacon-node/test/utils/validationData/attestation.ts index fa3c4d479ade..48ba91c9cdd6 100644 --- a/packages/beacon-node/test/utils/validationData/attestation.ts +++ b/packages/beacon-node/test/utils/validationData/attestation.ts @@ -79,9 +79,9 @@ export function getAttestationValidData(opts: AttestationValidDataOpts): { }; const shufflingCache = new ShufflingCache(); - shufflingCache.processState(state, state.epochCtx.currentShuffling.epoch); - shufflingCache.processState(state, state.epochCtx.nextShuffling.epoch); - const dependentRoot = getShufflingDecisionBlock(state, state.epochCtx.currentShuffling.epoch); + shufflingCache.processState(state, state.epochCtx.epoch); + shufflingCache.processState(state, state.epochCtx.nextEpoch); + const dependentRoot = getShufflingDecisionBlock(state, state.epochCtx.epoch); const forkChoice = { getBlock: (root) => { diff --git a/packages/state-transition/src/block/processAttestationPhase0.ts b/packages/state-transition/src/block/processAttestationPhase0.ts index 248ba83b4ed2..455bef8b0677 100644 --- a/packages/state-transition/src/block/processAttestationPhase0.ts +++ b/packages/state-transition/src/block/processAttestationPhase0.ts @@ -72,7 +72,7 @@ export function validateAttestation( `committeeIndex=${data.index} committeeCount=${committeeCount}` ); } - if (!(data.target.epoch === epochCtx.previousShuffling.epoch || data.target.epoch === epochCtx.epoch)) { + if (!(data.target.epoch === epochCtx.previousEpoch || data.target.epoch === epochCtx.epoch)) { throw new Error( "Attestation target epoch not in previous or current epoch: " + `targetEpoch=${data.target.epoch} currentEpoch=${epochCtx.epoch}` diff --git a/packages/state-transition/src/cache/epochCache.ts b/packages/state-transition/src/cache/epochCache.ts index 9565898eb09d..08081ee2dcc0 100644 --- a/packages/state-transition/src/cache/epochCache.ts +++ b/packages/state-transition/src/cache/epochCache.ts @@ -197,8 +197,15 @@ export class EpochCache { nextSyncCommitteeIndexed: SyncCommitteeCache; // TODO: Helper stats - epoch: Epoch; syncPeriod: SyncPeriod; + epoch: Epoch; + + get previousEpoch(): Epoch { + return this.epoch - 1; + } + get nextEpoch(): Epoch { + return this.epoch + 1; + } constructor(data: { config: BeaconConfig; @@ -350,7 +357,12 @@ export class EpochCache { // Allow to create CachedBeaconState for empty states, or no active validators const proposers = currentShuffling.activeIndices.length > 0 - ? computeProposers(currentProposerSeed, currentShuffling, effectiveBalanceIncrements) + ? computeProposers( + currentEpoch, + currentProposerSeed, + currentShuffling.activeIndices, + effectiveBalanceIncrements + ) : []; const proposersNextEpoch: ProposersDeferred = { @@ -507,23 +519,27 @@ export class EpochCache { ): void { this.previousShuffling = this.currentShuffling; this.currentShuffling = this.nextShuffling; - const currEpoch = this.currentShuffling.epoch; - const nextEpoch = currEpoch + 1; + this.epoch += 1; this.nextShuffling = computeEpochShuffling( state, epochTransitionCache.nextEpochShufflingActiveValidatorIndices, - nextEpoch + this.nextEpoch ); // Roll current proposers into previous proposers for metrics this.proposersPrevEpoch = this.proposers; - const currentProposerSeed = getSeed(state, this.currentShuffling.epoch, DOMAIN_BEACON_PROPOSER); - this.proposers = computeProposers(currentProposerSeed, this.currentShuffling, this.effectiveBalanceIncrements); + const currentProposerSeed = getSeed(state, this.epoch, DOMAIN_BEACON_PROPOSER); + this.proposers = computeProposers( + this.epoch, + currentProposerSeed, + this.currentShuffling.activeIndices, + this.effectiveBalanceIncrements + ); // Only pre-compute the seed since it's very cheap. Do the expensive computeProposers() call only on demand. - this.proposersNextEpoch = {computed: false, seed: getSeed(state, this.nextShuffling.epoch, DOMAIN_BEACON_PROPOSER)}; + this.proposersNextEpoch = {computed: false, seed: getSeed(state, this.nextEpoch, DOMAIN_BEACON_PROPOSER)}; // TODO: DEDUPLICATE from createEpochCache // @@ -549,14 +565,14 @@ export class EpochCache { ); // Maybe advance exitQueueEpoch at the end of the epoch if there haven't been any exists for a while - const exitQueueEpoch = computeActivationExitEpoch(currEpoch); + const exitQueueEpoch = computeActivationExitEpoch(this.epoch); if (exitQueueEpoch > this.exitQueueEpoch) { this.exitQueueEpoch = exitQueueEpoch; this.exitQueueChurn = 0; } this.totalActiveBalanceIncrements = epochTransitionCache.nextEpochTotalActiveBalanceByIncrement; - if (currEpoch >= this.config.ALTAIR_FORK_EPOCH) { + if (this.epoch >= this.config.ALTAIR_FORK_EPOCH) { this.syncParticipantReward = computeSyncParticipantReward(this.totalActiveBalanceIncrements); this.syncProposerReward = Math.floor(this.syncParticipantReward * PROPOSER_WEIGHT_FACTOR); this.baseRewardPerIncrement = computeBaseRewardPerIncrement(this.totalActiveBalanceIncrements); @@ -612,10 +628,10 @@ export class EpochCache { getBeaconProposer(slot: Slot): ValidatorIndex { const epoch = computeEpochAtSlot(slot); - if (epoch !== this.currentShuffling.epoch) { + if (epoch !== this.epoch) { throw new EpochCacheError({ code: EpochCacheErrorCode.PROPOSER_EPOCH_MISMATCH, - currentEpoch: this.currentShuffling.epoch, + currentEpoch: this.epoch, requestedEpoch: epoch, }); } @@ -666,8 +682,9 @@ export class EpochCache { getBeaconProposersNextEpoch(): ValidatorIndex[] { if (!this.proposersNextEpoch.computed) { const indexes = computeProposers( + this.nextEpoch, this.proposersNextEpoch.seed, - this.nextShuffling, + this.nextShuffling.activeIndices, this.effectiveBalanceIncrements ); this.proposersNextEpoch = {computed: true, indexes}; @@ -732,10 +749,8 @@ export class EpochCache { * Return null if no assignment.. */ getCommitteeAssignment(epoch: Epoch, validatorIndex: ValidatorIndex): phase0.CommitteeAssignment | null { - if (epoch > this.currentShuffling.epoch + 1) { - throw Error( - `Requesting committee assignment for more than 1 epoch ahead: ${epoch} > ${this.currentShuffling.epoch} + 1` - ); + if (epoch > this.epoch + 1) { + throw Error(`Requesting committee assignment for more than 1 epoch ahead: ${epoch} > ${this.epoch} + 1`); } const epochStartSlot = computeStartSlotAtEpoch(epoch); @@ -780,7 +795,7 @@ export class EpochCache { if (shuffling === null) { throw new EpochCacheError({ code: EpochCacheErrorCode.COMMITTEE_EPOCH_OUT_OF_RANGE, - currentEpoch: this.currentShuffling.epoch, + currentEpoch: this.epoch, requestedEpoch: epoch, }); } @@ -789,11 +804,11 @@ export class EpochCache { } getShufflingAtEpochOrNull(epoch: Epoch): EpochShuffling | null { - if (epoch === this.previousShuffling.epoch) { + if (epoch === this.previousEpoch) { return this.previousShuffling; - } else if (epoch === this.currentShuffling.epoch) { + } else if (epoch === this.epoch) { return this.currentShuffling; - } else if (epoch === this.nextShuffling.epoch) { + } else if (epoch === this.nextEpoch) { return this.nextShuffling; } else { return null; diff --git a/packages/state-transition/src/cache/epochTransitionCache.ts b/packages/state-transition/src/cache/epochTransitionCache.ts index dc4edf26e084..91476c735a9b 100644 --- a/packages/state-transition/src/cache/epochTransitionCache.ts +++ b/packages/state-transition/src/cache/epochTransitionCache.ts @@ -175,7 +175,7 @@ export function beforeProcessEpoch( const {config, epochCtx} = state; const forkSeq = config.getForkSeq(state.slot); const currentEpoch = epochCtx.epoch; - const prevEpoch = epochCtx.previousShuffling.epoch; + const prevEpoch = epochCtx.previousEpoch; const nextEpoch = currentEpoch + 1; // active validator indices for nextShuffling is ready, we want to precalculate for the one after that const nextEpoch2 = currentEpoch + 2; diff --git a/packages/state-transition/src/epoch/processPendingAttestations.ts b/packages/state-transition/src/epoch/processPendingAttestations.ts index 8f68e9735036..91ee3594f355 100644 --- a/packages/state-transition/src/epoch/processPendingAttestations.ts +++ b/packages/state-transition/src/epoch/processPendingAttestations.ts @@ -24,7 +24,7 @@ export function processPendingAttestations( headFlag: number ): void { const {epochCtx, slot: stateSlot} = state; - const prevEpoch = epochCtx.previousShuffling.epoch; + const prevEpoch = epochCtx.previousEpoch; if (attestations.length === 0) { return; } diff --git a/packages/state-transition/src/util/epochShuffling.ts b/packages/state-transition/src/util/epochShuffling.ts index 12f270d29792..47c396817929 100644 --- a/packages/state-transition/src/util/epochShuffling.ts +++ b/packages/state-transition/src/util/epochShuffling.ts @@ -22,11 +22,6 @@ export type ReadonlyEpochShuffling = { }; export type EpochShuffling = { - /** - * Epoch being shuffled - */ - epoch: Epoch; - /** * Non-shuffled active validator indices */ @@ -92,7 +87,6 @@ export function computeEpochShuffling( } return { - epoch, activeIndices: _activeIndices, shuffling, committees, diff --git a/packages/state-transition/src/util/seed.ts b/packages/state-transition/src/util/seed.ts index cf48fda8bec4..d90ec76f83c7 100644 --- a/packages/state-transition/src/util/seed.ts +++ b/packages/state-transition/src/util/seed.ts @@ -20,17 +20,18 @@ import {computeEpochAtSlot} from "./epoch.js"; * Compute proposer indices for an epoch */ export function computeProposers( + epoch: Epoch, epochSeed: Uint8Array, - shuffling: {epoch: Epoch; activeIndices: ArrayLike}, + activeIndices: ArrayLike, effectiveBalanceIncrements: EffectiveBalanceIncrements ): number[] { - const startSlot = computeStartSlotAtEpoch(shuffling.epoch); + const startSlot = computeStartSlotAtEpoch(epoch); const proposers = []; for (let slot = startSlot; slot < startSlot + SLOTS_PER_EPOCH; slot++) { proposers.push( computeProposerIndex( effectiveBalanceIncrements, - shuffling.activeIndices, + activeIndices, digest(Buffer.concat([epochSeed, intToBytes(slot, 8)])) ) ); diff --git a/packages/state-transition/test/perf/util/shufflings.test.ts b/packages/state-transition/test/perf/util/shufflings.test.ts index e04dd405d960..98a2bb4e5564 100644 --- a/packages/state-transition/test/perf/util/shufflings.test.ts +++ b/packages/state-transition/test/perf/util/shufflings.test.ts @@ -27,8 +27,13 @@ describe("epoch shufflings", () => { itBench({ id: `computeProposers - vc ${numValidators}`, fn: () => { - const epochSeed = getSeed(state, state.epochCtx.nextShuffling.epoch, DOMAIN_BEACON_PROPOSER); - computeProposers(epochSeed, state.epochCtx.nextShuffling, state.epochCtx.effectiveBalanceIncrements); + const epochSeed = getSeed(state, state.epochCtx.nextEpoch, DOMAIN_BEACON_PROPOSER); + computeProposers( + state.epochCtx.nextEpoch, + epochSeed, + state.epochCtx.nextShuffling.activeIndices, + state.epochCtx.effectiveBalanceIncrements + ); }, }); diff --git a/packages/state-transition/test/unit/util/balance.test.ts b/packages/state-transition/test/unit/util/balance.test.ts index 5b666cb0524e..fd488a0582c9 100644 --- a/packages/state-transition/test/unit/util/balance.test.ts +++ b/packages/state-transition/test/unit/util/balance.test.ts @@ -88,7 +88,7 @@ describe("getEffectiveBalanceIncrementsZeroInactive", () => { ...generateValidators(5, {activation: Infinity, exit: Infinity, balance: 32e9}), ], }); - const justifiedEpoch = justifiedState.epochCtx.currentShuffling.epoch; + const justifiedEpoch = justifiedState.epochCtx.epoch; const validators = justifiedState.validators.getAllReadonlyValues(); const effectiveBalances = getEffectiveBalanceIncrementsZeroed(validators.length); From 874c38c8654d3fe8be2b5ba987d45cbe3c84dff1 Mon Sep 17 00:00:00 2001 From: matthewkeil Date: Tue, 20 Feb 2024 13:59:35 +0800 Subject: [PATCH 02/23] feat: move activeIndices off of EpochShuffling and implement on EpochCache --- .../src/chain/blocks/importBlock.ts | 2 +- packages/beacon-node/src/network/network.ts | 2 +- .../state-transition/src/cache/epochCache.ts | 61 +++++++++++++------ .../src/epoch/processSyncCommitteeUpdates.ts | 2 +- .../src/slot/upgradeStateToAltair.ts | 2 +- packages/state-transition/src/util/balance.ts | 4 +- .../src/util/weakSubjectivity.ts | 2 +- 7 files changed, 49 insertions(+), 26 deletions(-) diff --git a/packages/beacon-node/src/chain/blocks/importBlock.ts b/packages/beacon-node/src/chain/blocks/importBlock.ts index 9c9a2e811deb..661f1c34d08c 100644 --- a/packages/beacon-node/src/chain/blocks/importBlock.ts +++ b/packages/beacon-node/src/chain/blocks/importBlock.ts @@ -366,7 +366,7 @@ export async function importBlock( // Note: in-lined code from previos handler of ChainEvent.checkpoint this.logger.verbose("Checkpoint processed", toCheckpointHex(cp)); - const activeValidatorsCount = checkpointState.epochCtx.currentShuffling.activeIndices.length; + const activeValidatorsCount = checkpointState.epochCtx.currentActiveIndices.length; this.metrics?.currentActiveValidators.set(activeValidatorsCount); this.metrics?.currentValidators.set({status: "active"}, activeValidatorsCount); diff --git a/packages/beacon-node/src/network/network.ts b/packages/beacon-node/src/network/network.ts index f4b57fe0c658..380485133ba1 100644 --- a/packages/beacon-node/src/network/network.ts +++ b/packages/beacon-node/src/network/network.ts @@ -128,7 +128,7 @@ export class Network implements INetwork { const events = new NetworkEventBus(); const aggregatorTracker = new AggregatorTracker(); - const activeValidatorCount = chain.getHeadState().epochCtx.currentShuffling.activeIndices.length; + const activeValidatorCount = chain.getHeadState().epochCtx.currentActiveIndices.length; const initialStatus = chain.getStatus(); if (opts.useWorker) { diff --git a/packages/state-transition/src/cache/epochCache.ts b/packages/state-transition/src/cache/epochCache.ts index 08081ee2dcc0..89a045de048b 100644 --- a/packages/state-transition/src/cache/epochCache.ts +++ b/packages/state-transition/src/cache/epochCache.ts @@ -128,6 +128,16 @@ export class EpochCache { currentShuffling: EpochShuffling; /** Same as previousShuffling */ nextShuffling: EpochShuffling; + + /** + * Validator indexes are used in a few places but and are built separate from + * the shufflings so store them on the EpochCache so they are not tied to the + * whether the shuffling is computed or not + */ + previousActiveIndices: ValidatorIndex[]; + currentActiveIndices: ValidatorIndex[]; + nextActiveIndices: ValidatorIndex[]; + /** * Effective balances, for altair processAttestations() */ @@ -217,6 +227,9 @@ export class EpochCache { previousShuffling: EpochShuffling; currentShuffling: EpochShuffling; nextShuffling: EpochShuffling; + previousActiveIndices: ValidatorIndex[]; + currentActiveIndices: ValidatorIndex[]; + nextActiveIndices: ValidatorIndex[]; effectiveBalanceIncrements: EffectiveBalanceIncrements; totalSlashingsByIncrement: number; syncParticipantReward: number; @@ -243,6 +256,9 @@ export class EpochCache { this.previousShuffling = data.previousShuffling; this.currentShuffling = data.currentShuffling; this.nextShuffling = data.nextShuffling; + this.previousActiveIndices = data.previousActiveIndices; + this.currentActiveIndices = data.currentActiveIndices; + this.nextActiveIndices = data.nextActiveIndices; this.effectiveBalanceIncrements = data.effectiveBalanceIncrements; this.totalSlashingsByIncrement = data.totalSlashingsByIncrement; this.syncParticipantReward = data.syncParticipantReward; @@ -356,11 +372,11 @@ export class EpochCache { // Allow to create CachedBeaconState for empty states, or no active validators const proposers = - currentShuffling.activeIndices.length > 0 + currentActiveIndices.length > 0 ? computeProposers( currentEpoch, currentProposerSeed, - currentShuffling.activeIndices, + currentActiveIndices, effectiveBalanceIncrements ) : []; @@ -405,11 +421,11 @@ export class EpochCache { // activeIndices size is dependent on the state epoch. The epoch is advanced after running the epoch transition, and // the first block of the epoch process_block() call. So churnLimit must be computed at the end of the before epoch // transition and the result is valid until the end of the next epoch transition - const churnLimit = getChurnLimit(config, currentShuffling.activeIndices.length); + const churnLimit = getChurnLimit(config, currentActiveIndices.length); const activationChurnLimit = getActivationChurnLimit( config, config.getForkSeq(state.slot), - currentShuffling.activeIndices.length + currentActiveIndices.length ); if (exitQueueChurn >= churnLimit) { exitQueueEpoch += 1; @@ -446,6 +462,9 @@ export class EpochCache { previousShuffling, currentShuffling, nextShuffling, + previousActiveIndices, + currentActiveIndices, + nextActiveIndices, effectiveBalanceIncrements, totalSlashingsByIncrement, syncParticipantReward, @@ -484,6 +503,9 @@ export class EpochCache { previousShuffling: this.previousShuffling, currentShuffling: this.currentShuffling, nextShuffling: this.nextShuffling, + previousActiveIndices: this.previousActiveIndices, + currentActiveIndices: this.currentActiveIndices, + nextActiveIndices: this.nextActiveIndices, // Uint8Array, requires cloning, but it is cloned only when necessary before an epoch transition // See EpochCache.beforeEpochTransition() effectiveBalanceIncrements: this.effectiveBalanceIncrements, @@ -517,10 +539,20 @@ export class EpochCache { nextEpochTotalActiveBalanceByIncrement: number; } ): void { + // Advance time units + // state.slot is advanced right before calling this function + // ``` + // postState.slot++; + // afterProcessEpoch(postState, epochTransitionCache); + // ``` + this.epoch = computeEpochAtSlot(state.slot); + this.syncPeriod = computeSyncPeriodAtEpoch(this.epoch); + this.previousActiveIndices = this.currentActiveIndices; + this.currentActiveIndices = this.nextActiveIndices + this.nextActiveIndices = epochTransitionCache.nextEpochShufflingActiveValidatorIndices; + this.previousShuffling = this.currentShuffling; this.currentShuffling = this.nextShuffling; - this.epoch += 1; - this.nextShuffling = computeEpochShuffling( state, epochTransitionCache.nextEpochShufflingActiveValidatorIndices, @@ -534,7 +566,7 @@ export class EpochCache { this.proposers = computeProposers( this.epoch, currentProposerSeed, - this.currentShuffling.activeIndices, + this.currentActiveIndices, this.effectiveBalanceIncrements ); @@ -557,11 +589,11 @@ export class EpochCache { // activeIndices size is dependent on the state epoch. The epoch is advanced after running the epoch transition, and // the first block of the epoch process_block() call. So churnLimit must be computed at the end of the before epoch // transition and the result is valid until the end of the next epoch transition - this.churnLimit = getChurnLimit(this.config, this.currentShuffling.activeIndices.length); + this.churnLimit = getChurnLimit(this.config, this.currentActiveIndices.length); this.activationChurnLimit = getActivationChurnLimit( this.config, this.config.getForkSeq(state.slot), - this.currentShuffling.activeIndices.length + this.currentActiveIndices.length ); // Maybe advance exitQueueEpoch at the end of the epoch if there haven't been any exists for a while @@ -580,15 +612,6 @@ export class EpochCache { this.previousTargetUnslashedBalanceIncrements = this.currentTargetUnslashedBalanceIncrements; this.currentTargetUnslashedBalanceIncrements = 0; - - // Advance time units - // state.slot is advanced right before calling this function - // ``` - // postState.slot++; - // afterProcessEpoch(postState, epochTransitionCache); - // ``` - this.epoch = computeEpochAtSlot(state.slot); - this.syncPeriod = computeSyncPeriodAtEpoch(this.epoch); } beforeEpochTransition(): void { @@ -684,7 +707,7 @@ export class EpochCache { const indexes = computeProposers( this.nextEpoch, this.proposersNextEpoch.seed, - this.nextShuffling.activeIndices, + this.nextActiveIndices, this.effectiveBalanceIncrements ); this.proposersNextEpoch = {computed: true, indexes}; diff --git a/packages/state-transition/src/epoch/processSyncCommitteeUpdates.ts b/packages/state-transition/src/epoch/processSyncCommitteeUpdates.ts index dc1f39274399..538628378c6c 100644 --- a/packages/state-transition/src/epoch/processSyncCommitteeUpdates.ts +++ b/packages/state-transition/src/epoch/processSyncCommitteeUpdates.ts @@ -14,7 +14,7 @@ export function processSyncCommitteeUpdates(state: CachedBeaconStateAltair): voi const nextEpoch = state.epochCtx.epoch + 1; if (nextEpoch % EPOCHS_PER_SYNC_COMMITTEE_PERIOD === 0) { - const activeValidatorIndices = state.epochCtx.nextShuffling.activeIndices; + const activeValidatorIndices = state.epochCtx.nextActiveIndices; const {effectiveBalanceIncrements} = state.epochCtx; const nextSyncCommitteeIndices = getNextSyncCommitteeIndices( diff --git a/packages/state-transition/src/slot/upgradeStateToAltair.ts b/packages/state-transition/src/slot/upgradeStateToAltair.ts index 0afa43930ef0..bdd3d9edc24c 100644 --- a/packages/state-transition/src/slot/upgradeStateToAltair.ts +++ b/packages/state-transition/src/slot/upgradeStateToAltair.ts @@ -71,7 +71,7 @@ export function upgradeStateToAltair(statePhase0: CachedBeaconStatePhase0): Cach const {syncCommittee, indices} = getNextSyncCommittee( stateAltair, - stateAltair.epochCtx.nextShuffling.activeIndices, + stateAltair.epochCtx.nextActiveIndices, stateAltair.epochCtx.effectiveBalanceIncrements ); const syncCommitteeView = ssz.altair.SyncCommittee.toViewDU(syncCommittee); diff --git a/packages/state-transition/src/util/balance.ts b/packages/state-transition/src/util/balance.ts index e305c745ab72..9618e81464b5 100644 --- a/packages/state-transition/src/util/balance.ts +++ b/packages/state-transition/src/util/balance.ts @@ -51,7 +51,7 @@ export function decreaseBalance(state: BeaconStateAllForks, index: ValidatorInde export function getEffectiveBalanceIncrementsZeroInactive( justifiedState: CachedBeaconStateAllForks ): EffectiveBalanceIncrements { - const {activeIndices} = justifiedState.epochCtx.currentShuffling; + const {currentActiveIndices} = justifiedState.epochCtx; // 5x faster than reading from state.validators, with validator Nodes as values const validatorCount = justifiedState.validators.length; const {effectiveBalanceIncrements} = justifiedState.epochCtx; @@ -66,7 +66,7 @@ export function getEffectiveBalanceIncrementsZeroInactive( const validators = justifiedState.validators.getAllReadonly(); let j = 0; for (let i = 0; i < validatorCount; i++) { - if (i === activeIndices[j]) { + if (i === currentActiveIndices[j]) { // active validator j++; if (validators[i].slashed) { diff --git a/packages/state-transition/src/util/weakSubjectivity.ts b/packages/state-transition/src/util/weakSubjectivity.ts index 4614534bcb27..9d4c827e301a 100644 --- a/packages/state-transition/src/util/weakSubjectivity.ts +++ b/packages/state-transition/src/util/weakSubjectivity.ts @@ -36,7 +36,7 @@ export function computeWeakSubjectivityPeriodCachedState( config: ChainForkConfig, state: CachedBeaconStateAllForks ): number { - const activeValidatorCount = state.epochCtx.currentShuffling.activeIndices.length; + const activeValidatorCount = state.epochCtx.currentActiveIndices.length; return computeWeakSubjectivityPeriodFromConstituents( activeValidatorCount, state.epochCtx.totalActiveBalanceIncrements, From 7f9a6407ac5f5218fd416a1a42bb706188c39a0d Mon Sep 17 00:00:00 2001 From: matthewkeil Date: Tue, 20 Feb 2024 14:25:09 +0800 Subject: [PATCH 03/23] feat: add shufflingDecisionRoot to epochCtx to allow pulling of shuffling from shufflingCache --- .../beacon-node/src/chain/shufflingCache.ts | 1 + .../test/utils/validationData/attestation.ts | 10 +--- .../state-transition/src/cache/epochCache.ts | 48 +++++++++++++------ .../src/util/epochShuffling.ts | 5 +- 4 files changed, 40 insertions(+), 24 deletions(-) diff --git a/packages/beacon-node/src/chain/shufflingCache.ts b/packages/beacon-node/src/chain/shufflingCache.ts index 751b92b1f2cf..7c4518b64f33 100644 --- a/packages/beacon-node/src/chain/shufflingCache.ts +++ b/packages/beacon-node/src/chain/shufflingCache.ts @@ -198,6 +198,7 @@ function isPromiseCacheItem(item: CacheItem): item is PromiseCacheItem { return item.type === CacheItemType.promise; } +// TODO: @tuyennhv why is this here and not in state-transition with `getShufflingDecisionBlock`? /** * Get the shuffling decision block root for the given epoch of given state * - Special case close to genesis block, return the genesis block root diff --git a/packages/beacon-node/test/utils/validationData/attestation.ts b/packages/beacon-node/test/utils/validationData/attestation.ts index 48ba91c9cdd6..d3788e55ced3 100644 --- a/packages/beacon-node/test/utils/validationData/attestation.ts +++ b/packages/beacon-node/test/utils/validationData/attestation.ts @@ -1,10 +1,5 @@ import {BitArray, toHexString} from "@chainsafe/ssz"; -import { - computeEpochAtSlot, - computeSigningRoot, - computeStartSlotAtEpoch, - getShufflingDecisionBlock, -} from "@lodestar/state-transition"; +import {computeEpochAtSlot, computeSigningRoot, computeStartSlotAtEpoch} from "@lodestar/state-transition"; import {ProtoBlock, IForkChoice, ExecutionStatus} from "@lodestar/fork-choice"; import {DOMAIN_BEACON_ATTESTER} from "@lodestar/params"; import {phase0, Slot, ssz} from "@lodestar/types"; @@ -81,7 +76,6 @@ export function getAttestationValidData(opts: AttestationValidDataOpts): { const shufflingCache = new ShufflingCache(); shufflingCache.processState(state, state.epochCtx.epoch); shufflingCache.processState(state, state.epochCtx.nextEpoch); - const dependentRoot = getShufflingDecisionBlock(state, state.epochCtx.epoch); const forkChoice = { getBlock: (root) => { @@ -92,7 +86,7 @@ export function getAttestationValidData(opts: AttestationValidDataOpts): { if (rootHex !== toHexString(beaconBlockRoot)) return null; return headBlock; }, - getDependentRoot: () => dependentRoot, + getDependentRoot: () => state.epochCtx.currentShufflingDecisionRoot, } as Partial as IForkChoice; const committeeIndices = state.epochCtx.getBeaconCommittee(attSlot, attIndex); diff --git a/packages/state-transition/src/cache/epochCache.ts b/packages/state-transition/src/cache/epochCache.ts index 89a045de048b..3683c136b4ae 100644 --- a/packages/state-transition/src/cache/epochCache.ts +++ b/packages/state-transition/src/cache/epochCache.ts @@ -1,6 +1,6 @@ import {CoordType} from "@chainsafe/bls/types"; import bls from "@chainsafe/bls"; -import {BLSSignature, CommitteeIndex, Epoch, Slot, ValidatorIndex, phase0, SyncPeriod} from "@lodestar/types"; +import {BLSSignature, CommitteeIndex, Epoch, Slot, ValidatorIndex, phase0, SyncPeriod, RootHex} from "@lodestar/types"; import {createBeaconConfig, BeaconConfig, ChainConfig} from "@lodestar/config"; import { ATTESTATION_SUBNET_COUNT, @@ -138,6 +138,13 @@ export class EpochCache { currentActiveIndices: ValidatorIndex[]; nextActiveIndices: ValidatorIndex[]; + /** + * RootHex of decision block determining the shufflings + */ + previousShufflingDecisionRoot: RootHex; + currentShufflingDecisionRoot: RootHex; + nextShufflingDecisionRoot: RootHex; + /** * Effective balances, for altair processAttestations() */ @@ -230,6 +237,9 @@ export class EpochCache { previousActiveIndices: ValidatorIndex[]; currentActiveIndices: ValidatorIndex[]; nextActiveIndices: ValidatorIndex[]; + previousShufflingDecisionRoot: RootHex; + currentShufflingDecisionRoot: RootHex; + nextShufflingDecisionRoot: RootHex; effectiveBalanceIncrements: EffectiveBalanceIncrements; totalSlashingsByIncrement: number; syncParticipantReward: number; @@ -259,6 +269,9 @@ export class EpochCache { this.previousActiveIndices = data.previousActiveIndices; this.currentActiveIndices = data.currentActiveIndices; this.nextActiveIndices = data.nextActiveIndices; + this.previousShufflingDecisionRoot = data.previousShufflingDecisionRoot; + this.currentShufflingDecisionRoot = data.currentShufflingDecisionRoot; + this.nextShufflingDecisionRoot = data.nextShufflingDecisionRoot; this.effectiveBalanceIncrements = data.effectiveBalanceIncrements; this.totalSlashingsByIncrement = data.totalSlashingsByIncrement; this.syncParticipantReward = data.syncParticipantReward; @@ -314,12 +327,12 @@ export class EpochCache { // BeaconChain could provide a shuffling cache to avoid re-computing shuffling every epoch // in that case, we don't need to compute shufflings again - const previousShufflingDecisionBlock = getShufflingDecisionBlock(state, previousEpoch); - const cachedPreviousShuffling = opts?.shufflingGetter?.(previousEpoch, previousShufflingDecisionBlock); - const currentShufflingDecisionBlock = getShufflingDecisionBlock(state, currentEpoch); - const cachedCurrentShuffling = opts?.shufflingGetter?.(currentEpoch, currentShufflingDecisionBlock); - const nextShufflingDecisionBlock = getShufflingDecisionBlock(state, nextEpoch); - const cachedNextShuffling = opts?.shufflingGetter?.(nextEpoch, nextShufflingDecisionBlock); + const previousShufflingDecisionRoot = getShufflingDecisionBlock(state, previousEpoch); + const cachedPreviousShuffling = opts?.shufflingGetter?.(previousEpoch, previousShufflingDecisionRoot); + const currentShufflingDecisionRoot = getShufflingDecisionBlock(state, currentEpoch); + const cachedCurrentShuffling = opts?.shufflingGetter?.(currentEpoch, currentShufflingDecisionRoot); + const nextShufflingDecisionRoot = getShufflingDecisionBlock(state, nextEpoch); + const cachedNextShuffling = opts?.shufflingGetter?.(nextEpoch, nextShufflingDecisionRoot); for (let i = 0; i < validatorCount; i++) { const validator = validators[i]; @@ -373,12 +386,7 @@ export class EpochCache { // Allow to create CachedBeaconState for empty states, or no active validators const proposers = currentActiveIndices.length > 0 - ? computeProposers( - currentEpoch, - currentProposerSeed, - currentActiveIndices, - effectiveBalanceIncrements - ) + ? computeProposers(currentEpoch, currentProposerSeed, currentActiveIndices, effectiveBalanceIncrements) : []; const proposersNextEpoch: ProposersDeferred = { @@ -465,6 +473,9 @@ export class EpochCache { previousActiveIndices, currentActiveIndices, nextActiveIndices, + previousShufflingDecisionRoot, + currentShufflingDecisionRoot, + nextShufflingDecisionRoot, effectiveBalanceIncrements, totalSlashingsByIncrement, syncParticipantReward, @@ -506,6 +517,9 @@ export class EpochCache { previousActiveIndices: this.previousActiveIndices, currentActiveIndices: this.currentActiveIndices, nextActiveIndices: this.nextActiveIndices, + previousShufflingDecisionRoot: this.previousShufflingDecisionRoot, + currentShufflingDecisionRoot: this.currentShufflingDecisionRoot, + nextShufflingDecisionRoot: this.nextShufflingDecisionRoot, // Uint8Array, requires cloning, but it is cloned only when necessary before an epoch transition // See EpochCache.beforeEpochTransition() effectiveBalanceIncrements: this.effectiveBalanceIncrements, @@ -547,9 +561,15 @@ export class EpochCache { // ``` this.epoch = computeEpochAtSlot(state.slot); this.syncPeriod = computeSyncPeriodAtEpoch(this.epoch); + this.previousActiveIndices = this.currentActiveIndices; - this.currentActiveIndices = this.nextActiveIndices + this.previousShufflingDecisionRoot = this.currentShufflingDecisionRoot; + + this.currentActiveIndices = this.nextActiveIndices; + this.currentShufflingDecisionRoot = this.nextShufflingDecisionRoot; + this.nextActiveIndices = epochTransitionCache.nextEpochShufflingActiveValidatorIndices; + this.nextShufflingDecisionRoot = getShufflingDecisionBlock(state, this.nextEpoch); this.previousShuffling = this.currentShuffling; this.currentShuffling = this.nextShuffling; diff --git a/packages/state-transition/src/util/epochShuffling.ts b/packages/state-transition/src/util/epochShuffling.ts index 47c396817929..b2db3bb7a414 100644 --- a/packages/state-transition/src/util/epochShuffling.ts +++ b/packages/state-transition/src/util/epochShuffling.ts @@ -1,13 +1,14 @@ import {toHexString} from "@chainsafe/ssz"; -import {Epoch, RootHex, ValidatorIndex} from "@lodestar/types"; +import {ssz, Epoch, RootHex, ValidatorIndex} from "@lodestar/types"; import {intDiv} from "@lodestar/utils"; import { DOMAIN_BEACON_ATTESTER, + GENESIS_SLOT, MAX_COMMITTEES_PER_SLOT, SLOTS_PER_EPOCH, TARGET_COMMITTEE_SIZE, } from "@lodestar/params"; -import {BeaconStateAllForks} from "../types.js"; +import {BeaconStateAllForks, CachedBeaconStateAllForks} from "../types.js"; import {getSeed} from "./seed.js"; import {unshuffleList} from "./shuffle.js"; import {computeStartSlotAtEpoch} from "./epoch.js"; From 33730b60f5a6d55392621298478116734c03788c Mon Sep 17 00:00:00 2001 From: matthewkeil Date: Tue, 20 Feb 2024 14:36:38 +0800 Subject: [PATCH 04/23] refactor: move getShufflingDecisionBlock with other shufflingDecision functions --- packages/state-transition/src/cache/epochCache.ts | 4 +++- .../state-transition/src/util/epochShuffling.ts | 13 ++----------- .../src/util/shufflingDecisionRoot.ts | 13 +++++++++++-- 3 files changed, 16 insertions(+), 14 deletions(-) diff --git a/packages/state-transition/src/cache/epochCache.ts b/packages/state-transition/src/cache/epochCache.ts index 3683c136b4ae..59c3adc61c01 100644 --- a/packages/state-transition/src/cache/epochCache.ts +++ b/packages/state-transition/src/cache/epochCache.ts @@ -26,7 +26,9 @@ import { computeProposers, getActivationChurnLimit, } from "../util/index.js"; -import {computeEpochShuffling, EpochShuffling, getShufflingDecisionBlock} from "../util/epochShuffling.js"; +// TODO: (matthewkeil) why are the `util` imports below not from the index.js? +import {getShufflingDecisionBlock} from "../util/shufflingDecisionRoot.js"; +import {computeEpochShuffling, EpochShuffling} from "../util/epochShuffling.js"; import {computeBaseRewardPerIncrement, computeSyncParticipantReward} from "../util/syncCommittee.js"; import {sumTargetUnslashedBalanceIncrements} from "../util/targetUnslashedBalance.js"; import {getTotalSlashingsByIncrement} from "../epoch/processSlashings.js"; diff --git a/packages/state-transition/src/util/epochShuffling.ts b/packages/state-transition/src/util/epochShuffling.ts index b2db3bb7a414..e48886e51a4b 100644 --- a/packages/state-transition/src/util/epochShuffling.ts +++ b/packages/state-transition/src/util/epochShuffling.ts @@ -1,18 +1,14 @@ -import {toHexString} from "@chainsafe/ssz"; -import {ssz, Epoch, RootHex, ValidatorIndex} from "@lodestar/types"; +import {Epoch, ValidatorIndex} from "@lodestar/types"; import {intDiv} from "@lodestar/utils"; import { DOMAIN_BEACON_ATTESTER, - GENESIS_SLOT, MAX_COMMITTEES_PER_SLOT, SLOTS_PER_EPOCH, TARGET_COMMITTEE_SIZE, } from "@lodestar/params"; -import {BeaconStateAllForks, CachedBeaconStateAllForks} from "../types.js"; +import {BeaconStateAllForks} from "../types.js"; import {getSeed} from "./seed.js"; import {unshuffleList} from "./shuffle.js"; -import {computeStartSlotAtEpoch} from "./epoch.js"; -import {getBlockRootAtSlot} from "./blockRoot.js"; /** * Readonly interface for EpochShuffling. @@ -94,8 +90,3 @@ export function computeEpochShuffling( committeesPerSlot, }; } - -export function getShufflingDecisionBlock(state: BeaconStateAllForks, epoch: Epoch): RootHex { - const pivotSlot = computeStartSlotAtEpoch(epoch - 1) - 1; - return toHexString(getBlockRootAtSlot(state, pivotSlot)); -} diff --git a/packages/state-transition/src/util/shufflingDecisionRoot.ts b/packages/state-transition/src/util/shufflingDecisionRoot.ts index 10af814e9af3..3ff3a2d0bfdb 100644 --- a/packages/state-transition/src/util/shufflingDecisionRoot.ts +++ b/packages/state-transition/src/util/shufflingDecisionRoot.ts @@ -1,8 +1,17 @@ -import {Epoch, Root, Slot} from "@lodestar/types"; -import {CachedBeaconStateAllForks} from "../types.js"; +import {Epoch, Root, RootHex, Slot} from "@lodestar/types"; +import {toHexString} from "@lodestar/utils"; +import {BeaconStateAllForks, CachedBeaconStateAllForks} from "../types.js"; import {getBlockRootAtSlot} from "./blockRoot.js"; import {computeStartSlotAtEpoch} from "./epoch.js"; +/** + * Returns hex string representation of the block root for a given state and epoch + */ +export function getShufflingDecisionBlock(state: BeaconStateAllForks, epoch: Epoch): RootHex { + const pivotSlot = computeStartSlotAtEpoch(epoch - 1) - 1; + return toHexString(getBlockRootAtSlot(state, pivotSlot)); +} + /** * Returns the block root which decided the proposer shuffling for the current epoch. This root * can be used to key this proposer shuffling. From a661fff80deef0043e0358061f134016c28463be Mon Sep 17 00:00:00 2001 From: matthewkeil Date: Tue, 20 Feb 2024 21:45:46 +0800 Subject: [PATCH 05/23] feat: implement BaseShufflingCache but may need to revert to putting actual class in beacon-node and building mock --- packages/beacon-node/src/chain/chain.ts | 33 ++- packages/beacon-node/src/chain/options.ts | 4 +- .../beacon-node/src/chain/shufflingCache.ts | 142 +++--------- .../stateCache/persistentCheckpointsCache.ts | 16 +- .../src/cache/baseShufflingCache.ts | 219 ++++++++++++++++++ .../state-transition/src/cache/epochCache.ts | 126 +++++----- .../state-transition/src/cache/stateCache.ts | 1 + packages/state-transition/src/cache/types.ts | 5 +- packages/state-transition/src/index.ts | 1 + 9 files changed, 332 insertions(+), 215 deletions(-) create mode 100644 packages/state-transition/src/cache/baseShufflingCache.ts diff --git a/packages/beacon-node/src/chain/chain.ts b/packages/beacon-node/src/chain/chain.ts index abbea14e6098..bbae90e73574 100644 --- a/packages/beacon-node/src/chain/chain.ts +++ b/packages/beacon-node/src/chain/chain.ts @@ -223,17 +223,28 @@ export class BeaconChain implements IBeaconChain { // pubkeys takes ~30 seconds for 350k keys (mainnet 2022Q2). // When the BeaconStateCache is created in eth1 genesis builder it may be incorrect. Until we can ensure that // it's safe to re-use _ANY_ BeaconStateCache, this option is disabled by default and only used in tests. - const cachedState = - isCachedBeaconState(anchorState) && opts.skipCreateStateCacheIfAvailable - ? anchorState - : createCachedBeaconState(anchorState, { - config, - pubkey2index: new PubkeyIndexMap(), - index2pubkey: [], - }); - this.shufflingCache.processState(cachedState, cachedState.epochCtx.previousEpoch); - this.shufflingCache.processState(cachedState, cachedState.epochCtx.epoch); - this.shufflingCache.processState(cachedState, cachedState.epochCtx.nextEpoch); + let cachedState: CachedBeaconStateAllForks; + if (isCachedBeaconState(anchorState) && opts.skipCreateStateCacheIfAvailable) { + cachedState = anchorState; + if (anchorState.epochCtx.shufflingCache.hasItems()) { + cachedState.epochCtx.shufflingCache = this.shufflingCache.clone(anchorState.epochCtx.shufflingCache); + } + } else { + cachedState = createCachedBeaconState(anchorState, { + config, + shufflingCache: this.shufflingCache, + pubkey2index: new PubkeyIndexMap(), + index2pubkey: [], + }); + } + /** + * These should already be processed when creating the shufflingCache.fromState + * + * TODO: (matthewkeil) double check this is correct + */ + // this.shufflingCache.processState(cachedState, cachedState.epochCtx.previousEpoch); + // this.shufflingCache.processState(cachedState, cachedState.epochCtx.epoch); + // this.shufflingCache.processState(cachedState, cachedState.epochCtx.nextEpoch); // Persist single global instance of state caches this.pubkey2index = cachedState.epochCtx.pubkey2index; diff --git a/packages/beacon-node/src/chain/options.ts b/packages/beacon-node/src/chain/options.ts index e687099a0cb4..671347d5e980 100644 --- a/packages/beacon-node/src/chain/options.ts +++ b/packages/beacon-node/src/chain/options.ts @@ -3,14 +3,14 @@ import {defaultOptions as defaultValidatorOptions} from "@lodestar/validator"; import {ArchiverOpts} from "./archiver/index.js"; import {ForkChoiceOpts} from "./forkChoice/index.js"; import {LightClientServerOpts} from "./lightClient/index.js"; -import {ShufflingCacheOpts} from "./shufflingCache.js"; +import {ShufflingCacheOptions} from "./shufflingCache.js"; export type IChainOptions = BlockProcessOpts & PoolOpts & SeenCacheOpts & ForkChoiceOpts & ArchiverOpts & - ShufflingCacheOpts & + ShufflingCacheOptions & LightClientServerOpts & { blsVerifyAllMainThread?: boolean; blsVerifyAllMultiThread?: boolean; diff --git a/packages/beacon-node/src/chain/shufflingCache.ts b/packages/beacon-node/src/chain/shufflingCache.ts index 7c4518b64f33..38e207f6eb28 100644 --- a/packages/beacon-node/src/chain/shufflingCache.ts +++ b/packages/beacon-node/src/chain/shufflingCache.ts @@ -1,46 +1,17 @@ import {toHexString} from "@chainsafe/ssz"; -import {CachedBeaconStateAllForks, EpochShuffling, getShufflingDecisionBlock} from "@lodestar/state-transition"; +import { + BaseShufflingCache, + BaseShufflingCacheOptions, + CachedBeaconStateAllForks, + EpochShuffling, + getShufflingDecisionBlock, +} from "@lodestar/state-transition"; import {Epoch, RootHex, ssz} from "@lodestar/types"; -import {MapDef, pruneSetToMax} from "@lodestar/utils"; import {GENESIS_SLOT} from "@lodestar/params"; import {Metrics} from "../metrics/metrics.js"; import {computeAnchorCheckpoint} from "./initState.js"; -/** - * Same value to CheckpointBalancesCache, with the assumption that we don't have to use it for old epochs. In the worse case: - * - when loading state bytes from disk, we need to compute shuffling for all epochs (~1s as of Sep 2023) - * - don't have shuffling to verify attestations, need to do 1 epoch transition to add shuffling to this cache. This never happens - * with default chain option of maxSkipSlots = 32 - **/ -const MAX_EPOCHS = 4; - -/** - * With default chain option of maxSkipSlots = 32, there should be no shuffling promise. If that happens a lot, it could blow up Lodestar, - * with MAX_EPOCHS = 4, only allow 2 promise at a time. Note that regen already bounds number of concurrent requests at 1 already. - */ -const MAX_PROMISES = 2; - -enum CacheItemType { - shuffling, - promise, -} - -type ShufflingCacheItem = { - type: CacheItemType.shuffling; - shuffling: EpochShuffling; -}; - -type PromiseCacheItem = { - type: CacheItemType.promise; - promise: Promise; - resolveFn: (shuffling: EpochShuffling) => void; -}; - -type CacheItem = ShufflingCacheItem | PromiseCacheItem; - -export type ShufflingCacheOpts = { - maxShufflingCacheEpochs?: number; -}; +export interface ShufflingCacheOptions extends BaseShufflingCacheOptions {} /** * A shuffling cache to help: @@ -48,18 +19,12 @@ export type ShufflingCacheOpts = { * - if a shuffling is not available (which does not happen with default chain option of maxSkipSlots = 32), track a promise to make sure we don't compute the same shuffling twice * - skip computing shuffling when loading state bytes from disk */ -export class ShufflingCache { - /** LRU cache implemented as a map, pruned every time we add an item */ - private readonly itemsByDecisionRootByEpoch: MapDef> = new MapDef( - () => new Map() - ); - - private readonly maxEpochs: number; - +export class ShufflingCache extends BaseShufflingCache { constructor( private readonly metrics: Metrics | null = null, - opts: ShufflingCacheOpts = {} + opts: ShufflingCacheOptions = {} ) { + super(opts); if (metrics) { metrics.shufflingCache.size.addCollect(() => metrics.shufflingCache.size.set( @@ -67,8 +32,6 @@ export class ShufflingCache { ) ); } - - this.maxEpochs = opts.maxShufflingCacheEpochs ?? MAX_EPOCHS; } /** @@ -76,20 +39,20 @@ export class ShufflingCache { */ processState(state: CachedBeaconStateAllForks, shufflingEpoch: Epoch): EpochShuffling { const decisionBlockHex = getDecisionBlock(state, shufflingEpoch); - let shuffling: EpochShuffling; - switch (shufflingEpoch) { - case state.epochCtx.nextEpoch: - shuffling = state.epochCtx.nextShuffling; - break; - case state.epochCtx.epoch: - shuffling = state.epochCtx.currentShuffling; - break; - case state.epochCtx.previousEpoch: - shuffling = state.epochCtx.previousShuffling; - break; - default: - throw new Error(`Shuffling not found from state ${state.slot} for epoch ${shufflingEpoch}`); - } + // let shuffling: EpochShuffling; + // switch (shufflingEpoch) { + // case state.epochCtx.nextEpoch: + // shuffling = state.epochCtx.nextShuffling; + // break; + // case state.epochCtx.epoch: + // shuffling = state.epochCtx.currentShuffling; + // break; + // case state.epochCtx.previousEpoch: + // shuffling = state.epochCtx.previousShuffling; + // break; + // default: + // throw new Error(`Shuffling not found from state ${state.slot} for epoch ${shufflingEpoch}`); + // } let cacheItem = this.itemsByDecisionRootByEpoch.getOrDefault(shufflingEpoch).get(decisionBlockHex); if (cacheItem !== undefined) { @@ -131,13 +94,11 @@ export class ShufflingCache { `Too many shuffling promises: ${promiseCount}, shufflingEpoch: ${shufflingEpoch}, decisionRootHex: ${decisionRootHex}` ); } - let resolveFn: ((shuffling: EpochShuffling) => void) | null = null; + + let resolveFn!: (shuffling: EpochShuffling) => void; const promise = new Promise((resolve) => { resolveFn = resolve; }); - if (resolveFn === null) { - throw new Error("Promise Constructor was not executed immediately"); - } const cacheItem: PromiseCacheItem = { type: CacheItemType.promise, @@ -147,55 +108,6 @@ export class ShufflingCache { this.add(shufflingEpoch, decisionRootHex, cacheItem); this.metrics?.shufflingCache.insertPromiseCount.inc(); } - - /** - * Most of the time, this should return a shuffling immediately. - * If there's a promise, it means we are computing the same shuffling, so we wait for the promise to resolve. - * Return null if we don't have a shuffling for this epoch and dependentRootHex. - */ - async get(shufflingEpoch: Epoch, decisionRootHex: RootHex): Promise { - const cacheItem = this.itemsByDecisionRootByEpoch.getOrDefault(shufflingEpoch).get(decisionRootHex); - if (cacheItem === undefined) { - return null; - } - - if (isShufflingCacheItem(cacheItem)) { - return cacheItem.shuffling; - } else { - // promise - return cacheItem.promise; - } - } - - /** - * Same to get() function but synchronous. - */ - getSync(shufflingEpoch: Epoch, decisionRootHex: RootHex): EpochShuffling | null { - const cacheItem = this.itemsByDecisionRootByEpoch.getOrDefault(shufflingEpoch).get(decisionRootHex); - if (cacheItem === undefined) { - return null; - } - - if (isShufflingCacheItem(cacheItem)) { - return cacheItem.shuffling; - } - - // ignore promise - return null; - } - - private add(shufflingEpoch: Epoch, decisionBlock: RootHex, cacheItem: CacheItem): void { - this.itemsByDecisionRootByEpoch.getOrDefault(shufflingEpoch).set(decisionBlock, cacheItem); - pruneSetToMax(this.itemsByDecisionRootByEpoch, this.maxEpochs); - } -} - -function isShufflingCacheItem(item: CacheItem): item is ShufflingCacheItem { - return item.type === CacheItemType.shuffling; -} - -function isPromiseCacheItem(item: CacheItem): item is PromiseCacheItem { - return item.type === CacheItemType.promise; } // TODO: @tuyennhv why is this here and not in state-transition with `getShufflingDecisionBlock`? diff --git a/packages/beacon-node/src/chain/stateCache/persistentCheckpointsCache.ts b/packages/beacon-node/src/chain/stateCache/persistentCheckpointsCache.ts index 4aea5ad53a6a..47e32b19dede 100644 --- a/packages/beacon-node/src/chain/stateCache/persistentCheckpointsCache.ts +++ b/packages/beacon-node/src/chain/stateCache/persistentCheckpointsCache.ts @@ -214,20 +214,8 @@ export class PersistentCheckpointStateCache implements CheckpointStateCache { } sszTimer?.(); const timer = this.metrics?.stateReloadDuration.startTimer(); - const newCachedState = loadCachedBeaconState( - seedState, - stateBytes, - { - shufflingGetter: (shufflingEpoch, decisionRootHex) => { - const shuffling = this.shufflingCache.getSync(shufflingEpoch, decisionRootHex); - if (shuffling == null) { - this.metrics?.stateReloadShufflingCacheMiss.inc(); - } - return shuffling; - }, - }, - validatorsBytes - ); + // @tuyennhv why are we not passing through the EpochCacheOptions from the seed state? + const newCachedState = loadCachedBeaconState(seedState, stateBytes, {}, validatorsBytes); newCachedState.commit(); const stateRoot = toHexString(newCachedState.hashTreeRoot()); timer?.(); diff --git a/packages/state-transition/src/cache/baseShufflingCache.ts b/packages/state-transition/src/cache/baseShufflingCache.ts new file mode 100644 index 000000000000..27f19b6680fb --- /dev/null +++ b/packages/state-transition/src/cache/baseShufflingCache.ts @@ -0,0 +1,219 @@ +import {Epoch, RootHex} from "@lodestar/types"; +import {LodestarError, MapDef, pruneSetToMax} from "@lodestar/utils"; +import {EpochShuffling, computeEpochShuffling} from "../util/index.js"; +import {BeaconStateAllForks} from "./types.js"; + +/** + * With default chain option of maxSkipSlots = 32, there should be no shuffling promise. If that happens a lot, it could blow up Lodestar, + * with MAX_EPOCHS = 4, only allow 2 promise at a time. Note that regen already bounds number of concurrent requests at 1 already. + */ +const MAX_PROMISES = 2; + +/** + * Same value to CheckpointBalancesCache, with the assumption that we don't have to use it for old epochs. In the worse case: + * - when loading state bytes from disk, we need to compute shuffling for all epochs (~1s as of Sep 2023) + * - don't have shuffling to verify attestations, need to do 1 epoch transition to add shuffling to this cache. This never happens + * with default chain option of maxSkipSlots = 32 + **/ +const SHUFFLING_CACHE_MAX_EPOCHS = 4; + +export interface IShufflingCache { + add(shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex, shuffling: EpochShuffling): void; + get(shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex): Promise; + getOrNull(shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex): EpochShuffling | null; + getOrError(shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex): EpochShuffling; + buildSync( + state: BeaconStateAllForks, + shufflingEpoch: Epoch, + shufflingDecisionRoot: RootHex, + activeIndexes: number[] + ): EpochShuffling; + build( + state: BeaconStateAllForks, + shufflingEpoch: Epoch, + shufflingDecisionRoot: RootHex, + activeIndexes: number[] + ): Promise; +} + +export enum ShufflingCacheErrorCode { + NO_SHUFFLING_FOUND = "EPOCH_SHUFFLING_NO_SHUFFLING_FOUND", + SHUFFLING_PROMISE_NOT_RESOLVED = "EPOCH_SHUFFLING_SHUFFLING_PROMISE_NOT_RESOLVED", +} + +type ShufflingCacheErrorType = + | {code: ShufflingCacheErrorCode.NO_SHUFFLING_FOUND; epoch: Epoch; shufflingDecisionRoot: RootHex} + | {code: ShufflingCacheErrorCode.SHUFFLING_PROMISE_NOT_RESOLVED; epoch: Epoch; shufflingDecisionRoot: RootHex}; + +export class ShufflingCacheError extends LodestarError {} + +export enum ShufflingCacheItemType { + shuffling, + promise, +} + +export type ShufflingCacheShufflingItem = { + type: ShufflingCacheItemType.shuffling; + shuffling: EpochShuffling; +}; + +export type ShufflingCachePromiseItem = { + type: ShufflingCacheItemType.promise; + promise: Promise; + resolveFn: (shuffling: EpochShuffling) => void; +}; + +export type ShufflingCacheItem = ShufflingCacheShufflingItem | ShufflingCachePromiseItem; + +type ShufflingResolution = (shuffling: EpochShuffling) => void; + +export interface BaseShufflingCacheOptions { + maxShufflingCacheEpochs?: number; +} + +export class BaseShufflingCache implements IShufflingCache { + /** LRU cache implemented as a map, pruned every time we add an item */ + protected readonly itemsByDecisionRootByEpoch: MapDef> = new MapDef( + () => new Map() + ); + protected readonly maxEpochs: number; + + constructor(opts: BaseShufflingCacheOptions = {}) { + this.maxEpochs = opts.maxShufflingCacheEpochs ?? SHUFFLING_CACHE_MAX_EPOCHS; + } + + async get(shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex): Promise { + const cacheItem = this.itemsByDecisionRootByEpoch.getOrDefault(shufflingEpoch).get(shufflingDecisionRoot); + if (cacheItem === undefined) { + return null; + } + if (this.isShufflingCacheItem(cacheItem)) { + return cacheItem.shuffling; + } else { + return cacheItem.promise; + } + } + + getOrError(shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex): EpochShuffling { + const cacheItem = this.itemsByDecisionRootByEpoch.getOrDefault(shufflingEpoch).get(shufflingDecisionRoot); + if (cacheItem === undefined) { + throw new ShufflingCacheError({ + code: ShufflingCacheErrorCode.NO_SHUFFLING_FOUND, + epoch: shufflingEpoch, + shufflingDecisionRoot, + }); + } + if (this.isPromiseCacheItem(cacheItem)) { + throw new ShufflingCacheError({ + code: ShufflingCacheErrorCode.SHUFFLING_PROMISE_NOT_RESOLVED, + epoch: shufflingEpoch, + shufflingDecisionRoot, + }); + } + return cacheItem.shuffling; + } + + getOrNull(shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex): EpochShuffling | null { + const cacheItem = this.itemsByDecisionRootByEpoch.getOrDefault(shufflingEpoch).get(shufflingDecisionRoot); + if (cacheItem === undefined || this.isPromiseCacheItem(cacheItem)) { + return null; + } + return cacheItem.shuffling; + } + + add(shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex, shuffling: EpochShuffling): void { + this.itemsByDecisionRootByEpoch + .getOrDefault(shufflingEpoch) + .set(shufflingDecisionRoot, {type: ShufflingCacheItemType.shuffling, shuffling}); + pruneSetToMax(this.itemsByDecisionRootByEpoch, this.maxEpochs); + } + + buildSync( + state: BeaconStateAllForks, + shufflingEpoch: Epoch, + shufflingDecisionRoot: RootHex, + activeIndexes: number[] + ): EpochShuffling { + const cacheItem = this.itemsByDecisionRootByEpoch.getOrDefault(shufflingEpoch).get(shufflingDecisionRoot); + if (!cacheItem) { + // TODO: (matthewkeil) Add metric here for cache miss + return this._build(state, shufflingEpoch, shufflingDecisionRoot, activeIndexes); + } + if (this.isShufflingCacheItem(cacheItem)) { + // TODO: (matthewkeil) Add metric here for cache hit + return cacheItem.shuffling; + } + // Perhaps we should throw an error instead + // + // TODO: (matthewkeil) Add metric here for throwing away and recreating the shuffling + const resolveFn = cacheItem.resolveFn; + const shuffling = this._build(state, shufflingEpoch, shufflingDecisionRoot, activeIndexes); + resolveFn(shuffling); + return shuffling; + } + + async build( + state: BeaconStateAllForks, + shufflingEpoch: Epoch, + shufflingDecisionRoot: RootHex, + activeIndexes: number[] + ): Promise { + const cacheItem = this.itemsByDecisionRootByEpoch.getOrDefault(shufflingEpoch).get(shufflingDecisionRoot); + if (!!cacheItem && this.isShufflingCacheItem(cacheItem)) { + // TODO: (matthewkeil) Add metric here for cache hit + return cacheItem.shuffling; + } + + // TODO: (matthewkeil) Add metric here for cache miss + let resolveFn: ShufflingResolution; + if (!cacheItem) { + resolveFn = this._insertShufflingPromise(shufflingEpoch, shufflingDecisionRoot); + } else { + resolveFn = cacheItem.resolveFn; + } + + // TODO: (matthewkeil) replace this sync call with a worker and async build function that uses + // a nice'd thread to build in core idle time + const shuffling = this._build(state, shufflingEpoch, shufflingDecisionRoot, activeIndexes); + resolveFn(shuffling); + return shuffling; + } + + protected _build( + state: BeaconStateAllForks, + shufflingEpoch: Epoch, + shufflingDecisionRoot: RootHex, + activeIndexes: number[] + ): EpochShuffling { + const shuffling = computeEpochShuffling(state, activeIndexes, shufflingEpoch); + this.add(shufflingEpoch, shufflingDecisionRoot, shuffling); + return shuffling; + } + + protected _insertShufflingPromise(shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex): ShufflingResolution { + const promiseCount = Array.from(this.itemsByDecisionRootByEpoch.values()) + .flatMap((innerMap) => Array.from(innerMap.values())) + .filter((item) => this.isPromiseCacheItem(item)).length; + if (promiseCount >= MAX_PROMISES) { + throw new Error( + `Too many shuffling promises: ${promiseCount}, shufflingEpoch: ${shufflingEpoch}, shufflingDecisionRoot: ${shufflingDecisionRoot}` + ); + } + let resolveFn!: ShufflingResolution; + const promise = new Promise((resolve) => { + resolveFn = resolve; + }); + this.itemsByDecisionRootByEpoch + .getOrDefault(shufflingEpoch) + .set(shufflingDecisionRoot, {type: ShufflingCacheItemType.promise, promise, resolveFn}); + return resolveFn; + } + + protected isShufflingCacheItem(item: ShufflingCacheItem): item is ShufflingCacheShufflingItem { + return item.type === ShufflingCacheItemType.shuffling; + } + + protected isPromiseCacheItem(item: ShufflingCacheItem): item is ShufflingCachePromiseItem { + return item.type === ShufflingCacheItemType.promise; + } +} diff --git a/packages/state-transition/src/cache/epochCache.ts b/packages/state-transition/src/cache/epochCache.ts index 59c3adc61c01..7410054ba71c 100644 --- a/packages/state-transition/src/cache/epochCache.ts +++ b/packages/state-transition/src/cache/epochCache.ts @@ -28,25 +28,27 @@ import { } from "../util/index.js"; // TODO: (matthewkeil) why are the `util` imports below not from the index.js? import {getShufflingDecisionBlock} from "../util/shufflingDecisionRoot.js"; -import {computeEpochShuffling, EpochShuffling} from "../util/epochShuffling.js"; +import {EpochShuffling} from "../util/epochShuffling.js"; import {computeBaseRewardPerIncrement, computeSyncParticipantReward} from "../util/syncCommittee.js"; import {sumTargetUnslashedBalanceIncrements} from "../util/targetUnslashedBalance.js"; import {getTotalSlashingsByIncrement} from "../epoch/processSlashings.js"; import {EffectiveBalanceIncrements, getEffectiveBalanceIncrementsWithLen} from "./effectiveBalanceIncrements.js"; import {Index2PubkeyCache, PubkeyIndexMap, syncPubkeys} from "./pubkeyCache.js"; -import {BeaconStateAllForks, BeaconStateAltair, ShufflingGetter} from "./types.js"; +import {BeaconStateAllForks, BeaconStateAltair} from "./types.js"; import { computeSyncCommitteeCache, getSyncCommitteeCache, SyncCommitteeCache, SyncCommitteeCacheEmpty, } from "./syncCommitteeCache.js"; +import {BaseShufflingCache, IShufflingCache} from "./baseShufflingCache.js"; /** `= PROPOSER_WEIGHT / (WEIGHT_DENOMINATOR - PROPOSER_WEIGHT)` */ export const PROPOSER_WEIGHT_FACTOR = PROPOSER_WEIGHT / (WEIGHT_DENOMINATOR - PROPOSER_WEIGHT); export type EpochCacheImmutableData = { config: BeaconConfig; + shufflingCache: IShufflingCache; pubkey2index: PubkeyIndexMap; index2pubkey: Index2PubkeyCache; }; @@ -54,7 +56,6 @@ export type EpochCacheImmutableData = { export type EpochCacheOpts = { skipSyncCommitteeCache?: boolean; skipSyncPubkeys?: boolean; - shufflingGetter?: ShufflingGetter; }; /** Defers computing proposers by persisting only the seed, and dropping it once indexes are computed */ @@ -84,6 +85,8 @@ type ProposersDeferred = {computed: false; seed: Uint8Array} | {computed: true; **/ export class EpochCache { config: BeaconConfig; + shufflingCache: IShufflingCache; + /** * Unique globally shared pubkey registry. There should only exist one for the entire application. * @@ -119,18 +122,6 @@ export class EpochCache { */ proposersNextEpoch: ProposersDeferred; - /** - * Shuffling of validator indexes. Immutable through the epoch, then it's replaced entirely. - * Note: Per spec definition, shuffling will always be defined. They are never called before loadState() - * - * $VALIDATOR_COUNT x Number - */ - previousShuffling: EpochShuffling; - /** Same as previousShuffling */ - currentShuffling: EpochShuffling; - /** Same as previousShuffling */ - nextShuffling: EpochShuffling; - /** * Validator indexes are used in a few places but and are built separate from * the shufflings so store them on the EpochCache so they are not tied to the @@ -228,14 +219,12 @@ export class EpochCache { constructor(data: { config: BeaconConfig; + shufflingCache: IShufflingCache; pubkey2index: PubkeyIndexMap; index2pubkey: Index2PubkeyCache; proposers: number[]; proposersPrevEpoch: number[] | null; proposersNextEpoch: ProposersDeferred; - previousShuffling: EpochShuffling; - currentShuffling: EpochShuffling; - nextShuffling: EpochShuffling; previousActiveIndices: ValidatorIndex[]; currentActiveIndices: ValidatorIndex[]; nextActiveIndices: ValidatorIndex[]; @@ -260,14 +249,12 @@ export class EpochCache { syncPeriod: SyncPeriod; }) { this.config = data.config; + this.shufflingCache = data.shufflingCache; this.pubkey2index = data.pubkey2index; this.index2pubkey = data.index2pubkey; this.proposers = data.proposers; this.proposersPrevEpoch = data.proposersPrevEpoch; this.proposersNextEpoch = data.proposersNextEpoch; - this.previousShuffling = data.previousShuffling; - this.currentShuffling = data.currentShuffling; - this.nextShuffling = data.nextShuffling; this.previousActiveIndices = data.previousActiveIndices; this.currentActiveIndices = data.currentActiveIndices; this.nextActiveIndices = data.nextActiveIndices; @@ -300,7 +287,7 @@ export class EpochCache { */ static createFromState( state: BeaconStateAllForks, - {config, pubkey2index, index2pubkey}: EpochCacheImmutableData, + {config, shufflingCache, pubkey2index, index2pubkey}: EpochCacheImmutableData, opts?: EpochCacheOpts ): EpochCache { // syncPubkeys here to ensure EpochCacheImmutableData is popualted before computing the rest of caches @@ -330,11 +317,11 @@ export class EpochCache { // BeaconChain could provide a shuffling cache to avoid re-computing shuffling every epoch // in that case, we don't need to compute shufflings again const previousShufflingDecisionRoot = getShufflingDecisionBlock(state, previousEpoch); - const cachedPreviousShuffling = opts?.shufflingGetter?.(previousEpoch, previousShufflingDecisionRoot); + const cachedPreviousShuffling = shufflingCache.getOrNull(previousEpoch, previousShufflingDecisionRoot); const currentShufflingDecisionRoot = getShufflingDecisionBlock(state, currentEpoch); - const cachedCurrentShuffling = opts?.shufflingGetter?.(currentEpoch, currentShufflingDecisionRoot); + const cachedCurrentShuffling = shufflingCache.getOrNull(currentEpoch, currentShufflingDecisionRoot); const nextShufflingDecisionRoot = getShufflingDecisionBlock(state, nextEpoch); - const cachedNextShuffling = opts?.shufflingGetter?.(nextEpoch, nextShufflingDecisionRoot); + const cachedNextShuffling = shufflingCache.getOrNull(nextEpoch, nextShufflingDecisionRoot); for (let i = 0; i < validatorCount; i++) { const validator = validators[i]; @@ -377,11 +364,26 @@ export class EpochCache { throw Error("totalActiveBalanceIncrements >= Number.MAX_SAFE_INTEGER. MAX_EFFECTIVE_BALANCE is too low."); } - const currentShuffling = cachedCurrentShuffling ?? computeEpochShuffling(state, currentActiveIndices, currentEpoch); - const previousShuffling = - cachedPreviousShuffling ?? - (isGenesis ? currentShuffling : computeEpochShuffling(state, previousActiveIndices, previousEpoch)); - const nextShuffling = cachedNextShuffling ?? computeEpochShuffling(state, nextActiveIndices, nextEpoch); + if (!cachedCurrentShuffling) { + // this.metrics?.stateReloadShufflingCacheMiss.inc(); + shufflingCache.buildSync(state, currentEpoch, currentShufflingDecisionRoot, currentActiveIndices); + } + if (!cachedPreviousShuffling) { + // this.metrics?.stateReloadShufflingCacheMiss.inc(); + if (isGenesis) { + shufflingCache.add( + GENESIS_EPOCH, + previousShufflingDecisionRoot, + shufflingCache.getOrNull(currentEpoch, currentShufflingDecisionRoot) as EpochShuffling + ); + } else { + shufflingCache.buildSync(state, previousEpoch, previousShufflingDecisionRoot, previousActiveIndices); + } + } + if (!cachedNextShuffling) { + // this.metrics?.stateReloadShufflingCacheMiss.inc(); + shufflingCache.buildSync(state, nextEpoch, nextShufflingDecisionRoot, nextActiveIndices); + } const currentProposerSeed = getSeed(state, currentEpoch, DOMAIN_BEACON_PROPOSER); @@ -463,15 +465,13 @@ export class EpochCache { return new EpochCache({ config, + shufflingCache, pubkey2index, index2pubkey, proposers, // On first epoch, set to null to prevent unnecessary work since this is only used for metrics proposersPrevEpoch: null, proposersNextEpoch, - previousShuffling, - currentShuffling, - nextShuffling, previousActiveIndices, currentActiveIndices, nextActiveIndices, @@ -506,6 +506,7 @@ export class EpochCache { // All data is completely replaced, or only-appended return new EpochCache({ config: this.config, + shufflingCache: this.shufflingCache, // Common append-only structures shared with all states, no need to clone pubkey2index: this.pubkey2index, index2pubkey: this.index2pubkey, @@ -513,9 +514,6 @@ export class EpochCache { proposers: this.proposers, proposersPrevEpoch: this.proposersPrevEpoch, proposersNextEpoch: this.proposersNextEpoch, - previousShuffling: this.previousShuffling, - currentShuffling: this.currentShuffling, - nextShuffling: this.nextShuffling, previousActiveIndices: this.previousActiveIndices, currentActiveIndices: this.currentActiveIndices, nextActiveIndices: this.nextActiveIndices, @@ -572,14 +570,7 @@ export class EpochCache { this.nextActiveIndices = epochTransitionCache.nextEpochShufflingActiveValidatorIndices; this.nextShufflingDecisionRoot = getShufflingDecisionBlock(state, this.nextEpoch); - - this.previousShuffling = this.currentShuffling; - this.currentShuffling = this.nextShuffling; - this.nextShuffling = computeEpochShuffling( - state, - epochTransitionCache.nextEpochShufflingActiveValidatorIndices, - this.nextEpoch - ); + this.shufflingCache.buildSync(state, this.nextEpoch, this.nextShufflingDecisionRoot, this.nextActiveIndices); // Roll current proposers into previous proposers for metrics this.proposersPrevEpoch = this.proposers; @@ -825,39 +816,29 @@ export class EpochCache { this.index2pubkey[index] = bls.PublicKey.fromBytes(pubkey, CoordType.jacobian); // Optimize for aggregation } - getShufflingAtSlot(slot: Slot): EpochShuffling { - const epoch = computeEpochAtSlot(slot); - return this.getShufflingAtEpoch(epoch); - } - - getShufflingAtSlotOrNull(slot: Slot): EpochShuffling | null { - const epoch = computeEpochAtSlot(slot); - return this.getShufflingAtEpochOrNull(epoch); - } - - getShufflingAtEpoch(epoch: Epoch): EpochShuffling { - const shuffling = this.getShufflingAtEpochOrNull(epoch); - if (shuffling === null) { + getShufflingDecisionRootAtEpoch(epoch: Epoch): string { + if (epoch === this.previousEpoch) { + return this.previousShufflingDecisionRoot; + } else if (epoch === this.epoch) { + return this.currentShufflingDecisionRoot; + } else if (epoch === this.nextEpoch) { + return this.nextShufflingDecisionRoot; + } else { throw new EpochCacheError({ - code: EpochCacheErrorCode.COMMITTEE_EPOCH_OUT_OF_RANGE, + code: EpochCacheErrorCode.NO_SHUFFLING_DECISION_ROOT, currentEpoch: this.epoch, requestedEpoch: epoch, }); } + } - return shuffling; + getShufflingAtSlot(slot: Slot): EpochShuffling { + const epoch = computeEpochAtSlot(slot); + return this.shufflingCache.getOrError(epoch, this.getShufflingDecisionRootAtEpoch(epoch)); } - getShufflingAtEpochOrNull(epoch: Epoch): EpochShuffling | null { - if (epoch === this.previousEpoch) { - return this.previousShuffling; - } else if (epoch === this.epoch) { - return this.currentShuffling; - } else if (epoch === this.nextEpoch) { - return this.nextShuffling; - } else { - return null; - } + getShufflingAtEpoch(epoch: Epoch): EpochShuffling { + return this.shufflingCache.getOrError(epoch, this.getShufflingDecisionRootAtEpoch(epoch)); } /** @@ -930,6 +911,7 @@ export enum EpochCacheErrorCode { COMMITTEE_EPOCH_OUT_OF_RANGE = "EPOCH_CONTEXT_ERROR_COMMITTEE_EPOCH_OUT_OF_RANGE", NO_SYNC_COMMITTEE = "EPOCH_CONTEXT_ERROR_NO_SYNC_COMMITTEE", PROPOSER_EPOCH_MISMATCH = "EPOCH_CONTEXT_ERROR_PROPOSER_EPOCH_MISMATCH", + NO_SHUFFLING_DECISION_ROOT = "EPOCH_CONTEXT_ERROR_NO_SHUFFLING_DECISION_ROOT", } type EpochCacheErrorType = @@ -951,6 +933,11 @@ type EpochCacheErrorType = code: EpochCacheErrorCode.PROPOSER_EPOCH_MISMATCH; requestedEpoch: Epoch; currentEpoch: Epoch; + } + | { + code: EpochCacheErrorCode.NO_SHUFFLING_DECISION_ROOT; + currentEpoch: Epoch; + requestedEpoch: Epoch; }; export class EpochCacheError extends LodestarError {} @@ -961,6 +948,7 @@ export function createEmptyEpochCacheImmutableData( ): EpochCacheImmutableData { return { config: createBeaconConfig(chainConfig, state.genesisValidatorsRoot), + shufflingCache: new BaseShufflingCache(), // This is a test state, there's no need to have a global shared cache of keys pubkey2index: new PubkeyIndexMap(), index2pubkey: [], diff --git a/packages/state-transition/src/cache/stateCache.ts b/packages/state-transition/src/cache/stateCache.ts index 8b45152a3646..c9ea92e18163 100644 --- a/packages/state-transition/src/cache/stateCache.ts +++ b/packages/state-transition/src/cache/stateCache.ts @@ -187,6 +187,7 @@ export function loadCachedBeaconState; export type BeaconStateAltair = CompositeViewDU; @@ -21,5 +20,3 @@ export type BeaconStateAllForks = | BeaconStateDeneb; export type BeaconStateExecutions = BeaconStateBellatrix | BeaconStateCapella | BeaconStateDeneb; - -export type ShufflingGetter = (shufflingEpoch: Epoch, dependentRoot: RootHex) => EpochShuffling | null; diff --git a/packages/state-transition/src/index.ts b/packages/state-transition/src/index.ts index 0ef460e784af..703e2a30ec84 100644 --- a/packages/state-transition/src/index.ts +++ b/packages/state-transition/src/index.ts @@ -2,6 +2,7 @@ export * from "./stateTransition.js"; export * from "./constants/index.js"; export * from "./util/index.js"; export * from "./signatureSets/index.js"; +export * from "./cache/baseShufflingCache.js"; export type {EpochTransitionStep} from "./epoch/index.js"; export type {BeaconStateTransitionMetrics} from "./metrics.js"; From 53b11eaa4eee192f9809ba0db0e07a8c9f6cf35c Mon Sep 17 00:00:00 2001 From: matthewkeil Date: Fri, 23 Feb 2024 13:26:26 +0800 Subject: [PATCH 06/23] feat: implement BaseShufflingCache --- .../src/chain/blocks/importBlock.ts | 6 --- .../beacon-node/src/chain/genesis/genesis.ts | 2 +- packages/beacon-node/src/chain/interface.ts | 4 +- .../stateCache/persistentCheckpointsCache.ts | 2 +- .../src/node/utils/interop/state.ts | 4 +- packages/beacon-node/src/node/utils/state.ts | 4 +- .../test/e2e/interop/genesisState.test.ts | 4 +- .../test/spec/presets/genesis.test.ts | 5 ++- .../test/utils/cachedBeaconState.ts | 7 +++- .../beacon-node/test/utils/node/beacon.ts | 2 +- packages/beacon-node/test/utils/state.ts | 9 +++++ .../test/utils/validationData/attestation.ts | 6 +-- packages/cli/src/cmds/dev/files.ts | 7 +++- packages/cli/src/cmds/dev/handler.ts | 7 +++- .../state-transition/src/cache/epochCache.ts | 21 ++++++++-- .../state-transition/src/cache/stateCache.ts | 3 ++ .../perf/block/processAttestation.test.ts | 6 +-- packages/state-transition/test/perf/util.ts | 9 +++++ .../perf/util/loadState/loadState.test.ts | 20 ++++++---- .../test/perf/util/shufflings.test.ts | 10 ++--- .../test/unit/cachedBeaconState.test.ts | 40 ++++++------------- .../test/unit/upgradeState.test.ts | 5 +++ .../test/unit/util/cachedBeaconState.test.ts | 8 +++- packages/state-transition/test/utils/state.ts | 7 ++++ 24 files changed, 121 insertions(+), 77 deletions(-) diff --git a/packages/beacon-node/src/chain/blocks/importBlock.ts b/packages/beacon-node/src/chain/blocks/importBlock.ts index 661f1c34d08c..85c8a3663986 100644 --- a/packages/beacon-node/src/chain/blocks/importBlock.ts +++ b/packages/beacon-node/src/chain/blocks/importBlock.ts @@ -349,12 +349,6 @@ export async function importBlock( this.logger.verbose("After importBlock caching postState without SSZ cache", {slot: postState.slot}); } - if (parentEpoch < blockEpoch) { - // current epoch and previous epoch are likely cached in previous states - this.shufflingCache.processState(postState, postState.epochCtx.nextEpoch); - this.logger.verbose("Processed shuffling for next epoch", {parentEpoch, blockEpoch, slot: blockSlot}); - } - if (blockSlot % SLOTS_PER_EPOCH === 0) { // Cache state to preserve epoch transition work const checkpointState = postState; diff --git a/packages/beacon-node/src/chain/genesis/genesis.ts b/packages/beacon-node/src/chain/genesis/genesis.ts index 979476c69530..335f1de18bd6 100644 --- a/packages/beacon-node/src/chain/genesis/genesis.ts +++ b/packages/beacon-node/src/chain/genesis/genesis.ts @@ -86,7 +86,7 @@ export class GenesisBuilder implements IGenesisBuilder { } // TODO - PENDING: Ensure EpochCacheImmutableData is created only once - this.state = createCachedBeaconState(stateView, createEmptyEpochCacheImmutableData(config, stateView)); + this.state = createCachedBeaconState(stateView, createEmptyEpochCacheImmutableData(config, logger, stateView)); this.config = this.state.config; this.activatedValidatorCount = getActiveValidatorIndices(stateView, GENESIS_EPOCH).length; } diff --git a/packages/beacon-node/src/chain/interface.ts b/packages/beacon-node/src/chain/interface.ts index 55f5ebf485a2..d12daf0de7b9 100644 --- a/packages/beacon-node/src/chain/interface.ts +++ b/packages/beacon-node/src/chain/interface.ts @@ -17,6 +17,7 @@ import { BeaconStateAllForks, CachedBeaconStateAllForks, EpochShuffling, + IShufflingCache, Index2PubkeyCache, PubkeyIndexMap, } from "@lodestar/state-transition"; @@ -51,7 +52,6 @@ import {IChainOptions} from "./options.js"; import {AssembledBlockType, BlockAttributes, BlockType} from "./produceBlock/produceBlockBody.js"; import {SeenAttestationDatas} from "./seenCache/seenAttestationData.js"; import {SeenGossipBlockInput} from "./seenCache/index.js"; -import {ShufflingCache} from "./shufflingCache.js"; import {BlockRewards} from "./rewards/blockRewards.js"; import {SyncCommitteeRewards} from "./rewards/syncCommitteeRewards.js"; @@ -114,7 +114,7 @@ export interface IBeaconChain { readonly checkpointBalancesCache: CheckpointBalancesCache; readonly producedContentsCache: Map; readonly producedBlockRoot: Map; - readonly shufflingCache: ShufflingCache; + readonly shufflingCache: IShufflingCache; readonly producedBlindedBlockRoot: Set; readonly opts: IChainOptions; diff --git a/packages/beacon-node/src/chain/stateCache/persistentCheckpointsCache.ts b/packages/beacon-node/src/chain/stateCache/persistentCheckpointsCache.ts index 47e32b19dede..a86633d5c1f9 100644 --- a/packages/beacon-node/src/chain/stateCache/persistentCheckpointsCache.ts +++ b/packages/beacon-node/src/chain/stateCache/persistentCheckpointsCache.ts @@ -215,7 +215,7 @@ export class PersistentCheckpointStateCache implements CheckpointStateCache { sszTimer?.(); const timer = this.metrics?.stateReloadDuration.startTimer(); // @tuyennhv why are we not passing through the EpochCacheOptions from the seed state? - const newCachedState = loadCachedBeaconState(seedState, stateBytes, {}, validatorsBytes); + const newCachedState = loadCachedBeaconState(seedState, stateBytes, this.logger, {}, validatorsBytes); newCachedState.commit(); const stateRoot = toHexString(newCachedState.hashTreeRoot()); timer?.(); diff --git a/packages/beacon-node/src/node/utils/interop/state.ts b/packages/beacon-node/src/node/utils/interop/state.ts index f3efc9894587..d9fd5cb51774 100644 --- a/packages/beacon-node/src/node/utils/interop/state.ts +++ b/packages/beacon-node/src/node/utils/interop/state.ts @@ -4,6 +4,7 @@ import {BeaconStateAllForks, initializeBeaconStateFromEth1} from "@lodestar/stat import {createEmptyEpochCacheImmutableData} from "@lodestar/state-transition"; import {ForkName, GENESIS_SLOT} from "@lodestar/params"; +import {Logger} from "@lodestar/utils"; import {DepositTree} from "../../../db/repositories/depositDataRoot.js"; export const INTEROP_BLOCK_HASH = Buffer.alloc(32, "B"); @@ -23,6 +24,7 @@ export type InteropStateOpts = { export function getInteropState( config: ChainForkConfig, + logger: Logger, { genesisTime = Math.floor(Date.now() / 1000), eth1BlockHash = INTEROP_BLOCK_HASH, @@ -45,7 +47,7 @@ export function getInteropState( latestPayloadHeader.baseFeePerGas = GENESIS_BASE_FEE_PER_GAS; const state = initializeBeaconStateFromEth1( config, - createEmptyEpochCacheImmutableData(config, {genesisValidatorsRoot: Buffer.alloc(32, 0)}), + createEmptyEpochCacheImmutableData(config, logger, {genesisValidatorsRoot: Buffer.alloc(32, 0)}), eth1BlockHash, eth1Timestamp, deposits, diff --git a/packages/beacon-node/src/node/utils/state.ts b/packages/beacon-node/src/node/utils/state.ts index 25bd77c82274..31f8f4e82bf4 100644 --- a/packages/beacon-node/src/node/utils/state.ts +++ b/packages/beacon-node/src/node/utils/state.ts @@ -1,12 +1,14 @@ import {ChainForkConfig} from "@lodestar/config"; import {BeaconStateAllForks} from "@lodestar/state-transition"; import {phase0, ssz} from "@lodestar/types"; +import {Logger} from "@lodestar/utils"; import {IBeaconDb} from "../../db/index.js"; import {interopDeposits} from "./interop/deposits.js"; import {getInteropState, InteropStateOpts} from "./interop/state.js"; export function initDevState( config: ChainForkConfig, + logger: Logger, validatorCount: number, interopStateOpts: InteropStateOpts ): {deposits: phase0.Deposit[]; state: BeaconStateAllForks} { @@ -16,7 +18,7 @@ export function initDevState( validatorCount, interopStateOpts ); - const state = getInteropState(config, interopStateOpts, deposits); + const state = getInteropState(config, logger, interopStateOpts, deposits); return {deposits, state}; } diff --git a/packages/beacon-node/test/e2e/interop/genesisState.test.ts b/packages/beacon-node/test/e2e/interop/genesisState.test.ts index 2287c6a1deb8..07b206026c97 100644 --- a/packages/beacon-node/test/e2e/interop/genesisState.test.ts +++ b/packages/beacon-node/test/e2e/interop/genesisState.test.ts @@ -2,6 +2,8 @@ import {describe, it, expect} from "vitest"; import {toHexString} from "@chainsafe/ssz"; import {config} from "@lodestar/config/default"; import {ssz} from "@lodestar/types"; +import {getNodeLogger} from "@lodestar/logger/node"; +import {LogLevel} from "@lodestar/utils"; import {initDevState} from "../../../src/node/utils/state.js"; import {interopDeposits} from "../../../src/node/utils/interop/deposits.js"; @@ -60,7 +62,7 @@ describe("interop / initDevState", () => { it("Create correct genesisState", () => { const validatorCount = 8; - const {state} = initDevState(config, validatorCount, { + const {state} = initDevState(config, getNodeLogger({level: LogLevel.info}), validatorCount, { genesisTime: 1644000000, eth1BlockHash: Buffer.alloc(32, 0xaa), eth1Timestamp: 1644000000, diff --git a/packages/beacon-node/test/spec/presets/genesis.test.ts b/packages/beacon-node/test/spec/presets/genesis.test.ts index ba3351a2103c..f9b342846722 100644 --- a/packages/beacon-node/test/spec/presets/genesis.test.ts +++ b/packages/beacon-node/test/spec/presets/genesis.test.ts @@ -8,10 +8,11 @@ import { initializeBeaconStateFromEth1, isValidGenesisState, } from "@lodestar/state-transition"; -import {bnToNum} from "@lodestar/utils"; +import {LogLevel, bnToNum} from "@lodestar/utils"; import {ForkName} from "@lodestar/params"; import {ACTIVE_PRESET} from "@lodestar/params"; +import {getNodeLogger} from "@lodestar/logger/node"; import {expectEqualBeaconState} from "../utils/expectEqualBeaconState.js"; import {TestRunnerFn} from "../utils/types.js"; import {getConfig} from "../../utils/config.js"; @@ -42,7 +43,7 @@ const genesisInitialization: TestRunnerFn( state: T, chainConfig: ChainForkConfig ): T & BeaconStateCache { - return createCachedBeaconState(state, createEmptyEpochCacheImmutableData(chainConfig, state)); + return createCachedBeaconState( + state, + createEmptyEpochCacheImmutableData(chainConfig, getNodeLogger({level: LogLevel.info}), state) + ); } diff --git a/packages/beacon-node/test/utils/node/beacon.ts b/packages/beacon-node/test/utils/node/beacon.ts index 0163fa148102..d4486a6ab201 100644 --- a/packages/beacon-node/test/utils/node/beacon.ts +++ b/packages/beacon-node/test/utils/node/beacon.ts @@ -70,7 +70,7 @@ export async function getDevBeaconNode( let anchorState = opts.anchorState; if (!anchorState) { - const {state, deposits} = initDevState(config, validatorCount, opts); + const {state, deposits} = initDevState(config, logger, validatorCount, opts); anchorState = state; // Is it necessary to persist deposits and genesis block? diff --git a/packages/beacon-node/test/utils/state.ts b/packages/beacon-node/test/utils/state.ts index a0fa42be555e..5773d43709f4 100644 --- a/packages/beacon-node/test/utils/state.ts +++ b/packages/beacon-node/test/utils/state.ts @@ -7,12 +7,15 @@ import { PubkeyIndexMap, CachedBeaconStateBellatrix, BeaconStateBellatrix, + BaseShufflingCache, } from "@lodestar/state-transition"; import {allForks, altair, bellatrix, ssz} from "@lodestar/types"; import {createBeaconConfig, ChainForkConfig} from "@lodestar/config"; import {FAR_FUTURE_EPOCH, ForkName, ForkSeq, MAX_EFFECTIVE_BALANCE, SYNC_COMMITTEE_SIZE} from "@lodestar/params"; import {ExecutionStatus, ProtoBlock} from "@lodestar/fork-choice"; +import {getNodeLogger} from "@lodestar/logger/node"; +import {LogLevel} from "@lodestar/utils"; import {ZERO_HASH_HEX} from "../../src/constants/constants.js"; import {generateValidator, generateValidators} from "./validator.js"; import {getConfig} from "./config.js"; @@ -103,6 +106,8 @@ export function generateCachedState(opts?: TestBeaconState): CachedBeaconStateAl const state = generateState(opts, config); return createCachedBeaconState(state, { config: createBeaconConfig(config, state.genesisValidatorsRoot), + logger: getNodeLogger({level: LogLevel.debug}), + shufflingCache: new BaseShufflingCache(), // This is a performance test, there's no need to have a global shared cache of keys pubkey2index: new PubkeyIndexMap(), index2pubkey: [], @@ -117,6 +122,8 @@ export function generateCachedAltairState(opts?: TestBeaconState, altairForkEpoc const state = generateState(opts, config); return createCachedBeaconState(state, { config: createBeaconConfig(config, state.genesisValidatorsRoot), + logger: getNodeLogger({level: LogLevel.debug}), + shufflingCache: new BaseShufflingCache(), // This is a performance test, there's no need to have a global shared cache of keys pubkey2index: new PubkeyIndexMap(), index2pubkey: [], @@ -131,6 +138,8 @@ export function generateCachedBellatrixState(opts?: TestBeaconState): CachedBeac const state = generateState(opts, config); return createCachedBeaconState(state as BeaconStateBellatrix, { config: createBeaconConfig(config, state.genesisValidatorsRoot), + logger: getNodeLogger({level: LogLevel.debug}), + shufflingCache: new BaseShufflingCache(), // This is a performance test, there's no need to have a global shared cache of keys pubkey2index: new PubkeyIndexMap(), index2pubkey: [], diff --git a/packages/beacon-node/test/utils/validationData/attestation.ts b/packages/beacon-node/test/utils/validationData/attestation.ts index d3788e55ced3..04121be95856 100644 --- a/packages/beacon-node/test/utils/validationData/attestation.ts +++ b/packages/beacon-node/test/utils/validationData/attestation.ts @@ -73,10 +73,6 @@ export function getAttestationValidData(opts: AttestationValidDataOpts): { ...{executionPayloadBlockHash: null, executionStatus: ExecutionStatus.PreMerge}, }; - const shufflingCache = new ShufflingCache(); - shufflingCache.processState(state, state.epochCtx.epoch); - shufflingCache.processState(state, state.epochCtx.nextEpoch); - const forkChoice = { getBlock: (root) => { if (!ssz.Root.equals(root, beaconBlockRoot)) return null; @@ -139,7 +135,7 @@ export function getAttestationValidData(opts: AttestationValidDataOpts): { : new BlsMultiThreadWorkerPool({}, {logger: testLogger(), metrics: null}), waitForBlock: () => Promise.resolve(false), index2pubkey: state.epochCtx.index2pubkey, - shufflingCache, + shufflingCache: state.epochCtx.shufflingCache, opts: defaultChainOptions, } as Partial as IBeaconChain; diff --git a/packages/cli/src/cmds/dev/files.ts b/packages/cli/src/cmds/dev/files.ts index 9baf0dc845dd..f99f94ae548e 100644 --- a/packages/cli/src/cmds/dev/files.ts +++ b/packages/cli/src/cmds/dev/files.ts @@ -3,8 +3,9 @@ import path from "node:path"; import {Keystore} from "@chainsafe/bls-keystore"; import {nodeUtils} from "@lodestar/beacon-node"; import {chainConfigToJson, ChainForkConfig} from "@lodestar/config"; -import {dumpYaml} from "@lodestar/utils"; +import {dumpYaml, LogLevel} from "@lodestar/utils"; import {interopSecretKey} from "@lodestar/state-transition"; +import {getNodeLogger} from "@lodestar/logger/node"; import {PersistedKeysBackend} from "../validator/keymanager/persistedKeys.js"; /* eslint-disable no-console */ @@ -17,7 +18,9 @@ export async function writeTestnetFiles( const genesisTime = Math.floor(Date.now() / 1000); const eth1BlockHash = Buffer.alloc(32, 0); - const {state} = nodeUtils.initDevState(config, genesisValidators, {genesisTime, eth1BlockHash}); + // dummy logger to init state for serialization + const logger = getNodeLogger({level: LogLevel.info}); + const {state} = nodeUtils.initDevState(config, logger, genesisValidators, {genesisTime, eth1BlockHash}); // Write testnet data fs.mkdirSync(targetDir, {recursive: true}); diff --git a/packages/cli/src/cmds/dev/handler.ts b/packages/cli/src/cmds/dev/handler.ts index 3018bb8790a6..b13ad61301ff 100644 --- a/packages/cli/src/cmds/dev/handler.ts +++ b/packages/cli/src/cmds/dev/handler.ts @@ -1,7 +1,8 @@ import fs from "node:fs"; import {rimraf} from "rimraf"; -import {toHex, fromHex} from "@lodestar/utils"; +import {toHex, fromHex, LogLevel} from "@lodestar/utils"; import {nodeUtils} from "@lodestar/beacon-node"; +import {getNodeLogger} from "@lodestar/logger/lib/node.js"; import {GlobalArgs} from "../../options/index.js"; import {mkdir, onGracefulShutdown} from "../../util/index.js"; import {getBeaconConfigFromArgs} from "../../config/beaconParams.js"; @@ -59,8 +60,10 @@ export async function devHandler(args: IDevArgs & GlobalArgs): Promise { const validatorCount = args.genesisValidators ?? 8; const genesisTime = args.genesisTime ?? Math.floor(Date.now() / 1000) + 5; const eth1BlockHash = fromHex(args.genesisEth1Hash ?? toHex(Buffer.alloc(32, 0x0b))); + // dummy logger to init state for serialization + const logger = getNodeLogger({level: LogLevel.info}); - const {state} = nodeUtils.initDevState(config, validatorCount, {genesisTime, eth1BlockHash}); + const {state} = nodeUtils.initDevState(config, logger, validatorCount, {genesisTime, eth1BlockHash}); args.genesisStateFile = "genesis.ssz"; fs.writeFileSync(args.genesisStateFile, state.serialize()); diff --git a/packages/state-transition/src/cache/epochCache.ts b/packages/state-transition/src/cache/epochCache.ts index 7410054ba71c..6d6f462d4ac3 100644 --- a/packages/state-transition/src/cache/epochCache.ts +++ b/packages/state-transition/src/cache/epochCache.ts @@ -13,7 +13,7 @@ import { SLOTS_PER_EPOCH, WEIGHT_DENOMINATOR, } from "@lodestar/params"; -import {LodestarError} from "@lodestar/utils"; +import {LodestarError, Logger} from "@lodestar/utils"; import { computeActivationExitEpoch, computeEpochAtSlot, @@ -48,6 +48,7 @@ export const PROPOSER_WEIGHT_FACTOR = PROPOSER_WEIGHT / (WEIGHT_DENOMINATOR - PR export type EpochCacheImmutableData = { config: BeaconConfig; + logger: Logger; shufflingCache: IShufflingCache; pubkey2index: PubkeyIndexMap; index2pubkey: Index2PubkeyCache; @@ -85,6 +86,7 @@ type ProposersDeferred = {computed: false; seed: Uint8Array} | {computed: true; **/ export class EpochCache { config: BeaconConfig; + logger: Logger; shufflingCache: IShufflingCache; /** @@ -219,6 +221,7 @@ export class EpochCache { constructor(data: { config: BeaconConfig; + logger: Logger; shufflingCache: IShufflingCache; pubkey2index: PubkeyIndexMap; index2pubkey: Index2PubkeyCache; @@ -249,6 +252,7 @@ export class EpochCache { syncPeriod: SyncPeriod; }) { this.config = data.config; + this.logger = data.logger; this.shufflingCache = data.shufflingCache; this.pubkey2index = data.pubkey2index; this.index2pubkey = data.index2pubkey; @@ -287,7 +291,7 @@ export class EpochCache { */ static createFromState( state: BeaconStateAllForks, - {config, shufflingCache, pubkey2index, index2pubkey}: EpochCacheImmutableData, + {config, logger, shufflingCache, pubkey2index, index2pubkey}: EpochCacheImmutableData, opts?: EpochCacheOpts ): EpochCache { // syncPubkeys here to ensure EpochCacheImmutableData is popualted before computing the rest of caches @@ -465,6 +469,7 @@ export class EpochCache { return new EpochCache({ config, + logger, shufflingCache, pubkey2index, index2pubkey, @@ -506,6 +511,7 @@ export class EpochCache { // All data is completely replaced, or only-appended return new EpochCache({ config: this.config, + logger: this.logger, shufflingCache: this.shufflingCache, // Common append-only structures shared with all states, no need to clone pubkey2index: this.pubkey2index, @@ -570,7 +576,14 @@ export class EpochCache { this.nextActiveIndices = epochTransitionCache.nextEpochShufflingActiveValidatorIndices; this.nextShufflingDecisionRoot = getShufflingDecisionBlock(state, this.nextEpoch); - this.shufflingCache.buildSync(state, this.nextEpoch, this.nextShufflingDecisionRoot, this.nextActiveIndices); + this.shufflingCache + .build(state, this.nextEpoch, this.nextShufflingDecisionRoot, this.nextActiveIndices) + .catch((e) => + this.logger.error( + `Error building shuffling cache for epoch ${this.nextEpoch} on decisionRoot ${this.nextShufflingDecisionRoot}`, + e + ) + ); // Roll current proposers into previous proposers for metrics this.proposersPrevEpoch = this.proposers; @@ -944,10 +957,12 @@ export class EpochCacheError extends LodestarError {} export function createEmptyEpochCacheImmutableData( chainConfig: ChainConfig, + logger: Logger, state: Pick ): EpochCacheImmutableData { return { config: createBeaconConfig(chainConfig, state.genesisValidatorsRoot), + logger, shufflingCache: new BaseShufflingCache(), // This is a test state, there's no need to have a global shared cache of keys pubkey2index: new PubkeyIndexMap(), diff --git a/packages/state-transition/src/cache/stateCache.ts b/packages/state-transition/src/cache/stateCache.ts index c9ea92e18163..4f3cd4976e82 100644 --- a/packages/state-transition/src/cache/stateCache.ts +++ b/packages/state-transition/src/cache/stateCache.ts @@ -1,6 +1,7 @@ import bls from "@chainsafe/bls"; import {CoordType} from "@chainsafe/bls/types"; import {BeaconConfig} from "@lodestar/config"; +import {Logger} from "@lodestar/utils"; import {loadState} from "../util/loadState/loadState.js"; import {EpochCache, EpochCacheImmutableData, EpochCacheOpts} from "./epochCache.js"; import { @@ -164,6 +165,7 @@ export function createCachedBeaconState( export function loadCachedBeaconState( cachedSeedState: T, stateBytes: Uint8Array, + logger: Logger, opts?: EpochCacheOpts, seedValidatorsBytes?: Uint8Array ): T { @@ -187,6 +189,7 @@ export function loadCachedBeaconState }, fn: (state) => { const {currentEpochParticipation} = state; - const numAttesters = Math.floor( - (state.epochCtx.currentShuffling.activeIndices.length * ratio) / SLOTS_PER_EPOCH - ); + const numAttesters = Math.floor((state.epochCtx.currentActiveIndices.length * ratio) / SLOTS_PER_EPOCH); // just get committees of slot 10 let count = 0; - for (const committees of state.epochCtx.currentShuffling.committees[10]) { + for (const committees of state.epochCtx.getShufflingAtSlot(state.slot).committees[10]) { for (const committee of committees) { currentEpochParticipation.set(committee, 0b111); count++; diff --git a/packages/state-transition/test/perf/util.ts b/packages/state-transition/test/perf/util.ts index 4df6746ea938..213239265740 100644 --- a/packages/state-transition/test/perf/util.ts +++ b/packages/state-transition/test/perf/util.ts @@ -1,6 +1,7 @@ import {CoordType, PublicKey, SecretKey} from "@chainsafe/bls/types"; import bls from "@chainsafe/bls"; import {BitArray, fromHexString} from "@chainsafe/ssz"; +import {getNodeLogger} from "@lodestar/logger/node"; import {allForks, phase0, ssz, Slot, altair} from "@lodestar/types"; import {config} from "@lodestar/config/default"; import {createBeaconConfig, createChainForkConfig} from "@lodestar/config"; @@ -12,6 +13,7 @@ import { SLOTS_PER_EPOCH, SLOTS_PER_HISTORICAL_ROOT, } from "@lodestar/params"; +import {LogLevel} from "@lodestar/utils"; import { interopSecretKey, computeEpochAtSlot, @@ -20,6 +22,7 @@ import { newFilledArray, createCachedBeaconState, computeCommitteeCount, + BaseShufflingCache, } from "../../src/index.js"; import { CachedBeaconStateAllForks, @@ -127,6 +130,8 @@ export function generatePerfTestCachedStatePhase0(opts?: {goBackOneSlot: boolean state.slot -= 1; phase0CachedState23637 = createCachedBeaconState(state, { config: createBeaconConfig(config, state.genesisValidatorsRoot), + logger: getNodeLogger({level: LogLevel.info}), + shufflingCache: new BaseShufflingCache(), pubkey2index, index2pubkey, }); @@ -232,6 +237,8 @@ export function generatePerfTestCachedStateAltair(opts?: { state.slot -= 1; altairCachedState23637 = createCachedBeaconState(state, { config: createBeaconConfig(altairConfig, state.genesisValidatorsRoot), + logger: getNodeLogger({level: LogLevel.info}), + shufflingCache: new BaseShufflingCache(), pubkey2index, index2pubkey, }); @@ -435,6 +442,8 @@ export function generateTestCachedBeaconStateOnlyValidators({ state, { config: createBeaconConfig(config, state.genesisValidatorsRoot), + logger: getNodeLogger({level: LogLevel.info}), + shufflingCache: new BaseShufflingCache(), pubkey2index, index2pubkey, }, diff --git a/packages/state-transition/test/perf/util/loadState/loadState.test.ts b/packages/state-transition/test/perf/util/loadState/loadState.test.ts index 5d40c64f6ab4..b1dc7254088a 100644 --- a/packages/state-transition/test/perf/util/loadState/loadState.test.ts +++ b/packages/state-transition/test/perf/util/loadState/loadState.test.ts @@ -1,10 +1,13 @@ import bls from "@chainsafe/bls"; import {CoordType} from "@chainsafe/bls/types"; import {itBench, setBenchOpts} from "@dapplion/benchmark"; +import {getNodeLogger} from "@lodestar/logger/node"; +import {LogLevel} from "@lodestar/utils"; import {loadState} from "../../../../src/util/loadState/loadState.js"; import {createCachedBeaconState} from "../../../../src/cache/stateCache.js"; import {Index2PubkeyCache, PubkeyIndexMap} from "../../../../src/cache/pubkeyCache.js"; import {generatePerfTestCachedStateAltair} from "../../util.js"; +import {BaseShufflingCache} from "../../../../src/index.js"; /** * This benchmark shows a stable performance from 2s to 3s on a Mac M1. And it does not really depend on the seed validators, @@ -62,12 +65,14 @@ describe("loadState", function () { } const newStateBytes = newState.serialize(); - return {seedState, newStateBytes}; + const logger = getNodeLogger({level: LogLevel.error}); + const shufflingCache = new BaseShufflingCache(); + return {seedState, newStateBytes, logger, shufflingCache}; }, - beforeEach: ({seedState, newStateBytes}) => { - return {seedState: seedState.clone(), newStateBytes}; + beforeEach: ({seedState, newStateBytes, logger, shufflingCache}) => { + return {seedState: seedState.clone(), newStateBytes, logger, shufflingCache}; }, - fn: ({seedState, newStateBytes}) => { + fn: ({seedState, newStateBytes, logger, shufflingCache}) => { const {state: migratedState, modifiedValidators} = loadState(seedState.config, seedState, newStateBytes); migratedState.hashTreeRoot(); // Get the validators sub tree once for all the loop @@ -80,17 +85,16 @@ describe("loadState", function () { pubkey2index.set(pubkey, validatorIndex); index2pubkey[validatorIndex] = bls.PublicKey.fromBytes(pubkey, CoordType.jacobian); } - // skip computimg shuffling in performance test because in reality we have a ShufflingCache - // eslint-disable-next-line @typescript-eslint/explicit-function-return-type - const shufflingGetter = () => seedState.epochCtx.currentShuffling; createCachedBeaconState( migratedState, { config: seedState.config, + logger, + shufflingCache, pubkey2index, index2pubkey, }, - {skipSyncPubkeys: true, skipSyncCommitteeCache: true, shufflingGetter} + {skipSyncPubkeys: true, skipSyncCommitteeCache: true} ); }, }); diff --git a/packages/state-transition/test/perf/util/shufflings.test.ts b/packages/state-transition/test/perf/util/shufflings.test.ts index 98a2bb4e5564..a16a3b05f3d3 100644 --- a/packages/state-transition/test/perf/util/shufflings.test.ts +++ b/packages/state-transition/test/perf/util/shufflings.test.ts @@ -31,7 +31,7 @@ describe("epoch shufflings", () => { computeProposers( state.epochCtx.nextEpoch, epochSeed, - state.epochCtx.nextShuffling.activeIndices, + state.epochCtx.nextActiveIndices, state.epochCtx.effectiveBalanceIncrements ); }, @@ -40,18 +40,14 @@ describe("epoch shufflings", () => { itBench({ id: `computeEpochShuffling - vc ${numValidators}`, fn: () => { - computeEpochShuffling(state, state.epochCtx.nextShuffling.activeIndices, nextEpoch); + computeEpochShuffling(state, state.epochCtx.nextActiveIndices, nextEpoch); }, }); itBench({ id: `getNextSyncCommittee - vc ${numValidators}`, fn: () => { - getNextSyncCommittee( - state, - state.epochCtx.nextShuffling.activeIndices, - state.epochCtx.effectiveBalanceIncrements - ); + getNextSyncCommittee(state, state.epochCtx.nextActiveIndices, state.epochCtx.effectiveBalanceIncrements); }, }); }); diff --git a/packages/state-transition/test/unit/cachedBeaconState.test.ts b/packages/state-transition/test/unit/cachedBeaconState.test.ts index 2891cd3e6216..26702cbc2ae9 100644 --- a/packages/state-transition/test/unit/cachedBeaconState.test.ts +++ b/packages/state-transition/test/unit/cachedBeaconState.test.ts @@ -1,14 +1,17 @@ import {describe, it, expect} from "vitest"; import {Epoch, ssz, RootHex} from "@lodestar/types"; -import {toHexString} from "@lodestar/utils"; +import {LogLevel, toHexString} from "@lodestar/utils"; import {config as defaultConfig} from "@lodestar/config/default"; import {createBeaconConfig} from "@lodestar/config"; +import {getNodeLogger} from "@lodestar/logger/node"; import {createCachedBeaconStateTest} from "../utils/state.js"; import {PubkeyIndexMap} from "../../src/cache/pubkeyCache.js"; import {createCachedBeaconState, loadCachedBeaconState} from "../../src/cache/stateCache.js"; import {interopPubkeysCached} from "../utils/interop.js"; import {modifyStateSameValidator, newStateWithValidators} from "../utils/capella.js"; -import {EpochShuffling, getShufflingDecisionBlock} from "../../src/util/epochShuffling.js"; +import {EpochShuffling} from "../../src/util/epochShuffling.js"; +import {getShufflingDecisionBlock} from "../../src/util/shufflingDecisionRoot.js"; +import {BaseShufflingCache} from "../../src/cache/baseShufflingCache.js"; describe("CachedBeaconState", () => { it("Clone and mutate", () => { @@ -59,10 +62,13 @@ describe("CachedBeaconState", () => { const stateView = newStateWithValidators(numValidator); const config = createBeaconConfig(defaultConfig, stateView.genesisValidatorsRoot); + const logger = getNodeLogger({level: LogLevel.info}); const seedState = createCachedBeaconState( stateView, { config, + logger, + shufflingCache: new BaseShufflingCache(), pubkey2index: new PubkeyIndexMap(), index2pubkey: [], }, @@ -129,42 +135,20 @@ describe("CachedBeaconState", () => { // confirm loadState() result const stateBytes = state.serialize(); - const newCachedState = loadCachedBeaconState(seedState, stateBytes, {skipSyncCommitteeCache: true}); + const newCachedState = loadCachedBeaconState(seedState, stateBytes, logger, {skipSyncCommitteeCache: true}); const newStateBytes = newCachedState.serialize(); expect(newStateBytes).toEqual(stateBytes); expect(newCachedState.hashTreeRoot()).toEqual(state.hashTreeRoot()); - const shufflingGetter = (shufflingEpoch: Epoch, dependentRoot: RootHex): EpochShuffling | null => { - if ( - shufflingEpoch === seedState.epochCtx.epoch - 1 && - dependentRoot === getShufflingDecisionBlock(seedState, shufflingEpoch) - ) { - return seedState.epochCtx.previousShuffling; - } - - if ( - shufflingEpoch === seedState.epochCtx.epoch && - dependentRoot === getShufflingDecisionBlock(seedState, shufflingEpoch) - ) { - return seedState.epochCtx.currentShuffling; - } - - if ( - shufflingEpoch === seedState.epochCtx.epoch + 1 && - dependentRoot === getShufflingDecisionBlock(seedState, shufflingEpoch) - ) { - return seedState.epochCtx.nextShuffling; - } - - return null; - }; const cachedState = createCachedBeaconState( state, { config, + logger, + shufflingCache: seedState.epochCtx.shufflingCache, pubkey2index: new PubkeyIndexMap(), index2pubkey: [], }, - {skipSyncCommitteeCache: true, shufflingGetter} + {skipSyncCommitteeCache: true} ); // validatorCountDelta < 0 is unrealistic and shuffling computation results in a different result if (validatorCountDelta >= 0) { diff --git a/packages/state-transition/test/unit/upgradeState.test.ts b/packages/state-transition/test/unit/upgradeState.test.ts index 2ea8eef182ac..bd8588b3aa12 100644 --- a/packages/state-transition/test/unit/upgradeState.test.ts +++ b/packages/state-transition/test/unit/upgradeState.test.ts @@ -3,10 +3,13 @@ import {ssz} from "@lodestar/types"; import {ForkName} from "@lodestar/params"; import {createBeaconConfig, ChainForkConfig, createChainForkConfig} from "@lodestar/config"; import {config as chainConfig} from "@lodestar/config/default"; +import {getNodeLogger} from "@lodestar/logger/node"; +import {LogLevel} from "@lodestar/utils"; import {upgradeStateToDeneb} from "../../src/slot/upgradeStateToDeneb.js"; import {createCachedBeaconState} from "../../src/cache/stateCache.js"; import {PubkeyIndexMap} from "../../src/cache/pubkeyCache.js"; +import {BaseShufflingCache} from "../../src/cache/baseShufflingCache.js"; describe("upgradeState", () => { it("upgradeStateToDeneb", () => { @@ -16,6 +19,8 @@ describe("upgradeState", () => { capellaState, { config: createBeaconConfig(config, capellaState.genesisValidatorsRoot), + logger: getNodeLogger({level: LogLevel.error}), + shufflingCache: new BaseShufflingCache(), pubkey2index: new PubkeyIndexMap(), index2pubkey: [], }, diff --git a/packages/state-transition/test/unit/util/cachedBeaconState.test.ts b/packages/state-transition/test/unit/util/cachedBeaconState.test.ts index 654e0752adb8..6cbd75e69522 100644 --- a/packages/state-transition/test/unit/util/cachedBeaconState.test.ts +++ b/packages/state-transition/test/unit/util/cachedBeaconState.test.ts @@ -2,7 +2,11 @@ import {describe, it} from "vitest"; import {createBeaconConfig} from "@lodestar/config"; import {config} from "@lodestar/config/default"; import {ssz} from "@lodestar/types"; -import {createCachedBeaconState, PubkeyIndexMap} from "../../../src/index.js"; +import {getNodeLogger} from "@lodestar/logger/node"; +import {LogLevel} from "@lodestar/utils"; +import {BaseShufflingCache} from "../../../src/cache/baseShufflingCache.js"; +import {PubkeyIndexMap} from "../../../src/cache/pubkeyCache.js"; +import {createCachedBeaconState} from "../../../src/cache/stateCache.js"; describe("CachedBeaconState", () => { it("Create empty CachedBeaconState", () => { @@ -10,6 +14,8 @@ describe("CachedBeaconState", () => { createCachedBeaconState(emptyState, { config: createBeaconConfig(config, emptyState.genesisValidatorsRoot), + logger: getNodeLogger({level: LogLevel.info}), + shufflingCache: new BaseShufflingCache(), pubkey2index: new PubkeyIndexMap(), index2pubkey: [], }); diff --git a/packages/state-transition/test/utils/state.ts b/packages/state-transition/test/utils/state.ts index 29a1f98b5562..c1e3c1e349d4 100644 --- a/packages/state-transition/test/utils/state.ts +++ b/packages/state-transition/test/utils/state.ts @@ -10,6 +10,8 @@ import {phase0, ssz} from "@lodestar/types"; import {config} from "@lodestar/config/default"; import {createBeaconConfig, ChainForkConfig} from "@lodestar/config"; +import {getNodeLogger} from "@lodestar/logger/node"; +import {LogLevel} from "@lodestar/utils"; import {ZERO_HASH} from "../../src/constants/index.js"; import {newZeroedArray} from "../../src/util/index.js"; @@ -22,6 +24,7 @@ import { } from "../../src/index.js"; import {BeaconStateCache} from "../../src/cache/stateCache.js"; import {EpochCacheOpts} from "../../src/cache/epochCache.js"; +import {BaseShufflingCache} from "../../src/cache/baseShufflingCache.js"; /** * Copy of BeaconState, but all fields are marked optional to allow for swapping out variables as needed. @@ -91,6 +94,8 @@ export function generateCachedState( const state = generateState(opts); return createCachedBeaconState(state, { config: createBeaconConfig(config, state.genesisValidatorsRoot), + logger: getNodeLogger({level: LogLevel.error}), + shufflingCache: new BaseShufflingCache(), // This is a test state, there's no need to have a global shared cache of keys pubkey2index: new PubkeyIndexMap(), index2pubkey: [], @@ -106,6 +111,8 @@ export function createCachedBeaconStateTest( state, { config: createBeaconConfig(configCustom, state.genesisValidatorsRoot), + logger: getNodeLogger({level: LogLevel.error}), + shufflingCache: new BaseShufflingCache(), // This is a test state, there's no need to have a global shared cache of keys pubkey2index: new PubkeyIndexMap(), index2pubkey: [], From e778316c219dfed68fffac76f7c280524ae0eb0e Mon Sep 17 00:00:00 2001 From: matthewkeil Date: Fri, 23 Feb 2024 14:01:17 +0800 Subject: [PATCH 07/23] refactor: rename BaseShufflingCache --- packages/beacon-node/src/chain/options.ts | 2 +- .../stateCache/persistentCheckpointsCache.ts | 10 ++- packages/beacon-node/test/utils/state.ts | 8 +- .../state-transition/src/cache/epochCache.ts | 4 +- ...aseShufflingCache.ts => shufflingCache.ts} | 86 +++++++++++++------ packages/state-transition/src/index.ts | 2 +- packages/state-transition/test/perf/util.ts | 8 +- .../perf/util/loadState/loadState.test.ts | 4 +- .../test/unit/cachedBeaconState.test.ts | 4 +- .../test/unit/upgradeState.test.ts | 4 +- .../test/unit/util/cachedBeaconState.test.ts | 4 +- packages/state-transition/test/utils/state.ts | 6 +- 12 files changed, 88 insertions(+), 54 deletions(-) rename packages/state-transition/src/cache/{baseShufflingCache.ts => shufflingCache.ts} (74%) diff --git a/packages/beacon-node/src/chain/options.ts b/packages/beacon-node/src/chain/options.ts index 671347d5e980..a7b7cd8e899b 100644 --- a/packages/beacon-node/src/chain/options.ts +++ b/packages/beacon-node/src/chain/options.ts @@ -1,9 +1,9 @@ import {SAFE_SLOTS_TO_IMPORT_OPTIMISTICALLY} from "@lodestar/params"; import {defaultOptions as defaultValidatorOptions} from "@lodestar/validator"; +import {ShufflingCacheOptions} from "@lodestar/state-transition"; import {ArchiverOpts} from "./archiver/index.js"; import {ForkChoiceOpts} from "./forkChoice/index.js"; import {LightClientServerOpts} from "./lightClient/index.js"; -import {ShufflingCacheOptions} from "./shufflingCache.js"; export type IChainOptions = BlockProcessOpts & PoolOpts & diff --git a/packages/beacon-node/src/chain/stateCache/persistentCheckpointsCache.ts b/packages/beacon-node/src/chain/stateCache/persistentCheckpointsCache.ts index a86633d5c1f9..88c6a2ff1fb7 100644 --- a/packages/beacon-node/src/chain/stateCache/persistentCheckpointsCache.ts +++ b/packages/beacon-node/src/chain/stateCache/persistentCheckpointsCache.ts @@ -1,13 +1,17 @@ import {fromHexString, toHexString} from "@chainsafe/ssz"; import {phase0, Epoch, RootHex} from "@lodestar/types"; -import {CachedBeaconStateAllForks, computeStartSlotAtEpoch, getBlockRootAtSlot} from "@lodestar/state-transition"; +import { + loadCachedBeaconState, + ShufflingCache, + CachedBeaconStateAllForks, + computeStartSlotAtEpoch, + getBlockRootAtSlot, +} from "@lodestar/state-transition"; import {Logger, MapDef, sleep} from "@lodestar/utils"; import {routes} from "@lodestar/api"; -import {loadCachedBeaconState} from "@lodestar/state-transition"; import {INTERVALS_PER_SLOT} from "@lodestar/params"; import {Metrics} from "../../metrics/index.js"; import {IClock} from "../../util/clock.js"; -import {ShufflingCache} from "../shufflingCache.js"; import {BufferPool, BufferWithKey} from "../../util/bufferPool.js"; import {StateCloneOpts} from "../regen/interface.js"; import {MapTracker} from "./mapMetrics.js"; diff --git a/packages/beacon-node/test/utils/state.ts b/packages/beacon-node/test/utils/state.ts index 5773d43709f4..29aaed79f82b 100644 --- a/packages/beacon-node/test/utils/state.ts +++ b/packages/beacon-node/test/utils/state.ts @@ -7,7 +7,7 @@ import { PubkeyIndexMap, CachedBeaconStateBellatrix, BeaconStateBellatrix, - BaseShufflingCache, + ShufflingCache, } from "@lodestar/state-transition"; import {allForks, altair, bellatrix, ssz} from "@lodestar/types"; import {createBeaconConfig, ChainForkConfig} from "@lodestar/config"; @@ -107,7 +107,7 @@ export function generateCachedState(opts?: TestBeaconState): CachedBeaconStateAl return createCachedBeaconState(state, { config: createBeaconConfig(config, state.genesisValidatorsRoot), logger: getNodeLogger({level: LogLevel.debug}), - shufflingCache: new BaseShufflingCache(), + shufflingCache: new ShufflingCache(), // This is a performance test, there's no need to have a global shared cache of keys pubkey2index: new PubkeyIndexMap(), index2pubkey: [], @@ -123,7 +123,7 @@ export function generateCachedAltairState(opts?: TestBeaconState, altairForkEpoc return createCachedBeaconState(state, { config: createBeaconConfig(config, state.genesisValidatorsRoot), logger: getNodeLogger({level: LogLevel.debug}), - shufflingCache: new BaseShufflingCache(), + shufflingCache: new ShufflingCache(), // This is a performance test, there's no need to have a global shared cache of keys pubkey2index: new PubkeyIndexMap(), index2pubkey: [], @@ -139,7 +139,7 @@ export function generateCachedBellatrixState(opts?: TestBeaconState): CachedBeac return createCachedBeaconState(state as BeaconStateBellatrix, { config: createBeaconConfig(config, state.genesisValidatorsRoot), logger: getNodeLogger({level: LogLevel.debug}), - shufflingCache: new BaseShufflingCache(), + shufflingCache: new ShufflingCache(), // This is a performance test, there's no need to have a global shared cache of keys pubkey2index: new PubkeyIndexMap(), index2pubkey: [], diff --git a/packages/state-transition/src/cache/epochCache.ts b/packages/state-transition/src/cache/epochCache.ts index 6d6f462d4ac3..88084cc70de9 100644 --- a/packages/state-transition/src/cache/epochCache.ts +++ b/packages/state-transition/src/cache/epochCache.ts @@ -41,7 +41,7 @@ import { SyncCommitteeCache, SyncCommitteeCacheEmpty, } from "./syncCommitteeCache.js"; -import {BaseShufflingCache, IShufflingCache} from "./baseShufflingCache.js"; +import {ShufflingCache, IShufflingCache} from "./shufflingCache.js"; /** `= PROPOSER_WEIGHT / (WEIGHT_DENOMINATOR - PROPOSER_WEIGHT)` */ export const PROPOSER_WEIGHT_FACTOR = PROPOSER_WEIGHT / (WEIGHT_DENOMINATOR - PROPOSER_WEIGHT); @@ -963,7 +963,7 @@ export function createEmptyEpochCacheImmutableData( return { config: createBeaconConfig(chainConfig, state.genesisValidatorsRoot), logger, - shufflingCache: new BaseShufflingCache(), + shufflingCache: new ShufflingCache(), // This is a test state, there's no need to have a global shared cache of keys pubkey2index: new PubkeyIndexMap(), index2pubkey: [], diff --git a/packages/state-transition/src/cache/baseShufflingCache.ts b/packages/state-transition/src/cache/shufflingCache.ts similarity index 74% rename from packages/state-transition/src/cache/baseShufflingCache.ts rename to packages/state-transition/src/cache/shufflingCache.ts index 27f19b6680fb..c90849055918 100644 --- a/packages/state-transition/src/cache/baseShufflingCache.ts +++ b/packages/state-transition/src/cache/shufflingCache.ts @@ -1,4 +1,5 @@ import {Epoch, RootHex} from "@lodestar/types"; +import type {Metrics} from "@lodestar/beacon-node"; import {LodestarError, MapDef, pruneSetToMax} from "@lodestar/utils"; import {EpochShuffling, computeEpochShuffling} from "../util/index.js"; import {BeaconStateAllForks} from "./types.js"; @@ -20,6 +21,7 @@ const SHUFFLING_CACHE_MAX_EPOCHS = 4; export interface IShufflingCache { add(shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex, shuffling: EpochShuffling): void; get(shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex): Promise; + // getAll(): MapDef>; getOrNull(shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex): EpochShuffling | null; getOrError(shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex): EpochShuffling; buildSync( @@ -38,11 +40,13 @@ export interface IShufflingCache { export enum ShufflingCacheErrorCode { NO_SHUFFLING_FOUND = "EPOCH_SHUFFLING_NO_SHUFFLING_FOUND", + REGEN_ERROR_NO_SHUFFLING_FOUND = "REGEN_ERROR_NO_SHUFFLING_FOUND", SHUFFLING_PROMISE_NOT_RESOLVED = "EPOCH_SHUFFLING_SHUFFLING_PROMISE_NOT_RESOLVED", } type ShufflingCacheErrorType = | {code: ShufflingCacheErrorCode.NO_SHUFFLING_FOUND; epoch: Epoch; shufflingDecisionRoot: RootHex} + | {code: ShufflingCacheErrorCode.REGEN_ERROR_NO_SHUFFLING_FOUND; epoch: Epoch; shufflingDecisionRoot: RootHex} | {code: ShufflingCacheErrorCode.SHUFFLING_PROMISE_NOT_RESOLVED; epoch: Epoch; shufflingDecisionRoot: RootHex}; export class ShufflingCacheError extends LodestarError {} @@ -67,19 +71,39 @@ export type ShufflingCacheItem = ShufflingCacheShufflingItem | ShufflingCachePro type ShufflingResolution = (shuffling: EpochShuffling) => void; -export interface BaseShufflingCacheOptions { +export interface ShufflingCacheOptions { maxShufflingCacheEpochs?: number; } -export class BaseShufflingCache implements IShufflingCache { +/** + * A shuffling cache to help: + * - get committee quickly for attestation verification + * - if a shuffling is not available (which does not happen with default chain option of maxSkipSlots = 32), track a promise to make sure we don't compute the same shuffling twice + * - skip computing shuffling when loading state bytes from disk + */ +export class ShufflingCache implements IShufflingCache { /** LRU cache implemented as a map, pruned every time we add an item */ - protected readonly itemsByDecisionRootByEpoch: MapDef> = new MapDef( + private readonly itemsByDecisionRootByEpoch: MapDef> = new MapDef( () => new Map() ); - protected readonly maxEpochs: number; + private readonly maxEpochs: number; - constructor(opts: BaseShufflingCacheOptions = {}) { + constructor( + private metrics: Metrics | null = null, + opts: ShufflingCacheOptions = {} + ) { this.maxEpochs = opts.maxShufflingCacheEpochs ?? SHUFFLING_CACHE_MAX_EPOCHS; + if (metrics) { + metrics.shufflingCache.size.addCollect(() => + metrics.shufflingCache.size.set( + Array.from(this.itemsByDecisionRootByEpoch.values()).reduce((total, innerMap) => total + innerMap.size, 0) + ) + ); + } + } + + addMetrics(metrics: Metrics): void { + this.metrics = metrics; } async get(shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex): Promise { @@ -135,18 +159,22 @@ export class BaseShufflingCache implements IShufflingCache { activeIndexes: number[] ): EpochShuffling { const cacheItem = this.itemsByDecisionRootByEpoch.getOrDefault(shufflingEpoch).get(shufflingDecisionRoot); - if (!cacheItem) { - // TODO: (matthewkeil) Add metric here for cache miss - return this._build(state, shufflingEpoch, shufflingDecisionRoot, activeIndexes); - } - if (this.isShufflingCacheItem(cacheItem)) { + let resolveFn: ShufflingResolution; + if (cacheItem) { // TODO: (matthewkeil) Add metric here for cache hit - return cacheItem.shuffling; + if (this.isShufflingCacheItem(cacheItem)) { + return cacheItem.shuffling; + } + // Perhaps we should throw an error instead. Should not happen ideally + // + // TODO: (matthewkeil) Add metric here for race condition recreating the shuffling + // because this sync call will finish first if its running on thread + resolveFn = cacheItem.resolveFn; + } else { + // TODO: (matthewkeil) Add metric here for cache miss + resolveFn = this._insertShufflingPromise(shufflingEpoch, shufflingDecisionRoot); } - // Perhaps we should throw an error instead - // - // TODO: (matthewkeil) Add metric here for throwing away and recreating the shuffling - const resolveFn = cacheItem.resolveFn; + const shuffling = this._build(state, shufflingEpoch, shufflingDecisionRoot, activeIndexes); resolveFn(shuffling); return shuffling; @@ -159,27 +187,29 @@ export class BaseShufflingCache implements IShufflingCache { activeIndexes: number[] ): Promise { const cacheItem = this.itemsByDecisionRootByEpoch.getOrDefault(shufflingEpoch).get(shufflingDecisionRoot); - if (!!cacheItem && this.isShufflingCacheItem(cacheItem)) { + if (cacheItem) { // TODO: (matthewkeil) Add metric here for cache hit - return cacheItem.shuffling; + if (this.isShufflingCacheItem(cacheItem)) { + return cacheItem.shuffling; + } + return cacheItem.promise; } // TODO: (matthewkeil) Add metric here for cache miss - let resolveFn: ShufflingResolution; - if (!cacheItem) { - resolveFn = this._insertShufflingPromise(shufflingEpoch, shufflingDecisionRoot); - } else { - resolveFn = cacheItem.resolveFn; - } - + // + // this is to prevent multiple calls to get shuffling for the same epoch and dependent root + // any subsequent calls of the same epoch and dependent root will wait for this promise to resolve + const resolveFn = this._insertShufflingPromise(shufflingEpoch, shufflingDecisionRoot); // TODO: (matthewkeil) replace this sync call with a worker and async build function that uses // a nice'd thread to build in core idle time + // + // Building will overwrite the ShufflingCachePromiseItem with the ShufflingCacheShufflingItem const shuffling = this._build(state, shufflingEpoch, shufflingDecisionRoot, activeIndexes); resolveFn(shuffling); return shuffling; } - protected _build( + private _build( state: BeaconStateAllForks, shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex, @@ -190,7 +220,7 @@ export class BaseShufflingCache implements IShufflingCache { return shuffling; } - protected _insertShufflingPromise(shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex): ShufflingResolution { + private _insertShufflingPromise(shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex): ShufflingResolution { const promiseCount = Array.from(this.itemsByDecisionRootByEpoch.values()) .flatMap((innerMap) => Array.from(innerMap.values())) .filter((item) => this.isPromiseCacheItem(item)).length; @@ -209,11 +239,11 @@ export class BaseShufflingCache implements IShufflingCache { return resolveFn; } - protected isShufflingCacheItem(item: ShufflingCacheItem): item is ShufflingCacheShufflingItem { + private isShufflingCacheItem(item: ShufflingCacheItem): item is ShufflingCacheShufflingItem { return item.type === ShufflingCacheItemType.shuffling; } - protected isPromiseCacheItem(item: ShufflingCacheItem): item is ShufflingCachePromiseItem { + private isPromiseCacheItem(item: ShufflingCacheItem): item is ShufflingCachePromiseItem { return item.type === ShufflingCacheItemType.promise; } } diff --git a/packages/state-transition/src/index.ts b/packages/state-transition/src/index.ts index 703e2a30ec84..37bd92637830 100644 --- a/packages/state-transition/src/index.ts +++ b/packages/state-transition/src/index.ts @@ -2,7 +2,7 @@ export * from "./stateTransition.js"; export * from "./constants/index.js"; export * from "./util/index.js"; export * from "./signatureSets/index.js"; -export * from "./cache/baseShufflingCache.js"; +export * from "./cache/shufflingCache.js"; export type {EpochTransitionStep} from "./epoch/index.js"; export type {BeaconStateTransitionMetrics} from "./metrics.js"; diff --git a/packages/state-transition/test/perf/util.ts b/packages/state-transition/test/perf/util.ts index 213239265740..1ea729f92e1d 100644 --- a/packages/state-transition/test/perf/util.ts +++ b/packages/state-transition/test/perf/util.ts @@ -22,7 +22,7 @@ import { newFilledArray, createCachedBeaconState, computeCommitteeCount, - BaseShufflingCache, + ShufflingCache, } from "../../src/index.js"; import { CachedBeaconStateAllForks, @@ -131,7 +131,7 @@ export function generatePerfTestCachedStatePhase0(opts?: {goBackOneSlot: boolean phase0CachedState23637 = createCachedBeaconState(state, { config: createBeaconConfig(config, state.genesisValidatorsRoot), logger: getNodeLogger({level: LogLevel.info}), - shufflingCache: new BaseShufflingCache(), + shufflingCache: new ShufflingCache(), pubkey2index, index2pubkey, }); @@ -238,7 +238,7 @@ export function generatePerfTestCachedStateAltair(opts?: { altairCachedState23637 = createCachedBeaconState(state, { config: createBeaconConfig(altairConfig, state.genesisValidatorsRoot), logger: getNodeLogger({level: LogLevel.info}), - shufflingCache: new BaseShufflingCache(), + shufflingCache: new ShufflingCache(), pubkey2index, index2pubkey, }); @@ -443,7 +443,7 @@ export function generateTestCachedBeaconStateOnlyValidators({ { config: createBeaconConfig(config, state.genesisValidatorsRoot), logger: getNodeLogger({level: LogLevel.info}), - shufflingCache: new BaseShufflingCache(), + shufflingCache: new ShufflingCache(), pubkey2index, index2pubkey, }, diff --git a/packages/state-transition/test/perf/util/loadState/loadState.test.ts b/packages/state-transition/test/perf/util/loadState/loadState.test.ts index b1dc7254088a..1a6eaaf1e20a 100644 --- a/packages/state-transition/test/perf/util/loadState/loadState.test.ts +++ b/packages/state-transition/test/perf/util/loadState/loadState.test.ts @@ -7,7 +7,7 @@ import {loadState} from "../../../../src/util/loadState/loadState.js"; import {createCachedBeaconState} from "../../../../src/cache/stateCache.js"; import {Index2PubkeyCache, PubkeyIndexMap} from "../../../../src/cache/pubkeyCache.js"; import {generatePerfTestCachedStateAltair} from "../../util.js"; -import {BaseShufflingCache} from "../../../../src/index.js"; +import {ShufflingCache} from "../../../../src/index.js"; /** * This benchmark shows a stable performance from 2s to 3s on a Mac M1. And it does not really depend on the seed validators, @@ -66,7 +66,7 @@ describe("loadState", function () { const newStateBytes = newState.serialize(); const logger = getNodeLogger({level: LogLevel.error}); - const shufflingCache = new BaseShufflingCache(); + const shufflingCache = new ShufflingCache(); return {seedState, newStateBytes, logger, shufflingCache}; }, beforeEach: ({seedState, newStateBytes, logger, shufflingCache}) => { diff --git a/packages/state-transition/test/unit/cachedBeaconState.test.ts b/packages/state-transition/test/unit/cachedBeaconState.test.ts index 26702cbc2ae9..efad7efd0555 100644 --- a/packages/state-transition/test/unit/cachedBeaconState.test.ts +++ b/packages/state-transition/test/unit/cachedBeaconState.test.ts @@ -11,7 +11,7 @@ import {interopPubkeysCached} from "../utils/interop.js"; import {modifyStateSameValidator, newStateWithValidators} from "../utils/capella.js"; import {EpochShuffling} from "../../src/util/epochShuffling.js"; import {getShufflingDecisionBlock} from "../../src/util/shufflingDecisionRoot.js"; -import {BaseShufflingCache} from "../../src/cache/baseShufflingCache.js"; +import {ShufflingCache} from "../../src/cache/shufflingCache.js"; describe("CachedBeaconState", () => { it("Clone and mutate", () => { @@ -68,7 +68,7 @@ describe("CachedBeaconState", () => { { config, logger, - shufflingCache: new BaseShufflingCache(), + shufflingCache: new ShufflingCache(), pubkey2index: new PubkeyIndexMap(), index2pubkey: [], }, diff --git a/packages/state-transition/test/unit/upgradeState.test.ts b/packages/state-transition/test/unit/upgradeState.test.ts index bd8588b3aa12..d2fe87a73a33 100644 --- a/packages/state-transition/test/unit/upgradeState.test.ts +++ b/packages/state-transition/test/unit/upgradeState.test.ts @@ -9,7 +9,7 @@ import {LogLevel} from "@lodestar/utils"; import {upgradeStateToDeneb} from "../../src/slot/upgradeStateToDeneb.js"; import {createCachedBeaconState} from "../../src/cache/stateCache.js"; import {PubkeyIndexMap} from "../../src/cache/pubkeyCache.js"; -import {BaseShufflingCache} from "../../src/cache/baseShufflingCache.js"; +import {ShufflingCache} from "../../src/cache/shufflingCache.js"; describe("upgradeState", () => { it("upgradeStateToDeneb", () => { @@ -20,7 +20,7 @@ describe("upgradeState", () => { { config: createBeaconConfig(config, capellaState.genesisValidatorsRoot), logger: getNodeLogger({level: LogLevel.error}), - shufflingCache: new BaseShufflingCache(), + shufflingCache: new ShufflingCache(), pubkey2index: new PubkeyIndexMap(), index2pubkey: [], }, diff --git a/packages/state-transition/test/unit/util/cachedBeaconState.test.ts b/packages/state-transition/test/unit/util/cachedBeaconState.test.ts index 6cbd75e69522..6fb441d955d8 100644 --- a/packages/state-transition/test/unit/util/cachedBeaconState.test.ts +++ b/packages/state-transition/test/unit/util/cachedBeaconState.test.ts @@ -4,7 +4,7 @@ import {config} from "@lodestar/config/default"; import {ssz} from "@lodestar/types"; import {getNodeLogger} from "@lodestar/logger/node"; import {LogLevel} from "@lodestar/utils"; -import {BaseShufflingCache} from "../../../src/cache/baseShufflingCache.js"; +import {ShufflingCache} from "../../../src/cache/shufflingCache.js"; import {PubkeyIndexMap} from "../../../src/cache/pubkeyCache.js"; import {createCachedBeaconState} from "../../../src/cache/stateCache.js"; @@ -15,7 +15,7 @@ describe("CachedBeaconState", () => { createCachedBeaconState(emptyState, { config: createBeaconConfig(config, emptyState.genesisValidatorsRoot), logger: getNodeLogger({level: LogLevel.info}), - shufflingCache: new BaseShufflingCache(), + shufflingCache: new ShufflingCache(), pubkey2index: new PubkeyIndexMap(), index2pubkey: [], }); diff --git a/packages/state-transition/test/utils/state.ts b/packages/state-transition/test/utils/state.ts index c1e3c1e349d4..3e1b33ec7097 100644 --- a/packages/state-transition/test/utils/state.ts +++ b/packages/state-transition/test/utils/state.ts @@ -24,7 +24,7 @@ import { } from "../../src/index.js"; import {BeaconStateCache} from "../../src/cache/stateCache.js"; import {EpochCacheOpts} from "../../src/cache/epochCache.js"; -import {BaseShufflingCache} from "../../src/cache/baseShufflingCache.js"; +import {ShufflingCache} from "../../src/cache/shufflingCache.js"; /** * Copy of BeaconState, but all fields are marked optional to allow for swapping out variables as needed. @@ -95,7 +95,7 @@ export function generateCachedState( return createCachedBeaconState(state, { config: createBeaconConfig(config, state.genesisValidatorsRoot), logger: getNodeLogger({level: LogLevel.error}), - shufflingCache: new BaseShufflingCache(), + shufflingCache: new ShufflingCache(), // This is a test state, there's no need to have a global shared cache of keys pubkey2index: new PubkeyIndexMap(), index2pubkey: [], @@ -112,7 +112,7 @@ export function createCachedBeaconStateTest( { config: createBeaconConfig(configCustom, state.genesisValidatorsRoot), logger: getNodeLogger({level: LogLevel.error}), - shufflingCache: new BaseShufflingCache(), + shufflingCache: new ShufflingCache(), // This is a test state, there's no need to have a global shared cache of keys pubkey2index: new PubkeyIndexMap(), index2pubkey: [], From 0b23e0e374795d2f5d8b967e19f36e729573c128 Mon Sep 17 00:00:00 2001 From: matthewkeil Date: Tue, 5 Mar 2024 18:27:48 +0800 Subject: [PATCH 08/23] feat: add ShufflingCache metrics --- .../state-transition/src/cache/epochCache.ts | 40 ++-- .../src/cache/shufflingCache.ts | 171 ++++++++++++------ 2 files changed, 139 insertions(+), 72 deletions(-) diff --git a/packages/state-transition/src/cache/epochCache.ts b/packages/state-transition/src/cache/epochCache.ts index 88084cc70de9..ff91adf77bf3 100644 --- a/packages/state-transition/src/cache/epochCache.ts +++ b/packages/state-transition/src/cache/epochCache.ts @@ -41,7 +41,7 @@ import { SyncCommitteeCache, SyncCommitteeCacheEmpty, } from "./syncCommitteeCache.js"; -import {ShufflingCache, IShufflingCache} from "./shufflingCache.js"; +import {ShufflingCache, IShufflingCache, ShufflingCacheCaller} from "./shufflingCache.js"; /** `= PROPOSER_WEIGHT / (WEIGHT_DENOMINATOR - PROPOSER_WEIGHT)` */ export const PROPOSER_WEIGHT_FACTOR = PROPOSER_WEIGHT / (WEIGHT_DENOMINATOR - PROPOSER_WEIGHT); @@ -55,6 +55,7 @@ export type EpochCacheImmutableData = { }; export type EpochCacheOpts = { + isReload?: boolean; skipSyncCommitteeCache?: boolean; skipSyncPubkeys?: boolean; }; @@ -321,11 +322,15 @@ export class EpochCache { // BeaconChain could provide a shuffling cache to avoid re-computing shuffling every epoch // in that case, we don't need to compute shufflings again const previousShufflingDecisionRoot = getShufflingDecisionBlock(state, previousEpoch); - const cachedPreviousShuffling = shufflingCache.getOrNull(previousEpoch, previousShufflingDecisionRoot); + const cachedPreviousShuffling = shufflingCache.getOrNull( + previousEpoch, + previousShufflingDecisionRoot, + opts?.isReload + ); const currentShufflingDecisionRoot = getShufflingDecisionBlock(state, currentEpoch); - const cachedCurrentShuffling = shufflingCache.getOrNull(currentEpoch, currentShufflingDecisionRoot); + const cachedCurrentShuffling = shufflingCache.getOrNull(currentEpoch, currentShufflingDecisionRoot, opts?.isReload); const nextShufflingDecisionRoot = getShufflingDecisionBlock(state, nextEpoch); - const cachedNextShuffling = shufflingCache.getOrNull(nextEpoch, nextShufflingDecisionRoot); + const cachedNextShuffling = shufflingCache.getOrNull(nextEpoch, nextShufflingDecisionRoot, opts?.isReload); for (let i = 0; i < validatorCount; i++) { const validator = validators[i]; @@ -369,23 +374,14 @@ export class EpochCache { } if (!cachedCurrentShuffling) { - // this.metrics?.stateReloadShufflingCacheMiss.inc(); shufflingCache.buildSync(state, currentEpoch, currentShufflingDecisionRoot, currentActiveIndices); } if (!cachedPreviousShuffling) { - // this.metrics?.stateReloadShufflingCacheMiss.inc(); - if (isGenesis) { - shufflingCache.add( - GENESIS_EPOCH, - previousShufflingDecisionRoot, - shufflingCache.getOrNull(currentEpoch, currentShufflingDecisionRoot) as EpochShuffling - ); - } else { + if (!isGenesis) { shufflingCache.buildSync(state, previousEpoch, previousShufflingDecisionRoot, previousActiveIndices); } } if (!cachedNextShuffling) { - // this.metrics?.stateReloadShufflingCacheMiss.inc(); shufflingCache.buildSync(state, nextEpoch, nextShufflingDecisionRoot, nextActiveIndices); } @@ -650,7 +646,9 @@ export class EpochCache { * Return the beacon committee at slot for index. */ getBeaconCommittee(slot: Slot, index: CommitteeIndex): Uint32Array { - const slotCommittees = this.getShufflingAtSlot(slot).committees[slot % SLOTS_PER_EPOCH]; + const slotCommittees = this.getShufflingAtSlot(slot, ShufflingCacheCaller.getBeaconCommittee).committees[ + slot % SLOTS_PER_EPOCH + ]; if (index >= slotCommittees.length) { throw new EpochCacheError({ code: EpochCacheErrorCode.COMMITTEE_INDEX_OUT_OF_RANGE, @@ -662,7 +660,7 @@ export class EpochCache { } getCommitteeCountPerSlot(epoch: Epoch): number { - return this.getShufflingAtEpoch(epoch).committeesPerSlot; + return this.getShufflingAtEpoch(epoch, ShufflingCacheCaller.getCommitteeCountPerSlot).committeesPerSlot; } /** @@ -766,7 +764,7 @@ export class EpochCache { const requestedValidatorIndicesSet = new Set(requestedValidatorIndices); const duties = new Map(); - const epochCommittees = this.getShufflingAtEpoch(epoch).committees; + const epochCommittees = this.getShufflingAtEpoch(epoch, ShufflingCacheCaller.getCommitteeAssignments).committees; for (let epochSlot = 0; epochSlot < SLOTS_PER_EPOCH; epochSlot++) { const slotCommittees = epochCommittees[epochSlot]; for (let i = 0, committeesAtSlot = slotCommittees.length; i < committeesAtSlot; i++) { @@ -845,13 +843,13 @@ export class EpochCache { } } - getShufflingAtSlot(slot: Slot): EpochShuffling { + getShufflingAtSlot(slot: Slot, caller: ShufflingCacheCaller): EpochShuffling { const epoch = computeEpochAtSlot(slot); - return this.shufflingCache.getOrError(epoch, this.getShufflingDecisionRootAtEpoch(epoch)); + return this.shufflingCache.getOrError(epoch, this.getShufflingDecisionRootAtEpoch(epoch), caller); } - getShufflingAtEpoch(epoch: Epoch): EpochShuffling { - return this.shufflingCache.getOrError(epoch, this.getShufflingDecisionRootAtEpoch(epoch)); + getShufflingAtEpoch(epoch: Epoch, caller: ShufflingCacheCaller): EpochShuffling { + return this.shufflingCache.getOrError(epoch, this.getShufflingDecisionRootAtEpoch(epoch), caller); } /** diff --git a/packages/state-transition/src/cache/shufflingCache.ts b/packages/state-transition/src/cache/shufflingCache.ts index c90849055918..a84c9c4cde82 100644 --- a/packages/state-transition/src/cache/shufflingCache.ts +++ b/packages/state-transition/src/cache/shufflingCache.ts @@ -1,6 +1,5 @@ import {Epoch, RootHex} from "@lodestar/types"; -import type {Metrics} from "@lodestar/beacon-node"; -import {LodestarError, MapDef, pruneSetToMax} from "@lodestar/utils"; +import {GaugeExtra, LodestarError, MapDef, NoLabels, pruneSetToMax} from "@lodestar/utils"; import {EpochShuffling, computeEpochShuffling} from "../util/index.js"; import {BeaconStateAllForks} from "./types.js"; @@ -8,7 +7,7 @@ import {BeaconStateAllForks} from "./types.js"; * With default chain option of maxSkipSlots = 32, there should be no shuffling promise. If that happens a lot, it could blow up Lodestar, * with MAX_EPOCHS = 4, only allow 2 promise at a time. Note that regen already bounds number of concurrent requests at 1 already. */ -const MAX_PROMISES = 2; +export const SHUFFLING_CACHE_MAX_PROMISES = 2; /** * Same value to CheckpointBalancesCache, with the assumption that we don't have to use it for old epochs. In the worse case: @@ -16,14 +15,26 @@ const MAX_PROMISES = 2; * - don't have shuffling to verify attestations, need to do 1 epoch transition to add shuffling to this cache. This never happens * with default chain option of maxSkipSlots = 32 **/ -const SHUFFLING_CACHE_MAX_EPOCHS = 4; +export const SHUFFLING_CACHE_MAX_EPOCHS = 4; + +export enum ShufflingCacheCaller { + testing = "testing", + buildShuffling = "buildShuffling", + attestationVerification = "attestationVerification", + createFromState = "createFromState", + getBeaconCommittee = "getBeaconCommittee", + getEpochCommittees = "getEpochCommittees", + getAttestationsForBlock = "getAttestationsForBlock", + getCommitteeCountPerSlot = "getCommitteeCountPerSlot", + getCommitteeAssignments = "getCommitteeAssignments", +} export interface IShufflingCache { add(shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex, shuffling: EpochShuffling): void; get(shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex): Promise; // getAll(): MapDef>; - getOrNull(shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex): EpochShuffling | null; - getOrError(shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex): EpochShuffling; + getOrNull(shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex, isReload?: boolean): EpochShuffling | null; + getOrError(shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex, caller: ShufflingCacheCaller): EpochShuffling; buildSync( state: BeaconStateAllForks, shufflingEpoch: Epoch, @@ -38,6 +49,22 @@ export interface IShufflingCache { ): Promise; } +export interface ShufflingCacheMetrics { + shufflingCache: { + size: GaugeExtra; + + cacheMiss: GaugeExtra<{caller: ShufflingCacheCaller}>; + cacheMissUnresolvedPromise: GaugeExtra<{caller: ShufflingCacheCaller}>; + cacheHit: GaugeExtra<{caller: ShufflingCacheCaller}>; + cacheHitUnresolvedPromise: GaugeExtra<{caller: ShufflingCacheCaller}>; + cacheHitRebuildPromise: GaugeExtra; + + reloadCacheMiss: GaugeExtra; + reloadCacheMissUnresolvedPromise: GaugeExtra; + reloadCacheHit: GaugeExtra; + }; +} + export enum ShufflingCacheErrorCode { NO_SHUFFLING_FOUND = "EPOCH_SHUFFLING_NO_SHUFFLING_FOUND", REGEN_ERROR_NO_SHUFFLING_FOUND = "REGEN_ERROR_NO_SHUFFLING_FOUND", @@ -87,62 +114,67 @@ export class ShufflingCache implements IShufflingCache { () => new Map() ); private readonly maxEpochs: number; + private metrics: ShufflingCacheMetrics | null = null; - constructor( - private metrics: Metrics | null = null, - opts: ShufflingCacheOptions = {} - ) { + constructor(metrics: ShufflingCacheMetrics | null = null, opts: ShufflingCacheOptions = {}) { this.maxEpochs = opts.maxShufflingCacheEpochs ?? SHUFFLING_CACHE_MAX_EPOCHS; - if (metrics) { - metrics.shufflingCache.size.addCollect(() => - metrics.shufflingCache.size.set( - Array.from(this.itemsByDecisionRootByEpoch.values()).reduce((total, innerMap) => total + innerMap.size, 0) - ) - ); - } + this.addMetrics(metrics); } - addMetrics(metrics: Metrics): void { - this.metrics = metrics; + addMetrics(metrics: ShufflingCacheMetrics | null): void { + if (!this.metrics) { + this.metrics = metrics; + if (metrics) { + metrics.shufflingCache.size.addCollect(() => + metrics.shufflingCache.size.set( + Array.from(this.itemsByDecisionRootByEpoch.values()).reduce((total, innerMap) => total + innerMap.size, 0) + ) + ); + } + } } + /** + * Used for attestation verifications. Will immediately return a shuffling if it is available, + * otherwise it will return the promise for the shuffling and the consumer will need to wait for + * it to be calculated. Consumer await covers both cases. If shuffling is not available returns + * null and does not attempt to compute shuffling. + */ async get(shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex): Promise { const cacheItem = this.itemsByDecisionRootByEpoch.getOrDefault(shufflingEpoch).get(shufflingDecisionRoot); if (cacheItem === undefined) { + this.metrics?.shufflingCache.cacheMiss.inc({caller: ShufflingCacheCaller.attestationVerification}); return null; } if (this.isShufflingCacheItem(cacheItem)) { + this.metrics?.shufflingCache.cacheHit.inc({ + caller: ShufflingCacheCaller.attestationVerification, + }); return cacheItem.shuffling; } else { + this.metrics?.shufflingCache.cacheHitUnresolvedPromise.inc({ + caller: ShufflingCacheCaller.attestationVerification, + }); return cacheItem.promise; } } - getOrError(shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex): EpochShuffling { - const cacheItem = this.itemsByDecisionRootByEpoch.getOrDefault(shufflingEpoch).get(shufflingDecisionRoot); - if (cacheItem === undefined) { - throw new ShufflingCacheError({ - code: ShufflingCacheErrorCode.NO_SHUFFLING_FOUND, - epoch: shufflingEpoch, - shufflingDecisionRoot, - }); - } - if (this.isPromiseCacheItem(cacheItem)) { - throw new ShufflingCacheError({ - code: ShufflingCacheErrorCode.SHUFFLING_PROMISE_NOT_RESOLVED, - epoch: shufflingEpoch, - shufflingDecisionRoot, - }); - } - return cacheItem.shuffling; + /** + * Will synchronously get a shuffling if it is available or will throw an error if not. Metrics are collected + * by this._get + */ + getOrError(shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex, caller: ShufflingCacheCaller): EpochShuffling { + // Will throw for error case so always returns a value + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + return this._get(shufflingEpoch, shufflingDecisionRoot, false, true, caller)!; } - getOrNull(shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex): EpochShuffling | null { - const cacheItem = this.itemsByDecisionRootByEpoch.getOrDefault(shufflingEpoch).get(shufflingDecisionRoot); - if (cacheItem === undefined || this.isPromiseCacheItem(cacheItem)) { - return null; - } - return cacheItem.shuffling; + /** + * Will synchronously get a shuffling if it is available or will return null if not. The consumer + * will have to then submit for building the shuffling. Metrics are collected by this._get + */ + getOrNull(shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex, isReload = false): EpochShuffling | null { + return this._get(shufflingEpoch, shufflingDecisionRoot, isReload, false, ShufflingCacheCaller.createFromState); } add(shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex, shuffling: EpochShuffling): void { @@ -161,17 +193,17 @@ export class ShufflingCache implements IShufflingCache { const cacheItem = this.itemsByDecisionRootByEpoch.getOrDefault(shufflingEpoch).get(shufflingDecisionRoot); let resolveFn: ShufflingResolution; if (cacheItem) { - // TODO: (matthewkeil) Add metric here for cache hit if (this.isShufflingCacheItem(cacheItem)) { + this.metrics?.shufflingCache.cacheHit.inc({caller: ShufflingCacheCaller.buildShuffling}); return cacheItem.shuffling; } - // Perhaps we should throw an error instead. Should not happen ideally + // Add metric here for race condition recreating the shuffling because + // this sync call will finish first if its running on thread // - // TODO: (matthewkeil) Add metric here for race condition recreating the shuffling - // because this sync call will finish first if its running on thread + // TODO: (matthewkeil) Perhaps we should throw an error instead. Should not happen ideally + this.metrics?.shufflingCache.cacheHitRebuildPromise.inc(); resolveFn = cacheItem.resolveFn; } else { - // TODO: (matthewkeil) Add metric here for cache miss resolveFn = this._insertShufflingPromise(shufflingEpoch, shufflingDecisionRoot); } @@ -188,15 +220,14 @@ export class ShufflingCache implements IShufflingCache { ): Promise { const cacheItem = this.itemsByDecisionRootByEpoch.getOrDefault(shufflingEpoch).get(shufflingDecisionRoot); if (cacheItem) { - // TODO: (matthewkeil) Add metric here for cache hit if (this.isShufflingCacheItem(cacheItem)) { + this.metrics?.shufflingCache.cacheHit.inc({caller: ShufflingCacheCaller.buildShuffling}); return cacheItem.shuffling; } + this.metrics?.shufflingCache.cacheHitUnresolvedPromise.inc({caller: ShufflingCacheCaller.buildShuffling}); return cacheItem.promise; } - // TODO: (matthewkeil) Add metric here for cache miss - // // this is to prevent multiple calls to get shuffling for the same epoch and dependent root // any subsequent calls of the same epoch and dependent root will wait for this promise to resolve const resolveFn = this._insertShufflingPromise(shufflingEpoch, shufflingDecisionRoot); @@ -209,6 +240,44 @@ export class ShufflingCache implements IShufflingCache { return shuffling; } + private _get( + shufflingEpoch: Epoch, + shufflingDecisionRoot: RootHex, + isReload: boolean, + shouldError: boolean, + caller: ShufflingCacheCaller + ): EpochShuffling | null { + const cacheItem = this.itemsByDecisionRootByEpoch.getOrDefault(shufflingEpoch).get(shufflingDecisionRoot); + if (cacheItem === undefined) { + isReload + ? this.metrics?.shufflingCache.reloadCacheMiss.inc() + : this.metrics?.shufflingCache.cacheMiss.inc({caller}); + if (shouldError) { + throw new ShufflingCacheError({ + code: ShufflingCacheErrorCode.NO_SHUFFLING_FOUND, + epoch: shufflingEpoch, + shufflingDecisionRoot, + }); + } + return null; + } + if (this.isPromiseCacheItem(cacheItem)) { + isReload + ? this.metrics?.shufflingCache.reloadCacheMissUnresolvedPromise.inc() + : this.metrics?.shufflingCache.cacheMissUnresolvedPromise.inc({caller}); + if (shouldError) { + throw new ShufflingCacheError({ + code: ShufflingCacheErrorCode.SHUFFLING_PROMISE_NOT_RESOLVED, + epoch: shufflingEpoch, + shufflingDecisionRoot, + }); + } + return null; + } + isReload ? this.metrics?.shufflingCache.reloadCacheHit.inc() : this.metrics?.shufflingCache.cacheHit.inc({caller}); + return cacheItem.shuffling; + } + private _build( state: BeaconStateAllForks, shufflingEpoch: Epoch, @@ -224,7 +293,7 @@ export class ShufflingCache implements IShufflingCache { const promiseCount = Array.from(this.itemsByDecisionRootByEpoch.values()) .flatMap((innerMap) => Array.from(innerMap.values())) .filter((item) => this.isPromiseCacheItem(item)).length; - if (promiseCount >= MAX_PROMISES) { + if (promiseCount >= SHUFFLING_CACHE_MAX_PROMISES) { throw new Error( `Too many shuffling promises: ${promiseCount}, shufflingEpoch: ${shufflingEpoch}, shufflingDecisionRoot: ${shufflingDecisionRoot}` ); From 4cd6039d8839101ec27d0491f41224d3cd9bead0 Mon Sep 17 00:00:00 2001 From: matthewkeil Date: Thu, 7 Mar 2024 00:54:41 +0800 Subject: [PATCH 09/23] feat: finish refactor and get sim test and state-transition unit test working --- .../src/api/impl/beacon/state/index.ts | 3 +- packages/beacon-node/src/chain/chain.ts | 43 +++--- packages/beacon-node/src/chain/interface.ts | 4 +- .../opPools/aggregatedAttestationPool.ts | 3 +- .../beacon-node/src/chain/shufflingCache.ts | 123 ------------------ .../stateCache/persistentCheckpointsCache.ts | 8 +- .../src/metrics/metrics/lodestar.ts | 39 ++++-- .../test/mocks/mockedBeaconChain.ts | 6 +- .../beacon-node/test/mocks/shufflingMock.ts | 9 +- .../test/unit/chain/shufflingCache.test.ts | 54 -------- .../stateCache/fifoBlockStateCache.test.ts | 10 -- .../persistentCheckpointsCache.test.ts | 8 +- .../stateCache/stateContextCache.test.ts | 10 -- .../test/utils/validationData/attestation.ts | 1 - packages/cli/src/cmds/dev/handler.ts | 2 +- .../utils/simulation/SimulationEnvironment.ts | 2 +- .../state-transition/src/cache/epochCache.ts | 8 +- .../src/cache/shufflingCache.ts | 61 ++++----- .../perf/block/processAttestation.test.ts | 4 +- packages/state-transition/test/perf/util.ts | 15 ++- .../test/unit/shufflingCache.test.ts | 121 +++++++++++++++++ 21 files changed, 234 insertions(+), 300 deletions(-) delete mode 100644 packages/beacon-node/src/chain/shufflingCache.ts delete mode 100644 packages/beacon-node/test/unit/chain/shufflingCache.test.ts create mode 100644 packages/state-transition/test/unit/shufflingCache.test.ts diff --git a/packages/beacon-node/src/api/impl/beacon/state/index.ts b/packages/beacon-node/src/api/impl/beacon/state/index.ts index c9f74b45a9f2..a49f44f103d3 100644 --- a/packages/beacon-node/src/api/impl/beacon/state/index.ts +++ b/packages/beacon-node/src/api/impl/beacon/state/index.ts @@ -6,6 +6,7 @@ import { computeStartSlotAtEpoch, getCurrentEpoch, getRandaoMix, + ShufflingCacheCaller, } from "@lodestar/state-transition"; import {EPOCHS_PER_HISTORICAL_VECTOR} from "@lodestar/params"; import {ApiError} from "../../errors.js"; @@ -195,7 +196,7 @@ export function getBeaconStateApi({ const epoch = filters?.epoch ?? computeEpochAtSlot(state.slot); const startSlot = computeStartSlotAtEpoch(epoch); - const shuffling = stateCached.epochCtx.getShufflingAtEpoch(epoch); + const shuffling = stateCached.epochCtx.getShufflingAtEpoch(epoch, ShufflingCacheCaller.getEpochCommittees); const committees = shuffling.committees; const committeesFlat = committees.flatMap((slotCommittees, slotInEpoch) => { const slot = startSlot + slotInEpoch; diff --git a/packages/beacon-node/src/chain/chain.ts b/packages/beacon-node/src/chain/chain.ts index bbae90e73574..8cc1ed50ee5c 100644 --- a/packages/beacon-node/src/chain/chain.ts +++ b/packages/beacon-node/src/chain/chain.ts @@ -12,6 +12,9 @@ import { Index2PubkeyCache, PubkeyIndexMap, EpochShuffling, + ShufflingCache, + ShufflingCacheError, + ShufflingCacheErrorCode, } from "@lodestar/state-transition"; import {BeaconConfig} from "@lodestar/config"; import { @@ -77,7 +80,6 @@ import {computeNewStateRoot} from "./produceBlock/computeNewStateRoot.js"; import {BlockInput} from "./blocks/types.js"; import {SeenAttestationDatas} from "./seenCache/seenAttestationData.js"; import {BlockRewards, computeBlockRewards} from "./rewards/blockRewards.js"; -import {ShufflingCache} from "./shufflingCache.js"; import {StateContextCache} from "./stateCache/stateContextCache.js"; import {SeenGossipBlockInput} from "./seenCache/index.js"; import {CheckpointStateCache} from "./stateCache/stateContextCheckpointsCache.js"; @@ -216,7 +218,6 @@ export class BeaconChain implements IBeaconChain { this.beaconProposerCache = new BeaconProposerCache(opts); this.checkpointBalancesCache = new CheckpointBalancesCache(); - this.shufflingCache = new ShufflingCache(metrics, this.opts); // Restore state caches // anchorState may already by a CachedBeaconState. If so, don't create the cache again, since deserializing all @@ -226,25 +227,18 @@ export class BeaconChain implements IBeaconChain { let cachedState: CachedBeaconStateAllForks; if (isCachedBeaconState(anchorState) && opts.skipCreateStateCacheIfAvailable) { cachedState = anchorState; - if (anchorState.epochCtx.shufflingCache.hasItems()) { - cachedState.epochCtx.shufflingCache = this.shufflingCache.clone(anchorState.epochCtx.shufflingCache); - } + cachedState.epochCtx.shufflingCache.addMetrics(metrics); + this.shufflingCache = cachedState.epochCtx.shufflingCache; } else { + this.shufflingCache = new ShufflingCache(metrics, this.opts); cachedState = createCachedBeaconState(anchorState, { config, + logger: this.logger, shufflingCache: this.shufflingCache, pubkey2index: new PubkeyIndexMap(), index2pubkey: [], }); } - /** - * These should already be processed when creating the shufflingCache.fromState - * - * TODO: (matthewkeil) double check this is correct - */ - // this.shufflingCache.processState(cachedState, cachedState.epochCtx.previousEpoch); - // this.shufflingCache.processState(cachedState, cachedState.epochCtx.epoch); - // this.shufflingCache.processState(cachedState, cachedState.epochCtx.nextEpoch); // Persist single global instance of state caches this.pubkey2index = cachedState.epochCtx.pubkey2index; @@ -722,22 +716,13 @@ export class BeaconChain implements IBeaconChain { attHeadBlock: ProtoBlock, regenCaller: RegenCaller ): Promise { - // this is to prevent multiple calls to get shuffling for the same epoch and dependent root - // any subsequent calls of the same epoch and dependent root will wait for this promise to resolve - this.shufflingCache.insertPromise(attEpoch, shufflingDependentRoot); const blockEpoch = computeEpochAtSlot(attHeadBlock.slot); - let state: CachedBeaconStateAllForks; if (blockEpoch < attEpoch - 1) { // thanks to one epoch look ahead, we don't need to dial up to attEpoch const targetSlot = computeStartSlotAtEpoch(attEpoch - 1); this.metrics?.gossipAttestation.useHeadBlockStateDialedToTargetEpoch.inc({caller: regenCaller}); - state = await this.regen.getBlockSlotState( - attHeadBlock.blockRoot, - targetSlot, - {dontTransferCache: true}, - regenCaller - ); + await this.regen.getBlockSlotState(attHeadBlock.blockRoot, targetSlot, {dontTransferCache: true}, regenCaller); } else if (blockEpoch > attEpoch) { // should not happen, handled inside attestation verification code throw Error(`Block epoch ${blockEpoch} is after attestation epoch ${attEpoch}`); @@ -746,11 +731,19 @@ export class BeaconChain implements IBeaconChain { // it's not likely to hit this since these shufflings are cached already // so handle just in case this.metrics?.gossipAttestation.useHeadBlockState.inc({caller: regenCaller}); - state = await this.regen.getState(attHeadBlock.stateRoot, regenCaller); + await this.regen.getState(attHeadBlock.stateRoot, regenCaller); } // resolve the promise to unblock other calls of the same epoch and dependent root - return this.shufflingCache.processState(state, attEpoch); + const shuffling = await this.shufflingCache.get(attEpoch, shufflingDependentRoot); + if (!shuffling) { + throw new ShufflingCacheError({ + code: ShufflingCacheErrorCode.REGEN_ERROR_NO_SHUFFLING_FOUND, + epoch: attEpoch, + shufflingDecisionRoot: shufflingDependentRoot, + }); + } + return shuffling; } /** diff --git a/packages/beacon-node/src/chain/interface.ts b/packages/beacon-node/src/chain/interface.ts index d12daf0de7b9..13a1918fb591 100644 --- a/packages/beacon-node/src/chain/interface.ts +++ b/packages/beacon-node/src/chain/interface.ts @@ -17,7 +17,7 @@ import { BeaconStateAllForks, CachedBeaconStateAllForks, EpochShuffling, - IShufflingCache, + ShufflingCache, Index2PubkeyCache, PubkeyIndexMap, } from "@lodestar/state-transition"; @@ -114,7 +114,7 @@ export interface IBeaconChain { readonly checkpointBalancesCache: CheckpointBalancesCache; readonly producedContentsCache: Map; readonly producedBlockRoot: Map; - readonly shufflingCache: IShufflingCache; + readonly shufflingCache: ShufflingCache; readonly producedBlindedBlockRoot: Set; readonly opts: IChainOptions; diff --git a/packages/beacon-node/src/chain/opPools/aggregatedAttestationPool.ts b/packages/beacon-node/src/chain/opPools/aggregatedAttestationPool.ts index 03cf447b44f4..bad608a4b80c 100644 --- a/packages/beacon-node/src/chain/opPools/aggregatedAttestationPool.ts +++ b/packages/beacon-node/src/chain/opPools/aggregatedAttestationPool.ts @@ -9,6 +9,7 @@ import { computeEpochAtSlot, computeStartSlotAtEpoch, getBlockRootAtSlot, + ShufflingCacheCaller, } from "@lodestar/state-transition"; import {IForkChoice, EpochDifference} from "@lodestar/fork-choice"; import {toHex, MapDef} from "@lodestar/utils"; @@ -149,7 +150,7 @@ export class AggregatedAttestationPool { } const slotDelta = stateSlot - slot; - const shuffling = state.epochCtx.getShufflingAtEpoch(epoch); + const shuffling = state.epochCtx.getShufflingAtEpoch(epoch, ShufflingCacheCaller.getAttestationsForBlock); const slotCommittees = shuffling.committees[slot % SLOTS_PER_EPOCH]; for (const [committeeIndex, attestationGroupByData] of attestationGroupByDataHashByIndex.entries()) { // all attestations will be validated against the state in next step so we can get committee from the state diff --git a/packages/beacon-node/src/chain/shufflingCache.ts b/packages/beacon-node/src/chain/shufflingCache.ts deleted file mode 100644 index 38e207f6eb28..000000000000 --- a/packages/beacon-node/src/chain/shufflingCache.ts +++ /dev/null @@ -1,123 +0,0 @@ -import {toHexString} from "@chainsafe/ssz"; -import { - BaseShufflingCache, - BaseShufflingCacheOptions, - CachedBeaconStateAllForks, - EpochShuffling, - getShufflingDecisionBlock, -} from "@lodestar/state-transition"; -import {Epoch, RootHex, ssz} from "@lodestar/types"; -import {GENESIS_SLOT} from "@lodestar/params"; -import {Metrics} from "../metrics/metrics.js"; -import {computeAnchorCheckpoint} from "./initState.js"; - -export interface ShufflingCacheOptions extends BaseShufflingCacheOptions {} - -/** - * A shuffling cache to help: - * - get committee quickly for attestation verification - * - if a shuffling is not available (which does not happen with default chain option of maxSkipSlots = 32), track a promise to make sure we don't compute the same shuffling twice - * - skip computing shuffling when loading state bytes from disk - */ -export class ShufflingCache extends BaseShufflingCache { - constructor( - private readonly metrics: Metrics | null = null, - opts: ShufflingCacheOptions = {} - ) { - super(opts); - if (metrics) { - metrics.shufflingCache.size.addCollect(() => - metrics.shufflingCache.size.set( - Array.from(this.itemsByDecisionRootByEpoch.values()).reduce((total, innerMap) => total + innerMap.size, 0) - ) - ); - } - } - - /** - * Extract shuffling from state and add to cache - */ - processState(state: CachedBeaconStateAllForks, shufflingEpoch: Epoch): EpochShuffling { - const decisionBlockHex = getDecisionBlock(state, shufflingEpoch); - // let shuffling: EpochShuffling; - // switch (shufflingEpoch) { - // case state.epochCtx.nextEpoch: - // shuffling = state.epochCtx.nextShuffling; - // break; - // case state.epochCtx.epoch: - // shuffling = state.epochCtx.currentShuffling; - // break; - // case state.epochCtx.previousEpoch: - // shuffling = state.epochCtx.previousShuffling; - // break; - // default: - // throw new Error(`Shuffling not found from state ${state.slot} for epoch ${shufflingEpoch}`); - // } - - let cacheItem = this.itemsByDecisionRootByEpoch.getOrDefault(shufflingEpoch).get(decisionBlockHex); - if (cacheItem !== undefined) { - // update existing promise - if (isPromiseCacheItem(cacheItem)) { - // unblock consumers of this promise - cacheItem.resolveFn(shuffling); - // then update item type to shuffling - cacheItem = { - type: CacheItemType.shuffling, - shuffling, - }; - this.add(shufflingEpoch, decisionBlockHex, cacheItem); - // we updated type to CacheItemType.shuffling so the above fields are not used anyway - this.metrics?.shufflingCache.processStateUpdatePromise.inc(); - } else { - // ShufflingCacheItem, do nothing - this.metrics?.shufflingCache.processStateNoOp.inc(); - } - } else { - // not found, new shuffling - this.add(shufflingEpoch, decisionBlockHex, {type: CacheItemType.shuffling, shuffling}); - this.metrics?.shufflingCache.processStateInsertNew.inc(); - } - - return shuffling; - } - - /** - * Insert a promise to make sure we don't regen state for the same shuffling. - * Bound by MAX_SHUFFLING_PROMISE to make sure our node does not blow up. - */ - insertPromise(shufflingEpoch: Epoch, decisionRootHex: RootHex): void { - const promiseCount = Array.from(this.itemsByDecisionRootByEpoch.values()) - .flatMap((innerMap) => Array.from(innerMap.values())) - .filter((item) => isPromiseCacheItem(item)).length; - if (promiseCount >= MAX_PROMISES) { - throw new Error( - `Too many shuffling promises: ${promiseCount}, shufflingEpoch: ${shufflingEpoch}, decisionRootHex: ${decisionRootHex}` - ); - } - - let resolveFn!: (shuffling: EpochShuffling) => void; - const promise = new Promise((resolve) => { - resolveFn = resolve; - }); - - const cacheItem: PromiseCacheItem = { - type: CacheItemType.promise, - promise, - resolveFn, - }; - this.add(shufflingEpoch, decisionRootHex, cacheItem); - this.metrics?.shufflingCache.insertPromiseCount.inc(); - } -} - -// TODO: @tuyennhv why is this here and not in state-transition with `getShufflingDecisionBlock`? -/** - * Get the shuffling decision block root for the given epoch of given state - * - Special case close to genesis block, return the genesis block root - * - This is similar to forkchoice.getDependentRoot() function, otherwise we cannot get cached shuffing in attestation verification when syncing from genesis. - */ -function getDecisionBlock(state: CachedBeaconStateAllForks, epoch: Epoch): RootHex { - return state.slot > GENESIS_SLOT - ? getShufflingDecisionBlock(state, epoch) - : toHexString(ssz.phase0.BeaconBlockHeader.hashTreeRoot(computeAnchorCheckpoint(state.config, state).blockHeader)); -} diff --git a/packages/beacon-node/src/chain/stateCache/persistentCheckpointsCache.ts b/packages/beacon-node/src/chain/stateCache/persistentCheckpointsCache.ts index 88c6a2ff1fb7..e95b03fd653e 100644 --- a/packages/beacon-node/src/chain/stateCache/persistentCheckpointsCache.ts +++ b/packages/beacon-node/src/chain/stateCache/persistentCheckpointsCache.ts @@ -219,7 +219,13 @@ export class PersistentCheckpointStateCache implements CheckpointStateCache { sszTimer?.(); const timer = this.metrics?.stateReloadDuration.startTimer(); // @tuyennhv why are we not passing through the EpochCacheOptions from the seed state? - const newCachedState = loadCachedBeaconState(seedState, stateBytes, this.logger, {}, validatorsBytes); + const newCachedState = loadCachedBeaconState( + seedState, + stateBytes, + this.logger, + {isReload: true}, + validatorsBytes + ); newCachedState.commit(); const stateRoot = toHexString(newCachedState.hashTreeRoot()); timer?.(); diff --git a/packages/beacon-node/src/metrics/metrics/lodestar.ts b/packages/beacon-node/src/metrics/metrics/lodestar.ts index 4f82969cff81..77750953f3e9 100644 --- a/packages/beacon-node/src/metrics/metrics/lodestar.ts +++ b/packages/beacon-node/src/metrics/metrics/lodestar.ts @@ -1,4 +1,9 @@ -import {EpochTransitionStep, StateCloneSource, StateHashTreeRootSource} from "@lodestar/state-transition"; +import { + EpochTransitionStep, + ShufflingCacheCaller, + StateCloneSource, + StateHashTreeRootSource, +} from "@lodestar/state-transition"; import {allForks} from "@lodestar/types"; import {BlockSource} from "../../chain/blocks/types.js"; import {JobQueueItemType} from "../../chain/bls/index.js"; @@ -1273,21 +1278,29 @@ export function createLodestarMetrics( name: "lodestar_shuffling_cache_size", help: "Shuffling cache size", }), - processStateInsertNew: register.gauge({ - name: "lodestar_shuffling_cache_process_state_insert_new_total", - help: "Total number of times processState is called resulting a new shuffling", + cacheMiss: register.gauge<{caller: ShufflingCacheCaller}>({ + name: "lodestar_shuffling_cache_miss_total", + help: "Total number of times a request to get a shuffling missed", + labelNames: ["caller"], }), - processStateUpdatePromise: register.gauge({ - name: "lodestar_shuffling_cache_process_state_update_promise_total", - help: "Total number of times processState is called resulting a promise being updated with shuffling", + cacheMissUnresolvedPromise: register.gauge<{caller: ShufflingCacheCaller}>({ + name: "lodestar_shuffling_cache_miss_unresolved_promise_total", + help: "Total number of times a request to synchronously get a shuffling but the promise for the shuffling was not resolved yet", + labelNames: ["caller"], }), - processStateNoOp: register.gauge({ - name: "lodestar_shuffling_cache_process_state_no_op_total", - help: "Total number of times processState is called resulting no changes", + cacheHit: register.gauge<{caller: ShufflingCacheCaller}>({ + name: "lodestar_shuffling_cache_hit_total", + help: "Total number of times a request to get a shuffling returned a shuffling immediately", + labelNames: ["caller"], + }), + cacheHitUnresolvedPromise: register.gauge<{caller: ShufflingCacheCaller}>({ + name: "lodestar_shuffling_cache_hit_unresolved_promise_total", + help: "Total number of times a request to get a shuffling returned a promise to be resolved by the consumer", + labelNames: ["caller"], }), - insertPromiseCount: register.gauge({ - name: "lodestar_shuffling_cache_insert_promise_count", - help: "Total number of times insertPromise is called", + cacheHitRebuildPromise: register.gauge({ + name: "lodestar_shuffling_cache_hit_rebuild_promise_total", + help: "Total number of times a synchronous request to build a shuffling threw away an existing promise and rebuilt a new one", }), }, diff --git a/packages/beacon-node/test/mocks/mockedBeaconChain.ts b/packages/beacon-node/test/mocks/mockedBeaconChain.ts index 21881875ba61..099b7a902be8 100644 --- a/packages/beacon-node/test/mocks/mockedBeaconChain.ts +++ b/packages/beacon-node/test/mocks/mockedBeaconChain.ts @@ -4,6 +4,7 @@ import {config as defaultConfig} from "@lodestar/config/default"; import {ChainForkConfig} from "@lodestar/config"; import {ForkChoice, ProtoBlock, EpochDifference} from "@lodestar/fork-choice"; import {Logger} from "@lodestar/utils"; +import {ShufflingCache} from "@lodestar/state-transition"; import {BeaconChain} from "../../src/chain/chain.js"; import {ChainEventEmitter} from "../../src/chain/emitter.js"; import {ExecutionEngineHttp} from "../../src/execution/engine/index.js"; @@ -14,8 +15,8 @@ import {BeaconProposerCache} from "../../src/chain/beaconProposerCache.js"; import {LightClientServer} from "../../src/chain/lightClient/index.js"; import {Clock} from "../../src/util/clock.js"; import {QueuedStateRegenerator} from "../../src/chain/regen/index.js"; -import {ShufflingCache} from "../../src/chain/shufflingCache.js"; import {getMockedLogger} from "./loggerMock.js"; +import {getMockedShufflingCache} from "./shufflingMock.js"; export type MockedBeaconChain = Mocked & { logger: Mocked; @@ -70,7 +71,6 @@ vi.mock("@lodestar/fork-choice", async (importActual) => { vi.mock("../../src/chain/regen/index.js"); vi.mock("../../src/eth1/index.js"); vi.mock("../../src/chain/beaconProposerCache.js"); -vi.mock("../../src/chain/shufflingCache.js"); vi.mock("../../src/chain/lightClient/index.js"); vi.mock("../../src/chain/opPools/index.js", async (importActual) => { @@ -131,7 +131,7 @@ vi.mock("../../src/chain/chain.js", async (importActual) => { // eslint-disable-next-line @typescript-eslint/ban-ts-comment // @ts-expect-error beaconProposerCache: new BeaconProposerCache(), - shufflingCache: new ShufflingCache(), + shufflingCache: getMockedShufflingCache(), produceCommonBlockBody: vi.fn(), produceBlock: vi.fn(), produceBlindedBlock: vi.fn(), diff --git a/packages/beacon-node/test/mocks/shufflingMock.ts b/packages/beacon-node/test/mocks/shufflingMock.ts index 76482f760872..d19b9a754818 100644 --- a/packages/beacon-node/test/mocks/shufflingMock.ts +++ b/packages/beacon-node/test/mocks/shufflingMock.ts @@ -1,10 +1,9 @@ import {vi, Mocked} from "vitest"; -import {ShufflingCache} from "../../src/chain/shufflingCache.js"; +// eslint-disable-next-line import/no-relative-packages +import {ShufflingCache} from "../../../state-transition/src/cache/shufflingCache.js"; -export type MockedShufflingCache = Mocked; +vi.mock("../../../state-transition/src/cache/shufflingCache.js"); -vi.mock("../../src/chain/shufflingCache.js"); - -export function getMockedShufflingCache(): MockedShufflingCache { +export function getMockedShufflingCache(): Mocked { return vi.mocked(new ShufflingCache({} as any)); } diff --git a/packages/beacon-node/test/unit/chain/shufflingCache.test.ts b/packages/beacon-node/test/unit/chain/shufflingCache.test.ts deleted file mode 100644 index 60cfb8a8691a..000000000000 --- a/packages/beacon-node/test/unit/chain/shufflingCache.test.ts +++ /dev/null @@ -1,54 +0,0 @@ -import {describe, it, expect, beforeEach} from "vitest"; - -import {getShufflingDecisionBlock} from "@lodestar/state-transition"; -// eslint-disable-next-line import/no-relative-packages -import {generateTestCachedBeaconStateOnlyValidators} from "../../../../state-transition/test/perf/util.js"; -import {ShufflingCache} from "../../../src/chain/shufflingCache.js"; - -describe("ShufflingCache", function () { - const vc = 64; - const stateSlot = 100; - const state = generateTestCachedBeaconStateOnlyValidators({vc, slot: stateSlot}); - const currentEpoch = state.epochCtx.epoch; - let shufflingCache: ShufflingCache; - - beforeEach(() => { - shufflingCache = new ShufflingCache(null, {maxShufflingCacheEpochs: 1}); - shufflingCache.processState(state, currentEpoch); - }); - - it("should get shuffling from cache", async function () { - const decisionRoot = getShufflingDecisionBlock(state, currentEpoch); - expect(await shufflingCache.get(currentEpoch, decisionRoot)).toEqual(state.epochCtx.currentShuffling); - }); - - it("should bound by maxSize(=1)", async function () { - const decisionRoot = getShufflingDecisionBlock(state, currentEpoch); - expect(await shufflingCache.get(currentEpoch, decisionRoot)).toEqual(state.epochCtx.currentShuffling); - // insert promises at the same epoch does not prune the cache - shufflingCache.insertPromise(currentEpoch, "0x00"); - expect(await shufflingCache.get(currentEpoch, decisionRoot)).toEqual(state.epochCtx.currentShuffling); - // insert shufflings at other epochs does prune the cache - shufflingCache.processState(state, currentEpoch + 1); - // the current shuffling is not available anymore - expect(await shufflingCache.get(currentEpoch, decisionRoot)).toBeNull(); - }); - - it("should return shuffling from promise", async function () { - const nextDecisionRoot = getShufflingDecisionBlock(state, currentEpoch + 1); - shufflingCache.insertPromise(currentEpoch + 1, nextDecisionRoot); - const shufflingRequest0 = shufflingCache.get(currentEpoch + 1, nextDecisionRoot); - const shufflingRequest1 = shufflingCache.get(currentEpoch + 1, nextDecisionRoot); - shufflingCache.processState(state, currentEpoch + 1); - expect(await shufflingRequest0).toEqual(state.epochCtx.nextShuffling); - expect(await shufflingRequest1).toEqual(state.epochCtx.nextShuffling); - }); - - it("should support up to 2 promises at a time", async function () { - // insert 2 promises at the same epoch - shufflingCache.insertPromise(currentEpoch, "0x00"); - shufflingCache.insertPromise(currentEpoch, "0x01"); - // inserting other promise should throw error - expect(() => shufflingCache.insertPromise(currentEpoch, "0x02")).toThrow(); - }); -}); diff --git a/packages/beacon-node/test/unit/chain/stateCache/fifoBlockStateCache.test.ts b/packages/beacon-node/test/unit/chain/stateCache/fifoBlockStateCache.test.ts index 1f4f407a4be4..d8ab1d305574 100644 --- a/packages/beacon-node/test/unit/chain/stateCache/fifoBlockStateCache.test.ts +++ b/packages/beacon-node/test/unit/chain/stateCache/fifoBlockStateCache.test.ts @@ -1,6 +1,5 @@ import {describe, it, expect, beforeEach} from "vitest"; import {toHexString} from "@chainsafe/ssz"; -import {EpochShuffling} from "@lodestar/state-transition"; import {SLOTS_PER_EPOCH} from "@lodestar/params"; import {CachedBeaconStateAllForks} from "@lodestar/state-transition/src/types.js"; import {FIFOBlockStateCache} from "../../../../src/chain/stateCache/index.js"; @@ -8,24 +7,15 @@ import {generateCachedState} from "../../../utils/state.js"; describe("FIFOBlockStateCache", function () { let cache: FIFOBlockStateCache; - const shuffling: EpochShuffling = { - activeIndices: new Uint32Array(), - shuffling: new Uint32Array(), - committees: [], - committeesPerSlot: 1, - }; const state1 = generateCachedState({slot: 0}); const key1 = toHexString(state1.hashTreeRoot()); - state1.epochCtx.currentShuffling = {...shuffling}; const state2 = generateCachedState({slot: 1 * SLOTS_PER_EPOCH}); const key2 = toHexString(state2.hashTreeRoot()); - state2.epochCtx.currentShuffling = {...shuffling}; const state3 = generateCachedState({slot: 2 * SLOTS_PER_EPOCH}); const key3 = toHexString(state3.hashTreeRoot()); - state3.epochCtx.currentShuffling = {...shuffling}; beforeEach(function () { // max 2 items diff --git a/packages/beacon-node/test/unit/chain/stateCache/persistentCheckpointsCache.test.ts b/packages/beacon-node/test/unit/chain/stateCache/persistentCheckpointsCache.test.ts index af7c118f5e99..bd113de3f5ea 100644 --- a/packages/beacon-node/test/unit/chain/stateCache/persistentCheckpointsCache.test.ts +++ b/packages/beacon-node/test/unit/chain/stateCache/persistentCheckpointsCache.test.ts @@ -1,12 +1,16 @@ import {describe, it, expect, beforeAll, beforeEach} from "vitest"; import {SLOTS_PER_EPOCH, SLOTS_PER_HISTORICAL_ROOT} from "@lodestar/params"; -import {CachedBeaconStateAllForks, computeEpochAtSlot, computeStartSlotAtEpoch} from "@lodestar/state-transition"; +import { + CachedBeaconStateAllForks, + computeEpochAtSlot, + computeStartSlotAtEpoch, + ShufflingCache, +} from "@lodestar/state-transition"; import {RootHex, phase0} from "@lodestar/types"; import {mapValues, toHexString} from "@lodestar/utils"; import {PersistentCheckpointStateCache} from "../../../../src/chain/stateCache/persistentCheckpointsCache.js"; import {checkpointToDatastoreKey} from "../../../../src/chain/stateCache/datastore/index.js"; import {generateCachedState} from "../../../utils/state.js"; -import {ShufflingCache} from "../../../../src/chain/shufflingCache.js"; import {testLogger} from "../../../utils/logger.js"; import {getTestDatastore} from "../../../utils/chain/stateCache/datastore.js"; import {CheckpointHex} from "../../../../src/chain/stateCache/types.js"; diff --git a/packages/beacon-node/test/unit/chain/stateCache/stateContextCache.test.ts b/packages/beacon-node/test/unit/chain/stateCache/stateContextCache.test.ts index 04476f30f054..8d3f4a59e2b4 100644 --- a/packages/beacon-node/test/unit/chain/stateCache/stateContextCache.test.ts +++ b/packages/beacon-node/test/unit/chain/stateCache/stateContextCache.test.ts @@ -1,6 +1,5 @@ import {toHexString} from "@chainsafe/ssz"; import {describe, it, expect, beforeEach} from "vitest"; -import {EpochShuffling} from "@lodestar/state-transition"; import {SLOTS_PER_EPOCH} from "@lodestar/params"; import {Root} from "@lodestar/types"; import {StateContextCache} from "../../../../src/chain/stateCache/index.js"; @@ -10,30 +9,21 @@ import {ZERO_HASH} from "../../../../src/constants/index.js"; describe("StateContextCache", function () { let cache: StateContextCache; let key1: Root, key2: Root; - const shuffling: EpochShuffling = { - activeIndices: new Uint32Array(), - shuffling: new Uint32Array(), - committees: [], - committeesPerSlot: 1, - }; beforeEach(function () { // max 2 items cache = new StateContextCache({maxStates: 2}); const state1 = generateCachedState({slot: 0}); key1 = state1.hashTreeRoot(); - state1.epochCtx.currentShuffling = {...shuffling}; cache.add(state1); const state2 = generateCachedState({slot: 1 * SLOTS_PER_EPOCH}); key2 = state2.hashTreeRoot(); - state2.epochCtx.currentShuffling = {...shuffling}; cache.add(state2); }); it("should prune", function () { expect(cache.size).toBe(2); const state3 = generateCachedState({slot: 2 * SLOTS_PER_EPOCH}); - state3.epochCtx.currentShuffling = {...shuffling}; cache.add(state3); expect(cache.size).toBe(3); diff --git a/packages/beacon-node/test/utils/validationData/attestation.ts b/packages/beacon-node/test/utils/validationData/attestation.ts index 04121be95856..1eb2c31a0b88 100644 --- a/packages/beacon-node/test/utils/validationData/attestation.ts +++ b/packages/beacon-node/test/utils/validationData/attestation.ts @@ -19,7 +19,6 @@ import {SeenAggregatedAttestations} from "../../../src/chain/seenCache/seenAggre import {SeenAttestationDatas} from "../../../src/chain/seenCache/seenAttestationData.js"; import {defaultChainOptions} from "../../../src/chain/options.js"; import {testLogger} from "../logger.js"; -import {ShufflingCache} from "../../../src/chain/shufflingCache.js"; export type AttestationValidDataOpts = { currentSlot?: Slot; diff --git a/packages/cli/src/cmds/dev/handler.ts b/packages/cli/src/cmds/dev/handler.ts index b13ad61301ff..58351690a352 100644 --- a/packages/cli/src/cmds/dev/handler.ts +++ b/packages/cli/src/cmds/dev/handler.ts @@ -2,7 +2,7 @@ import fs from "node:fs"; import {rimraf} from "rimraf"; import {toHex, fromHex, LogLevel} from "@lodestar/utils"; import {nodeUtils} from "@lodestar/beacon-node"; -import {getNodeLogger} from "@lodestar/logger/lib/node.js"; +import {getNodeLogger} from "@lodestar/logger/node"; import {GlobalArgs} from "../../options/index.js"; import {mkdir, onGracefulShutdown} from "../../util/index.js"; import {getBeaconConfigFromArgs} from "../../config/beaconParams.js"; diff --git a/packages/cli/test/utils/simulation/SimulationEnvironment.ts b/packages/cli/test/utils/simulation/SimulationEnvironment.ts index 47ef9770d3ac..8f699e6c829c 100644 --- a/packages/cli/test/utils/simulation/SimulationEnvironment.ts +++ b/packages/cli/test/utils/simulation/SimulationEnvironment.ts @@ -323,7 +323,7 @@ export class SimulationEnvironment { throw new Error(`Eth1 genesis not found for node "${this.nodes[i].id}"`); } - const genesisState = nodeUtils.initDevState(this.forkConfig, this.keysCount, { + const genesisState = nodeUtils.initDevState(this.forkConfig, this.logger, this.keysCount, { genesisTime: this.options.genesisTime + this.forkConfig.GENESIS_DELAY, eth1BlockHash: fromHexString(eth1Genesis.hash), }).state; diff --git a/packages/state-transition/src/cache/epochCache.ts b/packages/state-transition/src/cache/epochCache.ts index ff91adf77bf3..86c1a3c91e1e 100644 --- a/packages/state-transition/src/cache/epochCache.ts +++ b/packages/state-transition/src/cache/epochCache.ts @@ -41,7 +41,7 @@ import { SyncCommitteeCache, SyncCommitteeCacheEmpty, } from "./syncCommitteeCache.js"; -import {ShufflingCache, IShufflingCache, ShufflingCacheCaller} from "./shufflingCache.js"; +import {ShufflingCache, ShufflingCacheCaller} from "./shufflingCache.js"; /** `= PROPOSER_WEIGHT / (WEIGHT_DENOMINATOR - PROPOSER_WEIGHT)` */ export const PROPOSER_WEIGHT_FACTOR = PROPOSER_WEIGHT / (WEIGHT_DENOMINATOR - PROPOSER_WEIGHT); @@ -49,7 +49,7 @@ export const PROPOSER_WEIGHT_FACTOR = PROPOSER_WEIGHT / (WEIGHT_DENOMINATOR - PR export type EpochCacheImmutableData = { config: BeaconConfig; logger: Logger; - shufflingCache: IShufflingCache; + shufflingCache: ShufflingCache; pubkey2index: PubkeyIndexMap; index2pubkey: Index2PubkeyCache; }; @@ -88,7 +88,7 @@ type ProposersDeferred = {computed: false; seed: Uint8Array} | {computed: true; export class EpochCache { config: BeaconConfig; logger: Logger; - shufflingCache: IShufflingCache; + shufflingCache: ShufflingCache; /** * Unique globally shared pubkey registry. There should only exist one for the entire application. @@ -223,7 +223,7 @@ export class EpochCache { constructor(data: { config: BeaconConfig; logger: Logger; - shufflingCache: IShufflingCache; + shufflingCache: ShufflingCache; pubkey2index: PubkeyIndexMap; index2pubkey: Index2PubkeyCache; proposers: number[]; diff --git a/packages/state-transition/src/cache/shufflingCache.ts b/packages/state-transition/src/cache/shufflingCache.ts index a84c9c4cde82..ed8813db167b 100644 --- a/packages/state-transition/src/cache/shufflingCache.ts +++ b/packages/state-transition/src/cache/shufflingCache.ts @@ -20,8 +20,10 @@ export const SHUFFLING_CACHE_MAX_EPOCHS = 4; export enum ShufflingCacheCaller { testing = "testing", buildShuffling = "buildShuffling", + synchronousBuildShuffling = "synchronousBuildShuffling", attestationVerification = "attestationVerification", createFromState = "createFromState", + reloadCreateFromState = "reloadCreateFromState", getBeaconCommittee = "getBeaconCommittee", getEpochCommittees = "getEpochCommittees", getAttestationsForBlock = "getAttestationsForBlock", @@ -29,39 +31,14 @@ export enum ShufflingCacheCaller { getCommitteeAssignments = "getCommitteeAssignments", } -export interface IShufflingCache { - add(shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex, shuffling: EpochShuffling): void; - get(shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex): Promise; - // getAll(): MapDef>; - getOrNull(shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex, isReload?: boolean): EpochShuffling | null; - getOrError(shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex, caller: ShufflingCacheCaller): EpochShuffling; - buildSync( - state: BeaconStateAllForks, - shufflingEpoch: Epoch, - shufflingDecisionRoot: RootHex, - activeIndexes: number[] - ): EpochShuffling; - build( - state: BeaconStateAllForks, - shufflingEpoch: Epoch, - shufflingDecisionRoot: RootHex, - activeIndexes: number[] - ): Promise; -} - export interface ShufflingCacheMetrics { shufflingCache: { size: GaugeExtra; - cacheMiss: GaugeExtra<{caller: ShufflingCacheCaller}>; cacheMissUnresolvedPromise: GaugeExtra<{caller: ShufflingCacheCaller}>; cacheHit: GaugeExtra<{caller: ShufflingCacheCaller}>; cacheHitUnresolvedPromise: GaugeExtra<{caller: ShufflingCacheCaller}>; cacheHitRebuildPromise: GaugeExtra; - - reloadCacheMiss: GaugeExtra; - reloadCacheMissUnresolvedPromise: GaugeExtra; - reloadCacheHit: GaugeExtra; }; } @@ -96,7 +73,7 @@ export type ShufflingCachePromiseItem = { export type ShufflingCacheItem = ShufflingCacheShufflingItem | ShufflingCachePromiseItem; -type ShufflingResolution = (shuffling: EpochShuffling) => void; +export type ShufflingResolution = (shuffling: EpochShuffling) => void; export interface ShufflingCacheOptions { maxShufflingCacheEpochs?: number; @@ -108,7 +85,7 @@ export interface ShufflingCacheOptions { * - if a shuffling is not available (which does not happen with default chain option of maxSkipSlots = 32), track a promise to make sure we don't compute the same shuffling twice * - skip computing shuffling when loading state bytes from disk */ -export class ShufflingCache implements IShufflingCache { +export class ShufflingCache { /** LRU cache implemented as a map, pruned every time we add an item */ private readonly itemsByDecisionRootByEpoch: MapDef> = new MapDef( () => new Map() @@ -119,6 +96,12 @@ export class ShufflingCache implements IShufflingCache { constructor(metrics: ShufflingCacheMetrics | null = null, opts: ShufflingCacheOptions = {}) { this.maxEpochs = opts.maxShufflingCacheEpochs ?? SHUFFLING_CACHE_MAX_EPOCHS; this.addMetrics(metrics); + // just used for testing and don't want to pollute the public api + Object.defineProperty(this, "allAsArray", { + enumerable: false, + value: () => + Array.from(this.itemsByDecisionRootByEpoch.values()).flatMap((innerMap) => Array.from(innerMap.values())), + }); } addMetrics(metrics: ShufflingCacheMetrics | null): void { @@ -166,7 +149,7 @@ export class ShufflingCache implements IShufflingCache { getOrError(shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex, caller: ShufflingCacheCaller): EpochShuffling { // Will throw for error case so always returns a value // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - return this._get(shufflingEpoch, shufflingDecisionRoot, false, true, caller)!; + return this._get(shufflingEpoch, shufflingDecisionRoot, true, caller)!; } /** @@ -174,7 +157,12 @@ export class ShufflingCache implements IShufflingCache { * will have to then submit for building the shuffling. Metrics are collected by this._get */ getOrNull(shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex, isReload = false): EpochShuffling | null { - return this._get(shufflingEpoch, shufflingDecisionRoot, isReload, false, ShufflingCacheCaller.createFromState); + return this._get( + shufflingEpoch, + shufflingDecisionRoot, + false, + isReload ? ShufflingCacheCaller.reloadCreateFromState : ShufflingCacheCaller.createFromState + ); } add(shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex, shuffling: EpochShuffling): void { @@ -194,7 +182,7 @@ export class ShufflingCache implements IShufflingCache { let resolveFn: ShufflingResolution; if (cacheItem) { if (this.isShufflingCacheItem(cacheItem)) { - this.metrics?.shufflingCache.cacheHit.inc({caller: ShufflingCacheCaller.buildShuffling}); + this.metrics?.shufflingCache.cacheHit.inc({caller: ShufflingCacheCaller.synchronousBuildShuffling}); return cacheItem.shuffling; } // Add metric here for race condition recreating the shuffling because @@ -202,9 +190,11 @@ export class ShufflingCache implements IShufflingCache { // // TODO: (matthewkeil) Perhaps we should throw an error instead. Should not happen ideally this.metrics?.shufflingCache.cacheHitRebuildPromise.inc(); + // add log statement here with epoch and decision root for this miss resolveFn = cacheItem.resolveFn; } else { resolveFn = this._insertShufflingPromise(shufflingEpoch, shufflingDecisionRoot); + this.metrics?.shufflingCache.cacheMiss.inc({caller: ShufflingCacheCaller.synchronousBuildShuffling}); } const shuffling = this._build(state, shufflingEpoch, shufflingDecisionRoot, activeIndexes); @@ -243,15 +233,12 @@ export class ShufflingCache implements IShufflingCache { private _get( shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex, - isReload: boolean, shouldError: boolean, caller: ShufflingCacheCaller ): EpochShuffling | null { const cacheItem = this.itemsByDecisionRootByEpoch.getOrDefault(shufflingEpoch).get(shufflingDecisionRoot); if (cacheItem === undefined) { - isReload - ? this.metrics?.shufflingCache.reloadCacheMiss.inc() - : this.metrics?.shufflingCache.cacheMiss.inc({caller}); + this.metrics?.shufflingCache.cacheMiss.inc({caller}); if (shouldError) { throw new ShufflingCacheError({ code: ShufflingCacheErrorCode.NO_SHUFFLING_FOUND, @@ -262,9 +249,7 @@ export class ShufflingCache implements IShufflingCache { return null; } if (this.isPromiseCacheItem(cacheItem)) { - isReload - ? this.metrics?.shufflingCache.reloadCacheMissUnresolvedPromise.inc() - : this.metrics?.shufflingCache.cacheMissUnresolvedPromise.inc({caller}); + this.metrics?.shufflingCache.cacheMissUnresolvedPromise.inc({caller}); if (shouldError) { throw new ShufflingCacheError({ code: ShufflingCacheErrorCode.SHUFFLING_PROMISE_NOT_RESOLVED, @@ -274,7 +259,7 @@ export class ShufflingCache implements IShufflingCache { } return null; } - isReload ? this.metrics?.shufflingCache.reloadCacheHit.inc() : this.metrics?.shufflingCache.cacheHit.inc({caller}); + this.metrics?.shufflingCache.cacheHit.inc({caller}); return cacheItem.shuffling; } diff --git a/packages/state-transition/test/perf/block/processAttestation.test.ts b/packages/state-transition/test/perf/block/processAttestation.test.ts index a9ccbb8352b3..7c2461fb2b64 100644 --- a/packages/state-transition/test/perf/block/processAttestation.test.ts +++ b/packages/state-transition/test/perf/block/processAttestation.test.ts @@ -11,7 +11,7 @@ import { SYNC_COMMITTEE_SIZE, } from "@lodestar/params"; import {phase0} from "@lodestar/types"; -import {CachedBeaconStateAllForks, CachedBeaconStateAltair} from "../../../src/index.js"; +import {CachedBeaconStateAllForks, CachedBeaconStateAltair, ShufflingCacheCaller} from "../../../src/index.js"; import {processAttestationsAltair} from "../../../src/block/processAttestationsAltair.js"; import {generatePerfTestCachedStateAltair, perfStateId} from "../util.js"; import {BlockAltairOpts, getBlockAltair} from "./util.js"; @@ -118,7 +118,7 @@ describe("altair processAttestation - CachedEpochParticipation.setStatus", () => const numAttesters = Math.floor((state.epochCtx.currentActiveIndices.length * ratio) / SLOTS_PER_EPOCH); // just get committees of slot 10 let count = 0; - for (const committees of state.epochCtx.getShufflingAtSlot(state.slot).committees[10]) { + for (const committees of state.epochCtx.getShufflingAtSlot(state.slot, ShufflingCacheCaller.testing).committees[10]) { for (const committee of committees) { currentEpochParticipation.set(committee, 0b111); count++; diff --git a/packages/state-transition/test/perf/util.ts b/packages/state-transition/test/perf/util.ts index 1ea729f92e1d..cbe82ea561ed 100644 --- a/packages/state-transition/test/perf/util.ts +++ b/packages/state-transition/test/perf/util.ts @@ -23,6 +23,7 @@ import { createCachedBeaconState, computeCommitteeCount, ShufflingCache, + ShufflingCacheCaller, } from "../../src/index.js"; import { CachedBeaconStateAllForks, @@ -147,7 +148,10 @@ export function generatePerfTestCachedStatePhase0(opts?: {goBackOneSlot: boolean const slotInEpoch = i % SLOTS_PER_EPOCH; const slot = previousEpoch * SLOTS_PER_EPOCH + slotInEpoch; const index = i % committeesPerSlot; - const shuffling = phase0CachedState23637.epochCtx.getShufflingAtEpoch(previousEpoch); + const shuffling = phase0CachedState23637.epochCtx.getShufflingAtEpoch( + previousEpoch, + ShufflingCacheCaller.testing + ); const committee = shuffling.committees[slotInEpoch][index]; phase0CachedState23637.previousEpochAttestations.push( ssz.phase0.PendingAttestation.toViewDU({ @@ -171,7 +175,10 @@ export function generatePerfTestCachedStatePhase0(opts?: {goBackOneSlot: boolean const slotInEpoch = i % SLOTS_PER_EPOCH; const slot = currentEpoch * SLOTS_PER_EPOCH + slotInEpoch; const index = i % committeesPerSlot; - const shuffling = phase0CachedState23637.epochCtx.getShufflingAtEpoch(previousEpoch); + const shuffling = phase0CachedState23637.epochCtx.getShufflingAtEpoch( + previousEpoch, + ShufflingCacheCaller.testing + ); const committee = shuffling.committees[slotInEpoch][index]; phase0CachedState23637.currentEpochAttestations.push( @@ -388,9 +395,11 @@ function buildPerformanceStatePhase0(pubkeysArg?: Uint8Array[]): phase0.BeaconSt export function generateTestCachedBeaconStateOnlyValidators({ vc, slot, + maxShufflingCacheEpochs, }: { vc: number; slot: Slot; + maxShufflingCacheEpochs?: number; }): CachedBeaconStateAllForks { // Generate only some publicKeys const {pubkeys, pubkeysMod, pubkeysModObj} = getPubkeys(vc); @@ -443,7 +452,7 @@ export function generateTestCachedBeaconStateOnlyValidators({ { config: createBeaconConfig(config, state.genesisValidatorsRoot), logger: getNodeLogger({level: LogLevel.info}), - shufflingCache: new ShufflingCache(), + shufflingCache: new ShufflingCache(null, {maxShufflingCacheEpochs}), pubkey2index, index2pubkey, }, diff --git a/packages/state-transition/test/unit/shufflingCache.test.ts b/packages/state-transition/test/unit/shufflingCache.test.ts new file mode 100644 index 000000000000..32d37d5f55d8 --- /dev/null +++ b/packages/state-transition/test/unit/shufflingCache.test.ts @@ -0,0 +1,121 @@ +import {describe, it, expect} from "vitest"; +import {generateTestCachedBeaconStateOnlyValidators} from "../perf/util.js"; +import {ShufflingCache, ShufflingCacheItemType, ShufflingResolution} from "../../src/cache/shufflingCache.js"; + +function countPromises(cache: ShufflingCache): number { + // eslint-disable-next-line @typescript-eslint/no-unsafe-call, @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-unsafe-return + return (cache as any).allAsArray().filter((item: any) => item.type === ShufflingCacheItemType.promise).length; +} + +function countShufflings(cache: ShufflingCache): number { + // eslint-disable-next-line @typescript-eslint/no-unsafe-call, @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-unsafe-return + return (cache as any).allAsArray().filter((item: any) => item.type === ShufflingCacheItemType.shuffling).length; +} + +describe("ShufflingCache", function () { + const vc = 64; + const stateSlot = 100; + const state = generateTestCachedBeaconStateOnlyValidators({vc, slot: stateSlot}); + const currentEpoch = state.epochCtx.epoch; + + it("should get shuffling from cache", async function () { + const shufflingCache = new ShufflingCache(); + const shuffling = shufflingCache.buildSync( + state, + state.epochCtx.epoch, + state.epochCtx.currentShufflingDecisionRoot, + state.epochCtx.currentActiveIndices + ); + expect(shufflingCache.getOrNull(currentEpoch, state.epochCtx.currentShufflingDecisionRoot)).toEqual(shuffling); + }); + + it("should bound by maxSize(=1)", async function () { + const shufflingCache = new ShufflingCache(null, {maxShufflingCacheEpochs: 1}); + const currentShuffling = shufflingCache.buildSync( + state, + state.epochCtx.epoch, + state.epochCtx.currentShufflingDecisionRoot, + state.epochCtx.currentActiveIndices + ); + expect(shufflingCache.getOrNull(currentEpoch, state.epochCtx.currentShufflingDecisionRoot)).toEqual( + currentShuffling + ); + + const nextShuffling = shufflingCache.buildSync( + state, + state.epochCtx.nextEpoch, + state.epochCtx.nextShufflingDecisionRoot, + state.epochCtx.nextActiveIndices + ); + // insert shuffling at another epoch to prune the cache + expect(shufflingCache.getOrNull(state.epochCtx.nextEpoch, state.epochCtx.nextShufflingDecisionRoot)).toEqual( + nextShuffling + ); + // the current shuffling is not available anymore + expect(shufflingCache.getOrNull(currentEpoch, state.epochCtx.currentShufflingDecisionRoot)).toBeNull(); + }); + + it("should return shuffling from promise correctly", async function () { + const shufflingCache = new ShufflingCache(); + + /* eslint-disable @typescript-eslint/no-unsafe-call, @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-unsafe-assignment */ + const resolveFn: ShufflingResolution = (shufflingCache as any)._insertShufflingPromise( + state.epochCtx.epoch, + state.epochCtx.currentShufflingDecisionRoot + ); + /* eslint-enable @typescript-eslint/no-unsafe-call, @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-unsafe-assignment */ + + expect(countPromises(shufflingCache)).toEqual(1); + const shufflingRequest0 = shufflingCache.get(state.epochCtx.epoch, state.epochCtx.currentShufflingDecisionRoot); + expect(shufflingRequest0).toBeInstanceOf(Promise); + const shufflingRequest1 = shufflingCache.get(state.epochCtx.epoch, state.epochCtx.currentShufflingDecisionRoot); + expect(shufflingRequest1).toBeInstanceOf(Promise); + + const DELAY_WAIT_TIME = 2000; + const race = await Promise.race([ + new Promise((resolve) => setTimeout(() => resolve("delay"), DELAY_WAIT_TIME)), + shufflingRequest0, + shufflingRequest1, + ]); + expect(typeof race).toEqual("string"); + expect(race).toEqual("delay"); + + // double check delay is less than a quarter of the wait time so we are sure its actually waiting + // for the inserted promises above + const startTime = Date.now(); + const shuffling = shufflingCache.buildSync( + state, + state.epochCtx.epoch, + state.epochCtx.currentShufflingDecisionRoot, + state.epochCtx.currentActiveIndices + ); + const endTime = Date.now(); + expect(endTime - startTime).toBeLessThan(DELAY_WAIT_TIME / 4); + + const [result0, result1] = await Promise.all([shufflingRequest0, shufflingRequest1]); + expect(result0).toEqual(shuffling); + expect(result1).toEqual(shuffling); + expect(countPromises(shufflingCache)).toEqual(0); + expect(countShufflings(shufflingCache)).toEqual(1); + + // double check that re-resolving for thrown away promises does not change the result + resolveFn("bad data" as any); + expect(result1).toEqual(shuffling); + }); + + it("should support up to 2 promises at a time", async function () { + const shufflingCache = new ShufflingCache(); + /* eslint-disable @typescript-eslint/no-unsafe-call, @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-unsafe-return */ + (shufflingCache as any)._insertShufflingPromise(state.epochCtx.epoch, state.epochCtx.currentShufflingDecisionRoot); + expect(countPromises(shufflingCache)).toEqual(1); + (shufflingCache as any)._insertShufflingPromise(state.epochCtx.nextEpoch, state.epochCtx.nextShufflingDecisionRoot); + expect(countPromises(shufflingCache)).toEqual(2); + expect(() => + (shufflingCache as any)._insertShufflingPromise( + state.epochCtx.nextEpoch, + state.epochCtx.nextShufflingDecisionRoot + ) + ).toThrow(); + /* eslint-enable @typescript-eslint/no-unsafe-call, @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-unsafe-return */ + }); +}); From e3e5e8f6df2dd0c61030cdfe0927da468f56eddf Mon Sep 17 00:00:00 2001 From: matthewkeil Date: Fri, 8 Mar 2024 17:27:53 +0800 Subject: [PATCH 10/23] fix: check-types issue by making IShufflingCache interface --- packages/beacon-node/src/chain/chain.ts | 3 ++- packages/beacon-node/src/chain/interface.ts | 4 ++-- .../stateCache/persistentCheckpointsCache.ts | 6 ++--- .../test/mocks/mockedBeaconChain.ts | 4 ++-- .../beacon-node/test/mocks/shufflingMock.ts | 4 ++-- .../state-transition/src/cache/epochCache.ts | 8 +++---- .../src/cache/shufflingCache.ts | 22 ++++++++++++++++++- .../test/unit/cachedBeaconState.test.ts | 4 +--- 8 files changed, 37 insertions(+), 18 deletions(-) diff --git a/packages/beacon-node/src/chain/chain.ts b/packages/beacon-node/src/chain/chain.ts index 8cc1ed50ee5c..e6f4c56015b7 100644 --- a/packages/beacon-node/src/chain/chain.ts +++ b/packages/beacon-node/src/chain/chain.ts @@ -13,6 +13,7 @@ import { PubkeyIndexMap, EpochShuffling, ShufflingCache, + IShufflingCache, ShufflingCacheError, ShufflingCacheErrorCode, } from "@lodestar/state-transition"; @@ -138,7 +139,7 @@ export class BeaconChain implements IBeaconChain { readonly beaconProposerCache: BeaconProposerCache; readonly checkpointBalancesCache: CheckpointBalancesCache; - readonly shufflingCache: ShufflingCache; + readonly shufflingCache: IShufflingCache; /** Map keyed by executionPayload.blockHash of the block for those blobs */ readonly producedContentsCache = new Map(); diff --git a/packages/beacon-node/src/chain/interface.ts b/packages/beacon-node/src/chain/interface.ts index 13a1918fb591..d12daf0de7b9 100644 --- a/packages/beacon-node/src/chain/interface.ts +++ b/packages/beacon-node/src/chain/interface.ts @@ -17,7 +17,7 @@ import { BeaconStateAllForks, CachedBeaconStateAllForks, EpochShuffling, - ShufflingCache, + IShufflingCache, Index2PubkeyCache, PubkeyIndexMap, } from "@lodestar/state-transition"; @@ -114,7 +114,7 @@ export interface IBeaconChain { readonly checkpointBalancesCache: CheckpointBalancesCache; readonly producedContentsCache: Map; readonly producedBlockRoot: Map; - readonly shufflingCache: ShufflingCache; + readonly shufflingCache: IShufflingCache; readonly producedBlindedBlockRoot: Set; readonly opts: IChainOptions; diff --git a/packages/beacon-node/src/chain/stateCache/persistentCheckpointsCache.ts b/packages/beacon-node/src/chain/stateCache/persistentCheckpointsCache.ts index e95b03fd653e..72c1658cb6d0 100644 --- a/packages/beacon-node/src/chain/stateCache/persistentCheckpointsCache.ts +++ b/packages/beacon-node/src/chain/stateCache/persistentCheckpointsCache.ts @@ -2,7 +2,7 @@ import {fromHexString, toHexString} from "@chainsafe/ssz"; import {phase0, Epoch, RootHex} from "@lodestar/types"; import { loadCachedBeaconState, - ShufflingCache, + IShufflingCache, CachedBeaconStateAllForks, computeStartSlotAtEpoch, getBlockRootAtSlot, @@ -32,7 +32,7 @@ type PersistentCheckpointStateCacheModules = { logger: Logger; clock?: IClock | null; signal?: AbortSignal; - shufflingCache: ShufflingCache; + shufflingCache: IShufflingCache; datastore: CPStateDatastore; getHeadState?: GetHeadStateFn; bufferPool?: BufferPool; @@ -107,7 +107,7 @@ export class PersistentCheckpointStateCache implements CheckpointStateCache { private readonly maxEpochsInMemory: number; private readonly processLateBlock: boolean; private readonly datastore: CPStateDatastore; - private readonly shufflingCache: ShufflingCache; + private readonly shufflingCache: IShufflingCache; private readonly getHeadState?: GetHeadStateFn; private readonly bufferPool?: BufferPool; diff --git a/packages/beacon-node/test/mocks/mockedBeaconChain.ts b/packages/beacon-node/test/mocks/mockedBeaconChain.ts index 099b7a902be8..f02d399dd434 100644 --- a/packages/beacon-node/test/mocks/mockedBeaconChain.ts +++ b/packages/beacon-node/test/mocks/mockedBeaconChain.ts @@ -4,7 +4,7 @@ import {config as defaultConfig} from "@lodestar/config/default"; import {ChainForkConfig} from "@lodestar/config"; import {ForkChoice, ProtoBlock, EpochDifference} from "@lodestar/fork-choice"; import {Logger} from "@lodestar/utils"; -import {ShufflingCache} from "@lodestar/state-transition"; +import {IShufflingCache} from "@lodestar/state-transition"; import {BeaconChain} from "../../src/chain/chain.js"; import {ChainEventEmitter} from "../../src/chain/emitter.js"; import {ExecutionEngineHttp} from "../../src/execution/engine/index.js"; @@ -28,7 +28,7 @@ export type MockedBeaconChain = Mocked & { opPool: Mocked; aggregatedAttestationPool: Mocked; beaconProposerCache: Mocked; - shufflingCache: Mocked; + shufflingCache: Mocked; regen: Mocked; bls: { verifySignatureSets: Mock<[boolean]>; diff --git a/packages/beacon-node/test/mocks/shufflingMock.ts b/packages/beacon-node/test/mocks/shufflingMock.ts index d19b9a754818..2086f9c032a2 100644 --- a/packages/beacon-node/test/mocks/shufflingMock.ts +++ b/packages/beacon-node/test/mocks/shufflingMock.ts @@ -1,9 +1,9 @@ import {vi, Mocked} from "vitest"; // eslint-disable-next-line import/no-relative-packages -import {ShufflingCache} from "../../../state-transition/src/cache/shufflingCache.js"; +import {IShufflingCache, ShufflingCache} from "../../../state-transition/src/cache/shufflingCache.js"; vi.mock("../../../state-transition/src/cache/shufflingCache.js"); -export function getMockedShufflingCache(): Mocked { +export function getMockedShufflingCache(): Mocked { return vi.mocked(new ShufflingCache({} as any)); } diff --git a/packages/state-transition/src/cache/epochCache.ts b/packages/state-transition/src/cache/epochCache.ts index 86c1a3c91e1e..ff91adf77bf3 100644 --- a/packages/state-transition/src/cache/epochCache.ts +++ b/packages/state-transition/src/cache/epochCache.ts @@ -41,7 +41,7 @@ import { SyncCommitteeCache, SyncCommitteeCacheEmpty, } from "./syncCommitteeCache.js"; -import {ShufflingCache, ShufflingCacheCaller} from "./shufflingCache.js"; +import {ShufflingCache, IShufflingCache, ShufflingCacheCaller} from "./shufflingCache.js"; /** `= PROPOSER_WEIGHT / (WEIGHT_DENOMINATOR - PROPOSER_WEIGHT)` */ export const PROPOSER_WEIGHT_FACTOR = PROPOSER_WEIGHT / (WEIGHT_DENOMINATOR - PROPOSER_WEIGHT); @@ -49,7 +49,7 @@ export const PROPOSER_WEIGHT_FACTOR = PROPOSER_WEIGHT / (WEIGHT_DENOMINATOR - PR export type EpochCacheImmutableData = { config: BeaconConfig; logger: Logger; - shufflingCache: ShufflingCache; + shufflingCache: IShufflingCache; pubkey2index: PubkeyIndexMap; index2pubkey: Index2PubkeyCache; }; @@ -88,7 +88,7 @@ type ProposersDeferred = {computed: false; seed: Uint8Array} | {computed: true; export class EpochCache { config: BeaconConfig; logger: Logger; - shufflingCache: ShufflingCache; + shufflingCache: IShufflingCache; /** * Unique globally shared pubkey registry. There should only exist one for the entire application. @@ -223,7 +223,7 @@ export class EpochCache { constructor(data: { config: BeaconConfig; logger: Logger; - shufflingCache: ShufflingCache; + shufflingCache: IShufflingCache; pubkey2index: PubkeyIndexMap; index2pubkey: Index2PubkeyCache; proposers: number[]; diff --git a/packages/state-transition/src/cache/shufflingCache.ts b/packages/state-transition/src/cache/shufflingCache.ts index ed8813db167b..2c14282a864a 100644 --- a/packages/state-transition/src/cache/shufflingCache.ts +++ b/packages/state-transition/src/cache/shufflingCache.ts @@ -79,13 +79,33 @@ export interface ShufflingCacheOptions { maxShufflingCacheEpochs?: number; } +export interface IShufflingCache { + addMetrics(metrics: ShufflingCacheMetrics | null): void; + get(shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex): Promise; + getOrError(shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex, caller: ShufflingCacheCaller): EpochShuffling; + getOrNull(shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex, isReload?: boolean): EpochShuffling | null; + add(shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex, shuffling: EpochShuffling): void; + buildSync( + state: BeaconStateAllForks, + shufflingEpoch: Epoch, + shufflingDecisionRoot: RootHex, + activeIndexes: number[] + ): EpochShuffling; + build( + state: BeaconStateAllForks, + shufflingEpoch: Epoch, + shufflingDecisionRoot: RootHex, + activeIndexes: number[] + ): Promise; +} + /** * A shuffling cache to help: * - get committee quickly for attestation verification * - if a shuffling is not available (which does not happen with default chain option of maxSkipSlots = 32), track a promise to make sure we don't compute the same shuffling twice * - skip computing shuffling when loading state bytes from disk */ -export class ShufflingCache { +export class ShufflingCache implements IShufflingCache { /** LRU cache implemented as a map, pruned every time we add an item */ private readonly itemsByDecisionRootByEpoch: MapDef> = new MapDef( () => new Map() diff --git a/packages/state-transition/test/unit/cachedBeaconState.test.ts b/packages/state-transition/test/unit/cachedBeaconState.test.ts index efad7efd0555..bb014f234d04 100644 --- a/packages/state-transition/test/unit/cachedBeaconState.test.ts +++ b/packages/state-transition/test/unit/cachedBeaconState.test.ts @@ -1,5 +1,5 @@ import {describe, it, expect} from "vitest"; -import {Epoch, ssz, RootHex} from "@lodestar/types"; +import {ssz} from "@lodestar/types"; import {LogLevel, toHexString} from "@lodestar/utils"; import {config as defaultConfig} from "@lodestar/config/default"; import {createBeaconConfig} from "@lodestar/config"; @@ -9,8 +9,6 @@ import {PubkeyIndexMap} from "../../src/cache/pubkeyCache.js"; import {createCachedBeaconState, loadCachedBeaconState} from "../../src/cache/stateCache.js"; import {interopPubkeysCached} from "../utils/interop.js"; import {modifyStateSameValidator, newStateWithValidators} from "../utils/capella.js"; -import {EpochShuffling} from "../../src/util/epochShuffling.js"; -import {getShufflingDecisionBlock} from "../../src/util/shufflingDecisionRoot.js"; import {ShufflingCache} from "../../src/cache/shufflingCache.js"; describe("CachedBeaconState", () => { From 79f9b7e7baf427a52c84dbc64ec61ec78f5d76e4 Mon Sep 17 00:00:00 2001 From: matthewkeil Date: Fri, 8 Mar 2024 18:03:07 +0800 Subject: [PATCH 11/23] chore: lint --- packages/beacon-node/src/chain/blocks/importBlock.ts | 1 - .../test/perf/block/processAttestation.test.ts | 3 ++- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/beacon-node/src/chain/blocks/importBlock.ts b/packages/beacon-node/src/chain/blocks/importBlock.ts index 85c8a3663986..75cc23083c25 100644 --- a/packages/beacon-node/src/chain/blocks/importBlock.ts +++ b/packages/beacon-node/src/chain/blocks/importBlock.ts @@ -64,7 +64,6 @@ export async function importBlock( const blockRootHex = toHexString(blockRoot); const currentEpoch = computeEpochAtSlot(this.forkChoice.getTime()); const blockEpoch = computeEpochAtSlot(blockSlot); - const parentEpoch = computeEpochAtSlot(parentBlockSlot); const prevFinalizedEpoch = this.forkChoice.getFinalizedCheckpoint().epoch; const blockDelaySec = (fullyVerifiedBlock.seenTimestampSec - postState.genesisTime) % this.config.SECONDS_PER_SLOT; const recvToValLatency = Date.now() / 1000 - (opts.seenTimestampSec ?? Date.now() / 1000); diff --git a/packages/state-transition/test/perf/block/processAttestation.test.ts b/packages/state-transition/test/perf/block/processAttestation.test.ts index 7c2461fb2b64..6e4a93d3e50b 100644 --- a/packages/state-transition/test/perf/block/processAttestation.test.ts +++ b/packages/state-transition/test/perf/block/processAttestation.test.ts @@ -118,7 +118,8 @@ describe("altair processAttestation - CachedEpochParticipation.setStatus", () => const numAttesters = Math.floor((state.epochCtx.currentActiveIndices.length * ratio) / SLOTS_PER_EPOCH); // just get committees of slot 10 let count = 0; - for (const committees of state.epochCtx.getShufflingAtSlot(state.slot, ShufflingCacheCaller.testing).committees[10]) { + for (const committees of state.epochCtx.getShufflingAtSlot(state.slot, ShufflingCacheCaller.testing) + .committees[10]) { for (const committee of committees) { currentEpochParticipation.set(committee, 0b111); count++; From 12f28283e05cad1616f53552101f853673343d4d Mon Sep 17 00:00:00 2001 From: Cayman Date: Mon, 8 Jul 2024 14:05:25 -0400 Subject: [PATCH 12/23] chore: clean shuffling test getter --- .../src/cache/shufflingCache.ts | 6 ------ .../test/unit/shufflingCache.test.ts | 17 ++++++++++++----- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/packages/state-transition/src/cache/shufflingCache.ts b/packages/state-transition/src/cache/shufflingCache.ts index 2c14282a864a..b53ebf8a2d1c 100644 --- a/packages/state-transition/src/cache/shufflingCache.ts +++ b/packages/state-transition/src/cache/shufflingCache.ts @@ -116,12 +116,6 @@ export class ShufflingCache implements IShufflingCache { constructor(metrics: ShufflingCacheMetrics | null = null, opts: ShufflingCacheOptions = {}) { this.maxEpochs = opts.maxShufflingCacheEpochs ?? SHUFFLING_CACHE_MAX_EPOCHS; this.addMetrics(metrics); - // just used for testing and don't want to pollute the public api - Object.defineProperty(this, "allAsArray", { - enumerable: false, - value: () => - Array.from(this.itemsByDecisionRootByEpoch.values()).flatMap((innerMap) => Array.from(innerMap.values())), - }); } addMetrics(metrics: ShufflingCacheMetrics | null): void { diff --git a/packages/state-transition/test/unit/shufflingCache.test.ts b/packages/state-transition/test/unit/shufflingCache.test.ts index 32d37d5f55d8..4c2f2bbfda13 100644 --- a/packages/state-transition/test/unit/shufflingCache.test.ts +++ b/packages/state-transition/test/unit/shufflingCache.test.ts @@ -1,15 +1,22 @@ import {describe, it, expect} from "vitest"; import {generateTestCachedBeaconStateOnlyValidators} from "../perf/util.js"; -import {ShufflingCache, ShufflingCacheItemType, ShufflingResolution} from "../../src/cache/shufflingCache.js"; +import { + ShufflingCache, + ShufflingCacheItem, + ShufflingCacheItemType, + ShufflingResolution, +} from "../../src/cache/shufflingCache.js"; + +function allShufflingItems(c: ShufflingCache): ShufflingCacheItem[] { + return Array.from(c["itemsByDecisionRootByEpoch"].values()).flatMap((innerMap) => Array.from(innerMap.values())); +} function countPromises(cache: ShufflingCache): number { - // eslint-disable-next-line @typescript-eslint/no-unsafe-call, @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-unsafe-return - return (cache as any).allAsArray().filter((item: any) => item.type === ShufflingCacheItemType.promise).length; + return allShufflingItems(cache).filter((item) => item.type === ShufflingCacheItemType.promise).length; } function countShufflings(cache: ShufflingCache): number { - // eslint-disable-next-line @typescript-eslint/no-unsafe-call, @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-unsafe-return - return (cache as any).allAsArray().filter((item: any) => item.type === ShufflingCacheItemType.shuffling).length; + return allShufflingItems(cache).filter((item) => item.type === ShufflingCacheItemType.shuffling).length; } describe("ShufflingCache", function () { From 54de65693bfc3b9523cebe0802bad99641277a09 Mon Sep 17 00:00:00 2001 From: Cayman Date: Mon, 8 Jul 2024 14:08:13 -0400 Subject: [PATCH 13/23] chore: clean test typing --- packages/state-transition/src/cache/shufflingCache.ts | 2 +- .../state-transition/test/unit/shufflingCache.test.ts | 11 ++--------- 2 files changed, 3 insertions(+), 10 deletions(-) diff --git a/packages/state-transition/src/cache/shufflingCache.ts b/packages/state-transition/src/cache/shufflingCache.ts index b53ebf8a2d1c..95186dcf9512 100644 --- a/packages/state-transition/src/cache/shufflingCache.ts +++ b/packages/state-transition/src/cache/shufflingCache.ts @@ -73,7 +73,7 @@ export type ShufflingCachePromiseItem = { export type ShufflingCacheItem = ShufflingCacheShufflingItem | ShufflingCachePromiseItem; -export type ShufflingResolution = (shuffling: EpochShuffling) => void; +type ShufflingResolution = (shuffling: EpochShuffling) => void; export interface ShufflingCacheOptions { maxShufflingCacheEpochs?: number; diff --git a/packages/state-transition/test/unit/shufflingCache.test.ts b/packages/state-transition/test/unit/shufflingCache.test.ts index 4c2f2bbfda13..ce355d0e23f5 100644 --- a/packages/state-transition/test/unit/shufflingCache.test.ts +++ b/packages/state-transition/test/unit/shufflingCache.test.ts @@ -1,11 +1,6 @@ import {describe, it, expect} from "vitest"; import {generateTestCachedBeaconStateOnlyValidators} from "../perf/util.js"; -import { - ShufflingCache, - ShufflingCacheItem, - ShufflingCacheItemType, - ShufflingResolution, -} from "../../src/cache/shufflingCache.js"; +import {ShufflingCache, ShufflingCacheItem, ShufflingCacheItemType} from "../../src/cache/shufflingCache.js"; function allShufflingItems(c: ShufflingCache): ShufflingCacheItem[] { return Array.from(c["itemsByDecisionRootByEpoch"].values()).flatMap((innerMap) => Array.from(innerMap.values())); @@ -65,12 +60,10 @@ describe("ShufflingCache", function () { it("should return shuffling from promise correctly", async function () { const shufflingCache = new ShufflingCache(); - /* eslint-disable @typescript-eslint/no-unsafe-call, @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-unsafe-assignment */ - const resolveFn: ShufflingResolution = (shufflingCache as any)._insertShufflingPromise( + const resolveFn = shufflingCache["_insertShufflingPromise"]( state.epochCtx.epoch, state.epochCtx.currentShufflingDecisionRoot ); - /* eslint-enable @typescript-eslint/no-unsafe-call, @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-unsafe-assignment */ expect(countPromises(shufflingCache)).toEqual(1); const shufflingRequest0 = shufflingCache.get(state.epochCtx.epoch, state.epochCtx.currentShufflingDecisionRoot); From 51cc95847c5d095464547958acdbbb8a654080d3 Mon Sep 17 00:00:00 2001 From: Cayman Date: Mon, 8 Jul 2024 14:55:18 -0400 Subject: [PATCH 14/23] chore: clean getOrNull fn signature --- .../state-transition/src/cache/epochCache.ts | 11 ++-- .../src/cache/shufflingCache.ts | 63 ++++++++++--------- .../test/unit/shufflingCache.test.ts | 47 ++++++++++---- 3 files changed, 73 insertions(+), 48 deletions(-) diff --git a/packages/state-transition/src/cache/epochCache.ts b/packages/state-transition/src/cache/epochCache.ts index ff91adf77bf3..4a685bab2969 100644 --- a/packages/state-transition/src/cache/epochCache.ts +++ b/packages/state-transition/src/cache/epochCache.ts @@ -321,16 +321,13 @@ export class EpochCache { // BeaconChain could provide a shuffling cache to avoid re-computing shuffling every epoch // in that case, we don't need to compute shufflings again + const caller = opts?.isReload ? ShufflingCacheCaller.reloadCreateFromState : ShufflingCacheCaller.createFromState; const previousShufflingDecisionRoot = getShufflingDecisionBlock(state, previousEpoch); - const cachedPreviousShuffling = shufflingCache.getOrNull( - previousEpoch, - previousShufflingDecisionRoot, - opts?.isReload - ); + const cachedPreviousShuffling = shufflingCache.getOrNull(previousEpoch, previousShufflingDecisionRoot, caller); const currentShufflingDecisionRoot = getShufflingDecisionBlock(state, currentEpoch); - const cachedCurrentShuffling = shufflingCache.getOrNull(currentEpoch, currentShufflingDecisionRoot, opts?.isReload); + const cachedCurrentShuffling = shufflingCache.getOrNull(currentEpoch, currentShufflingDecisionRoot, caller); const nextShufflingDecisionRoot = getShufflingDecisionBlock(state, nextEpoch); - const cachedNextShuffling = shufflingCache.getOrNull(nextEpoch, nextShufflingDecisionRoot, opts?.isReload); + const cachedNextShuffling = shufflingCache.getOrNull(nextEpoch, nextShufflingDecisionRoot, caller); for (let i = 0; i < validatorCount; i++) { const validator = validators[i]; diff --git a/packages/state-transition/src/cache/shufflingCache.ts b/packages/state-transition/src/cache/shufflingCache.ts index 95186dcf9512..86267738a0ad 100644 --- a/packages/state-transition/src/cache/shufflingCache.ts +++ b/packages/state-transition/src/cache/shufflingCache.ts @@ -18,7 +18,6 @@ export const SHUFFLING_CACHE_MAX_PROMISES = 2; export const SHUFFLING_CACHE_MAX_EPOCHS = 4; export enum ShufflingCacheCaller { - testing = "testing", buildShuffling = "buildShuffling", synchronousBuildShuffling = "synchronousBuildShuffling", attestationVerification = "attestationVerification", @@ -73,8 +72,6 @@ export type ShufflingCachePromiseItem = { export type ShufflingCacheItem = ShufflingCacheShufflingItem | ShufflingCachePromiseItem; -type ShufflingResolution = (shuffling: EpochShuffling) => void; - export interface ShufflingCacheOptions { maxShufflingCacheEpochs?: number; } @@ -83,7 +80,7 @@ export interface IShufflingCache { addMetrics(metrics: ShufflingCacheMetrics | null): void; get(shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex): Promise; getOrError(shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex, caller: ShufflingCacheCaller): EpochShuffling; - getOrNull(shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex, isReload?: boolean): EpochShuffling | null; + getOrNull(shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex, caller: ShufflingCacheCaller): EpochShuffling | null; add(shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex, shuffling: EpochShuffling): void; buildSync( state: BeaconStateAllForks, @@ -170,13 +167,12 @@ export class ShufflingCache implements IShufflingCache { * Will synchronously get a shuffling if it is available or will return null if not. The consumer * will have to then submit for building the shuffling. Metrics are collected by this._get */ - getOrNull(shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex, isReload = false): EpochShuffling | null { - return this._get( - shufflingEpoch, - shufflingDecisionRoot, - false, - isReload ? ShufflingCacheCaller.reloadCreateFromState : ShufflingCacheCaller.createFromState - ); + getOrNull( + shufflingEpoch: Epoch, + shufflingDecisionRoot: RootHex, + caller: ShufflingCacheCaller + ): EpochShuffling | null { + return this._get(shufflingEpoch, shufflingDecisionRoot, false, caller); } add(shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex, shuffling: EpochShuffling): void { @@ -193,7 +189,6 @@ export class ShufflingCache implements IShufflingCache { activeIndexes: number[] ): EpochShuffling { const cacheItem = this.itemsByDecisionRootByEpoch.getOrDefault(shufflingEpoch).get(shufflingDecisionRoot); - let resolveFn: ShufflingResolution; if (cacheItem) { if (this.isShufflingCacheItem(cacheItem)) { this.metrics?.shufflingCache.cacheHit.inc({caller: ShufflingCacheCaller.synchronousBuildShuffling}); @@ -205,14 +200,12 @@ export class ShufflingCache implements IShufflingCache { // TODO: (matthewkeil) Perhaps we should throw an error instead. Should not happen ideally this.metrics?.shufflingCache.cacheHitRebuildPromise.inc(); // add log statement here with epoch and decision root for this miss - resolveFn = cacheItem.resolveFn; } else { - resolveFn = this._insertShufflingPromise(shufflingEpoch, shufflingDecisionRoot); this.metrics?.shufflingCache.cacheMiss.inc({caller: ShufflingCacheCaller.synchronousBuildShuffling}); } - const shuffling = this._build(state, shufflingEpoch, shufflingDecisionRoot, activeIndexes); - resolveFn(shuffling); + const shuffling = this._buildSync(state, shufflingEpoch, shufflingDecisionRoot, activeIndexes); + cacheItem?.resolveFn(shuffling); return shuffling; } @@ -234,13 +227,9 @@ export class ShufflingCache implements IShufflingCache { // this is to prevent multiple calls to get shuffling for the same epoch and dependent root // any subsequent calls of the same epoch and dependent root will wait for this promise to resolve - const resolveFn = this._insertShufflingPromise(shufflingEpoch, shufflingDecisionRoot); - // TODO: (matthewkeil) replace this sync call with a worker and async build function that uses - // a nice'd thread to build in core idle time - // - // Building will overwrite the ShufflingCachePromiseItem with the ShufflingCacheShufflingItem - const shuffling = this._build(state, shufflingEpoch, shufflingDecisionRoot, activeIndexes); - resolveFn(shuffling); + const item = this._insertShufflingPromise(shufflingEpoch, shufflingDecisionRoot); + const shuffling = await this._build(state, shufflingEpoch, shufflingDecisionRoot, activeIndexes); + item.resolveFn(shuffling); return shuffling; } @@ -277,7 +266,7 @@ export class ShufflingCache implements IShufflingCache { return cacheItem.shuffling; } - private _build( + private _buildSync( state: BeaconStateAllForks, shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex, @@ -288,7 +277,22 @@ export class ShufflingCache implements IShufflingCache { return shuffling; } - private _insertShufflingPromise(shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex): ShufflingResolution { + private async _build( + state: BeaconStateAllForks, + shufflingEpoch: Epoch, + shufflingDecisionRoot: RootHex, + activeIndexes: number[] + ): Promise { + // TODO: (matthewkeil) replace this sync call with a worker and async build function that uses + // a nice'd thread to build in core idle time + // + // Building will overwrite the ShufflingCachePromiseItem with the ShufflingCacheShufflingItem + const shuffling = computeEpochShuffling(state, activeIndexes, shufflingEpoch); + this.add(shufflingEpoch, shufflingDecisionRoot, shuffling); + return shuffling; + } + + private _insertShufflingPromise(shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex): ShufflingCachePromiseItem { const promiseCount = Array.from(this.itemsByDecisionRootByEpoch.values()) .flatMap((innerMap) => Array.from(innerMap.values())) .filter((item) => this.isPromiseCacheItem(item)).length; @@ -297,14 +301,13 @@ export class ShufflingCache implements IShufflingCache { `Too many shuffling promises: ${promiseCount}, shufflingEpoch: ${shufflingEpoch}, shufflingDecisionRoot: ${shufflingDecisionRoot}` ); } - let resolveFn!: ShufflingResolution; + let resolveFn!: (s: EpochShuffling) => void; const promise = new Promise((resolve) => { resolveFn = resolve; }); - this.itemsByDecisionRootByEpoch - .getOrDefault(shufflingEpoch) - .set(shufflingDecisionRoot, {type: ShufflingCacheItemType.promise, promise, resolveFn}); - return resolveFn; + const item: ShufflingCachePromiseItem = {type: ShufflingCacheItemType.promise, promise, resolveFn}; + this.itemsByDecisionRootByEpoch.getOrDefault(shufflingEpoch).set(shufflingDecisionRoot, item); + return item; } private isShufflingCacheItem(item: ShufflingCacheItem): item is ShufflingCacheShufflingItem { diff --git a/packages/state-transition/test/unit/shufflingCache.test.ts b/packages/state-transition/test/unit/shufflingCache.test.ts index ce355d0e23f5..eba0c76b7923 100644 --- a/packages/state-transition/test/unit/shufflingCache.test.ts +++ b/packages/state-transition/test/unit/shufflingCache.test.ts @@ -1,6 +1,11 @@ import {describe, it, expect} from "vitest"; import {generateTestCachedBeaconStateOnlyValidators} from "../perf/util.js"; -import {ShufflingCache, ShufflingCacheItem, ShufflingCacheItemType} from "../../src/cache/shufflingCache.js"; +import { + ShufflingCache, + ShufflingCacheCaller, + ShufflingCacheItem, + ShufflingCacheItemType, +} from "../../src/cache/shufflingCache.js"; function allShufflingItems(c: ShufflingCache): ShufflingCacheItem[] { return Array.from(c["itemsByDecisionRootByEpoch"].values()).flatMap((innerMap) => Array.from(innerMap.values())); @@ -28,7 +33,13 @@ describe("ShufflingCache", function () { state.epochCtx.currentShufflingDecisionRoot, state.epochCtx.currentActiveIndices ); - expect(shufflingCache.getOrNull(currentEpoch, state.epochCtx.currentShufflingDecisionRoot)).toEqual(shuffling); + expect( + shufflingCache.getOrNull( + currentEpoch, + state.epochCtx.currentShufflingDecisionRoot, + ShufflingCacheCaller.createFromState + ) + ).toEqual(shuffling); }); it("should bound by maxSize(=1)", async function () { @@ -39,9 +50,13 @@ describe("ShufflingCache", function () { state.epochCtx.currentShufflingDecisionRoot, state.epochCtx.currentActiveIndices ); - expect(shufflingCache.getOrNull(currentEpoch, state.epochCtx.currentShufflingDecisionRoot)).toEqual( - currentShuffling - ); + expect( + shufflingCache.getOrNull( + currentEpoch, + state.epochCtx.currentShufflingDecisionRoot, + ShufflingCacheCaller.createFromState + ) + ).toEqual(currentShuffling); const nextShuffling = shufflingCache.buildSync( state, @@ -50,17 +65,27 @@ describe("ShufflingCache", function () { state.epochCtx.nextActiveIndices ); // insert shuffling at another epoch to prune the cache - expect(shufflingCache.getOrNull(state.epochCtx.nextEpoch, state.epochCtx.nextShufflingDecisionRoot)).toEqual( - nextShuffling - ); + expect( + shufflingCache.getOrNull( + state.epochCtx.nextEpoch, + state.epochCtx.nextShufflingDecisionRoot, + ShufflingCacheCaller.createFromState + ) + ).toEqual(nextShuffling); // the current shuffling is not available anymore - expect(shufflingCache.getOrNull(currentEpoch, state.epochCtx.currentShufflingDecisionRoot)).toBeNull(); + expect( + shufflingCache.getOrNull( + currentEpoch, + state.epochCtx.currentShufflingDecisionRoot, + ShufflingCacheCaller.createFromState + ) + ).toBeNull(); }); it("should return shuffling from promise correctly", async function () { const shufflingCache = new ShufflingCache(); - const resolveFn = shufflingCache["_insertShufflingPromise"]( + const cacheItem = shufflingCache["_insertShufflingPromise"]( state.epochCtx.epoch, state.epochCtx.currentShufflingDecisionRoot ); @@ -99,7 +124,7 @@ describe("ShufflingCache", function () { expect(countShufflings(shufflingCache)).toEqual(1); // double check that re-resolving for thrown away promises does not change the result - resolveFn("bad data" as any); + cacheItem.resolveFn("bad data" as any); expect(result1).toEqual(shuffling); }); From 90b4b69763ba3b741548c963036dcfad1e15229d Mon Sep 17 00:00:00 2001 From: Cayman Date: Mon, 8 Jul 2024 14:59:33 -0400 Subject: [PATCH 15/23] chore: rename getSync --- packages/state-transition/src/cache/epochCache.ts | 6 +++--- packages/state-transition/src/cache/shufflingCache.ts | 8 ++------ .../state-transition/test/unit/shufflingCache.test.ts | 8 ++++---- 3 files changed, 9 insertions(+), 13 deletions(-) diff --git a/packages/state-transition/src/cache/epochCache.ts b/packages/state-transition/src/cache/epochCache.ts index 4a685bab2969..92e55a2c1b9a 100644 --- a/packages/state-transition/src/cache/epochCache.ts +++ b/packages/state-transition/src/cache/epochCache.ts @@ -323,11 +323,11 @@ export class EpochCache { // in that case, we don't need to compute shufflings again const caller = opts?.isReload ? ShufflingCacheCaller.reloadCreateFromState : ShufflingCacheCaller.createFromState; const previousShufflingDecisionRoot = getShufflingDecisionBlock(state, previousEpoch); - const cachedPreviousShuffling = shufflingCache.getOrNull(previousEpoch, previousShufflingDecisionRoot, caller); + const cachedPreviousShuffling = shufflingCache.getSync(previousEpoch, previousShufflingDecisionRoot, caller); const currentShufflingDecisionRoot = getShufflingDecisionBlock(state, currentEpoch); - const cachedCurrentShuffling = shufflingCache.getOrNull(currentEpoch, currentShufflingDecisionRoot, caller); + const cachedCurrentShuffling = shufflingCache.getSync(currentEpoch, currentShufflingDecisionRoot, caller); const nextShufflingDecisionRoot = getShufflingDecisionBlock(state, nextEpoch); - const cachedNextShuffling = shufflingCache.getOrNull(nextEpoch, nextShufflingDecisionRoot, caller); + const cachedNextShuffling = shufflingCache.getSync(nextEpoch, nextShufflingDecisionRoot, caller); for (let i = 0; i < validatorCount; i++) { const validator = validators[i]; diff --git a/packages/state-transition/src/cache/shufflingCache.ts b/packages/state-transition/src/cache/shufflingCache.ts index 86267738a0ad..3bd23d8f745e 100644 --- a/packages/state-transition/src/cache/shufflingCache.ts +++ b/packages/state-transition/src/cache/shufflingCache.ts @@ -80,7 +80,7 @@ export interface IShufflingCache { addMetrics(metrics: ShufflingCacheMetrics | null): void; get(shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex): Promise; getOrError(shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex, caller: ShufflingCacheCaller): EpochShuffling; - getOrNull(shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex, caller: ShufflingCacheCaller): EpochShuffling | null; + getSync(shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex, caller: ShufflingCacheCaller): EpochShuffling | null; add(shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex, shuffling: EpochShuffling): void; buildSync( state: BeaconStateAllForks, @@ -167,11 +167,7 @@ export class ShufflingCache implements IShufflingCache { * Will synchronously get a shuffling if it is available or will return null if not. The consumer * will have to then submit for building the shuffling. Metrics are collected by this._get */ - getOrNull( - shufflingEpoch: Epoch, - shufflingDecisionRoot: RootHex, - caller: ShufflingCacheCaller - ): EpochShuffling | null { + getSync(shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex, caller: ShufflingCacheCaller): EpochShuffling | null { return this._get(shufflingEpoch, shufflingDecisionRoot, false, caller); } diff --git a/packages/state-transition/test/unit/shufflingCache.test.ts b/packages/state-transition/test/unit/shufflingCache.test.ts index eba0c76b7923..04158d215fa3 100644 --- a/packages/state-transition/test/unit/shufflingCache.test.ts +++ b/packages/state-transition/test/unit/shufflingCache.test.ts @@ -34,7 +34,7 @@ describe("ShufflingCache", function () { state.epochCtx.currentActiveIndices ); expect( - shufflingCache.getOrNull( + shufflingCache.getSync( currentEpoch, state.epochCtx.currentShufflingDecisionRoot, ShufflingCacheCaller.createFromState @@ -51,7 +51,7 @@ describe("ShufflingCache", function () { state.epochCtx.currentActiveIndices ); expect( - shufflingCache.getOrNull( + shufflingCache.getSync( currentEpoch, state.epochCtx.currentShufflingDecisionRoot, ShufflingCacheCaller.createFromState @@ -66,7 +66,7 @@ describe("ShufflingCache", function () { ); // insert shuffling at another epoch to prune the cache expect( - shufflingCache.getOrNull( + shufflingCache.getSync( state.epochCtx.nextEpoch, state.epochCtx.nextShufflingDecisionRoot, ShufflingCacheCaller.createFromState @@ -74,7 +74,7 @@ describe("ShufflingCache", function () { ).toEqual(nextShuffling); // the current shuffling is not available anymore expect( - shufflingCache.getOrNull( + shufflingCache.getSync( currentEpoch, state.epochCtx.currentShufflingDecisionRoot, ShufflingCacheCaller.createFromState From 8cb173494c9ddeed3fecf2291d96466309f9e06e Mon Sep 17 00:00:00 2001 From: Cayman Date: Mon, 8 Jul 2024 15:08:11 -0400 Subject: [PATCH 16/23] chore: simplify shuffling cache interface --- packages/beacon-node/src/chain/chain.ts | 10 ++--- .../state-transition/src/cache/epochCache.ts | 23 ++++++++-- .../src/cache/shufflingCache.ts | 43 +------------------ 3 files changed, 25 insertions(+), 51 deletions(-) diff --git a/packages/beacon-node/src/chain/chain.ts b/packages/beacon-node/src/chain/chain.ts index e6f4c56015b7..fd75a1d60434 100644 --- a/packages/beacon-node/src/chain/chain.ts +++ b/packages/beacon-node/src/chain/chain.ts @@ -14,8 +14,6 @@ import { EpochShuffling, ShufflingCache, IShufflingCache, - ShufflingCacheError, - ShufflingCacheErrorCode, } from "@lodestar/state-transition"; import {BeaconConfig} from "@lodestar/config"; import { @@ -738,11 +736,9 @@ export class BeaconChain implements IBeaconChain { // resolve the promise to unblock other calls of the same epoch and dependent root const shuffling = await this.shufflingCache.get(attEpoch, shufflingDependentRoot); if (!shuffling) { - throw new ShufflingCacheError({ - code: ShufflingCacheErrorCode.REGEN_ERROR_NO_SHUFFLING_FOUND, - epoch: attEpoch, - shufflingDecisionRoot: shufflingDependentRoot, - }); + // This will be essentially unreachable considering regen should build the shuffling for this epoch + // but need to handle anyhow + throw new Error("UNREACHABLE: Shuffling not found for attestation verification"); } return shuffling; } diff --git a/packages/state-transition/src/cache/epochCache.ts b/packages/state-transition/src/cache/epochCache.ts index 92e55a2c1b9a..36d7632ae18f 100644 --- a/packages/state-transition/src/cache/epochCache.ts +++ b/packages/state-transition/src/cache/epochCache.ts @@ -841,12 +841,22 @@ export class EpochCache { } getShufflingAtSlot(slot: Slot, caller: ShufflingCacheCaller): EpochShuffling { - const epoch = computeEpochAtSlot(slot); - return this.shufflingCache.getOrError(epoch, this.getShufflingDecisionRootAtEpoch(epoch), caller); + return this.getShufflingAtEpoch(computeEpochAtSlot(slot), caller); } getShufflingAtEpoch(epoch: Epoch, caller: ShufflingCacheCaller): EpochShuffling { - return this.shufflingCache.getOrError(epoch, this.getShufflingDecisionRootAtEpoch(epoch), caller); + const decisionRoot = this.getShufflingDecisionRootAtEpoch(epoch); + const shuffling = this.shufflingCache.getSync(epoch, decisionRoot, caller); + if (shuffling == null) { + throw new EpochCacheError({ + code: EpochCacheErrorCode.NO_SHUFFLING_AVAILABLE, + currentEpoch: this.epoch, + requestedEpoch: epoch, + decisionRoot, + }); + } + + return shuffling; } /** @@ -920,6 +930,7 @@ export enum EpochCacheErrorCode { NO_SYNC_COMMITTEE = "EPOCH_CONTEXT_ERROR_NO_SYNC_COMMITTEE", PROPOSER_EPOCH_MISMATCH = "EPOCH_CONTEXT_ERROR_PROPOSER_EPOCH_MISMATCH", NO_SHUFFLING_DECISION_ROOT = "EPOCH_CONTEXT_ERROR_NO_SHUFFLING_DECISION_ROOT", + NO_SHUFFLING_AVAILABLE = "EPOCH_CONTEXT_ERROR_NO_SHUFFLING_AVAILABLE", } type EpochCacheErrorType = @@ -946,6 +957,12 @@ type EpochCacheErrorType = code: EpochCacheErrorCode.NO_SHUFFLING_DECISION_ROOT; currentEpoch: Epoch; requestedEpoch: Epoch; + } + | { + code: EpochCacheErrorCode.NO_SHUFFLING_AVAILABLE; + currentEpoch: Epoch; + requestedEpoch: Epoch; + decisionRoot: string; }; export class EpochCacheError extends LodestarError {} diff --git a/packages/state-transition/src/cache/shufflingCache.ts b/packages/state-transition/src/cache/shufflingCache.ts index 3bd23d8f745e..01eeee173db2 100644 --- a/packages/state-transition/src/cache/shufflingCache.ts +++ b/packages/state-transition/src/cache/shufflingCache.ts @@ -1,5 +1,5 @@ import {Epoch, RootHex} from "@lodestar/types"; -import {GaugeExtra, LodestarError, MapDef, NoLabels, pruneSetToMax} from "@lodestar/utils"; +import {GaugeExtra, MapDef, NoLabels, pruneSetToMax} from "@lodestar/utils"; import {EpochShuffling, computeEpochShuffling} from "../util/index.js"; import {BeaconStateAllForks} from "./types.js"; @@ -41,19 +41,6 @@ export interface ShufflingCacheMetrics { }; } -export enum ShufflingCacheErrorCode { - NO_SHUFFLING_FOUND = "EPOCH_SHUFFLING_NO_SHUFFLING_FOUND", - REGEN_ERROR_NO_SHUFFLING_FOUND = "REGEN_ERROR_NO_SHUFFLING_FOUND", - SHUFFLING_PROMISE_NOT_RESOLVED = "EPOCH_SHUFFLING_SHUFFLING_PROMISE_NOT_RESOLVED", -} - -type ShufflingCacheErrorType = - | {code: ShufflingCacheErrorCode.NO_SHUFFLING_FOUND; epoch: Epoch; shufflingDecisionRoot: RootHex} - | {code: ShufflingCacheErrorCode.REGEN_ERROR_NO_SHUFFLING_FOUND; epoch: Epoch; shufflingDecisionRoot: RootHex} - | {code: ShufflingCacheErrorCode.SHUFFLING_PROMISE_NOT_RESOLVED; epoch: Epoch; shufflingDecisionRoot: RootHex}; - -export class ShufflingCacheError extends LodestarError {} - export enum ShufflingCacheItemType { shuffling, promise, @@ -79,7 +66,6 @@ export interface ShufflingCacheOptions { export interface IShufflingCache { addMetrics(metrics: ShufflingCacheMetrics | null): void; get(shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex): Promise; - getOrError(shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex, caller: ShufflingCacheCaller): EpochShuffling; getSync(shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex, caller: ShufflingCacheCaller): EpochShuffling | null; add(shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex, shuffling: EpochShuffling): void; buildSync( @@ -153,22 +139,12 @@ export class ShufflingCache implements IShufflingCache { } } - /** - * Will synchronously get a shuffling if it is available or will throw an error if not. Metrics are collected - * by this._get - */ - getOrError(shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex, caller: ShufflingCacheCaller): EpochShuffling { - // Will throw for error case so always returns a value - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - return this._get(shufflingEpoch, shufflingDecisionRoot, true, caller)!; - } - /** * Will synchronously get a shuffling if it is available or will return null if not. The consumer * will have to then submit for building the shuffling. Metrics are collected by this._get */ getSync(shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex, caller: ShufflingCacheCaller): EpochShuffling | null { - return this._get(shufflingEpoch, shufflingDecisionRoot, false, caller); + return this._get(shufflingEpoch, shufflingDecisionRoot, caller); } add(shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex, shuffling: EpochShuffling): void { @@ -232,30 +208,15 @@ export class ShufflingCache implements IShufflingCache { private _get( shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex, - shouldError: boolean, caller: ShufflingCacheCaller ): EpochShuffling | null { const cacheItem = this.itemsByDecisionRootByEpoch.getOrDefault(shufflingEpoch).get(shufflingDecisionRoot); if (cacheItem === undefined) { this.metrics?.shufflingCache.cacheMiss.inc({caller}); - if (shouldError) { - throw new ShufflingCacheError({ - code: ShufflingCacheErrorCode.NO_SHUFFLING_FOUND, - epoch: shufflingEpoch, - shufflingDecisionRoot, - }); - } return null; } if (this.isPromiseCacheItem(cacheItem)) { this.metrics?.shufflingCache.cacheMissUnresolvedPromise.inc({caller}); - if (shouldError) { - throw new ShufflingCacheError({ - code: ShufflingCacheErrorCode.SHUFFLING_PROMISE_NOT_RESOLVED, - epoch: shufflingEpoch, - shufflingDecisionRoot, - }); - } return null; } this.metrics?.shufflingCache.cacheHit.inc({caller}); From 481b3f917d06a58ac64dda4de8b1c0facf520c26 Mon Sep 17 00:00:00 2001 From: Cayman Date: Mon, 8 Jul 2024 15:21:33 -0400 Subject: [PATCH 17/23] chore: remove add from IShufflingCache --- packages/state-transition/src/cache/shufflingCache.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/state-transition/src/cache/shufflingCache.ts b/packages/state-transition/src/cache/shufflingCache.ts index 01eeee173db2..1b6ef539dcbe 100644 --- a/packages/state-transition/src/cache/shufflingCache.ts +++ b/packages/state-transition/src/cache/shufflingCache.ts @@ -67,7 +67,6 @@ export interface IShufflingCache { addMetrics(metrics: ShufflingCacheMetrics | null): void; get(shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex): Promise; getSync(shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex, caller: ShufflingCacheCaller): EpochShuffling | null; - add(shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex, shuffling: EpochShuffling): void; buildSync( state: BeaconStateAllForks, shufflingEpoch: Epoch, From 38c9b67f118b5b93e50b4e51354237db9bcd9088 Mon Sep 17 00:00:00 2001 From: Cayman Date: Mon, 8 Jul 2024 15:28:54 -0400 Subject: [PATCH 18/23] chore: use caller everywhere and small cleanup --- packages/beacon-node/src/chain/chain.ts | 7 ++- .../src/chain/validation/attestation.ts | 7 ++- .../src/cache/shufflingCache.ts | 52 +++++++++---------- .../test/unit/shufflingCache.test.ts | 12 ++++- 4 files changed, 46 insertions(+), 32 deletions(-) diff --git a/packages/beacon-node/src/chain/chain.ts b/packages/beacon-node/src/chain/chain.ts index fd75a1d60434..17f9ea60b431 100644 --- a/packages/beacon-node/src/chain/chain.ts +++ b/packages/beacon-node/src/chain/chain.ts @@ -14,6 +14,7 @@ import { EpochShuffling, ShufflingCache, IShufflingCache, + ShufflingCacheCaller, } from "@lodestar/state-transition"; import {BeaconConfig} from "@lodestar/config"; import { @@ -734,7 +735,11 @@ export class BeaconChain implements IBeaconChain { } // resolve the promise to unblock other calls of the same epoch and dependent root - const shuffling = await this.shufflingCache.get(attEpoch, shufflingDependentRoot); + const shuffling = await this.shufflingCache.get( + attEpoch, + shufflingDependentRoot, + ShufflingCacheCaller.attestationVerification + ); if (!shuffling) { // This will be essentially unreachable considering regen should build the shuffling for this epoch // but need to handle anyhow diff --git a/packages/beacon-node/src/chain/validation/attestation.ts b/packages/beacon-node/src/chain/validation/attestation.ts index fc39534b45e6..5ef5b281a9bd 100644 --- a/packages/beacon-node/src/chain/validation/attestation.ts +++ b/packages/beacon-node/src/chain/validation/attestation.ts @@ -11,6 +11,7 @@ import { EpochShuffling, computeStartSlotAtEpoch, computeSigningRoot, + ShufflingCacheCaller, } from "@lodestar/state-transition"; import {BeaconConfig} from "@lodestar/config"; import {AttestationError, AttestationErrorCode, GossipAction} from "../errors/index.js"; @@ -584,7 +585,11 @@ export async function getShufflingForAttestationVerification( const blockEpoch = computeEpochAtSlot(attHeadBlock.slot); const shufflingDependentRoot = getShufflingDependentRoot(chain.forkChoice, attEpoch, blockEpoch, attHeadBlock); - const shuffling = await chain.shufflingCache.get(attEpoch, shufflingDependentRoot); + const shuffling = await chain.shufflingCache.get( + attEpoch, + shufflingDependentRoot, + ShufflingCacheCaller.attestationVerification + ); if (shuffling) { // most of the time, we should get the shuffling from cache chain.metrics?.gossipAttestation.shufflingCacheHit.inc({caller: regenCaller}); diff --git a/packages/state-transition/src/cache/shufflingCache.ts b/packages/state-transition/src/cache/shufflingCache.ts index 1b6ef539dcbe..2bca9e28e686 100644 --- a/packages/state-transition/src/cache/shufflingCache.ts +++ b/packages/state-transition/src/cache/shufflingCache.ts @@ -65,7 +65,11 @@ export interface ShufflingCacheOptions { export interface IShufflingCache { addMetrics(metrics: ShufflingCacheMetrics | null): void; - get(shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex): Promise; + get( + shufflingEpoch: Epoch, + shufflingDecisionRoot: RootHex, + caller: ShufflingCacheCaller + ): Promise; getSync(shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex, caller: ShufflingCacheCaller): EpochShuffling | null; buildSync( state: BeaconStateAllForks, @@ -119,21 +123,21 @@ export class ShufflingCache implements IShufflingCache { * it to be calculated. Consumer await covers both cases. If shuffling is not available returns * null and does not attempt to compute shuffling. */ - async get(shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex): Promise { + async get( + shufflingEpoch: Epoch, + shufflingDecisionRoot: RootHex, + caller: ShufflingCacheCaller + ): Promise { const cacheItem = this.itemsByDecisionRootByEpoch.getOrDefault(shufflingEpoch).get(shufflingDecisionRoot); if (cacheItem === undefined) { - this.metrics?.shufflingCache.cacheMiss.inc({caller: ShufflingCacheCaller.attestationVerification}); + this.metrics?.shufflingCache.cacheMiss.inc({caller}); return null; } if (this.isShufflingCacheItem(cacheItem)) { - this.metrics?.shufflingCache.cacheHit.inc({ - caller: ShufflingCacheCaller.attestationVerification, - }); + this.metrics?.shufflingCache.cacheHit.inc({caller}); return cacheItem.shuffling; } else { - this.metrics?.shufflingCache.cacheHitUnresolvedPromise.inc({ - caller: ShufflingCacheCaller.attestationVerification, - }); + this.metrics?.shufflingCache.cacheHitUnresolvedPromise.inc({caller}); return cacheItem.promise; } } @@ -143,7 +147,17 @@ export class ShufflingCache implements IShufflingCache { * will have to then submit for building the shuffling. Metrics are collected by this._get */ getSync(shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex, caller: ShufflingCacheCaller): EpochShuffling | null { - return this._get(shufflingEpoch, shufflingDecisionRoot, caller); + const cacheItem = this.itemsByDecisionRootByEpoch.getOrDefault(shufflingEpoch).get(shufflingDecisionRoot); + if (cacheItem === undefined) { + this.metrics?.shufflingCache.cacheMiss.inc({caller}); + return null; + } + if (this.isPromiseCacheItem(cacheItem)) { + this.metrics?.shufflingCache.cacheMissUnresolvedPromise.inc({caller}); + return null; + } + this.metrics?.shufflingCache.cacheHit.inc({caller}); + return cacheItem.shuffling; } add(shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex, shuffling: EpochShuffling): void { @@ -204,24 +218,6 @@ export class ShufflingCache implements IShufflingCache { return shuffling; } - private _get( - shufflingEpoch: Epoch, - shufflingDecisionRoot: RootHex, - caller: ShufflingCacheCaller - ): EpochShuffling | null { - const cacheItem = this.itemsByDecisionRootByEpoch.getOrDefault(shufflingEpoch).get(shufflingDecisionRoot); - if (cacheItem === undefined) { - this.metrics?.shufflingCache.cacheMiss.inc({caller}); - return null; - } - if (this.isPromiseCacheItem(cacheItem)) { - this.metrics?.shufflingCache.cacheMissUnresolvedPromise.inc({caller}); - return null; - } - this.metrics?.shufflingCache.cacheHit.inc({caller}); - return cacheItem.shuffling; - } - private _buildSync( state: BeaconStateAllForks, shufflingEpoch: Epoch, diff --git a/packages/state-transition/test/unit/shufflingCache.test.ts b/packages/state-transition/test/unit/shufflingCache.test.ts index 04158d215fa3..97fab183b8ab 100644 --- a/packages/state-transition/test/unit/shufflingCache.test.ts +++ b/packages/state-transition/test/unit/shufflingCache.test.ts @@ -91,9 +91,17 @@ describe("ShufflingCache", function () { ); expect(countPromises(shufflingCache)).toEqual(1); - const shufflingRequest0 = shufflingCache.get(state.epochCtx.epoch, state.epochCtx.currentShufflingDecisionRoot); + const shufflingRequest0 = shufflingCache.get( + state.epochCtx.epoch, + state.epochCtx.currentShufflingDecisionRoot, + ShufflingCacheCaller.attestationVerification + ); expect(shufflingRequest0).toBeInstanceOf(Promise); - const shufflingRequest1 = shufflingCache.get(state.epochCtx.epoch, state.epochCtx.currentShufflingDecisionRoot); + const shufflingRequest1 = shufflingCache.get( + state.epochCtx.epoch, + state.epochCtx.currentShufflingDecisionRoot, + ShufflingCacheCaller.attestationVerification + ); expect(shufflingRequest1).toBeInstanceOf(Promise); const DELAY_WAIT_TIME = 2000; From aa29df278f460b5f46ee97e1664e4b834e100b99 Mon Sep 17 00:00:00 2001 From: Cayman Date: Mon, 8 Jul 2024 15:34:11 -0400 Subject: [PATCH 19/23] chore: small naming --- .../src/cache/shufflingCache.ts | 74 +++++++------------ 1 file changed, 28 insertions(+), 46 deletions(-) diff --git a/packages/state-transition/src/cache/shufflingCache.ts b/packages/state-transition/src/cache/shufflingCache.ts index 2bca9e28e686..7edcf5e73d59 100644 --- a/packages/state-transition/src/cache/shufflingCache.ts +++ b/packages/state-transition/src/cache/shufflingCache.ts @@ -65,22 +65,13 @@ export interface ShufflingCacheOptions { export interface IShufflingCache { addMetrics(metrics: ShufflingCacheMetrics | null): void; - get( - shufflingEpoch: Epoch, - shufflingDecisionRoot: RootHex, - caller: ShufflingCacheCaller - ): Promise; - getSync(shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex, caller: ShufflingCacheCaller): EpochShuffling | null; - buildSync( - state: BeaconStateAllForks, - shufflingEpoch: Epoch, - shufflingDecisionRoot: RootHex, - activeIndexes: number[] - ): EpochShuffling; + get(epoch: Epoch, decisionRoot: RootHex, caller: ShufflingCacheCaller): Promise; + getSync(epoch: Epoch, decisionRoot: RootHex, caller: ShufflingCacheCaller): EpochShuffling | null; + buildSync(state: BeaconStateAllForks, epoch: Epoch, decisionRoot: RootHex, activeIndexes: number[]): EpochShuffling; build( state: BeaconStateAllForks, - shufflingEpoch: Epoch, - shufflingDecisionRoot: RootHex, + epoch: Epoch, + decisionRoot: RootHex, activeIndexes: number[] ): Promise; } @@ -123,12 +114,8 @@ export class ShufflingCache implements IShufflingCache { * it to be calculated. Consumer await covers both cases. If shuffling is not available returns * null and does not attempt to compute shuffling. */ - async get( - shufflingEpoch: Epoch, - shufflingDecisionRoot: RootHex, - caller: ShufflingCacheCaller - ): Promise { - const cacheItem = this.itemsByDecisionRootByEpoch.getOrDefault(shufflingEpoch).get(shufflingDecisionRoot); + async get(epoch: Epoch, decisionRoot: RootHex, caller: ShufflingCacheCaller): Promise { + const cacheItem = this.itemsByDecisionRootByEpoch.getOrDefault(epoch).get(decisionRoot); if (cacheItem === undefined) { this.metrics?.shufflingCache.cacheMiss.inc({caller}); return null; @@ -146,8 +133,8 @@ export class ShufflingCache implements IShufflingCache { * Will synchronously get a shuffling if it is available or will return null if not. The consumer * will have to then submit for building the shuffling. Metrics are collected by this._get */ - getSync(shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex, caller: ShufflingCacheCaller): EpochShuffling | null { - const cacheItem = this.itemsByDecisionRootByEpoch.getOrDefault(shufflingEpoch).get(shufflingDecisionRoot); + getSync(epoch: Epoch, decisionRoot: RootHex, caller: ShufflingCacheCaller): EpochShuffling | null { + const cacheItem = this.itemsByDecisionRootByEpoch.getOrDefault(epoch).get(decisionRoot); if (cacheItem === undefined) { this.metrics?.shufflingCache.cacheMiss.inc({caller}); return null; @@ -160,20 +147,15 @@ export class ShufflingCache implements IShufflingCache { return cacheItem.shuffling; } - add(shufflingEpoch: Epoch, shufflingDecisionRoot: RootHex, shuffling: EpochShuffling): void { + add(epoch: Epoch, decisionRoot: RootHex, shuffling: EpochShuffling): void { this.itemsByDecisionRootByEpoch - .getOrDefault(shufflingEpoch) - .set(shufflingDecisionRoot, {type: ShufflingCacheItemType.shuffling, shuffling}); + .getOrDefault(epoch) + .set(decisionRoot, {type: ShufflingCacheItemType.shuffling, shuffling}); pruneSetToMax(this.itemsByDecisionRootByEpoch, this.maxEpochs); } - buildSync( - state: BeaconStateAllForks, - shufflingEpoch: Epoch, - shufflingDecisionRoot: RootHex, - activeIndexes: number[] - ): EpochShuffling { - const cacheItem = this.itemsByDecisionRootByEpoch.getOrDefault(shufflingEpoch).get(shufflingDecisionRoot); + buildSync(state: BeaconStateAllForks, epoch: Epoch, decisionRoot: RootHex, activeIndexes: number[]): EpochShuffling { + const cacheItem = this.itemsByDecisionRootByEpoch.getOrDefault(epoch).get(decisionRoot); if (cacheItem) { if (this.isShufflingCacheItem(cacheItem)) { this.metrics?.shufflingCache.cacheHit.inc({caller: ShufflingCacheCaller.synchronousBuildShuffling}); @@ -189,18 +171,18 @@ export class ShufflingCache implements IShufflingCache { this.metrics?.shufflingCache.cacheMiss.inc({caller: ShufflingCacheCaller.synchronousBuildShuffling}); } - const shuffling = this._buildSync(state, shufflingEpoch, shufflingDecisionRoot, activeIndexes); + const shuffling = this._buildSync(state, epoch, decisionRoot, activeIndexes); cacheItem?.resolveFn(shuffling); return shuffling; } async build( state: BeaconStateAllForks, - shufflingEpoch: Epoch, - shufflingDecisionRoot: RootHex, + epoch: Epoch, + decisionRoot: RootHex, activeIndexes: number[] ): Promise { - const cacheItem = this.itemsByDecisionRootByEpoch.getOrDefault(shufflingEpoch).get(shufflingDecisionRoot); + const cacheItem = this.itemsByDecisionRootByEpoch.getOrDefault(epoch).get(decisionRoot); if (cacheItem) { if (this.isShufflingCacheItem(cacheItem)) { this.metrics?.shufflingCache.cacheHit.inc({caller: ShufflingCacheCaller.buildShuffling}); @@ -212,35 +194,35 @@ export class ShufflingCache implements IShufflingCache { // this is to prevent multiple calls to get shuffling for the same epoch and dependent root // any subsequent calls of the same epoch and dependent root will wait for this promise to resolve - const item = this._insertShufflingPromise(shufflingEpoch, shufflingDecisionRoot); - const shuffling = await this._build(state, shufflingEpoch, shufflingDecisionRoot, activeIndexes); + const item = this._insertShufflingPromise(epoch, decisionRoot); + const shuffling = await this._build(state, epoch, decisionRoot, activeIndexes); item.resolveFn(shuffling); return shuffling; } private _buildSync( state: BeaconStateAllForks, - shufflingEpoch: Epoch, - shufflingDecisionRoot: RootHex, + epoch: Epoch, + decisionRoot: RootHex, activeIndexes: number[] ): EpochShuffling { - const shuffling = computeEpochShuffling(state, activeIndexes, shufflingEpoch); - this.add(shufflingEpoch, shufflingDecisionRoot, shuffling); + const shuffling = computeEpochShuffling(state, activeIndexes, epoch); + this.add(epoch, decisionRoot, shuffling); return shuffling; } private async _build( state: BeaconStateAllForks, - shufflingEpoch: Epoch, - shufflingDecisionRoot: RootHex, + epoch: Epoch, + decisionRoot: RootHex, activeIndexes: number[] ): Promise { // TODO: (matthewkeil) replace this sync call with a worker and async build function that uses // a nice'd thread to build in core idle time // // Building will overwrite the ShufflingCachePromiseItem with the ShufflingCacheShufflingItem - const shuffling = computeEpochShuffling(state, activeIndexes, shufflingEpoch); - this.add(shufflingEpoch, shufflingDecisionRoot, shuffling); + const shuffling = computeEpochShuffling(state, activeIndexes, epoch); + this.add(epoch, decisionRoot, shuffling); return shuffling; } From dd6855105fdc3154999d7aac1127fad1c6075544 Mon Sep 17 00:00:00 2001 From: Cayman Date: Mon, 8 Jul 2024 15:35:41 -0400 Subject: [PATCH 20/23] chore: re-add ShufflingCacheCaller.testing --- .../src/cache/shufflingCache.ts | 1 + .../test/unit/shufflingCache.test.ts | 24 +++++-------------- 2 files changed, 7 insertions(+), 18 deletions(-) diff --git a/packages/state-transition/src/cache/shufflingCache.ts b/packages/state-transition/src/cache/shufflingCache.ts index 7edcf5e73d59..2c2b827b2a13 100644 --- a/packages/state-transition/src/cache/shufflingCache.ts +++ b/packages/state-transition/src/cache/shufflingCache.ts @@ -18,6 +18,7 @@ export const SHUFFLING_CACHE_MAX_PROMISES = 2; export const SHUFFLING_CACHE_MAX_EPOCHS = 4; export enum ShufflingCacheCaller { + testing = "testing", buildShuffling = "buildShuffling", synchronousBuildShuffling = "synchronousBuildShuffling", attestationVerification = "attestationVerification", diff --git a/packages/state-transition/test/unit/shufflingCache.test.ts b/packages/state-transition/test/unit/shufflingCache.test.ts index 97fab183b8ab..7bb50adae928 100644 --- a/packages/state-transition/test/unit/shufflingCache.test.ts +++ b/packages/state-transition/test/unit/shufflingCache.test.ts @@ -34,11 +34,7 @@ describe("ShufflingCache", function () { state.epochCtx.currentActiveIndices ); expect( - shufflingCache.getSync( - currentEpoch, - state.epochCtx.currentShufflingDecisionRoot, - ShufflingCacheCaller.createFromState - ) + shufflingCache.getSync(currentEpoch, state.epochCtx.currentShufflingDecisionRoot, ShufflingCacheCaller.testing) ).toEqual(shuffling); }); @@ -51,11 +47,7 @@ describe("ShufflingCache", function () { state.epochCtx.currentActiveIndices ); expect( - shufflingCache.getSync( - currentEpoch, - state.epochCtx.currentShufflingDecisionRoot, - ShufflingCacheCaller.createFromState - ) + shufflingCache.getSync(currentEpoch, state.epochCtx.currentShufflingDecisionRoot, ShufflingCacheCaller.testing) ).toEqual(currentShuffling); const nextShuffling = shufflingCache.buildSync( @@ -69,16 +61,12 @@ describe("ShufflingCache", function () { shufflingCache.getSync( state.epochCtx.nextEpoch, state.epochCtx.nextShufflingDecisionRoot, - ShufflingCacheCaller.createFromState + ShufflingCacheCaller.testing ) ).toEqual(nextShuffling); // the current shuffling is not available anymore expect( - shufflingCache.getSync( - currentEpoch, - state.epochCtx.currentShufflingDecisionRoot, - ShufflingCacheCaller.createFromState - ) + shufflingCache.getSync(currentEpoch, state.epochCtx.currentShufflingDecisionRoot, ShufflingCacheCaller.testing) ).toBeNull(); }); @@ -94,13 +82,13 @@ describe("ShufflingCache", function () { const shufflingRequest0 = shufflingCache.get( state.epochCtx.epoch, state.epochCtx.currentShufflingDecisionRoot, - ShufflingCacheCaller.attestationVerification + ShufflingCacheCaller.testing ); expect(shufflingRequest0).toBeInstanceOf(Promise); const shufflingRequest1 = shufflingCache.get( state.epochCtx.epoch, state.epochCtx.currentShufflingDecisionRoot, - ShufflingCacheCaller.attestationVerification + ShufflingCacheCaller.testing ); expect(shufflingRequest1).toBeInstanceOf(Promise); From ec8bd026596bbe675aa49b107fd4b47d7791af8c Mon Sep 17 00:00:00 2001 From: Cayman Date: Mon, 8 Jul 2024 15:41:34 -0400 Subject: [PATCH 21/23] chore: add minor comment --- packages/state-transition/src/cache/shufflingCache.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/state-transition/src/cache/shufflingCache.ts b/packages/state-transition/src/cache/shufflingCache.ts index 2c2b827b2a13..11de944235be 100644 --- a/packages/state-transition/src/cache/shufflingCache.ts +++ b/packages/state-transition/src/cache/shufflingCache.ts @@ -19,16 +19,18 @@ export const SHUFFLING_CACHE_MAX_EPOCHS = 4; export enum ShufflingCacheCaller { testing = "testing", + // used here or epoch cache buildShuffling = "buildShuffling", synchronousBuildShuffling = "synchronousBuildShuffling", - attestationVerification = "attestationVerification", createFromState = "createFromState", reloadCreateFromState = "reloadCreateFromState", getBeaconCommittee = "getBeaconCommittee", - getEpochCommittees = "getEpochCommittees", - getAttestationsForBlock = "getAttestationsForBlock", getCommitteeCountPerSlot = "getCommitteeCountPerSlot", getCommitteeAssignments = "getCommitteeAssignments", + // used in beacon node + attestationVerification = "attestationVerification", + getEpochCommittees = "getEpochCommittees", + getAttestationsForBlock = "getAttestationsForBlock", } export interface ShufflingCacheMetrics { From 6fb944ffdd117141fbc03236967c62c35a33dd2d Mon Sep 17 00:00:00 2001 From: Cayman Date: Mon, 8 Jul 2024 15:58:51 -0400 Subject: [PATCH 22/23] chore: more refactoring --- .../src/cache/shufflingCache.ts | 68 +++++-------------- 1 file changed, 17 insertions(+), 51 deletions(-) diff --git a/packages/state-transition/src/cache/shufflingCache.ts b/packages/state-transition/src/cache/shufflingCache.ts index 11de944235be..2c3e28eff204 100644 --- a/packages/state-transition/src/cache/shufflingCache.ts +++ b/packages/state-transition/src/cache/shufflingCache.ts @@ -151,31 +151,23 @@ export class ShufflingCache implements IShufflingCache { } add(epoch: Epoch, decisionRoot: RootHex, shuffling: EpochShuffling): void { - this.itemsByDecisionRootByEpoch - .getOrDefault(epoch) - .set(decisionRoot, {type: ShufflingCacheItemType.shuffling, shuffling}); + const items = this.itemsByDecisionRootByEpoch.getOrDefault(epoch); + const item = items.get(decisionRoot); + if (item !== undefined && this.isPromiseCacheItem(item)) { + item.resolveFn(shuffling); + } + items.set(decisionRoot, {type: ShufflingCacheItemType.shuffling, shuffling}); pruneSetToMax(this.itemsByDecisionRootByEpoch, this.maxEpochs); } buildSync(state: BeaconStateAllForks, epoch: Epoch, decisionRoot: RootHex, activeIndexes: number[]): EpochShuffling { - const cacheItem = this.itemsByDecisionRootByEpoch.getOrDefault(epoch).get(decisionRoot); - if (cacheItem) { - if (this.isShufflingCacheItem(cacheItem)) { - this.metrics?.shufflingCache.cacheHit.inc({caller: ShufflingCacheCaller.synchronousBuildShuffling}); - return cacheItem.shuffling; - } - // Add metric here for race condition recreating the shuffling because - // this sync call will finish first if its running on thread - // - // TODO: (matthewkeil) Perhaps we should throw an error instead. Should not happen ideally - this.metrics?.shufflingCache.cacheHitRebuildPromise.inc(); - // add log statement here with epoch and decision root for this miss - } else { - this.metrics?.shufflingCache.cacheMiss.inc({caller: ShufflingCacheCaller.synchronousBuildShuffling}); + const cachedShuffling = this.getSync(epoch, decisionRoot, ShufflingCacheCaller.synchronousBuildShuffling); + if (cachedShuffling !== null) { + return cachedShuffling; } - const shuffling = this._buildSync(state, epoch, decisionRoot, activeIndexes); - cacheItem?.resolveFn(shuffling); + const shuffling = computeEpochShuffling(state, activeIndexes, epoch); + this.add(epoch, decisionRoot, shuffling); return shuffling; } @@ -185,41 +177,15 @@ export class ShufflingCache implements IShufflingCache { decisionRoot: RootHex, activeIndexes: number[] ): Promise { - const cacheItem = this.itemsByDecisionRootByEpoch.getOrDefault(epoch).get(decisionRoot); - if (cacheItem) { - if (this.isShufflingCacheItem(cacheItem)) { - this.metrics?.shufflingCache.cacheHit.inc({caller: ShufflingCacheCaller.buildShuffling}); - return cacheItem.shuffling; - } - this.metrics?.shufflingCache.cacheHitUnresolvedPromise.inc({caller: ShufflingCacheCaller.buildShuffling}); - return cacheItem.promise; + // will await any pending build already in progress + const cachedShuffling = await this.get(epoch, decisionRoot, ShufflingCacheCaller.buildShuffling); + if (cachedShuffling !== null) { + return cachedShuffling; } - // this is to prevent multiple calls to get shuffling for the same epoch and dependent root + // this is to prevent multiple builds for the same epoch and dependent root // any subsequent calls of the same epoch and dependent root will wait for this promise to resolve - const item = this._insertShufflingPromise(epoch, decisionRoot); - const shuffling = await this._build(state, epoch, decisionRoot, activeIndexes); - item.resolveFn(shuffling); - return shuffling; - } - - private _buildSync( - state: BeaconStateAllForks, - epoch: Epoch, - decisionRoot: RootHex, - activeIndexes: number[] - ): EpochShuffling { - const shuffling = computeEpochShuffling(state, activeIndexes, epoch); - this.add(epoch, decisionRoot, shuffling); - return shuffling; - } - - private async _build( - state: BeaconStateAllForks, - epoch: Epoch, - decisionRoot: RootHex, - activeIndexes: number[] - ): Promise { + this._insertShufflingPromise(epoch, decisionRoot); // TODO: (matthewkeil) replace this sync call with a worker and async build function that uses // a nice'd thread to build in core idle time // From d6338fe9fc67af09d3b946c345e4e88aa1550d02 Mon Sep 17 00:00:00 2001 From: Cayman Date: Mon, 8 Jul 2024 16:06:55 -0400 Subject: [PATCH 23/23] chore: more comments --- .../src/cache/shufflingCache.ts | 36 +++++++++++++------ 1 file changed, 25 insertions(+), 11 deletions(-) diff --git a/packages/state-transition/src/cache/shufflingCache.ts b/packages/state-transition/src/cache/shufflingCache.ts index 2c3e28eff204..2aad44e54b24 100644 --- a/packages/state-transition/src/cache/shufflingCache.ts +++ b/packages/state-transition/src/cache/shufflingCache.ts @@ -68,9 +68,33 @@ export interface ShufflingCacheOptions { export interface IShufflingCache { addMetrics(metrics: ShufflingCacheMetrics | null): void; - get(epoch: Epoch, decisionRoot: RootHex, caller: ShufflingCacheCaller): Promise; + + /** + * Will synchronously get a shuffling if it is available or will return null if not. + */ getSync(epoch: Epoch, decisionRoot: RootHex, caller: ShufflingCacheCaller): EpochShuffling | null; + + /** + * Will immediately return a shuffling if it is available, or a promise to an in-progress shuffling calculation. + * + * If shuffling is not available returns null and does not attempt to compute shuffling. + */ + get(epoch: Epoch, decisionRoot: RootHex, caller: ShufflingCacheCaller): Promise; + + /** + * Will synchronously get a shuffling if it is available. + * + * If a shuffling is not immediately available, a shuffling will be calculated. + * + * NOTE: this may recalculate an already in-progress shuffling. + */ buildSync(state: BeaconStateAllForks, epoch: Epoch, decisionRoot: RootHex, activeIndexes: number[]): EpochShuffling; + + /** + * Will immediately return a shuffling if it is available, or a promise to an in-progress shuffling calculation. + * + * If neither is available, a shuffling will be calculated. + */ build( state: BeaconStateAllForks, epoch: Epoch, @@ -111,12 +135,6 @@ export class ShufflingCache implements IShufflingCache { } } - /** - * Used for attestation verifications. Will immediately return a shuffling if it is available, - * otherwise it will return the promise for the shuffling and the consumer will need to wait for - * it to be calculated. Consumer await covers both cases. If shuffling is not available returns - * null and does not attempt to compute shuffling. - */ async get(epoch: Epoch, decisionRoot: RootHex, caller: ShufflingCacheCaller): Promise { const cacheItem = this.itemsByDecisionRootByEpoch.getOrDefault(epoch).get(decisionRoot); if (cacheItem === undefined) { @@ -132,10 +150,6 @@ export class ShufflingCache implements IShufflingCache { } } - /** - * Will synchronously get a shuffling if it is available or will return null if not. The consumer - * will have to then submit for building the shuffling. Metrics are collected by this._get - */ getSync(epoch: Epoch, decisionRoot: RootHex, caller: ShufflingCacheCaller): EpochShuffling | null { const cacheItem = this.itemsByDecisionRootByEpoch.getOrDefault(epoch).get(decisionRoot); if (cacheItem === undefined) {