From 044ecb0ac295d60f2a4e93adb328ce8a986b218d Mon Sep 17 00:00:00 2001 From: Marek Date: Sat, 8 Apr 2023 00:26:36 +0200 Subject: [PATCH] fix: Inconsistent Docs for Downloading ZKP Params (#6464) * Use `ATTEMPTS` instead of `RETRIES` This commit also removes the prefix `retry_` from the function names that download the parameters. The reason for removing it was that I consider the retries implicit, and the primary objective of the functions is to download the params, not retry to download them. * Docs cosmetics * Apply suggestions from code review Co-authored-by: Arya * Change `ATTEMPTS` back to `RETRIES` --------- Co-authored-by: Arya --- .../src/primitives/groth16/params.rs | 63 +++++++++---------- 1 file changed, 31 insertions(+), 32 deletions(-) diff --git a/zebra-consensus/src/primitives/groth16/params.rs b/zebra-consensus/src/primitives/groth16/params.rs index b47751bed..52fb20b75 100644 --- a/zebra-consensus/src/primitives/groth16/params.rs +++ b/zebra-consensus/src/primitives/groth16/params.rs @@ -15,19 +15,19 @@ use crate::BoxError; /// But `zebrad start` downloads blocks at the same time, so we allow some extra time. pub const PARAMETER_DOWNLOAD_TIMEOUT: u64 = 60 * 60; -/// The maximum number of times to retry download parameters. +/// The maximum number of times Zebra retries to download the parameters. /// -/// Zebra will retry to download Sprout of Sapling parameters only if they -/// failed for whatever reason. -pub const PARAMETER_DOWNLOAD_MAX_RETRIES: usize = 3; +/// Zebra will retry the download only if the previous attempt fails. +pub const PARAMETER_DOWNLOAD_MAX_RETRIES: usize = 2; lazy_static::lazy_static! { /// Groth16 Zero-Knowledge Proof parameters for the Sapling and Sprout circuits. /// - /// When this static is accessed: - /// - the parameters are downloaded if needed, then cached to a shared directory, - /// - the file hashes are checked, for both newly downloaded and previously cached files, - /// - the parameters are loaded into Zebra. + /// This static is accessed when Zebra: + /// + /// - needs to download and cache the parameters to a shared directory, or + /// - checks the file hashes for both newly downloaded and previously cached files, or + /// - loads the parameters. /// /// # Panics /// @@ -61,24 +61,23 @@ pub struct SproutParameters { } impl Groth16Parameters { - /// Download if needed, cache, check, and load the Sprout and Sapling Groth16 parameters. + /// Loads the Sprout and Sapling Groth16 parameters, checking the sizes and hashes of the files. + /// + /// If the parameters are not present, they are automatically downloaded and cached. /// /// # Panics /// - /// If the parameters were downloaded to the wrong path. - /// After `PARAMETER_DOWNLOAD_MAX_RETRIES` failed download attempts. - /// If the downloaded or pre-existing parameter files are invalid. + /// - If the parameters were downloaded to a wrong path. + /// - After [`PARAMETER_DOWNLOAD_MAX_RETRIES`] failed download retries. + /// - If the downloaded or pre-existing parameter files are invalid. fn new() -> Groth16Parameters { let params_directory = Groth16Parameters::directory(); let sapling_spend_path = params_directory.join(zcash_proofs::SAPLING_SPEND_NAME); let sapling_output_path = params_directory.join(zcash_proofs::SAPLING_OUTPUT_NAME); let sprout_path = params_directory.join(zcash_proofs::SPROUT_NAME); - Groth16Parameters::retry_download_sapling_parameters( - &sapling_spend_path, - &sapling_output_path, - ); - Groth16Parameters::retry_download_sprout_parameters(&sprout_path); + Groth16Parameters::download_sapling_parameters(&sapling_spend_path, &sapling_output_path); + Groth16Parameters::download_sprout_parameters(&sprout_path); // TODO: if loading fails, log a message including `failure_hint` tracing::info!("checking and loading Zcash Sapling and Sprout parameters"); @@ -119,13 +118,13 @@ impl Groth16Parameters { ) } - /// Download Sapling parameters and retry [`PARAMETER_DOWNLOAD_MAX_RETRIES`] if it fails. + /// Downloads the Sapling parameters. /// /// # Panics /// - /// If the parameters were downloaded to the wrong path. - /// After `PARAMETER_DOWNLOAD_MAX_RETRIES` failed download attempts. - fn retry_download_sapling_parameters(sapling_spend_path: &Path, sapling_output_path: &Path) { + /// - If the parameters were downloaded to a wrong path. + /// - After [`PARAMETER_DOWNLOAD_MAX_RETRIES`] failed download retries. + fn download_sapling_parameters(sapling_spend_path: &Path, sapling_output_path: &Path) { // TODO: instead of the path check, add a zcash_proofs argument to skip hashing existing files // (we check them on load anyway) if !sapling_spend_path.exists() || !sapling_output_path.exists() { @@ -137,7 +136,7 @@ impl Groth16Parameters { sapling_output_path, ) { retries += 1; - if retries >= PARAMETER_DOWNLOAD_MAX_RETRIES { + if retries > PARAMETER_DOWNLOAD_MAX_RETRIES { panic!( "error downloading Sapling parameter files after {} retries. {:?} {}", PARAMETER_DOWNLOAD_MAX_RETRIES, @@ -154,12 +153,12 @@ impl Groth16Parameters { } } - /// Try to download the Sapling parameters once, and return the result. + /// Tries to download the Sapling parameters once and returns the result. /// /// # Panics /// - /// If the parameters were downloaded to different paths to `sapling_spend_path` - /// or `sapling_output_path`. + /// If the parameters were downloaded to paths different to `sapling_spend_path` or + /// `sapling_output_path`. fn download_sapling_parameters_once( sapling_spend_path: &Path, sapling_output_path: &Path, @@ -173,20 +172,20 @@ impl Groth16Parameters { Ok(new_sapling_paths) } - /// Download Sprout parameters and retry [`PARAMETER_DOWNLOAD_MAX_RETRIES`] if it fails. + /// Downloads the Sprout parameters. /// /// # Panics /// - /// If the parameters were downloaded to the wrong path. - /// After `PARAMETER_DOWNLOAD_MAX_RETRIES` failed download attempts. - fn retry_download_sprout_parameters(sprout_path: &Path) { + /// - If the parameters were downloaded to a wrong path. + /// - After [`PARAMETER_DOWNLOAD_MAX_RETRIES`] failed download retries. + fn download_sprout_parameters(sprout_path: &Path) { if !sprout_path.exists() { tracing::info!("downloading Zcash Sprout parameters"); let mut retries = 0; while let Err(error) = Groth16Parameters::download_sprout_parameters_once(sprout_path) { retries += 1; - if retries >= PARAMETER_DOWNLOAD_MAX_RETRIES { + if retries > PARAMETER_DOWNLOAD_MAX_RETRIES { panic!( "error downloading Sprout parameter files after {} retries. {:?} {}", PARAMETER_DOWNLOAD_MAX_RETRIES, @@ -203,11 +202,11 @@ impl Groth16Parameters { } } - /// Try to download the Sprout parameters once, and return the result. + /// Tries to download the Sprout parameters once and returns the result. /// /// # Panics /// - /// If the parameters were downloaded to a different path to `sprout_path`. + /// If the parameters were downloaded to a path different to `sprout_path`. fn download_sprout_parameters_once(sprout_path: &Path) -> Result { let new_sprout_path = zcash_proofs::download_sprout_parameters(Some(PARAMETER_DOWNLOAD_TIMEOUT))?;