Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Allow arbitrary noir functions to be unconstrained #1044

Merged
merged 3 commits into from
Mar 28, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 29 additions & 3 deletions crates/noirc_driver/src/contract.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
use std::collections::BTreeMap;

use noirc_frontend::ContractVisibility;

use crate::CompiledProgram;

/// Each function in the contract will be compiled
Expand All @@ -12,10 +10,28 @@ use crate::CompiledProgram;
/// One of these being a function type.
#[derive(serde::Serialize, serde::Deserialize)]
pub struct ContractFunction {
pub func_type: ContractVisibility,
pub func_type: ContractFunctionType,
pub function: CompiledProgram,
}

/// 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(serde::Serialize, serde::Deserialize, Debug, Copy, Clone, PartialEq, Eq)]
#[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.
Open,
/// This function cannot constrain any values and can use nondeterministic features
/// like arrays of a dynamic size.
Unconstrained,
}

#[derive(serde::Serialize, serde::Deserialize)]
pub struct CompiledContract {
/// The name of the contract.
Expand All @@ -24,3 +40,13 @@ pub struct CompiledContract {
/// stored in this `BTreeMap`.
pub functions: BTreeMap<String, ContractFunction>,
}

impl ContractFunctionType {
pub 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,
}
}
}
4 changes: 3 additions & 1 deletion crates/noirc_driver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

use acvm::Language;
use clap::Args;
use contract::ContractFunction;
use contract::{ContractFunction, ContractFunctionType};
use fm::FileType;
use iter_extended::{try_btree_map, try_vecmap};
use noirc_abi::FunctionSignature;
Expand Down Expand Up @@ -210,6 +210,8 @@ impl Driver {
.contract_visibility
.expect("Expected contract function to have a contract visibility");

let func_type = ContractFunctionType::new(func_type, func_meta.is_unconstrained);

Ok((function_name, ContractFunction { func_type, function }))
})?;

Expand Down
15 changes: 6 additions & 9 deletions crates/noirc_frontend/src/ast/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -313,9 +313,11 @@ pub struct FunctionDefinition {
// 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>,
/// True if this function was defined with the 'open' keyword
pub is_open: bool,

/// True if this function was defined with the 'unconstrained' keyword
pub is_unconstrained: bool,

pub generics: UnresolvedGenerics,
pub parameters: Vec<(Pattern, UnresolvedType, noirc_abi::AbiVisibility)>,
Expand All @@ -327,20 +329,15 @@ pub struct FunctionDefinition {

/// 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 {
pub enum ContractFunctionType {
/// 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)]
Expand Down
4 changes: 2 additions & 2 deletions crates/noirc_frontend/src/hir/resolution/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -245,8 +245,8 @@ 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(),
"Only functions defined within contracts can set their contract function type".into(),
"Non-contract functions cannot be 'open'".into(),
span,
),
}
Expand Down
30 changes: 14 additions & 16 deletions crates/noirc_frontend/src/hir/resolution/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ use crate::{
Statement,
};
use crate::{
ArrayLiteral, ContractVisibility, Generics, LValue, NoirStruct, Path, Pattern, Shared,
ArrayLiteral, ContractFunctionType, Generics, LValue, NoirStruct, Path, Pattern, Shared,
StructType, Type, TypeBinding, TypeVariable, UnresolvedGenerics, UnresolvedType,
UnresolvedTypeExpression, ERROR_IDENT,
};
Expand Down Expand Up @@ -636,6 +636,7 @@ impl<'a> Resolver<'a> {
kind: func.kind,
attributes,
contract_visibility: self.handle_contract_visibility(func),
is_unconstrained: func.def.is_unconstrained,
location,
typ,
parameters: parameters.into(),
Expand All @@ -644,22 +645,19 @@ 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(),
})
fn handle_contract_visibility(&mut self, func: &NoirFunction) -> Option<ContractFunctionType> {
jfecher marked this conversation as resolved.
Show resolved Hide resolved
if func.def.is_open {
if self.in_contract() {
Some(ContractFunctionType::Open)
} else {
self.push_err(ResolverError::ContractVisibilityInNormalFunction {
jfecher marked this conversation as resolved.
Show resolved Hide resolved
span: func.name_ident().span(),
});
None
}
} else {
Some(ContractFunctionType::Secret)
}

contract_visibility
}

fn declare_numeric_generics(&mut self, params: &[Type], return_type: &Type) {
Expand Down
1 change: 1 addition & 0 deletions crates/noirc_frontend/src/hir/type_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@ mod test {
attributes: None,
location,
contract_visibility: None,
is_unconstrained: false,
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),
Expand Down
6 changes: 4 additions & 2 deletions crates/noirc_frontend/src/hir_def/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use super::expr::{HirBlockExpression, HirExpression, HirIdent};
use super::stmt::HirPattern;
use crate::node_interner::{ExprId, NodeInterner};
use crate::{token::Attribute, FunctionKind};
use crate::{ContractVisibility, Type};
use crate::{ContractFunctionType, Type};

/// A Hir function is a block expression
/// with a list of statements
Expand Down Expand Up @@ -123,7 +123,9 @@ pub struct FuncMeta {

/// 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 contract_visibility: Option<ContractFunctionType>,

pub is_unconstrained: bool,

pub parameters: Parameters,

Expand Down
6 changes: 3 additions & 3 deletions crates/noirc_frontend/src/lexer/token.rs
Original file line number Diff line number Diff line change
Expand Up @@ -437,7 +437,7 @@ pub enum Keyword {
String,
Return,
Struct,
Unsafe,
Unconstrained,
Use,
While,
}
Expand Down Expand Up @@ -469,7 +469,7 @@ impl fmt::Display for Keyword {
Keyword::String => write!(f, "str"),
Keyword::Return => write!(f, "return"),
Keyword::Struct => write!(f, "struct"),
Keyword::Unsafe => write!(f, "unsafe"),
Keyword::Unconstrained => write!(f, "unconstrained"),
Keyword::Use => write!(f, "use"),
Keyword::While => write!(f, "while"),
}
Expand Down Expand Up @@ -504,7 +504,7 @@ impl Keyword {
"str" => Keyword::String,
"return" => Keyword::Return,
"struct" => Keyword::Struct,
"unsafe" => Keyword::Unsafe,
"unconstrained" => Keyword::Unconstrained,
"use" => Keyword::Use,
"while" => Keyword::While,

Expand Down
28 changes: 14 additions & 14 deletions crates/noirc_frontend/src/parser/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,9 @@ 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, ContractVisibility,
FunctionDefinition, Ident, IfExpression, ImportStatement, InfixExpression, LValue, Lambda,
NoirFunction, NoirImpl, NoirStruct, Path, PathKind, Pattern, Recoverable, UnaryOp,
UnresolvedTypeExpression,
BinaryOp, BinaryOpKind, BlockExpression, CompTime, ConstrainStatement, FunctionDefinition,
Ident, IfExpression, ImportStatement, InfixExpression, LValue, Lambda, NoirFunction, NoirImpl,
NoirStruct, Path, PathKind, Pattern, Recoverable, UnaryOp, UnresolvedTypeExpression,
};

use chumsky::prelude::*;
Expand Down Expand Up @@ -162,7 +161,7 @@ fn function_definition(allow_self: bool) -> impl NoirParser<NoirFunction> {
.map(
|(
(
((((attribute, contract_visibility), name), generics), parameters),
((((attribute, (is_unconstrained, is_open)), name), generics), parameters),
(return_visibility, return_type),
),
body,
Expand All @@ -171,7 +170,8 @@ fn function_definition(allow_self: bool) -> impl NoirParser<NoirFunction> {
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,
is_open,
is_unconstrained,
generics,
parameters,
body,
Expand All @@ -183,14 +183,14 @@ fn function_definition(allow_self: bool) -> impl NoirParser<NoirFunction> {
)
}

/// contract_visibility: 'open' | 'unsafe' | %empty
fn contract_visibility() -> impl NoirParser<Option<ContractVisibility>> {
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"),
})
/// contract_visibility: 'open' | 'unconstrained' | %empty
///
/// returns (is_unconstrained, is_open) for whether each keyword was present
fn contract_visibility() -> impl NoirParser<(bool, bool)> {
jfecher marked this conversation as resolved.
Show resolved Hide resolved
keyword(Keyword::Unconstrained)
.or_not()
.then(keyword(Keyword::Open).or_not())
.map(|(unconstrained, open)| (unconstrained.is_some(), open.is_some()))
}

/// non_empty_ident_list: ident ',' non_empty_ident_list
Expand Down