Skip to content

Commit

Permalink
fix(prover): Remove redundant LoadingMode (#1496)
Browse files Browse the repository at this point in the history
This is PR 1 of many to move us to safe Prover Protocol Upgrades.

`ProtocolVersionLoadingMode` is a crutch that was added as part of
transitioning to Boojum. This served as a manual intervention mechanism
during protocol upgrades. It was a suboptimal choice (read as technical
debt) done due to rushed migration to boojum.

A proper fix involves reading the information from database, which in
turn is updated by information received from contracts. This change was
done recently (1.4.2 migration), but the old cludge was left in place
(you guessed it, due to rushed migration).

This PR removes this cludge and makes sure that nobody will try to use
it assuming it will work (whilst it does work, it requires "specialized"
knowledge on how to use it and may cause us submitting broken proofs to
L1).
  • Loading branch information
EmilLuta committed Mar 26, 2024
1 parent 2c7a6bf commit e7583f4
Show file tree
Hide file tree
Showing 10 changed files with 32 additions and 146 deletions.
8 changes: 0 additions & 8 deletions core/lib/config/src/configs/proof_data_handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,10 @@ use std::time::Duration;

use serde::Deserialize;

#[derive(Debug, Deserialize, Clone, Copy, PartialEq)]
pub enum ProtocolVersionLoadingMode {
FromDb,
FromEnvVar,
}

#[derive(Debug, Deserialize, Clone, PartialEq)]
pub struct ProofDataHandlerConfig {
pub http_port: u16,
pub proof_generation_timeout_in_secs: u16,
pub protocol_version_loading_mode: ProtocolVersionLoadingMode,
pub fri_protocol_version_id: u16,
}

impl ProofDataHandlerConfig {
Expand Down
11 changes: 0 additions & 11 deletions core/lib/config/src/testonly.rs
Original file line number Diff line number Diff line change
Expand Up @@ -690,22 +690,11 @@ impl RandomConfig for configs::ObjectStoreConfig {
}
}

impl RandomConfig for configs::proof_data_handler::ProtocolVersionLoadingMode {
fn sample(g: &mut Gen<impl Rng>) -> Self {
match g.rng.gen_range(0..2) {
0 => Self::FromDb,
_ => Self::FromEnvVar,
}
}
}

impl RandomConfig for configs::ProofDataHandlerConfig {
fn sample(g: &mut Gen<impl Rng>) -> Self {
Self {
http_port: g.gen(),
proof_generation_timeout_in_secs: g.gen(),
protocol_version_loading_mode: g.gen(),
fri_protocol_version_id: g.gen(),
}
}
}
Expand Down
6 changes: 0 additions & 6 deletions core/lib/env_config/src/proof_data_handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@ impl FromEnv for ProofDataHandlerConfig {

#[cfg(test)]
mod tests {
use zksync_config::configs::proof_data_handler::ProtocolVersionLoadingMode;

use super::*;
use crate::test_utils::EnvMutex;

Expand All @@ -21,8 +19,6 @@ mod tests {
ProofDataHandlerConfig {
http_port: 3320,
proof_generation_timeout_in_secs: 18000,
protocol_version_loading_mode: ProtocolVersionLoadingMode::FromEnvVar,
fri_protocol_version_id: 2,
}
}

Expand All @@ -31,8 +27,6 @@ mod tests {
let config = r#"
PROOF_DATA_HANDLER_PROOF_GENERATION_TIMEOUT_IN_SECS="18000"
PROOF_DATA_HANDLER_HTTP_PORT="3320"
PROOF_DATA_HANDLER_PROTOCOL_VERSION_LOADING_MODE="FromEnvVar"
PROOF_DATA_HANDLER_FRI_PROTOCOL_VERSION_ID="2"
"#;
let mut lock = MUTEX.lock();
lock.set_env(config);
Expand Down
28 changes: 0 additions & 28 deletions core/lib/protobuf_config/src/proof_data_handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,23 +4,6 @@ use zksync_protobuf::{repr::ProtoRepr, required};

use crate::proto::proof_data_handler as proto;

impl proto::ProtocolVersionLoadingMode {
fn new(x: &configs::proof_data_handler::ProtocolVersionLoadingMode) -> Self {
type From = configs::proof_data_handler::ProtocolVersionLoadingMode;
match x {
From::FromDb => Self::FromDb,
From::FromEnvVar => Self::FromEnvVar,
}
}
fn parse(&self) -> configs::proof_data_handler::ProtocolVersionLoadingMode {
type To = configs::proof_data_handler::ProtocolVersionLoadingMode;
match self {
Self::FromDb => To::FromDb,
Self::FromEnvVar => To::FromEnvVar,
}
}
}

impl ProtoRepr for proto::ProofDataHandler {
type Type = configs::ProofDataHandlerConfig;
fn read(&self) -> anyhow::Result<Self::Type> {
Expand All @@ -31,24 +14,13 @@ impl ProtoRepr for proto::ProofDataHandler {
proof_generation_timeout_in_secs: required(&self.proof_generation_timeout_in_secs)
.and_then(|x| Ok((*x).try_into()?))
.context("proof_generation_timeout_in_secs")?,
protocol_version_loading_mode: required(&self.protocol_version_loading_mode)
.and_then(|x| Ok(proto::ProtocolVersionLoadingMode::try_from(*x)?))
.context("protocol_version_loading_mode")?
.parse(),
fri_protocol_version_id: required(&self.fri_protocol_version_id)
.and_then(|x| Ok((*x).try_into()?))
.context("fri_protocol_version_id")?,
})
}

fn build(this: &Self::Type) -> Self {
Self {
http_port: Some(this.http_port.into()),
proof_generation_timeout_in_secs: Some(this.proof_generation_timeout_in_secs.into()),
protocol_version_loading_mode: Some(
proto::ProtocolVersionLoadingMode::new(&this.protocol_version_loading_mode).into(),
),
fri_protocol_version_id: Some(this.fri_protocol_version_id.into()),
}
}
}
2 changes: 0 additions & 2 deletions core/lib/protobuf_config/src/proto/proof_data_handler.proto
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,4 @@ enum ProtocolVersionLoadingMode {
message ProofDataHandler {
optional uint32 http_port = 1; // required; u16
optional uint32 proof_generation_timeout_in_secs = 2; // required; s
optional ProtocolVersionLoadingMode protocol_version_loading_mode = 3; // required
optional uint32 fri_protocol_version_id = 4; // required; u16
}
4 changes: 0 additions & 4 deletions core/lib/zksync_core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -701,10 +701,6 @@ pub async fn initialize_components(
.proof_data_handler_config
.clone()
.context("proof_data_handler_config")?,
configs
.contracts_config
.clone()
.context("contracts_config")?,
store_factory.create_store().await,
connection_pool.clone(),
stop_receiver.clone(),
Expand Down
29 changes: 2 additions & 27 deletions core/lib/zksync_core/src/proof_data_handler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,49 +3,24 @@ use std::{net::SocketAddr, sync::Arc};
use anyhow::Context as _;
use axum::{extract::Path, routing::post, Json, Router};
use tokio::sync::watch;
use zksync_config::{
configs::{proof_data_handler::ProtocolVersionLoadingMode, ProofDataHandlerConfig},
ContractsConfig,
};
use zksync_config::configs::ProofDataHandlerConfig;
use zksync_dal::{ConnectionPool, Core};
use zksync_object_store::ObjectStore;
use zksync_prover_interface::api::{ProofGenerationDataRequest, SubmitProofRequest};
use zksync_types::{
protocol_version::{L1VerifierConfig, VerifierParams},
H256,
};

use crate::proof_data_handler::request_processor::RequestProcessor;

mod request_processor;

fn fri_l1_verifier_config(contracts_config: &ContractsConfig) -> L1VerifierConfig {
L1VerifierConfig {
params: VerifierParams {
recursion_node_level_vk_hash: contracts_config.fri_recursion_node_level_vk_hash,
recursion_leaf_level_vk_hash: contracts_config.fri_recursion_leaf_level_vk_hash,
// The base layer commitment is not used in the FRI prover verification.
recursion_circuits_set_vks_hash: H256::zero(),
},
recursion_scheduler_level_vk_hash: contracts_config.snark_wrapper_vk_hash,
}
}

pub async fn run_server(
config: ProofDataHandlerConfig,
contracts_config: ContractsConfig,
blob_store: Arc<dyn ObjectStore>,
pool: ConnectionPool<Core>,
mut stop_receiver: watch::Receiver<bool>,
) -> anyhow::Result<()> {
let bind_address = SocketAddr::from(([0, 0, 0, 0], config.http_port));
tracing::debug!("Starting proof data handler server on {bind_address}");
let l1_verifier_config: Option<L1VerifierConfig> = match config.protocol_version_loading_mode {
ProtocolVersionLoadingMode::FromDb => None,
ProtocolVersionLoadingMode::FromEnvVar => Some(fri_l1_verifier_config(&contracts_config)),
};
let get_proof_gen_processor =
RequestProcessor::new(blob_store, pool, config, l1_verifier_config);
let get_proof_gen_processor = RequestProcessor::new(blob_store, pool, config);
let submit_proof_processor = get_proof_gen_processor.clone();
let app = Router::new()
.route(
Expand Down
77 changes: 28 additions & 49 deletions core/lib/zksync_core/src/proof_data_handler/request_processor.rs
Original file line number Diff line number Diff line change
@@ -1,26 +1,21 @@
use std::{convert::TryFrom, sync::Arc};
use std::sync::Arc;

use axum::{
extract::Path,
http::StatusCode,
response::{IntoResponse, Response},
Json,
};
use zksync_config::configs::{
proof_data_handler::ProtocolVersionLoadingMode, ProofDataHandlerConfig,
};
use zksync_config::configs::ProofDataHandlerConfig;
use zksync_dal::{ConnectionPool, Core, CoreDal, SqlxError};
use zksync_object_store::{ObjectStore, ObjectStoreError};
use zksync_prover_interface::api::{
ProofGenerationData, ProofGenerationDataRequest, ProofGenerationDataResponse,
SubmitProofRequest, SubmitProofResponse,
};
use zksync_types::{
basic_fri_types::Eip4844Blobs,
commitment::serialize_commitments,
protocol_version::{FriProtocolVersionId, L1VerifierConfig},
web3::signing::keccak256,
L1BatchNumber, H256,
basic_fri_types::Eip4844Blobs, commitment::serialize_commitments,
protocol_version::FriProtocolVersionId, web3::signing::keccak256, L1BatchNumber, H256,
};
use zksync_utils::u256_to_h256;

Expand All @@ -29,7 +24,6 @@ pub(crate) struct RequestProcessor {
blob_store: Arc<dyn ObjectStore>,
pool: ConnectionPool<Core>,
config: ProofDataHandlerConfig,
l1_verifier_config: Option<L1VerifierConfig>,
}

pub(crate) enum RequestProcessorError {
Expand Down Expand Up @@ -69,13 +63,11 @@ impl RequestProcessor {
blob_store: Arc<dyn ObjectStore>,
pool: ConnectionPool<Core>,
config: ProofDataHandlerConfig,
l1_verifier_config: Option<L1VerifierConfig>,
) -> Self {
Self {
blob_store,
pool,
config,
l1_verifier_config,
}
}

Expand Down Expand Up @@ -105,44 +97,31 @@ impl RequestProcessor {
.await
.map_err(RequestProcessorError::ObjectStore)?;

let (l1_verifier_config, fri_protocol_version_id) = match self.config.protocol_version_loading_mode {
ProtocolVersionLoadingMode::FromDb => {

let header = self
.pool
.connection()
.await
.unwrap()
.blocks_dal()
.get_l1_batch_header(l1_batch_number)
.await
.unwrap()
.expect(&format!("Missing header for {}", l1_batch_number));

let protocol_version = header.protocol_version.unwrap();
// TODO: What invariants have to hold such that protocol version = fri protocol version?
let fri_protocol_version = FriProtocolVersionId::from(protocol_version);
(self
.pool
.connection()
.await
.unwrap()
.protocol_versions_dal()
.l1_verifier_config_for_version(protocol_version)
.await
.expect(&format!(
"Missing l1 verifier info for protocol version {protocol_version:?}",
)), fri_protocol_version)

}
ProtocolVersionLoadingMode::FromEnvVar => {
(self.l1_verifier_config
.expect("l1_verifier_config must be set while running ProtocolVersionLoadingMode::FromEnvVar mode"),
FriProtocolVersionId::try_from(self.config.fri_protocol_version_id)
.expect("Invalid FRI protocol version id"))
let header = self
.pool
.connection()
.await
.unwrap()
.blocks_dal()
.get_l1_batch_header(l1_batch_number)
.await
.unwrap()
.expect(&format!("Missing header for {}", l1_batch_number));

}
};
let protocol_version = header.protocol_version.unwrap();
// TODO: What invariants have to hold such that protocol version = fri protocol version?
let fri_protocol_version_id = FriProtocolVersionId::from(protocol_version);
let l1_verifier_config = self
.pool
.connection()
.await
.unwrap()
.protocol_versions_dal()
.l1_verifier_config_for_version(protocol_version)
.await
.expect(&format!(
"Missing l1 verifier info for protocol version {protocol_version:?}",
));

let storage_batch = self
.pool
Expand Down
1 change: 0 additions & 1 deletion core/node/node_framework/examples/main_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,6 @@ impl MainNodeBuilder {
fn add_proof_data_handler_layer(mut self) -> anyhow::Result<Self> {
self.node.add_layer(ProofDataHandlerLayer::new(
ProofDataHandlerConfig::from_env()?,
ContractsConfig::from_env()?,
));
Ok(self)
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use std::sync::Arc;

use zksync_config::{configs::ProofDataHandlerConfig, ContractsConfig};
use zksync_config::configs::ProofDataHandlerConfig;
use zksync_core::proof_data_handler;
use zksync_dal::{ConnectionPool, Core};
use zksync_object_store::ObjectStore;
Expand All @@ -22,17 +22,12 @@ use crate::{
#[derive(Debug)]
pub struct ProofDataHandlerLayer {
proof_data_handler_config: ProofDataHandlerConfig,
contracts_config: ContractsConfig,
}

impl ProofDataHandlerLayer {
pub fn new(
proof_data_handler_config: ProofDataHandlerConfig,
contracts_config: ContractsConfig,
) -> Self {
pub fn new(proof_data_handler_config: ProofDataHandlerConfig) -> Self {
Self {
proof_data_handler_config,
contracts_config,
}
}
}
Expand All @@ -51,7 +46,6 @@ impl WiringLayer for ProofDataHandlerLayer {

context.add_task(Box::new(ProofDataHandlerTask {
proof_data_handler_config: self.proof_data_handler_config,
contracts_config: self.contracts_config,
blob_store: object_store.0,
main_pool,
}));
Expand All @@ -63,7 +57,6 @@ impl WiringLayer for ProofDataHandlerLayer {
#[derive(Debug)]
struct ProofDataHandlerTask {
proof_data_handler_config: ProofDataHandlerConfig,
contracts_config: ContractsConfig,
blob_store: Arc<dyn ObjectStore>,
main_pool: ConnectionPool<Core>,
}
Expand All @@ -77,7 +70,6 @@ impl Task for ProofDataHandlerTask {
async fn run(self: Box<Self>, stop_receiver: StopReceiver) -> anyhow::Result<()> {
proof_data_handler::run_server(
self.proof_data_handler_config,
self.contracts_config,
self.blob_store,
self.main_pool,
stop_receiver.0,
Expand Down

0 comments on commit e7583f4

Please sign in to comment.