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

fix: Reject invalid expression with in CLI parser #6287

Merged
merged 3 commits into from
Oct 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion acvm-repo/acvm/src/compiler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ mod transformers;

pub use optimizers::optimize;
use optimizers::optimize_internal;
pub use transformers::transform;
use transformers::transform_internal;
pub use transformers::{transform, MIN_EXPRESSION_WIDTH};

/// This module moves and decomposes acir opcodes. The transformation map allows consumers of this module to map
/// metadata they had about the opcodes to the new opcode structure generated after the transformation.
Expand Down
9 changes: 7 additions & 2 deletions acvm-repo/acvm/src/compiler/transformers/csat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@
};
use indexmap::IndexMap;

/// Minimum width accepted by the `CSatTransformer`.
pub const MIN_EXPRESSION_WIDTH: usize = 3;

/// A transformer which processes any [`Expression`]s to break them up such that they
/// fit within the [`ProofSystemCompiler`][crate::ProofSystemCompiler]'s width.
///
Expand All @@ -22,9 +25,11 @@
}

impl CSatTransformer {
// Configure the width for the optimizer
/// Create an optimizer with a given width.
///
/// Panics if `width` is less than `MIN_EXPRESSION_WIDTH`.
pub(crate) fn new(width: usize) -> CSatTransformer {
assert!(width > 2);
assert!(width >= MIN_EXPRESSION_WIDTH, "width has to be at least {MIN_EXPRESSION_WIDTH}");

CSatTransformer { width, solvable_witness: HashSet::new() }
}
Expand Down Expand Up @@ -87,7 +92,7 @@
// This optimization will search for combinations of terms which can be represented in a single assert-zero opcode
// Case 1 : qM * wL * wR + qL * wL + qR * wR + qO * wO + qC
// This polynomial does not require any further optimizations, it can be safely represented in one opcode
// ie a polynomial with 1 mul(bi-variate) term and 3 (univariate) terms where 2 of those terms match the bivariate term

Check warning on line 95 in acvm-repo/acvm/src/compiler/transformers/csat.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (bivariate)
// wL and wR, we can represent it in one opcode
// GENERALIZED for WIDTH: instead of the number 3, we use `WIDTH`
//
Expand Down
1 change: 1 addition & 0 deletions acvm-repo/acvm/src/compiler/transformers/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use indexmap::IndexMap;
mod csat;

pub(crate) use csat::CSatTransformer;
pub use csat::MIN_EXPRESSION_WIDTH;

use super::{transform_assert_messages, AcirTransformationMap};

Expand Down
7 changes: 6 additions & 1 deletion compiler/noirc_driver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

use abi_gen::{abi_type_from_hir_type, value_from_hir_expression};
use acvm::acir::circuit::ExpressionWidth;
use acvm::compiler::MIN_EXPRESSION_WIDTH;
use clap::Args;
use fm::{FileId, FileManager};
use iter_extended::vecmap;
Expand Down Expand Up @@ -134,7 +135,11 @@ pub fn parse_expression_width(input: &str) -> Result<ExpressionWidth, std::io::E

match width {
0 => Ok(ExpressionWidth::Unbounded),
_ => Ok(ExpressionWidth::Bounded { width }),
w if w >= MIN_EXPRESSION_WIDTH => Ok(ExpressionWidth::Bounded { width }),
_ => Err(Error::new(
ErrorKind::InvalidInput,
format!("has to be 0 or at least {MIN_EXPRESSION_WIDTH}"),
)),
}
}

Expand Down
16 changes: 16 additions & 0 deletions tooling/nargo_cli/src/cli/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,3 +112,19 @@ pub(crate) fn start_cli() -> eyre::Result<()> {
println!("{markdown}");
Ok(())
}

#[cfg(test)]
mod tests {
use clap::Parser;
#[test]
fn test_parse_invalid_expression_width() {
let cmd = "nargo --program-dir . compile --expression-width 1";
let res = super::NargoCli::try_parse_from(cmd.split_ascii_whitespace());

let err = res.expect_err("should fail because of invalid width");
assert!(err.to_string().contains("expression-width"));
assert!(err
.to_string()
.contains(acvm::compiler::MIN_EXPRESSION_WIDTH.to_string().as_str()));
}
}
Loading