Skip to content

Commit

Permalink
feat(acvm): Execute multiple circuits (AztecProtocol/aztec-packages#5380
Browse files Browse the repository at this point in the history
)

Resolves #4428

This is a followup to
AztecProtocol/aztec-packages#5341 which does the
initial ACIR generation work for multiple ACIR functions.

Execution is now done by moving `execute_circuit` to be part of a
stateful `ProgramExecutor` that builds a witness stack for every
completed `execute_circuit` call. An initial `execute_program` function
instantiates the `ProgramExecutor` and starts execution on our `main`
entry point circuit. When a `Call` opcode is reached we pause execution
and recursively call `execute_circuit`, which then returns the solved
witness for that call. We then resolve the outputs of that execution by
reading the return witnesses from the inner solved witness. We then push
the nested call solved witness onto the witness stack and continue
execution of our main ACVM instance. This is quite similar to the
process used by foreign calls with Brillig, except it is now done with
the main ACVM rather than the contained Brillig VM.

This witness stack and program (list of `Circuit` functions) then gets
passed to the backend. For now, I have only done an additive
`prove_and_verify_ultra_honk_program` to show the process working for
the basic non-inlined ACIR programs supplied here. I wanted to leave any
WASM exports or ACVM JS changes for a follow-up PR as they are quite a
bit of changes on their own.

---------

Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>
Co-authored-by: Tom French <tom@tomfren.ch>
Co-authored-by: jfecher <jfecher11@gmail.com>
  • Loading branch information
4 people committed Mar 29, 2024
2 parents 60155f2 + 9d150d9 commit e625c7e
Show file tree
Hide file tree
Showing 65 changed files with 1,231 additions and 371 deletions.
2 changes: 1 addition & 1 deletion .aztec-sync-commit
Original file line number Diff line number Diff line change
@@ -1 +1 @@
a066544cda095adda0c1dc7918c64ecad8656b91
bb719200034e3bc6db09fb56538dadca4203abf4
14 changes: 1 addition & 13 deletions .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -79,15 +79,6 @@ jobs:
- name: Install Yarn dependencies
uses: ./.github/actions/setup

- name: Install wasm-bindgen-cli
uses: taiki-e/install-action@v2
with:
tool: wasm-bindgen-cli@0.2.86

- name: Install wasm-opt
run: |
npm i wasm-opt -g
- name: Query new noir version
id: noir-version
run: |
Expand Down Expand Up @@ -116,20 +107,17 @@ jobs:
if: ${{ always() }}

needs:
- release-please
- update-acvm-workspace-package-versions
- update-docs

env:
# We treat any skipped or failing jobs as a failure for the workflow as a whole.
FAIL: ${{ contains(needs.*.result, 'failure') || contains(needs.*.result, 'cancelled') || contains(needs.*.result, 'skipped') }}
FAIL: ${{ contains(needs.*.result, 'failure') || contains(needs.*.result, 'cancelled') || contains(needs.*.result, 'skipped') }

steps:
- name: Add warning to sticky comment
uses: marocchino/sticky-pull-request-comment@v2
with:
# We need to specify the PR on which to make the comment as workflow is triggered by push.
number: ${{ fromJSON(needs.release-please.outputs.release-pr).number }}
# delete the comment in case failures have been fixed
delete: ${{ !env.FAIL }}
message: "The release workflow has not completed successfully. Releasing now will result in a broken release"
Expand Down
4 changes: 2 additions & 2 deletions acvm-repo/acir/src/circuit/opcodes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,8 @@ impl std::fmt::Display for Opcode {
}
Opcode::Call { id, inputs, outputs } => {
write!(f, "CALL func {}: ", id)?;
writeln!(f, "inputs: {:?}", inputs)?;
writeln!(f, "outputs: {:?}", outputs)
write!(f, "inputs: {:?}, ", inputs)?;
write!(f, "outputs: {:?}", outputs)
}
}
}
Expand Down
14 changes: 14 additions & 0 deletions acvm-repo/acir/src/native_types/witness_stack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,20 @@ pub struct StackItem {
pub witness: WitnessMap,
}

impl WitnessStack {
pub fn push(&mut self, index: u32, witness: WitnessMap) {
self.stack.push(StackItem { index, witness });
}

pub fn peek(&self) -> Option<&StackItem> {
self.stack.last()
}

pub fn length(&self) -> usize {
self.stack.len()
}
}

impl From<WitnessMap> for WitnessStack {
fn from(witness: WitnessMap) -> Self {
let stack = vec![StackItem { index: 0, witness }];
Expand Down
7 changes: 6 additions & 1 deletion acvm-repo/acvm/src/compiler/transformers/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,12 @@ pub(super) fn transform_internal(
new_acir_opcode_positions.push(acir_opcode_positions[index]);
transformed_opcodes.push(opcode);
}
Opcode::Call { .. } => todo!("Handle Call opcodes in the ACVM"),
Opcode::Call { .. } => {
// `Call` does not write values to the `WitnessMap`
// A separate ACIR function should have its own respective `WitnessMap`
new_acir_opcode_positions.push(acir_opcode_positions[index]);
transformed_opcodes.push(opcode);
}
}
}

Expand Down
96 changes: 95 additions & 1 deletion acvm-repo/acvm/src/pwg/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,12 @@ pub enum ACVMStatus {
///
/// Once this is done, the ACVM can be restarted to solve the remaining opcodes.
RequiresForeignCall(ForeignCallWaitInfo),

/// The ACVM has encountered a request for an ACIR [call][acir::circuit::Opcode]
/// to execute a separate ACVM instance. The result of the ACIR call must be passd back to the ACVM.

Check warning on line 54 in acvm-repo/acvm/src/pwg/mod.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (passd)
///
/// Once this is done, the ACVM can be restarted to solve the remaining opcodes.
RequiresAcirCall(AcirCallWaitInfo),
}

impl std::fmt::Display for ACVMStatus {
Expand All @@ -58,6 +64,7 @@ impl std::fmt::Display for ACVMStatus {
ACVMStatus::InProgress => write!(f, "In progress"),
ACVMStatus::Failure(_) => write!(f, "Execution failure"),
ACVMStatus::RequiresForeignCall(_) => write!(f, "Waiting on foreign call"),
ACVMStatus::RequiresAcirCall(_) => write!(f, "Waiting on acir call"),
}
}
}
Expand Down Expand Up @@ -117,6 +124,10 @@ pub enum OpcodeResolutionError {
BlackBoxFunctionFailed(BlackBoxFunc, String),
#[error("Failed to solve brillig function, reason: {message}")]
BrilligFunctionFailed { message: String, call_stack: Vec<OpcodeLocation> },
#[error("Attempted to call `main` with a `Call` opcode")]
AcirMainCallAttempted { opcode_location: ErrorLocation },
#[error("{results_size:?} result values were provided for {outputs_size:?} call output witnesses, most likely due to bad ACIR codegen")]
AcirCallOutputsMismatch { opcode_location: ErrorLocation, results_size: u32, outputs_size: u32 },
}

impl From<BlackBoxResolutionError> for OpcodeResolutionError {
Expand Down Expand Up @@ -147,6 +158,13 @@ pub struct ACVM<'a, B: BlackBoxFunctionSolver> {
witness_map: WitnessMap,

brillig_solver: Option<BrilligSolver<'a, B>>,

/// A counter maintained throughout an ACVM process that determines
/// whether the caller has resolved the results of an ACIR [call][Opcode::Call].
acir_call_counter: usize,
/// Represents the outputs of all ACIR calls during an ACVM process
/// List is appended onto by the caller upon reaching a [ACVMStatus::RequiresAcirCall]
acir_call_results: Vec<Vec<FieldElement>>,
}

impl<'a, B: BlackBoxFunctionSolver> ACVM<'a, B> {
Expand All @@ -161,6 +179,8 @@ impl<'a, B: BlackBoxFunctionSolver> ACVM<'a, B> {
instruction_pointer: 0,
witness_map: initial_witness,
brillig_solver: None,
acir_call_counter: 0,
acir_call_results: Vec::default(),
}
}

Expand Down Expand Up @@ -244,6 +264,29 @@ impl<'a, B: BlackBoxFunctionSolver> ACVM<'a, B> {
self.status(ACVMStatus::InProgress);
}

/// Sets the status of the VM to `RequiresAcirCall`
/// Indicating that the VM is now waiting for an ACIR call to be resolved
fn wait_for_acir_call(&mut self, acir_call: AcirCallWaitInfo) -> ACVMStatus {
self.status(ACVMStatus::RequiresAcirCall(acir_call))
}

/// Resolves an ACIR call's result (simply a list of fields) using a result calculated by a separate ACVM instance.
///
/// The current ACVM instance can then be restarted to solve the remaining ACIR opcodes.
pub fn resolve_pending_acir_call(&mut self, call_result: Vec<FieldElement>) {
if !matches!(self.status, ACVMStatus::RequiresAcirCall(_)) {
panic!("ACVM is not expecting an ACIR call response as no call was made");
}

if self.acir_call_counter < self.acir_call_results.len() {
panic!("No unresolved ACIR calls");
}
self.acir_call_results.push(call_result);

// Now that the ACIR call has been resolved then we can resume execution.
self.status(ACVMStatus::InProgress);
}

/// Executes the ACVM's circuit until execution halts.
///
/// Execution can halt due to three reasons:
Expand Down Expand Up @@ -281,7 +324,10 @@ impl<'a, B: BlackBoxFunctionSolver> ACVM<'a, B> {
Ok(Some(foreign_call)) => return self.wait_for_foreign_call(foreign_call),
res => res.map(|_| ()),
},
Opcode::Call { .. } => todo!("Handle Call opcodes in the ACVM"),
Opcode::Call { .. } => match self.solve_call_opcode() {
Ok(Some(input_values)) => return self.wait_for_acir_call(input_values),
res => res.map(|_| ()),
},
};
self.handle_opcode_resolution(resolution)
}
Expand Down Expand Up @@ -400,6 +446,46 @@ impl<'a, B: BlackBoxFunctionSolver> ACVM<'a, B> {
self.brillig_solver = Some(solver);
self.solve_opcode()
}

pub fn solve_call_opcode(&mut self) -> Result<Option<AcirCallWaitInfo>, OpcodeResolutionError> {
let Opcode::Call { id, inputs, outputs } = &self.opcodes[self.instruction_pointer] else {
unreachable!("Not executing a Call opcode");
};
if *id == 0 {
return Err(OpcodeResolutionError::AcirMainCallAttempted {
opcode_location: ErrorLocation::Resolved(OpcodeLocation::Acir(
self.instruction_pointer(),
)),
});
}

if self.acir_call_counter >= self.acir_call_results.len() {
let mut initial_witness = WitnessMap::default();
for (i, input_witness) in inputs.iter().enumerate() {
let input_value = *witness_to_value(&self.witness_map, *input_witness)?;
initial_witness.insert(Witness(i as u32), input_value);
}
return Ok(Some(AcirCallWaitInfo { id: *id, initial_witness }));
}

let result_values = &self.acir_call_results[self.acir_call_counter];
if outputs.len() != result_values.len() {
return Err(OpcodeResolutionError::AcirCallOutputsMismatch {
opcode_location: ErrorLocation::Resolved(OpcodeLocation::Acir(
self.instruction_pointer(),
)),
results_size: result_values.len() as u32,
outputs_size: outputs.len() as u32,
});
}

for (output_witness, result_value) in outputs.iter().zip(result_values) {
insert_value(output_witness, *result_value, &mut self.witness_map)?;
}

self.acir_call_counter += 1;
Ok(None)
}
}

// Returns the concrete value for a particular witness
Expand Down Expand Up @@ -469,3 +555,11 @@ fn any_witness_from_expression(expr: &Expression) -> Option<Witness> {
Some(expr.linear_combinations[0].1)
}
}

#[derive(Debug, Clone, PartialEq)]
pub struct AcirCallWaitInfo {
/// Index in the list of ACIR function's that should be called
pub id: u32,
/// Initial witness for the given circuit to be called
pub initial_witness: WitnessMap,
}
3 changes: 3 additions & 0 deletions acvm-repo/acvm_js/src/execute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,9 @@ pub async fn execute_circuit_with_black_box_solver(

acvm.resolve_pending_foreign_call(result);
}
ACVMStatus::RequiresAcirCall(_) => {
todo!("Handle acir calls in acvm JS");
}
}
}

Expand Down
24 changes: 12 additions & 12 deletions compiler/noirc_driver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,13 @@
#![warn(unreachable_pub)]
#![warn(clippy::semicolon_if_nothing_returned)]

use acvm::acir::circuit::{ExpressionWidth, Program};
use acvm::acir::circuit::ExpressionWidth;
use clap::Args;
use fm::{FileId, FileManager};
use iter_extended::vecmap;
use noirc_abi::{AbiParameter, AbiType, ContractEvent};
use noirc_errors::{CustomDiagnostic, FileDiagnostic};
use noirc_evaluator::create_circuit;
use noirc_evaluator::create_program;
use noirc_evaluator::errors::RuntimeError;
use noirc_frontend::debug::build_debug_crate_file;
use noirc_frontend::graph::{CrateId, CrateName};
Expand Down Expand Up @@ -484,23 +484,23 @@ pub fn compile_no_check(
return Ok(cached_program.expect("cache must exist for hashes to match"));
}
let visibility = program.return_visibility;
let (circuit, debug, input_witnesses, return_witnesses, warnings) = create_circuit(
program,
options.show_ssa,
options.show_brillig,
options.force_brillig,
options.benchmark_codegen,
)?;

let (program, debug, warnings, input_witnesses, return_witnesses) =
create_program(program, options.show_ssa, options.show_brillig, options.force_brillig)?;

let abi =
abi_gen::gen_abi(context, &main_function, input_witnesses, return_witnesses, visibility);
let file_map = filter_relevant_files(&[debug.clone()], &context.file_manager);
let file_map = filter_relevant_files(&debug, &context.file_manager);

Ok(CompiledProgram {
hash,
// TODO(https://github.com/noir-lang/noir/issues/4428)
program: Program { functions: vec![circuit] },
debug,
program,
// TODO(https://github.com/noir-lang/noir/issues/4428)
// Debug info is only relevant for errors at execution time which is not yet supported
// The CompileProgram `debug` field is used in multiple places and is better
// left to be updated once execution of multiple ACIR functions is enabled
debug: debug[0].clone(),
abi,
file_map,
noir_version: NOIR_ARTIFACT_VERSION_STRING.to_string(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -373,8 +373,9 @@ mod tests {
use crate::ssa::ssa_gen::Ssa;

fn create_test_environment() -> (Ssa, FunctionContext, BrilligContext) {
let builder =
FunctionBuilder::new("main".to_string(), Id::test_new(0), RuntimeType::Brillig);
let mut builder = FunctionBuilder::new("main".to_string(), Id::test_new(0));
builder.set_runtime(RuntimeType::Brillig);

let ssa = builder.finish();
let mut brillig_context = create_context();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,8 @@ mod test {
// }

let main_id = Id::test_new(1);
let mut builder = FunctionBuilder::new("main".into(), main_id, RuntimeType::Brillig);
let mut builder = FunctionBuilder::new("main".into(), main_id);
builder.set_runtime(RuntimeType::Brillig);

let b1 = builder.insert_block();
let b2 = builder.insert_block();
Expand Down Expand Up @@ -425,7 +426,8 @@ mod test {
// }

let main_id = Id::test_new(1);
let mut builder = FunctionBuilder::new("main".into(), main_id, RuntimeType::Brillig);
let mut builder = FunctionBuilder::new("main".into(), main_id);
builder.set_runtime(RuntimeType::Brillig);

let b1 = builder.insert_block();
let b2 = builder.insert_block();
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_evaluator/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,4 @@ pub mod ssa;

pub mod brillig;

pub use ssa::create_circuit;
pub use ssa::create_program;

0 comments on commit e625c7e

Please sign in to comment.