Skip to content

Commit

Permalink
refactor: optimise l1 to l2 message fetching (#8672)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
LHerskind authored and Rumata888 committed Sep 27, 2024
1 parent 077c410 commit ba512d1
Show file tree
Hide file tree
Showing 8 changed files with 66 additions and 24 deletions.
5 changes: 5 additions & 0 deletions l1-contracts/src/core/messagebridge/Inbox.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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;
Expand Down
38 changes: 21 additions & 17 deletions yarn-project/archiver/src/archiver/archiver.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@ interface MockRollupContractRead {
status: (args: readonly [bigint]) => Promise<[bigint, `0x${string}`, bigint, `0x${string}`, `0x${string}`]>;
}

interface MockInboxContractRead {
totalMessagesInserted: () => Promise<bigint>;
}

describe('Archiver', () => {
const rollupAddress = EthAddress.ZERO;
const inboxAddress = EthAddress.ZERO;
Expand All @@ -45,6 +49,7 @@ describe('Archiver', () => {
let now: number;

let rollupRead: MockProxy<MockRollupContractRead>;
let inboxRead: MockProxy<MockInboxContractRead>;
let archiver: Archiver;
let blocks: L2Block[];

Expand Down Expand Up @@ -79,6 +84,9 @@ describe('Archiver', () => {
);

((archiver as any).rollup as any).read = rollupRead;

inboxRead = mock<MockInboxContractRead>();
((archiver as any).inbox as any).read = inboxRead;
});

afterEach(async () => {
Expand All @@ -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())],
Expand Down Expand Up @@ -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: [
Expand All @@ -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();
Expand All @@ -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: [
Expand Down Expand Up @@ -303,24 +312,19 @@ 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: [
makeL2BlockProposedEvent(70n, 1n, blocks[0].archive.root.toString()),
makeL2BlockProposedEvent(80n, 2n, blocks[1].archive.root.toString()),
],
});
mockGetLogs({
messageSent: [],
});
mockGetLogs({
messageSent: [],
});

rollupTxs.forEach(tx => publicClient.getTransaction.mockResolvedValueOnce(tx));

Expand Down
20 changes: 13 additions & 7 deletions yarn-project/archiver/src/archiver/archiver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -245,21 +245,24 @@ 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}.`,
);
return;
}

const retrievedL1ToL2Messages = await retrieveL1ToL2Messages(
this.inbox,
blockUntilSynced,
messagesSynchedTo + 1n,
currentL1BlockNumber,
);

await this.store.addL1ToL2Messages(retrievedL1ToL2Messages);

this.log.verbose(
Expand Down Expand Up @@ -780,4 +783,7 @@ class ArchiverStoreHelper
getContractArtifact(address: AztecAddress): Promise<ContractArtifact | undefined> {
return this.store.getContractArtifact(address);
}
getTotalL1ToL2MessageCount(): Promise<bigint> {
return this.store.getTotalL1ToL2MessageCount();
}
}
6 changes: 6 additions & 0 deletions yarn-project/archiver/src/archiver/archiver_store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,12 @@ export interface ArchiverDataStore {
*/
getL1ToL2MessageIndex(l1ToL2Message: Fr, startIndex: bigint): Promise<bigint | undefined>;

/**
* Get the total number of L1 to L2 messages
* @returns The number of L1 to L2 messages in the store
*/
getTotalL1ToL2MessageCount(): Promise<bigint>;

/**
* 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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,10 @@ export class KVArchiverDataStore implements ArchiverDataStore {
return this.#logStore.deleteLogs(blocks);
}

getTotalL1ToL2MessageCount(): Promise<bigint> {
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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ export class MessageStore {
#l1ToL2Messages: AztecMap<string, Buffer>;
#l1ToL2MessageIndices: AztecMap<string, bigint[]>; // We store array of bigints here because there can be duplicate messages
#lastSynchedL1Block: AztecSingleton<bigint>;
#totalMessageCount: AztecSingleton<bigint>;

#log = createDebugLogger('aztec:archiver:message_store');

Expand All @@ -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;
}

/**
Expand Down Expand Up @@ -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;
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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`);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,10 @@ export class MemoryArchiverStore implements ArchiverDataStore {
return Promise.resolve(true);
}

getTotalL1ToL2MessageCount(): Promise<bigint> {
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.
Expand Down

0 comments on commit ba512d1

Please sign in to comment.