chore: add more lints related to oracle calls

compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs

@@ -281,7 +281,7 @@
     #[tracing::instrument(level = "trace", skip_all)]
     pub(crate) fn into_acir(self, brillig: &Brillig) -> Result<Artifacts, RuntimeError> {
         let mut acirs = Vec::new();
         // TODO: can we parallelise this?
         let mut shared_context = SharedContext::default();
         for function in self.functions.values() {
             let context = Context::new(&mut shared_context);
@@ -839,9 +839,10 @@
                         self.handle_ssa_call_outputs(result_ids, outputs, dfg)?;
                     }
                     Value::ForeignFunction(_) => {
+                        // TODO: Remove this once elaborator is default frontend. This is now caught by a lint inside the frontend.
                         return Err(RuntimeError::UnconstrainedOracleReturnToConstrained {
                             call_stack: self.acir_context.get_call_stack(),
-                        })
+                        });
                     }
                     _ => unreachable!("expected calling a function but got {function_value:?}"),
                 }

compiler/noirc_frontend/src/elaborator/lints.rs

@@ -10,7 +10,7 @@ use crate::{
         HirExpression, HirLiteral, NodeInterner, NoirFunction, UnaryOp, UnresolvedTypeData,
         Visibility,
     },
-    node_interner::{DefinitionKind, ExprId},
+    node_interner::{DefinitionKind, ExprId, FuncId},
     Type,
 };
 use acvm::AcirField;
@@ -72,6 +72,40 @@ pub(super) fn low_level_function_outside_stdlib(
     }
 }
 
+/// Oracle definitions (functions with the `#[oracle]` attribute) must be marked as unconstrained.
+pub(super) fn oracle_not_marked_unconstrained(func: &NoirFunction) -> Option<ResolverError> {
+    let is_oracle_function =
+        func.attributes().function.as_ref().map_or(false, |func| func.is_oracle());
+    if is_oracle_function && !func.def.is_unconstrained {
+        Some(ResolverError::OracleMarkedAsConstrained { ident: func.name_ident().clone() })
+    } else {
+        None
+    }
+}
+
+/// Oracle functions may not be called by constrained functions directly.
+///
+/// In order for a constrained function to call an oracle it must first call through an unconstrained function.
+pub(super) fn oracle_called_from_constrained_function(
+    interner: &NodeInterner,
+    called_func: &FuncId,
+    calling_from_constrained_runtime: bool,
+    span: Span,
+) -> Option<ResolverError> {
+    if !calling_from_constrained_runtime {
+        return None;
+    }
+
+    let function_attributes = interner.function_attributes(called_func);
+    let is_oracle_call =
+        function_attributes.function.as_ref().map_or(false, |func| func.is_oracle());
+    if is_oracle_call {
+        Some(ResolverError::UnconstrainedOracleReturnToConstrained { span })
+    } else {
+        None
+    }
+}
+
 /// `pub` is required on return types for entry point functions
 pub(super) fn missing_pub(func: &NoirFunction, is_entry_point: bool) -> Option<ResolverError> {
     if is_entry_point

compiler/noirc_frontend/src/elaborator/mod.rs

@@ -556,6 +556,7 @@ impl<'context> Elaborator<'context> {
         self.run_lint(|elaborator| {
             lints::unnecessary_pub_return(func, elaborator.pub_allowed(func)).map(Into::into)
         });
+        self.run_lint(|_| lints::oracle_not_marked_unconstrained(func).map(Into::into));
         self.run_lint(|elaborator| {
             lints::low_level_function_outside_stdlib(func, elaborator.crate_id).map(Into::into)
         });

compiler/noirc_frontend/src/elaborator/types.rs

@@ -17,7 +17,7 @@ use crate::{
     },
     hir_def::{
         expr::{
-            HirBinaryOp, HirCallExpression, HirIdent, HirMemberAccess, HirMethodReference,
+            HirBinaryOp, HirCallExpression, HirMemberAccess, HirMethodReference,
             HirPrefixExpression,
         },
         function::{FuncMeta, Parameters},
@@ -1150,6 +1150,19 @@ impl<'context> Elaborator<'context> {
         let is_unconstrained_call = self.is_unconstrained_call(call.func);
         let crossing_runtime_boundary = is_current_func_constrained && is_unconstrained_call;
         if crossing_runtime_boundary {
+            let called_func_id = self
+                .interner
+                .lookup_function_from_expr(&call.func)
+                .expect("Called function should exist");
+            self.run_lint(|elaborator| {
+                lints::oracle_called_from_constrained_function(
+                    elaborator.interner,
+                    &called_func_id,
+                    is_current_func_constrained,
+                    span,
+                )
+                .map(Into::into)
+            });
             let errors = lints::unconstrained_function_args(&args);
             for error in errors {
                 self.push_err(error);
@@ -1168,15 +1181,12 @@ impl<'context> Elaborator<'context> {
     }
 
     fn is_unconstrained_call(&self, expr: ExprId) -> bool {
-        if let HirExpression::Ident(HirIdent { id, .. }, _) = self.interner.expression(&expr) {
-            if let Some(DefinitionKind::Function(func_id)) =
-                self.interner.try_definition(id).map(|def| &def.kind)
-            {
-                let modifiers = self.interner.function_modifiers(func_id);
-                return modifiers.is_unconstrained;
-            }
+        if let Some(func_id) = self.interner.lookup_function_from_expr(&expr) {
+            let modifiers = self.interner.function_modifiers(&func_id);
+            modifiers.is_unconstrained
+        } else {
+            false
         }
-        false
     }
 
     /// Check if the given method type requires a mutable reference to the object type, and check

compiler/noirc_frontend/src/hir/resolution/errors.rs

@@ -80,6 +80,12 @@ pub enum ResolverError {
     AbiAttributeOutsideContract { span: Span },
     #[error("Usage of the `#[foreign]` or `#[builtin]` function attributes are not allowed outside of the Noir standard library")]
     LowLevelFunctionOutsideOfStdlib { ident: Ident },
+    #[error(
+        "Usage of the `#[oracle]` function attribute is only valid on unconstrained functions"
+    )]
+    OracleMarkedAsConstrained { ident: Ident },
+    #[error("Oracle functions cannot be called directly from constrained functions")]
+    UnconstrainedOracleReturnToConstrained { span: Span },
     #[error("Dependency cycle found, '{item}' recursively depends on itself: {cycle} ")]
     DependencyCycle { span: Span, item: String, cycle: String },
     #[error("break/continue are only allowed in unconstrained functions")]
@@ -327,6 +333,16 @@ impl<'a> From<&'a ResolverError> for Diagnostic {
                 "Usage of the `#[foreign]` or `#[builtin]` function attributes are not allowed outside of the Noir standard library".into(),
                 ident.span(),
             ),
+            ResolverError::OracleMarkedAsConstrained { ident } => Diagnostic::simple_error(
+                error.to_string(),
+                "Oracle functions must have the `unconstrained` keyword applied".into(),
+                ident.span(),
+            ),
+            ResolverError::UnconstrainedOracleReturnToConstrained { span } => Diagnostic::simple_error(
+                error.to_string(),
+                "This oracle call must be wrapped in a call to another unconstrained function before being returned to a constrained runtime".into(),
+                *span,
+            ),
             ResolverError::DependencyCycle { span, item, cycle } => {
                 Diagnostic::simple_error(
                     "Dependency cycle found".into(),

compiler/noirc_frontend/src/node_interner.rs

@@ -7,9 +7,9 @@
 use iter_extended::vecmap;
 use noirc_arena::{Arena, Index};
 use noirc_errors::{Location, Span, Spanned};
 use petgraph::algo::tarjan_scc;
 use petgraph::prelude::DiGraph;
 use petgraph::prelude::NodeIndex as PetGraphIndex;
 
 use crate::ast::Ident;
 use crate::graph::CrateId;
@@ -20,6 +20,7 @@
 
 use crate::ast::{BinaryOpKind, FunctionDefinition, ItemVisibility};
 use crate::hir::resolution::errors::ResolverError;
+use crate::hir_def::expr::HirIdent;
 use crate::hir_def::stmt::HirLetStatement;
 use crate::hir_def::traits::TraitImpl;
 use crate::hir_def::traits::{Trait, TraitConstraint};
@@ -62,7 +63,7 @@
     function_modules: HashMap<FuncId, ModuleId>,
 
     /// This graph tracks dependencies between different global definitions.
     /// This is used to ensure the absense of dependency cycles for globals and types.
     dependency_graph: DiGraph<DependencyId, ()>,
 
     /// To keep track of where each DependencyId is in `dependency_graph`, we need
@@ -479,7 +480,7 @@
             function_modifiers: HashMap::new(),
             function_modules: HashMap::new(),
             func_id_to_trait: HashMap::new(),
             dependency_graph: petgraph::graph::DiGraph::new(),
             dependency_graph_indices: HashMap::new(),
             id_to_location: HashMap::new(),
             definitions: vec![],
@@ -824,6 +825,21 @@
         self.function_modules[&func]
     }
 
+    /// Returns the [`FuncId`] corresponding to the function referred to by `expr_id`
+    pub fn lookup_function_from_expr(&self, expr: &ExprId) -> Option<FuncId> {
+        if let HirExpression::Ident(HirIdent { id, .. }, _) = self.expression(expr) {
+            if let Some(DefinitionKind::Function(func_id)) =
+                self.try_definition(id).map(|def| &def.kind)
+            {
+                Some(*func_id)
+            } else {
+                None
+            }
+        } else {
+            None
+        }
+    }
+
     /// Returns the interned HIR function corresponding to `func_id`
     //
     // Cloning HIR structures is cheap, so we return owned structures
@@ -1640,7 +1656,7 @@
     }
 
     pub(crate) fn check_for_dependency_cycles(&self) -> Vec<(CompilationError, FileId)> {
         let strongly_connected_components = tarjan_scc(&self.dependency_graph);
         let mut errors = Vec::new();
 
         let mut push_error = |item: String, scc: &[_], i, location: Location| {

test_programs/execution_success/schnorr/src/main.nr

@@ -30,7 +30,7 @@ fn main(
     let pub_key = embedded_curve_ops::EmbeddedCurvePoint { x: pub_key_x, y: pub_key_y, is_infinite: false };
     let valid_signature = verify_signature_noir(pub_key, signature, message2);
     assert(valid_signature);
-    assert_valid_signature(pub_key,signature,message2);
+    assert_valid_signature(pub_key, signature, message2);
 }
 
 // TODO: to put in the stdlib once we have numeric generics