From cbc6b77571443f21883f242cd1e74b8a0996072b Mon Sep 17 00:00:00 2001 From: vezenovm Date: Tue, 13 Feb 2024 20:11:02 +0000 Subject: [PATCH 1/2] attach types to AcirDynamicMemory to account for blackbox funcs needing acccurate bitsizes for witnesses --- CHANGELOG.md | 0 CONTRIBUTING.md | 0 Cargo.lock | 0 Cargo.toml | 0 Dockerfile | 0 Dockerfile.ci | 0 Dockerfile.packages | 0 LICENSE-APACHE | 0 LICENSE-MIT | 0 README.md | 0 SUPPORT.md | 0 acvm-repo/acvm/src/pwg/blackbox/hash.rs | 1 + .../src/ssa/acir_gen/acir_ir/acir_variable.rs | 17 +++- .../noirc_evaluator/src/ssa/acir_gen/mod.rs | 84 ++++++++++++++++--- .../noirc_evaluator/src/ssa/ir/instruction.rs | 5 +- cspell.json | 0 default.nix | 0 deny.toml | 0 flake.lock | 0 flake.nix | 0 package.json | 0 release-please-config.json | 0 rust-toolchain.toml | 0 shell.nix | 0 .../array_dynamic_blackbox_input/Nargo.toml | 7 ++ .../array_dynamic_blackbox_input/Prover.toml | 4 + .../array_dynamic_blackbox_input/src/main.nr | 27 ++++++ .../nested_array_dynamic/src/main.nr | 2 +- wasm-bindgen-cli.nix | 0 yarn.lock | 0 30 files changed, 130 insertions(+), 17 deletions(-) mode change 100644 => 100755 CHANGELOG.md mode change 100644 => 100755 CONTRIBUTING.md mode change 100644 => 100755 Cargo.lock mode change 100644 => 100755 Cargo.toml mode change 100644 => 100755 Dockerfile mode change 100644 => 100755 Dockerfile.ci mode change 100644 => 100755 Dockerfile.packages mode change 100644 => 100755 LICENSE-APACHE mode change 100644 => 100755 LICENSE-MIT mode change 100644 => 100755 README.md mode change 100644 => 100755 SUPPORT.md mode change 100644 => 100755 cspell.json mode change 100644 => 100755 default.nix mode change 100644 => 100755 deny.toml mode change 100644 => 100755 flake.lock mode change 100644 => 100755 flake.nix mode change 100644 => 100755 package.json mode change 100644 => 100755 release-please-config.json mode change 100644 => 100755 rust-toolchain.toml mode change 100644 => 100755 shell.nix create mode 100644 test_programs/execution_success/array_dynamic_blackbox_input/Nargo.toml create mode 100644 test_programs/execution_success/array_dynamic_blackbox_input/Prover.toml create mode 100644 test_programs/execution_success/array_dynamic_blackbox_input/src/main.nr mode change 100644 => 100755 wasm-bindgen-cli.nix mode change 100644 => 100755 yarn.lock diff --git a/CHANGELOG.md b/CHANGELOG.md old mode 100644 new mode 100755 diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md old mode 100644 new mode 100755 diff --git a/Cargo.lock b/Cargo.lock old mode 100644 new mode 100755 diff --git a/Cargo.toml b/Cargo.toml old mode 100644 new mode 100755 diff --git a/Dockerfile b/Dockerfile old mode 100644 new mode 100755 diff --git a/Dockerfile.ci b/Dockerfile.ci old mode 100644 new mode 100755 diff --git a/Dockerfile.packages b/Dockerfile.packages old mode 100644 new mode 100755 diff --git a/LICENSE-APACHE b/LICENSE-APACHE old mode 100644 new mode 100755 diff --git a/LICENSE-MIT b/LICENSE-MIT old mode 100644 new mode 100755 diff --git a/README.md b/README.md old mode 100644 new mode 100755 diff --git a/SUPPORT.md b/SUPPORT.md old mode 100644 new mode 100755 diff --git a/acvm-repo/acvm/src/pwg/blackbox/hash.rs b/acvm-repo/acvm/src/pwg/blackbox/hash.rs index 06489822c92..1b6a99ffc3e 100644 --- a/acvm-repo/acvm/src/pwg/blackbox/hash.rs +++ b/acvm-repo/acvm/src/pwg/blackbox/hash.rs @@ -19,6 +19,7 @@ pub(super) fn solve_generic_256_hash_opcode( black_box_func: BlackBoxFunc, ) -> Result<(), OpcodeResolutionError> { let message_input = get_hash_input(initial_witness, inputs, var_message_size)?; + // print!("message_input: {:?}", message_input.clone()); let digest: [u8; 32] = hash_function(&message_input)?; let outputs: [Witness; 32] = outputs.try_into().map_err(|_| { diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs index 2360d053887..519cc32b34a 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs @@ -67,6 +67,13 @@ impl AcirType { pub(crate) fn unsigned(bit_size: u32) -> Self { AcirType::NumericType(NumericType::Unsigned { bit_size }) } + + pub(crate) fn to_numeric_type(self) -> NumericType { + match self { + AcirType::NumericType(numeric_type) => numeric_type, + AcirType::Array(_, _) => unreachable!("cannot fetch a numeric type for an array type"), + } + } } impl From for AcirType { @@ -88,6 +95,12 @@ impl<'a> From<&'a SsaType> for AcirType { } } +impl From for AcirType { + fn from(value: NumericType) -> Self { + AcirType::NumericType(value) + } +} + #[derive(Debug, Default)] /// Context object which holds the relationship between /// `Variables`(AcirVar) and types such as `Expression` and `Witness` @@ -1415,13 +1428,13 @@ impl AcirContext { } Ok(values) } - AcirValue::DynamicArray(AcirDynamicArray { block_id, len, .. }) => { + AcirValue::DynamicArray(AcirDynamicArray { block_id, len, value_types, .. }) => { try_vecmap(0..len, |i| { let index_var = self.add_constant(i); Ok::<(AcirVar, AcirType), InternalError>(( self.read_from_memory(block_id, &index_var)?, - AcirType::field(), + value_types[i].into(), )) }) } diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs index 46be6efcadd..e8d5dda9f52 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs @@ -99,6 +99,13 @@ pub(crate) struct AcirDynamicArray { block_id: BlockId, /// Length of the array len: usize, + /// An ACIR dynamic array is a flat structure, so we use + /// the inner structure of an `AcirType::NumericType` directly. + /// Some usages of ACIR arrays (e.g. black box hashes) require the bit size + /// of every value to be known, thus we store the types as part of the dynamic + /// array definition. + /// The length of the value types vector must match the `len` field in this structure. + value_types: Vec, /// Identification for the ACIR dynamic array /// inner element type sizes array element_type_sizes: Option, @@ -1007,15 +1014,45 @@ impl Context { } else { None }; + + let array_acir_value = self.convert_value(array_id, dfg); + let value_types = Self::dynamic_array_value_types(array_acir_value); + // Compiler sanity check + assert!(value_types.len() == array_len, "ICE: The length of the flattened type array should match the length of the dynamic array"); + let result_value = AcirValue::DynamicArray(AcirDynamicArray { block_id: result_block_id, len: array_len, + value_types, element_type_sizes, }); self.define_result(dfg, instruction, result_value); Ok(()) } + // fn get_dynamic_array_value_types( + // &mut self, + // array_id: ValueId, + // dfg: &DataFlowGraph, + // ) -> Vec { + // let array_acir_value = self.convert_value(array_id, dfg); + // match array_acir_value { + // AcirValue::Array(_) => array_acir_value.flatten().into_iter().map(|(_, typ)| typ.to_numeric_type()).collect::>(), + // AcirValue::DynamicArray(AcirDynamicArray { value_types, .. }) => value_types, + // _ => unreachable!("An AcirValue::Var cannot be used as an array value"), + // } + // } + + fn dynamic_array_value_types( + array_acir_value: AcirValue, + ) -> Vec { + match array_acir_value { + AcirValue::Array(_) => array_acir_value.flatten().into_iter().map(|(_, typ)| typ.to_numeric_type()).collect::>(), + AcirValue::DynamicArray(AcirDynamicArray { value_types, .. }) => value_types, + _ => unreachable!("An AcirValue::Var cannot be used as an array value"), + } + } + fn array_set_value( &mut self, value: &AcirValue, @@ -1093,7 +1130,7 @@ impl Context { &mut self, array_typ: &Type, array_id: ValueId, - array_acir_value: Option, + supplied_array_value: Option<&AcirValue>, dfg: &DataFlowGraph, ) -> Result { let element_type_sizes = self.internal_block_id(&array_id); @@ -1119,10 +1156,11 @@ impl Context { Value::Instruction { .. } | Value::Param { .. } => { // An instruction representing the slice means it has been processed previously during ACIR gen. // Use the previously defined result of an array operation to fetch the internal type information. - let array_acir_value = if let Some(array_acir_value) = array_acir_value { + let value = &self.convert_value(array_id, dfg); + let array_acir_value = if let Some(array_acir_value) = supplied_array_value { array_acir_value } else { - self.convert_value(array_id, dfg) + value }; match array_acir_value { AcirValue::DynamicArray(AcirDynamicArray { @@ -1138,7 +1176,7 @@ impl Context { } )?; self.copy_dynamic_array( - inner_elem_type_sizes, + *inner_elem_type_sizes, element_type_sizes, type_sizes_array_len, )?; @@ -1192,7 +1230,7 @@ impl Context { } } - // The final array should will the flattened index at each outer array index + // The final array should be the flattened index of each element of the outer array let init_values = vecmap(flat_elem_type_sizes, |type_size| { let var = self.acir_context.add_constant(type_size); AcirValue::Var(var, AcirType::field()) @@ -1600,7 +1638,9 @@ impl Context { } } - let inputs = vecmap(&arguments_no_slice_len, |arg| self.convert_value(*arg, dfg)); + let inputs = vecmap(&arguments_no_slice_len, |arg| { + self.convert_value(*arg, dfg) + }); let output_count = result_ids.iter().fold(0usize, |sum, result_id| { sum + dfg.try_get_array_length(*result_id).unwrap_or(1) @@ -1683,15 +1723,20 @@ impl Context { Some(self.init_element_type_sizes_array( &slice_typ, slice_contents, - Some(new_slice_val), + Some(&new_slice_val), dfg, )?) } else { None }; + + let value_types = Self::dynamic_array_value_types(new_slice_val); + assert!(value_types.len() == new_elem_size, "ICE: Value types array must match new slice size"); + let result = AcirValue::DynamicArray(AcirDynamicArray { block_id: result_block_id, len: new_elem_size, + value_types, element_type_sizes, }); Ok(vec![AcirValue::Var(new_slice_length, AcirType::field()), result]) @@ -1738,15 +1783,20 @@ impl Context { Some(self.init_element_type_sizes_array( &slice_typ, slice_contents, - Some(new_slice_val), + Some(&new_slice_val), dfg, )?) } else { None }; + + let value_types = Self::dynamic_array_value_types(new_slice_val); + assert!(value_types.len() == new_slice_size, "ICE: Value types array must match new slice size"); + let result = AcirValue::DynamicArray(AcirDynamicArray { block_id: result_block_id, len: new_slice_size, + value_types, element_type_sizes, }); @@ -1943,15 +1993,20 @@ impl Context { Some(self.init_element_type_sizes_array( &slice_typ, slice_contents, - Some(slice), + Some(&slice), dfg, )?) } else { None }; + + let value_types = Self::dynamic_array_value_types(slice); + assert!(value_types.len() == slice_size, "ICE: Value types array must match new slice size"); + let result = AcirValue::DynamicArray(AcirDynamicArray { block_id: result_block_id, len: slice_size, + value_types, element_type_sizes, }); @@ -2059,15 +2114,20 @@ impl Context { Some(self.init_element_type_sizes_array( &slice_typ, slice_contents, - Some(new_slice_val), + Some(&new_slice_val), dfg, )?) } else { None }; + + let value_types = Self::dynamic_array_value_types(new_slice_val); + assert!(value_types.len() == slice_size, "ICE: Value types array must match new slice size"); + let result = AcirValue::DynamicArray(AcirDynamicArray { block_id: result_block_id, len: slice_size, + value_types, element_type_sizes, }); @@ -2094,14 +2154,14 @@ impl Context { self.slice_intrinsic_input(old_slice, var)?; } } - AcirValue::DynamicArray(AcirDynamicArray { block_id, len, .. }) => { + AcirValue::DynamicArray(AcirDynamicArray { block_id, len, value_types, .. }) => { for i in 0..len { // We generate witnesses corresponding to the array values let index_var = self.acir_context.add_constant(i); let value_read_var = self.acir_context.read_from_memory(block_id, &index_var)?; - let value_read = AcirValue::Var(value_read_var, AcirType::field()); + let value_read = AcirValue::Var(value_read_var, value_types[i].into()); old_slice.push_back(value_read); } diff --git a/compiler/noirc_evaluator/src/ssa/ir/instruction.rs b/compiler/noirc_evaluator/src/ssa/ir/instruction.rs index 0b6c7074e45..83142cf2a57 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/instruction.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/instruction.rs @@ -241,7 +241,8 @@ impl Instruction { // In ACIR, a division with a false predicate outputs (0,0), so it cannot replace another instruction unless they have the same predicate bin.operator != BinaryOp::Div } - Cast(_, _) | Truncate { .. } | Not(_) | ArrayGet { .. } | ArraySet { .. } => true, + // Cast(_, _) | Truncate { .. } | Not(_) | ArrayGet { .. } | ArraySet { .. } => true, + Cast(_, _) | Truncate { .. } | Not(_) | ArrayGet { .. } | ArraySet { .. } => true, // These either have side-effects or interact with memory Constrain(..) @@ -277,7 +278,7 @@ impl Instruction { | Not(_) | Truncate { .. } | Allocate - | Load { .. } + | Load { .. } | ArrayGet { .. } | ArraySet { .. } => false, diff --git a/cspell.json b/cspell.json old mode 100644 new mode 100755 diff --git a/default.nix b/default.nix old mode 100644 new mode 100755 diff --git a/deny.toml b/deny.toml old mode 100644 new mode 100755 diff --git a/flake.lock b/flake.lock old mode 100644 new mode 100755 diff --git a/flake.nix b/flake.nix old mode 100644 new mode 100755 diff --git a/package.json b/package.json old mode 100644 new mode 100755 diff --git a/release-please-config.json b/release-please-config.json old mode 100644 new mode 100755 diff --git a/rust-toolchain.toml b/rust-toolchain.toml old mode 100644 new mode 100755 diff --git a/shell.nix b/shell.nix old mode 100644 new mode 100755 diff --git a/test_programs/execution_success/array_dynamic_blackbox_input/Nargo.toml b/test_programs/execution_success/array_dynamic_blackbox_input/Nargo.toml new file mode 100644 index 00000000000..03da304acc3 --- /dev/null +++ b/test_programs/execution_success/array_dynamic_blackbox_input/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "array_dynamic_blackbox_input" +type = "bin" +authors = [""] +compiler_version = ">=0.24.0" + +[dependencies] \ No newline at end of file diff --git a/test_programs/execution_success/array_dynamic_blackbox_input/Prover.toml b/test_programs/execution_success/array_dynamic_blackbox_input/Prover.toml new file mode 100644 index 00000000000..f429436c162 --- /dev/null +++ b/test_programs/execution_success/array_dynamic_blackbox_input/Prover.toml @@ -0,0 +1,4 @@ +index = "1" +leaf = ["51", "109", "224", "175", "60", "42", "79", "222", "117", "255", "174", "79", "126", "242", "74", "34", "100", "35", "20", "200", "109", "89", "191", "219", "41", "10", "118", "217", "165", "224", "215", "109"] +path = ["0", "1", "2", "3", "4", "5", "6", "7", "8", "9", "10", "11", "12", "13", "14", "15", "16", "17", "18", "19", "20", "21", "22", "23", "24", "25", "26", "27", "28", "29", "30", "31", "32", "33", "34", "35", "36", "37", "38", "39", "40", "41", "42", "43", "44", "45", "46", "47", "48", "49", "50", "51", "52", "53", "54", "55", "56", "57", "58", "59", "60", "61", "62", "63"] +root = [79, 230, 126, 184, 98, 125, 226, 58, 117, 45, 140, 15, 72, 118, 89, 173, 117, 161, 166, 0, 214, 125, 13, 16, 113, 81, 173, 156, 97, 15, 57, 216] \ No newline at end of file diff --git a/test_programs/execution_success/array_dynamic_blackbox_input/src/main.nr b/test_programs/execution_success/array_dynamic_blackbox_input/src/main.nr new file mode 100644 index 00000000000..aabf7fc9d5c --- /dev/null +++ b/test_programs/execution_success/array_dynamic_blackbox_input/src/main.nr @@ -0,0 +1,27 @@ +fn main(leaf: [u8; 32], path: [u8; 64], index: u32, root: [u8; 32]) { + compute_root(leaf, path, index, root); +} + +fn compute_root(leaf: [u8; 32], path: [u8; 64], _index: u32, root: [u8; 32]) { + let mut current = leaf; + let mut index = _index; + + for i in 0..2 { + let mut hash_input = [0; 64]; + let offset = i * 32; + let is_right = (index & 1) != 0; + let a = if is_right { 32 } else { 0 }; + let b = if is_right { 0 } else { 32 }; + + for j in 0..32 { + hash_input[j + a] = current[j]; + hash_input[j + b] = path[offset + j]; + } + + current = dep::std::hash::sha256(hash_input); + index = index >> 1; + } + + // Regression for issue #4258 + assert(root == current); +} \ No newline at end of file diff --git a/test_programs/execution_success/nested_array_dynamic/src/main.nr b/test_programs/execution_success/nested_array_dynamic/src/main.nr index 2c53822d6b9..55c7fbf31e9 100644 --- a/test_programs/execution_success/nested_array_dynamic/src/main.nr +++ b/test_programs/execution_success/nested_array_dynamic/src/main.nr @@ -9,7 +9,7 @@ struct Foo { } struct FooParent { - array: [Field; 3], + array: [u8; 3], foos: [Foo; 4], } diff --git a/wasm-bindgen-cli.nix b/wasm-bindgen-cli.nix old mode 100644 new mode 100755 diff --git a/yarn.lock b/yarn.lock old mode 100644 new mode 100755 From 6c2fbaf6a1531e89f7fe3ca3b63aa9f038cee941 Mon Sep 17 00:00:00 2001 From: vezenovm Date: Tue, 13 Feb 2024 20:17:02 +0000 Subject: [PATCH 2/2] remove old comments --- acvm-repo/acvm/src/pwg/blackbox/hash.rs | 1 - compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs | 13 ------------- 2 files changed, 14 deletions(-) diff --git a/acvm-repo/acvm/src/pwg/blackbox/hash.rs b/acvm-repo/acvm/src/pwg/blackbox/hash.rs index 1b6a99ffc3e..06489822c92 100644 --- a/acvm-repo/acvm/src/pwg/blackbox/hash.rs +++ b/acvm-repo/acvm/src/pwg/blackbox/hash.rs @@ -19,7 +19,6 @@ pub(super) fn solve_generic_256_hash_opcode( black_box_func: BlackBoxFunc, ) -> Result<(), OpcodeResolutionError> { let message_input = get_hash_input(initial_witness, inputs, var_message_size)?; - // print!("message_input: {:?}", message_input.clone()); let digest: [u8; 32] = hash_function(&message_input)?; let outputs: [Witness; 32] = outputs.try_into().map_err(|_| { diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs index e8d5dda9f52..daf74e53453 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs @@ -1030,19 +1030,6 @@ impl Context { Ok(()) } - // fn get_dynamic_array_value_types( - // &mut self, - // array_id: ValueId, - // dfg: &DataFlowGraph, - // ) -> Vec { - // let array_acir_value = self.convert_value(array_id, dfg); - // match array_acir_value { - // AcirValue::Array(_) => array_acir_value.flatten().into_iter().map(|(_, typ)| typ.to_numeric_type()).collect::>(), - // AcirValue::DynamicArray(AcirDynamicArray { value_types, .. }) => value_types, - // _ => unreachable!("An AcirValue::Var cannot be used as an array value"), - // } - // } - fn dynamic_array_value_types( array_acir_value: AcirValue, ) -> Vec {