Skip to content

Commit

Permalink
feat: impl Default for U128 (noir-lang/noir#6984)
Browse files Browse the repository at this point in the history
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)
  • Loading branch information
AztecBot committed Jan 8, 2025
2 parents 91ad65e + 365d50b commit f87034a
Show file tree
Hide file tree
Showing 17 changed files with 602 additions and 243 deletions.
2 changes: 1 addition & 1 deletion .noir-sync-commit
Original file line number Diff line number Diff line change
@@ -1 +1 @@
8bb3908281d531160db7d7898c67fb2647792e6e
3c488f4b272f460383341c51270b87bfe2b94468
2 changes: 1 addition & 1 deletion noir/noir-repo/.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: |
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
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
env:
NARGO_IGNORE_TEST_FAILURES_FROM_FOREIGN_CALLS: true

Expand Down
23 changes: 18 additions & 5 deletions noir/noir-repo/acvm-repo/acvm/src/pwg/memory_op.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,19 @@ pub(crate) struct MemoryOpSolver<F> {
}

impl<F: AcirField> MemoryOpSolver<F> {
fn index_from_field(&self, index: F) -> Result<MemoryIndex, OpcodeResolutionError<F>> {
if index.num_bits() <= 32 {
let memory_index = index.try_to_u64().unwrap() as MemoryIndex;
Ok(memory_index)
} else {
Err(OpcodeResolutionError::IndexOutOfBounds {
opcode_location: ErrorLocation::Unresolved,
index,
array_size: self.block_len,
})
}
}

fn write_memory_index(
&mut self,
index: MemoryIndex,
Expand All @@ -29,7 +42,7 @@ impl<F: AcirField> MemoryOpSolver<F> {
if index >= self.block_len {
return Err(OpcodeResolutionError::IndexOutOfBounds {
opcode_location: ErrorLocation::Unresolved,
index,
index: F::from(index as u128),
array_size: self.block_len,
});
}
Expand All @@ -40,7 +53,7 @@ impl<F: AcirField> MemoryOpSolver<F> {
fn read_memory_index(&self, index: MemoryIndex) -> Result<F, OpcodeResolutionError<F>> {
self.block_value.get(&index).copied().ok_or(OpcodeResolutionError::IndexOutOfBounds {
opcode_location: ErrorLocation::Unresolved,
index,
index: F::from(index as u128),
array_size: self.block_len,
})
}
Expand Down Expand Up @@ -72,7 +85,7 @@ impl<F: AcirField> MemoryOpSolver<F> {

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

// Calculate the value associated with this memory operation.
//
Expand Down Expand Up @@ -193,9 +206,9 @@ mod tests {
err,
Some(crate::pwg::OpcodeResolutionError::IndexOutOfBounds {
opcode_location: _,
index: 2,
index,
array_size: 2
})
}) if index == FieldElement::from(2u128)
));
}

Expand Down
2 changes: 1 addition & 1 deletion noir/noir-repo/acvm-repo/acvm/src/pwg/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ pub enum OpcodeResolutionError<F> {
payload: Option<ResolvedAssertionPayload<F>>,
},
#[error("Index out of bounds, array has size {array_size:?}, but index was {index:?}")]
IndexOutOfBounds { opcode_location: ErrorLocation, index: u32, array_size: u32 },
IndexOutOfBounds { opcode_location: ErrorLocation, index: F, array_size: u32 },
#[error("Cannot solve opcode: {invalid_input_bit_size}")]
InvalidInputBitSize {
opcode_location: ErrorLocation,
Expand Down
85 changes: 42 additions & 43 deletions noir/noir-repo/compiler/noirc_evaluator/src/acir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1984,14 +1984,7 @@ impl<'a> Context<'a> {

if let NumericType::Unsigned { bit_size } = &num_type {
// Check for integer overflow
self.check_unsigned_overflow(
result,
*bit_size,
binary.lhs,
binary.rhs,
dfg,
binary.operator,
)?;
self.check_unsigned_overflow(result, *bit_size, binary, dfg)?;
}

Ok(result)
Expand All @@ -2002,47 +1995,18 @@ impl<'a> Context<'a> {
&mut self,
result: AcirVar,
bit_size: u32,
lhs: ValueId,
rhs: ValueId,
binary: &Binary,
dfg: &DataFlowGraph,
op: BinaryOp,
) -> Result<(), RuntimeError> {
// We try to optimize away operations that are guaranteed not to overflow
let max_lhs_bits = dfg.get_value_max_num_bits(lhs);
let max_rhs_bits = dfg.get_value_max_num_bits(rhs);

let msg = match op {
BinaryOp::Add => {
if std::cmp::max(max_lhs_bits, max_rhs_bits) < bit_size {
// `lhs` and `rhs` have both been casted up from smaller types and so cannot overflow.
return Ok(());
}
"attempt to add with overflow".to_string()
}
BinaryOp::Sub => {
if dfg.is_constant(lhs) && max_lhs_bits > max_rhs_bits {
// `lhs` is a fixed constant and `rhs` is restricted such that `lhs - rhs > 0`
// Note strict inequality as `rhs > lhs` while `max_lhs_bits == max_rhs_bits` is possible.
return Ok(());
}
"attempt to subtract with overflow".to_string()
}
BinaryOp::Mul => {
if bit_size == 1 || max_lhs_bits + max_rhs_bits <= bit_size {
// Either performing boolean multiplication (which cannot overflow),
// or `lhs` and `rhs` have both been casted up from smaller types and so cannot overflow.
return Ok(());
}
"attempt to multiply with overflow".to_string()
}
_ => return Ok(()),
let Some(msg) = binary.check_unsigned_overflow_msg(dfg, bit_size) else {
return Ok(());
};

let with_pred = self.acir_context.mul_var(result, self.current_side_effects_enabled_var)?;
self.acir_context.range_constrain_var(
with_pred,
&NumericType::Unsigned { bit_size },
Some(msg),
Some(msg.to_string()),
)?;
Ok(())
}
Expand Down Expand Up @@ -2888,8 +2852,9 @@ mod test {
use acvm::{
acir::{
circuit::{
brillig::BrilligFunctionId, opcodes::AcirFunctionId, ExpressionWidth, Opcode,
OpcodeLocation,
brillig::BrilligFunctionId,
opcodes::{AcirFunctionId, BlackBoxFuncCall},
ExpressionWidth, Opcode, OpcodeLocation,
},
native_types::Witness,
},
Expand All @@ -2913,6 +2878,8 @@ mod test {
},
};

use super::Ssa;

fn build_basic_foo_with_return(
builder: &mut FunctionBuilder,
foo_id: FunctionId,
Expand Down Expand Up @@ -3659,4 +3626,36 @@ mod test {
"Should have {expected_num_normal_calls} BrilligCall opcodes to normal Brillig functions but got {num_normal_brillig_calls}"
);
}

#[test]
fn multiply_with_bool_should_not_emit_range_check() {
let src = "
acir(inline) fn main f0 {
b0(v0: bool, v1: u32):
enable_side_effects v0
v2 = cast v0 as u32
v3 = mul v2, v1
return v3
}
";
let ssa = Ssa::from_str(src).unwrap();
let brillig = ssa.to_brillig(false);

let (mut acir_functions, _brillig_functions, _, _) = ssa
.into_acir(&brillig, ExpressionWidth::default())
.expect("Should compile manually written SSA into ACIR");

assert_eq!(acir_functions.len(), 1);

let opcodes = acir_functions[0].take_opcodes();

for opcode in opcodes {
if let Opcode::BlackBoxFuncCall(BlackBoxFuncCall::RANGE { input }) = opcode {
assert!(
input.to_witness().0 <= 1,
"only input witnesses should have range checks: {opcode:?}"
);
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1478,88 +1478,70 @@ impl<'block> BrilligBlock<'block> {
is_signed: bool,
) {
let bit_size = left.bit_size;
let max_lhs_bits = dfg.get_value_max_num_bits(binary.lhs);
let max_rhs_bits = dfg.get_value_max_num_bits(binary.rhs);

if bit_size == FieldElement::max_num_bits() {
if bit_size == FieldElement::max_num_bits() || is_signed {
return;
}

match (binary_operation, is_signed) {
(BrilligBinaryOp::Add, false) => {
if std::cmp::max(max_lhs_bits, max_rhs_bits) < bit_size {
// `left` and `right` have both been casted up from smaller types and so cannot overflow.
return;
}

let condition =
SingleAddrVariable::new(self.brillig_context.allocate_register(), 1);
// Check that lhs <= result
self.brillig_context.binary_instruction(
left,
result,
condition,
BrilligBinaryOp::LessThanEquals,
);
self.brillig_context
.codegen_constrain(condition, Some("attempt to add with overflow".to_string()));
self.brillig_context.deallocate_single_addr(condition);
}
(BrilligBinaryOp::Sub, false) => {
if dfg.is_constant(binary.lhs) && max_lhs_bits > max_rhs_bits {
// `left` is a fixed constant and `right` is restricted such that `left - right > 0`
// Note strict inequality as `right > left` while `max_lhs_bits == max_rhs_bits` is possible.
return;
}

let condition =
SingleAddrVariable::new(self.brillig_context.allocate_register(), 1);
// Check that rhs <= lhs
self.brillig_context.binary_instruction(
right,
left,
condition,
BrilligBinaryOp::LessThanEquals,
);
self.brillig_context.codegen_constrain(
condition,
Some("attempt to subtract with overflow".to_string()),
);
self.brillig_context.deallocate_single_addr(condition);
}
(BrilligBinaryOp::Mul, false) => {
if bit_size == 1 || max_lhs_bits + max_rhs_bits <= bit_size {
// Either performing boolean multiplication (which cannot overflow),
// or `left` and `right` have both been casted up from smaller types and so cannot overflow.
return;
if let Some(msg) = binary.check_unsigned_overflow_msg(dfg, bit_size) {
match binary_operation {
BrilligBinaryOp::Add => {
let condition =
SingleAddrVariable::new(self.brillig_context.allocate_register(), 1);
// Check that lhs <= result
self.brillig_context.binary_instruction(
left,
result,
condition,
BrilligBinaryOp::LessThanEquals,
);
self.brillig_context.codegen_constrain(condition, Some(msg.to_string()));
self.brillig_context.deallocate_single_addr(condition);
}

let is_right_zero =
SingleAddrVariable::new(self.brillig_context.allocate_register(), 1);
let zero = self.brillig_context.make_constant_instruction(0_usize.into(), bit_size);
self.brillig_context.binary_instruction(
zero,
right,
is_right_zero,
BrilligBinaryOp::Equals,
);
self.brillig_context.codegen_if_not(is_right_zero.address, |ctx| {
let condition = SingleAddrVariable::new(ctx.allocate_register(), 1);
let division = SingleAddrVariable::new(ctx.allocate_register(), bit_size);
// Check that result / rhs == lhs
ctx.binary_instruction(result, right, division, BrilligBinaryOp::UnsignedDiv);
ctx.binary_instruction(division, left, condition, BrilligBinaryOp::Equals);
ctx.codegen_constrain(
BrilligBinaryOp::Sub => {
let condition =
SingleAddrVariable::new(self.brillig_context.allocate_register(), 1);
// Check that rhs <= lhs
self.brillig_context.binary_instruction(
right,
left,
condition,
Some("attempt to multiply with overflow".to_string()),
BrilligBinaryOp::LessThanEquals,
);
ctx.deallocate_single_addr(condition);
ctx.deallocate_single_addr(division);
});
self.brillig_context.deallocate_single_addr(is_right_zero);
self.brillig_context.deallocate_single_addr(zero);
self.brillig_context.codegen_constrain(condition, Some(msg.to_string()));
self.brillig_context.deallocate_single_addr(condition);
}
BrilligBinaryOp::Mul => {
let is_right_zero =
SingleAddrVariable::new(self.brillig_context.allocate_register(), 1);
let zero =
self.brillig_context.make_constant_instruction(0_usize.into(), bit_size);
self.brillig_context.binary_instruction(
zero,
right,
is_right_zero,
BrilligBinaryOp::Equals,
);
self.brillig_context.codegen_if_not(is_right_zero.address, |ctx| {
let condition = SingleAddrVariable::new(ctx.allocate_register(), 1);
let division = SingleAddrVariable::new(ctx.allocate_register(), bit_size);
// Check that result / rhs == lhs
ctx.binary_instruction(
result,
right,
division,
BrilligBinaryOp::UnsignedDiv,
);
ctx.binary_instruction(division, left, condition, BrilligBinaryOp::Equals);
ctx.codegen_constrain(condition, Some(msg.to_string()));
ctx.deallocate_single_addr(condition);
ctx.deallocate_single_addr(division);
});
self.brillig_context.deallocate_single_addr(is_right_zero);
self.brillig_context.deallocate_single_addr(zero);
}
_ => {}
}
_ => {}
}
}

Expand Down
Loading

0 comments on commit f87034a

Please sign in to comment.