diff --git a/.aztec-sync-commit b/.aztec-sync-commit index 18e53f2bd3b..d05b164ef59 100644 --- a/.aztec-sync-commit +++ b/.aztec-sync-commit @@ -1 +1 @@ -e44ef7042c87d3c78a14413ad7d54e4ed642ad89 +deb69180f3a3670c7a512d4c401c3ad8b5da0e17 diff --git a/acvm-repo/blackbox_solver/src/bigint.rs b/acvm-repo/blackbox_solver/src/bigint.rs index 5b19f03a238..b8bc9dc0d70 100644 --- a/acvm-repo/blackbox_solver/src/bigint.rs +++ b/acvm-repo/blackbox_solver/src/bigint.rs @@ -18,7 +18,7 @@ pub struct BigIntSolver { } impl BigIntSolver { - pub(crate) fn get_bigint( + pub fn get_bigint( &self, id: u32, func: BlackBoxFunc, @@ -32,7 +32,7 @@ impl BigIntSolver { .cloned() } - pub(crate) fn get_modulus( + pub fn get_modulus( &self, id: u32, func: BlackBoxFunc, diff --git a/acvm-repo/brillig_vm/src/black_box.rs b/acvm-repo/brillig_vm/src/black_box.rs index 53599f79bc7..d37258036fc 100644 --- a/acvm-repo/brillig_vm/src/black_box.rs +++ b/acvm-repo/brillig_vm/src/black_box.rs @@ -421,6 +421,14 @@ impl BrilligBigintSolver { rhs: u32, func: BlackBoxFunc, ) -> Result { + let modulus_lhs = self.bigint_solver.get_modulus(lhs, func)?; + let modulus_rhs = self.bigint_solver.get_modulus(rhs, func)?; + if modulus_lhs != modulus_rhs { + return Err(BlackBoxResolutionError::Failed( + func, + "moduli should be identical in BigInt operation".to_string(), + )); + } let id = self.create_bigint_id(); self.bigint_solver.bigint_op(lhs, rhs, id, func)?; Ok(id) diff --git a/aztec_macros/src/transforms/contract_interface.rs b/aztec_macros/src/transforms/contract_interface.rs index e79cee66407..56107de77c5 100644 --- a/aztec_macros/src/transforms/contract_interface.rs +++ b/aztec_macros/src/transforms/contract_interface.rs @@ -1,15 +1,18 @@ +use acvm::acir::AcirField; + use noirc_errors::Location; use noirc_frontend::ast::{Ident, NoirFunction, UnresolvedTypeData}; use noirc_frontend::{ graph::CrateId, - macros_api::{FileId, HirContext, HirExpression, HirLiteral, HirStatement}, + macros_api::{FieldElement, FileId, HirContext, HirExpression, HirLiteral, HirStatement}, parse_program, parser::SortedModule, Type, }; +use tiny_keccak::{Hasher, Keccak}; + use crate::utils::{ - constants::SELECTOR_PLACEHOLDER, errors::AztecMacroError, hir_utils::{collect_crate_structs, get_contract_module_data, signature_of_type}, }; @@ -64,11 +67,6 @@ pub fn stub_function(aztec_visibility: &str, func: &NoirFunction, is_static_call .join(", "); let fn_return_type: noirc_frontend::ast::UnresolvedType = func.return_type(); - let fn_selector = format!( - "dep::aztec::protocol_types::abis::function_selector::FunctionSelector::from_signature(\"{}\")", - SELECTOR_PLACEHOLDER - ); - let parameters = func.parameters(); let is_void = if matches!(fn_return_type.typ, UnresolvedTypeData::Unit) { "Void" } else { "" }; let is_static = if is_static_call { "Static" } else { "" }; @@ -160,7 +158,7 @@ pub fn stub_function(aztec_visibility: &str, func: &NoirFunction, is_static_call let fn_body = format!( "{} - let selector = {}; + let selector = dep::aztec::protocol_types::abis::function_selector::FunctionSelector::from_field(0); dep::aztec::context::{}{}{}CallInterface {{ target_contract: self.target_contract, selector, @@ -172,7 +170,6 @@ pub fn stub_function(aztec_visibility: &str, func: &NoirFunction, is_static_call {} }}", args, - fn_selector, aztec_visibility, is_static, is_void, @@ -291,27 +288,34 @@ pub fn generate_contract_interface( Ok(()) } -fn compute_fn_signature(fn_name: &str, parameters: &[Type]) -> String { - format!( +fn compute_fn_signature_hash(fn_name: &str, parameters: &[Type]) -> u32 { + let signature = format!( "{}({})", fn_name, parameters.iter().map(signature_of_type).collect::>().join(",") - ) + ); + let mut keccak = Keccak::v256(); + let mut result = [0u8; 32]; + keccak.update(signature.as_bytes()); + keccak.finalize(&mut result); + // Take the first 4 bytes of the hash and convert them to an integer + // If you change the following value you have to change NUM_BYTES_PER_NOTE_TYPE_ID in l1_note_payload.ts as well + let num_bytes_per_note_type_id = 4; + u32::from_be_bytes(result[0..num_bytes_per_note_type_id].try_into().unwrap()) } // Updates the function signatures in the contract interface with the actual ones, replacing the placeholder. -// This is done by locating the contract interface struct, its functions (stubs) and assuming the last statement of each -// is the constructor for a CallInterface. This constructor has a selector field that holds a -// FunctionSelector::from_signature function that receives the signature as a string literal. +// This is done by locating the contract interface struct, its functions (stubs) and assuming the second to last statement of each +// is a let statement initializing the selector with a FunctionSelector::from_field call. pub fn update_fn_signatures_in_contract_interface( crate_id: &CrateId, context: &mut HirContext, ) -> Result<(), (AztecMacroError, FileId)> { - if let Some((name, _, file_id)) = get_contract_module_data(context, crate_id) { + if let Some((struct_name, _, file_id)) = get_contract_module_data(context, crate_id) { let maybe_interface_struct = collect_crate_structs(crate_id, context).iter().find_map(|struct_id| { let r#struct = context.def_interner.get_struct(*struct_id); - if r#struct.borrow().name.0.contents == name { + if r#struct.borrow().name.0.contents == struct_name { Some(r#struct) } else { None @@ -329,7 +333,7 @@ pub fn update_fn_signatures_in_contract_interface( continue; } - let fn_signature = compute_fn_signature( + let fn_signature_hash = compute_fn_signature_hash( name, &fn_parameters .iter() @@ -381,14 +385,12 @@ pub fn update_fn_signatures_in_contract_interface( context.def_interner.expression(¤t_fn_signature_expression_id); match current_fn_signature_expression { - HirExpression::Literal(HirLiteral::Str(signature)) => { - if signature != SELECTOR_PLACEHOLDER { + HirExpression::Literal(HirLiteral::Integer(value, _)) => { + if !value.is_zero() { Err(( AztecMacroError::CouldNotGenerateContractInterface { - secondary_message: Some(format!( - "Function signature argument must be a placeholder: {}", - SELECTOR_PLACEHOLDER - )), + secondary_message: Some( + "Function signature argument must be a placeholder with value 0".to_string()), }, file_id, )) @@ -397,20 +399,25 @@ pub fn update_fn_signatures_in_contract_interface( } } _ => Err(( - AztecMacroError::CouldNotAssignStorageSlots { + AztecMacroError::CouldNotGenerateContractInterface { secondary_message: Some( - "Function signature argument must be a literal string".to_string(), + "Function signature argument must be a literal field element" + .to_string(), ), }, file_id, )), }?; - context - .def_interner - .update_expression(current_fn_signature_expression_id, |expr| { - *expr = HirExpression::Literal(HirLiteral::Str(fn_signature)) - }); + context.def_interner.update_expression( + current_fn_signature_expression_id, + |expr| { + *expr = HirExpression::Literal(HirLiteral::Integer( + FieldElement::from(fn_signature_hash as u128), + false, + )) + }, + ); } } } diff --git a/aztec_macros/src/transforms/note_interface.rs b/aztec_macros/src/transforms/note_interface.rs index 49525fc2ae1..1369e6515b7 100644 --- a/aztec_macros/src/transforms/note_interface.rs +++ b/aztec_macros/src/transforms/note_interface.rs @@ -100,7 +100,9 @@ pub fn generate_note_interface_impl(module: &mut SortedModule) -> Result<(), Azt }) .collect::, _>>()?; let [note_serialized_len, note_bytes_len]: [_; 2] = - note_interface_generics.try_into().unwrap(); + note_interface_generics.try_into().expect( + "NoteInterface must be generic over 2 types, NOTE_FIELDS_LEN and NOTE_BYTES_LEN", + ); // Automatically inject the header field if it's not present let (header_field_name, _) = if let Some(existing_header) = diff --git a/aztec_macros/src/utils/constants.rs b/aztec_macros/src/utils/constants.rs index 2178f7a2526..3e93b2aa545 100644 --- a/aztec_macros/src/utils/constants.rs +++ b/aztec_macros/src/utils/constants.rs @@ -1,3 +1,2 @@ pub const FUNCTION_TREE_HEIGHT: u32 = 5; pub const MAX_CONTRACT_PRIVATE_FUNCTIONS: usize = 2_usize.pow(FUNCTION_TREE_HEIGHT); -pub const SELECTOR_PLACEHOLDER: &str = "SELECTOR_PLACEHOLDER"; diff --git a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_black_box.rs b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_black_box.rs index aa9cb8cd7a3..8e2b2fb7a29 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_black_box.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_black_box.rs @@ -6,7 +6,7 @@ use acvm::{ use crate::brillig::brillig_ir::{ brillig_variable::{BrilligVariable, BrilligVector, SingleAddrVariable}, debug_show::DebugToString, - BrilligBinaryOp, BrilligContext, + BrilligContext, }; /// Transforms SSA's black box function calls into the corresponding brillig instructions @@ -239,11 +239,10 @@ pub(crate) fn convert_black_box_call( BlackBoxFunc::RecursiveAggregation => {} BlackBoxFunc::BigIntAdd => { if let ( - [BrilligVariable::SingleAddr(lhs), BrilligVariable::SingleAddr(lhs_modulus), BrilligVariable::SingleAddr(rhs), BrilligVariable::SingleAddr(rhs_modulus)], - [BrilligVariable::SingleAddr(output), BrilligVariable::SingleAddr(modulus_id)], + [BrilligVariable::SingleAddr(lhs), BrilligVariable::SingleAddr(_lhs_modulus), BrilligVariable::SingleAddr(rhs), BrilligVariable::SingleAddr(_rhs_modulus)], + [BrilligVariable::SingleAddr(output), BrilligVariable::SingleAddr(_modulus_id)], ) = (function_arguments, function_results) { - prepare_bigint_output(brillig_context, lhs_modulus, rhs_modulus, modulus_id); brillig_context.black_box_op_instruction(BlackBoxOp::BigIntAdd { lhs: lhs.address, rhs: rhs.address, @@ -257,11 +256,10 @@ pub(crate) fn convert_black_box_call( } BlackBoxFunc::BigIntSub => { if let ( - [BrilligVariable::SingleAddr(lhs), BrilligVariable::SingleAddr(lhs_modulus), BrilligVariable::SingleAddr(rhs), BrilligVariable::SingleAddr(rhs_modulus)], - [BrilligVariable::SingleAddr(output), BrilligVariable::SingleAddr(modulus_id)], + [BrilligVariable::SingleAddr(lhs), BrilligVariable::SingleAddr(_lhs_modulus), BrilligVariable::SingleAddr(rhs), BrilligVariable::SingleAddr(_rhs_modulus)], + [BrilligVariable::SingleAddr(output), BrilligVariable::SingleAddr(_modulus_id)], ) = (function_arguments, function_results) { - prepare_bigint_output(brillig_context, lhs_modulus, rhs_modulus, modulus_id); brillig_context.black_box_op_instruction(BlackBoxOp::BigIntSub { lhs: lhs.address, rhs: rhs.address, @@ -275,11 +273,10 @@ pub(crate) fn convert_black_box_call( } BlackBoxFunc::BigIntMul => { if let ( - [BrilligVariable::SingleAddr(lhs), BrilligVariable::SingleAddr(lhs_modulus), BrilligVariable::SingleAddr(rhs), BrilligVariable::SingleAddr(rhs_modulus)], - [BrilligVariable::SingleAddr(output), BrilligVariable::SingleAddr(modulus_id)], + [BrilligVariable::SingleAddr(lhs), BrilligVariable::SingleAddr(_lhs_modulus), BrilligVariable::SingleAddr(rhs), BrilligVariable::SingleAddr(_rhs_modulus)], + [BrilligVariable::SingleAddr(output), BrilligVariable::SingleAddr(_modulus_id)], ) = (function_arguments, function_results) { - prepare_bigint_output(brillig_context, lhs_modulus, rhs_modulus, modulus_id); brillig_context.black_box_op_instruction(BlackBoxOp::BigIntMul { lhs: lhs.address, rhs: rhs.address, @@ -293,11 +290,10 @@ pub(crate) fn convert_black_box_call( } BlackBoxFunc::BigIntDiv => { if let ( - [BrilligVariable::SingleAddr(lhs), BrilligVariable::SingleAddr(lhs_modulus), BrilligVariable::SingleAddr(rhs), BrilligVariable::SingleAddr(rhs_modulus)], - [BrilligVariable::SingleAddr(output), BrilligVariable::SingleAddr(modulus_id)], + [BrilligVariable::SingleAddr(lhs), BrilligVariable::SingleAddr(_lhs_modulus), BrilligVariable::SingleAddr(rhs), BrilligVariable::SingleAddr(_rhs_modulus)], + [BrilligVariable::SingleAddr(output), BrilligVariable::SingleAddr(_modulus_id)], ) = (function_arguments, function_results) { - prepare_bigint_output(brillig_context, lhs_modulus, rhs_modulus, modulus_id); brillig_context.black_box_op_instruction(BlackBoxOp::BigIntDiv { lhs: lhs.address, rhs: rhs.address, @@ -416,27 +412,3 @@ fn convert_array_or_vector( ), } } - -fn prepare_bigint_output( - brillig_context: &mut BrilligContext, - lhs_modulus: &SingleAddrVariable, - rhs_modulus: &SingleAddrVariable, - modulus_id: &SingleAddrVariable, -) { - // Check moduli - let condition = brillig_context.allocate_register(); - let condition_adr = SingleAddrVariable { address: condition, bit_size: 1 }; - brillig_context.binary_instruction( - *lhs_modulus, - *rhs_modulus, - condition_adr, - BrilligBinaryOp::Equals, - ); - brillig_context.codegen_constrain( - condition_adr, - Some("moduli should be identical in BigInt operation".to_string()), - ); - brillig_context.deallocate_register(condition); - - brillig_context.mov_instruction(modulus_id.address, lhs_modulus.address); -} diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs index 585afc27919..1bdc9aaf4eb 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs @@ -975,6 +975,8 @@ impl<'a> Context<'a> { .into()) } }; + // Ensure that array id is fully resolved. + let array = dfg.resolve(array); let array_id = dfg.resolve(array); let array_typ = dfg.type_of_value(array_id); @@ -992,7 +994,6 @@ impl<'a> Context<'a> { // If we find one, we will use it when computing the index under the enable_side_effect predicate // If not, array_get(..) will use a fallback costing one multiplication in the worst case. // cf. https://github.com/noir-lang/noir/pull/4971 - // For simplicity we compute the offset only for simple arrays let is_simple_array = dfg.instruction_results(instruction).len() == 1 && can_omit_element_sizes_array(&array_typ); @@ -1123,13 +1124,14 @@ impl<'a> Context<'a> { /// It is a dummy value because in the case of a false predicate, the value stored at the requested index will be itself. fn convert_array_operation_inputs( &mut self, - array: ValueId, + array_id: ValueId, dfg: &DataFlowGraph, index: ValueId, store_value: Option, offset: usize, ) -> Result<(AcirVar, Option), RuntimeError> { - let (array_id, array_typ, block_id) = self.check_array_is_initialized(array, dfg)?; + let array_typ = dfg.type_of_value(array_id); + let block_id = self.ensure_array_is_initialized(array_id, dfg)?; let index_var = self.convert_numeric_value(index, dfg)?; let index_var = self.get_flattened_index(&array_typ, array_id, index_var, dfg)?; @@ -1248,22 +1250,22 @@ impl<'a> Context<'a> { dfg: &DataFlowGraph, mut index_side_effect: bool, ) -> Result { - let (array_id, _, block_id) = self.check_array_is_initialized(array, dfg)?; + let block_id = self.ensure_array_is_initialized(array, dfg)?; let results = dfg.instruction_results(instruction); let res_typ = dfg.type_of_value(results[0]); // Get operations to call-data parameters are replaced by a get to the call-data-bus array if let Some(call_data) = self.data_bus.call_data { - if self.data_bus.call_data_map.contains_key(&array_id) { + if self.data_bus.call_data_map.contains_key(&array) { // TODO: the block_id of call-data must be notified to the backend // TODO: should we do the same for return-data? let type_size = res_typ.flattened_size(); let type_size = self.acir_context.add_constant(FieldElement::from(type_size as i128)); let offset = self.acir_context.mul_var(var_index, type_size)?; - let bus_index = self.acir_context.add_constant(FieldElement::from( - self.data_bus.call_data_map[&array_id] as i128, - )); + let bus_index = self + .acir_context + .add_constant(FieldElement::from(self.data_bus.call_data_map[&array] as i128)); let new_index = self.acir_context.add_var(offset, bus_index)?; return self.array_get(instruction, call_data, new_index, dfg, index_side_effect); } @@ -1277,8 +1279,7 @@ impl<'a> Context<'a> { let mut value = self.array_get_value(&res_typ, block_id, &mut var_index)?; if let AcirValue::Var(value_var, typ) = &value { - let array_id = dfg.resolve(array_id); - let array_typ = dfg.type_of_value(array_id); + let array_typ = dfg.type_of_value(array); if let (Type::Numeric(numeric_type), AcirType::NumericType(num)) = (array_typ.first(), typ) { @@ -1362,7 +1363,7 @@ impl<'a> Context<'a> { } }; - let (array_id, array_typ, block_id) = self.check_array_is_initialized(array, dfg)?; + let block_id = self.ensure_array_is_initialized(array, dfg)?; // Every array has a length in its type, so we fetch that from // the SSA IR. @@ -1371,10 +1372,11 @@ impl<'a> Context<'a> { // However, this size is simply the capacity of a slice. The capacity is dependent upon the witness // and may contain data for which we want to restrict access. The true slice length is tracked in a // a separate SSA value and restrictions on slice indices should be generated elsewhere in the SSA. + let array_typ = dfg.type_of_value(array); let array_len = if !array_typ.contains_slice_element() { array_typ.flattened_size() } else { - self.flattened_slice_size(array_id, dfg) + self.flattened_slice_size(array, dfg) }; // Since array_set creates a new array, we create a new block ID for this @@ -1397,18 +1399,13 @@ impl<'a> Context<'a> { self.array_set_value(&store_value, result_block_id, &mut var_index)?; let element_type_sizes = if !can_omit_element_sizes_array(&array_typ) { - let acir_value = self.convert_value(array_id, dfg); - Some(self.init_element_type_sizes_array( - &array_typ, - array_id, - Some(&acir_value), - dfg, - )?) + let acir_value = self.convert_value(array, dfg); + Some(self.init_element_type_sizes_array(&array_typ, array, Some(&acir_value), dfg)?) } else { None }; - let value_types = self.convert_value(array_id, dfg).flat_numeric_types(); + let value_types = self.convert_value(array, 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"); @@ -1454,37 +1451,33 @@ impl<'a> Context<'a> { Ok(()) } - fn check_array_is_initialized( + fn ensure_array_is_initialized( &mut self, array: ValueId, dfg: &DataFlowGraph, - ) -> Result<(ValueId, Type, BlockId), RuntimeError> { - // Fetch the internal SSA ID for the array - let array_id = dfg.resolve(array); - - let array_typ = dfg.type_of_value(array_id); - + ) -> Result { // Use the SSA ID to get or create its block ID - let block_id = self.block_id(&array_id); + let block_id = self.block_id(&array); // Check if the array has already been initialized in ACIR gen // if not, we initialize it using the values from SSA let already_initialized = self.initialized_arrays.contains(&block_id); if !already_initialized { - let value = &dfg[array_id]; + let value = &dfg[array]; match value { Value::Array { .. } | Value::Instruction { .. } => { - let value = self.convert_value(array_id, dfg); + let value = self.convert_value(array, dfg); + let array_typ = dfg.type_of_value(array); let len = if !array_typ.contains_slice_element() { array_typ.flattened_size() } else { - self.flattened_slice_size(array_id, dfg) + self.flattened_slice_size(array, dfg) }; self.initialize_array(block_id, len, Some(value))?; } _ => { return Err(InternalError::General { - message: format!("Array {array_id} should be initialized"), + message: format!("Array {array} should be initialized"), call_stack: self.acir_context.get_call_stack(), } .into()); @@ -1492,7 +1485,7 @@ impl<'a> Context<'a> { } } - Ok((array_id, array_typ, block_id)) + Ok(block_id) } fn init_element_type_sizes_array( @@ -1746,7 +1739,7 @@ impl<'a> Context<'a> { /// Converts an SSA terminator's return values into their ACIR representations fn get_num_return_witnesses( - &mut self, + &self, terminator: &TerminatorInstruction, dfg: &DataFlowGraph, ) -> usize { @@ -1800,7 +1793,7 @@ impl<'a> Context<'a> { has_constant_return |= self.acir_context.is_constant(&acir_var); if is_databus { // We do not return value for the data bus. - self.check_array_is_initialized( + self.ensure_array_is_initialized( self.data_bus.return_data.expect( "`is_databus == true` implies `data_bus.return_data` is `Some`", ), @@ -2167,8 +2160,9 @@ impl<'a> Context<'a> { Ok(vec![AcirValue::Var(self.acir_context.add_constant(len), AcirType::field())]) } Intrinsic::AsSlice => { - let (slice_contents, slice_typ, block_id) = - self.check_array_is_initialized(arguments[0], dfg)?; + let slice_contents = arguments[0]; + let slice_typ = dfg.type_of_value(slice_contents); + let block_id = self.ensure_array_is_initialized(slice_contents, dfg)?; assert!(!slice_typ.is_nested_slice(), "ICE: Nested slice used in ACIR generation"); let result_block_id = self.block_id(&result_ids[1]); @@ -2212,8 +2206,9 @@ impl<'a> Context<'a> { Intrinsic::SlicePushBack => { // arguments = [slice_length, slice_contents, ...elements_to_push] let slice_length = self.convert_value(arguments[0], dfg).into_var()?; - let (slice_contents, slice_typ, _) = - self.check_array_is_initialized(arguments[1], dfg)?; + let slice_contents = arguments[1]; + let slice_typ = dfg.type_of_value(slice_contents); + assert!(!slice_typ.is_nested_slice(), "ICE: Nested slice used in ACIR generation"); let slice = self.convert_value(slice_contents, dfg); @@ -2279,9 +2274,8 @@ impl<'a> Context<'a> { Intrinsic::SlicePushFront => { // arguments = [slice_length, slice_contents, ...elements_to_push] let slice_length = self.convert_value(arguments[0], dfg).into_var()?; - - let (slice_contents, slice_typ, _) = - self.check_array_is_initialized(arguments[1], dfg)?; + let slice_contents = arguments[1]; + let slice_typ = dfg.type_of_value(slice_contents); assert!(!slice_typ.is_nested_slice(), "ICE: Nested slice used in ACIR generation"); let slice: AcirValue = self.convert_value(slice_contents, dfg); @@ -2344,6 +2338,7 @@ impl<'a> Context<'a> { Intrinsic::SlicePopBack => { // arguments = [slice_length, slice_contents] let slice_length = self.convert_value(arguments[0], dfg).into_var()?; + let slice_contents = arguments[1]; let one = self.acir_context.add_constant(FieldElement::one()); let new_slice_length = self.acir_context.sub_var(slice_length, one)?; @@ -2352,8 +2347,8 @@ impl<'a> Context<'a> { // the elements stored at that index will no longer be able to be accessed. let mut var_index = new_slice_length; - let (slice_contents, slice_typ, block_id) = - self.check_array_is_initialized(arguments[1], dfg)?; + let slice_typ = dfg.type_of_value(slice_contents); + let block_id = self.ensure_array_is_initialized(slice_contents, dfg)?; assert!(!slice_typ.is_nested_slice(), "ICE: Nested slice used in ACIR generation"); let mut popped_elements = Vec::new(); @@ -2378,9 +2373,11 @@ impl<'a> Context<'a> { Intrinsic::SlicePopFront => { // arguments = [slice_length, slice_contents] let slice_length = self.convert_value(arguments[0], dfg).into_var()?; + let slice_contents = arguments[1]; + + let slice_typ = dfg.type_of_value(slice_contents); + let block_id = self.ensure_array_is_initialized(slice_contents, dfg)?; - let (slice_contents, slice_typ, block_id) = - self.check_array_is_initialized(arguments[1], dfg)?; assert!(!slice_typ.is_nested_slice(), "ICE: Nested slice used in ACIR generation"); let one = self.acir_context.add_constant(FieldElement::one()); @@ -2419,9 +2416,11 @@ impl<'a> Context<'a> { Intrinsic::SliceInsert => { // arguments = [slice_length, slice_contents, insert_index, ...elements_to_insert] let slice_length = self.convert_value(arguments[0], dfg).into_var()?; + let slice_contents = arguments[1]; + + let slice_typ = dfg.type_of_value(slice_contents); + let block_id = self.ensure_array_is_initialized(slice_contents, dfg)?; - let (slice_contents, slice_typ, block_id) = - self.check_array_is_initialized(arguments[1], dfg)?; assert!(!slice_typ.is_nested_slice(), "ICE: Nested slice used in ACIR generation"); let slice = self.convert_value(slice_contents, dfg); @@ -2558,9 +2557,11 @@ impl<'a> Context<'a> { Intrinsic::SliceRemove => { // arguments = [slice_length, slice_contents, remove_index] let slice_length = self.convert_value(arguments[0], dfg).into_var()?; + let slice_contents = arguments[1]; + + let slice_typ = dfg.type_of_value(slice_contents); + let block_id = self.ensure_array_is_initialized(slice_contents, dfg)?; - let (slice_contents, slice_typ, block_id) = - self.check_array_is_initialized(arguments[1], dfg)?; assert!(!slice_typ.is_nested_slice(), "ICE: Nested slice used in ACIR generation"); let slice = self.convert_value(slice_contents, dfg); diff --git a/compiler/noirc_frontend/src/elaborator/expressions.rs b/compiler/noirc_frontend/src/elaborator/expressions.rs index 5cda8787241..e8098723692 100644 --- a/compiler/noirc_frontend/src/elaborator/expressions.rs +++ b/compiler/noirc_frontend/src/elaborator/expressions.rs @@ -27,7 +27,7 @@ use crate::{ HirStatement, Ident, IndexExpression, Literal, MemberAccessExpression, MethodCallExpression, PrefixExpression, }, - node_interner::{DefinitionKind, ExprId, FuncId, ReferenceId, TraitMethodId}, + node_interner::{DefinitionKind, ExprId, FuncId, TraitMethodId}, token::Tokens, QuotedType, Shared, StructType, Type, }; @@ -429,9 +429,9 @@ impl<'context> Elaborator<'context> { struct_generics, }); - let referenced = ReferenceId::Struct(struct_type.borrow().id); - let reference = ReferenceId::Reference(Location::new(span, self.file), is_self_type); - self.interner.add_reference(referenced, reference); + let struct_id = struct_type.borrow().id; + let reference_location = Location::new(span, self.file); + self.interner.add_struct_reference(struct_id, reference_location, is_self_type); (expr, Type::Struct(struct_type, generics)) } @@ -485,11 +485,11 @@ impl<'context> Elaborator<'context> { } if let Some(expected_index) = expected_index { - let struct_id = struct_type.borrow().id; - let referenced = ReferenceId::StructMember(struct_id, expected_index); - let reference = - ReferenceId::Reference(Location::new(field_name.span(), self.file), false); - self.interner.add_reference(referenced, reference); + self.interner.add_struct_member_reference( + struct_type.borrow().id, + expected_index, + Location::new(field_name.span(), self.file), + ); } ret.push((field_name, resolved)); diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index e1d104c4971..7fa05c9612e 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -1471,13 +1471,11 @@ impl<'context> Elaborator<'context> { if let Some(trait_id) = trait_id { let trait_name = trait_impl.trait_path.last_segment(); - - let referenced = ReferenceId::Trait(trait_id); - let reference = ReferenceId::Reference( + self.interner.add_trait_reference( + trait_id, Location::new(trait_name.span(), trait_impl.file_id), trait_name.is_self_type_name(), ); - self.interner.add_reference(referenced, reference); } } } diff --git a/compiler/noirc_frontend/src/elaborator/patterns.rs b/compiler/noirc_frontend/src/elaborator/patterns.rs index 7c920230b9d..d9576c77666 100644 --- a/compiler/noirc_frontend/src/elaborator/patterns.rs +++ b/compiler/noirc_frontend/src/elaborator/patterns.rs @@ -15,9 +15,7 @@ use crate::{ stmt::HirPattern, }, macros_api::{HirExpression, Ident, Path, Pattern}, - node_interner::{ - DefinitionId, DefinitionKind, ExprId, FuncId, GlobalId, ReferenceId, TraitImplKind, - }, + node_interner::{DefinitionId, DefinitionKind, ExprId, FuncId, GlobalId, TraitImplKind}, Shared, StructType, Type, TypeBindings, }; @@ -204,14 +202,12 @@ impl<'context> Elaborator<'context> { let struct_id = struct_type.borrow().id; - let referenced = ReferenceId::Struct(struct_id); - let reference = ReferenceId::Reference(Location::new(name_span, self.file), is_self_type); - self.interner.add_reference(referenced, reference); + let reference_location = Location::new(name_span, self.file); + self.interner.add_struct_reference(struct_id, reference_location, is_self_type); for (field_index, field) in fields.iter().enumerate() { - let referenced = ReferenceId::StructMember(struct_id, field_index); - let reference = ReferenceId::Reference(Location::new(field.0.span(), self.file), false); - self.interner.add_reference(referenced, reference); + let reference_location = Location::new(field.0.span(), self.file); + self.interner.add_struct_member_reference(struct_id, field_index, reference_location); } HirPattern::Struct(expected_type, fields, location) @@ -494,7 +490,6 @@ impl<'context> Elaborator<'context> { // This lookup allows support of such statements: let x = foo::bar::SOME_GLOBAL + 10; // If the expression is a singular indent, we search the resolver's current scope as normal. let span = path.span(); - let is_self_type_name = path.last_segment().is_self_type_name(); let (hir_ident, var_scope_index) = self.get_ident_from_path(path); if hir_ident.id != DefinitionId::dummy_id() { @@ -504,10 +499,7 @@ impl<'context> Elaborator<'context> { self.interner.add_function_dependency(current_item, func_id); } - let variable = - ReferenceId::Reference(hir_ident.location, is_self_type_name); - let function = ReferenceId::Function(func_id); - self.interner.add_reference(function, variable); + self.interner.add_function_reference(func_id, hir_ident.location); } DefinitionKind::Global(global_id) => { if let Some(global) = self.unresolved_globals.remove(&global_id) { @@ -517,10 +509,7 @@ impl<'context> Elaborator<'context> { self.interner.add_global_dependency(current_item, global_id); } - let variable = - ReferenceId::Reference(hir_ident.location, is_self_type_name); - let global = ReferenceId::Global(global_id); - self.interner.add_reference(global, variable); + self.interner.add_global_reference(global_id, hir_ident.location); } DefinitionKind::GenericType(_) => { // Initialize numeric generics to a polymorphic integer type in case @@ -536,10 +525,8 @@ impl<'context> Elaborator<'context> { // only local variables can be captured by closures. self.resolve_local_variable(hir_ident.clone(), var_scope_index); - let referenced = ReferenceId::Local(hir_ident.id); - let reference = - ReferenceId::Reference(Location::new(span, self.file), false); - self.interner.add_reference(referenced, reference); + let reference_location = Location::new(span, self.file); + self.interner.add_local_reference(hir_ident.id, reference_location); } } } diff --git a/compiler/noirc_frontend/src/elaborator/scope.rs b/compiler/noirc_frontend/src/elaborator/scope.rs index b7016280453..af6fc0e5d5e 100644 --- a/compiler/noirc_frontend/src/elaborator/scope.rs +++ b/compiler/noirc_frontend/src/elaborator/scope.rs @@ -53,11 +53,11 @@ impl<'context> Elaborator<'context> { resolver.resolve(self.def_maps, path.clone(), &mut Some(&mut references))?; for (referenced, ident) in references.iter().zip(path.segments) { - let reference = ReferenceId::Reference( + self.interner.add_reference( + *referenced, Location::new(ident.span(), self.file), ident.is_self_type_name(), ); - self.interner.add_reference(*referenced, reference); } } else { path_resolution = resolver.resolve(self.def_maps, path, &mut None)?; diff --git a/compiler/noirc_frontend/src/elaborator/statements.rs b/compiler/noirc_frontend/src/elaborator/statements.rs index 13c59e3494e..ba3dcccca99 100644 --- a/compiler/noirc_frontend/src/elaborator/statements.rs +++ b/compiler/noirc_frontend/src/elaborator/statements.rs @@ -15,7 +15,7 @@ use crate::{ macros_api::{ ForLoopStatement, ForRange, HirStatement, LetStatement, Path, Statement, StatementKind, }, - node_interner::{DefinitionId, DefinitionKind, GlobalId, ReferenceId, StmtId}, + node_interner::{DefinitionId, DefinitionKind, GlobalId, StmtId}, Type, }; @@ -255,9 +255,8 @@ impl<'context> Elaborator<'context> { typ.follow_bindings() }; - let referenced = ReferenceId::Local(ident.id); - let reference = ReferenceId::Reference(Location::new(span, self.file), false); - self.interner.add_reference(referenced, reference); + let reference_location = Location::new(span, self.file); + self.interner.add_local_reference(ident.id, reference_location); (HirLValue::Ident(ident.clone(), typ.clone()), typ, mutable) } @@ -380,9 +379,8 @@ impl<'context> Elaborator<'context> { Type::Struct(s, args) => { let s = s.borrow(); if let Some((field, index)) = s.get_field(field_name, args) { - let referenced = ReferenceId::StructMember(s.id, index); - let reference = ReferenceId::Reference(Location::new(span, self.file), false); - self.interner.add_reference(referenced, reference); + let reference_location = Location::new(span, self.file); + self.interner.add_struct_member_reference(s.id, index, reference_location); return Some((field, index)); } diff --git a/compiler/noirc_frontend/src/elaborator/types.rs b/compiler/noirc_frontend/src/elaborator/types.rs index 4e9b3620760..9134fc851b9 100644 --- a/compiler/noirc_frontend/src/elaborator/types.rs +++ b/compiler/noirc_frontend/src/elaborator/types.rs @@ -31,8 +31,7 @@ use crate::{ UnaryOp, UnresolvedType, UnresolvedTypeData, }, node_interner::{ - DefinitionKind, DependencyId, ExprId, GlobalId, ReferenceId, TraitId, TraitImplKind, - TraitMethodId, + DefinitionKind, DependencyId, ExprId, GlobalId, TraitId, TraitImplKind, TraitMethodId, }, Generics, Kind, ResolvedGeneric, Type, TypeBinding, TypeVariable, TypeVariableKind, }; @@ -154,30 +153,23 @@ impl<'context> Elaborator<'context> { }; if let Some(unresolved_span) = typ.span { + let location = Location::new(unresolved_span, self.file); + match resolved_type { Type::Struct(ref struct_type, _) => { // Record the location of the type reference - self.interner.push_type_ref_location( - resolved_type.clone(), - Location::new(unresolved_span, self.file), - ); + self.interner.push_type_ref_location(resolved_type.clone(), location); if !is_synthetic { - let referenced = ReferenceId::Struct(struct_type.borrow().id); - let reference = ReferenceId::Reference( - Location::new(unresolved_span, self.file), + self.interner.add_struct_reference( + struct_type.borrow().id, + location, is_self_type_name, ); - self.interner.add_reference(referenced, reference); } } Type::Alias(ref alias_type, _) => { - let referenced = ReferenceId::Alias(alias_type.borrow().id); - let reference = ReferenceId::Reference( - Location::new(unresolved_span, self.file), - is_self_type_name, - ); - self.interner.add_reference(referenced, reference); + self.interner.add_alias_reference(alias_type.borrow().id, location); } _ => (), } @@ -369,10 +361,8 @@ impl<'context> Elaborator<'context> { self.interner.add_global_dependency(current_item, id); } - let referenced = ReferenceId::Global(id); - let reference = - ReferenceId::Reference(Location::new(path.span(), self.file), false); - self.interner.add_reference(referenced, reference); + let reference_location = Location::new(path.span(), self.file); + self.interner.add_global_reference(id, reference_location); Some(Type::Constant(self.eval_global_as_array_length(id, path))) } diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs index b474ccff0cc..ba93eb87ef8 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -359,9 +359,11 @@ impl DefCollector { let file_id = current_def_map.file_id(module_id); for (referenced, ident) in references.iter().zip(&collected_import.path.segments) { - let reference = - ReferenceId::Reference(Location::new(ident.span(), file_id), false); - context.def_interner.add_reference(*referenced, reference); + context.def_interner.add_reference( + *referenced, + Location::new(ident.span(), file_id), + false, + ); } resolved_import @@ -412,17 +414,13 @@ impl DefCollector { } } - let handle_missing_file = |err| { - errors.push((CompilationError::DebugComptimeScopeNotFound(err), root_file_id)); - None - }; - let debug_comptime_in_file: Option = - debug_comptime_in_file.and_then(|debug_comptime_in_file| { - context - .file_manager - .find_by_path_suffix(debug_comptime_in_file) - .unwrap_or_else(handle_missing_file) - }); + let debug_comptime_in_file = debug_comptime_in_file.and_then(|debug_comptime_in_file| { + let file = context.file_manager.find_by_path_suffix(debug_comptime_in_file); + file.unwrap_or_else(|error| { + errors.push((CompilationError::DebugComptimeScopeNotFound(error), root_file_id)); + None + }) + }); if !use_legacy { let mut more_errors = Elaborator::elaborate( @@ -432,6 +430,14 @@ impl DefCollector { debug_comptime_in_file, ); errors.append(&mut more_errors); + + for macro_processor in macro_processors { + macro_processor.process_typed_ast(&crate_id, context).unwrap_or_else( + |(macro_err, file_id)| { + errors.push((macro_err.into(), file_id)); + }, + ); + } return errors; } @@ -545,18 +551,28 @@ fn add_import_reference( return; } - let referenced = match def_id { - crate::macros_api::ModuleDefId::ModuleId(module_id) => ReferenceId::Module(module_id), - crate::macros_api::ModuleDefId::FunctionId(func_id) => ReferenceId::Function(func_id), - crate::macros_api::ModuleDefId::TypeId(struct_id) => ReferenceId::Struct(struct_id), - crate::macros_api::ModuleDefId::TraitId(trait_id) => ReferenceId::Trait(trait_id), + let location = Location::new(name.span(), file_id); + + match def_id { + crate::macros_api::ModuleDefId::ModuleId(module_id) => { + interner.add_module_reference(module_id, location); + } + crate::macros_api::ModuleDefId::FunctionId(func_id) => { + interner.add_function_reference(func_id, location); + } + crate::macros_api::ModuleDefId::TypeId(struct_id) => { + interner.add_struct_reference(struct_id, location, false); + } + crate::macros_api::ModuleDefId::TraitId(trait_id) => { + interner.add_trait_reference(trait_id, location, false); + } crate::macros_api::ModuleDefId::TypeAliasId(type_alias_id) => { - ReferenceId::Alias(type_alias_id) + interner.add_alias_reference(type_alias_id, location); + } + crate::macros_api::ModuleDefId::GlobalId(global_id) => { + interner.add_global_reference(global_id, location); } - crate::macros_api::ModuleDefId::GlobalId(global_id) => ReferenceId::Global(global_id), }; - let reference = ReferenceId::Reference(Location::new(name.span(), file_id), false); - interner.add_reference(referenced, reference); } fn inject_prelude( diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs index 48985116f4f..bcd24ca8ed3 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -649,9 +649,7 @@ impl<'a> ModCollector<'a> { ) { Ok(child_mod_id) => { // Track that the "foo" in `mod foo;` points to the module "foo" - let referenced = ReferenceId::Module(child_mod_id); - let reference = ReferenceId::Reference(location, false); - context.def_interner.add_reference(referenced, reference); + context.def_interner.add_module_reference(child_mod_id, location); errors.extend(collect_defs( self.def_collector, diff --git a/compiler/noirc_frontend/src/locations.rs b/compiler/noirc_frontend/src/locations.rs index 0efe385aa0a..5ca8c1493eb 100644 --- a/compiler/noirc_frontend/src/locations.rs +++ b/compiler/noirc_frontend/src/locations.rs @@ -3,7 +3,11 @@ use noirc_errors::Location; use rangemap::RangeMap; use rustc_hash::FxHashMap; -use crate::{macros_api::NodeInterner, node_interner::ReferenceId}; +use crate::{ + hir::def_map::ModuleId, + macros_api::{NodeInterner, StructId}, + node_interner::{DefinitionId, FuncId, GlobalId, ReferenceId, TraitId, TypeAliasId}, +}; use petgraph::prelude::NodeIndex as PetGraphIndex; #[derive(Debug, Default)] @@ -58,11 +62,65 @@ impl NodeInterner { } } - pub(crate) fn add_reference(&mut self, referenced: ReferenceId, reference: ReferenceId) { + pub(crate) fn add_module_reference(&mut self, id: ModuleId, location: Location) { + self.add_reference(ReferenceId::Module(id), location, false); + } + + pub(crate) fn add_struct_reference( + &mut self, + id: StructId, + location: Location, + is_self_type: bool, + ) { + self.add_reference(ReferenceId::Struct(id), location, is_self_type); + } + + pub(crate) fn add_struct_member_reference( + &mut self, + id: StructId, + member_index: usize, + location: Location, + ) { + self.add_reference(ReferenceId::StructMember(id, member_index), location, false); + } + + pub(crate) fn add_trait_reference( + &mut self, + id: TraitId, + location: Location, + is_self_type: bool, + ) { + self.add_reference(ReferenceId::Trait(id), location, is_self_type); + } + + pub(crate) fn add_alias_reference(&mut self, id: TypeAliasId, location: Location) { + self.add_reference(ReferenceId::Alias(id), location, false); + } + + pub(crate) fn add_function_reference(&mut self, id: FuncId, location: Location) { + self.add_reference(ReferenceId::Function(id), location, false); + } + + pub(crate) fn add_global_reference(&mut self, id: GlobalId, location: Location) { + self.add_reference(ReferenceId::Global(id), location, false); + } + + pub(crate) fn add_local_reference(&mut self, id: DefinitionId, location: Location) { + self.add_reference(ReferenceId::Local(id), location, false); + } + + pub(crate) fn add_reference( + &mut self, + referenced: ReferenceId, + location: Location, + is_self_type: bool, + ) { if !self.track_references { return; } + let reference = ReferenceId::Reference(location, is_self_type); + let referenced_index = self.get_or_insert_reference(referenced); let reference_location = self.reference_location(reference); let reference_index = self.reference_graph.add_node(reference); @@ -120,14 +178,8 @@ impl NodeInterner { include_referenced: bool, include_self_type_name: bool, ) -> Option> { - let node_index = self.location_indices.get_node_from_location(location)?; - - let reference_node = self.reference_graph[node_index]; - let referenced_node_index = if let ReferenceId::Reference(_, _) = reference_node { - self.referenced_index(node_index)? - } else { - node_index - }; + let referenced_node = self.find_referenced(location)?; + let referenced_node_index = self.reference_graph_indices[&referenced_node]; let found_locations = self.find_all_references_for_index( referenced_node_index, @@ -138,6 +190,19 @@ impl NodeInterner { Some(found_locations) } + // Returns the `ReferenceId` that is referenced by the given location, if any. + pub fn find_referenced(&self, location: Location) -> Option { + let node_index = self.location_indices.get_node_from_location(location)?; + + let reference_node = self.reference_graph[node_index]; + if let ReferenceId::Reference(_, _) = reference_node { + let node_index = self.referenced_index(node_index)?; + Some(self.reference_graph[node_index]) + } else { + Some(reference_node) + } + } + // Given a referenced node index, find all references to it and return their locations, optionally together // with the reference node's location if `include_referenced` is true. // If `include_self_type_name` is true, references where "Self" is written are returned, diff --git a/docs/docs/explainers/explainer-oracle.md b/docs/docs/explainers/explainer-oracle.md index b84ca5dd986..821e1f95c04 100644 --- a/docs/docs/explainers/explainer-oracle.md +++ b/docs/docs/explainers/explainer-oracle.md @@ -31,7 +31,7 @@ In short, anything that can be constrained in a Noir program but needs to be fet Just like in The Matrix, Oracles are powerful. But with great power, comes great responsibility. Just because you're using them in a Noir program doesn't mean they're true. Noir has no superpowers. If you want to prove that Portugal won the Euro Cup 2016, you're still relying on potentially untrusted information. -To give a concrete example, Alice wants to login to the [NounsDAO](https://nouns.wtf/) forum with her username "noir_nouner" by proving she owns a noun without revealing her ethereum address. Her Noir program could have a oracle call like this: +To give a concrete example, Alice wants to login to the [NounsDAO](https://nouns.wtf/) forum with her username "noir_nouner" by proving she owns a noun without revealing her ethereum address. Her Noir program could have an oracle call like this: ```rust #[oracle(getNoun)] @@ -52,6 +52,6 @@ If you don't constrain the return of your oracle, you could be clearly opening a On CLI, Nargo resolves oracles by making JSON RPC calls, which means it would require an RPC node to be running. -In JavaScript, NoirJS accepts and resolves arbitrary call handlers (that is, not limited to JSON) as long as they matches the expected types the developer defines. Refer to [Foreign Call Handler](../reference/NoirJS/noir_js/type-aliases/ForeignCallHandler.md) to learn more about NoirJS's call handling. +In JavaScript, NoirJS accepts and resolves arbitrary call handlers (that is, not limited to JSON) as long as they match the expected types the developer defines. Refer to [Foreign Call Handler](../reference/NoirJS/noir_js/type-aliases/ForeignCallHandler.md) to learn more about NoirJS's call handling. If you want to build using oracles, follow through to the [oracle guide](../how_to/how-to-oracles.md) for a simple example on how to do that. diff --git a/docs/docs/how_to/how-to-oracles.md b/docs/docs/how_to/how-to-oracles.md index df41276cfe1..2f69902062c 100644 --- a/docs/docs/how_to/how-to-oracles.md +++ b/docs/docs/how_to/how-to-oracles.md @@ -46,7 +46,7 @@ unconstrained fn get_sqrt(number: Field) -> Field { } ``` -In this example, we're wrapping our oracle function in a unconstrained method, and decorating it with `oracle(getSqrt)`. We can then call the unconstrained function as we would call any other function: +In this example, we're wrapping our oracle function in an unconstrained method, and decorating it with `oracle(getSqrt)`. We can then call the unconstrained function as we would call any other function: ```rust fn main(input: Field) { @@ -234,7 +234,7 @@ const client = new JSONRPCClient((jsonRPCRequest) => { // declaring a function that takes the name of the foreign call (getSqrt) and the inputs const foreignCallHandler = async (name, input) => { // notice that the "inputs" parameter contains *all* the inputs - // in this case we to make the RPC request with the first parameter "numbers", which would be input[0] + // in this case we make the RPC request with the first parameter "numbers", which would be input[0] const oracleReturn = await client.request(name, [ input[0].map((i) => i.toString("hex")), ]); diff --git a/docs/docs/how_to/how-to-recursion.md b/docs/docs/how_to/how-to-recursion.md index aac84e29fac..71f02fa5435 100644 --- a/docs/docs/how_to/how-to-recursion.md +++ b/docs/docs/how_to/how-to-recursion.md @@ -47,7 +47,7 @@ In a standard recursive app, you're also dealing with at least two circuits. For - `main`: a circuit of type `assert(x != y)`, where `main` is marked with a `#[recursive]` attribute. This attribute states that the backend should generate proofs that are friendly for verification within another circuit. - `recursive`: a circuit that verifies `main` -For a full example on how recursive proofs work, please refer to the [noir-examples](https://github.com/noir-lang/noir-examples) repository. We will *not* be using it as a reference for this guide. +For a full example of how recursive proofs work, please refer to the [noir-examples](https://github.com/noir-lang/noir-examples) repository. We will *not* be using it as a reference for this guide. ## Step 1: Setup diff --git a/docs/docs/how_to/how-to-solidity-verifier.md b/docs/docs/how_to/how-to-solidity-verifier.md index e6ed9abaec6..c800d91ac69 100644 --- a/docs/docs/how_to/how-to-solidity-verifier.md +++ b/docs/docs/how_to/how-to-solidity-verifier.md @@ -40,7 +40,7 @@ Generating a Solidity Verifier contract is actually a one-command process. Howev ## Step 1 - Generate a contract -This is by far the most straight-forward step. Just run: +This is by far the most straightforward step. Just run: ```sh nargo compile @@ -99,7 +99,7 @@ This time we will see a warning about an unused function parameter. This is expe ## Step 3 - Deploying -At this point we should have a compiled contract read to deploy. If we navigate to the deploy section in Remix, we will see many different environments we can deploy to. The steps to deploy on each environment would be out-of-scope for this guide, so we will just use the default Remix VM. +At this point we should have a compiled contract ready to deploy. If we navigate to the deploy section in Remix, we will see many different environments we can deploy to. The steps to deploy on each environment would be out-of-scope for this guide, so we will just use the default Remix VM. Looking closely, we will notice that our "Solidity Verifier" is actually three contracts working together: @@ -111,7 +111,7 @@ Remix will take care of the dependencies for us so we can simply deploy the Ultr ![Deploying UltraVerifier](@site/static/img/how-tos/solidity_verifier_5.png) -A contract will show up in the "Deployed Contracts" section, where we can retrieve the Verification Key Hash. This is particularly useful for double-checking the deployer contract is the correct one. +A contract will show up in the "Deployed Contracts" section, where we can retrieve the Verification Key Hash. This is particularly useful for double-checking that the deployer contract is the correct one. :::note diff --git a/noir_stdlib/src/embedded_curve_ops.nr b/noir_stdlib/src/embedded_curve_ops.nr index f54072d8cbd..f02d2702d6b 100644 --- a/noir_stdlib/src/embedded_curve_ops.nr +++ b/noir_stdlib/src/embedded_curve_ops.nr @@ -88,24 +88,25 @@ impl Eq for EmbeddedCurveScalar { // // The embedded curve being used is decided by the // underlying proof system. -#[foreign(multi_scalar_mul)] // docs:start:multi_scalar_mul pub fn multi_scalar_mul( points: [EmbeddedCurvePoint; N], scalars: [EmbeddedCurveScalar; N] -) -> [Field; 3] +) -> EmbeddedCurvePoint // docs:end:multi_scalar_mul -{} +{ + let point_array = multi_scalar_mul_array_return(points, scalars); + EmbeddedCurvePoint { x: point_array[0], y: point_array[1], is_infinite: point_array[2] as bool } +} + +#[foreign(multi_scalar_mul)] +fn multi_scalar_mul_array_return(points: [EmbeddedCurvePoint; N], scalars: [EmbeddedCurveScalar; N]) -> [Field; 3] {} // docs:start:fixed_base_scalar_mul -pub fn fixed_base_scalar_mul( - scalar_low: Field, - scalar_high: Field -) -> [Field; 3] +pub fn fixed_base_scalar_mul(scalar: EmbeddedCurveScalar) -> EmbeddedCurvePoint // docs:end:fixed_base_scalar_mul { let g1 = EmbeddedCurvePoint { x: 1, y: 17631683881184975370165255887551781615748388533673675138860, is_infinite: false }; - let scalar = EmbeddedCurveScalar { lo: scalar_low, hi: scalar_high }; multi_scalar_mul([g1], [scalar]) } diff --git a/noir_stdlib/src/hash/keccak.nr b/noir_stdlib/src/hash/keccak.nr new file mode 100644 index 00000000000..7623f5a2256 --- /dev/null +++ b/noir_stdlib/src/hash/keccak.nr @@ -0,0 +1,122 @@ +global LIMBS_PER_BLOCK = 17; //BLOCK_SIZE / 8; +global NUM_KECCAK_LANES = 25; +global BLOCK_SIZE = 136; //(1600 - BITS * 2) / WORD_SIZE; +global WORD_SIZE = 8; + +use crate::collections::bounded_vec::BoundedVec; + +#[foreign(keccakf1600)] +fn keccakf1600(input: [u64; 25]) -> [u64; 25] {} + +fn keccak256(input: [u8; N], message_size: u32) -> [u8; 32] { + // No var keccak for now + assert(N == message_size); + + //1. format_input_lanes + let max_blocks = (N + BLOCK_SIZE) / BLOCK_SIZE; + //maximum number of bytes to hash + let max_blocks_length = (BLOCK_SIZE * (max_blocks)); + assert(max_blocks_length < 1000); + let mut block_bytes :BoundedVec = BoundedVec::from_array(input); + for i in N..N + BLOCK_SIZE { + let data = if i == N { + 1 + } else if i == max_blocks_length - 1 { + 0x80 + } else { + 0 + }; + block_bytes.push(data); + } + + // keccak lanes interpret memory as little-endian integers, + // means we need to swap our byte ordering + let num_limbs = max_blocks * LIMBS_PER_BLOCK; //max_blocks_length / WORD_SIZE; + for i in 0..num_limbs { + let mut temp = [0; 8]; + + for j in 0..8 { + temp[j] = block_bytes.get(8*i+j); + } + for j in 0..8 { + block_bytes.set(8 * i + j, temp[7 - j]); + } + } + let byte_size = max_blocks_length; + assert(num_limbs < 1000); + let mut sliced_buffer = [0; 1000]; + // populate a vector of 64-bit limbs from our byte array + for i in 0..num_limbs { + let mut sliced = 0; + if (i * WORD_SIZE + WORD_SIZE > byte_size) { + let slice_size = byte_size - (i * WORD_SIZE); + let byte_shift = (WORD_SIZE - slice_size) * 8; + let mut v = 1; + for k in 0..slice_size { + sliced += v * (block_bytes.get(i * WORD_SIZE+7-k) as Field); + v *= 256; + } + let w = 1 << (byte_shift as u8); + sliced *= w as Field; + } else { + let mut v = 1; + for k in 0..WORD_SIZE { + sliced += v * (block_bytes.get(i * WORD_SIZE+7-k) as Field); + v *= 256; + } + } + sliced_buffer[i] = sliced as u64; + } + + //2. sponge_absorb + let num_blocks = max_blocks; + let mut state : [u64;NUM_KECCAK_LANES]= [0; NUM_KECCAK_LANES]; + + for i in 0..num_blocks { + if (i == 0) { + for j in 0..LIMBS_PER_BLOCK { + state[j] = sliced_buffer[j]; + } + } else { + for j in 0..LIMBS_PER_BLOCK { + state[j] = state[j] ^ sliced_buffer[i * LIMBS_PER_BLOCK + j]; + } + } + state = keccakf1600(state); + } + + //3. sponge_squeeze + let mut result = [0; 32]; + for i in 0..4 { + let lane = state[i] as Field; + let lane_le = lane.to_le_bytes(8); + for j in 0..8 { + result[8*i+j] = lane_le[j]; + } + } + result +} + +mod tests { + use crate::hash::keccak::keccak256; + + #[test] + fn smoke_test() { + let input = [0xbd]; + let result = [ + 0x5a, 0x50, 0x2f, 0x9f, 0xca, 0x46, 0x7b, 0x26, 0x6d, 0x5b, 0x78, 0x33, 0x65, 0x19, 0x37, 0xe8, 0x05, 0x27, 0x0c, 0xa3, 0xf3, 0xaf, 0x1c, 0x0d, 0xd2, 0x46, 0x2d, 0xca, 0x4b, 0x3b, 0x1a, 0xbf + ]; + assert_eq(keccak256(input, input.len()), result); + } + + #[test] + fn hash_hello_world() { + // "hello world" + let input = [72, 101, 108, 108, 111, 32, 119, 111, 114, 108, 100, 33]; + let result = [ + 0xec, 0xd0, 0xe1, 0x8, 0xa9, 0x8e, 0x19, 0x2a, 0xf1, 0xd2, 0xc2, 0x50, 0x55, 0xf4, 0xe3, 0xbe, 0xd7, 0x84, 0xb5, 0xc8, 0x77, 0x20, 0x4e, 0x73, 0x21, 0x9a, 0x52, 0x3, 0x25, 0x1f, 0xea, 0xab + ]; + assert_eq(keccak256(input, input.len()), result); + } +} + diff --git a/noir_stdlib/src/hash/mod.nr b/noir_stdlib/src/hash/mod.nr index 65f3b9419ff..9655862dd1b 100644 --- a/noir_stdlib/src/hash/mod.nr +++ b/noir_stdlib/src/hash/mod.nr @@ -1,6 +1,7 @@ mod poseidon; mod mimc; mod poseidon2; +mod keccak; use crate::default::Default; use crate::uint128::U128; @@ -43,8 +44,7 @@ fn pedersen_commitment_with_separator_noir(input: [Field; N], separa points[i] = EmbeddedCurveScalar::from_field(input[i]); } let generators = derive_generators("DEFAULT_DOMAIN_SEPARATOR".as_bytes(), separator); - let values = multi_scalar_mul(generators, points); - EmbeddedCurvePoint { x: values[0], y: values[1], is_infinite: values[2] as bool } + multi_scalar_mul(generators, points) } #[no_predicates] @@ -61,10 +61,7 @@ pub fn pedersen_hash(input: [Field; N]) -> Field } #[field(bn254)] -fn derive_generators( - domain_separator_bytes: [u8; M], - starting_index: u32 -) -> [EmbeddedCurvePoint; N] { +fn derive_generators(domain_separator_bytes: [u8; M], starting_index: u32) -> [EmbeddedCurvePoint; N] { crate::assert_constant(domain_separator_bytes); crate::assert_constant(starting_index); __derive_generators(domain_separator_bytes, starting_index) @@ -72,7 +69,10 @@ fn derive_generators( #[builtin(derive_pedersen_generators)] #[field(bn254)] -fn __derive_generators(domain_separator_bytes: [u8; M], starting_index: u32) -> [EmbeddedCurvePoint; N] {} +fn __derive_generators( + domain_separator_bytes: [u8; M], + starting_index: u32 +) -> [EmbeddedCurvePoint; N] {} fn pedersen_hash_with_separator_noir(input: [Field; N], separator: u32) -> Field { let v1 = pedersen_commitment_with_separator(input, separator); @@ -80,7 +80,7 @@ fn pedersen_hash_with_separator_noir(input: [Field; N], separator: u multi_scalar_mul( [length_generator[0], v1], [EmbeddedCurveScalar { lo: N as Field, hi: 0 }, EmbeddedCurveScalar { lo: 1, hi: 0 }] - )[0] + ).x } #[foreign(pedersen_hash)] diff --git a/test_programs/execution_success/embedded_curve_ops/src/main.nr b/test_programs/execution_success/embedded_curve_ops/src/main.nr index 4eeda39c6aa..5372f73df23 100644 --- a/test_programs/execution_success/embedded_curve_ops/src/main.nr +++ b/test_programs/execution_success/embedded_curve_ops/src/main.nr @@ -4,8 +4,8 @@ fn main(priv_key: Field, pub_x: pub Field, pub_y: pub Field) { let scalar = std::embedded_curve_ops::EmbeddedCurveScalar { lo: priv_key, hi: 0 }; // Test that multi_scalar_mul correctly derives the public key let res = std::embedded_curve_ops::multi_scalar_mul([g1], [scalar]); - assert(res[0] == pub_x); - assert(res[1] == pub_y); + assert(res.x == pub_x); + assert(res.y == pub_y); // Test that double function calling embedded_curve_add works as expected let pub_point = std::embedded_curve_ops::EmbeddedCurvePoint { x: pub_x, y: pub_y, is_infinite: false }; @@ -18,5 +18,5 @@ fn main(priv_key: Field, pub_x: pub Field, pub_y: pub Field) { let res = std::embedded_curve_ops::multi_scalar_mul([g1, g1], [scalar, scalar]); // The results should be double the g1 point because the scalars are 1 and we pass in g1 twice - assert(double.x == res[0]); + assert(double.x == res.x); } diff --git a/test_programs/execution_success/regression_5045/src/main.nr b/test_programs/execution_success/regression_5045/src/main.nr index cf39b2f97e4..d1bc4f663fd 100644 --- a/test_programs/execution_success/regression_5045/src/main.nr +++ b/test_programs/execution_success/regression_5045/src/main.nr @@ -15,6 +15,6 @@ fn main(is_active: bool) { [a, bad], [EmbeddedCurveScalar { lo: 1, hi: 0 }, EmbeddedCurveScalar { lo: 1, hi: 0 }] ); - assert(e[0] != d.x); + assert(e.x != d.x); } } diff --git a/test_programs/execution_success/schnorr/src/main.nr b/test_programs/execution_success/schnorr/src/main.nr index 5bc0ca9fefb..cf22fd371d1 100644 --- a/test_programs/execution_success/schnorr/src/main.nr +++ b/test_programs/execution_success/schnorr/src/main.nr @@ -50,7 +50,7 @@ pub fn verify_signature_noir(public_key: embedded_curve_ops::EmbeddedCurvePoi let g1 = embedded_curve_ops::EmbeddedCurvePoint { x: 1, y: 17631683881184975370165255887551781615748388533673675138860, is_infinite: false }; let r = embedded_curve_ops::multi_scalar_mul([g1, public_key], [sig_s, sig_e]); // compare the _hashes_ rather than field elements modulo r - let pedersen_hash = std::hash::pedersen_hash([r[0], public_key.x, public_key.y]); + let pedersen_hash = std::hash::pedersen_hash([r.x, public_key.x, public_key.y]); let mut hash_input = [0; M]; let pde = pedersen_hash.to_be_bytes(32); @@ -62,7 +62,7 @@ pub fn verify_signature_noir(public_key: embedded_curve_ops::EmbeddedCurvePoi } let result = std::hash::blake2s(hash_input); - is_ok = (r[2] == 0); + is_ok = !r.is_infinite; for i in 0..32 { if result[i] != signature[32 + i] { is_ok = false; @@ -101,7 +101,7 @@ pub fn assert_valid_signature(public_key: embedded_curve_ops::EmbeddedCurvePo let g1 = embedded_curve_ops::EmbeddedCurvePoint { x: 1, y: 17631683881184975370165255887551781615748388533673675138860, is_infinite: false }; let r = embedded_curve_ops::multi_scalar_mul([g1, public_key], [sig_s, sig_e]); // compare the _hashes_ rather than field elements modulo r - let pedersen_hash = std::hash::pedersen_hash([r[0], public_key.x, public_key.y]); + let pedersen_hash = std::hash::pedersen_hash([r.x, public_key.x, public_key.y]); let mut hash_input = [0; M]; let pde = pedersen_hash.to_be_bytes(32); @@ -113,7 +113,7 @@ pub fn assert_valid_signature(public_key: embedded_curve_ops::EmbeddedCurvePo } let result = std::hash::blake2s(hash_input); - assert(r[2] == 0); + assert(!r.is_infinite); for i in 0..32 { assert(result[i] == signature[32 + i]); } diff --git a/test_programs/execution_success/simple_shield/src/main.nr b/test_programs/execution_success/simple_shield/src/main.nr index d84288b9fd6..fd2fc20d08f 100644 --- a/test_programs/execution_success/simple_shield/src/main.nr +++ b/test_programs/execution_success/simple_shield/src/main.nr @@ -10,12 +10,11 @@ fn main( to_pubkey_x: Field, to_pubkey_y: Field ) -> pub [Field; 2] { + let priv_key_as_scalar = std::embedded_curve_ops::EmbeddedCurveScalar { lo: priv_key, hi: 0 }; // Compute public key from private key to show ownership - let pubkey = std::embedded_curve_ops::fixed_base_scalar_mul(priv_key, 0); - let pubkey_x = pubkey[0]; - let pubkey_y = pubkey[1]; + let pubkey = std::embedded_curve_ops::fixed_base_scalar_mul(priv_key_as_scalar); // Compute input note commitment - let note_commitment = std::hash::pedersen_commitment([pubkey_x, pubkey_y]); + let note_commitment = std::hash::pedersen_commitment([pubkey.x, pubkey.y]); // Compute input note nullifier let nullifier = std::hash::pedersen_commitment([note_commitment.x, index, priv_key]); // Compute output note nullifier diff --git a/test_programs/noir_test_success/embedded_curve_ops/src/main.nr b/test_programs/noir_test_success/embedded_curve_ops/src/main.nr index 225e86397fd..0c2c333fa62 100644 --- a/test_programs/noir_test_success/embedded_curve_ops/src/main.nr +++ b/test_programs/noir_test_success/embedded_curve_ops/src/main.nr @@ -10,28 +10,28 @@ use std::embedded_curve_ops::{EmbeddedCurvePoint, EmbeddedCurveScalar, multi_sca let s1 = EmbeddedCurveScalar { lo: 1, hi: 0 }; let a = multi_scalar_mul([g1], [s1]); - assert(a[2] == 0); + assert(!a.is_infinite); assert(g1 + zero == g1); assert(g1 - g1 == zero); assert(g1 - zero == g1); assert(zero + zero == zero); assert( multi_scalar_mul([g1], [s1]) - == [1, 17631683881184975370165255887551781615748388533673675138860, 0] + == EmbeddedCurvePoint { x: 1, y: 17631683881184975370165255887551781615748388533673675138860, is_infinite: false } ); - assert(multi_scalar_mul([g1, g1], [s1, s1]) == [g2.x, g2.y, 0]); + assert(multi_scalar_mul([g1, g1], [s1, s1]) == g2); assert( multi_scalar_mul( [g1, zero], [EmbeddedCurveScalar { lo: 2, hi: 0 }, EmbeddedCurveScalar { lo: 42, hi: 25 }] ) - == [g2.x, g2.y, 0] + == g2 ); assert( multi_scalar_mul( [g1, g1, zero], [s1, s1, EmbeddedCurveScalar { lo: 42, hi: 25 }] ) - == [g2.x, g2.y, 0] + == g2 ); } diff --git a/test_programs/rebuild.sh b/test_programs/rebuild.sh index 13479f58b4b..a70f69d531d 100755 --- a/test_programs/rebuild.sh +++ b/test_programs/rebuild.sh @@ -45,7 +45,7 @@ rm -rf $current_dir/acir_artifacts mkdir -p $current_dir/acir_artifacts # Gather directories to process. -dirs_to_process=() +# dirs_to_process=() for dir in $base_path/*; do if [[ ! -d $dir ]] || [[ " ${excluded_dirs[@]} " =~ " $(basename "$dir") " ]]; then continue diff --git a/tooling/debugger/ignored-tests.txt b/tooling/debugger/ignored-tests.txt index a3971d437fb..2a6648b094b 100644 --- a/tooling/debugger/ignored-tests.txt +++ b/tooling/debugger/ignored-tests.txt @@ -1,6 +1,4 @@ -bigint brillig_references -brillig_to_bytes_integration debug_logs fold_after_inlined_calls fold_basic @@ -12,7 +10,5 @@ fold_fibonacci fold_numeric_generic_poseidon is_unconstrained macros -modulus references -regression_4709 -to_bytes_integration +regression_4709 \ No newline at end of file diff --git a/tooling/lsp/src/lib.rs b/tooling/lsp/src/lib.rs index b62f97a4918..e91e0fb3325 100644 --- a/tooling/lsp/src/lib.rs +++ b/tooling/lsp/src/lib.rs @@ -229,43 +229,74 @@ fn byte_span_to_range<'a, F: files::Files<'a> + ?Sized>( } } -pub(crate) fn resolve_workspace_for_source_path(file_path: &Path) -> Result { - if let Some(toml_path) = find_file_manifest(file_path) { - resolve_workspace_from_toml( - &toml_path, - PackageSelection::All, - Some(NOIR_ARTIFACT_VERSION_STRING.to_string()), - ) - .map_err(|err| LspError::WorkspaceResolutionError(err.to_string())) - } else { - let Some(parent_folder) = file_path - .parent() - .and_then(|f| f.file_name()) - .and_then(|file_name_os_str| file_name_os_str.to_str()) - else { - return Err(LspError::WorkspaceResolutionError(format!( - "Could not resolve parent folder for file: {:?}", - file_path - ))); - }; - let assumed_package = Package { - version: None, - compiler_required_version: Some(NOIR_ARTIFACT_VERSION_STRING.to_string()), - root_dir: PathBuf::from(parent_folder), - package_type: PackageType::Binary, - entry_path: PathBuf::from(file_path), - name: CrateName::from_str(parent_folder) - .map_err(|err| LspError::WorkspaceResolutionError(err.to_string()))?, - dependencies: BTreeMap::new(), - }; - let workspace = Workspace { - root_dir: PathBuf::from(parent_folder), - members: vec![assumed_package], - selected_package_index: Some(0), - is_assumed: true, - }; - Ok(workspace) +pub(crate) fn resolve_workspace_for_source_path( + file_path: &Path, + root_path: &Option, +) -> Result { + // If there's a LSP root path, starting from file_path go up the directory tree + // searching for Nargo.toml files. The last one we find is the one we'll use + // (we'll assume Noir workspaces aren't nested) + if let Some(root_path) = root_path { + let mut current_path = file_path; + let mut current_toml_path = None; + while current_path.starts_with(root_path) { + if let Some(toml_path) = find_file_manifest(current_path) { + current_toml_path = Some(toml_path); + + if let Some(next_path) = current_path.parent() { + current_path = next_path; + } else { + break; + } + } else { + break; + } + } + + if let Some(toml_path) = current_toml_path { + return resolve_workspace_from_toml( + &toml_path, + PackageSelection::All, + Some(NOIR_ARTIFACT_VERSION_STRING.to_string()), + ) + .map_err(|err| LspError::WorkspaceResolutionError(err.to_string())); + } } + + let Some(parent_folder) = file_path + .parent() + .and_then(|f| f.file_name()) + .and_then(|file_name_os_str| file_name_os_str.to_str()) + else { + return Err(LspError::WorkspaceResolutionError(format!( + "Could not resolve parent folder for file: {:?}", + file_path + ))); + }; + let assumed_package = Package { + version: None, + compiler_required_version: Some(NOIR_ARTIFACT_VERSION_STRING.to_string()), + root_dir: PathBuf::from(parent_folder), + package_type: PackageType::Binary, + entry_path: PathBuf::from(file_path), + name: CrateName::from_str(parent_folder) + .map_err(|err| LspError::WorkspaceResolutionError(err.to_string()))?, + dependencies: BTreeMap::new(), + }; + let workspace = Workspace { + root_dir: PathBuf::from(parent_folder), + members: vec![assumed_package], + selected_package_index: Some(0), + is_assumed: true, + }; + Ok(workspace) +} + +pub(crate) fn workspace_package_for_file<'a>( + workspace: &'a Workspace, + file_path: &Path, +) -> Option<&'a Package> { + workspace.members.iter().find(|package| file_path.starts_with(&package.root_dir)) } pub(crate) fn prepare_package<'file_manager, 'parsed_files>( diff --git a/tooling/lsp/src/notifications/mod.rs b/tooling/lsp/src/notifications/mod.rs index 46a7b1cf866..a5ded236514 100644 --- a/tooling/lsp/src/notifications/mod.rs +++ b/tooling/lsp/src/notifications/mod.rs @@ -14,7 +14,7 @@ use crate::types::{ use crate::{ byte_span_to_range, get_package_tests_in_crate, parse_diff, prepare_source, - resolve_workspace_for_source_path, LspState, + resolve_workspace_for_source_path, workspace_package_for_file, LspState, }; pub(super) fn on_initialized( @@ -39,7 +39,7 @@ pub(super) fn on_did_open_text_document( let document_uri = params.text_document.uri; - match process_noir_document(document_uri, state) { + match process_workspace_for_noir_document(document_uri, state) { Ok(_) => { state.open_documents_count += 1; ControlFlow::Continue(()) @@ -55,11 +55,14 @@ pub(super) fn on_did_change_text_document( let text = params.content_changes.into_iter().next().unwrap().text; state.input_files.insert(params.text_document.uri.to_string(), text.clone()); + let file_path = params.text_document.uri.to_file_path().unwrap(); + let (mut context, crate_id) = prepare_source(text, state); let _ = check_crate(&mut context, crate_id, false, false, false, None); let workspace = match resolve_workspace_for_source_path( params.text_document.uri.to_file_path().unwrap().as_path(), + &state.root_path, ) { Ok(workspace) => workspace, Err(lsp_error) => { @@ -70,7 +73,8 @@ pub(super) fn on_did_change_text_document( .into())) } }; - let package = match workspace.members.first() { + + let package = match workspace_package_for_file(&workspace, &file_path) { Some(package) => package, None => { return ControlFlow::Break(Err(ResponseError::new( @@ -110,13 +114,16 @@ pub(super) fn on_did_save_text_document( ) -> ControlFlow> { let document_uri = params.text_document.uri; - match process_noir_document(document_uri, state) { + match process_workspace_for_noir_document(document_uri, state) { Ok(_) => ControlFlow::Continue(()), Err(err) => ControlFlow::Break(Err(err)), } } -fn process_noir_document( +// Given a Noir document, find the workspace it's contained in (an assumed workspace is created if +// it's only contained in a package), then type-checks the workspace's packages, +// caching code lenses and type definitions, and notifying about compilation errors. +pub(crate) fn process_workspace_for_noir_document( document_uri: lsp_types::Url, state: &mut LspState, ) -> Result<(), async_lsp::Error> { @@ -124,9 +131,10 @@ fn process_noir_document( ResponseError::new(ErrorCode::REQUEST_FAILED, "URI is not a valid file path") })?; - let workspace = resolve_workspace_for_source_path(&file_path).map_err(|lsp_error| { - ResponseError::new(ErrorCode::REQUEST_FAILED, lsp_error.to_string()) - })?; + let workspace = + resolve_workspace_for_source_path(&file_path, &state.root_path).map_err(|lsp_error| { + ResponseError::new(ErrorCode::REQUEST_FAILED, lsp_error.to_string()) + })?; let mut workspace_file_manager = file_manager_with_stdlib(&workspace.root_dir); insert_all_files_for_workspace_into_file_manager(&workspace, &mut workspace_file_manager); diff --git a/tooling/lsp/src/requests/code_lens_request.rs b/tooling/lsp/src/requests/code_lens_request.rs index 0b8803edc6f..11ea4fa74bf 100644 --- a/tooling/lsp/src/requests/code_lens_request.rs +++ b/tooling/lsp/src/requests/code_lens_request.rs @@ -61,8 +61,12 @@ fn on_code_lens_request_inner( ResponseError::new(ErrorCode::REQUEST_FAILED, "Could not read file from disk") })?; - let workspace = resolve_workspace_for_source_path(file_path.as_path()).unwrap(); - let package = workspace.members.first().unwrap(); + let workspace = + resolve_workspace_for_source_path(file_path.as_path(), &state.root_path).unwrap(); + + let package = crate::workspace_package_for_file(&workspace, &file_path).ok_or_else(|| { + ResponseError::new(ErrorCode::REQUEST_FAILED, "Could not find package for file") + })?; let (mut context, crate_id) = prepare_source(source_string, state); // We ignore the warnings and errors produced by compilation for producing code lenses diff --git a/tooling/lsp/src/requests/goto_declaration.rs b/tooling/lsp/src/requests/goto_declaration.rs index 627cd8d203e..1f87f2c87d4 100644 --- a/tooling/lsp/src/requests/goto_declaration.rs +++ b/tooling/lsp/src/requests/goto_declaration.rs @@ -20,7 +20,7 @@ fn on_goto_definition_inner( state: &mut LspState, params: GotoDeclarationParams, ) -> Result { - process_request(state, params.text_document_position_params, |location, interner, files| { + process_request(state, params.text_document_position_params, |location, interner, files, _| { interner.get_declaration_location_from(location).and_then(|found_location| { let file_id = found_location.file; let definition_position = to_lsp_location(files, file_id, found_location.span)?; diff --git a/tooling/lsp/src/requests/goto_definition.rs b/tooling/lsp/src/requests/goto_definition.rs index 3713e8b646a..27d3503a2fd 100644 --- a/tooling/lsp/src/requests/goto_definition.rs +++ b/tooling/lsp/src/requests/goto_definition.rs @@ -29,7 +29,7 @@ fn on_goto_definition_inner( params: GotoDefinitionParams, return_type_location_instead: bool, ) -> Result { - process_request(state, params.text_document_position_params, |location, interner, files| { + process_request(state, params.text_document_position_params, |location, interner, files, _| { interner.get_definition_location_from(location, return_type_location_instead).and_then( |found_location| { let file_id = found_location.file; diff --git a/tooling/lsp/src/requests/mod.rs b/tooling/lsp/src/requests/mod.rs index 48299ff7459..9ece797a57a 100644 --- a/tooling/lsp/src/requests/mod.rs +++ b/tooling/lsp/src/requests/mod.rs @@ -1,4 +1,4 @@ -use std::future::Future; +use std::{collections::HashMap, future::Future}; use crate::{ parse_diff, resolve_workspace_for_source_path, @@ -270,15 +270,18 @@ pub(crate) fn process_request( callback: F, ) -> Result where - F: FnOnce(noirc_errors::Location, &NodeInterner, &FileMap) -> T, + F: FnOnce(noirc_errors::Location, &NodeInterner, &FileMap, &HashMap) -> T, { let file_path = text_document_position_params.text_document.uri.to_file_path().map_err(|_| { ResponseError::new(ErrorCode::REQUEST_FAILED, "URI is not a valid file path") })?; - let workspace = resolve_workspace_for_source_path(file_path.as_path()).unwrap(); - let package = workspace.members.first().unwrap(); + let workspace = + resolve_workspace_for_source_path(file_path.as_path(), &state.root_path).unwrap(); + let package = crate::workspace_package_for_file(&workspace, &file_path).ok_or_else(|| { + ResponseError::new(ErrorCode::REQUEST_FAILED, "Could not find package for file") + })?; let package_root_path: String = package.root_dir.as_os_str().to_string_lossy().into(); @@ -306,7 +309,81 @@ where &text_document_position_params.position, )?; - Ok(callback(location, interner, files)) + Ok(callback(location, interner, files, &state.cached_definitions)) +} +pub(crate) fn find_all_references_in_workspace( + location: noirc_errors::Location, + interner: &NodeInterner, + cached_interners: &HashMap, + files: &FileMap, + include_declaration: bool, + include_self_type_name: bool, +) -> Option> { + // First find the node that's referenced by the given location, if any + let referenced = interner.find_referenced(location); + + if let Some(referenced) = referenced { + // If we found the referenced node, find its location + let referenced_location = interner.reference_location(referenced); + + // Now we find all references that point to this location, in all interners + // (there's one interner per package, and all interners in a workspace rely on the + // same FileManager so a Location/FileId in one package is the same as in another package) + let mut locations = find_all_references( + referenced_location, + interner, + files, + include_declaration, + include_self_type_name, + ); + for interner in cached_interners.values() { + locations.extend(find_all_references( + referenced_location, + interner, + files, + include_declaration, + include_self_type_name, + )); + } + + // The LSP client usually removes duplicate loctions, but we do it here just in case they don't + locations.sort_by_key(|location| { + ( + location.uri.to_string(), + location.range.start.line, + location.range.start.character, + location.range.end.line, + location.range.end.character, + ) + }); + locations.dedup(); + + if locations.is_empty() { + None + } else { + Some(locations) + } + } else { + None + } +} + +pub(crate) fn find_all_references( + referenced_location: noirc_errors::Location, + interner: &NodeInterner, + files: &FileMap, + include_declaration: bool, + include_self_type_name: bool, +) -> Vec { + interner + .find_all_references(referenced_location, include_declaration, include_self_type_name) + .map(|locations| { + locations + .iter() + .filter_map(|location| to_lsp_location(files, location.file, location.span)) + .collect() + }) + .unwrap_or_default() } #[cfg(test)] diff --git a/tooling/lsp/src/requests/references.rs b/tooling/lsp/src/requests/references.rs index f8c23632936..6c872656c28 100644 --- a/tooling/lsp/src/requests/references.rs +++ b/tooling/lsp/src/requests/references.rs @@ -5,32 +5,38 @@ use lsp_types::{Location, ReferenceParams}; use crate::LspState; -use super::{process_request, to_lsp_location}; +use super::{find_all_references_in_workspace, process_request}; pub(crate) fn on_references_request( state: &mut LspState, params: ReferenceParams, ) -> impl Future>, ResponseError>> { - let result = - process_request(state, params.text_document_position, |location, interner, files| { - interner.find_all_references(location, params.context.include_declaration, true).map( - |locations| { - locations - .iter() - .filter_map(|location| to_lsp_location(files, location.file, location.span)) - .collect() - }, + let include_declaration = params.context.include_declaration; + let result = process_request( + state, + params.text_document_position, + |location, interner, files, cached_interners| { + find_all_references_in_workspace( + location, + interner, + cached_interners, + files, + include_declaration, + true, ) - }); + }, + ); future::ready(result) } #[cfg(test)] mod references_tests { use super::*; + use crate::notifications; use crate::test_utils::{self, search_in_file}; use lsp_types::{ - PartialResultParams, ReferenceContext, TextDocumentPositionParams, WorkDoneProgressParams, + PartialResultParams, Position, Range, ReferenceContext, TextDocumentPositionParams, Url, + WorkDoneProgressParams, }; use tokio::test; @@ -91,4 +97,70 @@ mod references_tests { async fn test_on_references_request_without_including_declaration() { check_references_succeeds("rename_function", "another_function", 0, false).await; } + + #[test] + async fn test_on_references_request_works_accross_workspace_packages() { + let (mut state, noir_text_document) = test_utils::init_lsp_server("workspace").await; + + // noir_text_document is always `src/main.nr` in the workspace directory, so let's go to the workspace dir + let noir_text_document = noir_text_document.to_file_path().unwrap(); + let workspace_dir = noir_text_document.parent().unwrap().parent().unwrap(); + + // Let's check that we can find references to `function_one` by doing that in the package "one" + // and getting results in the package "two" too. + let one_lib = Url::from_file_path(workspace_dir.join("one/src/lib.nr")).unwrap(); + let two_lib = Url::from_file_path(workspace_dir.join("two/src/lib.nr")).unwrap(); + + // We call this to open the document, so that the entire workspace is analyzed + notifications::process_workspace_for_noir_document(one_lib.clone(), &mut state).unwrap(); + + let params = ReferenceParams { + text_document_position: TextDocumentPositionParams { + text_document: lsp_types::TextDocumentIdentifier { uri: one_lib.clone() }, + position: Position { line: 0, character: 7 }, + }, + work_done_progress_params: WorkDoneProgressParams { work_done_token: None }, + partial_result_params: PartialResultParams { partial_result_token: None }, + context: ReferenceContext { include_declaration: true }, + }; + + let mut locations = on_references_request(&mut state, params) + .await + .expect("Could not execute on_references_request") + .unwrap(); + + // The definition, a use in "two", and a call in "two" + assert_eq!(locations.len(), 3); + + locations.sort_by_cached_key(|location| { + (location.uri.to_file_path().unwrap(), location.range.start.line) + }); + + assert_eq!(locations[0].uri, one_lib); + assert_eq!( + locations[0].range, + Range { + start: Position { line: 0, character: 7 }, + end: Position { line: 0, character: 19 }, + } + ); + + assert_eq!(locations[1].uri, two_lib); + assert_eq!( + locations[1].range, + Range { + start: Position { line: 0, character: 9 }, + end: Position { line: 0, character: 21 }, + } + ); + + assert_eq!(locations[2].uri, two_lib); + assert_eq!( + locations[2].range, + Range { + start: Position { line: 3, character: 4 }, + end: Position { line: 3, character: 16 }, + } + ); + } } diff --git a/tooling/lsp/src/requests/rename.rs b/tooling/lsp/src/requests/rename.rs index 906a5cbcaab..245c2ed0882 100644 --- a/tooling/lsp/src/requests/rename.rs +++ b/tooling/lsp/src/requests/rename.rs @@ -11,13 +11,13 @@ use noirc_frontend::node_interner::ReferenceId; use crate::LspState; -use super::{process_request, to_lsp_location}; +use super::{find_all_references_in_workspace, process_request}; pub(crate) fn on_prepare_rename_request( state: &mut LspState, params: TextDocumentPositionParams, ) -> impl Future, ResponseError>> { - let result = process_request(state, params, |location, interner, _| { + let result = process_request(state, params, |location, interner, _, _| { let reference_id = interner.reference_at_location(location); let rename_possible = match reference_id { // Rename shouldn't be possible when triggered on top of "Self" @@ -34,32 +34,30 @@ pub(crate) fn on_rename_request( state: &mut LspState, params: RenameParams, ) -> impl Future, ResponseError>> { - let result = - process_request(state, params.text_document_position, |location, interner, files| { - let rename_changes = - interner.find_all_references(location, true, false).map(|locations| { - let rs = locations.iter().fold( - HashMap::new(), - |mut acc: HashMap>, location| { - let file_id = location.file; - let span = location.span; - - let Some(lsp_location) = to_lsp_location(files, file_id, span) else { - return acc; - }; - - let edit = TextEdit { - range: lsp_location.range, - new_text: params.new_name.clone(), - }; - - acc.entry(lsp_location.uri).or_default().push(edit); - - acc - }, - ); - rs - }); + let result = process_request( + state, + params.text_document_position, + |location, interner, files, cached_interners| { + let rename_changes = find_all_references_in_workspace( + location, + interner, + cached_interners, + files, + true, + false, + ) + .map(|locations| { + let rs = locations.iter().fold( + HashMap::new(), + |mut acc: HashMap>, location| { + let edit = + TextEdit { range: location.range, new_text: params.new_name.clone() }; + acc.entry(location.uri.clone()).or_default().push(edit); + acc + }, + ); + rs + }); let response = WorkspaceEdit { changes: rename_changes, @@ -68,7 +66,8 @@ pub(crate) fn on_rename_request( }; Some(response) - }); + }, + ); future::ready(result) } diff --git a/tooling/lsp/test_programs/workspace/Nargo.toml b/tooling/lsp/test_programs/workspace/Nargo.toml new file mode 100644 index 00000000000..d0a0badc295 --- /dev/null +++ b/tooling/lsp/test_programs/workspace/Nargo.toml @@ -0,0 +1,2 @@ +[workspace] +members = ["one", "two"] diff --git a/tooling/lsp/test_programs/workspace/one/Nargo.toml b/tooling/lsp/test_programs/workspace/one/Nargo.toml new file mode 100644 index 00000000000..39838d73362 --- /dev/null +++ b/tooling/lsp/test_programs/workspace/one/Nargo.toml @@ -0,0 +1,4 @@ +[package] +name = "one" +authors = [] +type = "lib" diff --git a/tooling/lsp/test_programs/workspace/one/src/lib.nr b/tooling/lsp/test_programs/workspace/one/src/lib.nr new file mode 100644 index 00000000000..d4f660e35fb --- /dev/null +++ b/tooling/lsp/test_programs/workspace/one/src/lib.nr @@ -0,0 +1 @@ +pub fn function_one() {} diff --git a/tooling/lsp/test_programs/workspace/two/Nargo.toml b/tooling/lsp/test_programs/workspace/two/Nargo.toml new file mode 100644 index 00000000000..26d99b65df1 --- /dev/null +++ b/tooling/lsp/test_programs/workspace/two/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "two" +authors = [] +type = "lib" + +[dependencies] +one = { path = "../one" } \ No newline at end of file diff --git a/tooling/lsp/test_programs/workspace/two/src/lib.nr b/tooling/lsp/test_programs/workspace/two/src/lib.nr new file mode 100644 index 00000000000..adf7079b4c9 --- /dev/null +++ b/tooling/lsp/test_programs/workspace/two/src/lib.nr @@ -0,0 +1,5 @@ +use one::function_one; + +pub fn function_two() { + function_one() +} diff --git a/tooling/profiler/src/cli/gates_flamegraph_cmd.rs b/tooling/profiler/src/cli/gates_flamegraph_cmd.rs index 154ac38f4bb..98e89e42015 100644 --- a/tooling/profiler/src/cli/gates_flamegraph_cmd.rs +++ b/tooling/profiler/src/cli/gates_flamegraph_cmd.rs @@ -19,6 +19,9 @@ pub(crate) struct GatesFlamegraphCommand { #[clap(long, short)] backend_path: String, + #[arg(trailing_var_arg = true, allow_hyphen_values = true)] + backend_extra_args: Vec, + /// The output folder for the flamegraph svg files #[clap(long, short)] output: String, @@ -27,7 +30,10 @@ pub(crate) struct GatesFlamegraphCommand { pub(crate) fn run(args: GatesFlamegraphCommand) -> eyre::Result<()> { run_with_provider( &PathBuf::from(args.artifact_path), - &BackendGatesProvider { backend_path: PathBuf::from(args.backend_path) }, + &BackendGatesProvider { + backend_path: PathBuf::from(args.backend_path), + extra_args: args.backend_extra_args, + }, &InfernoFlamegraphGenerator { count_name: "gates".to_string() }, &PathBuf::from(args.output), ) diff --git a/tooling/profiler/src/gates_provider.rs b/tooling/profiler/src/gates_provider.rs index caed2666426..f96b1292987 100644 --- a/tooling/profiler/src/gates_provider.rs +++ b/tooling/profiler/src/gates_provider.rs @@ -10,12 +10,20 @@ pub(crate) trait GatesProvider { pub(crate) struct BackendGatesProvider { pub(crate) backend_path: PathBuf, + pub(crate) extra_args: Vec, } impl GatesProvider for BackendGatesProvider { fn get_gates(&self, artifact_path: &Path) -> eyre::Result { - let backend_gates_response = - Command::new(&self.backend_path).arg("gates").arg("-b").arg(artifact_path).output()?; + let mut backend_gates_cmd = Command::new(&self.backend_path); + + backend_gates_cmd.arg("gates").arg("-b").arg(artifact_path); + + for arg in &self.extra_args { + backend_gates_cmd.arg(arg); + } + + let backend_gates_response = backend_gates_cmd.output()?; // Parse the backend gates command stdout as json let backend_gates_response: BackendGatesResponse =