feat: merge-train/spartan (#23748)

BEGIN_COMMIT_OVERRIDE
test(slash): attest eagerly when skipping checkpoint validation (#23722)
END_COMMIT_OVERRIDE

Author: AztecBot

yarn-project/p2p/src/config.ts

@@ -246,6 +246,13 @@ export interface P2PConfig
 
   /** Accept proposal gossip regardless of slot timing (for testing only). */
   skipProposalSlotValidation?: boolean;
+
+  /**
+   * Whether this node skips checkpoint proposal validation and always attests. When set, the checkpoint
+   * attestation is created and broadcast before the embedded last block is processed, so it is not delayed
+   * past the slot's attestation window by that block's re-execution. Mirrors the validator config flag.
+   */
+  skipCheckpointProposalValidation?: boolean;
 }
 
 export const DEFAULT_P2P_PORT = 40400;
@@ -580,6 +587,11 @@ export const p2pConfigMappings: ConfigMappingsType<P2PConfig> = {
     description: 'Accept proposal gossip regardless of slot timing (for testing only)',
     ...booleanConfigHelper(false),
   },
+  skipCheckpointProposalValidation: {
+    description:
+      'Skip checkpoint proposal validation and always attest, broadcasting the attestation before processing the embedded last block',
+    ...booleanConfigHelper(false),
+  },
   minTxPoolAgeMs: {
     env: 'P2P_MIN_TX_POOL_AGE_MS',
     description: 'Minimum age (ms) a transaction must have been in the pool before it is eligible for block building.',

yarn-project/p2p/src/services/libp2p/libp2p_service.test.ts

@@ -955,6 +955,93 @@ describe('LibP2PService', () => {
       expect(storedBlock).toBeDefined();
     });
 
+    it('skipCheckpointProposalValidation: attests before (not gated by) slow last-block processing', async () => {
+      // Recreate the service in skip-validation mode and re-register the checkpoint/block callbacks on it.
+      service = createTestLibP2PServiceWithPools(
+        mockPeerManager,
+        reportMessageValidationResultSpy,
+        attestationPool,
+        mockTxPool,
+        mockEpochCache,
+        { skipCheckpointProposalValidation: true },
+      );
+      service.registerBlockReceivedCallback(blockReceivedCallback as any);
+      service.registerValidatorCheckpointReceivedCallback(validatorCheckpointReceivedCallback as any);
+      service.registerAllNodesCheckpointReceivedCallback(allNodesCheckpointReceivedCallback as any);
+
+      const checkpointHeader = makeCheckpointHeader(1, { slotNumber: targetSlot });
+      const blockHeader = makeBlockHeader(1, { slotNumber: targetSlot });
+      const proposal = await makeCheckpointProposal({ signer, checkpointHeader, lastBlock: { blockHeader } });
+
+      // Block processing hangs until released, simulating waiting for the parent block up to the
+      // re-execution deadline. In skip mode the attestation must not be blocked behind it.
+      let releaseBlock!: () => void;
+      blockReceivedCallback.mockReturnValue(
+        new Promise<boolean>(resolve => {
+          releaseBlock = () => resolve(true);
+        }),
+      );
+
+      // Resolves once the checkpoint attestation callback runs; if it were serialized behind the hung block
+      // processing this would never resolve and the test would time out.
+      let signalCheckpoint!: () => void;
+      const checkpointInvoked = new Promise<void>(resolve => {
+        signalCheckpoint = resolve;
+      });
+      validatorCheckpointReceivedCallback.mockImplementation(() => {
+        signalCheckpoint();
+        return Promise.resolve([]);
+      });
+
+      const handled = service.handleGossipedCheckpointProposal(proposal.toBuffer(), 'msg-1', mockPeerId);
+
+      await checkpointInvoked;
+      expect(validatorCheckpointReceivedCallback).toHaveBeenCalledTimes(1);
+
+      releaseBlock();
+      await handled;
+      expect(blockReceivedCallback).toHaveBeenCalledTimes(1);
+    });
+
+    it('default: processes the last block before the checkpoint proposal', async () => {
+      const checkpointHeader = makeCheckpointHeader(1, { slotNumber: targetSlot });
+      const blockHeader = makeBlockHeader(1, { slotNumber: targetSlot });
+      const proposal = await makeCheckpointProposal({ signer, checkpointHeader, lastBlock: { blockHeader } });
+
+      // Block processing hangs until released and signals when it starts; with validation enabled the
+      // checkpoint callback must wait for the block to finish.
+      let releaseBlock!: () => void;
+      let signalBlockStarted!: () => void;
+      const blockStarted = new Promise<void>(resolve => {
+        signalBlockStarted = resolve;
+      });
+      blockReceivedCallback.mockImplementation(() => {
+        signalBlockStarted();
+        return new Promise<boolean>(resolve => {
+          releaseBlock = () => resolve(true);
+        });
+      });
+
+      let checkpointInvoked = false;
+      validatorCheckpointReceivedCallback.mockImplementation(() => {
+        checkpointInvoked = true;
+        return Promise.resolve([]);
+      });
+
+      const handled = service.handleGossipedCheckpointProposal(proposal.toBuffer(), 'msg-1', mockPeerId);
+
+      // Wait until block processing is in flight (hung), then flush microtasks. The checkpoint callback
+      // must not have run, since it is gated behind the block.
+      await blockStarted;
+      await new Promise(resolve => setImmediate(resolve));
+      expect(checkpointInvoked).toBe(false);
+
+      // Once the block completes, the checkpoint proposal is processed.
+      releaseBlock();
+      await handled;
+      expect(checkpointInvoked).toBe(true);
+    });
+
     it('lastBlock processed even when checkpoint cap exceeded', async () => {
       const checkpointHeader = makeCheckpointHeader(1, { slotNumber: targetSlot });
       const blockHeader = makeBlockHeader(1, { slotNumber: targetSlot });
@@ -1525,6 +1612,7 @@ function createTestLibP2PServiceWithPools(
   attestationPool: AttestationPool,
   mockTxPool: MockProxy<TxPoolV2>,
   mockEpochCache: MockProxy<EpochCacheInterface>,
+  configOverrides?: Partial<P2PConfig>,
 ): TestLibP2PService {
   const mockNode = mock<PubSubLibp2p>();
   mockNode.services = {
@@ -1539,5 +1627,6 @@ function createTestLibP2PServiceWithPools(
     attestationPool,
     txPool: mockTxPool,
     epochCache: mockEpochCache,
+    configOverrides,
   });
 }

yarn-project/p2p/src/services/libp2p/libp2p_service.ts

@@ -1342,17 +1342,33 @@ export class LibP2PService extends WithTracer implements P2PService {
       TopicType.checkpoint_proposal,
     );
 
+    // Process checkpoint proposal if valid and not equivocated.
+    const processCheckpointFn = () =>
+      result === TopicValidatorResult.Accept && checkpoint && !isEquivocated
+        ? this.processValidCheckpointProposal(checkpoint.toCore(), source)
+        : Promise.resolve();
+
     // If the checkpoint contained a valid last block, we process it even if the checkpoint itself is to be rejected
     // TODO(palla/mbps): Is this ok? Should we be considering a block from a checkpoint that was equivocated?
-    if (processBlock && checkpoint?.getBlockProposal()) {
-      await this.processValidBlockProposal(checkpoint.getBlockProposal()!, source);
-    }
-
-    if (result !== TopicValidatorResult.Accept || !checkpoint || isEquivocated) {
+    const processBlockFn = () =>
+      processBlock && checkpoint && checkpoint.getBlockProposal()
+        ? this.processValidBlockProposal(checkpoint.getBlockProposal()!, source)
+        : Promise.resolve();
+
+    // A node that skips checkpoint validation attests without re-executing the embedded last block, so run
+    // the checkpoint callback first: this creates and broadcasts the attestation before the block is
+    // processed. Otherwise the block's re-execution — which can stall until the re-execution deadline
+    // waiting for a parent that may never arrive — would delay the attestation past the slot's attestation
+    // window, after which peers reject it as stale.
+    if (this.config.skipCheckpointProposalValidation) {
+      await processCheckpointFn();
+      await processBlockFn();
       return;
     }
 
-    await this.processValidCheckpointProposal(checkpoint.toCore(), source);
+    // Process the block first, since it's required for the checkpoint proposal validation.
+    await processBlockFn();
+    await processCheckpointFn();
   }
 
   /**

yarn-project/validator-client/src/proposal_handler.ts

@@ -311,14 +311,6 @@ export class ProposalHandler {
           return undefined;
         }
 
-        if (this.config.skipCheckpointProposalValidation) {
-          this.log.warn(
-            `Skipping all-nodes checkpoint proposal validation for slot ${proposal.slotNumber}`,
-            proposalInfo,
-          );
-          return undefined;
-        }
-
         const result = await this.handleCheckpointProposal(proposal, proposalInfo);
         if (!result.isValid) {
           await this.checkpointProposalValidationFailureCallback?.(proposal, result, proposalInfo);