From 650ffc5053cdca4b6ad2e027fa1f4fd90ef64871 Mon Sep 17 00:00:00 2001 From: Nikita Masych <92444221+NikitaMasych@users.noreply.github.com> Date: Fri, 23 Feb 2024 16:55:17 +0200 Subject: [PATCH 1/7] feat: Add HashMap to the stdlib (#4242) # Description This PR shall bring HashMap into the `stdlib` of Noir. ## Problem\* Resolves #4241 ## Summary\* Implementation of `HashMap` with open addressing and quadratic probing scheme. Since Noir requires knowing loop bounds (and recursive calls) at compile time, `HashMap` is of fixed capacity and **no** dynamic resize is accomplished with regard to load factor. Furthermore, contribution includes implementation of `PedersenHasher` to be used for now. One can examine potentially better and less heavy prehash functions. I tried to conform with best practices of engineering, however since Noir is in rapid development, there are certain things which may be optimized in future, both from the code style and performance point of view. ## Additional Context I put the `PedersenHasher` among the `poseidon.nr` and `mimc.nr`, so one can consider moving declaration of other pedersen-related functionality there, however that would be a breaking change. ## Documentation\* Check one: - [ ] No documentation needed. - [ ] Documentation included in this PR. - [x] **[Exceptional Case]** Documentation to be submitted in a separate PR. # PR Checklist\* - [x] I have tested the changes locally. - [x] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings. --- noir_stdlib/src/collections.nr | 1 + noir_stdlib/src/collections/map.nr | 456 ++++++++++++++++++ noir_stdlib/src/hash.nr | 52 ++ noir_stdlib/src/hash/pedersen.nr | 24 + .../hashmap_load_factor/Nargo.toml | 6 + .../hashmap_load_factor/Prover.toml | 26 + .../hashmap_load_factor/src/main.nr | 35 ++ .../execution_success/hashmap/Nargo.toml | 6 + .../execution_success/hashmap/Prover.toml | 26 + .../execution_success/hashmap/src/main.nr | 192 ++++++++ .../execution_success/hashmap/src/utils.nr | 10 + 11 files changed, 834 insertions(+) create mode 100644 noir_stdlib/src/collections/map.nr create mode 100644 noir_stdlib/src/hash/pedersen.nr create mode 100644 test_programs/compile_failure/hashmap_load_factor/Nargo.toml create mode 100644 test_programs/compile_failure/hashmap_load_factor/Prover.toml create mode 100644 test_programs/compile_failure/hashmap_load_factor/src/main.nr create mode 100644 test_programs/execution_success/hashmap/Nargo.toml create mode 100644 test_programs/execution_success/hashmap/Prover.toml create mode 100644 test_programs/execution_success/hashmap/src/main.nr create mode 100644 test_programs/execution_success/hashmap/src/utils.nr diff --git a/noir_stdlib/src/collections.nr b/noir_stdlib/src/collections.nr index 177ca96816f..2d952f4d6cd 100644 --- a/noir_stdlib/src/collections.nr +++ b/noir_stdlib/src/collections.nr @@ -1,2 +1,3 @@ mod vec; mod bounded_vec; +mod map; diff --git a/noir_stdlib/src/collections/map.nr b/noir_stdlib/src/collections/map.nr new file mode 100644 index 00000000000..d9eb83ff5dc --- /dev/null +++ b/noir_stdlib/src/collections/map.nr @@ -0,0 +1,456 @@ +use crate::cmp::Eq; +use crate::collections::vec::Vec; +use crate::option::Option; +use crate::default::Default; +use crate::hash::{Hash, Hasher, BuildHasher}; + +// We use load factor α_max = 0.75. +// Upon exceeding it, assert will fail in order to inform the user +// about performance degradation, so that he can adjust the capacity. +global MAX_LOAD_FACTOR_NUMERATOR = 3; +global MAX_LOAD_FACTOR_DEN0MINATOR = 4; + +// Hash table with open addressing and quadratic probing. +// Size of the underlying table must be known at compile time. +// It is advised to select capacity N as a power of two, or a prime number +// because utilized probing scheme is best tailored for it. +struct HashMap { + _table: [Slot; N], + + // Amount of valid elements in the map. + _len: u64, + + _build_hasher: B +} + +// Data unit in the HashMap table. +// In case Noir adds support for enums in the future, this +// should be refactored to have three states: +// 1. (key, value) +// 2. (empty) +// 3. (deleted) +struct Slot { + _key_value: Option<(K, V)>, + _is_deleted: bool, +} + +impl Default for Slot{ + fn default() -> Self{ + Slot{ + _key_value: Option::none(), + _is_deleted: false + } + } +} + +impl Slot { + fn is_valid(self) -> bool { + !self._is_deleted & self._key_value.is_some() + } + + fn is_available(self) -> bool { + self._is_deleted | self._key_value.is_none() + } + + fn key_value(self) -> Option<(K, V)> { + self._key_value + } + + fn key_value_unchecked(self) -> (K, V) { + self._key_value.unwrap_unchecked() + } + + fn set(&mut self, key: K, value: V) { + self._key_value = Option::some((key, value)); + self._is_deleted = false; + } + + // Shall not override `_key_value` with Option::none(), + // because we must be able to differentiate empty + // and deleted slots for lookup. + fn mark_deleted(&mut self) { + self._is_deleted = true; + } +} + +// While conducting lookup, we iterate attempt from 0 to N - 1 due to heuristic, +// that if we have went that far without finding desired, +// it is very unlikely to be after - performance will be heavily degraded. +impl HashMap { + // Creates a new instance of HashMap with specified BuildHasher. + pub fn with_hasher(_build_hasher: B) -> Self + where + B: BuildHasher { + let _table = [Slot::default(); N]; + let _len = 0; + Self { _table, _len, _build_hasher } + } + + // Clears the map, removing all key-value entries. + pub fn clear(&mut self) { + self._table = [Slot::default(); N]; + self._len = 0; + } + + // Returns true if the map contains a value for the specified key. + pub fn contains_key( + self, + key: K + ) -> bool + where + K: Hash + Eq, + B: BuildHasher, + H: Hasher { + self.get(key).is_some() + } + + // Returns true if the map contains no elements. + pub fn is_empty(self) -> bool { + self._len == 0 + } + + // Get the Option<(K, V) array of valid entries + // with a length of map capacity. First len() elements + // are safe to unwrap_unchecked(), whilst remaining + // are guaranteed to be Option::none(). + // + // This design is reasoned by compile-time limitations and + // temporary nested slices ban. + pub fn entries(self) -> [Option<(K, V)>; N] { + let mut entries = [Option::none(); N]; + let mut valid_amount = 0; + + for slot in self._table { + if slot.is_valid() { + entries[valid_amount] = slot.key_value(); + valid_amount += 1; + } + } + + let msg = f"Amount of valid elements should have been {self._len} times, but got {valid_amount}."; + assert(valid_amount == self._len, msg); + + entries + } + + // Get the Option array of valid keys + // with a length of map capacity. First len() elements + // are safe to unwrap_unchecked(), whilst remaining + // are guaranteed to be Option::none(). + // + // This design is reasoned by compile-time limitations and + // temporary nested slices ban. + pub fn keys(self) -> [Option; N] { + let mut keys = [Option::none(); N]; + let mut valid_amount = 0; + + for slot in self._table { + if slot.is_valid() { + let (key, _) = slot.key_value_unchecked(); + keys[valid_amount] = Option::some(key); + valid_amount += 1; + } + } + + let msg = f"Amount of valid elements should have been {self._len} times, but got {valid_amount}."; + assert(valid_amount == self._len, msg); + + keys + } + + // Get the Option array of valid values + // with a length of map capacity. First len() elements + // are safe to unwrap_unchecked(), whilst remaining + // are guaranteed to be Option::none(). + // + // This design is reasoned by compile-time limitations and + // temporary nested slices ban. + pub fn values(self) -> [Option; N] { + let mut values = [Option::none(); N]; + let mut valid_amount = 0; + + for slot in self._table { + if slot.is_valid() { + let (_, value) = slot.key_value_unchecked(); + values[valid_amount] = Option::some(value); + valid_amount += 1; + } + } + + let msg = f"Amount of valid elements should have been {self._len} times, but got {valid_amount}."; + assert(valid_amount == self._len, msg); + + values + } + + // For each key-value entry applies mutator function. + pub fn iter_mut( + &mut self, + f: fn(K, V) -> (K, V) + ) + where + K: Eq + Hash, + B: BuildHasher, + H: Hasher { + let mut entries = self.entries(); + let mut new_map = HashMap::with_hasher(self._build_hasher); + + for i in 0..N { + if i < self._len { + let entry = entries[i].unwrap_unchecked(); + let (key, value) = f(entry.0, entry.1); + new_map.insert(key, value); + } + } + + self._table = new_map._table; + } + + // For each key applies mutator function. + pub fn iter_keys_mut( + &mut self, + f: fn(K) -> K + ) + where + K: Eq + Hash, + B: BuildHasher, + H: Hasher { + let mut entries = self.entries(); + let mut new_map = HashMap::with_hasher(self._build_hasher); + + for i in 0..N { + if i < self._len { + let entry = entries[i].unwrap_unchecked(); + let (key, value) = (f(entry.0), entry.1); + new_map.insert(key, value); + } + } + + self._table = new_map._table; + } + + // For each value applies mutator function. + pub fn iter_values_mut(&mut self, f: fn(V) -> V) { + for i in 0..N { + let mut slot = self._table[i]; + if slot.is_valid() { + let (key, value) = slot.key_value_unchecked(); + slot.set(key, f(value)); + self._table[i] = slot; + } + } + } + + // Retains only the elements specified by the predicate. + pub fn retain(&mut self, f: fn(K, V) -> bool) { + for index in 0..N { + let mut slot = self._table[index]; + if slot.is_valid() { + let (key, value) = slot.key_value_unchecked(); + if !f(key, value) { + slot.mark_deleted(); + self._len -= 1; + self._table[index] = slot; + } + } + } + } + + // Amount of active key-value entries. + pub fn len(self) -> u64 { + self._len + } + + // Get the compile-time map capacity. + pub fn capacity(_self: Self) -> u64 { + N + } + + // Get the value by key. If it does not exist, returns none(). + pub fn get( + self, + key: K + ) -> Option + where + K: Eq + Hash, + B: BuildHasher, + H: Hasher { + let mut result = Option::none(); + + let hash = self.hash(key); + let mut break = false; + + for attempt in 0..N { + if !break { + let index = self.quadratic_probe(hash, attempt as u64); + let slot = self._table[index]; + + // Not marked as deleted and has key-value. + if slot.is_valid() { + let (current_key, value) = slot.key_value_unchecked(); + if current_key == key { + result = Option::some(value); + break = true; + } + } + } + } + + result + } + + // Insert key-value entry. In case key was already present, value is overridden. + pub fn insert( + &mut self, + key: K, + value: V + ) + where + K: Eq + Hash, + B: BuildHasher, + H: Hasher { + self.assert_load_factor(); + + let hash = self.hash(key); + let mut break = false; + + for attempt in 0..N { + if !break { + let index = self.quadratic_probe(hash, attempt as u64); + let mut slot = self._table[index]; + let mut insert = false; + + // Either marked as deleted or has unset key-value. + if slot.is_available() { + insert = true; + self._len += 1; + } else { + let (current_key, _) = slot.key_value_unchecked(); + if current_key == key { + insert = true; + } + } + + if insert { + slot.set(key, value); + self._table[index] = slot; + break = true; + } + } + } + } + + // Remove key-value entry. If key is not present, HashMap remains unchanged. + pub fn remove( + &mut self, + key: K + ) + where + K: Eq + Hash, + B: BuildHasher, + H: Hasher { + let hash = self.hash(key); + let mut break = false; + + for attempt in 0..N { + if !break { + let index = self.quadratic_probe(hash, attempt as u64); + let mut slot = self._table[index]; + + // Not marked as deleted and has key-value. + if slot.is_valid() { + let (current_key, _) = slot.key_value_unchecked(); + if current_key == key { + slot.mark_deleted(); + self._table[index] = slot; + self._len -= 1; + break = true; + } + } + } + } + } + + // Apply HashMap's hasher onto key to obtain pre-hash for probing. + fn hash( + self, + key: K + ) -> u64 + where + K: Hash, + B: BuildHasher, + H: Hasher { + let mut hasher = self._build_hasher.build_hasher(); + key.hash(&mut hasher); + hasher.finish() as u64 + } + + // Probing scheme: quadratic function. + // We use 0.5 constant near variadic attempt and attempt^2 monomials. + // This ensures good uniformity of distribution for table sizes + // equal to prime numbers or powers of two. + fn quadratic_probe(_self: Self, hash: u64, attempt: u64) -> u64 { + (hash + (attempt + attempt * attempt) / 2) % N + } + + // Amount of elements in the table in relation to available slots exceeds α_max. + // To avoid a comparatively more expensive division operation + // we conduct cross-multiplication instead. + // n / m >= MAX_LOAD_FACTOR_NUMERATOR / MAX_LOAD_FACTOR_DEN0MINATOR + // n * MAX_LOAD_FACTOR_DEN0MINATOR >= m * MAX_LOAD_FACTOR_NUMERATOR + fn assert_load_factor(self) { + let lhs = self._len * MAX_LOAD_FACTOR_DEN0MINATOR; + let rhs = self._table.len() as u64 * MAX_LOAD_FACTOR_NUMERATOR; + let exceeded = lhs >= rhs; + assert(!exceeded, "Load factor is exceeded, consider increasing the capacity."); + } +} + +// Equality class on HashMap has to test that they have +// equal sets of key-value entries, +// thus one is a subset of the other and vice versa. +impl Eq for HashMap +where + K: Eq + Hash, + V: Eq, + B: BuildHasher, + H: Hasher +{ + fn eq(self, other: HashMap) -> bool{ + let mut equal = false; + + if self.len() == other.len(){ + equal = true; + for slot in self._table{ + // Not marked as deleted and has key-value. + if equal & slot.is_valid(){ + let (key, value) = slot.key_value_unchecked(); + let other_value = other.get(key); + + if other_value.is_none(){ + equal = false; + }else{ + let other_value = other_value.unwrap_unchecked(); + if value != other_value{ + equal = false; + } + } + } + } + } + + equal + } +} + +impl Default for HashMap +where + B: BuildHasher + Default, + H: Hasher + Default +{ + fn default() -> Self{ + let _build_hasher = B::default(); + let map: HashMap = HashMap::with_hasher(_build_hasher); + map + } +} diff --git a/noir_stdlib/src/hash.nr b/noir_stdlib/src/hash.nr index cc864039a90..7a931f7c047 100644 --- a/noir_stdlib/src/hash.nr +++ b/noir_stdlib/src/hash.nr @@ -1,5 +1,8 @@ mod poseidon; mod mimc; +mod pedersen; + +use crate::default::Default; #[foreign(sha256)] // docs:start:sha256 @@ -74,3 +77,52 @@ pub fn poseidon2_permutation(_input: [u8; N], _state_length: u32) -> [u8; N] #[foreign(sha256_compression)] pub fn sha256_compression(_input: [u32; 16], _state: [u32; 8]) -> [u32; 8] {} + +// Generic hashing support. +// Partially ported and impacted by rust. + +// Hash trait shall be implemented per type. +trait Hash{ + fn hash(self, state: &mut H) where H: Hasher; +} + +// Hasher trait shall be implemented by algorithms to provide hash-agnostic means. +// TODO: consider making the types generic here ([u8], [Field], etc.) +trait Hasher{ + fn finish(self) -> Field; + + fn write(&mut self, input: [Field]); +} + +// BuildHasher is a factory trait, responsible for production of specific Hasher. +trait BuildHasher where H: Hasher{ + fn build_hasher(self) -> H; +} + +struct BuildHasherDefault; + +impl BuildHasher for BuildHasherDefault +where + H: Hasher + Default +{ + fn build_hasher(_self: Self) -> H{ + H::default() + } +} + +impl Default for BuildHasherDefault +where + H: Hasher + Default +{ + fn default() -> Self{ + BuildHasherDefault{} + } +} + +// TODO: add implementations for the remainder of primitive types. +impl Hash for Field{ + fn hash(self, state: &mut H) where H: Hasher{ + let input: [Field] = [self]; + H::write(state, input); + } +} diff --git a/noir_stdlib/src/hash/pedersen.nr b/noir_stdlib/src/hash/pedersen.nr new file mode 100644 index 00000000000..ace6851099d --- /dev/null +++ b/noir_stdlib/src/hash/pedersen.nr @@ -0,0 +1,24 @@ +use crate::hash::{Hasher, pedersen_hash}; +use crate::default::Default; + +struct PedersenHasher{ + _state: [Field] +} + +impl Hasher for PedersenHasher { + fn finish(self) -> Field { + pedersen_hash(self._state) + } + + fn write(&mut self, input: [Field]){ + self._state = self._state.append(input); + } +} + +impl Default for PedersenHasher{ + fn default() -> Self{ + PedersenHasher{ + _state: [] + } + } +} diff --git a/test_programs/compile_failure/hashmap_load_factor/Nargo.toml b/test_programs/compile_failure/hashmap_load_factor/Nargo.toml new file mode 100644 index 00000000000..92da5a357f4 --- /dev/null +++ b/test_programs/compile_failure/hashmap_load_factor/Nargo.toml @@ -0,0 +1,6 @@ +[package] +name = "hashmap_load_factor" +type = "bin" +authors = [""] + +[dependencies] \ No newline at end of file diff --git a/test_programs/compile_failure/hashmap_load_factor/Prover.toml b/test_programs/compile_failure/hashmap_load_factor/Prover.toml new file mode 100644 index 00000000000..e54319c61e9 --- /dev/null +++ b/test_programs/compile_failure/hashmap_load_factor/Prover.toml @@ -0,0 +1,26 @@ +# Expected 6 key-value entries for hashmap capacity of 8. +# These must be distinct (both key-to-key, and value-to-value) for correct testing. + +[[input]] +key = 2 +value = 17 + +[[input]] +key = 3 +value = 19 + +[[input]] +key = 5 +value = 23 + +[[input]] +key = 7 +value = 29 + +[[input]] +key = 11 +value = 31 + +[[input]] +key = 41 +value = 43 \ No newline at end of file diff --git a/test_programs/compile_failure/hashmap_load_factor/src/main.nr b/test_programs/compile_failure/hashmap_load_factor/src/main.nr new file mode 100644 index 00000000000..ade43f898e1 --- /dev/null +++ b/test_programs/compile_failure/hashmap_load_factor/src/main.nr @@ -0,0 +1,35 @@ +use dep::std::collections::map::HashMap; +use dep::std::hash::BuildHasherDefault; +use dep::std::hash::pedersen::PedersenHasher; + +struct Entry{ + key: Field, + value: Field +} + +global HASHMAP_CAP = 8; +global HASHMAP_LEN = 6; + +fn allocate_hashmap() -> HashMap> { + HashMap::default() +} + +fn main(input: [Entry; HASHMAP_LEN]) { + test_load_factor(input); +} + +// In this test we exceed load factor: +// α_max = 0.75, thus for capacity of 8 and lenght of 6 +// insertion of new unique key (7-th) should throw assertion error. +fn test_load_factor(input: [Entry; HASHMAP_LEN]) { + let mut hashmap = allocate_hashmap(); + + for entry in input { + hashmap.insert(entry.key, entry.value); + } + + // We use prime numbers for testing, + // therefore it is guaranteed that doubling key we get unique value. + let key = input[0].key * 2; + hashmap.insert(key, input[0].value); +} diff --git a/test_programs/execution_success/hashmap/Nargo.toml b/test_programs/execution_success/hashmap/Nargo.toml new file mode 100644 index 00000000000..c09debc9833 --- /dev/null +++ b/test_programs/execution_success/hashmap/Nargo.toml @@ -0,0 +1,6 @@ +[package] +name = "hashmap" +type = "bin" +authors = [""] + +[dependencies] \ No newline at end of file diff --git a/test_programs/execution_success/hashmap/Prover.toml b/test_programs/execution_success/hashmap/Prover.toml new file mode 100644 index 00000000000..84d4c0733e4 --- /dev/null +++ b/test_programs/execution_success/hashmap/Prover.toml @@ -0,0 +1,26 @@ +# Input: 6 key-value entries for hashmap capacity of 8. +# These must be distinct (both key-to-key, and value-to-value) for correct testing. + +[[input]] +key = 2 +value = 17 + +[[input]] +key = 3 +value = 19 + +[[input]] +key = 5 +value = 23 + +[[input]] +key = 7 +value = 29 + +[[input]] +key = 11 +value = 31 + +[[input]] +key = 41 +value = 43 \ No newline at end of file diff --git a/test_programs/execution_success/hashmap/src/main.nr b/test_programs/execution_success/hashmap/src/main.nr new file mode 100644 index 00000000000..597a5c0b7de --- /dev/null +++ b/test_programs/execution_success/hashmap/src/main.nr @@ -0,0 +1,192 @@ +mod utils; + +use dep::std::collections::map::HashMap; +use dep::std::hash::BuildHasherDefault; +use dep::std::hash::pedersen::PedersenHasher; +use dep::std::cmp::Eq; + +use utils::cut; + +type K = Field; +type V = Field; + +// It is more convenient and readable to use structs as input. +struct Entry{ + key: Field, + value: Field +} + +global HASHMAP_CAP = 8; +global HASHMAP_LEN = 6; + +global FIELD_CMP = |a: Field, b: Field| a.lt(b); + +global K_CMP = FIELD_CMP; +global V_CMP = FIELD_CMP; +global KV_CMP = |a: (K, V), b: (K, V)| a.0.lt(b.0); + +global ALLOCATE_HASHMAP = || -> HashMap> + HashMap::default(); + +fn main(input: [Entry; HASHMAP_LEN]) { + test_sequential(input[0].key, input[0].value); + test_multiple_equal_insert(input[1].key, input[1].value); + test_value_override(input[2].key, input[2].value, input[3].value); + test_insert_and_methods(input); + test_hashmaps_equality(input); + test_retain(); + test_iterators(); + test_mut_iterators(); +} + +// Insert, get, remove. +fn test_sequential(key: K, value: V) { + let mut hashmap = ALLOCATE_HASHMAP(); + assert(hashmap.is_empty(), "New HashMap should be empty."); + + hashmap.insert(key, value); + assert(hashmap.len() == 1, "HashMap after one insert should have a length of 1 element."); + + let got = hashmap.get(key); + assert(got.is_some(), "Got none value."); + let got = got.unwrap_unchecked(); + assert(value == got, f"Inserted {value} but got {got} for the same key."); + + hashmap.remove(key); + assert(hashmap.is_empty(), "HashMap after one insert and corresponding removal should be empty."); + let got = hashmap.get(key); + assert(got.is_none(), "Value has been removed, but is still available (not none)."); +} + +// Insert same pair several times. +fn test_multiple_equal_insert(key: K, value: V) { + let mut hashmap = ALLOCATE_HASHMAP(); + assert(hashmap.is_empty(), "New HashMap should be empty."); + + for _ in 0..HASHMAP_LEN { + hashmap.insert(key, value); + } + + let len = hashmap.len(); + assert(len == 1, f"HashMap length must be 1, got {len}."); + + let got = hashmap.get(key); + assert(got.is_some(), "Got none value."); + let got = got.unwrap_unchecked(); + assert(value == got, f"Inserted {value} but got {got} for the same key."); +} + +// Override value for existing pair. +fn test_value_override(key: K, value: V, new_value: V) { + let mut hashmap = ALLOCATE_HASHMAP(); + assert(hashmap.is_empty(), "New hashmap should be empty."); + + hashmap.insert(key, value); + hashmap.insert(key, new_value); + assert(hashmap.len() == 1, "HashMap length is invalid."); + + let got = hashmap.get(key); + assert(got.is_some(), "Got none value."); + let got = got.unwrap_unchecked(); + assert(got == new_value, f"Expected {new_value}, but got {got}."); +} + +// Insert several distinct pairs and test auxiliary methods. +fn test_insert_and_methods(input: [Entry; HASHMAP_LEN]) { + let mut hashmap = ALLOCATE_HASHMAP(); + assert(hashmap.is_empty(), "New HashMap should be empty."); + + for entry in input { + hashmap.insert(entry.key, entry.value); + } + + assert(hashmap.len() == HASHMAP_LEN, "hashmap.len() does not match input lenght."); + + for entry in input { + assert(hashmap.contains_key(entry.key), f"Not found inserted key {entry.key}."); + } + + hashmap.clear(); + assert(hashmap.is_empty(), "HashMap after clear() should be empty."); +} + +// Insert several pairs and test retaining. +fn test_retain() { + let mut hashmap = ALLOCATE_HASHMAP(); + assert(hashmap.is_empty(), "New HashMap should be empty."); + + let (key, value) = (5, 11); + hashmap.insert(key, value); + let (key, value) = (2, 13); + hashmap.insert(key, value); + let (key, value) = (11, 5); + hashmap.insert(key, value); + + let predicate = |key: K, value: V| -> bool {key * value == 55}; + hashmap.retain(predicate); + + assert(hashmap.len() == 2, "HashMap should have retained 2 elements."); + assert(hashmap.get(2).is_none(), "Pair should have been removed, since it does not match predicate."); +} + +// Equality trait check. +fn test_hashmaps_equality(input: [Entry; HASHMAP_LEN]) { + let mut hashmap_1 = ALLOCATE_HASHMAP(); + let mut hashmap_2 = ALLOCATE_HASHMAP(); + + for entry in input { + hashmap_1.insert(entry.key, entry.value); + hashmap_2.insert(entry.key, entry.value); + } + + assert(hashmap_1 == hashmap_2, "HashMaps should be equal."); + + hashmap_2.remove(input[0].key); + + assert(hashmap_1 != hashmap_2, "HashMaps should not be equal."); +} + +// Test entries, keys, values. +fn test_iterators() { + let mut hashmap = ALLOCATE_HASHMAP(); + + hashmap.insert(2, 3); + hashmap.insert(5, 7); + hashmap.insert(11, 13); + + let keys: [K; 3] = cut(hashmap.keys()).map(|k: Option| k.unwrap_unchecked()).sort_via(K_CMP); + let values: [V; 3] = cut(hashmap.values()).map(|v: Option| v.unwrap_unchecked()).sort_via(V_CMP); + let entries: [(K, V); 3] = cut(hashmap.entries()).map(|e: Option<(K, V)>| e.unwrap_unchecked()).sort_via(KV_CMP); + + assert(keys == [2, 5, 11], "Got incorrect iteration of keys."); + assert(values == [3, 7, 13], "Got incorrect iteration of values."); + assert(entries == [(2, 3), (5, 7), (11, 13)], "Got incorrect iteration of entries."); +} + +// Test mutable iteration over keys, values and entries. +fn test_mut_iterators() { + let mut hashmap = ALLOCATE_HASHMAP(); + + hashmap.insert(2, 3); + hashmap.insert(5, 7); + hashmap.insert(11, 13); + + let f = |k: K| -> K{ k * 3}; + hashmap.iter_keys_mut(f); + + let f = |v: V| -> V{ v * 5}; + hashmap.iter_values_mut(f); + + let keys: [K; 3] = cut(hashmap.keys()).map(|k: Option| k.unwrap_unchecked()).sort_via(K_CMP); + let values: [V; 3] = cut(hashmap.values()).map(|v: Option| v.unwrap_unchecked()).sort_via(V_CMP); + + assert(keys == [6, 15, 33], f"Got incorrect iteration of keys: {keys}"); + assert(values == [15, 35, 65], "Got incorrect iteration of values."); + + let f = |k: K, v: V| -> (K, V){(k * 2, v * 2)}; + hashmap.iter_mut(f); + + let entries: [(K, V); 3] = cut(hashmap.entries()).map(|e: Option<(K, V)>| e.unwrap_unchecked()).sort_via(KV_CMP); + + assert(entries == [(12, 30), (30, 70), (66, 130)], "Got incorrect iteration of entries."); +} diff --git a/test_programs/execution_success/hashmap/src/utils.nr b/test_programs/execution_success/hashmap/src/utils.nr new file mode 100644 index 00000000000..45c9ca9bbf7 --- /dev/null +++ b/test_programs/execution_success/hashmap/src/utils.nr @@ -0,0 +1,10 @@ +// Compile-time: cuts the M first elements from the [T; N] array. +pub(crate) fn cut(input: [T; N]) -> [T; M] { + assert(M as u64 < N as u64, "M should be less than N."); + + let mut new = [dep::std::unsafe::zeroed(); M]; + for i in 0..M { + new[i] = input[i]; + } + new +} From 16d5f18c68cc3da1d11c98e101e3942d2437c3a8 Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Fri, 23 Feb 2024 15:06:15 +0000 Subject: [PATCH 2/7] chore(ssa): Remove mem2reg run before flattening (#4415) # Description ## Problem\* Before https://github.com/noir-lang/noir/pull/4240 we needed mem2reg to be run as to not panic when fetching slice lengths. ## Summary\* After the linked PR we have an improved startegy for tracking slice capacities by generating a slice capacities map before merging of values. This should enable us to remove a mem2reg pass that is run before flattening. ## Additional Context ## Documentation\* Check one: - [ ] No documentation needed. - [ ] Documentation included in this PR. - [ ] **[Exceptional Case]** Documentation to be submitted in a separate PR. # PR Checklist\* - [ ] I have tested the changes locally. - [ ] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings. --- compiler/noirc_evaluator/src/ssa.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa.rs b/compiler/noirc_evaluator/src/ssa.rs index d19c4467235..0bb81efe977 100644 --- a/compiler/noirc_evaluator/src/ssa.rs +++ b/compiler/noirc_evaluator/src/ssa.rs @@ -54,11 +54,6 @@ pub(crate) fn optimize_into_acir( .try_run_pass(Ssa::evaluate_assert_constant, "After Assert Constant:")? .try_run_pass(Ssa::unroll_loops, "After Unrolling:")? .run_pass(Ssa::simplify_cfg, "After Simplifying:") - // Run mem2reg before flattening to handle any promotion - // of values that can be accessed after loop unrolling. - // If there are slice mergers uncovered by loop unrolling - // and this pass is missed, slice merging will fail inside of flattening. - .run_pass(Ssa::mem2reg, "After Mem2Reg:") .run_pass(Ssa::flatten_cfg, "After Flattening:") .run_pass(Ssa::remove_bit_shifts, "After Removing Bit Shifts:") // Run mem2reg once more with the flattened CFG to catch any remaining loads/stores From 27c66b3d0741e68ed591ae8a16b47b30bc87175f Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Sat, 24 Feb 2024 14:00:22 +0000 Subject: [PATCH 3/7] fix: remove print from monomorphization pass (#4417) # Description ## Problem\* Resolves ## Summary\* We're currently printing out every expression we're monomorphising. ## Additional Context ## Documentation\* Check one: - [x] No documentation needed. - [ ] Documentation included in this PR. - [ ] **[Exceptional Case]** Documentation to be submitted in a separate PR. # PR Checklist\* - [x] I have tested the changes locally. - [x] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings. --- compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs index 6f59fa13274..d8857f9e599 100644 --- a/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs @@ -128,7 +128,6 @@ impl<'a> FunctionContext<'a> { } fn codegen_expression(&mut self, expr: &Expression) -> Result { - eprintln!("Codegen {expr}"); match expr { Expression::Ident(ident) => Ok(self.codegen_ident(ident)), Expression::Literal(literal) => self.codegen_literal(literal), From 33860678a642a76d8251ef42ffbe6d8a5a013528 Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Sat, 24 Feb 2024 14:15:25 +0000 Subject: [PATCH 4/7] chore: remove unwanted prints (#4419) # Description ## Problem\* Resolves ## Summary\* This removes some unwanted prints which were left in from #4376 ## Additional Context ## Documentation\* Check one: - [x] No documentation needed. - [ ] Documentation included in this PR. - [ ] **[Exceptional Case]** Documentation to be submitted in a separate PR. # PR Checklist\* - [x] I have tested the changes locally. - [x] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings. --- compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs index d8857f9e599..d95295ae3c9 100644 --- a/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs @@ -347,10 +347,8 @@ impl<'a> FunctionContext<'a> { } fn codegen_binary(&mut self, binary: &ast::Binary) -> Result { - eprintln!("Start binary"); let lhs = self.codegen_non_tuple_expression(&binary.lhs)?; let rhs = self.codegen_non_tuple_expression(&binary.rhs)?; - eprintln!("Insert binary"); Ok(self.insert_binary(lhs, binary.operator, rhs, binary.location)) } From 29e9b5e5d0f7a00c806639e900f2f8209675ee0e Mon Sep 17 00:00:00 2001 From: guipublic <47281315+guipublic@users.noreply.github.com> Date: Mon, 26 Feb 2024 14:54:28 +0100 Subject: [PATCH 5/7] chore: do not panic when dividing by zero (#4424) # Description ## Problem\* Resolves #2480 ## Summary\* In BrilligVM, we have now the same behaviour regarding division by 0. Whether it is a field or integer division, we return 0 when dividing by zero. Since we have a constraint which checks that the inverse times itself is 1, this constraint will always fail if we divide by zero (instead of panic or returning an error). ## Additional Context ## Documentation\* Check one: - [X] No documentation needed. - [ ] Documentation included in this PR. - [ ] **[Exceptional Case]** Documentation to be submitted in a separate PR. # PR Checklist\* - [X] I have tested the changes locally. - [X] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings. --- acvm-repo/brillig_vm/src/arithmetic.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/acvm-repo/brillig_vm/src/arithmetic.rs b/acvm-repo/brillig_vm/src/arithmetic.rs index 263a733e3c4..9d7b6fe8f02 100644 --- a/acvm-repo/brillig_vm/src/arithmetic.rs +++ b/acvm-repo/brillig_vm/src/arithmetic.rs @@ -36,18 +36,20 @@ pub(crate) fn evaluate_binary_bigint_op( BinaryIntOp::UnsignedDiv => { let b_mod = b % bit_modulo; if b_mod.is_zero() { - return Err("Division by zero".to_owned()); + BigUint::zero() + } else { + (a % bit_modulo) / b_mod } - (a % bit_modulo) / b_mod } // Perform signed division by first converting a and b to signed integers and then back to unsigned after the operation. BinaryIntOp::SignedDiv => { let b_signed = to_big_signed(b, bit_size); if b_signed.is_zero() { - return Err("Division by zero".to_owned()); + BigUint::zero() + } else { + let signed_div = to_big_signed(a, bit_size) / b_signed; + to_big_unsigned(signed_div, bit_size) } - let signed_div = to_big_signed(a, bit_size) / b_signed; - to_big_unsigned(signed_div, bit_size) } // Perform a == operation, returning 0 or 1 BinaryIntOp::Equals => { From 7cd5fdb3d2a53475b7c8681231d517cab30f9f9b Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Mon, 26 Feb 2024 13:56:14 +0000 Subject: [PATCH 6/7] feat: expose separate functions to compile programs vs contracts in `noir_wasm` (#4413) # Description ## Problem\* Resolves ## Summary\* This PR exposes separate functions to compile contracts vs programs in the wasm compiler. This allows us to simplify various code paths as we don't need to deal with the potential for the two artifact types as this just leads to us asserting types and breaking type safety. ## Additional Context ## Documentation\* Check one: - [x] No documentation needed. - [ ] Documentation included in this PR. - [ ] **[Exceptional Case]** Documentation to be submitted in a separate PR. # PR Checklist\* - [x] I have tested the changes locally. - [x] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings. --- compiler/wasm/src/compile.rs | 212 ++++++++++-------- compiler/wasm/src/compile_new.rs | 75 +++++-- compiler/wasm/src/index.cts | 77 ++++++- compiler/wasm/src/index.mts | 77 ++++++- compiler/wasm/src/lib.rs | 4 +- compiler/wasm/src/noir/noir-wasm-compiler.ts | 61 ++++- compiler/wasm/src/types/noir_artifact.ts | 19 -- .../test/compiler/browser/compile.test.ts | 8 +- .../wasm/test/compiler/node/compile.test.ts | 8 +- .../wasm/test/compiler/shared/compile.test.ts | 10 +- compiler/wasm/test/wasm/browser/index.test.ts | 10 +- compiler/wasm/test/wasm/node/index.test.ts | 10 +- 12 files changed, 377 insertions(+), 194 deletions(-) diff --git a/compiler/wasm/src/compile.rs b/compiler/wasm/src/compile.rs index c8b1680bc00..ca6c8efedb1 100644 --- a/compiler/wasm/src/compile.rs +++ b/compiler/wasm/src/compile.rs @@ -6,8 +6,7 @@ use nargo::artifacts::{ program::ProgramArtifact, }; use noirc_driver::{ - add_dep, compile_contract, compile_main, file_manager_with_stdlib, prepare_crate, - prepare_dependency, CompileOptions, CompiledContract, CompiledProgram, + add_dep, file_manager_with_stdlib, prepare_crate, prepare_dependency, CompileOptions, NOIR_ARTIFACT_VERSION_STRING, }; use noirc_evaluator::errors::SsaReport; @@ -60,51 +59,64 @@ extern "C" { #[derive(Clone, Debug, PartialEq, Eq)] pub type JsDependencyGraph; - #[wasm_bindgen(extends = Object, js_name = "CompileResult", typescript_type = "CompileResult")] + #[wasm_bindgen(extends = Object, js_name = "ProgramCompileResult", typescript_type = "ProgramCompileResult")] #[derive(Clone, Debug, PartialEq, Eq)] - pub type JsCompileResult; + pub type JsCompileProgramResult; #[wasm_bindgen(constructor, js_class = "Object")] - fn constructor() -> JsCompileResult; + fn constructor() -> JsCompileProgramResult; + + #[wasm_bindgen(extends = Object, js_name = "ContractCompileResult", typescript_type = "ContractCompileResult")] + #[derive(Clone, Debug, PartialEq, Eq)] + pub type JsCompileContractResult; + + #[wasm_bindgen(constructor, js_class = "Object")] + fn constructor() -> JsCompileContractResult; } -impl JsCompileResult { - const CONTRACT_PROP: &'static str = "contract"; +impl JsCompileProgramResult { const PROGRAM_PROP: &'static str = "program"; const WARNINGS_PROP: &'static str = "warnings"; - pub fn new(resp: CompileResult) -> JsCompileResult { - let obj = JsCompileResult::constructor(); - match resp { - CompileResult::Contract { contract, warnings } => { - js_sys::Reflect::set( - &obj, - &JsString::from(JsCompileResult::CONTRACT_PROP), - &::from_serde(&contract).unwrap(), - ) - .unwrap(); - js_sys::Reflect::set( - &obj, - &JsString::from(JsCompileResult::WARNINGS_PROP), - &::from_serde(&warnings).unwrap(), - ) - .unwrap(); - } - CompileResult::Program { program, warnings } => { - js_sys::Reflect::set( - &obj, - &JsString::from(JsCompileResult::PROGRAM_PROP), - &::from_serde(&program).unwrap(), - ) - .unwrap(); - js_sys::Reflect::set( - &obj, - &JsString::from(JsCompileResult::WARNINGS_PROP), - &::from_serde(&warnings).unwrap(), - ) - .unwrap(); - } - }; + pub fn new(program: ProgramArtifact, warnings: Vec) -> JsCompileProgramResult { + let obj = JsCompileProgramResult::constructor(); + + js_sys::Reflect::set( + &obj, + &JsString::from(JsCompileProgramResult::PROGRAM_PROP), + &::from_serde(&program).unwrap(), + ) + .unwrap(); + js_sys::Reflect::set( + &obj, + &JsString::from(JsCompileProgramResult::WARNINGS_PROP), + &::from_serde(&warnings).unwrap(), + ) + .unwrap(); + + obj + } +} + +impl JsCompileContractResult { + const CONTRACT_PROP: &'static str = "contract"; + const WARNINGS_PROP: &'static str = "warnings"; + + pub fn new(contract: ContractArtifact, warnings: Vec) -> JsCompileContractResult { + let obj = JsCompileContractResult::constructor(); + + js_sys::Reflect::set( + &obj, + &JsString::from(JsCompileContractResult::CONTRACT_PROP), + &::from_serde(&contract).unwrap(), + ) + .unwrap(); + js_sys::Reflect::set( + &obj, + &JsString::from(JsCompileContractResult::WARNINGS_PROP), + &::from_serde(&warnings).unwrap(), + ) + .unwrap(); obj } @@ -144,73 +156,98 @@ pub(crate) fn parse_all(fm: &FileManager) -> ParsedFiles { fm.as_file_map().all_file_ids().map(|&file_id| (file_id, parse_file(fm, file_id))).collect() } -pub enum CompileResult { - Contract { contract: ContractArtifact, warnings: Vec }, - Program { program: ProgramArtifact, warnings: Vec }, -} - #[wasm_bindgen] -pub fn compile( +pub fn compile_program( entry_point: String, - contracts: Option, dependency_graph: Option, file_source_map: PathToFileSourceMap, -) -> Result { +) -> Result { console_error_panic_hook::set_once(); - - let dependency_graph: DependencyGraph = if let Some(dependency_graph) = dependency_graph { - ::into_serde(&JsValue::from(dependency_graph)) - .map_err(|err| err.to_string())? - } else { - DependencyGraph { root_dependencies: vec![], library_dependencies: HashMap::new() } - }; - - let fm = file_manager_with_source_map(file_source_map); - let parsed_files = parse_all(&fm); - let mut context = Context::new(fm, parsed_files); - - let path = Path::new(&entry_point); - let crate_id = prepare_crate(&mut context, path); - - process_dependency_graph(&mut context, dependency_graph); + let (crate_id, mut context) = prepare_context(entry_point, dependency_graph, file_source_map)?; let compile_options = CompileOptions::default(); - // For now we default to a bounded width of 3, though we can add it as a parameter let expression_width = acvm::acir::circuit::ExpressionWidth::Bounded { width: 3 }; - if contracts.unwrap_or_default() { - let compiled_contract = compile_contract(&mut context, crate_id, &compile_options) + let compiled_program = + noirc_driver::compile_main(&mut context, crate_id, &compile_options, None) .map_err(|errs| { CompileError::with_file_diagnostics( - "Failed to compile contract", + "Failed to compile program", errs, &context.file_manager, ) })? .0; - let optimized_contract = - nargo::ops::transform_contract(compiled_contract, expression_width); + let optimized_program = nargo::ops::transform_program(compiled_program, expression_width); + let warnings = optimized_program.warnings.clone(); - let compile_output = generate_contract_artifact(optimized_contract); - Ok(JsCompileResult::new(compile_output)) - } else { - let compiled_program = compile_main(&mut context, crate_id, &compile_options, None) + Ok(JsCompileProgramResult::new(optimized_program.into(), warnings)) +} + +#[wasm_bindgen] +pub fn compile_contract( + entry_point: String, + dependency_graph: Option, + file_source_map: PathToFileSourceMap, +) -> Result { + console_error_panic_hook::set_once(); + let (crate_id, mut context) = prepare_context(entry_point, dependency_graph, file_source_map)?; + + let compile_options = CompileOptions::default(); + // For now we default to a bounded width of 3, though we can add it as a parameter + let expression_width = acvm::acir::circuit::ExpressionWidth::Bounded { width: 3 }; + + let compiled_contract = + noirc_driver::compile_contract(&mut context, crate_id, &compile_options) .map_err(|errs| { CompileError::with_file_diagnostics( - "Failed to compile program", + "Failed to compile contract", errs, &context.file_manager, ) })? .0; - let optimized_program = nargo::ops::transform_program(compiled_program, expression_width); + let optimized_contract = nargo::ops::transform_contract(compiled_contract, expression_width); - let compile_output = generate_program_artifact(optimized_program); - Ok(JsCompileResult::new(compile_output)) - } + let functions = + optimized_contract.functions.into_iter().map(ContractFunctionArtifact::from).collect(); + + let contract_artifact = ContractArtifact { + noir_version: String::from(NOIR_ARTIFACT_VERSION_STRING), + name: optimized_contract.name, + functions, + events: optimized_contract.events, + file_map: optimized_contract.file_map, + }; + + Ok(JsCompileContractResult::new(contract_artifact, optimized_contract.warnings)) +} + +fn prepare_context( + entry_point: String, + dependency_graph: Option, + file_source_map: PathToFileSourceMap, +) -> Result<(CrateId, Context<'static, 'static>), JsCompileError> { + let dependency_graph: DependencyGraph = if let Some(dependency_graph) = dependency_graph { + ::into_serde(&JsValue::from(dependency_graph)) + .map_err(|err| err.to_string())? + } else { + DependencyGraph { root_dependencies: vec![], library_dependencies: HashMap::new() } + }; + + let fm = file_manager_with_source_map(file_source_map); + let parsed_files = parse_all(&fm); + let mut context = Context::new(fm, parsed_files); + + let path = Path::new(&entry_point); + let crate_id = prepare_crate(&mut context, path); + + process_dependency_graph(&mut context, dependency_graph); + + Ok((crate_id, context)) } // Create a new FileManager with the given source map @@ -270,25 +307,6 @@ fn add_noir_lib(context: &mut Context, library_name: &CrateName) -> CrateId { prepare_dependency(context, &path_to_lib) } -pub(crate) fn generate_program_artifact(program: CompiledProgram) -> CompileResult { - let warnings = program.warnings.clone(); - CompileResult::Program { program: program.into(), warnings } -} - -pub(crate) fn generate_contract_artifact(contract: CompiledContract) -> CompileResult { - let functions = contract.functions.into_iter().map(ContractFunctionArtifact::from).collect(); - - let contract_artifact = ContractArtifact { - noir_version: String::from(NOIR_ARTIFACT_VERSION_STRING), - name: contract.name, - functions, - events: contract.events, - file_map: contract.file_map, - }; - - CompileResult::Contract { contract: contract_artifact, warnings: contract.warnings } -} - #[cfg(test)] mod test { use noirc_driver::prepare_crate; diff --git a/compiler/wasm/src/compile_new.rs b/compiler/wasm/src/compile_new.rs index f8fbed4f470..2a5f7ab6545 100644 --- a/compiler/wasm/src/compile_new.rs +++ b/compiler/wasm/src/compile_new.rs @@ -1,10 +1,12 @@ use crate::compile::{ - file_manager_with_source_map, generate_contract_artifact, generate_program_artifact, parse_all, - JsCompileResult, PathToFileSourceMap, + file_manager_with_source_map, parse_all, JsCompileContractResult, JsCompileProgramResult, + PathToFileSourceMap, }; use crate::errors::{CompileError, JsCompileError}; +use nargo::artifacts::contract::{ContractArtifact, ContractFunctionArtifact}; use noirc_driver::{ add_dep, compile_contract, compile_main, prepare_crate, prepare_dependency, CompileOptions, + NOIR_ARTIFACT_VERSION_STRING, }; use noirc_frontend::{ graph::{CrateId, CrateName}, @@ -92,7 +94,7 @@ impl CompilerContext { pub fn compile_program( mut self, program_width: usize, - ) -> Result { + ) -> Result { let compile_options = CompileOptions::default(); let np_language = acvm::acir::circuit::ExpressionWidth::Bounded { width: program_width }; @@ -110,15 +112,15 @@ impl CompilerContext { .0; let optimized_program = nargo::ops::transform_program(compiled_program, np_language); + let warnings = optimized_program.warnings.clone(); - let compile_output = generate_program_artifact(optimized_program); - Ok(JsCompileResult::new(compile_output)) + Ok(JsCompileProgramResult::new(optimized_program.into(), warnings)) } pub fn compile_contract( mut self, program_width: usize, - ) -> Result { + ) -> Result { let compile_options = CompileOptions::default(); let np_language = acvm::acir::circuit::ExpressionWidth::Bounded { width: program_width }; let root_crate_id = *self.context.root_crate_id(); @@ -136,24 +138,64 @@ impl CompilerContext { let optimized_contract = nargo::ops::transform_contract(compiled_contract, np_language); - let compile_output = generate_contract_artifact(optimized_contract); - Ok(JsCompileResult::new(compile_output)) + let functions = + optimized_contract.functions.into_iter().map(ContractFunctionArtifact::from).collect(); + + let contract_artifact = ContractArtifact { + noir_version: String::from(NOIR_ARTIFACT_VERSION_STRING), + name: optimized_contract.name, + functions, + events: optimized_contract.events, + file_map: optimized_contract.file_map, + }; + + Ok(JsCompileContractResult::new(contract_artifact, optimized_contract.warnings)) } } /// This is a method that exposes the same API as `compile` /// But uses the Context based APi internally #[wasm_bindgen] -pub fn compile_( +pub fn compile_program_( entry_point: String, - contracts: Option, dependency_graph: Option, file_source_map: PathToFileSourceMap, -) -> Result { - use std::collections::HashMap; +) -> Result { + console_error_panic_hook::set_once(); + + let compiler_context = + prepare_compiler_context(entry_point, dependency_graph, file_source_map)?; + let program_width = 3; + compiler_context.compile_program(program_width) +} + +/// This is a method that exposes the same API as `compile` +/// But uses the Context based APi internally +#[wasm_bindgen] +pub fn compile_contract_( + entry_point: String, + dependency_graph: Option, + file_source_map: PathToFileSourceMap, +) -> Result { console_error_panic_hook::set_once(); + let compiler_context = + prepare_compiler_context(entry_point, dependency_graph, file_source_map)?; + let program_width = 3; + + compiler_context.compile_contract(program_width) +} + +/// This is a method that exposes the same API as `prepare_context` +/// But uses the Context based API internally +fn prepare_compiler_context( + entry_point: String, + dependency_graph: Option, + file_source_map: PathToFileSourceMap, +) -> Result { + use std::collections::HashMap; + let dependency_graph: crate::compile::DependencyGraph = if let Some(dependency_graph) = dependency_graph { ::into_serde( @@ -218,14 +260,7 @@ pub fn compile_( } } - let is_contract = contracts.unwrap_or(false); - let program_width = 3; - - if is_contract { - compiler_context.compile_contract(program_width) - } else { - compiler_context.compile_program(program_width) - } + Ok(compiler_context) } #[cfg(test)] diff --git a/compiler/wasm/src/index.cts b/compiler/wasm/src/index.cts index 7c707e662d8..234bfa7280c 100644 --- a/compiler/wasm/src/index.cts +++ b/compiler/wasm/src/index.cts @@ -2,7 +2,7 @@ import { FileManager } from './noir/file-manager/file-manager'; import { createNodejsFileManager } from './noir/file-manager/nodejs-file-manager'; import { NoirWasmCompiler } from './noir/noir-wasm-compiler'; import { LogData, LogFn } from './utils'; -import { CompilationResult } from './types/noir_artifact'; +import { ContractCompilationArtifacts, ProgramCompilationArtifacts } from './types/noir_artifact'; import { inflateDebugSymbols } from './noir/debug'; /** @@ -17,36 +17,86 @@ import { inflateDebugSymbols } from './noir/debug'; * ```typescript * // Node.js * - * import { compile, createFileManager } from '@noir-lang/noir_wasm'; + * import { compile_program, createFileManager } from '@noir-lang/noir_wasm'; * * const fm = createFileManager(myProjectPath); - * const myCompiledCode = await compile(fm); + * const myCompiledCode = await compile_program(fm); * ``` * * ```typescript * // Browser * - * import { compile, createFileManager } from '@noir-lang/noir_wasm'; + * import { compile_program, createFileManager } from '@noir-lang/noir_wasm'; * * const fm = createFileManager('/'); * for (const path of files) { * await fm.writeFile(path, await getFileAsStream(path)); * } - * const myCompiledCode = await compile(fm); + * const myCompiledCode = await compile_program(fm); * ``` */ -async function compile( +async function compile_program( fileManager: FileManager, projectPath?: string, logFn?: LogFn, debugLogFn?: LogFn, -): Promise { +): Promise { + const compiler = await setup_compiler(fileManager, projectPath, logFn, debugLogFn); + return await compiler.compile_program(); +} + +/** + * Compiles a Noir project + * + * @param fileManager - The file manager to use + * @param projectPath - The path to the project inside the file manager. Defaults to the root of the file manager + * @param logFn - A logging function. If not provided, console.log will be used + * @param debugLogFn - A debug logging function. If not provided, logFn will be used + * + * @example + * ```typescript + * // Node.js + * + * import { compile_contract, createFileManager } from '@noir-lang/noir_wasm'; + * + * const fm = createFileManager(myProjectPath); + * const myCompiledCode = await compile_contract(fm); + * ``` + * + * ```typescript + * // Browser + * + * import { compile_contract, createFileManager } from '@noir-lang/noir_wasm'; + * + * const fm = createFileManager('/'); + * for (const path of files) { + * await fm.writeFile(path, await getFileAsStream(path)); + * } + * const myCompiledCode = await compile_contract(fm); + * ``` + */ +async function compile_contract( + fileManager: FileManager, + projectPath?: string, + logFn?: LogFn, + debugLogFn?: LogFn, +): Promise { + const compiler = await setup_compiler(fileManager, projectPath, logFn, debugLogFn); + return await compiler.compile_contract(); +} + +async function setup_compiler( + fileManager: FileManager, + projectPath?: string, + logFn?: LogFn, + debugLogFn?: LogFn, +): Promise { if (logFn && !debugLogFn) { debugLogFn = logFn; } const cjs = await require('../build/cjs'); - const compiler = await NoirWasmCompiler.new( + return await NoirWasmCompiler.new( fileManager, projectPath ?? fileManager.getDataDir(), cjs, @@ -72,9 +122,16 @@ async function compile( }, }, ); - return await compiler.compile(); } const createFileManager = createNodejsFileManager; -export { compile, createFileManager, inflateDebugSymbols, CompilationResult }; +export { + compile_program as compile, + compile_program, + compile_contract, + createFileManager, + inflateDebugSymbols, + ProgramCompilationArtifacts, + ContractCompilationArtifacts, +}; diff --git a/compiler/wasm/src/index.mts b/compiler/wasm/src/index.mts index d4ed0beccfc..326a7337117 100644 --- a/compiler/wasm/src/index.mts +++ b/compiler/wasm/src/index.mts @@ -2,7 +2,7 @@ import { FileManager } from './noir/file-manager/file-manager'; import { createNodejsFileManager } from './noir/file-manager/nodejs-file-manager'; import { NoirWasmCompiler } from './noir/noir-wasm-compiler'; import { LogData, LogFn } from './utils'; -import { CompilationResult } from './types/noir_artifact'; +import { ContractCompilationArtifacts, ProgramCompilationArtifacts } from './types/noir_artifact'; import { inflateDebugSymbols } from './noir/debug'; /** @@ -17,30 +17,80 @@ import { inflateDebugSymbols } from './noir/debug'; * ```typescript * // Node.js * - * import { compile, createFileManager } from '@noir-lang/noir_wasm'; + * import { compile_program, createFileManager } from '@noir-lang/noir_wasm'; * * const fm = createFileManager(myProjectPath); - * const myCompiledCode = await compile(fm); + * const myCompiledCode = await compile_program(fm); * ``` * * ```typescript * // Browser * - * import { compile, createFileManager } from '@noir-lang/noir_wasm'; + * import { compile_program, createFileManager } from '@noir-lang/noir_wasm'; * * const fm = createFileManager('/'); * for (const path of files) { * await fm.writeFile(path, await getFileAsStream(path)); * } - * const myCompiledCode = await compile(fm); + * const myCompiledCode = await compile_program(fm); * ``` */ -async function compile( +async function compile_program( fileManager: FileManager, projectPath?: string, logFn?: LogFn, debugLogFn?: LogFn, -): Promise { +): Promise { + const compiler = await setup_compiler(fileManager, projectPath, logFn, debugLogFn); + return await compiler.compile_program(); +} + +/** + * Compiles a Noir project + * + * @param fileManager - The file manager to use + * @param projectPath - The path to the project inside the file manager. Defaults to the root of the file manager + * @param logFn - A logging function. If not provided, console.log will be used + * @param debugLogFn - A debug logging function. If not provided, logFn will be used + * + * @example + * ```typescript + * // Node.js + * + * import { compile_contract, createFileManager } from '@noir-lang/noir_wasm'; + * + * const fm = createFileManager(myProjectPath); + * const myCompiledCode = await compile_contract(fm); + * ``` + * + * ```typescript + * // Browser + * + * import { compile_contract, createFileManager } from '@noir-lang/noir_wasm'; + * + * const fm = createFileManager('/'); + * for (const path of files) { + * await fm.writeFile(path, await getFileAsStream(path)); + * } + * const myCompiledCode = await compile_contract(fm); + * ``` + */ +async function compile_contract( + fileManager: FileManager, + projectPath?: string, + logFn?: LogFn, + debugLogFn?: LogFn, +): Promise { + const compiler = await setup_compiler(fileManager, projectPath, logFn, debugLogFn); + return await compiler.compile_contract(); +} + +async function setup_compiler( + fileManager: FileManager, + projectPath?: string, + logFn?: LogFn, + debugLogFn?: LogFn, +): Promise { if (logFn && !debugLogFn) { debugLogFn = logFn; } @@ -48,7 +98,7 @@ async function compile( const esm = await import(/* webpackMode: "eager" */ '../build/esm'); await esm.default(); - const compiler = await NoirWasmCompiler.new( + return await NoirWasmCompiler.new( fileManager, projectPath ?? fileManager.getDataDir(), esm, @@ -74,9 +124,16 @@ async function compile( }, }, ); - return await compiler.compile(); } const createFileManager = createNodejsFileManager; -export { compile, createFileManager, inflateDebugSymbols, CompilationResult }; +export { + compile_program as compile, + compile_program, + compile_contract, + createFileManager, + inflateDebugSymbols, + ProgramCompilationArtifacts, + ContractCompilationArtifacts, +}; diff --git a/compiler/wasm/src/lib.rs b/compiler/wasm/src/lib.rs index 174d9b9ce9c..6753faf2009 100644 --- a/compiler/wasm/src/lib.rs +++ b/compiler/wasm/src/lib.rs @@ -18,10 +18,10 @@ mod compile; mod compile_new; mod errors; -pub use compile::compile; +pub use compile::{compile_contract, compile_program}; // Expose the new Context-Centric API -pub use compile_new::{compile_, CompilerContext, CrateIDWrapper}; +pub use compile_new::{compile_contract_, compile_program_, CompilerContext, CrateIDWrapper}; use wasm_bindgen::{prelude::wasm_bindgen, JsValue}; #[derive(Serialize, Deserialize)] diff --git a/compiler/wasm/src/noir/noir-wasm-compiler.ts b/compiler/wasm/src/noir/noir-wasm-compiler.ts index 2a0af5d8fee..1ec3af1fd65 100644 --- a/compiler/wasm/src/noir/noir-wasm-compiler.ts +++ b/compiler/wasm/src/noir/noir-wasm-compiler.ts @@ -6,7 +6,7 @@ import { LocalDependencyResolver } from './dependencies/local-dependency-resolve import { FileManager } from './file-manager/file-manager'; import { Package } from './package'; import { LogFn } from '../utils'; -import { CompilationResult } from '../types/noir_artifact'; +import { ContractCompilationArtifacts, ProgramCompilationArtifacts } from '../types/noir_artifact'; /** Compilation options */ export type NoirWasmCompileOptions = { @@ -84,21 +84,64 @@ export class NoirWasmCompiler { /** * Compile EntryPoint */ + public async compile_program(): Promise { + console.log(`Compiling at ${this.#package.getEntryPointPath()}`); + + if (this.#package.getType() !== 'bin') { + throw new Error(`Expected to find package type "bin" but found ${this.#package.getType()}`); + } + await this.#dependencyManager.resolveDependencies(); + this.#debugLog(`Dependencies: ${this.#dependencyManager.getPackageNames().join(', ')}`); + + try { + const entrypoint = this.#package.getEntryPointPath(); + const deps = { + /* eslint-disable camelcase */ + root_dependencies: this.#dependencyManager.getEntrypointDependencies(), + library_dependencies: this.#dependencyManager.getLibraryDependencies(), + /* eslint-enable camelcase */ + }; + const packageSources = await this.#package.getSources(this.#fm); + const librarySources = ( + await Promise.all( + this.#dependencyManager + .getLibraries() + .map(async ([alias, library]) => await library.package.getSources(this.#fm, alias)), + ) + ).flat(); + [...packageSources, ...librarySources].forEach((sourceFile) => { + this.#debugLog(`Adding source ${sourceFile.path}`); + this.#sourceMap.add_source_code(sourceFile.path, sourceFile.source); + }); + const result = this.#wasmCompiler.compile_program(entrypoint, deps, this.#sourceMap); + + return result; + } catch (err) { + if (err instanceof Error && err.name === 'CompileError') { + const logs = await this.#processCompileError(err); + for (const log of logs) { + this.#log(log); + } + throw new Error(logs.join('\n')); + } + + throw err; + } + } + /** * Compile EntryPoint */ - public async compile(): Promise { + public async compile_contract(): Promise { console.log(`Compiling at ${this.#package.getEntryPointPath()}`); - if (!(this.#package.getType() === 'contract' || this.#package.getType() === 'bin')) { - throw new Error(`Only supports compiling "contract" and "bin" package types (${this.#package.getType()})`); + if (this.#package.getType() !== 'contract') { + throw new Error(`Expected to find package type "contract" but found ${this.#package.getType()}`); } await this.#dependencyManager.resolveDependencies(); this.#debugLog(`Dependencies: ${this.#dependencyManager.getPackageNames().join(', ')}`); try { - const isContract: boolean = this.#package.getType() === 'contract'; - const entrypoint = this.#package.getEntryPointPath(); const deps = { /* eslint-disable camelcase */ @@ -118,11 +161,7 @@ export class NoirWasmCompiler { this.#debugLog(`Adding source ${sourceFile.path}`); this.#sourceMap.add_source_code(sourceFile.path, sourceFile.source); }); - const result = this.#wasmCompiler.compile(entrypoint, isContract, deps, this.#sourceMap); - - if ((isContract && !('contract' in result)) || (!isContract && !('program' in result))) { - throw new Error('Invalid compilation result'); - } + const result = this.#wasmCompiler.compile_contract(entrypoint, deps, this.#sourceMap); return result; } catch (err) { diff --git a/compiler/wasm/src/types/noir_artifact.ts b/compiler/wasm/src/types/noir_artifact.ts index e636212a487..832a6ed9bf9 100644 --- a/compiler/wasm/src/types/noir_artifact.ts +++ b/compiler/wasm/src/types/noir_artifact.ts @@ -180,22 +180,3 @@ export interface ProgramCompilationArtifacts { /** Compilation warnings. */ warnings: Warning[]; } - -/** - * output of Noir Wasm compilation, can be for a contract or lib/binary - */ -export type CompilationResult = ContractCompilationArtifacts | ProgramCompilationArtifacts; - -/** - * Check if it has Contract unique property - */ -export function isContractCompilationArtifacts(artifact: CompilationResult): artifact is ContractCompilationArtifacts { - return (artifact as ContractCompilationArtifacts).contract !== undefined; -} - -/** - * Check if it has Contract unique property - */ -export function isProgramCompilationArtifacts(artifact: CompilationResult): artifact is ProgramCompilationArtifacts { - return (artifact as ProgramCompilationArtifacts).program !== undefined; -} diff --git a/compiler/wasm/test/compiler/browser/compile.test.ts b/compiler/wasm/test/compiler/browser/compile.test.ts index b7e6c27427f..7d4b3da55aa 100644 --- a/compiler/wasm/test/compiler/browser/compile.test.ts +++ b/compiler/wasm/test/compiler/browser/compile.test.ts @@ -1,7 +1,7 @@ /* eslint-disable @typescript-eslint/ban-ts-comment */ import { getPaths } from '../../shared'; import { expect } from '@esm-bundle/chai'; -import { compile, createFileManager } from '@noir-lang/noir_wasm'; +import { compile_program, compile_contract, createFileManager } from '@noir-lang/noir_wasm'; import { ContractArtifact, ProgramArtifact } from '../../../src/types/noir_artifact'; import { shouldCompileContractIdentically, shouldCompileProgramIdentically } from '../shared/compile.test'; @@ -33,7 +33,7 @@ describe('noir-compiler/browser', () => { await fm.writeFile(path, (await getFile(path)).body as ReadableStream); } const nargoArtifact = (await getPrecompiledSource(simpleScriptExpectedArtifact)) as ProgramArtifact; - const noirWasmArtifact = await compile(fm, '/fixtures/simple'); + const noirWasmArtifact = await compile_program(fm, '/fixtures/simple'); return { nargoArtifact, noirWasmArtifact }; }, @@ -51,7 +51,7 @@ describe('noir-compiler/browser', () => { await fm.writeFile(path, (await getFile(path)).body as ReadableStream); } const nargoArtifact = (await getPrecompiledSource(depsScriptExpectedArtifact)) as ProgramArtifact; - const noirWasmArtifact = await compile(fm, '/fixtures/with-deps'); + const noirWasmArtifact = await compile_program(fm, '/fixtures/with-deps'); return { nargoArtifact, noirWasmArtifact }; }, @@ -69,7 +69,7 @@ describe('noir-compiler/browser', () => { await fm.writeFile(path, (await getFile(path)).body as ReadableStream); } const nargoArtifact = (await getPrecompiledSource(contractExpectedArtifact)) as ContractArtifact; - const noirWasmArtifact = await compile(fm, '/fixtures/noir-contract'); + const noirWasmArtifact = await compile_contract(fm, '/fixtures/noir-contract'); return { nargoArtifact, noirWasmArtifact }; }, diff --git a/compiler/wasm/test/compiler/node/compile.test.ts b/compiler/wasm/test/compiler/node/compile.test.ts index 9af98195825..811dc95ce16 100644 --- a/compiler/wasm/test/compiler/node/compile.test.ts +++ b/compiler/wasm/test/compiler/node/compile.test.ts @@ -2,7 +2,7 @@ import { join, resolve } from 'path'; import { getPaths } from '../../shared'; import { expect } from 'chai'; -import { compile, createFileManager } from '@noir-lang/noir_wasm'; +import { compile_program, compile_contract, createFileManager } from '@noir-lang/noir_wasm'; import { readFile } from 'fs/promises'; import { ContractArtifact, ProgramArtifact } from '../../../src/types/noir_artifact'; import { shouldCompileContractIdentically, shouldCompileProgramIdentically } from '../shared/compile.test'; @@ -15,7 +15,7 @@ describe('noir-compiler/node', () => { const fm = createFileManager(simpleScriptProjectPath); const nargoArtifact = JSON.parse((await readFile(simpleScriptExpectedArtifact)).toString()) as ProgramArtifact; - const noirWasmArtifact = await compile(fm); + const noirWasmArtifact = await compile_program(fm); return { nargoArtifact, noirWasmArtifact }; }, expect); @@ -24,7 +24,7 @@ describe('noir-compiler/node', () => { const fm = createFileManager(depsScriptProjectPath); const nargoArtifact = JSON.parse((await readFile(depsScriptExpectedArtifact)).toString()) as ProgramArtifact; - const noirWasmArtifact = await compile(fm); + const noirWasmArtifact = await compile_program(fm); return { nargoArtifact, noirWasmArtifact }; }, expect); @@ -33,7 +33,7 @@ describe('noir-compiler/node', () => { const fm = createFileManager(contractProjectPath); const nargoArtifact = JSON.parse((await readFile(contractExpectedArtifact)).toString()) as ContractArtifact; - const noirWasmArtifact = await compile(fm); + const noirWasmArtifact = await compile_contract(fm); return { nargoArtifact, noirWasmArtifact }; }, expect); }); diff --git a/compiler/wasm/test/compiler/shared/compile.test.ts b/compiler/wasm/test/compiler/shared/compile.test.ts index 88e8e8c8e5a..52cef14968b 100644 --- a/compiler/wasm/test/compiler/shared/compile.test.ts +++ b/compiler/wasm/test/compiler/shared/compile.test.ts @@ -1,4 +1,4 @@ -import { CompilationResult, inflateDebugSymbols } from '@noir-lang/noir_wasm'; +import { inflateDebugSymbols } from '@noir-lang/noir_wasm'; import { type expect as Expect } from 'chai'; import { ContractArtifact, @@ -11,7 +11,7 @@ import { } from '../../../src/types/noir_artifact'; export function shouldCompileProgramIdentically( - compileFn: () => Promise<{ nargoArtifact: ProgramArtifact; noirWasmArtifact: CompilationResult }>, + compileFn: () => Promise<{ nargoArtifact: ProgramArtifact; noirWasmArtifact: ProgramCompilationArtifacts }>, expect: typeof Expect, timeout = 5000, ) { @@ -24,7 +24,7 @@ export function shouldCompileProgramIdentically( normalizeVersion(nargoArtifact); // Prepare noir-wasm artifact - const noirWasmProgram = (noirWasmArtifact as unknown as ProgramCompilationArtifacts).program; + const noirWasmProgram = noirWasmArtifact.program; expect(noirWasmProgram).not.to.be.undefined; const [_noirWasmDebugInfos, norWasmFileMap] = deleteProgramDebugMetadata(noirWasmProgram); normalizeVersion(noirWasmProgram); @@ -47,7 +47,7 @@ export function shouldCompileProgramIdentically( } export function shouldCompileContractIdentically( - compileFn: () => Promise<{ nargoArtifact: ContractArtifact; noirWasmArtifact: CompilationResult }>, + compileFn: () => Promise<{ nargoArtifact: ContractArtifact; noirWasmArtifact: ContractCompilationArtifacts }>, expect: typeof Expect, timeout = 5000, ) { @@ -60,7 +60,7 @@ export function shouldCompileContractIdentically( normalizeVersion(nargoArtifact); // Prepare noir-wasm artifact - const noirWasmContract = (noirWasmArtifact as unknown as ContractCompilationArtifacts).contract; + const noirWasmContract = noirWasmArtifact.contract; expect(noirWasmContract).not.to.be.undefined; const [noirWasmDebugInfos, norWasmFileMap] = deleteContractDebugMetadata(noirWasmContract); normalizeVersion(noirWasmContract); diff --git a/compiler/wasm/test/wasm/browser/index.test.ts b/compiler/wasm/test/wasm/browser/index.test.ts index 3122fa57945..b59b4ae417a 100644 --- a/compiler/wasm/test/wasm/browser/index.test.ts +++ b/compiler/wasm/test/wasm/browser/index.test.ts @@ -2,7 +2,7 @@ import { getPaths } from '../../shared'; import { expect } from '@esm-bundle/chai'; -import init, { compile, PathToFileSourceMap, compile_, CompilerContext } from '../../../build/esm'; +import init, { compile_program, PathToFileSourceMap, compile_program_, CompilerContext } from '../../../build/esm'; // @ts-ignore await init(); @@ -35,7 +35,7 @@ describe('noir wasm compilation', () => { it('matching nargos compilation', async () => { const sourceMap = new PathToFileSourceMap(); sourceMap.add_source_code('script/main.nr', await getFileAsString(simpleScriptSourcePath)); - const wasmCircuit = compile('script/main.nr', undefined, undefined, sourceMap); + const wasmCircuit = compile_program('script/main.nr', undefined, sourceMap); const cliCircuit = await getPrecompiledSource(simpleScriptExpectedArtifact); if (!('program' in wasmCircuit)) { @@ -58,9 +58,8 @@ describe('noir wasm compilation', () => { }); it('matching nargos compilation', async () => { - const wasmCircuit = compile( + const wasmCircuit = compile_program( 'script/main.nr', - false, { root_dependencies: ['lib_a'], library_dependencies: { @@ -132,9 +131,8 @@ describe('noir wasm compilation', () => { }).timeout(60 * 20e3); it('matching nargos compilation - context-implementation-compile-api', async () => { - const wasmCircuit = await compile_( + const wasmCircuit = await compile_program_( 'script/main.nr', - false, { root_dependencies: ['lib_a'], library_dependencies: { diff --git a/compiler/wasm/test/wasm/node/index.test.ts b/compiler/wasm/test/wasm/node/index.test.ts index c73ce7477e5..23c87cc059a 100644 --- a/compiler/wasm/test/wasm/node/index.test.ts +++ b/compiler/wasm/test/wasm/node/index.test.ts @@ -3,7 +3,7 @@ import { readFileSync } from 'fs'; import { join, resolve } from 'path'; import { expect } from 'chai'; -import { compile, PathToFileSourceMap, compile_, CompilerContext } from '../../../build/cjs'; +import { compile_program, PathToFileSourceMap, compile_program_, CompilerContext } from '../../../build/cjs'; const basePath = resolve(join(__dirname, '../../')); const { @@ -26,7 +26,7 @@ describe('noir wasm compilation', () => { it('matching nargos compilation', async () => { const sourceMap = new PathToFileSourceMap(); sourceMap.add_source_code(simpleScriptSourcePath, readFileSync(simpleScriptSourcePath, 'utf-8')); - const wasmCircuit = compile(simpleScriptSourcePath, undefined, undefined, sourceMap); + const wasmCircuit = compile_program(simpleScriptSourcePath, undefined, sourceMap); const cliCircuit = await getPrecompiledSource(simpleScriptExpectedArtifact); if (!('program' in wasmCircuit)) { @@ -49,9 +49,8 @@ describe('noir wasm compilation', () => { }); it('matching nargos compilation', async () => { - const wasmCircuit = compile( + const wasmCircuit = compile_program( 'script/main.nr', - false, { root_dependencies: ['lib_a'], library_dependencies: { @@ -123,9 +122,8 @@ describe('noir wasm compilation', () => { }).timeout(60 * 20e3); it('matching nargos compilation - context-implementation-compile-api', async () => { - const wasmCircuit = await compile_( + const wasmCircuit = await compile_program_( 'script/main.nr', - false, { root_dependencies: ['lib_a'], library_dependencies: { From 176fab42970ff0a9797b7f8c7ce53817e7d85b90 Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Mon, 26 Feb 2024 14:05:41 +0000 Subject: [PATCH 7/7] chore(ci): prevent msrv checks from blocking PRs (#4414) # Description ## Problem\* Resolves ## Summary\* Current the MSRV check CI runs on every PR so if one of our dependencies breaks us, all merges halt until we fix this. This is unnecessary as we only need to stop publishing releases and normal development work can continue. This PR switches this workflow to instead run only on master and on a nightly schedule. If the workflow fails then an issue will be raised. ## Additional Context ## Documentation\* Check one: - [x] No documentation needed. - [ ] Documentation included in this PR. - [ ] **[Exceptional Case]** Documentation to be submitted in a separate PR. # PR Checklist\* - [x] I have tested the changes locally. - [x] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings. Co-authored-by: kevaundray --- .github/ACVM_NOT_PUBLISHABLE.md | 13 +++++++ .../workflows/test-rust-workspace-msrv.yml | 37 +++++++++++++------ .github/workflows/test-rust-workspace.yml | 20 +++++----- cspell.json | 1 + 4 files changed, 49 insertions(+), 22 deletions(-) create mode 100644 .github/ACVM_NOT_PUBLISHABLE.md diff --git a/.github/ACVM_NOT_PUBLISHABLE.md b/.github/ACVM_NOT_PUBLISHABLE.md new file mode 100644 index 00000000000..e7eacb3b523 --- /dev/null +++ b/.github/ACVM_NOT_PUBLISHABLE.md @@ -0,0 +1,13 @@ +--- +title: "ACVM crates are not publishable" +assignees: TomAFrench kevaundray savio-sou +--- + + +The ACVM crates are currently unpublishable, making a release will NOT push our crates to crates.io. + +This is likely due to a crate we depend on bumping its MSRV above our own. Our lockfile is not taken into account when publishing to crates.io (as people downloading our crate don't use it) so we need to be able to use the most up to date versions of our dependencies (including transient dependencies) specified. + +Check the [MSRV check]({{env.WORKFLOW_URL}}) workflow for details. + +This issue was raised by the workflow `{{env.WORKFLOW_NAME}}` diff --git a/.github/workflows/test-rust-workspace-msrv.yml b/.github/workflows/test-rust-workspace-msrv.yml index 061fc65ca8b..0b2855fa834 100644 --- a/.github/workflows/test-rust-workspace-msrv.yml +++ b/.github/workflows/test-rust-workspace-msrv.yml @@ -6,8 +6,9 @@ name: Test (MSRV check) # We must then always be able to build the workspace using the latest versions of all of our dependencies, so we explicitly update them and build in this workflow. on: - pull_request: - merge_group: + schedule: + # Run a nightly check at 2 AM UTC + - cron: "0 2 * * *" push: branches: - master @@ -100,13 +101,25 @@ jobs: - run-tests steps: - - name: Report overall success - run: | - if [[ $FAIL == true ]]; then - exit 1 - else - exit 0 - fi - env: - # We treat any cancelled, skipped or failing jobs as a failure for the workflow as a whole. - FAIL: ${{ contains(needs.*.result, 'failure') || contains(needs.*.result, 'cancelled') || contains(needs.*.result, 'skipped') }} + - name: Report overall success + run: | + if [[ $FAIL == true ]]; then + exit 1 + else + exit 0 + fi + env: + # We treat any cancelled, skipped or failing jobs as a failure for the workflow as a whole. + FAIL: ${{ contains(needs.*.result, 'failure') || contains(needs.*.result, 'cancelled') || contains(needs.*.result, 'skipped') }} + + # Raise an issue if the tests failed + - name: Alert on failed publish + uses: JasonEtco/create-an-issue@v2 + if: ${{ failure() }} + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + WORKFLOW_NAME: ${{ github.workflow }} + WORKFLOW_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} + with: + update_existing: true + filename: .github/JS_PUBLISH_FAILED.md \ No newline at end of file diff --git a/.github/workflows/test-rust-workspace.yml b/.github/workflows/test-rust-workspace.yml index c12dcaba0ba..22684de3044 100644 --- a/.github/workflows/test-rust-workspace.yml +++ b/.github/workflows/test-rust-workspace.yml @@ -88,13 +88,13 @@ jobs: - run-tests steps: - - name: Report overall success - run: | - if [[ $FAIL == true ]]; then - exit 1 - else - exit 0 - fi - env: - # We treat any cancelled, skipped or failing jobs as a failure for the workflow as a whole. - FAIL: ${{ contains(needs.*.result, 'failure') || contains(needs.*.result, 'cancelled') || contains(needs.*.result, 'skipped') }} + - name: Report overall success + run: | + if [[ $FAIL == true ]]; then + exit 1 + else + exit 0 + fi + env: + # We treat any cancelled, skipped or failing jobs as a failure for the workflow as a whole. + FAIL: ${{ contains(needs.*.result, 'failure') || contains(needs.*.result, 'cancelled') || contains(needs.*.result, 'skipped') }} diff --git a/cspell.json b/cspell.json index be6b7c5c7e8..23659b39c68 100644 --- a/cspell.json +++ b/cspell.json @@ -118,6 +118,7 @@ "monomorphizes", "monomorphizing", "montcurve", + "MSRV", "nand", "nargo", "neovim",