-
Notifications
You must be signed in to change notification settings - Fork 128
feat(l2): verify batches in chunks with aligned #3242
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
Conversation
…d_use_hardcoded_vk
…d_use_hardcoded_vk
…d_use_hardcoded_vk
> [!WARNING] > Merge after #3242 **Motivation** In case one RPC or Beacon client fails, we should retry with others to avoid getting stuck. **Description** - Implements a wrapper over `aligned_sdk::estimate_fee()` to retry using all available RPC urls. - Implements a wrapper over `aligned_sdk::check_proof_verification()` to retry using all available RPC and Beacon clients. Closes #3059
ManuelBilbao
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, just some little comments
| loop { | ||
| if !batch_number_has_all_needed_proofs(batch_number, &[ProverType::Aligned]) | ||
| .is_ok_and(|has_all_proofs| has_all_proofs) | ||
| { | ||
| return Ok(proofs); | ||
| } | ||
| proofs.push(( | ||
| batch_number, | ||
| read_proof(batch_number, StateFileType::BatchProof(ProverType::Aligned))?, | ||
| )); | ||
| batch_number += 1; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit
| loop { | |
| if !batch_number_has_all_needed_proofs(batch_number, &[ProverType::Aligned]) | |
| .is_ok_and(|has_all_proofs| has_all_proofs) | |
| { | |
| return Ok(proofs); | |
| } | |
| proofs.push(( | |
| batch_number, | |
| read_proof(batch_number, StateFileType::BatchProof(ProverType::Aligned))?, | |
| )); | |
| batch_number += 1; | |
| } | |
| while !batch_number_has_all_needed_proofs(batch_number, &[ProverType::Aligned]) | |
| .is_ok_and(|has_all_proofs| has_all_proofs) { | |
| proofs.push(( | |
| batch_number, | |
| read_proof(batch_number, StateFileType::BatchProof(ProverType::Aligned))?, | |
| )); | |
| batch_number += 1; | |
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done 8c1dfae!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to update this functions too as in non-based ones?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. Updated here cb11a21
| require( | ||
| batchNumber == lastVerifiedBatch + 1, | ||
| "OnChainProposer: incorrect batch number" | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be checked just for firstBatchNumber outside the loop.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right! Done 52d4f58.
**Motivation** In #3242, `verifyBatchesAligned()` was updated in the based `OnChainProposer` to be consistent with the non-based one, but the `onlySequencer` identifier was added by mistake. **Description** Removes the `onlySequencer` identifier. Closes None
**Motivation** In #3242, `verifyBatchesAligned()` was updated in the based `OnChainProposer` to be consistent with the non-based one, but the `onlySequencer` identifier was added by mistake. **Description** Removes the `onlySequencer` identifier. Closes None
**Motivation** Aligned verifies proofs in batches, so it's possible to have an array of proofs ready to be verified at once. **Description** - Modifies `l1_proof_verifier` to check all already aggregated proofs and build a single verify transaction for them. - Updates `verifyBatchAligned()` in the `OnChainProposer` contract to accept an array of proofs. > [!WARNING] > lambdaclass#3276 was accidentally merged into this PR, so the diff includes changes from both. Closes lambdaclass#3168 --------- Co-authored-by: Ivan Litteri <[email protected]>
**Motivation** In lambdaclass#3242, `verifyBatchesAligned()` was updated in the based `OnChainProposer` to be consistent with the non-based one, but the `onlySequencer` identifier was added by mistake. **Description** Removes the `onlySequencer` identifier. Closes None
**Motivation** Aligned verifies proofs in batches, so it's possible to have an array of proofs ready to be verified at once. **Description** - Modifies `l1_proof_verifier` to check all already aggregated proofs and build a single verify transaction for them. - Updates `verifyBatchAligned()` in the `OnChainProposer` contract to accept an array of proofs. > [!WARNING] > lambdaclass#3276 was accidentally merged into this PR, so the diff includes changes from both. Closes lambdaclass#3168 --------- Co-authored-by: Ivan Litteri <[email protected]>
**Motivation** In lambdaclass#3242, `verifyBatchesAligned()` was updated in the based `OnChainProposer` to be consistent with the non-based one, but the `onlySequencer` identifier was added by mistake. **Description** Removes the `onlySequencer` identifier. Closes None
Motivation
Aligned verifies proofs in batches, so it's possible to have an array of proofs ready to be verified at once.
Description
l1_proof_verifierto check all already aggregated proofs and build a single verify transaction for them.verifyBatchAligned()in theOnChainProposercontract to accept an array of proofs.Warning
#3276 was accidentally merged into this PR, so the diff includes changes from both.
Closes #3168