Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

added srs_cache cli arg #2940

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

martyall
Copy link
Contributor

@martyall martyall commented Jan 9, 2025

This has been floating around for a while but is extremely useful to save time on loading the SRS

@martyall martyall requested a review from dannywillems January 9, 2025 16:57
srs
let srs: SRS<Vesta> = match &args.srs_cache {
Some(cache) => {
debug!("Loading SRS from cache {}", cache);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would check that the SRS is of the expected size (DOMAIN_SIZE).

@@ -1,6 +1,6 @@
use ark_ff::UniformRand;
use clap::Parser;
use kimchi::circuits::domains::EvaluationDomains;
use kimchi::{circuits::domains::EvaluationDomains, precomputed_srs::TestSRS};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of the scope of this PR: it would be nice to move this structure TestSRS in poly_commitment in ipa.rs, next to SRS. We should also have a unified structure for TestSRS and SRS. It is mostly the encoding the difference. I guess we can have a parameter or something to serde.

@@ -105,6 +105,8 @@ impl From<MipsVmConfigurationArgs> for VmConfiguration {
pub struct RunArgs {
#[arg(long = "preimage-db-dir", value_name = "PREIMAGE_DB_DIR")]
pub preimage_db_dir: Option<String>,
#[arg(long = "srs-cache", value_name = "SRS_CACHE")]
Copy link
Member

@dannywillems dannywillems Jan 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe simply srs or srs_filename or srs_path?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants