From e3af37c0e41bd1c111582984becafd4ec5036416 Mon Sep 17 00:00:00 2001 From: benesjan Date: Thu, 15 Aug 2024 11:00:02 +0000 Subject: [PATCH 1/3] refactor: pedersen hash related cleanup in aztec.nr --- noir-projects/aztec-nr/authwit/src/account.nr | 3 +-- .../src/oracle/get_nullifier_membership_witness.nr | 2 +- .../aztec/src/oracle/get_public_data_witness.nr | 3 +-- noir-projects/aztec-nr/aztec/src/state_vars/map.nr | 2 +- .../shared_mutable_private_getter.nr | 14 ++------------ .../contracts/test_contract/src/test_note.nr | 5 +---- .../src/types/balances_map.nr | 2 +- .../token_contract/src/types/balances_map.nr | 2 +- .../src/types/balances_map.nr | 2 +- .../crates/types/src/abis/gas.nr | 5 ++--- .../crates/types/src/abis/gas_fees.nr | 2 +- .../crates/types/src/abis/gas_settings.nr | 3 +-- .../crates/types/src/address/eth_address.nr | 5 +---- yarn-project/end-to-end/src/e2e_keys.test.ts | 6 +++--- 14 files changed, 18 insertions(+), 38 deletions(-) diff --git a/noir-projects/aztec-nr/authwit/src/account.nr b/noir-projects/aztec-nr/authwit/src/account.nr index d64de349db1..e1e3b5de50b 100644 --- a/noir-projects/aztec-nr/authwit/src/account.nr +++ b/noir-projects/aztec-nr/authwit/src/account.nr @@ -1,5 +1,4 @@ -use dep::aztec::context::{PrivateContext, PublicContext}; -use dep::aztec::protocol_types::{address::AztecAddress, abis::function_selector::FunctionSelector, hash::pedersen_hash}; +use dep::aztec::context::PrivateContext; use crate::entrypoint::{app::AppPayload, fee::FeePayload}; use crate::auth::{IS_VALID_SELECTOR, compute_authwit_message_hash}; diff --git a/noir-projects/aztec-nr/aztec/src/oracle/get_nullifier_membership_witness.nr b/noir-projects/aztec-nr/aztec/src/oracle/get_nullifier_membership_witness.nr index 47f7da7a048..5050988fee2 100644 --- a/noir-projects/aztec-nr/aztec/src/oracle/get_nullifier_membership_witness.nr +++ b/noir-projects/aztec-nr/aztec/src/oracle/get_nullifier_membership_witness.nr @@ -1,6 +1,6 @@ use dep::protocol_types::{ abis::nullifier_leaf_preimage::{NullifierLeafPreimage, NULLIFIER_LEAF_PREIMAGE_LENGTH}, - constants::NULLIFIER_TREE_HEIGHT, hash::pedersen_hash, utils::arr_copy_slice + constants::NULLIFIER_TREE_HEIGHT, utils::arr_copy_slice }; // INDEX_LENGTH + NULLIFIER_LEAF_PREIMAGE_LENGTH + NULLIFIER_TREE_HEIGHT diff --git a/noir-projects/aztec-nr/aztec/src/oracle/get_public_data_witness.nr b/noir-projects/aztec-nr/aztec/src/oracle/get_public_data_witness.nr index 3056607a03a..89ac1f84364 100644 --- a/noir-projects/aztec-nr/aztec/src/oracle/get_public_data_witness.nr +++ b/noir-projects/aztec-nr/aztec/src/oracle/get_public_data_witness.nr @@ -1,6 +1,5 @@ use dep::protocol_types::{ - constants::PUBLIC_DATA_TREE_HEIGHT, hash::pedersen_hash, - public_data_tree_leaf_preimage::PublicDataTreeLeafPreimage, traits::{Hash, Serialize}, + constants::PUBLIC_DATA_TREE_HEIGHT, public_data_tree_leaf_preimage::PublicDataTreeLeafPreimage, utils::arr_copy_slice }; diff --git a/noir-projects/aztec-nr/aztec/src/state_vars/map.nr b/noir-projects/aztec-nr/aztec/src/state_vars/map.nr index a4e03072828..f1d5604952d 100644 --- a/noir-projects/aztec-nr/aztec/src/state_vars/map.nr +++ b/noir-projects/aztec-nr/aztec/src/state_vars/map.nr @@ -1,4 +1,4 @@ -use dep::protocol_types::{hash::pedersen_hash, storage::map::derive_storage_slot_in_map, traits::ToField}; +use dep::protocol_types::{storage::map::derive_storage_slot_in_map, traits::ToField}; use crate::state_vars::storage::Storage; // docs:start:map diff --git a/noir-projects/aztec-nr/aztec/src/state_vars/shared_mutable/shared_mutable_private_getter.nr b/noir-projects/aztec-nr/aztec/src/state_vars/shared_mutable/shared_mutable_private_getter.nr index 2378a102ee4..84d3816b66b 100644 --- a/noir-projects/aztec-nr/aztec/src/state_vars/shared_mutable/shared_mutable_private_getter.nr +++ b/noir-projects/aztec-nr/aztec/src/state_vars/shared_mutable/shared_mutable_private_getter.nr @@ -1,16 +1,6 @@ -use dep::protocol_types::{ - hash::{pedersen_hash, poseidon2_hash}, traits::{FromField, ToField}, address::AztecAddress, - header::Header -}; +use dep::protocol_types::{traits::{FromField, ToField}, address::AztecAddress, header::Header}; -use crate::context::PrivateContext; -use crate::state_vars::{ - storage::Storage, - shared_mutable::{ - shared_mutable::SharedMutable, scheduled_delay_change::ScheduledDelayChange, - scheduled_value_change::ScheduledValueChange -} -}; +use crate::{context::PrivateContext, state_vars::shared_mutable::shared_mutable::SharedMutable}; struct SharedMutablePrivateGetter { context: &mut PrivateContext, 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 7689d675362..a2bc9c31bff 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 @@ -1,7 +1,4 @@ -use dep::aztec::{ - note::{note_header::NoteHeader, note_interface::NoteInterface}, hash::pedersen_hash, - context::PrivateContext -}; +use dep::aztec::{note::{note_header::NoteHeader, note_interface::NoteInterface}, context::PrivateContext}; global TEST_NOTE_LEN: Field = 1; // TEST_NOTE_LENGTH * 32 + 32(storage_slot as bytes) + 32(note_type_id as bytes) diff --git a/noir-projects/noir-contracts/contracts/token_blacklist_contract/src/types/balances_map.nr b/noir-projects/noir-contracts/contracts/token_blacklist_contract/src/types/balances_map.nr index 1ce88b39889..3345d56ab81 100644 --- a/noir-projects/noir-contracts/contracts/token_blacklist_contract/src/types/balances_map.nr +++ b/noir-projects/noir-contracts/contracts/token_blacklist_contract/src/types/balances_map.nr @@ -1,6 +1,6 @@ use dep::aztec::prelude::{AztecAddress, NoteGetterOptions, NoteViewerOptions, NoteHeader, NoteInterface, PrivateSet, Map}; use dep::aztec::{ - context::{PrivateContext, UnconstrainedContext}, hash::pedersen_hash, + context::{PrivateContext, UnconstrainedContext}, protocol_types::constants::MAX_NOTE_HASH_READ_REQUESTS_PER_CALL, note::{note_getter::view_notes, note_getter_options::SortOrder, note_emission::OuterNoteEmission} }; diff --git a/noir-projects/noir-contracts/contracts/token_contract/src/types/balances_map.nr b/noir-projects/noir-contracts/contracts/token_contract/src/types/balances_map.nr index b7975db3c5a..43488a3d1fb 100644 --- a/noir-projects/noir-contracts/contracts/token_contract/src/types/balances_map.nr +++ b/noir-projects/noir-contracts/contracts/token_contract/src/types/balances_map.nr @@ -1,6 +1,6 @@ use dep::aztec::prelude::{AztecAddress, NoteGetterOptions, NoteViewerOptions, NoteHeader, NoteInterface, PrivateSet, Map}; use dep::aztec::{ - context::{PrivateContext, UnconstrainedContext}, hash::pedersen_hash, + context::{PrivateContext, UnconstrainedContext}, protocol_types::constants::MAX_NOTE_HASH_READ_REQUESTS_PER_CALL, note::{ note_getter::view_notes, note_getter_options::SortOrder, diff --git a/noir-projects/noir-contracts/contracts/token_with_refunds_contract/src/types/balances_map.nr b/noir-projects/noir-contracts/contracts/token_with_refunds_contract/src/types/balances_map.nr index b7975db3c5a..43488a3d1fb 100644 --- a/noir-projects/noir-contracts/contracts/token_with_refunds_contract/src/types/balances_map.nr +++ b/noir-projects/noir-contracts/contracts/token_with_refunds_contract/src/types/balances_map.nr @@ -1,6 +1,6 @@ use dep::aztec::prelude::{AztecAddress, NoteGetterOptions, NoteViewerOptions, NoteHeader, NoteInterface, PrivateSet, Map}; use dep::aztec::{ - context::{PrivateContext, UnconstrainedContext}, hash::pedersen_hash, + context::{PrivateContext, UnconstrainedContext}, protocol_types::constants::MAX_NOTE_HASH_READ_REQUESTS_PER_CALL, note::{ note_getter::view_notes, note_getter_options::SortOrder, diff --git a/noir-projects/noir-protocol-circuits/crates/types/src/abis/gas.nr b/noir-projects/noir-protocol-circuits/crates/types/src/abis/gas.nr index 7cc44c36585..3e24b2188c1 100644 --- a/noir-projects/noir-protocol-circuits/crates/types/src/abis/gas.nr +++ b/noir-projects/noir-protocol-circuits/crates/types/src/abis/gas.nr @@ -1,8 +1,7 @@ use crate::{ abis::function_selector::FunctionSelector, address::{EthAddress, AztecAddress}, - constants::{GAS_LENGTH, FIXED_DA_GAS, FIXED_L2_GAS}, hash::pedersen_hash, - traits::{Deserialize, Hash, Serialize, Empty}, abis::side_effect::Ordered, utils::reader::Reader, - abis::gas_fees::GasFees + constants::{GAS_LENGTH, FIXED_DA_GAS, FIXED_L2_GAS}, traits::{Deserialize, Hash, Serialize, Empty}, + abis::side_effect::Ordered, utils::reader::Reader, abis::gas_fees::GasFees }; use std::ops::{Add, Sub}; diff --git a/noir-projects/noir-protocol-circuits/crates/types/src/abis/gas_fees.nr b/noir-projects/noir-protocol-circuits/crates/types/src/abis/gas_fees.nr index e1ad93d61a0..01fc730ee01 100644 --- a/noir-projects/noir-protocol-circuits/crates/types/src/abis/gas_fees.nr +++ b/noir-projects/noir-protocol-circuits/crates/types/src/abis/gas_fees.nr @@ -1,6 +1,6 @@ use crate::{ abis::function_selector::FunctionSelector, address::{EthAddress, AztecAddress}, - constants::GAS_FEES_LENGTH, hash::pedersen_hash, traits::{Deserialize, Hash, Serialize, Empty}, + constants::GAS_FEES_LENGTH, traits::{Deserialize, Hash, Serialize, Empty}, abis::side_effect::Ordered, utils::reader::Reader }; diff --git a/noir-projects/noir-protocol-circuits/crates/types/src/abis/gas_settings.nr b/noir-projects/noir-protocol-circuits/crates/types/src/abis/gas_settings.nr index 5baff152aca..4460c71bb31 100644 --- a/noir-projects/noir-protocol-circuits/crates/types/src/abis/gas_settings.nr +++ b/noir-projects/noir-protocol-circuits/crates/types/src/abis/gas_settings.nr @@ -5,8 +5,7 @@ use crate::{ GAS_SETTINGS_LENGTH, DEFAULT_GAS_LIMIT, DEFAULT_TEARDOWN_GAS_LIMIT, DEFAULT_MAX_FEE_PER_GAS, DEFAULT_INCLUSION_FEE }, - hash::pedersen_hash, traits::{Deserialize, Hash, Serialize, Empty}, abis::side_effect::Ordered, - utils::reader::Reader + traits::{Deserialize, Hash, Serialize, Empty}, abis::side_effect::Ordered, utils::reader::Reader }; struct GasSettings { diff --git a/noir-projects/noir-protocol-circuits/crates/types/src/address/eth_address.nr b/noir-projects/noir-protocol-circuits/crates/types/src/address/eth_address.nr index 77a93ce26de..101a16da637 100644 --- a/noir-projects/noir-protocol-circuits/crates/types/src/address/eth_address.nr +++ b/noir-projects/noir-protocol-circuits/crates/types/src/address/eth_address.nr @@ -1,7 +1,4 @@ -use crate::{ - constants::ETH_ADDRESS_LENGTH, hash::pedersen_hash, - traits::{Empty, ToField, Serialize, Deserialize}, utils -}; +use crate::{constants::ETH_ADDRESS_LENGTH, traits::{Empty, ToField, Serialize, Deserialize}, utils}; struct EthAddress{ inner : Field diff --git a/yarn-project/end-to-end/src/e2e_keys.test.ts b/yarn-project/end-to-end/src/e2e_keys.test.ts index 48afcf7d924..70f74471fca 100644 --- a/yarn-project/end-to-end/src/e2e_keys.test.ts +++ b/yarn-project/end-to-end/src/e2e_keys.test.ts @@ -50,9 +50,9 @@ describe('Key Registry', () => { afterAll(() => teardown()); describe('using nsk_app to detect nullification', () => { - // This test checks that it possible to detect that a note has been nullified just by using nsk_app. Note that - // this only works for non-transient notes as transient ones never emit a note hash which makes it impossible - // to brute force their nullifier. + // This test checks that it is possible to detect that a note has been nullified just by using nsk_app. Note + // that this only works for non-transient notes as transient ones never emit a note hash which makes it + // impossible to brute force their nullifier. // This might seem to make the scheme useless in practice. This could not be the case because if you have // a note of funds, when you create the transient you are nullifying that note. So even if I cannot see when you // nullified the transient ones, I can see that you nullified the first. From e6a55a838fcf52b375858eeef5c1007c220cfb62 Mon Sep 17 00:00:00 2001 From: benesjan Date: Thu, 15 Aug 2024 11:09:37 +0000 Subject: [PATCH 2/3] proper shared mutable generators --- .../src/state_vars/shared_mutable/shared_mutable.nr | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/noir-projects/aztec-nr/aztec/src/state_vars/shared_mutable/shared_mutable.nr b/noir-projects/aztec-nr/aztec/src/state_vars/shared_mutable/shared_mutable.nr index 685e2e64f0b..21385e134a9 100644 --- a/noir-projects/aztec-nr/aztec/src/state_vars/shared_mutable/shared_mutable.nr +++ b/noir-projects/aztec-nr/aztec/src/state_vars/shared_mutable/shared_mutable.nr @@ -18,6 +18,11 @@ struct SharedMutable { storage_slot: Field, } +// Separators separating storage slot of different values within the same state variable +global VALUE_CHANGE_SEPARATOR: u32 = 0; +global DELAY_CHANGE_SEPARATOR: u32 = 1; +global HASH_SEPARATOR: u32 = 2; + // This will make the Aztec macros require that T implements the Serialize trait, and allocate N storage slots to // this state variable. This is incorrect, since what we actually store is: // - a ScheduledValueChange, which requires 1 + 2 * M storage slots, where M is the serialization length of T @@ -76,15 +81,15 @@ impl SharedMutable Field { - pedersen_hash([self.storage_slot, 0], 0) + pedersen_hash([self.storage_slot], VALUE_CHANGE_SEPARATOR) } fn get_delay_change_storage_slot(self) -> Field { - pedersen_hash([self.storage_slot, 1], 0) + pedersen_hash([self.storage_slot], DELAY_CHANGE_SEPARATOR) } fn get_hash_storage_slot(self) -> Field { - pedersen_hash([self.storage_slot, 2], 0) + pedersen_hash([self.storage_slot], HASH_SEPARATOR) } // It may seem odd that we take a header and address instead of reading from e.g. a PrivateContext, but this lets us From 30ed03966dbfc70923fb38bd58b2790d47377869 Mon Sep 17 00:00:00 2001 From: benesjan Date: Thu, 15 Aug 2024 11:25:17 +0000 Subject: [PATCH 3/3] avm test using derive_storage_slot_in_map --- .../noir-contracts/contracts/avm_test_contract/src/main.nr | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/noir-projects/noir-contracts/contracts/avm_test_contract/src/main.nr b/noir-projects/noir-contracts/contracts/avm_test_contract/src/main.nr index bc048e52adc..235d83bfddb 100644 --- a/noir-projects/noir-contracts/contracts/avm_test_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/avm_test_contract/src/main.nr @@ -30,7 +30,7 @@ contract AvmTest { use dep::aztec::state_vars::PublicMutable; use dep::aztec::protocol_types::{address::{AztecAddress, EthAddress}, constants::L1_TO_L2_MESSAGE_LENGTH, point::Point, scalar::Scalar}; use dep::aztec::oracle::get_contract_instance::{get_contract_instance_avm, get_contract_instance_internal_avm}; - use dep::aztec::protocol_types::abis::function_selector::FunctionSelector; + use dep::aztec::protocol_types::{abis::function_selector::FunctionSelector, storage::map::derive_storage_slot_in_map}; use dep::aztec::context::gas::GasOpts; use dep::compressed_string::CompressedString; @@ -76,7 +76,7 @@ contract AvmTest { fn set_storage_map(to: AztecAddress, amount: u32) -> Field { storage.map.at(to).write(amount); // returns storage slot for key - std::hash::pedersen_hash([storage.map.storage_slot, to.to_field()]) + derive_storage_slot_in_map(storage.map.storage_slot, to) } #[aztec(public)] @@ -84,7 +84,7 @@ contract AvmTest { let new_balance = storage.map.at(to).read().add(amount); storage.map.at(to).write(new_balance); // returns storage slot for key - std::hash::pedersen_hash([storage.map.storage_slot, to.to_field()]) + derive_storage_slot_in_map(storage.map.storage_slot, to) } #[aztec(public)]