From 67d55e07bc44567c5991a132e7f86d49f39a984f Mon Sep 17 00:00:00 2001 From: Patrick Beza Date: Fri, 6 Sep 2024 11:53:47 +0200 Subject: [PATCH 1/3] fix(tee-prover): fix deserialization of duration in envy config Relevant logs from the `stage` environment showcasing the issue: https://grafana.matterlabs.dev/goto/IC-9k4eIR?orgId=1 Error message from the above logs: ``` Error: missing value for field initial_retry_backoff ``` The root cause of the problem supposedly boils down to the mismatch between the expected format of `TEE_PROVER_INITIAL_RETRY_BACKOFF` and the actual format. We export it as follows: ``` export TEE_PROVER_INITIAL_RETRY_BACKOFF=1 ``` which is not supported as explained here: https://github.com/serde-rs/serde/issues/339#issuecomment-539453327 --- Cargo.lock | 1 + core/bin/zksync_tee_prover/Cargo.toml | 1 + core/bin/zksync_tee_prover/src/config.rs | 4 ++++ 3 files changed, 6 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index 2d6263f7ab4e..bd91284b96d4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -9624,6 +9624,7 @@ dependencies = [ "reqwest 0.12.5", "secp256k1", "serde", + "serde_with", "thiserror", "tokio", "tracing", diff --git a/core/bin/zksync_tee_prover/Cargo.toml b/core/bin/zksync_tee_prover/Cargo.toml index 85908eebeaaa..6af9b7cbd2af 100644 --- a/core/bin/zksync_tee_prover/Cargo.toml +++ b/core/bin/zksync_tee_prover/Cargo.toml @@ -18,6 +18,7 @@ envy.workspace = true reqwest.workspace = true secp256k1 = { workspace = true, features = ["serde"] } serde = { workspace = true, features = ["derive"] } +serde_with.workspace = true thiserror.workspace = true tokio = { workspace = true, features = ["full"] } tracing.workspace = true diff --git a/core/bin/zksync_tee_prover/src/config.rs b/core/bin/zksync_tee_prover/src/config.rs index 5b009e33f25e..4e5260f63021 100644 --- a/core/bin/zksync_tee_prover/src/config.rs +++ b/core/bin/zksync_tee_prover/src/config.rs @@ -2,11 +2,13 @@ use std::{path::PathBuf, time::Duration}; use secp256k1::SecretKey; use serde::Deserialize; +use serde_with::{serde_as, DurationSecondsWithFrac}; use url::Url; use zksync_env_config::FromEnv; use zksync_types::tee_types::TeeType; /// Configuration for the TEE prover. +#[serde_with::serde_as] #[derive(Debug, Clone, Deserialize)] pub(crate) struct TeeProverConfig { /// The private key used to sign the proofs. @@ -22,10 +24,12 @@ pub(crate) struct TeeProverConfig { pub max_retries: usize, /// Initial back-off interval when retrying recovery on a retriable error. Each subsequent retry interval /// will be multiplied by [`Self.retry_backoff_multiplier`]. + #[serde_as(as = "DurationSecondsWithFrac")] pub initial_retry_backoff: Duration, /// Multiplier for the back-off interval when retrying recovery on a retriable error. pub retry_backoff_multiplier: f32, /// Maximum back-off interval when retrying recovery on a retriable error. + #[serde_as(as = "DurationSecondsWithFrac")] pub max_backoff: Duration, } From b2488eebd48135527be17890ad1d45451558248a Mon Sep 17 00:00:00 2001 From: Patrick Beza Date: Fri, 6 Sep 2024 13:44:41 +0200 Subject: [PATCH 2/3] Address Igor's code review comments --- Cargo.lock | 1 - core/bin/zksync_tee_prover/Cargo.toml | 1 - core/bin/zksync_tee_prover/src/config.rs | 22 ++++++++++++++-------- 3 files changed, 14 insertions(+), 10 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index bd91284b96d4..2d6263f7ab4e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -9624,7 +9624,6 @@ dependencies = [ "reqwest 0.12.5", "secp256k1", "serde", - "serde_with", "thiserror", "tokio", "tracing", diff --git a/core/bin/zksync_tee_prover/Cargo.toml b/core/bin/zksync_tee_prover/Cargo.toml index 6af9b7cbd2af..85908eebeaaa 100644 --- a/core/bin/zksync_tee_prover/Cargo.toml +++ b/core/bin/zksync_tee_prover/Cargo.toml @@ -18,7 +18,6 @@ envy.workspace = true reqwest.workspace = true secp256k1 = { workspace = true, features = ["serde"] } serde = { workspace = true, features = ["derive"] } -serde_with.workspace = true thiserror.workspace = true tokio = { workspace = true, features = ["full"] } tracing.workspace = true diff --git a/core/bin/zksync_tee_prover/src/config.rs b/core/bin/zksync_tee_prover/src/config.rs index 4e5260f63021..1c2eb229d616 100644 --- a/core/bin/zksync_tee_prover/src/config.rs +++ b/core/bin/zksync_tee_prover/src/config.rs @@ -2,13 +2,11 @@ use std::{path::PathBuf, time::Duration}; use secp256k1::SecretKey; use serde::Deserialize; -use serde_with::{serde_as, DurationSecondsWithFrac}; use url::Url; use zksync_env_config::FromEnv; use zksync_types::tee_types::TeeType; /// Configuration for the TEE prover. -#[serde_with::serde_as] #[derive(Debug, Clone, Deserialize)] pub(crate) struct TeeProverConfig { /// The private key used to sign the proofs. @@ -24,13 +22,21 @@ pub(crate) struct TeeProverConfig { pub max_retries: usize, /// Initial back-off interval when retrying recovery on a retriable error. Each subsequent retry interval /// will be multiplied by [`Self.retry_backoff_multiplier`]. - #[serde_as(as = "DurationSecondsWithFrac")] - pub initial_retry_backoff: Duration, + pub initial_retry_backoff_sec: u64, /// Multiplier for the back-off interval when retrying recovery on a retriable error. pub retry_backoff_multiplier: f32, /// Maximum back-off interval when retrying recovery on a retriable error. - #[serde_as(as = "DurationSecondsWithFrac")] - pub max_backoff: Duration, + pub max_backoff_sec: u64, +} + +impl TeeProverConfig { + pub fn initial_retry_backoff(&self) -> Duration { + Duration::from_secs(self.initial_retry_backoff_sec) + } + + pub fn max_backoff(&self) -> Duration { + Duration::from_secs(self.max_backoff_sec) + } } impl FromEnv for TeeProverConfig { @@ -43,9 +49,9 @@ impl FromEnv for TeeProverConfig { /// export TEE_PROVER_TEE_TYPE="sgx" /// export TEE_PROVER_API_URL="http://127.0.0.1:3320" /// export TEE_PROVER_MAX_RETRIES=10 - /// export TEE_PROVER_INITIAL_RETRY_BACKOFF=1 + /// export TEE_PROVER_INITIAL_RETRY_BACKOFF_SEC=1 /// export TEE_PROVER_RETRY_BACKOFF_MULTIPLIER=2.0 - /// export TEE_PROVER_MAX_BACKOFF=128 + /// export TEE_PROVER_MAX_BACKOFF_SEC=128 /// ``` fn from_env() -> anyhow::Result { let config: Self = envy::prefixed("TEE_PROVER_").from_env()?; From 98ddc69cb61d1b8e9b59c85151d0dc08cd28d5d1 Mon Sep 17 00:00:00 2001 From: Patrick Beza Date: Fri, 6 Sep 2024 13:58:19 +0200 Subject: [PATCH 3/3] Fixup --- core/bin/zksync_tee_prover/src/tee_prover.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/core/bin/zksync_tee_prover/src/tee_prover.rs b/core/bin/zksync_tee_prover/src/tee_prover.rs index 3d227118e57f..1511f0c88e3d 100644 --- a/core/bin/zksync_tee_prover/src/tee_prover.rs +++ b/core/bin/zksync_tee_prover/src/tee_prover.rs @@ -129,7 +129,7 @@ impl Task for TeeProver { .await?; let mut retries = 1; - let mut backoff = config.initial_retry_backoff; + let mut backoff = config.initial_retry_backoff(); let mut observer = METRICS.job_waiting_time.start(); loop { @@ -141,7 +141,7 @@ impl Task for TeeProver { let need_to_sleep = match result { Ok(batch_number) => { retries = 1; - backoff = config.initial_retry_backoff; + backoff = config.initial_retry_backoff(); if let Some(batch_number) = batch_number { observer.observe(); observer = METRICS.job_waiting_time.start(); @@ -162,7 +162,7 @@ impl Task for TeeProver { retries += 1; backoff = std::cmp::min( backoff.mul_f32(config.retry_backoff_multiplier), - config.max_backoff, + config.max_backoff(), ); true }