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

refactor(l2): handle not committed block #1192

Closed
wants to merge 17 commits into from

Conversation

fborello-lambda
Copy link
Contributor

@fborello-lambda fborello-lambda commented Nov 15, 2024

Motivation

The prover_server was sending blocks to prove even if the block had not been committed. While this approach helped avoid keeping the prover in an idle state, in practice, the prover is typically slower than the commitment process. Therefore, waiting for the block to be committed provides an additional safety measure.

Description

  • The prover_server queries the OnChainProposer for the lastCommittedBlock after receiving a Request from the prover_client.
    • If the block has been committed, it sends a valid input to the prover.
    • else, nothing is sent, and the prover remains idle, waiting until the block is committed.
  • Comments regarding the require statements of issue L2: Differentiate require Errors in OnChainProposer Verification #1164 were added.

Closes #1165
Closes #1164

@fborello-lambda fborello-lambda self-assigned this Nov 15, 2024
@fborello-lambda fborello-lambda force-pushed the l2/handle_not_committed_block branch from 94a1a89 to a195fe5 Compare November 15, 2024 20:11
@fborello-lambda fborello-lambda marked this pull request as ready for review November 15, 2024 21:17
@fborello-lambda fborello-lambda requested a review from a team as a code owner November 15, 2024 21:17
@fborello-lambda fborello-lambda marked this pull request as draft November 19, 2024 14:32
ilitteri
ilitteri previously approved these changes Nov 19, 2024
Copy link
Contributor

@ilitteri ilitteri left a comment

Choose a reason for hiding this comment

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

LGTM. Left one comment.

crates/l2/proposer/prover_server.rs Outdated Show resolved Hide resolved
crates/l2/proposer/prover_server.rs Outdated Show resolved Hide resolved
crates/l2/proposer/prover_server.rs Outdated Show resolved Hide resolved
@ilitteri ilitteri dismissed their stale review November 19, 2024 14:35

The PR was converted into draft during the review.

@fborello-lambda fborello-lambda marked this pull request as ready for review November 19, 2024 15:31
}
Err(e) => {
error!("Failed to send proof to block {block_number:#x}. Manual intervention required: {e}");
panic!("Failed to send proof to block {block_number:#x}. Manual intervention required: {e}");
Copy link
Collaborator

@jrchatruc jrchatruc Nov 20, 2024

Choose a reason for hiding this comment

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

Instead of panicking here, let's just backoff, wait a while and try sending the transaction again. An error when sending the proof (or on any other request to the L1 for that matter) is something that can (and will) happen every now and then. Anywhere where we interact with L1 we should follow a pattern like this; retry after a little while, not panic.

@jrchatruc jrchatruc closed this Nov 22, 2024
@ilitteri ilitteri deleted the l2/handle_not_committed_block branch December 2, 2024 19:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

L2: wait or handle not committed block L2: Differentiate require Errors in OnChainProposer Verification
3 participants