From 9ca4c8a58b4f97be34d7303148c744b4be23e767 Mon Sep 17 00:00:00 2001 From: benesjan Date: Thu, 15 Aug 2024 10:56:11 +0000 Subject: [PATCH] refactor: pedersen hash related cleanup in aztec.nr --- l1-contracts/src/core/libraries/ConstantsGen.sol | 1 + 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 +- .../state_vars/shared_mutable/shared_mutable.nr | 11 ++++++++--- .../shared_mutable_private_getter.nr | 14 ++------------ .../contracts/avm_test_contract/src/main.nr | 6 +++--- .../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 +---- .../crates/types/src/constants.nr | 1 + .../crates/types/src/storage/map.nr | 6 +++--- yarn-project/circuits.js/src/constants.gen.ts | 1 + yarn-project/circuits.js/src/hash/map_slot.test.ts | 2 +- yarn-project/circuits.js/src/hash/map_slot.ts | 6 ++++-- yarn-project/end-to-end/src/e2e_keys.test.ts | 6 +++--- 22 files changed, 40 insertions(+), 50 deletions(-) diff --git a/l1-contracts/src/core/libraries/ConstantsGen.sol b/l1-contracts/src/core/libraries/ConstantsGen.sol index 1983d5e2d22d..7b7e92e6e0a5 100644 --- a/l1-contracts/src/core/libraries/ConstantsGen.sol +++ b/l1-contracts/src/core/libraries/ConstantsGen.sol @@ -97,6 +97,7 @@ library Constants { uint256 internal constant ARGS_HASH_CHUNK_COUNT = 16; uint256 internal constant MAX_ARGS_LENGTH = 256; uint256 internal constant INITIALIZATION_SLOT_SEPARATOR = 1000000000; + uint256 internal constant MAP_STORAGE_SLOT_SEPARATOR = 1000000001; uint256 internal constant INITIAL_L2_BLOCK_NUM = 1; uint256 internal constant BLOB_SIZE_IN_BYTES = 126976; uint256 internal constant ETHEREUM_SLOT_DURATION = 12; diff --git a/noir-projects/aztec-nr/authwit/src/account.nr b/noir-projects/aztec-nr/authwit/src/account.nr index d64de349db1c..e1e3b5de50b9 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 47f7da7a0488..5050988fee2f 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 3056607a03a8..89ac1f843647 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 a4e030728286..f1d5604952d5 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.nr b/noir-projects/aztec-nr/aztec/src/state_vars/shared_mutable/shared_mutable.nr index 685e2e64f0be..21385e134a9e 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 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 2378a102ee4f..84d3816b66b6 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/avm_test_contract/src/main.nr b/noir-projects/noir-contracts/contracts/avm_test_contract/src/main.nr index bc048e52adc2..235d83bfddbc 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)] 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 7689d6753628..a2bc9c31bffe 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 1ce88b39889f..3345d56ab818 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 b7975db3c5a3..43488a3d1fb2 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 b7975db3c5a3..43488a3d1fb2 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 7cc44c365853..3e24b2188c1e 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 e1ad93d61a0f..01fc730ee018 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 5baff152aca0..4460c71bb31f 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 77a93ce26ded..101a16da6379 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/noir-projects/noir-protocol-circuits/crates/types/src/constants.nr b/noir-projects/noir-protocol-circuits/crates/types/src/constants.nr index 9c2aaa1c4801..f059e0594dc7 100644 --- a/noir-projects/noir-protocol-circuits/crates/types/src/constants.nr +++ b/noir-projects/noir-protocol-circuits/crates/types/src/constants.nr @@ -127,6 +127,7 @@ global MAX_ARGS_LENGTH: u32 = ARGS_HASH_CHUNK_COUNT * ARGS_HASH_CHUNK_LENGTH; // The initialization slot is computed by adding the constant below to the variable's storage slot. This constant has // to be large enough so that it's ensured that it doesn't collide with storage slots of other variables. global INITIALIZATION_SLOT_SEPARATOR: Field = 1000_000_000; +global MAP_STORAGE_SLOT_SEPARATOR: u32 = 1000_000_001; global INITIAL_L2_BLOCK_NUM: Field = 1; global BLOB_SIZE_IN_BYTES: Field = 31 * 4096; global ETHEREUM_SLOT_DURATION: u32 = 12; diff --git a/noir-projects/noir-protocol-circuits/crates/types/src/storage/map.nr b/noir-projects/noir-protocol-circuits/crates/types/src/storage/map.nr index 13a82bc58b58..8e9e738b33ff 100644 --- a/noir-projects/noir-protocol-circuits/crates/types/src/storage/map.nr +++ b/noir-projects/noir-protocol-circuits/crates/types/src/storage/map.nr @@ -1,7 +1,7 @@ -use crate::{hash::pedersen_hash, traits::ToField}; +use crate::{hash::pedersen_hash, constants::MAP_STORAGE_SLOT_SEPARATOR, traits::ToField}; pub fn derive_storage_slot_in_map(storage_slot: Field, key: K) -> Field where K: ToField { - pedersen_hash([storage_slot, key.to_field()], 0) + pedersen_hash([storage_slot, key.to_field()], MAP_STORAGE_SLOT_SEPARATOR) } mod test { @@ -15,7 +15,7 @@ mod test { let slot = derive_storage_slot_in_map(map_slot, key); // The following value was generated by `map_slot.test.ts` - let slot_from_typescript = 0x2499880e2b1b831785c17286f99a0d5122fee784ce7b1c04e380c4a991da819a; + let slot_from_typescript = 0x160e1bbd52a39bdb5ce2024c61d96dc2c1e9f0653e0b348e920828d29d334330; assert_eq(slot, slot_from_typescript); } diff --git a/yarn-project/circuits.js/src/constants.gen.ts b/yarn-project/circuits.js/src/constants.gen.ts index e28d7e01cae9..7d411ad276a1 100644 --- a/yarn-project/circuits.js/src/constants.gen.ts +++ b/yarn-project/circuits.js/src/constants.gen.ts @@ -83,6 +83,7 @@ export const ARGS_HASH_CHUNK_LENGTH = 16; export const ARGS_HASH_CHUNK_COUNT = 16; export const MAX_ARGS_LENGTH = 256; export const INITIALIZATION_SLOT_SEPARATOR = 1000000000; +export const MAP_STORAGE_SLOT_SEPARATOR = 1000000001; export const INITIAL_L2_BLOCK_NUM = 1; export const BLOB_SIZE_IN_BYTES = 126976; export const ETHEREUM_SLOT_DURATION = 12; diff --git a/yarn-project/circuits.js/src/hash/map_slot.test.ts b/yarn-project/circuits.js/src/hash/map_slot.test.ts index e43047d4b2d3..d8d66dbccc67 100644 --- a/yarn-project/circuits.js/src/hash/map_slot.test.ts +++ b/yarn-project/circuits.js/src/hash/map_slot.test.ts @@ -12,7 +12,7 @@ describe('Map slot', () => { const slot = deriveStorageSlotInMap(mapSlot, key); expect(slot.toString()).toMatchInlineSnapshot( - `"0x2499880e2b1b831785c17286f99a0d5122fee784ce7b1c04e380c4a991da819a"`, + `"0x160e1bbd52a39bdb5ce2024c61d96dc2c1e9f0653e0b348e920828d29d334330"`, ); // Run with AZTEC_GENERATE_TEST_DATA=1 to update noir test data diff --git a/yarn-project/circuits.js/src/hash/map_slot.ts b/yarn-project/circuits.js/src/hash/map_slot.ts index d14b85b0ea9f..d246abfda295 100644 --- a/yarn-project/circuits.js/src/hash/map_slot.ts +++ b/yarn-project/circuits.js/src/hash/map_slot.ts @@ -1,6 +1,8 @@ -import { pedersenHash } from '@aztec/foundation/crypto'; import { type Fr } from '@aztec/foundation/fields'; +import { MAP_STORAGE_SLOT_SEPARATOR } from '../constants.gen.js'; +import { pedersenHash } from '@aztec/foundation/crypto'; + /** * Computes the resulting storage slot for an entry in a map. * @param mapSlot - The slot of the map within state. @@ -14,5 +16,5 @@ export function deriveStorageSlotInMap( toField: () => Fr; }, ): Fr { - return pedersenHash([mapSlot, key.toField()]); + return pedersenHash([mapSlot, key.toField()], MAP_STORAGE_SLOT_SEPARATOR); } 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 48afcf7d924d..70f74471fcaf 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.