diff --git a/acir/src/circuit/brillig.rs b/acir/src/circuit/brillig.rs index e37ef2948..efc381675 100644 --- a/acir/src/circuit/brillig.rs +++ b/acir/src/circuit/brillig.rs @@ -22,8 +22,10 @@ pub enum BrilligOutputs { pub struct Brillig { pub inputs: Vec, pub outputs: Vec, - /// 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, + /// The Brillig VM bytecode to be executed by this ACIR opcode. pub bytecode: Vec, /// Predicate of the Brillig execution - indicates if it should be skipped pub predicate: Option, diff --git a/acvm/src/pwg/mod.rs b/acvm/src/pwg/mod.rs index aa23a3be2..31da6fa31 100644 --- a/acvm/src/pwg/mod.rs +++ b/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, @@ -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[..]); diff --git a/brillig_vm/src/lib.rs b/brillig_vm/src/lib.rs index 4d7e6cac2..4f95b5e21 100644 --- a/brillig_vm/src/lib.rs +++ b/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, } +impl From> for ForeignCallResult { + fn from(values: Vec) -> Self { + ForeignCallResult { values } + } +} + #[derive(Debug, PartialEq, Eq, Clone)] /// VM encapsulates the state of the Brillig VM during execution. pub struct VM {