fix: Handle proven chain events referring to unseen blocks

yarn-project/end-to-end/src/e2e_p2p/validators_sentinel.test.ts

@@ -1,18 +1,20 @@
 import type { AztecNodeService } from '@aztec/aztec-node';
-import { retryUntil } from '@aztec/aztec.js';
+import { retryUntil, sleep } from '@aztec/aztec.js';
+import type { ValidatorsStats } from '@aztec/stdlib/validators';
 
 import { jest } from '@jest/globals';
 import fs from 'fs';
 import 'jest-extended';
 import os from 'os';
 import path from 'path';
 
-import { createNodes } from '../fixtures/setup_p2p_test.js';
+import { createNode, createNodes } from '../fixtures/setup_p2p_test.js';
 import { P2PNetworkTest, SHORTENED_BLOCK_TIME_CONFIG } from './p2p_network.js';
 
 const NUM_NODES = 4;
 const NUM_VALIDATORS = NUM_NODES + 1; // We create an extra validator, who will not have a running node
 const BOOT_NODE_UDP_PORT = 40900;
+const BLOCK_COUNT = 3;
 
 const DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), 'validators-sentinel-'));
 
@@ -22,7 +24,7 @@ describe('e2e_p2p_validators_sentinel', () => {
   let t: P2PNetworkTest;
   let nodes: AztecNodeService[];
 
-  beforeEach(async () => {
+  beforeAll(async () => {
     t = await P2PNetworkTest.create({
       testName: 'e2e_p2p_network',
       numberOfNodes: NUM_VALIDATORS,
@@ -39,69 +41,101 @@ describe('e2e_p2p_validators_sentinel', () => {
     await t.applyBaseSnapshots();
     await t.setup();
     await t.removeInitialNode();
+
+    t.logger.info(`Setup complete`, { validators: t.validators });
   });
 
-  afterEach(async () => {
+  afterAll(async () => {
     await t.stopNodes(nodes);
     await t.teardown();
     for (let i = 0; i < NUM_NODES; i++) {
       fs.rmSync(`${DATA_DIR}-${i}`, { recursive: true, force: true, maxRetries: 3 });
     }
   });
 
-  it('collects stats for offline validator', async () => {
-    if (!t.bootstrapNodeEnr) {
-      throw new Error('Bootstrap node ENR is not available');
-    }
+  describe('with an offline validator', () => {
+    let stats: ValidatorsStats;
+    beforeAll(async () => {
+      t.logger.info('Creating nodes');
+      nodes = await createNodes(
+        t.ctx.aztecNodeConfig,
+        t.ctx.dateProvider,
+        t.bootstrapNodeEnr,
+        NUM_NODES, // Note we do not create the last validator yet, so it shows as offline
+        BOOT_NODE_UDP_PORT,
+        t.prefilledPublicData,
+        DATA_DIR,
+      );
+
+      const currentBlock = t.monitor.l2BlockNumber;
+      const blockCount = BLOCK_COUNT;
+      const timeout = SHORTENED_BLOCK_TIME_CONFIG.aztecSlotDuration * blockCount * 8;
+      t.logger.info(`Waiting until L2 block ${currentBlock + blockCount}`, { currentBlock, blockCount, timeout });
+      await retryUntil(() => t.monitor.l2BlockNumber >= currentBlock + blockCount, 'blocks mined', timeout);
+
+      stats = await nodes[0].getValidatorsStats();
+      t.logger.info(`Collected validator stats at block ${t.monitor.l2BlockNumber}`, { stats });
+    });
+
+    it('collects stats on offline validator', () => {
+      const offlineValidator = t.validators.at(-1)!.attester.toLowerCase();
+      t.logger.info(`Asserting stats for offline validator ${offlineValidator}`);
+      const offlineStats = stats.stats[offlineValidator];
+      const historyLength = offlineStats.history.length;
+      expect(offlineStats.history.length).toBeGreaterThanOrEqual(BLOCK_COUNT - 1);
+      expect(offlineStats.history.every(h => h.status.endsWith('-missed'))).toBeTrue();
+      expect(offlineStats.missedAttestations.count + offlineStats.missedProposals.count).toEqual(historyLength);
+      expect(offlineStats.missedAttestations.rate).toEqual(1);
+      expect(offlineStats.missedProposals.rate).toBeOneOf([1, NaN]);
+    });
 
-    t.logger.info('Creating nodes');
-    nodes = await createNodes(
-      t.ctx.aztecNodeConfig,
-      t.ctx.dateProvider,
-      t.bootstrapNodeEnr,
-      NUM_NODES,
-      BOOT_NODE_UDP_PORT,
-      t.prefilledPublicData,
-      DATA_DIR,
-    );
-
-    // Wait for a few blocks to be mined
-    const currentBlock = t.monitor.l2BlockNumber;
-    const blockCount = 3;
-    const timeout = SHORTENED_BLOCK_TIME_CONFIG.aztecSlotDuration * blockCount * 8;
-    t.logger.info(`Waiting until L2 block ${currentBlock + blockCount}`, { currentBlock, blockCount, timeout });
-    await retryUntil(() => t.monitor.l2BlockNumber >= currentBlock + blockCount, 'blocks mined', timeout);
-
-    const stats = await nodes[0].getValidatorsStats();
-    t.logger.info(`Collected validator stats at block ${t.monitor.l2BlockNumber}`, { stats, validators: t.validators });
-
-    // Check stats for the offline validator
-    const offlineValidator = t.validators.at(-1)!.attester.toLowerCase();
-    t.logger.info(`Asserting stats for offline validator ${offlineValidator}`);
-    const offlineStats = stats.stats[offlineValidator];
-    const historyLength = offlineStats.history.length;
-    expect(offlineStats.history.length).toBeGreaterThanOrEqual(blockCount - 1);
-    expect(offlineStats.history.every(h => h.status.endsWith('-missed'))).toBeTrue();
-    expect(offlineStats.missedAttestations.count + offlineStats.missedProposals.count).toEqual(historyLength);
-    expect(offlineStats.missedAttestations.rate).toEqual(1);
-    expect(offlineStats.missedProposals.rate).toBeOneOf([1, NaN]);
-
-    // Check stats for a validator that mined a block
-    const [proposerValidator, proposerStats] = Object.entries(stats.stats).find(([_, v]) =>
-      v?.history?.some(h => h.status === 'block-mined'),
-    )!;
-    t.logger.info(`Asserting stats for proposer validator ${proposerValidator}`);
-    expect(t.validators.map(v => v.attester.toLowerCase())).toContain(proposerValidator);
-    expect(proposerStats.history.length).toBeGreaterThanOrEqual(blockCount - 1);
-    expect(proposerStats.missedProposals.rate).toBeLessThan(1);
-
-    // Check stats for a validator that attested to a block
-    const [attestorValidator, attestorStats] = Object.entries(stats.stats).find(([_, v]) =>
-      v?.history?.some(h => h.status === 'attestation-sent'),
-    )!;
-    t.logger.info(`Asserting stats for attestor validator ${attestorValidator}`);
-    expect(t.validators.map(v => v.attester.toLowerCase())).toContain(attestorValidator);
-    expect(attestorStats.history.length).toBeGreaterThanOrEqual(blockCount - 1);
-    expect(attestorStats.missedAttestations.rate).toBeLessThan(1);
+    it('collects stats on a block builder', () => {
+      const [proposerValidator, proposerStats] = Object.entries(stats.stats).find(([_, v]) =>
+        v?.history?.some(h => h.status === 'block-mined'),
+      )!;
+      t.logger.info(`Asserting stats for proposer validator ${proposerValidator}`);
+      expect(proposerStats).toBeDefined();
+      expect(t.validators.map(v => v.attester.toLowerCase())).toContain(proposerValidator);
+      expect(proposerStats.history.length).toBeGreaterThanOrEqual(BLOCK_COUNT - 1);
+      expect(proposerStats.missedProposals.rate).toBeLessThan(1);
+    });
+
+    it('collects stats on an attestor', () => {
+      const [attestorValidator, attestorStats] = Object.entries(stats.stats).find(([_, v]) =>
+        v?.history?.some(h => h.status === 'attestation-sent'),
+      )!;
+      t.logger.info(`Asserting stats for attestor validator ${attestorValidator}`);
+      expect(attestorStats).toBeDefined();
+      expect(t.validators.map(v => v.attester.toLowerCase())).toContain(attestorValidator);
+      expect(attestorStats.history.length).toBeGreaterThanOrEqual(BLOCK_COUNT - 1);
+      expect(attestorStats.missedAttestations.rate).toBeLessThan(1);
+    });
+
+    // Regression test for #13142
+    it('starts a sentinel on a fresh node', async () => {
+      const l2BlockNumber = t.monitor.l2BlockNumber;
+      const nodeIndex = NUM_NODES + 1;
+      const newNode = await createNode(
+        t.ctx.aztecNodeConfig,
+        t.ctx.dateProvider,
+        BOOT_NODE_UDP_PORT + nodeIndex + 1,
+        t.bootstrapNodeEnr!,
+        nodeIndex,
+        t.prefilledPublicData,
+        `${DATA_DIR}-i`,
+      );
+
+      t.logger.info(`Waiting for a few more blocks to be mined`);
+      const timeout = SHORTENED_BLOCK_TIME_CONFIG.aztecSlotDuration * 4 * 8;
+      await retryUntil(() => t.monitor.l2BlockNumber > l2BlockNumber + 3, 'more blocks mined', timeout);
+      await sleep(1000);
+
+      const stats = await newNode.getValidatorsStats();
+      t.logger.info(`Collected validator stats from new node at block ${t.monitor.l2BlockNumber}`, { stats });
+      const newNodeValidator = t.validators.at(-1)!.attester.toLowerCase();
+      expect(stats.stats[newNodeValidator]).toBeDefined();
+      expect(stats.stats[newNodeValidator].history.length).toBeGreaterThanOrEqual(1);
+      expect(Object.keys(stats.stats).length).toBeGreaterThan(1);
+    });
   });
 });

yarn-project/kv-store/src/stores/l2_tips_memory_store.ts

@@ -48,19 +48,26 @@ export class L2TipsMemoryStore implements L2BlockStreamEventHandler, L2BlockStre
         break;
       }
       case 'chain-pruned':
-        this.l2TipsStore.set('latest', event.blockNumber);
+        this.saveTag('latest', event.block);
         break;
       case 'chain-proven':
-        this.l2TipsStore.set('proven', event.blockNumber);
+        this.saveTag('proven', event.block);
         break;
       case 'chain-finalized':
-        this.l2TipsStore.set('finalized', event.blockNumber);
+        this.saveTag('finalized', event.block);
         for (const key of this.l2BlockHashesStore.keys()) {
-          if (key < event.blockNumber) {
+          if (key < event.block.number) {
             this.l2BlockHashesStore.delete(key);
           }
         }
         break;
     }
   }
+
+  private saveTag(name: L2BlockTag, block: L2BlockId) {
+    this.l2TipsStore.set(name, block.number);
+    if (block.hash) {
+      this.l2BlockHashesStore.set(block.number, block.hash);
+    }
+  }
 }

yarn-project/kv-store/src/stores/l2_tips_store.ts

@@ -56,17 +56,24 @@ export class L2TipsKVStore implements L2BlockStreamEventHandler, L2BlockStreamLo
         break;
       }
       case 'chain-pruned':
-        await this.l2TipsStore.set('latest', event.blockNumber);
+        await this.saveTag('latest', event.block);
         break;
       case 'chain-proven':
-        await this.l2TipsStore.set('proven', event.blockNumber);
+        await this.saveTag('proven', event.block);
         break;
       case 'chain-finalized':
-        await this.l2TipsStore.set('finalized', event.blockNumber);
-        for await (const key of this.l2BlockHashesStore.keysAsync({ end: event.blockNumber })) {
+        await this.saveTag('finalized', event.block);
+        for await (const key of this.l2BlockHashesStore.keysAsync({ end: event.block.number })) {
           await this.l2BlockHashesStore.delete(key);
         }
         break;
     }
   }
+
+  private async saveTag(name: L2BlockTag, block: L2BlockId) {
+    await this.l2TipsStore.set(name, block.number);
+    if (block.hash) {
+      await this.l2BlockHashesStore.set(block.number, block.hash);
+    }
+  }
 }

yarn-project/kv-store/src/stores/l2_tips_store_suite.test.ts

@@ -1,6 +1,6 @@
 import { times } from '@aztec/foundation/collection';
 import { Fr } from '@aztec/foundation/fields';
-import type { L2Block, PublishedL2Block } from '@aztec/stdlib/block';
+import type { L2Block, L2BlockId, PublishedL2Block } from '@aztec/stdlib/block';
 import type { BlockHeader } from '@aztec/stdlib/tx';
 
 import { expect } from 'chai';
@@ -20,6 +20,11 @@ export function testL2TipsStore(makeTipsStore: () => Promise<L2TipsStore>) {
     signatures: [],
   });
 
+  const makeBlockId = (number: number): L2BlockId => ({
+    number,
+    hash: new Fr(number).toString(),
+  });
+
   const makeTip = (number: number) => ({ number, hash: number === 0 ? undefined : new Fr(number).toString() });
 
   const makeTips = (latest: number, proven: number, finalized: number) => ({
@@ -36,9 +41,9 @@ export function testL2TipsStore(makeTipsStore: () => Promise<L2TipsStore>) {
   it('stores chain tips', async () => {
     await tipsStore.handleBlockStreamEvent({ type: 'blocks-added', blocks: times(20, i => makeBlock(i + 1)) });
 
-    await tipsStore.handleBlockStreamEvent({ type: 'chain-finalized', blockNumber: 5 });
-    await tipsStore.handleBlockStreamEvent({ type: 'chain-proven', blockNumber: 8 });
-    await tipsStore.handleBlockStreamEvent({ type: 'chain-pruned', blockNumber: 10 });
+    await tipsStore.handleBlockStreamEvent({ type: 'chain-finalized', block: makeBlockId(5) });
+    await tipsStore.handleBlockStreamEvent({ type: 'chain-proven', block: makeBlockId(8) });
+    await tipsStore.handleBlockStreamEvent({ type: 'chain-pruned', block: makeBlockId(10) });
 
     const tips = await tipsStore.getL2Tips();
     expect(tips).to.deep.equal(makeTips(10, 8, 5));
@@ -57,8 +62,8 @@ export function testL2TipsStore(makeTipsStore: () => Promise<L2TipsStore>) {
 
   it('clears block hashes when setting finalized chain', async () => {
     await tipsStore.handleBlockStreamEvent({ type: 'blocks-added', blocks: times(5, i => makeBlock(i + 1)) });
-    await tipsStore.handleBlockStreamEvent({ type: 'chain-proven', blockNumber: 3 });
-    await tipsStore.handleBlockStreamEvent({ type: 'chain-finalized', blockNumber: 3 });
+    await tipsStore.handleBlockStreamEvent({ type: 'chain-proven', block: makeBlockId(3) });
+    await tipsStore.handleBlockStreamEvent({ type: 'chain-finalized', block: makeBlockId(3) });
 
     const tips = await tipsStore.getL2Tips();
     expect(tips).to.deep.equal(makeTips(5, 3, 3));
@@ -70,4 +75,13 @@ export function testL2TipsStore(makeTipsStore: () => Promise<L2TipsStore>) {
     expect(await tipsStore.getL2BlockHash(4)).to.deep.equal(new Fr(4).toString());
     expect(await tipsStore.getL2BlockHash(5)).to.deep.equal(new Fr(5).toString());
   });
+
+  // Regression test for #13142
+  it('does not blow up when setting proven chain on an unseen block number', async () => {
+    await tipsStore.handleBlockStreamEvent({ type: 'blocks-added', blocks: [makeBlock(5)] });
+    await tipsStore.handleBlockStreamEvent({ type: 'chain-proven', block: makeBlockId(3) });
+
+    const tips = await tipsStore.getL2Tips();
+    expect(tips).to.deep.equal(makeTips(5, 3, 0));
+  });
 }

yarn-project/p2p/src/client/p2p_client.ts

@@ -295,12 +295,12 @@ export class P2PClient<T extends P2PClientType = P2PClientType.Full>
         break;
       case 'chain-proven': {
         const from = (await this.getSyncedProvenBlockNum()) + 1;
-        const limit = event.blockNumber - from + 1;
+        const limit = event.block.number - from + 1;
         await this.handleProvenL2Blocks(await this.l2BlockSource.getBlocks(from, limit));
         break;
       }
       case 'chain-pruned':
-        await this.handlePruneL2Blocks(event.blockNumber);
+        await this.handlePruneL2Blocks(event.block.number);
         break;
       default: {
         const _: never = event;

yarn-project/pxe/src/synchronizer/synchronizer.test.ts

@@ -64,7 +64,7 @@ describe('Synchronizer', () => {
       type: 'blocks-added',
       blocks: await timesParallel(5, randomPublishedL2Block),
     });
-    await synchronizer.handleBlockStreamEvent({ type: 'chain-pruned', blockNumber: 3 });
+    await synchronizer.handleBlockStreamEvent({ type: 'chain-pruned', block: { number: 3, hash: '0x3' } });
 
     expect(removeNotesAfter).toHaveBeenCalledWith(3);
     expect(unnullifyNotesAfter).toHaveBeenCalledWith(3, 4);

yarn-project/pxe/src/synchronizer/synchronizer.ts

@@ -59,18 +59,18 @@ export class Synchronizer implements L2BlockStreamEventHandler {
         break;
       }
       case 'chain-pruned': {
-        this.log.warn(`Pruning data after block ${event.blockNumber} due to reorg`);
+        this.log.warn(`Pruning data after block ${event.block.number} due to reorg`);
         // We first unnullify and then remove so that unnullified notes that were created after the block number end up deleted.
         const lastSynchedBlockNumber = await this.syncDataProvider.getBlockNumber();
-        await this.noteDataProvider.unnullifyNotesAfter(event.blockNumber, lastSynchedBlockNumber);
-        await this.noteDataProvider.removeNotesAfter(event.blockNumber);
+        await this.noteDataProvider.unnullifyNotesAfter(event.block.number, lastSynchedBlockNumber);
+        await this.noteDataProvider.removeNotesAfter(event.block.number);
         // Remove all note tagging indexes to force a full resync. This is suboptimal, but unless we track the
         // block number in which each index is used it's all we can do.
         await this.taggingDataProvider.resetNoteSyncData();
         // Update the header to the last block.
-        const newHeader = await this.node.getBlockHeader(event.blockNumber);
+        const newHeader = await this.node.getBlockHeader(event.block.number);
         if (!newHeader) {
-          this.log.error(`Block header not found for block number ${event.blockNumber} during chain prune`);
+          this.log.error(`Block header not found for block number ${event.block.number} during chain prune`);
         } else {
           await this.syncDataProvider.setHeader(newHeader);
         }