Skip to content

Commit

Permalink
fix: Inconsistent Docs for Downloading ZKP Params (#6464)
Browse files Browse the repository at this point in the history 
* 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 <[email protected]>

* Change `ATTEMPTS` back to `RETRIES`

---------

Co-authored-by: Arya <[email protected]>
  • Loading branch information
@upbqdn @arya2
upbqdn and arya2 authored Apr 7, 2023
1 parent 2162cbd commit 044ecb0
Showing 1 changed file with 31 additions and 32 deletions.
63 changes: 31 additions & 32 deletions zebra-consensus/src/primitives/groth16/params.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
///
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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() {
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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<PathBuf, BoxError> {
let new_sprout_path =
zcash_proofs::download_sprout_parameters(Some(PARAMETER_DOWNLOAD_TIMEOUT))?;
Expand Down

0 comments on commit 044ecb0

Please sign in to comment.