From d8e549aad6bae0f96621c57b58a301693463f732 Mon Sep 17 00:00:00 2001 From: guipublic <47281315+guipublic@users.noreply.github.com> Date: Fri, 25 Oct 2024 11:23:02 +0200 Subject: [PATCH] chore: switch to btreeset for deterministic ordering (#6348) # Description ## Problem\* The optimisation pass from PR #6268 iterates over hash maps and hash sets, which does not guarantee deterministic ordering. This is an issue as it could lead to different results for the same ACIR input. ## Summary\* Uses BTreeMap/Set when iterating. ## Additional Context ## Documentation\* Check one: - [X] No documentation needed. - [ ] Documentation included in this PR. - [ ] **[For Experimental Features]** Documentation to be submitted in a separate PR. # PR Checklist\* - [X] I have tested the changes locally. - [X] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings. --- .../opcodes/black_box_function_call.rs | 6 ++--- .../compiler/optimizers/merge_expressions.rs | 22 +++++++++---------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/acvm-repo/acir/src/circuit/opcodes/black_box_function_call.rs b/acvm-repo/acir/src/circuit/opcodes/black_box_function_call.rs index 7e76530b3ca..fa51caf5155 100644 --- a/acvm-repo/acir/src/circuit/opcodes/black_box_function_call.rs +++ b/acvm-repo/acir/src/circuit/opcodes/black_box_function_call.rs @@ -1,4 +1,4 @@ -use std::collections::HashSet; +use std::collections::BTreeSet; use crate::native_types::Witness; use crate::{AcirField, BlackBoxFunc}; @@ -392,8 +392,8 @@ impl BlackBoxFuncCall { } } - pub fn get_input_witnesses(&self) -> HashSet { - let mut result = HashSet::new(); + pub fn get_input_witnesses(&self) -> BTreeSet { + let mut result = BTreeSet::new(); for input in self.get_inputs_vec() { if let ConstantOrWitnessEnum::Witness(w) = input.input() { result.insert(w); diff --git a/acvm-repo/acvm/src/compiler/optimizers/merge_expressions.rs b/acvm-repo/acvm/src/compiler/optimizers/merge_expressions.rs index 4291b997fde..ddf86f60f77 100644 --- a/acvm-repo/acvm/src/compiler/optimizers/merge_expressions.rs +++ b/acvm-repo/acvm/src/compiler/optimizers/merge_expressions.rs @@ -1,4 +1,4 @@ -use std::collections::{HashMap, HashSet}; +use std::collections::{BTreeMap, BTreeSet, HashMap}; use acir::{ circuit::{brillig::BrilligInputs, directives::Directive, opcodes::BlockId, Circuit, Opcode}, @@ -7,7 +7,7 @@ use acir::{ }; pub(crate) struct MergeExpressionsOptimizer { - resolved_blocks: HashMap>, + resolved_blocks: HashMap>, } impl MergeExpressionsOptimizer { @@ -26,7 +26,7 @@ impl MergeExpressionsOptimizer { // Keep track, for each witness, of the gates that use it let circuit_inputs = circuit.circuit_arguments(); self.resolved_blocks = HashMap::new(); - let mut used_witness: HashMap> = HashMap::new(); + let mut used_witness: BTreeMap> = BTreeMap::new(); for (i, opcode) in circuit.opcodes.iter().enumerate() { let witnesses = self.witness_inputs(opcode); if let Opcode::MemoryInit { block_id, .. } = opcode { @@ -54,7 +54,7 @@ impl MergeExpressionsOptimizer { let mut to_keep = true; let input_witnesses = self.witness_inputs(&opcode); for w in input_witnesses.clone() { - let empty_gates = HashSet::new(); + let empty_gates = BTreeSet::new(); let gates_using_w = used_witness.get(&w).unwrap_or(&empty_gates); // We only consider witness which are used in exactly two arithmetic gates if gates_using_w.len() == 2 { @@ -104,15 +104,15 @@ impl MergeExpressionsOptimizer { (new_circuit, new_acir_opcode_positions) } - fn expr_wit(expr: &Expression) -> HashSet { - let mut result = HashSet::new(); + fn expr_wit(expr: &Expression) -> BTreeSet { + let mut result = BTreeSet::new(); result.extend(expr.mul_terms.iter().flat_map(|i| vec![i.1, i.2])); result.extend(expr.linear_combinations.iter().map(|i| i.1)); result } - fn brillig_input_wit(&self, input: &BrilligInputs) -> HashSet { - let mut result = HashSet::new(); + fn brillig_input_wit(&self, input: &BrilligInputs) -> BTreeSet { + let mut result = BTreeSet::new(); match input { BrilligInputs::Single(expr) => { result.extend(Self::expr_wit(expr)); @@ -131,15 +131,15 @@ impl MergeExpressionsOptimizer { } // Returns the input witnesses used by the opcode - fn witness_inputs(&self, opcode: &Opcode) -> HashSet { - let mut witnesses = HashSet::new(); + fn witness_inputs(&self, opcode: &Opcode) -> BTreeSet { + let mut witnesses = BTreeSet::new(); match opcode { Opcode::AssertZero(expr) => Self::expr_wit(expr), Opcode::BlackBoxFuncCall(bb_func) => bb_func.get_input_witnesses(), Opcode::Directive(Directive::ToLeRadix { a, .. }) => Self::expr_wit(a), Opcode::MemoryOp { block_id: _, op, predicate } => { //index et value, et predicate - let mut witnesses = HashSet::new(); + let mut witnesses = BTreeSet::new(); witnesses.extend(Self::expr_wit(&op.index)); witnesses.extend(Self::expr_wit(&op.value)); if let Some(p) = predicate {