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

feat: --pedantic-solving flag #6716

Merged
merged 51 commits into from
Jan 7, 2025
Merged
Show file tree
Hide file tree
Changes from 44 commits
Commits
Show all changes
51 commits
Select commit Hold shift + click to select a range
b5ee668
test input sizes and predicate values with --pedantic_solving flag, a…
michaeljklein Dec 4, 2024
f65a0b0
add pedantic_solving parameter wherever needed, default to true when …
michaeljklein Dec 5, 2024
d3e7bc5
Update acvm-repo/blackbox_solver/src/bigint.rs
michaeljklein Dec 6, 2024
d0b877f
Merge branch 'master' into michaeljklein/pedantic-flag
michaeljklein Dec 6, 2024
b83d6b5
remove default from structs that require the pedantic_solving paramet…
michaeljklein Dec 6, 2024
9f1d8c1
Merge branch 'master' into michaeljklein/pedantic-flag
michaeljklein Dec 6, 2024
46b75fb
Merge branch 'master' into michaeljklein/pedantic-flag
michaeljklein Dec 6, 2024
1e3e3cb
Merge branch 'master' into michaeljklein/pedantic-flag
michaeljklein Dec 9, 2024
0833b11
Merge branch 'master' into michaeljklein/pedantic-flag
michaeljklein Dec 9, 2024
b314b21
wip moving 'pedantic_solving'
michaeljklein Dec 9, 2024
4b0df94
wip debugging: use StubbedBlackBoxSolver::default to enable pedantic_…
michaeljklein Dec 10, 2024
5fe6eaa
wip debugging, use 'point == ..Affine::zero()' to check for infinity,…
michaeljklein Dec 11, 2024
85d0e50
Merge branch 'master' into michaeljklein/pedantic-flag
michaeljklein Dec 11, 2024
a895f64
fix typo, update smoke test to add two different points, cargo clippy…
michaeljklein Dec 11, 2024
3b42dd9
Update acvm-repo/blackbox_solver/src/bigint.rs
michaeljklein Dec 11, 2024
f7ee3b8
Update acvm-repo/acvm/src/pwg/mod.rs
michaeljklein Dec 11, 2024
7c7766a
Merge branch 'master' into michaeljklein/pedantic-flag
michaeljklein Dec 11, 2024
8796f15
fix small refactor and use single constant for pedantic_solving in me…
michaeljklein Dec 11, 2024
3e89c31
updating lsp usage of solvers, cargo clippy/fmt
michaeljklein Dec 11, 2024
1ad3ba1
Merge branch 'master' into michaeljklein/pedantic-flag
michaeljklein Dec 11, 2024
7b5d66e
update test error message
michaeljklein Dec 11, 2024
588e4a0
Merge branch 'master' into michaeljklein/pedantic-flag
TomAFrench Dec 12, 2024
813b8a5
Merge branch 'master' into michaeljklein/pedantic-flag
michaeljklein Dec 12, 2024
3bbd894
Merge branch 'master' into michaeljklein/pedantic-flag
michaeljklein Dec 12, 2024
fd6085b
cargo fmt
michaeljklein Dec 12, 2024
d22a1bd
Merge branch 'master' into michaeljklein/pedantic-flag
michaeljklein Dec 14, 2024
4ed1e24
Merge branch 'master' into michaeljklein/pedantic-flag
michaeljklein Dec 16, 2024
a1f824b
Update acvm-repo/bn254_blackbox_solver/src/embedded_curve_ops.rs
michaeljklein Dec 16, 2024
8ea09ba
use '--pedantic-solving' in CI and example programs
michaeljklein Dec 16, 2024
b67ab01
Merge branch 'master' into michaeljklein/pedantic-flag
michaeljklein Dec 16, 2024
e8e27db
Merge branch 'master' into michaeljklein/pedantic-flag
michaeljklein Dec 16, 2024
f5e00e0
Merge branch 'master' into michaeljklein/pedantic-flag
michaeljklein Dec 16, 2024
6146477
revert nargo->cargo testing change
michaeljklein Dec 17, 2024
6b919bb
Merge branch 'master' into michaeljklein/pedantic-flag
michaeljklein Dec 17, 2024
0556d1c
Update examples/prove_and_verify/prove_and_verify.sh
TomAFrench Dec 17, 2024
b9af2d1
Merge branch 'master' into michaeljklein/pedantic-flag
TomAFrench Dec 19, 2024
c4d8c87
Merge branch 'master' into michaeljklein/pedantic-flag
michaeljklein Dec 19, 2024
c30af47
Merge branch 'master' into michaeljklein/pedantic-flag
michaeljklein Dec 19, 2024
c12ff56
Merge branch 'master' into michaeljklein/pedantic-flag
michaeljklein Dec 20, 2024
91c9440
Merge branch 'master' into michaeljklein/pedantic-flag
michaeljklein Dec 21, 2024
a447530
Merge branch 'master' into michaeljklein/pedantic-flag
michaeljklein Jan 3, 2025
905f4f2
fixup after merge
michaeljklein Jan 3, 2025
fed6e7c
Merge branch 'master' into michaeljklein/pedantic-flag
michaeljklein Jan 3, 2025
6186750
Merge branch 'master' into michaeljklein/pedantic-flag
michaeljklein Jan 5, 2025
55de7d6
Merge branch 'master' into michaeljklein/pedantic-flag
michaeljklein Jan 6, 2025
b219538
Merge branch 'master' into michaeljklein/pedantic-flag
michaeljklein Jan 6, 2025
56362d4
allow clippy::too_many_arguments when constructing Elaborator
michaeljklein Jan 6, 2025
a7820ad
Merge branch 'master' into michaeljklein/pedantic-flag
michaeljklein Jan 7, 2025
5adbaae
cargo fmt
michaeljklein Jan 7, 2025
75f912e
fix after merge
michaeljklein Jan 7, 2025
b013d23
Merge branch 'master' into michaeljklein/pedantic-flag
michaeljklein Jan 7, 2025
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 .github/workflows/test-js-packages.yml
Original file line number Diff line number Diff line change
Expand Up @@ -597,7 +597,7 @@ jobs:
- name: Run nargo test
working-directory: ./test-repo/${{ matrix.project.path }}
run: |
out=$(nargo test --silence-warnings --skip-brillig-constraints-check --format json ${{ matrix.project.nargo_args }}) && echo "$out" > ${{ github.workspace }}/noir-repo/.github/critical_libraries_status/${{ matrix.project.repo }}/${{ matrix.project.path }}.actual.jsonl
out=$(nargo test --silence-warnings --pedantic-solving --skip-brillig-constraints-check --format json ${{ matrix.project.nargo_args }}) && echo "$out" > ${{ github.workspace }}/noir-repo/.github/critical_libraries_status/${{ matrix.project.repo }}/${{ matrix.project.path }}.actual.jsonl
env:
NARGO_IGNORE_TEST_FAILURES_FROM_FOREIGN_CALLS: true

Expand Down
46 changes: 46 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 8 additions & 1 deletion acvm-repo/acvm/src/pwg/blackbox/bigint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,16 @@ use crate::pwg::OpcodeResolutionError;
/// - When it encounters a bigint operation opcode, it performs the operation on the stored values
/// and store the result using the provided ID.
/// - When it gets a to_bytes opcode, it simply looks up the value and resolves the output witness accordingly.
#[derive(Default)]
pub(crate) struct AcvmBigIntSolver {
bigint_solver: BigIntSolver,
}

impl AcvmBigIntSolver {
pub(crate) fn with_pedantic_solving(pedantic_solving: bool) -> AcvmBigIntSolver {
let bigint_solver = BigIntSolver::with_pedantic_solving(pedantic_solving);
AcvmBigIntSolver { bigint_solver }
}

pub(crate) fn bigint_from_bytes<F: AcirField>(
&mut self,
inputs: &[FunctionInput<F>],
Expand All @@ -39,6 +43,9 @@ impl AcvmBigIntSolver {
outputs: &[Witness],
initial_witness: &mut WitnessMap<F>,
) -> Result<(), OpcodeResolutionError<F>> {
if self.bigint_solver.pedantic_solving() && outputs.len() != 32 {
panic!("--pedantic-solving: bigint_to_bytes: outputs.len() != 32: {}", outputs.len());
}
let mut bytes = self.bigint_solver.bigint_to_bytes(input)?;
while bytes.len() < outputs.len() {
bytes.push(0);
Expand Down
12 changes: 8 additions & 4 deletions acvm-repo/acvm/src/pwg/blackbox/logic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,14 @@ pub(super) fn and<F: AcirField>(
lhs: &FunctionInput<F>,
rhs: &FunctionInput<F>,
output: &Witness,
pedantic_solving: bool,
) -> Result<(), OpcodeResolutionError<F>> {
assert_eq!(
lhs.num_bits(),
rhs.num_bits(),
"number of bits specified for each input must be the same"
);
solve_logic_opcode(initial_witness, lhs, rhs, *output, |left, right| {
solve_logic_opcode(initial_witness, lhs, rhs, *output, pedantic_solving, |left, right| {
bit_and(left, right, lhs.num_bits())
})
}
Expand All @@ -32,13 +33,14 @@ pub(super) fn xor<F: AcirField>(
lhs: &FunctionInput<F>,
rhs: &FunctionInput<F>,
output: &Witness,
pedantic_solving: bool,
) -> Result<(), OpcodeResolutionError<F>> {
assert_eq!(
lhs.num_bits(),
rhs.num_bits(),
"number of bits specified for each input must be the same"
);
solve_logic_opcode(initial_witness, lhs, rhs, *output, |left, right| {
solve_logic_opcode(initial_witness, lhs, rhs, *output, pedantic_solving, |left, right| {
bit_xor(left, right, lhs.num_bits())
})
}
Expand All @@ -49,11 +51,13 @@ fn solve_logic_opcode<F: AcirField>(
a: &FunctionInput<F>,
b: &FunctionInput<F>,
result: Witness,
pedantic_solving: bool,
logic_op: impl Fn(F, F) -> F,
) -> Result<(), OpcodeResolutionError<F>> {
// TODO(https://github.com/noir-lang/noir/issues/5985): re-enable these once we figure out how to combine these with existing
// TODO(https://github.com/noir-lang/noir/issues/5985): re-enable these by
// default once we figure out how to combine these with existing
// noirc_frontend/noirc_evaluator overflow error messages
let skip_bitsize_checks = true;
let skip_bitsize_checks = !pedantic_solving;
TomAFrench marked this conversation as resolved.
Show resolved Hide resolved
let w_l_value = input_to_value(initial_witness, *a, skip_bitsize_checks)?;
let w_r_value = input_to_value(initial_witness, *b, skip_bitsize_checks)?;
let assignment = logic_op(w_l_value, w_r_value);
Expand Down
12 changes: 9 additions & 3 deletions acvm-repo/acvm/src/pwg/blackbox/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
use hash::{solve_generic_256_hash_opcode, solve_sha_256_permutation_opcode};
use logic::{and, xor};
pub(crate) use range::solve_range_opcode;
use signature::ecdsa::{secp256k1_prehashed, secp256r1_prehashed};

Check warning on line 30 in acvm-repo/acvm/src/pwg/blackbox/mod.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (prehashed)

Check warning on line 30 in acvm-repo/acvm/src/pwg/blackbox/mod.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (prehashed)

/// Check if all of the inputs to the function have assignments
///
Expand Down Expand Up @@ -76,9 +76,15 @@
BlackBoxFuncCall::AES128Encrypt { inputs, iv, key, outputs } => {
solve_aes128_encryption_opcode(initial_witness, inputs, iv, key, outputs)
}
BlackBoxFuncCall::AND { lhs, rhs, output } => and(initial_witness, lhs, rhs, output),
BlackBoxFuncCall::XOR { lhs, rhs, output } => xor(initial_witness, lhs, rhs, output),
BlackBoxFuncCall::RANGE { input } => solve_range_opcode(initial_witness, input),
BlackBoxFuncCall::AND { lhs, rhs, output } => {
and(initial_witness, lhs, rhs, output, backend.pedantic_solving())
}
BlackBoxFuncCall::XOR { lhs, rhs, output } => {
xor(initial_witness, lhs, rhs, output, backend.pedantic_solving())
}
BlackBoxFuncCall::RANGE { input } => {
solve_range_opcode(initial_witness, input, backend.pedantic_solving())
}
BlackBoxFuncCall::Blake2s { inputs, outputs } => {
solve_generic_256_hash_opcode(initial_witness, inputs, None, outputs, blake2s)
}
Expand Down Expand Up @@ -106,7 +112,7 @@
signature,
hashed_message: message,
output,
} => secp256k1_prehashed(

Check warning on line 115 in acvm-repo/acvm/src/pwg/blackbox/mod.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (prehashed)
initial_witness,
public_key_x,
public_key_y,
Expand All @@ -120,7 +126,7 @@
signature,
hashed_message: message,
output,
} => secp256r1_prehashed(

Check warning on line 129 in acvm-repo/acvm/src/pwg/blackbox/mod.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (prehashed)
initial_witness,
public_key_x,
public_key_y,
Expand Down
5 changes: 3 additions & 2 deletions acvm-repo/acvm/src/pwg/blackbox/range.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,11 @@ use acir::{circuit::opcodes::FunctionInput, native_types::WitnessMap, AcirField}
pub(crate) fn solve_range_opcode<F: AcirField>(
initial_witness: &WitnessMap<F>,
input: &FunctionInput<F>,
pedantic_solving: bool,
) -> Result<(), OpcodeResolutionError<F>> {
// TODO(https://github.com/noir-lang/noir/issues/5985):
// re-enable bitsize checks
let skip_bitsize_checks = true;
// re-enable bitsize checks by default
let skip_bitsize_checks = !pedantic_solving;
let w_value = input_to_value(initial_witness, *input, skip_bitsize_checks)?;
if w_value.num_bits() > input.num_bits() {
return Err(OpcodeResolutionError::UnsatisfiedConstrain {
Expand Down
30 changes: 25 additions & 5 deletions acvm-repo/acvm/src/pwg/memory_op.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ impl<F: AcirField> MemoryOpSolver<F> {
op: &MemOp<F>,
initial_witness: &mut WitnessMap<F>,
predicate: &Option<Expression<F>>,
pedantic_solving: bool,
) -> Result<(), OpcodeResolutionError<F>> {
let operation = get_value(&op.operation, initial_witness)?;

Expand All @@ -83,7 +84,9 @@ impl<F: AcirField> MemoryOpSolver<F> {
let is_read_operation = operation.is_zero();

// Fetch whether or not the predicate is false (e.g. equal to zero)
let skip_operation = is_predicate_false(initial_witness, predicate)?;
let opcode_location = ErrorLocation::Unresolved;
let skip_operation =
is_predicate_false(initial_witness, predicate, pedantic_solving, &opcode_location)?;

if is_read_operation {
// `value_read = arr[memory_index]`
Expand Down Expand Up @@ -131,6 +134,9 @@ mod tests {

use super::MemoryOpSolver;

// use pedantic_solving for tests
const PEDANTIC_SOLVING: bool = true;

#[test]
fn test_solver() {
let mut initial_witness = WitnessMap::from(BTreeMap::from_iter([
Expand All @@ -150,7 +156,9 @@ mod tests {
block_solver.init(&init, &initial_witness).unwrap();

for op in trace {
block_solver.solve_memory_op(&op, &mut initial_witness, &None).unwrap();
block_solver
.solve_memory_op(&op, &mut initial_witness, &None, PEDANTIC_SOLVING)
.unwrap();
}

assert_eq!(initial_witness[&Witness(4)], FieldElement::from(2u128));
Expand All @@ -175,7 +183,9 @@ mod tests {
let mut err = None;
for op in invalid_trace {
if err.is_none() {
err = block_solver.solve_memory_op(&op, &mut initial_witness, &None).err();
err = block_solver
.solve_memory_op(&op, &mut initial_witness, &None, PEDANTIC_SOLVING)
.err();
}
}

Expand Down Expand Up @@ -209,7 +219,12 @@ mod tests {
for op in invalid_trace {
if err.is_none() {
err = block_solver
.solve_memory_op(&op, &mut initial_witness, &Some(Expression::zero()))
.solve_memory_op(
&op,
&mut initial_witness,
&Some(Expression::zero()),
PEDANTIC_SOLVING,
)
.err();
}
}
Expand Down Expand Up @@ -241,7 +256,12 @@ mod tests {
for op in invalid_trace {
if err.is_none() {
err = block_solver
.solve_memory_op(&op, &mut initial_witness, &Some(Expression::zero()))
.solve_memory_op(
&op,
&mut initial_witness,
&Some(Expression::zero()),
PEDANTIC_SOLVING,
)
.err();
}
}
Expand Down
Loading
Loading