From aefa5a6d6dd155b3c1776a6519c3c0f4cee7705d Mon Sep 17 00:00:00 2001 From: sirasistant Date: Wed, 17 Apr 2024 09:54:41 +0000 Subject: [PATCH] fix: Don't reuse brillig with slice arguments --- .../src/core/libraries/ConstantsGen.sol | 2 +- .../crates/types/src/constants.nr | 2 +- .../src/brillig/brillig_ir/artifact.rs | 2 +- .../noirc_evaluator/src/ssa/acir_gen/mod.rs | 30 ++++++++++++------- 4 files changed, 23 insertions(+), 13 deletions(-) diff --git a/l1-contracts/src/core/libraries/ConstantsGen.sol b/l1-contracts/src/core/libraries/ConstantsGen.sol index fe139215971..883f546a905 100644 --- a/l1-contracts/src/core/libraries/ConstantsGen.sol +++ b/l1-contracts/src/core/libraries/ConstantsGen.sol @@ -83,7 +83,7 @@ library Constants { uint256 internal constant DEPLOYER_CONTRACT_INSTANCE_DEPLOYED_MAGIC_VALUE = 0x85864497636cf755ae7bde03f267ce01a520981c21c3682aaf82a631; uint256 internal constant DEPLOYER_CONTRACT_ADDRESS = - 0x2d8e7aedc70b65d49e6aa0794d8d12721896c177e87126701f6e60d184358e74; + 0x0b98aeb0111208b95d8d71f484f849d7ab44b3e34c545d13736a707ce3cb0839; uint256 internal constant L1_TO_L2_MESSAGE_ORACLE_CALL_LENGTH = 17; uint256 internal constant MAX_NOTE_FIELDS_LENGTH = 20; uint256 internal constant GET_NOTE_ORACLE_RETURN_LENGTH = 23; diff --git a/noir-projects/noir-protocol-circuits/crates/types/src/constants.nr b/noir-projects/noir-protocol-circuits/crates/types/src/constants.nr index 96f1a6dcdd2..c4b67523bc6 100644 --- a/noir-projects/noir-protocol-circuits/crates/types/src/constants.nr +++ b/noir-projects/noir-protocol-circuits/crates/types/src/constants.nr @@ -118,7 +118,7 @@ global REGISTERER_UNCONSTRAINED_FUNCTION_BROADCASTED_MAGIC_VALUE = 0xe7af8166354 // CONTRACT INSTANCE CONSTANTS // sha224sum 'struct ContractInstanceDeployed' global DEPLOYER_CONTRACT_INSTANCE_DEPLOYED_MAGIC_VALUE = 0x85864497636cf755ae7bde03f267ce01a520981c21c3682aaf82a631; -global DEPLOYER_CONTRACT_ADDRESS = 0x2d8e7aedc70b65d49e6aa0794d8d12721896c177e87126701f6e60d184358e74; +global DEPLOYER_CONTRACT_ADDRESS = 0x0b98aeb0111208b95d8d71f484f849d7ab44b3e34c545d13736a707ce3cb0839; // NOIR CONSTANTS - constants used only in yarn-packages/noir-contracts // Some are defined here because Noir doesn't yet support globals referencing other globals yet. diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/artifact.rs b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/artifact.rs index 8bd1bfda78f..8a4f469f5c9 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/artifact.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/artifact.rs @@ -4,7 +4,7 @@ use std::collections::{BTreeMap, HashMap}; use crate::ssa::ir::dfg::CallStack; /// Represents a parameter or a return value of an entry point function. -#[derive(Debug, Clone)] +#[derive(Debug, Clone, Eq, PartialEq, Hash, PartialOrd, Ord)] pub(crate) enum BrilligParameter { /// A single address parameter or return value. Holds the bit size of the parameter. SingleAddr(u32), diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs index 8019707644b..02b381d79fc 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs @@ -52,12 +52,18 @@ struct SharedContext { /// There can be Brillig functions specified in SSA which do not act as /// entry points in ACIR (e.g. only called by other Brillig functions) /// This mapping is necessary to use the correct function pointer for a Brillig call. - brillig_generated_func_pointers: BTreeMap, + /// This uses the brillig parameters in the map since using slices with different lengths + /// needs to create different brillig entrypoints + brillig_generated_func_pointers: BTreeMap<(FunctionId, Vec), u32>, } impl SharedContext { - fn generated_brillig_pointer(&self, func_id: &FunctionId) -> Option<&u32> { - self.brillig_generated_func_pointers.get(func_id) + fn generated_brillig_pointer( + &self, + func_id: FunctionId, + arguments: Vec, + ) -> Option<&u32> { + self.brillig_generated_func_pointers.get(&(func_id, arguments)) } fn generated_brillig(&self, func_pointer: usize) -> &GeneratedBrillig { @@ -67,10 +73,11 @@ impl SharedContext { fn insert_generated_brillig( &mut self, func_id: FunctionId, + arguments: Vec, generated_pointer: u32, code: GeneratedBrillig, ) { - self.brillig_generated_func_pointers.insert(func_id, generated_pointer); + self.brillig_generated_func_pointers.insert((func_id, arguments), generated_pointer); self.generated_brillig.push(code); } @@ -356,7 +363,7 @@ impl<'a> Context<'a> { let outputs: Vec = vecmap(main_func.returns(), |result_id| dfg.type_of_value(*result_id).into()); - let code = self.gen_brillig_for(main_func, arguments, brillig)?; + let code = self.gen_brillig_for(main_func, arguments.clone(), brillig)?; // We specifically do not attempt execution of the brillig code being generated as this can result in it being // replaced with constraints on witnesses to the program outputs. @@ -370,7 +377,7 @@ impl<'a> Context<'a> { // We are guaranteed to have a Brillig function pointer of `0` as main itself is marked as unconstrained 0, )?; - self.shared_context.insert_generated_brillig(main_func.id(), 0, code); + self.shared_context.insert_generated_brillig(main_func.id(), arguments, 0, code); let output_vars: Vec<_> = output_values .iter() @@ -657,6 +664,7 @@ impl<'a> Context<'a> { } } let inputs = vecmap(arguments, |arg| self.convert_value(*arg, dfg)); + let arguments = self.gen_brillig_parameters(arguments, dfg); let outputs: Vec = vecmap(result_ids, |result_id| { dfg.type_of_value(*result_id).into() @@ -664,8 +672,9 @@ impl<'a> Context<'a> { // Check whether we have already generated Brillig for this function // If we have, re-use the generated code to set-up the Brillig call. - let output_values = if let Some(generated_pointer) = - self.shared_context.generated_brillig_pointer(id) + let output_values = if let Some(generated_pointer) = self + .shared_context + .generated_brillig_pointer(*id, arguments.clone()) { let code = self .shared_context @@ -680,8 +689,8 @@ impl<'a> Context<'a> { *generated_pointer, )? } else { - let arguments = self.gen_brillig_parameters(arguments, dfg); - let code = self.gen_brillig_for(func, arguments, brillig)?; + let code = + self.gen_brillig_for(func, arguments.clone(), brillig)?; let generated_pointer = self.shared_context.new_generated_pointer(); let output_values = self.acir_context.brillig_call( @@ -695,6 +704,7 @@ impl<'a> Context<'a> { )?; self.shared_context.insert_generated_brillig( *id, + arguments, generated_pointer, code, );