Skip to content

Commit

Permalink
feat: Add ability to create a proof for a workspace member using `nar…
Browse files Browse the repository at this point in the history
…go prove -p {crate_name}` (#1930)

* add package name

* reworking

* fix fails

* Update crates/nargo_cli/src/cli/prove_cmd.rs

---------

Co-authored-by: kevaundray <kevtheappdev@gmail.com>
  • Loading branch information
kek kek kek and kevaundray authored Jul 17, 2023
1 parent ea47936 commit 266126f
Show file tree
Hide file tree
Showing 17 changed files with 130 additions and 41 deletions.
1 change: 1 addition & 0 deletions crates/nargo/src/manifest/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ pub struct WorkspaceConfig {
#[allow(dead_code)]
#[derive(Default, Debug, Deserialize, Clone)]
pub struct PackageMetadata {
pub name: Option<String>,
// Note: a package name is not needed unless there is a registry
authors: Vec<String>,
// If not compiler version is supplied, the latest is used
Expand Down
2 changes: 1 addition & 1 deletion crates/nargo_cli/src/cli/check_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ fn check_from_path<B: Backend>(
program_dir: &Path,
compile_options: &CompileOptions,
) -> Result<(), CliError<B>> {
let mut context = resolve_root_manifest(program_dir)?;
let mut context = resolve_root_manifest(program_dir, None)?;
check_crate_and_report_errors(
&mut context,
compile_options.deny_warnings,
Expand Down
2 changes: 1 addition & 1 deletion crates/nargo_cli/src/cli/codegen_verifier_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ pub(crate) fn run<B: Backend>(
}
None => {
let (program, _) =
compile_circuit(backend, config.program_dir.as_ref(), &args.compile_options)?;
compile_circuit(backend, None, config.program_dir.as_ref(), &args.compile_options)?;
let common_reference_string =
update_common_reference_string(backend, &common_reference_string, &program.circuit)
.map_err(CliError::CommonReferenceStringError)?;
Expand Down
8 changes: 5 additions & 3 deletions crates/nargo_cli/src/cli/compile_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ pub(crate) fn run<B: Backend>(

// If contracts is set we're compiling every function in a 'contract' rather than just 'main'.
if args.contracts {
let mut context = resolve_root_manifest(&config.program_dir)?;
let mut context = resolve_root_manifest(&config.program_dir, None)?;

let result = compile_contracts(&mut context, &args.compile_options);
let contracts = report_errors(result, &context, args.compile_options.deny_warnings)?;
Expand Down Expand Up @@ -101,7 +101,8 @@ pub(crate) fn run<B: Backend>(
);
}
} else {
let (program, _) = compile_circuit(backend, &config.program_dir, &args.compile_options)?;
let (program, _) =
compile_circuit(backend, None, &config.program_dir, &args.compile_options)?;
common_reference_string =
update_common_reference_string(backend, &common_reference_string, &program.circuit)
.map_err(CliError::CommonReferenceStringError)?;
Expand All @@ -119,10 +120,11 @@ pub(crate) fn run<B: Backend>(

pub(crate) fn compile_circuit<B: Backend>(
backend: &B,
package: Option<String>,
program_dir: &Path,
compile_options: &CompileOptions,
) -> Result<(CompiledProgram, Context), CliError<B>> {
let mut context = resolve_root_manifest(program_dir)?;
let mut context = resolve_root_manifest(program_dir, package)?;
let result = compile_main(&mut context, compile_options);
let mut program = report_errors(result, &context, compile_options.deny_warnings)?;

Expand Down
2 changes: 1 addition & 1 deletion crates/nargo_cli/src/cli/execute_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ fn execute_with_path<B: Backend>(
prover_name: String,
compile_options: &CompileOptions,
) -> Result<(Option<InputValue>, WitnessMap), CliError<B>> {
let (compiled_program, context) = compile_circuit(backend, program_dir, compile_options)?;
let (compiled_program, context) = compile_circuit(backend, None, program_dir, compile_options)?;
let CompiledProgram { abi, circuit, debug } = compiled_program;

// Parse the initial witness values from Prover.toml
Expand Down
3 changes: 2 additions & 1 deletion crates/nargo_cli/src/cli/gates_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ fn count_gates_with_path<B: Backend, P: AsRef<Path>>(
program_dir: P,
compile_options: &CompileOptions,
) -> Result<(), CliError<B>> {
let (compiled_program, _) = compile_circuit(backend, program_dir.as_ref(), compile_options)?;
let (compiled_program, _) =
compile_circuit(backend, None, program_dir.as_ref(), compile_options)?;
let num_opcodes = compiled_program.circuit.opcodes.len();

println!(
Expand Down
7 changes: 6 additions & 1 deletion crates/nargo_cli/src/cli/prove_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@ pub(crate) struct ProveCommand {

#[clap(flatten)]
compile_options: CompileOptions,

#[clap(long)]
package: Option<String>,
}

pub(crate) fn run<B: Backend>(
Expand All @@ -67,6 +70,7 @@ pub(crate) fn run<B: Backend>(
args.proof_name,
args.prover_name,
args.verifier_name,
args.package,
config.program_dir,
proof_dir,
circuit_build_path,
Expand All @@ -83,6 +87,7 @@ pub(crate) fn prove_with_path<B: Backend, P: AsRef<Path>>(
proof_name: Option<String>,
prover_name: String,
verifier_name: String,
package: Option<String>,
program_dir: P,
proof_dir: P,
circuit_build_path: Option<PathBuf>,
Expand All @@ -104,7 +109,7 @@ pub(crate) fn prove_with_path<B: Backend, P: AsRef<Path>>(
}
None => {
let (program, context) =
compile_circuit(backend, program_dir.as_ref(), compile_options)?;
compile_circuit(backend, package, program_dir.as_ref(), compile_options)?;
let common_reference_string =
update_common_reference_string(backend, &common_reference_string, &program.circuit)
.map_err(CliError::CommonReferenceStringError)?;
Expand Down
2 changes: 1 addition & 1 deletion crates/nargo_cli/src/cli/test_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ fn run_tests<B: Backend>(
test_name: &str,
compile_options: &CompileOptions,
) -> Result<(), CliError<B>> {
let mut context = resolve_root_manifest(program_dir)?;
let mut context = resolve_root_manifest(program_dir, None)?;
check_crate_and_report_errors(
&mut context,
compile_options.deny_warnings,
Expand Down
3 changes: 2 additions & 1 deletion crates/nargo_cli/src/cli/verify_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,8 @@ fn verify_with_path<B: Backend, P: AsRef<Path>>(
(common_reference_string, program)
}
None => {
let (program, _) = compile_circuit(backend, program_dir.as_ref(), compile_options)?;
let (program, _) =
compile_circuit(backend, None, program_dir.as_ref(), compile_options)?;
let common_reference_string =
update_common_reference_string(backend, &common_reference_string, &program.circuit)
.map_err(CliError::CommonReferenceStringError)?;
Expand Down
133 changes: 103 additions & 30 deletions crates/nargo_cli/src/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@ use std::{
path::{Path, PathBuf},
};

use nargo::manifest::{Dependency, Manifest, PackageManifest};
use nargo::manifest::{Dependency, Manifest, PackageManifest, WorkspaceConfig};
use noirc_driver::{add_dep, create_local_crate, create_non_local_crate};
use noirc_frontend::{
graph::{CrateId, CrateType},
graph::{CrateId, CrateName, CrateType},
hir::Context,
};
use thiserror::Error;
Expand Down Expand Up @@ -49,6 +49,20 @@ pub(crate) enum DependencyResolutionError {
/// Use workspace as a dependency is not currently supported
#[error("use workspace as a dependency is not currently supported")]
WorkspaceDependency,

/// Multiple workspace roots found in the same workspace
#[error("multiple workspace roots found in the same workspace:\n{}\n{}", root.display(), member.display())]
MultipleWorkspace { root: PathBuf, member: PathBuf },

/// Invalid character `-` in package name
#[error("invalid character `-` in package name")]
InvalidPackageName,

#[error("package specification `{0}` did not match any packages")]
PackageNotFound(String),

#[error("two packages named `{0}` in this workspace")]
PackageCollision(String),
}

#[derive(Debug, Clone)]
Expand All @@ -72,44 +86,30 @@ struct CachedDep {
/// XXX: Need to handle when a local package changes!
pub(crate) fn resolve_root_manifest(
dir_path: &std::path::Path,
package: Option<String>,
) -> Result<Context, DependencyResolutionError> {
let mut context = Context::default();

let manifest_path = super::find_package_manifest(dir_path)?;
let manifest = super::manifest::parse(&manifest_path)?;

match manifest {
Manifest::Package(package) => {
Manifest::Package(inner) => {
let (entry_path, crate_type) = super::lib_or_bin(dir_path)?;
let crate_id = create_local_crate(&mut context, entry_path, crate_type);

let crate_id = create_local_crate(&mut context, entry_path, crate_type);
let pkg_root = manifest_path.parent().expect("Every manifest path has a parent.");
resolve_manifest(&mut context, crate_id, package, pkg_root)?;

resolve_package_manifest(&mut context, crate_id, inner, pkg_root)?;
}
Manifest::Workspace(workspace) => {
let config = workspace.config;
let members = config.members;

let maybe_local = config
.default_member
.or_else(|| members.last().cloned())
.map(|member| dir_path.join(member));

let default_member = match maybe_local {
Some(member) => member,
None => {
return Err(DependencyResolutionError::EmptyWorkspace { path: manifest_path })
}
};

let (entry_path, _crate_type) = super::lib_or_bin(default_member)?;
let _local = create_local_crate(&mut context, entry_path, CrateType::Workspace);

for member in members {
let path: PathBuf = dir_path.join(member);
let (entry_path, crate_type) = super::lib_or_bin(path)?;
create_non_local_crate(&mut context, entry_path, crate_type);
}
resolve_workspace_manifest(
&mut context,
package,
manifest_path,
dir_path,
workspace.config,
)?;
}
};

Expand All @@ -122,7 +122,7 @@ pub(crate) fn resolve_root_manifest(
// We do not need to add stdlib, as it's implicitly
// imported. However, it may be helpful to have the stdlib imported by the
// package manager.
fn resolve_manifest(
fn resolve_package_manifest(
context: &mut Context,
parent_crate: CrateId,
manifest: PackageManifest,
Expand Down Expand Up @@ -154,11 +154,84 @@ fn resolve_manifest(
return Err(DependencyResolutionError::RemoteDepWithLocalDep { dependency_path });
}
// TODO: Why did it create a new resolver?
resolve_manifest(context, crate_id, dep_meta.manifest, &dependency_path)?;
resolve_package_manifest(context, crate_id, dep_meta.manifest, &dependency_path)?;
}
Ok(())
}

fn crate_name(name: Option<CrateName>) -> String {
name.map(|name| name.as_string()).unwrap_or_else(|| "[unnamed]".to_string())
}

fn resolve_workspace_manifest(
context: &mut Context,
mut local_package: Option<String>,
manifest_path: PathBuf,
dir_path: &Path,
workspace: WorkspaceConfig,
) -> Result<(), DependencyResolutionError> {
let members = workspace.members;
let mut packages = HashMap::new();

if members.is_empty() {
return Err(DependencyResolutionError::EmptyWorkspace { path: manifest_path });
}

for member in &members {
let member_path: PathBuf = dir_path.join(member);
let member_member_path = super::find_package_manifest(&member_path)?;
let member_manifest = super::manifest::parse(&member_member_path)?;

match member_manifest {
Manifest::Package(inner) => {
let name = inner
.package
.name
.map(|name| {
CrateName::new(&name)
.map_err(|_name| DependencyResolutionError::InvalidPackageName)
})
.transpose()?;

if packages.insert(name.clone(), member_path).is_some() {
return Err(DependencyResolutionError::PackageCollision(crate_name(name)));
}

if local_package.is_none() && workspace.default_member.as_ref() == Some(member) {
local_package = name.as_ref().map(CrateName::as_string);
}
}
Manifest::Workspace(_) => {
return Err(DependencyResolutionError::MultipleWorkspace {
root: manifest_path,
member: member_member_path,
})
}
}
}

let local_package = match local_package {
Some(local_package) => CrateName::new(&local_package)
.map_err(|_| DependencyResolutionError::InvalidPackageName)?
.into(),
None => packages.keys().last().expect("non-empty packages").clone(),
};

let local_crate = packages
.remove(&local_package)
.ok_or_else(|| DependencyResolutionError::PackageNotFound(crate_name(local_package)))?;

let (entry_path, _crate_type) = super::lib_or_bin(local_crate)?;
create_local_crate(context, entry_path, CrateType::Workspace);

for (_, package_path) in packages.drain() {
let (entry_path, crate_type) = super::lib_or_bin(package_path)?;
create_non_local_crate(context, entry_path, crate_type);
}

Ok(())
}

/// If the dependency is remote, download the dependency
/// and return the directory path along with the metadata
/// Needed to fill the CachedDep struct
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
[package]
name = "a"
authors = [""]
compiler_version = "0.8.0"

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
[package]
name = "b"
authors = [""]
compiler_version = "0.8.0"

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
[package]
name = "a"
authors = [""]
compiler_version = "0.8.0"

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
[package]
name = "a"
authors = [""]
compiler_version = "0.8.0"

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
[package]
name = "b"
authors = [""]
compiler_version = "0.8.0"

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
[package]
name = "a"
authors = [""]
compiler_version = "0.8.0"

Expand Down
2 changes: 1 addition & 1 deletion crates/noirc_frontend/src/graph/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ impl CrateId {
}
}

#[derive(Debug, Clone, PartialEq, Eq)]
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
pub struct CrateName(SmolStr);

impl CrateName {
Expand Down

0 comments on commit 266126f

Please sign in to comment.