Skip to content

Commit 976062a

Browse files
spalladinospalladino
committed
fix(fast-inbox): validate compact L1-to-L2 message indices in the archiver (A-1384)
The flip made the Inbox emit a compact zero-based totalMessagesInserted index, but the archiver message store still validated the legacy per-checkpoint tree-geometry range, so it rejected every real message with 'Invalid index 0 ... expected value in range [..)'. Replace the checkpoint range/gap/first-index checks with the compact contiguous check (index == previous + 1, first == 0), mirroring the eventual node-cleanup. The legacy 128-bit rolling-hash check, the checkpointNumber field and the vestigial per-checkpoint getter stay until that cleanup. Move the test fixtures to compact indices (keeping checkpointNumber/rollingHash) and drop the store tests that only exercised the geometry getter / removed checks.
1 parent 4121ad7 commit 976062a

3 files changed

Lines changed: 21 additions & 169 deletions

File tree

‎yarn-project/archiver/src/store/message_store.test.ts‎

Lines changed: 5 additions & 133 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import { NUMBER_OF_L1_L2_MESSAGES_PER_ROLLUP } from '@aztec/constants';
21
import { CheckpointNumber } from '@aztec/foundation/branded-types';
32
import { Buffer16, Buffer32 } from '@aztec/foundation/buffer';
43
import { Fr } from '@aztec/foundation/curves/bn254';
@@ -7,7 +6,6 @@ import { openTmpStore } from '@aztec/kv-store/lmdb-v2';
76
import { Checkpoint, type PublishedCheckpoint } from '@aztec/stdlib/checkpoint';
87
import '@aztec/stdlib/testing/jest';
98

10-
import { L1ToL2MessagesNotReadyError } from '../errors.js';
119
import type { InboxMessage } from '../structs/inbox_message.js';
1210
import {
1311
makeInboxMessage,
@@ -103,25 +101,20 @@ describe('MessageStore', () => {
103101
await messageStore.addL1ToL2Messages([msg]);
104102

105103
await checkMessages([msg]);
106-
expect(await messageStore.getL1ToL2Messages(CheckpointNumber(1))).toEqual([msg.leaf]);
107104
});
108105

109106
it('stores single message', async () => {
110107
const msg = makeInboxMessage(Buffer16.ZERO, { checkpointNumber: CheckpointNumber(2) });
111108
await messageStore.addL1ToL2Messages([msg]);
112109

113110
await checkMessages([msg]);
114-
expect(await messageStore.getL1ToL2Messages(CheckpointNumber(2))).toEqual([msg.leaf]);
115111
});
116112

117-
it('stores and returns messages across different blocks', async () => {
113+
it('stores messages across different blocks', async () => {
118114
const msgs = makeInboxMessages(5, { initialCheckpointNumber });
119115
await messageStore.addL1ToL2Messages(msgs);
120116

121117
await checkMessages(msgs);
122-
expect(await messageStore.getL1ToL2Messages(CheckpointNumber(initialCheckpointNumber + 2))).toEqual(
123-
[msgs[2]].map(m => m.leaf),
124-
);
125118
});
126119

127120
it('stores the same messages again', async () => {
@@ -132,56 +125,25 @@ describe('MessageStore', () => {
132125
await checkMessages(msgs);
133126
});
134127

135-
it('stores and returns messages across different blocks with gaps', async () => {
136-
const msgs1 = makeInboxMessages(3, { initialCheckpointNumber: CheckpointNumber(1) });
137-
const msgs2 = makeInboxMessages(3, {
138-
initialCheckpointNumber: CheckpointNumber(20),
139-
initialHash: msgs1.at(-1)!.rollingHash,
140-
initialInboxHash: msgs1.at(-1)!.inboxRollingHash,
141-
});
142-
143-
await messageStore.addL1ToL2Messages(msgs1);
144-
await messageStore.addL1ToL2Messages(msgs2);
145-
146-
await checkMessages([...msgs1, ...msgs2]);
147-
148-
expect(await messageStore.getL1ToL2Messages(CheckpointNumber(1))).toEqual([msgs1[0].leaf]);
149-
expect(await messageStore.getL1ToL2Messages(CheckpointNumber(4))).toEqual([]);
150-
expect(await messageStore.getL1ToL2Messages(CheckpointNumber(20))).toEqual([msgs2[0].leaf]);
151-
expect(await messageStore.getL1ToL2Messages(CheckpointNumber(24))).toEqual([]);
152-
});
153-
154-
it('stores and returns messages with block numbers larger than a byte', async () => {
128+
it('stores messages with block numbers larger than a byte', async () => {
155129
const msgs = makeInboxMessages(5, { initialCheckpointNumber: CheckpointNumber(1000) });
156130
await messageStore.addL1ToL2Messages(msgs);
157131

158132
await checkMessages(msgs);
159-
expect(await messageStore.getL1ToL2Messages(CheckpointNumber(1002))).toEqual([msgs[2]].map(m => m.leaf));
160133
});
161134

162-
it('stores and returns multiple messages per block', async () => {
135+
it('stores multiple messages per block', async () => {
163136
const msgs = makeInboxMessagesWithFullBlocks(4);
164137
await messageStore.addL1ToL2Messages(msgs);
165138

166139
await checkMessages(msgs);
167-
const blockMessages = await messageStore.getL1ToL2Messages(CheckpointNumber(initialCheckpointNumber + 1));
168-
expect(blockMessages).toHaveLength(NUMBER_OF_L1_L2_MESSAGES_PER_ROLLUP);
169-
expect(blockMessages).toEqual(
170-
msgs.slice(NUMBER_OF_L1_L2_MESSAGES_PER_ROLLUP, NUMBER_OF_L1_L2_MESSAGES_PER_ROLLUP * 2).map(m => m.leaf),
171-
);
172140
});
173141

174142
it('stores messages in multiple operations', async () => {
175143
const msgs = makeInboxMessages(20, { initialCheckpointNumber });
176144
await messageStore.addL1ToL2Messages(msgs.slice(0, 10));
177145
await messageStore.addL1ToL2Messages(msgs.slice(10, 20));
178146

179-
expect(await messageStore.getL1ToL2Messages(CheckpointNumber(initialCheckpointNumber + 2))).toEqual(
180-
[msgs[2]].map(m => m.leaf),
181-
);
182-
expect(await messageStore.getL1ToL2Messages(CheckpointNumber(initialCheckpointNumber + 12))).toEqual(
183-
[msgs[12]].map(m => m.leaf),
184-
);
185147
await checkMessages(msgs);
186148
});
187149

@@ -206,13 +168,6 @@ describe('MessageStore', () => {
206168
await expect(messageStore.addL1ToL2Messages(msgs)).rejects.toThrow(MessageStoreError);
207169
});
208170

209-
it('throws if block number for the first message is out of order', async () => {
210-
const msgs = makeInboxMessages(4, { initialCheckpointNumber });
211-
msgs[2].checkpointNumber = CheckpointNumber(initialCheckpointNumber - 1);
212-
await messageStore.addL1ToL2Messages(msgs.slice(0, 2));
213-
await expect(messageStore.addL1ToL2Messages(msgs.slice(2, 4))).rejects.toThrow(MessageStoreError);
214-
});
215-
216171
it('throws if rolling hash is not correct', async () => {
217172
const msgs = makeInboxMessages(5);
218173
msgs[1].rollingHash = Buffer16.random();
@@ -226,108 +181,25 @@ describe('MessageStore', () => {
226181
await expect(messageStore.addL1ToL2Messages(msgs.slice(2, 4))).rejects.toThrow(MessageStoreError);
227182
});
228183

229-
it('throws if index is not in the correct range', async () => {
184+
it('throws if index skips ahead', async () => {
230185
const msgs = makeInboxMessages(5, { initialCheckpointNumber });
231186
msgs.at(-1)!.index += 100n;
232187
await expect(messageStore.addL1ToL2Messages(msgs)).rejects.toThrow(MessageStoreError);
233188
});
234189

235-
it('throws if first index in block has gaps', async () => {
190+
it('throws if index does not follow previous one', async () => {
236191
const msgs = makeInboxMessages(4, { initialCheckpointNumber });
237192
msgs[2].index++;
238193
await expect(messageStore.addL1ToL2Messages(msgs)).rejects.toThrow(MessageStoreError);
239194
});
240195

241-
it('throws if index does not follow previous one', async () => {
242-
const msgs = makeInboxMessages(2, {
243-
initialCheckpointNumber,
244-
overrideFn: (msg, i) => ({
245-
...msg,
246-
checkpointNumber: CheckpointNumber(2),
247-
index: BigInt(i + NUMBER_OF_L1_L2_MESSAGES_PER_ROLLUP * 2),
248-
}),
249-
});
250-
msgs[1].index++;
251-
await expect(messageStore.addL1ToL2Messages(msgs)).rejects.toThrow(MessageStoreError);
252-
});
253-
254-
it('removes messages up to the given block number', async () => {
255-
const msgs = makeInboxMessagesWithFullBlocks(4, { initialCheckpointNumber: CheckpointNumber(1) });
256-
257-
await messageStore.addL1ToL2Messages(msgs);
258-
await checkMessages(msgs);
259-
260-
expect(await messageStore.getL1ToL2Messages(CheckpointNumber(1))).toHaveLength(
261-
NUMBER_OF_L1_L2_MESSAGES_PER_ROLLUP,
262-
);
263-
expect(await messageStore.getL1ToL2Messages(CheckpointNumber(2))).toHaveLength(
264-
NUMBER_OF_L1_L2_MESSAGES_PER_ROLLUP,
265-
);
266-
expect(await messageStore.getL1ToL2Messages(CheckpointNumber(3))).toHaveLength(
267-
NUMBER_OF_L1_L2_MESSAGES_PER_ROLLUP,
268-
);
269-
expect(await messageStore.getL1ToL2Messages(CheckpointNumber(4))).toHaveLength(
270-
NUMBER_OF_L1_L2_MESSAGES_PER_ROLLUP,
271-
);
272-
273-
await messageStore.rollbackL1ToL2MessagesToCheckpoint(CheckpointNumber(2));
274-
275-
expect(await messageStore.getL1ToL2Messages(CheckpointNumber(1))).toHaveLength(
276-
NUMBER_OF_L1_L2_MESSAGES_PER_ROLLUP,
277-
);
278-
expect(await messageStore.getL1ToL2Messages(CheckpointNumber(2))).toHaveLength(
279-
NUMBER_OF_L1_L2_MESSAGES_PER_ROLLUP,
280-
);
281-
expect(await messageStore.getL1ToL2Messages(CheckpointNumber(3))).toHaveLength(0);
282-
expect(await messageStore.getL1ToL2Messages(CheckpointNumber(4))).toHaveLength(0);
283-
284-
await checkMessages(msgs.slice(0, NUMBER_OF_L1_L2_MESSAGES_PER_ROLLUP * 2));
285-
});
286-
287196
it('removes messages starting with the given index', async () => {
288197
const msgs = makeInboxMessagesWithFullBlocks(4, { initialCheckpointNumber: CheckpointNumber(1) });
289198
await messageStore.addL1ToL2Messages(msgs);
290199

291200
await messageStore.removeL1ToL2Messages(msgs[13].index);
292201
await checkMessages(msgs.slice(0, 13));
293202
});
294-
295-
describe('inbox tree in progress guard', () => {
296-
it('throws when checkpointNumber >= treeInProgress', async () => {
297-
const msgs = makeInboxMessages(3, { initialCheckpointNumber: CheckpointNumber(5) });
298-
await messageStore.addL1ToL2Messages(msgs);
299-
300-
// Set treeInProgress to 7, meaning checkpoints 5 and 6 are sealed, 7+ are not
301-
await messageStore.setMessageSyncState({ l1BlockNumber: 1n, l1BlockHash: Buffer32.random() }, 7n);
302-
303-
// Sealed checkpoint should succeed
304-
await expect(messageStore.getL1ToL2Messages(CheckpointNumber(5))).resolves.toEqual([msgs[0].leaf]);
305-
306-
// Unsealed checkpoint (== treeInProgress) should throw
307-
await expect(messageStore.getL1ToL2Messages(CheckpointNumber(7))).rejects.toThrow(L1ToL2MessagesNotReadyError);
308-
309-
// Future checkpoint should also throw
310-
await expect(messageStore.getL1ToL2Messages(CheckpointNumber(8))).rejects.toThrow(L1ToL2MessagesNotReadyError);
311-
});
312-
313-
it('returns messages when checkpointNumber < treeInProgress', async () => {
314-
const msgs = makeInboxMessages(3, { initialCheckpointNumber: CheckpointNumber(10) });
315-
await messageStore.addL1ToL2Messages(msgs);
316-
317-
await messageStore.setMessageSyncState({ l1BlockNumber: 1n, l1BlockHash: Buffer32.random() }, 13n);
318-
319-
await expect(messageStore.getL1ToL2Messages(CheckpointNumber(10))).resolves.toEqual([msgs[0].leaf]);
320-
await expect(messageStore.getL1ToL2Messages(CheckpointNumber(11))).resolves.toEqual([msgs[1].leaf]);
321-
});
322-
323-
it('skips guard when treeInProgress is not set', async () => {
324-
const msgs = makeInboxMessages(2, { initialCheckpointNumber: CheckpointNumber(1) });
325-
await messageStore.addL1ToL2Messages(msgs);
326-
327-
// No setMessageSyncState call — guard should be permissive
328-
await expect(messageStore.getL1ToL2Messages(CheckpointNumber(1))).resolves.toEqual([msgs[0].leaf]);
329-
});
330-
});
331203
});
332204

333205
describe('Inbox buckets', () => {

‎yarn-project/archiver/src/store/message_store.ts‎

Lines changed: 6 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -215,37 +215,14 @@ export class MessageStore {
215215
);
216216
}
217217

218-
// Check index corresponds to the checkpoint number.
219-
const [expectedStart, expectedEnd] = InboxLeaf.indexRangeForCheckpoint(message.checkpointNumber);
220-
if (message.index < expectedStart || message.index >= expectedEnd) {
218+
// Check the compact-indexed messages arrive contiguously (AZIP-22 Fast Inbox): the global insertion index of
219+
// each message is exactly one past the previous one, independent of the checkpoint it landed in. The flipped
220+
// Inbox emits this compact totalMessagesInserted index, so the legacy per-checkpoint range no longer applies.
221+
const expectedIndex = lastMessage === undefined ? 0n : lastMessage.index + 1n;
222+
if (message.index !== expectedIndex) {
221223
throw new MessageStoreError(
222224
`Invalid index ${message.index} for incoming L1 to L2 message ${message.leaf.toString()} ` +
223-
`at checkpoint ${message.checkpointNumber} (expected value in range [${expectedStart}, ${expectedEnd}))`,
224-
message,
225-
);
226-
}
227-
228-
// Check there are no gaps in the indices within the same checkpoint.
229-
if (
230-
lastMessage &&
231-
message.checkpointNumber === lastMessage.checkpointNumber &&
232-
message.index !== lastMessage.index + 1n
233-
) {
234-
throw new MessageStoreError(
235-
`Missing prior message for incoming L1 to L2 message ${message.leaf.toString()} ` +
236-
`with index ${message.index}`,
237-
message,
238-
);
239-
}
240-
241-
// Check the first message in a checkpoint has the correct index.
242-
if (
243-
(!lastMessage || message.checkpointNumber > lastMessage.checkpointNumber) &&
244-
message.index !== expectedStart
245-
) {
246-
throw new MessageStoreError(
247-
`Message ${message.leaf.toString()} for checkpoint ${message.checkpointNumber} has wrong index ` +
248-
`${message.index} (expected ${expectedStart})`,
225+
`(expected ${expectedIndex})`,
249226
message,
250227
);
251228
}

‎yarn-project/archiver/src/test/mock_structs.ts‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import { AztecAddress } from '@aztec/stdlib/aztec-address';
1515
import { CommitteeAttestation, L2Block } from '@aztec/stdlib/block';
1616
import { Checkpoint, L1PublishedData, PublishedCheckpoint } from '@aztec/stdlib/checkpoint';
1717
import { PrivateLog, PublicLog, SiloedTag, Tag } from '@aztec/stdlib/logs';
18-
import { InboxLeaf, updateInboxRollingHash } from '@aztec/stdlib/messaging';
18+
import { updateInboxRollingHash } from '@aztec/stdlib/messaging';
1919
import { orderAttestations } from '@aztec/stdlib/p2p';
2020
import { CheckpointHeader } from '@aztec/stdlib/rollup';
2121
import { makeCheckpointAttestationFromCheckpoint } from '@aztec/stdlib/testing';
@@ -33,7 +33,8 @@ export function makeInboxMessage(
3333
const { l1BlockHash = Buffer32.random() } = overrides;
3434
const { leaf = Fr.random() } = overrides;
3535
const { rollingHash = updateRollingHash(previousRollingHash, leaf) } = overrides;
36-
const { index = InboxLeaf.smallestIndexForCheckpoint(checkpointNumber) } = overrides;
36+
// Compact global insertion index (AZIP-22 Fast Inbox): defaults to the first slot.
37+
const { index = 0n } = overrides;
3738
const { inboxRollingHash = updateInboxRollingHash(Fr.ZERO, leaf) } = overrides;
3839
// Default each message to its own bucket, keyed monotonically off its global index.
3940
const { bucketSeq = index + 1n } = overrides;
@@ -57,6 +58,7 @@ export function makeInboxMessages(
5758
opts: {
5859
initialHash?: Buffer16;
5960
initialInboxHash?: Fr;
61+
initialIndex?: bigint;
6062
initialCheckpointNumber?: CheckpointNumber;
6163
messagesPerCheckpoint?: number;
6264
overrideFn?: (msg: InboxMessage, index: number) => InboxMessage;
@@ -65,6 +67,7 @@ export function makeInboxMessages(
6567
const {
6668
initialHash = Buffer16.ZERO,
6769
initialInboxHash = Fr.ZERO,
70+
initialIndex = 0n,
6871
overrideFn = msg => msg,
6972
initialCheckpointNumber = CheckpointNumber(1),
7073
messagesPerCheckpoint = 1,
@@ -74,16 +77,16 @@ export function makeInboxMessages(
7477
let rollingHash = initialHash;
7578
let inboxRollingHash = initialInboxHash;
7679
for (let i = 0; i < totalCount; i++) {
77-
const msgIndex = i % messagesPerCheckpoint;
7880
const checkpointNumber = CheckpointNumber.fromBigInt(
7981
BigInt(initialCheckpointNumber) + BigInt(i) / BigInt(messagesPerCheckpoint),
8082
);
8183
const leaf = Fr.random();
84+
// Compact global insertion index (AZIP-22 Fast Inbox): contiguous from initialIndex, independent of checkpoint.
8285
const message = overrideFn(
8386
makeInboxMessage(rollingHash, {
8487
leaf,
8588
checkpointNumber,
86-
index: InboxLeaf.smallestIndexForCheckpoint(checkpointNumber) + BigInt(msgIndex),
89+
index: initialIndex + BigInt(i),
8790
inboxRollingHash: updateInboxRollingHash(inboxRollingHash, leaf),
8891
}),
8992
i,
@@ -102,13 +105,13 @@ export function makeInboxMessagesWithFullBlocks(
102105
): InboxMessage[] {
103106
const { initialCheckpointNumber = CheckpointNumber(13) } = opts;
104107
return makeInboxMessages(NUMBER_OF_L1_L2_MESSAGES_PER_ROLLUP * blockCount, {
108+
// Keep the compact global index from makeInboxMessages; only spread the (now-vestigial) checkpoint assignment
109+
// across blocks so multi-block coverage still exercises differing checkpoint numbers.
105110
overrideFn: (msg, i) => {
106111
const checkpointNumber = CheckpointNumber(
107112
initialCheckpointNumber + Math.floor(i / NUMBER_OF_L1_L2_MESSAGES_PER_ROLLUP),
108113
);
109-
const index =
110-
InboxLeaf.smallestIndexForCheckpoint(checkpointNumber) + BigInt(i % NUMBER_OF_L1_L2_MESSAGES_PER_ROLLUP);
111-
return { ...msg, checkpointNumber, index };
114+
return { ...msg, checkpointNumber };
112115
},
113116
});
114117
}

0 commit comments

Comments
 (0)