fix(acir): Use types on dynamic arrays (#4364)

# Description

## Problem\*

Resolves #4356 

Supercedes #4360

## Summary\*

An ACIR dynamic array is a pointer to flat memory. We have been treating
this flat memory as a list of fields, however, this breaks if we do in
fact need accurate numeric type information such as when working black
box function inputs. For example for hash inputs we set up the byte
array based upon the bit size. This needs to be the correct bit size or
else we will get a lot of extra garbage when calling
`fetch_nearest_bytes` on a FieldElement.

This PR attaches a list of `Vec<NumericType>` to the `AcirDynamicArray`
structure. This gives us the expected output result for `sha` then.

We probably could restrict the `AcirDynamicArray` to be created only
through a constructor where we check that the `value_types` match the
supplied len in size. I left it for a follow-up as this is a quick fix
but I can do it as part of this PR.

## Additional Context



## Documentation\*

Check one:
- [ ] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[Exceptional Case]** Documentation to be submitted in a separate
PR.

# PR Checklist\*

- [ ] I have tested the changes locally.
- [ ] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.

---------

Co-authored-by: TomAFrench <tom@tomfren.ch>

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<SsaType> for AcirType {
@@ -88,6 +95,12 @@ impl<'a> From<&'a SsaType> for AcirType {
     }
 }
 
+impl From<NumericType> 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(),
                     ))
                 })
             }

compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs

@@ -99,6 +99,18 @@ 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 functions) require the bit size
+    /// of every value to be known, thus we store the types as part of the dynamic
+    /// array definition.
+    ///
+    /// A dynamic non-homogenous array can potentially have values of differing types.
+    /// Thus, we store a vector of types rather than a single type, as a dynamic non-homogenous array
+    /// is still represented in ACIR by a single `AcirDynamicArray` structure.
+    ///
+    /// The length of the value types vector must match the `len` field in this structure.
+    value_types: Vec<NumericType>,
     /// Identification for the ACIR dynamic array
     /// inner element type sizes array
     element_type_sizes: Option<BlockId>,
@@ -150,6 +162,16 @@ impl AcirValue {
             AcirValue::DynamicArray(_) => unimplemented!("Cannot flatten a dynamic array"),
         }
     }
+
+    fn flat_numeric_types(self) -> Vec<NumericType> {
+        match self {
+            AcirValue::Array(_) => {
+                self.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"),
+        }
+    }
 }
 
 impl Ssa {
@@ -1007,9 +1029,15 @@ impl Context {
         } else {
             None
         };
+
+        let value_types = self.convert_value(array_id, dfg).flat_numeric_types();
+        // Compiler sanity check
+        assert_eq!(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);
@@ -1093,7 +1121,7 @@ impl Context {
         &mut self,
         array_typ: &Type,
         array_id: ValueId,
-        array_acir_value: Option<AcirValue>,
+        supplied_acir_value: Option<&AcirValue>,
         dfg: &DataFlowGraph,
     ) -> Result<BlockId, RuntimeError> {
         let element_type_sizes = self.internal_block_id(&array_id);
@@ -1119,26 +1147,23 @@ 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 {
-                            array_acir_value
-                        } else {
-                            self.convert_value(array_id, dfg)
-                        };
+                        let array_acir_value = &self.convert_value(array_id, dfg);
+                        let array_acir_value = supplied_acir_value.unwrap_or(array_acir_value);
                         match array_acir_value {
                             AcirValue::DynamicArray(AcirDynamicArray {
                                 element_type_sizes: inner_elem_type_sizes,
                                 ..
                             }) => {
                                 if let Some(inner_elem_type_sizes) = inner_elem_type_sizes {
-                                    if self.initialized_arrays.contains(&inner_elem_type_sizes) {
-                                        let type_sizes_array_len = self.internal_mem_block_lengths.get(&inner_elem_type_sizes).copied().ok_or_else(||
+                                    if self.initialized_arrays.contains(inner_elem_type_sizes) {
+                                        let type_sizes_array_len = *self.internal_mem_block_lengths.get(inner_elem_type_sizes).ok_or_else(||
                                             InternalError::General {
                                                 message: format!("Array {array_id}'s inner element type sizes array does not have a tracked length"),
                                                 call_stack: self.acir_context.get_call_stack(),
                                             }
                                         )?;
                                         self.copy_dynamic_array(
-                                            inner_elem_type_sizes,
+                                            *inner_elem_type_sizes,
                                             element_type_sizes,
                                             type_sizes_array_len,
                                         )?;
@@ -1683,15 +1708,24 @@ 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 = new_slice_val.flat_numeric_types();
+                assert_eq!(
+                    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 +1772,24 @@ 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 = new_slice_val.flat_numeric_types();
+                assert_eq!(
+                    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 +1986,24 @@ impl Context {
                     Some(self.init_element_type_sizes_array(
                         &slice_typ,
                         slice_contents,
-                        Some(slice),
+                        Some(&slice),
                         dfg,
                     )?)
                 } else {
                     None
                 };
+
+                let value_types = slice.flat_numeric_types();
+                assert_eq!(
+                    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 +2111,24 @@ 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 = new_slice_val.flat_numeric_types();
+                assert_eq!(
+                    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,
                 });
 

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]

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]

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);
+}

test_programs/execution_success/array_dynamic_nested_blackbox_input/Nargo.toml

@@ -0,0 +1,7 @@
+[package]
+name = "array_dynamic_nested_blackbox_input"
+type = "bin"
+authors = [""]
+compiler_version = ">=0.24.0"
+
+[dependencies]

test_programs/execution_success/array_dynamic_nested_blackbox_input/Prover.toml

@@ -0,0 +1,23 @@
+y = "3"
+hash_result = [50, 53, 90, 252, 105, 236, 223, 30, 135, 229, 193, 172, 51, 139, 8, 32, 188, 104, 151, 115, 129, 168, 27, 71, 203, 47, 40, 228, 89, 177, 129, 100]
+
+[[x]]
+a = "1"
+b = ["2", "3", "20"]
+
+[x.bar]
+inner = ["100", "101", "102"]
+
+[[x]]
+a = "4" # idx = 3, flattened start idx = 7
+b = ["5", "6", "21"] # idx = 4, flattened start idx = 8
+
+[x.bar]
+inner = ["103", "104", "105"] # idx = 5, flattened start idx = 11
+
+[[x]]
+a = "7"
+b = ["8", "9", "22"]
+
+[x.bar]
+inner = ["106", "107", "108"]

test_programs/execution_success/array_dynamic_nested_blackbox_input/src/main.nr

@@ -0,0 +1,20 @@
+struct Bar {
+    inner: [u8; 3],
+}
+
+struct Foo {
+    a: Field,
+    b: [Field; 3],
+    bar: Bar,
+}
+
+fn main(mut x: [Foo; 3], y: pub Field, hash_result: pub [u8; 32]) {
+    // Simple dynamic array set for entire inner most array
+    x[y - 1].bar.inner = [106, 107, 10];
+    let mut hash_input = x[y - 1].bar.inner;
+    // Make sure that we are passing a dynamic array to the black box function call
+    // by setting the array using a dynamic index here
+    hash_input[y - 1] = 0;
+    let hash = dep::std::hash::sha256(hash_input);
+    assert_eq(hash, hash_result);
+}