Insert L1 to L2 messages in the tree before public execution in public rollup circuits

[spalladino]

We have a lingering issue with L1 to L2 messages: if a tx consumes an L1 to L2 message on the same block it is seen by the archiver, then that block is not provable. This is because of a mismatch between how the sequencer and the prover node work.

The sequencer uses the block_builder, which when starting a new block, first inserts all L1 to L2 messages into the tree to make them available, so when txs are processed by the public_processor, they can access the messages.

aztec-packages/yarn-project/sequencer-client/src/sequencer/block_builder.ts

Lines 65 to 73 in 45e1fad

	await blockFactory.startNewBlock(newGlobalVariables, l1ToL2Messages);
	const [publicProcessorDuration, [processedTxs, failedTxs, usedTxs]] = await elapsed(() =>
	processor.process(pendingTxs, opts, validator),
	);
	// All real transactions have been added, set the block as full and pad if needed
	await blockFactory.addTxs(processedTxs);
	const block = await blockFactory.setBlockCompleted();

However, the prover node doesn't. It forks world-state immediately before the block it wants to prove, and then calls the public_processor without first adding the L1 to L2 messages.

aztec-packages/yarn-project/prover-node/src/job/epoch-proving-job.ts

Lines 153 to 158 in 45e1fad

	// Process public fns
	const db = await this.dbProvider.fork(block.number - 1);
	const publicProcessor = this.publicProcessorFactory.create(db, globalVariables, true);
	const processed = await this.processTxs(publicProcessor, txs);
	await this.prover.addTxs(processed);
	await db.close();

So, in #16375 we changed it so that we do add the L1 to L2 messages before we start processing. But then we started getting this error:

[17:19:39.610] ERROR: prover-client:broker-circuit-prover-facade Job errored with 'Circuit execution failed: last_l1_to_l2 in constants does not match the value in the previous block header' id=1:BLOCK_ROOT_ROLLUP:fa05417b0e9ee6e429e870e21962107691067177f857f0b33fb3869c7399bb2a type=SINGLE_TX_BLOCK_ROOT_ROLLUP

This error comes from a mismatch between the previous_rollup_data[0].base_or_merge_rollup_public_inputs.constants.last_l1_to_l2 and data.previous_block_header.state.l1_to_l2_message_tree from the SingleTxBlockRootRollupInputs (if I managed to follow the data flow correctly).

aztec-packages/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/block_root/components/block_root_rollup_inputs_validator.nr

Lines 111 to 116 in 45e1fad

	// Validate last_l1_to_l2.
	assert_eq(
	constants.last_l1_to_l2,
	previous_block_header.state.l1_to_l2_message_tree,
	"last_l1_to_l2 in constants does not match the value in the previous block header",
	);

Now, the constants.last_l1_to_l2 in the public base rollup gets populated from the AVM start_tree_snapshots in the public base rollup:

aztec-packages/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/base/public_base_rollup.nr

Line 92 in 45e1fad

last_l1_to_l2: avm_data.start_tree_snapshots.l1_to_l2_message_tree,

So, if I understand correctly, this means that circuits are expecting that the L1 to L2 message tree does not change between the end of the previous block and the start of AVM execution, which would contradict what the sequencer is doing by inserting messages before tx execution starts.

So my question is: what is the expected behaviour? Should we be injecting the L1 to L2 messages for a given block into their tree before we start tx public execution, or at the end?

[iAmMichaelConnor]

It's always been my mental model that we should inject the L1->L2 messages into the tree at the start; before we begin public execution (and before private execution... although perhaps "before private execition" isn't possible anymore, because I think we now read the L1->L2 root from the anchor block instead of as a standalone root).
I.e. we should make the time between sending and consuming an L1->L2 message as small as possible.

[LeilaWang]

Yeah "before private execution" is not possible.

As for public, the circuits currently behave the other way: the avm receives the last l1_to_l2 root, which is the root at the end of the previous block. Then in the current block root rollup circuit, it validates that the l1_to_l2 root used by the avm matches the value in the previous archive, then the circuit builds the new root with the given l1_to_l2 messages.

I will change the circuits to take the new l1_to_l2 root from avm instead.

Though it's super weird that the new test in #16375 passes 🤔

[spalladino]

I will change the circuits to take the new l1_to_l2 root from avm instead.

Thanks @LeilaWang!

Though it's super weird that the new test in #16375 passes 🤔

My bet is that the test is not doing what it's supposed to do, that's why we end up seeing the error pop up in a different e2e test by sheer luck. Let me know if you can check that as part of the circuit work, I can take a look if not!

[dbanks12]

This sounds good to me! And I don't think it really has implications for the AVM since it just changes what root we're getting from public base