Skip to content
This repository has been archived by the owner on Apr 9, 2024. It is now read-only.

feat!: ACVM no longer takes ownership over opcodes #558

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all 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
4 changes: 0 additions & 4 deletions acir/src/circuit/brillig.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
use crate::native_types::{Expression, Witness};
use brillig::ForeignCallResult;
use brillig::Opcode as BrilligOpcode;
use serde::{Deserialize, Serialize};

Expand All @@ -23,9 +22,6 @@ 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.
// 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<BrilligOpcode>,
/// Predicate of the Brillig execution - indicates if it should be skipped
Expand Down
27 changes: 11 additions & 16 deletions acir/tests/test_program_serialization.rs
Original file line number Diff line number Diff line change
Expand Up @@ -181,8 +181,6 @@ fn simple_brillig_foreign_call() {
outputs: vec![
BrilligOutputs::Simple(w_inverted), // Output Register 1
],
// stack of foreign call/oracle resolutions, starts empty
foreign_call_results: vec![],
bytecode: vec![brillig::Opcode::ForeignCall {
function: "invert".into(),
destinations: vec![RegisterOrMemory::RegisterIndex(RegisterIndex::from(0))],
Expand All @@ -203,11 +201,10 @@ fn simple_brillig_foreign_call() {
circuit.write(&mut bytes).unwrap();

let expected_serialization: Vec<u8> = vec![
31, 139, 8, 0, 0, 0, 0, 0, 0, 255, 173, 143, 81, 10, 0, 16, 16, 68, 199, 42, 57, 14, 55,
112, 25, 31, 126, 124, 72, 206, 79, 161, 86, 225, 135, 87, 219, 78, 187, 53, 205, 104, 0,
2, 29, 201, 52, 103, 222, 220, 216, 230, 13, 43, 254, 121, 25, 158, 151, 54, 153, 117, 27,
53, 116, 136, 197, 167, 124, 107, 184, 64, 236, 73, 56, 83, 1, 18, 139, 122, 157, 67, 1, 0,
0,
31, 139, 8, 0, 0, 0, 0, 0, 0, 255, 173, 143, 49, 10, 64, 33, 12, 67, 99, 63, 124, 60, 142,
222, 192, 203, 56, 184, 56, 136, 120, 126, 5, 21, 226, 160, 139, 62, 40, 13, 45, 132, 68,
3, 80, 232, 124, 164, 153, 121, 115, 99, 155, 59, 172, 122, 231, 101, 56, 175, 80, 86, 221,
230, 31, 58, 196, 226, 83, 62, 53, 91, 16, 122, 10, 246, 84, 99, 243, 0, 30, 59, 1, 0, 0,
];

assert_eq!(bytes, expected_serialization)
Expand Down Expand Up @@ -248,8 +245,6 @@ fn complex_brillig_foreign_call() {
BrilligOutputs::Simple(a_plus_b_plus_c), // Output Register 1
BrilligOutputs::Simple(a_plus_b_plus_c_times_2), // Output Register 2
],
// stack of foreign call/oracle resolutions, starts empty
foreign_call_results: vec![],
bytecode: vec![
// Oracles are named 'foreign calls' in brillig
brillig::Opcode::ForeignCall {
Expand Down Expand Up @@ -280,13 +275,13 @@ fn complex_brillig_foreign_call() {
circuit.write(&mut bytes).unwrap();

let expected_serialization: Vec<u8> = vec![
31, 139, 8, 0, 0, 0, 0, 0, 0, 255, 213, 83, 219, 10, 128, 48, 8, 245, 210, 101, 159, 179,
254, 160, 127, 137, 222, 138, 122, 236, 243, 27, 228, 64, 44, 232, 33, 7, 237, 128, 56,
157, 147, 131, 103, 6, 0, 64, 184, 192, 201, 72, 206, 40, 177, 70, 174, 27, 197, 199, 111,
24, 208, 175, 87, 44, 197, 145, 42, 224, 200, 5, 56, 230, 255, 240, 83, 189, 61, 117, 113,
157, 31, 63, 236, 79, 147, 172, 77, 214, 73, 220, 139, 15, 106, 214, 168, 114, 249, 126,
218, 214, 125, 153, 15, 54, 37, 90, 26, 155, 39, 227, 95, 223, 232, 230, 4, 247, 157, 215,
56, 1, 153, 86, 63, 138, 44, 4, 0, 0,
31, 139, 8, 0, 0, 0, 0, 0, 0, 255, 213, 83, 219, 10, 128, 48, 8, 117, 174, 139, 159, 179,
254, 160, 127, 137, 222, 138, 122, 236, 243, 19, 114, 32, 22, 244, 144, 131, 118, 64, 156,
178, 29, 14, 59, 74, 0, 16, 224, 66, 228, 64, 57, 7, 169, 53, 242, 189, 81, 114, 250, 134,
33, 248, 113, 165, 82, 26, 177, 2, 141, 177, 128, 198, 60, 15, 63, 245, 219, 211, 23, 215,
255, 139, 15, 251, 211, 112, 180, 28, 157, 212, 189, 100, 82, 179, 64, 170, 63, 109, 235,
190, 204, 135, 166, 178, 150, 216, 62, 154, 252, 250, 70, 147, 35, 220, 119, 93, 227, 4,
182, 131, 81, 25, 36, 4, 0, 0,
];

assert_eq!(bytes, expected_serialization)
Expand Down
5 changes: 3 additions & 2 deletions acvm/src/pwg/brillig.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use acir::{
brillig::{RegisterIndex, Value},
brillig::{ForeignCallResult, RegisterIndex, Value},
circuit::{
brillig::{Brillig, BrilligInputs, BrilligOutputs},
OpcodeLocation,
Expand All @@ -20,6 +20,7 @@ impl BrilligSolver {
pub(super) fn solve<B: BlackBoxFunctionSolver>(
initial_witness: &mut WitnessMap,
brillig: &Brillig,
foreign_call_results: &[ForeignCallResult],
bb_solver: &B,
acir_index: usize,
) -> Result<Option<ForeignCallWaitInfo>, OpcodeResolutionError> {
Expand Down Expand Up @@ -80,7 +81,7 @@ impl BrilligSolver {
input_registers,
input_memory,
brillig.bytecode.clone(),
brillig.foreign_call_results.clone(),
foreign_call_results.to_vec(),
bb_solver,
);

Expand Down
30 changes: 20 additions & 10 deletions acvm/src/pwg/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ impl From<BlackBoxResolutionError> for OpcodeResolutionError {
}
}

pub struct ACVM<'backend, B: BlackBoxFunctionSolver> {
pub struct ACVM<'opcodes, 'backend, B: BlackBoxFunctionSolver> {
TomAFrench marked this conversation as resolved.
Show resolved Hide resolved
status: ACVMStatus,

backend: &'backend B,
Expand All @@ -135,15 +135,23 @@ pub struct ACVM<'backend, B: BlackBoxFunctionSolver> {
block_solvers: HashMap<BlockId, MemoryOpSolver>,

/// A list of opcodes which are to be executed by the ACVM.
opcodes: Vec<Opcode>,
opcodes: &'opcodes [Opcode],
/// Index of the next opcode to be executed.
instruction_pointer: usize,

witness_map: WitnessMap,

/// 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.
foreign_call_results: HashMap<usize, Vec<ForeignCallResult>>,
}

impl<'backend, B: BlackBoxFunctionSolver> ACVM<'backend, B> {
pub fn new(backend: &'backend B, opcodes: Vec<Opcode>, initial_witness: WitnessMap) -> Self {
impl<'opcodes, 'backend, B: BlackBoxFunctionSolver> ACVM<'opcodes, 'backend, B> {
pub fn new(
backend: &'backend B,
opcodes: &'opcodes [Opcode],
initial_witness: WitnessMap,
) -> Self {
let status = if opcodes.is_empty() { ACVMStatus::Solved } else { ACVMStatus::InProgress };
ACVM {
status,
Expand All @@ -152,6 +160,7 @@ impl<'backend, B: BlackBoxFunctionSolver> ACVM<'backend, B> {
opcodes,
instruction_pointer: 0,
witness_map: initial_witness,
foreign_call_results: HashMap::default(),
}
}

Expand All @@ -164,7 +173,7 @@ impl<'backend, B: BlackBoxFunctionSolver> ACVM<'backend, B> {

/// Returns a slice containing the opcodes of the circuit being executed.
pub fn opcodes(&self) -> &[Opcode] {
&self.opcodes
self.opcodes
}

/// Returns the index of the current opcode to be executed.
Expand Down Expand Up @@ -217,11 +226,9 @@ impl<'backend, B: BlackBoxFunctionSolver> ACVM<'backend, B> {
}

// We want to inject the foreign call result into the brillig opcode which initiated the call.
let opcode = &mut self.opcodes[self.instruction_pointer];
let Opcode::Brillig(brillig) = opcode else {
unreachable!("ACVM can only enter `RequiresForeignCall` state on a Brillig opcode");
};
brillig.foreign_call_results.push(foreign_call_result);
let foreign_call_results =
self.foreign_call_results.entry(self.instruction_pointer).or_default();
foreign_call_results.push(foreign_call_result);

// Now that the foreign call has been resolved then we can resume execution.
self.status(ACVMStatus::InProgress);
Expand Down Expand Up @@ -261,6 +268,9 @@ impl<'backend, B: BlackBoxFunctionSolver> ACVM<'backend, B> {
match BrilligSolver::solve(
&mut self.witness_map,
brillig,
self.foreign_call_results
.get(&self.instruction_pointer)
.unwrap_or(&Vec::default()),
self.backend,
self.instruction_pointer,
) {
Expand Down
24 changes: 7 additions & 17 deletions acvm/tests/solver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,6 @@ fn inversion_brillig_oracle_equivalence() {
BrilligOutputs::Simple(w_oracle), // Output Register 1
BrilligOutputs::Simple(w_equal_res), // Output Register 2
],
// stack of foreign call/oracle resolutions, starts empty
foreign_call_results: vec![],
bytecode: vec![
equal_opcode,
// Oracles are named 'foreign calls' in brillig
Expand Down Expand Up @@ -130,7 +128,7 @@ fn inversion_brillig_oracle_equivalence() {
])
.into();

let mut acvm = ACVM::new(&StubbedBackend, opcodes, witness_assignments);
let mut acvm = ACVM::new(&StubbedBackend, &opcodes, witness_assignments);
// use the partial witness generation solver with our acir program
let solver_status = acvm.solve();

Expand Down Expand Up @@ -213,8 +211,6 @@ fn double_inversion_brillig_oracle() {
BrilligOutputs::Simple(w_ij_oracle), // Output Register 3
BrilligOutputs::Simple(w_equal_res), // Output Register 4
],
// stack of foreign call/oracle resolutions, starts empty
foreign_call_results: vec![],
bytecode: vec![
equal_opcode,
// Oracles are named 'foreign calls' in brillig
Expand Down Expand Up @@ -260,7 +256,7 @@ fn double_inversion_brillig_oracle() {
])
.into();

let mut acvm = ACVM::new(&StubbedBackend, opcodes, witness_assignments);
let mut acvm = ACVM::new(&StubbedBackend, &opcodes, witness_assignments);

// use the partial witness generation solver with our acir program
let solver_status = acvm.solve();
Expand Down Expand Up @@ -339,8 +335,6 @@ fn oracle_dependent_execution() {
BrilligOutputs::Simple(w_y), // Output Register 2 - from input
BrilligOutputs::Simple(w_y_inv), // Output Register 3
],
// stack of foreign call/oracle resolutions, starts empty
foreign_call_results: vec![],
bytecode: vec![
// Oracles are named 'foreign calls' in brillig
BrilligOpcode::ForeignCall {
Expand Down Expand Up @@ -381,7 +375,7 @@ fn oracle_dependent_execution() {
let witness_assignments =
BTreeMap::from([(w_x, FieldElement::from(2u128)), (w_y, FieldElement::from(2u128))]).into();

let mut acvm = ACVM::new(&StubbedBackend, opcodes, witness_assignments);
let mut acvm = ACVM::new(&StubbedBackend, &opcodes, witness_assignments);

// use the partial witness generation solver with our acir program
let solver_status = acvm.solve();
Expand Down Expand Up @@ -468,8 +462,6 @@ fn brillig_oracle_predicate() {
},
],
predicate: Some(Expression::default()),
// oracle results
foreign_call_results: vec![],
});

let opcodes = vec![brillig_opcode];
Expand All @@ -480,7 +472,7 @@ fn brillig_oracle_predicate() {
])
.into();

let mut acvm = ACVM::new(&StubbedBackend, opcodes, witness_assignments);
let mut acvm = ACVM::new(&StubbedBackend, &opcodes, witness_assignments);
let solver_status = acvm.solve();
assert_eq!(solver_status, ACVMStatus::Solved, "should be fully solved");

Expand Down Expand Up @@ -513,7 +505,7 @@ fn unsatisfied_opcode_resolved() {
values.insert(d, FieldElement::from(2_i128));

let opcodes = vec![Opcode::Arithmetic(opcode_a)];
let mut acvm = ACVM::new(&StubbedBackend, opcodes, values);
let mut acvm = ACVM::new(&StubbedBackend, &opcodes, values);
let solver_status = acvm.solve();
assert_eq!(
solver_status,
Expand Down Expand Up @@ -569,8 +561,6 @@ fn unsatisfied_opcode_resolved_brillig() {
outputs: vec![BrilligOutputs::Simple(w_result)],
bytecode: vec![equal_opcode, jmp_if_opcode, trap_opcode, stop_opcode],
predicate: Some(Expression::one()),
// oracle results
foreign_call_results: vec![],
});

let opcode_a = Expression {
Expand All @@ -595,7 +585,7 @@ fn unsatisfied_opcode_resolved_brillig() {

let opcodes = vec![brillig_opcode, Opcode::Arithmetic(opcode_a)];

let mut acvm = ACVM::new(&StubbedBackend, opcodes, values);
let mut acvm = ACVM::new(&StubbedBackend, &opcodes, values);
let solver_status = acvm.solve();
assert_eq!(
solver_status,
Expand Down Expand Up @@ -639,7 +629,7 @@ fn memory_operations() {

let opcodes = vec![init, read_op, expression];

let mut acvm = ACVM::new(&StubbedBackend, opcodes, initial_witness);
let mut acvm = ACVM::new(&StubbedBackend, &opcodes, initial_witness);
let solver_status = acvm.solve();
assert_eq!(solver_status, ACVMStatus::Solved);
let witness_map = acvm.finalize();
Expand Down
20 changes: 10 additions & 10 deletions acvm/tests/stdlib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ macro_rules! test_uint_inner {
let uint = $uint::new(w);
let (w, extra_opcodes, _) = uint.rol(y, 2);
let witness_assignments = BTreeMap::from([(Witness(1), fe)]).into();
let mut acvm = ACVM::new(&StubbedBackend, extra_opcodes, witness_assignments);
let mut acvm = ACVM::new(&StubbedBackend, &extra_opcodes, witness_assignments);
let solver_status = acvm.solve();

prop_assert_eq!(acvm.witness_map().get(&w.get_inner()).unwrap(), &FieldElement::from(result as u128));
Expand All @@ -89,7 +89,7 @@ macro_rules! test_uint_inner {
let uint = $uint::new(w);
let (w, extra_opcodes, _) = uint.ror(y, 2);
let witness_assignments = BTreeMap::from([(Witness(1), fe)]).into();
let mut acvm = ACVM::new(&StubbedBackend, extra_opcodes, witness_assignments);
let mut acvm = ACVM::new(&StubbedBackend, &extra_opcodes, witness_assignments);
let solver_status = acvm.solve();

prop_assert_eq!(acvm.witness_map().get(&w.get_inner()).unwrap(), &FieldElement::from(result as u128));
Expand All @@ -109,7 +109,7 @@ macro_rules! test_uint_inner {
let u32_2 = $uint::new(w2);
let (q_w, r_w, extra_opcodes, _) = $uint::euclidean_division(&u32_1, &u32_2, 3);
let witness_assignments = BTreeMap::from([(Witness(1), lhs),(Witness(2), rhs)]).into();
let mut acvm = ACVM::new(&StubbedBackend, extra_opcodes, witness_assignments);
let mut acvm = ACVM::new(&StubbedBackend, &extra_opcodes, witness_assignments);
let solver_status = acvm.solve();

prop_assert_eq!(acvm.witness_map().get(&q_w.get_inner()).unwrap(), &FieldElement::from(q as u128));
Expand All @@ -135,7 +135,7 @@ macro_rules! test_uint_inner {
let (w2, extra_opcodes, _) = w.add(&u32_3, num_witness);
opcodes.extend(extra_opcodes);
let witness_assignments = BTreeMap::from([(Witness(1), lhs), (Witness(2), rhs), (Witness(3), rhs_z)]).into();
let mut acvm = ACVM::new(&StubbedBackend, opcodes, witness_assignments);
let mut acvm = ACVM::new(&StubbedBackend, &opcodes, witness_assignments);
let solver_status = acvm.solve();

prop_assert_eq!(acvm.witness_map().get(&w2.get_inner()).unwrap(), &result);
Expand All @@ -160,7 +160,7 @@ macro_rules! test_uint_inner {
let (w2, extra_opcodes, _) = w.sub(&u32_3, num_witness);
opcodes.extend(extra_opcodes);
let witness_assignments = BTreeMap::from([(Witness(1), lhs), (Witness(2), rhs), (Witness(3), rhs_z)]).into();
let mut acvm = ACVM::new(&StubbedBackend, opcodes, witness_assignments);
let mut acvm = ACVM::new(&StubbedBackend, &opcodes, witness_assignments);
let solver_status = acvm.solve();

prop_assert_eq!(acvm.witness_map().get(&w2.get_inner()).unwrap(), &result);
Expand All @@ -175,7 +175,7 @@ macro_rules! test_uint_inner {
let u32_1 = $uint::new(w1);
let (w, extra_opcodes, _) = u32_1.leftshift(y, 2);
let witness_assignments = BTreeMap::from([(Witness(1), lhs)]).into();
let mut acvm = ACVM::new(&StubbedBackend, extra_opcodes, witness_assignments);
let mut acvm = ACVM::new(&StubbedBackend, &extra_opcodes, witness_assignments);
let solver_status = acvm.solve();

prop_assert_eq!(acvm.witness_map().get(&w.get_inner()).unwrap(), &FieldElement::from(result as u128));
Expand All @@ -190,7 +190,7 @@ macro_rules! test_uint_inner {
let u32_1 = $uint::new(w1);
let (w, extra_opcodes, _) = u32_1.rightshift(y, 2);
let witness_assignments = BTreeMap::from([(Witness(1), lhs)]).into();
let mut acvm = ACVM::new(&StubbedBackend, extra_opcodes, witness_assignments);
let mut acvm = ACVM::new(&StubbedBackend, &extra_opcodes, witness_assignments);
let solver_status = acvm.solve();

prop_assert_eq!(acvm.witness_map().get(&w.get_inner()).unwrap(), &FieldElement::from(result as u128));
Expand All @@ -208,7 +208,7 @@ macro_rules! test_uint_inner {
let u32_2 = $uint::new(w2);
let (w, extra_opcodes, _) = u32_1.less_than_comparison(&u32_2, 3);
let witness_assignments = BTreeMap::from([(Witness(1), lhs), (Witness(2), rhs)]).into();
let mut acvm = ACVM::new(&StubbedBackend, extra_opcodes, witness_assignments);
let mut acvm = ACVM::new(&StubbedBackend, &extra_opcodes, witness_assignments);
let solver_status = acvm.solve();

prop_assert_eq!(acvm.witness_map().get(&w.get_inner()).unwrap(), &FieldElement::from(result as u128));
Expand Down Expand Up @@ -290,7 +290,7 @@ macro_rules! test_hashes {
let circuit = compile(circuit, Language::PLONKCSat{ width: 3 }, $opcode_support).unwrap().0;

// solve witnesses
let mut acvm = ACVM::new(&StubbedBackend, circuit.opcodes, witness_assignments.into());
let mut acvm = ACVM::new(&StubbedBackend, &circuit.opcodes, witness_assignments.into());
let solver_status = acvm.solve();

prop_assert_eq!(solver_status, ACVMStatus::Solved, "should be fully solved");
Expand Down Expand Up @@ -346,7 +346,7 @@ proptest! {
let circuit = compile(circuit, Language::PLONKCSat{ width: 3 }, does_not_support_hash_to_field).unwrap().0;

// solve witnesses
let mut acvm = ACVM::new(&StubbedBackend, circuit.opcodes, witness_assignments.into());
let mut acvm = ACVM::new(&StubbedBackend, &circuit.opcodes, witness_assignments.into());
let solver_status = acvm.solve();

prop_assert_eq!(solver_status, ACVMStatus::Solved, "should be fully solved");
Expand Down
Loading
Loading