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 <aryasolhi@gmail.com>

* Change `ATTEMPTS` back to `RETRIES`

---------

Co-authored-by: Arya <aryasolhi@gmail.com>
This commit is contained in:
Marek 2023-04-08 00:26:36 +02:00 committed by GitHub
parent 2162cbdcc0
commit 044ecb0ac2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
1 changed files with 31 additions and 32 deletions

View File

@ -15,19 +15,19 @@ use crate::BoxError;
/// But `zebrad start` downloads blocks at the same time, so we allow some extra time. /// But `zebrad start` downloads blocks at the same time, so we allow some extra time.
pub const PARAMETER_DOWNLOAD_TIMEOUT: u64 = 60 * 60; 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 /// Zebra will retry the download only if the previous attempt fails.
/// failed for whatever reason. pub const PARAMETER_DOWNLOAD_MAX_RETRIES: usize = 2;
pub const PARAMETER_DOWNLOAD_MAX_RETRIES: usize = 3;
lazy_static::lazy_static! { lazy_static::lazy_static! {
/// Groth16 Zero-Knowledge Proof parameters for the Sapling and Sprout circuits. /// Groth16 Zero-Knowledge Proof parameters for the Sapling and Sprout circuits.
/// ///
/// When this static is accessed: /// This static is accessed when Zebra:
/// - 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, /// - needs to download and cache the parameters to a shared directory, or
/// - the parameters are loaded into Zebra. /// - checks the file hashes for both newly downloaded and previously cached files, or
/// - loads the parameters.
/// ///
/// # Panics /// # Panics
/// ///
@ -61,24 +61,23 @@ pub struct SproutParameters {
} }
impl Groth16Parameters { 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 /// # Panics
/// ///
/// If the parameters were downloaded to the wrong path. /// - If the parameters were downloaded to a wrong path.
/// After `PARAMETER_DOWNLOAD_MAX_RETRIES` failed download attempts. /// - After [`PARAMETER_DOWNLOAD_MAX_RETRIES`] failed download retries.
/// If the downloaded or pre-existing parameter files are invalid. /// - If the downloaded or pre-existing parameter files are invalid.
fn new() -> Groth16Parameters { fn new() -> Groth16Parameters {
let params_directory = Groth16Parameters::directory(); let params_directory = Groth16Parameters::directory();
let sapling_spend_path = params_directory.join(zcash_proofs::SAPLING_SPEND_NAME); 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 sapling_output_path = params_directory.join(zcash_proofs::SAPLING_OUTPUT_NAME);
let sprout_path = params_directory.join(zcash_proofs::SPROUT_NAME); let sprout_path = params_directory.join(zcash_proofs::SPROUT_NAME);
Groth16Parameters::retry_download_sapling_parameters( Groth16Parameters::download_sapling_parameters(&sapling_spend_path, &sapling_output_path);
&sapling_spend_path, Groth16Parameters::download_sprout_parameters(&sprout_path);
&sapling_output_path,
);
Groth16Parameters::retry_download_sprout_parameters(&sprout_path);
// TODO: if loading fails, log a message including `failure_hint` // TODO: if loading fails, log a message including `failure_hint`
tracing::info!("checking and loading Zcash Sapling and Sprout parameters"); 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 /// # Panics
/// ///
/// If the parameters were downloaded to the wrong path. /// - If the parameters were downloaded to a wrong path.
/// After `PARAMETER_DOWNLOAD_MAX_RETRIES` failed download attempts. /// - After [`PARAMETER_DOWNLOAD_MAX_RETRIES`] failed download retries.
fn retry_download_sapling_parameters(sapling_spend_path: &Path, sapling_output_path: &Path) { 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 // TODO: instead of the path check, add a zcash_proofs argument to skip hashing existing files
// (we check them on load anyway) // (we check them on load anyway)
if !sapling_spend_path.exists() || !sapling_output_path.exists() { if !sapling_spend_path.exists() || !sapling_output_path.exists() {
@ -137,7 +136,7 @@ impl Groth16Parameters {
sapling_output_path, sapling_output_path,
) { ) {
retries += 1; retries += 1;
if retries >= PARAMETER_DOWNLOAD_MAX_RETRIES { if retries > PARAMETER_DOWNLOAD_MAX_RETRIES {
panic!( panic!(
"error downloading Sapling parameter files after {} retries. {:?} {}", "error downloading Sapling parameter files after {} retries. {:?} {}",
PARAMETER_DOWNLOAD_MAX_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 /// # Panics
/// ///
/// If the parameters were downloaded to different paths to `sapling_spend_path` /// If the parameters were downloaded to paths different to `sapling_spend_path` or
/// or `sapling_output_path`. /// `sapling_output_path`.
fn download_sapling_parameters_once( fn download_sapling_parameters_once(
sapling_spend_path: &Path, sapling_spend_path: &Path,
sapling_output_path: &Path, sapling_output_path: &Path,
@ -173,20 +172,20 @@ impl Groth16Parameters {
Ok(new_sapling_paths) Ok(new_sapling_paths)
} }
/// Download Sprout parameters and retry [`PARAMETER_DOWNLOAD_MAX_RETRIES`] if it fails. /// Downloads the Sprout parameters.
/// ///
/// # Panics /// # Panics
/// ///
/// If the parameters were downloaded to the wrong path. /// - If the parameters were downloaded to a wrong path.
/// After `PARAMETER_DOWNLOAD_MAX_RETRIES` failed download attempts. /// - After [`PARAMETER_DOWNLOAD_MAX_RETRIES`] failed download retries.
fn retry_download_sprout_parameters(sprout_path: &Path) { fn download_sprout_parameters(sprout_path: &Path) {
if !sprout_path.exists() { if !sprout_path.exists() {
tracing::info!("downloading Zcash Sprout parameters"); tracing::info!("downloading Zcash Sprout parameters");
let mut retries = 0; let mut retries = 0;
while let Err(error) = Groth16Parameters::download_sprout_parameters_once(sprout_path) { while let Err(error) = Groth16Parameters::download_sprout_parameters_once(sprout_path) {
retries += 1; retries += 1;
if retries >= PARAMETER_DOWNLOAD_MAX_RETRIES { if retries > PARAMETER_DOWNLOAD_MAX_RETRIES {
panic!( panic!(
"error downloading Sprout parameter files after {} retries. {:?} {}", "error downloading Sprout parameter files after {} retries. {:?} {}",
PARAMETER_DOWNLOAD_MAX_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 /// # 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<PathBuf, BoxError> { fn download_sprout_parameters_once(sprout_path: &Path) -> Result<PathBuf, BoxError> {
let new_sprout_path = let new_sprout_path =
zcash_proofs::download_sprout_parameters(Some(PARAMETER_DOWNLOAD_TIMEOUT))?; zcash_proofs::download_sprout_parameters(Some(PARAMETER_DOWNLOAD_TIMEOUT))?;