Skip to content

Commit

Permalink
chore: switch to btreeset for deterministic ordering (#6348)
Browse files Browse the repository at this point in the history
# 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.
  • Loading branch information
guipublic authored Oct 25, 2024
1 parent 81c612f commit d8e549a
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 14 deletions.
6 changes: 3 additions & 3 deletions acvm-repo/acir/src/circuit/opcodes/black_box_function_call.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::collections::HashSet;
use std::collections::BTreeSet;

use crate::native_types::Witness;
use crate::{AcirField, BlackBoxFunc};
Expand Down Expand Up @@ -392,8 +392,8 @@ impl<F: Copy> BlackBoxFuncCall<F> {
}
}

pub fn get_input_witnesses(&self) -> HashSet<Witness> {
let mut result = HashSet::new();
pub fn get_input_witnesses(&self) -> BTreeSet<Witness> {
let mut result = BTreeSet::new();
for input in self.get_inputs_vec() {
if let ConstantOrWitnessEnum::Witness(w) = input.input() {
result.insert(w);
Expand Down
22 changes: 11 additions & 11 deletions acvm-repo/acvm/src/compiler/optimizers/merge_expressions.rs
Original file line number Diff line number Diff line change
@@ -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},
Expand All @@ -7,7 +7,7 @@ use acir::{
};

pub(crate) struct MergeExpressionsOptimizer {
resolved_blocks: HashMap<BlockId, HashSet<Witness>>,
resolved_blocks: HashMap<BlockId, BTreeSet<Witness>>,
}

impl MergeExpressionsOptimizer {
Expand All @@ -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<Witness, HashSet<usize>> = HashMap::new();
let mut used_witness: BTreeMap<Witness, BTreeSet<usize>> = BTreeMap::new();
for (i, opcode) in circuit.opcodes.iter().enumerate() {
let witnesses = self.witness_inputs(opcode);
if let Opcode::MemoryInit { block_id, .. } = opcode {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -104,15 +104,15 @@ impl MergeExpressionsOptimizer {
(new_circuit, new_acir_opcode_positions)
}

fn expr_wit<F>(expr: &Expression<F>) -> HashSet<Witness> {
let mut result = HashSet::new();
fn expr_wit<F>(expr: &Expression<F>) -> BTreeSet<Witness> {
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<F>(&self, input: &BrilligInputs<F>) -> HashSet<Witness> {
let mut result = HashSet::new();
fn brillig_input_wit<F>(&self, input: &BrilligInputs<F>) -> BTreeSet<Witness> {
let mut result = BTreeSet::new();
match input {
BrilligInputs::Single(expr) => {
result.extend(Self::expr_wit(expr));
Expand All @@ -131,15 +131,15 @@ impl MergeExpressionsOptimizer {
}

// Returns the input witnesses used by the opcode
fn witness_inputs<F: AcirField>(&self, opcode: &Opcode<F>) -> HashSet<Witness> {
let mut witnesses = HashSet::new();
fn witness_inputs<F: AcirField>(&self, opcode: &Opcode<F>) -> BTreeSet<Witness> {
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 {
Expand Down

0 comments on commit d8e549a

Please sign in to comment.