Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: Elliptic Curve Virtual Machine Circuit #1268

Merged
merged 43 commits into from
Sep 15, 2023
Merged

feat: Elliptic Curve Virtual Machine Circuit #1268

merged 43 commits into from
Sep 15, 2023

Conversation

zac-williamson
Copy link
Contributor

@zac-williamson zac-williamson commented Jul 30, 2023 •
edited
Loading

Description

This PR defines a new Honk flavour, ECCVM, that describes a circuit that evaluates a virtual machine designed to compute elliptic curve operations.

A full specification of the ECCVM is at https://hackmd.io/@aztec-network/rJ5xhuCsn?type=view

The intention is to use the ECCVM to efficiently evaluate large numbers of elliptic curve multiscalar multiplications over the grumpkin curve, inside a BN254 circuit.

The ECCVM is agnostic to the underlying curves, however. The only requirement is that the ECCVM operations are over a curve whose coordinate field is equal to the ECCVM::FF prime field (which is parametrisable).

This PR adds the following key entities:

  • ECCVM flavour parametrisation
  • ECCVMCircuitBuilder : constructs an ECCVM circuit given an opcode trace for the ECCVM. This class also defines the ECCVM instruction set and can be used to assemble an ECCVM opcode trace
  • ECCVMProver
  • ECCVMVerifier
  • The algebraic relations used by the Prover/Verifier to evaluate an ECCVM relation

A full specification of the ECCVM is at https://hackmd.io/@aztec-network/rJ5xhuCsn?type=view

Checklist:

  • I have reviewed my diff in github, line by line.
  • Every change is related to the PR description.
  • I have linked this pull request to the issue(s) that it resolves.
  • There are no unexpected formatting changes, superfluous debug logs, or commented-out code.
  • The branch has been merged or rebased against the head of its merge target.
  • I'm happy for the PR to be merged at the reviewer's next convenience.

@ludamad ludamad added the crypto cryptography label Aug 1, 2023
Rumata888
Rumata888 reviewed Sep 11, 2023
View reviewed changes
...ts/cpp/barretenberg/cpp/src/barretenberg/proof_system/relations/ecc_vm/ecc_wnaf_relation.cpp Outdated
*/
std::get<11>(accumulator) += precompute_select * scalar_sum_new * scaled_transition;
// (2, 3 combined): q_transition * (pc - pc_shift - 1) + (-q_transition + 1) * (pc_shift - pc)
// => q_transition * (2 * (pc - pc_shift) - 1) + (pc_shift - pc)
Copy link
Contributor

Choose a reason for hiding this comment

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

This formula doesn't match the one you have in the code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I...think it does? I rewrote the comment formula to better match the code. Can you double check this as I am fairly confident the formula matches

Copy link
Contributor

Choose a reason for hiding this comment

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

In the code you are multiplying the whole ((2 * (pc - pc_shift) - 1) + (pc_shift - pc)) by q_transition. In the comments just the (-2* pc_delta -1)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh got it. Thanks for spotting that Kesha, will update.

Rumata888
Rumata888 reviewed Sep 11, 2023
View reviewed changes
...ts/cpp/barretenberg/cpp/src/barretenberg/proof_system/relations/ecc_vm/ecc_wnaf_relation.cpp Outdated
std::get<18>(accumulator) += precompute_select_zero * round;
std::get<19>(accumulator) += precompute_select_zero * pc;

// TODO(@zac-williamson)
Copy link
Contributor

Choose a reason for hiding this comment

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

Issue?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#2226

Rumata888
Rumata888 reviewed Sep 11, 2023
View reviewed changes
.../barretenberg/cpp/src/barretenberg/proof_system/relations/ecc_vm/ecc_transcript_relation.cpp Outdated

/**
* @brief Validate `op` opcode is well formed.
* `op` is defined to be q_reset_accumulator + 2 * q_eq + 2 * q_mul + 4 * q_add,
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be 1, 2,4, 8, tight?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, thanks for spotting will change

Rumata888
Rumata888 reviewed Sep 11, 2023
View reviewed changes
.../barretenberg/cpp/src/barretenberg/proof_system/relations/ecc_vm/ecc_transcript_relation.cpp Outdated
* @brief Validate `op` opcode is well formed.
* `op` is defined to be q_reset_accumulator + 2 * q_eq + 2 * q_mul + 4 * q_add,
* where q_reset_accumulator, q_eq, q_mul, q_add are all boolean
* (todo, bool constrain?)
Copy link
Contributor

Choose a reason for hiding this comment

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

Please create an issue

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#2223

Rumata888
Rumata888 reviewed Sep 11, 2023
View reviewed changes
...nberg/cpp/src/barretenberg/proof_system/circuit_builder/eccvm/eccvm_circuit_builder.test.cpp Outdated
@@ -0,0 +1,273 @@
#include "barretenberg/crypto/generators/generator_data.hpp"
Copy link
Contributor

Choose a reason for hiding this comment

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

There is no check for reset opcode

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The eq circuit builder method actually creates both an eq and a reset opcode. I've renamed to eq to eq_and_reset to better describe its function.

Rumata888
Rumata888 reviewed Sep 11, 2023
View reviewed changes
.../cpp/barretenberg/cpp/src/barretenberg/proof_system/relations/ecc_vm/ecc_lookup_relation.hpp Outdated

/**
* @brief Expression for the StandardArithmetic gate.
* @dbetails The relation is defined as C(extended_edges(X)...) =
Copy link
Contributor

Choose a reason for hiding this comment

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

Leftover comments

Rumata888
Rumata888 reviewed Sep 11, 2023
View reviewed changes
...its/cpp/barretenberg/cpp/src/barretenberg/proof_system/relations/ecc_vm/ecc_set_relation.cpp Outdated
wnaf_slice += wnaf_slice;
wnaf_slice += s1;

// todo can optimize
Copy link
Contributor

Choose a reason for hiding this comment

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

Issues?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What's the issue? Sorry could you clarify?

Copy link
Contributor

Choose a reason for hiding this comment

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

Could you create issues for these todos?

Rumata888
Rumata888 reviewed Sep 11, 2023
View reviewed changes
...its/cpp/barretenberg/cpp/src/barretenberg/proof_system/relations/ecc_vm/ecc_set_relation.cpp Outdated
* @brief First term: tuple of (pc, round, wnaf_slice), computed when slicing scalar multipliers into slices,
* as part of ECCVMWnafRelation.
* If precompute_select = 1, tuple entry = (wnaf-slice + point-counter * beta + msm-round * beta_sqr).
* There are 4 tuple entires per row.
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo

Copy link
Contributor Author

Choose a reason for hiding this comment

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

could you clarify? what's the typo?

Copy link
Contributor

Choose a reason for hiding this comment

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

4 tuple "entires" per row

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks, fixed

Rumata888
Rumata888 reviewed Sep 11, 2023
View reviewed changes
...its/cpp/barretenberg/cpp/src/barretenberg/proof_system/relations/ecc_vm/ecc_msm_relation.cpp Outdated
std::get<9>(accumulator) += (x4_delta * collision_inverse4 - add_fourth_point) * scaling_factor;

// Validate that if q_add = 1 or q_skew = 1, add1 also is 1
// TODO(@zac-williamson) Once we have a stable base to work off of, remove q_add1 and replace with q_msm_add +
Copy link
Contributor

Choose a reason for hiding this comment

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

Issue?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

✅ #2222

@zac-williamson zac-williamson merged commit f85ecd9 into master Sep 15, 2023
2 checks passed
@zac-williamson zac-williamson deleted the zw/eccvm-v5 branch September 15, 2023 12:44
@AztecBot AztecBot mentioned this pull request Sep 15, 2023
Merged
kevaundray pushed a commit that referenced this pull request Sep 15, 2023
@AztecBot
chore(master): Release 0.7.4 (#2320)
01bbdaa 
🤖 I have created a release *beep* *boop*
---


<details><summary>aztec-packages: 0.7.4</summary>

##
[0.7.4](aztec-packages-v0.7.3...aztec-packages-v0.7.4)
(2023-09-15)


### Features

* Elliptic Curve Virtual Machine Circuit
([#1268](#1268))
([f85ecd9](f85ecd9))
* Exposing nargo version via `NodeInfo`
([#2333](#2333))
([1c2669c](1c2669c)),
closes
[#2332](#2332)
* Migrate accounts to auth witness
([#2281](#2281))
([91152af](91152af)),
closes
[#2043](#2043)


### Bug Fixes

* Aztec-nr mirror url
([#2321](#2321))
([aaf7f67](aaf7f67))
* **build:** Fixed paths on s3 deployments
([#2335](#2335))
([38c7979](38c7979))


### Miscellaneous

* Do not format boxes with global format
([#2326](#2326))
([2fe845f](2fe845f))
* Remove native token
([#2280](#2280))
([4032d01](4032d01))
* Rename getAccounts to getRegisteredAccounts
([#2330](#2330))
([c7f3776](c7f3776))
</details>

<details><summary>barretenberg.js: 0.7.4</summary>

##
[0.7.4](barretenberg.js-v0.7.3...barretenberg.js-v0.7.4)
(2023-09-15)


### Miscellaneous

* **barretenberg.js:** Synchronize aztec-packages versions
</details>

<details><summary>barretenberg: 0.7.4</summary>

##
[0.7.4](barretenberg-v0.7.3...barretenberg-v0.7.4)
(2023-09-15)


### Features

* Elliptic Curve Virtual Machine Circuit
([#1268](#1268))
([f85ecd9](f85ecd9))
</details>

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
@codygunton
Copy link
Contributor

This is a 6000-line PR with no description.

@codygunton
Copy link
Contributor

Not one of the new classes in circuit_builder/eccvm has a description.

@zac-williamson
Copy link
Contributor Author

This is a 6000-line PR with no description.

I've added a description to the PR. mea culpa.

Not one of the new classes in circuit_builder/eccvm has a description.

Kesha very kindly has offered to add these. My intention was to thoroughly document all the new classes, these ones I completely forgot about, my bad.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Rumata888 Rumata888 Rumata888 approved these changes

Assignees

@zac-williamson zac-williamson

Labels
crypto cryptography
Projects
A3
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@zac-williamson @codygunton @Rumata888 @ludamad @charlielye