feat: remove external blackbox solver from acir simulator (#6586)

We're no longer actually making any calls into the WASM from the `WasmBlackBoxFunctionSolver` so initializing it outside of `acvm_js` doesn't do anything anymore.
AztecProtocol · May 23, 2024 · 9c54590 · 9c54590
1 parent 288231b
commit 9c54590
Show file tree

Hide file tree

Showing 12 changed files with 40 additions and 230 deletions.
diff --git a/noir/noir-repo/acvm-repo/acvm_js/src/execute.rs b/noir/noir-repo/acvm-repo/acvm_js/src/execute.rs
@@ -19,20 +19,6 @@ use crate::{
     JsExecutionError, JsSolvedAndReturnWitness, JsWitnessMap, JsWitnessStack,
 };
 
-#[wasm_bindgen]
-pub struct WasmBlackBoxFunctionSolver(Bn254BlackBoxSolver);
-
-impl WasmBlackBoxFunctionSolver {
-    async fn initialize() -> WasmBlackBoxFunctionSolver {
-        WasmBlackBoxFunctionSolver(Bn254BlackBoxSolver::initialize().await)
-    }
-}
-
-#[wasm_bindgen(js_name = "createBlackBoxSolver")]
-pub async fn create_black_box_solver() -> WasmBlackBoxFunctionSolver {
-    WasmBlackBoxFunctionSolver::initialize().await
-}
-
 /// Executes an ACIR circuit to generate the solved witness from the initial witness.
 ///
 /// @param {Uint8Array} circuit - A serialized representation of an ACIR circuit
@@ -47,15 +33,9 @@ pub async fn execute_circuit(
 ) -> Result<JsWitnessMap, Error> {
     console_error_panic_hook::set_once();
 
-    let solver = WasmBlackBoxFunctionSolver::initialize().await;
-
-    let mut witness_stack = execute_program_with_native_type_return(
-        &solver,
-        program,
-        initial_witness,
-        &foreign_call_handler,
-    )
-    .await?;
+    let mut witness_stack =
+        execute_program_with_native_type_return(program, initial_witness, &foreign_call_handler)
+            .await?;
     let witness_map =
         witness_stack.pop().expect("Should have at least one witness on the stack").witness;
     Ok(witness_map.into())
@@ -64,14 +44,12 @@ pub async fn execute_circuit(
 /// Executes an ACIR circuit to generate the solved witness from the initial witness.
 /// This method also extracts the public return values from the solved witness into its own return witness.
 ///
-/// @param {&WasmBlackBoxFunctionSolver} solver - A black box solver.
 /// @param {Uint8Array} circuit - A serialized representation of an ACIR circuit
 /// @param {WitnessMap} initial_witness - The initial witness map defining all of the inputs to `circuit`..
 /// @param {ForeignCallHandler} foreign_call_handler - A callback to process any foreign calls from the circuit.
 /// @returns {SolvedAndReturnWitness} The solved witness calculated by executing the circuit on the provided inputs, as well as the return witness indices as specified by the circuit.
 #[wasm_bindgen(js_name = executeCircuitWithReturnWitness, skip_jsdoc)]
 pub async fn execute_circuit_with_return_witness(
-    solver: &WasmBlackBoxFunctionSolver,
     program: Vec<u8>,
     initial_witness: JsWitnessMap,
     foreign_call_handler: ForeignCallHandler,
@@ -82,7 +60,6 @@ pub async fn execute_circuit_with_return_witness(
     .map_err(|_| JsExecutionError::new("Failed to deserialize circuit. This is likely due to differing serialization formats between ACVM_JS and your compiler".to_string(), None, None))?;
 
     let mut witness_stack = execute_program_with_native_program_and_return(
-        solver,
         &program,
         initial_witness,
         &foreign_call_handler,
@@ -101,32 +78,10 @@ pub async fn execute_circuit_with_return_witness(
 
 /// Executes an ACIR circuit to generate the solved witness from the initial witness.
 ///
-/// @param {&WasmBlackBoxFunctionSolver} solver - A black box solver.
-/// @param {Uint8Array} circuit - A serialized representation of an ACIR circuit
-/// @param {WitnessMap} initial_witness - The initial witness map defining all of the inputs to `circuit`..
-/// @param {ForeignCallHandler} foreign_call_handler - A callback to process any foreign calls from the circuit.
-/// @returns {WitnessMap} The solved witness calculated by executing the circuit on the provided inputs.
-#[wasm_bindgen(js_name = executeCircuitWithBlackBoxSolver, skip_jsdoc)]
-pub async fn execute_circuit_with_black_box_solver(
-    solver: &WasmBlackBoxFunctionSolver,
-    program: Vec<u8>,
-    initial_witness: JsWitnessMap,
-    foreign_call_handler: ForeignCallHandler,
-) -> Result<JsWitnessMap, Error> {
-    console_error_panic_hook::set_once();
-
-    let mut witness_stack = execute_program_with_native_type_return(
-        solver,
-        program,
-        initial_witness,
-        &foreign_call_handler,
-    )
-    .await?;
-    let witness_map =
-        witness_stack.pop().expect("Should have at least one witness on the stack").witness;
-    Ok(witness_map.into())
-}
-
+/// @param {Uint8Array} program - A serialized representation of an ACIR program
+/// @param {WitnessMap} initial_witness - The initial witness map defining all of the inputs to `program`.
+/// @param {ForeignCallHandler} foreign_call_handler - A callback to process any foreign calls from the program.
+/// @returns {WitnessStack} The solved witness calculated by executing the program on the provided inputs.
 #[wasm_bindgen(js_name = executeProgram, skip_jsdoc)]
 pub async fn execute_program(
     program: Vec<u8>,
@@ -135,32 +90,14 @@ pub async fn execute_program(
 ) -> Result<JsWitnessStack, Error> {
     console_error_panic_hook::set_once();
 
-    let solver = WasmBlackBoxFunctionSolver::initialize().await;
-
-    execute_program_with_black_box_solver(&solver, program, initial_witness, &foreign_call_handler)
-        .await
-}
-
-#[wasm_bindgen(js_name = executeProgramWithBlackBoxSolver, skip_jsdoc)]
-pub async fn execute_program_with_black_box_solver(
-    solver: &WasmBlackBoxFunctionSolver,
-    program: Vec<u8>,
-    initial_witness: JsWitnessMap,
-    foreign_call_executor: &ForeignCallHandler,
-) -> Result<JsWitnessStack, Error> {
-    let witness_stack = execute_program_with_native_type_return(
-        solver,
-        program,
-        initial_witness,
-        foreign_call_executor,
-    )
-    .await?;
+    let witness_stack =
+        execute_program_with_native_type_return(program, initial_witness, &foreign_call_handler)
+            .await?;
 
     Ok(witness_stack.into())
 }
 
 async fn execute_program_with_native_type_return(
-    solver: &WasmBlackBoxFunctionSolver,
     program: Vec<u8>,
     initial_witness: JsWitnessMap,
     foreign_call_executor: &ForeignCallHandler,
@@ -171,25 +108,20 @@ async fn execute_program_with_native_type_return(
         None,
         None))?;
 
-    execute_program_with_native_program_and_return(
-        solver,
-        &program,
-        initial_witness,
-        foreign_call_executor,
-    )
-    .await
+    execute_program_with_native_program_and_return(&program, initial_witness, foreign_call_executor)
+        .await
 }
 
 async fn execute_program_with_native_program_and_return(
-    solver: &WasmBlackBoxFunctionSolver,
     program: &Program,
     initial_witness: JsWitnessMap,
     foreign_call_executor: &ForeignCallHandler,
 ) -> Result<WitnessStack, Error> {
+    let solver = Bn254BlackBoxSolver::initialize().await;
     let executor = ProgramExecutor::new(
         &program.functions,
         &program.unconstrained_functions,
-        &solver.0,
+        &solver,
         foreign_call_executor,
     );
     let witness_stack = executor.execute(initial_witness.into()).await?;

diff --git a/noir/noir-repo/acvm-repo/acvm_js/src/lib.rs b/noir/noir-repo/acvm-repo/acvm_js/src/lib.rs
@@ -23,10 +23,7 @@ pub use build_info::build_info;
 pub use compression::{
     compress_witness, compress_witness_stack, decompress_witness, decompress_witness_stack,
 };
-pub use execute::{
-    create_black_box_solver, execute_circuit, execute_circuit_with_black_box_solver,
-    execute_circuit_with_return_witness, execute_program, execute_program_with_black_box_solver,
-};
+pub use execute::{execute_circuit, execute_circuit_with_return_witness, execute_program};
 pub use js_execution_error::JsExecutionError;
 pub use js_witness_map::JsSolvedAndReturnWitness;
 pub use js_witness_map::JsWitnessMap;

diff --git a/noir/noir-repo/acvm-repo/acvm_js/test/browser/execute_circuit.test.ts b/noir/noir-repo/acvm-repo/acvm_js/test/browser/execute_circuit.test.ts
@@ -1,9 +1,6 @@
 import { expect } from '@esm-bundle/chai';
 import initACVM, {
-  createBlackBoxSolver,
   executeCircuit,
-  executeCircuitWithBlackBoxSolver,
-  WasmBlackBoxFunctionSolver,
   WitnessMap,
   initLogLevel,
   ForeignCallHandler,
@@ -123,22 +120,3 @@ it('successfully executes a MemoryOp opcode', async () => {
   expect(solvedWitness).to.be.deep.eq(expectedWitnessMap);
 });
 
-it('successfully executes two circuits with same backend', async function () {
-  // chose pedersen op here because it is the one with slow initialization
-  // that led to the decision to pull backend initialization into a separate
-  // function/wasmbind
-  const solver: WasmBlackBoxFunctionSolver = await createBlackBoxSolver();
-
-  const { bytecode, initialWitnessMap, expectedWitnessMap } = await import('../shared/pedersen');
-
-  const solvedWitness0: WitnessMap = await executeCircuitWithBlackBoxSolver(solver, bytecode, initialWitnessMap, () => {
-    throw Error('unexpected oracle');
-  });
-
-  expect(solvedWitness0).to.be.deep.eq(expectedWitnessMap);
-
-  const solvedWitness1: WitnessMap = await executeCircuitWithBlackBoxSolver(solver, bytecode, initialWitnessMap, () => {
-    throw Error('unexpected oracle');
-  });
-  expect(solvedWitness1).to.be.deep.eq(expectedWitnessMap);
-});
diff --git a/noir/noir-repo/acvm-repo/acvm_js/test/node/execute_circuit.test.ts b/noir/noir-repo/acvm-repo/acvm_js/test/node/execute_circuit.test.ts
@@ -1,9 +1,6 @@
 import { expect } from 'chai';
 import {
-  createBlackBoxSolver,
   executeCircuit,
-  executeCircuitWithBlackBoxSolver,
-  WasmBlackBoxFunctionSolver,
   WitnessMap,
   ForeignCallHandler,
   executeProgram,
@@ -120,40 +117,18 @@ it('successfully executes a MemoryOp opcode', async () => {
   expect(solvedWitness).to.be.deep.eq(expectedWitnessMap);
 });
 
-it('successfully executes two circuits with same backend', async function () {
-  this.timeout(10000);
-
-  // chose pedersen op here because it is the one with slow initialization
-  // that led to the decision to pull backend initialization into a separate
-  // function/wasmbind
-  const solver: WasmBlackBoxFunctionSolver = await createBlackBoxSolver();
-
-  const { bytecode, initialWitnessMap, expectedWitnessMap } = await import('../shared/pedersen');
-
-  const solvedWitness0 = await executeCircuitWithBlackBoxSolver(solver, bytecode, initialWitnessMap, () => {
-    throw Error('unexpected oracle');
-  });
-
-  const solvedWitness1 = await executeCircuitWithBlackBoxSolver(solver, bytecode, initialWitnessMap, () => {
-    throw Error('unexpected oracle');
-  });
-
-  expect(solvedWitness0).to.be.deep.eq(expectedWitnessMap);
-  expect(solvedWitness1).to.be.deep.eq(expectedWitnessMap);
-});
-
-it('successfully executes 500 circuits with same backend', async function () {
+it('successfully executes 500 pedersen circuits', async function () {
   this.timeout(100000);
 
-  // chose pedersen op here because it is the one with slow initialization
-  // that led to the decision to pull backend initialization into a separate
-  // function/wasmbind
-  const solver: WasmBlackBoxFunctionSolver = await createBlackBoxSolver();
+  // Pedersen opcodes used to have a large upfront cost due to generator calculation
+  // so we'd need to pass around the blackbox solver in JS to avoid redoing this work.
+  //
+  // This test now shows that we don't need to do this anymore without a performance regression.
 
   const { bytecode, initialWitnessMap, expectedWitnessMap } = await import('../shared/pedersen');
 
   for (let i = 0; i < 500; i++) {
-    const solvedWitness = await executeCircuitWithBlackBoxSolver(solver, bytecode, initialWitnessMap, () => {
+    const solvedWitness = await executeCircuit(bytecode, initialWitnessMap, () => {
       throw Error('unexpected oracle');
     });
 

diff --git a/noir/noir-repo/tooling/noir_js/src/witness_generation.ts b/noir/noir-repo/tooling/noir_js/src/witness_generation.ts
@@ -1,25 +1,8 @@
 import { abiDecodeError, abiEncode, InputMap } from '@noir-lang/noirc_abi';
 import { base64Decode } from './base64_decode.js';
-import {
-  WitnessStack,
-  ForeignCallHandler,
-  ForeignCallInput,
-  createBlackBoxSolver,
-  WasmBlackBoxFunctionSolver,
-  executeProgramWithBlackBoxSolver,
-  ExecutionError,
-} from '@noir-lang/acvm_js';
+import { WitnessStack, ForeignCallHandler, ForeignCallInput, ExecutionError, executeProgram } from '@noir-lang/acvm_js';
 import { Abi, CompiledCircuit } from '@noir-lang/types';
 
-let solver: Promise<WasmBlackBoxFunctionSolver>;
-
-const getSolver = (): Promise<WasmBlackBoxFunctionSolver> => {
-  if (!solver) {
-    solver = createBlackBoxSolver();
-  }
-  return solver;
-};
-
 const defaultForeignCallHandler: ForeignCallHandler = async (name: string, args: ForeignCallInput[]) => {
   if (name == 'print') {
     // By default we do not print anything for `print` foreign calls due to a need for formatting,
@@ -70,12 +53,7 @@ export async function generateWitness(
   // Execute the circuit to generate the rest of the witnesses and serialize
   // them into a Uint8Array.
   try {
-    const solvedWitness = await executeProgramWithBlackBoxSolver(
-      await getSolver(),
-      base64Decode(compiledProgram.bytecode),
-      witnessMap,
-      foreignCallHandler,
-    );
+    const solvedWitness = await executeProgram(base64Decode(compiledProgram.bytecode), witnessMap, foreignCallHandler);
     return solvedWitness;
   } catch (err) {
     // Typescript types catched errors as unknown or any, so we need to narrow its type to check if it has raw assertion payload.

diff --git a/yarn-project/simulator/src/acvm/acvm.ts b/yarn-project/simulator/src/acvm/acvm.ts
@@ -6,7 +6,6 @@ import {
   type ExecutionError,
   type ForeignCallInput,
   type ForeignCallOutput,
-  type WasmBlackBoxFunctionSolver,
   executeCircuitWithReturnWitness,
 } from '@noir-lang/acvm_js';
 
@@ -85,15 +84,13 @@ export function resolveOpcodeLocations(
  * The function call that executes an ACIR.
  */
 export async function acvm(
-  solver: WasmBlackBoxFunctionSolver,
   acir: Buffer,
   initialWitness: ACVMWitness,
   callback: ACIRCallback,
 ): Promise<ACIRExecutionResult> {
   const logger = createDebugLogger('aztec:simulator:acvm');
 
   const solvedAndReturnWitness = await executeCircuitWithReturnWitness(
-    solver,
     acir,
     initialWitness,
     async (name: string, args: ForeignCallInput[]) => {

diff --git a/yarn-project/simulator/src/client/private_execution.ts b/yarn-project/simulator/src/client/private_execution.ts
@@ -8,7 +8,6 @@ import { Oracle, acvm, extractCallStack } from '../acvm/index.js';
 import { ExecutionError } from '../common/errors.js';
 import { type ClientExecutionContext } from './client_execution_context.js';
 import { type ExecutionResult } from './execution_result.js';
-import { AcirSimulator } from './simulator.js';
 
 /**
  * Execute a private function and return the execution result.
@@ -24,19 +23,17 @@ export async function executePrivateFunction(
   const acir = artifact.bytecode;
   const initialWitness = context.getInitialWitness(artifact);
   const acvmCallback = new Oracle(context);
-  const acirExecutionResult = await acvm(await AcirSimulator.getSolver(), acir, initialWitness, acvmCallback).catch(
-    (err: Error) => {
-      throw new ExecutionError(
-        err.message,
-        {
-          contractAddress,
-          functionSelector,
-        },
-        extractCallStack(err, artifact.debug),
-        { cause: err },
-      );
-    },
-  );
+  const acirExecutionResult = await acvm(acir, initialWitness, acvmCallback).catch((err: Error) => {
+    throw new ExecutionError(
+      err.message,
+      {
+        contractAddress,
+        functionSelector,
+      },
+      extractCallStack(err, artifact.debug),
+      { cause: err },
+    );
+  });
   const partialWitness = acirExecutionResult.partialWitness;
   const returnWitness = witnessMapToFields(acirExecutionResult.returnWitness);
   const publicInputs = PrivateCircuitPublicInputs.fromFields(returnWitness);