Skip to content

Commit

Permalink
feat(prover): remove redundant config fields (#1787)
Browse files Browse the repository at this point in the history
## What ❔

Remove config fields which are meant for sampling
Update buckets for witness generation time metrics

## Why ❔

They are not used now anyway

## 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).
- [x] Tests for the changes have been added / updated.
- [x] Documentation comments have been added / updated.
- [x] Code has been formatted via `zk fmt` and `zk lint`.
- [x] Spellcheck has been run via `zk spellcheck`.
- [x] Linkcheck has been run via `zk linkcheck`.
  • Loading branch information
Artemka374 authored Apr 25, 2024
1 parent 3417941 commit a784ea6
Show file tree
Hide file tree
Showing 9 changed files with 14 additions and 189 deletions.
6 changes: 0 additions & 6 deletions core/lib/config/src/configs/fri_witness_generator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,10 @@ pub struct FriWitnessGeneratorConfig {
pub node_generation_timeout_in_secs: Option<u16>,
/// Max attempts for generating witness
pub max_attempts: u32,
// Percentage of the blocks that gets proven in the range [0.0, 1.0]
// when 0.0 implies all blocks are skipped and 1.0 implies all blocks are proven.
pub blocks_proving_percentage: Option<u8>,
pub dump_arguments_for_blocks: Vec<u32>,
// Optional l1 batch number to process block until(inclusive).
// This parameter is used in case of performing circuit upgrades(VK/Setup keys),
// to not let witness-generator pick new job and finish all the existing jobs with old circuit.
pub last_l1_batch_to_process: Option<u32>,
// Force process block with specified number when sampling is enabled.
pub force_process_block: Option<u32>,

// whether to write to public GCS bucket for https://github.com/matter-labs/era-boojum-validator-cli
pub shall_save_to_public_bucket: bool,
Expand Down
3 changes: 0 additions & 3 deletions core/lib/config/src/testonly.rs
Original file line number Diff line number Diff line change
Expand Up @@ -542,10 +542,7 @@ impl Distribution<configs::FriWitnessGeneratorConfig> for EncodeDist {
node_generation_timeout_in_secs: self.sample(rng),
scheduler_generation_timeout_in_secs: self.sample(rng),
max_attempts: self.sample(rng),
blocks_proving_percentage: self.sample(rng),
dump_arguments_for_blocks: self.sample_collect(rng),
last_l1_batch_to_process: self.sample(rng),
force_process_block: self.sample(rng),
shall_save_to_public_bucket: self.sample(rng),
}
}
Expand Down
9 changes: 0 additions & 9 deletions core/lib/env_config/src/fri_witness_generator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,7 @@ mod tests {
node_generation_timeout_in_secs: Some(800u16),
scheduler_generation_timeout_in_secs: Some(900u16),
max_attempts: 4,
blocks_proving_percentage: Some(30),
dump_arguments_for_blocks: vec![2, 3],
last_l1_batch_to_process: None,
force_process_block: Some(1),
shall_save_to_public_bucket: true,
}
}
Expand All @@ -41,9 +38,6 @@ mod tests {
FRI_WITNESS_NODE_GENERATION_TIMEOUT_IN_SECS=800
FRI_WITNESS_SCHEDULER_GENERATION_TIMEOUT_IN_SECS=900
FRI_WITNESS_MAX_ATTEMPTS=4
FRI_WITNESS_DUMP_ARGUMENTS_FOR_BLOCKS="2,3"
FRI_WITNESS_BLOCKS_PROVING_PERCENTAGE="30"
FRI_WITNESS_FORCE_PROCESS_BLOCK="1"
FRI_WITNESS_SHALL_SAVE_TO_PUBLIC_BUCKET=true
"#;
lock.set_env(config);
Expand All @@ -64,9 +58,6 @@ mod tests {
FRI_WITNESS_BASIC_GENERATION_TIMEOUT_IN_SECS=100
FRI_WITNESS_SCHEDULER_GENERATION_TIMEOUT_IN_SECS=200
FRI_WITNESS_MAX_ATTEMPTS=4
FRI_WITNESS_DUMP_ARGUMENTS_FOR_BLOCKS="2,3"
FRI_WITNESS_BLOCKS_PROVING_PERCENTAGE="30"
FRI_WITNESS_FORCE_PROCESS_BLOCK="1"
FRI_WITNESS_SHALL_SAVE_TO_PUBLIC_BUCKET=true
"#;
lock.set_env(config);
Expand Down
7 changes: 3 additions & 4 deletions core/lib/protobuf_config/src/proto/config/prover.proto
Original file line number Diff line number Diff line change
Expand Up @@ -70,16 +70,15 @@ message ProverGateway {

message WitnessGenerator {
optional uint32 generation_timeout_in_secs = 1; // required;
optional uint32 max_attempts = 2; // required
optional uint32 blocks_proving_percentage = 3; // optional; 0-100
repeated uint32 dump_arguments_for_blocks = 4;
optional uint32 max_attempts = 2; // required;
optional uint32 last_l1_batch_to_process = 5; // optional
optional uint32 force_process_block = 6; // optional
optional bool shall_save_to_public_bucket = 7; // required
optional uint32 basic_generation_timeout_in_secs = 8; // optional;
optional uint32 leaf_generation_timeout_in_secs = 9; // optional;
optional uint32 node_generation_timeout_in_secs = 10; // optional;
optional uint32 scheduler_generation_timeout_in_secs = 11; // optional;
reserved 3, 4, 6;
reserved "dump_arguments_for_blocks", "force_process_block", "blocks_proving_percentage";
}

message WitnessVectorGenerator {
Expand Down
10 changes: 0 additions & 10 deletions core/lib/protobuf_config/src/prover.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,14 +161,7 @@ impl ProtoRepr for proto::WitnessGenerator {
.and_then(|x| Ok((*x).try_into()?))
.context("generation_timeout_in_secs")?,
max_attempts: *required(&self.max_attempts).context("max_attempts")?,
blocks_proving_percentage: self
.blocks_proving_percentage
.map(|x| x.try_into())
.transpose()
.context("blocks_proving_percentage")?,
dump_arguments_for_blocks: self.dump_arguments_for_blocks.clone(),
last_l1_batch_to_process: self.last_l1_batch_to_process,
force_process_block: self.force_process_block,
shall_save_to_public_bucket: *required(&self.shall_save_to_public_bucket)
.context("shall_save_to_public_bucket")?,
basic_generation_timeout_in_secs: self
Expand Down Expand Up @@ -198,10 +191,7 @@ impl ProtoRepr for proto::WitnessGenerator {
Self {
generation_timeout_in_secs: Some(this.generation_timeout_in_secs.into()),
max_attempts: Some(this.max_attempts),
blocks_proving_percentage: this.blocks_proving_percentage.map(|x| x.into()),
dump_arguments_for_blocks: this.dump_arguments_for_blocks.clone(),
last_l1_batch_to_process: this.last_l1_batch_to_process,
force_process_block: this.force_process_block,
shall_save_to_public_bucket: Some(this.shall_save_to_public_bucket),
basic_generation_timeout_in_secs: this
.basic_generation_timeout_in_secs
Expand Down
2 changes: 0 additions & 2 deletions etc/env/base/fri_witness_generator.toml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,4 @@ leaf_generation_timeout_in_secs = 900
node_generation_timeout_in_secs = 900
scheduler_generation_timeout_in_secs = 900
max_attempts = 10
dump_arguments_for_blocks = "1"
force_process_block = 1
shall_save_to_public_bucket = true
2 changes: 0 additions & 2 deletions etc/env/file_based/general.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -174,10 +174,8 @@ prover:
zone_read_url: http://metadata.google.internal/computeMetadata/v1/instance/zone
shall_save_to_public_bucket: true
witness_generator:
dump_arguments_for_blocks: [ 2,3 ]
generation_timeout_in_secs: 900
max_attempts: 10
force_process_block: 1
shall_save_to_public_bucket: true
witness_vector_generator:
prover_instance_wait_timeout_in_secs: 200
Expand Down
157 changes: 9 additions & 148 deletions prover/witness_generator/src/basic_circuits.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use std::{
collections::{hash_map::DefaultHasher, HashMap, HashSet},
collections::{hash_map::DefaultHasher, HashSet},
hash::{Hash, Hasher},
sync::Arc,
time::Instant,
Expand All @@ -19,20 +19,15 @@ use circuit_definitions::{
use multivm::vm_1_4_2::{
constants::MAX_CYCLES_FOR_TX, HistoryDisabled, StorageOracle as VmStorageOracle,
};
use prover_dal::{
fri_witness_generator_dal::FriWitnessJobStatus, ConnectionPool, Prover, ProverDal,
};
use rand::Rng;
use serde::{Deserialize, Serialize};
use prover_dal::{ConnectionPool, Prover, ProverDal};
use tracing::Instrument;
use zkevm_test_harness::{
geometry_config::get_geometry_config, toolset::GeometryConfig,
utils::generate_eip4844_circuit_and_witness,
geometry_config::get_geometry_config, utils::generate_eip4844_circuit_and_witness,
zkevm_circuits::eip_4844::input::EIP4844OutputDataWitness,
};
use zksync_config::configs::FriWitnessGeneratorConfig;
use zksync_dal::{Core, CoreDal};
use zksync_object_store::{Bucket, ObjectStore, ObjectStoreFactory, StoredObject};
use zksync_object_store::{ObjectStore, ObjectStoreFactory};
use zksync_prover_fri_types::{
circuit_definitions::{
boojum::{
Expand All @@ -56,7 +51,7 @@ use zksync_types::{
AggregationRound, Eip4844Blobs, EIP_4844_BLOB_SIZE, MAX_4844_BLOBS_PER_BLOCK,
},
protocol_version::ProtocolVersionId,
Address, L1BatchNumber, BOOTLOADER_ADDRESS, H256, U256,
Address, L1BatchNumber, BOOTLOADER_ADDRESS, H256,
};
use zksync_utils::{bytes_to_chunks, h256_to_u256, u256_to_h256};

Expand Down Expand Up @@ -134,49 +129,15 @@ impl BasicWitnessGenerator {
async fn process_job_impl(
object_store: Arc<dyn ObjectStore>,
connection_pool: ConnectionPool<Core>,
prover_connection_pool: ConnectionPool<Prover>,
basic_job: BasicWitnessGeneratorJob,
started_at: Instant,
config: Arc<FriWitnessGeneratorConfig>,
) -> Option<BasicCircuitArtifacts> {
let BasicWitnessGeneratorJob {
block_number,
job,
eip_4844_blobs,
} = basic_job;
let shall_force_process_block = config
.force_process_block
.map_or(false, |block| block == block_number.0);

if let Some(blocks_proving_percentage) = config.blocks_proving_percentage {
// Generate random number in (0; 100).
let threshold = rand::thread_rng().gen_range(1..100);
// We get value higher than `blocks_proving_percentage` with prob = `1 - blocks_proving_percentage`.
// In this case job should be skipped.
if threshold > blocks_proving_percentage && !shall_force_process_block {
WITNESS_GENERATOR_METRICS.skipped_blocks.inc();
tracing::info!(
"Skipping witness generation for block {}, blocks_proving_percentage: {}",
block_number.0,
blocks_proving_percentage
);

let mut prover_storage = prover_connection_pool.connection().await.unwrap();
let mut transaction = prover_storage.start_transaction().await.unwrap();
transaction
.fri_proof_compressor_dal()
.skip_proof_compression_job(block_number)
.await;
transaction
.fri_witness_generator_dal()
.mark_witness_job(FriWitnessJobStatus::Skipped, block_number)
.await;
transaction.commit().await.unwrap();
return None;
}
}

WITNESS_GENERATOR_METRICS.sampled_blocks.inc();
tracing::info!(
"Starting witness generation of type {:?} for block {}",
AggregationRound::BasicCircuits,
Expand All @@ -186,7 +147,6 @@ impl BasicWitnessGenerator {
Some(
process_basic_circuits_job(
&*object_store,
config,
connection_pool,
started_at,
block_number,
Expand Down Expand Up @@ -254,22 +214,15 @@ impl JobProcessor for BasicWitnessGenerator {
job: BasicWitnessGeneratorJob,
started_at: Instant,
) -> tokio::task::JoinHandle<anyhow::Result<Option<BasicCircuitArtifacts>>> {
let config = Arc::clone(&self.config);
let object_store = Arc::clone(&self.object_store);
let connection_pool = self.connection_pool.clone();
let prover_connection_pool = self.prover_connection_pool.clone();
tokio::spawn(async move {
let block_number = job.block_number;
Ok(Self::process_job_impl(
object_store,
connection_pool,
prover_connection_pool,
job,
started_at,
config,
Ok(
Self::process_job_impl(object_store, connection_pool, job, started_at)
.instrument(tracing::info_span!("basic_circuit", %block_number))
.await,
)
.instrument(tracing::info_span!("basic_circuit", %block_number))
.await)
})
}

Expand Down Expand Up @@ -335,7 +288,6 @@ impl JobProcessor for BasicWitnessGenerator {
#[allow(clippy::too_many_arguments)]
async fn process_basic_circuits_job(
object_store: &dyn ObjectStore,
config: Arc<FriWitnessGeneratorConfig>,
connection_pool: ConnectionPool<Core>,
started_at: Instant,
block_number: L1BatchNumber,
Expand All @@ -348,7 +300,6 @@ async fn process_basic_circuits_job(
generate_witness(
block_number,
object_store,
config,
connection_pool,
witness_gen_input,
eip_4844_blobs,
Expand Down Expand Up @@ -535,7 +486,6 @@ async fn build_basic_circuits_witness_generator_input(
async fn generate_witness(
block_number: L1BatchNumber,
object_store: &dyn ObjectStore,
config: Arc<FriWitnessGeneratorConfig>,
connection_pool: ConnectionPool<Core>,
input: BasicCircuitWitnessGeneratorInput,
eip_4844_blobs: Eip4844Blobs,
Expand Down Expand Up @@ -643,29 +593,6 @@ async fn generate_witness(
hasher.finish()
);

let should_dump_arguments = config
.dump_arguments_for_blocks
.contains(&input.block_number.0);
if should_dump_arguments {
save_run_with_fixed_params_args_to_gcs(
object_store,
input.block_number.0,
last_miniblock_number.0,
Address::zero(),
BOOTLOADER_ADDRESS,
bootloader_code.clone(),
bootloader_contents.clone(),
false,
account_code_hash,
used_bytecodes.clone(),
Vec::default(),
MAX_CYCLES_FOR_TX as usize,
geometry_config,
tree.clone(),
)
.await;
}

// The following part is CPU-heavy, so we move it to a separate thread.
let rt_handle = tokio::runtime::Handle::current();

Expand Down Expand Up @@ -781,69 +708,3 @@ async fn generate_witness(
block_aux_witness,
)
}

#[allow(clippy::too_many_arguments)]
async fn save_run_with_fixed_params_args_to_gcs(
object_store: &dyn ObjectStore,
l1_batch_number: u32,
last_miniblock_number: u32,
caller: Address,
entry_point_address: Address,
entry_point_code: Vec<[u8; 32]>,
initial_heap_content: Vec<u8>,
zk_porter_is_available: bool,
default_aa_code_hash: U256,
used_bytecodes: HashMap<U256, Vec<[u8; 32]>>,
ram_verification_queries: Vec<(u32, U256)>,
cycle_limit: usize,
geometry: GeometryConfig,
tree: PrecalculatedMerklePathsProvider,
) {
let run_with_fixed_params_input = RunWithFixedParamsInput {
l1_batch_number,
last_miniblock_number,
caller,
entry_point_address,
entry_point_code,
initial_heap_content,
zk_porter_is_available,
default_aa_code_hash,
used_bytecodes,
ram_verification_queries,
cycle_limit,
geometry,
tree,
};
object_store
.put(L1BatchNumber(l1_batch_number), &run_with_fixed_params_input)
.await
.unwrap();
}

#[derive(Debug, Serialize, Deserialize, Clone, PartialEq)]
pub struct RunWithFixedParamsInput {
pub l1_batch_number: u32,
pub last_miniblock_number: u32,
pub caller: Address,
pub entry_point_address: Address,
pub entry_point_code: Vec<[u8; 32]>,
pub initial_heap_content: Vec<u8>,
pub zk_porter_is_available: bool,
pub default_aa_code_hash: U256,
pub used_bytecodes: HashMap<U256, Vec<[u8; 32]>>,
pub ram_verification_queries: Vec<(u32, U256)>,
pub cycle_limit: usize,
pub geometry: GeometryConfig,
pub tree: PrecalculatedMerklePathsProvider,
}

impl StoredObject for RunWithFixedParamsInput {
const BUCKET: Bucket = Bucket::WitnessInput;
type Key<'a> = L1BatchNumber;

fn encode_key(key: Self::Key<'_>) -> String {
format!("run_with_fixed_params_input_{}.bin", key)
}

zksync_object_store::serialize_using_bincode!();
}
7 changes: 2 additions & 5 deletions prover/witness_generator/src/metrics.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use std::time::Duration;

use vise::{Buckets, Counter, Family, Gauge, Histogram, LabeledFamily, Metrics};
use vise::{Buckets, Family, Gauge, Histogram, LabeledFamily, Metrics};
use zksync_prover_fri_utils::metrics::StageLabel;

#[derive(Debug, Metrics)]
Expand All @@ -10,13 +10,10 @@ pub(crate) struct WitnessGeneratorMetrics {
pub blob_fetch_time: Family<StageLabel, Histogram<Duration>>,
#[metrics(buckets = Buckets::LATENCIES)]
pub prepare_job_time: Family<StageLabel, Histogram<Duration>>,
#[metrics(buckets = Buckets::LATENCIES)]
#[metrics(buckets = Buckets::exponential(60.0..=61440.0, 2.0))]
pub witness_generation_time: Family<StageLabel, Histogram<Duration>>,
#[metrics(buckets = Buckets::LATENCIES)]
pub blob_save_time: Family<StageLabel, Histogram<Duration>>,

pub sampled_blocks: Counter,
pub skipped_blocks: Counter,
}

#[vise::register]
Expand Down

0 comments on commit a784ea6

Please sign in to comment.