From 99dd7df808448d7e42ac0f2f8ea7b3c93a2dcc5d Mon Sep 17 00:00:00 2001 From: PhilWindle Date: Thu, 28 Sep 2023 11:11:16 +0000 Subject: [PATCH 1/6] Prevent race conditions around data pulled from L1 --- .../archiver/src/archiver/archiver.test.ts | 71 +++++++++++++++++++ .../archiver/src/archiver/archiver.ts | 35 +++++++-- .../archiver/src/archiver/data_retrieval.ts | 54 ++++++++------ 3 files changed, 133 insertions(+), 27 deletions(-) diff --git a/yarn-project/archiver/src/archiver/archiver.test.ts b/yarn-project/archiver/src/archiver/archiver.test.ts index b3f0db13e55..65dc7fcbe8d 100644 --- a/yarn-project/archiver/src/archiver/archiver.test.ts +++ b/yarn-project/archiver/src/archiver/archiver.test.ts @@ -127,6 +127,77 @@ describe('Archiver', () => { await archiver.stop(); }, 10_000); + + it('does not sync past current block number', async () => { + const numL2BlocksInTest = 2; + const archiver = new Archiver( + publicClient, + EthAddress.fromString(rollupAddress), + EthAddress.fromString(inboxAddress), + EthAddress.fromString(registryAddress), + EthAddress.fromString(contractDeploymentEmitterAddress), + 0, + archiverStore, + 1000, + ); + + let latestBlockNum = await archiver.getBlockNumber(); + expect(latestBlockNum).toEqual(0); + + const createL1ToL2Messages = () => { + return [Fr.random().toString(true), Fr.random().toString(true)]; + }; + + const blocks = blockNums.map(x => L2Block.random(x, 4, x, x + 1, x * 2, x * 3)); + const rollupTxs = blocks.map(makeRollupTx); + // `L2Block.random(x)` creates some l1 to l2 messages. We add those, + // since it is expected by the test that these would be consumed. + // Archiver removes such messages from pending store. + // Also create some more messages to cancel and some that will stay pending. + + const additionalL1ToL2MessagesBlock102 = createL1ToL2Messages(); + const additionalL1ToL2MessagesBlock103 = createL1ToL2Messages(); + + const l1ToL2MessageAddedEvents = [ + makeL1ToL2MessageAddedEvents( + 100n, + blocks[0].newL1ToL2Messages.map(key => key.toString(true)), + ), + makeL1ToL2MessageAddedEvents( + 101n, + blocks[1].newL1ToL2Messages.map(key => key.toString(true)), + ), + makeL1ToL2MessageAddedEvents(102n, additionalL1ToL2MessagesBlock102), + makeL1ToL2MessageAddedEvents(103n, additionalL1ToL2MessagesBlock103), + ]; + + // Here we set the current L1 block number to 102. L1 to L2 messages after this should not be read. + publicClient.getBlockNumber.mockResolvedValue(102n); + // add all of the L1 to L2 messages to the mock + publicClient.getLogs + .mockResolvedValueOnce(l1ToL2MessageAddedEvents.flat()) + .mockResolvedValueOnce([]) + .mockResolvedValueOnce([makeL2BlockProcessedEvent(70n, 1n), makeL2BlockProcessedEvent(80n, 2n)]) + .mockResolvedValue([]); + rollupTxs.slice(0, numL2BlocksInTest).forEach(tx => publicClient.getTransaction.mockResolvedValueOnce(tx)); + + await archiver.start(false); + + // Wait until block 3 is processed. If this won't happen the test will fail with timeout. + while ((await archiver.getBlockNumber()) !== numL2BlocksInTest) { + await sleep(100); + } + + latestBlockNum = await archiver.getBlockNumber(); + expect(latestBlockNum).toEqual(numL2BlocksInTest); + + // Check that the only pending L1 to L2 messages are those from eth bock 102 + const expectedPendingMessageKeys = additionalL1ToL2MessagesBlock102; + const actualPendingMessageKeys = (await archiver.getPendingL1ToL2Messages(100)).map(key => key.toString(true)); + expect(actualPendingMessageKeys).toEqual(expectedPendingMessageKeys); + + await archiver.stop(); + }, 10_000); }); /** diff --git a/yarn-project/archiver/src/archiver/archiver.ts b/yarn-project/archiver/src/archiver/archiver.ts index f6f25e59c64..2384db75762 100644 --- a/yarn-project/archiver/src/archiver/archiver.ts +++ b/yarn-project/archiver/src/archiver/archiver.ts @@ -148,6 +148,33 @@ export class Archiver implements L2BlockSource, L2LogsSource, ContractDataSource return; } + // ********** Ensuring Consistency of data pulled from L1 ********** + + /** + * There are a number of calls in this sync operation to L1 for retrieving + * events and transaction data. There are a couple of things we need to bear in mind + * to ensure that data is read exactly once. + * + * The first is the problem of eventually consistent ETH service providers like Infura. + * We are not currently handling this correctly in the case of L1 to L2 messages and we will + * want to re-visit L2 Block and contract data retrieval at a later stage. This is not + * currently a problem but will need to be addressed before a mainnet release. + * + * The second is that in between the various calls to L1, the block number can move meaning some + * of the following calls will return data for blocks that were not present during earlier calls. + * This is a problem for example when setting the last block number marker for L1 to L2 messages - + * this.lastProcessedBlockNumber = currentBlockNumber; + * It's possible that we actually received messages in block currentBlockNumber + 1 meaning the next time + * we do this sync we get the same message again. Addtionally, the call to get cancelled L1 to L2 messages + * could read from a block not present when retrieving pending messages. If a message was added and cancelled + * in the same eth block then we could try and cancel a non-exitent pending message. + * + * To combat this for the time being we simply ensure that all data retrieval methods only retrieve + * data up to the currentBlockNumber captured at the top of this function. We might want to improve on this + * in future but for the time being it should give us the guarantees that we need + * + */ + // ********** Events that are processed in between blocks ********** // Process l1ToL2Messages, these are consumed as time passes, not each block @@ -155,15 +182,15 @@ export class Archiver implements L2BlockSource, L2LogsSource, ContractDataSource this.publicClient, this.inboxAddress, blockUntilSynced, - currentBlockNumber, this.lastProcessedBlockNumber + 1n, // + 1 to prevent re-including messages from the last processed block + currentBlockNumber, ); const retrievedCancelledL1ToL2Messages = await retrieveNewCancelledL1ToL2Messages( this.publicClient, this.inboxAddress, blockUntilSynced, - currentBlockNumber, this.lastProcessedBlockNumber + 1n, + currentBlockNumber, ); // TODO (#717): optimise this - there could be messages in confirmed that are also in pending. @@ -188,8 +215,8 @@ export class Archiver implements L2BlockSource, L2LogsSource, ContractDataSource this.publicClient, this.rollupAddress, blockUntilSynced, - currentBlockNumber, this.nextL2BlockFromBlock, + currentBlockNumber, nextExpectedL2BlockNum, ); @@ -202,8 +229,8 @@ export class Archiver implements L2BlockSource, L2LogsSource, ContractDataSource this.publicClient, this.contractDeploymentEmitterAddress, blockUntilSynced, - currentBlockNumber, this.nextL2BlockFromBlock, + currentBlockNumber, blockHashMapping, ); if (retrievedBlocks.retrievedData.length === 0) { diff --git a/yarn-project/archiver/src/archiver/data_retrieval.ts b/yarn-project/archiver/src/archiver/data_retrieval.ts index c1005b55137..3f1004e9217 100644 --- a/yarn-project/archiver/src/archiver/data_retrieval.ts +++ b/yarn-project/archiver/src/archiver/data_retrieval.ts @@ -34,8 +34,8 @@ type DataRetrieval = { * @param publicClient - The viem public client to use for transaction retrieval. * @param rollupAddress - The address of the rollup contract. * @param blockUntilSynced - If true, blocks until the archiver has fully synced. - * @param currentL1BlockNum - Latest available block number in the ETH node. * @param searchStartBlock - The block number to use for starting the search. + * @param searchEndBlock - The highest block number that we should search up to. * @param expectedNextL2BlockNum - The next L2 block number that we expect to find. * @returns An array of L2 Blocks and the next eth block to search from */ @@ -43,16 +43,19 @@ export async function retrieveBlocks( publicClient: PublicClient, rollupAddress: EthAddress, blockUntilSynced: boolean, - currentL1BlockNum: bigint, searchStartBlock: bigint, + searchEndBlock: bigint, expectedNextL2BlockNum: bigint, ): Promise> { const retrievedBlocks: L2Block[] = []; do { - if (searchStartBlock > currentL1BlockNum) { + if (searchStartBlock > searchEndBlock) { break; } - const l2BlockProcessedLogs = await getL2BlockProcessedLogs(publicClient, rollupAddress, searchStartBlock); + const unfilteredLogs = await getL2BlockProcessedLogs(publicClient, rollupAddress, searchStartBlock); + const l2BlockProcessedLogs = unfilteredLogs.filter( + log => log.blockNumber !== null && log.blockNumber <= searchEndBlock, + ); if (l2BlockProcessedLogs.length === 0) { break; } @@ -61,7 +64,7 @@ export async function retrieveBlocks( retrievedBlocks.push(...newBlocks); searchStartBlock = l2BlockProcessedLogs[l2BlockProcessedLogs.length - 1].blockNumber! + 1n; expectedNextL2BlockNum += BigInt(newBlocks.length); - } while (blockUntilSynced && searchStartBlock <= currentL1BlockNum); + } while (blockUntilSynced && searchStartBlock <= searchEndBlock); return { nextEthBlockNumber: searchStartBlock, retrievedData: retrievedBlocks }; } @@ -70,8 +73,8 @@ export async function retrieveBlocks( * @param publicClient - The viem public client to use for transaction retrieval. * @param contractDeploymentEmitterAddress - The address of the contract deployment emitter contract. * @param blockUntilSynced - If true, blocks until the archiver has fully synced. - * @param currentBlockNumber - Latest available block number in the ETH node. * @param searchStartBlock - The block number to use for starting the search. + * @param searchEndBlock - The highest block number that we should search up to. * @param blockHashMapping - A mapping from block number to relevant block hash. * @returns An array of ExtendedContractData and their equivalent L2 Block number along with the next eth block to search from.. */ @@ -79,27 +82,30 @@ export async function retrieveNewContractData( publicClient: PublicClient, contractDeploymentEmitterAddress: EthAddress, blockUntilSynced: boolean, - currentBlockNumber: bigint, searchStartBlock: bigint, + searchEndBlock: bigint, blockHashMapping: { [key: number]: Buffer | undefined }, ): Promise> { let retrievedNewContracts: [ExtendedContractData[], number][] = []; do { - if (searchStartBlock > currentBlockNumber) { + if (searchStartBlock > searchEndBlock) { break; } - const contractDataLogs = await getContractDeploymentLogs( + const unfilteredLogs = await getContractDeploymentLogs( publicClient, contractDeploymentEmitterAddress, searchStartBlock, ); + const contractDataLogs = unfilteredLogs.filter( + log => log.blockNumber !== null && log.blockNumber <= searchEndBlock, + ); if (contractDataLogs.length === 0) { break; } const newContracts = processContractDeploymentLogs(blockHashMapping, contractDataLogs); retrievedNewContracts = retrievedNewContracts.concat(newContracts); searchStartBlock = (contractDataLogs.findLast(cd => !!cd)?.blockNumber || searchStartBlock) + 1n; - } while (blockUntilSynced && searchStartBlock <= currentBlockNumber); + } while (blockUntilSynced && searchStartBlock <= searchEndBlock); return { nextEthBlockNumber: searchStartBlock, retrievedData: retrievedNewContracts }; } @@ -108,28 +114,31 @@ export async function retrieveNewContractData( * @param publicClient - The viem public client to use for transaction retrieval. * @param inboxAddress - The address of the inbox contract to fetch messages from. * @param blockUntilSynced - If true, blocks until the archiver has fully synced. - * @param currentBlockNumber - Latest available block number in the ETH node. * @param searchStartBlock - The block number to use for starting the search. + * @param searchEndBlock - The highest block number that we should search up to. * @returns An array of L1ToL2Message and next eth block to search from. */ export async function retrieveNewPendingL1ToL2Messages( publicClient: PublicClient, inboxAddress: EthAddress, blockUntilSynced: boolean, - currentBlockNumber: bigint, searchStartBlock: bigint, + searchEndBlock: bigint, ): Promise> { const retrievedNewL1ToL2Messages: L1ToL2Message[] = []; do { - if (searchStartBlock > currentBlockNumber) { + if (searchStartBlock > searchEndBlock) { break; } - const newL1ToL2MessageLogs = await getPendingL1ToL2MessageLogs(publicClient, inboxAddress, searchStartBlock); + const unfilteredLogs = await getPendingL1ToL2MessageLogs(publicClient, inboxAddress, searchStartBlock); + const newL1ToL2MessageLogs = unfilteredLogs.filter( + log => log.blockNumber !== null && log.blockNumber <= searchEndBlock, + ); const newL1ToL2Messages = processPendingL1ToL2MessageAddedLogs(newL1ToL2MessageLogs); retrievedNewL1ToL2Messages.push(...newL1ToL2Messages); // handles the case when there are no new messages: searchStartBlock = (newL1ToL2MessageLogs.findLast(msgLog => !!msgLog)?.blockNumber || searchStartBlock) + 1n; - } while (blockUntilSynced && searchStartBlock <= currentBlockNumber); + } while (blockUntilSynced && searchStartBlock <= searchEndBlock); return { nextEthBlockNumber: searchStartBlock, retrievedData: retrievedNewL1ToL2Messages }; } @@ -138,32 +147,31 @@ export async function retrieveNewPendingL1ToL2Messages( * @param publicClient - The viem public client to use for transaction retrieval. * @param inboxAddress - The address of the inbox contract to fetch messages from. * @param blockUntilSynced - If true, blocks until the archiver has fully synced. - * @param currentBlockNumber - Latest available block number in the ETH node. * @param searchStartBlock - The block number to use for starting the search. + * @param searchEndBlock - The highest block number that we should search up to. * @returns An array of message keys that were cancelled and next eth block to search from. */ export async function retrieveNewCancelledL1ToL2Messages( publicClient: PublicClient, inboxAddress: EthAddress, blockUntilSynced: boolean, - currentBlockNumber: bigint, searchStartBlock: bigint, + searchEndBlock: bigint, ): Promise> { const retrievedNewCancelledL1ToL2Messages: Fr[] = []; do { - if (searchStartBlock > currentBlockNumber) { + if (searchStartBlock > searchEndBlock) { break; } - const newL1ToL2MessageCancelledLogs = await getL1ToL2MessageCancelledLogs( - publicClient, - inboxAddress, - searchStartBlock, + const unfilteredLogs = await getL1ToL2MessageCancelledLogs(publicClient, inboxAddress, searchStartBlock); + const newL1ToL2MessageCancelledLogs = unfilteredLogs.filter( + log => log.blockNumber !== null && log.blockNumber <= searchEndBlock, ); const newCancelledL1ToL2Messages = processCancelledL1ToL2MessagesLogs(newL1ToL2MessageCancelledLogs); retrievedNewCancelledL1ToL2Messages.push(...newCancelledL1ToL2Messages); // handles the case when there are no new messages: searchStartBlock = (newL1ToL2MessageCancelledLogs.findLast(msgLog => !!msgLog)?.blockNumber || searchStartBlock) + 1n; - } while (blockUntilSynced && searchStartBlock <= currentBlockNumber); + } while (blockUntilSynced && searchStartBlock <= searchEndBlock); return { nextEthBlockNumber: searchStartBlock, retrievedData: retrievedNewCancelledL1ToL2Messages }; } From d82459d448a436747974e1951fce92fd3a91c5db Mon Sep 17 00:00:00 2001 From: PhilWindle Date: Thu, 28 Sep 2023 11:46:54 +0000 Subject: [PATCH 2/6] Set toBlock in viem for log retrieval --- .../archiver/src/archiver/data_retrieval.ts | 29 ++++++++++++++----- .../archiver/src/archiver/eth_log_handlers.ts | 12 ++++++++ 2 files changed, 34 insertions(+), 7 deletions(-) diff --git a/yarn-project/archiver/src/archiver/data_retrieval.ts b/yarn-project/archiver/src/archiver/data_retrieval.ts index 3f1004e9217..1c2a01cc821 100644 --- a/yarn-project/archiver/src/archiver/data_retrieval.ts +++ b/yarn-project/archiver/src/archiver/data_retrieval.ts @@ -52,9 +52,10 @@ export async function retrieveBlocks( if (searchStartBlock > searchEndBlock) { break; } - const unfilteredLogs = await getL2BlockProcessedLogs(publicClient, rollupAddress, searchStartBlock); + const unfilteredLogs = await getL2BlockProcessedLogs(publicClient, rollupAddress, searchStartBlock, searchEndBlock); + // ensure we only pulled logs from the expected block range const l2BlockProcessedLogs = unfilteredLogs.filter( - log => log.blockNumber !== null && log.blockNumber <= searchEndBlock, + log => log.blockNumber !== null && log.blockNumber >= searchStartBlock && log.blockNumber <= searchEndBlock, ); if (l2BlockProcessedLogs.length === 0) { break; @@ -95,9 +96,11 @@ export async function retrieveNewContractData( publicClient, contractDeploymentEmitterAddress, searchStartBlock, + searchEndBlock, ); + // ensure we only pulled logs from the expected block range const contractDataLogs = unfilteredLogs.filter( - log => log.blockNumber !== null && log.blockNumber <= searchEndBlock, + log => log.blockNumber !== null && log.blockNumber >= searchStartBlock && log.blockNumber <= searchEndBlock, ); if (contractDataLogs.length === 0) { break; @@ -130,9 +133,15 @@ export async function retrieveNewPendingL1ToL2Messages( if (searchStartBlock > searchEndBlock) { break; } - const unfilteredLogs = await getPendingL1ToL2MessageLogs(publicClient, inboxAddress, searchStartBlock); + const unfilteredLogs = await getPendingL1ToL2MessageLogs( + publicClient, + inboxAddress, + searchStartBlock, + searchEndBlock, + ); + // ensure we only pulled logs from the expected block range const newL1ToL2MessageLogs = unfilteredLogs.filter( - log => log.blockNumber !== null && log.blockNumber <= searchEndBlock, + log => log.blockNumber !== null && log.blockNumber >= searchStartBlock && log.blockNumber <= searchEndBlock, ); const newL1ToL2Messages = processPendingL1ToL2MessageAddedLogs(newL1ToL2MessageLogs); retrievedNewL1ToL2Messages.push(...newL1ToL2Messages); @@ -163,9 +172,15 @@ export async function retrieveNewCancelledL1ToL2Messages( if (searchStartBlock > searchEndBlock) { break; } - const unfilteredLogs = await getL1ToL2MessageCancelledLogs(publicClient, inboxAddress, searchStartBlock); + const unfilteredLogs = await getL1ToL2MessageCancelledLogs( + publicClient, + inboxAddress, + searchStartBlock, + searchEndBlock, + ); + // ensure we only pulled logs from the expected block range const newL1ToL2MessageCancelledLogs = unfilteredLogs.filter( - log => log.blockNumber !== null && log.blockNumber <= searchEndBlock, + log => log.blockNumber !== null && log.blockNumber >= searchStartBlock && log.blockNumber <= searchEndBlock, ); const newCancelledL1ToL2Messages = processCancelledL1ToL2MessagesLogs(newL1ToL2MessageCancelledLogs); retrievedNewCancelledL1ToL2Messages.push(...newCancelledL1ToL2Messages); diff --git a/yarn-project/archiver/src/archiver/eth_log_handlers.ts b/yarn-project/archiver/src/archiver/eth_log_handlers.ts index 58f94109efc..30c6bbcea3a 100644 --- a/yarn-project/archiver/src/archiver/eth_log_handlers.ts +++ b/yarn-project/archiver/src/archiver/eth_log_handlers.ts @@ -116,12 +116,14 @@ async function getBlockFromCallData( * @param publicClient - The viem public client to use for transaction retrieval. * @param rollupAddress - The address of the rollup contract. * @param fromBlock - First block to get logs from (inclusive). + * @param toBlock - Last block to get logs from (inclusive). * @returns An array of `L2BlockProcessed` logs. */ export async function getL2BlockProcessedLogs( publicClient: PublicClient, rollupAddress: EthAddress, fromBlock: bigint, + toBlock: bigint, ) { // Note: For some reason the return type of `getLogs` would not get correctly derived if I didn't set the abiItem // as a standalone constant. @@ -133,6 +135,7 @@ export async function getL2BlockProcessedLogs( address: getAddress(rollupAddress.toString()), event: abiItem, fromBlock, + toBlock: toBlock + 1n, // the toBlock argument in getLogs is exclusive }); } @@ -141,12 +144,14 @@ export async function getL2BlockProcessedLogs( * @param publicClient - The viem public client to use for transaction retrieval. * @param contractDeploymentEmitterAddress - The address of the L2 contract deployment emitter contract. * @param fromBlock - First block to get logs from (inclusive). + * @param toBlock - Last block to get logs from (inclusive). * @returns An array of `ContractDeployment` logs. */ export async function getContractDeploymentLogs( publicClient: PublicClient, contractDeploymentEmitterAddress: EthAddress, fromBlock: bigint, + toBlock: bigint, ): Promise[]> { const abiItem = getAbiItem({ abi: ContractDeploymentEmitterAbi, @@ -156,6 +161,7 @@ export async function getContractDeploymentLogs( address: getAddress(contractDeploymentEmitterAddress.toString()), event: abiItem, fromBlock, + toBlock: toBlock + 1n, // the toBlock argument in getLogs is exclusive }); } @@ -205,12 +211,14 @@ export function processContractDeploymentLogs( * @param publicClient - The viem public client to use for transaction retrieval. * @param inboxAddress - The address of the inbox contract. * @param fromBlock - First block to get logs from (inclusive). + * @param toBlock - Last block to get logs from (inclusive). * @returns An array of `MessageAdded` logs. */ export async function getPendingL1ToL2MessageLogs( publicClient: PublicClient, inboxAddress: EthAddress, fromBlock: bigint, + toBlock: bigint, ): Promise[]> { const abiItem = getAbiItem({ abi: InboxAbi, @@ -220,6 +228,7 @@ export async function getPendingL1ToL2MessageLogs( address: getAddress(inboxAddress.toString()), event: abiItem, fromBlock, + toBlock: toBlock + 1n, // the toBlock argument in getLogs is exclusive }); } @@ -228,12 +237,14 @@ export async function getPendingL1ToL2MessageLogs( * @param publicClient - The viem public client to use for transaction retrieval. * @param inboxAddress - The address of the inbox contract. * @param fromBlock - First block to get logs from (inclusive). + * @param toBlock - Last block to get logs from (inclusive). * @returns An array of `L1ToL2MessageCancelled` logs. */ export async function getL1ToL2MessageCancelledLogs( publicClient: PublicClient, inboxAddress: EthAddress, fromBlock: bigint, + toBlock: bigint, ): Promise[]> { const abiItem = getAbiItem({ abi: InboxAbi, @@ -243,5 +254,6 @@ export async function getL1ToL2MessageCancelledLogs( address: getAddress(inboxAddress.toString()), event: abiItem, fromBlock, + toBlock: toBlock + 1n, // the toBlock argument in getLogs is exclusive }); } From e3cf59a947b7814ddcc45467405041d6021bd959 Mon Sep 17 00:00:00 2001 From: PhilWindle Date: Thu, 28 Sep 2023 12:44:08 +0000 Subject: [PATCH 3/6] Test fix --- .../archiver/src/archiver/archiver.test.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/yarn-project/archiver/src/archiver/archiver.test.ts b/yarn-project/archiver/src/archiver/archiver.test.ts index 65dc7fcbe8d..971c07539d2 100644 --- a/yarn-project/archiver/src/archiver/archiver.test.ts +++ b/yarn-project/archiver/src/archiver/archiver.test.ts @@ -63,27 +63,27 @@ describe('Archiver', () => { blocks[1].newL1ToL2Messages.map(key => key.toString(true)), ), makeL1ToL2MessageAddedEvents( - 1000n, + 2501n, blocks[2].newL1ToL2Messages.map(key => key.toString(true)), ), - makeL1ToL2MessageAddedEvents(102n, [ + makeL1ToL2MessageAddedEvents(2502n, [ messageToCancel1, messageToCancel2, messageToStayPending1, messageToStayPending2, ]), ]; - publicClient.getBlockNumber.mockResolvedValueOnce(2500n).mockResolvedValueOnce(2501n).mockResolvedValueOnce(2502n); + publicClient.getBlockNumber.mockResolvedValueOnce(2500n).mockResolvedValueOnce(2600n).mockResolvedValueOnce(2700n); // logs should be created in order of how archiver syncs. publicClient.getLogs .mockResolvedValueOnce(l1ToL2MessageAddedEvents.slice(0, 2).flat()) .mockResolvedValueOnce([]) // no messages to cancel .mockResolvedValueOnce([makeL2BlockProcessedEvent(101n, 1n)]) - .mockResolvedValueOnce([makeContractDeploymentEvent(103n, blocks[0])]) + .mockResolvedValueOnce([makeContractDeploymentEvent(103n, blocks[0])]) // the first loop of the archiver ends here at block 2500 .mockResolvedValueOnce(l1ToL2MessageAddedEvents.slice(2, 4).flat()) - .mockResolvedValueOnce(makeL1ToL2MessageCancelledEvents(1100n, l1ToL2MessagesToCancel)) - .mockResolvedValueOnce([makeL2BlockProcessedEvent(1101n, 2n), makeL2BlockProcessedEvent(1150n, 3n)]) - .mockResolvedValueOnce([makeContractDeploymentEvent(1102n, blocks[1])]) + .mockResolvedValueOnce(makeL1ToL2MessageCancelledEvents(2503n, l1ToL2MessagesToCancel)) + .mockResolvedValueOnce([makeL2BlockProcessedEvent(2510n, 2n), makeL2BlockProcessedEvent(2520n, 3n)]) + .mockResolvedValueOnce([makeContractDeploymentEvent(2540n, blocks[1])]) .mockResolvedValue([]); rollupTxs.forEach(tx => publicClient.getTransaction.mockResolvedValueOnce(tx)); From c7a2f364152205e415ed88f7b56b761d41b8c2b4 Mon Sep 17 00:00:00 2001 From: PhilWindle Date: Thu, 28 Sep 2023 12:51:28 +0000 Subject: [PATCH 4/6] Remove redundant check --- .../archiver/src/archiver/data_retrieval.ts | 27 ++++++------------- 1 file changed, 8 insertions(+), 19 deletions(-) diff --git a/yarn-project/archiver/src/archiver/data_retrieval.ts b/yarn-project/archiver/src/archiver/data_retrieval.ts index 1c2a01cc821..52e5c5181e3 100644 --- a/yarn-project/archiver/src/archiver/data_retrieval.ts +++ b/yarn-project/archiver/src/archiver/data_retrieval.ts @@ -52,10 +52,11 @@ export async function retrieveBlocks( if (searchStartBlock > searchEndBlock) { break; } - const unfilteredLogs = await getL2BlockProcessedLogs(publicClient, rollupAddress, searchStartBlock, searchEndBlock); - // ensure we only pulled logs from the expected block range - const l2BlockProcessedLogs = unfilteredLogs.filter( - log => log.blockNumber !== null && log.blockNumber >= searchStartBlock && log.blockNumber <= searchEndBlock, + const l2BlockProcessedLogs = await getL2BlockProcessedLogs( + publicClient, + rollupAddress, + searchStartBlock, + searchEndBlock, ); if (l2BlockProcessedLogs.length === 0) { break; @@ -92,16 +93,12 @@ export async function retrieveNewContractData( if (searchStartBlock > searchEndBlock) { break; } - const unfilteredLogs = await getContractDeploymentLogs( + const contractDataLogs = await getContractDeploymentLogs( publicClient, contractDeploymentEmitterAddress, searchStartBlock, searchEndBlock, ); - // ensure we only pulled logs from the expected block range - const contractDataLogs = unfilteredLogs.filter( - log => log.blockNumber !== null && log.blockNumber >= searchStartBlock && log.blockNumber <= searchEndBlock, - ); if (contractDataLogs.length === 0) { break; } @@ -133,16 +130,12 @@ export async function retrieveNewPendingL1ToL2Messages( if (searchStartBlock > searchEndBlock) { break; } - const unfilteredLogs = await getPendingL1ToL2MessageLogs( + const newL1ToL2MessageLogs = await getPendingL1ToL2MessageLogs( publicClient, inboxAddress, searchStartBlock, searchEndBlock, ); - // ensure we only pulled logs from the expected block range - const newL1ToL2MessageLogs = unfilteredLogs.filter( - log => log.blockNumber !== null && log.blockNumber >= searchStartBlock && log.blockNumber <= searchEndBlock, - ); const newL1ToL2Messages = processPendingL1ToL2MessageAddedLogs(newL1ToL2MessageLogs); retrievedNewL1ToL2Messages.push(...newL1ToL2Messages); // handles the case when there are no new messages: @@ -172,16 +165,12 @@ export async function retrieveNewCancelledL1ToL2Messages( if (searchStartBlock > searchEndBlock) { break; } - const unfilteredLogs = await getL1ToL2MessageCancelledLogs( + const newL1ToL2MessageCancelledLogs = await getL1ToL2MessageCancelledLogs( publicClient, inboxAddress, searchStartBlock, searchEndBlock, ); - // ensure we only pulled logs from the expected block range - const newL1ToL2MessageCancelledLogs = unfilteredLogs.filter( - log => log.blockNumber !== null && log.blockNumber >= searchStartBlock && log.blockNumber <= searchEndBlock, - ); const newCancelledL1ToL2Messages = processCancelledL1ToL2MessagesLogs(newL1ToL2MessageCancelledLogs); retrievedNewCancelledL1ToL2Messages.push(...newCancelledL1ToL2Messages); // handles the case when there are no new messages: From 52085ef00a23b46c4344ea38737971b8208d0624 Mon Sep 17 00:00:00 2001 From: PhilWindle Date: Thu, 28 Sep 2023 13:00:12 +0000 Subject: [PATCH 5/6] Test fix --- yarn-project/archiver/src/archiver/archiver.test.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/yarn-project/archiver/src/archiver/archiver.test.ts b/yarn-project/archiver/src/archiver/archiver.test.ts index 971c07539d2..6bf50ce7aee 100644 --- a/yarn-project/archiver/src/archiver/archiver.test.ts +++ b/yarn-project/archiver/src/archiver/archiver.test.ts @@ -175,7 +175,13 @@ describe('Archiver', () => { publicClient.getBlockNumber.mockResolvedValue(102n); // add all of the L1 to L2 messages to the mock publicClient.getLogs - .mockResolvedValueOnce(l1ToL2MessageAddedEvents.flat()) + .mockImplementationOnce((args?: any) => { + return Promise.resolve( + l1ToL2MessageAddedEvents + .flat() + .filter(x => x.blockNumber! >= args.fromBlock && x.blockNumber! < args.toBlock), + ); + }) .mockResolvedValueOnce([]) .mockResolvedValueOnce([makeL2BlockProcessedEvent(70n, 1n), makeL2BlockProcessedEvent(80n, 2n)]) .mockResolvedValue([]); From d798216b001a4013d7c00921aa76b2c6c147be74 Mon Sep 17 00:00:00 2001 From: PhilWindle Date: Thu, 28 Sep 2023 13:01:44 +0000 Subject: [PATCH 6/6] Typo --- yarn-project/archiver/src/archiver/archiver.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/yarn-project/archiver/src/archiver/archiver.ts b/yarn-project/archiver/src/archiver/archiver.ts index 2384db75762..ea9c82d3822 100644 --- a/yarn-project/archiver/src/archiver/archiver.ts +++ b/yarn-project/archiver/src/archiver/archiver.ts @@ -167,7 +167,7 @@ export class Archiver implements L2BlockSource, L2LogsSource, ContractDataSource * It's possible that we actually received messages in block currentBlockNumber + 1 meaning the next time * we do this sync we get the same message again. Addtionally, the call to get cancelled L1 to L2 messages * could read from a block not present when retrieving pending messages. If a message was added and cancelled - * in the same eth block then we could try and cancel a non-exitent pending message. + * in the same eth block then we could try and cancel a non-existent pending message. * * To combat this for the time being we simply ensure that all data retrieval methods only retrieve * data up to the currentBlockNumber captured at the top of this function. We might want to improve on this