Skip to content

feat: validating partial note sender#14379

Merged
benesjan merged 14 commits into
masterfrom
05-19-feat_validating_partial_note_sender
May 21, 2025
Merged

feat: validating partial note sender#14379
benesjan merged 14 commits into
masterfrom
05-19-feat_validating_partial_note_sender

Conversation

@benesjan

@benesjan benesjan commented May 19, 2025

Copy link
Copy Markdown
Contributor

Ensures a partial note can be completed only by the expected sender by including the sender in the partial note validity commitment.

Fixes #14363

benesjan commented May 19, 2025

Copy link
Copy Markdown
Contributor Author

Comment thread noir-projects/noir-contracts/contracts/app/nft_contract/src/types/nft_note.nr Outdated
Comment thread noir-projects/noir-contracts/contracts/app/nft_contract/src/types/nft_note.nr Outdated
Comment thread noir-projects/noir-contracts/contracts/app/nft_contract/src/main.nr Outdated
@benesjan benesjan force-pushed the 05-19-feat_validating_partial_note_sender branch from a82d82c to 0a9edad Compare May 20, 2025 07:30
@benesjan benesjan marked this pull request as draft May 20, 2025 07:30
@benesjan benesjan marked this pull request as ready for review May 20, 2025 07:37
Comment thread noir-projects/noir-contracts/contracts/app/nft_contract/src/main.nr
@benesjan benesjan requested a review from nventuro May 20, 2025 08:33
Comment thread noir-projects/noir-contracts/contracts/app/nft_contract/src/main.nr Outdated
Comment thread noir-projects/noir-contracts/contracts/app/nft_contract/src/types/nft_note.nr Outdated
@benesjan benesjan requested a review from nventuro May 20, 2025 10:22
@benesjan benesjan force-pushed the 05-19-feat_validating_partial_note_sender branch from a83c79c to 7d2b8ee Compare May 20, 2025 14:17
@fcarreiro fcarreiro removed their request for review May 20, 2025 15:11
/// Computes a partial note validity commitment. This commitment is stored in public storage when creating the
/// partial note in private and then is used to check that completer and note are valid upon partial note
/// completion in public (completer is the entity that can complete the partial note).
pub fn compute_validity_commitment(self, completer: AztecAddress) -> Field {

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.

Introduced a completer term since I needed to differentiate from from for it all to not be super twisted.

/// in the enqueued call).
#[private]
fn mint_to_private(
// TODO(benesjan): This allows minter to set arbitrary `from`. That seems undesirable. Will nuke it in a followup PR.

@benesjan benesjan May 20, 2025

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.

Reviewer agrees?

I've also noticed that we lack tests for mint_to_private. Would add them in another PR as well.

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.

Yes, this is a problem. from here means the account that will share the tagging secret with to. In the future this might get removed entirely.

);

NFT::at(context.this_address())._store_nft_set_partial_note(partial_note).enqueue(context);
// We can't simply return the partial note because we won't be able to later on verify that it was created

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.

This comment was in fungible token and I copied it here.

};

use dep::uint_note::uint_note::{PartialUintNote, UintNote};
use aztec::protocol_types::traits::ToField;

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.

Quite annoying that we have this contract to maintain. Had to do the updates here since the same note was used.

@nventuro nventuro left a comment

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.

Once the validitiy commitment insertion happens all in one place in private much of this can be made nicer.

/// in the enqueued call).
#[private]
fn mint_to_private(
// TODO(benesjan): This allows minter to set arbitrary `from`. That seems undesirable. Will nuke it in a followup PR.

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.

Yes, this is a problem. from here means the account that will share the tagging secret with to. In the future this might get removed entirely.

@nventuro nventuro added this pull request to the merge queue May 21, 2025
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks May 21, 2025
@benesjan

Copy link
Copy Markdown
Contributor Author

Re-adding it to queue to see if the failure was just a flake

@benesjan benesjan added this pull request to the merge queue May 21, 2025
Merged via the queue into master with commit de9880c May 21, 2025
6 checks passed
@benesjan benesjan deleted the 05-19-feat_validating_partial_note_sender branch May 21, 2025 14:37
Thunkar pushed a commit that referenced this pull request May 23, 2025
Ensures a partial note can be completed only by the expected sender by
including the sender in the partial note validity commitment.

Fixes #14363
github-merge-queue Bot pushed a commit that referenced this pull request May 27, 2025
🤖 I have created a release *beep* *boop*
---


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


### ⚠ BREAKING CHANGES

* Removes normalize() calls on pairing points
([#14285](#14285))

### Features

* Adds StarknetZK WASM bindings to bb.js
([#14372](#14372))
([224b219](224b219))
* 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

* bump defaults
([#14474](#14474))
([ade9a56](ade9a56))
* eccvm_circuit_builder overrun
([#14484](#14484))
([5e01c07](5e01c07))
* issues with syncNotes --> syncPrivateState renaming
([#14442](#14442))
([78de410](78de410))
* **playground:** fix sfpc version when creating account
([#14481](#14481))
([5486a22](5486a22))
* update sponsored fpc address in playground
([#14472](#14472))
([4c14bc9](4c14bc9))
* wip merge master to next
([249ab4c](249ab4c))
* wip merge master to next
([b7850c9](b7850c9))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

---------

Co-authored-by: AztecBot <tech@aztecprotocol.com>
Co-authored-by: Rahul Kothari <rahul.kothari.201@gmail.com>
Co-authored-by: saleel <saleel@saleel.xyz>
Co-authored-by: James Zaki <james.zaki@proton.me>
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 --&gt; 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 --&gt; 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.

Restrict the sender that can complete a partial note

2 participants