chore!: Remove open keyword from Noir (#4967)

Removes the unused `open` keyword from Noir, as requested
[here](#4898 (comment)).

**Breaking change**: This PR changes the format of Noir contract build
artifacts, so a full rebuild of contracts is needed after this PR is
merged. No other action is needed by the user. If they fail to do it,
they'll see a message along the lines of _No custom attributes found for
contract function foo. Try rebuilding the contract with the latest nargo
version._ (cc @AztecProtocol/devrel @rahul-kothari)

---------

Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>

avm-transpiler/src/transpile_contract.rs

@@ -4,7 +4,6 @@ use regex::Regex;
 use serde::{Deserialize, Serialize};
 
 use acvm::acir::circuit::Circuit;
-use noirc_driver::ContractFunctionType;
 
 use crate::transpile::brillig_to_avm;
 use crate::utils::extract_brillig_from_acir;
@@ -40,8 +39,8 @@ pub struct CompiledAcirContract {
 #[derive(Debug, Serialize, Deserialize)]
 pub struct AvmContractFunction {
     pub name: String,
-    pub function_type: ContractFunctionType,
-    pub is_internal: bool,
+    pub is_unconstrained: bool,
+    pub custom_attributes: Vec<String>,
     pub abi: serde_json::Value,
     pub bytecode: String, // base64
     pub debug_symbols: serde_json::Value,
@@ -52,8 +51,8 @@ pub struct AvmContractFunction {
 #[derive(Debug, Serialize, Deserialize)]
 pub struct AcirContractFunction {
     pub name: String,
-    pub function_type: ContractFunctionType,
-    pub is_internal: bool,
+    pub is_unconstrained: bool,
+    pub custom_attributes: Vec<String>,
     pub abi: serde_json::Value,
     #[serde(
         serialize_with = "Circuit::serialize_circuit_base64",
@@ -82,7 +81,9 @@ impl From<CompiledAcirContract> for TranspiledContract {
         let re = Regex::new(r"avm_.*$").unwrap();
         for function in contract.functions {
             // TODO(4269): once functions are tagged for transpilation to AVM, check tag
-            if function.function_type == ContractFunctionType::Open
+            if function
+                .custom_attributes
+                .contains(&"aztec(public-vm)".to_string())
                 && re.is_match(function.name.as_str())
             {
                 info!(
@@ -99,8 +100,8 @@ impl From<CompiledAcirContract> for TranspiledContract {
                 // Push modified function entry to ABI
                 functions.push(AvmOrAcirContractFunction::Avm(AvmContractFunction {
                     name: function.name,
-                    function_type: function.function_type,
-                    is_internal: function.is_internal,
+                    is_unconstrained: function.is_unconstrained,
+                    custom_attributes: function.custom_attributes,
                     abi: function.abi,
                     bytecode: base64::prelude::BASE64_STANDARD.encode(avm_bytecode),
                     debug_symbols: function.debug_symbols,

noir/noir-repo/aztec_macros/src/lib.rs

@@ -151,10 +151,7 @@ fn transform_module(
             transform_vm_function(func, storage_defined)
                 .map_err(|err| (err, crate_graph.root_file_id))?;
             has_transformed_module = true;
-        }
-
-        // Add the storage struct to the beginning of the function if it is unconstrained in an aztec contract
-        if storage_defined && func.def.is_unconstrained {
+        } else if storage_defined && func.def.is_unconstrained {
             transform_unconstrained(func);
             has_transformed_module = true;
         }

noir/noir-repo/aztec_macros/src/transforms/functions.rs

@@ -40,7 +40,6 @@ pub fn transform_function(
     if is_internal {
         let is_internal_check = create_internal_check(func.name());
         func.def.body.0.insert(0, is_internal_check);
-        func.def.is_internal = true;
     }
 
     // Add initialization check
@@ -87,10 +86,10 @@ pub fn transform_function(
     func.def.return_visibility = Visibility::Public;
 
     // Distinct return types are only required for private functions
-    // Public functions should have open auto-inferred
+    // Public functions should have unconstrained auto-inferred
     match ty {
         "Private" => func.def.return_distinctness = Distinctness::Distinct,
-        "Public" => func.def.is_open = true,
+        "Public" => func.def.is_unconstrained = true,
         _ => (),
     }
 
@@ -113,7 +112,7 @@ pub fn transform_vm_function(
     func.def.body.0.insert(0, create_context);
 
     // We want the function to be seen as a public function
-    func.def.is_open = true;
+    func.def.is_unconstrained = true;
 
     // NOTE: the line below is a temporary hack to trigger external transpilation tools
     // It will be removed once the transpiler is integrated into the Noir compiler

noir/noir-repo/compiler/noirc_driver/src/contract.rs

@@ -9,23 +9,6 @@ use noirc_evaluator::errors::SsaReport;
 
 use super::debug::DebugFile;
 
-/// Describes the types of smart contract functions that are allowed.
-/// Unlike the similar enum in noirc_frontend, 'open' and 'unconstrained'
-/// are mutually exclusive here. In the case a function is both, 'unconstrained'
-/// takes precedence.
-#[derive(Serialize, Deserialize, Debug, Copy, Clone, PartialEq, Eq)]
-pub enum ContractFunctionType {
-    /// This function will be executed in a private
-    /// context.
-    Secret,
-    /// This function will be executed in a public
-    /// context.
-    Open,
-    /// This function cannot constrain any values and can use nondeterministic features
-    /// like arrays of a dynamic size.
-    Unconstrained,
-}
-
 #[derive(Clone, Debug, Serialize, Deserialize)]
 pub struct CompiledContract {
     pub noir_version: String,
@@ -55,9 +38,9 @@ pub struct CompiledContract {
 pub struct ContractFunction {
     pub name: String,
 
-    pub function_type: ContractFunctionType,
+    pub is_unconstrained: bool,
 
-    pub is_internal: bool,
+    pub custom_attributes: Vec<String>,
 
     pub abi: Abi,
 
@@ -69,13 +52,3 @@ pub struct ContractFunction {
 
     pub debug: DebugInfo,
 }
-
-impl ContractFunctionType {
-    pub(super) fn new(kind: noirc_frontend::ContractFunctionType, is_unconstrained: bool) -> Self {
-        match (kind, is_unconstrained) {
-            (_, true) => Self::Unconstrained,
-            (noirc_frontend::ContractFunctionType::Secret, false) => Self::Secret,
-            (noirc_frontend::ContractFunctionType::Open, false) => Self::Open,
-        }
-    }
-}

noir/noir-repo/compiler/noirc_driver/src/lib.rs

@@ -18,6 +18,7 @@ use noirc_frontend::hir::Context;
 use noirc_frontend::macros_api::MacroProcessor;
 use noirc_frontend::monomorphization::{monomorphize, monomorphize_debug, MonomorphizationError};
 use noirc_frontend::node_interner::FuncId;
+use noirc_frontend::token::SecondaryAttribute;
 use std::path::Path;
 use thiserror::Error;
 use tracing::info;
@@ -30,7 +31,7 @@ mod stdlib;
 
 use debug::filter_relevant_files;
 
-pub use contract::{CompiledContract, ContractFunction, ContractFunctionType};
+pub use contract::{CompiledContract, ContractFunction};
 pub use debug::DebugFile;
 pub use program::CompiledProgram;
 
@@ -404,19 +405,24 @@ fn compile_contract_inner(
         };
         warnings.extend(function.warnings);
         let modifiers = context.def_interner.function_modifiers(&function_id);
-        let func_type = modifiers
-            .contract_function_type
-            .expect("Expected contract function to have a contract visibility");
 
-        let function_type = ContractFunctionType::new(func_type, modifiers.is_unconstrained);
+        let custom_attributes = modifiers
+            .attributes
+            .secondary
+            .iter()
+            .filter_map(
+                |attr| if let SecondaryAttribute::Custom(tag) = attr { Some(tag) } else { None },
+            )
+            .cloned()
+            .collect();
 
         functions.push(ContractFunction {
             name,
-            function_type,
-            is_internal: modifiers.is_internal.unwrap_or(false),
+            custom_attributes,
             abi: function.abi,
             bytecode: function.circuit,
             debug: function.debug,
+            is_unconstrained: modifiers.is_unconstrained,
         });
     }
 

noir/noir-repo/compiler/noirc_frontend/src/ast/expression.rs

@@ -369,11 +369,6 @@ pub struct FunctionDefinition {
     // and `secondary` attributes (ones that do not change the function kind)
     pub attributes: Attributes,
 
-    /// True if this function was defined with the 'open' keyword
-    pub is_open: bool,
-
-    pub is_internal: bool,
-
     /// True if this function was defined with the 'unconstrained' keyword
     pub is_unconstrained: bool,
 
@@ -406,18 +401,6 @@ pub enum FunctionReturnType {
     Ty(UnresolvedType),
 }
 
-/// Describes the types of smart contract functions that are allowed.
-/// - All Noir programs in the non-contract context can be seen as `Secret`.
-#[derive(Debug, Copy, Clone, PartialEq, Eq)]
-pub enum ContractFunctionType {
-    /// This function will be executed in a private
-    /// context.
-    Secret,
-    /// This function will be executed in a public
-    /// context.
-    Open,
-}
-
 #[derive(Debug, PartialEq, Eq, Clone)]
 pub enum ArrayLiteral {
     Standard(Vec<Expression>),
@@ -674,8 +657,6 @@ impl FunctionDefinition {
         FunctionDefinition {
             name: name.clone(),
             attributes: Attributes::empty(),
-            is_open: false,
-            is_internal: false,
             is_unconstrained: false,
             visibility: FunctionVisibility::Private,
             generics: generics.clone(),

noir/noir-repo/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs

@@ -411,8 +411,6 @@ impl<'a> ModCollector<'a> {
                             // TODO(Maddiaa): Investigate trait implementations with attributes see: https://github.com/noir-lang/noir/issues/2629
                             attributes: crate::token::Attributes::empty(),
                             is_unconstrained: false,
-                            contract_function_type: None,
-                            is_internal: None,
                         };
 
                         let location = Location::new(name.span(), self.file_id);

noir/noir-repo/compiler/noirc_frontend/src/hir/resolution/errors.rs

@@ -64,14 +64,10 @@ pub enum ResolverError {
     IncorrectGenericCount { span: Span, item_name: String, actual: usize, expected: usize },
     #[error("{0}")]
     ParserError(Box<ParserError>),
-    #[error("Function is not defined in a contract yet sets its contract visibility")]
-    ContractFunctionTypeInNormalFunction { span: Span },
     #[error("Cannot create a mutable reference to {variable}, it was declared to be immutable")]
     MutableReferenceToImmutableVariable { variable: String, span: Span },
     #[error("Mutable references to array indices are unsupported")]
     MutableReferenceToArrayElement { span: Span },
-    #[error("Function is not defined in a contract yet sets is_internal")]
-    ContractFunctionInternalInNormalFunction { 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")]
@@ -278,22 +274,12 @@ impl From<ResolverError> for Diagnostic {
                 )
             }
             ResolverError::ParserError(error) => (*error).into(),
-            ResolverError::ContractFunctionTypeInNormalFunction { span } => Diagnostic::simple_error(
-                "Only functions defined within contracts can set their contract function type".into(),
-                "Non-contract functions cannot be 'open'".into(),
-                span,
-            ),
             ResolverError::MutableReferenceToImmutableVariable { variable, span } => {
                 Diagnostic::simple_error(format!("Cannot mutably reference the immutable variable {variable}"), format!("{variable} is immutable"), span)
             },
             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::ContractFunctionInternalInNormalFunction { span } => Diagnostic::simple_error(
-                "Only functions defined within contracts can set their functions to be internal".into(),
-                "Non-contract functions cannot be 'internal'".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(),

noir/noir-repo/compiler/noirc_frontend/src/hir/resolution/resolver.rs

@@ -37,10 +37,10 @@ use crate::{
     StatementKind,
 };
 use crate::{
-    ArrayLiteral, ContractFunctionType, Distinctness, ForRange, FunctionDefinition,
-    FunctionReturnType, FunctionVisibility, Generics, LValue, NoirStruct, NoirTypeAlias, Param,
-    Path, PathKind, Pattern, Shared, StructType, Type, TypeAlias, TypeVariable, TypeVariableKind,
-    UnaryOp, UnresolvedGenerics, UnresolvedTraitConstraint, UnresolvedType, UnresolvedTypeData,
+    ArrayLiteral, Distinctness, ForRange, FunctionDefinition, FunctionReturnType,
+    FunctionVisibility, Generics, LValue, NoirStruct, NoirTypeAlias, Param, Path, PathKind,
+    Pattern, Shared, StructType, Type, TypeAlias, TypeVariable, TypeVariableKind, UnaryOp,
+    UnresolvedGenerics, UnresolvedTraitConstraint, UnresolvedType, UnresolvedTypeData,
     UnresolvedTypeExpression, Visibility, ERROR_IDENT,
 };
 use fm::FileId;
@@ -233,8 +233,6 @@ impl<'a> Resolver<'a> {
         let def = FunctionDefinition {
             name: name.clone(),
             attributes: Attributes::empty(),
-            is_open: false,
-            is_internal: false,
             is_unconstrained: false,
             visibility: FunctionVisibility::Public, // Trait functions are always public
             generics: Vec::new(),                   // self.generics should already be set
@@ -976,9 +974,6 @@ impl<'a> Resolver<'a> {
 
         self.interner.push_definition_type(name_ident.id, typ.clone());
 
-        self.handle_function_type(&func_id);
-        self.handle_is_function_internal(&func_id);
-
         FuncMeta {
             name: name_ident,
             kind: func.kind,
@@ -1018,34 +1013,14 @@ impl<'a> Resolver<'a> {
     /// True if the `distinct` keyword is allowed on a function's return type
     fn distinct_allowed(&self, func: &NoirFunction) -> bool {
         if self.in_contract {
-            // "open" and "unconstrained" functions are compiled to brillig and thus duplication of
+            // "unconstrained" functions are compiled to brillig and thus duplication of
             // witness indices in their abis is not a concern.
-            !func.def.is_unconstrained && !func.def.is_open
+            !func.def.is_unconstrained
         } else {
             func.name() == MAIN_FUNCTION
         }
     }
 
-    fn handle_function_type(&mut self, function: &FuncId) {
-        let function_type = self.interner.function_modifiers(function).contract_function_type;
-
-        if !self.in_contract && function_type == Some(ContractFunctionType::Open) {
-            let span = self.interner.function_ident(function).span();
-            self.errors.push(ResolverError::ContractFunctionTypeInNormalFunction { span });
-            self.interner.function_modifiers_mut(function).contract_function_type = None;
-        }
-    }
-
-    fn handle_is_function_internal(&mut self, function: &FuncId) {
-        if !self.in_contract {
-            if self.interner.function_modifiers(function).is_internal == Some(true) {
-                let span = self.interner.function_ident(function).span();
-                self.push_err(ResolverError::ContractFunctionInternalInNormalFunction { span });
-            }
-            self.interner.function_modifiers_mut(function).is_internal = None;
-        }
-    }
-
     fn declare_numeric_generics(&mut self, params: &[Type], return_type: &Type) {
         if self.generics.is_empty() {
             return;

noir/noir-repo/compiler/noirc_frontend/src/lexer/token.rs

@@ -664,7 +664,6 @@ pub enum Keyword {
     Let,
     Mod,
     Mut,
-    Open,
     Pub,
     Return,
     ReturnData,
@@ -706,7 +705,6 @@ 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::Return => write!(f, "return"),
             Keyword::ReturnData => write!(f, "return_data"),
@@ -751,7 +749,6 @@ impl Keyword {
             "let" => Keyword::Let,
             "mod" => Keyword::Mod,
             "mut" => Keyword::Mut,
-            "open" => Keyword::Open,
             "pub" => Keyword::Pub,
             "return" => Keyword::Return,
             "return_data" => Keyword::ReturnData,

noir/noir-repo/compiler/noirc_frontend/src/monomorphization/mod.rs

@@ -28,8 +28,8 @@ use crate::{
     },
     node_interner::{self, DefinitionKind, NodeInterner, StmtId, TraitImplKind, TraitMethodId},
     token::FunctionAttribute,
-    ContractFunctionType, FunctionKind, IntegerBitSize, Signedness, Type, TypeBinding,
-    TypeBindings, TypeVariable, TypeVariableKind, UnaryOp, Visibility,
+    FunctionKind, IntegerBitSize, Signedness, Type, TypeBinding, TypeBindings, TypeVariable,
+    TypeVariableKind, UnaryOp, Visibility,
 };
 
 use self::ast::{Definition, FuncId, Function, LocalId, Program};
@@ -310,8 +310,7 @@ impl<'interner> Monomorphizer<'interner> {
             Type::TraitAsType(..) => &body_return_type,
             _ => meta.return_type(),
         });
-        let unconstrained = modifiers.is_unconstrained
-            || matches!(modifiers.contract_function_type, Some(ContractFunctionType::Open));
+        let unconstrained = modifiers.is_unconstrained;
 
         let parameters = self.parameters(&meta.parameters);
         let body = self.expr(body_expr_id)?;