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

chore: turn on masking in ultra and mega zk + oink clean-up #11693

Merged
merged 5 commits into from
Feb 4, 2025
Merged

chore: turn on masking in ultra and mega zk + oink clean-up #11693

merged 5 commits into from
Feb 4, 2025

Conversation

iakovenkos
Copy link
Contributor

@iakovenkos iakovenkos commented Feb 3, 2025 •
edited
Loading

We have a mechanism to mask witness commitments and evaluations in ZK Flavors, in this PR the masking is enabled.

It is also a precursor to short scalars in UH ZK Recursive verifier, as it eliminates the edge cases from bn254_endo_batch_mul.

Cleaned up oink prover using a newly introduced commit_with_type method

@iakovenkos iakovenkos marked this pull request as ready for review February 3, 2025 15:22
ledwards2225
ledwards2225 approved these changes Feb 3, 2025
View reviewed changes
Copy link
Contributor

@ledwards2225 ledwards2225 left a comment

Choose a reason for hiding this comment

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

LG- just a few comments

barretenberg/cpp/src/barretenberg/ultra_honk/mega_honk.test.cpp Outdated
@@ -113,6 +113,11 @@ TYPED_TEST(MegaHonkTests, Basic)
TYPED_TEST(MegaHonkTests, BasicStructured)
{
using Flavor = TypeParam;

// MegaZKFlavor is only used to prove the Hiding Circuit.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure I understand why we're skipping these specific tests. Is it because the mechanism doesn't work in the structured trace case? Can you make this a little more explicit?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done, agree that the original comment was misleading

I'm planning to switch the hiding circuit from Mega to MegaZK in a follow-up, will investigate how much time we spend on commitments and whether we could use structured polys there.

barretenberg/cpp/src/barretenberg/ultra_honk/oink_prover.cpp Outdated
transcript->send_to_verifier(domain_separator + wire_labels[idx], wire_comms[idx]);
commit_to_witness_polynomial(proving_key->proving_key.polynomials.w_l, commitment_labels.w_l);
commit_to_witness_polynomial(proving_key->proving_key.polynomials.w_r, commitment_labels.w_r);
commit_to_witness_polynomial(proving_key->proving_key.polynomials.w_o, commitment_labels.w_o);
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems like this will never use the structured commit - am I missing something?

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!

barretenberg/cpp/src/barretenberg/ultra_honk/oink_prover.hpp Outdated
private:
static void mask_witness_polynomial(Polynomial<FF>& polynomial)
{
const size_t circuit_size = polynomial.virtual_size();
Copy link
Contributor

Choose a reason for hiding this comment

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

The constexpr check here is redundant with the one in commit_to_witness_polynomial. If you want to ensure its only used for ZK flavors you could use a concept requires

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!

barretenberg/cpp/src/barretenberg/ultra_honk/oink_prover.hpp
* @param label
* @param type
*/
void commit_to_witness_polynomial(Polynomial<FF>& polynomial,
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps this is a bit of a nitpick but I don't love a method with a simple name that does more than it says, i.e. this doesn't just commit, it potentially adds blinding and adds the commit to the proof. I don't feel strongly though. Either way, could you move the definitions of any new methods to the .cpp file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, will move to cpp.

I see your point, the justification I had for myself is that commit and send_to_verifier are basically inseparable + when I added branching on ZK in each method, they became unreadable

ledwards2225
ledwards2225 reviewed Feb 4, 2025
View reviewed changes
barretenberg/cpp/src/barretenberg/ultra_honk/oink_prover.cpp
proving_key->proving_key.commitment_key->commit(proving_key->proving_key.polynomials.lookup_read_counts);
witness_commitments.lookup_read_tags =
proving_key->proving_key.commitment_key->commit(proving_key->proving_key.polynomials.lookup_read_tags);
commit_to_witness_polynomial(proving_key->proving_key.polynomials.lookup_read_counts,
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please turn on the use of commit_sparse here? Its not being used in master either but it definitely should be used

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done, I'm seeing -70ms in ClientIVCBench

@iakovenkos iakovenkos self-assigned this Feb 4, 2025
@iakovenkos iakovenkos merged commit 08e96fe into master Feb 4, 2025
25 checks passed
@iakovenkos iakovenkos deleted the si/enable-masking-ultra-mega branch February 4, 2025 10:54
@AztecBot AztecBot mentioned this pull request Feb 4, 2025
Merged
sklppy88 pushed a commit that referenced this pull request Feb 4, 2025
@AztecBot
chore(master): Release 0.74.0 (#11676)
08fc279 
🤖 I have created a release *beep* *boop*
---


<details><summary>aztec-package: 0.74.0</summary>

##
[0.74.0](aztec-package-v0.73.0...aztec-package-v0.74.0)
(2025-02-04)


### Miscellaneous

* Ensure new kv-store is used on the server
([#11662](#11662))
([aee1420](aee1420))
</details>

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

##
[0.74.0](barretenberg.js-v0.73.0...barretenberg.js-v0.74.0)
(2025-02-04)


### Miscellaneous

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

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

##
[0.74.0](aztec-packages-v0.73.0...aztec-packages-v0.74.0)
(2025-02-04)


### ⚠ BREAKING CHANGES

* time library
([#11542](#11542))

### Features

* `u128.ts` accepting string on input
([#11664](#11664))
([bb25992](bb25992))
* Add network, better drawer performance
([#11694](#11694))
([1f61822](1f61822))
* Skip calculation of partial sums when simulating blobs
([#11257](#11257))
([aca66f7](aca66f7))
* Time library
([#11542](#11542))
([3b463f9](3b463f9)),
closes
[#11520](#11520)
* UltraHonkZK contract
([#11553](#11553))
([a68369f](a68369f))


### Bug Fixes

* Add bootstrap.sh to rebuild_patterns
([#11683](#11683))
([e84a81a](e84a81a))
* **archiver:** Do not attempt to decode blob before filtering
([#11668](#11668))
([961cbdd](961cbdd))
* Barretenber/stdlib/logic bugs
([#11651](#11651))
([dddab22](dddab22))
* Barretenberg/stdlib/logic bugs (redo)
([#11691](#11691))
([6d0bad7](6d0bad7))
* **docs:** Keys docs update
([#11665](#11665))
([ce3d92c](ce3d92c))
* Revert "barretenberg/stdlib/logic bugs"
([#11689](#11689))
([b99570d](b99570d))
* Solidity verifier caching
([#11712](#11712))
([2ba1e71](2ba1e71))
* Use eth-execution label
([#11713](#11713))
([d3c31d8](d3c31d8))


### Miscellaneous

* Add tests for gov proposer
([#11633](#11633))
([5c6a48a](5c6a48a)),
closes
[#11681](#11681)
* **bb-prover:** Avm test skip and split
([#11717](#11717))
([1778867](1778867))
* Benchmark sha256 number of instructions executed in AVM
([#11253](#11253))
([aaf0d8c](aaf0d8c))
* Delete MerkleTrees implementation in JS
([#11697](#11697))
([1db7b78](1db7b78))
* Ensure new kv-store is used on the server
([#11662](#11662))
([aee1420](aee1420))
* Field encoding should use `fromString` instead of `fromHexString`
([#11585](#11585))
([43fdbb1](43fdbb1)),
closes
[#10331](#10331)
* Improve boxes
([#11656](#11656))
([46a3e85](46a3e85))
* Increase node pool count and don't use a release channel
([#11687](#11687))
([65a3f11](65a3f11))
* Mark contracts as pub
([#11241](#11241))
([b168601](b168601))
* Reduce memory requests on prover node
([#11678](#11678))
([a720151](a720151))
* Remove profiler cache fallback
([#11680](#11680))
([a305aef](a305aef))
* Remove some templates in templates
([#11698](#11698))
([61614b1](61614b1))
* Remove unused functions from public side effect trace
([#11600](#11600))
([54e9602](54e9602))
* Replace relative paths to noir-protocol-circuits
([739151e](739151e))
* Replace relative paths to noir-protocol-circuits
([bbd526c](bbd526c))
* **sequencer:** Add InvalidArchive to canProposeAtNextEthBlock ignored
errors
([#11682](#11682))
([eea4bd3](eea4bd3))
* **spartan:** Remove hardcoded keys and addresses - derive all from
mnemonic
([#11672](#11672))
([65f0e48](65f0e48))
* Turn off auto-upgrade in node-pools
([#11679](#11679))
([09f98a9](09f98a9))
* Turn on masking in ultra and mega zk + oink clean-up
([#11693](#11693))
([08e96fe](08e96fe))


### Documentation

* Update mig notes release version
([#11685](#11685))
([46a30b5](46a30b5))
</details>

<details><summary>barretenberg: 0.74.0</summary>

##
[0.74.0](barretenberg-v0.73.0...barretenberg-v0.74.0)
(2025-02-04)


### Features

* UltraHonkZK contract
([#11553](#11553))
([a68369f](a68369f))


### Bug Fixes

* Barretenber/stdlib/logic bugs
([#11651](#11651))
([dddab22](dddab22))
* Barretenberg/stdlib/logic bugs (redo)
([#11691](#11691))
([6d0bad7](6d0bad7))
* Revert "barretenberg/stdlib/logic bugs"
([#11689](#11689))
([b99570d](b99570d))


### Miscellaneous

* Ensure new kv-store is used on the server
([#11662](#11662))
([aee1420](aee1420))
* Remove some templates in templates
([#11698](#11698))
([61614b1](61614b1))
* Turn on masking in ultra and mega zk + oink clean-up
([#11693](#11693))
([08e96fe](08e96fe))
</details>

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
AztecBot added a commit to AztecProtocol/barretenberg that referenced this pull request Feb 5, 2025
@AztecBot
chore(master): Release 0.74.0 (#11676)
066edad 
🤖 I have created a release *beep* *boop*
---


<details><summary>aztec-package: 0.74.0</summary>

##
[0.74.0](AztecProtocol/aztec-packages@aztec-package-v0.73.0...aztec-package-v0.74.0)
(2025-02-04)


### Miscellaneous

* Ensure new kv-store is used on the server
([#11662](AztecProtocol/aztec-packages#11662))
([aee1420](AztecProtocol/aztec-packages@aee1420))
</details>

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

##
[0.74.0](AztecProtocol/aztec-packages@barretenberg.js-v0.73.0...barretenberg.js-v0.74.0)
(2025-02-04)


### Miscellaneous

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

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

##
[0.74.0](AztecProtocol/aztec-packages@aztec-packages-v0.73.0...aztec-packages-v0.74.0)
(2025-02-04)


### ⚠ BREAKING CHANGES

* time library
([#11542](AztecProtocol/aztec-packages#11542))

### Features

* `u128.ts` accepting string on input
([#11664](AztecProtocol/aztec-packages#11664))
([bb25992](AztecProtocol/aztec-packages@bb25992))
* Add network, better drawer performance
([#11694](AztecProtocol/aztec-packages#11694))
([1f61822](AztecProtocol/aztec-packages@1f61822))
* Skip calculation of partial sums when simulating blobs
([#11257](AztecProtocol/aztec-packages#11257))
([aca66f7](AztecProtocol/aztec-packages@aca66f7))
* Time library
([#11542](AztecProtocol/aztec-packages#11542))
([3b463f9](AztecProtocol/aztec-packages@3b463f9)),
closes
[#11520](AztecProtocol/aztec-packages#11520)
* UltraHonkZK contract
([#11553](AztecProtocol/aztec-packages#11553))
([a68369f](AztecProtocol/aztec-packages@a68369f))


### Bug Fixes

* Add bootstrap.sh to rebuild_patterns
([#11683](AztecProtocol/aztec-packages#11683))
([e84a81a](AztecProtocol/aztec-packages@e84a81a))
* **archiver:** Do not attempt to decode blob before filtering
([#11668](AztecProtocol/aztec-packages#11668))
([961cbdd](AztecProtocol/aztec-packages@961cbdd))
* Barretenber/stdlib/logic bugs
([#11651](AztecProtocol/aztec-packages#11651))
([dddab22](AztecProtocol/aztec-packages@dddab22))
* Barretenberg/stdlib/logic bugs (redo)
([#11691](AztecProtocol/aztec-packages#11691))
([6d0bad7](AztecProtocol/aztec-packages@6d0bad7))
* **docs:** Keys docs update
([#11665](AztecProtocol/aztec-packages#11665))
([ce3d92c](AztecProtocol/aztec-packages@ce3d92c))
* Revert "barretenberg/stdlib/logic bugs"
([#11689](AztecProtocol/aztec-packages#11689))
([b99570d](AztecProtocol/aztec-packages@b99570d))
* Solidity verifier caching
([#11712](AztecProtocol/aztec-packages#11712))
([2ba1e71](AztecProtocol/aztec-packages@2ba1e71))
* Use eth-execution label
([#11713](AztecProtocol/aztec-packages#11713))
([d3c31d8](AztecProtocol/aztec-packages@d3c31d8))


### Miscellaneous

* Add tests for gov proposer
([#11633](AztecProtocol/aztec-packages#11633))
([5c6a48a](AztecProtocol/aztec-packages@5c6a48a)),
closes
[#11681](AztecProtocol/aztec-packages#11681)
* **bb-prover:** Avm test skip and split
([#11717](AztecProtocol/aztec-packages#11717))
([1778867](AztecProtocol/aztec-packages@1778867))
* Benchmark sha256 number of instructions executed in AVM
([#11253](AztecProtocol/aztec-packages#11253))
([aaf0d8c](AztecProtocol/aztec-packages@aaf0d8c))
* Delete MerkleTrees implementation in JS
([#11697](AztecProtocol/aztec-packages#11697))
([1db7b78](AztecProtocol/aztec-packages@1db7b78))
* Ensure new kv-store is used on the server
([#11662](AztecProtocol/aztec-packages#11662))
([aee1420](AztecProtocol/aztec-packages@aee1420))
* Field encoding should use `fromString` instead of `fromHexString`
([#11585](AztecProtocol/aztec-packages#11585))
([43fdbb1](AztecProtocol/aztec-packages@43fdbb1)),
closes
[#10331](AztecProtocol/aztec-packages#10331)
* Improve boxes
([#11656](AztecProtocol/aztec-packages#11656))
([46a3e85](AztecProtocol/aztec-packages@46a3e85))
* Increase node pool count and don't use a release channel
([#11687](AztecProtocol/aztec-packages#11687))
([65a3f11](AztecProtocol/aztec-packages@65a3f11))
* Mark contracts as pub
([#11241](AztecProtocol/aztec-packages#11241))
([b168601](AztecProtocol/aztec-packages@b168601))
* Reduce memory requests on prover node
([#11678](AztecProtocol/aztec-packages#11678))
([a720151](AztecProtocol/aztec-packages@a720151))
* Remove profiler cache fallback
([#11680](AztecProtocol/aztec-packages#11680))
([a305aef](AztecProtocol/aztec-packages@a305aef))
* Remove some templates in templates
([#11698](AztecProtocol/aztec-packages#11698))
([61614b1](AztecProtocol/aztec-packages@61614b1))
* Remove unused functions from public side effect trace
([#11600](AztecProtocol/aztec-packages#11600))
([54e9602](AztecProtocol/aztec-packages@54e9602))
* Replace relative paths to noir-protocol-circuits
([739151e](AztecProtocol/aztec-packages@739151e))
* Replace relative paths to noir-protocol-circuits
([bbd526c](AztecProtocol/aztec-packages@bbd526c))
* **sequencer:** Add InvalidArchive to canProposeAtNextEthBlock ignored
errors
([#11682](AztecProtocol/aztec-packages#11682))
([eea4bd3](AztecProtocol/aztec-packages@eea4bd3))
* **spartan:** Remove hardcoded keys and addresses - derive all from
mnemonic
([#11672](AztecProtocol/aztec-packages#11672))
([65f0e48](AztecProtocol/aztec-packages@65f0e48))
* Turn off auto-upgrade in node-pools
([#11679](AztecProtocol/aztec-packages#11679))
([09f98a9](AztecProtocol/aztec-packages@09f98a9))
* Turn on masking in ultra and mega zk + oink clean-up
([#11693](AztecProtocol/aztec-packages#11693))
([08e96fe](AztecProtocol/aztec-packages@08e96fe))


### Documentation

* Update mig notes release version
([#11685](AztecProtocol/aztec-packages#11685))
([46a30b5](AztecProtocol/aztec-packages@46a30b5))
</details>

<details><summary>barretenberg: 0.74.0</summary>

##
[0.74.0](AztecProtocol/aztec-packages@barretenberg-v0.73.0...barretenberg-v0.74.0)
(2025-02-04)


### Features

* UltraHonkZK contract
([#11553](AztecProtocol/aztec-packages#11553))
([a68369f](AztecProtocol/aztec-packages@a68369f))


### Bug Fixes

* Barretenber/stdlib/logic bugs
([#11651](AztecProtocol/aztec-packages#11651))
([dddab22](AztecProtocol/aztec-packages@dddab22))
* Barretenberg/stdlib/logic bugs (redo)
([#11691](AztecProtocol/aztec-packages#11691))
([6d0bad7](AztecProtocol/aztec-packages@6d0bad7))
* Revert "barretenberg/stdlib/logic bugs"
([#11689](AztecProtocol/aztec-packages#11689))
([b99570d](AztecProtocol/aztec-packages@b99570d))


### Miscellaneous

* Ensure new kv-store is used on the server
([#11662](AztecProtocol/aztec-packages#11662))
([aee1420](AztecProtocol/aztec-packages@aee1420))
* Remove some templates in templates
([#11698](AztecProtocol/aztec-packages#11698))
([61614b1](AztecProtocol/aztec-packages@61614b1))
* Turn on masking in ultra and mega zk + oink clean-up
([#11693](AztecProtocol/aztec-packages#11693))
([08e96fe](AztecProtocol/aztec-packages@08e96fe))
</details>

---
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
Reviewers

@ledwards2225 ledwards2225 ledwards2225 approved these changes

Assignees

@iakovenkos iakovenkos

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@iakovenkos @ledwards2225