chore: add more lints related to oracle calls (#5193)

# Description

## Problem\*

Resolves <!-- Link to GitHub Issue -->

## Summary\*

This PR adds a new lint inspired by the review of
AztecProtocol/aztec-packages#6751 which enforces
that oracle functions must be marked as unconstrained.

I've also moved the error for calling an oracle function from a
constrained function into the frontend rather than waiting until ACIR
gen.

## Additional Context



## Documentation\*

Check one:
- [x] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[For Experimental Features]** Documentation to be submitted in a
separate PR.

# PR Checklist\*

- [x] I have tested the changes locally.
- [x] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.

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

@@ -839,9 +839,10 @@ impl<'a> Context<'a> {
                         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

@@ -565,6 +565,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},
@@ -1155,6 +1155,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);
@@ -1173,15 +1186,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

@@ -20,6 +20,7 @@ use crate::hir::def_map::{LocalModuleId, ModuleId};
 
 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};
@@ -824,6 +825,21 @@ impl NodeInterner {
         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