Skip to content

feat: Zw/goblin avm tests#12904

Merged
ledwards2225 merged 54 commits intomasterfrom
zw/goblin-avm-tests
Apr 1, 2025
Merged

feat: Zw/goblin avm tests#12904
ledwards2225 merged 54 commits intomasterfrom
zw/goblin-avm-tests

Conversation

@ledwards2225
Copy link
Contributor

@ledwards2225 ledwards2225 commented Mar 20, 2025

Introduces a new class AvmGoblinRecursiveVerifier that manages the Goblinized AVM recursive verification algorithm. The algorithm involves the construction of two circuits. The first is a Mega-arithmetized AVM recursive verifier. The second is an Ultra circuit that recursively verifies the proof of the first (which consists of a Mega proof plus a Goblin proof). The output of the algorithm is equivalent to a that of a Rollup Honk recursive verifier: a pairing point accumulator plus an IPA claim/proof (stemming from recursive verification of the ECCVM). See the class description for a more detailed description of the algorithm.

Currently the algorithm is tested in a standalone test only and is not yet integrated into ACIR. E.g. at this stage a noir program with a verify_proof call of type AVM will still generate a vanilla Ultra recursive verifier.

Note: This work is incomplete in a number of ways, the most significant of which I've documented in several sub-issues under this umbrella issue.


/**
* @brief Runs the Goblin recursive verifier consisting of ECCVM, Translator and Merge verifiers.
* // TODO(https://github.com/AztecProtocol/barretenberg/issues/1309): Implement correct pairing point aggregation.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

No meaningful changes here just highlighting yet another place where pairing point aggregation is missing.

// Add recursion constraints
size_t idx = 0;
for (auto& constraint : constraint_system.avm_recursion_constraints) {
// TODO(https://github.com/AztecProtocol/barretenberg/issues/1303): Utilize the version of this method that
Copy link
Contributor Author

@ledwards2225 ledwards2225 Mar 26, 2025

Choose a reason for hiding this comment

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

Leaving it as a TODO to actually enable in ACIR (via create_avm_recursion_constraints_goblin) the new algorithm implemented in this PR.

return P0 == other.P0 && P1 == other.P1;
};
template <typename BuilderType = void>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just making a general improvement here. The template param was not needed.

#include "barretenberg/vm/avm/trace/trace.hpp"
namespace bb::avm {

/**
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the main contribution of this PR. I've tried to describe the algorithm in sufficient detail here.

using GoblinRecursiveVerifier = stdlib::recursion::honk::GoblinRecursiveVerifier;
using GoblinRecursiveVerifierOutput = stdlib::recursion::honk::GoblinRecursiveVerifierOutput;

// STEP 1: To establish consistency of the the proof, public inputs and VK for the AVM between the inner (Mega)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// STEP 1: To establish consistency of the the proof, public inputs and VK for the AVM between the inner (Mega)
// STEP 1: To establish consistency of the proof, public inputs and VK for the AVM between the inner (Mega)

*
* The Ultra-arithmetized circuit C_U is responsible for recursive verification of {\pi_M, \pi_G}, i.e. it contains both
* a Mega and a Goblin recursive verifier. The output of this recursive verification is a pairing check accumulator and
* an IPA claim accumulator. To ensure proper transfer the AVM verifier inputs {\pi, pub_inputs, VK}_{AVM} between the
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* an IPA claim accumulator. To ensure proper transfer the AVM verifier inputs {\pi, pub_inputs, VK}_{AVM} between the
* an IPA claim accumulator. To ensure proper transfer of the AVM verifier inputs {\pi, pub_inputs, VK}_{AVM} between the

Copy link
Contributor

@lucasxia01 lucasxia01 left a comment

Choose a reason for hiding this comment

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

Seems pretty good to me in general. Just maybe can deduplicate some code with the Tube circuit.

* claims before final verification later on (e.g. at the root). This is analogous to the aggregation of pairing point
* inputs for proving systems that use KZG, such as Ultra/MegaHonk.
*
* The Ultra-arithmetized circuit C_U is responsible for recursive verification of {\pi_M, \pi_G}, i.e. it contains both
Copy link
Contributor

Choose a reason for hiding this comment

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

this circuit should be identical to the tube circuit right?

Copy link
Contributor

Choose a reason for hiding this comment

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

well apparently not because of this hash consistency thing

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah before looking at any of this my assumption was that they should essentially be the same. It seems possible actually that we need a similar hashing mechanism between the hiding circuit and the tube. In any case, doing this work revealed that "Goblin recursion" needs to become a more readily usable primitive that can be shared between the various places its used. I'll add a general issue to evaluate the differences between this context and the hiding/tube circuits and see whether one or the other needs to be updated and to in general share more code.

@ledwards2225 ledwards2225 merged commit baea4b3 into master Apr 1, 2025
7 checks passed
@ledwards2225 ledwards2225 deleted the zw/goblin-avm-tests branch April 1, 2025 03:55
charlielye pushed a commit that referenced this pull request Apr 3, 2025
🤖 I have created a new Aztec Packages release
---


##
[0.83.0](v0.82.3...v0.83.0)
(2025-04-03)


### ⚠ BREAKING CHANGES

* operation mouthwash
([#13171](#13171))
* change version and protocol version to rollupVersion
([#13145](#13145))
* processing events in Aztec.nr
([#12957](#12957))

### Features

* accept multiple consensus client endpoints
([#13022](#13022))
([1d0185d](1d0185d))
* add ecdsa non ssh account to cli wallet
([#13085](#13085))
([c5f9984](c5f9984))
* Add parent log link to top of CI logs.
([#13170](#13170))
([a851087](a851087))
* Add public data read gadget
([#13138](#13138))
([6bb76db](6bb76db))
* **avm:** merkle db hints (part 3)
([#13199](#13199))
([7f96676](7f96676))
* **Barretenberg:** static analyzer's routine
([#13207](#13207))
([84890c2](84890c2))
* derived pending notes capsules slot
([#13102](#13102))
([6307ba0](6307ba0))
* get mana limit from rollup by default.
([#13029](#13029))
([a406c54](a406c54))
* Increase CIVC depth with no rollup cost
([#13106](#13106))
([3a555ef](3a555ef))
* making SyncDataProvider throw before sync
([#13151](#13151))
([9840241](9840241))
* more benchmarks
([#13103](#13103))
([7a2c9b7](7a2c9b7))
* **noir:** Allow missing optional fields in msgpack
([#13141](#13141))
([493dede](493dede))
* processing events in Aztec.nr
([#12957](#12957))
([88c0e04](88c0e04))
* Prover id defaults to publisher address
([#13206](#13206))
([f459c3e](f459c3e))
* purge of log decryption in TS
([#12992](#12992))
([800ab8d](800ab8d))
* register private-only contracts in cli-wallet and misc improvements
([#13245](#13245))
([ad0530f](ad0530f))
* remove unary trick in decomposition and constraints polishing
([#13080](#13080))
([0e60255](0e60255))
* split public inputs from proof
([#12816](#12816))
([ca7151e](ca7151e))
* use magic address for fee juice portal in msgs
([#13241](#13241))
([6fbc378](6fbc378))
* util for computing proposer/forwarder address
([#13169](#13169))
([87809b2](87809b2))
* Zw/goblin avm tests
([#12904](#12904))
([baea4b3](baea4b3))


### Bug Fixes

* add check for rollup version in tx validator
([#13197](#13197))
([c8220c9](c8220c9)),
closes
[#13192](#13192)
* add flake
([13bdfa7](13bdfa7))
* add flakes
([547d50a](547d50a))
* **avm:** alu interface
([#13115](#13115))
([101ff78](101ff78))
* await transaction to make proposal
([#13132](#13132))
([180db9e](180db9e))
* bb mac publish on tag push
([#13174](#13174))
([8d46c8c](8d46c8c))
* bb workdir permission issue
([#13095](#13095))
([3685a80](3685a80))
* **bb:** dont publish .zst
([#13173](#13173))
([4dba4e9](4dba4e9))
* boolean config helper for cli args works now
([#13110](#13110))
([a93ce6e](a93ce6e))
* **docs:** Register FPC docs, contract deployment, events
([#13222](#13222))
([49aabfb](49aabfb))
* don't log sepolia acc mnemonic on github action
([#13178](#13178))
([809b44d](809b44d))
* eq instead of !==
([#13161](#13161))
([921e347](921e347))
* fetch the correct vk in getSolidityVerifier
([#13157](#13157))
([71b6719](71b6719))
* force anvil/blob networking to ipv4 on localhost. attempt to fix port
flakes
([#13099](#13099))
([970dae5](970dae5))
* fuzzing build issues
([#13114](#13114))
([bacae3d](bacae3d))
* Handle proven chain events referring to unseen blocks
([#13144](#13144))
([229515f](229515f)),
closes
[#13142](#13142)
* handling multiple identical logs in a tx
([#13184](#13184))
([c100499](c100499))
* incorrect blocknumber in syncTaggedLogs
([e58ea95](e58ea95))
* indexeddb multimap dupes
([#13254](#13254))
([1e470f0](1e470f0))
* load two more points
([#13119](#13119))
([2a2904a](2a2904a))
* Masternet to run with the nightly tag
([#13239](#13239))
([f16b70e](f16b70e))
* newline weirdness in package.json
([#13111](#13111))
([244ea99](244ea99))
* nightly deploy quotes
([#13253](#13253))
([775eecf](775eecf))
* prover config read from L1
([#13237](#13237))
([13426fe](13426fe))
* Race condition while unwinding blocks
([#13148](#13148))
([1c2291a](1c2291a))
* read and pass rollup version
([#13232](#13232))
([4ee1e4a](4ee1e4a))
* recursive sumcheck bugs
([#12885](#12885))
([a784802](a784802))
* reenable test cache
([4dfca94](4dfca94))
* remove [skip ci] edge condition
([#13228](#13228))
([4e699ea](4e699ea))
* separator in pending partial notes capsule array slot
([#13153](#13153))
([0d6ec63](0d6ec63))
* sysdig oops
([5cf6ad1](5cf6ad1))
* tagging bug
([#13061](#13061))
([280e1a6](280e1a6))
* test tracking failures in merge queue
([#13235](#13235))
([650fdc1](650fdc1))
* title and external check need to run in merge queue
([e326ed6](e326ed6))
* Transpile cmov
([#13194](#13194))
([6b94555](6b94555))
* trying to fix EADDRINUSE
([#13176](#13176))
([260a057](260a057))
* Uninstall gossipsub event handler on service stop
([#13190](#13190))
([081f30d](081f30d))
* use version from registry for rollup instead of config
([#12938](#12938))
([7dac390](7dac390))
* validate private double spends in txs with public funcs
([#13088](#13088))
([4555871](4555871))
* yolo flake and alert fix
([f8de52b](f8de52b))


### Miscellaneous

* add container id to netlog
([9ecdf15](9ecdf15))
* Add e2e test to bootstrap test_cmds
([#13146](#13146))
([09e4722](09e4722))
* add rollup version as universal cli option
([#13205](#13205))
([#13213](#13213))
([568d9e9](568d9e9))
* Add ultra versions of fuzzers in stdlib
([#13139](#13139))
([aea210b](aea210b))
* align TS and C++ indexed leaf types
([#13185](#13185))
([d2574cc](d2574cc))
* Assign bb test flake
([#13127](#13127))
([69fdb04](69fdb04))
* **avm:** remove check_interaction from tests
([#13136](#13136))
([7d875a6](7d875a6))
* change version and protocol version to rollupVersion
([#13145](#13145))
([24d7f8b](24d7f8b))
* Cleanup config vars for alpha-testnet
([#13204](#13204))
([153a606](153a606))
* convenient way to run app ivc from bb
([#13158](#13158))
([cb1a857](cb1a857))
* Cron snapshot upload in spartan
([#13108](#13108))
([7c520a8](7c520a8))
* Do not close store before stopping p2p client in tests
([#13223](#13223))
([63a843e](63a843e))
* docker flake diagnostics.
([48da272](48da272))
* **docs:** alpha-testnet versioning
([#13016](#13016))
([9ca5d3c](9ca5d3c))
* **docs:** Update CLI faucet command
([#13104](#13104))
([6eb71de](6eb71de))
* **docs:** Update versions-updating.md
([#13090](#13090))
([0310d4e](0310d4e))
* Enable debug logging for annoying unit tests
([#13191](#13191))
([5556524](5556524))
* enable sentinel in sentinel test
([#13219](#13219))
([f2cfe3f](f2cfe3f))
* Exclude nightly versions from stable fn
([#13196](#13196))
([a832e90](a832e90))
* Fix flake in e2e fees failures
([#13229](#13229))
([5defe47](5defe47))
* fix kind logs
([#13023](#13023))
([c6c2727](c6c2727)),
closes
[#13053](#13053)
* fuzzing build in ci
([#13105](#13105))
([1c08d38](1c08d38))
* Improve callstacks for public dispatch fns
([#13120](#13120))
([f67375d](f67375d))
* log out the slash factory when a new rollup is deployed
([#13131](#13131))
([4643a31](4643a31))
* Merge alpha back to master
([#13128](#13128))
([504c338](504c338))
* metric attributes
([#13126](#13126))
([f87d5e3](f87d5e3)),
closes
[#13063](#13063)
* minor tagging API improvement
([#13092](#13092))
([f5bcece](f5bcece))
* more benchmarking
([#13211](#13211))
([4b0a4ad](4b0a4ad))
* move unbound impl generics to their functions
([#13147](#13147))
([62f497f](62f497f))
* operation mouthwash
([#13171](#13171))
([384f4e5](384f4e5))
* remove catch-all branch from opcode match in transpiler
([#13210](#13210))
([2bc9ca2](2bc9ca2))
* remove templating by flavor in merge protocol
([#13098](#13098))
([cf5e217](cf5e217))
* rename journal dir and file to state manager & mv to up to public/
([#13159](#13159))
([f13be09](f13be09))
* replace relative paths to noir-protocol-circuits
([f18336d](f18336d))
* replace relative paths to noir-protocol-circuits
([d620c12](d620c12))
* replace relative paths to noir-protocol-circuits
([be3e74e](be3e74e))
* replace relative paths to noir-protocol-circuits
([5bfac63](5bfac63))
* replace relative paths to noir-protocol-circuits
([f41ab2c](f41ab2c))
* replace relative paths to noir-protocol-circuits
([806e560](806e560))
* streamlined log processing
([#13107](#13107))
([b184865](b184865))
* use testnet optimized trace
([#13135](#13135))
([5a4f2ac](5a4f2ac))
* uses a new larger redis cache. flake file tweaks.
([#13167](#13167))
([0951fb6](0951fb6))


### Documentation

* Add intermediate migration note versions
([#13183](#13183))
([f5de7b8](f5de7b8))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
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.

4 participants