fix: Start witness of ACIR generated by Noir start at zero not one

barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_to_constraint_buf.hpp

@@ -294,7 +294,8 @@ acir_format circuit_buf_to_acir_format(std::vector<uint8_t> const& buf)
     auto circuit = Circuit::Circuit::bincodeDeserialize(buf);
 
     acir_format af;
-    af.varnum = circuit.current_witness_index;
+    // `varnum` is the true number of variables, thus we add one to the index which starts at zero
+    af.varnum = circuit.current_witness_index + 1;
     af.public_inputs = join({ map(circuit.public_parameters.value, [](auto e) { return e.value; }),
                               map(circuit.return_values.value, [](auto e) { return e.value; }) });
     std::map<uint32_t, BlockConstraint> block_id_to_block_constraint;
@@ -340,9 +341,7 @@ WitnessVector witness_buf_to_witness_data(std::vector<uint8_t> const& buf)
 {
     auto w = WitnessMap::WitnessMap::bincodeDeserialize(buf);
     WitnessVector wv;
-    // TODO(https://github.com/AztecProtocol/barretenberg/issues/816): Does "index" need to be initialized to 0 once we
-    // get rid of the +1 offset in noir?
-    size_t index = 1;
+    size_t index = 0;
     for (auto& e : w.value) {
         // ACIR uses a sparse format for WitnessMap where unused witness indices may be left unassigned.
         // To ensure that witnesses sit at the correct indices in the `WitnessVector`, we fill any indices

barretenberg/cpp/src/barretenberg/dsl/acir_format/serde/acir.hpp

@@ -242,6 +242,9 @@ struct BlockId {
 // TODO(https://github.com/AztecProtocol/barretenberg/issues/825): This struct is more general than it needs / should be
 // allowed to be. We can only accommodate 1 quadratic term and 3 linear terms.
 struct Expression {
+    // WORKTODO: should be this:
+    // std::tuple<std::string, Circuit::Witness, Circuit::Witness> mul_term;
+    // WORKTODO: also, why is this string instead of uint256 or field or something
     std::vector<std::tuple<std::string, Circuit::Witness, Circuit::Witness>> mul_terms;
     std::vector<std::tuple<std::string, Circuit::Witness>> linear_combinations;
     std::string q_c;

barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.hpp

@@ -682,12 +682,6 @@ class UltraCircuitBuilder_ : public CircuitBuilderBase<typename Arithmetization:
         w_o().reserve(size_hint);
         w_4().reserve(size_hint);
 
-        // TODO(https://github.com/AztecProtocol/barretenberg/issues/816): Once the hardcoded +1 offset is removed from
-        // noir, we'll need to move the addition of the const zero variable to after the acir witness has been
-        // incorporated into variables.
-        this->zero_idx = put_constant_variable(FF::zero());
-        this->tau.insert({ DUMMY_TAG, DUMMY_TAG }); // TODO(luke): explain this
-
         for (size_t idx = 0; idx < varnum; ++idx) {
             // Zeros are added for variables whose existence is known but whose values are not yet known. The values may
             // be "set" later on via the assert_equal mechanism.
@@ -697,6 +691,11 @@ class UltraCircuitBuilder_ : public CircuitBuilderBase<typename Arithmetization:
 
         // Add the public_inputs from acir
         this->public_inputs = public_inputs;
+
+        // Add the const zero variable after the acir witness has been
+        // incorporated into variables.
+        this->zero_idx = put_constant_variable(FF::zero());
+        this->tau.insert({ DUMMY_TAG, DUMMY_TAG }); // TODO(luke): explain this
     };
     UltraCircuitBuilder_(const UltraCircuitBuilder_& other) = default;
     UltraCircuitBuilder_(UltraCircuitBuilder_&& other)

noir/compiler/noirc_evaluator/src/ssa.rs

@@ -93,8 +93,8 @@ pub fn create_circuit(
     let mut generated_acir =
         optimize_into_acir(program, enable_ssa_logging, enable_brillig_logging)?;
     let opcodes = generated_acir.take_opcodes();
+    let current_witness_index = generated_acir.current_witness_index();
     let GeneratedAcir {
-        current_witness_index,
         return_witnesses,
         locations,
         input_witnesses,

noir/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs

@@ -138,7 +138,7 @@ impl AcirContext {
     /// Adds a Variable to the context, whose exact value is resolved at
     /// runtime.
     pub(crate) fn add_variable(&mut self) -> AcirVar {
-        let var_index = self.acir_ir.next_witness_index();
+        let var_index = self.acir_ir.get_current_witness_and_update();
 
         let var_data = AcirVarData::Witness(var_index);
 
@@ -1290,6 +1290,7 @@ impl AcirContext {
         inputs: Vec<Witness>,
         warnings: Vec<SsaReport>,
     ) -> GeneratedAcir {
+        self.acir_ir.decrement_witness_index();
         self.acir_ir.input_witnesses = inputs;
         self.acir_ir.warnings = warnings;
         self.acir_ir
@@ -1347,7 +1348,7 @@ impl AcirContext {
         let mut b_outputs = Vec::new();
         let outputs_var = vecmap(outputs, |output| match output {
             AcirType::NumericType(_) => {
-                let witness_index = self.acir_ir.next_witness_index();
+                let witness_index = self.acir_ir.get_current_witness_and_update();
                 b_outputs.push(BrilligOutputs::Simple(witness_index));
                 let var = self.add_data(AcirVarData::Witness(witness_index));
                 AcirValue::Var(var, output.clone())
@@ -1412,7 +1413,7 @@ impl AcirContext {
                         array_values.push_back(nested_acir_value);
                     }
                     AcirType::NumericType(_) => {
-                        let witness_index = self.acir_ir.next_witness_index();
+                        let witness_index = self.acir_ir.get_current_witness_and_update();
                         witnesses.push(witness_index);
                         let var = self.add_data(AcirVarData::Witness(witness_index));
                         array_values.push_back(AcirValue::Var(var, element_type.clone()));
@@ -1497,7 +1498,7 @@ impl AcirContext {
         // Convert the inputs into expressions
         let inputs_expr = try_vecmap(inputs, |input| self.var_to_expression(input))?;
         // Generate output witnesses
-        let outputs_witness = vecmap(0..len, |_| self.acir_ir.next_witness_index());
+        let outputs_witness = vecmap(0..len, |_| self.acir_ir.get_current_witness_and_update());
         let output_expr =
             vecmap(&outputs_witness, |witness_index| Expression::from(*witness_index));
         let outputs_var = vecmap(&outputs_witness, |witness_index| {

noir/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs

@@ -30,7 +30,7 @@ pub(crate) struct GeneratedAcir {
     /// The next witness index that may be declared.
     ///
     /// Equivalent to acvm::acir::circuit::Circuit's field of the same name.
-    pub(crate) current_witness_index: u32,
+    current_witness_index: u32,
 
     /// The opcodes of which the compiled ACIR will comprise.
     opcodes: Vec<AcirOpcode>,
@@ -75,11 +75,16 @@ impl GeneratedAcir {
         std::mem::take(&mut self.opcodes)
     }
 
-    /// Updates the witness index counter and returns
-    /// the next witness index.
-    pub(crate) fn next_witness_index(&mut self) -> Witness {
+    /// Returns the current witness index and updates it 
+    /// so that we have a fresh witness the next time this method is called
+    pub(crate) fn get_current_witness_and_update(&mut self) -> Witness {
+        let current_witness_index = Witness(self.current_witness_index);
         self.current_witness_index += 1;
-        Witness(self.current_witness_index)
+        current_witness_index
+    }
+
+    pub(crate) fn decrement_witness_index(&mut self) {
+        self.current_witness_index -= 1;
     }
 
     /// Converts [`Expression`] `expr` into a [`Witness`].
@@ -100,7 +105,7 @@ impl GeneratedAcir {
     /// Once the `Expression` goes over degree-2, then it needs to be reduced to a `Witness`
     /// which has degree-1 in order to be able to continue the multiplication chain.
     pub(crate) fn create_witness_for_expression(&mut self, expression: &Expression) -> Witness {
-        let fresh_witness = self.next_witness_index();
+        let fresh_witness = self.get_current_witness_and_update();
 
         // Create a constraint that sets them to be equal to each other
         // Then return the witness as this can now be used in places
@@ -138,7 +143,7 @@ impl GeneratedAcir {
         intrinsics_check_inputs(func_name, input_count);
         intrinsics_check_outputs(func_name, output_count);
 
-        let outputs = vecmap(0..output_count, |_| self.next_witness_index());
+        let outputs = vecmap(0..output_count, |_| self.get_current_witness_and_update());
 
         // clone is needed since outputs is moved when used in blackbox function.
         let outputs_clone = outputs.clone();
@@ -269,7 +274,7 @@ impl GeneratedAcir {
             "ICE: Radix must be a power of 2"
         );
 
-        let limb_witnesses = vecmap(0..limb_count, |_| self.next_witness_index());
+        let limb_witnesses = vecmap(0..limb_count, |_| self.get_current_witness_and_update());
         self.push_opcode(AcirOpcode::Directive(Directive::ToLeRadix {
             a: input_expr.clone(),
             b: limb_witnesses.clone(),
@@ -354,7 +359,7 @@ impl GeneratedAcir {
     /// resulting `Witness` is constrained to be the inverse.
     pub(crate) fn brillig_inverse(&mut self, expr: Expression) -> Witness {
         // Create the witness for the result
-        let inverted_witness = self.next_witness_index();
+        let inverted_witness = self.get_current_witness_and_update();
 
         // Compute the inverse with brillig code
         let inverse_code = brillig_directive::directive_invert();
@@ -448,7 +453,7 @@ impl GeneratedAcir {
         // the prover can choose anything here.
         let z = self.brillig_inverse(t_witness.into());
 
-        let y = self.next_witness_index();
+        let y = self.get_current_witness_and_update();
 
         // Add constraint y == 1 - tz => y + tz - 1 == 0
         let y_is_boolean_constraint = Expression {
@@ -538,7 +543,7 @@ impl GeneratedAcir {
             bits_len += ((i + 1) as f32).log2().ceil() as u32;
         }
 
-        let bits = vecmap(0..bits_len, |_| self.next_witness_index());
+        let bits = vecmap(0..bits_len, |_| self.get_current_witness_and_update());
         let inputs = in_expr.iter().map(|a| vec![a.clone()]).collect();
         self.push_opcode(AcirOpcode::Directive(Directive::PermutationSort {
             inputs,

noir/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/sort.rs

@@ -25,7 +25,7 @@ impl GeneratedAcir {
         // witness for the input switches
         let mut conf = iter_extended::vecmap(0..n1, |i| {
             if generate_witness {
-                self.next_witness_index()
+                self.get_current_witness_and_update()
             } else {
                 bits[i]
             }
@@ -63,7 +63,7 @@ impl GeneratedAcir {
         let (w2, b2) = self.permutation_layer(&in_sub2, bits2, generate_witness)?;
         // apply the output switches
         for i in 0..(n - 1) / 2 {
-            let c = if generate_witness { self.next_witness_index() } else { bits[n1 + i] };
+            let c = if generate_witness { self.get_current_witness_and_update() } else { bits[n1 + i] };
             conf.push(c);
             let intermediate = self.mul_with_witness(&Expression::from(c), &(&b2[i] - &b1[i]));
             out_expr.push(&intermediate + &b1[i]);

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

@@ -294,7 +294,7 @@ impl Context {
         dfg: &DataFlowGraph,
     ) -> Result<Vec<Witness>, RuntimeError> {
         // The first witness (if any) is the next one
-        let start_witness = self.acir_context.current_witness_index().0 + 1;
+        let start_witness = self.acir_context.current_witness_index().0;
         for param_id in params {
             let typ = dfg.type_of_value(*param_id);
             let value = self.convert_ssa_block_param(&typ)?;