diff --git a/Cargo.lock b/Cargo.lock index 83884bbf57a..23b25c0b451 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2240,6 +2240,7 @@ dependencies = [ "noirc_abi", "noirc_errors", "rustc-hash", + "serde", "smol_str", "strum", "strum_macros", diff --git a/crates/noirc_driver/src/contract.rs b/crates/noirc_driver/src/contract.rs index 3f438355f3b..4e74f06a2e3 100644 --- a/crates/noirc_driver/src/contract.rs +++ b/crates/noirc_driver/src/contract.rs @@ -1,29 +1,9 @@ use std::collections::BTreeMap; +use noirc_frontend::ContractVisibility; + use crate::CompiledProgram; -/// ContractFunctionType describes the types -/// smart contract functions that are allowed. -/// -/// Note: -/// - All Noir programs in the non-contract context -/// can be seen as `Secret`. -/// - It may be possible to have `unconstrained` -/// functions in regular Noir programs. For now -/// we leave it as a property of only contract functions. -#[derive(serde::Serialize, serde::Deserialize)] -#[serde(rename_all = "snake_case")] -pub enum ContractFunctionType { - /// This function will be executed in a private - /// context. - Secret, - /// This function will be executed in a public - /// context. - Public, - /// A function which is non-deterministic - /// and does not require any constraints. - Unconstrained, -} /// Each function in the contract will be compiled /// as a separate noir program. /// @@ -32,7 +12,7 @@ pub enum ContractFunctionType { /// One of these being a function type. #[derive(serde::Serialize, serde::Deserialize)] pub struct ContractFunction { - pub func_type: ContractFunctionType, + pub func_type: ContractVisibility, pub function: CompiledProgram, } diff --git a/crates/noirc_driver/src/lib.rs b/crates/noirc_driver/src/lib.rs index 789ea90ae10..d1bcc7b71af 100644 --- a/crates/noirc_driver/src/lib.rs +++ b/crates/noirc_driver/src/lib.rs @@ -202,11 +202,14 @@ impl Driver { contract: Contract, options: &CompileOptions, ) -> Result { - let functions = try_btree_map(&contract.functions, |function| { - let function_name = self.function_name(*function).to_owned(); - let function = self.compile_no_check(options, *function)?; - let func_type = contract::ContractFunctionType::Secret; - // Note: currently we mark all of the contract methods as secret as we do not support public functions. + let functions = try_btree_map(&contract.functions, |function_id| { + let function_name = self.function_name(*function_id).to_owned(); + let function = self.compile_no_check(options, *function_id)?; + let func_meta = self.context.def_interner.function_meta(function_id); + let func_type = func_meta + .contract_visibility + .expect("Expected contract function to have a contract visibility"); + Ok((function_name, ContractFunction { func_type, function })) })?; diff --git a/crates/noirc_frontend/Cargo.toml b/crates/noirc_frontend/Cargo.toml index b05601c8ab6..b5551d17f51 100644 --- a/crates/noirc_frontend/Cargo.toml +++ b/crates/noirc_frontend/Cargo.toml @@ -16,8 +16,9 @@ iter-extended.workspace = true chumsky.workspace = true thiserror.workspace = true smol_str.workspace = true +serde.workspace = true rustc-hash = "1.1.0" [dev-dependencies] strum = "0.24" -strum_macros = "0.24" \ No newline at end of file +strum_macros = "0.24" diff --git a/crates/noirc_frontend/src/ast/expression.rs b/crates/noirc_frontend/src/ast/expression.rs index 15ae1d8aaa8..d1cfd4140ef 100644 --- a/crates/noirc_frontend/src/ast/expression.rs +++ b/crates/noirc_frontend/src/ast/expression.rs @@ -309,7 +309,14 @@ pub struct Lambda { #[derive(Debug, PartialEq, Eq, Clone)] pub struct FunctionDefinition { pub name: Ident, - pub attribute: Option, // XXX: Currently we only have one attribute defined. If more attributes are needed per function, we can make this a vector and make attribute definition more expressive + + // XXX: Currently we only have one attribute defined. If more attributes are needed per function, we can make this a vector and make attribute definition more expressive + pub attribute: Option, + + // The contract visibility is always None if the function is not in a contract. + // Otherwise, it is 'secret' (by default), 'open', or 'unsafe'. + pub contract_visibility: Option, + pub generics: UnresolvedGenerics, pub parameters: Vec<(Pattern, UnresolvedType, noirc_abi::AbiVisibility)>, pub body: BlockExpression, @@ -318,6 +325,24 @@ pub struct FunctionDefinition { pub return_visibility: noirc_abi::AbiVisibility, } +/// Describes the types of smart contract functions that are allowed. +/// - All Noir programs in the non-contract context can be seen as `Secret`. +/// - It may be possible to have `unsafe` functions in regular Noir programs. +/// For now we leave it as a property of only contract functions. +#[derive(serde::Serialize, serde::Deserialize, Debug, Copy, Clone, PartialEq, Eq)] +#[serde(rename_all = "snake_case")] +pub enum ContractVisibility { + /// This function will be executed in a private + /// context. + Secret, + /// This function will be executed in a public + /// context. + Open, + /// A function which is non-deterministic + /// and does not require any constraints. + Unsafe, +} + #[derive(Debug, PartialEq, Eq, Clone)] pub enum ArrayLiteral { Standard(Vec), diff --git a/crates/noirc_frontend/src/hir/def_map/mod.rs b/crates/noirc_frontend/src/hir/def_map/mod.rs index 7c0788050ce..abf07c8d2a8 100644 --- a/crates/noirc_frontend/src/hir/def_map/mod.rs +++ b/crates/noirc_frontend/src/hir/def_map/mod.rs @@ -36,6 +36,12 @@ pub struct ModuleId { pub local_id: LocalModuleId, } +impl ModuleId { + pub fn module(self, def_maps: &HashMap) -> &ModuleData { + &def_maps[&self.krate].modules()[self.local_id.0] + } +} + /// Map of all modules and scopes defined within a crate. /// /// The definitions of the crate are accessible indirectly via the scopes of each module. diff --git a/crates/noirc_frontend/src/hir/mod.rs b/crates/noirc_frontend/src/hir/mod.rs index d52278421a2..6d469a4229d 100644 --- a/crates/noirc_frontend/src/hir/mod.rs +++ b/crates/noirc_frontend/src/hir/mod.rs @@ -56,7 +56,7 @@ impl Context { } fn module(&self, module_id: def_map::ModuleId) -> &def_map::ModuleData { - &self.def_maps[&module_id.krate].modules()[module_id.local_id.0] + module_id.module(&self.def_maps) } /// Returns the next available storage slot in the given module. diff --git a/crates/noirc_frontend/src/hir/resolution/errors.rs b/crates/noirc_frontend/src/hir/resolution/errors.rs index 4d08388ff74..bf9a41cd434 100644 --- a/crates/noirc_frontend/src/hir/resolution/errors.rs +++ b/crates/noirc_frontend/src/hir/resolution/errors.rs @@ -61,6 +61,8 @@ pub enum ResolverError { }, #[error("{0}")] ParserError(ParserError), + #[error("Function is not defined in a contract yet sets its contract visibility")] + ContractVisibilityInNormalFunction { span: Span }, } impl ResolverError { @@ -242,6 +244,11 @@ impl From for Diagnostic { ) } ResolverError::ParserError(error) => error.into(), + ResolverError::ContractVisibilityInNormalFunction { span } => Diagnostic::simple_error( + "Only functions defined within contracts can set their contract visibility".into(), + "Non-contract functions cannot be 'open' or 'unsafe'".into(), + span, + ), } } } diff --git a/crates/noirc_frontend/src/hir/resolution/import.rs b/crates/noirc_frontend/src/hir/resolution/import.rs index ce7d1503fb7..836333ee5d9 100644 --- a/crates/noirc_frontend/src/hir/resolution/import.rs +++ b/crates/noirc_frontend/src/hir/resolution/import.rs @@ -73,9 +73,9 @@ pub fn resolve_imports( pub(super) fn allow_referencing_contracts( def_maps: &HashMap, krate: CrateId, - module: LocalModuleId, + local_id: LocalModuleId, ) -> bool { - def_maps[&krate].modules()[module.0].is_contract + ModuleId { krate, local_id }.module(def_maps).is_contract } pub fn resolve_path_to_ns( diff --git a/crates/noirc_frontend/src/hir/resolution/path_resolver.rs b/crates/noirc_frontend/src/hir/resolution/path_resolver.rs index 57cf4c709c7..eb0483fbf54 100644 --- a/crates/noirc_frontend/src/hir/resolution/path_resolver.rs +++ b/crates/noirc_frontend/src/hir/resolution/path_resolver.rs @@ -16,6 +16,8 @@ pub trait PathResolver { ) -> Result; fn local_module_id(&self) -> LocalModuleId; + + fn module_id(&self) -> ModuleId; } pub struct StandardPathResolver { @@ -41,6 +43,10 @@ impl PathResolver for StandardPathResolver { fn local_module_id(&self) -> LocalModuleId { self.module_id.local_id } + + fn module_id(&self) -> ModuleId { + self.module_id + } } /// Resolve the given path to a function or a type. diff --git a/crates/noirc_frontend/src/hir/resolution/resolver.rs b/crates/noirc_frontend/src/hir/resolution/resolver.rs index 709751e8176..3ed70034f03 100644 --- a/crates/noirc_frontend/src/hir/resolution/resolver.rs +++ b/crates/noirc_frontend/src/hir/resolution/resolver.rs @@ -33,9 +33,9 @@ use crate::{ Statement, }; use crate::{ - ArrayLiteral, Generics, LValue, NoirStruct, Path, Pattern, Shared, StructType, Type, - TypeBinding, TypeVariable, UnresolvedGenerics, UnresolvedType, UnresolvedTypeExpression, - ERROR_IDENT, + ArrayLiteral, ContractVisibility, Generics, LValue, NoirStruct, Path, Pattern, Shared, + StructType, Type, TypeBinding, TypeVariable, UnresolvedGenerics, UnresolvedType, + UnresolvedTypeExpression, ERROR_IDENT, }; use fm::FileId; use iter_extended::vecmap; @@ -635,6 +635,7 @@ impl<'a> Resolver<'a> { name: name_ident, kind: func.kind, attributes, + contract_visibility: self.handle_contract_visibility(func), location, typ, parameters: parameters.into(), @@ -643,6 +644,24 @@ impl<'a> Resolver<'a> { } } + fn handle_contract_visibility(&mut self, func: &NoirFunction) -> Option { + let mut contract_visibility = func.def.contract_visibility; + + if self.in_contract() && contract_visibility.is_none() { + // The default visibility is 'secret' for contract functions without visibility modifiers + contract_visibility = Some(ContractVisibility::Secret); + } + + if !self.in_contract() && contract_visibility.is_some() { + contract_visibility = None; + self.push_err(ResolverError::ContractVisibilityInNormalFunction { + span: func.name_ident().span(), + }) + } + + contract_visibility + } + fn declare_numeric_generics(&mut self, params: &[Type], return_type: &Type) { if self.generics.is_empty() { return; @@ -1196,6 +1215,11 @@ impl<'a> Resolver<'a> { _other => Err(Some(ResolverError::InvalidArrayLengthExpr { span })), } } + + fn in_contract(&self) -> bool { + let module_id = self.path_resolver.module_id(); + module_id.module(self.def_maps).is_contract + } } // XXX: These tests repeat a lot of code @@ -1209,6 +1233,7 @@ mod test { use fm::FileId; use iter_extended::vecmap; + use crate::hir::def_map::{ModuleData, ModuleId, ModuleOrigin}; use crate::hir::resolution::errors::ResolverError; use crate::hir::resolution::import::PathResolutionError; @@ -1241,9 +1266,22 @@ mod test { path_resolver.insert_func(name.to_owned(), id); } - let def_maps: HashMap = HashMap::new(); + let mut def_maps: HashMap = HashMap::new(); let file = FileId::default(); + let mut modules = arena::Arena::new(); + modules.insert(ModuleData::new(None, ModuleOrigin::File(file), false)); + + def_maps.insert( + CrateId::dummy_id(), + CrateDefMap { + root: path_resolver.local_module_id(), + modules, + krate: CrateId::dummy_id(), + extern_prelude: HashMap::new(), + }, + ); + let mut errors = Vec::new(); for func in program.functions { let id = interner.push_fn(HirFunction::empty()); @@ -1445,7 +1483,13 @@ mod test { } fn local_module_id(&self) -> LocalModuleId { - LocalModuleId::dummy_id() + // This is not LocalModuleId::dummy since we need to use this to index into a Vec + // later and do not want to push u32::MAX number of elements before we do. + LocalModuleId(arena::Index::from_raw_parts(0, 0)) + } + + fn module_id(&self) -> ModuleId { + ModuleId { krate: CrateId::dummy_id(), local_id: self.local_module_id() } } } diff --git a/crates/noirc_frontend/src/hir/type_check/mod.rs b/crates/noirc_frontend/src/hir/type_check/mod.rs index 08fc57f48e7..c91c6a2b6f4 100644 --- a/crates/noirc_frontend/src/hir/type_check/mod.rs +++ b/crates/noirc_frontend/src/hir/type_check/mod.rs @@ -75,6 +75,7 @@ mod test { use noirc_errors::{Location, Span}; use crate::graph::CrateId; + use crate::hir::def_map::{ModuleData, ModuleId, ModuleOrigin}; use crate::hir::resolution::import::PathResolutionError; use crate::hir_def::expr::HirIdent; use crate::hir_def::stmt::HirLetStatement; @@ -157,6 +158,7 @@ mod test { kind: FunctionKind::Normal, attributes: None, location, + contract_visibility: None, typ: Type::Function(vec![Type::field(None), Type::field(None)], Box::new(Type::Unit)), parameters: vec![ Param(Identifier(x), Type::field(None), noirc_abi::AbiVisibility::Private), @@ -244,7 +246,11 @@ mod test { } fn local_module_id(&self) -> LocalModuleId { - LocalModuleId::dummy_id() + LocalModuleId(arena::Index::from_raw_parts(0, 0)) + } + + fn module_id(&self) -> ModuleId { + ModuleId { krate: CrateId::dummy_id(), local_id: self.local_module_id() } } } @@ -275,12 +281,25 @@ mod test { let mut path_resolver = TestPathResolver(HashMap::new()); for (name, id) in func_namespace.into_iter().zip(func_ids.clone()) { - path_resolver.insert_func(name, id); + path_resolver.insert_func(name.to_owned(), id); } - let def_maps: HashMap = HashMap::new(); + let mut def_maps: HashMap = HashMap::new(); let file = FileId::default(); + let mut modules = arena::Arena::new(); + modules.insert(ModuleData::new(None, ModuleOrigin::File(file), false)); + + def_maps.insert( + CrateId::dummy_id(), + CrateDefMap { + root: path_resolver.local_module_id(), + modules, + krate: CrateId::dummy_id(), + extern_prelude: HashMap::new(), + }, + ); + let func_meta = vecmap(program.functions, |nf| { let resolver = Resolver::new(&mut interner, &path_resolver, &def_maps, file); let (hir_func, func_meta, resolver_errors) = resolver.resolve_function(nf, main_id); diff --git a/crates/noirc_frontend/src/hir_def/function.rs b/crates/noirc_frontend/src/hir_def/function.rs index 813c6c1fd85..31e5b9b0030 100644 --- a/crates/noirc_frontend/src/hir_def/function.rs +++ b/crates/noirc_frontend/src/hir_def/function.rs @@ -5,8 +5,8 @@ use noirc_errors::{Location, Span}; use super::expr::{HirBlockExpression, HirExpression, HirIdent}; use super::stmt::HirPattern; use crate::node_interner::{ExprId, NodeInterner}; -use crate::Type; use crate::{token::Attribute, FunctionKind}; +use crate::{ContractVisibility, Type}; /// A Hir function is a block expression /// with a list of statements @@ -121,6 +121,10 @@ pub struct FuncMeta { /// Attribute per function. pub attributes: Option, + /// This function's visibility in its contract. + /// If this function is not in a contract, this is always 'Secret'. + pub contract_visibility: Option, + pub parameters: Parameters, pub return_visibility: AbiVisibility, diff --git a/crates/noirc_frontend/src/lexer/token.rs b/crates/noirc_frontend/src/lexer/token.rs index 6cce4d351b4..beb029add39 100644 --- a/crates/noirc_frontend/src/lexer/token.rs +++ b/crates/noirc_frontend/src/lexer/token.rs @@ -432,10 +432,12 @@ pub enum Keyword { Let, Mod, Mut, + Open, Pub, String, Return, Struct, + Unsafe, Use, While, } @@ -462,10 +464,12 @@ impl fmt::Display for Keyword { Keyword::Let => write!(f, "let"), Keyword::Mod => write!(f, "mod"), Keyword::Mut => write!(f, "mut"), + Keyword::Open => write!(f, "open"), Keyword::Pub => write!(f, "pub"), Keyword::String => write!(f, "str"), Keyword::Return => write!(f, "return"), Keyword::Struct => write!(f, "struct"), + Keyword::Unsafe => write!(f, "unsafe"), Keyword::Use => write!(f, "use"), Keyword::While => write!(f, "while"), } @@ -495,10 +499,12 @@ impl Keyword { "let" => Keyword::Let, "mod" => Keyword::Mod, "mut" => Keyword::Mut, + "open" => Keyword::Open, "pub" => Keyword::Pub, "str" => Keyword::String, "return" => Keyword::Return, "struct" => Keyword::Struct, + "unsafe" => Keyword::Unsafe, "use" => Keyword::Use, "while" => Keyword::While, diff --git a/crates/noirc_frontend/src/parser/parser.rs b/crates/noirc_frontend/src/parser/parser.rs index 9e767d3819b..4a57c6145ba 100644 --- a/crates/noirc_frontend/src/parser/parser.rs +++ b/crates/noirc_frontend/src/parser/parser.rs @@ -33,9 +33,10 @@ use crate::lexer::Lexer; use crate::parser::{force, ignore_then_commit, statement_recovery}; use crate::token::{Attribute, Keyword, Token, TokenKind}; use crate::{ - BinaryOp, BinaryOpKind, BlockExpression, CompTime, ConstrainStatement, FunctionDefinition, - Ident, IfExpression, ImportStatement, InfixExpression, LValue, Lambda, NoirFunction, NoirImpl, - NoirStruct, Path, PathKind, Pattern, Recoverable, UnaryOp, UnresolvedTypeExpression, + BinaryOp, BinaryOpKind, BlockExpression, CompTime, ConstrainStatement, ContractVisibility, + FunctionDefinition, Ident, IfExpression, ImportStatement, InfixExpression, LValue, Lambda, + NoirFunction, NoirImpl, NoirStruct, Path, PathKind, Pattern, Recoverable, UnaryOp, + UnresolvedTypeExpression, }; use chumsky::prelude::*; @@ -146,11 +147,12 @@ fn contract(module_parser: impl NoirParser) -> impl NoirParser impl NoirParser { attribute() .or_not() + .then(contract_visibility()) .then_ignore(keyword(Keyword::Fn)) .then(ident()) .then(generics()) @@ -159,13 +161,17 @@ fn function_definition(allow_self: bool) -> impl NoirParser { .then(block(expression())) .map( |( - ((((attribute, name), generics), parameters), (return_visibility, return_type)), + ( + ((((attribute, contract_visibility), name), generics), parameters), + (return_visibility, return_type), + ), body, )| { FunctionDefinition { span: name.0.span(), name, attribute, // XXX: Currently we only have one attribute defined. If more attributes are needed per function, we can make this a vector and make attribute definition more expressive + contract_visibility, generics, parameters, body, @@ -177,6 +183,16 @@ fn function_definition(allow_self: bool) -> impl NoirParser { ) } +/// contract_visibility: 'open' | 'unsafe' | %empty +fn contract_visibility() -> impl NoirParser> { + keyword(Keyword::Open).or(keyword(Keyword::Unsafe)).or_not().map(|token| match token { + Some(Token::Keyword(Keyword::Open)) => Some(ContractVisibility::Open), + Some(Token::Keyword(Keyword::Unsafe)) => Some(ContractVisibility::Unsafe), + None => None, + _ => unreachable!("Only open and unsafe keywords are parsed here"), + }) +} + /// non_empty_ident_list: ident ',' non_empty_ident_list /// | ident ///