Skip to content
This repository was archived by the owner on Apr 18, 2025. It is now read-only.

Fix: L1Msg tx is allowed to skip by sequencer.#759

Closed
kunxian-xia wants to merge 10 commits into
developfrom
fix/allow-skip-l1msg
Closed

Fix: L1Msg tx is allowed to skip by sequencer.#759
kunxian-xia wants to merge 10 commits into
developfrom
fix/allow-skip-l1msg

Conversation

@kunxian-xia
Copy link
Copy Markdown

@kunxian-xia kunxian-xia commented Aug 12, 2023

Description

This PR makes two changes

  • num_txs in the calculation of chunk_data_hash is updated to the new definition.
  • do not change block table's assignment.
  • calculation of num_l1_msgs and num_l2_txs are done in tx circuit and copy their sum back to pi circuit.

Issue Link

The rollup contract allows the sequencer to skip "malicious" L1 enforced tx (i.e. L1Msg tx) that could cause circuit overflow. Therefore the semantic of num_txs in a chunk is different from the one we have right now.

  • Previously, it means that num_txs = len(chunk.txs).
  • However, according to the latest go impl of relayer, num_txs = num_l1_msgs + num_l2_txs and num_l1_msgs count in these skipped L1 msgs. This means that the num of txs that are handled by the EVM circuit does not necessarily equal to the new num_txs definition.

Previously we rely on the assumption that num_txs = len(chunk.txs) to make sure that

  1. all txs in each chunk's block are included in the tx table.
  2. each tx's block number is correct. You can't assign a wrong (BlockNumber, tx_id, tx.block_num+1) row in tx table.

The rules that we use (you can find more details at #733) is

  1. we use block table to get each block's cum_num_txs, prev_cum_num_txs and prev_cum_num_txs = cum_num_txs - num_txs.
  2. each tx must satisfy that tx.block.prev_cum_num_txs < tx_id <= tx.block.cum_num_txs.
  3. tx_id in tx table is continuous, i.e. tx_id' = tx_id + 1 (it's strictly increasing by 1).

Note that the evm circuit also relies on the rules 2 & 3 to make sure that txs in chunk cannot be skipped.

  1. EndTx -> BeginTx: next.state.tx_id = curr.state.tx_id + 1;
  2. EndTx -> EndInnerBlock: curr.state.tx_id == curr.block.cum_num_txs`.

However this rule no longer holds if we use the new definition of num_txs in block table to get each block's cum_num_txs, prev_cum_num_txs. We give an example of how block table and tx table will look like below.

  • block table

    block num num_txs prev_cnt cnt
    1 5 0 5
    2 3 5 8
  • tx table

    tx id block_num is_l1_msg queue_index total_l1_popped prev_cnt cnt
    1 1 true 3 2 0 5
    2 1 false 4 0 5
    3 1 false 4 0 5
    4 1 false 4 0 5
    5 2 true 6 4 5 8

It's easy to see that the 5th row will fail the 2nd rule. We can use a simple method to fix this issue:

allow the tx_ids are not continuous, they are allowed to contain gap. That is, tx_id' > tx_id.

The tx table using this method will look like:

tx id block_num is_l1_msg queue_index total_l1_popped prev_cnt cnt
1 1 true 3 2 0 5
3 1 false 4 0 5
4 1 false 4 0 5
5 1 false 4 0 5
8 2 true 6 4 5 8

It's easy to see that we can now pass the rule 2's check. But this method breaks the 3rd rule that evm circuit relies on. This means that we also need make changes to evm circuit (oops not good) which is not desirable.

Design choice

We want to keep the changes as small as possible. For example, we don't want to make changes to evm circuit. To achieve that, we decide to keep the original semantic of num_txs, cum_num_txs in block table (do not use num_l1_msgs + num_l2_txs as num_txs).

That is, the new definition is only applied in the calculation of chunk_data_hash. And we count the num_l1_msgs and num_l2_txs in tx table. And then enforce that the sum equals to the num_txs in pi column using copy constraints.

Continuing our previous example, block table assigned by this method will look like:

  • block table

    block num num_txs prev_cnt cnt
    1 4 0 4
    2 1 4 5
  • tx table

    tx id block_num is_l1_msg queue_index total_l1_popped prev_cnt cnt
    1 1 true 3 2 0 4
    2 1 false 4 0 4
    3 1 false 4 0 4
    4 1 false 4 0 4
    5 2 true 6 4 4 5

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

@lispc
Copy link
Copy Markdown

lispc commented Aug 14, 2023

oh i have picked the patch from v0.5.x to develop already...

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants