From ba512d10db75cbba4b932fc8ba852893b62caf6e Mon Sep 17 00:00:00 2001 From: Lasse Herskind <16536249+LHerskind@users.noreply.github.com> Date: Thu, 26 Sep 2024 19:45:40 +0100 Subject: [PATCH] refactor: optimise l1 to l2 message fetching (#8672) Takes the same approach as done in other optimisations tasks related to #8457 and before it go looking at the events, it will check values in the contract to figure out if there is even anything to look for. If we figure that there is nothing to look for, we update the l1 block number of interest, and call it a day. This allow getting rid of a bunch of get logs call and replacing them with a single function call instead. Does not impact the tests in E2E significantly because they are running on anvil without anything but our rollup happening so there is not a lot of wasted effort there, but should see meaningful changes when pointed at larger systems. --- l1-contracts/src/core/messagebridge/Inbox.sol | 5 +++ .../archiver/src/archiver/archiver.test.ts | 38 ++++++++++--------- .../archiver/src/archiver/archiver.ts | 20 ++++++---- .../archiver/src/archiver/archiver_store.ts | 6 +++ .../kv_archiver_store/kv_archiver_store.ts | 4 ++ .../kv_archiver_store/message_store.ts | 9 +++++ .../l1_to_l2_message_store.ts | 4 ++ .../memory_archiver_store.ts | 4 ++ 8 files changed, 66 insertions(+), 24 deletions(-) diff --git a/l1-contracts/src/core/messagebridge/Inbox.sol b/l1-contracts/src/core/messagebridge/Inbox.sol index 09d7d3c3a4e8..90069507ab95 100644 --- a/l1-contracts/src/core/messagebridge/Inbox.sol +++ b/l1-contracts/src/core/messagebridge/Inbox.sol @@ -34,6 +34,10 @@ contract Inbox is IInbox { mapping(uint256 blockNumber => FrontierLib.Tree tree) public trees; + // This value is not used much by the contract, but it is useful for synching the node faster + // as it can more easily figure out if it can just skip looking for events for a time period. + uint256 public totalMessagesInserted = 0; + constructor(address _rollup, uint256 _height) { ROLLUP = _rollup; @@ -86,6 +90,7 @@ contract Inbox is IInbox { bytes32 leaf = message.sha256ToField(); uint256 index = currentTree.insertLeaf(leaf); + totalMessagesInserted++; emit MessageSent(inProgress, index, leaf); return leaf; diff --git a/yarn-project/archiver/src/archiver/archiver.test.ts b/yarn-project/archiver/src/archiver/archiver.test.ts index 64860c7907b8..e99853a95f4b 100644 --- a/yarn-project/archiver/src/archiver/archiver.test.ts +++ b/yarn-project/archiver/src/archiver/archiver.test.ts @@ -33,6 +33,10 @@ interface MockRollupContractRead { status: (args: readonly [bigint]) => Promise<[bigint, `0x${string}`, bigint, `0x${string}`, `0x${string}`]>; } +interface MockInboxContractRead { + totalMessagesInserted: () => Promise; +} + describe('Archiver', () => { const rollupAddress = EthAddress.ZERO; const inboxAddress = EthAddress.ZERO; @@ -45,6 +49,7 @@ describe('Archiver', () => { let now: number; let rollupRead: MockProxy; + let inboxRead: MockProxy; let archiver: Archiver; let blocks: L2Block[]; @@ -79,6 +84,9 @@ describe('Archiver', () => { ); ((archiver as any).rollup as any).read = rollupRead; + + inboxRead = mock(); + ((archiver as any).inbox as any).read = inboxRead; }); afterEach(async () => { @@ -104,6 +112,8 @@ describe('Archiver', () => { blocks[0].archive.root.toString(), ]); + inboxRead.totalMessagesInserted.mockResolvedValueOnce(2n).mockResolvedValueOnce(6n); + mockGetLogs({ messageSent: [makeMessageSentEvent(98n, 1n, 0n), makeMessageSentEvent(99n, 1n, 1n)], L2BlockProposed: [makeL2BlockProposedEvent(101n, 1n, blocks[0].archive.root.toString())], @@ -209,6 +219,8 @@ describe('Archiver', () => { rollupRead.status.mockResolvedValue([0n, GENESIS_ROOT, 2n, blocks[1].archive.root.toString(), GENESIS_ROOT]); + inboxRead.totalMessagesInserted.mockResolvedValueOnce(2n).mockResolvedValueOnce(2n); + mockGetLogs({ messageSent: [makeMessageSentEvent(66n, 1n, 0n), makeMessageSentEvent(68n, 1n, 1n)], L2BlockProposed: [ @@ -232,7 +244,7 @@ describe('Archiver', () => { expect(loggerSpy).toHaveBeenCalledWith(errorMessage); }, 10_000); - it('skip event search if not blocks found', async () => { + it('skip event search if no changes found', async () => { const loggerSpy = jest.spyOn((archiver as any).log, 'verbose'); let latestBlockNum = await archiver.getBlockNumber(); @@ -247,11 +259,8 @@ describe('Archiver', () => { .mockResolvedValueOnce([0n, GENESIS_ROOT, 0n, GENESIS_ROOT, GENESIS_ROOT]) .mockResolvedValueOnce([0n, GENESIS_ROOT, 2n, blocks[1].archive.root.toString(), GENESIS_ROOT]); - // This can look slightly odd, but we will need to do an empty request for the messages, and will entirely skip - // a call to the proposed blocks because of changes with status. - mockGetLogs({ - messageSent: [], - }); + inboxRead.totalMessagesInserted.mockResolvedValueOnce(0n).mockResolvedValueOnce(2n); + mockGetLogs({ messageSent: [makeMessageSentEvent(66n, 1n, 0n), makeMessageSentEvent(68n, 1n, 1n)], L2BlockProposed: [ @@ -303,11 +312,12 @@ describe('Archiver', () => { .mockResolvedValueOnce(blocks[1].archive.root.toString()) .mockResolvedValueOnce(Fr.ZERO.toString()); - // This can look slightly odd, but we will need to do an empty request for the messages, and will entirely skip - // a call to the proposed blocks because of changes with status. - mockGetLogs({ - messageSent: [], - }); + inboxRead.totalMessagesInserted + .mockResolvedValueOnce(0n) + .mockResolvedValueOnce(2n) + .mockResolvedValueOnce(2n) + .mockResolvedValueOnce(2n); + mockGetLogs({ messageSent: [makeMessageSentEvent(66n, 1n, 0n), makeMessageSentEvent(68n, 1n, 1n)], L2BlockProposed: [ @@ -315,12 +325,6 @@ describe('Archiver', () => { makeL2BlockProposedEvent(80n, 2n, blocks[1].archive.root.toString()), ], }); - mockGetLogs({ - messageSent: [], - }); - mockGetLogs({ - messageSent: [], - }); rollupTxs.forEach(tx => publicClient.getTransaction.mockResolvedValueOnce(tx)); diff --git a/yarn-project/archiver/src/archiver/archiver.ts b/yarn-project/archiver/src/archiver/archiver.ts index 9b863f1de7c8..ce8994dff5b0 100644 --- a/yarn-project/archiver/src/archiver/archiver.ts +++ b/yarn-project/archiver/src/archiver/archiver.ts @@ -245,14 +245,10 @@ export class Archiver implements ArchiveSource { return; } - const retrievedL1ToL2Messages = await retrieveL1ToL2Messages( - this.inbox, - blockUntilSynced, - messagesSynchedTo + 1n, - currentL1BlockNumber, - ); + const localTotalMessageCount = await this.store.getTotalL1ToL2MessageCount(); + const destinationTotalMessageCount = await this.inbox.read.totalMessagesInserted(); - if (retrievedL1ToL2Messages.retrievedData.length === 0) { + if (localTotalMessageCount === destinationTotalMessageCount) { await this.store.setMessageSynchedL1BlockNumber(currentL1BlockNumber); this.log.verbose( `Retrieved no new L1 -> L2 messages between L1 blocks ${messagesSynchedTo + 1n} and ${currentL1BlockNumber}.`, @@ -260,6 +256,13 @@ export class Archiver implements ArchiveSource { return; } + const retrievedL1ToL2Messages = await retrieveL1ToL2Messages( + this.inbox, + blockUntilSynced, + messagesSynchedTo + 1n, + currentL1BlockNumber, + ); + await this.store.addL1ToL2Messages(retrievedL1ToL2Messages); this.log.verbose( @@ -780,4 +783,7 @@ class ArchiverStoreHelper getContractArtifact(address: AztecAddress): Promise { return this.store.getContractArtifact(address); } + getTotalL1ToL2MessageCount(): Promise { + return this.store.getTotalL1ToL2MessageCount(); + } } diff --git a/yarn-project/archiver/src/archiver/archiver_store.ts b/yarn-project/archiver/src/archiver/archiver_store.ts index 93f72a2b01f4..2218be901d4e 100644 --- a/yarn-project/archiver/src/archiver/archiver_store.ts +++ b/yarn-project/archiver/src/archiver/archiver_store.ts @@ -108,6 +108,12 @@ export interface ArchiverDataStore { */ getL1ToL2MessageIndex(l1ToL2Message: Fr, startIndex: bigint): Promise; + /** + * Get the total number of L1 to L2 messages + * @returns The number of L1 to L2 messages in the store + */ + getTotalL1ToL2MessageCount(): Promise; + /** * Gets up to `limit` amount of logs starting from `from`. * @param from - Number of the L2 block to which corresponds the first logs to be returned. diff --git a/yarn-project/archiver/src/archiver/kv_archiver_store/kv_archiver_store.ts b/yarn-project/archiver/src/archiver/kv_archiver_store/kv_archiver_store.ts index 6c34f8e8220e..7544fd0941a3 100644 --- a/yarn-project/archiver/src/archiver/kv_archiver_store/kv_archiver_store.ts +++ b/yarn-project/archiver/src/archiver/kv_archiver_store/kv_archiver_store.ts @@ -167,6 +167,10 @@ export class KVArchiverDataStore implements ArchiverDataStore { return this.#logStore.deleteLogs(blocks); } + getTotalL1ToL2MessageCount(): Promise { + return Promise.resolve(this.#messageStore.getTotalL1ToL2MessageCount()); + } + /** * Append L1 to L2 messages to the store. * @param messages - The L1 to L2 messages to be added to the store and the last processed L1 block. diff --git a/yarn-project/archiver/src/archiver/kv_archiver_store/message_store.ts b/yarn-project/archiver/src/archiver/kv_archiver_store/message_store.ts index 23a13a9ca321..3372785e6e45 100644 --- a/yarn-project/archiver/src/archiver/kv_archiver_store/message_store.ts +++ b/yarn-project/archiver/src/archiver/kv_archiver_store/message_store.ts @@ -17,6 +17,7 @@ export class MessageStore { #l1ToL2Messages: AztecMap; #l1ToL2MessageIndices: AztecMap; // We store array of bigints here because there can be duplicate messages #lastSynchedL1Block: AztecSingleton; + #totalMessageCount: AztecSingleton; #log = createDebugLogger('aztec:archiver:message_store'); @@ -26,6 +27,11 @@ export class MessageStore { this.#l1ToL2Messages = db.openMap('archiver_l1_to_l2_messages'); this.#l1ToL2MessageIndices = db.openMap('archiver_l1_to_l2_message_indices'); this.#lastSynchedL1Block = db.openSingleton('archiver_last_l1_block_new_messages'); + this.#totalMessageCount = db.openSingleton('archiver_l1_to_l2_message_count'); + } + + getTotalL1ToL2MessageCount(): bigint { + return this.#totalMessageCount.get() ?? 0n; } /** @@ -70,6 +76,9 @@ export class MessageStore { void this.#l1ToL2MessageIndices.set(message.leaf.toString(), indices); } + const lastTotalMessageCount = this.getTotalL1ToL2MessageCount(); + void this.#totalMessageCount.set(lastTotalMessageCount + BigInt(messages.retrievedData.length)); + return true; }); } diff --git a/yarn-project/archiver/src/archiver/memory_archiver_store/l1_to_l2_message_store.ts b/yarn-project/archiver/src/archiver/memory_archiver_store/l1_to_l2_message_store.ts index a922e62067e2..52f887056acb 100644 --- a/yarn-project/archiver/src/archiver/memory_archiver_store/l1_to_l2_message_store.ts +++ b/yarn-project/archiver/src/archiver/memory_archiver_store/l1_to_l2_message_store.ts @@ -19,6 +19,10 @@ export class L1ToL2MessageStore { constructor() {} + getTotalL1ToL2MessageCount(): bigint { + return BigInt(this.store.size); + } + addMessage(message: InboxLeaf) { if (message.index >= this.#l1ToL2MessagesSubtreeSize) { throw new Error(`Message index ${message.index} out of subtree range`); diff --git a/yarn-project/archiver/src/archiver/memory_archiver_store/memory_archiver_store.ts b/yarn-project/archiver/src/archiver/memory_archiver_store/memory_archiver_store.ts index 7facaa3d7031..9a1fe11ff581 100644 --- a/yarn-project/archiver/src/archiver/memory_archiver_store/memory_archiver_store.ts +++ b/yarn-project/archiver/src/archiver/memory_archiver_store/memory_archiver_store.ts @@ -219,6 +219,10 @@ export class MemoryArchiverStore implements ArchiverDataStore { return Promise.resolve(true); } + getTotalL1ToL2MessageCount(): Promise { + return Promise.resolve(this.l1ToL2Messages.getTotalL1ToL2MessageCount()); + } + /** * Append L1 to L2 messages to the store. * @param messages - The L1 to L2 messages to be added to the store and the last processed L1 block.