Skip to content

chore: migrate recursive proof test to ultrahonk#8038

Merged
TomAFrench merged 11 commits intomasterfrom
tf/migrate-js-recursion
Apr 22, 2025
Merged

chore: migrate recursive proof test to ultrahonk#8038
TomAFrench merged 11 commits intomasterfrom
tf/migrate-js-recursion

Conversation

@TomAFrench
Copy link
Member

@TomAFrench TomAFrench commented Apr 11, 2025

Looking to get a working e2e example of a recursive ultrahonk program being verified. It doesn't need to be done onchain yet but we should have an example on how to do this in JS (people are coming in to make their own at this point).

Issues I ran into here:

  • How big is a UH proof? It's not documented anywhere.
    • I've pulled the new values from verify_honk_proof test in aztec-packages.
    • SIZE_OF_PROOF_IF_LOGN_IS_28 implies that this depends on circuit size (which I remember being a thing at one point). Is there a way I can get bb.js to calculate this for me rather than having to do trial and error?
  • How big is a UH verification key in fields? It's not documented anywhere.
  • How do I get the verification key in JS?
  • How do I get the verification key hash in JS?

I threw this together based on the barretenberg readme and pulling code out of bb.js when necessary components aren't exposed. However I'm now getting unreachable errors inside acirWriteVkUltraHonk for some reason I haven't debugged as I'm blocked on the verification key hash anyway.

@ledwards2225 Are there resources which I'm missing on how to get these values out of barretenberg in JS? Perhaps @saleel can help do a pass on UX of bb.js for recursion considering Noirhack is coming up next week?

@saleel
Copy link
Member

saleel commented Apr 15, 2025

I think UltraHonkBackend::getVerificationKey() + deflattenFields(vk) should solve the problem. 0x0 can be passed as the value for vk_hash for now. I can continue on this PR and push to the same branch if its ok.

We normally point to noir-bb-bench as an example for recursion.
After 0.85.0 the proof size of recursive and non-recursive proof will be the same, so this will be much cleaner. I think the methods in bb.js can be improved as well. I will take a look this week.

What we really need is a documentation website for bb that explain all these flows (It is being worked on)

@TomAFrench
Copy link
Member Author

TomAFrench commented Apr 15, 2025

Hey, help on getting this PR over the line would be greatly appreciated. 🙏

I think UltraHonkBackend::getVerificationKey() + deflattenFields(vk) should solve the problem.

I think that depends on whether all elements in the verification key are fields though? I remember with UP there was tight packing of some smaller types so I wasn't sure that we could just chunk it into 32 bytes segments.

In any case, having a dedicated "convert this binary blob of a vk into fields to be fed into a circuit" function in bb.js (even if it's just a deflattenFields(vk) under the hood) would be amazing and take all guesswork out of the user code.

After 0.85.0 the proof size of recursive and non-recursive proof will be the same, so this will be much cleaner. I think the methods in bb.js can be improved as well. I will take a look this week.

Awesome, thank you so much. :)

On a related note, we should probably remove the generateRecursiveProofArtifacts method as it's causing more confusion that good now.

@saleel
Copy link
Member

saleel commented Apr 15, 2025

On a related note, we should probably remove the generateRecursiveProofArtifacts method as it's causing more confusion that good now.

Yes, this one should be removed. This was never implemented for Honk, but another method generateProofForRecursiveAggregation() which does the same thing was added on. We can use this method to fix this PR, but looks like the method got removed with 0.85.0. So I think we need to fix in bb.js first.
I will look in to this.

@saleel saleel force-pushed the tf/migrate-js-recursion branch from 90e2f6a to 1896b0c Compare April 22, 2025 07:20
Copy link
Contributor

@github-actions github-actions bot left a 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 'Compilation Time'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite Current: 3d92cdc Previous: c69acd7 Ratio
ram_blowup_regression 16.1 s 13.1 s 1.23

This comment was automatically generated by workflow using github-action-benchmark.

CC: @TomAFrench

@saleel
Copy link
Member

saleel commented Apr 22, 2025

I've pushed changes to make it work with bb 0.84.0 (current bb version in master)

Edit: Ah just noticed the test is supposed to verify in Solidity...updating

@saleel saleel marked this pull request as ready for review April 22, 2025 08:28
@saleel
Copy link
Member

saleel commented Apr 22, 2025

Updated to verify with Solidity.

  • This test was compiling the inner circuit with wasm (and toml parser to prepare inputs). Changing this to use an existing compiled circuit.

@TomAFrench TomAFrench added this pull request to the merge queue Apr 22, 2025
@TomAFrench TomAFrench removed this pull request from the merge queue due to a manual request Apr 22, 2025
@TomAFrench TomAFrench enabled auto-merge April 22, 2025 11:15
@TomAFrench TomAFrench added this pull request to the merge queue Apr 22, 2025
Merged via the queue into master with commit 3024dd3 Apr 22, 2025
113 checks passed
@TomAFrench TomAFrench deleted the tf/migrate-js-recursion branch April 22, 2025 11:44
github-merge-queue bot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request Apr 28, 2025
Automated pull of nightly from the
[noir](https://github.com/noir-lang/noir) programming language, a
dependency of Aztec.
BEGIN_COMMIT_OVERRIDE
feat: let `nargo test` use the new greybox fuzzer
(noir-lang/noir#8234)
chore(docs): add fixed bugs to list
(noir-lang/noir#8201)
fix(parser): do not use `Ident::default()`
(noir-lang/noir#8224)
feat(fuzz): Add experimental comptime code generator
(noir-lang/noir#8207)
fix: let SSA parser parse calls to print
(noir-lang/noir#8232)
feat: added 1, -1 case for inversions in brillig
(noir-lang/noir#8225)
chore: clippy (noir-lang/noir#8220)
fix: don't take implicitly added named generics when checking where
clauses (noir-lang/noir#8184)
chore(docs): Defunctionalization and some minor cleanup
(noir-lang/noir#8217)
feat: add `--no-fuzz` and `--only-fuzz` to `nargo test`
(noir-lang/noir#8213)
chore: add regression test for panic encountered in bigcurve library.
(noir-lang/noir#8211)
fix: validate numeric types when passing them to a FunctionBuilder
(noir-lang/noir#8215)
chore: refactor constrainedness checks on trait impls
(noir-lang/noir#8170)
chore(test): Add smoke test for AST generation
(noir-lang/noir#8048)
chore: bump more dependencies
(noir-lang/noir#8208)
chore: Don't use `i1` in the AST fuzzer
(noir-lang/noir#8204)
fix: Do not panic if RHS constant in division has more bits than the
operand (noir-lang/noir#8197)
chore: bump cargo deps (noir-lang/noir#8205)
fix: Handle truncating constants to 128 bits
(noir-lang/noir#8180)
fix!: disallow the `i1` type
(noir-lang/noir#8172)
chore(fuzz): Make sure `main` makes at least one call
(noir-lang/noir#8202)
fix: Use `get_local_or_global_instruction` in
`try_optimize_array_set_from_previous_get`
(noir-lang/noir#8200)
fix: returns 0 for right shift overflow
(noir-lang/noir#8189)
feat: avoid overflow check when subtracting from a value that is the
maximum for its bitsize (noir-lang/noir#8190)
feat: optimize `checked_to_unchecked` to take into account
multiplications (noir-lang/noir#8188)
chore: document ssa `inline_simple_functions`
(noir-lang/noir#8169)
chore(docs): remove_unreachable SSA pass
(noir-lang/noir#8196)
chore: Change AST fuzzer recursion limit
(noir-lang/noir#8173)
chore(acir): Unify how we display witness indices
(noir-lang/noir#8192)
fix: Detect ABI change of fuzzing harnesses
(noir-lang/noir#8193)
chore(acir): Test that `BLACKBOX::RANGE` display the number of bits
(noir-lang/noir#8191)
fix(debugger): send idle at loop end + fix test
(noir-lang/noir#8187)
fix: Use `get_local_or_global_instruction` when simplifying `IfElse`
(noir-lang/noir#8185)
fix(parser): avoid using `Location::dummy()`
(noir-lang/noir#8178)
chore: Push a bug list (noir-lang/noir#8186)
feat(greybox_fuzzer): Should_fail and should_fail_with
(noir-lang/noir#8118)
chore: move unsigned overflow check from acir/brillig to ssa
(noir-lang/noir#8163)
feat: location tree for debug_info
(noir-lang/noir#7034)
fix: Use `IntegerConstant` for loop boundaries in `unrolling`
(noir-lang/noir#8094)
fix(ssa): Recursive shared Brillig entry points
(noir-lang/noir#8099)
chore: enable '--pedantic-solving' on more tests
(noir-lang/noir#7701)
chore(readme): Update `acvm-repo` READMEs
(noir-lang/noir#8150)
chore(test): add `test_programs` dir for expected-panic tests
(noir-lang/noir#8147)
chore(docs): minor updates in solidity doc
(noir-lang/noir#8160)
feat(debugger): debug test functions
(noir-lang/noir#7958)
chore: migrate recursive proof test to ultrahonk
(noir-lang/noir#8038)
chore: bump glob package (noir-lang/noir#8159)
chore: bump external pinned commits
(noir-lang/noir#8139)
chore(docs): patch web app tutorial in 1.0.0-beta3 versioned docs
(noir-lang/noir#8158)
END_COMMIT_OVERRIDE

---------

Co-authored-by: AztecBot <tech@aztecprotocol.com>
Co-authored-by: Ary Borenszweig <asterite@gmail.com>
Co-authored-by: TomAFrench <tom@tomfren.ch>
github-merge-queue bot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request Apr 28, 2025
Automated pull of nightly from the
[noir](https://github.com/noir-lang/noir) programming language, a
dependency of Aztec.
BEGIN_COMMIT_OVERRIDE
feat: let `nargo test` use the new greybox fuzzer
(noir-lang/noir#8234)
chore(docs): add fixed bugs to list
(noir-lang/noir#8201)
fix(parser): do not use `Ident::default()`
(noir-lang/noir#8224)
feat(fuzz): Add experimental comptime code generator
(noir-lang/noir#8207)
fix: let SSA parser parse calls to print
(noir-lang/noir#8232)
feat: added 1, -1 case for inversions in brillig
(noir-lang/noir#8225)
chore: clippy (noir-lang/noir#8220)
fix: don't take implicitly added named generics when checking where
clauses (noir-lang/noir#8184)
chore(docs): Defunctionalization and some minor cleanup
(noir-lang/noir#8217)
feat: add `--no-fuzz` and `--only-fuzz` to `nargo test`
(noir-lang/noir#8213)
chore: add regression test for panic encountered in bigcurve library.
(noir-lang/noir#8211)
fix: validate numeric types when passing them to a FunctionBuilder
(noir-lang/noir#8215)
chore: refactor constrainedness checks on trait impls
(noir-lang/noir#8170)
chore(test): Add smoke test for AST generation
(noir-lang/noir#8048)
chore: bump more dependencies
(noir-lang/noir#8208)
chore: Don't use `i1` in the AST fuzzer
(noir-lang/noir#8204)
fix: Do not panic if RHS constant in division has more bits than the
operand (noir-lang/noir#8197)
chore: bump cargo deps (noir-lang/noir#8205)
fix: Handle truncating constants to 128 bits
(noir-lang/noir#8180)
fix!: disallow the `i1` type
(noir-lang/noir#8172)
chore(fuzz): Make sure `main` makes at least one call
(noir-lang/noir#8202)
fix: Use `get_local_or_global_instruction` in
`try_optimize_array_set_from_previous_get`
(noir-lang/noir#8200)
fix: returns 0 for right shift overflow
(noir-lang/noir#8189)
feat: avoid overflow check when subtracting from a value that is the
maximum for its bitsize (noir-lang/noir#8190)
feat: optimize `checked_to_unchecked` to take into account
multiplications (noir-lang/noir#8188)
chore: document ssa `inline_simple_functions`
(noir-lang/noir#8169)
chore(docs): remove_unreachable SSA pass
(noir-lang/noir#8196)
chore: Change AST fuzzer recursion limit
(noir-lang/noir#8173)
chore(acir): Unify how we display witness indices
(noir-lang/noir#8192)
fix: Detect ABI change of fuzzing harnesses
(noir-lang/noir#8193)
chore(acir): Test that `BLACKBOX::RANGE` display the number of bits
(noir-lang/noir#8191)
fix(debugger): send idle at loop end + fix test
(noir-lang/noir#8187)
fix: Use `get_local_or_global_instruction` when simplifying `IfElse`
(noir-lang/noir#8185)
fix(parser): avoid using `Location::dummy()`
(noir-lang/noir#8178)
chore: Push a bug list (noir-lang/noir#8186)
feat(greybox_fuzzer): Should_fail and should_fail_with
(noir-lang/noir#8118)
chore: move unsigned overflow check from acir/brillig to ssa
(noir-lang/noir#8163)
feat: location tree for debug_info
(noir-lang/noir#7034)
fix: Use `IntegerConstant` for loop boundaries in `unrolling`
(noir-lang/noir#8094)
fix(ssa): Recursive shared Brillig entry points
(noir-lang/noir#8099)
chore: enable '--pedantic-solving' on more tests
(noir-lang/noir#7701)
chore(readme): Update `acvm-repo` READMEs
(noir-lang/noir#8150)
chore(test): add `test_programs` dir for expected-panic tests
(noir-lang/noir#8147)
chore(docs): minor updates in solidity doc
(noir-lang/noir#8160)
feat(debugger): debug test functions
(noir-lang/noir#7958)
chore: migrate recursive proof test to ultrahonk
(noir-lang/noir#8038)
chore: bump glob package (noir-lang/noir#8159)
chore: bump external pinned commits
(noir-lang/noir#8139)
chore(docs): patch web app tutorial in 1.0.0-beta3 versioned docs
(noir-lang/noir#8158)
END_COMMIT_OVERRIDE

---------

Co-authored-by: AztecBot <tech@aztecprotocol.com>
Co-authored-by: Ary Borenszweig <asterite@gmail.com>
Co-authored-by: TomAFrench <tom@tomfren.ch>
github-merge-queue bot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request Apr 28, 2025
Automated pull of nightly from the
[noir](https://github.com/noir-lang/noir) programming language, a
dependency of Aztec.
BEGIN_COMMIT_OVERRIDE
feat: let `nargo test` use the new greybox fuzzer
(noir-lang/noir#8234)
chore(docs): add fixed bugs to list
(noir-lang/noir#8201)
fix(parser): do not use `Ident::default()`
(noir-lang/noir#8224)
feat(fuzz): Add experimental comptime code generator
(noir-lang/noir#8207)
fix: let SSA parser parse calls to print
(noir-lang/noir#8232)
feat: added 1, -1 case for inversions in brillig
(noir-lang/noir#8225)
chore: clippy (noir-lang/noir#8220)
fix: don't take implicitly added named generics when checking where
clauses (noir-lang/noir#8184)
chore(docs): Defunctionalization and some minor cleanup
(noir-lang/noir#8217)
feat: add `--no-fuzz` and `--only-fuzz` to `nargo test`
(noir-lang/noir#8213)
chore: add regression test for panic encountered in bigcurve library.
(noir-lang/noir#8211)
fix: validate numeric types when passing them to a FunctionBuilder
(noir-lang/noir#8215)
chore: refactor constrainedness checks on trait impls
(noir-lang/noir#8170)
chore(test): Add smoke test for AST generation
(noir-lang/noir#8048)
chore: bump more dependencies
(noir-lang/noir#8208)
chore: Don't use `i1` in the AST fuzzer
(noir-lang/noir#8204)
fix: Do not panic if RHS constant in division has more bits than the
operand (noir-lang/noir#8197)
chore: bump cargo deps (noir-lang/noir#8205)
fix: Handle truncating constants to 128 bits
(noir-lang/noir#8180)
fix!: disallow the `i1` type
(noir-lang/noir#8172)
chore(fuzz): Make sure `main` makes at least one call
(noir-lang/noir#8202)
fix: Use `get_local_or_global_instruction` in
`try_optimize_array_set_from_previous_get`
(noir-lang/noir#8200)
fix: returns 0 for right shift overflow
(noir-lang/noir#8189)
feat: avoid overflow check when subtracting from a value that is the
maximum for its bitsize (noir-lang/noir#8190)
feat: optimize `checked_to_unchecked` to take into account
multiplications (noir-lang/noir#8188)
chore: document ssa `inline_simple_functions`
(noir-lang/noir#8169)
chore(docs): remove_unreachable SSA pass
(noir-lang/noir#8196)
chore: Change AST fuzzer recursion limit
(noir-lang/noir#8173)
chore(acir): Unify how we display witness indices
(noir-lang/noir#8192)
fix: Detect ABI change of fuzzing harnesses
(noir-lang/noir#8193)
chore(acir): Test that `BLACKBOX::RANGE` display the number of bits
(noir-lang/noir#8191)
fix(debugger): send idle at loop end + fix test
(noir-lang/noir#8187)
fix: Use `get_local_or_global_instruction` when simplifying `IfElse`
(noir-lang/noir#8185)
fix(parser): avoid using `Location::dummy()`
(noir-lang/noir#8178)
chore: Push a bug list (noir-lang/noir#8186)
feat(greybox_fuzzer): Should_fail and should_fail_with
(noir-lang/noir#8118)
chore: move unsigned overflow check from acir/brillig to ssa
(noir-lang/noir#8163)
feat: location tree for debug_info
(noir-lang/noir#7034)
fix: Use `IntegerConstant` for loop boundaries in `unrolling`
(noir-lang/noir#8094)
fix(ssa): Recursive shared Brillig entry points
(noir-lang/noir#8099)
chore: enable '--pedantic-solving' on more tests
(noir-lang/noir#7701)
chore(readme): Update `acvm-repo` READMEs
(noir-lang/noir#8150)
chore(test): add `test_programs` dir for expected-panic tests
(noir-lang/noir#8147)
chore(docs): minor updates in solidity doc
(noir-lang/noir#8160)
feat(debugger): debug test functions
(noir-lang/noir#7958)
chore: migrate recursive proof test to ultrahonk
(noir-lang/noir#8038)
chore: bump glob package (noir-lang/noir#8159)
chore: bump external pinned commits
(noir-lang/noir#8139)
chore(docs): patch web app tutorial in 1.0.0-beta3 versioned docs
(noir-lang/noir#8158)
END_COMMIT_OVERRIDE

---------

Co-authored-by: AztecBot <tech@aztecprotocol.com>
Co-authored-by: Ary Borenszweig <asterite@gmail.com>
Co-authored-by: TomAFrench <tom@tomfren.ch>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants