From b70daf423890aa0f885ccf32531fa1583770c23c Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 6 Dec 2024 12:04:10 -0300 Subject: [PATCH 1/6] fix!: several format string fixes and improvements (#6703) Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com> --- Cargo.lock | 2 - .../noirc_evaluator/src/ssa/ssa_gen/mod.rs | 21 +- compiler/noirc_frontend/Cargo.toml | 1 - compiler/noirc_frontend/src/ast/expression.rs | 16 +- compiler/noirc_frontend/src/ast/visitor.rs | 6 +- .../src/elaborator/expressions.rs | 82 ++-- .../src/hir/comptime/display.rs | 2 +- .../noirc_frontend/src/hir/comptime/errors.rs | 12 +- .../src/hir/comptime/hir_to_display_ast.rs | 4 +- .../src/hir/comptime/interpreter.rs | 33 +- .../src/hir/resolution/errors.rs | 7 - compiler/noirc_frontend/src/hir_def/expr.rs | 4 +- compiler/noirc_frontend/src/lexer/errors.rs | 32 ++ compiler/noirc_frontend/src/lexer/lexer.rs | 359 ++++++++++++++++-- compiler/noirc_frontend/src/lexer/token.rs | 39 +- .../src/monomorphization/ast.rs | 4 +- .../src/monomorphization/mod.rs | 7 +- .../src/monomorphization/printer.rs | 6 +- compiler/noirc_frontend/src/parser/parser.rs | 6 +- .../src/parser/parser/expression.rs | 15 +- compiler/noirc_frontend/src/tests.rs | 14 +- compiler/noirc_printable_type/Cargo.toml | 1 - compiler/noirc_printable_type/src/lib.rs | 109 ++++-- noir_stdlib/src/collections/map.nr | 13 +- noir_stdlib/src/collections/umap.nr | 13 +- .../comptime_fmt_strings/src/main.nr | 2 +- .../execution_success/hashmap/src/main.nr | 5 +- .../execution_success/uhashmap/src/main.nr | 3 +- tooling/nargo_fmt/src/formatter/expression.rs | 7 +- 29 files changed, 633 insertions(+), 192 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 96ceb94fcdd..141a25f42a9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3206,7 +3206,6 @@ dependencies = [ "proptest", "proptest-derive 0.5.0", "rangemap", - "regex", "rustc-hash", "serde", "serde_json", @@ -3225,7 +3224,6 @@ dependencies = [ "acvm", "iter-extended", "jsonrpc", - "regex", "serde", "serde_json", "thiserror", diff --git a/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs index 2fe0a38af00..91a49018f76 100644 --- a/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs @@ -2,6 +2,7 @@ pub(crate) mod context; mod program; mod value; +use noirc_frontend::token::FmtStrFragment; pub(crate) use program::Ssa; use context::SharedContext; @@ -230,10 +231,26 @@ impl<'a> FunctionContext<'a> { Ok(self.builder.numeric_constant(*value as u128, Type::bool()).into()) } ast::Literal::Str(string) => Ok(self.codegen_string(string)), - ast::Literal::FmtStr(string, number_of_fields, fields) => { + ast::Literal::FmtStr(fragments, number_of_fields, fields) => { + let mut string = String::new(); + for fragment in fragments { + match fragment { + FmtStrFragment::String(value) => { + // Escape curly braces in non-interpolations + let value = value.replace('{', "{{").replace('}', "}}"); + string.push_str(&value); + } + FmtStrFragment::Interpolation(value, _span) => { + string.push('{'); + string.push_str(value); + string.push('}'); + } + } + } + // A caller needs multiple pieces of information to make use of a format string // The message string, the number of fields to be formatted, and the fields themselves - let string = self.codegen_string(string); + let string = self.codegen_string(&string); let field_count = self.builder.length_constant(*number_of_fields as u128); let fields = self.codegen_expression(fields)?; diff --git a/compiler/noirc_frontend/Cargo.toml b/compiler/noirc_frontend/Cargo.toml index 5d1520af54f..5f8f02689c8 100644 --- a/compiler/noirc_frontend/Cargo.toml +++ b/compiler/noirc_frontend/Cargo.toml @@ -25,7 +25,6 @@ num-bigint.workspace = true num-traits.workspace = true rustc-hash = "1.1.0" small-ord-set = "0.1.3" -regex = "1.9.1" cfg-if.workspace = true tracing.workspace = true petgraph = "0.6" diff --git a/compiler/noirc_frontend/src/ast/expression.rs b/compiler/noirc_frontend/src/ast/expression.rs index 2c8a9b6508d..ae622f46686 100644 --- a/compiler/noirc_frontend/src/ast/expression.rs +++ b/compiler/noirc_frontend/src/ast/expression.rs @@ -10,7 +10,7 @@ use crate::ast::{ use crate::node_interner::{ ExprId, InternedExpressionKind, InternedStatementKind, QuotedTypeId, StructId, }; -use crate::token::{Attributes, FunctionAttribute, Token, Tokens}; +use crate::token::{Attributes, FmtStrFragment, FunctionAttribute, Token, Tokens}; use crate::{Kind, Type}; use acvm::{acir::AcirField, FieldElement}; use iter_extended::vecmap; @@ -210,8 +210,8 @@ impl ExpressionKind { ExpressionKind::Literal(Literal::RawStr(contents, hashes)) } - pub fn format_string(contents: String) -> ExpressionKind { - ExpressionKind::Literal(Literal::FmtStr(contents)) + pub fn format_string(fragments: Vec, length: u32) -> ExpressionKind { + ExpressionKind::Literal(Literal::FmtStr(fragments, length)) } pub fn constructor( @@ -434,7 +434,7 @@ pub enum Literal { Integer(FieldElement, /*sign*/ bool), // false for positive integer and true for negative Str(String), RawStr(String, u8), - FmtStr(String), + FmtStr(Vec, u32 /* length */), Unit, } @@ -669,7 +669,13 @@ impl Display for Literal { std::iter::once('#').cycle().take(*num_hashes as usize).collect(); write!(f, "r{hashes}\"{string}\"{hashes}") } - Literal::FmtStr(string) => write!(f, "f\"{string}\""), + Literal::FmtStr(fragments, _length) => { + write!(f, "f\"")?; + for fragment in fragments { + fragment.fmt(f)?; + } + write!(f, "\"") + } Literal::Unit => write!(f, "()"), } } diff --git a/compiler/noirc_frontend/src/ast/visitor.rs b/compiler/noirc_frontend/src/ast/visitor.rs index f149c998eca..2f60532980a 100644 --- a/compiler/noirc_frontend/src/ast/visitor.rs +++ b/compiler/noirc_frontend/src/ast/visitor.rs @@ -16,7 +16,7 @@ use crate::{ InternedUnresolvedTypeData, QuotedTypeId, }, parser::{Item, ItemKind, ParsedSubModule}, - token::{MetaAttribute, SecondaryAttribute, Tokens}, + token::{FmtStrFragment, MetaAttribute, SecondaryAttribute, Tokens}, ParsedModule, QuotedType, }; @@ -172,7 +172,7 @@ pub trait Visitor { fn visit_literal_raw_str(&mut self, _: &str, _: u8) {} - fn visit_literal_fmt_str(&mut self, _: &str) {} + fn visit_literal_fmt_str(&mut self, _: &[FmtStrFragment], _length: u32) {} fn visit_literal_unit(&mut self) {} @@ -900,7 +900,7 @@ impl Literal { Literal::Integer(value, negative) => visitor.visit_literal_integer(*value, *negative), Literal::Str(str) => visitor.visit_literal_str(str), Literal::RawStr(str, length) => visitor.visit_literal_raw_str(str, *length), - Literal::FmtStr(str) => visitor.visit_literal_fmt_str(str), + Literal::FmtStr(fragments, length) => visitor.visit_literal_fmt_str(fragments, *length), Literal::Unit => visitor.visit_literal_unit(), } } diff --git a/compiler/noirc_frontend/src/elaborator/expressions.rs b/compiler/noirc_frontend/src/elaborator/expressions.rs index f801c1817ef..b5fab6faf9b 100644 --- a/compiler/noirc_frontend/src/elaborator/expressions.rs +++ b/compiler/noirc_frontend/src/elaborator/expressions.rs @@ -1,7 +1,6 @@ use acvm::{AcirField, FieldElement}; use iter_extended::vecmap; use noirc_errors::{Location, Span}; -use regex::Regex; use rustc_hash::FxHashSet as HashSet; use crate::{ @@ -29,7 +28,7 @@ use crate::{ traits::{ResolvedTraitBound, TraitConstraint}, }, node_interner::{DefinitionKind, ExprId, FuncId, InternedStatementKind, TraitMethodId}, - token::Tokens, + token::{FmtStrFragment, Tokens}, Kind, QuotedType, Shared, StructType, Type, }; @@ -167,7 +166,7 @@ impl<'context> Elaborator<'context> { let len = Type::Constant(str.len().into(), Kind::u32()); (Lit(HirLiteral::Str(str)), Type::String(Box::new(len))) } - Literal::FmtStr(str) => self.elaborate_fmt_string(str, span), + Literal::FmtStr(fragments, length) => self.elaborate_fmt_string(fragments, length), Literal::Array(array_literal) => { self.elaborate_array_literal(array_literal, span, true) } @@ -234,53 +233,50 @@ impl<'context> Elaborator<'context> { (HirExpression::Literal(constructor(expr)), typ) } - fn elaborate_fmt_string(&mut self, str: String, call_expr_span: Span) -> (HirExpression, Type) { - let re = Regex::new(r"\{([a-zA-Z0-9_]+)\}") - .expect("ICE: an invalid regex pattern was used for checking format strings"); - + fn elaborate_fmt_string( + &mut self, + fragments: Vec, + length: u32, + ) -> (HirExpression, Type) { let mut fmt_str_idents = Vec::new(); let mut capture_types = Vec::new(); - for field in re.find_iter(&str) { - let matched_str = field.as_str(); - let ident_name = &matched_str[1..(matched_str.len() - 1)]; - - let scope_tree = self.scopes.current_scope_tree(); - let variable = scope_tree.find(ident_name); - - let hir_ident = if let Some((old_value, _)) = variable { - old_value.num_times_used += 1; - old_value.ident.clone() - } else if let Ok((definition_id, _)) = - self.lookup_global(Path::from_single(ident_name.to_string(), call_expr_span)) - { - HirIdent::non_trait_method(definition_id, Location::new(call_expr_span, self.file)) - } else if ident_name.parse::().is_ok() { - self.push_err(ResolverError::NumericConstantInFormatString { - name: ident_name.to_owned(), - span: call_expr_span, - }); - continue; - } else { - self.push_err(ResolverError::VariableNotDeclared { - name: ident_name.to_owned(), - span: call_expr_span, - }); - continue; - }; + for fragment in &fragments { + if let FmtStrFragment::Interpolation(ident_name, string_span) = fragment { + let scope_tree = self.scopes.current_scope_tree(); + let variable = scope_tree.find(ident_name); + + let hir_ident = if let Some((old_value, _)) = variable { + old_value.num_times_used += 1; + old_value.ident.clone() + } else if let Ok((definition_id, _)) = + self.lookup_global(Path::from_single(ident_name.to_string(), *string_span)) + { + HirIdent::non_trait_method( + definition_id, + Location::new(*string_span, self.file), + ) + } else { + self.push_err(ResolverError::VariableNotDeclared { + name: ident_name.to_owned(), + span: *string_span, + }); + continue; + }; - let hir_expr = HirExpression::Ident(hir_ident.clone(), None); - let expr_id = self.interner.push_expr(hir_expr); - self.interner.push_expr_location(expr_id, call_expr_span, self.file); - let typ = self.type_check_variable(hir_ident, expr_id, None); - self.interner.push_expr_type(expr_id, typ.clone()); - capture_types.push(typ); - fmt_str_idents.push(expr_id); + let hir_expr = HirExpression::Ident(hir_ident.clone(), None); + let expr_id = self.interner.push_expr(hir_expr); + self.interner.push_expr_location(expr_id, *string_span, self.file); + let typ = self.type_check_variable(hir_ident, expr_id, None); + self.interner.push_expr_type(expr_id, typ.clone()); + capture_types.push(typ); + fmt_str_idents.push(expr_id); + } } - let len = Type::Constant(str.len().into(), Kind::u32()); + let len = Type::Constant(length.into(), Kind::u32()); let typ = Type::FmtString(Box::new(len), Box::new(Type::Tuple(capture_types))); - (HirExpression::Literal(HirLiteral::FmtStr(str, fmt_str_idents)), typ) + (HirExpression::Literal(HirLiteral::FmtStr(fragments, fmt_str_idents, length)), typ) } fn elaborate_prefix(&mut self, prefix: PrefixExpression, span: Span) -> (ExprId, Type) { diff --git a/compiler/noirc_frontend/src/hir/comptime/display.rs b/compiler/noirc_frontend/src/hir/comptime/display.rs index 560d11cfa2e..29d1448f07e 100644 --- a/compiler/noirc_frontend/src/hir/comptime/display.rs +++ b/compiler/noirc_frontend/src/hir/comptime/display.rs @@ -661,7 +661,7 @@ fn remove_interned_in_literal(interner: &NodeInterner, literal: Literal) -> Lite | Literal::Integer(_, _) | Literal::Str(_) | Literal::RawStr(_, _) - | Literal::FmtStr(_) + | Literal::FmtStr(_, _) | Literal::Unit => literal, } } diff --git a/compiler/noirc_frontend/src/hir/comptime/errors.rs b/compiler/noirc_frontend/src/hir/comptime/errors.rs index 446c4dae2d3..3df20b39209 100644 --- a/compiler/noirc_frontend/src/hir/comptime/errors.rs +++ b/compiler/noirc_frontend/src/hir/comptime/errors.rs @@ -240,6 +240,9 @@ pub enum InterpreterError { err: Box, location: Location, }, + CannotInterpretFormatStringWithErrors { + location: Location, + }, // These cases are not errors, they are just used to prevent us from running more code // until the loop can be resumed properly. These cases will never be displayed to users. @@ -315,7 +318,8 @@ impl InterpreterError { | InterpreterError::TypeAnnotationsNeededForMethodCall { location } | InterpreterError::CannotResolveExpression { location, .. } | InterpreterError::CannotSetFunctionBody { location, .. } - | InterpreterError::UnknownArrayLength { location, .. } => *location, + | InterpreterError::UnknownArrayLength { location, .. } + | InterpreterError::CannotInterpretFormatStringWithErrors { location } => *location, InterpreterError::FailedToParseMacro { error, file, .. } => { Location::new(error.span(), *file) @@ -664,6 +668,12 @@ impl<'a> From<&'a InterpreterError> for CustomDiagnostic { let secondary = format!("Evaluating the length failed with: `{err}`"); CustomDiagnostic::simple_error(msg, secondary, location.span) } + InterpreterError::CannotInterpretFormatStringWithErrors { location } => { + let msg = "Cannot interpret format string with errors".to_string(); + let secondary = + "Some of the variables to interpolate could not be evaluated".to_string(); + CustomDiagnostic::simple_error(msg, secondary, location.span) + } } } } diff --git a/compiler/noirc_frontend/src/hir/comptime/hir_to_display_ast.rs b/compiler/noirc_frontend/src/hir/comptime/hir_to_display_ast.rs index 5540a199cec..9338c0fc37f 100644 --- a/compiler/noirc_frontend/src/hir/comptime/hir_to_display_ast.rs +++ b/compiler/noirc_frontend/src/hir/comptime/hir_to_display_ast.rs @@ -121,9 +121,9 @@ impl HirExpression { HirExpression::Literal(HirLiteral::Str(string)) => { ExpressionKind::Literal(Literal::Str(string.clone())) } - HirExpression::Literal(HirLiteral::FmtStr(string, _exprs)) => { + HirExpression::Literal(HirLiteral::FmtStr(fragments, _exprs, length)) => { // TODO: Is throwing away the exprs here valid? - ExpressionKind::Literal(Literal::FmtStr(string.clone())) + ExpressionKind::Literal(Literal::FmtStr(fragments.clone(), *length)) } HirExpression::Literal(HirLiteral::Unit) => ExpressionKind::Literal(Literal::Unit), HirExpression::Block(expr) => ExpressionKind::Block(expr.to_display_ast(interner)), diff --git a/compiler/noirc_frontend/src/hir/comptime/interpreter.rs b/compiler/noirc_frontend/src/hir/comptime/interpreter.rs index 49fd86b73bb..dfa55a9d79b 100644 --- a/compiler/noirc_frontend/src/hir/comptime/interpreter.rs +++ b/compiler/noirc_frontend/src/hir/comptime/interpreter.rs @@ -20,7 +20,7 @@ use crate::monomorphization::{ perform_impl_bindings, perform_instantiation_bindings, resolve_trait_method, undo_instantiation_bindings, }; -use crate::token::Tokens; +use crate::token::{FmtStrFragment, Tokens}; use crate::TypeVariable; use crate::{ hir_def::{ @@ -623,8 +623,8 @@ impl<'local, 'interner> Interpreter<'local, 'interner> { self.evaluate_integer(value, is_negative, id) } HirLiteral::Str(string) => Ok(Value::String(Rc::new(string))), - HirLiteral::FmtStr(string, captures) => { - self.evaluate_format_string(string, captures, id) + HirLiteral::FmtStr(fragments, captures, _length) => { + self.evaluate_format_string(fragments, captures, id) } HirLiteral::Array(array) => self.evaluate_array(array, id), HirLiteral::Slice(array) => self.evaluate_slice(array, id), @@ -633,7 +633,7 @@ impl<'local, 'interner> Interpreter<'local, 'interner> { fn evaluate_format_string( &mut self, - string: String, + fragments: Vec, captures: Vec, id: ExprId, ) -> IResult { @@ -644,13 +644,12 @@ impl<'local, 'interner> Interpreter<'local, 'interner> { let mut values: VecDeque<_> = captures.into_iter().map(|capture| self.evaluate(capture)).collect::>()?; - for character in string.chars() { - match character { - '\\' => escaped = true, - '{' if !escaped => consuming = true, - '}' if !escaped && consuming => { - consuming = false; - + for fragment in fragments { + match fragment { + FmtStrFragment::String(string) => { + result.push_str(&string); + } + FmtStrFragment::Interpolation(_, span) => { if let Some(value) = values.pop_front() { // When interpolating a quoted value inside a format string, we don't include the // surrounding `quote {` ... `}` as if we are unquoting the quoted value inside the string. @@ -665,13 +664,15 @@ impl<'local, 'interner> Interpreter<'local, 'interner> { } else { result.push_str(&value.display(self.elaborator.interner).to_string()); } + } else { + // If we can't find a value for this fragment it means the interpolated value was not + // found or it errored. In this case we error here as well. + let location = self.elaborator.interner.expr_location(&id); + return Err(InterpreterError::CannotInterpretFormatStringWithErrors { + location, + }); } } - other if !consuming => { - escaped = false; - result.push(other); - } - _ => (), } } diff --git a/compiler/noirc_frontend/src/hir/resolution/errors.rs b/compiler/noirc_frontend/src/hir/resolution/errors.rs index 5c8e0a1b53e..774836f8992 100644 --- a/compiler/noirc_frontend/src/hir/resolution/errors.rs +++ b/compiler/noirc_frontend/src/hir/resolution/errors.rs @@ -77,8 +77,6 @@ pub enum ResolverError { MutableReferenceToImmutableVariable { variable: String, span: Span }, #[error("Mutable references to array indices are unsupported")] MutableReferenceToArrayElement { span: Span }, - #[error("Numeric constants should be printed without formatting braces")] - NumericConstantInFormatString { name: String, span: Span }, #[error("Closure environment must be a tuple or unit type")] InvalidClosureEnvironment { typ: Type, span: Span }, #[error("Nested slices, i.e. slices within an array or slice, are not supported")] @@ -378,11 +376,6 @@ impl<'a> From<&'a ResolverError> for Diagnostic { ResolverError::MutableReferenceToArrayElement { span } => { Diagnostic::simple_error("Mutable references to array elements are currently unsupported".into(), "Try storing the element in a fresh variable first".into(), *span) }, - ResolverError::NumericConstantInFormatString { name, span } => Diagnostic::simple_error( - format!("cannot find `{name}` in this scope "), - "Numeric constants should be printed without formatting braces".to_string(), - *span, - ), ResolverError::InvalidClosureEnvironment { span, typ } => Diagnostic::simple_error( format!("{typ} is not a valid closure environment type"), "Closure environment must be a tuple or unit type".to_string(), *span), diff --git a/compiler/noirc_frontend/src/hir_def/expr.rs b/compiler/noirc_frontend/src/hir_def/expr.rs index 5d3fe632a74..e243fc88cff 100644 --- a/compiler/noirc_frontend/src/hir_def/expr.rs +++ b/compiler/noirc_frontend/src/hir_def/expr.rs @@ -7,7 +7,7 @@ use crate::hir::type_check::generics::TraitGenerics; use crate::node_interner::{ DefinitionId, DefinitionKind, ExprId, FuncId, NodeInterner, StmtId, TraitMethodId, }; -use crate::token::Tokens; +use crate::token::{FmtStrFragment, Tokens}; use crate::Shared; use super::stmt::HirPattern; @@ -114,7 +114,7 @@ pub enum HirLiteral { Bool(bool), Integer(FieldElement, bool), //true for negative integer and false for positive Str(String), - FmtStr(String, Vec), + FmtStr(Vec, Vec, u32 /* length */), Unit, } diff --git a/compiler/noirc_frontend/src/lexer/errors.rs b/compiler/noirc_frontend/src/lexer/errors.rs index 8d799ef35d1..f95ccba061a 100644 --- a/compiler/noirc_frontend/src/lexer/errors.rs +++ b/compiler/noirc_frontend/src/lexer/errors.rs @@ -30,6 +30,10 @@ pub enum LexerErrorKind { UnterminatedBlockComment { span: Span }, #[error("Unterminated string literal")] UnterminatedStringLiteral { span: Span }, + #[error("Invalid format string: expected '}}', found {found:?}")] + InvalidFormatString { found: char, span: Span }, + #[error("Invalid format string: expected letter or underscore, found '}}'")] + EmptyFormatStringInterpolation { span: Span }, #[error( "'\\{escaped}' is not a valid escape sequence. Use '\\' for a literal backslash character." )] @@ -68,6 +72,8 @@ impl LexerErrorKind { LexerErrorKind::LogicalAnd { span } => *span, LexerErrorKind::UnterminatedBlockComment { span } => *span, LexerErrorKind::UnterminatedStringLiteral { span } => *span, + LexerErrorKind::InvalidFormatString { span, .. } => *span, + LexerErrorKind::EmptyFormatStringInterpolation { span, .. } => *span, LexerErrorKind::InvalidEscape { span, .. } => *span, LexerErrorKind::InvalidQuoteDelimiter { delimiter } => delimiter.to_span(), LexerErrorKind::NonAsciiComment { span, .. } => *span, @@ -130,6 +136,32 @@ impl LexerErrorKind { LexerErrorKind::UnterminatedBlockComment { span } => ("Unterminated block comment".to_string(), "Unterminated block comment".to_string(), *span), LexerErrorKind::UnterminatedStringLiteral { span } => ("Unterminated string literal".to_string(), "Unterminated string literal".to_string(), *span), + LexerErrorKind::InvalidFormatString { found, span } => { + if found == &'}' { + ( + "Invalid format string: unmatched '}}' found".to_string(), + "If you intended to print '}', you can escape it using '}}'".to_string(), + *span, + ) + } else { + ( + format!("Invalid format string: expected '}}', found {found:?}"), + if found == &'.' { + "Field access isn't supported in format strings".to_string() + } else { + "If you intended to print '{', you can escape it using '{{'".to_string() + }, + *span, + ) + } + } + LexerErrorKind::EmptyFormatStringInterpolation { span } => { + ( + "Invalid format string: expected letter or underscore, found '}}'".to_string(), + "If you intended to print '{' or '}', you can escape them using '{{' and '}}' respectively".to_string(), + *span, + ) + } LexerErrorKind::InvalidEscape { escaped, span } => (format!("'\\{escaped}' is not a valid escape sequence. Use '\\' for a literal backslash character."), "Invalid escape sequence".to_string(), *span), LexerErrorKind::InvalidQuoteDelimiter { delimiter } => { diff --git a/compiler/noirc_frontend/src/lexer/lexer.rs b/compiler/noirc_frontend/src/lexer/lexer.rs index 68dc142ff10..a5c4b2cd772 100644 --- a/compiler/noirc_frontend/src/lexer/lexer.rs +++ b/compiler/noirc_frontend/src/lexer/lexer.rs @@ -2,7 +2,7 @@ use crate::token::DocStyle; use super::{ errors::LexerErrorKind, - token::{IntType, Keyword, SpannedToken, Token, Tokens}, + token::{FmtStrFragment, IntType, Keyword, SpannedToken, Token, Tokens}, }; use acvm::{AcirField, FieldElement}; use noirc_errors::{Position, Span}; @@ -411,51 +411,190 @@ impl<'a> Lexer<'a> { let start = self.position; let mut string = String::new(); - while let Some(next) = self.next_char() { - let char = match next { - '"' => break, - '\\' => match self.next_char() { - Some('r') => '\r', - Some('n') => '\n', - Some('t') => '\t', - Some('0') => '\0', - Some('"') => '"', - Some('\\') => '\\', - Some(escaped) => { - let span = Span::inclusive(start, self.position); - return Err(LexerErrorKind::InvalidEscape { escaped, span }); - } - None => { - let span = Span::inclusive(start, self.position); - return Err(LexerErrorKind::UnterminatedStringLiteral { span }); - } - }, - other => other, - }; + loop { + if let Some(next) = self.next_char() { + let char = match next { + '"' => break, + '\\' => match self.next_char() { + Some('r') => '\r', + Some('n') => '\n', + Some('t') => '\t', + Some('0') => '\0', + Some('"') => '"', + Some('\\') => '\\', + Some(escaped) => { + let span = Span::inclusive(start, self.position); + return Err(LexerErrorKind::InvalidEscape { escaped, span }); + } + None => { + let span = Span::inclusive(start, self.position); + return Err(LexerErrorKind::UnterminatedStringLiteral { span }); + } + }, + other => other, + }; - string.push(char); + string.push(char); + } else { + let span = Span::inclusive(start, self.position); + return Err(LexerErrorKind::UnterminatedStringLiteral { span }); + } } let str_literal_token = Token::Str(string); - let end = self.position; Ok(str_literal_token.into_span(start, end)) } - // This differs from `eat_string_literal` in that we want the leading `f` to be captured in the Span fn eat_fmt_string(&mut self) -> SpannedTokenResult { let start = self.position; - self.next_char(); - let str_literal = self.eat_while(None, |ch| ch != '"'); + let mut fragments = Vec::new(); + let mut length = 0; + + loop { + // String fragment until '{' or '"' + let mut string = String::new(); + let mut found_curly = false; + + loop { + if let Some(next) = self.next_char() { + let char = match next { + '"' => break, + '\\' => match self.next_char() { + Some('r') => '\r', + Some('n') => '\n', + Some('t') => '\t', + Some('0') => '\0', + Some('"') => '"', + Some('\\') => '\\', + Some(escaped) => { + let span = Span::inclusive(start, self.position); + return Err(LexerErrorKind::InvalidEscape { escaped, span }); + } + None => { + let span = Span::inclusive(start, self.position); + return Err(LexerErrorKind::UnterminatedStringLiteral { span }); + } + }, + '{' if self.peek_char_is('{') => { + self.next_char(); + '{' + } + '}' if self.peek_char_is('}') => { + self.next_char(); + '}' + } + '}' => { + let error_position = self.position; + + // Keep consuming chars until we find the closing double quote + self.skip_until_string_end(); + + let span = Span::inclusive(error_position, error_position); + return Err(LexerErrorKind::InvalidFormatString { found: '}', span }); + } + '{' => { + found_curly = true; + break; + } + other => other, + }; + + string.push(char); + length += 1; + + if char == '{' || char == '}' { + // This might look a bit strange, but if there's `{{` or `}}` in the format string + // then it will be `{` and `}` in the string fragment respectively, but on the codegen + // phase it will be translated back to `{{` and `}}` to avoid executing an interpolation, + // thus the actual length of the codegen'd string will be one more than what we get here. + // + // We could just make the fragment include the double curly braces, but then the interpreter + // would need to undo the curly braces, so it's simpler to add them during codegen. + length += 1; + } + } else { + let span = Span::inclusive(start, self.position); + return Err(LexerErrorKind::UnterminatedStringLiteral { span }); + } + } + + if !string.is_empty() { + fragments.push(FmtStrFragment::String(string)); + } + + if !found_curly { + break; + } + + length += 1; // for the curly brace + + // Interpolation fragment until '}' or '"' + let mut string = String::new(); + let interpolation_start = self.position + 1; // + 1 because we are at '{' + let mut first_char = true; + while let Some(next) = self.next_char() { + let char = match next { + '}' => { + if string.is_empty() { + let error_position = self.position; + + // Keep consuming chars until we find the closing double quote + self.skip_until_string_end(); + + let span = Span::inclusive(error_position, error_position); + return Err(LexerErrorKind::EmptyFormatStringInterpolation { span }); + } + + break; + } + other => { + let is_valid_char = if first_char { + other.is_ascii_alphabetic() || other == '_' + } else { + other.is_ascii_alphanumeric() || other == '_' + }; + if !is_valid_char { + let error_position = self.position; + + // Keep consuming chars until we find the closing double quote + // (unless we bumped into a double quote now, in which case we are done) + if other != '"' { + self.skip_until_string_end(); + } - let str_literal_token = Token::FmtStr(str_literal); + let span = Span::inclusive(error_position, error_position); + return Err(LexerErrorKind::InvalidFormatString { found: other, span }); + } + first_char = false; + other + } + }; + length += 1; + string.push(char); + } + + length += 1; // for the closing curly brace - self.next_char(); // Advance past the closing quote + let interpolation_span = Span::from(interpolation_start..self.position); + fragments.push(FmtStrFragment::Interpolation(string, interpolation_span)); + } + let token = Token::FmtStr(fragments, length); let end = self.position; - Ok(str_literal_token.into_span(start, end)) + Ok(token.into_span(start, end)) + } + + fn skip_until_string_end(&mut self) { + while let Some(next) = self.next_char() { + if next == '\'' && self.peek_char_is('"') { + self.next_char(); + } else if next == '"' { + break; + } + } } fn eat_format_string_or_alpha_numeric(&mut self) -> SpannedTokenResult { @@ -962,6 +1101,155 @@ mod tests { } } + #[test] + fn test_eat_string_literal_with_escapes() { + let input = "let _word = \"hello\\n\\t\""; + + let expected = vec![ + Token::Keyword(Keyword::Let), + Token::Ident("_word".to_string()), + Token::Assign, + Token::Str("hello\n\t".to_string()), + ]; + let mut lexer = Lexer::new(input); + + for token in expected.into_iter() { + let got = lexer.next_token().unwrap(); + assert_eq!(got, token); + } + } + + #[test] + fn test_eat_string_literal_missing_double_quote() { + let input = "\"hello"; + let mut lexer = Lexer::new(input); + assert!(matches!( + lexer.next_token(), + Err(LexerErrorKind::UnterminatedStringLiteral { .. }) + )); + } + + #[test] + fn test_eat_fmt_string_literal_without_interpolations() { + let input = "let _word = f\"hello\""; + + let expected = vec![ + Token::Keyword(Keyword::Let), + Token::Ident("_word".to_string()), + Token::Assign, + Token::FmtStr(vec![FmtStrFragment::String("hello".to_string())], 5), + ]; + let mut lexer = Lexer::new(input); + + for token in expected.into_iter() { + let got = lexer.next_token().unwrap(); + assert_eq!(got, token); + } + } + + #[test] + fn test_eat_fmt_string_literal_with_escapes_without_interpolations() { + let input = "let _word = f\"hello\\n\\t{{x}}\""; + + let expected = vec![ + Token::Keyword(Keyword::Let), + Token::Ident("_word".to_string()), + Token::Assign, + Token::FmtStr(vec![FmtStrFragment::String("hello\n\t{x}".to_string())], 12), + ]; + let mut lexer = Lexer::new(input); + + for token in expected.into_iter() { + let got = lexer.next_token().unwrap(); + assert_eq!(got, token); + } + } + + #[test] + fn test_eat_fmt_string_literal_with_interpolations() { + let input = "let _word = f\"hello {world} and {_another} {vAr_123}\""; + + let expected = vec![ + Token::Keyword(Keyword::Let), + Token::Ident("_word".to_string()), + Token::Assign, + Token::FmtStr( + vec![ + FmtStrFragment::String("hello ".to_string()), + FmtStrFragment::Interpolation("world".to_string(), Span::from(21..26)), + FmtStrFragment::String(" and ".to_string()), + FmtStrFragment::Interpolation("_another".to_string(), Span::from(33..41)), + FmtStrFragment::String(" ".to_string()), + FmtStrFragment::Interpolation("vAr_123".to_string(), Span::from(44..51)), + ], + 38, + ), + ]; + let mut lexer = Lexer::new(input); + + for token in expected.into_iter() { + let got = lexer.next_token().unwrap().into_token(); + assert_eq!(got, token); + } + } + + #[test] + fn test_eat_fmt_string_literal_missing_double_quote() { + let input = "f\"hello"; + let mut lexer = Lexer::new(input); + assert!(matches!( + lexer.next_token(), + Err(LexerErrorKind::UnterminatedStringLiteral { .. }) + )); + } + + #[test] + fn test_eat_fmt_string_literal_invalid_char_in_interpolation() { + let input = "f\"hello {foo.bar}\" true"; + let mut lexer = Lexer::new(input); + assert!(matches!(lexer.next_token(), Err(LexerErrorKind::InvalidFormatString { .. }))); + + // Make sure the lexer went past the ending double quote for better recovery + let token = lexer.next_token().unwrap().into_token(); + assert!(matches!(token, Token::Bool(true))); + } + + #[test] + fn test_eat_fmt_string_literal_double_quote_inside_interpolation() { + let input = "f\"hello {world\" true"; + let mut lexer = Lexer::new(input); + assert!(matches!(lexer.next_token(), Err(LexerErrorKind::InvalidFormatString { .. }))); + + // Make sure the lexer stopped parsing the string literal when it found \" inside the interpolation + let token = lexer.next_token().unwrap().into_token(); + assert!(matches!(token, Token::Bool(true))); + } + + #[test] + fn test_eat_fmt_string_literal_unmatched_closing_curly() { + let input = "f\"hello }\" true"; + let mut lexer = Lexer::new(input); + assert!(matches!(lexer.next_token(), Err(LexerErrorKind::InvalidFormatString { .. }))); + + // Make sure the lexer went past the ending double quote for better recovery + let token = lexer.next_token().unwrap().into_token(); + assert!(matches!(token, Token::Bool(true))); + } + + #[test] + fn test_eat_fmt_string_literal_empty_interpolation() { + let input = "f\"{}\" true"; + let mut lexer = Lexer::new(input); + assert!(matches!( + lexer.next_token(), + Err(LexerErrorKind::EmptyFormatStringInterpolation { .. }) + )); + + // Make sure the lexer went past the ending double quote for better recovery + let token = lexer.next_token().unwrap().into_token(); + assert!(matches!(token, Token::Bool(true))); + } + #[test] fn test_eat_integer_literals() { let test_cases: Vec<(&str, Token)> = vec![ @@ -1151,7 +1439,7 @@ mod tests { format!("let s = r#####\"{s}\"#####;"), ], ), - (Some(Token::FmtStr("".to_string())), vec![format!("assert(x == y, f\"{s}\");")]), + (Some(Token::FmtStr(vec![], 0)), vec![format!("assert(x == y, f\"{s}\");")]), // expected token not found // (Some(Token::LineComment("".to_string(), None)), vec![ (None, vec![format!("//{s}"), format!("// {s}")]), @@ -1196,11 +1484,16 @@ mod tests { Err(LexerErrorKind::InvalidIntegerLiteral { .. }) | Err(LexerErrorKind::UnexpectedCharacter { .. }) | Err(LexerErrorKind::NonAsciiComment { .. }) - | Err(LexerErrorKind::UnterminatedBlockComment { .. }) => { + | Err(LexerErrorKind::UnterminatedBlockComment { .. }) + | Err(LexerErrorKind::UnterminatedStringLiteral { .. }) + | Err(LexerErrorKind::InvalidFormatString { .. }) => { expected_token_found = true; } Err(err) => { - panic!("Unexpected lexer error found: {:?}", err) + panic!( + "Unexpected lexer error found {:?} for input string {:?}", + err, blns_program_str + ) } } } diff --git a/compiler/noirc_frontend/src/lexer/token.rs b/compiler/noirc_frontend/src/lexer/token.rs index 836161c7c9f..f35515045db 100644 --- a/compiler/noirc_frontend/src/lexer/token.rs +++ b/compiler/noirc_frontend/src/lexer/token.rs @@ -25,7 +25,7 @@ pub enum BorrowedToken<'input> { Str(&'input str), /// the u8 is the number of hashes, i.e. r###.. RawStr(&'input str, u8), - FmtStr(&'input str), + FmtStr(&'input [FmtStrFragment], u32 /* length */), Keyword(Keyword), IntType(IntType), AttributeStart { @@ -136,7 +136,7 @@ pub enum Token { Str(String), /// the u8 is the number of hashes, i.e. r###.. RawStr(String, u8), - FmtStr(String), + FmtStr(Vec, u32 /* length */), Keyword(Keyword), IntType(IntType), AttributeStart { @@ -255,7 +255,7 @@ pub fn token_to_borrowed_token(token: &Token) -> BorrowedToken<'_> { Token::Int(n) => BorrowedToken::Int(*n), Token::Bool(b) => BorrowedToken::Bool(*b), Token::Str(ref b) => BorrowedToken::Str(b), - Token::FmtStr(ref b) => BorrowedToken::FmtStr(b), + Token::FmtStr(ref b, length) => BorrowedToken::FmtStr(b, *length), Token::RawStr(ref b, hashes) => BorrowedToken::RawStr(b, *hashes), Token::Keyword(k) => BorrowedToken::Keyword(*k), Token::AttributeStart { is_inner, is_tag } => { @@ -312,6 +312,35 @@ pub fn token_to_borrowed_token(token: &Token) -> BorrowedToken<'_> { } } +#[derive(Clone, PartialEq, Eq, Hash, Debug, PartialOrd, Ord)] +pub enum FmtStrFragment { + String(String), + Interpolation(String, Span), +} + +impl Display for FmtStrFragment { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + FmtStrFragment::String(string) => { + // Undo the escapes when displaying the fmt string + let string = string + .replace('{', "{{") + .replace('}', "}}") + .replace('\r', "\\r") + .replace('\n', "\\n") + .replace('\t', "\\t") + .replace('\0', "\\0") + .replace('\'', "\\'") + .replace('\"', "\\\""); + write!(f, "{}", string) + } + FmtStrFragment::Interpolation(string, _span) => { + write!(f, "{{{}}}", string) + } + } + } +} + #[derive(Clone, Copy, PartialEq, Eq, Hash, Debug, PartialOrd, Ord)] pub enum DocStyle { Outer, @@ -375,7 +404,7 @@ impl fmt::Display for Token { Token::Int(n) => write!(f, "{}", n), Token::Bool(b) => write!(f, "{b}"), Token::Str(ref b) => write!(f, "{b:?}"), - Token::FmtStr(ref b) => write!(f, "f{b:?}"), + Token::FmtStr(ref b, _length) => write!(f, "f{b:?}"), Token::RawStr(ref b, hashes) => { let h: String = std::iter::once('#').cycle().take(hashes as usize).collect(); write!(f, "r{h}{b:?}{h}") @@ -515,7 +544,7 @@ impl Token { | Token::Bool(_) | Token::Str(_) | Token::RawStr(..) - | Token::FmtStr(_) => TokenKind::Literal, + | Token::FmtStr(_, _) => TokenKind::Literal, Token::Keyword(_) => TokenKind::Keyword, Token::UnquoteMarker(_) => TokenKind::UnquoteMarker, Token::Quote(_) => TokenKind::Quote, diff --git a/compiler/noirc_frontend/src/monomorphization/ast.rs b/compiler/noirc_frontend/src/monomorphization/ast.rs index 5d9b66f4f96..c9ae3438e42 100644 --- a/compiler/noirc_frontend/src/monomorphization/ast.rs +++ b/compiler/noirc_frontend/src/monomorphization/ast.rs @@ -7,11 +7,11 @@ use noirc_errors::{ Location, }; -use crate::hir_def::function::FunctionSignature; use crate::{ ast::{BinaryOpKind, IntegerBitSize, Signedness, Visibility}, token::{Attributes, FunctionAttribute}, }; +use crate::{hir_def::function::FunctionSignature, token::FmtStrFragment}; use serde::{Deserialize, Serialize}; use super::HirType; @@ -112,7 +112,7 @@ pub enum Literal { Bool(bool), Unit, Str(String), - FmtStr(String, u64, Box), + FmtStr(Vec, u64, Box), } #[derive(Debug, Clone, Hash)] diff --git a/compiler/noirc_frontend/src/monomorphization/mod.rs b/compiler/noirc_frontend/src/monomorphization/mod.rs index 050f844146a..b31a5744d09 100644 --- a/compiler/noirc_frontend/src/monomorphization/mod.rs +++ b/compiler/noirc_frontend/src/monomorphization/mod.rs @@ -12,6 +12,7 @@ use crate::ast::{FunctionKind, IntegerBitSize, Signedness, UnaryOp, Visibility}; use crate::hir::comptime::InterpreterError; use crate::hir::type_check::{NoMatchingImplFoundError, TypeCheckError}; use crate::node_interner::{ExprId, ImplSearchErrorKind}; +use crate::token::FmtStrFragment; use crate::{ debug::DebugInstrumenter, hir_def::{ @@ -417,10 +418,10 @@ impl<'interner> Monomorphizer<'interner> { let expr = match self.interner.expression(&expr) { HirExpression::Ident(ident, generics) => self.ident(ident, expr, generics)?, HirExpression::Literal(HirLiteral::Str(contents)) => Literal(Str(contents)), - HirExpression::Literal(HirLiteral::FmtStr(contents, idents)) => { + HirExpression::Literal(HirLiteral::FmtStr(fragments, idents, _length)) => { let fields = try_vecmap(idents, |ident| self.expr(ident))?; Literal(FmtStr( - contents, + fragments, fields.len() as u64, Box::new(ast::Expression::Tuple(fields)), )) @@ -1846,7 +1847,7 @@ impl<'interner> Monomorphizer<'interner> { _ => unreachable!("ICE: format string fields should be structured in a tuple, but got a {zeroed_tuple}"), }; ast::Expression::Literal(ast::Literal::FmtStr( - "\0".repeat(*length as usize), + vec![FmtStrFragment::String("\0".repeat(*length as usize))], fields_len, Box::new(zeroed_tuple), )) diff --git a/compiler/noirc_frontend/src/monomorphization/printer.rs b/compiler/noirc_frontend/src/monomorphization/printer.rs index b6421b26a03..9c1072a4117 100644 --- a/compiler/noirc_frontend/src/monomorphization/printer.rs +++ b/compiler/noirc_frontend/src/monomorphization/printer.rs @@ -105,9 +105,11 @@ impl AstPrinter { super::ast::Literal::Integer(x, _, _, _) => x.fmt(f), super::ast::Literal::Bool(x) => x.fmt(f), super::ast::Literal::Str(s) => s.fmt(f), - super::ast::Literal::FmtStr(s, _, _) => { + super::ast::Literal::FmtStr(fragments, _, _) => { write!(f, "f\"")?; - s.fmt(f)?; + for fragment in fragments { + fragment.fmt(f)?; + } write!(f, "\"") } super::ast::Literal::Unit => { diff --git a/compiler/noirc_frontend/src/parser/parser.rs b/compiler/noirc_frontend/src/parser/parser.rs index c2f7b781873..fcc58c5d833 100644 --- a/compiler/noirc_frontend/src/parser/parser.rs +++ b/compiler/noirc_frontend/src/parser/parser.rs @@ -5,7 +5,7 @@ use noirc_errors::Span; use crate::{ ast::{Ident, ItemVisibility}, lexer::{Lexer, SpannedTokenResult}, - token::{IntType, Keyword, SpannedToken, Token, TokenKind, Tokens}, + token::{FmtStrFragment, IntType, Keyword, SpannedToken, Token, TokenKind, Tokens}, }; use super::{labels::ParsingRuleLabel, ParsedModule, ParserError, ParserErrorReason}; @@ -294,11 +294,11 @@ impl<'a> Parser<'a> { } } - fn eat_fmt_str(&mut self) -> Option { + fn eat_fmt_str(&mut self) -> Option<(Vec, u32)> { if matches!(self.token.token(), Token::FmtStr(..)) { let token = self.bump(); match token.into_token() { - Token::FmtStr(string) => Some(string), + Token::FmtStr(fragments, length) => Some((fragments, length)), _ => unreachable!(), } } else { diff --git a/compiler/noirc_frontend/src/parser/parser/expression.rs b/compiler/noirc_frontend/src/parser/parser/expression.rs index e1ecc972eeb..526a0c3dd6e 100644 --- a/compiler/noirc_frontend/src/parser/parser/expression.rs +++ b/compiler/noirc_frontend/src/parser/parser/expression.rs @@ -577,7 +577,7 @@ impl<'a> Parser<'a> { /// BlockExpression = Block fn parse_literal(&mut self) -> Option { if let Some(bool) = self.eat_bool() { - return Some(ExpressionKind::Literal(Literal::Bool(bool))); + return Some(ExpressionKind::boolean(bool)); } if let Some(int) = self.eat_int() { @@ -585,15 +585,15 @@ impl<'a> Parser<'a> { } if let Some(string) = self.eat_str() { - return Some(ExpressionKind::Literal(Literal::Str(string))); + return Some(ExpressionKind::string(string)); } if let Some((string, n)) = self.eat_raw_str() { - return Some(ExpressionKind::Literal(Literal::RawStr(string, n))); + return Some(ExpressionKind::raw_string(string, n)); } - if let Some(string) = self.eat_fmt_str() { - return Some(ExpressionKind::Literal(Literal::FmtStr(string))); + if let Some((fragments, length)) = self.eat_fmt_str() { + return Some(ExpressionKind::format_string(fragments, length)); } if let Some(tokens) = self.eat_quote() { @@ -865,10 +865,11 @@ mod tests { fn parses_fmt_str() { let src = "f\"hello\""; let expr = parse_expression_no_errors(src); - let ExpressionKind::Literal(Literal::FmtStr(string)) = expr.kind else { + let ExpressionKind::Literal(Literal::FmtStr(fragments, length)) = expr.kind else { panic!("Expected format string literal"); }; - assert_eq!(string, "hello"); + assert_eq!(fragments[0].to_string(), "hello"); + assert_eq!(length, 5); } #[test] diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index cba29d58ea3..8ddf1b571e6 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -1209,8 +1209,6 @@ fn resolve_fmt_strings() { let string = f"this is i: {i}"; println(string); - println(f"I want to print {0}"); - let new_val = 10; println(f"random_string{new_val}{new_val}"); } @@ -1220,7 +1218,7 @@ fn resolve_fmt_strings() { "#; let errors = get_program_errors(src); - assert!(errors.len() == 5, "Expected 5 errors, got: {:?}", errors); + assert!(errors.len() == 3, "Expected 5 errors, got: {:?}", errors); for (err, _file_id) in errors { match &err { @@ -1229,21 +1227,13 @@ fn resolve_fmt_strings() { }) => { assert_eq!(name, "i"); } - CompilationError::ResolverError(ResolverError::NumericConstantInFormatString { - name, - .. - }) => { - assert_eq!(name, "0"); - } CompilationError::TypeError(TypeCheckError::UnusedResultError { expr_type: _, expr_span, }) => { let a = src.get(expr_span.start() as usize..expr_span.end() as usize).unwrap(); assert!( - a == "println(string)" - || a == "println(f\"I want to print {0}\")" - || a == "println(f\"random_string{new_val}{new_val}\")" + a == "println(string)" || a == "println(f\"random_string{new_val}{new_val}\")" ); } _ => unimplemented!(), diff --git a/compiler/noirc_printable_type/Cargo.toml b/compiler/noirc_printable_type/Cargo.toml index 8bb56703e8a..8d0574aad64 100644 --- a/compiler/noirc_printable_type/Cargo.toml +++ b/compiler/noirc_printable_type/Cargo.toml @@ -14,7 +14,6 @@ workspace = true [dependencies] acvm.workspace = true iter-extended.workspace = true -regex = "1.9.1" serde.workspace = true serde_json.workspace = true thiserror.workspace = true diff --git a/compiler/noirc_printable_type/src/lib.rs b/compiler/noirc_printable_type/src/lib.rs index 838a2472125..d46b37c4ea2 100644 --- a/compiler/noirc_printable_type/src/lib.rs +++ b/compiler/noirc_printable_type/src/lib.rs @@ -2,7 +2,7 @@ use std::{collections::BTreeMap, str}; use acvm::{acir::AcirField, brillig_vm::brillig::ForeignCallParam}; use iter_extended::vecmap; -use regex::{Captures, Regex}; + use serde::{Deserialize, Serialize}; use thiserror::Error; @@ -253,24 +253,6 @@ fn to_string(value: &PrintableValue, typ: &PrintableType) -> Op Some(output) } -// Taken from Regex docs directly -fn replace_all( - re: &Regex, - haystack: &str, - mut replacement: impl FnMut(&Captures) -> Result, -) -> Result { - let mut new = String::with_capacity(haystack.len()); - let mut last_match = 0; - for caps in re.captures_iter(haystack) { - let m = caps.get(0).unwrap(); - new.push_str(&haystack[last_match..m.start()]); - new.push_str(&replacement(&caps)?); - last_match = m.end(); - } - new.push_str(&haystack[last_match..]); - Ok(new) -} - impl std::fmt::Display for PrintableValueDisplay { fn fmt(&self, fmt: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { @@ -279,18 +261,56 @@ impl std::fmt::Display for PrintableValueDisplay { write!(fmt, "{output_string}") } Self::FmtString(template, values) => { - let mut display_iter = values.iter(); - let re = Regex::new(r"\{([a-zA-Z0-9_]+)\}").map_err(|_| std::fmt::Error)?; + let mut values_iter = values.iter(); + write_template_replacing_interpolations(template, fmt, || { + values_iter.next().and_then(|(value, typ)| to_string(value, typ)) + }) + } + } + } +} + +fn write_template_replacing_interpolations( + template: &str, + fmt: &mut std::fmt::Formatter<'_>, + mut replacement: impl FnMut() -> Option, +) -> std::fmt::Result { + let mut last_index = 0; // How far we've written from the template + let mut char_indices = template.char_indices().peekable(); + while let Some((char_index, char)) = char_indices.next() { + // Keep going forward until we find a '{' + if char != '{' { + continue; + } - let formatted_str = replace_all(&re, template, |_: &Captures| { - let (value, typ) = display_iter.next().ok_or(std::fmt::Error)?; - to_string(value, typ).ok_or(std::fmt::Error) - })?; + // We'll either have to write an interpolation or '{{' if it's an escape, + // so let's write what we've seen so far in the template. + write!(fmt, "{}", &template[last_index..char_index])?; - write!(fmt, "{formatted_str}") + // If it's '{{', write '{' and keep going + if char_indices.peek().map(|(_, char)| char) == Some(&'{') { + write!(fmt, "{{")?; + (last_index, _) = char_indices.next().unwrap(); + continue; + } + + // Write the interpolation + if let Some(string) = replacement() { + write!(fmt, "{}", string)?; + } else { + return Err(std::fmt::Error); + } + + // Whatever was inside '{...}' doesn't matter, so skip until we find '}' + while let Some((_, char)) = char_indices.next() { + if char == '}' { + last_index = char_indices.peek().map(|(index, _)| *index).unwrap_or(template.len()); + break; } } } + + write!(fmt, "{}", &template[last_index..]) } /// This trims any leading zeroes. @@ -390,3 +410,40 @@ pub fn decode_string_value(field_elements: &[F]) -> String { let final_string = str::from_utf8(&string_as_slice).unwrap(); final_string.to_owned() } + +#[cfg(test)] +mod tests { + use acvm::FieldElement; + + use crate::{PrintableType, PrintableValue, PrintableValueDisplay}; + + #[test] + fn printable_value_display_to_string_without_interpolations() { + let template = "hello"; + let display = + PrintableValueDisplay::::FmtString(template.to_string(), vec![]); + assert_eq!(display.to_string(), template); + } + + #[test] + fn printable_value_display_to_string_with_curly_escapes() { + let template = "hello {{world}} {{{{double_escape}}}}"; + let display = + PrintableValueDisplay::::FmtString(template.to_string(), vec![]); + assert_eq!(display.to_string(), template); + } + + #[test] + fn printable_value_display_to_string_with_interpolations() { + let template = "hello {one} {{no}} {two} {{not_again}} {three} world"; + let values = vec![ + (PrintableValue::String("ONE".to_string()), PrintableType::String { length: 3 }), + (PrintableValue::String("TWO".to_string()), PrintableType::String { length: 3 }), + (PrintableValue::String("THREE".to_string()), PrintableType::String { length: 5 }), + ]; + let expected = "hello ONE {{no}} TWO {{not_again}} THREE world"; + let display = + PrintableValueDisplay::::FmtString(template.to_string(), values); + assert_eq!(display.to_string(), expected); + } +} diff --git a/noir_stdlib/src/collections/map.nr b/noir_stdlib/src/collections/map.nr index bcce08faab4..2b0da1b90ec 100644 --- a/noir_stdlib/src/collections/map.nr +++ b/noir_stdlib/src/collections/map.nr @@ -201,7 +201,10 @@ impl HashMap { } } - let msg = f"Amount of valid elements should have been {self._len} times, but got {entries.len()}."; + let self_len = self._len; + let entries_len = entries.len(); + let msg = + f"Amount of valid elements should have been {self_len} times, but got {entries_len}."; assert(entries.len() == self._len, msg); entries @@ -236,8 +239,10 @@ impl HashMap { } } + let self_len = self._len; + let keys_len = keys.len(); let msg = - f"Amount of valid elements should have been {self._len} times, but got {keys.len()}."; + f"Amount of valid elements should have been {self_len} times, but got {keys_len}."; assert(keys.len() == self._len, msg); keys @@ -271,8 +276,10 @@ impl HashMap { } } + let self_len = self._len; + let values_len = values.len(); let msg = - f"Amount of valid elements should have been {self._len} times, but got {values.len()}."; + f"Amount of valid elements should have been {self_len} times, but got {values_len}."; assert(values.len() == self._len, msg); values diff --git a/noir_stdlib/src/collections/umap.nr b/noir_stdlib/src/collections/umap.nr index 3e074551e9d..7aebeb437cf 100644 --- a/noir_stdlib/src/collections/umap.nr +++ b/noir_stdlib/src/collections/umap.nr @@ -138,7 +138,10 @@ impl UHashMap { } } - let msg = f"Amount of valid elements should have been {self._len} times, but got {entries.len()}."; + let self_len = self._len; + let entries_len = entries.len(); + let msg = + f"Amount of valid elements should have been {self_len} times, but got {entries_len}."; assert(entries.len() == self._len, msg); entries @@ -158,8 +161,10 @@ impl UHashMap { } } + let self_len = self._len; + let keys_len = keys.len(); let msg = - f"Amount of valid elements should have been {self._len} times, but got {keys.len()}."; + f"Amount of valid elements should have been {self_len} times, but got {keys_len}."; assert(keys.len() == self._len, msg); keys @@ -179,8 +184,10 @@ impl UHashMap { } } + let self_len = self._len; + let values_len = values.len(); let msg = - f"Amount of valid elements should have been {self._len} times, but got {values.len()}."; + f"Amount of valid elements should have been {self_len} times, but got {values_len}."; assert(values.len() == self._len, msg); values diff --git a/test_programs/compile_success_empty/comptime_fmt_strings/src/main.nr b/test_programs/compile_success_empty/comptime_fmt_strings/src/main.nr index ca337c822d8..8cdd15aaa0e 100644 --- a/test_programs/compile_success_empty/comptime_fmt_strings/src/main.nr +++ b/test_programs/compile_success_empty/comptime_fmt_strings/src/main.nr @@ -6,7 +6,7 @@ fn main() { // Can't print these at compile-time here since printing to stdout while // compiling breaks the test runner. - let s1 = f"x is {x}, fake interpolation: \{y}, y is {y}"; + let s1 = f"x is {x}, fake interpolation: {{y}}, y is {y}"; let s2 = std::mem::zeroed::>(); (s1, s2) }; diff --git a/test_programs/execution_success/hashmap/src/main.nr b/test_programs/execution_success/hashmap/src/main.nr index cfd4e4a9136..aab531ea559 100644 --- a/test_programs/execution_success/hashmap/src/main.nr +++ b/test_programs/execution_success/hashmap/src/main.nr @@ -104,10 +104,11 @@ fn test_insert_and_methods(input: [Entry; HASHMAP_LEN]) { hashmap.insert(entry.key, entry.value); } - assert(hashmap.len() == HASHMAP_LEN, "hashmap.len() does not match input lenght."); + assert(hashmap.len() == HASHMAP_LEN, "hashmap.len() does not match input length."); for entry in input { - assert(hashmap.contains_key(entry.key), f"Not found inserted key {entry.key}."); + let entry_key = entry.key; + assert(hashmap.contains_key(entry.key), f"Not found inserted key {entry_key}."); } hashmap.clear(); diff --git a/test_programs/execution_success/uhashmap/src/main.nr b/test_programs/execution_success/uhashmap/src/main.nr index b56a4fe1747..689ba9d4a04 100644 --- a/test_programs/execution_success/uhashmap/src/main.nr +++ b/test_programs/execution_success/uhashmap/src/main.nr @@ -104,7 +104,8 @@ unconstrained fn test_insert_and_methods(input: [Entry; HASHMAP_LEN]) { assert(hashmap.len() == HASHMAP_LEN, "hashmap.len() does not match input length."); for entry in input { - assert(hashmap.contains_key(entry.key), f"Not found inserted key {entry.key}."); + let entry_key = entry.key; + assert(hashmap.contains_key(entry.key), f"Not found inserted key {entry_key}."); } hashmap.clear(); diff --git a/tooling/nargo_fmt/src/formatter/expression.rs b/tooling/nargo_fmt/src/formatter/expression.rs index 0730d06ad72..ecc9fab18ce 100644 --- a/tooling/nargo_fmt/src/formatter/expression.rs +++ b/tooling/nargo_fmt/src/formatter/expression.rs @@ -104,11 +104,12 @@ impl<'a, 'b> ChunkFormatter<'a, 'b> { formatter.write_left_paren(); formatter.write_right_paren(); })), - Literal::Bool(_) | Literal::Str(_) | Literal::FmtStr(_) | Literal::RawStr(..) => group - .text(self.chunk(|formatter| { + Literal::Bool(_) | Literal::Str(_) | Literal::FmtStr(_, _) | Literal::RawStr(..) => { + group.text(self.chunk(|formatter| { formatter.write_current_token_as_in_source(); formatter.bump(); - })), + })); + } Literal::Integer(..) => group.text(self.chunk(|formatter| { if formatter.is_at(Token::Minus) { formatter.write_token(Token::Minus); From 1cd2b4d42ce618b251587045b75bf8a6a25374a6 Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Fri, 6 Dec 2024 15:07:48 +0000 Subject: [PATCH 2/6] chore: add script to check for critical libraries supporting a given Noir version (#6697) Co-authored-by: Savio <72797635+Savio-Sou@users.noreply.github.com> Co-authored-by: Akosh Farkash --- .github/workflows/test-js-packages.yml | 49 +++++++++++++++----------- CRITICAL_NOIR_LIBRARIES | 13 +++++++ scripts/check-critical-libraries.sh | 37 +++++++++++++++++++ 3 files changed, 79 insertions(+), 20 deletions(-) create mode 100644 CRITICAL_NOIR_LIBRARIES create mode 100755 scripts/check-critical-libraries.sh diff --git a/.github/workflows/test-js-packages.yml b/.github/workflows/test-js-packages.yml index 6a9a918b955..36ece11b1bf 100644 --- a/.github/workflows/test-js-packages.yml +++ b/.github/workflows/test-js-packages.yml @@ -521,8 +521,27 @@ jobs: working-directory: ./examples/codegen_verifier run: ./test.sh + critical-library-list: + name: Load critical library list + runs-on: ubuntu-latest + outputs: + libraries: ${{ steps.get_critical_libraries.outputs.libraries }} + + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Build list of libraries + id: get_critical_libraries + run: | + LIBRARIES=$(grep -Po "^https://github.com/\K.+" ./CRITICAL_NOIR_LIBRARIES | jq -R -s -c 'split("\n") | map(select(. != "")) | map({ repo: ., path: "./"})') + echo "libraries=$LIBRARIES" + echo "libraries=$LIBRARIES" >> $GITHUB_OUTPUT + env: + GH_TOKEN: ${{ github.token }} + external-repo-checks: - needs: [build-nargo] + needs: [build-nargo, critical-library-list] runs-on: ubuntu-latest # Only run when 'run-external-checks' label is present if: contains(github.event.pull_request.labels.*.name, 'run-external-checks') @@ -530,25 +549,15 @@ jobs: strategy: fail-fast: false matrix: - project: - - { repo: noir-lang/ec, path: ./ } - - { repo: noir-lang/eddsa, path: ./ } - - { repo: noir-lang/mimc, path: ./ } - - { repo: noir-lang/noir_sort, path: ./ } - - { repo: noir-lang/noir-edwards, path: ./ } - - { repo: noir-lang/noir-bignum, path: ./ } - - { repo: noir-lang/noir_bigcurve, path: ./ } - - { repo: noir-lang/noir_base64, path: ./ } - - { repo: noir-lang/noir_string_search, path: ./ } - - { repo: noir-lang/sparse_array, path: ./ } - - { repo: noir-lang/noir_rsa, path: ./lib } - - { repo: AztecProtocol/aztec-packages, path: ./noir-projects/aztec-nr } - - { repo: AztecProtocol/aztec-packages, path: ./noir-projects/noir-contracts } - - { repo: AztecProtocol/aztec-packages, path: ./noir-projects/noir-protocol-circuits/crates/parity-lib } - - { repo: AztecProtocol/aztec-packages, path: ./noir-projects/noir-protocol-circuits/crates/private-kernel-lib } - - { repo: AztecProtocol/aztec-packages, path: ./noir-projects/noir-protocol-circuits/crates/reset-kernel-lib } - - { repo: AztecProtocol/aztec-packages, path: ./noir-projects/noir-protocol-circuits/crates/rollup-lib } - - { repo: AztecProtocol/aztec-packages, path: ./noir-projects/noir-protocol-circuits/crates/types } + project: ${{ fromJson( needs.critical-library-list.outputs.libraries )}} + include: + - project: { repo: AztecProtocol/aztec-packages, path: ./noir-projects/aztec-nr } + - project: { repo: AztecProtocol/aztec-packages, path: ./noir-projects/noir-contracts } + - project: { repo: AztecProtocol/aztec-packages, path: ./noir-projects/noir-protocol-circuits/crates/parity-lib } + - project: { repo: AztecProtocol/aztec-packages, path: ./noir-projects/noir-protocol-circuits/crates/private-kernel-lib } + - project: { repo: AztecProtocol/aztec-packages, path: ./noir-projects/noir-protocol-circuits/crates/reset-kernel-lib } + - project: { repo: AztecProtocol/aztec-packages, path: ./noir-projects/noir-protocol-circuits/crates/rollup-lib } + - project: { repo: AztecProtocol/aztec-packages, path: ./noir-projects/noir-protocol-circuits/crates/types } name: Check external repo - ${{ matrix.project.repo }} steps: diff --git a/CRITICAL_NOIR_LIBRARIES b/CRITICAL_NOIR_LIBRARIES new file mode 100644 index 00000000000..c753b76a4fc --- /dev/null +++ b/CRITICAL_NOIR_LIBRARIES @@ -0,0 +1,13 @@ +https://github.com/noir-lang/ec +https://github.com/noir-lang/eddsa +https://github.com/noir-lang/mimc +https://github.com/noir-lang/schnorr +https://github.com/noir-lang/noir_sort +https://github.com/noir-lang/noir-edwards +https://github.com/noir-lang/noir-bignum +https://github.com/noir-lang/noir_bigcurve +https://github.com/noir-lang/noir_base64 +https://github.com/noir-lang/noir_string_search +https://github.com/noir-lang/sparse_array +https://github.com/noir-lang/noir_rsa +https://github.com/noir-lang/noir_json_parser diff --git a/scripts/check-critical-libraries.sh b/scripts/check-critical-libraries.sh new file mode 100755 index 00000000000..b492cf1d4bc --- /dev/null +++ b/scripts/check-critical-libraries.sh @@ -0,0 +1,37 @@ +#!/usr/bin/env bash +set -e + +# Run relative to repo root +cd $(dirname "$0")/../ + +if [[ -z $1 ]]; then + echo "Must specify Noir release to test against" >&2 + echo "usage: ./check-critical-libraries.sh " >&2 + exit 1 +fi +noirup -v $1 + +CRITICAL_LIBRARIES=$(grep -v "^#\|^$" ./CRITICAL_NOIR_LIBRARIES) +readarray -t REPOS_TO_CHECK < <(echo "$CRITICAL_LIBRARIES") + +getLatestReleaseTagForRepo() { + REPO_NAME=$1 + TAG=$(gh release list -R $REPO_NAME --json 'tagName,isLatest' -q '.[] | select(.isLatest == true).tagName') + if [[ -z $TAG ]]; then + echo "$REPO_NAME has no valid release" >&2 + exit 1 + fi + echo $TAG +} + +for REPO in ${REPOS_TO_CHECK[@]}; do + echo $REPO + TMP_DIR=$(mktemp -d) + + TAG=$(getLatestReleaseTagForRepo $REPO) + git clone $REPO -c advice.detachedHead=false --depth 1 --branch $TAG $TMP_DIR + + nargo test --program-dir $TMP_DIR + + rm -rf $TMP_DIR +done From da9a74a2c70be8ba2a72b7dfb24393c029a27564 Mon Sep 17 00:00:00 2001 From: guipublic <47281315+guipublic@users.noreply.github.com> Date: Fri, 6 Dec 2024 16:18:58 +0100 Subject: [PATCH 3/6] chore: optimise older opcodes in reverse order (#6476) Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com> Co-authored-by: TomAFrench --- .../compiler/optimizers/merge_expressions.rs | 152 ++++++++++-------- .../regression_6451/src/main.nr | 2 +- 2 files changed, 90 insertions(+), 64 deletions(-) diff --git a/acvm-repo/acvm/src/compiler/optimizers/merge_expressions.rs b/acvm-repo/acvm/src/compiler/optimizers/merge_expressions.rs index 0a55e4ca17c..f49cd61e813 100644 --- a/acvm-repo/acvm/src/compiler/optimizers/merge_expressions.rs +++ b/acvm-repo/acvm/src/compiler/optimizers/merge_expressions.rs @@ -12,26 +12,36 @@ use acir::{ use crate::compiler::CircuitSimulator; -pub(crate) struct MergeExpressionsOptimizer { +pub(crate) struct MergeExpressionsOptimizer { resolved_blocks: HashMap>, + modified_gates: HashMap>, + deleted_gates: BTreeSet, } -impl MergeExpressionsOptimizer { +impl MergeExpressionsOptimizer { pub(crate) fn new() -> Self { - MergeExpressionsOptimizer { resolved_blocks: HashMap::new() } + MergeExpressionsOptimizer { + resolved_blocks: HashMap::new(), + modified_gates: HashMap::new(), + deleted_gates: BTreeSet::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( + pub(crate) fn eliminate_intermediate_variable( &mut self, circuit: &Circuit, acir_opcode_positions: Vec, ) -> (Vec>, Vec) { + // Initialization + self.modified_gates.clear(); + self.deleted_gates.clear(); + self.resolved_blocks.clear(); + // 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: BTreeMap> = BTreeMap::new(); for (i, opcode) in circuit.opcodes.iter().enumerate() { let witnesses = self.witness_inputs(opcode); @@ -46,80 +56,89 @@ impl MergeExpressionsOptimizer { } } - 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, opcode_position)) in - circuit.opcodes.iter().zip(acir_opcode_positions).enumerate() - { + for (i, opcode) in circuit.opcodes.iter().enumerate() { if !matches!(opcode, Opcode::AssertZero(_)) { - new_circuit.push(opcode.clone()); - new_acir_opcode_positions.push(opcode_position); 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 { - let Some(gates_using_w) = used_witness.get(&w) else { - continue; - }; - // We only consider witness which are used in exactly two arithmetic gates - if gates_using_w.len() == 2 { - let first = *gates_using_w.first().expect("gates_using_w.len == 2"); - let second = *gates_using_w.last().expect("gates_using_w.len == 2"); - let b = if second == i { - first - } else { - // sanity check - assert!(i == first); - second + if let Some(opcode) = self.get_opcode(i, circuit) { + let input_witnesses = self.witness_inputs(&opcode); + for w in input_witnesses { + let Some(gates_using_w) = used_witness.get(&w) else { + continue; }; - - let second_gate = modified_gates.get(&b).unwrap_or(&circuit.opcodes[b]); - if let (Opcode::AssertZero(expr_define), Opcode::AssertZero(expr_use)) = - (&opcode, second_gate) - { - // We cannot merge an expression into an earlier opcode, because this - // would break the 'execution ordering' of the opcodes - // This case can happen because a previous merge would change an opcode - // and eliminate a witness from it, giving new opportunities for this - // witness to be used in only two expressions - // TODO: the missed optimization for the i>b case can be handled by - // - doing this pass again until there is no change, or - // - merging 'b' into 'i' instead - if i < b { - if let Some(expr) = Self::merge(expr_use, expr_define, w) { - modified_gates.insert(b, Opcode::AssertZero(expr)); - to_keep = false; - // Update the 'used_witness' map to account for the merge. - for w2 in CircuitSimulator::expr_wit(expr_define) { - if !circuit_inputs.contains(&w2) { - let v = used_witness.entry(w2).or_default(); - v.insert(b); - v.remove(&i); + // We only consider witness which are used in exactly two arithmetic gates + if gates_using_w.len() == 2 { + let first = *gates_using_w.first().expect("gates_using_w.len == 2"); + let second = *gates_using_w.last().expect("gates_using_w.len == 2"); + let b = if second == i { + first + } else { + // sanity check + assert!(i == first); + second + }; + // Merge the opcode with smaller index into the other one + // by updating modified_gates/deleted_gates/used_witness + // returns false if it could not merge them + let mut merge_opcodes = |op1, op2| -> bool { + if op1 == op2 { + return false; + } + let (source, target) = if op1 < op2 { (op1, op2) } else { (op2, op1) }; + let source_opcode = self.get_opcode(source, circuit); + let target_opcode = self.get_opcode(target, circuit); + if let ( + Some(Opcode::AssertZero(expr_use)), + Some(Opcode::AssertZero(expr_define)), + ) = (target_opcode, source_opcode) + { + if let Some(expr) = + Self::merge_expression(&expr_use, &expr_define, w) + { + self.modified_gates.insert(target, Opcode::AssertZero(expr)); + self.deleted_gates.insert(source); + // Update the 'used_witness' map to account for the merge. + let mut witness_list = CircuitSimulator::expr_wit(&expr_use); + witness_list.extend(CircuitSimulator::expr_wit(&expr_define)); + for w2 in witness_list { + if !circuit_inputs.contains(&w2) { + used_witness.entry(w2).and_modify(|v| { + v.insert(target); + v.remove(&source); + }); + } } + return true; } - // We need to stop here and continue with the next opcode - // because the merge invalidates the current opcode. - break; } + false + }; + + if merge_opcodes(b, i) { + // We need to stop here and continue with the next opcode + // because the merge invalidates the current opcode. + break; } } } } + } + + // Construct the new circuit from modified/deleted gates + let mut new_circuit = Vec::new(); + let mut new_acir_opcode_positions = Vec::new(); - if to_keep { - let opcode = modified_gates.get(&i).cloned().unwrap_or(opcode); - new_circuit.push(opcode); - new_acir_opcode_positions.push(opcode_position); + for (i, opcode_position) in acir_opcode_positions.iter().enumerate() { + if let Some(op) = self.get_opcode(i, circuit) { + new_circuit.push(op); + new_acir_opcode_positions.push(*opcode_position); } } (new_circuit, new_acir_opcode_positions) } - fn brillig_input_wit(&self, input: &BrilligInputs) -> BTreeSet { + fn brillig_input_wit(&self, input: &BrilligInputs) -> BTreeSet { let mut result = BTreeSet::new(); match input { BrilligInputs::Single(expr) => { @@ -152,7 +171,7 @@ impl MergeExpressionsOptimizer { } // Returns the input witnesses used by the opcode - fn witness_inputs(&self, opcode: &Opcode) -> BTreeSet { + fn witness_inputs(&self, opcode: &Opcode) -> BTreeSet { match opcode { Opcode::AssertZero(expr) => CircuitSimulator::expr_wit(expr), Opcode::BlackBoxFuncCall(bb_func) => { @@ -198,7 +217,7 @@ impl MergeExpressionsOptimizer { // Merge 'expr' into 'target' via Gaussian elimination on 'w' // Returns None if the expressions cannot be merged - fn merge( + fn merge_expression( target: &Expression, expr: &Expression, w: Witness, @@ -226,6 +245,13 @@ impl MergeExpressionsOptimizer { } None } + + fn get_opcode(&self, g: usize, circuit: &Circuit) -> Option> { + if self.deleted_gates.contains(&g) { + return None; + } + self.modified_gates.get(&g).or(circuit.opcodes.get(g)).cloned() + } } #[cfg(test)] diff --git a/test_programs/execution_success/regression_6451/src/main.nr b/test_programs/execution_success/regression_6451/src/main.nr index fbee6956dfa..b13b6c25a7e 100644 --- a/test_programs/execution_success/regression_6451/src/main.nr +++ b/test_programs/execution_success/regression_6451/src/main.nr @@ -9,7 +9,7 @@ fn main(x: Field) { value += term2; value.assert_max_bit_size::<1>(); - // Regression test for Aztec Packages issue #6451 + // Regression test for #6447 (Aztec Packages issue #9488) let y = unsafe { empty(x + 1) }; let z = y + x + 1; let z1 = z + y; From df71df7e875b40529aa9404d45fd391a8857568a Mon Sep 17 00:00:00 2001 From: Akosh Farkash Date: Fri, 6 Dec 2024 17:06:02 +0000 Subject: [PATCH 4/6] fix: git dependency trailing slash (#6725) Co-authored-by: Ary Borenszweig --- Cargo.lock | 1 + tooling/nargo_toml/Cargo.toml | 1 + tooling/nargo_toml/src/git.rs | 28 ++++++++++++++++++++++++---- 3 files changed, 26 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 141a25f42a9..e8226d5fc58 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2882,6 +2882,7 @@ dependencies = [ "noirc_frontend", "semver", "serde", + "test-case", "thiserror", "toml 0.7.8", "url 2.5.3", diff --git a/tooling/nargo_toml/Cargo.toml b/tooling/nargo_toml/Cargo.toml index e4766e44859..2bc24153836 100644 --- a/tooling/nargo_toml/Cargo.toml +++ b/tooling/nargo_toml/Cargo.toml @@ -25,3 +25,4 @@ noirc_driver.workspace = true semver = "1.0.20" [dev-dependencies] +test-case.workspace = true diff --git a/tooling/nargo_toml/src/git.rs b/tooling/nargo_toml/src/git.rs index 80e57247ae6..efaed4fabb9 100644 --- a/tooling/nargo_toml/src/git.rs +++ b/tooling/nargo_toml/src/git.rs @@ -3,16 +3,20 @@ use std::path::PathBuf; /// Creates a unique folder name for a GitHub repo /// by using its URL and tag fn resolve_folder_name(base: &url::Url, tag: &str) -> String { - let mut folder_name = base.domain().unwrap().to_owned(); - folder_name.push_str(base.path()); - folder_name.push_str(tag); - folder_name + let mut folder = PathBuf::from(""); + for part in [base.domain().unwrap(), base.path(), tag] { + folder.push(part.trim_start_matches('/')); + } + folder.to_string_lossy().into_owned() } +/// Path to the `nargo` directory under `$HOME`. fn nargo_crates() -> PathBuf { dirs::home_dir().unwrap().join("nargo") } +/// Target directory to download dependencies into, e.g. +/// `$HOME/nargo/github.com/noir-lang/noir-bignum/v0.1.2` fn git_dep_location(base: &url::Url, tag: &str) -> PathBuf { let folder_name = resolve_folder_name(base, tag); @@ -53,3 +57,19 @@ pub(crate) fn clone_git_repo(url: &str, tag: &str) -> Result { Ok(loc) } + +#[cfg(test)] +mod tests { + use test_case::test_case; + use url::Url; + + use super::resolve_folder_name; + + #[test_case("https://github.com/noir-lang/noir-bignum/"; "with slash")] + #[test_case("https://github.com/noir-lang/noir-bignum"; "without slash")] + fn test_resolve_folder_name(url: &str) { + let tag = "v0.4.2"; + let dir = resolve_folder_name(&Url::parse(url).unwrap(), tag); + assert_eq!(dir, "github.com/noir-lang/noir-bignum/v0.4.2"); + } +} From 6d0f86ba389a5b59b1d7fdcadcbce3e40eecaa48 Mon Sep 17 00:00:00 2001 From: guipublic <47281315+guipublic@users.noreply.github.com> Date: Fri, 6 Dec 2024 20:14:57 +0100 Subject: [PATCH 5/6] chore: simplify MSM with constant folding (#6650) --- compiler/noirc_evaluator/src/lib.rs | 6 +- .../src/ssa/ir/instruction/call/blackbox.rs | 240 +++++++++++++++--- .../embedded_curve_ops/src/main.nr | 18 ++ 3 files changed, 222 insertions(+), 42 deletions(-) diff --git a/compiler/noirc_evaluator/src/lib.rs b/compiler/noirc_evaluator/src/lib.rs index 8127e3d03ef..75ea557d3de 100644 --- a/compiler/noirc_evaluator/src/lib.rs +++ b/compiler/noirc_evaluator/src/lib.rs @@ -12,8 +12,7 @@ pub mod ssa; pub use ssa::create_program; pub use ssa::ir::instruction::ErrorType; -/// Trims leading whitespace from each line of the input string, according to -/// how much leading whitespace there is on the first non-empty line. +/// Trims leading whitespace from each line of the input string #[cfg(test)] pub(crate) fn trim_leading_whitespace_from_lines(src: &str) -> String { let mut lines = src.trim_end().lines(); @@ -21,11 +20,10 @@ pub(crate) fn trim_leading_whitespace_from_lines(src: &str) -> String { while first_line.is_empty() { first_line = lines.next().unwrap(); } - let indent = first_line.len() - first_line.trim_start().len(); let mut result = first_line.trim_start().to_string(); for line in lines { result.push('\n'); - result.push_str(&line[indent..]); + result.push_str(line.trim_start()); } result } diff --git a/compiler/noirc_evaluator/src/ssa/ir/instruction/call/blackbox.rs b/compiler/noirc_evaluator/src/ssa/ir/instruction/call/blackbox.rs index 301b75e0bd4..db085bd762f 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/instruction/call/blackbox.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/instruction/call/blackbox.rs @@ -2,10 +2,11 @@ use std::sync::Arc; use acvm::{acir::AcirField, BlackBoxFunctionSolver, BlackBoxResolutionError, FieldElement}; +use crate::ssa::ir::instruction::BlackBoxFunc; use crate::ssa::ir::{ basic_block::BasicBlockId, dfg::{CallStack, DataFlowGraph}, - instruction::{Instruction, SimplifyResult}, + instruction::{Instruction, Intrinsic, SimplifyResult}, types::Type, value::ValueId, }; @@ -70,52 +71,125 @@ pub(super) fn simplify_msm( block: BasicBlockId, call_stack: &CallStack, ) -> SimplifyResult { - // TODO: Handle MSMs where a subset of the terms are constant. + let mut is_constant; + match (dfg.get_array_constant(arguments[0]), dfg.get_array_constant(arguments[1])) { (Some((points, _)), Some((scalars, _))) => { - let Some(points) = points - .into_iter() - .map(|id| dfg.get_numeric_constant(id)) - .collect::>>() - else { - return SimplifyResult::None; - }; - - let Some(scalars) = scalars - .into_iter() - .map(|id| dfg.get_numeric_constant(id)) - .collect::>>() - else { - return SimplifyResult::None; - }; + // We decompose points and scalars into constant and non-constant parts in order to simplify MSMs where a subset of the terms are constant. + let mut constant_points = vec![]; + let mut constant_scalars_lo = vec![]; + let mut constant_scalars_hi = vec![]; + let mut var_points = vec![]; + let mut var_scalars = vec![]; + let len = scalars.len() / 2; + for i in 0..len { + match ( + dfg.get_numeric_constant(scalars[2 * i]), + dfg.get_numeric_constant(scalars[2 * i + 1]), + dfg.get_numeric_constant(points[3 * i]), + dfg.get_numeric_constant(points[3 * i + 1]), + dfg.get_numeric_constant(points[3 * i + 2]), + ) { + (Some(lo), Some(hi), _, _, _) if lo.is_zero() && hi.is_zero() => { + is_constant = true; + constant_scalars_lo.push(lo); + constant_scalars_hi.push(hi); + constant_points.push(FieldElement::zero()); + constant_points.push(FieldElement::zero()); + constant_points.push(FieldElement::one()); + } + (_, _, _, _, Some(infinity)) if infinity.is_one() => { + is_constant = true; + constant_scalars_lo.push(FieldElement::zero()); + constant_scalars_hi.push(FieldElement::zero()); + constant_points.push(FieldElement::zero()); + constant_points.push(FieldElement::zero()); + constant_points.push(FieldElement::one()); + } + (Some(lo), Some(hi), Some(x), Some(y), Some(infinity)) => { + is_constant = true; + constant_scalars_lo.push(lo); + constant_scalars_hi.push(hi); + constant_points.push(x); + constant_points.push(y); + constant_points.push(infinity); + } + _ => { + is_constant = false; + } + } - let mut scalars_lo = Vec::new(); - let mut scalars_hi = Vec::new(); - for (i, scalar) in scalars.into_iter().enumerate() { - if i % 2 == 0 { - scalars_lo.push(scalar); - } else { - scalars_hi.push(scalar); + if !is_constant { + var_points.push(points[3 * i]); + var_points.push(points[3 * i + 1]); + var_points.push(points[3 * i + 2]); + var_scalars.push(scalars[2 * i]); + var_scalars.push(scalars[2 * i + 1]); } } - let Ok((result_x, result_y, result_is_infinity)) = - solver.multi_scalar_mul(&points, &scalars_lo, &scalars_hi) - else { + // If there are no constant terms, we can't simplify + if constant_scalars_lo.is_empty() { + return SimplifyResult::None; + } + let Ok((result_x, result_y, result_is_infinity)) = solver.multi_scalar_mul( + &constant_points, + &constant_scalars_lo, + &constant_scalars_hi, + ) else { return SimplifyResult::None; }; - let result_x = dfg.make_constant(result_x, Type::field()); - let result_y = dfg.make_constant(result_y, Type::field()); - let result_is_infinity = dfg.make_constant(result_is_infinity, Type::field()); - - let elements = im::vector![result_x, result_y, result_is_infinity]; - let typ = Type::Array(Arc::new(vec![Type::field()]), 3); - let instruction = Instruction::MakeArray { elements, typ }; - let result_array = - dfg.insert_instruction_and_results(instruction, block, None, call_stack.clone()); - - SimplifyResult::SimplifiedTo(result_array.first()) + // If there are no variable term, we can directly return the constant result + if var_scalars.is_empty() { + let result_x = dfg.make_constant(result_x, Type::field()); + let result_y = dfg.make_constant(result_y, Type::field()); + let result_is_infinity = dfg.make_constant(result_is_infinity, Type::field()); + + let elements = im::vector![result_x, result_y, result_is_infinity]; + let typ = Type::Array(Arc::new(vec![Type::field()]), 3); + let instruction = Instruction::MakeArray { elements, typ }; + let result_array = dfg.insert_instruction_and_results( + instruction, + block, + None, + call_stack.clone(), + ); + + return SimplifyResult::SimplifiedTo(result_array.first()); + } + // If there is only one non-null constant term, we cannot simplify + if constant_scalars_lo.len() == 1 && result_is_infinity != FieldElement::one() { + return SimplifyResult::None; + } + // Add the constant part back to the non-constant part, if it is not null + let one = dfg.make_constant(FieldElement::one(), Type::field()); + let zero = dfg.make_constant(FieldElement::zero(), Type::field()); + if result_is_infinity.is_zero() { + var_scalars.push(one); + var_scalars.push(zero); + let result_x = dfg.make_constant(result_x, Type::field()); + let result_y = dfg.make_constant(result_y, Type::field()); + let result_is_infinity = dfg.make_constant(result_is_infinity, Type::bool()); + var_points.push(result_x); + var_points.push(result_y); + var_points.push(result_is_infinity); + } + // Construct the simplified MSM expression + let typ = Type::Array(Arc::new(vec![Type::field()]), var_scalars.len() as u32); + let scalars = Instruction::MakeArray { elements: var_scalars.into(), typ }; + let scalars = dfg + .insert_instruction_and_results(scalars, block, None, call_stack.clone()) + .first(); + let typ = Type::Array(Arc::new(vec![Type::field()]), var_points.len() as u32); + let points = Instruction::MakeArray { elements: var_points.into(), typ }; + let points = + dfg.insert_instruction_and_results(points, block, None, call_stack.clone()).first(); + let msm = dfg.import_intrinsic(Intrinsic::BlackBox(BlackBoxFunc::MultiScalarMul)); + SimplifyResult::SimplifiedToInstruction(Instruction::Call { + func: msm, + arguments: vec![points, scalars], + }) } _ => SimplifyResult::None, } @@ -261,3 +335,93 @@ pub(super) fn simplify_signature( _ => SimplifyResult::None, } } + +#[cfg(feature = "bn254")] +#[cfg(test)] +mod test { + use crate::ssa::opt::assert_normalized_ssa_equals; + use crate::ssa::Ssa; + + #[cfg(feature = "bn254")] + #[test] + fn full_constant_folding() { + let src = r#" + acir(inline) fn main f0 { + b0(): + v0 = make_array [Field 2, Field 3, Field 5, Field 5] : [Field; 4] + v1 = make_array [Field 1, Field 17631683881184975370165255887551781615748388533673675138860, Field 0, Field 1, Field 17631683881184975370165255887551781615748388533673675138860, Field 0] : [Field; 6] + v2 = call multi_scalar_mul (v1, v0) -> [Field; 3] + return v2 + }"#; + let ssa = Ssa::from_str(src).unwrap(); + + let expected_src = r#" + acir(inline) fn main f0 { + b0(): + v3 = make_array [Field 2, Field 3, Field 5, Field 5] : [Field; 4] + v7 = make_array [Field 1, Field 17631683881184975370165255887551781615748388533673675138860, Field 0, Field 1, Field 17631683881184975370165255887551781615748388533673675138860, Field 0] : [Field; 6] + v10 = make_array [Field 1478523918288173385110236399861791147958001875200066088686689589556927843200, Field 700144278551281040379388961242974992655630750193306467120985766322057145630, Field 0] : [Field; 3] + return v10 + } + "#; + assert_normalized_ssa_equals(ssa, expected_src); + } + + #[cfg(feature = "bn254")] + #[test] + fn simplify_zero() { + let src = r#" + acir(inline) fn main f0 { + b0(v0: Field, v1: Field): + v2 = make_array [v0, Field 0, Field 0, Field 0, v0, Field 0] : [Field; 6] + v3 = make_array [ + Field 0, Field 0, Field 1, v0, v1, Field 0, Field 1, v0, Field 0] : [Field; 9] + v4 = call multi_scalar_mul (v3, v2) -> [Field; 3] + + return v4 + + }"#; + let ssa = Ssa::from_str(src).unwrap(); + //First point is zero, second scalar is zero, so we should be left with the scalar mul of the last point. + let expected_src = r#" + acir(inline) fn main f0 { + b0(v0: Field, v1: Field): + v3 = make_array [v0, Field 0, Field 0, Field 0, v0, Field 0] : [Field; 6] + v5 = make_array [Field 0, Field 0, Field 1, v0, v1, Field 0, Field 1, v0, Field 0] : [Field; 9] + v6 = make_array [v0, Field 0] : [Field; 2] + v7 = make_array [Field 1, v0, Field 0] : [Field; 3] + v9 = call multi_scalar_mul(v7, v6) -> [Field; 3] + return v9 + } + "#; + assert_normalized_ssa_equals(ssa, expected_src); + } + + #[cfg(feature = "bn254")] + #[test] + fn partial_constant_folding() { + let src = r#" + acir(inline) fn main f0 { + b0(v0: Field, v1: Field): + v2 = make_array [Field 1, Field 0, v0, Field 0, Field 2, Field 0] : [Field; 6] + v3 = make_array [ + Field 1, Field 17631683881184975370165255887551781615748388533673675138860, Field 0, v0, v1, Field 0, Field 1, Field 17631683881184975370165255887551781615748388533673675138860, Field 0] : [Field; 9] + v4 = call multi_scalar_mul (v3, v2) -> [Field; 3] + return v4 + }"#; + let ssa = Ssa::from_str(src).unwrap(); + //First and last scalar/point are constant, so we should be left with the msm of the middle point and the folded constant point + let expected_src = r#" + acir(inline) fn main f0 { + b0(v0: Field, v1: Field): + v5 = make_array [Field 1, Field 0, v0, Field 0, Field 2, Field 0] : [Field; 6] + v7 = make_array [Field 1, Field 17631683881184975370165255887551781615748388533673675138860, Field 0, v0, v1, Field 0, Field 1, Field 17631683881184975370165255887551781615748388533673675138860, Field 0] : [Field; 9] + v8 = make_array [v0, Field 0, Field 1, Field 0] : [Field; 4] + v12 = make_array [v0, v1, Field 0, Field -3227352362257037263902424173275354266044964400219754872043023745437788450996, Field 8902249110305491597038405103722863701255802573786510474664632793109847672620, u1 0] : [Field; 6] + v14 = call multi_scalar_mul(v12, v8) -> [Field; 3] + return v14 + } + "#; + assert_normalized_ssa_equals(ssa, expected_src); + } +} diff --git a/test_programs/execution_success/embedded_curve_ops/src/main.nr b/test_programs/execution_success/embedded_curve_ops/src/main.nr index e69184b9c96..85cf60dc796 100644 --- a/test_programs/execution_success/embedded_curve_ops/src/main.nr +++ b/test_programs/execution_success/embedded_curve_ops/src/main.nr @@ -20,4 +20,22 @@ fn main(priv_key: Field, pub_x: pub Field, pub_y: pub Field) { // The results should be double the g1 point because the scalars are 1 and we pass in g1 twice assert(double.x == res.x); + + // Tests for #6549 + let const_scalar1 = std::embedded_curve_ops::EmbeddedCurveScalar { lo: 23, hi: 0 }; + let const_scalar2 = std::embedded_curve_ops::EmbeddedCurveScalar { lo: 0, hi: 23 }; + let const_scalar3 = std::embedded_curve_ops::EmbeddedCurveScalar { lo: 13, hi: 4 }; + let partial_mul = std::embedded_curve_ops::multi_scalar_mul( + [g1, double, pub_point, g1, g1], + [scalar, const_scalar1, scalar, const_scalar2, const_scalar3], + ); + assert(partial_mul.x == 0x2024c4eebfbc8a20018f8c95c7aab77c6f34f10cf785a6f04e97452d8708fda7); + // Check simplification by zero + let zero_point = std::embedded_curve_ops::EmbeddedCurvePoint { x: 0, y: 0, is_infinite: true }; + let const_zero = std::embedded_curve_ops::EmbeddedCurveScalar { lo: 0, hi: 0 }; + let partial_mul = std::embedded_curve_ops::multi_scalar_mul( + [zero_point, double, g1], + [scalar, const_zero, scalar], + ); + assert(partial_mul == g1); } From 66d32751311378701b075ee7b2106d61e531ae4f Mon Sep 17 00:00:00 2001 From: Aztec Bot <49558828+AztecBot@users.noreply.github.com> Date: Mon, 9 Dec 2024 05:24:22 -0500 Subject: [PATCH 6/6] feat: Sync from aztec-packages (#6730) Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com> Co-authored-by: Tom French --- .aztec-sync-commit | 2 +- acvm-repo/acir/codegen/acir.cpp | 128 +-------------- .../acir/src/circuit/black_box_functions.rs | 30 ---- acvm-repo/acir/src/circuit/mod.rs | 20 +-- .../opcodes/black_box_function_call.rs | 52 ------- .../acir/tests/test_program_serialization.rs | 65 +------- acvm-repo/acvm/src/pwg/blackbox/mod.rs | 20 +-- .../acvm/src/pwg/blackbox/signature/mod.rs | 1 - .../src/pwg/blackbox/signature/schnorr.rs | 36 ----- .../test/browser/execute_circuit.test.ts | 10 -- .../acvm_js/test/node/execute_circuit.test.ts | 10 -- .../acvm_js/test/shared/multi_scalar_mul.ts | 6 +- .../acvm_js/test/shared/schnorr_verify.ts | 101 ------------ .../src/curve_specific_solver.rs | 16 -- .../benches/criterion.rs | 35 +---- .../src/embedded_curve_ops.rs | 45 +++--- acvm-repo/bn254_blackbox_solver/src/lib.rs | 20 --- .../src/pedersen/commitment.rs | 77 --------- .../src/pedersen/hash.rs | 69 -------- .../bn254_blackbox_solver/src/pedersen/mod.rs | 2 - .../bn254_blackbox_solver/src/schnorr/mod.rs | 147 ------------------ acvm-repo/brillig/src/black_box.rs | 9 +- acvm-repo/brillig_vm/src/black_box.rs | 12 -- compiler/integration-tests/package.json | 2 +- .../src/acir/generated_acir.rs | 21 +-- .../brillig/brillig_gen/brillig_black_box.rs | 21 --- .../noirc_evaluator/src/brillig/brillig_ir.rs | 9 -- .../src/brillig/brillig_ir/debug_show.rs | 17 -- .../src/ssa/ir/instruction/call.rs | 1 - .../src/ssa/ir/instruction/call/blackbox.rs | 33 ---- .../cryptographic_primitives/schnorr.mdx | 10 -- docs/versioned_docs/version-v0.33.0/index.mdx | 2 +- noir_stdlib/src/schnorr.nr | 25 +-- scripts/install_bb.sh | 2 +- .../schnorr_simplification/src/main.nr | 11 +- .../execution_success/schnorr/src/main.nr | 12 +- tooling/lsp/src/solver.rs | 10 -- tooling/profiler/src/opcode_formatter.rs | 2 - tooling/readme.md | 2 +- yarn.lock | 11 +- 40 files changed, 62 insertions(+), 1042 deletions(-) delete mode 100644 acvm-repo/acvm/src/pwg/blackbox/signature/schnorr.rs delete mode 100644 acvm-repo/acvm_js/test/shared/schnorr_verify.ts delete mode 100644 acvm-repo/bn254_blackbox_solver/src/pedersen/commitment.rs delete mode 100644 acvm-repo/bn254_blackbox_solver/src/pedersen/hash.rs delete mode 100644 acvm-repo/bn254_blackbox_solver/src/pedersen/mod.rs delete mode 100644 acvm-repo/bn254_blackbox_solver/src/schnorr/mod.rs diff --git a/.aztec-sync-commit b/.aztec-sync-commit index 477ebbca903..46138f41929 100644 --- a/.aztec-sync-commit +++ b/.aztec-sync-commit @@ -1 +1 @@ -0577c1a70e9746bd06f07d2813af1be39e01ca02 +fca96007d6055dcf00b72a46630c680fcb6d190d diff --git a/acvm-repo/acir/codegen/acir.cpp b/acvm-repo/acir/codegen/acir.cpp index 2ae9a31d6ca..e94f36535d2 100644 --- a/acvm-repo/acir/codegen/acir.cpp +++ b/acvm-repo/acir/codegen/acir.cpp @@ -318,18 +318,6 @@ namespace Program { static EcdsaSecp256r1 bincodeDeserialize(std::vector); }; - struct SchnorrVerify { - Program::MemoryAddress public_key_x; - Program::MemoryAddress public_key_y; - Program::HeapVector message; - Program::HeapVector signature; - Program::MemoryAddress result; - - friend bool operator==(const SchnorrVerify&, const SchnorrVerify&); - std::vector bincodeSerialize() const; - static SchnorrVerify bincodeDeserialize(std::vector); - }; - struct MultiScalarMul { Program::HeapVector points; Program::HeapVector scalars; @@ -444,7 +432,7 @@ namespace Program { static ToRadix bincodeDeserialize(std::vector); }; - std::variant value; + std::variant value; friend bool operator==(const BlackBoxOp&, const BlackBoxOp&); std::vector bincodeSerialize() const; @@ -817,18 +805,6 @@ namespace Program { static Blake3 bincodeDeserialize(std::vector); }; - struct SchnorrVerify { - Program::FunctionInput public_key_x; - Program::FunctionInput public_key_y; - std::array signature; - std::vector message; - Program::Witness output; - - friend bool operator==(const SchnorrVerify&, const SchnorrVerify&); - std::vector bincodeSerialize() const; - static SchnorrVerify bincodeDeserialize(std::vector); - }; - struct EcdsaSecp256k1 { std::array public_key_x; std::array public_key_y; @@ -973,7 +949,7 @@ namespace Program { static Sha256Compression bincodeDeserialize(std::vector); }; - std::variant value; + std::variant value; friend bool operator==(const BlackBoxFuncCall&, const BlackBoxFuncCall&); std::vector bincodeSerialize() const; @@ -2528,56 +2504,6 @@ Program::BlackBoxFuncCall::Blake3 serde::Deserializable BlackBoxFuncCall::SchnorrVerify::bincodeSerialize() const { - auto serializer = serde::BincodeSerializer(); - serde::Serializable::serialize(*this, serializer); - return std::move(serializer).bytes(); - } - - inline BlackBoxFuncCall::SchnorrVerify BlackBoxFuncCall::SchnorrVerify::bincodeDeserialize(std::vector input) { - auto deserializer = serde::BincodeDeserializer(input); - auto value = serde::Deserializable::deserialize(deserializer); - if (deserializer.get_buffer_offset() < input.size()) { - throw serde::deserialization_error("Some input bytes were not read"); - } - return value; - } - -} // end of namespace Program - -template <> -template -void serde::Serializable::serialize(const Program::BlackBoxFuncCall::SchnorrVerify &obj, Serializer &serializer) { - serde::Serializable::serialize(obj.public_key_x, serializer); - serde::Serializable::serialize(obj.public_key_y, serializer); - serde::Serializable::serialize(obj.signature, serializer); - serde::Serializable::serialize(obj.message, serializer); - serde::Serializable::serialize(obj.output, serializer); -} - -template <> -template -Program::BlackBoxFuncCall::SchnorrVerify serde::Deserializable::deserialize(Deserializer &deserializer) { - Program::BlackBoxFuncCall::SchnorrVerify obj; - obj.public_key_x = serde::Deserializable::deserialize(deserializer); - obj.public_key_y = serde::Deserializable::deserialize(deserializer); - obj.signature = serde::Deserializable::deserialize(deserializer); - obj.message = serde::Deserializable::deserialize(deserializer); - obj.output = serde::Deserializable::deserialize(deserializer); - return obj; -} - namespace Program { inline bool operator==(const BlackBoxFuncCall::EcdsaSecp256k1 &lhs, const BlackBoxFuncCall::EcdsaSecp256k1 &rhs) { @@ -3518,56 +3444,6 @@ Program::BlackBoxOp::EcdsaSecp256r1 serde::Deserializable BlackBoxOp::SchnorrVerify::bincodeSerialize() const { - auto serializer = serde::BincodeSerializer(); - serde::Serializable::serialize(*this, serializer); - return std::move(serializer).bytes(); - } - - inline BlackBoxOp::SchnorrVerify BlackBoxOp::SchnorrVerify::bincodeDeserialize(std::vector input) { - auto deserializer = serde::BincodeDeserializer(input); - auto value = serde::Deserializable::deserialize(deserializer); - if (deserializer.get_buffer_offset() < input.size()) { - throw serde::deserialization_error("Some input bytes were not read"); - } - return value; - } - -} // end of namespace Program - -template <> -template -void serde::Serializable::serialize(const Program::BlackBoxOp::SchnorrVerify &obj, Serializer &serializer) { - serde::Serializable::serialize(obj.public_key_x, serializer); - serde::Serializable::serialize(obj.public_key_y, serializer); - serde::Serializable::serialize(obj.message, serializer); - serde::Serializable::serialize(obj.signature, serializer); - serde::Serializable::serialize(obj.result, serializer); -} - -template <> -template -Program::BlackBoxOp::SchnorrVerify serde::Deserializable::deserialize(Deserializer &deserializer) { - Program::BlackBoxOp::SchnorrVerify obj; - obj.public_key_x = serde::Deserializable::deserialize(deserializer); - obj.public_key_y = serde::Deserializable::deserialize(deserializer); - obj.message = serde::Deserializable::deserialize(deserializer); - obj.signature = serde::Deserializable::deserialize(deserializer); - obj.result = serde::Deserializable::deserialize(deserializer); - return obj; -} - namespace Program { inline bool operator==(const BlackBoxOp::MultiScalarMul &lhs, const BlackBoxOp::MultiScalarMul &rhs) { diff --git a/acvm-repo/acir/src/circuit/black_box_functions.rs b/acvm-repo/acir/src/circuit/black_box_functions.rs index 25842c14dbc..700589d2040 100644 --- a/acvm-repo/acir/src/circuit/black_box_functions.rs +++ b/acvm-repo/acir/src/circuit/black_box_functions.rs @@ -51,29 +51,6 @@ pub enum BlackBoxFunc { /// (witness, 8), constrained to be the blake3 of the inputs. Blake3, - /// Verify a Schnorr signature over the embedded curve - /// - inputs are: - /// - Public key as 2 (witness, 254) - /// - signature as a vector of 64 bytes (witness, 8) - /// - message as a vector of (witness, 8) - /// - output: A witness representing the result of the signature - /// verification; 0 for failure and 1 for success. - /// - /// Since the scalar field of the embedded curve is NOT the ACIR field, the - /// `(r,s)` signature is represented as a 64 bytes array for the two field - /// elements. On the other hand, the public key coordinates are ACIR fields. - /// The proving system decides how the message is to be hashed. Barretenberg - /// uses Blake2s. - /// - /// Verifies a Schnorr signature over a curve which is "pairing friendly" - /// with the curve on which the ACIR circuit is defined. - /// - /// The exact curve which this signature uses will vary based on the curve - /// being used by ACIR. For example, the BN254 curve supports Schnorr - /// signatures over the [Grumpkin][grumpkin] curve. - /// - /// [grumpkin]: https://hackmd.io/@aztec-network/ByzgNxBfd#2-Grumpkin---A-curve-on-top-of-BN-254-for-SNARK-efficient-group-operations - SchnorrVerify, /// Verifies a ECDSA signature over the secp256k1 curve. /// - inputs: /// - x coordinate of public key as 32 bytes @@ -81,11 +58,6 @@ pub enum BlackBoxFunc { /// - the signature, as a 64 bytes array /// - the hash of the message, as a vector of bytes /// - output: 0 for failure and 1 for success - /// - /// Inputs and outputs are similar to SchnorrVerify, except that because we - /// use a different curve (secp256k1), the field elements involved in the - /// signature and the public key are defined as an array of 32 bytes. - /// Another difference is that we assume the message is already hashed. EcdsaSecp256k1, /// Verifies a ECDSA signature over the secp256r1 curve. @@ -196,7 +168,6 @@ impl BlackBoxFunc { pub fn name(&self) -> &'static str { match self { BlackBoxFunc::AES128Encrypt => "aes128_encrypt", - BlackBoxFunc::SchnorrVerify => "schnorr_verify", BlackBoxFunc::Blake2s => "blake2s", BlackBoxFunc::Blake3 => "blake3", BlackBoxFunc::EcdsaSecp256k1 => "ecdsa_secp256k1", @@ -222,7 +193,6 @@ impl BlackBoxFunc { pub fn lookup(op_name: &str) -> Option { match op_name { "aes128_encrypt" => Some(BlackBoxFunc::AES128Encrypt), - "schnorr_verify" => Some(BlackBoxFunc::SchnorrVerify), "blake2s" => Some(BlackBoxFunc::Blake2s), "blake3" => Some(BlackBoxFunc::Blake3), "ecdsa_secp256k1" => Some(BlackBoxFunc::EcdsaSecp256k1), diff --git a/acvm-repo/acir/src/circuit/mod.rs b/acvm-repo/acir/src/circuit/mod.rs index 88605d3bdab..4ff581bf17a 100644 --- a/acvm-repo/acir/src/circuit/mod.rs +++ b/acvm-repo/acir/src/circuit/mod.rs @@ -406,29 +406,12 @@ mod tests { Opcode::BlackBoxFuncCall(BlackBoxFuncCall::Keccakf1600 { inputs, outputs }) } - fn schnorr_verify_opcode() -> Opcode { - let public_key_x = FunctionInput::witness(Witness(1), FieldElement::max_num_bits()); - let public_key_y = FunctionInput::witness(Witness(2), FieldElement::max_num_bits()); - let signature: Box<[FunctionInput; 64]> = - Box::new(std::array::from_fn(|i| FunctionInput::witness(Witness(i as u32 + 3), 8))); - let message: Vec> = vec![FunctionInput::witness(Witness(67), 8)]; - let output = Witness(68); - - Opcode::BlackBoxFuncCall(BlackBoxFuncCall::SchnorrVerify { - public_key_x, - public_key_y, - signature, - message, - output, - }) - } - #[test] fn serialization_roundtrip() { let circuit = Circuit { current_witness_index: 5, expression_width: ExpressionWidth::Unbounded, - opcodes: vec![and_opcode::(), range_opcode(), schnorr_verify_opcode()], + opcodes: vec![and_opcode::(), range_opcode()], private_parameters: BTreeSet::new(), public_parameters: PublicInputs(BTreeSet::from_iter(vec![Witness(2), Witness(12)])), return_values: PublicInputs(BTreeSet::from_iter(vec![Witness(4), Witness(12)])), @@ -462,7 +445,6 @@ mod tests { range_opcode(), and_opcode(), keccakf1600_opcode(), - schnorr_verify_opcode(), ], private_parameters: BTreeSet::new(), public_parameters: PublicInputs(BTreeSet::from_iter(vec![Witness(2)])), 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 e756eedefbc..9cf31e94eb4 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 @@ -108,17 +108,6 @@ pub enum BlackBoxFuncCall { inputs: Vec>, outputs: Box<[Witness; 32]>, }, - SchnorrVerify { - public_key_x: FunctionInput, - public_key_y: FunctionInput, - #[serde( - serialize_with = "serialize_big_array", - deserialize_with = "deserialize_big_array_into_box" - )] - signature: Box<[FunctionInput; 64]>, - message: Vec>, - output: Witness, - }, EcdsaSecp256k1 { public_key_x: Box<[FunctionInput; 32]>, public_key_y: Box<[FunctionInput; 32]>, @@ -234,7 +223,6 @@ impl BlackBoxFuncCall { BlackBoxFuncCall::RANGE { .. } => BlackBoxFunc::RANGE, BlackBoxFuncCall::Blake2s { .. } => BlackBoxFunc::Blake2s, BlackBoxFuncCall::Blake3 { .. } => BlackBoxFunc::Blake3, - BlackBoxFuncCall::SchnorrVerify { .. } => BlackBoxFunc::SchnorrVerify, BlackBoxFuncCall::EcdsaSecp256k1 { .. } => BlackBoxFunc::EcdsaSecp256k1, BlackBoxFuncCall::EcdsaSecp256r1 { .. } => BlackBoxFunc::EcdsaSecp256r1, BlackBoxFuncCall::MultiScalarMul { .. } => BlackBoxFunc::MultiScalarMul, @@ -288,21 +276,6 @@ impl BlackBoxFuncCall { vec![input1[0], input1[1], input2[0], input2[1]] } BlackBoxFuncCall::RANGE { input } => vec![*input], - BlackBoxFuncCall::SchnorrVerify { - public_key_x, - public_key_y, - signature, - message, - .. - } => { - let mut inputs: Vec> = - Vec::with_capacity(2 + signature.len() + message.len()); - inputs.push(*public_key_x); - inputs.push(*public_key_y); - inputs.extend(signature.iter().copied()); - inputs.extend(message.iter().copied()); - inputs - } BlackBoxFuncCall::EcdsaSecp256k1 { public_key_x, public_key_y, @@ -372,7 +345,6 @@ impl BlackBoxFuncCall { BlackBoxFuncCall::AND { output, .. } | BlackBoxFuncCall::XOR { output, .. } - | BlackBoxFuncCall::SchnorrVerify { output, .. } | BlackBoxFuncCall::EcdsaSecp256k1 { output, .. } | BlackBoxFuncCall::EcdsaSecp256r1 { output, .. } => vec![*output], BlackBoxFuncCall::MultiScalarMul { outputs, .. } @@ -525,22 +497,6 @@ mod tests { Opcode::BlackBoxFuncCall(BlackBoxFuncCall::Keccakf1600 { inputs, outputs }) } - fn schnorr_verify_opcode() -> Opcode { - let public_key_x = FunctionInput::witness(Witness(1), FieldElement::max_num_bits()); - let public_key_y = FunctionInput::witness(Witness(2), FieldElement::max_num_bits()); - let signature: Box<[FunctionInput; 64]> = - Box::new(std::array::from_fn(|i| FunctionInput::witness(Witness(i as u32 + 3), 8))); - let message: Vec> = vec![FunctionInput::witness(Witness(67), 8)]; - let output = Witness(68); - - Opcode::BlackBoxFuncCall(BlackBoxFuncCall::SchnorrVerify { - public_key_x, - public_key_y, - signature, - message, - output, - }) - } #[test] fn keccakf1600_serialization_roundtrip() { @@ -549,12 +505,4 @@ mod tests { let recovered_opcode = bincode::deserialize(&buf).unwrap(); assert_eq!(opcode, recovered_opcode); } - - #[test] - fn schnorr_serialization_roundtrip() { - let opcode = schnorr_verify_opcode::(); - let buf = bincode::serialize(&opcode).unwrap(); - let recovered_opcode = bincode::deserialize(&buf).unwrap(); - assert_eq!(opcode, recovered_opcode); - } } diff --git a/acvm-repo/acir/tests/test_program_serialization.rs b/acvm-repo/acir/tests/test_program_serialization.rs index 002bad0e7f3..305d94abcee 100644 --- a/acvm-repo/acir/tests/test_program_serialization.rs +++ b/acvm-repo/acir/tests/test_program_serialization.rs @@ -93,67 +93,10 @@ fn multi_scalar_mul_circuit() { let bytes = Program::serialize_program(&program); let expected_serialization: Vec = vec![ - 31, 139, 8, 0, 0, 0, 0, 0, 0, 255, 93, 77, 9, 10, 0, 32, 8, 243, 236, 248, 255, 127, 35, - 163, 5, 35, 97, 184, 205, 169, 42, 183, 102, 65, 193, 21, 218, 73, 31, 44, 116, 35, 238, - 228, 189, 108, 208, 60, 193, 91, 161, 23, 6, 114, 73, 121, 195, 157, 32, 95, 232, 255, 191, - 203, 181, 1, 243, 231, 24, 106, 192, 0, 0, 0, - ]; - - assert_eq!(bytes, expected_serialization) -} - -#[test] -fn schnorr_verify_circuit() { - let public_key_x = FunctionInput::witness(Witness(1), FieldElement::max_num_bits()); - let public_key_y = FunctionInput::witness(Witness(2), FieldElement::max_num_bits()); - let signature: [FunctionInput; 64] = (3..(3 + 64)) - .map(|i| FunctionInput::witness(Witness(i), 8)) - .collect::>() - .try_into() - .unwrap(); - let message = - ((3 + 64)..(3 + 64 + 10)).map(|i| FunctionInput::witness(Witness(i), 8)).collect(); - let output = Witness(3 + 64 + 10); - let last_input = output.witness_index() - 1; - - let schnorr = Opcode::BlackBoxFuncCall(BlackBoxFuncCall::SchnorrVerify { - public_key_x, - public_key_y, - signature: Box::new(signature), - message, - output, - }); - - let circuit: Circuit = Circuit { - current_witness_index: 100, - opcodes: vec![schnorr], - private_parameters: BTreeSet::from_iter((1..=last_input).map(Witness)), - return_values: PublicInputs(BTreeSet::from([output])), - ..Circuit::default() - }; - let program = Program { functions: vec![circuit], unconstrained_functions: vec![] }; - - let bytes = Program::serialize_program(&program); - - let expected_serialization: Vec = vec![ - 31, 139, 8, 0, 0, 0, 0, 0, 0, 255, 77, 211, 103, 78, 2, 81, 24, 70, 225, 193, 130, 96, 239, - 189, 96, 239, 189, 35, 34, 34, 34, 82, 118, 193, 254, 151, 64, 224, 132, 111, 146, 67, 50, - 153, 39, 250, 3, 114, 239, 121, 51, 201, 240, 211, 29, 60, 153, 48, 239, 108, 188, 121, - 122, 241, 30, 145, 71, 7, 79, 46, 60, 38, 143, 203, 89, 121, 66, 206, 201, 121, 121, 82, - 158, 146, 167, 229, 25, 121, 86, 158, 147, 231, 229, 5, 121, 81, 94, 146, 151, 229, 21, - 121, 85, 94, 147, 215, 229, 13, 121, 83, 222, 146, 183, 229, 29, 121, 87, 222, 147, 11, - 242, 190, 124, 32, 31, 202, 71, 242, 177, 124, 34, 159, 202, 103, 242, 185, 124, 33, 95, - 202, 87, 242, 181, 124, 35, 223, 202, 119, 242, 189, 252, 32, 63, 202, 79, 242, 179, 252, - 34, 191, 202, 111, 242, 187, 92, 148, 63, 228, 146, 252, 41, 151, 229, 47, 185, 34, 127, - 203, 213, 48, 157, 38, 241, 183, 31, 253, 191, 38, 255, 202, 117, 249, 79, 110, 200, 255, - 114, 83, 110, 201, 237, 112, 39, 190, 191, 173, 223, 193, 54, 217, 36, 91, 100, 131, 108, - 47, 221, 92, 62, 126, 51, 155, 98, 75, 108, 136, 237, 176, 25, 182, 194, 70, 216, 6, 155, - 96, 11, 108, 128, 246, 105, 158, 214, 105, 156, 182, 105, 154, 150, 105, 152, 118, 105, - 182, 144, 12, 27, 165, 77, 154, 164, 69, 26, 164, 61, 154, 163, 53, 26, 163, 45, 154, 162, - 37, 26, 162, 29, 154, 161, 21, 26, 161, 13, 154, 160, 5, 26, 224, 238, 185, 115, 238, 154, - 59, 46, 198, 157, 150, 226, 14, 203, 113, 103, 149, 184, 163, 106, 220, 69, 45, 206, 190, - 30, 103, 221, 136, 179, 109, 198, 89, 166, 103, 150, 158, 91, 162, 243, 244, 167, 15, 14, - 161, 226, 6, 24, 5, 0, 0, + 31, 139, 8, 0, 0, 0, 0, 0, 0, 255, 93, 77, 9, 10, 0, 48, 8, 114, 107, 231, 255, 255, 59, + 86, 204, 64, 22, 136, 102, 89, 5, 175, 182, 163, 80, 7, 47, 135, 73, 31, 56, 228, 42, 218, + 196, 203, 221, 38, 243, 78, 61, 28, 147, 119, 65, 31, 146, 53, 230, 210, 135, 252, 255, + 179, 90, 23, 212, 196, 199, 187, 192, 0, 0, 0, ]; assert_eq!(bytes, expected_serialization) diff --git a/acvm-repo/acvm/src/pwg/blackbox/mod.rs b/acvm-repo/acvm/src/pwg/blackbox/mod.rs index c3b1627ba65..5137b18179b 100644 --- a/acvm-repo/acvm/src/pwg/blackbox/mod.rs +++ b/acvm-repo/acvm/src/pwg/blackbox/mod.rs @@ -27,10 +27,7 @@ use embedded_curve_ops::{embedded_curve_add, multi_scalar_mul}; use hash::{solve_generic_256_hash_opcode, solve_sha_256_permutation_opcode}; use logic::{and, xor}; pub(crate) use range::solve_range_opcode; -use signature::{ - ecdsa::{secp256k1_prehashed, secp256r1_prehashed}, - schnorr::schnorr_verify, -}; +use signature::ecdsa::{secp256k1_prehashed, secp256r1_prehashed}; /// Check if all of the inputs to the function have assignments /// @@ -103,21 +100,6 @@ pub(crate) fn solve( } Ok(()) } - BlackBoxFuncCall::SchnorrVerify { - public_key_x, - public_key_y, - signature, - message, - output, - } => schnorr_verify( - backend, - initial_witness, - *public_key_x, - *public_key_y, - signature.as_ref(), - message, - *output, - ), BlackBoxFuncCall::EcdsaSecp256k1 { public_key_x, public_key_y, diff --git a/acvm-repo/acvm/src/pwg/blackbox/signature/mod.rs b/acvm-repo/acvm/src/pwg/blackbox/signature/mod.rs index 0cfb96740b8..b36ff499c6a 100644 --- a/acvm-repo/acvm/src/pwg/blackbox/signature/mod.rs +++ b/acvm-repo/acvm/src/pwg/blackbox/signature/mod.rs @@ -1,2 +1 @@ pub(super) mod ecdsa; -pub(super) mod schnorr; diff --git a/acvm-repo/acvm/src/pwg/blackbox/signature/schnorr.rs b/acvm-repo/acvm/src/pwg/blackbox/signature/schnorr.rs deleted file mode 100644 index a856303d065..00000000000 --- a/acvm-repo/acvm/src/pwg/blackbox/signature/schnorr.rs +++ /dev/null @@ -1,36 +0,0 @@ -use crate::{ - pwg::{ - blackbox::utils::{to_u8_array, to_u8_vec}, - input_to_value, insert_value, OpcodeResolutionError, - }, - BlackBoxFunctionSolver, -}; -use acir::{ - circuit::opcodes::FunctionInput, - native_types::{Witness, WitnessMap}, - AcirField, -}; - -#[allow(clippy::too_many_arguments)] -pub(crate) fn schnorr_verify( - backend: &impl BlackBoxFunctionSolver, - initial_witness: &mut WitnessMap, - public_key_x: FunctionInput, - public_key_y: FunctionInput, - signature: &[FunctionInput; 64], - message: &[FunctionInput], - output: Witness, -) -> Result<(), OpcodeResolutionError> { - let public_key_x: &F = &input_to_value(initial_witness, public_key_x, false)?; - let public_key_y: &F = &input_to_value(initial_witness, public_key_y, false)?; - - let signature = to_u8_array(initial_witness, signature)?; - let message = to_u8_vec(initial_witness, message)?; - - let valid_signature = - backend.schnorr_verify(public_key_x, public_key_y, &signature, &message)?; - - insert_value(&output, F::from(valid_signature), initial_witness)?; - - Ok(()) -} diff --git a/acvm-repo/acvm_js/test/browser/execute_circuit.test.ts b/acvm-repo/acvm_js/test/browser/execute_circuit.test.ts index aaa82f8f1e5..4d8f0acbd38 100644 --- a/acvm-repo/acvm_js/test/browser/execute_circuit.test.ts +++ b/acvm-repo/acvm_js/test/browser/execute_circuit.test.ts @@ -85,16 +85,6 @@ it('successfully executes a MultiScalarMul opcode', async () => { expect(solvedWitness).to.be.deep.eq(expectedWitnessMap); }); -it('successfully executes a SchnorrVerify opcode', async () => { - const { bytecode, initialWitnessMap, expectedWitnessMap } = await import('../shared/schnorr_verify'); - - const solvedWitness: WitnessMap = await executeCircuit(bytecode, initialWitnessMap, () => { - throw Error('unexpected oracle'); - }); - - expect(solvedWitness).to.be.deep.eq(expectedWitnessMap); -}); - it('successfully executes a MemoryOp opcode', async () => { const { bytecode, initialWitnessMap, expectedWitnessMap } = await import('../shared/memory_op'); diff --git a/acvm-repo/acvm_js/test/node/execute_circuit.test.ts b/acvm-repo/acvm_js/test/node/execute_circuit.test.ts index 120ad0fa738..67f7de2129c 100644 --- a/acvm-repo/acvm_js/test/node/execute_circuit.test.ts +++ b/acvm-repo/acvm_js/test/node/execute_circuit.test.ts @@ -86,16 +86,6 @@ it('successfully executes a MultiScalarMul opcode', async () => { expect(solvedWitness).to.be.deep.eq(expectedWitnessMap); }); -it('successfully executes a SchnorrVerify opcode', async () => { - const { bytecode, initialWitnessMap, expectedWitnessMap } = await import('../shared/schnorr_verify'); - - const solvedWitness: WitnessMap = await executeCircuit(bytecode, initialWitnessMap, () => { - throw Error('unexpected oracle'); - }); - - expect(solvedWitness).to.be.deep.eq(expectedWitnessMap); -}); - it('successfully executes a MemoryOp opcode', async () => { const { bytecode, initialWitnessMap, expectedWitnessMap } = await import('../shared/memory_op'); diff --git a/acvm-repo/acvm_js/test/shared/multi_scalar_mul.ts b/acvm-repo/acvm_js/test/shared/multi_scalar_mul.ts index 3ec589dd0c8..fac77e4ee27 100644 --- a/acvm-repo/acvm_js/test/shared/multi_scalar_mul.ts +++ b/acvm-repo/acvm_js/test/shared/multi_scalar_mul.ts @@ -1,8 +1,8 @@ // See `multi_scalar_mul_circuit` integration test in `acir/tests/test_program_serialization.rs`. export const bytecode = Uint8Array.from([ - 31, 139, 8, 0, 0, 0, 0, 0, 0, 255, 93, 77, 9, 10, 0, 32, 8, 243, 236, 248, 255, 127, 35, 163, 5, 35, 97, 184, 205, - 169, 42, 183, 102, 65, 193, 21, 218, 73, 31, 44, 116, 35, 238, 228, 189, 108, 208, 60, 193, 91, 161, 23, 6, 114, 73, - 121, 195, 157, 32, 95, 232, 255, 191, 203, 181, 1, 243, 231, 24, 106, 192, 0, 0, 0, + 31, 139, 8, 0, 0, 0, 0, 0, 0, 255, 93, 77, 9, 10, 0, 48, 8, 114, 107, 231, 255, 255, 59, 86, 204, 64, 22, 136, 102, + 89, 5, 175, 182, 163, 80, 7, 47, 135, 73, 31, 56, 228, 42, 218, 196, 203, 221, 38, 243, 78, 61, 28, 147, 119, 65, 31, + 146, 53, 230, 210, 135, 252, 255, 179, 90, 23, 212, 196, 199, 187, 192, 0, 0, 0, ]); export const initialWitnessMap = new Map([ [1, '0x0000000000000000000000000000000000000000000000000000000000000001'], diff --git a/acvm-repo/acvm_js/test/shared/schnorr_verify.ts b/acvm-repo/acvm_js/test/shared/schnorr_verify.ts deleted file mode 100644 index d2df63a8ddb..00000000000 --- a/acvm-repo/acvm_js/test/shared/schnorr_verify.ts +++ /dev/null @@ -1,101 +0,0 @@ -// See `schnorr_verify_circuit` integration test in `acir/tests/test_program_serialization.rs`. -export const bytecode = Uint8Array.from([ - 31, 139, 8, 0, 0, 0, 0, 0, 0, 255, 77, 211, 103, 78, 2, 81, 24, 70, 225, 193, 130, 96, 239, 189, 96, 239, 189, 35, 34, - 34, 34, 82, 118, 193, 254, 151, 64, 224, 132, 111, 146, 67, 50, 153, 39, 250, 3, 114, 239, 121, 51, 201, 240, 211, 29, - 60, 153, 48, 239, 108, 188, 121, 122, 241, 30, 145, 71, 7, 79, 46, 60, 38, 143, 203, 89, 121, 66, 206, 201, 121, 121, - 82, 158, 146, 167, 229, 25, 121, 86, 158, 147, 231, 229, 5, 121, 81, 94, 146, 151, 229, 21, 121, 85, 94, 147, 215, - 229, 13, 121, 83, 222, 146, 183, 229, 29, 121, 87, 222, 147, 11, 242, 190, 124, 32, 31, 202, 71, 242, 177, 124, 34, - 159, 202, 103, 242, 185, 124, 33, 95, 202, 87, 242, 181, 124, 35, 223, 202, 119, 242, 189, 252, 32, 63, 202, 79, 242, - 179, 252, 34, 191, 202, 111, 242, 187, 92, 148, 63, 228, 146, 252, 41, 151, 229, 47, 185, 34, 127, 203, 213, 48, 157, - 38, 241, 183, 31, 253, 191, 38, 255, 202, 117, 249, 79, 110, 200, 255, 114, 83, 110, 201, 237, 112, 39, 190, 191, 173, - 223, 193, 54, 217, 36, 91, 100, 131, 108, 47, 221, 92, 62, 126, 51, 155, 98, 75, 108, 136, 237, 176, 25, 182, 194, 70, - 216, 6, 155, 96, 11, 108, 128, 246, 105, 158, 214, 105, 156, 182, 105, 154, 150, 105, 152, 118, 105, 182, 144, 12, 27, - 165, 77, 154, 164, 69, 26, 164, 61, 154, 163, 53, 26, 163, 45, 154, 162, 37, 26, 162, 29, 154, 161, 21, 26, 161, 13, - 154, 160, 5, 26, 224, 238, 185, 115, 238, 154, 59, 46, 198, 157, 150, 226, 14, 203, 113, 103, 149, 184, 163, 106, 220, - 69, 45, 206, 190, 30, 103, 221, 136, 179, 109, 198, 89, 166, 103, 150, 158, 91, 162, 243, 244, 167, 15, 14, 161, 226, - 6, 24, 5, 0, 0, -]); - -export const initialWitnessMap = new Map([ - [1, '0x04b260954662e97f00cab9adb773a259097f7a274b83b113532bce27fa3fb96a'], - [2, '0x2fd51571db6c08666b0edfbfbc57d432068bccd0110a39b166ab243da0037197'], - [3, '0x000000000000000000000000000000000000000000000000000000000000002e'], - [4, '0x00000000000000000000000000000000000000000000000000000000000000ce'], - [5, '0x0000000000000000000000000000000000000000000000000000000000000052'], - [6, '0x00000000000000000000000000000000000000000000000000000000000000aa'], - [7, '0x0000000000000000000000000000000000000000000000000000000000000087'], - [8, '0x000000000000000000000000000000000000000000000000000000000000002a'], - [9, '0x0000000000000000000000000000000000000000000000000000000000000049'], - [10, '0x000000000000000000000000000000000000000000000000000000000000009d'], - [11, '0x0000000000000000000000000000000000000000000000000000000000000050'], - [12, '0x000000000000000000000000000000000000000000000000000000000000007c'], - [13, '0x000000000000000000000000000000000000000000000000000000000000009a'], - [14, '0x00000000000000000000000000000000000000000000000000000000000000aa'], - [15, '0x00000000000000000000000000000000000000000000000000000000000000df'], - [16, '0x0000000000000000000000000000000000000000000000000000000000000023'], - [17, '0x0000000000000000000000000000000000000000000000000000000000000034'], - [18, '0x0000000000000000000000000000000000000000000000000000000000000010'], - [19, '0x000000000000000000000000000000000000000000000000000000000000008a'], - [20, '0x0000000000000000000000000000000000000000000000000000000000000047'], - [21, '0x0000000000000000000000000000000000000000000000000000000000000063'], - [22, '0x00000000000000000000000000000000000000000000000000000000000000e8'], - [23, '0x0000000000000000000000000000000000000000000000000000000000000037'], - [24, '0x0000000000000000000000000000000000000000000000000000000000000054'], - [25, '0x0000000000000000000000000000000000000000000000000000000000000096'], - [26, '0x000000000000000000000000000000000000000000000000000000000000003e'], - [27, '0x00000000000000000000000000000000000000000000000000000000000000d5'], - [28, '0x00000000000000000000000000000000000000000000000000000000000000ae'], - [29, '0x0000000000000000000000000000000000000000000000000000000000000024'], - [30, '0x000000000000000000000000000000000000000000000000000000000000002d'], - [31, '0x0000000000000000000000000000000000000000000000000000000000000020'], - [32, '0x0000000000000000000000000000000000000000000000000000000000000080'], - [33, '0x000000000000000000000000000000000000000000000000000000000000004d'], - [34, '0x0000000000000000000000000000000000000000000000000000000000000047'], - [35, '0x00000000000000000000000000000000000000000000000000000000000000a5'], - [36, '0x00000000000000000000000000000000000000000000000000000000000000bb'], - [37, '0x00000000000000000000000000000000000000000000000000000000000000f6'], - [38, '0x00000000000000000000000000000000000000000000000000000000000000c3'], - [39, '0x000000000000000000000000000000000000000000000000000000000000000b'], - [40, '0x000000000000000000000000000000000000000000000000000000000000003b'], - [41, '0x0000000000000000000000000000000000000000000000000000000000000065'], - [42, '0x00000000000000000000000000000000000000000000000000000000000000c9'], - [43, '0x0000000000000000000000000000000000000000000000000000000000000001'], - [44, '0x0000000000000000000000000000000000000000000000000000000000000085'], - [45, '0x0000000000000000000000000000000000000000000000000000000000000006'], - [46, '0x000000000000000000000000000000000000000000000000000000000000009e'], - [47, '0x000000000000000000000000000000000000000000000000000000000000002f'], - [48, '0x0000000000000000000000000000000000000000000000000000000000000010'], - [49, '0x00000000000000000000000000000000000000000000000000000000000000e6'], - [50, '0x0000000000000000000000000000000000000000000000000000000000000030'], - [51, '0x000000000000000000000000000000000000000000000000000000000000004a'], - [52, '0x0000000000000000000000000000000000000000000000000000000000000018'], - [53, '0x000000000000000000000000000000000000000000000000000000000000007c'], - [54, '0x00000000000000000000000000000000000000000000000000000000000000d0'], - [55, '0x00000000000000000000000000000000000000000000000000000000000000ab'], - [56, '0x0000000000000000000000000000000000000000000000000000000000000031'], - [57, '0x00000000000000000000000000000000000000000000000000000000000000d5'], - [58, '0x0000000000000000000000000000000000000000000000000000000000000063'], - [59, '0x0000000000000000000000000000000000000000000000000000000000000084'], - [60, '0x00000000000000000000000000000000000000000000000000000000000000a3'], - [61, '0x00000000000000000000000000000000000000000000000000000000000000a6'], - [62, '0x00000000000000000000000000000000000000000000000000000000000000d5'], - [63, '0x0000000000000000000000000000000000000000000000000000000000000091'], - [64, '0x000000000000000000000000000000000000000000000000000000000000000d'], - [65, '0x000000000000000000000000000000000000000000000000000000000000009c'], - [66, '0x00000000000000000000000000000000000000000000000000000000000000f9'], - [67, '0x0000000000000000000000000000000000000000000000000000000000000000'], - [68, '0x0000000000000000000000000000000000000000000000000000000000000001'], - [69, '0x0000000000000000000000000000000000000000000000000000000000000002'], - [70, '0x0000000000000000000000000000000000000000000000000000000000000003'], - [71, '0x0000000000000000000000000000000000000000000000000000000000000004'], - [72, '0x0000000000000000000000000000000000000000000000000000000000000005'], - [73, '0x0000000000000000000000000000000000000000000000000000000000000006'], - [74, '0x0000000000000000000000000000000000000000000000000000000000000007'], - [75, '0x0000000000000000000000000000000000000000000000000000000000000008'], - [76, '0x0000000000000000000000000000000000000000000000000000000000000009'], -]); - -export const expectedWitnessMap = new Map(initialWitnessMap).set( - 77, - '0x0000000000000000000000000000000000000000000000000000000000000001', -); diff --git a/acvm-repo/blackbox_solver/src/curve_specific_solver.rs b/acvm-repo/blackbox_solver/src/curve_specific_solver.rs index 869017f52ee..b8fc3f47033 100644 --- a/acvm-repo/blackbox_solver/src/curve_specific_solver.rs +++ b/acvm-repo/blackbox_solver/src/curve_specific_solver.rs @@ -7,13 +7,6 @@ use crate::BlackBoxResolutionError; /// /// Returns an [`BlackBoxResolutionError`] if the backend does not support the given [`acir::BlackBoxFunc`]. pub trait BlackBoxFunctionSolver { - fn schnorr_verify( - &self, - public_key_x: &F, - public_key_y: &F, - signature: &[u8; 64], - message: &[u8], - ) -> Result; fn multi_scalar_mul( &self, points: &[F], @@ -48,15 +41,6 @@ impl StubbedBlackBoxSolver { } impl BlackBoxFunctionSolver for StubbedBlackBoxSolver { - fn schnorr_verify( - &self, - _public_key_x: &F, - _public_key_y: &F, - _signature: &[u8; 64], - _message: &[u8], - ) -> Result { - Err(Self::fail(BlackBoxFunc::SchnorrVerify)) - } fn multi_scalar_mul( &self, _points: &[F], diff --git a/acvm-repo/bn254_blackbox_solver/benches/criterion.rs b/acvm-repo/bn254_blackbox_solver/benches/criterion.rs index e7917fa1adc..fc566b70a26 100644 --- a/acvm-repo/bn254_blackbox_solver/benches/criterion.rs +++ b/acvm-repo/bn254_blackbox_solver/benches/criterion.rs @@ -2,8 +2,7 @@ use criterion::{criterion_group, criterion_main, Criterion}; use std::{hint::black_box, time::Duration}; use acir::{AcirField, FieldElement}; -use acvm_blackbox_solver::BlackBoxFunctionSolver; -use bn254_blackbox_solver::{poseidon2_permutation, Bn254BlackBoxSolver}; +use bn254_blackbox_solver::poseidon2_permutation; use pprof::criterion::{Output, PProfProfiler}; @@ -13,40 +12,10 @@ fn bench_poseidon2(c: &mut Criterion) { c.bench_function("poseidon2", |b| b.iter(|| poseidon2_permutation(black_box(&inputs), 4))); } -fn bench_schnorr_verify(c: &mut Criterion) { - let pub_key_x = FieldElement::from_hex( - "0x04b260954662e97f00cab9adb773a259097f7a274b83b113532bce27fa3fb96a", - ) - .unwrap(); - let pub_key_y = FieldElement::from_hex( - "0x2fd51571db6c08666b0edfbfbc57d432068bccd0110a39b166ab243da0037197", - ) - .unwrap(); - let sig_bytes: [u8; 64] = [ - 1, 13, 119, 112, 212, 39, 233, 41, 84, 235, 255, 93, 245, 172, 186, 83, 157, 253, 76, 77, - 33, 128, 178, 15, 214, 67, 105, 107, 177, 234, 77, 48, 27, 237, 155, 84, 39, 84, 247, 27, - 22, 8, 176, 230, 24, 115, 145, 220, 254, 122, 135, 179, 171, 4, 214, 202, 64, 199, 19, 84, - 239, 138, 124, 12, - ]; - - let message: &[u8] = &[0, 1, 2, 3, 4, 5, 6, 7, 8, 9]; - - c.bench_function("schnorr_verify", |b| { - b.iter(|| { - Bn254BlackBoxSolver.schnorr_verify( - black_box(&pub_key_x), - black_box(&pub_key_y), - black_box(&sig_bytes), - black_box(message), - ) - }) - }); -} - criterion_group!( name = benches; config = Criterion::default().sample_size(40).measurement_time(Duration::from_secs(20)).with_profiler(PProfProfiler::new(100, Output::Flamegraph(None))); - targets = bench_poseidon2, bench_schnorr_verify + targets = bench_poseidon2 ); criterion_main!(benches); diff --git a/acvm-repo/bn254_blackbox_solver/src/embedded_curve_ops.rs b/acvm-repo/bn254_blackbox_solver/src/embedded_curve_ops.rs index a02711fda1e..e599fd25593 100644 --- a/acvm-repo/bn254_blackbox_solver/src/embedded_curve_ops.rs +++ b/acvm-repo/bn254_blackbox_solver/src/embedded_curve_ops.rs @@ -1,6 +1,5 @@ // TODO(https://github.com/noir-lang/noir/issues/4932): rename this file to something more generic use ark_ec::AffineRepr; -use ark_ff::MontConfig; use num_bigint::BigUint; use crate::FieldElement; @@ -46,15 +45,15 @@ pub fn multi_scalar_mul( let mut bytes = scalar_high.to_be_bytes().to_vec(); bytes.extend_from_slice(&scalar_low.to_be_bytes()); - // Check if this is smaller than the grumpkin modulus let grumpkin_integer = BigUint::from_bytes_be(&bytes); - if grumpkin_integer >= grumpkin::FrConfig::MODULUS.into() { - return Err(BlackBoxResolutionError::Failed( - BlackBoxFunc::MultiScalarMul, - format!("{} is not a valid grumpkin scalar", grumpkin_integer.to_str_radix(16)), - )); - } + // Check if this is smaller than the grumpkin modulus + // if grumpkin_integer >= grumpkin::FrConfig::MODULUS.into() { + // return Err(BlackBoxResolutionError::Failed( + // BlackBoxFunc::MultiScalarMul, + // format!("{} is not a valid grumpkin scalar", grumpkin_integer.to_str_radix(16)), + // )); + // } let iteration_output_point = grumpkin::SWAffine::from(point.mul_bigint(grumpkin_integer.to_u64_digits())); @@ -120,8 +119,6 @@ fn create_point( mod tests { use super::*; - use ark_ff::BigInteger; - fn get_generator() -> [FieldElement; 3] { let generator = grumpkin::SWAffine::generator(); let generator_x = FieldElement::from_repr(*generator.x().unwrap()); @@ -175,23 +172,23 @@ mod tests { assert_eq!(res, expected_error); } - #[test] - fn rejects_grumpkin_modulus() { - let x = grumpkin::FrConfig::MODULUS.to_bytes_be(); + // #[test] + // fn rejects_grumpkin_modulus() { + // let x = grumpkin::FrConfig::MODULUS.to_bytes_be(); - let low = FieldElement::from_be_bytes_reduce(&x[16..32]); - let high = FieldElement::from_be_bytes_reduce(&x[0..16]); + // let low = FieldElement::from_be_bytes_reduce(&x[16..32]); + // let high = FieldElement::from_be_bytes_reduce(&x[0..16]); - let res = multi_scalar_mul(&get_generator(), &[low], &[high]); + // let res = multi_scalar_mul(&get_generator(), &[low], &[high]); - assert_eq!( - res, - Err(BlackBoxResolutionError::Failed( - BlackBoxFunc::MultiScalarMul, - "30644e72e131a029b85045b68181585d97816a916871ca8d3c208c16d87cfd47 is not a valid grumpkin scalar".into(), - )) - ); - } + // assert_eq!( + // res, + // Err(BlackBoxResolutionError::Failed( + // BlackBoxFunc::MultiScalarMul, + // "30644e72e131a029b85045b68181585d97816a916871ca8d3c208c16d87cfd47 is not a valid grumpkin scalar".into(), + // )) + // ); + // } #[test] fn rejects_invalid_point() { diff --git a/acvm-repo/bn254_blackbox_solver/src/lib.rs b/acvm-repo/bn254_blackbox_solver/src/lib.rs index d74c17a52b5..f738a375ab1 100644 --- a/acvm-repo/bn254_blackbox_solver/src/lib.rs +++ b/acvm-repo/bn254_blackbox_solver/src/lib.rs @@ -6,9 +6,7 @@ use acvm_blackbox_solver::{BlackBoxFunctionSolver, BlackBoxResolutionError}; mod embedded_curve_ops; mod generator; -mod pedersen; mod poseidon2; -mod schnorr; pub use embedded_curve_ops::{embedded_curve_add, multi_scalar_mul}; pub use generator::generators::derive_generators; @@ -25,24 +23,6 @@ type FieldElement = acir::acir_field::GenericFieldElement; pub struct Bn254BlackBoxSolver; impl BlackBoxFunctionSolver for Bn254BlackBoxSolver { - fn schnorr_verify( - &self, - public_key_x: &FieldElement, - public_key_y: &FieldElement, - signature: &[u8; 64], - message: &[u8], - ) -> Result { - let sig_s: [u8; 32] = signature[0..32].try_into().unwrap(); - let sig_e: [u8; 32] = signature[32..64].try_into().unwrap(); - Ok(schnorr::verify_signature( - public_key_x.into_repr(), - public_key_y.into_repr(), - sig_s, - sig_e, - message, - )) - } - fn multi_scalar_mul( &self, points: &[FieldElement], diff --git a/acvm-repo/bn254_blackbox_solver/src/pedersen/commitment.rs b/acvm-repo/bn254_blackbox_solver/src/pedersen/commitment.rs deleted file mode 100644 index 03f03fcf5ab..00000000000 --- a/acvm-repo/bn254_blackbox_solver/src/pedersen/commitment.rs +++ /dev/null @@ -1,77 +0,0 @@ -// Taken from: https://github.com/laudiacay/barustenberg/blob/df6bc6f095fe7f288bf6a12e7317fd8eb33d68ae/barustenberg/src/crypto/pedersen/pederson.rs - -use ark_ec::{short_weierstrass::Affine, AffineRepr, CurveGroup}; -use ark_ff::{MontConfig, PrimeField}; -use grumpkin::{Fq, FqConfig, Fr, FrConfig, GrumpkinParameters}; - -use crate::generator::generators::{derive_generators, DEFAULT_DOMAIN_SEPARATOR}; - -/// Given a vector of fields, generate a pedersen commitment using the indexed generators. -pub(crate) fn commit_native_with_index( - inputs: &[Fq], - starting_index: u32, -) -> Affine { - let generators = - derive_generators(DEFAULT_DOMAIN_SEPARATOR, inputs.len() as u32, starting_index); - - // As |F_r| > |F_q|, we can safely convert any `F_q` into an `F_r` uniquely. - assert!(FrConfig::MODULUS > FqConfig::MODULUS); - - inputs.iter().enumerate().fold(Affine::zero(), |mut acc, (i, input)| { - acc = (acc + (generators[i] * Fr::from_bigint(input.into_bigint()).unwrap()).into_affine()) - .into_affine(); - acc - }) -} - -#[cfg(test)] -mod test { - - use acir::AcirField; - use ark_ec::short_weierstrass::Affine; - use ark_std::{One, Zero}; - use grumpkin::Fq; - - use crate::pedersen::commitment::commit_native_with_index; - use crate::FieldElement; - - #[test] - fn commitment() { - // https://github.com/AztecProtocol/aztec-packages/blob/72931bdb8202c34042cdfb8cee2ef44b75939879/barretenberg/cpp/src/barretenberg/crypto/pedersen_commitment/pedersen.test.cpp#L10-L18 - let res = commit_native_with_index(&[Fq::one(), Fq::one()], 0); - let expected = Affine::new( - FieldElement::from_hex( - "0x2f7a8f9a6c96926682205fb73ee43215bf13523c19d7afe36f12760266cdfe15", - ) - .unwrap() - .into_repr(), - FieldElement::from_hex( - "0x01916b316adbbf0e10e39b18c1d24b33ec84b46daddf72f43878bcc92b6057e6", - ) - .unwrap() - .into_repr(), - ); - - assert_eq!(res, expected); - } - - #[test] - fn commitment_with_zero() { - // https://github.com/AztecProtocol/aztec-packages/blob/72931bdb8202c34042cdfb8cee2ef44b75939879/barretenberg/cpp/src/barretenberg/crypto/pedersen_commitment/pedersen.test.cpp#L20-L29 - let res = commit_native_with_index(&[Fq::zero(), Fq::one()], 0); - let expected = Affine::new( - FieldElement::from_hex( - "0x054aa86a73cb8a34525e5bbed6e43ba1198e860f5f3950268f71df4591bde402", - ) - .unwrap() - .into_repr(), - FieldElement::from_hex( - "0x209dcfbf2cfb57f9f6046f44d71ac6faf87254afc7407c04eb621a6287cac126", - ) - .unwrap() - .into_repr(), - ); - - assert_eq!(res, expected); - } -} diff --git a/acvm-repo/bn254_blackbox_solver/src/pedersen/hash.rs b/acvm-repo/bn254_blackbox_solver/src/pedersen/hash.rs deleted file mode 100644 index 152526a9943..00000000000 --- a/acvm-repo/bn254_blackbox_solver/src/pedersen/hash.rs +++ /dev/null @@ -1,69 +0,0 @@ -// Taken from: https://github.com/laudiacay/barustenberg/blob/df6bc6f095fe7f288bf6a12e7317fd8eb33d68ae/barustenberg/src/crypto/pedersen/pederson_hash.rs - -use std::sync::OnceLock; - -use ark_ec::{short_weierstrass::Affine, CurveConfig, CurveGroup}; -use grumpkin::GrumpkinParameters; - -use crate::generator::generators::derive_generators; - -use super::commitment::commit_native_with_index; - -/// Given a vector of fields, generate a pedersen hash using the indexed generators. -pub(crate) fn hash_with_index( - inputs: &[grumpkin::Fq], - starting_index: u32, -) -> ::BaseField { - let length_as_scalar: ::ScalarField = - (inputs.len() as u64).into(); - let length_prefix = *length_generator() * length_as_scalar; - let result = length_prefix + commit_native_with_index(inputs, starting_index); - result.into_affine().x -} - -fn length_generator() -> &'static Affine { - static INSTANCE: OnceLock> = OnceLock::new(); - INSTANCE.get_or_init(|| derive_generators("pedersen_hash_length".as_bytes(), 1, 0)[0]) -} - -#[cfg(test)] -pub(crate) mod test { - - use super::*; - use crate::FieldElement; - - use acir::AcirField; - use ark_std::One; - use grumpkin::Fq; - - //reference: https://github.com/AztecProtocol/barretenberg/blob/master/cpp/src/barretenberg/crypto/pedersen_hash/pedersen.test.cpp - #[test] - fn hash_one() { - // https://github.com/AztecProtocol/aztec-packages/blob/72931bdb8202c34042cdfb8cee2ef44b75939879/barretenberg/cpp/src/barretenberg/crypto/pedersen_hash/pedersen.test.cpp#L21-L26 - let res = hash_with_index(&[Fq::one(), Fq::one()], 0); - - assert_eq!( - res, - FieldElement::from_hex( - "0x07ebfbf4df29888c6cd6dca13d4bb9d1a923013ddbbcbdc3378ab8845463297b", - ) - .unwrap() - .into_repr(), - ); - } - - #[test] - fn test_hash_with_index() { - // https://github.com/AztecProtocol/aztec-packages/blob/72931bdb8202c34042cdfb8cee2ef44b75939879/barretenberg/cpp/src/barretenberg/crypto/pedersen_hash/pedersen.test.cpp#L28-L33 - let res = hash_with_index(&[Fq::one(), Fq::one()], 5); - - assert_eq!( - res, - FieldElement::from_hex( - "0x1c446df60816b897cda124524e6b03f36df0cec333fad87617aab70d7861daa6", - ) - .unwrap() - .into_repr(), - ); - } -} diff --git a/acvm-repo/bn254_blackbox_solver/src/pedersen/mod.rs b/acvm-repo/bn254_blackbox_solver/src/pedersen/mod.rs deleted file mode 100644 index c3c4ed56450..00000000000 --- a/acvm-repo/bn254_blackbox_solver/src/pedersen/mod.rs +++ /dev/null @@ -1,2 +0,0 @@ -pub(crate) mod commitment; -pub(crate) mod hash; diff --git a/acvm-repo/bn254_blackbox_solver/src/schnorr/mod.rs b/acvm-repo/bn254_blackbox_solver/src/schnorr/mod.rs deleted file mode 100644 index 8e3a40803f8..00000000000 --- a/acvm-repo/bn254_blackbox_solver/src/schnorr/mod.rs +++ /dev/null @@ -1,147 +0,0 @@ -use acvm_blackbox_solver::blake2s; -use ark_ec::{ - short_weierstrass::{Affine, SWCurveConfig}, - AffineRepr, CurveConfig, CurveGroup, -}; -use ark_ff::{BigInteger, PrimeField, Zero}; -use grumpkin::{Fq, GrumpkinParameters}; - -pub(crate) fn verify_signature( - pub_key_x: Fq, - pub_key_y: Fq, - sig_s_bytes: [u8; 32], - sig_e_bytes: [u8; 32], - message: &[u8], -) -> bool { - let pub_key = Affine::::new_unchecked(pub_key_x, pub_key_y); - - if !pub_key.is_on_curve() - || !pub_key.is_in_correct_subgroup_assuming_on_curve() - || pub_key.is_zero() - { - return false; - } - - let sig_s = - ::ScalarField::from_be_bytes_mod_order(&sig_s_bytes); - let sig_e = - ::ScalarField::from_be_bytes_mod_order(&sig_e_bytes); - - if sig_s.is_zero() || sig_e.is_zero() { - return false; - } - - // R = g^{sig.s} • pub^{sig.e} - let r = GrumpkinParameters::GENERATOR * sig_s + pub_key * sig_e; - if r.is_zero() { - // this result implies k == 0, which would be catastrophic for the prover. - // it is a cheap check that ensures this doesn't happen. - return false; - } - - // compare the _hashes_ rather than field elements modulo r - // e = H(pedersen(r, pk.x, pk.y), m), where r = R.x - let target_e_bytes = schnorr_generate_challenge(message, pub_key_x, pub_key_y, r.into_affine()); - - sig_e_bytes == target_e_bytes -} - -fn schnorr_generate_challenge( - message: &[u8], - pub_key_x: Fq, - pub_key_y: Fq, - r: Affine, -) -> [u8; 32] { - // create challenge message pedersen_commitment(R.x, pubkey) - - let r_x = *r.x().expect("r has been checked to be non-zero"); - let pedersen_hash = crate::pedersen::hash::hash_with_index(&[r_x, pub_key_x, pub_key_y], 0); - - let mut hash_input: Vec = pedersen_hash.into_bigint().to_bytes_be(); - hash_input.extend(message); - - blake2s(&hash_input).unwrap() -} - -#[cfg(test)] -mod schnorr_tests { - use acir::AcirField; - - use super::verify_signature; - use crate::FieldElement; - - #[test] - fn verifies_valid_signature() { - let pub_key_x: grumpkin::Fq = FieldElement::from_hex( - "0x04b260954662e97f00cab9adb773a259097f7a274b83b113532bce27fa3fb96a", - ) - .unwrap() - .into_repr(); - let pub_key_y: grumpkin::Fq = FieldElement::from_hex( - "0x2fd51571db6c08666b0edfbfbc57d432068bccd0110a39b166ab243da0037197", - ) - .unwrap() - .into_repr(); - let sig_s_bytes: [u8; 32] = [ - 1, 13, 119, 112, 212, 39, 233, 41, 84, 235, 255, 93, 245, 172, 186, 83, 157, 253, 76, - 77, 33, 128, 178, 15, 214, 67, 105, 107, 177, 234, 77, 48, - ]; - let sig_e_bytes: [u8; 32] = [ - 27, 237, 155, 84, 39, 84, 247, 27, 22, 8, 176, 230, 24, 115, 145, 220, 254, 122, 135, - 179, 171, 4, 214, 202, 64, 199, 19, 84, 239, 138, 124, 12, - ]; - let message: &[u8] = &[0, 1, 2, 3, 4, 5, 6, 7, 8, 9]; - - assert!(verify_signature(pub_key_x, pub_key_y, sig_s_bytes, sig_e_bytes, message)); - } - - #[test] - fn rejects_zero_e() { - let pub_key_x: grumpkin::Fq = FieldElement::from_hex( - "0x04b260954662e97f00cab9adb773a259097f7a274b83b113532bce27fa3fb96a", - ) - .unwrap() - .into_repr(); - let pub_key_y: grumpkin::Fq = FieldElement::from_hex( - "0x2fd51571db6c08666b0edfbfbc57d432068bccd0110a39b166ab243da0037197", - ) - .unwrap() - .into_repr(); - let sig_s_bytes: [u8; 32] = [ - 1, 13, 119, 112, 212, 39, 233, 41, 84, 235, 255, 93, 245, 172, 186, 83, 157, 253, 76, - 77, 33, 128, 178, 15, 214, 67, 105, 107, 177, 234, 77, 48, - ]; - let sig_e_bytes: [u8; 32] = [ - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, - 0, 0, 0, - ]; - let message: &[u8] = &[0, 1, 2, 3, 4, 5, 6, 7, 8, 9]; - - assert!(!verify_signature(pub_key_x, pub_key_y, sig_s_bytes, sig_e_bytes, message)); - } - - #[test] - fn rejects_zero_s() { - let pub_key_x: grumpkin::Fq = FieldElement::from_hex( - "0x04b260954662e97f00cab9adb773a259097f7a274b83b113532bce27fa3fb96a", - ) - .unwrap() - .into_repr(); - let pub_key_y: grumpkin::Fq = FieldElement::from_hex( - "0x2fd51571db6c08666b0edfbfbc57d432068bccd0110a39b166ab243da0037197", - ) - .unwrap() - .into_repr(); - let sig_s_bytes: [u8; 32] = [ - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, - 0, 0, 0, - ]; - let sig_e_bytes: [u8; 32] = [ - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, - 0, 0, 0, - ]; - let message: &[u8] = &[0, 1, 2, 3, 4, 5, 6, 7, 8, 9]; - - assert!(!verify_signature(pub_key_x, pub_key_y, sig_s_bytes, sig_e_bytes, message)); - } -} diff --git a/acvm-repo/brillig/src/black_box.rs b/acvm-repo/brillig/src/black_box.rs index cbb268c0a50..f185b36e6c8 100644 --- a/acvm-repo/brillig/src/black_box.rs +++ b/acvm-repo/brillig/src/black_box.rs @@ -43,14 +43,7 @@ pub enum BlackBoxOp { signature: HeapArray, result: MemoryAddress, }, - /// Verifies a Schnorr signature over a curve which is "pairing friendly" with the curve on which the Brillig bytecode is defined. - SchnorrVerify { - public_key_x: MemoryAddress, - public_key_y: MemoryAddress, - message: HeapVector, - signature: HeapVector, - result: MemoryAddress, - }, + /// Performs multi scalar multiplication over the embedded curve. MultiScalarMul { points: HeapVector, diff --git a/acvm-repo/brillig_vm/src/black_box.rs b/acvm-repo/brillig_vm/src/black_box.rs index 19e2dd7553d..79aea2adf76 100644 --- a/acvm-repo/brillig_vm/src/black_box.rs +++ b/acvm-repo/brillig_vm/src/black_box.rs @@ -141,17 +141,6 @@ pub(crate) fn evaluate_black_box memory.write(*result_address, result.into()); Ok(()) } - BlackBoxOp::SchnorrVerify { public_key_x, public_key_y, message, signature, result } => { - let public_key_x = *memory.read(*public_key_x).extract_field().unwrap(); - let public_key_y = *memory.read(*public_key_y).extract_field().unwrap(); - let message: Vec = to_u8_vec(read_heap_vector(memory, message)); - let signature: [u8; 64] = - to_u8_vec(read_heap_vector(memory, signature)).try_into().unwrap(); - let verified = - solver.schnorr_verify(&public_key_x, &public_key_y, &signature, &message)?; - memory.write(*result, verified.into()); - Ok(()) - } BlackBoxOp::MultiScalarMul { points, scalars, outputs: result } => { let points: Vec = read_heap_vector(memory, points) .iter() @@ -362,7 +351,6 @@ fn black_box_function_from_op(op: &BlackBoxOp) -> BlackBoxFunc { BlackBoxOp::Keccakf1600 { .. } => BlackBoxFunc::Keccakf1600, BlackBoxOp::EcdsaSecp256k1 { .. } => BlackBoxFunc::EcdsaSecp256k1, BlackBoxOp::EcdsaSecp256r1 { .. } => BlackBoxFunc::EcdsaSecp256r1, - BlackBoxOp::SchnorrVerify { .. } => BlackBoxFunc::SchnorrVerify, BlackBoxOp::MultiScalarMul { .. } => BlackBoxFunc::MultiScalarMul, BlackBoxOp::EmbeddedCurveAdd { .. } => BlackBoxFunc::EmbeddedCurveAdd, BlackBoxOp::BigIntAdd { .. } => BlackBoxFunc::BigIntAdd, diff --git a/compiler/integration-tests/package.json b/compiler/integration-tests/package.json index a9d437da792..bfaa1cabd16 100644 --- a/compiler/integration-tests/package.json +++ b/compiler/integration-tests/package.json @@ -13,7 +13,7 @@ "lint": "NODE_NO_WARNINGS=1 eslint . --ext .ts --ignore-path ./.eslintignore --max-warnings 0" }, "dependencies": { - "@aztec/bb.js": "0.63.1", + "@aztec/bb.js": "0.66.0", "@noir-lang/noir_js": "workspace:*", "@noir-lang/noir_wasm": "workspace:*", "@nomicfoundation/hardhat-chai-matchers": "^2.0.0", diff --git a/compiler/noirc_evaluator/src/acir/generated_acir.rs b/compiler/noirc_evaluator/src/acir/generated_acir.rs index 91206abe732..3b29c0319ab 100644 --- a/compiler/noirc_evaluator/src/acir/generated_acir.rs +++ b/compiler/noirc_evaluator/src/acir/generated_acir.rs @@ -225,19 +225,6 @@ impl GeneratedAcir { inputs: inputs[0].clone(), outputs: outputs.try_into().expect("Compiler should generate correct size outputs"), }, - BlackBoxFunc::SchnorrVerify => { - BlackBoxFuncCall::SchnorrVerify { - public_key_x: inputs[0][0], - public_key_y: inputs[1][0], - // Schnorr signature is an r & s, 32 bytes each - signature: inputs[2] - .clone() - .try_into() - .expect("Compiler should generate correct size inputs"), - message: inputs[3].clone(), - output: outputs[0], - } - } BlackBoxFunc::EcdsaSecp256k1 => { BlackBoxFuncCall::EcdsaSecp256k1 { // 32 bytes for each public key co-ordinate @@ -715,9 +702,7 @@ fn black_box_func_expected_input_size(name: BlackBoxFunc) -> Option { // Signature verification algorithms will take in a variable // number of inputs, since the message/hashed-message can vary in size. - BlackBoxFunc::SchnorrVerify - | BlackBoxFunc::EcdsaSecp256k1 - | BlackBoxFunc::EcdsaSecp256r1 => None, + BlackBoxFunc::EcdsaSecp256k1 | BlackBoxFunc::EcdsaSecp256r1 => None, // Inputs for multi scalar multiplication is an arbitrary number of [point, scalar] pairs. BlackBoxFunc::MultiScalarMul => None, @@ -762,9 +747,7 @@ fn black_box_expected_output_size(name: BlackBoxFunc) -> Option { BlackBoxFunc::RANGE => Some(0), // Signature verification algorithms will return a boolean - BlackBoxFunc::SchnorrVerify - | BlackBoxFunc::EcdsaSecp256k1 - | BlackBoxFunc::EcdsaSecp256r1 => Some(1), + BlackBoxFunc::EcdsaSecp256k1 | BlackBoxFunc::EcdsaSecp256r1 => Some(1), // Output of operations over the embedded curve // will be 2 field elements representing the point. 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 3685c9540f3..2ddcea26570 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 @@ -144,27 +144,6 @@ pub(crate) fn convert_black_box_call { - if let ( - [BrilligVariable::SingleAddr(public_key_x), BrilligVariable::SingleAddr(public_key_y), signature, message], - [BrilligVariable::SingleAddr(result_register)], - ) = (function_arguments, function_results) - { - let message = convert_array_or_vector(brillig_context, *message, bb_func); - let signature = convert_array_or_vector(brillig_context, *signature, bb_func); - brillig_context.black_box_op_instruction(BlackBoxOp::SchnorrVerify { - public_key_x: public_key_x.address, - public_key_y: public_key_y.address, - message, - signature, - result: result_register.address, - }); - brillig_context.deallocate_heap_vector(message); - brillig_context.deallocate_heap_vector(signature); - } else { - unreachable!("ICE: Schnorr verify expects two registers for the public key, an array for signature, an array for the message hash and one result register") - } - } BlackBoxFunc::MultiScalarMul => { if let ([points, scalars], [BrilligVariable::BrilligArray(outputs)]) = (function_arguments, function_results) diff --git a/compiler/noirc_evaluator/src/brillig/brillig_ir.rs b/compiler/noirc_evaluator/src/brillig/brillig_ir.rs index b4e10035af6..8d5f14cee94 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_ir.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_ir.rs @@ -253,15 +253,6 @@ pub(crate) mod tests { pub(crate) struct DummyBlackBoxSolver; impl BlackBoxFunctionSolver for DummyBlackBoxSolver { - fn schnorr_verify( - &self, - _public_key_x: &FieldElement, - _public_key_y: &FieldElement, - _signature: &[u8; 64], - _message: &[u8], - ) -> Result { - Ok(true) - } fn multi_scalar_mul( &self, _points: &[FieldElement], 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 55a24264fbb..ef1b5432128 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_ir/debug_show.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_ir/debug_show.rs @@ -326,23 +326,6 @@ impl DebugShow { result ); } - BlackBoxOp::SchnorrVerify { - public_key_x, - public_key_y, - message, - signature, - result, - } => { - debug_println!( - self.enable_debug_trace, - " SCHNORR_VERIFY {} {} {} {} -> {}", - public_key_x, - public_key_y, - message, - signature, - result - ); - } BlackBoxOp::BigIntAdd { lhs, rhs, output } => { debug_println!( self.enable_debug_trace, diff --git a/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs b/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs index 7709e5bc0e1..a8db5e2ff94 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs @@ -600,7 +600,6 @@ fn simplify_black_box_func( BlackBoxFunc::EmbeddedCurveAdd => { blackbox::simplify_ec_add(dfg, solver, arguments, block, call_stack) } - BlackBoxFunc::SchnorrVerify => blackbox::simplify_schnorr_verify(dfg, solver, arguments), BlackBoxFunc::BigIntAdd | BlackBoxFunc::BigIntSub diff --git a/compiler/noirc_evaluator/src/ssa/ir/instruction/call/blackbox.rs b/compiler/noirc_evaluator/src/ssa/ir/instruction/call/blackbox.rs index db085bd762f..016d7ffa25b 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/instruction/call/blackbox.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/instruction/call/blackbox.rs @@ -230,39 +230,6 @@ pub(super) fn simplify_poseidon2_permutation( } } -pub(super) fn simplify_schnorr_verify( - dfg: &mut DataFlowGraph, - solver: impl BlackBoxFunctionSolver, - arguments: &[ValueId], -) -> SimplifyResult { - match ( - dfg.get_numeric_constant(arguments[0]), - dfg.get_numeric_constant(arguments[1]), - dfg.get_array_constant(arguments[2]), - dfg.get_array_constant(arguments[3]), - ) { - (Some(public_key_x), Some(public_key_y), Some((signature, _)), Some((message, _))) - if array_is_constant(dfg, &signature) && array_is_constant(dfg, &message) => - { - let signature = to_u8_vec(dfg, signature); - let signature: [u8; 64] = - signature.try_into().expect("Compiler should produce correctly sized signature"); - - let message = to_u8_vec(dfg, message); - - let Ok(valid_signature) = - solver.schnorr_verify(&public_key_x, &public_key_y, &signature, &message) - else { - return SimplifyResult::None; - }; - - let valid_signature = dfg.make_constant(valid_signature.into(), Type::bool()); - SimplifyResult::SimplifiedTo(valid_signature) - } - _ => SimplifyResult::None, - } -} - pub(super) fn simplify_hash( dfg: &mut DataFlowGraph, arguments: &[ValueId], diff --git a/docs/docs/noir/standard_library/cryptographic_primitives/schnorr.mdx b/docs/docs/noir/standard_library/cryptographic_primitives/schnorr.mdx index 286a0ac6c7d..4c859043787 100644 --- a/docs/docs/noir/standard_library/cryptographic_primitives/schnorr.mdx +++ b/docs/docs/noir/standard_library/cryptographic_primitives/schnorr.mdx @@ -10,7 +10,6 @@ import BlackBoxInfo from '@site/src/components/Notes/_blackbox'; ## schnorr::verify_signature Verifier for Schnorr signatures over the embedded curve (for BN254 it is Grumpkin). -See schnorr::verify_signature_slice for a version that works directly on slices. #include_code schnorr_verify noir_stdlib/src/schnorr.nr rust @@ -34,13 +33,4 @@ const signature = Array.from( ... ``` - -## schnorr::verify_signature_slice - -Verifier for Schnorr signatures over the embedded curve (for BN254 it is Grumpkin) -where the message is a slice. - -#include_code schnorr_verify_slice noir_stdlib/src/schnorr.nr rust - - diff --git a/docs/versioned_docs/version-v0.33.0/index.mdx b/docs/versioned_docs/version-v0.33.0/index.mdx index a6bd306f91d..9ed9662b0b9 100644 --- a/docs/versioned_docs/version-v0.33.0/index.mdx +++ b/docs/versioned_docs/version-v0.33.0/index.mdx @@ -51,7 +51,7 @@ Noir can be used both in complex cloud-based backends and in user's smartphones, Aztec Contracts leverage Noir to allow for the storage and execution of private information. Writing an Aztec Contract is as easy as writing Noir, and Aztec developers can easily interact with the network storage and execution through the [Aztec.nr](https://docs.aztec.network/developers/contracts/main) library. - Soliditry Verifier Example + Solidity Verifier Example Noir can auto-generate Solidity verifier contracts that verify Noir proofs. This allows for non-interactive verification of proofs containing private information in an immutable system. This feature powers a multitude of use-case scenarios, from P2P chess tournaments, to [Aztec Layer-2 Blockchain](https://docs.aztec.network/) diff --git a/noir_stdlib/src/schnorr.nr b/noir_stdlib/src/schnorr.nr index a43e75537ee..d9d494e3093 100644 --- a/noir_stdlib/src/schnorr.nr +++ b/noir_stdlib/src/schnorr.nr @@ -1,32 +1,13 @@ use crate::embedded_curve_ops::{EmbeddedCurvePoint, EmbeddedCurveScalar}; -#[foreign(schnorr_verify)] // docs:start:schnorr_verify pub fn verify_signature( - public_key_x: Field, - public_key_y: Field, + public_key: EmbeddedCurvePoint, signature: [u8; 64], message: [u8; N], ) -> bool // docs:end:schnorr_verify -{} - -#[foreign(schnorr_verify)] -// docs:start:schnorr_verify_slice -pub fn verify_signature_slice( - public_key_x: Field, - public_key_y: Field, - signature: [u8; 64], - message: [u8], -) -> bool -// docs:end:schnorr_verify_slice -{} - -pub fn verify_signature_noir( - public_key: EmbeddedCurvePoint, - signature: [u8; 64], - message: [u8; N], -) -> bool { +{ //scalar lo/hi from bytes let sig_s = EmbeddedCurveScalar::from_bytes(signature, 0); let sig_e = EmbeddedCurveScalar::from_bytes(signature, 32); @@ -109,6 +90,6 @@ fn test_zero_signature() { }; let signature: [u8; 64] = [0; 64]; let message: [u8; _] = [2; 64]; // every message - let verified = verify_signature_noir(public_key, signature, message); + let verified = verify_signature(public_key, signature, message); assert(!verified); } diff --git a/scripts/install_bb.sh b/scripts/install_bb.sh index db98f17c503..3d1dc038ab8 100755 --- a/scripts/install_bb.sh +++ b/scripts/install_bb.sh @@ -1,6 +1,6 @@ #!/bin/bash -VERSION="0.63.0" +VERSION="0.66.0" BBUP_PATH=~/.bb/bbup diff --git a/test_programs/compile_success_empty/schnorr_simplification/src/main.nr b/test_programs/compile_success_empty/schnorr_simplification/src/main.nr index cdfa8337094..53b71fc3842 100644 --- a/test_programs/compile_success_empty/schnorr_simplification/src/main.nr +++ b/test_programs/compile_success_empty/schnorr_simplification/src/main.nr @@ -1,9 +1,14 @@ +use std::embedded_curve_ops::EmbeddedCurvePoint; + // Note: If main has any unsized types, then the verifier will never be able // to figure out the circuit instance fn main() { let message = [0, 1, 2, 3, 4, 5, 6, 7, 8, 9]; - let pub_key_x = 0x04b260954662e97f00cab9adb773a259097f7a274b83b113532bce27fa3fb96a; - let pub_key_y = 0x2fd51571db6c08666b0edfbfbc57d432068bccd0110a39b166ab243da0037197; + let pub_key = EmbeddedCurvePoint { + x: 0x04b260954662e97f00cab9adb773a259097f7a274b83b113532bce27fa3fb96a, + y: 0x2fd51571db6c08666b0edfbfbc57d432068bccd0110a39b166ab243da0037197, + is_infinite: false, + }; let signature = [ 1, 13, 119, 112, 212, 39, 233, 41, 84, 235, 255, 93, 245, 172, 186, 83, 157, 253, 76, 77, 33, 128, 178, 15, 214, 67, 105, 107, 177, 234, 77, 48, 27, 237, 155, 84, 39, 84, 247, 27, @@ -11,6 +16,6 @@ fn main() { 239, 138, 124, 12, ]; - let valid_signature = std::schnorr::verify_signature(pub_key_x, pub_key_y, signature, message); + let valid_signature = std::schnorr::verify_signature(pub_key, signature, message); assert(valid_signature); } diff --git a/test_programs/execution_success/schnorr/src/main.nr b/test_programs/execution_success/schnorr/src/main.nr index 21845cd54fa..ab3c65372c5 100644 --- a/test_programs/execution_success/schnorr/src/main.nr +++ b/test_programs/execution_success/schnorr/src/main.nr @@ -13,18 +13,12 @@ fn main( // We want to make sure that we can accurately verify a signature whose message is a slice vs. an array let message_field_bytes: [u8; 10] = message_field.to_be_bytes(); - // Is there ever a situation where someone would want - // to ensure that a signature was invalid? - // Check that passing a slice as the message is valid - let valid_signature = - std::schnorr::verify_signature_slice(pub_key_x, pub_key_y, signature, message_field_bytes); - assert(valid_signature); // Check that passing an array as the message is valid - let valid_signature = std::schnorr::verify_signature(pub_key_x, pub_key_y, signature, message); - assert(valid_signature); let pub_key = embedded_curve_ops::EmbeddedCurvePoint { x: pub_key_x, y: pub_key_y, is_infinite: false }; - let valid_signature = std::schnorr::verify_signature_noir(pub_key, signature, message); + let valid_signature = std::schnorr::verify_signature(pub_key, signature, message_field_bytes); + assert(valid_signature); + let valid_signature = std::schnorr::verify_signature(pub_key, signature, message); assert(valid_signature); std::schnorr::assert_valid_signature(pub_key, signature, message); } diff --git a/tooling/lsp/src/solver.rs b/tooling/lsp/src/solver.rs index 3c2d7499880..a36e30a944e 100644 --- a/tooling/lsp/src/solver.rs +++ b/tooling/lsp/src/solver.rs @@ -6,16 +6,6 @@ use acvm::BlackBoxFunctionSolver; pub(super) struct WrapperSolver(pub(super) Box>); impl BlackBoxFunctionSolver for WrapperSolver { - fn schnorr_verify( - &self, - public_key_x: &acvm::FieldElement, - public_key_y: &acvm::FieldElement, - signature: &[u8; 64], - message: &[u8], - ) -> Result { - self.0.schnorr_verify(public_key_x, public_key_y, signature, message) - } - fn multi_scalar_mul( &self, points: &[acvm::FieldElement], diff --git a/tooling/profiler/src/opcode_formatter.rs b/tooling/profiler/src/opcode_formatter.rs index b4367de9e7e..d1081de6c8f 100644 --- a/tooling/profiler/src/opcode_formatter.rs +++ b/tooling/profiler/src/opcode_formatter.rs @@ -10,7 +10,6 @@ fn format_blackbox_function(call: &BlackBoxFuncCall) -> String { BlackBoxFuncCall::RANGE { .. } => "range".to_string(), BlackBoxFuncCall::Blake2s { .. } => "blake2s".to_string(), BlackBoxFuncCall::Blake3 { .. } => "blake3".to_string(), - BlackBoxFuncCall::SchnorrVerify { .. } => "schnorr_verify".to_string(), BlackBoxFuncCall::EcdsaSecp256k1 { .. } => "ecdsa_secp256k1".to_string(), BlackBoxFuncCall::EcdsaSecp256r1 { .. } => "ecdsa_secp256r1".to_string(), BlackBoxFuncCall::MultiScalarMul { .. } => "multi_scalar_mul".to_string(), @@ -33,7 +32,6 @@ fn format_blackbox_op(call: &BlackBoxOp) -> String { BlackBoxOp::AES128Encrypt { .. } => "aes128_encrypt".to_string(), BlackBoxOp::Blake2s { .. } => "blake2s".to_string(), BlackBoxOp::Blake3 { .. } => "blake3".to_string(), - BlackBoxOp::SchnorrVerify { .. } => "schnorr_verify".to_string(), BlackBoxOp::EcdsaSecp256k1 { .. } => "ecdsa_secp256k1".to_string(), BlackBoxOp::EcdsaSecp256r1 { .. } => "ecdsa_secp256r1".to_string(), BlackBoxOp::MultiScalarMul { .. } => "multi_scalar_mul".to_string(), diff --git a/tooling/readme.md b/tooling/readme.md index 20d1b560b5b..3172062241a 100644 --- a/tooling/readme.md +++ b/tooling/readme.md @@ -4,7 +4,7 @@ Below we briefly describe the purpose of each tool-related crate in this reposit ## nargo -This is the default package manager used by Noir. One may draw similarities to Rusts' Cargo. +This is the default package manager used by Noir. One may draw similarities to Rust's Cargo. ## nargo_fmt diff --git a/yarn.lock b/yarn.lock index f7b7b3df372..77962512b08 100644 --- a/yarn.lock +++ b/yarn.lock @@ -221,18 +221,19 @@ __metadata: languageName: node linkType: hard -"@aztec/bb.js@npm:0.63.1": - version: 0.63.1 - resolution: "@aztec/bb.js@npm:0.63.1" +"@aztec/bb.js@npm:0.66.0": + version: 0.66.0 + resolution: "@aztec/bb.js@npm:0.66.0" dependencies: comlink: ^4.4.1 commander: ^10.0.1 debug: ^4.3.4 fflate: ^0.8.0 + pako: ^2.1.0 tslib: ^2.4.0 bin: bb.js: dest/node/main.js - checksum: b80730f1cb87e4d2ca21d991a42950bc069367896db309ab3f909c5f53efa9291538d51e35bc3c6d2eea042ca33c279ae59eb3f5d844a24336c7bb9664c2404b + checksum: 7295bf6543afe1c2db795a95c7ed40806de63c77e44bb4dacb2ec6a9171d1d34749150844ab47bc2499d06676e623acb408857b6aa9da702d3c576efadb8c906 languageName: node linkType: hard @@ -14123,7 +14124,7 @@ __metadata: version: 0.0.0-use.local resolution: "integration-tests@workspace:compiler/integration-tests" dependencies: - "@aztec/bb.js": 0.63.1 + "@aztec/bb.js": 0.66.0 "@noir-lang/noir_js": "workspace:*" "@noir-lang/noir_wasm": "workspace:*" "@nomicfoundation/hardhat-chai-matchers": ^2.0.0