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

feat(Inbox)!: make index in inbox global #9110

Merged
merged 5 commits into from
Oct 10, 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
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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

having the L2Block number here is redundant information now as it can be derived from the index. For this reason I would nuke it from the event.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spoke with Lasse and we think we should keep the l2block in the event as it helps people search for their index on L1!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also helps with alpha team to make messages unique


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
Loading