-
Notifications
You must be signed in to change notification settings - Fork 270
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.
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
feat(sol-honk): integrate solidity honk verifier with bb and acir tests #7573
Conversation
@@ -0,0 +1,104 @@ | |||
#pragma once |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have thrown this in the honk/utils path as it seemed fitting, however this path only has testing code. @codygunton do you think this should stay here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another file like this is in barretenberg/solidity_helpers but idk exactly what makes sense
export PROOF_AS_FIELDS="$(pwd)/proof_fields.json" | ||
|
||
# Create a proof, write the solidity contract, write the proof as fields in order to extract the public inputs | ||
$BIN prove_keccak_ultra_honk -o proof |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have included a second bb command that uses the UltraKeccak flavor for proving, in its current form this is not very user friendly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🫡
const proofStr = `0x${proof.toString("hex").substring(64 * numPublicInputs)}`; | ||
// Cut the number of public inputs out of the proof string | ||
let proofStr = proof.toString("hex"); | ||
if (testingHonk) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The verifier's interface is verify(proof, publicInputs[]). However, in it's current form honk proofs have 3 elements before the public inputs, so I (and the user) will need to slice these out in order to have proofs which can be processed by the verifier.
I did not want to change the serialization of the proofs in this pr, but it is something to think about ahead of the noir switch over
@@ -25,30 +25,6 @@ import {ecMul, ecAdd, ecSub, negateInplace, convertProofPoint} from "../utils.so | |||
// Field arithmetic libraries | |||
import {Fr, FrLib} from "../Fr.sol"; | |||
|
|||
struct Proof { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As proof sizes are fixed in the current form, the proof struct can be shared across instances, hence its deletion.
The remaining changes in the solidity files are majority formatting changes
Benchmark resultsMetrics with a significant change:
Detailed resultsAll benchmarks are run on txs on the This benchmark source data is available in JSON format on S3 here. Proof generationEach column represents the number of threads used in proof generation.
L2 block published to L1Each column represents the number of txs on an L2 block published to L1.
L2 chain processingEach column represents the number of blocks on the L2 chain where each block has 8 txs.
Circuits statsStats on running time and I/O sizes collected for every kernel circuit run across all benchmarks.
Stats on running time collected for app circuits
AVM SimulationTime to simulate various public functions in the AVM.
Public DB AccessTime to access various public DBs.
Tree insertion statsThe duration to insert a fixed batch of leaves into each tree type.
MiscellaneousTransaction sizes based on how many contract classes are registered in the tx.
Transaction size based on fee payment method | Metric | | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
⚠️ Performance Alert ⚠️
Possible performance regression was detected for benchmark 'C++ Benchmark'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.05
.
Benchmark suite | Current: 241508e | Previous: ba365c0 | Ratio |
---|---|---|---|
nativeClientIVCBench/Full/6 |
14219.871842999994 ms/iter |
13300.881290000007 ms/iter |
1.07 |
wasmClientIVCBench/Full/6 |
42200.995354000006 ms/iter |
39728.2553 ms/iter |
1.06 |
commit(t) |
4789014775 ns/iter |
3649675717 ns/iter |
1.31 |
commit(t) |
3942986247 ns/iter |
3649675717 ns/iter |
1.08 |
This comment was automatically generated by workflow using github-action-benchmark.
CC: @ludamad @codygunton
Changes to circuit sizes
🧾 Summary (100% most significant diffs)
Full diff report 👇
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated Github config to run the new tests and they pass. LGTM
export PROOF_AS_FIELDS="$(pwd)/proof_fields.json" | ||
|
||
# Create a proof, write the solidity contract, write the proof as fields in order to extract the public inputs | ||
$BIN prove_keccak_ultra_honk -o proof |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🫡
@@ -26,7 +26,6 @@ struct Transcript { | |||
Fr zmQuotient; | |||
// Derived | |||
Fr publicInputsDelta; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This entity was deleted but it has 20 vestiges in the code still; adding issue to clean up here
AztecProtocol/barretenberg#1061
.github/workflows/ci.yml
Outdated
- name: "BB Solidity Acir Tests" | ||
working-directory: ./barretenberg/ | ||
timeout-minutes: 40 | ||
run: earthly-ci --no-output ./+barretenberg-acir-tests-sol |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line is a mistake; it's running the wrong job
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thank you
@@ -0,0 +1,104 @@ | |||
#pragma once |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another file like this is in barretenberg/solidity_helpers but idk exactly what makes sense
@@ -92,21 +95,10 @@ inline void output_vk_sol_ultra_honk( | |||
print_g1(key->lagrange_first, "lagrangeFirst"); | |||
print_g1(key->lagrange_last, "lagrangeLast", /*last=*/ true); | |||
os << |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nb this will need to be reinstated when we have reinstated
* - stdout: The Solidity verifier contract is written to stdout as a string | ||
* - Filesystem: The Solidity verifier contract is written to the path specified by outputPath | ||
* | ||
* Note: The fact that the contract was computed is for an ACIR circuit is not of importance |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo: note unclear
🤖 I have created a release *beep* *boop* --- <details><summary>aztec-package: 0.47.1</summary> ## [0.47.1](aztec-package-v0.47.0...aztec-package-v0.47.1) (2024-07-30) ### Features * Do not evict tx objects from p2p tx pool immediately ([#7652](#7652)) ([96492dc](96492dc)) * Min and max block times for sequencer ([#7630](#7630)) ([2a013b8](2a013b8)) * Transaction sender bot ([#7586](#7586)) ([176fd08](176fd08)), closes [#7562](#7562) ### Bug Fixes * Aztec start --prover ([#7617](#7617)) ([ac206a6](ac206a6)) ### Miscellaneous * Add tx bot to terraform ([#7609](#7609)) ([6caa914](6caa914)) * Merge Devnet back to Master ([#7611](#7611)) ([112961c](112961c)) </details> <details><summary>barretenberg.js: 0.47.1</summary> ## [0.47.1](barretenberg.js-v0.47.0...barretenberg.js-v0.47.1) (2024-07-30) ### Miscellaneous * **barretenberg.js:** Synchronize aztec-packages versions </details> <details><summary>aztec-packages: 0.47.1</summary> ## [0.47.1](aztec-packages-v0.47.0...aztec-packages-v0.47.1) (2024-07-30) ### Features * Add recursive aggregation object to proving/verification keys ([#6770](#6770)) ([f48b069](f48b069)) * Adding aggregation to honk and rollup ([#7466](#7466)) ([2633aa9](2633aa9)) * Allow get_notes to return zero notes ([#7621](#7621)) ([e16452a](e16452a)) * **avm:** Pedersen commit in avm ([#7634](#7634)) ([45e7867](45e7867)) * **avm:** Pedersen commitment sim ([#7632](#7632)) ([cc420a0](cc420a0)) * Do not evict tx objects from p2p tx pool immediately ([#7652](#7652)) ([96492dc](96492dc)) * Handle L1toL2 msgs in prover-node ([#7654](#7654)) ([f35bac5](f35bac5)) * Leonidas ([#7597](#7597)) ([eda498a](eda498a)) * Min and max block times for sequencer ([#7630](#7630)) ([2a013b8](2a013b8)) * **sol-honk:** Integrate solidity honk verifier with bb and acir tests ([#7573](#7573)) ([344ca6f](344ca6f)) * **sol-honk:** Test verifying recursive proof ([#7576](#7576)) ([26408c1](26408c1)) * Static.aztec.network ([#7649](#7649)) ([9dffef5](9dffef5)) * Transaction sender bot ([#7586](#7586)) ([176fd08](176fd08)), closes [#7562](#7562) ### Bug Fixes * Aztec start --prover ([#7617](#7617)) ([ac206a6](ac206a6)) * Bb mac build ([#7619](#7619)) ([e3c5602](e3c5602)) * Faucet correctly derives its account ([#7645](#7645)) ([921773f](921773f)) * Uniswap e2e test "nonce too low" ([#7633](#7633)) ([e9e2318](e9e2318)) ### Miscellaneous * Add tx bot to terraform ([#7609](#7609)) ([6caa914](6caa914)) * **bb readme:** Add installation instructions and TODOs ([#7601](#7601)) ([1a97698](1a97698)) * **bb readme:** Update versioning instructions and add Honk Solidity verifier commands ([#7608](#7608)) ([9dd9195](9dd9195)) * Bench client ivc proof verification ([#7553](#7553)) ([b060309](b060309)) * Call requests ([#7483](#7483)) ([ffedf39](ffedf39)) * Constant inputs for most blackboxes ([#7613](#7613)) ([3247058](3247058)) * **docs:** Fix links ([#7602](#7602)) ([f2029be](f2029be)) * Merge Devnet back to Master ([#7611](#7611)) ([112961c](112961c)) * **p2p:** Tx serialization cleanup ([#7620](#7620)) ([022a899](022a899)) * Replace relative paths to noir-protocol-circuits ([ebaafc7](ebaafc7)) * Replace relative paths to noir-protocol-circuits ([d3b818d](d3b818d)) * Replace relative paths to noir-protocol-circuits ([4a26a19](4a26a19)) </details> <details><summary>barretenberg: 0.47.1</summary> ## [0.47.1](barretenberg-v0.47.0...barretenberg-v0.47.1) (2024-07-30) ### Features * Add recursive aggregation object to proving/verification keys ([#6770](#6770)) ([f48b069](f48b069)) * Adding aggregation to honk and rollup ([#7466](#7466)) ([2633aa9](2633aa9)) * **avm:** Pedersen commit in avm ([#7634](#7634)) ([45e7867](45e7867)) * **avm:** Pedersen commitment sim ([#7632](#7632)) ([cc420a0](cc420a0)) * **sol-honk:** Integrate solidity honk verifier with bb and acir tests ([#7573](#7573)) ([344ca6f](344ca6f)) * **sol-honk:** Test verifying recursive proof ([#7576](#7576)) ([26408c1](26408c1)) ### Bug Fixes * Bb mac build ([#7619](#7619)) ([e3c5602](e3c5602)) ### Miscellaneous * **bb readme:** Add installation instructions and TODOs ([#7601](#7601)) ([1a97698](1a97698)) * **bb readme:** Update versioning instructions and add Honk Solidity verifier commands ([#7608](#7608)) ([9dd9195](9dd9195)) * Call requests ([#7483](#7483)) ([ffedf39](ffedf39)) * Constant inputs for most blackboxes ([#7613](#7613)) ([3247058](3247058)) * Merge Devnet back to Master ([#7611](#7611)) ([112961c](112961c)) </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
🤖 I have created a release *beep* *boop* --- <details><summary>aztec-package: 0.47.1</summary> ## [0.47.1](AztecProtocol/aztec-packages@aztec-package-v0.47.0...aztec-package-v0.47.1) (2024-07-30) ### Features * Do not evict tx objects from p2p tx pool immediately ([#7652](AztecProtocol/aztec-packages#7652)) ([96492dc](AztecProtocol/aztec-packages@96492dc)) * Min and max block times for sequencer ([#7630](AztecProtocol/aztec-packages#7630)) ([2a013b8](AztecProtocol/aztec-packages@2a013b8)) * Transaction sender bot ([#7586](AztecProtocol/aztec-packages#7586)) ([176fd08](AztecProtocol/aztec-packages@176fd08)), closes [#7562](AztecProtocol/aztec-packages#7562) ### Bug Fixes * Aztec start --prover ([#7617](AztecProtocol/aztec-packages#7617)) ([ac206a6](AztecProtocol/aztec-packages@ac206a6)) ### Miscellaneous * Add tx bot to terraform ([#7609](AztecProtocol/aztec-packages#7609)) ([6caa914](AztecProtocol/aztec-packages@6caa914)) * Merge Devnet back to Master ([#7611](AztecProtocol/aztec-packages#7611)) ([112961c](AztecProtocol/aztec-packages@112961c)) </details> <details><summary>barretenberg.js: 0.47.1</summary> ## [0.47.1](AztecProtocol/aztec-packages@barretenberg.js-v0.47.0...barretenberg.js-v0.47.1) (2024-07-30) ### Miscellaneous * **barretenberg.js:** Synchronize aztec-packages versions </details> <details><summary>aztec-packages: 0.47.1</summary> ## [0.47.1](AztecProtocol/aztec-packages@aztec-packages-v0.47.0...aztec-packages-v0.47.1) (2024-07-30) ### Features * Add recursive aggregation object to proving/verification keys ([#6770](AztecProtocol/aztec-packages#6770)) ([f48b069](AztecProtocol/aztec-packages@f48b069)) * Adding aggregation to honk and rollup ([#7466](AztecProtocol/aztec-packages#7466)) ([2633aa9](AztecProtocol/aztec-packages@2633aa9)) * Allow get_notes to return zero notes ([#7621](AztecProtocol/aztec-packages#7621)) ([e16452a](AztecProtocol/aztec-packages@e16452a)) * **avm:** Pedersen commit in avm ([#7634](AztecProtocol/aztec-packages#7634)) ([45e7867](AztecProtocol/aztec-packages@45e7867)) * **avm:** Pedersen commitment sim ([#7632](AztecProtocol/aztec-packages#7632)) ([cc420a0](AztecProtocol/aztec-packages@cc420a0)) * Do not evict tx objects from p2p tx pool immediately ([#7652](AztecProtocol/aztec-packages#7652)) ([96492dc](AztecProtocol/aztec-packages@96492dc)) * Handle L1toL2 msgs in prover-node ([#7654](AztecProtocol/aztec-packages#7654)) ([f35bac5](AztecProtocol/aztec-packages@f35bac5)) * Leonidas ([#7597](AztecProtocol/aztec-packages#7597)) ([eda498a](AztecProtocol/aztec-packages@eda498a)) * Min and max block times for sequencer ([#7630](AztecProtocol/aztec-packages#7630)) ([2a013b8](AztecProtocol/aztec-packages@2a013b8)) * **sol-honk:** Integrate solidity honk verifier with bb and acir tests ([#7573](AztecProtocol/aztec-packages#7573)) ([344ca6f](AztecProtocol/aztec-packages@344ca6f)) * **sol-honk:** Test verifying recursive proof ([#7576](AztecProtocol/aztec-packages#7576)) ([26408c1](AztecProtocol/aztec-packages@26408c1)) * Static.aztec.network ([#7649](AztecProtocol/aztec-packages#7649)) ([9dffef5](AztecProtocol/aztec-packages@9dffef5)) * Transaction sender bot ([#7586](AztecProtocol/aztec-packages#7586)) ([176fd08](AztecProtocol/aztec-packages@176fd08)), closes [#7562](AztecProtocol/aztec-packages#7562) ### Bug Fixes * Aztec start --prover ([#7617](AztecProtocol/aztec-packages#7617)) ([ac206a6](AztecProtocol/aztec-packages@ac206a6)) * Bb mac build ([#7619](AztecProtocol/aztec-packages#7619)) ([e3c5602](AztecProtocol/aztec-packages@e3c5602)) * Faucet correctly derives its account ([#7645](AztecProtocol/aztec-packages#7645)) ([921773f](AztecProtocol/aztec-packages@921773f)) * Uniswap e2e test "nonce too low" ([#7633](AztecProtocol/aztec-packages#7633)) ([e9e2318](AztecProtocol/aztec-packages@e9e2318)) ### Miscellaneous * Add tx bot to terraform ([#7609](AztecProtocol/aztec-packages#7609)) ([6caa914](AztecProtocol/aztec-packages@6caa914)) * **bb readme:** Add installation instructions and TODOs ([#7601](AztecProtocol/aztec-packages#7601)) ([1a97698](AztecProtocol/aztec-packages@1a97698)) * **bb readme:** Update versioning instructions and add Honk Solidity verifier commands ([#7608](AztecProtocol/aztec-packages#7608)) ([9dd9195](AztecProtocol/aztec-packages@9dd9195)) * Bench client ivc proof verification ([#7553](AztecProtocol/aztec-packages#7553)) ([b060309](AztecProtocol/aztec-packages@b060309)) * Call requests ([#7483](AztecProtocol/aztec-packages#7483)) ([ffedf39](AztecProtocol/aztec-packages@ffedf39)) * Constant inputs for most blackboxes ([#7613](AztecProtocol/aztec-packages#7613)) ([3247058](AztecProtocol/aztec-packages@3247058)) * **docs:** Fix links ([#7602](AztecProtocol/aztec-packages#7602)) ([f2029be](AztecProtocol/aztec-packages@f2029be)) * Merge Devnet back to Master ([#7611](AztecProtocol/aztec-packages#7611)) ([112961c](AztecProtocol/aztec-packages@112961c)) * **p2p:** Tx serialization cleanup ([#7620](AztecProtocol/aztec-packages#7620)) ([022a899](AztecProtocol/aztec-packages@022a899)) * Replace relative paths to noir-protocol-circuits ([ebaafc7](AztecProtocol/aztec-packages@ebaafc7)) * Replace relative paths to noir-protocol-circuits ([d3b818d](AztecProtocol/aztec-packages@d3b818d)) * Replace relative paths to noir-protocol-circuits ([4a26a19](AztecProtocol/aztec-packages@4a26a19)) </details> <details><summary>barretenberg: 0.47.1</summary> ## [0.47.1](AztecProtocol/aztec-packages@barretenberg-v0.47.0...barretenberg-v0.47.1) (2024-07-30) ### Features * Add recursive aggregation object to proving/verification keys ([#6770](AztecProtocol/aztec-packages#6770)) ([f48b069](AztecProtocol/aztec-packages@f48b069)) * Adding aggregation to honk and rollup ([#7466](AztecProtocol/aztec-packages#7466)) ([2633aa9](AztecProtocol/aztec-packages@2633aa9)) * **avm:** Pedersen commit in avm ([#7634](AztecProtocol/aztec-packages#7634)) ([45e7867](AztecProtocol/aztec-packages@45e7867)) * **avm:** Pedersen commitment sim ([#7632](AztecProtocol/aztec-packages#7632)) ([cc420a0](AztecProtocol/aztec-packages@cc420a0)) * **sol-honk:** Integrate solidity honk verifier with bb and acir tests ([#7573](AztecProtocol/aztec-packages#7573)) ([344ca6f](AztecProtocol/aztec-packages@344ca6f)) * **sol-honk:** Test verifying recursive proof ([#7576](AztecProtocol/aztec-packages#7576)) ([26408c1](AztecProtocol/aztec-packages@26408c1)) ### Bug Fixes * Bb mac build ([#7619](AztecProtocol/aztec-packages#7619)) ([e3c5602](AztecProtocol/aztec-packages@e3c5602)) ### Miscellaneous * **bb readme:** Add installation instructions and TODOs ([#7601](AztecProtocol/aztec-packages#7601)) ([1a97698](AztecProtocol/aztec-packages@1a97698)) * **bb readme:** Update versioning instructions and add Honk Solidity verifier commands ([#7608](AztecProtocol/aztec-packages#7608)) ([9dd9195](AztecProtocol/aztec-packages@9dd9195)) * Call requests ([#7483](AztecProtocol/aztec-packages#7483)) ([ffedf39](AztecProtocol/aztec-packages@ffedf39)) * Constant inputs for most blackboxes ([#7613](AztecProtocol/aztec-packages#7613)) ([3247058](AztecProtocol/aztec-packages@3247058)) * Merge Devnet back to Master ([#7611](AztecProtocol/aztec-packages#7611)) ([112961c](AztecProtocol/aztec-packages@112961c)) </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Overview
Updated the existing solidity testing flow to run for a number of honk tests - some with public inputs and some without.