fix: avoid BeaconState commit() clone() in beacon-node

packages/beacon-node/src/api/impl/beacon/state/utils.ts

@@ -2,7 +2,7 @@ import {PubkeyIndexMap} from "@chainsafe/pubkey-index-map";
 import {routes} from "@lodestar/api";
 import {CheckpointWithHex, IForkChoice} from "@lodestar/fork-choice";
 import {GENESIS_SLOT} from "@lodestar/params";
-import {BeaconStateAllForks} from "@lodestar/state-transition";
+import {BeaconStateAllForks, CachedBeaconStateAllForks} from "@lodestar/state-transition";
 import {BLSPubkey, Epoch, RootHex, Slot, ValidatorIndex, getValidatorStatus, phase0} from "@lodestar/types";
 import {fromHex} from "@lodestar/utils";
 import {IBeaconChain} from "../../../../chain/index.js";
@@ -41,30 +41,10 @@ export function resolveStateId(
   return blockSlot;
 }
 
-export async function getStateResponse(
-  chain: IBeaconChain,
-  inStateId: routes.beacon.StateId
-): Promise<{state: BeaconStateAllForks; executionOptimistic: boolean; finalized: boolean}> {
-  const stateId = resolveStateId(chain.forkChoice, inStateId);
-
-  const res =
-    typeof stateId === "string"
-      ? await chain.getStateByStateRoot(stateId)
-      : typeof stateId === "number"
-        ? await chain.getStateBySlot(stateId)
-        : chain.getStateByCheckpoint(stateId);
-
-  if (!res) {
-    throw new ApiError(404, `State not found for id '${inStateId}'`);
-  }
-
-  return res;
-}
-
 export async function getStateResponseWithRegen(
   chain: IBeaconChain,
   inStateId: routes.beacon.StateId
-): Promise<{state: BeaconStateAllForks | Uint8Array; executionOptimistic: boolean; finalized: boolean}> {
+): Promise<{state: CachedBeaconStateAllForks | Uint8Array; executionOptimistic: boolean; finalized: boolean}> {
   const stateId = resolveStateId(chain.forkChoice, inStateId);
 
   const res =

packages/beacon-node/src/api/impl/lodestar/index.ts

@@ -198,7 +198,7 @@ export function getLodestarApi({
       const {state, executionOptimistic, finalized} = await getStateResponseWithRegen(chain, stateId);
 
       const stateView = (
-        state instanceof Uint8Array ? loadState(config, chain.getHeadState(), state).state : state.clone()
+        state instanceof Uint8Array ? loadState(config, chain.getHeadState(), state).state : state
       ) as BeaconStateCapella;
 
       const fork = config.getForkName(stateView.slot);

packages/beacon-node/src/api/impl/proof/index.ts

@@ -27,8 +27,7 @@ export function getProofApi(
       const state =
         res.state instanceof Uint8Array ? loadState(config, chain.getHeadState(), res.state).state : res.state;
 
-      // Commit any changes before computing the state root. In normal cases the state should have no changes here
-      state.commit();
+      // there should be no state changes in beacon-node so no need to commit() here
       const stateNode = state.node;
 
       const proof = createProof(stateNode, {type: ProofType.compactMulti, descriptor});

packages/beacon-node/src/api/impl/validator/index.ts

@@ -1040,9 +1040,7 @@ export function getValidatorApi(
           const res = await getStateResponseWithRegen(chain, startSlot);
 
           const stateViewDU =
-            res.state instanceof Uint8Array
-              ? loadState(config, chain.getHeadState(), res.state).state
-              : res.state.clone();
+            res.state instanceof Uint8Array ? loadState(config, chain.getHeadState(), res.state).state : res.state;
 
           state = createCachedBeaconState(
             stateViewDU,

packages/beacon-node/src/chain/blocks/importBlock.ts

@@ -423,8 +423,8 @@ export async function importBlock(
     const checkpointState = postState;
     const cp = getCheckpointFromState(checkpointState);
     this.regen.addCheckpointState(cp, checkpointState);
-    // consumers should not mutate or get the transfered cache
-    this.emitter.emit(ChainEvent.checkpoint, cp, checkpointState.clone(true));
+    // consumers should not mutate state ever
+    this.emitter.emit(ChainEvent.checkpoint, cp, checkpointState);
 
     // Note: in-lined code from previos handler of ChainEvent.checkpoint
     this.logger.verbose("Checkpoint processed", toCheckpointHex(cp));

packages/beacon-node/src/chain/blocks/verifyBlocksStateTransitionOnly.ts

@@ -59,6 +59,7 @@ export async function verifyBlocksStateTransitionOnly(
         // if block is trusted don't verify proposer or op signature
         verifyProposer: !useBlsBatchVerify && !validSignatures && !validProposerSignature,
         verifySignatures: !useBlsBatchVerify && !validSignatures,
+        dontTransferCache: false,
       },
       {metrics, validatorMonitor}
     );

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

@@ -504,7 +504,7 @@ export class BeaconChain implements IBeaconChain {
   async getStateBySlot(
     slot: Slot,
     opts?: StateGetOpts
-  ): Promise<{state: BeaconStateAllForks; executionOptimistic: boolean; finalized: boolean} | null> {
+  ): Promise<{state: CachedBeaconStateAllForks; executionOptimistic: boolean; finalized: boolean} | null> {
     const finalizedBlock = this.forkChoice.getFinalizedBlock();
 
     if (slot < finalizedBlock.slot) {
@@ -559,7 +559,7 @@ export class BeaconChain implements IBeaconChain {
   async getStateByStateRoot(
     stateRoot: RootHex,
     opts?: StateGetOpts
-  ): Promise<{state: BeaconStateAllForks; executionOptimistic: boolean; finalized: boolean} | null> {
+  ): Promise<{state: CachedBeaconStateAllForks | Uint8Array; executionOptimistic: boolean; finalized: boolean} | null> {
     if (opts?.allowRegen) {
       const state = await this.regen.getState(stateRoot, RegenCaller.restApi);
       const block = this.forkChoice.getBlock(state.latestBlockHeader.hashTreeRoot());
@@ -587,7 +587,8 @@ export class BeaconChain implements IBeaconChain {
       };
     }
 
-    const data = await this.db.stateArchive.getByRoot(fromHex(stateRoot));
+    // this is mostly useful for a node with `--chain.archiveStateEpochFrequency 1`
+    const data = await this.db.stateArchive.getBinaryByRoot(fromHex(stateRoot));
     return data && {state: data, executionOptimistic: false, finalized: true};
   }
 
@@ -1295,9 +1296,9 @@ export class BeaconChain implements IBeaconChain {
 
     preState = processSlots(preState, block.slot); // Dial preState's slot to block.slot
 
-    const postState = this.regen.getStateSync(toRootHex(block.stateRoot)) ?? undefined;
+    const proposerRewards = this.regen.getStateSync(toRootHex(block.stateRoot))?.proposerRewards ?? undefined;
 
-    return computeBlockRewards(this.config, block, preState.clone(), postState?.clone());
+    return computeBlockRewards(this.config, block, preState, proposerRewards);
   }
 
   async getAttestationsRewards(
@@ -1338,6 +1339,6 @@ export class BeaconChain implements IBeaconChain {
 
     preState = processSlots(preState, block.slot); // Dial preState's slot to block.slot
 
-    return computeSyncCommitteeRewards(this.config, this.index2pubkey, block, preState.clone(), validatorIds);
+    return computeSyncCommitteeRewards(this.config, this.index2pubkey, block, preState, validatorIds);
   }
 }

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

@@ -168,12 +168,12 @@ export interface IBeaconChain {
   getStateBySlot(
     slot: Slot,
     opts?: StateGetOpts
-  ): Promise<{state: BeaconStateAllForks; executionOptimistic: boolean; finalized: boolean} | null>;
+  ): Promise<{state: CachedBeaconStateAllForks; executionOptimistic: boolean; finalized: boolean} | null>;
   /** Returns a local state by state root */
   getStateByStateRoot(
     stateRoot: RootHex,
     opts?: StateGetOpts
-  ): Promise<{state: BeaconStateAllForks; executionOptimistic: boolean; finalized: boolean} | null>;
+  ): Promise<{state: CachedBeaconStateAllForks | Uint8Array; executionOptimistic: boolean; finalized: boolean} | null>;
   /** Return serialized bytes of a persisted checkpoint state */
   getPersistedCheckpointState(checkpoint?: phase0.Checkpoint): Promise<Uint8Array | null>;
   /** Returns a cached state by checkpoint */

packages/beacon-node/src/chain/lightClient/proofs.ts

@@ -12,7 +12,6 @@ import {BeaconBlockBody, SSZTypesFor, ssz} from "@lodestar/types";
 import {SyncCommitteeWitness} from "./types.js";
 
 export function getSyncCommitteesWitness(fork: ForkName, state: BeaconStateAllForks): SyncCommitteeWitness {
-  state.commit();
   const n1 = state.node;
   let witness: Uint8Array[];
   let currentSyncCommitteeRoot: Uint8Array;
@@ -71,7 +70,6 @@ export function getCurrentSyncCommitteeBranch(syncCommitteesWitness: SyncCommitt
 }
 
 export function getFinalizedRootProof(state: CachedBeaconStateAllForks): Uint8Array[] {
-  state.commit();
   const finalizedRootGindex = state.epochCtx.isPostElectra() ? FINALIZED_ROOT_GINDEX_ELECTRA : FINALIZED_ROOT_GINDEX;
   return new Tree(state.node).getSingleProof(BigInt(finalizedRootGindex));
 }

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

@@ -148,6 +148,7 @@ export class PrepareNextSlotScheduler {
             updatedPrepareState = (await this.chain.regen.getBlockSlotState(
               proposerHeadRoot,
               prepareSlot,
+              // only transfer cache if epoch transition because that's the state we will use to stateTransition() the 1st block of epoch
               {dontTransferCache: !isEpochTransition},
               RegenCaller.predictProposerHead
             )) as CachedBeaconStateExecutions;

packages/beacon-node/src/chain/regen/interface.ts

@@ -90,5 +90,5 @@ export interface IStateRegeneratorInternal {
   /**
    * Return the exact state with `stateRoot`
    */
-  getState(stateRoot: RootHex, rCaller: RegenCaller, opts?: StateRegenerationOpts): Promise<CachedBeaconStateAllForks>;
+  getState(stateRoot: RootHex, rCaller: RegenCaller): Promise<CachedBeaconStateAllForks>;
 }

packages/beacon-node/src/chain/regen/queued.ts

@@ -79,21 +79,15 @@ export class QueuedStateRegenerator implements IStateRegenerator {
 
   /**
    * Get a state from block state cache.
-   * This is not for block processing so don't transfer cache
    */
   getStateSync(stateRoot: RootHex): CachedBeaconStateAllForks | null {
-    return this.blockStateCache.get(stateRoot, {dontTransferCache: true});
+    return this.blockStateCache.get(stateRoot);
   }
 
   /**
    * Get state for block processing.
-   * By default, do not transfer cache except for the block at clock slot
-   * which is usually the gossip block.
    */
-  getPreStateSync(
-    block: BeaconBlock,
-    opts: StateRegenerationOpts = {dontTransferCache: true}
-  ): CachedBeaconStateAllForks | null {
+  getPreStateSync(block: BeaconBlock): CachedBeaconStateAllForks | null {
     const parentRoot = toRootHex(block.parentRoot);
     const parentBlock = this.forkChoice.getBlockHex(parentRoot);
     if (!parentBlock) {
@@ -108,7 +102,7 @@ export class QueuedStateRegenerator implements IStateRegenerator {
 
     // Check the checkpoint cache (if the pre-state is a checkpoint state)
     if (parentEpoch < blockEpoch) {
-      const checkpointState = this.checkpointStateCache.getLatest(parentRoot, blockEpoch, opts);
+      const checkpointState = this.checkpointStateCache.getLatest(parentRoot, blockEpoch);
       if (checkpointState && computeEpochAtSlot(checkpointState.slot) === blockEpoch) {
         return checkpointState;
       }
@@ -118,7 +112,7 @@ export class QueuedStateRegenerator implements IStateRegenerator {
     // Otherwise the state transition may not be cached and wasted. Queue for regen since the
     // work required will still be significant.
     if (parentEpoch === blockEpoch) {
-      const state = this.blockStateCache.get(parentBlock.stateRoot, opts);
+      const state = this.blockStateCache.get(parentBlock.stateRoot);
       if (state) {
         return state;
       }
@@ -132,21 +126,17 @@ export class QueuedStateRegenerator implements IStateRegenerator {
   }
 
   /**
-   * Get checkpoint state from cache, this function is not for block processing so don't transfer cache
+   * Get checkpoint state from cache
    */
   getCheckpointStateSync(cp: CheckpointHex): CachedBeaconStateAllForks | null {
-    return this.checkpointStateCache.get(cp, {dontTransferCache: true});
+    return this.checkpointStateCache.get(cp);
   }
 
   /**
-   * Get state closest to head, this function is not for block processing so don't transfer cache
+   * Get state closest to head
    */
   getClosestHeadState(head: ProtoBlock): CachedBeaconStateAllForks | null {
-    const opts = {dontTransferCache: true};
-    return (
-      this.checkpointStateCache.getLatest(head.blockRoot, Infinity, opts) ||
-      this.blockStateCache.get(head.stateRoot, opts)
-    );
+    return this.checkpointStateCache.getLatest(head.blockRoot, Infinity) || this.blockStateCache.get(head.stateRoot);
   }
 
   pruneOnCheckpoint(finalizedEpoch: Epoch, justifiedEpoch: Epoch, headStateRoot: RootHex): void {
@@ -181,10 +171,7 @@ export class QueuedStateRegenerator implements IStateRegenerator {
       maybeHeadStateRoot,
     };
     const headState =
-      newHeadStateRoot === maybeHeadStateRoot
-        ? maybeHeadState
-        : // maybeHeadState was already in block state cache so we don't transfer the cache
-          this.blockStateCache.get(newHeadStateRoot, {dontTransferCache: true});
+      newHeadStateRoot === maybeHeadStateRoot ? maybeHeadState : this.blockStateCache.get(newHeadStateRoot);
 
     if (headState) {
       this.blockStateCache.setHeadState(headState);
@@ -199,9 +186,7 @@ export class QueuedStateRegenerator implements IStateRegenerator {
 
       // for the new FIFOBlockStateCache, it's important to reload state to regen head state here if needed
       const allowDiskReload = true;
-      // transfer cache here because we want to regen state asap
-      const cloneOpts = {dontTransferCache: false};
-      this.regen.getState(newHeadStateRoot, RegenCaller.processBlock, cloneOpts, allowDiskReload).then(
+      this.regen.getState(newHeadStateRoot, RegenCaller.processBlock, allowDiskReload).then(
         (headStateRegen) => this.blockStateCache.setHeadState(headStateRegen),
         (e) => this.logger.error("Error on head state regen", logCtx, e)
       );
@@ -224,7 +209,7 @@ export class QueuedStateRegenerator implements IStateRegenerator {
     this.metrics?.regenFnCallTotal.inc({caller: rCaller, entrypoint: RegenFnName.getPreState});
 
     // First attempt to fetch the state from caches before queueing
-    const cachedState = this.getPreStateSync(block, opts);
+    const cachedState = this.getPreStateSync(block);
 
     if (cachedState !== null) {
       return cachedState;
@@ -243,7 +228,7 @@ export class QueuedStateRegenerator implements IStateRegenerator {
     this.metrics?.regenFnCallTotal.inc({caller: rCaller, entrypoint: RegenFnName.getCheckpointState});
 
     // First attempt to fetch the state from cache before queueing
-    const checkpointState = this.checkpointStateCache.get(toCheckpointHex(cp), opts);
+    const checkpointState = this.checkpointStateCache.get(toCheckpointHex(cp));
     if (checkpointState) {
       return checkpointState;
     }
@@ -271,22 +256,18 @@ export class QueuedStateRegenerator implements IStateRegenerator {
     return this.jobQueue.push({key: "getBlockSlotState", args: [blockRoot, slot, opts, rCaller]});
   }
 
-  async getState(
-    stateRoot: RootHex,
-    rCaller: RegenCaller,
-    opts: StateRegenerationOpts = {dontTransferCache: true}
-  ): Promise<CachedBeaconStateAllForks> {
+  async getState(stateRoot: RootHex, rCaller: RegenCaller): Promise<CachedBeaconStateAllForks> {
     this.metrics?.regenFnCallTotal.inc({caller: rCaller, entrypoint: RegenFnName.getState});
 
     // First attempt to fetch the state from cache before queueing
-    const state = this.blockStateCache.get(stateRoot, opts);
+    const state = this.blockStateCache.get(stateRoot);
     if (state) {
       return state;
     }
 
     // The state is not immediately available in the cache, enqueue the job
     this.metrics?.regenFnQueuedTotal.inc({caller: rCaller, entrypoint: RegenFnName.getState});
-    return this.jobQueue.push({key: "getState", args: [stateRoot, rCaller, opts]});
+    return this.jobQueue.push({key: "getState", args: [stateRoot, rCaller]});
   }
 
   private jobQueueProcessor = async (regenRequest: RegenRequest): Promise<CachedBeaconStateAllForks> => {