refactor: remove build() and async shuffling calculation

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

@@ -118,12 +118,7 @@ export class PrepareNextSlotScheduler {
         // the slot 0 of next epoch will likely use this Previous Root Checkpoint state for state transition so we transfer cache here
         // the resulting state with cache will be cached in Checkpoint State Cache which is used for the upcoming block processing
         // for other slots dontTransferCached=true because we don't run state transition on this state
-        //
-        // Shuffling calculation will be done asynchronously when passing asyncShufflingCalculation=true.  Shuffling will be queued in
-        // beforeProcessEpoch and should theoretically be ready immediately after the synchronous epoch transition finished and the
-        // event loop is free.  In long periods of non-finality too many forks will cause the shufflingCache to throw an error for
-        // too many queued shufflings so only run async during normal epoch transition. See issue ChainSafe/lodestar#7244
-        {dontTransferCache: !isEpochTransition, asyncShufflingCalculation: true},
+        {dontTransferCache: !isEpochTransition},
         RegenCaller.precomputeEpoch
       );
 

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

@@ -31,10 +31,6 @@ export enum RegenFnName {
 
 export type StateRegenerationOpts = {
   dontTransferCache: boolean;
-  /**
-   * Do not queue shuffling calculation async. Forces sync JIT calculation in afterProcessEpoch if not passed as `true`
-   */
-  asyncShufflingCalculation?: boolean;
 };
 
 export interface IStateRegenerator extends IStateRegeneratorInternal {

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

@@ -1,11 +1,4 @@
-import {
-  BeaconStateAllForks,
-  EpochShuffling,
-  IShufflingCache,
-  ShufflingBuildProps,
-  computeEpochShuffling,
-  computeEpochShufflingAsync,
-} from "@lodestar/state-transition";
+import {EpochShuffling, IShufflingCache, ShufflingBuildProps, computeEpochShuffling} from "@lodestar/state-transition";
 import {Epoch, RootHex} from "@lodestar/types";
 import {LodestarError, Logger, MapDef, pruneSetToMax} from "@lodestar/utils";
 import {Metrics} from "../metrics/metrics.js";
@@ -170,33 +163,11 @@ export class ShufflingCache implements IShufflingCache {
     return shuffling as T extends ShufflingBuildProps ? EpochShuffling : EpochShuffling | null;
   }
 
-  /**
-   * Queue asynchronous build for an EpochShuffling, triggered from state-transition
-   */
-  build(epoch: number, decisionRoot: string, state: BeaconStateAllForks, activeIndices: Uint32Array): void {
-    this.insertPromise(epoch, decisionRoot);
-    /**
-     * TODO: (@matthewkeil) This will get replaced by a proper build queue and a worker to do calculations
-     * on a NICE thread
-     */
-    const timer = this.metrics?.shufflingCache.shufflingCalculationTime.startTimer({source: "build"});
-    computeEpochShufflingAsync(state, activeIndices, epoch)
-      .then((shuffling) => {
-        this.set(shuffling, decisionRoot);
-      })
-      .catch((err) =>
-        this.logger?.error(`error building shuffling for epoch ${epoch} at decisionRoot ${decisionRoot}`, {}, err)
-      )
-      .finally(() => {
-        timer?.();
-      });
-  }
-
   /**
    * Add an EpochShuffling to the ShufflingCache. If a promise for the shuffling is present it will
    * resolve the promise with the built shuffling
    */
-  private set(shuffling: EpochShuffling, decisionRoot: string): void {
+  set(shuffling: EpochShuffling, decisionRoot: string): void {
     const shufflingAtEpoch = this.itemsByDecisionRootByEpoch.getOrDefault(shuffling.epoch);
     // if a pending shuffling promise exists, resolve it
     const cacheItem = shufflingAtEpoch.get(decisionRoot);

packages/beacon-node/test/unit/chain/shufflingCache.test.ts

@@ -29,7 +29,7 @@ describe("ShufflingCache", () => {
     shufflingCache.insertPromise(currentEpoch, "0x00");
     expect(await shufflingCache.get(currentEpoch, currentDecisionRoot)).toEqual(state.epochCtx.currentShuffling);
     // insert shuffling at other epochs does prune the cache
-    shufflingCache["set"](state.epochCtx.previousShuffling, state.epochCtx.previousDecisionRoot);
+    shufflingCache.set(state.epochCtx.previousShuffling, state.epochCtx.previousDecisionRoot);
     // the current shuffling is not available anymore
     expect(await shufflingCache.get(currentEpoch, currentDecisionRoot)).toBeNull();
   });
@@ -40,7 +40,7 @@ describe("ShufflingCache", () => {
     shufflingCache.insertPromise(previousEpoch, previousDecisionRoot);
     const shufflingRequest0 = shufflingCache.get(previousEpoch, previousDecisionRoot);
     const shufflingRequest1 = shufflingCache.get(previousEpoch, previousDecisionRoot);
-    shufflingCache["set"](state.epochCtx.previousShuffling, previousDecisionRoot);
+    shufflingCache.set(state.epochCtx.previousShuffling, previousDecisionRoot);
     expect(await shufflingRequest0).toEqual(state.epochCtx.previousShuffling);
     expect(await shufflingRequest1).toEqual(state.epochCtx.previousShuffling);
   });

packages/state-transition/src/cache/epochCache.ts

@@ -678,35 +678,10 @@ export class EpochCache {
     this.nextDecisionRoot = epochTransitionCache.nextShufflingDecisionRoot;
     this.nextActiveIndices = epochTransitionCache.nextShufflingActiveIndices;
     if (this.shufflingCache) {
-      if (!epochTransitionCache.asyncShufflingCalculation) {
-        this.nextShuffling = this.shufflingCache.getSync(epochAfterUpcoming, this.nextDecisionRoot, {
-          state,
-          activeIndices: this.nextActiveIndices,
-        });
-      } else {
-        this.nextShuffling = null;
-        // This promise will resolve immediately after the synchronous code of the state-transition runs. Until
-        // the build is done on a worker thread it will be calculated immediately after the epoch transition
-        // completes.  Once the work is done concurrently it should be ready by time this get runs so the promise
-        // will resolve directly on the next spin of the event loop because the epoch transition and shuffling take
-        // about the same time to calculate so theoretically its ready now.  Do not await here though in case it
-        // is not ready yet as the transition must not be asynchronous.
-        this.shufflingCache
-          .get(epochAfterUpcoming, this.nextDecisionRoot)
-          .then((shuffling) => {
-            if (!shuffling) {
-              throw new Error("EpochShuffling not returned from get in afterProcessEpoch");
-            }
-            this.nextShuffling = shuffling;
-          })
-          .catch((err) => {
-            this.shufflingCache?.logger?.error(
-              "EPOCH_CONTEXT_SHUFFLING_BUILD_ERROR",
-              {epoch: epochAfterUpcoming, decisionRoot: epochTransitionCache.nextShufflingDecisionRoot},
-              err
-            );
-          });
-      }
+      this.nextShuffling = this.shufflingCache.getSync(epochAfterUpcoming, this.nextDecisionRoot, {
+        state,
+        activeIndices: this.nextActiveIndices,
+      });
     } else {
       // Only for testing. shufflingCache should always be available in prod
       this.nextShuffling = computeEpochShuffling(state, this.nextActiveIndices, epochAfterUpcoming);

packages/state-transition/src/cache/epochTransitionCache.ts

@@ -32,10 +32,6 @@ export type EpochTransitionCacheOpts = {
    * Assert progressive balances the same to EpochTransitionCache
    */
   assertCorrectProgressiveBalances?: boolean;
-  /**
-   * Do not queue shuffling calculation async. Forces sync JIT calculation in afterProcessEpoch
-   */
-  asyncShufflingCalculation?: boolean;
 };
 
 /**
@@ -179,12 +175,6 @@ export interface EpochTransitionCache {
    */
   nextEpochTotalActiveBalanceByIncrement: number;
 
-  /**
-   * Compute the shuffling sync or async.  Defaults to synchronous.  Need to pass `true` with the
-   * `EpochTransitionCacheOpts`
-   */
-  asyncShufflingCalculation: boolean;
-
   /**
    * Track by validator index if it's active in the prev epoch.
    * Used in metrics
@@ -380,7 +370,6 @@ export function beforeProcessEpoch(
   });
 
   // Trigger async build of shuffling for epoch after next (nextShuffling post epoch transition)
-  const epochAfterNext = state.epochCtx.nextEpoch + 1;
   // cannot call calculateShufflingDecisionRoot here because spec prevent getting current slot
   // as a decision block.  we are part way through the transition though and this was added in
   // process slot beforeProcessEpoch happens so it available and valid
@@ -396,11 +385,6 @@ export function beforeProcessEpoch(
     nextShufflingActiveIndices[i] = nextEpochShufflingActiveValidatorIndices[i];
   }
 
-  const asyncShufflingCalculation = opts?.asyncShufflingCalculation ?? false;
-  if (asyncShufflingCalculation) {
-    state.epochCtx.shufflingCache?.build(epochAfterNext, nextShufflingDecisionRoot, state, nextShufflingActiveIndices);
-  }
-
   if (totalActiveStakeByIncrement < 1) {
     totalActiveStakeByIncrement = 1;
   } else if (totalActiveStakeByIncrement >= Number.MAX_SAFE_INTEGER) {
@@ -526,7 +510,6 @@ export function beforeProcessEpoch(
     indicesToEject,
     nextShufflingDecisionRoot,
     nextShufflingActiveIndices,
-    asyncShufflingCalculation,
     // to be updated in processEffectiveBalanceUpdates
     nextEpochTotalActiveBalanceByIncrement: 0,
     isActivePrevEpoch,

packages/state-transition/src/util/epochShuffling.ts

@@ -52,9 +52,9 @@ export interface IShufflingCache {
   get(epoch: Epoch, decisionRoot: RootHex): Promise<EpochShuffling | null>;
 
   /**
-   * Queue asynchronous build for an EpochShuffling
+   * Add an EpochShuffling to the ShufflingCache.
    */
-  build(epoch: Epoch, decisionRoot: RootHex, state: BeaconStateAllForks, activeIndices: Uint32Array): void;
+  set(shuffling: EpochShuffling, decisionRoot: RootHex): void;
 }
 
 /**