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

feat!: add backend-solvable blackboxes to brillig & unify implementations #422

Merged
merged 9 commits into from
Jul 11, 2023

Conversation

sirasistant
Copy link
Contributor

@sirasistant sirasistant commented Jul 11, 2023

Description

Problem*

Implements pedersen, schnorr and fixedBaseScalarMul for Brillig and resolves #402

Summary*

Context:

  • Right now ACIR depends on the whole Brillig VM
  • If the brillig VM depends on a solver that imports BlackBoxFunc, you get a cyclic dependency
  • Also, the BlackBoxFunctionSolver trait uses OpcodeResolutionError which is ACVM specific

My proposal:

  • Separate the Brillig_vm crate into a brillig crate and a brillig_vm crate (akin to acir/acvm)
    • Acir only depends on brillig crate, does not import the brillig_vm crate
    • ACVM depends on the brillig_vm crate for solving brillig opcodes
  • Add a new crate blackbox_solver
    • exports functions for implementation-fixed blackboxes
    • exports BlackBoxFunctionSolver trait for backend dependant ones (will eventually be removed)
    • exports an enum BlackBoxResolutionError
    • The ACVM OpcodeResolutionError implements a From
  • Modify Brillig VM to use a &BlackBoxFunctionSolver and use that for pedersen, schnorr and fixed base scalar mul
  • Modify ACVM to pass the solver to the brillig VM

Additional Context

PR Checklist*

  • I have tested the changes locally.
  • I have formatted the changes with Prettier and/or cargo fmt on default settings.

@sirasistant sirasistant marked this pull request as ready for review July 11, 2023 09:38
@sirasistant
Copy link
Contributor Author

It's a breaking change due to the change that acir now only exports brillig, not the whole brillig_vm, and the change that now BlackBoxFunctionSolver trait returns results with error type BlackBoxResolutionError instead of OpcodeResolutionError

@sirasistant sirasistant changed the title feat!: add backend-solvable opcodes to brillig & unify implementations feat!: add backend-solvable blackboxes to brillig & unify implementations Jul 11, 2023
Copy link
Contributor

@kevaundray kevaundray left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks good to me, one nit about doc commenting the new crates

@sirasistant sirasistant added this pull request to the merge queue Jul 11, 2023
Merged via the queue into master with commit 093342e Jul 11, 2023
@kevaundray kevaundray mentioned this pull request Jul 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implementation of blackbox functions should be shareable by the ACVM and Brillig_vm
2 participants