From b12b7ecb995988c731be5f1f2f67fda952f1a228 Mon Sep 17 00:00:00 2001 From: guipublic <47281315+guipublic@users.noreply.github.com> Date: Fri, 3 Nov 2023 16:07:31 +0100 Subject: [PATCH] feat: Use ranges instead of a vector for input witness (#3314) --- compiler/noirc_evaluator/src/ssa.rs | 25 ++++++++++++---- .../noirc_evaluator/src/ssa/abi_gen/mod.rs | 30 +++++++++++++++---- .../src/ssa/acir_gen/acir_ir/acir_variable.rs | 26 ++++++++++++++-- .../ssa/acir_gen/acir_ir/generated_acir.rs | 3 +- .../noirc_evaluator/src/ssa/acir_gen/mod.rs | 7 +++-- .../assert_lt/target/assert_lt.json | 2 +- tooling/noirc_abi/src/lib.rs | 27 ++++++++++++----- tooling/noirc_abi_wasm/src/lib.rs | 4 +-- .../noirc_abi_wasm/test/shared/abi_encode.ts | 2 +- .../test/shared/array_as_field.ts | 2 +- .../test/shared/field_as_array.ts | 2 +- .../test/shared/uint_overflow.ts | 2 +- 12 files changed, 99 insertions(+), 33 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa.rs b/compiler/noirc_evaluator/src/ssa.rs index ff13878e129..444780ec534 100644 --- a/compiler/noirc_evaluator/src/ssa.rs +++ b/compiler/noirc_evaluator/src/ssa.rs @@ -7,7 +7,10 @@ //! This module heavily borrows from Cranelift #![allow(dead_code)] -use std::collections::BTreeSet; +use std::{ + collections::{BTreeMap, BTreeSet}, + ops::Range, +}; use crate::errors::{RuntimeError, SsaReport}; use acvm::acir::{ @@ -87,14 +90,12 @@ pub fn create_circuit( .. } = generated_acir; - let abi = gen_abi(context, func_sig, &input_witnesses, return_witnesses.clone()); + let abi = gen_abi(context, func_sig, input_witnesses, return_witnesses.clone()); let public_abi = abi.clone().public_abi(); - let public_parameters = - PublicInputs(public_abi.param_witnesses.values().flatten().copied().collect()); + let public_parameters = PublicInputs(tree_to_set(&public_abi.param_witnesses)); - let all_parameters: BTreeSet = - abi.param_witnesses.values().flatten().copied().collect(); + let all_parameters: BTreeSet = tree_to_set(&abi.param_witnesses); let private_parameters = all_parameters.difference(&public_parameters.0).copied().collect(); let return_values = PublicInputs(return_witnesses.into_iter().collect()); @@ -162,3 +163,15 @@ impl SsaBuilder { self } } + +// Flatten the witnesses in the map into a BTreeSet +fn tree_to_set(input: &BTreeMap>>) -> BTreeSet { + let mut result = BTreeSet::new(); + for range in input.values().flatten() { + for i in range.start.witness_index()..range.end.witness_index() { + result.insert(Witness(i)); + } + } + + result +} diff --git a/compiler/noirc_evaluator/src/ssa/abi_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/abi_gen/mod.rs index e4b4026bf21..948fb7d5263 100644 --- a/compiler/noirc_evaluator/src/ssa/abi_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/abi_gen/mod.rs @@ -11,6 +11,7 @@ use noirc_frontend::{ }, node_interner::NodeInterner, }; +use std::ops::Range; /// Attempts to retrieve the name of this parameter. Returns None /// if this parameter is a tuple or struct pattern. @@ -38,7 +39,7 @@ pub fn into_abi_params(context: &Context, params: Vec) -> Vec>, return_witnesses: Vec, ) -> Abi { let (parameters, return_type) = func_sig; @@ -52,16 +53,33 @@ pub(crate) fn gen_abi( // parameter's constituent values live. fn param_witnesses_from_abi_param( abi_params: &Vec, - input_witnesses: &[Witness], -) -> BTreeMap> { + input_witnesses: Vec>, +) -> BTreeMap>> { let mut idx = 0_usize; + if input_witnesses.is_empty() { + return BTreeMap::new(); + } + let mut processed_range = input_witnesses[idx].start.witness_index(); btree_map(abi_params, |param| { let num_field_elements_needed = param.typ.field_count(); let mut wit = Vec::new(); - for _ in 0..num_field_elements_needed { - wit.push(input_witnesses[idx]); - idx += 1; + let mut processed_fields = 0; + while processed_fields < num_field_elements_needed { + let end = input_witnesses[idx].end.witness_index(); + if num_field_elements_needed <= end - processed_range { + wit.push( + Witness(processed_range)..Witness(processed_range + num_field_elements_needed), + ); + processed_range += num_field_elements_needed; + processed_fields += num_field_elements_needed; + } else { + // consume the current range + wit.push(Witness(processed_range)..input_witnesses[idx].end); + processed_fields += end - processed_range; + idx += 1; + processed_range = input_witnesses[idx].start.witness_index(); + } } (param.name.clone(), wit) }) 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 908189ad785..17c51f3a116 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 @@ -23,6 +23,7 @@ use acvm::{BlackBoxFunctionSolver, BlackBoxResolutionError}; use fxhash::FxHashMap as HashMap; use iter_extended::{try_vecmap, vecmap}; use num_bigint::BigUint; +use std::ops::RangeInclusive; use std::{borrow::Cow, hash::Hash}; #[derive(Clone, Debug, PartialEq, Eq, Hash)] @@ -1176,8 +1177,29 @@ impl AcirContext { } /// Terminates the context and takes the resulting `GeneratedAcir` - pub(crate) fn finish(mut self, inputs: Vec, warnings: Vec) -> GeneratedAcir { - self.acir_ir.input_witnesses = vecmap(inputs, Witness); + pub(crate) fn finish( + mut self, + inputs: Vec>, + warnings: Vec, + ) -> GeneratedAcir { + let mut current_range = 0..0; + for range in inputs { + if current_range.end == *range.start() { + current_range.end = range.end() + 1; + } else { + if current_range.end != 0 { + self.acir_ir + .input_witnesses + .push(Witness(current_range.start)..Witness(current_range.end)); + } + current_range = *range.start()..range.end() + 1; + } + } + if current_range.end != 0 { + self.acir_ir + .input_witnesses + .push(Witness(current_range.start)..Witness(current_range.end)); + } self.acir_ir.warnings = warnings; self.acir_ir } diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs index f29d3c9ec05..9bd83096adf 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs @@ -23,6 +23,7 @@ use acvm::{ }; use iter_extended::vecmap; use num_bigint::BigUint; +use std::ops::Range; #[derive(Debug, Default)] /// The output of the Acir-gen pass @@ -42,7 +43,7 @@ pub(crate) struct GeneratedAcir { pub(crate) return_witnesses: Vec, /// All witness indices which are inputs to the main function - pub(crate) input_witnesses: Vec, + pub(crate) input_witnesses: Vec>, /// Correspondence between an opcode index (in opcodes) and the source code call stack which generated it pub(crate) locations: BTreeMap, diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs index 35b970f1e06..23a24c984f5 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs @@ -25,6 +25,7 @@ use crate::brillig::brillig_ir::BrilligContext; use crate::brillig::{brillig_gen::brillig_fn::FunctionContext as BrilligFunctionContext, Brillig}; use crate::errors::{InternalError, InternalWarning, RuntimeError, SsaReport}; pub(crate) use acir_ir::generated_acir::GeneratedAcir; + use acvm::{ acir::{circuit::opcodes::BlockId, native_types::Expression}, FieldElement, @@ -203,7 +204,7 @@ impl Context { let warnings = self.convert_ssa_return(entry_block.unwrap_terminator(), dfg)?; - Ok(self.acir_context.finish(input_witness.collect(), warnings)) + Ok(self.acir_context.finish(vec![input_witness], warnings)) } fn convert_brillig_main( @@ -239,8 +240,8 @@ impl Context { for acir_var in output_vars { self.acir_context.return_var(acir_var)?; } - - Ok(self.acir_context.finish(witness_inputs, Vec::new())) + let witnesses = vecmap(witness_inputs, |input| RangeInclusive::new(input, input)); + Ok(self.acir_context.finish(witnesses, Vec::new())) } /// Adds and binds `AcirVar`s for each numeric block parameter or block parameter array element. diff --git a/tooling/noir_js/test/noir_compiled_examples/assert_lt/target/assert_lt.json b/tooling/noir_js/test/noir_compiled_examples/assert_lt/target/assert_lt.json index 3b2b1b2c5a1..c01b2c5d3f5 100644 --- a/tooling/noir_js/test/noir_compiled_examples/assert_lt/target/assert_lt.json +++ b/tooling/noir_js/test/noir_compiled_examples/assert_lt/target/assert_lt.json @@ -1 +1 @@ -{"hash":13834844072603749544,"backend":"acvm-backend-barretenberg","abi":{"parameters":[{"name":"x","type":{"kind":"integer","sign":"unsigned","width":64},"visibility":"private"},{"name":"y","type":{"kind":"integer","sign":"unsigned","width":64},"visibility":"public"}],"param_witnesses":{"x":[1],"y":[2]},"return_type":{"kind":"integer","sign":"unsigned","width":64},"return_witnesses":[12]},"bytecode":"H4sIAAAAAAAA/+1WUW6DMAx1QksZoGr72jUcAiX8VbvJ0Oj9j7ChJpKbtXw0NpvUWkImUXixn53w3gDgHc6mfh7t/ZGMtR9TU96HeYuHtp36ZjLWfGIzjK7DthsPzjjTue6rcdZOrnX9MA49Dqa1kzl1gz3h2bL7sTDCMhmJbylmTDOT8WEhjXfjH/DcB8u8zwVygWifmL/9lTnWzSWKsxHA3QJf00vlveWvERJIUU4x0eb86aEJppljVox9oO+Py8QTV1Jnw6a85t7vSL8pwvN89j7gd88o8q79Gr2wRt3AeSFz4XvRSyokl5MAtSfgGO2ZCewdsDibLRVrDzIXTMxfqiLIGXPeMdY1gb/Fg8+tznJY50eSGmfB2DNrqciCD+tCRc4X5FNFJmIWnkhu3BL+t4qc8y75aySqIkvGOP9CRWKaGQ0ydUrsgUUVWXlfw4OpyAouVWQN66pITDPDqSJfQaZxuVVkxZhzzVgLTv5uHbDwXhN+vwGywklHPBQAAA=="} \ No newline at end of file +{"hash":13834844072603749544,"backend":"acvm-backend-barretenberg","abi":{"parameters":[{"name":"x","type":{"kind":"integer","sign":"unsigned","width":64},"visibility":"private"},{"name":"y","type":{"kind":"integer","sign":"unsigned","width":64},"visibility":"public"}],"param_witnesses":{"x":[{ "start": 1, "end": 2 }],"y":[{ "start": 2, "end": 3 }]},"return_type":{"kind":"integer","sign":"unsigned","width":64},"return_witnesses":[12]},"bytecode":"H4sIAAAAAAAA/+1WUW6DMAx1QksZoGr72jUcAiX8VbvJ0Oj9j7ChJpKbtXw0NpvUWkImUXixn53w3gDgHc6mfh7t/ZGMtR9TU96HeYuHtp36ZjLWfGIzjK7DthsPzjjTue6rcdZOrnX9MA49Dqa1kzl1gz3h2bL7sTDCMhmJbylmTDOT8WEhjXfjH/DcB8u8zwVygWifmL/9lTnWzSWKsxHA3QJf00vlveWvERJIUU4x0eb86aEJppljVox9oO+Py8QTV1Jnw6a85t7vSL8pwvN89j7gd88o8q79Gr2wRt3AeSFz4XvRSyokl5MAtSfgGO2ZCewdsDibLRVrDzIXTMxfqiLIGXPeMdY1gb/Fg8+tznJY50eSGmfB2DNrqciCD+tCRc4X5FNFJmIWnkhu3BL+t4qc8y75aySqIkvGOP9CRWKaGQ0ydUrsgUUVWXlfw4OpyAouVWQN66pITDPDqSJfQaZxuVVkxZhzzVgLTv5uHbDwXhN+vwGywklHPBQAAA=="} \ No newline at end of file diff --git a/tooling/noirc_abi/src/lib.rs b/tooling/noirc_abi/src/lib.rs index 9753dc21f14..7092f05c26e 100644 --- a/tooling/noirc_abi/src/lib.rs +++ b/tooling/noirc_abi/src/lib.rs @@ -3,8 +3,6 @@ #![warn(unreachable_pub)] #![warn(clippy::semicolon_if_nothing_returned)] -use std::{collections::BTreeMap, str}; - use acvm::{ acir::native_types::{Witness, WitnessMap}, FieldElement, @@ -16,6 +14,8 @@ use noirc_frontend::{ hir::Context, Signedness, StructType, Type, TypeBinding, TypeVariableKind, Visibility, }; use serde::{Deserialize, Serialize}; +use std::ops::Range; +use std::{collections::BTreeMap, str}; // This is the ABI used to bridge the different TOML formats for the initial // witness, the partial witness generator and the interpreter. // @@ -218,7 +218,7 @@ pub struct Abi { pub parameters: Vec, /// A map from the ABI's parameters to the indices they are written to in the [`WitnessMap`]. /// This defines how to convert between the [`InputMap`] and [`WitnessMap`]. - pub param_witnesses: BTreeMap>, + pub param_witnesses: BTreeMap>>, pub return_type: Option, pub return_witnesses: Vec, } @@ -315,13 +315,14 @@ impl Abi { let mut witness_map: BTreeMap = encoded_input_map .iter() .flat_map(|(param_name, encoded_param_fields)| { - let param_witness_indices = &self.param_witnesses[param_name]; + let param_witness_indices = range_to_vec(&self.param_witnesses[param_name]); param_witness_indices .iter() .zip(encoded_param_fields.iter()) .map(|(&witness, &field_element)| (witness, field_element)) + .collect::>() }) - .collect(); + .collect::>(); // When encoding public inputs to be passed to the verifier, the user can must provide a return value // to be inserted into the witness map. This is not needed when generating a witness when proving the circuit. @@ -398,7 +399,7 @@ impl Abi { let public_inputs_map = try_btree_map(self.parameters.clone(), |AbiParameter { name, typ, .. }| { let param_witness_values = - try_vecmap(self.param_witnesses[&name].clone(), |witness_index| { + try_vecmap(range_to_vec(&self.param_witnesses[&name]), |witness_index| { witness_map .get(&witness_index) .ok_or_else(|| AbiError::MissingParamWitnessValue { @@ -529,6 +530,16 @@ impl ContractEvent { } } +fn range_to_vec(ranges: &[Range]) -> Vec { + let mut result = Vec::new(); + for range in ranges { + for witness in range.start.witness_index()..range.end.witness_index() { + result.push(witness.into()); + } + } + result +} + #[cfg(test)] mod test { use std::collections::BTreeMap; @@ -554,8 +565,8 @@ mod test { ], // Note that the return value shares a witness with `thing2` param_witnesses: BTreeMap::from([ - ("thing1".to_string(), vec![Witness(1), Witness(2)]), - ("thing2".to_string(), vec![Witness(3)]), + ("thing1".to_string(), vec![(Witness(1)..Witness(3))]), + ("thing2".to_string(), vec![(Witness(3)..Witness(4))]), ]), return_type: Some(AbiType::Field), return_witnesses: vec![Witness(3)], diff --git a/tooling/noirc_abi_wasm/src/lib.rs b/tooling/noirc_abi_wasm/src/lib.rs index ea03aa8abe7..734ca1ff40e 100644 --- a/tooling/noirc_abi_wasm/src/lib.rs +++ b/tooling/noirc_abi_wasm/src/lib.rs @@ -57,7 +57,7 @@ export type AbiType = { kind: "array", length: number, type: AbiType } | { kind: "tuple", fields: AbiType[] } | { kind: "struct", path: string, fields: [string, AbiType][] }; - + export type AbiParameter = { name: string, type: AbiType, @@ -66,7 +66,7 @@ export type AbiParameter = { export type Abi = { parameters: AbiParameter[], - param_witnesses: Record, + param_witnesses: Record, return_type: AbiType | null, return_witnesses: number[], } diff --git a/tooling/noirc_abi_wasm/test/shared/abi_encode.ts b/tooling/noirc_abi_wasm/test/shared/abi_encode.ts index 28379745dec..cb80c6710ba 100644 --- a/tooling/noirc_abi_wasm/test/shared/abi_encode.ts +++ b/tooling/noirc_abi_wasm/test/shared/abi_encode.ts @@ -9,7 +9,7 @@ export const abi: Abi = { visibility: 'private', }, ], - param_witnesses: { foo: [1], bar: [2, 3] }, + param_witnesses: { foo: [{ start: 1, end: 2 }], bar: [{ start: 2, end: 4 }] }, return_type: null, return_witnesses: [], }; diff --git a/tooling/noirc_abi_wasm/test/shared/array_as_field.ts b/tooling/noirc_abi_wasm/test/shared/array_as_field.ts index ba58f075702..0cc0035fa68 100644 --- a/tooling/noirc_abi_wasm/test/shared/array_as_field.ts +++ b/tooling/noirc_abi_wasm/test/shared/array_as_field.ts @@ -8,7 +8,7 @@ export const abi: Abi = { visibility: 'private', }, ], - param_witnesses: { foo: [1, 2] }, + param_witnesses: { foo: [{ start: 1, end: 3 }] }, return_type: null, return_witnesses: [], }; diff --git a/tooling/noirc_abi_wasm/test/shared/field_as_array.ts b/tooling/noirc_abi_wasm/test/shared/field_as_array.ts index 931720d5e1b..6ae709459de 100644 --- a/tooling/noirc_abi_wasm/test/shared/field_as_array.ts +++ b/tooling/noirc_abi_wasm/test/shared/field_as_array.ts @@ -8,7 +8,7 @@ export const abi: Abi = { visibility: 'private', }, ], - param_witnesses: { foo: [1, 2] }, + param_witnesses: { foo: [{ start: 1, end: 3 }] }, return_type: null, return_witnesses: [], }; diff --git a/tooling/noirc_abi_wasm/test/shared/uint_overflow.ts b/tooling/noirc_abi_wasm/test/shared/uint_overflow.ts index ee87e050b23..c6e066e2bcd 100644 --- a/tooling/noirc_abi_wasm/test/shared/uint_overflow.ts +++ b/tooling/noirc_abi_wasm/test/shared/uint_overflow.ts @@ -8,7 +8,7 @@ export const abi: Abi = { visibility: 'private', }, ], - param_witnesses: { foo: [1] }, + param_witnesses: { foo: [{ start: 1, end: 2 }] }, return_type: null, return_witnesses: [], };