Skip to content

Commit f87034a

Browse files
author
AztecBot
committed
feat: impl Default for U128 (noir-lang/noir#6984)
fix: Do not emit range check for multiplication by bool (noir-lang/noir#6983) fix: do not panic on indices which are not valid `u32`s (noir-lang/noir#6976) feat!: require trait method calls (`foo.bar()`) to have the trait in scope (imported) (noir-lang/noir#6895) feat!: type-check trait default methods (noir-lang/noir#6645) feat: `--pedantic-solving` flag (noir-lang/noir#6716) feat!: update `aes128_encrypt` to return an array (noir-lang/noir#6973) fix: wrong module to lookup trait when using crate or super (noir-lang/noir#6974) fix: Start RC at 1 again (noir-lang/noir#6958) feat!: turn TypeIsMorePrivateThenItem into an error (noir-lang/noir#6953) fix: don't fail parsing macro if there are parser warnings (noir-lang/noir#6969) fix: error on missing function parameters (noir-lang/noir#6967) feat: don't report warnings for dependencies (noir-lang/noir#6926) chore: simplify boolean in a mul of a mul (noir-lang/noir#6951) feat(ssa): Immediately simplify away RefCount instructions in ACIR functions (noir-lang/noir#6893) chore: Move comment as part of #6945 (noir-lang/noir#6959) chore: Separate unconstrained functions during monomorphization (noir-lang/noir#6894) feat!: turn CannotReexportItemWithLessVisibility into an error (noir-lang/noir#6952) feat: lock on Nargo.toml on several nargo commands (noir-lang/noir#6941) feat: don't simplify SSA instructions when creating them from a string (noir-lang/noir#6948) chore: add reproduction case for bignum test failure (noir-lang/noir#6464) chore: bump `noir-gates-diff` (noir-lang/noir#6949) feat(test): Enable the test fuzzer for Wasm (noir-lang/noir#6835) chore: also print test output to stdout in CI (noir-lang/noir#6930) fix: Non-determinism from under constrained checks (noir-lang/noir#6945) chore: use logs for benchmarking (noir-lang/noir#6911) chore: bump `noir-gates-diff` (noir-lang/noir#6944) chore: bump `noir-gates-diff` (noir-lang/noir#6943) fix: Show output of `test_program_is_idempotent` on failure (noir-lang/noir#6942) chore: delete a bunch of dead code from `noirc_evaluator` (noir-lang/noir#6939) feat: require trait function calls (`Foo::bar()`) to have the trait in scope (imported) (noir-lang/noir#6882) chore: Bump arkworks to version `0.5.0` (noir-lang/noir#6871)
2 parents 91ad65e + 365d50b commit f87034a

File tree

17 files changed

+602
-243
lines changed

17 files changed

+602
-243
lines changed

.noir-sync-commit

+1-1
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
8bb3908281d531160db7d7898c67fb2647792e6e
1+
3c488f4b272f460383341c51270b87bfe2b94468

noir/noir-repo/.github/workflows/test-js-packages.yml

+1-1
Original file line numberDiff line numberDiff line change
@@ -597,7 +597,7 @@ jobs:
597597
- name: Run nargo test
598598
working-directory: ./test-repo/${{ matrix.project.path }}
599599
run: |
600-
nargo test --silence-warnings --pedantic-solving --skip-brillig-constraints-check --format json ${{ matrix.project.nargo_args }} | tee ${{ github.workspace }}/noir-repo/.github/critical_libraries_status/${{ matrix.project.repo }}/${{ matrix.project.path }}.actual.jsonl
600+
nargo test --silence-warnings --skip-brillig-constraints-check --format json ${{ matrix.project.nargo_args }} | tee ${{ github.workspace }}/noir-repo/.github/critical_libraries_status/${{ matrix.project.repo }}/${{ matrix.project.path }}.actual.jsonl
601601
env:
602602
NARGO_IGNORE_TEST_FAILURES_FROM_FOREIGN_CALLS: true
603603

noir/noir-repo/acvm-repo/acvm/src/pwg/memory_op.rs

+18-5
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,19 @@ pub(crate) struct MemoryOpSolver<F> {
2121
}
2222

2323
impl<F: AcirField> MemoryOpSolver<F> {
24+
fn index_from_field(&self, index: F) -> Result<MemoryIndex, OpcodeResolutionError<F>> {
25+
if index.num_bits() <= 32 {
26+
let memory_index = index.try_to_u64().unwrap() as MemoryIndex;
27+
Ok(memory_index)
28+
} else {
29+
Err(OpcodeResolutionError::IndexOutOfBounds {
30+
opcode_location: ErrorLocation::Unresolved,
31+
index,
32+
array_size: self.block_len,
33+
})
34+
}
35+
}
36+
2437
fn write_memory_index(
2538
&mut self,
2639
index: MemoryIndex,
@@ -29,7 +42,7 @@ impl<F: AcirField> MemoryOpSolver<F> {
2942
if index >= self.block_len {
3043
return Err(OpcodeResolutionError::IndexOutOfBounds {
3144
opcode_location: ErrorLocation::Unresolved,
32-
index,
45+
index: F::from(index as u128),
3346
array_size: self.block_len,
3447
});
3548
}
@@ -40,7 +53,7 @@ impl<F: AcirField> MemoryOpSolver<F> {
4053
fn read_memory_index(&self, index: MemoryIndex) -> Result<F, OpcodeResolutionError<F>> {
4154
self.block_value.get(&index).copied().ok_or(OpcodeResolutionError::IndexOutOfBounds {
4255
opcode_location: ErrorLocation::Unresolved,
43-
index,
56+
index: F::from(index as u128),
4457
array_size: self.block_len,
4558
})
4659
}
@@ -72,7 +85,7 @@ impl<F: AcirField> MemoryOpSolver<F> {
7285

7386
// Find the memory index associated with this memory operation.
7487
let index = get_value(&op.index, initial_witness)?;
75-
let memory_index = index.try_to_u64().unwrap() as MemoryIndex;
88+
let memory_index = self.index_from_field(index)?;
7689

7790
// Calculate the value associated with this memory operation.
7891
//
@@ -193,9 +206,9 @@ mod tests {
193206
err,
194207
Some(crate::pwg::OpcodeResolutionError::IndexOutOfBounds {
195208
opcode_location: _,
196-
index: 2,
209+
index,
197210
array_size: 2
198-
})
211+
}) if index == FieldElement::from(2u128)
199212
));
200213
}
201214

noir/noir-repo/acvm-repo/acvm/src/pwg/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ pub enum OpcodeResolutionError<F> {
126126
payload: Option<ResolvedAssertionPayload<F>>,
127127
},
128128
#[error("Index out of bounds, array has size {array_size:?}, but index was {index:?}")]
129-
IndexOutOfBounds { opcode_location: ErrorLocation, index: u32, array_size: u32 },
129+
IndexOutOfBounds { opcode_location: ErrorLocation, index: F, array_size: u32 },
130130
#[error("Cannot solve opcode: {invalid_input_bit_size}")]
131131
InvalidInputBitSize {
132132
opcode_location: ErrorLocation,

noir/noir-repo/compiler/noirc_evaluator/src/acir/mod.rs

+42-43
Original file line numberDiff line numberDiff line change
@@ -1984,14 +1984,7 @@ impl<'a> Context<'a> {
19841984

19851985
if let NumericType::Unsigned { bit_size } = &num_type {
19861986
// Check for integer overflow
1987-
self.check_unsigned_overflow(
1988-
result,
1989-
*bit_size,
1990-
binary.lhs,
1991-
binary.rhs,
1992-
dfg,
1993-
binary.operator,
1994-
)?;
1987+
self.check_unsigned_overflow(result, *bit_size, binary, dfg)?;
19951988
}
19961989

19971990
Ok(result)
@@ -2002,47 +1995,18 @@ impl<'a> Context<'a> {
20021995
&mut self,
20031996
result: AcirVar,
20041997
bit_size: u32,
2005-
lhs: ValueId,
2006-
rhs: ValueId,
1998+
binary: &Binary,
20071999
dfg: &DataFlowGraph,
2008-
op: BinaryOp,
20092000
) -> Result<(), RuntimeError> {
2010-
// We try to optimize away operations that are guaranteed not to overflow
2011-
let max_lhs_bits = dfg.get_value_max_num_bits(lhs);
2012-
let max_rhs_bits = dfg.get_value_max_num_bits(rhs);
2013-
2014-
let msg = match op {
2015-
BinaryOp::Add => {
2016-
if std::cmp::max(max_lhs_bits, max_rhs_bits) < bit_size {
2017-
// `lhs` and `rhs` have both been casted up from smaller types and so cannot overflow.
2018-
return Ok(());
2019-
}
2020-
"attempt to add with overflow".to_string()
2021-
}
2022-
BinaryOp::Sub => {
2023-
if dfg.is_constant(lhs) && max_lhs_bits > max_rhs_bits {
2024-
// `lhs` is a fixed constant and `rhs` is restricted such that `lhs - rhs > 0`
2025-
// Note strict inequality as `rhs > lhs` while `max_lhs_bits == max_rhs_bits` is possible.
2026-
return Ok(());
2027-
}
2028-
"attempt to subtract with overflow".to_string()
2029-
}
2030-
BinaryOp::Mul => {
2031-
if bit_size == 1 || max_lhs_bits + max_rhs_bits <= bit_size {
2032-
// Either performing boolean multiplication (which cannot overflow),
2033-
// or `lhs` and `rhs` have both been casted up from smaller types and so cannot overflow.
2034-
return Ok(());
2035-
}
2036-
"attempt to multiply with overflow".to_string()
2037-
}
2038-
_ => return Ok(()),
2001+
let Some(msg) = binary.check_unsigned_overflow_msg(dfg, bit_size) else {
2002+
return Ok(());
20392003
};
20402004

20412005
let with_pred = self.acir_context.mul_var(result, self.current_side_effects_enabled_var)?;
20422006
self.acir_context.range_constrain_var(
20432007
with_pred,
20442008
&NumericType::Unsigned { bit_size },
2045-
Some(msg),
2009+
Some(msg.to_string()),
20462010
)?;
20472011
Ok(())
20482012
}
@@ -2888,8 +2852,9 @@ mod test {
28882852
use acvm::{
28892853
acir::{
28902854
circuit::{
2891-
brillig::BrilligFunctionId, opcodes::AcirFunctionId, ExpressionWidth, Opcode,
2892-
OpcodeLocation,
2855+
brillig::BrilligFunctionId,
2856+
opcodes::{AcirFunctionId, BlackBoxFuncCall},
2857+
ExpressionWidth, Opcode, OpcodeLocation,
28932858
},
28942859
native_types::Witness,
28952860
},
@@ -2913,6 +2878,8 @@ mod test {
29132878
},
29142879
};
29152880

2881+
use super::Ssa;
2882+
29162883
fn build_basic_foo_with_return(
29172884
builder: &mut FunctionBuilder,
29182885
foo_id: FunctionId,
@@ -3659,4 +3626,36 @@ mod test {
36593626
"Should have {expected_num_normal_calls} BrilligCall opcodes to normal Brillig functions but got {num_normal_brillig_calls}"
36603627
);
36613628
}
3629+
3630+
#[test]
3631+
fn multiply_with_bool_should_not_emit_range_check() {
3632+
let src = "
3633+
acir(inline) fn main f0 {
3634+
b0(v0: bool, v1: u32):
3635+
enable_side_effects v0
3636+
v2 = cast v0 as u32
3637+
v3 = mul v2, v1
3638+
return v3
3639+
}
3640+
";
3641+
let ssa = Ssa::from_str(src).unwrap();
3642+
let brillig = ssa.to_brillig(false);
3643+
3644+
let (mut acir_functions, _brillig_functions, _, _) = ssa
3645+
.into_acir(&brillig, ExpressionWidth::default())
3646+
.expect("Should compile manually written SSA into ACIR");
3647+
3648+
assert_eq!(acir_functions.len(), 1);
3649+
3650+
let opcodes = acir_functions[0].take_opcodes();
3651+
3652+
for opcode in opcodes {
3653+
if let Opcode::BlackBoxFuncCall(BlackBoxFuncCall::RANGE { input }) = opcode {
3654+
assert!(
3655+
input.to_witness().0 <= 1,
3656+
"only input witnesses should have range checks: {opcode:?}"
3657+
);
3658+
}
3659+
}
3660+
}
36623661
}

noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs

+56-74
Original file line numberDiff line numberDiff line change
@@ -1478,88 +1478,70 @@ impl<'block> BrilligBlock<'block> {
14781478
is_signed: bool,
14791479
) {
14801480
let bit_size = left.bit_size;
1481-
let max_lhs_bits = dfg.get_value_max_num_bits(binary.lhs);
1482-
let max_rhs_bits = dfg.get_value_max_num_bits(binary.rhs);
14831481

1484-
if bit_size == FieldElement::max_num_bits() {
1482+
if bit_size == FieldElement::max_num_bits() || is_signed {
14851483
return;
14861484
}
14871485

1488-
match (binary_operation, is_signed) {
1489-
(BrilligBinaryOp::Add, false) => {
1490-
if std::cmp::max(max_lhs_bits, max_rhs_bits) < bit_size {
1491-
// `left` and `right` have both been casted up from smaller types and so cannot overflow.
1492-
return;
1493-
}
1494-
1495-
let condition =
1496-
SingleAddrVariable::new(self.brillig_context.allocate_register(), 1);
1497-
// Check that lhs <= result
1498-
self.brillig_context.binary_instruction(
1499-
left,
1500-
result,
1501-
condition,
1502-
BrilligBinaryOp::LessThanEquals,
1503-
);
1504-
self.brillig_context
1505-
.codegen_constrain(condition, Some("attempt to add with overflow".to_string()));
1506-
self.brillig_context.deallocate_single_addr(condition);
1507-
}
1508-
(BrilligBinaryOp::Sub, false) => {
1509-
if dfg.is_constant(binary.lhs) && max_lhs_bits > max_rhs_bits {
1510-
// `left` is a fixed constant and `right` is restricted such that `left - right > 0`
1511-
// Note strict inequality as `right > left` while `max_lhs_bits == max_rhs_bits` is possible.
1512-
return;
1513-
}
1514-
1515-
let condition =
1516-
SingleAddrVariable::new(self.brillig_context.allocate_register(), 1);
1517-
// Check that rhs <= lhs
1518-
self.brillig_context.binary_instruction(
1519-
right,
1520-
left,
1521-
condition,
1522-
BrilligBinaryOp::LessThanEquals,
1523-
);
1524-
self.brillig_context.codegen_constrain(
1525-
condition,
1526-
Some("attempt to subtract with overflow".to_string()),
1527-
);
1528-
self.brillig_context.deallocate_single_addr(condition);
1529-
}
1530-
(BrilligBinaryOp::Mul, false) => {
1531-
if bit_size == 1 || max_lhs_bits + max_rhs_bits <= bit_size {
1532-
// Either performing boolean multiplication (which cannot overflow),
1533-
// or `left` and `right` have both been casted up from smaller types and so cannot overflow.
1534-
return;
1486+
if let Some(msg) = binary.check_unsigned_overflow_msg(dfg, bit_size) {
1487+
match binary_operation {
1488+
BrilligBinaryOp::Add => {
1489+
let condition =
1490+
SingleAddrVariable::new(self.brillig_context.allocate_register(), 1);
1491+
// Check that lhs <= result
1492+
self.brillig_context.binary_instruction(
1493+
left,
1494+
result,
1495+
condition,
1496+
BrilligBinaryOp::LessThanEquals,
1497+
);
1498+
self.brillig_context.codegen_constrain(condition, Some(msg.to_string()));
1499+
self.brillig_context.deallocate_single_addr(condition);
15351500
}
1536-
1537-
let is_right_zero =
1538-
SingleAddrVariable::new(self.brillig_context.allocate_register(), 1);
1539-
let zero = self.brillig_context.make_constant_instruction(0_usize.into(), bit_size);
1540-
self.brillig_context.binary_instruction(
1541-
zero,
1542-
right,
1543-
is_right_zero,
1544-
BrilligBinaryOp::Equals,
1545-
);
1546-
self.brillig_context.codegen_if_not(is_right_zero.address, |ctx| {
1547-
let condition = SingleAddrVariable::new(ctx.allocate_register(), 1);
1548-
let division = SingleAddrVariable::new(ctx.allocate_register(), bit_size);
1549-
// Check that result / rhs == lhs
1550-
ctx.binary_instruction(result, right, division, BrilligBinaryOp::UnsignedDiv);
1551-
ctx.binary_instruction(division, left, condition, BrilligBinaryOp::Equals);
1552-
ctx.codegen_constrain(
1501+
BrilligBinaryOp::Sub => {
1502+
let condition =
1503+
SingleAddrVariable::new(self.brillig_context.allocate_register(), 1);
1504+
// Check that rhs <= lhs
1505+
self.brillig_context.binary_instruction(
1506+
right,
1507+
left,
15531508
condition,
1554-
Some("attempt to multiply with overflow".to_string()),
1509+
BrilligBinaryOp::LessThanEquals,
15551510
);
1556-
ctx.deallocate_single_addr(condition);
1557-
ctx.deallocate_single_addr(division);
1558-
});
1559-
self.brillig_context.deallocate_single_addr(is_right_zero);
1560-
self.brillig_context.deallocate_single_addr(zero);
1511+
self.brillig_context.codegen_constrain(condition, Some(msg.to_string()));
1512+
self.brillig_context.deallocate_single_addr(condition);
1513+
}
1514+
BrilligBinaryOp::Mul => {
1515+
let is_right_zero =
1516+
SingleAddrVariable::new(self.brillig_context.allocate_register(), 1);
1517+
let zero =
1518+
self.brillig_context.make_constant_instruction(0_usize.into(), bit_size);
1519+
self.brillig_context.binary_instruction(
1520+
zero,
1521+
right,
1522+
is_right_zero,
1523+
BrilligBinaryOp::Equals,
1524+
);
1525+
self.brillig_context.codegen_if_not(is_right_zero.address, |ctx| {
1526+
let condition = SingleAddrVariable::new(ctx.allocate_register(), 1);
1527+
let division = SingleAddrVariable::new(ctx.allocate_register(), bit_size);
1528+
// Check that result / rhs == lhs
1529+
ctx.binary_instruction(
1530+
result,
1531+
right,
1532+
division,
1533+
BrilligBinaryOp::UnsignedDiv,
1534+
);
1535+
ctx.binary_instruction(division, left, condition, BrilligBinaryOp::Equals);
1536+
ctx.codegen_constrain(condition, Some(msg.to_string()));
1537+
ctx.deallocate_single_addr(condition);
1538+
ctx.deallocate_single_addr(division);
1539+
});
1540+
self.brillig_context.deallocate_single_addr(is_right_zero);
1541+
self.brillig_context.deallocate_single_addr(zero);
1542+
}
1543+
_ => {}
15611544
}
1562-
_ => {}
15631545
}
15641546
}
15651547

0 commit comments

Comments
 (0)