Skip to content

Commit

Permalink
feat: make index in inbox global (#9110)
Browse files Browse the repository at this point in the history
fix #9085
  • Loading branch information
rahul-kothari authored Oct 10, 2024
1 parent c33d38b commit 375c017
Show file tree
Hide file tree
Showing 15 changed files with 107 additions and 94 deletions.
4 changes: 4 additions & 0 deletions docs/docs/migration_notes.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ keywords: [sandbox, aztec, notes, migration, updating, upgrading]

Aztec is in full-speed development. Literally every version breaks compatibility with the previous ones. This page attempts to target errors and difficulties you might encounter when upgrading, and how to resolve them.

## 0.58.0
### [l1-contracts] Inbox's MessageSent event emits global tree index
Earlier `MessageSent` event in Inbox emitted a subtree index (index of the message in the subtree of the l2Block). But the nodes and Aztec.nr expects the index in the global L1_TO_L2_MESSAGES_TREE. So to make it easier to parse this, Inbox now emits this global index.

## 0.57.0

### Changes to PXE API and `ContractFunctionInteraction``
Expand Down
4 changes: 2 additions & 2 deletions l1-contracts/src/core/interfaces/messagebridge/IInbox.sol
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@ interface IInbox {
/**
* @notice Emitted when a message is sent
* @param l2BlockNumber - The L2 block number in which the message is included
* @param index - The index of the message in the block
* @param index - The index of the message in the L1 to L2 messages tree
* @param hash - The hash of the message
*/
event MessageSent(uint256 indexed l2BlockNumber, uint256 index, bytes32 hash);
event MessageSent(uint256 indexed l2BlockNumber, uint256 index, bytes32 indexed hash);

// docs:start:send_l1_to_l2_message
/**
Expand Down
6 changes: 5 additions & 1 deletion l1-contracts/src/core/messagebridge/Inbox.sol
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,11 @@ contract Inbox is IInbox {
});

bytes32 leaf = message.sha256ToField();
uint256 index = currentTree.insertLeaf(leaf);
// this is the global leaf index and not index in the l2Block subtree
// such that users can simply use it and don't need access to a node if they are to consume it in public.
// trees are constant size so global index = tree number * size + subtree index
uint256 index =
(inProgress - Constants.INITIAL_L2_BLOCK_NUM) * SIZE + currentTree.insertLeaf(leaf);
totalMessagesInserted++;
emit MessageSent(inProgress, index, leaf);

Expand Down
9 changes: 6 additions & 3 deletions l1-contracts/test/Inbox.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ contract InboxTest is Test {
using Hash for DataStructures.L1ToL2Msg;

uint256 internal constant FIRST_REAL_TREE_NUM = Constants.INITIAL_L2_BLOCK_NUM + 1;
// We set low depth (5) to ensure we sufficiently test the tree transitions
uint256 internal constant HEIGHT = 5;
uint256 internal constant SIZE = 2 ** HEIGHT;

InboxHarness internal inbox;
uint256 internal version = 0;
Expand All @@ -23,8 +26,7 @@ contract InboxTest is Test {

function setUp() public {
address rollup = address(this);
// We set low depth (5) to ensure we sufficiently test the tree transitions
inbox = new InboxHarness(rollup, 5);
inbox = new InboxHarness(rollup, HEIGHT);
emptyTreeRoot = inbox.getEmptyRoot();
}

Expand Down Expand Up @@ -87,7 +89,8 @@ contract InboxTest is Test {
bytes32 leaf = message.sha256ToField();
vm.expectEmit(true, true, true, true);
// event we expect
emit IInbox.MessageSent(FIRST_REAL_TREE_NUM, 0, leaf);
uint256 globalLeafIndex = (FIRST_REAL_TREE_NUM - 1) * SIZE;
emit IInbox.MessageSent(FIRST_REAL_TREE_NUM, globalLeafIndex, leaf);
// event we will get
bytes32 insertedLeaf =
inbox.sendL2Message(message.recipient, message.content, message.secretHash);
Expand Down
7 changes: 5 additions & 2 deletions l1-contracts/test/portals/TokenPortal.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ contract TokenPortalTest is Test {
event MessageConsumed(bytes32 indexed messageHash, address indexed recipient);

uint256 internal constant FIRST_REAL_TREE_NUM = Constants.INITIAL_L2_BLOCK_NUM + 1;
uint256 internal constant L1_TO_L2_MSG_SUBTREE_SIZE = 2 ** Constants.L1_TO_L2_MSG_SUBTREE_HEIGHT;

Registry internal registry;

Expand Down Expand Up @@ -122,7 +123,8 @@ contract TokenPortalTest is Test {
// Check the event was emitted
vm.expectEmit(true, true, true, true);
// event we expect
emit IInbox.MessageSent(FIRST_REAL_TREE_NUM, 0, expectedLeaf);
uint256 globalLeafIndex = (FIRST_REAL_TREE_NUM - 1) * L1_TO_L2_MSG_SUBTREE_SIZE;
emit IInbox.MessageSent(FIRST_REAL_TREE_NUM, globalLeafIndex, expectedLeaf);
// event we will get

// Perform op
Expand All @@ -147,7 +149,8 @@ contract TokenPortalTest is Test {
// Check the event was emitted
vm.expectEmit(true, true, true, true);
// event we expect
emit IInbox.MessageSent(FIRST_REAL_TREE_NUM, 0, expectedLeaf);
uint256 globalLeafIndex = (FIRST_REAL_TREE_NUM - 1) * L1_TO_L2_MSG_SUBTREE_SIZE;
emit IInbox.MessageSent(FIRST_REAL_TREE_NUM, globalLeafIndex, expectedLeaf);

// Perform op
bytes32 leaf = tokenPortal.depositToAztecPublic(to, amount, secretHashForL2MessageConsumption);
Expand Down
41 changes: 30 additions & 11 deletions yarn-project/archiver/src/archiver/archiver.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import {
EncryptedL2BlockL2Logs,
EncryptedNoteL2BlockL2Logs,
InboxLeaf,
L2Block,
LogType,
UnencryptedL2BlockL2Logs,
Expand Down Expand Up @@ -115,16 +116,19 @@ describe('Archiver', () => {
inboxRead.totalMessagesInserted.mockResolvedValueOnce(2n).mockResolvedValueOnce(6n);

mockGetLogs({
messageSent: [makeMessageSentEvent(98n, 1n, 0n), makeMessageSentEvent(99n, 1n, 1n)],
messageSent: [
makeMessageSentEventWithIndexInL2BlockSubtree(98n, 1n, 0n),
makeMessageSentEventWithIndexInL2BlockSubtree(99n, 1n, 1n),
],
L2BlockProposed: [makeL2BlockProposedEvent(101n, 1n, blocks[0].archive.root.toString())],
});

mockGetLogs({
messageSent: [
makeMessageSentEvent(2504n, 2n, 0n),
makeMessageSentEvent(2505n, 2n, 1n),
makeMessageSentEvent(2505n, 2n, 2n),
makeMessageSentEvent(2506n, 3n, 1n),
makeMessageSentEventWithIndexInL2BlockSubtree(2504n, 2n, 0n),
makeMessageSentEventWithIndexInL2BlockSubtree(2505n, 2n, 1n),
makeMessageSentEventWithIndexInL2BlockSubtree(2505n, 2n, 2n),
makeMessageSentEventWithIndexInL2BlockSubtree(2506n, 3n, 1n),
],
L2BlockProposed: [
makeL2BlockProposedEvent(2510n, 2n, blocks[1].archive.root.toString()),
Expand Down Expand Up @@ -222,7 +226,10 @@ describe('Archiver', () => {
inboxRead.totalMessagesInserted.mockResolvedValueOnce(2n).mockResolvedValueOnce(2n);

mockGetLogs({
messageSent: [makeMessageSentEvent(66n, 1n, 0n), makeMessageSentEvent(68n, 1n, 1n)],
messageSent: [
makeMessageSentEventWithIndexInL2BlockSubtree(66n, 1n, 0n),
makeMessageSentEventWithIndexInL2BlockSubtree(68n, 1n, 1n),
],
L2BlockProposed: [
makeL2BlockProposedEvent(70n, 1n, blocks[0].archive.root.toString()),
makeL2BlockProposedEvent(80n, 2n, blocks[1].archive.root.toString()),
Expand Down Expand Up @@ -262,7 +269,10 @@ describe('Archiver', () => {
inboxRead.totalMessagesInserted.mockResolvedValueOnce(0n).mockResolvedValueOnce(2n);

mockGetLogs({
messageSent: [makeMessageSentEvent(66n, 1n, 0n), makeMessageSentEvent(68n, 1n, 1n)],
messageSent: [
makeMessageSentEventWithIndexInL2BlockSubtree(66n, 1n, 0n),
makeMessageSentEventWithIndexInL2BlockSubtree(68n, 1n, 1n),
],
L2BlockProposed: [
makeL2BlockProposedEvent(70n, 1n, blocks[0].archive.root.toString()),
makeL2BlockProposedEvent(80n, 2n, blocks[1].archive.root.toString()),
Expand Down Expand Up @@ -319,7 +329,10 @@ describe('Archiver', () => {
.mockResolvedValueOnce(2n);

mockGetLogs({
messageSent: [makeMessageSentEvent(66n, 1n, 0n), makeMessageSentEvent(68n, 1n, 1n)],
messageSent: [
makeMessageSentEventWithIndexInL2BlockSubtree(66n, 1n, 0n),
makeMessageSentEventWithIndexInL2BlockSubtree(68n, 1n, 1n),
],
L2BlockProposed: [
makeL2BlockProposedEvent(70n, 1n, blocks[0].archive.root.toString()),
makeL2BlockProposedEvent(80n, 2n, blocks[1].archive.root.toString()),
Expand Down Expand Up @@ -367,7 +380,7 @@ describe('Archiver', () => {

// logs should be created in order of how archiver syncs.
const mockGetLogs = (logs: {
messageSent?: ReturnType<typeof makeMessageSentEvent>[];
messageSent?: ReturnType<typeof makeMessageSentEventWithIndexInL2BlockSubtree>[];
L2BlockProposed?: ReturnType<typeof makeL2BlockProposedEvent>[];
}) => {
if (logs.messageSent) {
Expand Down Expand Up @@ -396,10 +409,16 @@ function makeL2BlockProposedEvent(l1BlockNum: bigint, l2BlockNum: bigint, archiv
/**
* Makes fake L1ToL2 MessageSent events for testing purposes.
* @param l1BlockNum - L1 block number.
* @param l2BlockNumber - The L2 block number of in which the message was included.
* @param l2BlockNumber - The L2 block number for which the message was included.
* @param indexInSubtree - the index in the l2Block's subtree in the L1 to L2 Messages Tree.
* @returns MessageSent event logs.
*/
function makeMessageSentEvent(l1BlockNum: bigint, l2BlockNumber: bigint, index: bigint) {
function makeMessageSentEventWithIndexInL2BlockSubtree(
l1BlockNum: bigint,
l2BlockNumber: bigint,
indexInSubtree: bigint,
) {
const index = indexInSubtree + InboxLeaf.smallestIndexFromL2Block(l2BlockNumber);
return {
blockNumber: l1BlockNum,
args: {
Expand Down
26 changes: 11 additions & 15 deletions yarn-project/archiver/src/archiver/archiver_store_test_suite.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ export function describeArchiverDataStore(testName: string, getStore: () => Arch
it('returns the L1 block number that most recently added messages from inbox', async () => {
await store.addL1ToL2Messages({
lastProcessedL1BlockNumber: 1n,
retrievedData: [new InboxLeaf(0n, 0n, Fr.ZERO)],
retrievedData: [new InboxLeaf(1n, Fr.ZERO)],
});
await expect(store.getSynchPoint()).resolves.toEqual({
blocksSynchedTo: undefined,
Expand Down Expand Up @@ -226,7 +226,10 @@ export function describeArchiverDataStore(testName: string, getStore: () => Arch
const l1ToL2MessageSubtreeSize = 2 ** L1_TO_L2_MSG_SUBTREE_HEIGHT;

const generateBlockMessages = (blockNumber: bigint, numMessages: number) =>
Array.from({ length: numMessages }, (_, i) => new InboxLeaf(blockNumber, BigInt(i), Fr.random()));
Array.from(
{ length: numMessages },
(_, i) => new InboxLeaf(InboxLeaf.smallestIndexFromL2Block(blockNumber) + BigInt(i), Fr.random()),
);

it('returns messages in correct order', async () => {
const msgs = generateBlockMessages(l2BlockNumber, l1ToL2MessageSubtreeSize);
Expand All @@ -241,35 +244,28 @@ export function describeArchiverDataStore(testName: string, getStore: () => Arch
it('throws if it is impossible to sequence messages correctly', async () => {
const msgs = generateBlockMessages(l2BlockNumber, l1ToL2MessageSubtreeSize - 1);
// We replace a message with index 4 with a message with index at the end of the tree
// --> with that there will be a gap and it will be impossible to sequence the messages
msgs[4] = new InboxLeaf(l2BlockNumber, BigInt(l1ToL2MessageSubtreeSize - 1), Fr.random());
// --> with that there will be a gap and it will be impossible to sequence the
// end of tree = start of next tree/block - 1
msgs[4] = new InboxLeaf(InboxLeaf.smallestIndexFromL2Block(l2BlockNumber + 1n) - 1n, Fr.random());

await store.addL1ToL2Messages({ lastProcessedL1BlockNumber: 100n, retrievedData: msgs });
await expect(async () => {
await store.getL1ToL2Messages(l2BlockNumber);
}).rejects.toThrow(`L1 to L2 message gap found in block ${l2BlockNumber}`);
});

it('throws if adding more messages than fits into a block', async () => {
const msgs = generateBlockMessages(l2BlockNumber, l1ToL2MessageSubtreeSize + 1);

await expect(async () => {
await store.addL1ToL2Messages({ lastProcessedL1BlockNumber: 100n, retrievedData: msgs });
}).rejects.toThrow(`Message index ${l1ToL2MessageSubtreeSize} out of subtree range`);
});

it('correctly handles duplicate messages', async () => {
const messageHash = Fr.random();

const msgs = [new InboxLeaf(1n, 0n, messageHash), new InboxLeaf(2n, 0n, messageHash)];
const msgs = [new InboxLeaf(0n, messageHash), new InboxLeaf(16n, messageHash)];

await store.addL1ToL2Messages({ lastProcessedL1BlockNumber: 100n, retrievedData: msgs });

const index1 = (await store.getL1ToL2MessageIndex(messageHash, 0n))!;
expect(index1).toBe(0n);
const index2 = await store.getL1ToL2MessageIndex(messageHash, index1 + 1n);

expect(index2).toBeDefined();
expect(index2).toBeGreaterThan(index1);
expect(index2).toBe(16n);
});
});

Expand Down
4 changes: 2 additions & 2 deletions yarn-project/archiver/src/archiver/data_retrieval.ts
Original file line number Diff line number Diff line change
Expand Up @@ -194,8 +194,8 @@ export async function retrieveL1ToL2Messages(
}

for (const log of messageSentLogs) {
const { l2BlockNumber, index, hash } = log.args;
retrievedL1ToL2Messages.push(new InboxLeaf(l2BlockNumber!, index!, Fr.fromString(hash!)));
const { index, hash } = log.args;
retrievedL1ToL2Messages.push(new InboxLeaf(index!, Fr.fromString(hash!)));
}

// handles the case when there are no new messages:
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,5 @@
import { type InboxLeaf } from '@aztec/circuit-types';
import {
Fr,
INITIAL_L2_BLOCK_NUM,
L1_TO_L2_MSG_SUBTREE_HEIGHT,
NUMBER_OF_L1_L2_MESSAGES_PER_ROLLUP,
} from '@aztec/circuits.js';
import { InboxLeaf } from '@aztec/circuit-types';
import { Fr, L1_TO_L2_MSG_SUBTREE_HEIGHT } from '@aztec/circuits.js';
import { createDebugLogger } from '@aztec/foundation/log';
import { type AztecKVStore, type AztecMap, type AztecSingleton } from '@aztec/kv-store';

Expand Down Expand Up @@ -61,18 +56,11 @@ export class MessageStore {
void this.#lastSynchedL1Block.set(messages.lastProcessedL1BlockNumber);

for (const message of messages.retrievedData) {
if (message.index >= this.#l1ToL2MessagesSubtreeSize) {
throw new Error(`Message index ${message.index} out of subtree range`);
}
const key = `${message.blockNumber}-${message.index}`;
const key = `${message.index}`;
void this.#l1ToL2Messages.setIfNotExists(key, message.leaf.toBuffer());

const indexInTheWholeTree =
(message.blockNumber - BigInt(INITIAL_L2_BLOCK_NUM)) * BigInt(NUMBER_OF_L1_L2_MESSAGES_PER_ROLLUP) +
message.index;

const indices = this.#l1ToL2MessageIndices.get(message.leaf.toString()) ?? [];
indices.push(indexInTheWholeTree);
indices.push(message.index);
void this.#l1ToL2MessageIndices.set(message.leaf.toString(), indices);
}

Expand All @@ -98,9 +86,10 @@ export class MessageStore {
getL1ToL2Messages(blockNumber: bigint): Fr[] {
const messages: Fr[] = [];
let undefinedMessageFound = false;
for (let messageIndex = 0; messageIndex < this.#l1ToL2MessagesSubtreeSize; messageIndex++) {
const startIndex = Number(InboxLeaf.smallestIndexFromL2Block(blockNumber));
for (let i = startIndex; i < startIndex + this.#l1ToL2MessagesSubtreeSize; i++) {
// This is inefficient but probably fine for now.
const key = `${blockNumber}-${messageIndex}`;
const key = `${i}`;
const message = this.#l1ToL2Messages.get(key);
if (message) {
if (undefinedMessageFound) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ describe('l1_to_l2_message_store', () => {
it('adds a message and correctly returns its index', () => {
const blockNumber = 236n;
const msgs = Array.from({ length: 10 }, (_, i) => {
return new InboxLeaf(blockNumber, BigInt(i), Fr.random());
return new InboxLeaf(InboxLeaf.smallestIndexFromL2Block(blockNumber) + BigInt(i), Fr.random());
});
for (const m of msgs) {
store.addMessage(m);
Expand All @@ -25,17 +25,17 @@ describe('l1_to_l2_message_store', () => {
expect(retrievedMsgs.length).toEqual(10);

const msg = msgs[4];
const index = store.getMessageIndex(msg.leaf, 0n);
expect(index).toEqual(
(blockNumber - BigInt(INITIAL_L2_BLOCK_NUM)) * BigInt(NUMBER_OF_L1_L2_MESSAGES_PER_ROLLUP) + msg.index,
const expectedIndex = store.getMessageIndex(msg.leaf, 0n)!;
expect(expectedIndex).toEqual(
(blockNumber - BigInt(INITIAL_L2_BLOCK_NUM)) * BigInt(NUMBER_OF_L1_L2_MESSAGES_PER_ROLLUP) + 4n,
);
});

it('correctly handles duplicate messages', () => {
const messageHash = Fr.random();

store.addMessage(new InboxLeaf(1n, 0n, messageHash));
store.addMessage(new InboxLeaf(2n, 0n, messageHash));
store.addMessage(new InboxLeaf(0n, messageHash)); // l2 block 1
store.addMessage(new InboxLeaf(16n, messageHash)); // l2 block 2

const index1 = store.getMessageIndex(messageHash, 0n)!;
const index2 = store.getMessageIndex(messageHash, index1 + 1n);
Expand Down
Loading

0 comments on commit 375c017

Please sign in to comment.