Verify SyncCommittee signatures in batch

packages/beacon-state-transition/src/allForks/signatureSets/index.ts

@@ -47,7 +47,7 @@ export function getAllBlockSignatureSetsExceptProposer(
   if (computeEpochAtSlot(state.config, signedBlock.message.slot) >= state.config.params.ALTAIR_FORK_EPOCH) {
     const syncCommitteeSignatureSet = getSyncCommitteeSignatureSet(
       state as CachedBeaconState<altair.BeaconState>,
-      (signedBlock as altair.SignedBeaconBlock).message.body.syncAggregate
+      (signedBlock as altair.SignedBeaconBlock).message
     );
     // There may be no participants in this syncCommitteeSignature, so it must not be validated
     if (syncCommitteeSignatureSet) {

packages/beacon-state-transition/src/altair/block/index.ts

@@ -29,5 +29,5 @@ export function processBlock(
   processRandao(state as CachedBeaconState<allForks.BeaconState>, block, verifySignatures);
   processEth1Data(state as CachedBeaconState<allForks.BeaconState>, block.body);
   processOperations(state, block.body, verifySignatures);
-  processSyncCommittee(state, block.body.syncAggregate, verifySignatures);
+  processSyncCommittee(state, block, verifySignatures);
 }

packages/beacon-state-transition/src/altair/block/processSyncCommittee.ts

@@ -15,15 +15,16 @@ import {BitList, isTreeBacked, TreeBacked} from "@chainsafe/ssz";
 
 export function processSyncCommittee(
   state: CachedBeaconState<altair.BeaconState>,
-  syncAggregate: altair.SyncAggregate,
+  block: altair.BeaconBlock,
   verifySignatures = true
 ): void {
   const {syncParticipantReward, syncProposerReward} = state.epochCtx;
-  const participantIndices = getParticipantIndices(state, syncAggregate);
+  const participantIndices = getParticipantIndices(state, block.body.syncAggregate);
 
   // different from the spec but not sure how to get through signature verification for default/empty SyncAggregate in the spec test
   if (verifySignatures) {
-    const signatureSet = getSyncCommitteeSignatureSet(state, syncAggregate, participantIndices);
+    // This is to conform to the spec - we want the signature to be verified
+    const signatureSet = getSyncCommitteeSignatureSet(state, block, participantIndices);
     // When there's no participation we consider the signature valid and just ignore i
     if (signatureSet !== null && !verifySignatureSet(signatureSet)) {
       throw Error("Sync committee signature invalid");
@@ -39,13 +40,13 @@ export function processSyncCommittee(
 
 export function getSyncCommitteeSignatureSet(
   state: CachedBeaconState<altair.BeaconState>,
-  syncAggregate: altair.SyncAggregate,
+  block: altair.BeaconBlock,
   /** Optional parameter to prevent computing it twice */
   participantIndices?: number[]
 ): ISignatureSet | null {
   const {config, epochCtx} = state;
-
-  const previousSlot = Math.max(state.slot, 1) - 1;
+  const {syncAggregate} = block.body;
+  const previousSlot = Math.max(block.slot, 1) - 1;
   const rootSigned = getBlockRootAtSlot(config, state, previousSlot);
 
   if (!participantIndices) {

packages/lodestar/src/chain/blocks/process.ts

@@ -119,10 +119,20 @@ export async function processChainSegment(
     }
 
     try {
-      // It's important to process up to block slot to get through signature verification at fork transition
-      let preState = await regen.getBlockSlotState(firstBlock.message.parentRoot, firstBlock.message.slot);
-
-      // Verify the signature of the blocks, returning early if the signature is invalid.
+      let preState = await regen.getPreState(firstBlock.message);
+      for (const block of blocksInEpoch) {
+        preState = await runStateTransition({emitter, forkChoice, metrics}, checkpointStateCache, preState, {
+          reprocess: job.reprocess,
+          prefinalized: job.prefinalized,
+          signedBlock: block,
+          validProposerSignature: true,
+          validSignatures: true,
+        });
+        importedBlocks++;
+        // this avoids keeping our node busy processing blocks
+        await sleep(0);
+      }
+      // Verify the signature afterall bc all SyncCommittee signed roots are only known at this point
       if (!job.validSignatures && !opts?.disableBlsBatchVerify) {
         const signatureSets: ISignatureSet[] = [];
         for (const block of blocksInEpoch) {
@@ -141,19 +151,6 @@ export async function processChainSegment(
           });
         }
       }
-
-      for (const block of blocksInEpoch) {
-        preState = await runStateTransition({emitter, forkChoice, metrics}, checkpointStateCache, preState, {
-          reprocess: job.reprocess,
-          prefinalized: job.prefinalized,
-          signedBlock: block,
-          validProposerSignature: true,
-          validSignatures: true,
-        });
-        importedBlocks++;
-        // this avoids keeping our node busy processing blocks
-        await sleep(0);
-      }
     } catch (e) {
       if (e instanceof RegenError) {
         throw new ChainSegmentError({

packages/spec-test-runner/test/spec/altair/operations/syncCommittee/sync_committee_minimal.test.ts

@@ -20,10 +20,14 @@ describeDirectorySpecTest<IProcessSyncCommitteeTestCase, altair.BeaconState>(
       config,
       testcase.pre as TreeBacked<altair.BeaconState>
     ) as CachedBeaconState<altair.BeaconState>;
-    altair.processSyncCommittee(
-      wrappedState,
-      config.types.altair.SyncAggregate.createTreeBackedFromStruct(testcase["sync_aggregate"])
-    );
+
+    const block = config.types.altair.BeaconBlock.defaultValue();
+
+    // processSyncCommittee() needs the full block to get the slot
+    block.slot = wrappedState.slot;
+    block.body.syncAggregate = config.types.altair.SyncAggregate.createTreeBackedFromStruct(testcase["sync_aggregate"]);
+
+    altair.processSyncCommittee(wrappedState, block);
     return wrappedState;
   },
   {