Skip to content

Commit

Permalink
refactor: pedersen hash related cleanup in aztec.nr
Browse files Browse the repository at this point in the history
  • Loading branch information
benesjan committed Aug 15, 2024
1 parent 3d61bdf commit 9ca4c8a
Show file tree
Hide file tree
Showing 22 changed files with 40 additions and 50 deletions.
1 change: 1 addition & 0 deletions l1-contracts/src/core/libraries/ConstantsGen.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
3 changes: 1 addition & 2 deletions noir-projects/aztec-nr/authwit/src/account.nr
Original file line number Diff line number Diff line change
@@ -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};
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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
};

Expand Down
2 changes: 1 addition & 1 deletion noir-projects/aztec-nr/aztec/src/state_vars/map.nr
Original file line number Diff line number Diff line change
@@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@ struct SharedMutable<T, let INITIAL_DELAY: u32, Context> {
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<N> trait, and allocate N storage slots to
// this state variable. This is incorrect, since what we actually store is:
// - a ScheduledValueChange<T>, which requires 1 + 2 * M storage slots, where M is the serialization length of T
Expand Down Expand Up @@ -76,15 +81,15 @@ impl<T, let INITIAL_DELAY: u32, Context> SharedMutable<T, INITIAL_DELAY, Context
// - a ScheduledDelaChange
// - the hash of both of these (via `hash_scheduled_data`)
fn get_value_change_storage_slot(self) -> 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
Expand Down
Original file line number Diff line number Diff line change
@@ -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<T, INITIAL_DELAY> {
context: &mut PrivateContext,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -76,15 +76,15 @@ 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)]
fn add_storage_map(to: AztecAddress, amount: u32) -> Field {
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)]
Expand Down
Original file line number Diff line number Diff line change
@@ -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)
Expand Down
Original file line number Diff line number Diff line change
@@ -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}
};
Expand Down
Original file line number Diff line number Diff line change
@@ -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,
Expand Down
Original file line number Diff line number Diff line change
@@ -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,
Expand Down
Original file line number Diff line number Diff line change
@@ -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};

Expand Down
Original file line number Diff line number Diff line change
@@ -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
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
@@ -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<K>(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 {
Expand All @@ -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);
}
Expand Down
1 change: 1 addition & 0 deletions yarn-project/circuits.js/src/constants.gen.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion yarn-project/circuits.js/src/hash/map_slot.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 4 additions & 2 deletions yarn-project/circuits.js/src/hash/map_slot.ts
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -14,5 +16,5 @@ export function deriveStorageSlotInMap(
toField: () => Fr;
},
): Fr {
return pedersenHash([mapSlot, key.toField()]);
return pedersenHash([mapSlot, key.toField()], MAP_STORAGE_SLOT_SEPARATOR);
}
6 changes: 3 additions & 3 deletions yarn-project/end-to-end/src/e2e_keys.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down

0 comments on commit 9ca4c8a

Please sign in to comment.