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: Poll DB and sign L1 batches (BFT-474) #2399

Merged
merged 7 commits into from
Jul 9, 2024

Conversation

aakoshh
Copy link
Contributor

@aakoshh aakoshh commented Jul 8, 2024

What ❔

Implements PersistentBatchStore methods added in matter-labs/era-consensus#148

  • Add ConsensusDal::unsigned_batch_numbers to retrieve L1 batch numbers without a QC after a given minimum batch number
  • Add Store::unsigned_batch_numbers which return the unsigned batches that are max 10 batches older than the last one
  • Add ConsensusDal::get_batch_to_sign after finding out what to put into hash
  • Update ConsensusDal::insert_batch_certificate to validate the correct hash value
  • Test unsigned_batch_numbers
  • Test get_batch_to_sign and insert_batch_certificate
  • Point era-consensus at the latest version instead of https://github.com/matter-labs/era-consensus/tree/bft-474-pairing-prev-version then delete that branch

The hash check has been implemented against the StoredBatchInfo::hash() which isn't available in the DAL crate.

Why ❔

The consensus Executor will poll unsigned_batch_numbers and submit attestations over get_batch_to_sign. These methods decide 1) which batches to sign and 2) what content to sign.

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.

@aakoshh aakoshh changed the base branch from main to bft-476-l1-batch-qc-db July 8, 2024 12:12
@aakoshh aakoshh marked this pull request as ready for review July 8, 2024 13:56
@aakoshh
Copy link
Contributor Author

aakoshh commented Jul 9, 2024

@RomanBrodetski I think it's correct to sign the hash of StoredBatchInfo and not just the commitment field because this is a record that gets converted to RLP before hashing, it's meant to be sent to the L1 contract, and therefore the attestation should cover the whole content, not just one field in it.

It might even be enough to sign just this and not the hash of attester::Batch itself, since it already contains the number. Alas, that would mean we couldn't verify a BatchQC on its own and someone could change the number in it the message.

So Solidity would have to verify something like keccak256(abi.encode([batch.number, keccak256(abi.encode(<stored-batch-info>))])

@aakoshh aakoshh force-pushed the bft-474-poll-db-and-sign-batch branch from 5f43aff to ba0d8b2 Compare July 9, 2024 15:16
brunoffranca pushed a commit to matter-labs/era-consensus that referenced this pull request Jul 9, 2024
## What ❔

Followup for some design discussions in
#148

* Removes `BatchStore::unsigned_batch_numbers`
* Add `BatchStore::earliest_batch_number_to_sign`
* Add `BatchStore::latest_batch_number`
* Changes `AttesterRunner` to have a mutable range of
`earliest_batch_number` and `latest_batch_number` instead of querying
`unsigned_batch_numbers` and filtering it with a `min_batch_number`

## Why ❔

This might be more intuitive than the original version. 

Another motivation was that the current implementation of
`unsigned_batch_numbers` in
matter-labs/zksync-era#2399 has to do two
queries: 1) to figure out the latest batch number and 2) to find all
batches without certificates within a limited range of the latest batch.

Here we only query the latest batch number in each loop, and we forego
the check on whether a QC already exists, otherwise we'd be back at
doing two queries, or some ugly book keeping to only check it on the
first loop after a restart. It should be cheap enough to publish a
duplicate vote.
Base automatically changed from bft-476-l1-batch-qc-db to main July 9, 2024 16:20
@aakoshh aakoshh force-pushed the bft-474-poll-db-and-sign-batch branch from ba0d8b2 to 512a789 Compare July 9, 2024 16:24
brunoffranca pushed a commit to matter-labs/era-consensus that referenced this pull request Jul 9, 2024
## What ❔

Bumps the version to 0.1.0-rc.2

## Why ❔

So that we can merge matter-labs/zksync-era#2399
and matter-labs/zksync-era#2410
@aakoshh aakoshh changed the base branch from main to bft-474-l1-batch-signing July 9, 2024 19:27
@aakoshh aakoshh merged commit 4096144 into bft-474-l1-batch-signing Jul 9, 2024
47 checks passed
@aakoshh aakoshh deleted the bft-474-poll-db-and-sign-batch branch July 9, 2024 19:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants