From da88884ad83b1bb84b00d1299b31028ec165f5cd Mon Sep 17 00:00:00 2001 From: SamiAlHassan <150172180+SamiAlHassan@users.noreply.github.com> Date: Mon, 18 Mar 2024 17:12:13 +0000 Subject: [PATCH 1/5] improve nargo check --- tooling/nargo_cli/src/cli/check_cmd.rs | 34 ++++++++++++++++++++------ 1 file changed, 26 insertions(+), 8 deletions(-) diff --git a/tooling/nargo_cli/src/cli/check_cmd.rs b/tooling/nargo_cli/src/cli/check_cmd.rs index 897073f4e20..76a7f26a33c 100644 --- a/tooling/nargo_cli/src/cli/check_cmd.rs +++ b/tooling/nargo_cli/src/cli/check_cmd.rs @@ -34,6 +34,10 @@ pub(crate) struct CheckCommand { #[clap(long, conflicts_with = "package")] workspace: bool, + /// Force overwrite of existing files + #[clap(long = "override")] + allow_override: bool, + #[clap(flatten)] compile_options: CompileOptions, } @@ -58,8 +62,11 @@ pub(crate) fn run( let parsed_files = parse_all(&workspace_file_manager); for package in &workspace { - check_package(&workspace_file_manager, &parsed_files, package, &args.compile_options)?; - println!("[{}] Constraint system successfully built!", package.name); + + let any_file_written = check_package(&workspace_file_manager, &parsed_files, package, &args.compile_options, args.allow_override)?; + if any_file_written { + println!("[{}] Constraint system successfully built!", package.name); + } } Ok(()) } @@ -69,7 +76,8 @@ fn check_package( parsed_files: &ParsedFiles, package: &Package, compile_options: &CompileOptions, -) -> Result<(), CompileError> { + allow_override: bool, +) -> Result { let (mut context, crate_id) = prepare_package(file_manager, parsed_files, package); check_crate_and_report_errors( &mut context, @@ -81,27 +89,37 @@ fn check_package( if package.is_library() || package.is_contract() { // Libraries do not have ABIs while contracts have many, so we cannot generate a `Prover.toml` file. - Ok(()) + Ok(false) } else { // XXX: We can have a --overwrite flag to determine if you want to overwrite the Prover/Verifier.toml files if let Some((parameters, return_type)) = compute_function_abi(&context, &crate_id) { let path_to_prover_input = package.prover_input_path(); let path_to_verifier_input = package.verifier_input_path(); + + // Before writing the file, check if it exists and whether override is set + let should_write_prover = !path_to_prover_input.exists() || allow_override; + let should_write_verifier = !path_to_verifier_input.exists() || allow_override; - // If they are not available, then create them and populate them based on the ABI - if !path_to_prover_input.exists() { + if should_write_prover { let prover_toml = create_input_toml_template(parameters.clone(), None); write_to_file(prover_toml.as_bytes(), &path_to_prover_input); + } else { + eprintln!("Warning: Prover.toml already exists. Use --override to force overwrite."); } - if !path_to_verifier_input.exists() { + + if should_write_verifier { let public_inputs = parameters.into_iter().filter(|param| param.is_public()).collect(); let verifier_toml = create_input_toml_template(public_inputs, return_type); write_to_file(verifier_toml.as_bytes(), &path_to_verifier_input); + } else { + eprintln!("Warning: Verifier.toml already exists. Use --override to force overwrite."); } - Ok(()) + let any_file_written = should_write_prover || should_write_verifier; + + Ok(any_file_written) } else { Err(CompileError::MissingMainFunction(package.name.clone())) } From ac3937a11c437e8bf980ae2d53c9e03f747fbaa0 Mon Sep 17 00:00:00 2001 From: SamiAlHassan <150172180+SamiAlHassan@users.noreply.github.com> Date: Tue, 19 Mar 2024 13:37:13 +0000 Subject: [PATCH 2/5] ran cargo fmt --- tooling/nargo_cli/src/cli/check_cmd.rs | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/tooling/nargo_cli/src/cli/check_cmd.rs b/tooling/nargo_cli/src/cli/check_cmd.rs index 76a7f26a33c..92b4434e9c0 100644 --- a/tooling/nargo_cli/src/cli/check_cmd.rs +++ b/tooling/nargo_cli/src/cli/check_cmd.rs @@ -36,8 +36,8 @@ pub(crate) struct CheckCommand { /// Force overwrite of existing files #[clap(long = "override")] - allow_override: bool, - + allow_override: bool, + #[clap(flatten)] compile_options: CompileOptions, } @@ -62,8 +62,13 @@ pub(crate) fn run( let parsed_files = parse_all(&workspace_file_manager); for package in &workspace { - - let any_file_written = check_package(&workspace_file_manager, &parsed_files, package, &args.compile_options, args.allow_override)?; + let any_file_written = check_package( + &workspace_file_manager, + &parsed_files, + package, + &args.compile_options, + args.allow_override, + )?; if any_file_written { println!("[{}] Constraint system successfully built!", package.name); } @@ -95,7 +100,7 @@ fn check_package( if let Some((parameters, return_type)) = compute_function_abi(&context, &crate_id) { let path_to_prover_input = package.prover_input_path(); let path_to_verifier_input = package.verifier_input_path(); - + // Before writing the file, check if it exists and whether override is set let should_write_prover = !path_to_prover_input.exists() || allow_override; let should_write_verifier = !path_to_verifier_input.exists() || allow_override; @@ -104,7 +109,9 @@ fn check_package( let prover_toml = create_input_toml_template(parameters.clone(), None); write_to_file(prover_toml.as_bytes(), &path_to_prover_input); } else { - eprintln!("Warning: Prover.toml already exists. Use --override to force overwrite."); + eprintln!( + "Warning: Prover.toml already exists. Use --override to force overwrite." + ); } if should_write_verifier { @@ -114,7 +121,9 @@ fn check_package( let verifier_toml = create_input_toml_template(public_inputs, return_type); write_to_file(verifier_toml.as_bytes(), &path_to_verifier_input); } else { - eprintln!("Warning: Verifier.toml already exists. Use --override to force overwrite."); + eprintln!( + "Warning: Verifier.toml already exists. Use --override to force overwrite." + ); } let any_file_written = should_write_prover || should_write_verifier; From 3f0e49fdccd50c77694540b88242f963df7060bd Mon Sep 17 00:00:00 2001 From: SamiAlHassan <150172180+SamiAlHassan@users.noreply.github.com> Date: Tue, 19 Mar 2024 15:06:49 +0000 Subject: [PATCH 3/5] apply suggested changes --- tooling/nargo_cli/src/cli/check_cmd.rs | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/tooling/nargo_cli/src/cli/check_cmd.rs b/tooling/nargo_cli/src/cli/check_cmd.rs index 92b4434e9c0..1ce9316e14c 100644 --- a/tooling/nargo_cli/src/cli/check_cmd.rs +++ b/tooling/nargo_cli/src/cli/check_cmd.rs @@ -35,8 +35,8 @@ pub(crate) struct CheckCommand { workspace: bool, /// Force overwrite of existing files - #[clap(long = "override")] - allow_override: bool, + #[clap(long = "overwrite")] + allow_overwrite: bool, #[clap(flatten)] compile_options: CompileOptions, @@ -67,7 +67,7 @@ pub(crate) fn run( &parsed_files, package, &args.compile_options, - args.allow_override, + args.allow_overwrite, )?; if any_file_written { println!("[{}] Constraint system successfully built!", package.name); @@ -76,12 +76,14 @@ pub(crate) fn run( Ok(()) } +/// Evaluates the necessity to create or update Prover.toml and Verifier.toml based on the allow_overwrite flag and files' existence. +/// Returns `true` if any file was generated or updated, `false` otherwise. fn check_package( file_manager: &FileManager, parsed_files: &ParsedFiles, package: &Package, compile_options: &CompileOptions, - allow_override: bool, + allow_overwrite: bool, ) -> Result { let (mut context, crate_id) = prepare_package(file_manager, parsed_files, package); check_crate_and_report_errors( @@ -101,16 +103,16 @@ fn check_package( let path_to_prover_input = package.prover_input_path(); let path_to_verifier_input = package.verifier_input_path(); - // Before writing the file, check if it exists and whether override is set - let should_write_prover = !path_to_prover_input.exists() || allow_override; - let should_write_verifier = !path_to_verifier_input.exists() || allow_override; + // Before writing the file, check if it exists and whether overwrite is set + let should_write_prover = !path_to_prover_input.exists() || allow_overwrite; + let should_write_verifier = !path_to_verifier_input.exists() || allow_overwrite; if should_write_prover { let prover_toml = create_input_toml_template(parameters.clone(), None); write_to_file(prover_toml.as_bytes(), &path_to_prover_input); } else { eprintln!( - "Warning: Prover.toml already exists. Use --override to force overwrite." + "Warning: Prover.toml already exists. Use --overwrite to force overwrite." ); } @@ -122,7 +124,7 @@ fn check_package( write_to_file(verifier_toml.as_bytes(), &path_to_verifier_input); } else { eprintln!( - "Warning: Verifier.toml already exists. Use --override to force overwrite." + "Warning: Verifier.toml already exists. Use --overwrite to force overwrite." ); } From 1679b6cc7fe13ac525ae3195ec6a0c7429ce45ba Mon Sep 17 00:00:00 2001 From: SamiAlHassan <150172180+SamiAlHassan@users.noreply.github.com> Date: Thu, 21 Mar 2024 13:57:04 +0000 Subject: [PATCH 4/5] Warning --> Note --- tooling/nargo_cli/src/cli/check_cmd.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tooling/nargo_cli/src/cli/check_cmd.rs b/tooling/nargo_cli/src/cli/check_cmd.rs index 1ce9316e14c..63738a38969 100644 --- a/tooling/nargo_cli/src/cli/check_cmd.rs +++ b/tooling/nargo_cli/src/cli/check_cmd.rs @@ -112,7 +112,7 @@ fn check_package( write_to_file(prover_toml.as_bytes(), &path_to_prover_input); } else { eprintln!( - "Warning: Prover.toml already exists. Use --overwrite to force overwrite." + "Note: Prover.toml already exists. Use --overwrite to force overwrite." ); } @@ -124,7 +124,7 @@ fn check_package( write_to_file(verifier_toml.as_bytes(), &path_to_verifier_input); } else { eprintln!( - "Warning: Verifier.toml already exists. Use --overwrite to force overwrite." + "Note: Verifier.toml already exists. Use --overwrite to force overwrite." ); } From f55f4d4fa3618849b2c5d92f85bd7546140a96c1 Mon Sep 17 00:00:00 2001 From: SamiAlHassan <150172180+SamiAlHassan@users.noreply.github.com> Date: Thu, 21 Mar 2024 14:33:28 +0000 Subject: [PATCH 5/5] ran cargo fmt again --- tooling/nargo_cli/src/cli/check_cmd.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tooling/nargo_cli/src/cli/check_cmd.rs b/tooling/nargo_cli/src/cli/check_cmd.rs index 63738a38969..2b729e44b8a 100644 --- a/tooling/nargo_cli/src/cli/check_cmd.rs +++ b/tooling/nargo_cli/src/cli/check_cmd.rs @@ -111,9 +111,7 @@ fn check_package( let prover_toml = create_input_toml_template(parameters.clone(), None); write_to_file(prover_toml.as_bytes(), &path_to_prover_input); } else { - eprintln!( - "Note: Prover.toml already exists. Use --overwrite to force overwrite." - ); + eprintln!("Note: Prover.toml already exists. Use --overwrite to force overwrite."); } if should_write_verifier {