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(Validium): refactor batch commit data generation #1325

Conversation

Oppen
Copy link
Contributor

@Oppen Oppen commented Mar 1, 2024

What ❔

Reworks L1BatchCommitDataGenerator, CommitBatches and CommitBatchInfo.
This PR moves most of the encoding responsibility to per-operation-mode
structures derived from CommitBatches and CommitBatchInfo, modifying
their Tokenize implementations. In turn, the L1BatchCommitDataGenerator
implementations now differ only on which variant they instantiate to
delegate the request for tokenization.
After that, all aggregation logic and data generators is moved to eth_sender.

The goal is to apply these recommendations.

NOTE: the following tests are failing both here and in the base branch:

     Summary [ 230.521s] 853 tests run: 850 passed, 3 failed, 2 skipped
        FAIL [   1.969s] zksync_core eth_sender::tests::correct_order_for_confirmations
        FAIL [   1.883s] zksync_core eth_sender::tests::skipped_l1_batch_at_the_start
        FAIL [   1.848s] zksync_core eth_sender::tests::skipped_l1_batch_in_the_middle

Why ❔

The original changes for the L1 publish behaviour were a bit disperse over the
codebase, leading to hard to understand code and a more complex dependency graph.

Checklist

  • PR title corresponds to the body of PR (we generate changelog entries from PRs).
  • Tests for the changes have been added / updated.
  • Documentation comments have been added / updated.
  • Code has been formatted via zk fmt and zk lint.
  • Spellcheck has been run via zk spellcheck.
  • Linkcheck has been run via zk linkcheck.

@ilitteri ilitteri requested review from popzxc and ilitteri March 1, 2024 21:04
@ilitteri ilitteri changed the title feat: refactor batch commit data generation feat(Validium): refactor batch commit data generation Mar 1, 2024
@Oppen Oppen marked this pull request as draft March 1, 2024 22:03
@Oppen Oppen marked this pull request as ready for review March 4, 2024 17:51
Oppen added 10 commits March 4, 2024 15:19
- First splits the functionality of
  `L1CommitBatchesDataGenerator::l1_commit_data` into two separate
  functions, one for a single batch and one for the whole array that
  includes the previous block.
- Then uses that for publish criteria and consistency checking, as they
  work on a per-batch basis.
@Oppen Oppen force-pushed the feat_validium_data_commit_abstraction_v2 branch from 693f4c4 to 2192b61 Compare March 4, 2024 18:19
Copy link
Member

@popzxc popzxc left a comment

Choose a reason for hiding this comment

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

Nice progress here!

@Oppen Oppen requested a review from popzxc March 5, 2024 14:42
@ilitteri ilitteri merged commit 941639d into matter-labs:feat_validium_pubdata_abstraction Mar 5, 2024
2 checks passed
@ilitteri ilitteri deleted the feat_validium_data_commit_abstraction_v2 branch March 5, 2024 18:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants