Skip to content

Commit

Permalink
fix(prover): Remove FriProtocolVersionId (#1510)
Browse files Browse the repository at this point in the history
This is PR 2 of many to move us to safe Protocol Upgrades (phase 1).

`FriProtocolVersionId` was introduced as a cludge to serve as
communication between core (which was saved from L1 straight to DB) and
prover (which was manually configured from env vars). Whilst in theory
it's possible to have an upgrade on core side without affecting prover,
I don't envision this happening any time soon. Right now we maintain 2
different protocol numbers that cause confusion. This PR unifies the
protocol ids.

Other relevant notes:
- this is not exactly backwards compatible -- we'll need to deploy both
core and prover at the same time, otherwise provers will go in crash
loop. We can make it backwards compatible, but at our track record of
deployments (and speed of deployments), I'm concerned we will slow
everything down for no real reason. I see very small risks here
- database tables in prover's end should be renamed to protocol_version
as well (to be covered in next PR)
- all our code base assumes the old prover's interface to hashes which
is confusing and unnecessary. I propose we address those with following
PR. The points there are (to be done on both databases and configs):
- rename `recursion_scheduler_level_vk_hash` to `snark_wrapper_vk_hash`.
This is what it represents today. I believe we can't change
`recursion_scheduler_level_vk_hash` without changing
`snark_wrapper_vk_hash`.
    - delete `recursion_circuits_set_vks_hash`; it's all 0.
  • Loading branch information
EmilLuta committed Mar 27, 2024
1 parent da41f63 commit 6aa51b0
Show file tree
Hide file tree
Showing 12 changed files with 42 additions and 125 deletions.
79 changes: 0 additions & 79 deletions core/lib/basic_types/src/protocol_version.rs
Expand Up @@ -143,85 +143,6 @@ impl TryFrom<U256> for ProtocolVersionId {
}
}

// TODO: Do we even need this? I reckon we could merge this with `ProtocolVersionId`.
#[repr(u16)]
#[derive(
Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, TryFromPrimitive, Serialize, Deserialize,
)]
pub enum FriProtocolVersionId {
Version0 = 0,
Version1,
Version2,
Version3,
Version4,
Version5,
Version6,
Version7,
Version8,
Version9,
Version10,
Version11,
Version12,
Version13,
Version14,
Version15,
Version16,
Version17,
Version18,
Version19,
Version20,
Version21,
Version22,
Version23,
}

impl FriProtocolVersionId {
pub fn latest() -> Self {
Self::Version22
}

pub fn next() -> Self {
Self::Version23
}
}

impl Default for FriProtocolVersionId {
fn default() -> Self {
Self::latest()
}
}

impl From<ProtocolVersionId> for FriProtocolVersionId {
fn from(protocol_version: ProtocolVersionId) -> Self {
match protocol_version {
ProtocolVersionId::Version0 => FriProtocolVersionId::Version0,
ProtocolVersionId::Version1 => FriProtocolVersionId::Version1,
ProtocolVersionId::Version2 => FriProtocolVersionId::Version2,
ProtocolVersionId::Version3 => FriProtocolVersionId::Version3,
ProtocolVersionId::Version4 => FriProtocolVersionId::Version4,
ProtocolVersionId::Version5 => FriProtocolVersionId::Version5,
ProtocolVersionId::Version6 => FriProtocolVersionId::Version6,
ProtocolVersionId::Version7 => FriProtocolVersionId::Version7,
ProtocolVersionId::Version8 => FriProtocolVersionId::Version8,
ProtocolVersionId::Version9 => FriProtocolVersionId::Version9,
ProtocolVersionId::Version10 => FriProtocolVersionId::Version10,
ProtocolVersionId::Version11 => FriProtocolVersionId::Version11,
ProtocolVersionId::Version12 => FriProtocolVersionId::Version12,
ProtocolVersionId::Version13 => FriProtocolVersionId::Version13,
ProtocolVersionId::Version14 => FriProtocolVersionId::Version14,
ProtocolVersionId::Version15 => FriProtocolVersionId::Version15,
ProtocolVersionId::Version16 => FriProtocolVersionId::Version16,
ProtocolVersionId::Version17 => FriProtocolVersionId::Version17,
ProtocolVersionId::Version18 => FriProtocolVersionId::Version18,
ProtocolVersionId::Version19 => FriProtocolVersionId::Version19,
ProtocolVersionId::Version20 => FriProtocolVersionId::Version20,
ProtocolVersionId::Version21 => FriProtocolVersionId::Version21,
ProtocolVersionId::Version22 => FriProtocolVersionId::Version22,
ProtocolVersionId::Version23 => FriProtocolVersionId::Version23,
}
}
}

#[derive(Debug, Clone, Copy, Default, Eq, PartialEq, Serialize, Deserialize)]
pub struct VerifierParams {
pub recursion_node_level_vk_hash: H256,
Expand Down
4 changes: 2 additions & 2 deletions core/lib/prover_interface/src/api.rs
Expand Up @@ -4,7 +4,7 @@
use serde::{Deserialize, Serialize};
use zksync_types::{
basic_fri_types::Eip4844Blobs,
protocol_version::{FriProtocolVersionId, L1VerifierConfig},
protocol_version::{L1VerifierConfig, ProtocolVersionId},
L1BatchNumber,
};

Expand All @@ -14,7 +14,7 @@ use crate::{inputs::PrepareBasicCircuitsJob, outputs::L1BatchProofForL1};
pub struct ProofGenerationData {
pub l1_batch_number: L1BatchNumber,
pub data: PrepareBasicCircuitsJob,
pub fri_protocol_version_id: FriProtocolVersionId,
pub protocol_version_id: ProtocolVersionId,
pub l1_verifier_config: L1VerifierConfig,
pub eip_4844_blobs: Eip4844Blobs,
}
Expand Down
14 changes: 6 additions & 8 deletions core/lib/zksync_core/src/proof_data_handler/request_processor.rs
Expand Up @@ -14,8 +14,8 @@ use zksync_prover_interface::api::{
SubmitProofRequest, SubmitProofResponse,
};
use zksync_types::{
basic_fri_types::Eip4844Blobs, commitment::serialize_commitments,
protocol_version::FriProtocolVersionId, web3::signing::keccak256, L1BatchNumber, H256,
basic_fri_types::Eip4844Blobs, commitment::serialize_commitments, web3::signing::keccak256,
L1BatchNumber, H256,
};
use zksync_utils::u256_to_h256;

Expand Down Expand Up @@ -108,19 +108,17 @@ impl RequestProcessor {
.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 protocol_version_id = header.protocol_version.unwrap();
let l1_verifier_config = self
.pool
.connection()
.await
.unwrap()
.protocol_versions_dal()
.l1_verifier_config_for_version(protocol_version)
.l1_verifier_config_for_version(protocol_version_id)
.await
.expect(&format!(
"Missing l1 verifier info for protocol version {protocol_version:?}",
"Missing l1 verifier info for protocol version {protocol_version_id:?}",
));

let storage_batch = self
Expand All @@ -144,7 +142,7 @@ impl RequestProcessor {
let proof_gen_data = ProofGenerationData {
l1_batch_number,
data: blob,
fri_protocol_version_id,
protocol_version_id,
l1_verifier_config,
eip_4844_blobs,
};
Expand Down
2 changes: 0 additions & 2 deletions etc/env/base/proof_data_handler.toml
@@ -1,5 +1,3 @@
[proof_data_handler]
http_port=3320
proof_generation_timeout_in_secs=18000
protocol_version_loading_mode="FromEnvVar"
fri_protocol_version_id=2
8 changes: 4 additions & 4 deletions prover/prover_dal/src/fri_protocol_versions_dal.rs
@@ -1,6 +1,6 @@
use std::convert::TryFrom;

use zksync_basic_types::protocol_version::{FriProtocolVersionId, L1VerifierConfig};
use zksync_basic_types::protocol_version::{L1VerifierConfig, ProtocolVersionId};
use zksync_db_connection::connection::Connection;

use crate::Prover;
Expand All @@ -13,7 +13,7 @@ pub struct FriProtocolVersionsDal<'a, 'c> {
impl FriProtocolVersionsDal<'_, '_> {
pub async fn save_prover_protocol_version(
&mut self,
id: FriProtocolVersionId,
id: ProtocolVersionId,
l1_verifier_config: L1VerifierConfig,
) {
sqlx::query!(
Expand Down Expand Up @@ -56,7 +56,7 @@ impl FriProtocolVersionsDal<'_, '_> {
pub async fn protocol_version_for(
&mut self,
vk_commitments: &L1VerifierConfig,
) -> Vec<FriProtocolVersionId> {
) -> Vec<ProtocolVersionId> {
sqlx::query!(
r#"
SELECT
Expand Down Expand Up @@ -87,7 +87,7 @@ impl FriProtocolVersionsDal<'_, '_> {
.await
.unwrap()
.into_iter()
.map(|row| FriProtocolVersionId::try_from(row.id as u16).unwrap())
.map(|row| ProtocolVersionId::try_from(row.id as u16).unwrap())
.collect()
}
}
10 changes: 5 additions & 5 deletions prover/prover_dal/src/fri_prover_dal.rs
Expand Up @@ -3,7 +3,7 @@ use std::{collections::HashMap, convert::TryFrom, time::Duration};

use zksync_basic_types::{
basic_fri_types::{AggregationRound, CircuitIdRoundTuple},
protocol_version::FriProtocolVersionId,
protocol_version::ProtocolVersionId,
prover_dal::{FriProverJobMetadata, JobCountStatistics, StuckJobs, EIP_4844_CIRCUIT_ID},
L1BatchNumber,
};
Expand All @@ -25,7 +25,7 @@ impl FriProverDal<'_, '_> {
circuit_ids_and_urls: Vec<(u8, String)>,
aggregation_round: AggregationRound,
depth: u16,
protocol_version_id: FriProtocolVersionId,
protocol_version_id: ProtocolVersionId,
) {
let latency = MethodLatency::new("save_fri_prover_jobs");
for (sequence_number, (circuit_id, circuit_blob_url)) in
Expand All @@ -52,7 +52,7 @@ impl FriProverDal<'_, '_> {

pub async fn get_next_job(
&mut self,
protocol_versions: &[FriProtocolVersionId],
protocol_versions: &[ProtocolVersionId],
picked_by: &str,
) -> Option<FriProverJobMetadata> {
let protocol_versions: Vec<i32> = protocol_versions.iter().map(|&id| id as i32).collect();
Expand Down Expand Up @@ -113,7 +113,7 @@ impl FriProverDal<'_, '_> {
pub async fn get_next_job_for_circuit_id_round(
&mut self,
circuits_to_pick: &[CircuitIdRoundTuple],
protocol_versions: &[FriProtocolVersionId],
protocol_versions: &[ProtocolVersionId],
picked_by: &str,
) -> Option<FriProverJobMetadata> {
let circuit_ids: Vec<_> = circuits_to_pick
Expand Down Expand Up @@ -356,7 +356,7 @@ impl FriProverDal<'_, '_> {
aggregation_round: AggregationRound,
circuit_blob_url: &str,
is_node_final_proof: bool,
protocol_version_id: FriProtocolVersionId,
protocol_version_id: ProtocolVersionId,
) {
sqlx::query!(
r#"
Expand Down
20 changes: 10 additions & 10 deletions prover/prover_dal/src/fri_witness_generator_dal.rs
Expand Up @@ -4,7 +4,7 @@ use std::{collections::HashMap, convert::TryFrom, time::Duration};
use sqlx::Row;
use zksync_basic_types::{
basic_fri_types::{AggregationRound, Eip4844Blobs},
protocol_version::FriProtocolVersionId,
protocol_version::ProtocolVersionId,
prover_dal::{
JobCountStatistics, LeafAggregationJobMetadata, NodeAggregationJobMetadata, StuckJobs,
},
Expand Down Expand Up @@ -38,7 +38,7 @@ impl FriWitnessGeneratorDal<'_, '_> {
&mut self,
block_number: L1BatchNumber,
object_key: &str,
protocol_version_id: FriProtocolVersionId,
protocol_version_id: ProtocolVersionId,
eip_4844_blobs: Eip4844Blobs,
) {
let blobs_raw: Vec<u8> = eip_4844_blobs.into();
Expand Down Expand Up @@ -73,7 +73,7 @@ impl FriWitnessGeneratorDal<'_, '_> {
pub async fn get_next_basic_circuit_witness_job(
&mut self,
last_l1_batch_to_process: u32,
protocol_versions: &[FriProtocolVersionId],
protocol_versions: &[ProtocolVersionId],
picked_by: &str,
) -> Option<(L1BatchNumber, Eip4844Blobs)> {
let protocol_versions: Vec<i32> = protocol_versions.iter().map(|&id| id as i32).collect();
Expand Down Expand Up @@ -301,7 +301,7 @@ impl FriWitnessGeneratorDal<'_, '_> {
closed_form_inputs_and_urls: &Vec<(u8, String, usize)>,
scheduler_partial_input_blob_url: &str,
base_layer_to_recursive_layer_circuit_id: fn(u8) -> u8,
protocol_version_id: FriProtocolVersionId,
protocol_version_id: ProtocolVersionId,
) {
{
let latency = MethodLatency::new("create_aggregation_jobs_fri");
Expand Down Expand Up @@ -398,7 +398,7 @@ impl FriWitnessGeneratorDal<'_, '_> {

pub async fn get_next_leaf_aggregation_job(
&mut self,
protocol_versions: &[FriProtocolVersionId],
protocol_versions: &[ProtocolVersionId],
picked_by: &str,
) -> Option<LeafAggregationJobMetadata> {
let protocol_versions: Vec<i32> = protocol_versions.iter().map(|&id| id as i32).collect();
Expand Down Expand Up @@ -585,7 +585,7 @@ impl FriWitnessGeneratorDal<'_, '_> {

pub async fn get_next_node_aggregation_job(
&mut self,
protocol_versions: &[FriProtocolVersionId],
protocol_versions: &[ProtocolVersionId],
picked_by: &str,
) -> Option<NodeAggregationJobMetadata> {
let protocol_versions: Vec<i32> = protocol_versions.iter().map(|&id| id as i32).collect();
Expand Down Expand Up @@ -715,7 +715,7 @@ impl FriWitnessGeneratorDal<'_, '_> {
number_of_dependent_jobs: Option<i32>,
depth: u16,
aggregations_url: &str,
protocol_version_id: FriProtocolVersionId,
protocol_version_id: ProtocolVersionId,
) {
sqlx::query!(
r#"
Expand Down Expand Up @@ -986,7 +986,7 @@ impl FriWitnessGeneratorDal<'_, '_> {

pub async fn get_next_scheduler_witness_job(
&mut self,
protocol_versions: &[FriProtocolVersionId],
protocol_versions: &[ProtocolVersionId],
picked_by: &str,
) -> Option<L1BatchNumber> {
let protocol_versions: Vec<i32> = protocol_versions.iter().map(|&id| id as i32).collect();
Expand Down Expand Up @@ -1132,7 +1132,7 @@ impl FriWitnessGeneratorDal<'_, '_> {
pub async fn protocol_version_for_l1_batch(
&mut self,
l1_batch_number: L1BatchNumber,
) -> FriProtocolVersionId {
) -> ProtocolVersionId {
sqlx::query!(
r#"
SELECT
Expand All @@ -1148,7 +1148,7 @@ impl FriWitnessGeneratorDal<'_, '_> {
.await
.unwrap()
.protocol_version
.map(|id| FriProtocolVersionId::try_from(id as u16).unwrap())
.map(|id| ProtocolVersionId::try_from(id as u16).unwrap())
.unwrap()
}
}
4 changes: 2 additions & 2 deletions prover/prover_fri_gateway/src/proof_gen_data_fetcher.rs
Expand Up @@ -16,14 +16,14 @@ impl PeriodicApiStruct {
let mut connection = self.pool.connection().await.unwrap();
connection
.fri_protocol_versions_dal()
.save_prover_protocol_version(data.fri_protocol_version_id, data.l1_verifier_config)
.save_prover_protocol_version(data.protocol_version_id, data.l1_verifier_config)
.await;
connection
.fri_witness_generator_dal()
.save_witness_inputs(
data.l1_batch_number,
&blob_url,
data.fri_protocol_version_id,
data.protocol_version_id,
data.eip_4844_blobs,
)
.await;
Expand Down
8 changes: 4 additions & 4 deletions prover/witness_generator/src/basic_circuits.rs
Expand Up @@ -54,8 +54,8 @@ use zksync_types::{
basic_fri_types::{
AggregationRound, Eip4844Blobs, EIP_4844_BLOB_SIZE, MAX_4844_BLOBS_PER_BLOCK,
},
protocol_version::FriProtocolVersionId,
Address, L1BatchNumber, ProtocolVersionId, BOOTLOADER_ADDRESS, H256, U256,
protocol_version::ProtocolVersionId,
Address, L1BatchNumber, BOOTLOADER_ADDRESS, H256, U256,
};
use zksync_utils::{bytes_to_chunks, h256_to_u256, u256_to_h256};

Expand Down Expand Up @@ -108,7 +108,7 @@ pub struct BasicWitnessGenerator {
public_blob_store: Option<Arc<dyn ObjectStore>>,
connection_pool: ConnectionPool<Core>,
prover_connection_pool: ConnectionPool<Prover>,
protocol_versions: Vec<FriProtocolVersionId>,
protocol_versions: Vec<ProtocolVersionId>,
}

impl BasicWitnessGenerator {
Expand All @@ -118,7 +118,7 @@ impl BasicWitnessGenerator {
public_blob_store: Option<Arc<dyn ObjectStore>>,
connection_pool: ConnectionPool<Core>,
prover_connection_pool: ConnectionPool<Prover>,
protocol_versions: Vec<FriProtocolVersionId>,
protocol_versions: Vec<ProtocolVersionId>,
) -> Self {
Self {
config: Arc::new(config),
Expand Down
6 changes: 3 additions & 3 deletions prover/witness_generator/src/leaf_aggregation.rs
Expand Up @@ -29,7 +29,7 @@ use zksync_prover_fri_types::{
use zksync_prover_fri_utils::get_recursive_layer_circuit_id_for_base_layer;
use zksync_queued_job_processor::JobProcessor;
use zksync_types::{
basic_fri_types::AggregationRound, protocol_version::FriProtocolVersionId,
basic_fri_types::AggregationRound, protocol_version::ProtocolVersionId,
prover_dal::LeafAggregationJobMetadata, L1BatchNumber,
};
use zksync_vk_setup_data_server_fri::keystore::Keystore;
Expand Down Expand Up @@ -74,15 +74,15 @@ pub struct LeafAggregationWitnessGenerator {
config: FriWitnessGeneratorConfig,
object_store: Arc<dyn ObjectStore>,
prover_connection_pool: ConnectionPool<Prover>,
protocol_versions: Vec<FriProtocolVersionId>,
protocol_versions: Vec<ProtocolVersionId>,
}

impl LeafAggregationWitnessGenerator {
pub async fn new(
config: FriWitnessGeneratorConfig,
store_factory: &ObjectStoreFactory,
prover_connection_pool: ConnectionPool<Prover>,
protocol_versions: Vec<FriProtocolVersionId>,
protocol_versions: Vec<ProtocolVersionId>,
) -> Self {
Self {
config,
Expand Down

0 comments on commit 6aa51b0

Please sign in to comment.