diff --git a/yarn-project/p2p/src/client/p2p_client.test.ts b/yarn-project/p2p/src/client/p2p_client.test.ts index 18707f11f101..98ff2f85d7fa 100644 --- a/yarn-project/p2p/src/client/p2p_client.test.ts +++ b/yarn-project/p2p/src/client/p2p_client.test.ts @@ -210,14 +210,14 @@ describe('P2P Client', () => { }); describe('Chain prunes', () => { - it('passes deleteAllTxs: false when prune does not cross a checkpoint boundary', async () => { + it('detects checkpoint prune when checkpoint number stays the same', async () => { client = createClient({ txPoolDeleteTxsAfterReorg: true }); blockSource.setProvenBlockNumber(0); // Only checkpoint up to block 90 — blocks 91-100 are proposed but not checkpointed blockSource.setCheckpointedBlockNumber(90); await client.start(); - // Prune 5 blocks (91-100): checkpointed tip stays at checkpoint 90 + // Prune 10 blocks (91-100): checkpoint number stays at 90, so this is a checkpoint prune blockSource.removeBlocks(10); await client.sync(); @@ -228,36 +228,77 @@ describe('P2P Client', () => { await client.stop(); }); - it('passes deleteAllTxs: true when prune crosses a checkpoint boundary', async () => { + it('detects checkpoint prune when checkpoint number increases by one', async () => { client = createClient({ txPoolDeleteTxsAfterReorg: true }); blockSource.setProvenBlockNumber(0); - // Checkpoint all 100 blocks blockSource.setCheckpointedBlockNumber(100); await client.start(); - // Prune 5 blocks (96-100): checkpointed tip moves from checkpoint 100 to 95 - blockSource.removeBlocks(5); + // Propose block 101 and sync it + blockSource.addProposedBlocks([await L2Block.random(BlockNumber(101))]); + await client.sync(); + + // Replace block 101 with a different block and checkpoint it + blockSource.removeBlocks(1); + blockSource.addProposedBlocks([await L2Block.random(BlockNumber(101))]); + blockSource.setCheckpointedBlockNumber(101); await client.sync(); + // Checkpoint advanced from 100 to 101: not an epoch prune expect(txPool.handlePrunedBlocks).toHaveBeenCalledWith( - { number: BlockNumber(95), hash: expect.any(String) }, + { number: BlockNumber(100), hash: expect.any(String) }, + { deleteAllTxs: false }, + ); + await client.stop(); + }); + + it('detects checkpoint prune when checkpoint number decreases by one', async () => { + client = createClient({ txPoolDeleteTxsAfterReorg: true }); + blockSource.setProvenBlockNumber(0); + // Checkpoint all 100 blocks — client stores checkpoint number 100 + blockSource.setCheckpointedBlockNumber(100); + await client.start(); + + // Prune 1 block: checkpoint number drops by 1, so checkpoint prune + blockSource.removeBlocks(1); + await client.sync(); + + expect(txPool.handlePrunedBlocks).toHaveBeenCalledWith( + { number: BlockNumber(99), hash: expect.any(String) }, + { deleteAllTxs: false }, + ); + await client.stop(); + }); + + it('detects epoch prune when checkpoint number decreases by more than 1', async () => { + client = createClient({ txPoolDeleteTxsAfterReorg: true }); + blockSource.setProvenBlockNumber(0); + // Checkpoint all 100 blocks — client stores checkpoint number 100 + blockSource.setCheckpointedBlockNumber(100); + await client.start(); + + // Prune 2 blocks (99-100): checkpoint number drops from 100 to 98, so this is an epoch prune + blockSource.removeBlocks(2); + await client.sync(); + + expect(txPool.handlePrunedBlocks).toHaveBeenCalledWith( + { number: BlockNumber(98), hash: expect.any(String) }, { deleteAllTxs: true }, ); await client.stop(); }); - it('passes deleteAllTxs: false for cross-checkpoint prune when txPoolDeleteTxsAfterReorg is disabled', async () => { + it('does not delete all txs on epoch prune when txPoolDeleteTxsAfterReorg is disabled', async () => { // Default config has txPoolDeleteTxsAfterReorg: false blockSource.setProvenBlockNumber(0); // Checkpoint all 100 blocks blockSource.setCheckpointedBlockNumber(100); await client.start(); - // Prune 5 blocks (96-100): checkpointed tip moves from checkpoint 100 to 95 + // Prune 5 blocks (96-100): epoch prune but flag is off blockSource.removeBlocks(5); await client.sync(); - // Should delete all txs but flag is off expect(txPool.handlePrunedBlocks).toHaveBeenCalledWith( { number: BlockNumber(95), hash: expect.any(String) }, { deleteAllTxs: false }, diff --git a/yarn-project/p2p/src/client/p2p_client.ts b/yarn-project/p2p/src/client/p2p_client.ts index f53b3b0504b5..b9a49ac95ea8 100644 --- a/yarn-project/p2p/src/client/p2p_client.ts +++ b/yarn-project/p2p/src/client/p2p_client.ts @@ -669,20 +669,27 @@ export class P2PClient extends WithTracer implements P2P { } /** - * Returns true if the prune crossed a checkpoint boundary. - * If the old and new checkpoint numbers are the same, the prune is within a single checkpoint. - * If they differ, the prune spans across checkpoints (epoch prune). + * Returns true if the prune is an epoch prune (new checkpoint number is less than old). + * If the checkpoint number stays the same or increases, the prune is within a checkpoint. */ private async isEpochPrune(newCheckpoint: CheckpointId): Promise { const tips = await this.l2Tips.getL2Tips(); const oldCheckpointNumber = tips.checkpointed.checkpoint.number; - if (oldCheckpointNumber <= CheckpointNumber.ZERO) { + if (oldCheckpointNumber <= CheckpointNumber.INITIAL) { return false; } const newCheckpointNumber = newCheckpoint.number; - const isEpochPrune = oldCheckpointNumber !== newCheckpointNumber; + // We check that the new checkpoint number is less than the old checkpoint number in order to consider it an epoch prune. + // To be more certain that it is an epoch prune, we will check that at least 2 checkpoints were removed. + // This means we should avoid thinking checkpoints removed by L1 re-orgs are epoch prunes + const thresholdForEpochPrune = CheckpointNumber(oldCheckpointNumber - 2); + const isEpochPrune = newCheckpointNumber <= thresholdForEpochPrune; if (isEpochPrune) { - this.log.info(`Detected epoch prune to ${newCheckpointNumber}`, { oldCheckpointNumber, newCheckpointNumber }); + this.log.info(`Detected epoch prune to ${newCheckpointNumber}`, { + oldCheckpointNumber, + newCheckpointNumber, + thresholdForEpochPrune, + }); } return isEpochPrune; }