diff --git a/yarn-project/archiver/src/modules/data_store_updater.test.ts b/yarn-project/archiver/src/modules/data_store_updater.test.ts index 4a2c8f659e3d..ff086c1c6a27 100644 --- a/yarn-project/archiver/src/modules/data_store_updater.test.ts +++ b/yarn-project/archiver/src/modules/data_store_updater.test.ts @@ -341,6 +341,63 @@ describe('ArchiverDataStoreUpdater', () => { expect(await store.contractClasses.getContractClass(contractClassId)).toBeUndefined(); expect(await store.contractInstances.getContractInstance(instanceAddress, timestamp)).toBeUndefined(); }); + + it('accepts a re-included already-stored checkpoint carrying contract data (A-1350)', async () => { + const block = await L2Block.random(BlockNumber(1), { + checkpointNumber: CheckpointNumber(1), + indexWithinCheckpoint: IndexWithinCheckpoint(0), + }); + block.body.txEffects[0].contractClassLogs = [contractClassLog]; + block.body.txEffects[0].privateLogs = [PrivateLog.fromBuffer(getSampleContractInstancePublishedEventPayload())]; + + const checkpoint = makeCheckpoint([block]); + await updater.addCheckpoints([makePublishedCheckpoint(checkpoint, 10)]); + expect(await store.contractClasses.getContractClass(contractClassId)).toBeDefined(); + + // Simulate an L1 reorg that re-includes the same checkpoint at a later L1 block. + await expect(updater.addCheckpoints([makePublishedCheckpoint(checkpoint, 999)])).resolves.toBeDefined(); + + // L1 metadata must reflect the re-inclusion and contract data must still be present. + const stored = await store.blocks.getCheckpointData(CheckpointNumber(1)); + expect(stored?.l1.blockNumber).toBe(999n); + expect(await store.contractClasses.getContractClass(contractClassId)).toBeDefined(); + const timestamp = block.header.globalVariables.timestamp + 1n; + expect(await store.contractInstances.getContractInstance(instanceAddress, timestamp)).toBeDefined(); + }); + + it('extracts only the newly-inserted suffix when a re-included checkpoint is batched with a new one (A-1350)', async () => { + // Checkpoint 1 (block 1) carries the contract class log; checkpoint 2 (block 2) carries the + // contract instance log. Ingest checkpoint 1, then re-present it (at a new L1 block) batched with + // the brand-new checkpoint 2. Only checkpoint 2's block is new, so its instance must be extracted + // while re-extracting checkpoint 1's already-stored class is skipped rather than throwing. + const block1 = await L2Block.random(BlockNumber(1), { + checkpointNumber: CheckpointNumber(1), + indexWithinCheckpoint: IndexWithinCheckpoint(0), + }); + block1.body.txEffects[0].contractClassLogs = [contractClassLog]; + + const checkpoint1 = makeCheckpoint([block1]); + await updater.addCheckpoints([makePublishedCheckpoint(checkpoint1, 10)]); + expect(await store.contractClasses.getContractClass(contractClassId)).toBeDefined(); + + const block2 = await L2Block.random(BlockNumber(2), { + checkpointNumber: CheckpointNumber(2), + indexWithinCheckpoint: IndexWithinCheckpoint(0), + lastArchive: block1.archive, + }); + block2.body.txEffects[0].privateLogs = [PrivateLog.fromBuffer(getSampleContractInstancePublishedEventPayload())]; + const checkpoint2 = makeCheckpoint([block2], CheckpointNumber(2)); + + // Re-present checkpoint 1 at a new L1 block, batched with the new checkpoint 2. + await expect( + updater.addCheckpoints([makePublishedCheckpoint(checkpoint1, 999), makePublishedCheckpoint(checkpoint2, 20)]), + ).resolves.toBeDefined(); + + // Checkpoint 1's class stays stored (not re-extracted), and checkpoint 2's instance was extracted. + expect(await store.contractClasses.getContractClass(contractClassId)).toBeDefined(); + const timestamp = block2.header.globalVariables.timestamp + 1n; + expect(await store.contractInstances.getContractInstance(instanceAddress, timestamp)).toBeDefined(); + }); }); describe('logs handling', () => { diff --git a/yarn-project/archiver/src/modules/data_store_updater.ts b/yarn-project/archiver/src/modules/data_store_updater.ts index bf87942d24dc..8ab9997c91d6 100644 --- a/yarn-project/archiver/src/modules/data_store_updater.ts +++ b/yarn-project/archiver/src/modules/data_store_updater.ts @@ -123,10 +123,11 @@ export class ArchiverDataStoreUpdater { // Before adding checkpoints, check for conflicts with local blocks if any const { prunedBlocks, lastAlreadyInsertedBlockNumber } = await this.pruneMismatchingLocalBlocks(checkpoints); - await this.stores.blocks.addCheckpoints(checkpoints); + const insertedCheckpoints = await this.stores.blocks.addCheckpoints(checkpoints); - // Filter out blocks that were already inserted via addProposedBlock() to avoid duplicating logs/contract data - const newBlocks = checkpoints + // Skip blocks already inserted via addProposedBlock() and blocks of already-stored checkpoints + // re-included by an L1 reorg: their logs/contract data were extracted when first inserted. + const newBlocks = insertedCheckpoints .flatMap((ch: PublishedCheckpoint) => ch.checkpoint.blocks) .filter(b => lastAlreadyInsertedBlockNumber === undefined || b.number > lastAlreadyInsertedBlockNumber); diff --git a/yarn-project/archiver/src/store/block_store.test.ts b/yarn-project/archiver/src/store/block_store.test.ts index 31dbb3fe47a6..39b7efebc43e 100644 --- a/yarn-project/archiver/src/store/block_store.test.ts +++ b/yarn-project/archiver/src/store/block_store.test.ts @@ -98,7 +98,7 @@ describe('BlockStore', () => { describe('addCheckpoints', () => { it('returns success when adding checkpoints', async () => { - await expect(blockStore.addCheckpoints(publishedCheckpoints)).resolves.toBe(true); + await expect(blockStore.addCheckpoints(publishedCheckpoints)).resolves.toEqual(publishedCheckpoints); }); it('accepts duplicate checkpoints with matching archives and updates L1 info', async () => { @@ -118,8 +118,10 @@ describe('BlockStore', () => { makeL1PublishedData(999), first3[2].attestations, ); - // Also add checkpoint 4 (the next one) in the same batch - await blockStore.addCheckpoints([cp3WithNewL1, publishedCheckpoints[3]]); + // Also add checkpoint 4 (the next one) in the same batch; only checkpoint 4 is newly inserted. + await expect(blockStore.addCheckpoints([cp3WithNewL1, publishedCheckpoints[3]])).resolves.toEqual([ + publishedCheckpoints[3], + ]); // Checkpoint 3's L1 info should be updated const afterData = await blockStore.getCheckpointData(CheckpointNumber(3)); @@ -135,8 +137,8 @@ describe('BlockStore', () => { const first3 = publishedCheckpoints.slice(0, 3); await blockStore.addCheckpoints(first3); - // Re-add the same 3 checkpoints — should succeed without error - await expect(blockStore.addCheckpoints(first3)).resolves.toBe(true); + // Re-add the same 3 checkpoints — should succeed without inserting anything new + await expect(blockStore.addCheckpoints(first3)).resolves.toEqual([]); }); it('throws on duplicate checkpoints with mismatching archives', async () => { @@ -284,7 +286,7 @@ describe('BlockStore', () => { ); const publishedCheckpoint = makePublishedCheckpoint(checkpoint, 10); - await expect(blockStore.addCheckpoints([publishedCheckpoint])).resolves.toBe(true); + await expect(blockStore.addCheckpoints([publishedCheckpoint])).resolves.toEqual([publishedCheckpoint]); }); it('throws on duplicate checkpoint with different content', async () => { @@ -314,7 +316,7 @@ describe('BlockStore', () => { ); const publishedCheckpoint2 = makePublishedCheckpoint(checkpoint2, 10); - await expect(blockStore.addCheckpoints([publishedCheckpoint])).resolves.toBe(true); + await expect(blockStore.addCheckpoints([publishedCheckpoint])).resolves.toEqual([publishedCheckpoint]); await expect(blockStore.addCheckpoints([publishedCheckpoint2])).rejects.toThrow( 'already exists in store but with a different archive', ); @@ -352,7 +354,7 @@ describe('BlockStore', () => { blocksPerCheckpoint: 2, }); - await expect(blockStore.addCheckpoints(checkpoints)).resolves.toBe(true); + await expect(blockStore.addCheckpoints(checkpoints)).resolves.toEqual(checkpoints); // Verify blocks have correct checkpoint assignments const block1 = await blockStore.getBlock({ number: BlockNumber(1) }); @@ -2081,7 +2083,7 @@ describe('BlockStore', () => { const publishedCheckpoint2 = makePublishedCheckpoint(checkpoint2, 10); // This should NOT throw - addCheckpoints uses .set() which is idempotent - await expect(blockStore.addCheckpoints([publishedCheckpoint2])).resolves.toBe(true); + await expect(blockStore.addCheckpoints([publishedCheckpoint2])).resolves.toEqual([publishedCheckpoint2]); // Verify block exists and is consistent const storedBlock = await blockStore.getBlock({ number: BlockNumber(2) }); diff --git a/yarn-project/archiver/src/store/block_store.ts b/yarn-project/archiver/src/store/block_store.ts index c1615ef601e5..37d7ec5b5ce2 100644 --- a/yarn-project/archiver/src/store/block_store.ts +++ b/yarn-project/archiver/src/store/block_store.ts @@ -306,12 +306,18 @@ export class BlockStore { /** * Append new checkpoints to the store's list. + * Checkpoints at the start of the batch that are already stored (e.g. re-included by an L1 reorg) + * are accepted if their archive root matches: their L1 metadata is updated but they are not + * re-inserted, and they are excluded from the returned array. * @param checkpoints - The L2 checkpoints to be added to the store. - * @returns True if the operation is successful. + * @returns The checkpoints that were actually inserted (excluding already-stored ones). */ - async addCheckpoints(checkpoints: PublishedCheckpoint[], opts: { force?: boolean } = {}): Promise { + async addCheckpoints( + checkpoints: PublishedCheckpoint[], + opts: { force?: boolean } = {}, + ): Promise { if (checkpoints.length === 0) { - return true; + return []; } return await this.db.transactionAsync(async () => { @@ -324,7 +330,7 @@ export class BlockStore { if (!opts.force && firstCheckpointNumber <= previousCheckpointNumber) { checkpoints = await this.skipOrUpdateAlreadyStoredCheckpoints(checkpoints, previousCheckpointNumber); if (checkpoints.length === 0) { - return true; + return []; } // Re-check sequentiality after skipping const newFirstNumber = checkpoints[0].checkpoint.number; @@ -387,7 +393,7 @@ export class BlockStore { } await this.advanceSynchedL1BlockNumber(checkpoints[checkpoints.length - 1].l1.blockNumber); - return true; + return checkpoints; }); } diff --git a/yarn-project/archiver/src/store/contract_class_store.test.ts b/yarn-project/archiver/src/store/contract_class_store.test.ts index 375bd3bb7320..4a7a9b3661e0 100644 --- a/yarn-project/archiver/src/store/contract_class_store.test.ts +++ b/yarn-project/archiver/src/store/contract_class_store.test.ts @@ -44,12 +44,22 @@ describe('ContractClassStore', () => { await expect(contractClassStore.getContractClass(contractClass.id)).resolves.toBeUndefined(); }); - it('throws if the same contract class is added again', async () => { + it('throws if the same contract class is added again at a different block', async () => { await expect( contractClassStore.addContractClasses([await withCommitment(contractClass)], BlockNumber(blockNum + 1)), ).rejects.toThrow(/already exists/); }); + it('treats re-adding the same contract class at the same block as a no-op (A-1350)', async () => { + // An L1 reorg can re-present an already-stored checkpoint, replaying this class at the same block. + const originalCommitment = await computePublicBytecodeCommitment(contractClass.packedBytecode); + await expect( + contractClassStore.addContractClasses([await withCommitment(contractClass)], BlockNumber(blockNum)), + ).resolves.not.toThrow(); + await expect(contractClassStore.getContractClass(contractClass.id)).resolves.toMatchObject(contractClass); + await expect(contractClassStore.getBytecodeCommitment(contractClass.id)).resolves.toEqual(originalCommitment); + }); + it('returns contract class if deleted at a later block number', async () => { await contractClassStore.deleteContractClasses([contractClass], BlockNumber(blockNum + 1)); await expect(contractClassStore.getContractClass(contractClass.id)).resolves.toMatchObject(contractClass); diff --git a/yarn-project/archiver/src/store/contract_class_store.ts b/yarn-project/archiver/src/store/contract_class_store.ts index 09e27c73c5fd..b6558c003f70 100644 --- a/yarn-project/archiver/src/store/contract_class_store.ts +++ b/yarn-project/archiver/src/store/contract_class_store.ts @@ -50,13 +50,19 @@ export class ContractClassStore { ): Promise { await this.db.transactionAsync(async () => { const key = contractClass.id.toString(); - if (await this.#contractClasses.hasAsync(key)) { + const existing = await this.#contractClasses.getAsync(key); + if (existing !== undefined) { // Protocol contracts are preloaded at block 0, so a later on-chain (re-)publish of a bundled // protocol class id is valid and must be a no-op. Keep the existing block-0 entry untouched // (do not bump its block number) so it survives reorgs of the publishing block. if (isProtocolContractClass(contractClass.id)) { return; } + // An L1 reorg can re-present an already-stored checkpoint, replaying this class at the same + // block; treat that as a no-op. A duplicate at a different block still signals double-processing. + if (deserializeContractClassPublic(existing).l2BlockNumber === blockNumber) { + return; + } throw new Error(`Contract class ${key} already exists, cannot add again at block ${blockNumber}`); } await this.#contractClasses.set( diff --git a/yarn-project/archiver/src/store/contract_instance_store.test.ts b/yarn-project/archiver/src/store/contract_instance_store.test.ts index 2cd7490945c6..17dd14ddd18b 100644 --- a/yarn-project/archiver/src/store/contract_instance_store.test.ts +++ b/yarn-project/archiver/src/store/contract_instance_store.test.ts @@ -49,11 +49,24 @@ describe('ContractInstanceStore', () => { ).resolves.toBeUndefined(); }); - it('throws when adding the same contract instance twice', async () => { + it('throws when adding the same contract instance again at a different block', async () => { await expect(contractInstanceStore.addContractInstances([contractInstance], BlockNumber(2))).rejects.toThrow( /already exists/, ); }); + + it('treats re-adding the same contract instance at the same block as a no-op (A-1350)', async () => { + // An L1 reorg can re-present an already-stored checkpoint, replaying this instance at the same block. + await expect( + contractInstanceStore.addContractInstances([contractInstance], BlockNumber(blockNum)), + ).resolves.not.toThrow(); + await expect( + contractInstanceStore.getContractInstance(contractInstance.address, timestamp), + ).resolves.toMatchObject(contractInstance); + await expect( + contractInstanceStore.getContractInstanceDeploymentBlockNumber(contractInstance.address), + ).resolves.toEqual(blockNum); + }); }); describe('protocol contract instances (A-1257)', () => { diff --git a/yarn-project/archiver/src/store/contract_instance_store.ts b/yarn-project/archiver/src/store/contract_instance_store.ts index 3c7da25f9b9d..dd58039cb978 100644 --- a/yarn-project/archiver/src/store/contract_instance_store.ts +++ b/yarn-project/archiver/src/store/contract_instance_store.ts @@ -78,8 +78,14 @@ export class ContractInstanceStore { if (isProtocolContract(contractInstance.address)) { return; } + const existingBlockNumber = await this.#contractInstancePublishedAt.getAsync(key); + // An L1 reorg can re-present an already-stored checkpoint, replaying this instance at the same + // block; treat that as a no-op. A duplicate at a different block still signals double-processing. + if (existingBlockNumber === blockNumber) { + return; + } throw new Error( - `Contract instance at ${key} already exists (deployed at block ${await this.#contractInstancePublishedAt.getAsync(key)}), cannot add again at block ${blockNumber}`, + `Contract instance at ${key} already exists (deployed at block ${existingBlockNumber}), cannot add again at block ${blockNumber}`, ); } await this.#contractInstances.set(key, new SerializableContractInstance(contractInstance).toBuffer());