From 309c973cbc5acfcae7d2b86a34512068bf67182a Mon Sep 17 00:00:00 2001 From: dapplion <35266934+dapplion@users.noreply.github.com> Date: Mon, 11 Jul 2022 12:19:16 +0200 Subject: [PATCH 1/2] Clone states only when necessary --- packages/beacon-node/src/chain/blocks/importBlock.ts | 2 +- packages/beacon-node/src/chain/regen/regen.ts | 5 +++-- .../beacon-node/src/chain/stateCache/stateContextCache.ts | 4 ++-- .../src/chain/stateCache/stateContextCheckpointsCache.ts | 4 ++-- packages/state-transition/src/stateTransition.ts | 2 ++ 5 files changed, 10 insertions(+), 7 deletions(-) diff --git a/packages/beacon-node/src/chain/blocks/importBlock.ts b/packages/beacon-node/src/chain/blocks/importBlock.ts index b8ad62ef761..eb70ce24bb9 100644 --- a/packages/beacon-node/src/chain/blocks/importBlock.ts +++ b/packages/beacon-node/src/chain/blocks/importBlock.ts @@ -185,7 +185,7 @@ export async function importBlock(chain: ImportBlockModules, fullyVerifiedBlock: // - Write block and state to snapshot_cache if (block.message.slot % SLOTS_PER_EPOCH === 0) { // Cache state to preserve epoch transition work - const checkpointState = postState.clone(); + const checkpointState = postState; const cp = getCheckpointFromState(checkpointState); chain.checkpointStateCache.add(cp, checkpointState); pendingEvents.push(ChainEvent.checkpoint, cp, checkpointState); diff --git a/packages/beacon-node/src/chain/regen/regen.ts b/packages/beacon-node/src/chain/regen/regen.ts index 0241a6ef432..534d9abd8fb 100644 --- a/packages/beacon-node/src/chain/regen/regen.ts +++ b/packages/beacon-node/src/chain/regen/regen.ts @@ -242,7 +242,7 @@ async function processSlotsToNearestCheckpoint( const preSlot = preState.slot; const postSlot = slot; const preEpoch = computeEpochAtSlot(preSlot); - let postState = preState.clone(); + let postState = preState; const {checkpointStateCache, emitter, metrics} = modules; for ( @@ -250,10 +250,11 @@ async function processSlotsToNearestCheckpoint( nextEpochSlot <= postSlot; nextEpochSlot += SLOTS_PER_EPOCH ) { + // processSlots calls .clone() before mutating postState = processSlots(postState, nextEpochSlot, metrics); // Cache state to preserve epoch transition work - const checkpointState = postState.clone(); + const checkpointState = postState; const cp = getCheckpointFromState(checkpointState); checkpointStateCache.add(cp, checkpointState); emitter.emit(ChainEvent.checkpoint, cp, checkpointState); diff --git a/packages/beacon-node/src/chain/stateCache/stateContextCache.ts b/packages/beacon-node/src/chain/stateCache/stateContextCache.ts index 65aec427048..d73caf6b89f 100644 --- a/packages/beacon-node/src/chain/stateCache/stateContextCache.ts +++ b/packages/beacon-node/src/chain/stateCache/stateContextCache.ts @@ -43,7 +43,7 @@ export class StateContextCache { // clonedCount + 1 as there's a .clone() below this.metrics?.stateClonedCount.observe(item.clonedCount + 1); - return item.clone(); + return item; } add(item: CachedBeaconStateAllForks): void { @@ -52,7 +52,7 @@ export class StateContextCache { return; } this.metrics?.adds.inc(); - this.cache.set(key, item.clone()); + this.cache.set(key, item); const epoch = item.epochCtx.currentShuffling.epoch; const blockRoots = this.epochIndex.get(epoch); if (blockRoots) { diff --git a/packages/beacon-node/src/chain/stateCache/stateContextCheckpointsCache.ts b/packages/beacon-node/src/chain/stateCache/stateContextCheckpointsCache.ts index e42a856b741..31b830213ce 100644 --- a/packages/beacon-node/src/chain/stateCache/stateContextCheckpointsCache.ts +++ b/packages/beacon-node/src/chain/stateCache/stateContextCheckpointsCache.ts @@ -49,7 +49,7 @@ export class CheckpointStateCache { this.preComputedCheckpointHits = (this.preComputedCheckpointHits ?? 0) + 1; } - return item.clone(); + return item; } add(cp: phase0.Checkpoint, item: CachedBeaconStateAllForks): void { @@ -59,7 +59,7 @@ export class CheckpointStateCache { return; } this.metrics?.adds.inc(); - this.cache.set(key, item.clone()); + this.cache.set(key, item); this.epochIndex.getOrDefault(cp.epoch).add(cpHex.rootHex); } diff --git a/packages/state-transition/src/stateTransition.ts b/packages/state-transition/src/stateTransition.ts index f91502d20f4..5c897be243e 100644 --- a/packages/state-transition/src/stateTransition.ts +++ b/packages/state-transition/src/stateTransition.ts @@ -27,6 +27,7 @@ export function stateTransition( const block = signedBlock.message; const blockSlot = block.slot; + // .clone() before mutating state in state transition let postState = state.clone(); // State is already a ViewDU, which won't commit changes. Equivalent to .setStateCachesAsTransient() @@ -81,6 +82,7 @@ export function processSlots( slot: Slot, metrics?: IBeaconStateTransitionMetrics | null ): CachedBeaconStateAllForks { + // .clone() before mutating state in state transition let postState = state.clone(); // State is already a ViewDU, which won't commit changes. Equivalent to .setStateCachesAsTransient() From 5347c3c36b4520413eaae6fb3d4d6b05acca788e Mon Sep 17 00:00:00 2001 From: dapplion <35266934+dapplion@users.noreply.github.com> Date: Mon, 11 Jul 2022 22:39:51 +0200 Subject: [PATCH 2/2] Use validators.getReadonly when not need mutable reference --- packages/beacon-node/src/api/impl/beacon/state/index.ts | 4 ++-- packages/beacon-node/src/chain/opPools/opPool.ts | 8 ++++---- .../state-transition/src/block/initiateValidatorExit.ts | 8 ++++++-- .../state-transition/src/block/processAttesterSlashing.ts | 2 +- packages/state-transition/src/block/processBlockHeader.ts | 2 +- .../state-transition/src/block/processProposerSlashing.ts | 2 +- .../src/epoch/processSyncCommitteeUpdates.ts | 4 +++- packages/state-transition/src/util/syncCommittee.ts | 2 +- packages/state-transition/test/perf/analyzeEpochs.ts | 2 +- 9 files changed, 20 insertions(+), 14 deletions(-) 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 b50b9487099..6632bfcf8db 100644 --- a/packages/beacon-node/src/api/impl/beacon/state/index.ts +++ b/packages/beacon-node/src/api/impl/beacon/state/index.ts @@ -57,7 +57,7 @@ export function getBeaconStateApi({ for (const id of filters.id) { const validatorIndex = getStateValidatorIndex(id, state, pubkey2index); if (validatorIndex != null) { - const validator = validators.get(validatorIndex); + const validator = validators.getReadonly(validatorIndex); if (filters.statuses && !filters.statuses.includes(getValidatorStatus(validator, currentEpoch))) { continue; } @@ -99,7 +99,7 @@ export function getBeaconStateApi({ return { data: toValidatorResponse( validatorIndex, - state.validators.get(validatorIndex), + state.validators.getReadonly(validatorIndex), state.balances.get(validatorIndex), getCurrentEpoch(state) ), diff --git a/packages/beacon-node/src/chain/opPools/opPool.ts b/packages/beacon-node/src/chain/opPools/opPool.ts index 26f69c89af2..32e406cde2f 100644 --- a/packages/beacon-node/src/chain/opPools/opPool.ts +++ b/packages/beacon-node/src/chain/opPools/opPool.ts @@ -139,7 +139,7 @@ export class OpPool { for (const proposerSlashing of this.proposerSlashings.values()) { const index = proposerSlashing.signedHeader1.message.proposerIndex; - const validator = state.validators.get(index); + const validator = state.validators.getReadonly(index); if (!validator.slashed && validator.activationEpoch <= stateEpoch && stateEpoch < validator.withdrawableEpoch) { proposerSlashings.push(proposerSlashing); // Set of validators to be slashed, so we don't attempt to construct invalid attester slashings. @@ -156,7 +156,7 @@ export class OpPool { const slashableIndices = new Set(); for (let i = 0; i < attesterSlashing.intersectingIndices.length; i++) { const index = attesterSlashing.intersectingIndices[i]; - const validator = state.validators.get(index); + const validator = state.validators.getReadonly(index); // If we already have a slashing for this index, we can continue on to the next slashing if (toBeSlashedIndices.has(index)) { @@ -232,7 +232,7 @@ export class OpPool { // // We cannot check the `slashed` field since the `head` is not finalized and // a fork could un-slash someone. - if (headState.validators.get(index).exitEpoch > finalizedEpoch) { + if (headState.validators.getReadonly(index).exitEpoch > finalizedEpoch) { continue attesterSlashing; } } @@ -249,7 +249,7 @@ export class OpPool { const finalizedEpoch = headState.finalizedCheckpoint.epoch; for (const [key, proposerSlashing] of this.proposerSlashings.entries()) { const index = proposerSlashing.signedHeader1.message.proposerIndex; - if (headState.validators.get(index).exitEpoch <= finalizedEpoch) { + if (headState.validators.getReadonly(index).exitEpoch <= finalizedEpoch) { this.proposerSlashings.delete(key); } } diff --git a/packages/state-transition/src/block/initiateValidatorExit.ts b/packages/state-transition/src/block/initiateValidatorExit.ts index 26739d9b83c..f202ae5c136 100644 --- a/packages/state-transition/src/block/initiateValidatorExit.ts +++ b/packages/state-transition/src/block/initiateValidatorExit.ts @@ -1,5 +1,6 @@ import {FAR_FUTURE_EPOCH} from "@lodestar/params"; -import {phase0} from "@lodestar/types"; +import {CompositeViewDU} from "@chainsafe/ssz"; +import {ssz} from "@lodestar/types"; import {CachedBeaconStateAllForks} from "../types.js"; /** @@ -22,7 +23,10 @@ import {CachedBeaconStateAllForks} from "../types.js"; * ``` * Forcing consumers to pass the SubTree of `validator` directly mitigates this issue. */ -export function initiateValidatorExit(state: CachedBeaconStateAllForks, validator: phase0.Validator): void { +export function initiateValidatorExit( + state: CachedBeaconStateAllForks, + validator: CompositeViewDU +): void { const {config, epochCtx} = state; // return if validator already initiated exit diff --git a/packages/state-transition/src/block/processAttesterSlashing.ts b/packages/state-transition/src/block/processAttesterSlashing.ts index a58fd1c6650..7b1678aee36 100644 --- a/packages/state-transition/src/block/processAttesterSlashing.ts +++ b/packages/state-transition/src/block/processAttesterSlashing.ts @@ -26,7 +26,7 @@ export function processAttesterSlashing( const validators = state.validators; // Get the validators sub tree once for all indices // Spec requires to sort indexes beforehand for (const index of intersectingIndices.sort((a, b) => a - b)) { - if (isSlashableValidator(validators.get(index), state.epochCtx.currentShuffling.epoch)) { + if (isSlashableValidator(validators.getReadonly(index), state.epochCtx.currentShuffling.epoch)) { slashValidator(fork, state, index); slashedAny = true; } diff --git a/packages/state-transition/src/block/processBlockHeader.ts b/packages/state-transition/src/block/processBlockHeader.ts index 589a28b459c..4eac4e70a89 100644 --- a/packages/state-transition/src/block/processBlockHeader.ts +++ b/packages/state-transition/src/block/processBlockHeader.ts @@ -52,7 +52,7 @@ export function processBlockHeader(state: CachedBeaconStateAllForks, block: allF }); // verify proposer is not slashed. Only once per block, may use the slower read from tree - if (state.validators.get(proposerIndex).slashed) { + if (state.validators.getReadonly(proposerIndex).slashed) { throw new Error("Block proposer is slashed"); } } diff --git a/packages/state-transition/src/block/processProposerSlashing.ts b/packages/state-transition/src/block/processProposerSlashing.ts index 991612959df..70bd37092f9 100644 --- a/packages/state-transition/src/block/processProposerSlashing.ts +++ b/packages/state-transition/src/block/processProposerSlashing.ts @@ -49,7 +49,7 @@ export function assertValidProposerSlashing( } // verify the proposer is slashable - const proposer = state.validators.get(header1.proposerIndex); + const proposer = state.validators.getReadonly(header1.proposerIndex); if (!isSlashableValidator(proposer, state.epochCtx.epoch)) { throw new Error("ProposerSlashing proposer is not slashable"); } diff --git a/packages/state-transition/src/epoch/processSyncCommitteeUpdates.ts b/packages/state-transition/src/epoch/processSyncCommitteeUpdates.ts index 81c4a28cefa..dc1f3927439 100644 --- a/packages/state-transition/src/epoch/processSyncCommitteeUpdates.ts +++ b/packages/state-transition/src/epoch/processSyncCommitteeUpdates.ts @@ -24,7 +24,9 @@ export function processSyncCommitteeUpdates(state: CachedBeaconStateAltair): voi ); // Using the index2pubkey cache is slower because it needs the serialized pubkey. - const nextSyncCommitteePubkeys = nextSyncCommitteeIndices.map((index) => state.validators.get(index).pubkey); + const nextSyncCommitteePubkeys = nextSyncCommitteeIndices.map( + (index) => state.validators.getReadonly(index).pubkey + ); // Rotate syncCommittee in state state.currentSyncCommittee = state.nextSyncCommittee; diff --git a/packages/state-transition/src/util/syncCommittee.ts b/packages/state-transition/src/util/syncCommittee.ts index ccc2365b0fb..2dcc46ae8ae 100644 --- a/packages/state-transition/src/util/syncCommittee.ts +++ b/packages/state-transition/src/util/syncCommittee.ts @@ -26,7 +26,7 @@ export function getNextSyncCommittee( const indices = getNextSyncCommitteeIndices(state, activeValidatorIndices, effectiveBalanceIncrements); // Using the index2pubkey cache is slower because it needs the serialized pubkey. - const pubkeys = indices.map((index) => state.validators.get(index).pubkey); + const pubkeys = indices.map((index) => state.validators.getReadonly(index).pubkey); return { indices, diff --git a/packages/state-transition/test/perf/analyzeEpochs.ts b/packages/state-transition/test/perf/analyzeEpochs.ts index 7df4df3b4b7..98f3c999bf3 100644 --- a/packages/state-transition/test/perf/analyzeEpochs.ts +++ b/packages/state-transition/test/perf/analyzeEpochs.ts @@ -104,7 +104,7 @@ async function analyzeEpochs(network: NetworkName, fromEpoch?: number): Promise< const validatorKeys = Object.keys(validatorChangesCountZero) as (keyof typeof validatorChangesCountZero)[]; for (let i = 0; i < validatorCount; i++) { const validatorPrev = state.validators[i]; - const validatorNext = postState.validators.get(i); + const validatorNext = postState.validators.getReadonly(i); for (const key of validatorKeys) { const valuePrev = validatorPrev[key]; const valueNext = validatorNext[key];