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

feat: Add Brillig opcode to introduce custom non-determinism to ACVM #152

Merged
merged 131 commits into from
May 23, 2023

Conversation

kevaundray
Copy link
Contributor

@kevaundray kevaundray commented Mar 24, 2023

Related issue(s)

(If it does not already exist, first create a GitHub issue that describes the problem this Pull Request (PR) solves before creating the PR and link it here.)

Resolves (link to issue)

Description

This is a generalization of our directive functions. Right now we have directives which are defined at compile time in the code, this VM will allow us to instead have the user write the non-deterministic in their frontend like Noir , this gets compiled down to this bytecode and then ran on the VM.

From conversations with Guillaume, the directive will then look like:

struct ByteCodeDirective{
   inputs : Vec<Expression>,
   outputs : Vec<Witness>,
   bytecode : Vec<Opcode> 
}

Summary of changes

(Describe the changes in this PR. Point out breaking changes if any.)

Dependency additions / changes

(If applicable.)

Test additions / changes

(If applicable.)

Checklist

  • I have tested the changes locally.
  • I have formatted the changes with Prettier and/or cargo fmt with default settings.
  • I have linked this PR to the issue(s) that it resolves.
  • I have reviewed the changes on GitHub, line by line.
  • I have ensured all changes are covered in the description.

Additional context

(If applicable.)

@kevaundray kevaundray changed the title Add initial code for non-deterministic vm feat: Add initial code for non-deterministic vm Mar 24, 2023
@kevaundray
Copy link
Contributor Author

Also spoke with Guillaume and it seems we could get the Oracle opcode into this instruction set rather than in the ACIR opcode set.

We have two main goals for this:

  • Have this working with invert directive
  • Have it working with Oracle directive however we choose to integrate it

The invert directive will be a full e2e test that shows that we can replace our current directives with this bytecode. Integrating the other directives will be fairly straightforward

kevaundray and others added 25 commits March 27, 2023 17:48
- use ValueKind
- refactor opcodes needed
* add serialization for brillig

* cargo fmt
* solve the brillig directive

* move brilling_bytecode out of acvm

* go back to correct process_opcodes

* cargo fmt

* switch to using insert_witness method
- RegisterIndex -> RegisterMemIndex
* trap opcode in Opcode enum

* process trap opcode
* brillig jmp process_opcode

* making jump test

* starting jmp opcode test

* test brillig jumps

* debug cleanup

* remove comments from Value in vm
* jmp if not opcode and test

* jmp if not opode and test

* fix jmpifnot test

* carog fmt

* remove unnecessary registers in jmpif test

* cargo fmt
* MOV opcode in brillig and a basic test

* cargo fmt

* fixed move to not only move source into dest, not a full swap

* cargo fmt
* add lt and lte comparisons to BinaryOp + tests

* briefly delete test

* cargo fmt

* temp remove test
@TomAFrench TomAFrench changed the title feat: Add initial code for non-deterministic vm feat: Add Brillig opcode which introduces non-determinism to ACVM May 23, 2023
@TomAFrench TomAFrench changed the title feat: Add Brillig opcode which introduces non-determinism to ACVM feat: Add Brillig opcode to introduce custom non-determinism to ACVM May 23, 2023
* chore: make fields private on `Value` and `RegisterIndex`

* Update brillig_bytecode/src/value.rs

---------

Co-authored-by: kevaundray <[email protected]>
@TomAFrench
Copy link
Member

Once #152 (comment) is resolved either way then I can remove the block from this PR.

If we want to merge this today in UK time then we should prioritise this so we can resolve before I go to bed (otherwise I'll have to merge in the morning).

@kevaundray
Copy link
Contributor Author

Just resolved this

brillig_vm/src/value.rs Outdated Show resolved Hide resolved
Copy link
Member

@TomAFrench TomAFrench left a comment

Choose a reason for hiding this comment

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

one nit

@kevaundray kevaundray added this pull request to the merge queue May 23, 2023
Merged via the queue into master with commit 3c6740a May 23, 2023
@github-actions github-actions bot mentioned this pull request May 23, 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.

7 participants