From 1420859cf8df93f0823f948e0169162bfd07c025 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Venturo?= Date: Wed, 25 Sep 2024 10:08:35 -0300 Subject: [PATCH] feat!: make compute_nullifier_without_context unconstrained (#8742) With https://github.com/noir-lang/noir/issues/5717 closed, we can now make `compute_nullifier_without_context` `unconstrained`, as it should have been. This clears the multiple warnings caused by calling `get_nsk_app` without an `unsafe` block. I also moved the implementation of `TransparentNote` around, since we were calling the now unconstrained version. This nicely would've resulted in a warning about a missing `unsafe` block had I not changed it :grin: --------- Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com> --- docs/docs/migration_notes.md | 10 ++++++++++ .../aztec-nr/address-note/src/address_note.nr | 2 +- .../aztec/src/encrypted_logs/incoming_body.nr | 2 +- .../aztec-nr/aztec/src/note/note_interface.nr | 3 +-- .../aztec-nr/aztec/src/test/mocks/mock_note.nr | 2 +- noir-projects/aztec-nr/uint-note/src/uint_note.nr | 2 +- .../aztec-nr/value-note/src/value_note.nr | 2 +- .../src/subscription_note.nr | 2 +- .../docs_example_contract/src/types/card_note.nr | 2 +- .../contracts/ecdsa_public_key_note/src/lib.nr | 2 +- .../contracts/nft_contract/src/types/nft_note.nr | 2 +- .../src/public_key_note.nr | 2 +- .../spam_contract/src/types/token_note.nr | 2 +- .../contracts/test_contract/src/test_note.nr | 2 +- .../src/types/token_note.nr | 2 +- .../src/types/transparent_note.nr | 14 ++++++++------ .../token_contract/src/types/token_note.nr | 2 +- .../token_contract/src/types/transparent_note.nr | 15 +++++++++------ 18 files changed, 42 insertions(+), 28 deletions(-) diff --git a/docs/docs/migration_notes.md b/docs/docs/migration_notes.md index 1bd1a285c5b..25735e13590 100644 --- a/docs/docs/migration_notes.md +++ b/docs/docs/migration_notes.md @@ -8,6 +8,16 @@ Aztec is in full-speed development. Literally every version breaks compatibility ## TBD +### [Aztec.nr] Changes to `NullifiableNote` + +The `compute_nullifier_without_context` function is now `unconstrained`. It had always been meant to be called in unconstrained contexts (which is why it did not receive the `context` object), but now that Noir supports trait functions being `unconstrained` this can be implemented properly. Users must add the `unconstrained` keyword to their implementations of the trait: + +```diff +impl NullifiableNote for MyCustomNote { +- fn compute_nullifier_without_context(self) -> Field { ++ unconstrained fn compute_nullifier_without_context(self) -> Field { +``` + ### [Aztec.nr] Make `TestEnvironment` unconstrained All of `TestEnvironment`'s functions are now `unconstrained`, preventing accidentally calling them in a constrained circuit, among other kinds of user error. Becuase they work with mutable references, and these are not allowed to cross the constrained/unconstrained barrier, tests that use `TestEnvironment` must also become `unconstrained`. The recommended practice is to make _all_ Noir tests and test helper functions be `unconstrained: diff --git a/noir-projects/aztec-nr/address-note/src/address_note.nr b/noir-projects/aztec-nr/address-note/src/address_note.nr index 310c94b3f51..9da6a381c1a 100644 --- a/noir-projects/aztec-nr/address-note/src/address_note.nr +++ b/noir-projects/aztec-nr/address-note/src/address_note.nr @@ -30,7 +30,7 @@ impl NullifiableNote for AddressNote { ) } - fn compute_nullifier_without_context(self) -> Field { + unconstrained fn compute_nullifier_without_context(self) -> Field { let note_hash_for_nullify = compute_note_hash_for_nullify(self); let secret = get_nsk_app(self.npk_m_hash); poseidon2_hash_with_separator( diff --git a/noir-projects/aztec-nr/aztec/src/encrypted_logs/incoming_body.nr b/noir-projects/aztec-nr/aztec/src/encrypted_logs/incoming_body.nr index 5cb0e012e61..171ed82ebce 100644 --- a/noir-projects/aztec-nr/aztec/src/encrypted_logs/incoming_body.nr +++ b/noir-projects/aztec-nr/aztec/src/encrypted_logs/incoming_body.nr @@ -63,7 +63,7 @@ mod test { 1 } - fn compute_nullifier_without_context(_self: Self) -> Field { + unconstrained fn compute_nullifier_without_context(_self: Self) -> Field { 1 } } diff --git a/noir-projects/aztec-nr/aztec/src/note/note_interface.nr b/noir-projects/aztec-nr/aztec/src/note/note_interface.nr index c7ecc774ad4..0ec39088aad 100644 --- a/noir-projects/aztec-nr/aztec/src/note/note_interface.nr +++ b/noir-projects/aztec-nr/aztec/src/note/note_interface.nr @@ -19,7 +19,7 @@ pub trait NullifiableNote { // Unlike compute_nullifier, this function does not take a note hash since it'll only be invoked in unconstrained // contexts, where there is no gate count. - fn compute_nullifier_without_context(self) -> Field; + unconstrained fn compute_nullifier_without_context(self) -> Field; } // docs:start:note_interface @@ -48,4 +48,3 @@ pub trait NoteInterface { fn compute_note_hash(self) -> Field; } // docs:end:note_interface - diff --git a/noir-projects/aztec-nr/aztec/src/test/mocks/mock_note.nr b/noir-projects/aztec-nr/aztec/src/test/mocks/mock_note.nr index ff7d61d03b2..13f1723d481 100644 --- a/noir-projects/aztec-nr/aztec/src/test/mocks/mock_note.nr +++ b/noir-projects/aztec-nr/aztec/src/test/mocks/mock_note.nr @@ -24,7 +24,7 @@ impl NullifiableNote for MockNote { ) } - fn compute_nullifier_without_context(self) -> Field { + unconstrained fn compute_nullifier_without_context(self) -> Field { // We don't use any kind of secret here since this is only a mock note and having it here would make tests // more cumbersome let note_hash_for_nullify = compute_note_hash_for_nullify(self); diff --git a/noir-projects/aztec-nr/uint-note/src/uint_note.nr b/noir-projects/aztec-nr/uint-note/src/uint_note.nr index 4998e957371..8e4485a8f09 100644 --- a/noir-projects/aztec-nr/uint-note/src/uint_note.nr +++ b/noir-projects/aztec-nr/uint-note/src/uint_note.nr @@ -26,7 +26,7 @@ impl NullifiableNote for UintNote { ) } - fn compute_nullifier_without_context(self) -> Field { + unconstrained fn compute_nullifier_without_context(self) -> Field { let note_hash_for_nullify = compute_note_hash_for_nullify(self); let secret = get_nsk_app(self.npk_m_hash); poseidon2_hash_with_separator( diff --git a/noir-projects/aztec-nr/value-note/src/value_note.nr b/noir-projects/aztec-nr/value-note/src/value_note.nr index 62c5b7875d0..cc581512017 100644 --- a/noir-projects/aztec-nr/value-note/src/value_note.nr +++ b/noir-projects/aztec-nr/value-note/src/value_note.nr @@ -36,7 +36,7 @@ impl NullifiableNote for ValueNote { // docs:end:nullifier - fn compute_nullifier_without_context(self) -> Field { + unconstrained fn compute_nullifier_without_context(self) -> Field { let note_hash_for_nullify = compute_note_hash_for_nullify(self); let secret = get_nsk_app(self.npk_m_hash); poseidon2_hash_with_separator( diff --git a/noir-projects/noir-contracts/contracts/app_subscription_contract/src/subscription_note.nr b/noir-projects/noir-contracts/contracts/app_subscription_contract/src/subscription_note.nr index 67cf85873a9..29a55da9218 100644 --- a/noir-projects/noir-contracts/contracts/app_subscription_contract/src/subscription_note.nr +++ b/noir-projects/noir-contracts/contracts/app_subscription_contract/src/subscription_note.nr @@ -27,7 +27,7 @@ impl NullifiableNote for SubscriptionNote { ) } - fn compute_nullifier_without_context(self) -> Field { + unconstrained fn compute_nullifier_without_context(self) -> Field { let note_hash_for_nullify = compute_note_hash_for_nullify(self); let secret = get_nsk_app(self.npk_m_hash); poseidon2_hash_with_separator( diff --git a/noir-projects/noir-contracts/contracts/docs_example_contract/src/types/card_note.nr b/noir-projects/noir-contracts/contracts/docs_example_contract/src/types/card_note.nr index d67b2822f8c..4b4dd746b8d 100644 --- a/noir-projects/noir-contracts/contracts/docs_example_contract/src/types/card_note.nr +++ b/noir-projects/noir-contracts/contracts/docs_example_contract/src/types/card_note.nr @@ -38,7 +38,7 @@ impl NullifiableNote for CardNote { ) } - fn compute_nullifier_without_context(self) -> Field { + unconstrained fn compute_nullifier_without_context(self) -> Field { let note_hash_for_nullify = compute_note_hash_for_nullify(self); let secret = get_nsk_app(self.npk_m_hash); poseidon2_hash_with_separator( diff --git a/noir-projects/noir-contracts/contracts/ecdsa_public_key_note/src/lib.nr b/noir-projects/noir-contracts/contracts/ecdsa_public_key_note/src/lib.nr index d3fbc101101..1f88a0851a3 100644 --- a/noir-projects/noir-contracts/contracts/ecdsa_public_key_note/src/lib.nr +++ b/noir-projects/noir-contracts/contracts/ecdsa_public_key_note/src/lib.nr @@ -136,7 +136,7 @@ impl NullifiableNote for EcdsaPublicKeyNote { ) } - fn compute_nullifier_without_context(self) -> Field { + unconstrained fn compute_nullifier_without_context(self) -> Field { let note_hash_for_nullify = compute_note_hash_for_nullify(self); let secret = get_nsk_app(self.npk_m_hash); poseidon2_hash_with_separator( diff --git a/noir-projects/noir-contracts/contracts/nft_contract/src/types/nft_note.nr b/noir-projects/noir-contracts/contracts/nft_contract/src/types/nft_note.nr index e4960ee4f71..ea331467bcc 100644 --- a/noir-projects/noir-contracts/contracts/nft_contract/src/types/nft_note.nr +++ b/noir-projects/noir-contracts/contracts/nft_contract/src/types/nft_note.nr @@ -27,7 +27,7 @@ impl NullifiableNote for NFTNote { ) } - fn compute_nullifier_without_context(self) -> Field { + unconstrained fn compute_nullifier_without_context(self) -> Field { let note_hash_for_nullify = compute_note_hash_for_nullify(self); let secret = get_nsk_app(self.npk_m_hash); poseidon2_hash_with_separator( diff --git a/noir-projects/noir-contracts/contracts/schnorr_account_contract/src/public_key_note.nr b/noir-projects/noir-contracts/contracts/schnorr_account_contract/src/public_key_note.nr index 9082b48d49e..7098b66ab11 100644 --- a/noir-projects/noir-contracts/contracts/schnorr_account_contract/src/public_key_note.nr +++ b/noir-projects/noir-contracts/contracts/schnorr_account_contract/src/public_key_note.nr @@ -27,7 +27,7 @@ impl NullifiableNote for PublicKeyNote { ) } - fn compute_nullifier_without_context(self) -> Field { + unconstrained fn compute_nullifier_without_context(self) -> Field { let note_hash_for_nullify = compute_note_hash_for_nullify(self); let secret = get_nsk_app(self.npk_m_hash); poseidon2_hash_with_separator( diff --git a/noir-projects/noir-contracts/contracts/spam_contract/src/types/token_note.nr b/noir-projects/noir-contracts/contracts/spam_contract/src/types/token_note.nr index 33a85b27e7b..b14bc87f94c 100644 --- a/noir-projects/noir-contracts/contracts/spam_contract/src/types/token_note.nr +++ b/noir-projects/noir-contracts/contracts/spam_contract/src/types/token_note.nr @@ -35,7 +35,7 @@ impl NullifiableNote for TokenNote { } // docs:end:nullifier - fn compute_nullifier_without_context(self) -> Field { + unconstrained fn compute_nullifier_without_context(self) -> Field { let note_hash_for_nullify = compute_note_hash_for_nullify(self); let secret = get_nsk_app(self.npk_m_hash); poseidon2_hash_with_separator( diff --git a/noir-projects/noir-contracts/contracts/test_contract/src/test_note.nr b/noir-projects/noir-contracts/contracts/test_contract/src/test_note.nr index 72fec9b015f..8092554dad4 100644 --- a/noir-projects/noir-contracts/contracts/test_contract/src/test_note.nr +++ b/noir-projects/noir-contracts/contracts/test_contract/src/test_note.nr @@ -19,7 +19,7 @@ impl NullifiableNote for TestNote { 0 } - fn compute_nullifier_without_context(_self: Self) -> Field { + unconstrained fn compute_nullifier_without_context(_self: Self) -> Field { // This note is expected to be shared between users and for this reason can't be nullified using a secret. 0 } diff --git a/noir-projects/noir-contracts/contracts/token_blacklist_contract/src/types/token_note.nr b/noir-projects/noir-contracts/contracts/token_blacklist_contract/src/types/token_note.nr index ad19de9860f..1dd1ed8aba7 100644 --- a/noir-projects/noir-contracts/contracts/token_blacklist_contract/src/types/token_note.nr +++ b/noir-projects/noir-contracts/contracts/token_blacklist_contract/src/types/token_note.nr @@ -34,7 +34,7 @@ impl NullifiableNote for TokenNote { } // docs:end:nullifier - fn compute_nullifier_without_context(self) -> Field { + unconstrained fn compute_nullifier_without_context(self) -> Field { let note_hash_for_nullify = compute_note_hash_for_nullify(self); let secret = get_nsk_app(self.npk_m_hash); poseidon2_hash_with_separator( diff --git a/noir-projects/noir-contracts/contracts/token_blacklist_contract/src/types/transparent_note.nr b/noir-projects/noir-contracts/contracts/token_blacklist_contract/src/types/transparent_note.nr index d5007d83b2a..5b440c545b2 100644 --- a/noir-projects/noir-contracts/contracts/token_blacklist_contract/src/types/transparent_note.nr +++ b/noir-projects/noir-contracts/contracts/token_blacklist_contract/src/types/transparent_note.nr @@ -6,6 +6,8 @@ use dep::aztec::{ macros::notes::note }; +use dep::std::mem::zeroed; + // Transparent note represents a note that is created in the clear (public execution), but can only be spent by those // that know the preimage of the "secret_hash" (the secret). This is typically used when shielding a token balance. // Owner of the tokens provides a "secret_hash" as an argument to the public "shield" function and then the tokens @@ -17,11 +19,6 @@ pub struct TransparentNote { } impl NullifiableNote for TransparentNote { - - fn compute_nullifier(self, _context: &mut PrivateContext, _note_hash_for_nullify: Field) -> Field { - self.compute_nullifier_without_context() - } - // Computing a nullifier in a transparent note is not guarded by making secret a part of the nullifier preimage (as // is common in other cases) and instead is guarded by the functionality of "redeem_shield" function. There we do // the following: @@ -30,13 +27,18 @@ impl NullifiableNote for TransparentNote { // 3) the "get_notes" oracle constrains that the secret hash in the returned note matches the one computed in // circuit. // This achieves that the note can only be spent by the party that knows the secret. - fn compute_nullifier_without_context(self) -> Field { + fn compute_nullifier(self, _context: &mut PrivateContext, _note_hash_for_nullify: Field) -> Field { let note_hash_for_nullify = compute_note_hash_for_nullify(self); poseidon2_hash_with_separator( [note_hash_for_nullify], GENERATOR_INDEX__NOTE_NULLIFIER as Field ) } + + unconstrained fn compute_nullifier_without_context(self) -> Field { + // compute_nullifier ignores both of its parameters so we can reuse it here + self.compute_nullifier(zeroed(), zeroed()) + } } impl TransparentNote { diff --git a/noir-projects/noir-contracts/contracts/token_contract/src/types/token_note.nr b/noir-projects/noir-contracts/contracts/token_contract/src/types/token_note.nr index 365724e6868..f1133488ad3 100644 --- a/noir-projects/noir-contracts/contracts/token_contract/src/types/token_note.nr +++ b/noir-projects/noir-contracts/contracts/token_contract/src/types/token_note.nr @@ -36,7 +36,7 @@ impl NullifiableNote for TokenNote { } // docs:end:nullifier - fn compute_nullifier_without_context(self) -> Field { + unconstrained fn compute_nullifier_without_context(self) -> Field { let note_hash_for_nullify = compute_note_hash_for_nullify(self); let secret = get_nsk_app(self.npk_m_hash); poseidon2_hash_with_separator( diff --git a/noir-projects/noir-contracts/contracts/token_contract/src/types/transparent_note.nr b/noir-projects/noir-contracts/contracts/token_contract/src/types/transparent_note.nr index c0dbd3662f1..19ec7ac1812 100644 --- a/noir-projects/noir-contracts/contracts/token_contract/src/types/transparent_note.nr +++ b/noir-projects/noir-contracts/contracts/token_contract/src/types/transparent_note.nr @@ -5,6 +5,8 @@ use dep::aztec::{ macros::notes::note }; +use dep::std::mem::zeroed; + // Transparent note represents a note that is created in the clear (public execution), but can only be spent by those // that know the preimage of the "secret_hash" (the secret). This is typically used when shielding a token balance. // Owner of the tokens provides a "secret_hash" as an argument to the public "shield" function and then the tokens @@ -16,10 +18,6 @@ pub struct TransparentNote { } impl NullifiableNote for TransparentNote { - fn compute_nullifier(self, _context: &mut PrivateContext, _note_hash_for_nullify: Field) -> Field { - self.compute_nullifier_without_context() - } - // Computing a nullifier in a transparent note is not guarded by making secret a part of the nullifier preimage (as // is common in other cases) and instead is guarded by the functionality of "redeem_shield" function. There we do // the following: @@ -28,13 +26,18 @@ impl NullifiableNote for TransparentNote { // 3) the "get_notes" oracle constrains that the secret hash in the returned note matches the one computed in // circuit. // This achieves that the note can only be spent by the party that knows the secret. - fn compute_nullifier_without_context(self) -> Field { + fn compute_nullifier(self, _context: &mut PrivateContext, _note_hash_for_nullify: Field) -> Field { let note_hash_for_nullify = compute_note_hash_for_nullify(self); poseidon2_hash_with_separator( [note_hash_for_nullify], GENERATOR_INDEX__NOTE_NULLIFIER as Field ) } + + unconstrained fn compute_nullifier_without_context(self) -> Field { + // compute_nullifier ignores both of its parameters so we can reuse it here + self.compute_nullifier(zeroed(), zeroed()) + } } impl TransparentNote { @@ -50,4 +53,4 @@ impl Eq for TransparentNote { } } -// docs:end:token_types_all \ No newline at end of file +// docs:end:token_types_all