From 5622b506513f7f1fb491a6be011f90eca1ea96f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Bene=C5=A1?= Date: Thu, 12 Oct 2023 14:39:02 +0200 Subject: [PATCH] refactor: from < genesis allowed in getBlocks (#2816) Fixes #411 + renamed getL2Blocks in archiver as getBlocks to make it consistent with the naming in `AztecNode` interface. + removed the `getBlocksLength` function from archiver interface. It's redundant because `getBlockNumber` can be used instead. --- .../archiver/src/archiver/archiver.ts | 57 +++++++++---------- .../src/archiver/archiver_store.test.ts | 30 ++++++---- .../archiver/src/archiver/archiver_store.ts | 37 +++++------- .../aztec-node/src/aztec-node/server.ts | 6 +- yarn-project/p2p/src/client/mocks.ts | 4 +- yarn-project/p2p/src/client/p2p_client.ts | 2 +- .../src/sequencer/sequencer.ts | 2 +- .../l2_block_downloader.ts | 4 +- yarn-project/types/src/l2_block_source.ts | 4 +- .../server_world_state_synchronizer.test.ts | 2 +- .../server_world_state_synchronizer.ts | 2 +- 11 files changed, 73 insertions(+), 77 deletions(-) diff --git a/yarn-project/archiver/src/archiver/archiver.ts b/yarn-project/archiver/src/archiver/archiver.ts index 82087dc9fa9..9fd05cd1830 100644 --- a/yarn-project/archiver/src/archiver/archiver.ts +++ b/yarn-project/archiver/src/archiver/archiver.ts @@ -11,7 +11,6 @@ import { EncodedContractFunction, ExtendedContractData, GetUnencryptedLogsResponse, - INITIAL_L2_BLOCK_NUM, L1ToL2Message, L1ToL2MessageSource, L2Block, @@ -50,17 +49,17 @@ export class Archiver implements L2BlockSource, L2LogsSource, ContractDataSource /** * Next L1 block number to fetch `L2BlockProcessed` logs from (i.e. `fromBlock` in eth_getLogs). */ - private nextL2BlockFromBlock = 0n; + private nextL2BlockFromL1Block = 0n; /** * Last Processed Block Number */ - private lastProcessedBlockNumber = 0n; + private lastProcessedL1BlockNumber = 0n; /** * Use this to track logged block in order to avoid repeating the same message. */ - private lastLoggedBlockNumber = 0n; + private lastLoggedL1BlockNumber = 0n; /** * Creates a new instance of the Archiver. @@ -85,8 +84,8 @@ export class Archiver implements L2BlockSource, L2LogsSource, ContractDataSource private readonly pollingIntervalMs = 10_000, private readonly log: DebugLogger = createDebugLogger('aztec:archiver'), ) { - this.nextL2BlockFromBlock = BigInt(searchStartBlock); - this.lastProcessedBlockNumber = BigInt(searchStartBlock); + this.nextL2BlockFromL1Block = BigInt(searchStartBlock); + this.lastProcessedL1BlockNumber = BigInt(searchStartBlock); } /** @@ -140,12 +139,12 @@ export class Archiver implements L2BlockSource, L2LogsSource, ContractDataSource * @param blockUntilSynced - If true, blocks until the archiver has fully synced. */ private async sync(blockUntilSynced: boolean) { - const currentBlockNumber = await this.publicClient.getBlockNumber(); - if (currentBlockNumber <= this.lastProcessedBlockNumber) { + const currentL1BlockNumber = await this.publicClient.getBlockNumber(); + if (currentL1BlockNumber <= this.lastProcessedL1BlockNumber) { // reducing logs, otherwise this gets triggered on every loop (1s) - if (currentBlockNumber !== this.lastLoggedBlockNumber) { - this.log(`No new blocks to process, current block number: ${currentBlockNumber}`); - this.lastLoggedBlockNumber = currentBlockNumber; + if (currentL1BlockNumber !== this.lastLoggedL1BlockNumber) { + this.log(`No new blocks to process, current block number: ${currentL1BlockNumber}`); + this.lastLoggedL1BlockNumber = currentL1BlockNumber; } return; } @@ -184,15 +183,15 @@ export class Archiver implements L2BlockSource, L2LogsSource, ContractDataSource this.publicClient, this.inboxAddress, blockUntilSynced, - this.lastProcessedBlockNumber + 1n, // + 1 to prevent re-including messages from the last processed block - currentBlockNumber, + this.lastProcessedL1BlockNumber + 1n, // + 1 to prevent re-including messages from the last processed block + currentL1BlockNumber, ); const retrievedCancelledL1ToL2Messages = await retrieveNewCancelledL1ToL2Messages( this.publicClient, this.inboxAddress, blockUntilSynced, - this.lastProcessedBlockNumber + 1n, - currentBlockNumber, + this.lastProcessedL1BlockNumber + 1n, + currentL1BlockNumber, ); // TODO (#717): optimise this - there could be messages in confirmed that are also in pending. @@ -204,21 +203,21 @@ export class Archiver implements L2BlockSource, L2LogsSource, ContractDataSource this.log('Removing pending l1 to l2 messages from store where messages were cancelled'); await this.store.cancelPendingL1ToL2Messages(retrievedCancelledL1ToL2Messages.retrievedData); - this.lastProcessedBlockNumber = currentBlockNumber; + this.lastProcessedL1BlockNumber = currentL1BlockNumber; // ********** Events that are processed per block ********** // Read all data from chain and then write to our stores at the end - const nextExpectedL2BlockNum = BigInt(this.store.getBlocksLength() + INITIAL_L2_BLOCK_NUM); + const nextExpectedL2BlockNum = BigInt((await this.store.getBlockNumber()) + 1); this.log( - `Retrieving chain state from L1 block: ${this.nextL2BlockFromBlock}, next expected l2 block number: ${nextExpectedL2BlockNum}`, + `Retrieving chain state from L1 block: ${this.nextL2BlockFromL1Block}, next expected l2 block number: ${nextExpectedL2BlockNum}`, ); const retrievedBlocks = await retrieveBlocks( this.publicClient, this.rollupAddress, blockUntilSynced, - this.nextL2BlockFromBlock, - currentBlockNumber, + this.nextL2BlockFromL1Block, + currentL1BlockNumber, nextExpectedL2BlockNum, ); @@ -231,8 +230,8 @@ export class Archiver implements L2BlockSource, L2LogsSource, ContractDataSource this.publicClient, this.contractDeploymentEmitterAddress, blockUntilSynced, - this.nextL2BlockFromBlock, - currentBlockNumber, + this.nextL2BlockFromL1Block, + currentL1BlockNumber, blockHashMapping, ); if (retrievedBlocks.retrievedData.length === 0) { @@ -270,14 +269,14 @@ export class Archiver implements L2BlockSource, L2LogsSource, ContractDataSource // store retrieved L2 blocks after removing new logs information. // remove logs to serve "lightweight" block information. Logs can be fetched separately if needed. - await this.store.addL2Blocks( + await this.store.addBlocks( retrievedBlocks.retrievedData.map(block => L2Block.fromFields(omit(block, ['newEncryptedLogs', 'newUnencryptedLogs']), block.getBlockHash()), ), ); // set the L1 block for the next search - this.nextL2BlockFromBlock = retrievedBlocks.nextEthBlockNumber; + this.nextL2BlockFromL1Block = retrievedBlocks.nextEthBlockNumber; } /** @@ -306,8 +305,8 @@ export class Archiver implements L2BlockSource, L2LogsSource, ContractDataSource * @param limit - The number of blocks to return. * @returns The requested L2 blocks. */ - public getL2Blocks(from: number, limit: number): Promise { - return this.store.getL2Blocks(from, limit); + public getBlocks(from: number, limit: number): Promise { + return this.store.getBlocks(from, limit); } /** @@ -315,12 +314,12 @@ export class Archiver implements L2BlockSource, L2LogsSource, ContractDataSource * @param number - The block number to return (inclusive). * @returns The requested L2 block. */ - public async getL2Block(number: number): Promise { + public async getBlock(number: number): Promise { // If the number provided is -ve, then return the latest block. if (number < 0) { - number = this.store.getBlocksLength(); + number = await this.store.getBlockNumber(); } - const blocks = await this.store.getL2Blocks(number, 1); + const blocks = await this.store.getBlocks(number, 1); return blocks.length === 0 ? undefined : blocks[0]; } diff --git a/yarn-project/archiver/src/archiver/archiver_store.test.ts b/yarn-project/archiver/src/archiver/archiver_store.test.ts index d97a813650f..f4e6aae5a13 100644 --- a/yarn-project/archiver/src/archiver/archiver_store.test.ts +++ b/yarn-project/archiver/src/archiver/archiver_store.test.ts @@ -24,7 +24,7 @@ describe('Archiver Memory Store', () => { const blocks = Array(10) .fill(0) .map((_, index) => L2Block.random(index)); - await archiverStore.addL2Blocks(blocks); + await archiverStore.addBlocks(blocks); // Offset indices by INITIAL_L2_BLOCK_NUM to ensure we are correctly aligned for (const [from, limit] of [ [0 + INITIAL_L2_BLOCK_NUM, 10], @@ -35,7 +35,7 @@ describe('Archiver Memory Store', () => { [11 + INITIAL_L2_BLOCK_NUM, 1], ]) { const expected = blocks.slice(from - INITIAL_L2_BLOCK_NUM, from - INITIAL_L2_BLOCK_NUM + limit); - const actual = await archiverStore.getL2Blocks(from, limit); + const actual = await archiverStore.getBlocks(from, limit); expect(expected).toEqual(actual); } }); @@ -64,10 +64,20 @@ describe('Archiver Memory Store', () => { const blocks = Array(10) .fill(0) .map((_, index) => L2Block.random(index)); - await archiverStore.addL2Blocks(blocks); - await expect(async () => await archiverStore.getL2Blocks(1, 0)).rejects.toThrow( - `Invalid block range from: 1, limit: 0`, - ); + await archiverStore.addBlocks(blocks); + await expect(async () => await archiverStore.getBlocks(1, 0)).rejects.toThrow(`Invalid limit: 0`); + }); + + it('returns from the beginning when "from" < genesis block', async () => { + const blocks = Array(10) + .fill(0) + .map((_, index) => L2Block.random(index)); + await archiverStore.addBlocks(blocks); + const from = -5; + const limit = 1; + const retrievedBlocks = await archiverStore.getBlocks(from, limit); + expect(retrievedBlocks.length).toEqual(1); + expect(retrievedBlocks[0]).toEqual(blocks[0]); }); test.each([LogType.ENCRYPTED, LogType.UNENCRYPTED])( @@ -77,9 +87,7 @@ describe('Archiver Memory Store', () => { .fill(0) .map(_ => L2BlockL2Logs.random(6, 3, 2)); await archiverStore.addLogs(logs, logType); - await expect(async () => await archiverStore.getLogs(1, 0, logType)).rejects.toThrow( - `Invalid block range from: 1, limit: 0`, - ); + await expect(async () => await archiverStore.getLogs(1, 0, logType)).rejects.toThrow(`Invalid limit: 0`); }, ); @@ -91,7 +99,7 @@ describe('Archiver Memory Store', () => { .fill(0) .map((_, index: number) => L2Block.random(index + 1, 4, 2, 3, 2, 2)); - await archiverStore.addL2Blocks(blocks); + await archiverStore.addBlocks(blocks); await archiverStore.addLogs( blocks.map(block => block.newUnencryptedLogs!), LogType.UNENCRYPTED, @@ -118,7 +126,7 @@ describe('Archiver Memory Store', () => { L2Block.random(index + 1, txsPerBlock, 2, numPublicFunctionCalls, 2, numUnencryptedLogs), ); - await archiverStore.addL2Blocks(blocks); + await archiverStore.addBlocks(blocks); await archiverStore.addLogs( blocks.map(block => block.newUnencryptedLogs!), LogType.UNENCRYPTED, diff --git a/yarn-project/archiver/src/archiver/archiver_store.ts b/yarn-project/archiver/src/archiver/archiver_store.ts index f24db5057f8..21f1c655ce9 100644 --- a/yarn-project/archiver/src/archiver/archiver_store.ts +++ b/yarn-project/archiver/src/archiver/archiver_store.ts @@ -30,7 +30,7 @@ export interface ArchiverDataStore { * @param blocks - The L2 blocks to be added to the store. * @returns True if the operation is successful. */ - addL2Blocks(blocks: L2Block[]): Promise; + addBlocks(blocks: L2Block[]): Promise; /** * Gets up to `limit` amount of L2 blocks starting from `from`. @@ -38,7 +38,7 @@ export interface ArchiverDataStore { * @param limit - The number of blocks to return. * @returns The requested L2 blocks. */ - getL2Blocks(from: number, limit: number): Promise; + getBlocks(from: number, limit: number): Promise; /** * Gets an l2 tx. @@ -150,12 +150,6 @@ export interface ArchiverDataStore { * @returns The number of the latest L2 block processed. */ getBlockNumber(): Promise; - - /** - * Gets the length of L2 blocks in store. - * @returns The length of L2 Blocks stored. - */ - getBlocksLength(): number; } /** @@ -215,7 +209,7 @@ export class MemoryArchiverStore implements ArchiverDataStore { * @param blocks - The L2 blocks to be added to the store. * @returns True if the operation is successful (always in this implementation). */ - public addL2Blocks(blocks: L2Block[]): Promise { + public addBlocks(blocks: L2Block[]): Promise { this.l2BlockContexts.push(...blocks.map(block => new L2BlockContext(block))); this.l2Txs.push(...blocks.flatMap(b => b.getTxs())); return Promise.resolve(true); @@ -299,18 +293,21 @@ export class MemoryArchiverStore implements ArchiverDataStore { * @param from - Number of the first block to return (inclusive). * @param limit - The number of blocks to return. * @returns The requested L2 blocks. + * @remarks When "from" is smaller than genesis block number, blocks from the beginning are returned. */ - public getL2Blocks(from: number, limit: number): Promise { + public getBlocks(from: number, limit: number): Promise { // Return an empty array if we are outside of range if (limit < 1) { - throw new Error(`Invalid block range from: ${from}, limit: ${limit}`); + throw new Error(`Invalid limit: ${limit}`); } - if (from < INITIAL_L2_BLOCK_NUM || from > this.l2BlockContexts.length) { + + const fromIndex = Math.max(from - INITIAL_L2_BLOCK_NUM, 0); + if (fromIndex >= this.l2BlockContexts.length) { return Promise.resolve([]); } - const startIndex = from - INITIAL_L2_BLOCK_NUM; - const endIndex = startIndex + limit; - return Promise.resolve(this.l2BlockContexts.slice(startIndex, endIndex).map(blockContext => blockContext.block)); + + const toIndex = fromIndex + limit; + return Promise.resolve(this.l2BlockContexts.slice(fromIndex, toIndex).map(blockContext => blockContext.block)); } /** @@ -354,7 +351,7 @@ export class MemoryArchiverStore implements ArchiverDataStore { */ getLogs(from: number, limit: number, logType: LogType): Promise { if (from < INITIAL_L2_BLOCK_NUM || limit < 1) { - throw new Error(`Invalid block range from: ${from}, limit: ${limit}`); + throw new Error(`Invalid limit: ${limit}`); } const logs = logType === LogType.ENCRYPTED ? this.encryptedLogsPerBlock : this.unencryptedLogsPerBlock; if (from > logs.length) { @@ -513,12 +510,4 @@ export class MemoryArchiverStore implements ArchiverDataStore { if (this.l2BlockContexts.length === 0) return Promise.resolve(INITIAL_L2_BLOCK_NUM - 1); return Promise.resolve(this.l2BlockContexts[this.l2BlockContexts.length - 1].block.number); } - - /** - * Gets the length of L2 blocks in store. - * @returns The length of L2 Blocks array. - */ - public getBlocksLength(): number { - return this.l2BlockContexts.length; - } } diff --git a/yarn-project/aztec-node/src/aztec-node/server.ts b/yarn-project/aztec-node/src/aztec-node/server.ts index 08bab77942a..e379e28c8e1 100644 --- a/yarn-project/aztec-node/src/aztec-node/server.ts +++ b/yarn-project/aztec-node/src/aztec-node/server.ts @@ -159,7 +159,7 @@ export class AztecNodeService implements AztecNode { * @returns The blocks requested. */ public async getBlock(number: number): Promise { - return await this.blockSource.getL2Block(number); + return await this.blockSource.getBlock(number); } /** @@ -169,7 +169,7 @@ export class AztecNodeService implements AztecNode { * @returns The blocks requested. */ public async getBlocks(from: number, limit: number): Promise { - return (await this.blockSource.getL2Blocks(from, limit)) ?? []; + return (await this.blockSource.getBlocks(from, limit)) ?? []; } /** @@ -401,7 +401,7 @@ export class AztecNodeService implements AztecNode { this.log.info(`Simulating tx ${await tx.getTxHash()}`); const blockNumber = (await this.blockSource.getBlockNumber()) + 1; const newGlobalVariables = await this.globalVariableBuilder.buildGlobalVariables(new Fr(blockNumber)); - const prevGlobalVariables = (await this.blockSource.getL2Block(-1))?.globalVariables ?? GlobalVariables.empty(); + const prevGlobalVariables = (await this.blockSource.getBlock(-1))?.globalVariables ?? GlobalVariables.empty(); // Instantiate merkle trees so uncommitted updates by this simulation are local to it. // TODO we should be able to remove this after https://github.com/AztecProtocol/aztec-packages/issues/1869 diff --git a/yarn-project/p2p/src/client/mocks.ts b/yarn-project/p2p/src/client/mocks.ts index 7face89993e..6f4014bdd7d 100644 --- a/yarn-project/p2p/src/client/mocks.ts +++ b/yarn-project/p2p/src/client/mocks.ts @@ -45,7 +45,7 @@ export class MockBlockSource implements L2BlockSource { * @param number - The block number to return (inclusive). * @returns The requested L2 block. */ - public getL2Block(number: number) { + public getBlock(number: number) { return Promise.resolve(this.l2Blocks[number]); } @@ -55,7 +55,7 @@ export class MockBlockSource implements L2BlockSource { * @param limit - The maximum number of blocks to return. * @returns The requested mocked L2 blocks. */ - public getL2Blocks(from: number, limit: number) { + public getBlocks(from: number, limit: number) { return Promise.resolve(this.l2Blocks.slice(from, from + limit)); } diff --git a/yarn-project/p2p/src/client/p2p_client.ts b/yarn-project/p2p/src/client/p2p_client.ts index e8e34868a78..faeaa4bb2f3 100644 --- a/yarn-project/p2p/src/client/p2p_client.ts +++ b/yarn-project/p2p/src/client/p2p_client.ts @@ -164,7 +164,7 @@ export class P2PClient implements P2P { // start looking for further blocks const blockProcess = async () => { while (!this.stopping) { - const blocks = await this.blockDownloader.getL2Blocks(); + const blocks = await this.blockDownloader.getBlocks(); await this.handleL2Blocks(blocks); } }; diff --git a/yarn-project/sequencer-client/src/sequencer/sequencer.ts b/yarn-project/sequencer-client/src/sequencer/sequencer.ts index c2014632262..586a19f70e5 100644 --- a/yarn-project/sequencer-client/src/sequencer/sequencer.ts +++ b/yarn-project/sequencer-client/src/sequencer/sequencer.ts @@ -141,7 +141,7 @@ export class Sequencer { this.state = SequencerState.CREATING_BLOCK; const newGlobalVariables = await this.globalsBuilder.buildGlobalVariables(new Fr(blockNumber)); - const prevGlobalVariables = (await this.l2BlockSource.getL2Block(-1))?.globalVariables ?? GlobalVariables.empty(); + const prevGlobalVariables = (await this.l2BlockSource.getBlock(-1))?.globalVariables ?? GlobalVariables.empty(); // Process txs and drop the ones that fail processing // We create a fresh processor each time to reset any cached state (eg storage writes) diff --git a/yarn-project/types/src/l2_block_downloader/l2_block_downloader.ts b/yarn-project/types/src/l2_block_downloader/l2_block_downloader.ts index 5b0f60d6049..1e3d1f1aa13 100644 --- a/yarn-project/types/src/l2_block_downloader/l2_block_downloader.ts +++ b/yarn-project/types/src/l2_block_downloader/l2_block_downloader.ts @@ -60,7 +60,7 @@ export class L2BlockDownloader { private async collectBlocks() { let totalBlocks = 0; while (true) { - const blocks = await this.l2BlockSource.getL2Blocks(this.from, 10); + const blocks = await this.l2BlockSource.getBlocks(this.from, 10); if (!blocks.length) { return totalBlocks; } @@ -87,7 +87,7 @@ export class L2BlockDownloader { * @param timeout - optional timeout value to prevent permanent blocking * @returns The next batch of blocks from the queue. */ - public async getL2Blocks(timeout?: number) { + public async getBlocks(timeout?: number): Promise { try { const blocks = await this.blockQueue.get(timeout); if (!blocks) { diff --git a/yarn-project/types/src/l2_block_source.ts b/yarn-project/types/src/l2_block_source.ts index 5da437eafeb..00de1abd27c 100644 --- a/yarn-project/types/src/l2_block_source.ts +++ b/yarn-project/types/src/l2_block_source.ts @@ -31,7 +31,7 @@ export interface L2BlockSource { * @param number - The block number to return (inclusive). * @returns The requested L2 block. */ - getL2Block(number: number): Promise; + getBlock(number: number): Promise; /** * Gets up to `limit` amount of L2 blocks starting from `from`. @@ -39,7 +39,7 @@ export interface L2BlockSource { * @param limit - The maximum number of blocks to return. * @returns The requested L2 blocks. */ - getL2Blocks(from: number, limit: number): Promise; + getBlocks(from: number, limit: number): Promise; /** * Gets an l2 tx. diff --git a/yarn-project/world-state/src/synchronizer/server_world_state_synchronizer.test.ts b/yarn-project/world-state/src/synchronizer/server_world_state_synchronizer.test.ts index 72e88e42af8..2d750c066f9 100644 --- a/yarn-project/world-state/src/synchronizer/server_world_state_synchronizer.test.ts +++ b/yarn-project/world-state/src/synchronizer/server_world_state_synchronizer.test.ts @@ -118,7 +118,7 @@ const log = createDebugLogger('aztec:server_world_state_synchronizer_test'); describe('server_world_state_synchronizer', () => { const rollupSource = mock({ getBlockNumber: jest.fn(getLatestBlockNumber), - getL2Blocks: jest.fn(consumeNextBlocks), + getBlocks: jest.fn(consumeNextBlocks), }); const merkleTreeDb = mock({ diff --git a/yarn-project/world-state/src/synchronizer/server_world_state_synchronizer.ts b/yarn-project/world-state/src/synchronizer/server_world_state_synchronizer.ts index 2f7e166ff2a..896b85a55de 100644 --- a/yarn-project/world-state/src/synchronizer/server_world_state_synchronizer.ts +++ b/yarn-project/world-state/src/synchronizer/server_world_state_synchronizer.ts @@ -173,7 +173,7 @@ export class ServerWorldStateSynchronizer implements WorldStateSynchronizer { */ private async collectAndProcessBlocks() { // This request for blocks will timeout after 1 second if no blocks are received - const blocks = await this.l2BlockDownloader.getL2Blocks(1); + const blocks = await this.l2BlockDownloader.getBlocks(1); await this.handleL2Blocks(blocks); await this.commitCurrentL2BlockNumber(); }