Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: optimise l1 to l2 message fetching #8672

Merged
merged 2 commits into from
Sep 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -37,6 +37,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 @@ -89,6 +93,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
Loading