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: Prevent mutating immutable bindings to mutable reference types #3008

Closed
wants to merge 14 commits into from
Closed
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
1 change: 0 additions & 1 deletion Cargo.lock

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

1 change: 0 additions & 1 deletion acvm-repo/acir_field/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ repository.workspace = true
hex.workspace = true
num-bigint.workspace = true
serde.workspace = true
num-traits.workspace = true

ark-bn254 = { version = "^0.4.0", optional = true, default-features = false, features = [
"curve",
Expand Down
26 changes: 1 addition & 25 deletions acvm-repo/acir_field/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,6 @@
#![warn(clippy::semicolon_if_nothing_returned)]
#![cfg_attr(not(test), warn(unused_crate_dependencies, unused_extern_crates))]

use num_bigint::BigUint;
use num_traits::Num;

cfg_if::cfg_if! {
if #[cfg(feature = "bn254")] {
mod generic_ark;
Expand All @@ -21,33 +18,12 @@ cfg_if::cfg_if! {
}
}

#[derive(Debug, PartialEq, Eq)]
#[derive(Debug)]
pub enum FieldOptions {
BN254,
BLS12_381,
}

impl FieldOptions {
pub fn to_string(&self) -> &str {
match self {
FieldOptions::BN254 => "bn254",
FieldOptions::BLS12_381 => "bls12_381",
}
}

pub fn is_native_field(str: &str) -> bool {
let big_num = if let Some(hex) = str.strip_prefix("0x") {
BigUint::from_str_radix(hex, 16)
} else {
BigUint::from_str_radix(str, 10)
};
if let Ok(big_num) = big_num {
big_num == FieldElement::modulus()
} else {
CHOSEN_FIELD.to_string() == str
}
}
}
// This is needed because features are additive through the dependency graph; if a dependency turns on the bn254, then it
// will be turned on in all crates that depend on it
#[macro_export]
Expand Down
2 changes: 1 addition & 1 deletion acvm-repo/acvm/src/compiler/optimizers/general.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ fn simplify_mul_terms(mut gate: Expression) -> Expression {
let mut hash_map: IndexMap<(Witness, Witness), FieldElement> = IndexMap::new();

// Canonicalize the ordering of the multiplication, lets just order by variable name
for (scale, w_l, w_r) in gate.mul_terms.into_iter() {
for (scale, w_l, w_r) in gate.mul_terms.clone().into_iter() {
let mut pair = [w_l, w_r];
// Sort using rust sort algorithm
pair.sort();
Expand Down
20 changes: 9 additions & 11 deletions acvm-repo/acvm/src/compiler/optimizers/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,22 +23,20 @@ pub fn optimize(acir: Circuit) -> (Circuit, AcirTransformationMap) {
/// Applies [`ProofSystemCompiler`][crate::ProofSystemCompiler] independent optimizations to a [`Circuit`].
pub(super) fn optimize_internal(acir: Circuit) -> (Circuit, AcirTransformationMap) {
// General optimizer pass
let opcodes: Vec<Opcode> = acir
.opcodes
.into_iter()
.map(|opcode| {
if let Opcode::Arithmetic(arith_expr) = opcode {
Opcode::Arithmetic(GeneralOptimizer::optimize(arith_expr))
} else {
opcode
let mut opcodes: Vec<Opcode> = Vec::new();
for opcode in acir.opcodes {
match opcode {
Opcode::Arithmetic(arith_expr) => {
opcodes.push(Opcode::Arithmetic(GeneralOptimizer::optimize(arith_expr)));
}
})
.collect();
other_opcode => opcodes.push(other_opcode),
};
}
let acir = Circuit { opcodes, ..acir };

// Track original acir opcode positions throughout the transformation passes of the compilation
// by applying the modifications done to the circuit opcodes and also to the opcode_positions (delete and insert)
let acir_opcode_positions = (0..acir.opcodes.len()).collect();
let acir_opcode_positions = acir.opcodes.iter().enumerate().map(|(i, _)| i).collect();

// Unused memory optimization pass
let memory_optimizer = UnusedMemoryOptimizer::new(acir);
Expand Down
17 changes: 12 additions & 5 deletions acvm-repo/acvm/src/compiler/optimizers/redundant_range.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,13 +74,13 @@ impl RangeOptimizer {

let mut new_order_list = Vec::with_capacity(order_list.len());
let mut optimized_opcodes = Vec::with_capacity(self.circuit.opcodes.len());
for (idx, opcode) in self.circuit.opcodes.into_iter().enumerate() {
let (witness, num_bits) = match extract_range_opcode(&opcode) {
for (idx, opcode) in self.circuit.opcodes.iter().enumerate() {
let (witness, num_bits) = match extract_range_opcode(opcode) {
Some(range_opcode) => range_opcode,
None => {
// If its not the range opcode, add it to the opcode
// list and continue;
optimized_opcodes.push(opcode);
optimized_opcodes.push(opcode.clone());
new_order_list.push(order_list[idx]);
continue;
}
Expand All @@ -101,11 +101,18 @@ impl RangeOptimizer {
if is_lowest_bit_size {
already_seen_witness.insert(witness);
new_order_list.push(order_list[idx]);
optimized_opcodes.push(opcode);
optimized_opcodes.push(opcode.clone());
}
}

(Circuit { opcodes: optimized_opcodes, ..self.circuit }, new_order_list)
(
Circuit {
current_witness_index: self.circuit.current_witness_index,
opcodes: optimized_opcodes,
..self.circuit
},
new_order_list,
)
}
}

Expand Down
6 changes: 3 additions & 3 deletions acvm-repo/acvm/src/compiler/optimizers/unused_memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,16 +42,16 @@ impl UnusedMemoryOptimizer {
) -> (Circuit, Vec<usize>) {
let mut new_order_list = Vec::with_capacity(order_list.len());
let mut optimized_opcodes = Vec::with_capacity(self.circuit.opcodes.len());
for (idx, opcode) in self.circuit.opcodes.into_iter().enumerate() {
for (idx, opcode) in self.circuit.opcodes.iter().enumerate() {
match opcode {
Opcode::MemoryInit { block_id, .. }
if self.unused_memory_initializations.contains(&block_id) =>
if self.unused_memory_initializations.contains(block_id) =>
{
// Drop opcode
}
_ => {
new_order_list.push(order_list[idx]);
optimized_opcodes.push(opcode);
optimized_opcodes.push(opcode.clone());
}
}
}
Expand Down
22 changes: 11 additions & 11 deletions acvm-repo/acvm/src/compiler/transformers/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,13 +76,13 @@ pub(super) fn transform_internal(
// maps a normalized expression to the intermediate variable which represents the expression, along with its 'norm'
// the 'norm' is simply the value of the first non zero coefficient in the expression, taken from the linear terms, or quadratic terms if there is none.
let mut intermediate_variables: IndexMap<Expression, (FieldElement, Witness)> = IndexMap::new();
for (index, opcode) in acir.opcodes.into_iter().enumerate() {
for (index, opcode) in acir.opcodes.iter().enumerate() {
match opcode {
Opcode::Arithmetic(arith_expr) => {
let len = intermediate_variables.len();

let arith_expr = transformer.transform(
arith_expr,
arith_expr.clone(),
&mut intermediate_variables,
&mut next_witness_index,
);
Expand All @@ -104,7 +104,7 @@ pub(super) fn transform_internal(
transformed_opcodes.push(Opcode::Arithmetic(opcode));
}
}
Opcode::BlackBoxFuncCall(ref func) => {
Opcode::BlackBoxFuncCall(func) => {
match func {
acir::circuit::opcodes::BlackBoxFuncCall::AND { output, .. }
| acir::circuit::opcodes::BlackBoxFuncCall::XOR { output, .. } => {
Expand Down Expand Up @@ -146,9 +146,9 @@ pub(super) fn transform_internal(
}

new_acir_opcode_positions.push(acir_opcode_positions[index]);
transformed_opcodes.push(opcode);
transformed_opcodes.push(opcode.clone());
}
Opcode::Directive(ref directive) => {
Opcode::Directive(directive) => {
match directive {
Directive::Quotient(quotient_directive) => {
transformer.mark_solvable(quotient_directive.q);
Expand All @@ -166,14 +166,14 @@ pub(super) fn transform_internal(
}
}
new_acir_opcode_positions.push(acir_opcode_positions[index]);
transformed_opcodes.push(opcode);
transformed_opcodes.push(opcode.clone());
}
Opcode::MemoryInit { .. } => {
// `MemoryInit` does not write values to the `WitnessMap`
new_acir_opcode_positions.push(acir_opcode_positions[index]);
transformed_opcodes.push(opcode);
transformed_opcodes.push(opcode.clone());
}
Opcode::MemoryOp { ref op, .. } => {
Opcode::MemoryOp { op, .. } => {
for (_, witness1, witness2) in &op.value.mul_terms {
transformer.mark_solvable(*witness1);
transformer.mark_solvable(*witness2);
Expand All @@ -182,9 +182,9 @@ pub(super) fn transform_internal(
transformer.mark_solvable(*witness);
}
new_acir_opcode_positions.push(acir_opcode_positions[index]);
transformed_opcodes.push(opcode);
transformed_opcodes.push(opcode.clone());
}
Opcode::Brillig(ref brillig) => {
Opcode::Brillig(brillig) => {
for output in &brillig.outputs {
match output {
BrilligOutputs::Simple(w) => transformer.mark_solvable(*w),
Expand All @@ -196,7 +196,7 @@ pub(super) fn transform_internal(
}
}
new_acir_opcode_positions.push(acir_opcode_positions[index]);
transformed_opcodes.push(opcode);
transformed_opcodes.push(opcode.clone());
}
}
}
Expand Down
7 changes: 3 additions & 4 deletions compiler/integration-tests/test/browser/recursion.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import { acvm, abi, generateWitness } from '@noir-lang/noir_js';
import * as TOML from 'smol-toml';
import { BarretenbergBackend } from '@noir-lang/backend_barretenberg';
import { getFile } from './utils.js';
import { Field, InputMap } from '@noir-lang/noirc_abi';

const logger = new Logger({ name: 'test', minLevel: TEST_LOG_LEVEL });

Expand Down Expand Up @@ -51,7 +50,7 @@ describe('It compiles noir program code, receiving circuit bytes and abi object.

it('Should generate valid inner proof for correct input, then verify proof within a proof', async () => {
const main_program = await getCircuit(circuit_main_source);
const main_inputs: InputMap = TOML.parse(circuit_main_toml) as InputMap;
const main_inputs = TOML.parse(circuit_main_toml);

const main_backend = new BarretenbergBackend(main_program);

Expand All @@ -70,10 +69,10 @@ describe('It compiles noir program code, receiving circuit bytes and abi object.
numPublicInputs,
);

const recursion_inputs: InputMap = {
const recursion_inputs = {
verification_key: vkAsFields,
proof: proofAsFields,
public_inputs: [main_inputs.y as Field],
public_inputs: [main_inputs.y],
key_hash: vkHash,
input_aggregation_object: ['0', '0', '0', '0', '0', '0', '0', '0', '0', '0', '0', '0', '0', '0', '0', '0'],
};
Expand Down
44 changes: 4 additions & 40 deletions compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -766,36 +766,6 @@ impl Context {

Ok(AcirValue::Array(elements))
}
(
AcirValue::DynamicArray(AcirDynamicArray { block_id, len, .. }),
AcirValue::Array(dummy_values),
) => {
let dummy_values = dummy_values
.into_iter()
.flat_map(|val| val.clone().flatten())
.map(|(var, typ)| AcirValue::Var(var, typ))
.collect::<Vec<_>>();

assert_eq!(
*len,
dummy_values.len(),
"ICE: The store value and dummy must have the same number of inner values"
);

let values = try_vecmap(0..*len, |i| {
let index_var = self.acir_context.add_constant(FieldElement::from(i as u128));

let read = self.acir_context.read_from_memory(*block_id, &index_var)?;
Ok::<AcirValue, RuntimeError>(AcirValue::Var(read, AcirType::field()))
})?;

let mut elements = im::Vector::new();
for (val, dummy_val) in values.iter().zip(dummy_values) {
elements.push_back(self.convert_array_set_store_value(val, &dummy_val)?);
}

Ok(AcirValue::Array(elements))
}
(AcirValue::DynamicArray(_), AcirValue::DynamicArray(_)) => {
unimplemented!("ICE: setting a dynamic array not supported");
}
Expand Down Expand Up @@ -955,14 +925,8 @@ impl Context {
self.array_set_value(value, block_id, var_index)?;
}
}
AcirValue::DynamicArray(AcirDynamicArray { block_id: inner_block_id, len, .. }) => {
let values = try_vecmap(0..len, |i| {
let index_var = self.acir_context.add_constant(FieldElement::from(i as u128));

let read = self.acir_context.read_from_memory(inner_block_id, &index_var)?;
Ok::<AcirValue, RuntimeError>(AcirValue::Var(read, AcirType::field()))
})?;
self.array_set_value(AcirValue::Array(values.into()), block_id, var_index)?;
AcirValue::DynamicArray(_) => {
unimplemented!("ICE: setting a dynamic array not supported");
}
}
Ok(())
Expand All @@ -987,7 +951,7 @@ impl Context {
if !already_initialized {
let value = &dfg[array_id];
match value {
Value::Array { .. } | Value::Instruction { .. } => {
Value::Array { .. } => {
let value = self.convert_value(array_id, dfg);
let len = if matches!(array_typ, Type::Array(_, _)) {
array_typ.flattened_size()
Expand All @@ -1001,7 +965,7 @@ impl Context {
message: format!("Array {array_id} should be initialized"),
call_stack: self.acir_context.get_call_stack(),
}
.into());
.into())
}
}
}
Expand Down
Loading