feat: add method to generate the new Brillig opcode from UnresolvedBrilligCall

acir/src/circuit/brillig.rs

@@ -22,8 +22,10 @@ pub enum BrilligOutputs {
 pub struct Brillig {
     pub inputs: Vec<BrilligInputs>,
     pub outputs: Vec<BrilligOutputs>,
-    /// Results of oracles/functions external to brillig like a database read
+    /// Results of oracles/functions external to brillig like a database read.
+    // Each element of this vector corresponds to a single foreign call but may contain several values.
     pub foreign_call_results: Vec<ForeignCallResult>,
+    /// The Brillig VM bytecode to be executed by this ACIR opcode.
     pub bytecode: Vec<brillig_vm::Opcode>,
     /// Predicate of the Brillig execution - indicates if it should be skipped
     pub predicate: Option<Expression>,

acvm/src/pwg/mod.rs

@@ -2,6 +2,7 @@
 
 use crate::{Language, PartialWitnessGenerator};
 use acir::{
+    brillig_vm::ForeignCallResult,
     circuit::brillig::Brillig,
     circuit::opcodes::{Opcode, OracleData},
     native_types::{Expression, Witness, WitnessMap},
@@ -36,7 +37,12 @@ pub enum PartialWitnessGeneratorStatus {
 
     /// The `PartialWitnessGenerator` has encountered a request for [oracle data][Opcode::Oracle] or a Brillig [foreign call][acir::brillig_vm::Opcode::ForeignCall].
     ///
-    /// The caller must resolve these opcodes externally and insert the results into the intermediate witness.
+    /// Both of these opcodes require information from outside of the ACVM to be inserted before restarting execution.
+    /// [`Opcode::Oracle`] and [`Opcode::Brillig`] opcodes require the return values to be inserted slightly differently.
+    /// `Oracle` opcodes expect their return values to be written directly into the witness map whereas a `Brillig` foreign call
+    /// result is inserted into the `Brillig` opcode which made the call using [`UnresolvedBrilligCall::resolve`].
+    /// (Note: this means that the updated opcode must then be passed back into the ACVM to be processed further.)
+    ///
     /// Once this is done, the `PartialWitnessGenerator` can be restarted to solve the remaining opcodes.
     RequiresOracleData {
         required_oracle_data: Vec<OracleData>,
@@ -253,16 +259,25 @@ pub fn insert_value(
 #[derive(Debug, PartialEq, Clone)]
 pub struct UnresolvedBrilligCall {
     /// The current Brillig VM process that has been paused.
-    /// This process will be updated by the caller after resolving a foreign call's outputs.
+    /// This process will be updated by the caller after resolving a foreign call's result.
     ///
-    /// The [foreign call's result][acir::brillig_vm::ForeignCallResult] should be appended to this current [Brillig call][Brillig].
-    /// The [PartialWitnessGenerator] can then be restarted with an updated [Brillig opcode][Opcode::Brillig]
-    /// to solve the remaining Brillig VM process as well as the remaining ACIR opcodes.
+    /// This can be done using [`UnresolvedBrilligCall::resolve`].
     pub brillig: Brillig,
     /// Inputs for a pending foreign call required to restart bytecode processing.
     pub foreign_call_wait_info: brillig::ForeignCallWaitInfo,
 }
 
+impl UnresolvedBrilligCall {
+    /// Inserts the [foreign call's result][acir::brillig_vm::ForeignCallResult] into the calling [`Brillig` opcode][Brillig].
+    ///
+    /// The [ACVM][solve] can then be restarted with the updated [Brillig opcode][Opcode::Brillig]
+    /// to solve the remaining Brillig VM process as well as the remaining ACIR opcodes.
+    pub fn resolve(mut self, foreign_call_result: ForeignCallResult) -> Brillig {
+        self.brillig.foreign_call_results.push(foreign_call_result);
+        self.brillig
+    }
+}
+
 #[deprecated(
     note = "For backwards compatibility, this method allows you to derive _sensible_ defaults for opcode support based on the np language. \n Backends should simply specify what they support."
 )]
@@ -295,9 +310,7 @@ mod tests {
     use std::collections::BTreeMap;
 
     use acir::{
-        brillig_vm::{
-            self, BinaryFieldOp, ForeignCallResult, RegisterIndex, RegisterValueOrArray, Value,
-        },
+        brillig_vm::{self, BinaryFieldOp, RegisterIndex, RegisterValueOrArray, Value},
         circuit::{
             brillig::{Brillig, BrilligInputs, BrilligOutputs},
             directives::Directive,
@@ -309,10 +322,7 @@ mod tests {
     };
 
     use crate::{
-        pwg::{
-            self, block::Blocks, OpcodeResolution, PartialWitnessGeneratorStatus,
-            UnresolvedBrilligCall,
-        },
+        pwg::{self, block::Blocks, OpcodeResolution, PartialWitnessGeneratorStatus},
         OpcodeResolutionError, PartialWitnessGenerator,
     };
 
@@ -518,14 +528,17 @@ mod tests {
         assert_eq!(unsolved_opcodes.len(), 0, "brillig should have been removed");
         assert_eq!(unresolved_brillig_calls.len(), 1, "should have a brillig oracle request");
 
-        let UnresolvedBrilligCall { foreign_call_wait_info, mut brillig } =
-            unresolved_brillig_calls.remove(0);
-        assert_eq!(foreign_call_wait_info.inputs.len(), 1, "Should be waiting for a single input");
+        let foreign_call = unresolved_brillig_calls.remove(0);
+        assert_eq!(
+            foreign_call.foreign_call_wait_info.inputs.len(),
+            1,
+            "Should be waiting for a single input"
+        );
         // As caller of VM, need to resolve foreign calls
+        let foreign_call_result =
+            vec![Value::from(foreign_call.foreign_call_wait_info.inputs[0].to_field().inverse())];
         // Alter Brillig oracle opcode with foreign call resolution
-        brillig.foreign_call_results.push(ForeignCallResult {
-            values: vec![Value::from(foreign_call_wait_info.inputs[0].to_field().inverse())],
-        });
+        let brillig: Brillig = foreign_call.resolve(foreign_call_result.into());
         let mut next_opcodes_for_solving = vec![Opcode::Brillig(brillig)];
         next_opcodes_for_solving.extend_from_slice(&unsolved_opcodes[..]);
         // After filling data request, continue solving
@@ -649,15 +662,16 @@ mod tests {
         assert_eq!(unsolved_opcodes.len(), 0, "brillig should have been removed");
         assert_eq!(unresolved_brillig_calls.len(), 1, "should have a brillig oracle request");
 
-        let UnresolvedBrilligCall { foreign_call_wait_info, mut brillig } =
-            unresolved_brillig_calls.remove(0);
-        assert_eq!(foreign_call_wait_info.inputs.len(), 1, "Should be waiting for a single input");
+        let foreign_call = unresolved_brillig_calls.remove(0);
+        assert_eq!(
+            foreign_call.foreign_call_wait_info.inputs.len(),
+            1,
+            "Should be waiting for a single input"
+        );
 
-        let x_plus_y_inverse = foreign_call_wait_info.inputs[0].to_field().inverse();
+        let x_plus_y_inverse = foreign_call.foreign_call_wait_info.inputs[0].to_field().inverse();
         // Alter Brillig oracle opcode
-        brillig
-            .foreign_call_results
-            .push(ForeignCallResult { values: vec![Value::from(x_plus_y_inverse)] });
+        let brillig: Brillig = foreign_call.resolve(vec![Value::from(x_plus_y_inverse)].into());
 
         let mut next_opcodes_for_solving = vec![Opcode::Brillig(brillig)];
         next_opcodes_for_solving.extend_from_slice(&unsolved_opcodes[..]);
@@ -672,16 +686,18 @@ mod tests {
         assert!(unsolved_opcodes.is_empty(), "should be fully solved");
         assert_eq!(unresolved_brillig_calls.len(), 1, "should have no unresolved oracles");
 
-        let UnresolvedBrilligCall { foreign_call_wait_info, mut brillig } =
-            unresolved_brillig_calls.remove(0);
-        assert_eq!(foreign_call_wait_info.inputs.len(), 1, "Should be waiting for a single input");
+        let foreign_call = unresolved_brillig_calls.remove(0);
+        assert_eq!(
+            foreign_call.foreign_call_wait_info.inputs.len(),
+            1,
+            "Should be waiting for a single input"
+        );
 
-        let i_plus_j_inverse = foreign_call_wait_info.inputs[0].to_field().inverse();
+        let i_plus_j_inverse = foreign_call.foreign_call_wait_info.inputs[0].to_field().inverse();
         assert_ne!(x_plus_y_inverse, i_plus_j_inverse);
         // Alter Brillig oracle opcode
-        brillig
-            .foreign_call_results
-            .push(ForeignCallResult { values: vec![Value::from(i_plus_j_inverse)] });
+        let brillig = foreign_call.resolve(vec![Value::from(i_plus_j_inverse)].into());
+
         let mut next_opcodes_for_solving = vec![Opcode::Brillig(brillig)];
         next_opcodes_for_solving.extend_from_slice(&unsolved_opcodes[..]);
 

brillig_vm/src/lib.rs

@@ -38,12 +38,21 @@ pub enum VMStatus {
     },
 }
 
+/// Represents the output of a [foreign call][Opcode::ForeignCall].
+///
+/// See [`VMStatus::ForeignCallWait`] for more information.
 #[derive(Debug, PartialEq, Eq, Serialize, Deserialize, Clone)]
 pub struct ForeignCallResult {
-    /// Resolved foreign call values
+    /// Resolved output values of the foreign call.
     pub values: Vec<Value>,
 }
 
+impl From<Vec<Value>> for ForeignCallResult {
+    fn from(values: Vec<Value>) -> Self {
+        ForeignCallResult { values }
+    }
+}
+
 #[derive(Debug, PartialEq, Eq, Clone)]
 /// VM encapsulates the state of the Brillig VM during execution.
 pub struct VM {