Skip to content

refactor: partial notes optimization with nullifier#14432

Merged
benesjan merged 6 commits intomasterfrom
05-21-refactor_partial_notes_optimization_with_nullifier
May 27, 2025
Merged

refactor: partial notes optimization with nullifier#14432
benesjan merged 6 commits intomasterfrom
05-21-refactor_partial_notes_optimization_with_nullifier

Conversation

@benesjan
Copy link
Copy Markdown
Contributor

@benesjan benesjan commented May 21, 2025

Fixes #14365

Instead of storing partial note validity commitment in public storage we store it in the nullifier tree. This yields the following savings:

  1. One less public function call since we can insert the nullifier directly from private (as opposed to us enqueueing a public call to write the info to pub storage),
  2. only 1 fields sent to DA instead of 2 since nullifier tree write does not need an index (unlike pub tree write).

What tests did I decide to write

  • I decided to not write any new single tx tests because that is already tested in the token contract in the transfer_to_private.nr::transfer_to_private_internal_orchestration test case.
  • I decided to not write any new multi tx tests because that is already tested in the token contract in the transfer_to_private.nr::transfer_to_private_external_orchestration test case.

Seems to be that it's all sufficiently covered already and hence no new test was needed.

Copy link
Copy Markdown
Contributor Author

benesjan commented May 21, 2025

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented May 21, 2025

Docs Preview

You can check your preview here.

@benesjan benesjan marked this pull request as ready for review May 21, 2025 10:16
@benesjan benesjan marked this pull request as draft May 21, 2025 10:42
@benesjan benesjan marked this pull request as ready for review May 21, 2025 11:57
@benesjan benesjan requested a review from nventuro May 21, 2025 11:57
@benesjan benesjan force-pushed the 05-21-refactor_partial_notes_optimization_with_nullifier branch from 73f26ab to 6bd058c Compare May 21, 2025 14:42
@benesjan benesjan force-pushed the 05-19-docs_comment_on_partial_note_resuse branch from bab43de to 41d6ed2 Compare May 21, 2025 14:42
Base automatically changed from 05-19-docs_comment_on_partial_note_resuse to master May 21, 2025 16:47
@benesjan benesjan force-pushed the 05-21-refactor_partial_notes_optimization_with_nullifier branch from 6bd058c to b69530c Compare May 21, 2025 18:18
Comment on lines +149 to +152
* - One for the fee note and one for the partial note validity commitment if we're using private fpc
*/
expect(txEffects!.data.nullifiers.length).toBe(
notesToCreate + 1 + (benchmarkingPaymentMethod === 'private_fpc' ? 1 : 0),
notesToCreate + 1 + (benchmarkingPaymentMethod === 'private_fpc' ? 2 : 0),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Interesting that we do not fully capture DA usage here (e.g. we don't see how this reduces pub storage writes). @Thunkar perhaps we could find this information in a local simulation?

@benesjan benesjan requested a review from nventuro May 22, 2025 19:54
Comment on lines +211 to +214
// We insert the partial note validity commitment into the nullifier tree to be later on able to verify that
// the partial note and completer are legitimate. See function docs of `insert_validity_commitment` for more
// details.
partial_note.insert_validity_commitment(context, sender_and_completer);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do you think it'd be too much if we made this part of the partial note creation? It'd put the entire process inside the partial note module - from the outside you just create and complete.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We want to force the dev to insert it and doing so manually in the contract here relies on him not forgetting it. So I think moving it the the note creation makes sense. Addressed it in 424f69d

I just renamed the function from partial to setup_partial_note as partial to me sounds less like a piece of complex involved thing (that the partial function has become) than setup_partial_note.

@benesjan benesjan force-pushed the 05-21-refactor_partial_notes_optimization_with_nullifier branch from eb033e5 to 424f69d Compare May 23, 2025 12:55
@benesjan benesjan requested a review from nventuro May 23, 2025 12:58
@benesjan benesjan force-pushed the 05-21-refactor_partial_notes_optimization_with_nullifier branch from 424f69d to a5a107b Compare May 27, 2025 12:43
@benesjan benesjan enabled auto-merge May 27, 2025 12:44
@benesjan benesjan added this pull request to the merge queue May 27, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 27, 2025
@benesjan benesjan added this pull request to the merge queue May 27, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 27, 2025
@benesjan benesjan added this pull request to the merge queue May 27, 2025
Merged via the queue into master with commit cc90823 May 27, 2025
6 checks passed
@benesjan benesjan deleted the 05-21-refactor_partial_notes_optimization_with_nullifier branch May 27, 2025 15:00
charlielye pushed a commit that referenced this pull request May 28, 2025
🤖 I have created a new Aztec Packages release
---


##
[0.87.3](v0.87.2...v0.87.3)
(2025-05-28)


### ⚠ BREAKING CHANGES

* app benches and unified PXE creation
([#14504](#14504))
* Removes normalize() calls on pairing points
([#14285](#14285))

### Features

* Adds StarknetZK WASM bindings to bb.js
([#14372](#14372))
([224b219](224b219))
* app benches and unified PXE creation
([#14504](#14504))
([aae5ab0](aae5ab0))
* **bb:** memory tracking for microbenchmarks
([#14445](#14445))
([fae2961](fae2961))
* enable provers to run in node chart
([#14405](#14405))
([94edd35](94edd35))
* measure oracles
([#14552](#14552))
([9cc6b54](9cc6b54))
* Removes normalize() calls on pairing points
([#14285](#14285))
([942b948](942b948))
* unbundled bb.js
([#14401](#14401))
([e0d9662](e0d9662))
* validating partial note sender
([#14379](#14379))
([de9880c](de9880c)),
closes
[#14363](#14363)


### Bug Fixes

* asan-fast caching
([#14468](#14468))
([fbd9ed6](fbd9ed6))
* attempt fix concurrency
([f31e706](f31e706))
* attempt to fix merge group base config
([6aa342b](6aa342b))
* bug in `TxProvingResult` schema
([#14498](#14498))
([#14530](#14530))
([0ac26fa](0ac26fa))
* bump defaults
([#14474](#14474))
([ade9a56](ade9a56))
* **docs:** Update getting started with testnet page
([#14536](#14536))
([66fc3eb](66fc3eb))
* don't create ./out when verifying
([#14556](#14556))
([ccb9981](ccb9981))
* eccvm_circuit_builder overrun
([#14484](#14484))
([5e01c07](5e01c07))
* issues with syncNotes --> syncPrivateState renaming
([#14442](#14442))
([78de410](78de410))
* Merge queue instances have pr name for uniquness in e.g. reorgs.
([#14562](#14562))
([1f012a9](1f012a9))
* nope
([470882a](470882a))
* **playground:** fix sfpc version when creating account
([#14481](#14481))
([5486a22](5486a22))
* processing events in contracts with no notes
([#14528](#14528))
([1bab9b4](1bab9b4)),
closes
[#14499](#14499)
* slack for flakes
([61829bc](61829bc))
* target branch
([e463723](e463723))
* test tracking
([#14513](#14513))
([2d6fc3a](2d6fc3a))
* try to fix release please
([f31c07e](f31c07e))
* try to fix release please
([d402c27](d402c27))
* try to fix release please
([cb959b8](cb959b8))
* try to fix release please
([227dbd2](227dbd2))
* update sponsored fpc address in playground
([#14472](#14472))
([4c14bc9](4c14bc9))
* wip merge master to next
([249ab4c](249ab4c))
* wip merge master to next
([b7850c9](b7850c9))


### Miscellaneous

* **bb:** avoid compile-time hash-to-curve
([#14177](#14177))
([d3863ff](d3863ff))
* **docs:** Update alpha-testnet docs to use version 0.87.2
([#14501](#14501))
([c86d3b0](c86d3b0))
* master-to-next
([#14454](#14454))
([06ac335](06ac335))
* **master:** release 0.87.3
([#14440](#14440))
([3ca26cd](3ca26cd))
* partial notes optimization with nullifier
([#14432](#14432))
([cc90823](cc90823))
* playground should release also at refname. attempt to make RP do
prerelease version.
([3c4fba5](3c4fba5))
* Protocol breaking changes must now go to next branch.
([#14423](#14423))
([44304b6](44304b6))
* release 0.87.3
([8340415](8340415))
* release please v4
([bf97805](bf97805))
* remove some expected failures
([#13843](#13843))
([2593e28](2593e28))
* select random rollup versions from distinct sets
([#14568](#14568))
([fe787cd](fe787cd))
* stdlib field pre-audit pt.0
([#14413](#14413))
([52458c2](52458c2))


### Documentation

* comment on partial note reuse
([#14383](#14383))
([42200c8](42200c8))
* fix image
([#14537](#14537))
([388e79b](388e79b)),
closes
[#14163](#14163)
* update node version instructions
([#14412](#14412))
([a2b1b9d](a2b1b9d))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
charlielye pushed a commit that referenced this pull request Jun 2, 2025
🤖 I have created a new Aztec Packages release
---


##
[0.87.6](v0.87.5...v0.87.6)
(2025-06-02)


### ⚠ BREAKING CHANGES

* app benches and unified PXE creation
([#14504](#14504))
* Removes normalize() calls on pairing points
([#14285](#14285))

### Features

* Adds StarknetZK WASM bindings to bb.js
([#14372](#14372))
([224b219](224b219))
* allow mempool to overflow before evicting
([#14641](#14641))
([f81425d](f81425d))
* app benches and unified PXE creation
([#14504](#14504))
([aae5ab0](aae5ab0))
* **bb:** memory tracking for microbenchmarks
([#14445](#14445))
([fae2961](fae2961))
* enable provers to run in node chart
([#14405](#14405))
([94edd35](94edd35))
* Ignore P2P messages previously seen
([#14665](#14665))
([2e50588](2e50588))
* measure oracles
([#14552](#14552))
([9cc6b54](9cc6b54))
* Removes normalize() calls on pairing points
([#14285](#14285))
([942b948](942b948))
* Request missing txs from block proposal sender
([#14341](#14341))
([3f97e8b](3f97e8b))
* unbundled bb.js
([#14401](#14401))
([e0d9662](e0d9662))
* validate notes in a single node roundtrip
([#14650](#14650))
([d467955](d467955))
* validating partial note sender
([#14379](#14379))
([de9880c](de9880c)),
closes
[#14363](#14363)


### Bug Fixes

* allow returning of tuples from contract funcs
([#14553](#14553))
([1c0955c](1c0955c))
* asan-fast caching
([#14468](#14468))
([fbd9ed6](fbd9ed6))
* attempt fix concurrency
([f31e706](f31e706))
* attempt to fix merge group base config
([6aa342b](6aa342b))
* bug in `TxProvingResult` schema
([#14498](#14498))
([#14530](#14530))
([0ac26fa](0ac26fa))
* bump defaults
([#14474](#14474))
([ade9a56](ade9a56))
* do not run concurrent idb transactions
([#14609](#14609))
([5a6f36a](5a6f36a))
* **docs:** Update create pxe interface
([#14587](#14587))
([b3b9c05](b3b9c05))
* **docs:** Update getting started with testnet page
([#14536](#14536))
([66fc3eb](66fc3eb))
* don't create ./out when verifying
([#14556](#14556))
([ccb9981](ccb9981))
* Don't manually close stream on p2p/reqresp/goodbye
([#14531](#14531))
([38cec7d](38cec7d))
* eccvm_circuit_builder overrun
([#14484](#14484))
([5e01c07](5e01c07))
* enable bundling txs with block proposals
([#14649](#14649))
([39b8c4e](39b8c4e))
* error on goodbye
([#14679](#14679))
([5112f5f](5112f5f))
* issues with syncNotes --> syncPrivateState renaming
([#14442](#14442))
([78de410](78de410))
* Merge queue instances have pr name for uniquness in e.g. reorgs.
([#14562](#14562))
([1f012a9](1f012a9))
* nope
([470882a](470882a))
* parse LOG_LEVELS in bb
([#14674](#14674))
([88203d6](88203d6))
* **playground:** fix sfpc version when creating account
([#14481](#14481))
([5486a22](5486a22))
* prevent world-state from spamming the logs
([#14594](#14594))
([8c725de](8c725de))
* processing events in contracts with no notes
([#14528](#14528))
([1bab9b4](1bab9b4)),
closes
[#14499](#14499)
* retrieve L1 to L2 messages in batches
([#14586](#14586))
([2be7f1b](2be7f1b))
* slack for flakes
([61829bc](61829bc))
* target branch
([e463723](e463723))
* test tracking
([#14513](#14513))
([2d6fc3a](2d6fc3a))
* try to fix release please
([f31c07e](f31c07e))
* try to fix release please
([d402c27](d402c27))
* try to fix release please
([cb959b8](cb959b8))
* try to fix release please
([227dbd2](227dbd2))
* update sponsored fpc address in playground
([#14472](#14472))
([4c14bc9](4c14bc9))
* wip merge master to next
([249ab4c](249ab4c))
* wip merge master to next
([b7850c9](b7850c9))


### Miscellaneous

* add testnet compat test
([#14601](#14601))
([1fbb350](1fbb350))
* Aztec simulator into PXE
([#14598](#14598))
([a999c8c](a999c8c))
* **bb:** avoid compile-time hash-to-curve
([#14177](#14177))
([d3863ff](d3863ff))
* capture RPC calls + correct timings
([#14633](#14633))
([5ee54ff](5ee54ff))
* Disable flood publish by default
([#14635](#14635))
([af8d879](af8d879))
* **docs:** Add scheduled typesense index job
([#14615](#14615))
([5f42e1b](5f42e1b))
* **docs:** Update alpha-testnet docs to use version 0.87.2
([#14501](#14501))
([c86d3b0](c86d3b0))
* drop txs per block to 8
([#14627](#14627))
([369f210](369f210))
* master-to-next
([#14454](#14454))
([06ac335](06ac335))
* **master:** release 0.87.3
([#14440](#14440))
([3ca26cd](3ca26cd))
* **master:** release 0.87.3
([#14582](#14582))
([8cea842](8cea842))
* **master:** release 0.87.4
([#14583](#14583))
([8077f63](8077f63))
* New prover chart for alpha-testnet
([#14514](#14514))
([031d7cb](031d7cb))
* partial notes optimization with nullifier
([#14432](#14432))
([cc90823](cc90823))
* playground should release also at refname. attempt to make RP do
prerelease version.
([3c4fba5](3c4fba5))
* Protocol breaking changes must now go to next branch.
([#14423](#14423))
([44304b6](44304b6))
* release 0.87.3
([8340415](8340415))
* release please v4
([bf97805](bf97805))
* remove some expected failures
([#13843](#13843))
([2593e28](2593e28))
* Revert "fix: processing events in contracts with no notes
([#14528](#14528))"
([#14596](#14596))
([3278e61](3278e61))
* select random rollup versions from distinct sets
([#14568](#14568))
([fe787cd](fe787cd))
* stdlib field pre-audit pt.0
([#14413](#14413))
([52458c2](52458c2))
* trick release-please
([cbcad3d](cbcad3d))
* trick release-please
([a0c3a21](a0c3a21))


### Documentation

* comment on partial note reuse
([#14383](#14383))
([42200c8](42200c8))
* create v0.87.4 as latest
([#14607](#14607))
([78c2469](78c2469))
* fix image
([#14537](#14537))
([388e79b](388e79b)),
closes
[#14163](#14163)
* update cli ref in node doc
([#14574](#14574))
([d54a8af](d54a8af))
* update node version instructions
([#14412](#14412))
([a2b1b9d](a2b1b9d))

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

Store partial note commitments in nullifiers instead of public storage

2 participants