Skip to content

Commit

Permalink
fix: Change no pending batches 404 error into a success response (#279)
Browse files Browse the repository at this point in the history
# What ❔

When no pending batches were found on the server API, the response was a
404 which was not handled on the client (gateway) side and was treated
as an HTTP client error, causing a misleading log line:

```2023-10-19T18:30:46.726413Z ERROR zksync_prover_fri_gateway::api_data_fetcher: HTTP request failed due to error: HTTP status client error (404 Not Found) for url (http://127.0.0.1:3320/proof_generation_data)```

When testing out the gpu proving pipeline for the ZK stack effort, the error was hit soon after starting all services.
This PR changes this so in info trace is printed stating that there are currently no pending batches to be proven.  

<!-- What are the changes this PR brings about? -->
<!-- Example: This PR adds a PR template to the repo. -->
<!-- (For bigger PRs adding more context is appreciated) -->

## Why ❔

There is some semantic dissonance on receiving an HTTP client error and a log line when calling the API, since it's possible and reasonable that there are no pending batches at some points.

<!-- Why are these changes done? What goal do they contribute to? What are the principles behind them? -->
<!-- Example: PR templates ensure PR reviewers, observers, and future iterators are in context about the evolution of repos. -->

## Checklist

<!-- Check your PR fulfills the following items. -->
<!-- For draft PRs check the boxes as you complete them. -->

- [x] 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.
- [x] Code has been formatted via `zk fmt` and `zk lint`.
  • Loading branch information
igamigo committed Nov 29, 2023
1 parent bd60f1c commit e8fd805
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 11 deletions.
2 changes: 1 addition & 1 deletion core/lib/types/src/prover_server_api/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ pub struct ProofGenerationDataRequest {}

#[derive(Debug, Serialize, Deserialize)]
pub enum ProofGenerationDataResponse {
Success(ProofGenerationData),
Success(Option<ProofGenerationData>),
Error(String),
}

Expand Down
19 changes: 10 additions & 9 deletions core/lib/zksync_core/src/proof_data_handler/request_processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,18 +31,13 @@ pub(crate) struct RequestProcessor {
}

pub(crate) enum RequestProcessorError {
NoPendingBatches,
ObjectStore(ObjectStoreError),
Sqlx(SqlxError),
}

impl IntoResponse for RequestProcessorError {
fn into_response(self) -> Response {
let (status_code, message) = match self {
Self::NoPendingBatches => (
StatusCode::NOT_FOUND,
"No pending batches to process".to_owned(),
),
RequestProcessorError::ObjectStore(err) => {
tracing::error!("GCS error: {:?}", err);
(
Expand Down Expand Up @@ -88,15 +83,19 @@ impl RequestProcessor {
) -> Result<Json<ProofGenerationDataResponse>, RequestProcessorError> {
tracing::info!("Received request for proof generation data: {:?}", request);

let l1_batch_number = self
let l1_batch_number_result = self
.pool
.access_storage()
.await
.unwrap()
.proof_generation_dal()
.get_next_block_to_be_proven(self.config.proof_generation_timeout())
.await
.ok_or(RequestProcessorError::NoPendingBatches)?;
.await;

let l1_batch_number = match l1_batch_number_result {
Some(number) => number,
None => return Ok(Json(ProofGenerationDataResponse::Success(None))), // no batches pending to be proven
};

let blob = self
.blob_store
Expand Down Expand Up @@ -125,7 +124,9 @@ impl RequestProcessor {
l1_verifier_config,
};

Ok(Json(ProofGenerationDataResponse::Success(proof_gen_data)))
Ok(Json(ProofGenerationDataResponse::Success(Some(
proof_gen_data,
))))
}

pub(crate) async fn submit_proof(
Expand Down
1 change: 1 addition & 0 deletions prover/prover_fri_gateway/src/api_data_fetcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ impl PeriodicApiStruct {
Resp: DeserializeOwned,
{
tracing::info!("Sending request to {}", endpoint);

self.client
.post(endpoint)
.json(&request)
Expand Down
6 changes: 5 additions & 1 deletion prover/prover_fri_gateway/src/proof_gen_data_fetcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ impl PeriodicApiStruct {
impl PeriodicApi<ProofGenerationDataRequest> for PeriodicApiStruct {
type JobId = ();
type Response = ProofGenerationDataResponse;

const SERVICE_NAME: &'static str = "ProofGenDataFetcher";

async fn get_next_request(&self) -> Option<(Self::JobId, ProofGenerationDataRequest)> {
Expand All @@ -49,7 +50,10 @@ impl PeriodicApi<ProofGenerationDataRequest> for PeriodicApiStruct {

async fn handle_response(&self, _: (), response: Self::Response) {
match response {
ProofGenerationDataResponse::Success(data) => {
ProofGenerationDataResponse::Success(None) => {
tracing::info!("There are currently no pending batches to be proven");
}
ProofGenerationDataResponse::Success(Some(data)) => {
tracing::info!("Received proof gen data for: {:?}", data.l1_batch_number);
self.save_proof_gen_data(data).await;
}
Expand Down

0 comments on commit e8fd805

Please sign in to comment.