feat: Implement 'open' and 'unconstrained' keywords (#1037)

* Implement 'open' and 'unconstrained' keywords

* Fix tests

* Fix failing tests

* Edit JSON Abi to include whether a function is open

* Update comment

* Code review

* Fix test

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,
 }
 

crates/noirc_driver/src/lib.rs

@@ -202,11 +202,14 @@ impl Driver {
         contract: Contract,
         options: &CompileOptions,
     ) -> Result<CompiledContract, ReportedError> {
-        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 }))
         })?;
 

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"
+strum_macros = "0.24"

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<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
+
+    // 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<Attribute>,
+
+    // 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<ContractVisibility>,
+
     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<Expression>),

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<CrateId, CrateDefMap>) -> &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.

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.

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<ResolverError> 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,
+            ),
         }
     }
 }

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<CrateId, CrateDefMap>,
     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(

crates/noirc_frontend/src/hir/resolution/path_resolver.rs

@@ -16,6 +16,8 @@ pub trait PathResolver {
     ) -> Result<ModuleDefId, PathResolutionError>;
 
     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.

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<ContractVisibility> {
+        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<CrateId, CrateDefMap> = HashMap::new();
+        let mut def_maps: HashMap<CrateId, CrateDefMap> = 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() }
         }
     }
 

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<CrateId, CrateDefMap> = HashMap::new();
+        let mut def_maps: HashMap<CrateId, CrateDefMap> = 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);

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<Attribute>,
 
+    /// This function's visibility in its contract.
+    /// If this function is not in a contract, this is always 'Secret'.
+    pub contract_visibility: Option<ContractVisibility>,
+
     pub parameters: Parameters,
 
     pub return_visibility: AbiVisibility,