From fb5d16b55775e8b81bc1b651ab107d9db19315a5 Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Thu, 24 Oct 2024 18:54:39 +0100 Subject: [PATCH 1/8] chore: run tests in metaprogramming.rs (#6339) # Description ## Problem\* Resolves ## Summary\* These tests weren't being run because of a missing `mod metaprogramming;`. I've also added a test for a requested feature from another team. ## Additional Context ## Documentation\* Check one: - [x] No documentation needed. - [ ] Documentation included in this PR. - [ ] **[For Experimental Features]** Documentation to be submitted in a separate PR. # PR Checklist\* - [x] I have tested the changes locally. - [x] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings. --- compiler/noirc_frontend/src/tests.rs | 1 + .../src/tests/metaprogramming.rs | 26 +++++++++++++++++-- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index ce4ad4d1bb9..0e0ad5b1a3f 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -2,6 +2,7 @@ mod bound_checks; mod imports; +mod metaprogramming; mod name_shadowing; mod references; mod traits; diff --git a/compiler/noirc_frontend/src/tests/metaprogramming.rs b/compiler/noirc_frontend/src/tests/metaprogramming.rs index ec52310b3d6..a5241cc26ed 100644 --- a/compiler/noirc_frontend/src/tests/metaprogramming.rs +++ b/compiler/noirc_frontend/src/tests/metaprogramming.rs @@ -1,6 +1,9 @@ -use crate::hir::def_collector::dc_crate::CompilationError; +use crate::hir::{ + def_collector::dc_crate::CompilationError, resolution::errors::ResolverError, + type_check::TypeCheckError, +}; -use super::get_program_errors; +use super::{assert_no_errors, get_program_errors}; // Regression for #5388 #[test] @@ -74,3 +77,22 @@ fn unquoted_integer_as_integer_token() { assert_no_errors(src); } + +#[test] +fn allows_references_to_structs_generated_by_macros() { + let src = r#" + comptime fn make_new_struct(_s: StructDefinition) -> Quoted { + quote { struct Bar {} } + } + + #[make_new_struct] + struct Foo {} + + fn main() { + let _ = Foo {}; + let _ = Bar {}; + } + "#; + + assert_no_errors(src); +} From 52f7c0b67fa2f70848512c87fabcefc4c5426dd1 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Thu, 24 Oct 2024 15:13:51 -0300 Subject: [PATCH 2/8] feat: let the formatter remove lambda block braces for single-statement blocks (#6335) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit …nt blocks # Description ## Problem Not a problem, but rustfmt seems to do the same and it can help reduce some of the code noise. ## Summary ## Additional Context Maybe this feature is controversial? I didn't find a way to turn it off in rustfmt. ## Documentation Check one: - [x] No documentation needed. - [ ] Documentation included in this PR. - [ ] **[For Experimental Features]** Documentation to be submitted in a separate PR. # PR Checklist - [x] I have tested the changes locally. - [x] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings. --- .../arithmetic_generics/src/main.nr | 10 ++-- .../comptime_closures/src/main.nr | 2 +- .../inner_outer_cl/src/main.nr | 2 +- .../use_callers_scope/src/main.nr | 2 +- .../higher_order_functions/src/main.nr | 2 +- tooling/nargo_fmt/src/chunks.rs | 55 ++++++++++++++++-- tooling/nargo_fmt/src/formatter/expression.rs | 57 +++++++++++++++++-- tooling/nargo_fmt/tests/expected/contract.nr | 2 +- 8 files changed, 112 insertions(+), 20 deletions(-) diff --git a/test_programs/compile_success_empty/arithmetic_generics/src/main.nr b/test_programs/compile_success_empty/arithmetic_generics/src/main.nr index f599d2879ee..ae50748c8e7 100644 --- a/test_programs/compile_success_empty/arithmetic_generics/src/main.nr +++ b/test_programs/compile_success_empty/arithmetic_generics/src/main.nr @@ -77,25 +77,25 @@ fn equiv_trans( x: Equiv, y: Equiv, ) -> Equiv, Equiv), V, (Equiv, Equiv)> { - Equiv { to_: |z| { y.to(x.to(z)) }, fro_: |z| { x.fro(y.fro(z)) } } + Equiv { to_: |z| y.to(x.to(z)), fro_: |z| x.fro(y.fro(z)) } } fn mul_one_r() -> Equiv, (), W, ()> { - Equiv { to_: |_x| { W {} }, fro_: |_x| { W {} } } + Equiv { to_: |_x| W {}, fro_: |_x| W {} } } fn add_equiv_r( _: Equiv, EN, W, EM>, ) -> Equiv, (), W, ()> { - Equiv { to_: |_x| { W {} }, fro_: |_x| { W {} } } + Equiv { to_: |_x| W {}, fro_: |_x| W {} } } fn mul_comm() -> Equiv, (), W, ()> { - Equiv { to_: |_x| { W {} }, fro_: |_x| { W {} } } + Equiv { to_: |_x| W {}, fro_: |_x| W {} } } fn mul_add() -> Equiv, (), W, ()> { - Equiv { to_: |_x| { W {} }, fro_: |_x| { W {} } } + Equiv { to_: |_x| W {}, fro_: |_x| W {} } } // (N + 1) * N == N * N + N diff --git a/test_programs/compile_success_empty/comptime_closures/src/main.nr b/test_programs/compile_success_empty/comptime_closures/src/main.nr index 132c6df6c91..e8a0366a304 100644 --- a/test_programs/compile_success_empty/comptime_closures/src/main.nr +++ b/test_programs/compile_success_empty/comptime_closures/src/main.nr @@ -6,7 +6,7 @@ fn main() { fn closure_test(mut x: Field) { let one = 1; - let add1 = |z| { (|| { *z += one; })() }; + let add1 = |z| (|| { *z += one; })(); let two = 2; let add2 = |z| { *z = *z + two; }; diff --git a/test_programs/compile_success_empty/inner_outer_cl/src/main.nr b/test_programs/compile_success_empty/inner_outer_cl/src/main.nr index 57890cd6b67..17cbabec48e 100644 --- a/test_programs/compile_success_empty/inner_outer_cl/src/main.nr +++ b/test_programs/compile_success_empty/inner_outer_cl/src/main.nr @@ -2,7 +2,7 @@ fn main() { let z1 = 0; let z2 = 1; let cl_outer = |x| { - let cl_inner = |y| { x + y + z2 }; + let cl_inner = |y| x + y + z2; cl_inner(1) + z1 }; let result = cl_outer(1); diff --git a/test_programs/compile_success_empty/use_callers_scope/src/main.nr b/test_programs/compile_success_empty/use_callers_scope/src/main.nr index bc93bc5187e..26429997f0e 100644 --- a/test_programs/compile_success_empty/use_callers_scope/src/main.nr +++ b/test_programs/compile_success_empty/use_callers_scope/src/main.nr @@ -19,7 +19,7 @@ mod bar { // Ensure closures can still access Bar even // though `map` separates them from `fn_attr`. - let _ = &[1, 2, 3].map(|_| { quote { Bar }.as_type() }); + let _ = &[1, 2, 3].map(|_| quote { Bar }.as_type()); } // use_callers_scope should also work nested diff --git a/test_programs/execution_success/higher_order_functions/src/main.nr b/test_programs/execution_success/higher_order_functions/src/main.nr index 6d094946f6c..1f9598f8591 100644 --- a/test_programs/execution_success/higher_order_functions/src/main.nr +++ b/test_programs/execution_success/higher_order_functions/src/main.nr @@ -5,7 +5,7 @@ fn main(w: Field) -> pub Field { assert(twice(|x| x * 2, 5) == 20); assert((|x, y| x + y + 1)(2, 3) == 6); // nested lambdas - assert((|a, b| { a + (|c| c + 2)(b) })(0, 1) == 3); + assert((|a, b| a + (|c| c + 2)(b))(0, 1) == 3); // Closures: let a = 42; let g = || a; diff --git a/tooling/nargo_fmt/src/chunks.rs b/tooling/nargo_fmt/src/chunks.rs index 073e03ea74a..d4fb540a221 100644 --- a/tooling/nargo_fmt/src/chunks.rs +++ b/tooling/nargo_fmt/src/chunks.rs @@ -466,7 +466,11 @@ pub(crate) enum GroupKind { /// fit in the current line and it's not a block, instead of splitting that expression /// somewhere that's probably undesired, we'll "turn it" into a block /// (write the "{" and "}" delimiters) and write the lambda body in the next line. - LambdaBody { is_block: bool }, + LambdaBody { + block_statement_count: Option, + has_comments: bool, + lambda_has_return_type: bool, + }, /// A method call. /// We track all this information to see, if we end up needing to format this call /// in multiple lines, if we can write everything up to the left parentheses (inclusive) @@ -762,14 +766,14 @@ impl<'a> Formatter<'a> { // If a lambda body doesn't fit in the current line and it's not a block, // we can turn it into a block and write it in the next line, so its contents fit. - if let GroupKind::LambdaBody { is_block: false } = group.kind { + if let GroupKind::LambdaBody { block_statement_count: None, .. } = group.kind { // Try to format it again in the next line, but we don't want to recurse // infinitely so we change the group kind. group.kind = GroupKind::Regular; self.write("{"); self.trim_spaces(); self.increase_indentation(); - self.write_line(); + self.write_line_without_skipping_whitespace_and_comments(); self.write_indentation(); self.format_chunk_group_impl(group); @@ -803,7 +807,7 @@ impl<'a> Formatter<'a> { // ) let comma_trimmed = self.trim_comma(); self.decrease_indentation(); - self.write_line(); + self.write_line_without_skipping_whitespace_and_comments(); self.write_indentation(); self.write("}"); if comma_trimmed { @@ -816,6 +820,24 @@ impl<'a> Formatter<'a> { return; } + // At this point we determined we are going to write this group in a single line. + // If the current group is a lambda body that is a block with a single statement, like this: + // + // |x| { 1 + 2 } + // + // given that we determined the block fits the current line, if we remove the surrounding + // `{ .. }` it will still fit the current line, and reduce some noise from the code + // (this is what rustfmt seems to do too). + if let GroupKind::LambdaBody { + block_statement_count: Some(1), + has_comments: false, + lambda_has_return_type: false, + } = group.kind + { + self.format_lambda_body_removing_braces(group); + return; + } + self.format_chunk_group_in_one_line(group); } @@ -939,6 +961,31 @@ impl<'a> Formatter<'a> { } } + fn format_lambda_body_removing_braces(&mut self, group: ChunkGroup) { + // Write to an intermediate string so we can remove the braces if needed. + let text_chunk = self.chunk_formatter().chunk(|formatter| { + formatter.format_chunk_group_in_one_line(group); + }); + let string = text_chunk.string; + + // Don't remove the braces if the lambda's body is a Semi expression. + if string.ends_with("; }") || string.ends_with("; },") { + self.write(&string); + return; + } + + let string = string.strip_prefix("{ ").unwrap(); + + // The lambda might have a trailing comma if it's inside an arguments list + if let Some(string) = string.strip_suffix(" },") { + self.write(string); + self.write(","); + } else { + let string = string.strip_suffix(" }").unwrap(); + self.write(string); + } + } + /// Appends the string to the current buffer line by line, with some pre-checks. fn write_chunk_lines(&mut self, string: &str) { for (index, line) in string.lines().enumerate() { diff --git a/tooling/nargo_fmt/src/formatter/expression.rs b/tooling/nargo_fmt/src/formatter/expression.rs index 6f7ebf56348..80b9886c047 100644 --- a/tooling/nargo_fmt/src/formatter/expression.rs +++ b/tooling/nargo_fmt/src/formatter/expression.rs @@ -198,6 +198,8 @@ impl<'a, 'b> ChunkFormatter<'a, 'b> { fn format_lambda(&mut self, lambda: Lambda) -> FormattedLambda { let mut group = ChunkGroup::new(); + let lambda_has_return_type = lambda.return_type.typ != UnresolvedTypeData::Unspecified; + let params_and_return_type_chunk = self.chunk(|formatter| { formatter.write_token(Token::Pipe); for (index, (pattern, typ)) in lambda.parameters.into_iter().enumerate() { @@ -218,7 +220,7 @@ impl<'a, 'b> ChunkFormatter<'a, 'b> { } formatter.write_token(Token::Pipe); formatter.write_space(); - if lambda.return_type.typ != UnresolvedTypeData::Unspecified { + if lambda_has_return_type { formatter.write_token(Token::Arrow); formatter.write_space(); formatter.format_type(lambda.return_type); @@ -230,16 +232,27 @@ impl<'a, 'b> ChunkFormatter<'a, 'b> { group.text(params_and_return_type_chunk); - let body_is_block = matches!(lambda.body.kind, ExpressionKind::Block(..)); + let block_statement_count = if let ExpressionKind::Block(block) = &lambda.body.kind { + Some(block.statements.len()) + } else { + None + }; let mut body_group = ChunkGroup::new(); - body_group.kind = GroupKind::LambdaBody { is_block: body_is_block }; + let comments_count_before_body = self.written_comments_count; self.format_expression(lambda.body, &mut body_group); + + body_group.kind = GroupKind::LambdaBody { + block_statement_count, + has_comments: self.written_comments_count > comments_count_before_body, + lambda_has_return_type, + }; + group.group(body_group); let first_line_width = params_and_return_type_chunk_width - + (if body_is_block { + + (if block_statement_count.is_some() { // 1 because we already have `|param1, param2, ..., paramN| ` (including the space) // so all that's left is a `{`. 1 @@ -1980,12 +1993,44 @@ global y = 1; } #[test] - fn format_lambda_with_block() { + fn format_lambda_with_block_simplifies() { let src = "global x = | | { 1 } ;"; - let expected = "global x = || { 1 };\n"; + let expected = "global x = || 1;\n"; + assert_format(src, expected); + } + + #[test] + fn format_lambda_with_block_does_not_simplify_if_it_ends_with_semicolon() { + let src = "global x = | | { 1; } ;"; + let expected = "global x = || { 1; };\n"; + assert_format(src, expected); + } + + #[test] + fn format_lambda_with_block_does_not_simplify_if_it_has_return_type() { + let src = "global x = | | -> i32 { 1 } ;"; + let expected = "global x = || -> i32 { 1 };\n"; + assert_format(src, expected); + } + + #[test] + fn format_lambda_with_simplifies_block_with_quote() { + let src = "global x = | | { quote { 1 } } ;"; + let expected = "global x = || quote { 1 };\n"; assert_format(src, expected); } + #[test] + fn format_lambda_with_block_simplifies_inside_arguments_list() { + let src = "global x = some_call(this_is_a_long_argument, | | { 1 });"; + let expected = "global x = some_call( + this_is_a_long_argument, + || 1, +); +"; + assert_format_with_max_width(src, expected, 20); + } + #[test] fn format_lambda_with_block_multiple_statements() { let src = "global x = | a, b | { 1; 2 } ;"; diff --git a/tooling/nargo_fmt/tests/expected/contract.nr b/tooling/nargo_fmt/tests/expected/contract.nr index ad53e61c911..86f8d56b62d 100644 --- a/tooling/nargo_fmt/tests/expected/contract.nr +++ b/tooling/nargo_fmt/tests/expected/contract.nr @@ -34,7 +34,7 @@ contract Benchmarking { notes: Map::new( context, 1, - |context, slot| { PrivateSet::new(context, slot, ValueNoteMethods) }, + |context, slot| PrivateSet::new(context, slot, ValueNoteMethods), ), balances: Map::new( context, From 3299c25cefb6e3eb4b55396b2f842138b658e42f Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Thu, 24 Oct 2024 15:16:31 -0300 Subject: [PATCH 3/8] fix: better formatting of leading/trailing line/block comments in expression lists (#6338) # Description ## Problem While fixing the bug [this PR](https://github.com/noir-lang/noir/pull/6337) was fixing I noticed that the formatter destroyed some array formatting, like this one: https://github.com/noir-lang/noir_json_parser/blob/bc7094394baeaa185c5bf56ae806e302c786bdd3/src/_string_tools/slice_packed_field.nr#L13-L19 ## Summary I decided to try to fix this because it's bad if the formatter doesn't respect this initial formatting. And this change if for any expression list, so it applies to arrays, tuples, call arguments, etc, so we only need to fix this once. ## Additional Context With this some leading spaces surrounding block comments are gone, but I think it's better because for example array literals don't have spaces after `[` and before `]`, and before this PR the formatter would generate `[ /* comment */ 1]` while now it generates `[/* comment */ 1]` which I think looks better (and is what rustfmt does too). I think I didn't want to spend too much time on these details on the initial formatter pass to avoid getting into an infinite improvement loop. ## Documentation Check one: - [x] No documentation needed. - [ ] Documentation included in this PR. - [ ] **[For Experimental Features]** Documentation to be submitted in a separate PR. # PR Checklist - [x] I have tested the changes locally. - [x] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings. --- tooling/nargo_fmt/src/chunks.rs | 15 +++- .../src/formatter/comments_and_whitespace.rs | 4 +- tooling/nargo_fmt/src/formatter/expression.rs | 90 ++++++++++++++++--- tooling/nargo_fmt/src/formatter/use_tree.rs | 2 +- .../nargo_fmt/src/formatter/use_tree_merge.rs | 2 +- tooling/nargo_fmt/tests/expected/tuple.nr | 24 ++--- 6 files changed, 102 insertions(+), 35 deletions(-) diff --git a/tooling/nargo_fmt/src/chunks.rs b/tooling/nargo_fmt/src/chunks.rs index d4fb540a221..673cf020aed 100644 --- a/tooling/nargo_fmt/src/chunks.rs +++ b/tooling/nargo_fmt/src/chunks.rs @@ -849,10 +849,10 @@ impl<'a> Formatter<'a> { } Chunk::TrailingComment(text_chunk) | Chunk::LeadingComment(text_chunk) => { self.write(&text_chunk.string); - self.write(" "); + self.write_space_without_skipping_whitespace_and_comments(); } Chunk::Group(chunks) => self.format_chunk_group_impl(chunks), - Chunk::SpaceOrLine => self.write(" "), + Chunk::SpaceOrLine => self.write_space_without_skipping_whitespace_and_comments(), Chunk::IncreaseIndentation => self.increase_indentation(), Chunk::DecreaseIndentation => self.decrease_indentation(), Chunk::PushIndentation => self.push_indentation(), @@ -915,9 +915,16 @@ impl<'a> Formatter<'a> { self.write_indentation(); } Chunk::LeadingComment(text_chunk) => { + let ends_with_newline = text_chunk.string.ends_with('\n'); self.write_chunk_lines(text_chunk.string.trim()); - self.write_line_without_skipping_whitespace_and_comments(); - self.write_indentation(); + + // Respect whether the leading comment had a newline before what comes next or not + if ends_with_newline { + self.write_line_without_skipping_whitespace_and_comments(); + self.write_indentation(); + } else { + self.write_space_without_skipping_whitespace_and_comments(); + } } Chunk::Group(mut group) => { if chunks.force_multiline_on_children_with_same_tag_if_multiline diff --git a/tooling/nargo_fmt/src/formatter/comments_and_whitespace.rs b/tooling/nargo_fmt/src/formatter/comments_and_whitespace.rs index e5f15bc397e..4aba0481e24 100644 --- a/tooling/nargo_fmt/src/formatter/comments_and_whitespace.rs +++ b/tooling/nargo_fmt/src/formatter/comments_and_whitespace.rs @@ -301,8 +301,8 @@ mod tests { let src = "global x = [ /* hello */ 1 , 2 ] ;"; let expected = "global x = [ - /* hello */ - 1, 2, + /* hello */ 1, + 2, ]; "; assert_format_with_max_width(src, expected, 20); diff --git a/tooling/nargo_fmt/src/formatter/expression.rs b/tooling/nargo_fmt/src/formatter/expression.rs index 80b9886c047..ebfa3ae78fb 100644 --- a/tooling/nargo_fmt/src/formatter/expression.rs +++ b/tooling/nargo_fmt/src/formatter/expression.rs @@ -446,23 +446,22 @@ impl<'a, 'b> ChunkFormatter<'a, 'b> { { let mut comments_chunk = self.skip_comments_and_whitespace_chunk(); - // If the comment is not empty but doesn't have newlines, it's surely `/* comment */`. - // We format that with spaces surrounding it so it looks, for example, like `Foo { /* comment */ field ..`. - if !comments_chunk.string.trim().is_empty() && !comments_chunk.has_newlines { - // Note: there's no space after `{}` because space will be produced by format_items_separated_by_comma - comments_chunk.string = if surround_with_spaces { - format!(" {}", comments_chunk.string.trim()) - } else { - format!(" {} ", comments_chunk.string.trim()) - }; - group.text(comments_chunk); - + // Handle leading block vs. line comments a bit differently. + if comments_chunk.string.trim().starts_with("/*") { group.increase_indentation(); if surround_with_spaces { group.space_or_line(); } else { group.line(); } + + // Note: there's no space before `{}` because it was just produced + comments_chunk.string = if surround_with_spaces { + comments_chunk.string.trim().to_string() + } else { + format!("{} ", comments_chunk.string.trim()) + }; + group.leading_comment(comments_chunk); } else { group.increase_indentation(); if surround_with_spaces { @@ -479,7 +478,24 @@ impl<'a, 'b> ChunkFormatter<'a, 'b> { group.text_attached_to_last_group(self.chunk(|formatter| { formatter.write_comma(); })); - group.trailing_comment(self.skip_comments_and_whitespace_chunk()); + let newlines_count_before_comment = self.following_newlines_count(); + group.text(self.chunk(|formatter| { + formatter.skip_whitespace(); + })); + if let Token::BlockComment(..) = &self.token { + // We let block comments be part of the item that's going to be formatted + } else { + // Line comments can be trailing or leading, depending on whether there are newlines before them + let comments_and_whitespace_chunk = self.skip_comments_and_whitespace_chunk(); + if !comments_and_whitespace_chunk.string.trim().is_empty() { + if newlines_count_before_comment > 0 { + group.line(); + group.leading_comment(comments_and_whitespace_chunk); + } else { + group.trailing_comment(comments_and_whitespace_chunk); + } + } + } group.space_or_line(); } format_item(self, expr, group); @@ -1326,6 +1342,56 @@ global y = 1; assert_format_with_config(src, expected, config); } + #[test] + fn format_short_array_with_block_comment_before_elements() { + let src = "global x = [ /* one */ 1, /* two */ 2 ] ;"; + let expected = "global x = [/* one */ 1, /* two */ 2];\n"; + + assert_format(src, expected); + } + + #[test] + fn format_long_array_with_block_comment_before_elements() { + let src = "global x = [ /* one */ 1, /* two */ 123456789012345, 3, 4 ] ;"; + let expected = "global x = [ + /* one */ 1, + /* two */ 123456789012345, + 3, + 4, +]; +"; + + let config = + Config { short_array_element_width_threshold: 5, max_width: 30, ..Config::default() }; + assert_format_with_config(src, expected, config); + } + + #[test] + fn format_long_array_with_line_comment_before_elements() { + let src = "global x = [ + // one + 1, + // two + 123456789012345, + 3, + 4, +]; +"; + let expected = "global x = [ + // one + 1, + // two + 123456789012345, + 3, + 4, +]; +"; + + let config = + Config { short_array_element_width_threshold: 5, max_width: 30, ..Config::default() }; + assert_format_with_config(src, expected, config); + } + #[test] fn format_cast() { let src = "global x = 1 as u8 ;"; diff --git a/tooling/nargo_fmt/src/formatter/use_tree.rs b/tooling/nargo_fmt/src/formatter/use_tree.rs index bc8dc3fcabb..98d63ef6611 100644 --- a/tooling/nargo_fmt/src/formatter/use_tree.rs +++ b/tooling/nargo_fmt/src/formatter/use_tree.rs @@ -211,7 +211,7 @@ mod tests { #[test] fn format_use_list_one_item_with_comments() { let src = " use foo::{ /* do not remove me */ bar, };"; - let expected = "use foo::{ /* do not remove me */ bar};\n"; + let expected = "use foo::{/* do not remove me */ bar};\n"; assert_format(src, expected); } diff --git a/tooling/nargo_fmt/src/formatter/use_tree_merge.rs b/tooling/nargo_fmt/src/formatter/use_tree_merge.rs index e24b7b8cbf6..834280ddba3 100644 --- a/tooling/nargo_fmt/src/formatter/use_tree_merge.rs +++ b/tooling/nargo_fmt/src/formatter/use_tree_merge.rs @@ -397,7 +397,7 @@ mod tests { #[test] fn format_use_list_one_item_with_comments() { let src = " use foo::{ /* do not remove me */ bar, };"; - let expected = "use foo::{ /* do not remove me */ bar};\n"; + let expected = "use foo::{/* do not remove me */ bar};\n"; assert_format(src, expected); } diff --git a/tooling/nargo_fmt/tests/expected/tuple.nr b/tooling/nargo_fmt/tests/expected/tuple.nr index d4b8b239815..29f32f83e55 100644 --- a/tooling/nargo_fmt/tests/expected/tuple.nr +++ b/tooling/nargo_fmt/tests/expected/tuple.nr @@ -4,13 +4,13 @@ fn main() { // hello 1, ); - ( /*hello*/ 1,); + (/*hello*/ 1,); (1/*hello*/,); (1,); - ( /*test*/ 1,); - ( /*a*/ 1/*b*/,); - ( /*a*/ 1/*b*/, /*c*/ 2/*d*/, /*c*/ 2/*d*/); - ( /*a*/ 1/*b*/, /*c*/ 2/*d*/, /*c*/ 2/*d*/, /*e*/ 3/*f*/); + (/*test*/ 1,); + (/*a*/ 1/*b*/,); + (/*a*/ 1/*b*/, /*c*/ 2/*d*/, /*c*/ 2/*d*/); + (/*a*/ 1/*b*/, /*c*/ 2/*d*/, /*c*/ 2/*d*/, /*e*/ 3/*f*/); (1 /*1*/, 2 /* 2*/); @@ -28,7 +28,7 @@ fn main() { 2, ); - ( /*1*/ 1, /*2*/ 2); + (/*1*/ 1, /*2*/ 2); ( ( @@ -39,14 +39,8 @@ fn main() { ), ); ( - /*a*/ - 1 - /*b*/, - /*c*/ - 2/*d*/, - /*c*/ - 2/*d*/, - /*e*/ - 3,/*f*/ + /*a*/ 1 + /*b*/, /*c*/ + 2/*d*/, /*c*/ 2/*d*/, /*e*/ 3,/*f*/ ); } From 0de3241bd290b1737ff831c30e5a2a0633a53eb3 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Thu, 24 Oct 2024 17:17:12 -0300 Subject: [PATCH 4/8] feat: check trait where clause (#6325) # Description ## Problem Resolves #6023 ## Summary It turns out that checking a trait's `where` clause is very similar to checking parent traits: for parent traits the type to check if `Self`, for where clause it's the specified type. So `trait Foo where Self: Constraint` is the same as `trait Foo: Constraint`. I thought about unifying the code to only have a single list of constraints but I don't know if it's worth it (we could maybe give different errors in one case or another, though right now the errors are the same). There's a chance this PR is a breaking change, because when I finished implementing it one test broke and I had to amend it. ## Additional Context ## Documentation Check one: - [x] No documentation needed. - [ ] Documentation included in this PR. - [ ] **[For Experimental Features]** Documentation to be submitted in a separate PR. # PR Checklist - [x] I have tested the changes locally. - [x] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings. --- compiler/noirc_frontend/src/elaborator/mod.rs | 100 +++++++++++++++- .../noirc_frontend/src/elaborator/traits.rs | 4 + compiler/noirc_frontend/src/hir_def/traits.rs | 6 + compiler/noirc_frontend/src/node_interner.rs | 1 + compiler/noirc_frontend/src/tests.rs | 15 ++- compiler/noirc_frontend/src/tests/traits.rs | 113 ++++++++++++++++++ 6 files changed, 229 insertions(+), 10 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index 641e2d1d57e..9f56b72f4e9 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -1041,11 +1041,14 @@ impl<'context> Elaborator<'context> { self.file = trait_impl.file_id; self.local_module = trait_impl.module_id; - self.check_parent_traits_are_implemented(&trait_impl); - - self.generics = trait_impl.resolved_generics; + self.generics = trait_impl.resolved_generics.clone(); self.current_trait_impl = trait_impl.impl_id; + self.add_trait_impl_assumed_trait_implementations(trait_impl.impl_id); + self.check_trait_impl_where_clause_matches_trait_where_clause(&trait_impl); + self.check_parent_traits_are_implemented(&trait_impl); + self.remove_trait_impl_assumed_trait_implementations(trait_impl.impl_id); + for (module, function, _) in &trait_impl.methods.functions { self.local_module = *module; let errors = check_trait_impl_method_matches_declaration(self.interner, *function); @@ -1059,6 +1062,95 @@ impl<'context> Elaborator<'context> { self.generics.clear(); } + fn add_trait_impl_assumed_trait_implementations(&mut self, impl_id: Option) { + if let Some(impl_id) = impl_id { + if let Some(trait_implementation) = self.interner.try_get_trait_implementation(impl_id) + { + for trait_constrain in &trait_implementation.borrow().where_clause { + let trait_bound = &trait_constrain.trait_bound; + self.interner.add_assumed_trait_implementation( + trait_constrain.typ.clone(), + trait_bound.trait_id, + trait_bound.trait_generics.clone(), + ); + } + } + } + } + + fn remove_trait_impl_assumed_trait_implementations(&mut self, impl_id: Option) { + if let Some(impl_id) = impl_id { + if let Some(trait_implementation) = self.interner.try_get_trait_implementation(impl_id) + { + for trait_constrain in &trait_implementation.borrow().where_clause { + self.interner.remove_assumed_trait_implementations_for_trait( + trait_constrain.trait_bound.trait_id, + ); + } + } + } + } + + fn check_trait_impl_where_clause_matches_trait_where_clause( + &mut self, + trait_impl: &UnresolvedTraitImpl, + ) { + let Some(trait_id) = trait_impl.trait_id else { + return; + }; + + let Some(the_trait) = self.interner.try_get_trait(trait_id) else { + return; + }; + + if the_trait.where_clause.is_empty() { + return; + } + + let impl_trait = the_trait.name.to_string(); + let the_trait_file = the_trait.location.file; + + let mut bindings = TypeBindings::new(); + bind_ordered_generics( + &the_trait.generics, + &trait_impl.resolved_trait_generics, + &mut bindings, + ); + + // Check that each of the trait's where clause constraints is satisfied + for trait_constraint in the_trait.where_clause.clone() { + let Some(trait_constraint_trait) = + self.interner.try_get_trait(trait_constraint.trait_bound.trait_id) + else { + continue; + }; + + let trait_constraint_type = trait_constraint.typ.substitute(&bindings); + let trait_bound = &trait_constraint.trait_bound; + + if self + .interner + .try_lookup_trait_implementation( + &trait_constraint_type, + trait_bound.trait_id, + &trait_bound.trait_generics.ordered, + &trait_bound.trait_generics.named, + ) + .is_err() + { + let missing_trait = + format!("{}{}", trait_constraint_trait.name, trait_bound.trait_generics); + self.push_err(ResolverError::TraitNotImplemented { + impl_trait: impl_trait.clone(), + missing_trait, + type_missing_trait: trait_constraint_type.to_string(), + span: trait_impl.object_type.span, + missing_trait_location: Location::new(trait_bound.span, the_trait_file), + }); + } + } + } + fn check_parent_traits_are_implemented(&mut self, trait_impl: &UnresolvedTraitImpl) { let Some(trait_id) = trait_impl.trait_id else { return; @@ -1182,7 +1274,7 @@ impl<'context> Elaborator<'context> { trait_id, trait_generics, file: trait_impl.file_id, - where_clause: where_clause.clone(), + where_clause, methods, }); diff --git a/compiler/noirc_frontend/src/elaborator/traits.rs b/compiler/noirc_frontend/src/elaborator/traits.rs index e877682972c..ae278616e03 100644 --- a/compiler/noirc_frontend/src/elaborator/traits.rs +++ b/compiler/noirc_frontend/src/elaborator/traits.rs @@ -32,6 +32,9 @@ impl<'context> Elaborator<'context> { &resolved_generics, ); + let where_clause = + this.resolve_trait_constraints(&unresolved_trait.trait_def.where_clause); + // Each associated type in this trait is also an implicit generic for associated_type in &this.interner.get_trait(*trait_id).associated_types { this.generics.push(associated_type.clone()); @@ -48,6 +51,7 @@ impl<'context> Elaborator<'context> { this.interner.update_trait(*trait_id, |trait_def| { trait_def.set_methods(methods); trait_def.set_trait_bounds(resolved_trait_bounds); + trait_def.set_where_clause(where_clause); }); }); diff --git a/compiler/noirc_frontend/src/hir_def/traits.rs b/compiler/noirc_frontend/src/hir_def/traits.rs index 534805c2dad..6fd3c4f7a24 100644 --- a/compiler/noirc_frontend/src/hir_def/traits.rs +++ b/compiler/noirc_frontend/src/hir_def/traits.rs @@ -75,6 +75,8 @@ pub struct Trait { /// The resolved trait bounds (for example in `trait Foo: Bar + Baz`, this would be `Bar + Baz`) pub trait_bounds: Vec, + + pub where_clause: Vec, } #[derive(Debug)] @@ -154,6 +156,10 @@ impl Trait { self.trait_bounds = trait_bounds; } + pub fn set_where_clause(&mut self, where_clause: Vec) { + self.where_clause = where_clause; + } + pub fn find_method(&self, name: &str) -> Option { for (idx, method) in self.methods.iter().enumerate() { if &method.name == name { diff --git a/compiler/noirc_frontend/src/node_interner.rs b/compiler/noirc_frontend/src/node_interner.rs index ca7e0c6aa59..2183cfba0ef 100644 --- a/compiler/noirc_frontend/src/node_interner.rs +++ b/compiler/noirc_frontend/src/node_interner.rs @@ -735,6 +735,7 @@ impl NodeInterner { method_ids: unresolved_trait.method_ids.clone(), associated_types, trait_bounds: Vec::new(), + where_clause: Vec::new(), }; self.traits.insert(type_id, new_trait); diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index 0e0ad5b1a3f..286986e5d61 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -2970,9 +2970,7 @@ fn uses_self_type_in_trait_where_clause() { } } - struct Bar { - - } + struct Bar {} impl Foo for Bar { @@ -2984,12 +2982,17 @@ fn uses_self_type_in_trait_where_clause() { "#; let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); + assert_eq!(errors.len(), 2); + + let CompilationError::ResolverError(ResolverError::TraitNotImplemented { .. }) = &errors[0].0 + else { + panic!("Expected a trait not implemented error, got {:?}", errors[0].0); + }; let CompilationError::TypeError(TypeCheckError::UnresolvedMethodCall { method_name, .. }) = - &errors[0].0 + &errors[1].0 else { - panic!("Expected an unresolved method call error, got {:?}", errors[0].0); + panic!("Expected an unresolved method call error, got {:?}", errors[1].0); }; assert_eq!(method_name, "trait_func"); diff --git a/compiler/noirc_frontend/src/tests/traits.rs b/compiler/noirc_frontend/src/tests/traits.rs index ee84cc0e890..88138ecde4d 100644 --- a/compiler/noirc_frontend/src/tests/traits.rs +++ b/compiler/noirc_frontend/src/tests/traits.rs @@ -148,3 +148,116 @@ fn trait_inheritance_missing_parent_implementation() { assert_eq!(typ, "Struct"); assert_eq!(impl_trait, "Bar"); } + +#[test] +fn errors_on_unknown_type_in_trait_where_clause() { + let src = r#" + pub trait Foo where T: Unknown {} + + fn main() { + } + "#; + let errors = get_program_errors(src); + assert_eq!(errors.len(), 1); +} + +#[test] +fn does_not_error_if_impl_trait_constraint_is_satisfied_for_concrete_type() { + let src = r#" + pub trait Greeter { + fn greet(self); + } + + pub trait Foo + where + T: Greeter, + { + fn greet(object: U) + where + U: Greeter, + { + object.greet(); + } + } + + pub struct SomeGreeter; + impl Greeter for SomeGreeter { + fn greet(self) {} + } + + pub struct Bar; + + impl Foo for Bar {} + + fn main() {} + "#; + assert_no_errors(src); +} + +#[test] +fn does_not_error_if_impl_trait_constraint_is_satisfied_for_type_variable() { + let src = r#" + pub trait Greeter { + fn greet(self); + } + + pub trait Foo where T: Greeter { + fn greet(object: T) { + object.greet(); + } + } + + pub struct Bar; + + impl Foo for Bar where T: Greeter { + } + + fn main() { + } + "#; + assert_no_errors(src); +} +#[test] +fn errors_if_impl_trait_constraint_is_not_satisfied() { + let src = r#" + pub trait Greeter { + fn greet(self); + } + + pub trait Foo + where + T: Greeter, + { + fn greet(object: U) + where + U: Greeter, + { + object.greet(); + } + } + + pub struct SomeGreeter; + + pub struct Bar; + + impl Foo for Bar {} + + fn main() {} + "#; + let errors = get_program_errors(src); + assert_eq!(errors.len(), 1); + + let CompilationError::ResolverError(ResolverError::TraitNotImplemented { + impl_trait, + missing_trait: the_trait, + type_missing_trait: typ, + .. + }) = &errors[0].0 + else { + panic!("Expected a TraitNotImplemented error, got {:?}", &errors[0].0); + }; + + assert_eq!(the_trait, "Greeter"); + assert_eq!(typ, "SomeGreeter"); + assert_eq!(impl_trait, "Foo"); +} From 4d87c9ac78b48b4bd0ae81316df28aab390d004e Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Thu, 24 Oct 2024 16:32:30 -0400 Subject: [PATCH 5/8] feat(profiler)!: New flamegraph command that profiles the opcodes executed (#6327) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Description ## Problem\* Resolves Initial profiling to get better numbers for the opcodes actually being executed by the Brillig VM. ## Summary\* cc @sirasistant for a lot of the initial code in this PR. The noir-profiler currently only looks at compiled artifacts to generate a flamegraph showing where constraints counts and/or opcodes counts are most prominent in a program. This PR adds execution profiling sampling to be part of the ACVM/Brillig VM and the noir-profiler. This will enable us to see if there are opcodes which are being unnecessarily executed (such as array copies). This initial PR simply adds the `execution-flamegraph` command while a follow-up will add extra metadata to look at Brillig procedures. After compiling `execution_success/sha256` with `--force-brillig` we can run the following command: ``` noir-profiler execution-opcodes -a ./target/sha256.json -p ./Prover.toml -o ./target/ ``` Result for execution flamegraph: Screenshot 2024-10-23 at 7 17 48 PM To compare here is the result of `noir-profiler opcodes-flamegraph`: Screenshot 2024-10-23 at 7 18 12 PM ## Additional Context This is a minor breaking change as I changed the names of the commands in the `noir-profiler` to not all be suffixed with `flamegraph`. ## Documentation\* Check one: - [ ] No documentation needed. - [ ] Documentation included in this PR. - [X] **[For Experimental Features]** Documentation to be submitted in a separate PR. # PR Checklist\* - [X] I have tested the changes locally. - [X] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings. --------- Co-authored-by: Akosh Farkash --- Cargo.lock | 3 + acvm-repo/acvm/src/pwg/brillig.rs | 36 ++++++- acvm-repo/acvm/src/pwg/mod.rs | 48 +++++++++- acvm-repo/brillig_vm/src/lib.rs | 55 ++++++++--- .../noirc_evaluator/src/brillig/brillig_ir.rs | 4 +- .../src/ssa/acir_gen/acir_ir/acir_variable.rs | 3 +- tooling/nargo/src/ops/execute.rs | 66 +++++++++++-- tooling/nargo/src/ops/mod.rs | 2 +- tooling/profiler/Cargo.toml | 4 + .../src/cli/execution_flamegraph_cmd.rs | 96 +++++++++++++++++++ .../profiler/src/cli/gates_flamegraph_cmd.rs | 2 +- tooling/profiler/src/cli/mod.rs | 11 ++- .../src/cli/opcodes_flamegraph_cmd.rs | 4 +- tooling/profiler/src/flamegraph.rs | 87 +++++++++++------ tooling/profiler/src/fs.rs | 29 +++++- 15 files changed, 385 insertions(+), 65 deletions(-) create mode 100644 tooling/profiler/src/cli/execution_flamegraph_cmd.rs diff --git a/Cargo.lock b/Cargo.lock index 3bfffca46e5..dabac0a7570 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2762,12 +2762,15 @@ name = "noir_profiler" version = "0.36.0" dependencies = [ "acir", + "bn254_blackbox_solver", "clap", "color-eyre", "const_format", "fm", + "fxhash", "im", "inferno", + "nargo", "noirc_abi", "noirc_artifacts", "noirc_driver", diff --git a/acvm-repo/acvm/src/pwg/brillig.rs b/acvm-repo/acvm/src/pwg/brillig.rs index 5ec3224dbaa..dffa45dbd7a 100644 --- a/acvm-repo/acvm/src/pwg/brillig.rs +++ b/acvm-repo/acvm/src/pwg/brillig.rs @@ -12,7 +12,7 @@ use acir::{ AcirField, }; use acvm_blackbox_solver::BlackBoxFunctionSolver; -use brillig_vm::{FailureReason, MemoryValue, VMStatus, VM}; +use brillig_vm::{BrilligProfilingSamples, FailureReason, MemoryValue, VMStatus, VM}; use serde::{Deserialize, Serialize}; use crate::{pwg::OpcodeNotSolvable, OpcodeResolutionError}; @@ -58,6 +58,7 @@ impl<'b, B: BlackBoxFunctionSolver, F: AcirField> BrilligSolver<'b, F, B> { /// Constructs a solver for a Brillig block given the bytecode and initial /// witness. + #[allow(clippy::too_many_arguments)] pub(crate) fn new_call( initial_witness: &WitnessMap, memory: &HashMap>, @@ -66,9 +67,16 @@ impl<'b, B: BlackBoxFunctionSolver, F: AcirField> BrilligSolver<'b, F, B> { bb_solver: &'b B, acir_index: usize, brillig_function_id: BrilligFunctionId, + profiling_active: bool, ) -> Result> { - let vm = - Self::setup_brillig_vm(initial_witness, memory, inputs, brillig_bytecode, bb_solver)?; + let vm = Self::setup_brillig_vm( + initial_witness, + memory, + inputs, + brillig_bytecode, + bb_solver, + profiling_active, + )?; Ok(Self { vm, acir_index, function_id: brillig_function_id }) } @@ -78,6 +86,7 @@ impl<'b, B: BlackBoxFunctionSolver, F: AcirField> BrilligSolver<'b, F, B> { inputs: &[BrilligInputs], brillig_bytecode: &'b [BrilligOpcode], bb_solver: &'b B, + profiling_active: bool, ) -> Result, OpcodeResolutionError> { // Set input values let mut calldata: Vec = Vec::new(); @@ -125,7 +134,7 @@ impl<'b, B: BlackBoxFunctionSolver, F: AcirField> BrilligSolver<'b, F, B> { // Instantiate a Brillig VM given the solved calldata // along with the Brillig bytecode. - let vm = VM::new(calldata, brillig_bytecode, vec![], bb_solver); + let vm = VM::new(calldata, brillig_bytecode, vec![], bb_solver, profiling_active); Ok(vm) } @@ -203,6 +212,25 @@ impl<'b, B: BlackBoxFunctionSolver, F: AcirField> BrilligSolver<'b, F, B> { self, witness: &mut WitnessMap, outputs: &[BrilligOutputs], + ) -> Result<(), OpcodeResolutionError> { + assert!(!self.vm.is_profiling_active(), "Expected VM profiling to not be active"); + self.finalize_inner(witness, outputs) + } + + pub(crate) fn finalize_with_profiling( + mut self, + witness: &mut WitnessMap, + outputs: &[BrilligOutputs], + ) -> Result> { + assert!(self.vm.is_profiling_active(), "Expected VM profiling to be active"); + self.finalize_inner(witness, outputs)?; + Ok(self.vm.take_profiling_samples()) + } + + fn finalize_inner( + &self, + witness: &mut WitnessMap, + outputs: &[BrilligOutputs], ) -> Result<(), OpcodeResolutionError> { // Finish the Brillig execution by writing the outputs to the witness map let vm_status = self.vm.get_status(); diff --git a/acvm-repo/acvm/src/pwg/mod.rs b/acvm-repo/acvm/src/pwg/mod.rs index c73893ceea6..fa3498da613 100644 --- a/acvm-repo/acvm/src/pwg/mod.rs +++ b/acvm-repo/acvm/src/pwg/mod.rs @@ -168,6 +168,14 @@ impl From for OpcodeResolutionError { } } +pub type ProfilingSamples = Vec; + +#[derive(Default)] +pub struct ProfilingSample { + pub call_stack: Vec, + pub brillig_function_id: Option, +} + pub struct ACVM<'a, F, B: BlackBoxFunctionSolver> { status: ACVMStatus, @@ -198,6 +206,10 @@ pub struct ACVM<'a, F, B: BlackBoxFunctionSolver> { unconstrained_functions: &'a [BrilligBytecode], assertion_payloads: &'a [(OpcodeLocation, AssertionPayload)], + + profiling_active: bool, + + profiling_samples: ProfilingSamples, } impl<'a, F: AcirField, B: BlackBoxFunctionSolver> ACVM<'a, F, B> { @@ -222,9 +234,16 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver> ACVM<'a, F, B> { acir_call_results: Vec::default(), unconstrained_functions, assertion_payloads, + profiling_active: false, + profiling_samples: Vec::new(), } } + // Enable profiling + pub fn with_profiler(&mut self, profiling_active: bool) { + self.profiling_active = profiling_active; + } + /// Returns a reference to the current state of the ACVM's [`WitnessMap`]. /// /// Once execution has completed, the witness map can be extracted using [`ACVM::finalize`] @@ -246,6 +265,10 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver> ACVM<'a, F, B> { self.instruction_pointer } + pub fn take_profiling_samples(&mut self) -> ProfilingSamples { + std::mem::take(&mut self.profiling_samples) + } + /// Finalize the ACVM execution, returning the resulting [`WitnessMap`]. pub fn finalize(self) -> WitnessMap { if self.status != ACVMStatus::Solved { @@ -503,6 +526,7 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver> ACVM<'a, F, B> { self.backend, self.instruction_pointer, *id, + self.profiling_active, )?, }; @@ -519,7 +543,28 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver> ACVM<'a, F, B> { } BrilligSolverStatus::Finished => { // Write execution outputs - solver.finalize(&mut self.witness_map, outputs)?; + if self.profiling_active { + let profiling_info = + solver.finalize_with_profiling(&mut self.witness_map, outputs)?; + profiling_info.into_iter().for_each(|sample| { + let mapped = + sample.call_stack.into_iter().map(|loc| OpcodeLocation::Brillig { + acir_index: self.instruction_pointer, + brillig_index: loc, + }); + self.profiling_samples.push(ProfilingSample { + call_stack: std::iter::once(OpcodeLocation::Acir( + self.instruction_pointer, + )) + .chain(mapped) + .collect(), + brillig_function_id: Some(*id), + }); + }); + } else { + solver.finalize(&mut self.witness_map, outputs)?; + } + Ok(None) } } @@ -575,6 +620,7 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver> ACVM<'a, F, B> { self.backend, self.instruction_pointer, *id, + self.profiling_active, ); match solver { Ok(solver) => StepResult::IntoBrillig(solver), diff --git a/acvm-repo/brillig_vm/src/lib.rs b/acvm-repo/brillig_vm/src/lib.rs index 1e5ad84eb8f..4d5ce4203f9 100644 --- a/acvm-repo/brillig_vm/src/lib.rs +++ b/acvm-repo/brillig_vm/src/lib.rs @@ -63,6 +63,15 @@ pub enum VMStatus { }, } +// A sample for each opcode that was executed. +pub type BrilligProfilingSamples = Vec; + +#[derive(Debug, PartialEq, Eq, Clone)] +pub struct BrilligProfilingSample { + // The call stack when processing a given opcode. + pub call_stack: Vec, +} + #[derive(Debug, PartialEq, Eq, Clone)] /// VM encapsulates the state of the Brillig VM during execution. pub struct VM<'a, F, B: BlackBoxFunctionSolver> { @@ -88,6 +97,10 @@ pub struct VM<'a, F, B: BlackBoxFunctionSolver> { black_box_solver: &'a B, // The solver for big integers bigint_solver: BrilligBigintSolver, + // Flag that determines whether we want to profile VM. + profiling_active: bool, + // Samples for profiling the VM execution. + profiling_samples: BrilligProfilingSamples, } impl<'a, F: AcirField, B: BlackBoxFunctionSolver> VM<'a, F, B> { @@ -97,6 +110,7 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver> VM<'a, F, B> { bytecode: &'a [Opcode], foreign_call_results: Vec>, black_box_solver: &'a B, + profiling_active: bool, ) -> Self { Self { calldata, @@ -109,9 +123,19 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver> VM<'a, F, B> { call_stack: Vec::new(), black_box_solver, bigint_solver: Default::default(), + profiling_active, + profiling_samples: Vec::with_capacity(bytecode.len()), } } + pub fn is_profiling_active(&self) -> bool { + self.profiling_active + } + + pub fn take_profiling_samples(&mut self) -> BrilligProfilingSamples { + std::mem::take(&mut self.profiling_samples) + } + /// Updates the current status of the VM. /// Returns the given status. fn status(&mut self, status: VMStatus) -> VMStatus { @@ -196,6 +220,15 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver> VM<'a, F, B> { /// Process a single opcode and modify the program counter. pub fn process_opcode(&mut self) -> VMStatus { + if self.profiling_active { + let call_stack: Vec = self.get_call_stack(); + self.profiling_samples.push(BrilligProfilingSample { call_stack }); + } + + self.process_opcode_internal() + } + + fn process_opcode_internal(&mut self) -> VMStatus { let opcode = &self.bytecode[self.program_counter]; match opcode { Opcode::BinaryFieldOp { op, lhs, rhs, destination: result } => { @@ -813,7 +846,7 @@ mod tests { }]; // Start VM - let mut vm = VM::new(calldata, &opcodes, vec![], &StubbedBlackBoxSolver); + let mut vm = VM::new(calldata, &opcodes, vec![], &StubbedBlackBoxSolver, false); let status = vm.process_opcode(); assert_eq!(status, VMStatus::Finished { return_data_offset: 0, return_data_size: 0 }); @@ -863,7 +896,7 @@ mod tests { Opcode::JumpIf { condition: destination, location: 6 }, ]; - let mut vm = VM::new(calldata, &opcodes, vec![], &StubbedBlackBoxSolver); + let mut vm = VM::new(calldata, &opcodes, vec![], &StubbedBlackBoxSolver, false); let status = vm.process_opcode(); assert_eq!(status, VMStatus::InProgress); @@ -921,7 +954,7 @@ mod tests { }, ]; - let mut vm = VM::new(calldata, &opcodes, vec![], &StubbedBlackBoxSolver); + let mut vm = VM::new(calldata, &opcodes, vec![], &StubbedBlackBoxSolver, false); let status = vm.process_opcode(); assert_eq!(status, VMStatus::InProgress); @@ -982,7 +1015,7 @@ mod tests { }, Opcode::Stop { return_data_offset: 1, return_data_size: 1 }, ]; - let mut vm = VM::new(calldata, opcodes, vec![], &StubbedBlackBoxSolver); + let mut vm = VM::new(calldata, opcodes, vec![], &StubbedBlackBoxSolver, false); let status = vm.process_opcode(); assert_eq!(status, VMStatus::InProgress); @@ -1033,7 +1066,7 @@ mod tests { }, Opcode::Stop { return_data_offset: 1, return_data_size: 1 }, ]; - let mut vm = VM::new(calldata, opcodes, vec![], &StubbedBlackBoxSolver); + let mut vm = VM::new(calldata, opcodes, vec![], &StubbedBlackBoxSolver, false); let status = vm.process_opcode(); assert_eq!(status, VMStatus::InProgress); @@ -1079,7 +1112,7 @@ mod tests { }, Opcode::Mov { destination: MemoryAddress::direct(2), source: MemoryAddress::direct(0) }, ]; - let mut vm = VM::new(calldata, opcodes, vec![], &StubbedBlackBoxSolver); + let mut vm = VM::new(calldata, opcodes, vec![], &StubbedBlackBoxSolver, false); let status = vm.process_opcode(); assert_eq!(status, VMStatus::InProgress); @@ -1144,7 +1177,7 @@ mod tests { condition: MemoryAddress::direct(1), }, ]; - let mut vm = VM::new(calldata, opcodes, vec![], &StubbedBlackBoxSolver); + let mut vm = VM::new(calldata, opcodes, vec![], &StubbedBlackBoxSolver, false); let status = vm.process_opcode(); assert_eq!(status, VMStatus::InProgress); @@ -1240,7 +1273,7 @@ mod tests { .chain(cast_opcodes) .chain([equal_opcode, not_equal_opcode, less_than_opcode, less_than_equal_opcode]) .collect(); - let mut vm = VM::new(calldata, &opcodes, vec![], &StubbedBlackBoxSolver); + let mut vm = VM::new(calldata, &opcodes, vec![], &StubbedBlackBoxSolver, false); // Calldata copy let status = vm.process_opcode(); @@ -1368,7 +1401,7 @@ mod tests { value: FieldElement::from(27_usize), }, ]; - let mut vm = VM::new(vec![], opcodes, vec![], &StubbedBlackBoxSolver); + let mut vm = VM::new(vec![], opcodes, vec![], &StubbedBlackBoxSolver, false); let status = vm.process_opcode(); assert_eq!(status, VMStatus::InProgress); @@ -1592,7 +1625,7 @@ mod tests { calldata: Vec, opcodes: &[Opcode], ) -> VM<'_, F, StubbedBlackBoxSolver> { - let mut vm = VM::new(calldata, opcodes, vec![], &StubbedBlackBoxSolver); + let mut vm = VM::new(calldata, opcodes, vec![], &StubbedBlackBoxSolver, false); brillig_execute(&mut vm); assert_eq!(vm.call_stack, vec![]); vm @@ -2271,7 +2304,7 @@ mod tests { }, ]; - let mut vm = VM::new(calldata, &opcodes, vec![], &StubbedBlackBoxSolver); + let mut vm = VM::new(calldata, &opcodes, vec![], &StubbedBlackBoxSolver, false); vm.process_opcode(); vm.process_opcode(); diff --git a/compiler/noirc_evaluator/src/brillig/brillig_ir.rs b/compiler/noirc_evaluator/src/brillig/brillig_ir.rs index 4964ff27f60..416f0300508 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_ir.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_ir.rs @@ -233,7 +233,8 @@ pub(crate) mod tests { calldata: Vec, bytecode: &[BrilligOpcode], ) -> (VM<'_, FieldElement, DummyBlackBoxSolver>, usize, usize) { - let mut vm = VM::new(calldata, bytecode, vec![], &DummyBlackBoxSolver); + let profiling_active = false; + let mut vm = VM::new(calldata, bytecode, vec![], &DummyBlackBoxSolver, profiling_active); let status = vm.process_opcodes(); if let VMStatus::Finished { return_data_offset, return_data_size } = status { @@ -301,6 +302,7 @@ pub(crate) mod tests { &bytecode, vec![ForeignCallResult { values: vec![ForeignCallParam::Array(number_sequence)] }], &DummyBlackBoxSolver, + false, ); let status = vm.process_opcodes(); assert_eq!(status, VMStatus::Finished { return_data_offset: 0, return_data_size: 0 }); diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs index db08b906185..811b728a9d5 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs @@ -2195,7 +2195,8 @@ fn execute_brillig( // We pass a stubbed solver here as a concrete solver implies a field choice which conflicts with this function // being generic. let solver = acvm::blackbox_solver::StubbedBlackBoxSolver; - let mut vm = VM::new(calldata, code, Vec::new(), &solver); + let profiling_active = false; + let mut vm = VM::new(calldata, code, Vec::new(), &solver, profiling_active); // Run the Brillig VM on these inputs, bytecode, etc! let vm_status = vm.process_opcodes(); diff --git a/tooling/nargo/src/ops/execute.rs b/tooling/nargo/src/ops/execute.rs index eb03bdf01c1..09ef554d2aa 100644 --- a/tooling/nargo/src/ops/execute.rs +++ b/tooling/nargo/src/ops/execute.rs @@ -3,7 +3,9 @@ use acvm::acir::circuit::{ OpcodeLocation, Program, ResolvedAssertionPayload, ResolvedOpcodeLocation, }; use acvm::acir::native_types::WitnessStack; -use acvm::pwg::{ACVMStatus, ErrorLocation, OpcodeNotSolvable, OpcodeResolutionError, ACVM}; +use acvm::pwg::{ + ACVMStatus, ErrorLocation, OpcodeNotSolvable, OpcodeResolutionError, ProfilingSamples, ACVM, +}; use acvm::{acir::circuit::Circuit, acir::native_types::WitnessMap}; use acvm::{AcirField, BlackBoxFunctionSolver}; @@ -32,6 +34,10 @@ struct ProgramExecutor<'a, F, B: BlackBoxFunctionSolver, E: ForeignCallExecut // This is used to fetch the function we want to execute // and to resolve call stack locations across many function calls. current_function_index: usize, + + // Flag that states whether we want to profile the VM. Profiling can add extra + // execution costs so we want to make sure we only trigger it explicitly. + profiling_active: bool, } impl<'a, F: AcirField, B: BlackBoxFunctionSolver, E: ForeignCallExecutor> @@ -42,6 +48,7 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver, E: ForeignCallExecutor> unconstrained_functions: &'a [BrilligBytecode], blackbox_solver: &'a B, foreign_call_executor: &'a mut E, + profiling_active: bool, ) -> Self { ProgramExecutor { functions, @@ -51,6 +58,7 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver, E: ForeignCallExecutor> foreign_call_executor, call_stack: Vec::default(), current_function_index: 0, + profiling_active, } } @@ -62,7 +70,7 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver, E: ForeignCallExecutor> fn execute_circuit( &mut self, initial_witness: WitnessMap, - ) -> Result, NargoError> { + ) -> Result<(WitnessMap, ProfilingSamples), NargoError> { let circuit = &self.functions[self.current_function_index]; let mut acvm = ACVM::new( self.blackbox_solver, @@ -71,6 +79,7 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver, E: ForeignCallExecutor> self.unconstrained_functions, &circuit.assert_messages, ); + acvm.with_profiler(self.profiling_active); loop { let solver_status = acvm.solve(); @@ -155,7 +164,8 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver, E: ForeignCallExecutor> // Execute the ACIR call let acir_to_call = &self.functions[call_info.id.as_usize()]; let initial_witness = call_info.initial_witness; - let call_solved_witness = self.execute_circuit(initial_witness)?; + // TODO: Profiling among multiple circuits is not supported + let (call_solved_witness, _) = self.execute_circuit(initial_witness)?; // Set tracking index back to the parent function after ACIR call execution self.current_function_index = acir_function_caller; @@ -184,25 +194,67 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver, E: ForeignCallExecutor> // included in a failure case. self.call_stack.clear(); - Ok(acvm.finalize()) + let profiling_samples = acvm.take_profiling_samples(); + Ok((acvm.finalize(), profiling_samples)) } } -#[tracing::instrument(level = "trace", skip_all)] pub fn execute_program, E: ForeignCallExecutor>( program: &Program, initial_witness: WitnessMap, blackbox_solver: &B, foreign_call_executor: &mut E, ) -> Result, NargoError> { + let profiling_active = false; + let (witness_stack, profiling_samples) = execute_program_inner( + program, + initial_witness, + blackbox_solver, + foreign_call_executor, + profiling_active, + )?; + assert!(profiling_samples.is_empty(), "Expected no profiling samples"); + + Ok(witness_stack) +} + +pub fn execute_program_with_profiling< + F: AcirField, + B: BlackBoxFunctionSolver, + E: ForeignCallExecutor, +>( + program: &Program, + initial_witness: WitnessMap, + blackbox_solver: &B, + foreign_call_executor: &mut E, +) -> Result<(WitnessStack, ProfilingSamples), NargoError> { + let profiling_active = true; + execute_program_inner( + program, + initial_witness, + blackbox_solver, + foreign_call_executor, + profiling_active, + ) +} + +#[tracing::instrument(level = "trace", skip_all)] +fn execute_program_inner, E: ForeignCallExecutor>( + program: &Program, + initial_witness: WitnessMap, + blackbox_solver: &B, + foreign_call_executor: &mut E, + profiling_active: bool, +) -> Result<(WitnessStack, ProfilingSamples), NargoError> { let mut executor = ProgramExecutor::new( &program.functions, &program.unconstrained_functions, blackbox_solver, foreign_call_executor, + profiling_active, ); - let main_witness = executor.execute_circuit(initial_witness)?; + let (main_witness, profiling_samples) = executor.execute_circuit(initial_witness)?; executor.witness_stack.push(0, main_witness); - Ok(executor.finalize()) + Ok((executor.finalize(), profiling_samples)) } diff --git a/tooling/nargo/src/ops/mod.rs b/tooling/nargo/src/ops/mod.rs index cada2f0e915..16680dab980 100644 --- a/tooling/nargo/src/ops/mod.rs +++ b/tooling/nargo/src/ops/mod.rs @@ -2,7 +2,7 @@ pub use self::compile::{ collect_errors, compile_contract, compile_program, compile_program_with_debug_instrumenter, compile_workspace, report_errors, }; -pub use self::execute::execute_program; +pub use self::execute::{execute_program, execute_program_with_profiling}; pub use self::foreign_calls::{DefaultForeignCallExecutor, ForeignCall, ForeignCallExecutor}; pub use self::optimize::{optimize_contract, optimize_program}; pub use self::transform::{transform_contract, transform_program}; diff --git a/tooling/profiler/Cargo.toml b/tooling/profiler/Cargo.toml index 136775d5831..c76b4c73e65 100644 --- a/tooling/profiler/Cargo.toml +++ b/tooling/profiler/Cargo.toml @@ -18,8 +18,10 @@ name = "noir-profiler" path = "src/main.rs" [dependencies] +bn254_blackbox_solver.workspace = true color-eyre.workspace = true clap.workspace = true +fxhash.workspace = true noirc_artifacts.workspace = true const_format.workspace = true serde.workspace = true @@ -28,7 +30,9 @@ fm.workspace = true inferno = "0.11.19" im.workspace = true acir.workspace = true +nargo.workspace = true noirc_errors.workspace = true +noirc_abi.workspace = true # Logs tracing-subscriber.workspace = true diff --git a/tooling/profiler/src/cli/execution_flamegraph_cmd.rs b/tooling/profiler/src/cli/execution_flamegraph_cmd.rs new file mode 100644 index 00000000000..a0b3d6a3128 --- /dev/null +++ b/tooling/profiler/src/cli/execution_flamegraph_cmd.rs @@ -0,0 +1,96 @@ +use std::path::{Path, PathBuf}; + +use acir::circuit::OpcodeLocation; +use acir::FieldElement; +use clap::Args; +use color_eyre::eyre::{self, Context}; + +use crate::flamegraph::{FlamegraphGenerator, InfernoFlamegraphGenerator, Sample}; +use crate::fs::{read_inputs_from_file, read_program_from_file}; +use crate::opcode_formatter::AcirOrBrilligOpcode; +use bn254_blackbox_solver::Bn254BlackBoxSolver; +use nargo::ops::DefaultForeignCallExecutor; +use noirc_abi::input_parser::Format; +use noirc_artifacts::debug::DebugArtifact; + +#[derive(Debug, Clone, Args)] +pub(crate) struct ExecutionFlamegraphCommand { + /// The path to the artifact JSON file + #[clap(long, short)] + artifact_path: PathBuf, + + /// The path to the Prover.toml file + #[clap(long, short)] + prover_toml_path: PathBuf, + + /// The output folder for the flamegraph svg files + #[clap(long, short)] + output: PathBuf, +} + +pub(crate) fn run(args: ExecutionFlamegraphCommand) -> eyre::Result<()> { + run_with_generator( + &args.artifact_path, + &args.prover_toml_path, + &InfernoFlamegraphGenerator { count_name: "samples".to_string() }, + &args.output, + ) +} + +fn run_with_generator( + artifact_path: &Path, + prover_toml_path: &Path, + flamegraph_generator: &impl FlamegraphGenerator, + output_path: &Path, +) -> eyre::Result<()> { + let program = + read_program_from_file(artifact_path).context("Error reading program from file")?; + + let (inputs_map, _) = read_inputs_from_file(prover_toml_path, Format::Toml, &program.abi)?; + + let initial_witness = program.abi.encode(&inputs_map, None)?; + + println!("Executing"); + let (_, profiling_samples) = nargo::ops::execute_program_with_profiling( + &program.bytecode, + initial_witness, + &Bn254BlackBoxSolver, + &mut DefaultForeignCallExecutor::new(true, None, None, None), + )?; + println!("Executed"); + + let profiling_samples: Vec> = profiling_samples + .into_iter() + .map(|sample| { + let call_stack = sample.call_stack; + let brillig_function_id = sample.brillig_function_id; + let last_entry = call_stack.last(); + let opcode = brillig_function_id + .and_then(|id| program.bytecode.unconstrained_functions.get(id.0 as usize)) + .and_then(|func| { + if let Some(OpcodeLocation::Brillig { brillig_index, .. }) = last_entry { + func.bytecode.get(*brillig_index) + } else { + None + } + }) + .map(|opcode| AcirOrBrilligOpcode::Brillig(opcode.clone())); + Sample { opcode, call_stack, count: 1, brillig_function_id } + }) + .collect(); + + let debug_artifact: DebugArtifact = program.into(); + + println!("Generating flamegraph with {} samples", profiling_samples.len()); + + flamegraph_generator.generate_flamegraph( + profiling_samples, + &debug_artifact.debug_symbols[0], + &debug_artifact, + artifact_path.to_str().unwrap(), + "main", + &Path::new(&output_path).join(Path::new(&format!("{}.svg", "main"))), + )?; + + Ok(()) +} diff --git a/tooling/profiler/src/cli/gates_flamegraph_cmd.rs b/tooling/profiler/src/cli/gates_flamegraph_cmd.rs index d5fefc4ecda..20cc1b747c3 100644 --- a/tooling/profiler/src/cli/gates_flamegraph_cmd.rs +++ b/tooling/profiler/src/cli/gates_flamegraph_cmd.rs @@ -84,7 +84,7 @@ fn run_with_provider( .zip(bytecode.opcodes) .enumerate() .map(|(index, (gates, opcode))| Sample { - opcode: AcirOrBrilligOpcode::Acir(opcode), + opcode: Some(AcirOrBrilligOpcode::Acir(opcode)), call_stack: vec![OpcodeLocation::Acir(index)], count: gates, brillig_function_id: None, diff --git a/tooling/profiler/src/cli/mod.rs b/tooling/profiler/src/cli/mod.rs index 7cfc6ed7c9e..80c6bceb3ce 100644 --- a/tooling/profiler/src/cli/mod.rs +++ b/tooling/profiler/src/cli/mod.rs @@ -2,6 +2,7 @@ use clap::{Parser, Subcommand}; use color_eyre::eyre; use const_format::formatcp; +mod execution_flamegraph_cmd; mod gates_flamegraph_cmd; mod opcodes_flamegraph_cmd; @@ -19,15 +20,17 @@ struct ProfilerCli { #[non_exhaustive] #[derive(Subcommand, Clone, Debug)] enum ProfilerCommand { - GatesFlamegraph(gates_flamegraph_cmd::GatesFlamegraphCommand), - OpcodesFlamegraph(opcodes_flamegraph_cmd::OpcodesFlamegraphCommand), + Gates(gates_flamegraph_cmd::GatesFlamegraphCommand), + Opcodes(opcodes_flamegraph_cmd::OpcodesFlamegraphCommand), + ExecutionOpcodes(execution_flamegraph_cmd::ExecutionFlamegraphCommand), } pub(crate) fn start_cli() -> eyre::Result<()> { let ProfilerCli { command } = ProfilerCli::parse(); match command { - ProfilerCommand::GatesFlamegraph(args) => gates_flamegraph_cmd::run(args), - ProfilerCommand::OpcodesFlamegraph(args) => opcodes_flamegraph_cmd::run(args), + ProfilerCommand::Gates(args) => gates_flamegraph_cmd::run(args), + ProfilerCommand::Opcodes(args) => opcodes_flamegraph_cmd::run(args), + ProfilerCommand::ExecutionOpcodes(args) => execution_flamegraph_cmd::run(args), } } diff --git a/tooling/profiler/src/cli/opcodes_flamegraph_cmd.rs b/tooling/profiler/src/cli/opcodes_flamegraph_cmd.rs index 863d45b96d1..bb3df86c339 100644 --- a/tooling/profiler/src/cli/opcodes_flamegraph_cmd.rs +++ b/tooling/profiler/src/cli/opcodes_flamegraph_cmd.rs @@ -60,7 +60,7 @@ fn run_with_generator( .iter() .enumerate() .map(|(index, opcode)| Sample { - opcode: AcirOrBrilligOpcode::Acir(opcode.clone()), + opcode: Some(AcirOrBrilligOpcode::Acir(opcode.clone())), call_stack: vec![OpcodeLocation::Acir(index)], count: 1, brillig_function_id: None, @@ -97,7 +97,7 @@ fn run_with_generator( .into_iter() .enumerate() .map(|(brillig_index, opcode)| Sample { - opcode: AcirOrBrilligOpcode::Brillig(opcode), + opcode: Some(AcirOrBrilligOpcode::Brillig(opcode)), call_stack: vec![OpcodeLocation::Brillig { acir_index: acir_opcode_index, brillig_index, diff --git a/tooling/profiler/src/flamegraph.rs b/tooling/profiler/src/flamegraph.rs index 488079de50a..a72168a20af 100644 --- a/tooling/profiler/src/flamegraph.rs +++ b/tooling/profiler/src/flamegraph.rs @@ -6,6 +6,7 @@ use acir::circuit::OpcodeLocation; use acir::AcirField; use color_eyre::eyre::{self}; use fm::codespan_files::Files; +use fxhash::FxHashMap as HashMap; use inferno::flamegraph::{from_lines, Options, TextTruncateDirection}; use noirc_errors::debug_info::DebugInfo; use noirc_errors::reporter::line_and_column_from_span; @@ -17,7 +18,7 @@ use super::opcode_formatter::format_opcode; #[derive(Debug)] pub(crate) struct Sample { - pub(crate) opcode: AcirOrBrilligOpcode, + pub(crate) opcode: Option>, pub(crate) call_stack: Vec, pub(crate) count: usize, pub(crate) brillig_function_id: Option, @@ -88,41 +89,61 @@ fn generate_folded_sorted_lines<'files, F: AcirField>( // Create a nested hashmap with the stack items, folding the gates for all the callsites that are equal let mut folded_stack_items = BTreeMap::new(); - samples.into_iter().for_each(|sample| { - let mut location_names: Vec = sample - .call_stack - .into_iter() - .flat_map(|opcode_location| { - debug_symbols.opcode_location(&opcode_location).unwrap_or_else(|| { - if let (Some(brillig_function_id), Some(brillig_location)) = - (sample.brillig_function_id, opcode_location.to_brillig_location()) - { - let brillig_locations = - debug_symbols.brillig_locations.get(&brillig_function_id); - if let Some(brillig_locations) = brillig_locations { - brillig_locations.get(&brillig_location).cloned().unwrap_or_default() - } else { - vec![] - } - } else { - vec![] - } + let mut resolution_cache: HashMap> = HashMap::default(); + for sample in samples { + let mut location_names = Vec::with_capacity(sample.call_stack.len()); + for opcode_location in sample.call_stack { + let callsite_labels = resolution_cache + .entry(opcode_location) + .or_insert_with(|| { + find_callsite_labels( + debug_symbols, + &opcode_location, + sample.brillig_function_id, + files, + ) }) - }) - .map(|location| location_to_callsite_label(location, files)) - .collect(); + .clone(); - if location_names.is_empty() { - location_names.push("unknown".to_string()); + location_names.extend(callsite_labels); } - location_names.push(format_opcode(&sample.opcode)); + if let Some(opcode) = &sample.opcode { + location_names.push(format_opcode(opcode)); + } add_locations_to_folded_stack_items(&mut folded_stack_items, location_names, sample.count); - }); + } to_folded_sorted_lines(&folded_stack_items, Default::default()) } +fn find_callsite_labels<'files>( + debug_symbols: &DebugInfo, + opcode_location: &OpcodeLocation, + brillig_function_id: Option, + files: &'files impl Files<'files, FileId = fm::FileId>, +) -> Vec { + let source_locations = debug_symbols.opcode_location(opcode_location).unwrap_or_else(|| { + if let (Some(brillig_function_id), Some(brillig_location)) = + (brillig_function_id, opcode_location.to_brillig_location()) + { + let brillig_locations = debug_symbols.brillig_locations.get(&brillig_function_id); + if let Some(brillig_locations) = brillig_locations { + brillig_locations.get(&brillig_location).cloned().unwrap_or_default() + } else { + vec![] + } + } else { + vec![] + } + }); + let callsite_labels: Vec<_> = source_locations + .into_iter() + .map(|location| location_to_callsite_label(location, files)) + .collect(); + callsite_labels +} + fn location_to_callsite_label<'files>( location: Location, files: &'files impl Files<'files, FileId = fm::FileId>, @@ -300,23 +321,27 @@ mod tests { let samples: Vec> = vec![ Sample { - opcode: AcirOrBrilligOpcode::Acir(AcirOpcode::AssertZero(Expression::default())), + opcode: Some(AcirOrBrilligOpcode::Acir(AcirOpcode::AssertZero( + Expression::default(), + ))), call_stack: vec![OpcodeLocation::Acir(0)], count: 10, brillig_function_id: None, }, Sample { - opcode: AcirOrBrilligOpcode::Acir(AcirOpcode::AssertZero(Expression::default())), + opcode: Some(AcirOrBrilligOpcode::Acir(AcirOpcode::AssertZero( + Expression::default(), + ))), call_stack: vec![OpcodeLocation::Acir(1)], count: 20, brillig_function_id: None, }, Sample { - opcode: AcirOrBrilligOpcode::Acir(AcirOpcode::MemoryInit { + opcode: Some(AcirOrBrilligOpcode::Acir(AcirOpcode::MemoryInit { block_id: BlockId(0), init: vec![], block_type: acir::circuit::opcodes::BlockType::Memory, - }), + })), call_stack: vec![OpcodeLocation::Acir(2)], count: 30, brillig_function_id: None, diff --git a/tooling/profiler/src/fs.rs b/tooling/profiler/src/fs.rs index e8eec2cbb14..43848504a7f 100644 --- a/tooling/profiler/src/fs.rs +++ b/tooling/profiler/src/fs.rs @@ -1,6 +1,10 @@ -use std::path::Path; +use std::{collections::BTreeMap, path::Path}; use color_eyre::eyre; +use noirc_abi::{ + input_parser::{Format, InputValue}, + Abi, InputMap, MAIN_RETURN_NAME, +}; use noirc_artifacts::program::ProgramArtifact; pub(crate) fn read_program_from_file>( @@ -13,3 +17,26 @@ pub(crate) fn read_program_from_file>( Ok(program) } + +/// Returns the circuit's parameters and its return value, if one exists. +/// # Examples +/// +/// ```ignore +/// let (input_map, return_value): (InputMap, Option) = +/// read_inputs_from_file(path, "Verifier", Format::Toml, &abi)?; +/// ``` +pub(crate) fn read_inputs_from_file( + file_path: &Path, + format: Format, + abi: &Abi, +) -> eyre::Result<(InputMap, Option)> { + if abi.is_empty() { + return Ok((BTreeMap::new(), None)); + } + + let input_string = std::fs::read_to_string(file_path)?; + let mut input_map = format.parse(&input_string, abi)?; + let return_value = input_map.remove(MAIN_RETURN_NAME); + + Ok((input_map, return_value)) +} From 392522880e102e275ebcf42f16651a8ffa0bbbd2 Mon Sep 17 00:00:00 2001 From: Aztec Bot <49558828+AztecBot@users.noreply.github.com> Date: Thu, 24 Oct 2024 17:15:43 -0400 Subject: [PATCH 6/8] feat: Sync from aztec-packages (#6345) Automated pull of Noir development from [aztec-packages](https://github.com/AztecProtocol/aztec-packages). BEGIN_COMMIT_OVERRIDE chore!: replace usage of vector in keccakf1600 input with array (https://github.com/AztecProtocol/aztec-packages/pull/9350) chore!: remove noir_js_backend_barretenberg (https://github.com/AztecProtocol/aztec-packages/pull/9338) END_COMMIT_OVERRIDE --------- Co-authored-by: Tom French --- .aztec-sync-commit | 2 +- .github/scripts/backend-barretenberg-build.sh | 4 - .github/scripts/backend-barretenberg-test.sh | 4 - .github/workflows/test-js-packages.yml | 11 +- acvm-repo/acir/codegen/acir.cpp | 12 +- acvm-repo/brillig/src/black_box.rs | 6 +- acvm-repo/brillig_vm/src/black_box.rs | 8 +- compiler/integration-tests/package.json | 64 ++++---- .../test/browser/compile_prove_verify.test.ts | 4 +- .../test/browser/recursion.test.ts | 6 +- .../onchain_recursive_verification.test.ts | 6 +- .../test/node/prove_and_verify.test.ts | 27 ++-- .../test/node/smart_contract_verifier.test.ts | 4 +- .../brillig/brillig_gen/brillig_black_box.rs | 29 ++-- .../src/brillig/brillig_ir/debug_show.rs | 4 +- docs/docs/how_to/how-to-recursion.md | 12 +- docs/docusaurus.config.ts | 31 ---- .../version-v0.36.0/how_to/_category_.json | 6 +- .../noir/standard_library/_category_.json | 8 +- .../reference/debugger/_category_.json | 8 +- package.json | 1 - scripts/bump-bb.sh | 4 +- scripts/install_bb.sh | 2 +- .../.eslintignore | 1 - .../.eslintrc.cjs | 3 - .../noir_js_backend_barretenberg/.gitignore | 2 - .../.mocharc.json | 11 -- tooling/noir_js_backend_barretenberg/fixup.sh | 18 --- .../noir_js_backend_barretenberg/package.json | 57 ------- .../src/backend.ts | 144 ------------------ .../src/base64_decode.ts | 13 -- .../noir_js_backend_barretenberg/src/index.ts | 6 - .../src/public_inputs.ts | 68 --------- .../src/serialize.ts | 8 - .../src/verifier.ts | 67 -------- .../tsconfig.cjs.json | 8 - .../tsconfig.json | 21 --- yarn.lock | 32 +--- 38 files changed, 114 insertions(+), 608 deletions(-) delete mode 100755 .github/scripts/backend-barretenberg-build.sh delete mode 100755 .github/scripts/backend-barretenberg-test.sh delete mode 100644 tooling/noir_js_backend_barretenberg/.eslintignore delete mode 100644 tooling/noir_js_backend_barretenberg/.eslintrc.cjs delete mode 100644 tooling/noir_js_backend_barretenberg/.gitignore delete mode 100644 tooling/noir_js_backend_barretenberg/.mocharc.json delete mode 100755 tooling/noir_js_backend_barretenberg/fixup.sh delete mode 100644 tooling/noir_js_backend_barretenberg/package.json delete mode 100644 tooling/noir_js_backend_barretenberg/src/backend.ts delete mode 100644 tooling/noir_js_backend_barretenberg/src/base64_decode.ts delete mode 100644 tooling/noir_js_backend_barretenberg/src/index.ts delete mode 100644 tooling/noir_js_backend_barretenberg/src/public_inputs.ts delete mode 100644 tooling/noir_js_backend_barretenberg/src/serialize.ts delete mode 100644 tooling/noir_js_backend_barretenberg/src/verifier.ts delete mode 100644 tooling/noir_js_backend_barretenberg/tsconfig.cjs.json delete mode 100644 tooling/noir_js_backend_barretenberg/tsconfig.json diff --git a/.aztec-sync-commit b/.aztec-sync-commit index b47eb7bdaec..6e1f033e292 100644 --- a/.aztec-sync-commit +++ b/.aztec-sync-commit @@ -1 +1 @@ -ab0c80d7493e6bdbc58dcd517b248de6ddd6fd67 +07d6dc29db2eb04154b8f0c66bd1efa74c0e8b9d diff --git a/.github/scripts/backend-barretenberg-build.sh b/.github/scripts/backend-barretenberg-build.sh deleted file mode 100755 index d90995397d8..00000000000 --- a/.github/scripts/backend-barretenberg-build.sh +++ /dev/null @@ -1,4 +0,0 @@ -#!/bin/bash -set -eu - -yarn workspace @noir-lang/backend_barretenberg build diff --git a/.github/scripts/backend-barretenberg-test.sh b/.github/scripts/backend-barretenberg-test.sh deleted file mode 100755 index 1bd6f8e410d..00000000000 --- a/.github/scripts/backend-barretenberg-test.sh +++ /dev/null @@ -1,4 +0,0 @@ -#!/bin/bash -set -eu - -yarn workspace @noir-lang/backend_barretenberg test diff --git a/.github/workflows/test-js-packages.yml b/.github/workflows/test-js-packages.yml index d1f70eb40e1..152d8b1653e 100644 --- a/.github/workflows/test-js-packages.yml +++ b/.github/workflows/test-js-packages.yml @@ -73,7 +73,7 @@ jobs: uses: actions/upload-artifact@v4 with: name: noirc_abi_wasm - path: | + path: | ./tooling/noirc_abi_wasm/nodejs ./tooling/noirc_abi_wasm/web retention-days: 10 @@ -263,9 +263,6 @@ jobs: - name: Build noir_js_types run: yarn workspace @noir-lang/types build - - name: Build barretenberg wrapper - run: yarn workspace @noir-lang/backend_barretenberg build - - name: Run noir_js tests run: | yarn workspace @noir-lang/noir_js build @@ -416,7 +413,7 @@ jobs: - name: Setup `integration-tests` run: | # Note the lack of spaces between package names. - PACKAGES_TO_BUILD="@noir-lang/types,@noir-lang/backend_barretenberg,@noir-lang/noir_js" + PACKAGES_TO_BUILD="@noir-lang/types,@noir-lang/noir_js" yarn workspaces foreach -vtp --from "{$PACKAGES_TO_BUILD}" run build - name: Run `integration-tests` @@ -461,7 +458,7 @@ jobs: - name: Setup `integration-tests` run: | # Note the lack of spaces between package names. - PACKAGES_TO_BUILD="@noir-lang/types,@noir-lang/backend_barretenberg,@noir-lang/noir_js" + PACKAGES_TO_BUILD="@noir-lang/types,@noir-lang/noir_js" yarn workspaces foreach -vtp --from "{$PACKAGES_TO_BUILD}" run build - name: Run `integration-tests` @@ -565,7 +562,7 @@ jobs: runs-on: ubuntu-latest # We want this job to always run (even if the dependant jobs fail) as we want this job to fail rather than skipping. if: ${{ always() }} - needs: + needs: - test-acvm_js-node - test-acvm_js-browser - test-noirc-abi diff --git a/acvm-repo/acir/codegen/acir.cpp b/acvm-repo/acir/codegen/acir.cpp index 637ac2ce201..1bb8931c642 100644 --- a/acvm-repo/acir/codegen/acir.cpp +++ b/acvm-repo/acir/codegen/acir.cpp @@ -286,7 +286,7 @@ namespace Program { }; struct Keccakf1600 { - Program::HeapVector message; + Program::HeapArray input; Program::HeapArray output; friend bool operator==(const Keccakf1600&, const Keccakf1600&); @@ -424,8 +424,8 @@ namespace Program { }; struct Sha256Compression { - Program::HeapVector input; - Program::HeapVector hash_values; + Program::HeapArray input; + Program::HeapArray hash_values; Program::HeapArray output; friend bool operator==(const Sha256Compression&, const Sha256Compression&); @@ -3498,7 +3498,7 @@ Program::BlackBoxOp::Blake3 serde::Deserializable:: namespace Program { inline bool operator==(const BlackBoxOp::Keccakf1600 &lhs, const BlackBoxOp::Keccakf1600 &rhs) { - if (!(lhs.message == rhs.message)) { return false; } + if (!(lhs.input == rhs.input)) { return false; } if (!(lhs.output == rhs.output)) { return false; } return true; } @@ -3523,7 +3523,7 @@ namespace Program { template <> template void serde::Serializable::serialize(const Program::BlackBoxOp::Keccakf1600 &obj, Serializer &serializer) { - serde::Serializable::serialize(obj.message, serializer); + serde::Serializable::serialize(obj.input, serializer); serde::Serializable::serialize(obj.output, serializer); } @@ -3531,7 +3531,7 @@ template <> template Program::BlackBoxOp::Keccakf1600 serde::Deserializable::deserialize(Deserializer &deserializer) { Program::BlackBoxOp::Keccakf1600 obj; - obj.message = serde::Deserializable::deserialize(deserializer); + obj.input = serde::Deserializable::deserialize(deserializer); obj.output = serde::Deserializable::deserialize(deserializer); return obj; } diff --git a/acvm-repo/brillig/src/black_box.rs b/acvm-repo/brillig/src/black_box.rs index b61c2272587..3264388c8ef 100644 --- a/acvm-repo/brillig/src/black_box.rs +++ b/acvm-repo/brillig/src/black_box.rs @@ -24,7 +24,7 @@ pub enum BlackBoxOp { }, /// Keccak Permutation function of 1600 width Keccakf1600 { - message: HeapVector, + input: HeapArray, output: HeapArray, }, /// Verifies a ECDSA signature over the secp256k1 curve. @@ -102,8 +102,8 @@ pub enum BlackBoxOp { len: MemoryAddress, }, Sha256Compression { - input: HeapVector, - hash_values: HeapVector, + input: HeapArray, + hash_values: HeapArray, output: HeapArray, }, ToRadix { diff --git a/acvm-repo/brillig_vm/src/black_box.rs b/acvm-repo/brillig_vm/src/black_box.rs index 4201d2ddba2..04dd85d9324 100644 --- a/acvm-repo/brillig_vm/src/black_box.rs +++ b/acvm-repo/brillig_vm/src/black_box.rs @@ -77,8 +77,8 @@ pub(crate) fn evaluate_black_box memory.write_slice(memory.read_ref(output.pointer), &to_value_vec(&bytes)); Ok(()) } - BlackBoxOp::Keccakf1600 { message, output } => { - let state_vec: Vec = read_heap_vector(memory, message) + BlackBoxOp::Keccakf1600 { input, output } => { + let state_vec: Vec = read_heap_array(memory, input) .iter() .map(|&memory_value| memory_value.try_into().unwrap()) .collect(); @@ -292,7 +292,7 @@ pub(crate) fn evaluate_black_box } BlackBoxOp::Sha256Compression { input, hash_values, output } => { let mut message = [0; 16]; - let inputs = read_heap_vector(memory, input); + let inputs = read_heap_array(memory, input); if inputs.len() != 16 { return Err(BlackBoxResolutionError::Failed( BlackBoxFunc::Sha256Compression, @@ -303,7 +303,7 @@ pub(crate) fn evaluate_black_box message[i] = input.try_into().unwrap(); } let mut state = [0; 8]; - let values = read_heap_vector(memory, hash_values); + let values = read_heap_array(memory, hash_values); if values.len() != 8 { return Err(BlackBoxResolutionError::Failed( BlackBoxFunc::Sha256Compression, diff --git a/compiler/integration-tests/package.json b/compiler/integration-tests/package.json index 64a638539d5..62ae699fc4e 100644 --- a/compiler/integration-tests/package.json +++ b/compiler/integration-tests/package.json @@ -1,34 +1,34 @@ { - "name": "integration-tests", - "license": "(MIT OR Apache-2.0)", - "main": "index.js", - "private": true, - "scripts": { - "build": "echo Integration Test build step", - "test": "yarn test:browser && yarn test:node", - "test:node": "bash ./scripts/setup.sh && hardhat test test/node/prove_and_verify.test.ts && hardhat test test/node/smart_contract_verifier.test.ts && hardhat test test/node/onchain_recursive_verification.test.ts", - "test:browser": "web-test-runner", - "test:integration:browser": "web-test-runner test/browser/**/*.test.ts", - "test:integration:browser:watch": "web-test-runner test/browser/**/*.test.ts --watch", - "lint": "NODE_NO_WARNINGS=1 eslint . --ext .ts --ignore-path ./.eslintignore --max-warnings 0" - }, - "dependencies": { - "@noir-lang/backend_barretenberg": "workspace:*", - "@noir-lang/noir_js": "workspace:*", - "@noir-lang/noir_wasm": "workspace:*", - "@nomicfoundation/hardhat-chai-matchers": "^2.0.0", - "@nomicfoundation/hardhat-ethers": "^3.0.0", - "@web/dev-server-esbuild": "^0.3.6", - "@web/dev-server-import-maps": "^0.2.0", - "@web/test-runner": "^0.18.1", - "@web/test-runner-playwright": "^0.11.0", - "eslint": "^8.57.0", - "eslint-plugin-prettier": "^5.1.3", - "ethers": "^6.7.1", - "hardhat": "^2.22.6", - "prettier": "3.2.5", - "smol-toml": "^1.1.2", - "toml": "^3.0.0", - "tslog": "^4.9.2" - } + "name": "integration-tests", + "license": "(MIT OR Apache-2.0)", + "main": "index.js", + "private": true, + "scripts": { + "build": "echo Integration Test build step", + "test": "yarn test:browser && yarn test:node", + "test:node": "bash ./scripts/setup.sh && hardhat test test/node/prove_and_verify.test.ts && hardhat test test/node/smart_contract_verifier.test.ts && hardhat test test/node/onchain_recursive_verification.test.ts", + "test:browser": "web-test-runner", + "test:integration:browser": "web-test-runner test/browser/**/*.test.ts", + "test:integration:browser:watch": "web-test-runner test/browser/**/*.test.ts --watch", + "lint": "NODE_NO_WARNINGS=1 eslint . --ext .ts --ignore-path ./.eslintignore --max-warnings 0" + }, + "dependencies": { + "@aztec/bb.js": "0.60.0", + "@noir-lang/noir_js": "workspace:*", + "@noir-lang/noir_wasm": "workspace:*", + "@nomicfoundation/hardhat-chai-matchers": "^2.0.0", + "@nomicfoundation/hardhat-ethers": "^3.0.0", + "@web/dev-server-esbuild": "^0.3.6", + "@web/dev-server-import-maps": "^0.2.0", + "@web/test-runner": "^0.18.1", + "@web/test-runner-playwright": "^0.11.0", + "eslint": "^8.57.0", + "eslint-plugin-prettier": "^5.1.3", + "ethers": "^6.7.1", + "hardhat": "^2.22.6", + "prettier": "3.2.5", + "smol-toml": "^1.1.2", + "toml": "^3.0.0", + "tslog": "^4.9.2" + } } diff --git a/compiler/integration-tests/test/browser/compile_prove_verify.test.ts b/compiler/integration-tests/test/browser/compile_prove_verify.test.ts index f18ceb85276..8d24b27ac25 100644 --- a/compiler/integration-tests/test/browser/compile_prove_verify.test.ts +++ b/compiler/integration-tests/test/browser/compile_prove_verify.test.ts @@ -4,7 +4,7 @@ import * as TOML from 'smol-toml'; import { compile, createFileManager } from '@noir-lang/noir_wasm'; import { Noir } from '@noir-lang/noir_js'; import { InputMap } from '@noir-lang/noirc_abi'; -import { BarretenbergBackend } from '@noir-lang/backend_barretenberg'; +import { UltraPlonkBackend } from '@aztec/bb.js'; import { getFile } from './utils.js'; @@ -59,7 +59,7 @@ test_cases.forEach((testInfo) => { const program = new Noir(noir_program); const { witness } = await program.execute(inputs); - const backend = new BarretenbergBackend(noir_program); + const backend = new UltraPlonkBackend(noir_program.bytecode); const proof = await backend.generateProof(witness); // JS verification diff --git a/compiler/integration-tests/test/browser/recursion.test.ts b/compiler/integration-tests/test/browser/recursion.test.ts index abbee7b96ad..4ee92d5b795 100644 --- a/compiler/integration-tests/test/browser/recursion.test.ts +++ b/compiler/integration-tests/test/browser/recursion.test.ts @@ -5,7 +5,7 @@ import { Logger } from 'tslog'; import { acvm, abi, Noir } from '@noir-lang/noir_js'; import * as TOML from 'smol-toml'; -import { BarretenbergBackend } from '@noir-lang/backend_barretenberg'; +import { UltraPlonkBackend } from '@aztec/bb.js'; import { getFile } from './utils.js'; import { Field, InputMap } from '@noir-lang/noirc_abi'; import { createFileManager, compile } from '@noir-lang/noir_wasm'; @@ -45,7 +45,7 @@ describe('It compiles noir program code, receiving circuit bytes and abi object. const main_program = await getCircuit(`${base_relative_path}/${circuit_main}`); const main_inputs: InputMap = TOML.parse(circuit_main_toml) as InputMap; - const main_backend = new BarretenbergBackend(main_program); + const main_backend = new UltraPlonkBackend(main_program.bytecode); const { witness: main_witnessUint8Array } = await new Noir(main_program).execute(main_inputs); @@ -73,7 +73,7 @@ describe('It compiles noir program code, receiving circuit bytes and abi object. const recursion_program = await getCircuit(`${base_relative_path}/${circuit_recursion}`); - const recursion_backend = new BarretenbergBackend(recursion_program); + const recursion_backend = new UltraPlonkBackend(recursion_program.bytecode); const { witness: recursion_witnessUint8Array } = await new Noir(recursion_program).execute(recursion_inputs); diff --git a/compiler/integration-tests/test/node/onchain_recursive_verification.test.ts b/compiler/integration-tests/test/node/onchain_recursive_verification.test.ts index f4647c478f1..b08d52e1604 100644 --- a/compiler/integration-tests/test/node/onchain_recursive_verification.test.ts +++ b/compiler/integration-tests/test/node/onchain_recursive_verification.test.ts @@ -6,7 +6,7 @@ import { resolve, join } from 'path'; import toml from 'toml'; import { Noir } from '@noir-lang/noir_js'; -import { BarretenbergBackend } from '@noir-lang/backend_barretenberg'; +import { UltraPlonkBackend } from '@aztec/bb.js'; import { Field, InputMap } from '@noir-lang/noirc_abi'; import { compile, createFileManager } from '@noir-lang/noir_wasm'; @@ -35,7 +35,7 @@ it.skip(`smart contract can verify a recursive proof`, async () => { // Intermediate proof - const inner_backend = new BarretenbergBackend(innerProgram); + const inner_backend = new UltraPlonkBackend(innerProgram.bytecode); const inner = new Noir(innerProgram); const inner_prover_toml = readFileSync( @@ -67,7 +67,7 @@ it.skip(`smart contract can verify a recursive proof`, async () => { const { witness: recursionWitness } = await recursion.execute(recursion_inputs); - const recursion_backend = new BarretenbergBackend(recursionProgram); + const recursion_backend = new UltraPlonkBackend(recursionProgram.bytecode); const recursion_proof = await recursion_backend.generateProof(recursionWitness); expect(await recursion_backend.verifyProof(recursion_proof)).to.be.true; diff --git a/compiler/integration-tests/test/node/prove_and_verify.test.ts b/compiler/integration-tests/test/node/prove_and_verify.test.ts index babc8ca5bb8..4353bccd90d 100644 --- a/compiler/integration-tests/test/node/prove_and_verify.test.ts +++ b/compiler/integration-tests/test/node/prove_and_verify.test.ts @@ -2,18 +2,13 @@ import { expect } from 'chai'; import assert_lt_json from '../../circuits/assert_lt/target/assert_lt.json' assert { type: 'json' }; import fold_fibonacci_json from '../../circuits/fold_fibonacci/target/fold_fibonacci.json' assert { type: 'json' }; import { Noir } from '@noir-lang/noir_js'; -import { - BarretenbergBackend as Backend, - BarretenbergVerifier as Verifier, - UltraHonkBackend, - UltraHonkVerifier, -} from '@noir-lang/backend_barretenberg'; +import { BarretenbergVerifier, UltraPlonkBackend, UltraHonkBackend } from '@aztec/bb.js'; import { CompiledCircuit } from '@noir-lang/types'; const assert_lt_program = assert_lt_json as CompiledCircuit; const fold_fibonacci_program = fold_fibonacci_json as CompiledCircuit; -const backend = new Backend(assert_lt_program); +const backend = new UltraPlonkBackend(assert_lt_program.bytecode); it('end-to-end proof creation and verification (outer)', async () => { // Noir.Js part @@ -53,8 +48,8 @@ it('end-to-end proof creation and verification (outer) -- Verifier API', async ( const verificationKey = await backend.getVerificationKey(); // Proof verification - const verifier = new Verifier(); - const isValid = await verifier.verifyProof(proof, verificationKey); + const verifier = new BarretenbergVerifier(); + const isValid = await verifier.verifyUltraPlonkProof(proof, verificationKey); expect(isValid).to.be.true; }); @@ -94,7 +89,7 @@ it('end-to-end proving and verification with different instances', async () => { // bb.js part const proof = await backend.generateProof(witness); - const verifier = new Backend(assert_lt_program); + const verifier = new UltraPlonkBackend(assert_lt_program.bytecode); const proof_is_valid = await verifier.verifyProof(proof); expect(proof_is_valid).to.be.true; }); @@ -148,7 +143,7 @@ it('end-to-end proof creation and verification for multiple ACIR circuits (inner // bb.js part // // Proof creation - const backend = new Backend(fold_fibonacci_program); + const backend = new UltraPlonkBackend(fold_fibonacci_program.bytecode); const proof = await backend.generateProof(witness); // Proof verification @@ -156,7 +151,7 @@ it('end-to-end proof creation and verification for multiple ACIR circuits (inner expect(isValid).to.be.true; }); -const honkBackend = new UltraHonkBackend(assert_lt_program); +const honkBackend = new UltraHonkBackend(assert_lt_program.bytecode); it('UltraHonk end-to-end proof creation and verification (outer)', async () => { // Noir.Js part @@ -196,8 +191,8 @@ it('UltraHonk end-to-end proof creation and verification (outer) -- Verifier API const verificationKey = await honkBackend.getVerificationKey(); // Proof verification - const verifier = new UltraHonkVerifier(); - const isValid = await verifier.verifyProof(proof, verificationKey); + const verifier = new BarretenbergVerifier(); + const isValid = await verifier.verifyUltraHonkProof(proof, verificationKey); expect(isValid).to.be.true; }); @@ -236,7 +231,7 @@ it('UltraHonk end-to-end proving and verification with different instances', asy // bb.js part const proof = await honkBackend.generateProof(witness); - const verifier = new UltraHonkBackend(assert_lt_program); + const verifier = new UltraHonkBackend(assert_lt_program.bytecode); const proof_is_valid = await verifier.verifyProof(proof); expect(proof_is_valid).to.be.true; }); @@ -283,7 +278,7 @@ it('UltraHonk end-to-end proof creation and verification for multiple ACIR circu // bb.js part // // Proof creation - const honkBackend = new UltraHonkBackend(fold_fibonacci_program); + const honkBackend = new UltraHonkBackend(fold_fibonacci_program.bytecode); const proof = await honkBackend.generateProof(witness); // Proof verification diff --git a/compiler/integration-tests/test/node/smart_contract_verifier.test.ts b/compiler/integration-tests/test/node/smart_contract_verifier.test.ts index e109cbcab6a..c43fba01424 100644 --- a/compiler/integration-tests/test/node/smart_contract_verifier.test.ts +++ b/compiler/integration-tests/test/node/smart_contract_verifier.test.ts @@ -6,7 +6,7 @@ import { resolve } from 'path'; import toml from 'toml'; import { Noir } from '@noir-lang/noir_js'; -import { BarretenbergBackend } from '@noir-lang/backend_barretenberg'; +import { UltraPlonkBackend } from '@aztec/bb.js'; import { compile, createFileManager } from '@noir-lang/noir_wasm'; @@ -46,7 +46,7 @@ test_cases.forEach((testInfo) => { const inputs = toml.parse(prover_toml); const { witness } = await program.execute(inputs); - const backend = new BarretenbergBackend(noir_program); + const backend = new UltraPlonkBackend(noir_program.bytecode); const proofData = await backend.generateProof(witness); // JS verification 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 10c0e8b8e8c..55f89dcb30f 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 @@ -60,19 +60,22 @@ pub(crate) fn convert_black_box_call { - if let ([message], [BrilligVariable::BrilligArray(result_array)]) = - (function_arguments, function_results) + if let ( + [BrilligVariable::BrilligArray(input_array)], + [BrilligVariable::BrilligArray(result_array)], + ) = (function_arguments, function_results) { - let message_vector = convert_array_or_vector(brillig_context, *message, bb_func); + let input_heap_array = + brillig_context.codegen_brillig_array_to_heap_array(*input_array); let output_heap_array = brillig_context.codegen_brillig_array_to_heap_array(*result_array); brillig_context.black_box_op_instruction(BlackBoxOp::Keccakf1600 { - message: message_vector, + input: input_heap_array, output: output_heap_array, }); - brillig_context.deallocate_heap_vector(message_vector); + brillig_context.deallocate_heap_array(input_heap_array); brillig_context.deallocate_heap_array(output_heap_array); } else { unreachable!("ICE: Keccakf1600 expects one array argument and one array result") @@ -348,21 +351,23 @@ pub(crate) fn convert_black_box_call { - if let ([message, hash_values], [BrilligVariable::BrilligArray(result_array)]) = - (function_arguments, function_results) + if let ( + [BrilligVariable::BrilligArray(input_array), BrilligVariable::BrilligArray(hash_values)], + [BrilligVariable::BrilligArray(result_array)], + ) = (function_arguments, function_results) { - let message_vector = convert_array_or_vector(brillig_context, *message, bb_func); - let hash_values = convert_array_or_vector(brillig_context, *hash_values, bb_func); + let input = brillig_context.codegen_brillig_array_to_heap_array(*input_array); + let hash_values = brillig_context.codegen_brillig_array_to_heap_array(*hash_values); let output = brillig_context.codegen_brillig_array_to_heap_array(*result_array); brillig_context.black_box_op_instruction(BlackBoxOp::Sha256Compression { - input: message_vector, + input, hash_values, output, }); - brillig_context.deallocate_heap_vector(message_vector); - brillig_context.deallocate_heap_vector(hash_values); + brillig_context.deallocate_heap_array(input); + brillig_context.deallocate_heap_array(hash_values); brillig_context.deallocate_heap_array(output); } else { unreachable!("ICE: Sha256Compression expects two array argument, one array result") diff --git a/compiler/noirc_evaluator/src/brillig/brillig_ir/debug_show.rs b/compiler/noirc_evaluator/src/brillig/brillig_ir/debug_show.rs index 7597da2be05..2a46a04cc91 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_ir/debug_show.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_ir/debug_show.rs @@ -270,8 +270,8 @@ impl DebugShow { outputs ); } - BlackBoxOp::Keccakf1600 { message, output } => { - debug_println!(self.enable_debug_trace, " KECCAKF1600 {} -> {}", message, output); + BlackBoxOp::Keccakf1600 { input, output } => { + debug_println!(self.enable_debug_trace, " KECCAKF1600 {} -> {}", input, output); } BlackBoxOp::Blake2s { message, output } => { debug_println!(self.enable_debug_trace, " BLAKE2S {} -> {}", message, output); diff --git a/docs/docs/how_to/how-to-recursion.md b/docs/docs/how_to/how-to-recursion.md index c8c4dc9f5b4..fac79a9cf49 100644 --- a/docs/docs/how_to/how-to-recursion.md +++ b/docs/docs/how_to/how-to-recursion.md @@ -1,6 +1,6 @@ --- title: How to use recursion on NoirJS -description: Learn how to implement recursion with NoirJS, a powerful tool for creating smart contracts on the EVM blockchain. This guide assumes familiarity with NoirJS, solidity verifiers, and the Barretenberg proving backend. Discover how to generate both final and intermediate proofs using `noir_js` and `backend_barretenberg`. +description: Learn how to implement recursion with NoirJS, a powerful tool for creating smart contracts on the EVM blockchain. This guide assumes familiarity with NoirJS, solidity verifiers, and the Barretenberg proving backend. Discover how to generate both final and intermediate proofs using `noir_js` and `bb.js`. keywords: [ "NoirJS", @@ -10,7 +10,6 @@ keywords: "solidity verifiers", "Barretenberg backend", "noir_js", - "backend_barretenberg", "intermediate proofs", "final proofs", "nargo compile", @@ -33,13 +32,6 @@ It is also assumed that you're not using `noir_wasm` for compilation, and instea As you've read in the [explainer](../explainers/explainer-recursion.md), a recursive proof is an intermediate proof. This means that it doesn't necessarily generate the final step that makes it verifiable in a smart contract. However, it is easy to verify within another circuit. -While "standard" usage of NoirJS packages abstracts final proofs, it currently lacks the necessary interface to abstract away intermediate proofs. This means that these proofs need to be created by using the backend directly. - -In short: - -- `noir_js` generates *only* final proofs -- `backend_barretenberg` generates both types of proofs - ::: In a standard recursive app, you're also dealing with at least two circuits. For the purpose of this guide, we will assume the following: @@ -147,7 +139,7 @@ Managing circuits and "who does what" can be confusing. To make sure your naming ```js const circuits = { - main: mainJSON, + main: mainJSON, recursive: recursiveJSON } const backends = { diff --git a/docs/docusaurus.config.ts b/docs/docusaurus.config.ts index c7af7e494d1..c53d11e3373 100644 --- a/docs/docusaurus.config.ts +++ b/docs/docusaurus.config.ts @@ -181,37 +181,6 @@ export default { membersWithOwnFile: ['Interface', 'Class', 'TypeAlias', 'Function'], }, ], - [ - 'docusaurus-plugin-typedoc', - { - id: 'noir_js_backend_barretenberg', - entryPoints: ['../tooling/noir_js_backend_barretenberg/src/index.ts'], - tsconfig: '../tooling/noir_js_backend_barretenberg/tsconfig.json', - entryPointStrategy: 'resolve', - out: 'processed-docs/reference/NoirJS/backend_barretenberg', - plugin: ['typedoc-plugin-markdown'], - name: 'backend_barretenberg', - disableSources: true, - excludePrivate: true, - skipErrorChecking: true, - sidebar: { - filteredIds: ['reference/NoirJS/backend_barretenberg/index'], - }, - readme: 'none', - hidePageHeader: true, - hideBreadcrumbs: true, - hideInPageTOC: true, - useCodeBlocks: true, - typeDeclarationFormat: 'table', - propertiesFormat: 'table', - parametersFormat: 'table', - enumMembersFormat: 'table', - indexFormat: 'table', - outputFileStrategy: 'members', - memberPageTitle: '{name}', - membersWithOwnFile: ['Interface', 'Class', 'TypeAlias'], - }, - ], [ 'docusaurus-plugin-typedoc', { diff --git a/docs/versioned_docs/version-v0.36.0/how_to/_category_.json b/docs/versioned_docs/version-v0.36.0/how_to/_category_.json index 23b560f610b..0b4f367ce53 100644 --- a/docs/versioned_docs/version-v0.36.0/how_to/_category_.json +++ b/docs/versioned_docs/version-v0.36.0/how_to/_category_.json @@ -1,5 +1,5 @@ { - "position": 1, - "collapsible": true, - "collapsed": true + "position": 1, + "collapsible": true, + "collapsed": true } diff --git a/docs/versioned_docs/version-v0.36.0/noir/standard_library/_category_.json b/docs/versioned_docs/version-v0.36.0/noir/standard_library/_category_.json index af04c0933fd..e275f03bc6a 100644 --- a/docs/versioned_docs/version-v0.36.0/noir/standard_library/_category_.json +++ b/docs/versioned_docs/version-v0.36.0/noir/standard_library/_category_.json @@ -1,6 +1,6 @@ { - "label": "Standard Library", - "position": 1, - "collapsible": true, - "collapsed": true + "label": "Standard Library", + "position": 1, + "collapsible": true, + "collapsed": true } diff --git a/docs/versioned_docs/version-v0.36.0/reference/debugger/_category_.json b/docs/versioned_docs/version-v0.36.0/reference/debugger/_category_.json index 27869205ad3..186aeb3654e 100644 --- a/docs/versioned_docs/version-v0.36.0/reference/debugger/_category_.json +++ b/docs/versioned_docs/version-v0.36.0/reference/debugger/_category_.json @@ -1,6 +1,6 @@ { - "label": "Debugger", - "position": 1, - "collapsible": true, - "collapsed": true + "label": "Debugger", + "position": 1, + "collapsible": true, + "collapsed": true } diff --git a/package.json b/package.json index 8abaced7bdd..00036838f9b 100644 --- a/package.json +++ b/package.json @@ -8,7 +8,6 @@ "tooling/noirc_abi_wasm", "tooling/noir_js", "tooling/noir_codegen", - "tooling/noir_js_backend_barretenberg", "acvm-repo/acvm_js", "docs" ], diff --git a/scripts/bump-bb.sh b/scripts/bump-bb.sh index 36c1f78ca05..661a8eb9d43 100755 --- a/scripts/bump-bb.sh +++ b/scripts/bump-bb.sh @@ -5,7 +5,7 @@ BB_VERSION=$1 sed -i.bak "s/^VERSION=.*/VERSION=\"$BB_VERSION\"/" ./scripts/install_bb.sh && rm ./scripts/install_bb.sh.bak tmp=$(mktemp) -BACKEND_BARRETENBERG_PACKAGE_JSON=./tooling/noir_js_backend_barretenberg/package.json -jq --arg v $BB_VERSION '.dependencies."@aztec/bb.js" = $v' $BACKEND_BARRETENBERG_PACKAGE_JSON > $tmp && mv $tmp $BACKEND_BARRETENBERG_PACKAGE_JSON +INTEGRATION_TESTS_PACKAGE_JSON=./compiler/integration-tests/package.json +jq --arg v $BB_VERSION '.dependencies."@aztec/bb.js" = $v' $INTEGRATION_TESTS_PACKAGE_JSON > $tmp && mv $tmp $INTEGRATION_TESTS_PACKAGE_JSON YARN_ENABLE_IMMUTABLE_INSTALLS=false yarn install diff --git a/scripts/install_bb.sh b/scripts/install_bb.sh index 596f0a54ba4..64baf78c182 100755 --- a/scripts/install_bb.sh +++ b/scripts/install_bb.sh @@ -1,6 +1,6 @@ #!/bin/bash -VERSION="0.58.0" +VERSION="0.60.0" BBUP_PATH=~/.bb/bbup diff --git a/tooling/noir_js_backend_barretenberg/.eslintignore b/tooling/noir_js_backend_barretenberg/.eslintignore deleted file mode 100644 index b512c09d476..00000000000 --- a/tooling/noir_js_backend_barretenberg/.eslintignore +++ /dev/null @@ -1 +0,0 @@ -node_modules \ No newline at end of file diff --git a/tooling/noir_js_backend_barretenberg/.eslintrc.cjs b/tooling/noir_js_backend_barretenberg/.eslintrc.cjs deleted file mode 100644 index 33335c2a877..00000000000 --- a/tooling/noir_js_backend_barretenberg/.eslintrc.cjs +++ /dev/null @@ -1,3 +0,0 @@ -module.exports = { - extends: ["../../.eslintrc.js"], -}; diff --git a/tooling/noir_js_backend_barretenberg/.gitignore b/tooling/noir_js_backend_barretenberg/.gitignore deleted file mode 100644 index 689b3ca0701..00000000000 --- a/tooling/noir_js_backend_barretenberg/.gitignore +++ /dev/null @@ -1,2 +0,0 @@ -crs -lib diff --git a/tooling/noir_js_backend_barretenberg/.mocharc.json b/tooling/noir_js_backend_barretenberg/.mocharc.json deleted file mode 100644 index e1023f56327..00000000000 --- a/tooling/noir_js_backend_barretenberg/.mocharc.json +++ /dev/null @@ -1,11 +0,0 @@ -{ - "require": "ts-node/register", - "loader": "ts-node/esm", - "extensions": [ - "ts", - "cjs" - ], - "spec": [ - "test/**/*.test.ts*" - ] -} diff --git a/tooling/noir_js_backend_barretenberg/fixup.sh b/tooling/noir_js_backend_barretenberg/fixup.sh deleted file mode 100755 index 80bd7ec71a5..00000000000 --- a/tooling/noir_js_backend_barretenberg/fixup.sh +++ /dev/null @@ -1,18 +0,0 @@ -#!/usr/bin/env bash - -# Put these package.json files in the cjs and -# mjs directory respectively, so that -# tools can recognise that the .js files are either -# commonjs or ESM files. -self_path=$(dirname "$(readlink -f "$0")") - -cjs_package='{ - "type": "commonjs" -}' - -esm_package='{ - "type": "module" -}' - -echo "$cjs_package" > $self_path/lib/cjs/package.json -echo "$esm_package" > $self_path/lib/esm/package.json \ No newline at end of file diff --git a/tooling/noir_js_backend_barretenberg/package.json b/tooling/noir_js_backend_barretenberg/package.json deleted file mode 100644 index 19306b69d8d..00000000000 --- a/tooling/noir_js_backend_barretenberg/package.json +++ /dev/null @@ -1,57 +0,0 @@ -{ - "name": "@noir-lang/backend_barretenberg", - "contributors": [ - "The Noir Team " - ], - "version": "0.36.0", - "packageManager": "yarn@3.5.1", - "license": "(MIT OR Apache-2.0)", - "type": "module", - "homepage": "https://noir-lang.org/", - "repository": { - "url": "https://github.com/noir-lang/noir.git", - "directory": "tooling/noir_js_backend_barretenberg", - "type": "git" - }, - "bugs": { - "url": "https://github.com/noir-lang/noir/issues" - }, - "source": "src/index.ts", - "main": "lib/cjs/index.js", - "module": "lib/esm/index.js", - "exports": { - "require": "./lib/cjs/index.js", - "types": "./lib/esm/index.d.ts", - "default": "./lib/esm/index.js" - }, - "types": "lib/esm/index.d.ts", - "files": [ - "lib", - "package.json" - ], - "scripts": { - "dev": "tsc --watch", - "generate:package": "bash ./fixup.sh", - "build": "yarn clean && tsc && tsc -p ./tsconfig.cjs.json && yarn generate:package", - "clean": "rm -rf ./lib", - "prettier": "prettier 'src/**/*.ts'", - "prettier:fix": "prettier --write 'src/**/*.ts' 'test/**/*.ts'", - "nightly:version": "jq --arg new_version \"-$(git rev-parse --short HEAD)$1\" '.version = .version + $new_version' package.json > package-tmp.json && mv package-tmp.json package.json", - "publish": "echo 📡 publishing `$npm_package_name` && yarn npm publish", - "lint": "NODE_NO_WARNINGS=1 eslint . --ext .ts --ignore-path ./.eslintignore --max-warnings 0" - }, - "dependencies": { - "@aztec/bb.js": "0.58.0", - "@noir-lang/types": "workspace:*", - "fflate": "^0.8.0" - }, - "devDependencies": { - "@types/node": "^20.6.2", - "@types/prettier": "^3", - "eslint": "^8.57.0", - "eslint-plugin-prettier": "^5.1.3", - "prettier": "3.2.5", - "ts-node": "^10.9.1", - "typescript": "5.4.2" - } -} diff --git a/tooling/noir_js_backend_barretenberg/src/backend.ts b/tooling/noir_js_backend_barretenberg/src/backend.ts deleted file mode 100644 index 785b0c73421..00000000000 --- a/tooling/noir_js_backend_barretenberg/src/backend.ts +++ /dev/null @@ -1,144 +0,0 @@ -import { acirToUint8Array } from './serialize.js'; -import { Backend, CompiledCircuit, ProofData, VerifierBackend } from '@noir-lang/types'; -import { deflattenFields } from './public_inputs.js'; -import { reconstructProofWithPublicInputs, reconstructProofWithPublicInputsHonk } from './verifier.js'; -import { BackendOptions, UltraPlonkBackend, UltraHonkBackend as UltraHonkBackendInternal } from '@aztec/bb.js'; -import { decompressSync as gunzip } from 'fflate'; - -// This is the number of bytes in a UltraPlonk proof -// minus the public inputs. -const numBytesInProofWithoutPublicInputs: number = 2144; - -export class BarretenbergBackend implements Backend, VerifierBackend { - protected backend!: UltraPlonkBackend; - - constructor(acirCircuit: CompiledCircuit, options: BackendOptions = { threads: 1 }) { - const acirBytecodeBase64 = acirCircuit.bytecode; - const acirUncompressedBytecode = acirToUint8Array(acirBytecodeBase64); - this.backend = new UltraPlonkBackend(acirUncompressedBytecode, options); - } - - /** @description Generates a proof */ - async generateProof(compressedWitness: Uint8Array): Promise { - const proofWithPublicInputs = await this.backend.generateProof(gunzip(compressedWitness)); - - const splitIndex = proofWithPublicInputs.length - numBytesInProofWithoutPublicInputs; - - const publicInputsConcatenated = proofWithPublicInputs.slice(0, splitIndex); - const proof = proofWithPublicInputs.slice(splitIndex); - const publicInputs = deflattenFields(publicInputsConcatenated); - - return { proof, publicInputs }; - } - - /** - * Generates artifacts that will be passed to a circuit that will verify this proof. - * - * Instead of passing the proof and verification key as a byte array, we pass them - * as fields which makes it cheaper to verify in a circuit. - * - * The proof that is passed here will have been created using a circuit - * that has the #[recursive] attribute on its `main` method. - * - * The number of public inputs denotes how many public inputs are in the inner proof. - * - * @example - * ```typescript - * const artifacts = await backend.generateRecursiveProofArtifacts(proof, numOfPublicInputs); - * ``` - */ - async generateRecursiveProofArtifacts( - proofData: ProofData, - numOfPublicInputs = 0, - ): Promise<{ - proofAsFields: string[]; - vkAsFields: string[]; - vkHash: string; - }> { - const proof = reconstructProofWithPublicInputs(proofData); - return this.backend.generateRecursiveProofArtifacts(proof, numOfPublicInputs); - } - - /** @description Verifies a proof */ - async verifyProof(proofData: ProofData): Promise { - const proof = reconstructProofWithPublicInputs(proofData); - return this.backend.verifyProof(proof); - } - - async getVerificationKey(): Promise { - return this.backend.getVerificationKey(); - } - - async destroy(): Promise { - await this.backend.destroy(); - } -} - -// Buffers are prepended with their size. The size takes 4 bytes. -const serializedBufferSize = 4; -const fieldByteSize = 32; -const publicInputOffset = 3; -const publicInputsOffsetBytes = publicInputOffset * fieldByteSize; - -export class UltraHonkBackend implements Backend, VerifierBackend { - // These type assertions are used so that we don't - // have to initialize `api` in the constructor. - // These are initialized asynchronously in the `init` function, - // constructors cannot be asynchronous which is why we do this. - - protected backend!: UltraHonkBackendInternal; - - constructor(acirCircuit: CompiledCircuit, options: BackendOptions = { threads: 1 }) { - const acirBytecodeBase64 = acirCircuit.bytecode; - const acirUncompressedBytecode = acirToUint8Array(acirBytecodeBase64); - this.backend = new UltraHonkBackendInternal(acirUncompressedBytecode, options); - } - - async generateProof(compressedWitness: Uint8Array): Promise { - const proofWithPublicInputs = await this.backend.generateProof(gunzip(compressedWitness)); - - const proofAsStrings = deflattenFields(proofWithPublicInputs.slice(4)); - - const numPublicInputs = Number(proofAsStrings[1]); - - // Account for the serialized buffer size at start - const publicInputsOffset = publicInputsOffsetBytes + serializedBufferSize; - // Get the part before and after the public inputs - const proofStart = proofWithPublicInputs.slice(0, publicInputsOffset); - const publicInputsSplitIndex = numPublicInputs * fieldByteSize; - const proofEnd = proofWithPublicInputs.slice(publicInputsOffset + publicInputsSplitIndex); - // Construct the proof without the public inputs - const proof = new Uint8Array([...proofStart, ...proofEnd]); - - // Fetch the number of public inputs out of the proof string - const publicInputsConcatenated = proofWithPublicInputs.slice( - publicInputsOffset, - publicInputsOffset + publicInputsSplitIndex, - ); - const publicInputs = deflattenFields(publicInputsConcatenated); - - return { proof, publicInputs }; - } - - async verifyProof(proofData: ProofData): Promise { - const proof = reconstructProofWithPublicInputsHonk(proofData); - return this.backend.verifyProof(proof); - } - - async getVerificationKey(): Promise { - return this.backend.getVerificationKey(); - } - - // TODO(https://github.com/noir-lang/noir/issues/5661): Update this to handle Honk recursive aggregation in the browser once it is ready in the backend itself - async generateRecursiveProofArtifacts( - proofData: ProofData, - numOfPublicInputs: number, - ): Promise<{ proofAsFields: string[]; vkAsFields: string[]; vkHash: string }> { - const proof = reconstructProofWithPublicInputsHonk(proofData); - return this.backend.generateRecursiveProofArtifacts(proof, numOfPublicInputs); - } - - async destroy(): Promise { - await this.backend.destroy(); - } -} diff --git a/tooling/noir_js_backend_barretenberg/src/base64_decode.ts b/tooling/noir_js_backend_barretenberg/src/base64_decode.ts deleted file mode 100644 index d53aed187c7..00000000000 --- a/tooling/noir_js_backend_barretenberg/src/base64_decode.ts +++ /dev/null @@ -1,13 +0,0 @@ -// Since this is a simple function, we can use feature detection to -// see if we are in the nodeJs environment or the browser environment. -export function base64Decode(input: string): Uint8Array { - if (typeof Buffer !== 'undefined') { - // Node.js environment - return Buffer.from(input, 'base64'); - } else if (typeof atob === 'function') { - // Browser environment - return Uint8Array.from(atob(input), (c) => c.charCodeAt(0)); - } else { - throw new Error('No implementation found for base64 decoding.'); - } -} diff --git a/tooling/noir_js_backend_barretenberg/src/index.ts b/tooling/noir_js_backend_barretenberg/src/index.ts deleted file mode 100644 index f1786396a2a..00000000000 --- a/tooling/noir_js_backend_barretenberg/src/index.ts +++ /dev/null @@ -1,6 +0,0 @@ -export { BarretenbergBackend, UltraHonkBackend } from './backend.js'; -export { BarretenbergVerifier, UltraHonkVerifier } from './verifier.js'; - -// typedoc exports -export { Backend, CompiledCircuit, ProofData } from '@noir-lang/types'; -export { BackendOptions } from '@aztec/bb.js'; diff --git a/tooling/noir_js_backend_barretenberg/src/public_inputs.ts b/tooling/noir_js_backend_barretenberg/src/public_inputs.ts deleted file mode 100644 index 10b4ee6ab32..00000000000 --- a/tooling/noir_js_backend_barretenberg/src/public_inputs.ts +++ /dev/null @@ -1,68 +0,0 @@ -import { WitnessMap } from '@noir-lang/types'; - -export function flattenFieldsAsArray(fields: string[]): Uint8Array { - const flattenedPublicInputs = fields.map(hexToUint8Array); - return flattenUint8Arrays(flattenedPublicInputs); -} - -export function deflattenFields(flattenedFields: Uint8Array): string[] { - const publicInputSize = 32; - const chunkedFlattenedPublicInputs: Uint8Array[] = []; - - for (let i = 0; i < flattenedFields.length; i += publicInputSize) { - const publicInput = flattenedFields.slice(i, i + publicInputSize); - chunkedFlattenedPublicInputs.push(publicInput); - } - - return chunkedFlattenedPublicInputs.map(uint8ArrayToHex); -} - -export function witnessMapToPublicInputs(publicInputs: WitnessMap): string[] { - const publicInputIndices = [...publicInputs.keys()].sort((a, b) => a - b); - const flattenedPublicInputs = publicInputIndices.map((index) => publicInputs.get(index) as string); - return flattenedPublicInputs; -} - -function flattenUint8Arrays(arrays: Uint8Array[]): Uint8Array { - const totalLength = arrays.reduce((acc, val) => acc + val.length, 0); - const result = new Uint8Array(totalLength); - - let offset = 0; - for (const arr of arrays) { - result.set(arr, offset); - offset += arr.length; - } - - return result; -} - -function uint8ArrayToHex(buffer: Uint8Array): string { - const hex: string[] = []; - - buffer.forEach(function (i) { - let h = i.toString(16); - if (h.length % 2) { - h = '0' + h; - } - hex.push(h); - }); - - return '0x' + hex.join(''); -} - -function hexToUint8Array(hex: string): Uint8Array { - const sanitised_hex = BigInt(hex).toString(16).padStart(64, '0'); - - const len = sanitised_hex.length / 2; - const u8 = new Uint8Array(len); - - let i = 0; - let j = 0; - while (i < len) { - u8[i] = parseInt(sanitised_hex.slice(j, j + 2), 16); - i += 1; - j += 2; - } - - return u8; -} diff --git a/tooling/noir_js_backend_barretenberg/src/serialize.ts b/tooling/noir_js_backend_barretenberg/src/serialize.ts deleted file mode 100644 index b15931848a0..00000000000 --- a/tooling/noir_js_backend_barretenberg/src/serialize.ts +++ /dev/null @@ -1,8 +0,0 @@ -import { decompressSync as gunzip } from 'fflate'; -import { base64Decode } from './base64_decode.js'; - -// Converts bytecode from a base64 string to a Uint8Array -export function acirToUint8Array(base64EncodedBytecode: string): Uint8Array { - const compressedByteCode = base64Decode(base64EncodedBytecode); - return gunzip(compressedByteCode); -} diff --git a/tooling/noir_js_backend_barretenberg/src/verifier.ts b/tooling/noir_js_backend_barretenberg/src/verifier.ts deleted file mode 100644 index 885ec80caa8..00000000000 --- a/tooling/noir_js_backend_barretenberg/src/verifier.ts +++ /dev/null @@ -1,67 +0,0 @@ -import { ProofData } from '@noir-lang/types'; -import { flattenFieldsAsArray } from './public_inputs.js'; -import { BackendOptions, BarretenbergVerifier as BarretenbergVerifierInternal } from '@aztec/bb.js'; - -export class BarretenbergVerifier { - private verifier!: BarretenbergVerifierInternal; - - constructor(options: BackendOptions = { threads: 1 }) { - this.verifier = new BarretenbergVerifierInternal(options); - } - - /** @description Verifies a proof */ - async verifyProof(proofData: ProofData, verificationKey: Uint8Array): Promise { - const proof = reconstructProofWithPublicInputs(proofData); - return this.verifier.verifyUltraplonkProof(proof, verificationKey); - } - - async destroy(): Promise { - await this.verifier.destroy(); - } -} - -export function reconstructProofWithPublicInputs(proofData: ProofData): Uint8Array { - // Flatten publicInputs - const publicInputsConcatenated = flattenFieldsAsArray(proofData.publicInputs); - - // Concatenate publicInputs and proof - const proofWithPublicInputs = Uint8Array.from([...publicInputsConcatenated, ...proofData.proof]); - - return proofWithPublicInputs; -} - -export class UltraHonkVerifier { - private verifier!: BarretenbergVerifierInternal; - - constructor(options: BackendOptions = { threads: 1 }) { - this.verifier = new BarretenbergVerifierInternal(options); - } - - /** @description Verifies a proof */ - async verifyProof(proofData: ProofData, verificationKey: Uint8Array): Promise { - const proof = reconstructProofWithPublicInputsHonk(proofData); - return this.verifier.verifyUltrahonkProof(proof, verificationKey); - } - - async destroy(): Promise { - await this.verifier.destroy(); - } -} - -const serializedBufferSize = 4; -const fieldByteSize = 32; -const publicInputOffset = 3; -const publicInputsOffsetBytes = publicInputOffset * fieldByteSize; - -export function reconstructProofWithPublicInputsHonk(proofData: ProofData): Uint8Array { - // Flatten publicInputs - const publicInputsConcatenated = flattenFieldsAsArray(proofData.publicInputs); - - const proofStart = proofData.proof.slice(0, publicInputsOffsetBytes + serializedBufferSize); - const proofEnd = proofData.proof.slice(publicInputsOffsetBytes + serializedBufferSize); - - // Concatenate publicInputs and proof - const proofWithPublicInputs = Uint8Array.from([...proofStart, ...publicInputsConcatenated, ...proofEnd]); - - return proofWithPublicInputs; -} diff --git a/tooling/noir_js_backend_barretenberg/tsconfig.cjs.json b/tooling/noir_js_backend_barretenberg/tsconfig.cjs.json deleted file mode 100644 index ac1e3784480..00000000000 --- a/tooling/noir_js_backend_barretenberg/tsconfig.cjs.json +++ /dev/null @@ -1,8 +0,0 @@ -{ - "extends": "./tsconfig.json", - "compilerOptions": { - "module": "CommonJS", - "moduleResolution": "Node", - "outDir": "./lib/cjs" - }, -} diff --git a/tooling/noir_js_backend_barretenberg/tsconfig.json b/tooling/noir_js_backend_barretenberg/tsconfig.json deleted file mode 100644 index 1e28c044bba..00000000000 --- a/tooling/noir_js_backend_barretenberg/tsconfig.json +++ /dev/null @@ -1,21 +0,0 @@ -{ - "compilerOptions": { - "target": "esnext", - "declaration": true, - "emitDeclarationOnly": false, - "module": "NodeNext", - "moduleResolution": "NodeNext", - "outDir": "./lib/esm", - "esModuleInterop": true, - "resolveJsonModule": true, - "strict": true, - "noImplicitAny": false, - }, - "include": ["src/**/*.ts"], - "exclude": ["node_modules"], - "references": [ - { - "path": "../noir_js_types" - } - ] -} diff --git a/yarn.lock b/yarn.lock index c0f6c0e75f3..b39f9c257c6 100644 --- a/yarn.lock +++ b/yarn.lock @@ -221,17 +221,18 @@ __metadata: languageName: node linkType: hard -"@aztec/bb.js@npm:0.58.0": - version: 0.58.0 - resolution: "@aztec/bb.js@npm:0.58.0" +"@aztec/bb.js@npm:0.60.0": + version: 0.60.0 + resolution: "@aztec/bb.js@npm:0.60.0" dependencies: comlink: ^4.4.1 commander: ^10.0.1 debug: ^4.3.4 + fflate: ^0.8.0 tslib: ^2.4.0 bin: bb.js: dest/node/main.js - checksum: b4e882a6668df737fab6e2223ee6b20ff499e8a0b67c18dcab8109efec47e674c6d8276e8c3b6662289d69f56b6e1d94d3312673fd0ace9909e33ebce7f10cbb + checksum: 74ab79d060624362e9d44dda11dc2445973460577b93dc0c16801158db7efb71cd612966d6169b46bfbbb4fd7c9c1b95ad8b6198b3c69d9f6de0ab0fb92387aa languageName: node linkType: hard @@ -5344,23 +5345,6 @@ __metadata: languageName: unknown linkType: soft -"@noir-lang/backend_barretenberg@workspace:*, @noir-lang/backend_barretenberg@workspace:tooling/noir_js_backend_barretenberg": - version: 0.0.0-use.local - resolution: "@noir-lang/backend_barretenberg@workspace:tooling/noir_js_backend_barretenberg" - dependencies: - "@aztec/bb.js": 0.58.0 - "@noir-lang/types": "workspace:*" - "@types/node": ^20.6.2 - "@types/prettier": ^3 - eslint: ^8.57.0 - eslint-plugin-prettier: ^5.1.3 - fflate: ^0.8.0 - prettier: 3.2.5 - ts-node: ^10.9.1 - typescript: 5.4.2 - languageName: unknown - linkType: soft - "@noir-lang/noir_codegen@workspace:tooling/noir_codegen": version: 0.0.0-use.local resolution: "@noir-lang/noir_codegen@workspace:tooling/noir_codegen" @@ -14139,7 +14123,7 @@ __metadata: version: 0.0.0-use.local resolution: "integration-tests@workspace:compiler/integration-tests" dependencies: - "@noir-lang/backend_barretenberg": "workspace:*" + "@aztec/bb.js": 0.60.0 "@noir-lang/noir_js": "workspace:*" "@noir-lang/noir_wasm": "workspace:*" "@nomicfoundation/hardhat-chai-matchers": ^2.0.0 @@ -21796,7 +21780,7 @@ __metadata: languageName: node linkType: hard -"typescript@npm:5.4.2, typescript@npm:^5.4.2": +"typescript@npm:^5.4.2": version: 5.4.2 resolution: "typescript@npm:5.4.2" bin: @@ -21806,7 +21790,7 @@ __metadata: languageName: node linkType: hard -"typescript@patch:typescript@5.4.2#~builtin, typescript@patch:typescript@^5.4.2#~builtin": +"typescript@patch:typescript@^5.4.2#~builtin": version: 5.4.2 resolution: "typescript@patch:typescript@npm%3A5.4.2#~builtin::version=5.4.2&hash=f3b441" bin: From ec75e8ec59e0f2a2169aea67372411ede4074d09 Mon Sep 17 00:00:00 2001 From: guipublic <47281315+guipublic@users.noreply.github.com> Date: Thu, 24 Oct 2024 23:19:27 +0200 Subject: [PATCH 7/8] feat: remove 'single use' intermediate variables (#6268) # Description ## Problem\* Resolves #6085 ## Summary\* This PR tries to benefit from Barretenberg's 'big-add gates' support, which was enabled by PR https://github.com/AztecProtocol/aztec-packages/pull/8960 It's a simple optimisation which removes intermediate variables usually created by the CSatTransformer if they are not re-used elsewhere. The PR assumes that the backend is able to handle infinite width, but still requires the CSatTransformer, which is not really consistent. I plan to make follow-up PRs to get rid of this (but I can't guarantee it will work). ## Additional Context I tested the optimisation on all 'execution_sucess' test cases. In most cases, there were no change at all, while in some cases we would win one or two (10 max) on the circuit size. However, in a few cases, listed below in the form "test case: circuit size with 'intermediate var' optimisation vs no optimisation", it can be more significant: 7_function: 2955 vs 2992 bit_shifts_runtime: 5451 vs 5761 eddsa: 65805 vs 70406 hashmap: 135023 vs 150661 nested_array_dynamic: 12594 vs 12922 nested_array_in_slice: 5371 vs 5449 poseidon_bn254_hash: 1028 vs 1060 poseidon_bn254_hash_width_3: 1028 vs 1495 poseidonsponge_x5_254: 1244 vs 1307 regression_5252: 76491 vs 83862 sha256_var_size_regression: 74093 vs 74529 sha2_byte: 93998 vs 94006 slice_dynamic_index: 6308 vs 6419 slices: 3835 vs 3874 to_be_bytes: 135 vs 143 to_bytes_consistent: 6 vs 51 to_bytes_integration: 434 vs 484 u128: 4662 vs 4707 u16_support: 3023 vs 3057 ## Documentation\* Check one: - [X] No documentation needed. - [ ] Documentation included in this PR. - [ ] **[For Experimental Features]** Documentation to be submitted in a separate PR. # PR Checklist\* - [X] I have tested the changes locally. - [X] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings. Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com> --- .../opcodes/black_box_function_call.rs | 12 ++ .../compiler/optimizers/merge_expressions.rs | 202 ++++++++++++++++++ acvm-repo/acvm/src/compiler/optimizers/mod.rs | 2 + .../acvm/src/compiler/transformers/mod.rs | 16 +- 4 files changed, 230 insertions(+), 2 deletions(-) create mode 100644 acvm-repo/acvm/src/compiler/optimizers/merge_expressions.rs diff --git a/acvm-repo/acir/src/circuit/opcodes/black_box_function_call.rs b/acvm-repo/acir/src/circuit/opcodes/black_box_function_call.rs index e06286d179e..7e76530b3ca 100644 --- a/acvm-repo/acir/src/circuit/opcodes/black_box_function_call.rs +++ b/acvm-repo/acir/src/circuit/opcodes/black_box_function_call.rs @@ -1,3 +1,5 @@ +use std::collections::HashSet; + use crate::native_types::Witness; use crate::{AcirField, BlackBoxFunc}; @@ -389,6 +391,16 @@ impl BlackBoxFuncCall { BlackBoxFuncCall::BigIntToLeBytes { outputs, .. } => outputs.to_vec(), } } + + pub fn get_input_witnesses(&self) -> HashSet { + let mut result = HashSet::new(); + for input in self.get_inputs_vec() { + if let ConstantOrWitnessEnum::Witness(w) = input.input() { + result.insert(w); + } + } + result + } } const ABBREVIATION_LIMIT: usize = 5; diff --git a/acvm-repo/acvm/src/compiler/optimizers/merge_expressions.rs b/acvm-repo/acvm/src/compiler/optimizers/merge_expressions.rs new file mode 100644 index 00000000000..4291b997fde --- /dev/null +++ b/acvm-repo/acvm/src/compiler/optimizers/merge_expressions.rs @@ -0,0 +1,202 @@ +use std::collections::{HashMap, HashSet}; + +use acir::{ + circuit::{brillig::BrilligInputs, directives::Directive, opcodes::BlockId, Circuit, Opcode}, + native_types::{Expression, Witness}, + AcirField, +}; + +pub(crate) struct MergeExpressionsOptimizer { + resolved_blocks: HashMap>, +} + +impl MergeExpressionsOptimizer { + pub(crate) fn new() -> Self { + MergeExpressionsOptimizer { resolved_blocks: HashMap::new() } + } + /// This pass analyzes the circuit and identifies intermediate variables that are + /// only used in two gates. It then merges the gate that produces the + /// intermediate variable into the second one that uses it + /// Note: This pass is only relevant for backends that can handle unlimited width + pub(crate) fn eliminate_intermediate_variable( + &mut self, + circuit: &Circuit, + acir_opcode_positions: Vec, + ) -> (Vec>, Vec) { + // Keep track, for each witness, of the gates that use it + let circuit_inputs = circuit.circuit_arguments(); + self.resolved_blocks = HashMap::new(); + let mut used_witness: HashMap> = HashMap::new(); + for (i, opcode) in circuit.opcodes.iter().enumerate() { + let witnesses = self.witness_inputs(opcode); + if let Opcode::MemoryInit { block_id, .. } = opcode { + self.resolved_blocks.insert(*block_id, witnesses.clone()); + } + for w in witnesses { + // We do not simplify circuit inputs + if !circuit_inputs.contains(&w) { + used_witness.entry(w).or_default().insert(i); + } + } + } + + let mut modified_gates: HashMap> = HashMap::new(); + let mut new_circuit = Vec::new(); + let mut new_acir_opcode_positions = Vec::new(); + // For each opcode, try to get a target opcode to merge with + for (i, opcode) in circuit.opcodes.iter().enumerate() { + if !matches!(opcode, Opcode::AssertZero(_)) { + new_circuit.push(opcode.clone()); + new_acir_opcode_positions.push(acir_opcode_positions[i]); + continue; + } + let opcode = modified_gates.get(&i).unwrap_or(opcode).clone(); + let mut to_keep = true; + let input_witnesses = self.witness_inputs(&opcode); + for w in input_witnesses.clone() { + let empty_gates = HashSet::new(); + let gates_using_w = used_witness.get(&w).unwrap_or(&empty_gates); + // We only consider witness which are used in exactly two arithmetic gates + if gates_using_w.len() == 2 { + let gates_using_w: Vec<_> = gates_using_w.iter().collect(); + let mut b = *gates_using_w[1]; + if b == i { + b = *gates_using_w[0]; + } else { + // sanity check + assert!(i == *gates_using_w[0]); + } + let second_gate = modified_gates.get(&b).unwrap_or(&circuit.opcodes[b]).clone(); + if let (Opcode::AssertZero(expr_define), Opcode::AssertZero(expr_use)) = + (opcode.clone(), second_gate) + { + if let Some(expr) = Self::merge(&expr_use, &expr_define, w) { + // sanity check + assert!(i < b); + modified_gates.insert(b, Opcode::AssertZero(expr)); + to_keep = false; + // Update the 'used_witness' map to account for the merge. + for w2 in Self::expr_wit(&expr_define) { + if !circuit_inputs.contains(&w2) { + let mut v = used_witness[&w2].clone(); + v.insert(b); + v.remove(&i); + used_witness.insert(w2, v); + } + } + // We need to stop here and continue with the next opcode + // because the merge invalidate the current opcode + break; + } + } + } + } + + if to_keep { + if modified_gates.contains_key(&i) { + new_circuit.push(modified_gates[&i].clone()); + } else { + new_circuit.push(opcode.clone()); + } + new_acir_opcode_positions.push(acir_opcode_positions[i]); + } + } + (new_circuit, new_acir_opcode_positions) + } + + fn expr_wit(expr: &Expression) -> HashSet { + let mut result = HashSet::new(); + result.extend(expr.mul_terms.iter().flat_map(|i| vec![i.1, i.2])); + result.extend(expr.linear_combinations.iter().map(|i| i.1)); + result + } + + fn brillig_input_wit(&self, input: &BrilligInputs) -> HashSet { + let mut result = HashSet::new(); + match input { + BrilligInputs::Single(expr) => { + result.extend(Self::expr_wit(expr)); + } + BrilligInputs::Array(exprs) => { + for expr in exprs { + result.extend(Self::expr_wit(expr)); + } + } + BrilligInputs::MemoryArray(block_id) => { + let witnesses = self.resolved_blocks.get(block_id).expect("Unknown block id"); + result.extend(witnesses); + } + } + result + } + + // Returns the input witnesses used by the opcode + fn witness_inputs(&self, opcode: &Opcode) -> HashSet { + let mut witnesses = HashSet::new(); + match opcode { + Opcode::AssertZero(expr) => Self::expr_wit(expr), + Opcode::BlackBoxFuncCall(bb_func) => bb_func.get_input_witnesses(), + Opcode::Directive(Directive::ToLeRadix { a, .. }) => Self::expr_wit(a), + Opcode::MemoryOp { block_id: _, op, predicate } => { + //index et value, et predicate + let mut witnesses = HashSet::new(); + witnesses.extend(Self::expr_wit(&op.index)); + witnesses.extend(Self::expr_wit(&op.value)); + if let Some(p) = predicate { + witnesses.extend(Self::expr_wit(p)); + } + witnesses + } + + Opcode::MemoryInit { block_id: _, init, block_type: _ } => { + init.iter().cloned().collect() + } + Opcode::BrilligCall { inputs, .. } => { + for i in inputs { + witnesses.extend(self.brillig_input_wit(i)); + } + witnesses + } + Opcode::Call { id: _, inputs, outputs: _, predicate } => { + for i in inputs { + witnesses.insert(*i); + } + if let Some(p) = predicate { + witnesses.extend(Self::expr_wit(p)); + } + witnesses + } + } + } + + // Merge 'expr' into 'target' via Gaussian elimination on 'w' + // Returns None if the expressions cannot be merged + fn merge( + target: &Expression, + expr: &Expression, + w: Witness, + ) -> Option> { + // Check that the witness is not part of multiplication terms + for m in &target.mul_terms { + if m.1 == w || m.2 == w { + return None; + } + } + for m in &expr.mul_terms { + if m.1 == w || m.2 == w { + return None; + } + } + + for k in &target.linear_combinations { + if k.1 == w { + for i in &expr.linear_combinations { + if i.1 == w { + return Some(target.add_mul(-(k.0 / i.0), expr)); + } + } + } + } + None + } +} diff --git a/acvm-repo/acvm/src/compiler/optimizers/mod.rs b/acvm-repo/acvm/src/compiler/optimizers/mod.rs index e20ad97a108..1947a80dc35 100644 --- a/acvm-repo/acvm/src/compiler/optimizers/mod.rs +++ b/acvm-repo/acvm/src/compiler/optimizers/mod.rs @@ -5,10 +5,12 @@ use acir::{ // mod constant_backpropagation; mod general; +mod merge_expressions; mod redundant_range; mod unused_memory; pub(crate) use general::GeneralOptimizer; +pub(crate) use merge_expressions::MergeExpressionsOptimizer; pub(crate) use redundant_range::RangeOptimizer; use tracing::info; diff --git a/acvm-repo/acvm/src/compiler/transformers/mod.rs b/acvm-repo/acvm/src/compiler/transformers/mod.rs index 4e29681cbed..b11d054a57b 100644 --- a/acvm-repo/acvm/src/compiler/transformers/mod.rs +++ b/acvm-repo/acvm/src/compiler/transformers/mod.rs @@ -10,7 +10,9 @@ mod csat; pub(crate) use csat::CSatTransformer; pub use csat::MIN_EXPRESSION_WIDTH; -use super::{transform_assert_messages, AcirTransformationMap}; +use super::{ + optimizers::MergeExpressionsOptimizer, transform_assert_messages, AcirTransformationMap, +}; /// Applies [`ProofSystemCompiler`][crate::ProofSystemCompiler] specific optimizations to a [`Circuit`]. pub fn transform( @@ -166,6 +168,16 @@ pub(super) fn transform_internal( // The transformer does not add new public inputs ..acir }; - + let mut merge_optimizer = MergeExpressionsOptimizer::new(); + let (opcodes, new_acir_opcode_positions) = + merge_optimizer.eliminate_intermediate_variable(&acir, new_acir_opcode_positions); + // n.b. we do not update current_witness_index after the eliminate_intermediate_variable pass, the real index could be less. + let acir = Circuit { + current_witness_index, + expression_width, + opcodes, + // The optimizer does not add new public inputs + ..acir + }; (acir, new_acir_opcode_positions) } From 600ffeb05cd2538604570f3e2aa1b2e560e577c3 Mon Sep 17 00:00:00 2001 From: Tom French Date: Thu, 24 Oct 2024 22:47:40 +0100 Subject: [PATCH 8/8] chore: add some tests for type aliases --- compiler/noirc_frontend/src/tests.rs | 1 + compiler/noirc_frontend/src/tests/aliases.rs | 52 ++++++++++++++++++++ 2 files changed, 53 insertions(+) create mode 100644 compiler/noirc_frontend/src/tests/aliases.rs diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index 286986e5d61..e06881127fd 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -1,5 +1,6 @@ #![cfg(test)] +mod aliases; mod bound_checks; mod imports; mod metaprogramming; diff --git a/compiler/noirc_frontend/src/tests/aliases.rs b/compiler/noirc_frontend/src/tests/aliases.rs new file mode 100644 index 00000000000..2ca460ca635 --- /dev/null +++ b/compiler/noirc_frontend/src/tests/aliases.rs @@ -0,0 +1,52 @@ +use super::assert_no_errors; + +#[test] +fn allows_usage_of_type_alias_as_argument_type() { + let src = r#" + type Foo = Field; + + fn accepts_a_foo(x: Foo) { + assert_eq(x, 42); + } + + fn main() { + accepts_a_foo(42); + } + "#; + assert_no_errors(src); +} + +#[test] +fn allows_usage_of_type_alias_as_return_type() { + let src = r#" + type Foo = Field; + + fn returns_a_foo() -> Foo { + 42 + } + + fn main() { + let _ = returns_a_foo(); + } + "#; + assert_no_errors(src); +} + +// This is a regression test for https://github.com/noir-lang/noir/issues/6347 +#[test] +#[should_panic = r#"ResolverError(Expected { span: Span(Span { start: ByteIndex(95), end: ByteIndex(98) }), expected: "type", got: "type alias" }"#] +fn allows_destructuring_a_type_alias_of_a_struct() { + let src = r#" + struct Foo { + inner: Field + } + + type Bar = Foo; + + fn main() { + let Bar { inner } = Foo { inner: 42 }; + assert_eq(inner, 42); + } + "#; + assert_no_errors(src); +}