diff --git a/noir-projects/aztec-nr/aztec/src/history/public_storage.nr b/noir-projects/aztec-nr/aztec/src/history/public_storage.nr index ff1fe6c01df6..fd1b0e52db7b 100644 --- a/noir-projects/aztec-nr/aztec/src/history/public_storage.nr +++ b/noir-projects/aztec-nr/aztec/src/history/public_storage.nr @@ -6,7 +6,7 @@ use dep::std::merkle::compute_merkle_root; use crate::{context::PrivateContext, oracle::get_public_data_witness::get_public_data_witness}; -fn _public_storage_historical_read(storage_slot: Field, contract_address: AztecAddress, header: Header) -> Field { +pub fn _public_storage_historical_read(storage_slot: Field, contract_address: AztecAddress, header: Header) -> Field { // 1) Compute the leaf slot by siloing the storage slot with the contract address let public_value_leaf_slot = pedersen_hash( [contract_address.to_field(), storage_slot], diff --git a/noir-projects/aztec-nr/aztec/src/keys/getters.nr b/noir-projects/aztec-nr/aztec/src/keys/getters.nr index d1735aa2dd9d..5791859d1852 100644 --- a/noir-projects/aztec-nr/aztec/src/keys/getters.nr +++ b/noir-projects/aztec-nr/aztec/src/keys/getters.nr @@ -1,5 +1,5 @@ use dep::protocol_types::{ - abis::key_validation_request::KeyValidationRequest, address::AztecAddress, + header::Header, abis::key_validation_request::KeyValidationRequest, address::AztecAddress, constants::CANONICAL_KEY_REGISTRY_ADDRESS, grumpkin_point::GrumpkinPoint, storage::map::derive_storage_slot_in_map }; @@ -17,8 +17,28 @@ pub fn get_npk_m(context: &mut PrivateContext, address: AztecAddress) -> Grumpki get_master_key(context, address, NULLIFIER_INDEX) } +// For historical access, we move the fetching of the header to the top level to minimize redundant fetching / proving validity of the header. +pub fn get_npk_m_at( + context: &mut PrivateContext, + address: AztecAddress, + block_number: u32, + header: Header +) -> GrumpkinPoint { + assert_eq(block_number, header.global_variables.block_number as u32); + get_master_key_at(context, address, NULLIFIER_INDEX, header) +} + pub fn get_npk_m_hash(context: &mut PrivateContext, address: AztecAddress) -> Field { - get_master_key(context, address, NULLIFIER_INDEX).hash() + get_npk_m(context, address).hash() +} + +pub fn get_npk_m_hash_at( + context: &mut PrivateContext, + address: AztecAddress, + block_number: u32, + header: Header +) -> Field { + get_npk_m_at(context, address, block_number, header).hash() } pub fn get_ivpk_m(context: &mut PrivateContext, address: AztecAddress) -> GrumpkinPoint { @@ -40,7 +60,23 @@ fn get_master_key( address: AztecAddress, key_index: Field ) -> GrumpkinPoint { - let key = fetch_key_from_registry(context, key_index, address); + let (x_coordinate_registry, y_coordinate_registry) = get_registry_with_key_index(context, key_index, address); + let key = fetch_key_from_registry(x_coordinate_registry, y_coordinate_registry); + fetch_key_from_oracle_if_zero(key, address, key_index) +} + +fn get_master_key_at( + context: &mut PrivateContext, + address: AztecAddress, + key_index: Field, + header: Header +) -> GrumpkinPoint { + let (x_coordinate_registry, y_coordinate_registry) = get_registry_with_key_index(context, key_index, address); + let key = fetch_key_from_registry_at(x_coordinate_registry, y_coordinate_registry, header); + fetch_key_from_oracle_if_zero(key, address, key_index) +} + +fn fetch_key_from_oracle_if_zero(key: GrumpkinPoint, address: AztecAddress, key_index: Field) -> GrumpkinPoint { if key.is_zero() { // Keys were not registered in registry yet --> fetch key from PXE let keys = fetch_and_constrain_keys(address); @@ -52,11 +88,11 @@ fn get_master_key( } } -fn fetch_key_from_registry( +fn get_registry_with_key_index( context: &mut PrivateContext, key_index: Field, address: AztecAddress -) -> GrumpkinPoint { +) -> (SharedMutablePrivateGetter, SharedMutablePrivateGetter) { let x_coordinate_map_slot = key_index * 2 + 1; let y_coordinate_map_slot = x_coordinate_map_slot + 1; let x_coordinate_derived_slot = derive_storage_slot_in_map(x_coordinate_map_slot, address); @@ -72,10 +108,29 @@ fn fetch_key_from_registry( AztecAddress::from_field(CANONICAL_KEY_REGISTRY_ADDRESS), y_coordinate_derived_slot ); - let x_coordinate = x_coordinate_registry.get_current_value_in_private(); - let y_coordinate = y_coordinate_registry.get_current_value_in_private(); - GrumpkinPoint::new(x_coordinate, y_coordinate) + (x_coordinate_registry, y_coordinate_registry) +} + +fn fetch_key_from_registry( + x_coordinate_registry: SharedMutablePrivateGetter, + y_coordinate_registry: SharedMutablePrivateGetter +) -> GrumpkinPoint { + let key_x_coordinate = x_coordinate_registry.get_current_value_in_private(); + let key_y_coordinate = y_coordinate_registry.get_current_value_in_private(); + + GrumpkinPoint::new(key_x_coordinate, key_y_coordinate) +} + +fn fetch_key_from_registry_at( + x_coordinate_registry: SharedMutablePrivateGetter, + y_coordinate_registry: SharedMutablePrivateGetter, + header: Header +) -> GrumpkinPoint { + let key_x_coordinate = x_coordinate_registry.get_current_value_in_private_at(header.global_variables.block_number as u32, header); + let key_y_coordinate = y_coordinate_registry.get_current_value_in_private_at(header.global_variables.block_number as u32, header); + + GrumpkinPoint::new(key_x_coordinate, key_y_coordinate) } // Passes only when keys were not rotated - is expected to be called only when keys were not registered yet 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 7e923806e73a..c3a816e91ebe 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,7 +1,7 @@ -use dep::protocol_types::{hash::pedersen_hash, traits::FromField, address::AztecAddress}; +use dep::protocol_types::{hash::pedersen_hash, traits::FromField, address::AztecAddress, header::Header}; +use crate::history::public_storage::{public_storage_historical_read, _public_storage_historical_read}; use crate::context::PrivateContext; -use crate::history::public_storage::public_storage_historical_read; use crate::public_storage; use crate::state_vars::{ storage::Storage, @@ -34,8 +34,9 @@ impl SharedMutablePrivateGetter { pub fn get_current_value_in_private(self) -> T where T: FromField { let mut context = self.context; + let header = context.historical_header; - let (value_change, delay_change, historical_block_number) = self.historical_read_from_public_storage(context); + let (value_change, delay_change, historical_block_number) = self.historical_read_from_public_storage(header); let effective_minimum_delay = delay_change.get_effective_minimum_delay_at(historical_block_number); let block_horizon = value_change.get_block_horizon(historical_block_number, effective_minimum_delay); @@ -43,27 +44,40 @@ impl SharedMutablePrivateGetter { value_change.get_current_at(historical_block_number) } + pub fn get_current_value_in_private_at( + self, + block_number: u32, + header: Header + ) -> T where T: FromField { + assert_eq(block_number, header.global_variables.block_number as u32); + // We don't need the delay change because even if we execute this in the future, we are looking at the value + // from a specific time in the past. + let (value_change, _, historical_block_number) = self.historical_read_from_public_storage(header); + + value_change.get_current_at(historical_block_number) + } + fn historical_read_from_public_storage( self, - context: PrivateContext + header: Header ) -> (ScheduledValueChange, ScheduledDelayChange, u32) where T: FromField { let value_change_slot = self.get_value_change_storage_slot(); let mut raw_value_change_fields = [0; 3]; for i in 0..3 { - raw_value_change_fields[i] = public_storage_historical_read( - context, + raw_value_change_fields[i] = _public_storage_historical_read( value_change_slot + i as Field, - self.other_contract_address + self.other_contract_address, + header, ); } let delay_change_slot = self.get_delay_change_storage_slot(); - let raw_delay_change_fields = [public_storage_historical_read(context, delay_change_slot, context.this_address())]; + let raw_delay_change_fields = [_public_storage_historical_read(delay_change_slot, self.other_contract_address, header)]; let value_change = ScheduledValueChange::deserialize(raw_value_change_fields); let delay_change = ScheduledDelayChange::deserialize(raw_delay_change_fields); - let historical_block_number = context.historical_header.global_variables.block_number as u32; + let historical_block_number = header.global_variables.block_number as u32; (value_change, delay_change, historical_block_number) } diff --git a/noir-projects/aztec-nr/easy-private-state/src/easy_private_uint.nr b/noir-projects/aztec-nr/easy-private-state/src/easy_private_uint.nr index eeed3726be01..254cc5894a04 100644 --- a/noir-projects/aztec-nr/easy-private-state/src/easy_private_uint.nr +++ b/noir-projects/aztec-nr/easy-private-state/src/easy_private_uint.nr @@ -49,11 +49,6 @@ impl EasyPrivateUint<&mut PrivateContext> { if maybe_notes[i].is_some() { let note = maybe_notes[i].unwrap_unchecked(); - // Ensure the notes are actually owned by the owner (to prevent user from generating a valid proof while - // spending someone else's notes). - // TODO (#6312): This will break with key rotation. Fix this. Will not be able to pass this assert after rotating key - assert(note.npk_m_hash.eq(owner_npk_m_hash)); - // Removes the note from the owner's set of notes. // docs:start:remove self.set.remove(note); diff --git a/noir-projects/aztec-nr/value-note/src/utils.nr b/noir-projects/aztec-nr/value-note/src/utils.nr index c3a39e7877d3..fbde98fb2654 100644 --- a/noir-projects/aztec-nr/value-note/src/utils.nr +++ b/noir-projects/aztec-nr/value-note/src/utils.nr @@ -55,23 +55,12 @@ pub fn decrement_by_at_most( let options = create_note_getter_options_for_decreasing_balance(max_amount); let opt_notes = balance.get_notes(options); - let owner_npk_m_hash = get_npk_m_hash(balance.context, owner); - let mut decremented = 0; for i in 0..opt_notes.len() { if opt_notes[i].is_some() { let note = opt_notes[i].unwrap_unchecked(); - // This is similar to destroy_note, except we only compute the owner_npk_m_hash once instead of doing it in - // each loop iteration. - - // Ensure the note is actually owned by the owner (to prevent user from generating a valid proof while - // spending someone else's notes). - // TODO (#6312): This will break with key rotation. Fix this. Will not be able to pass this after rotating keys. - assert(note.npk_m_hash.eq(owner_npk_m_hash)); - decremented += note.value; - - balance.remove(note); + decremented += destroy_note(balance, note); } } @@ -88,18 +77,7 @@ pub fn decrement_by_at_most( // Removes the note from the owner's set of notes. // Returns the value of the destroyed note. -pub fn destroy_note( - balance: PrivateSet, - owner: AztecAddress, - note: ValueNote -) -> Field { - // Ensure the note is actually owned by the owner (to prevent user from generating a valid proof while - // spending someone else's notes). - let owner_npk_m_hash = get_npk_m_hash(balance.context, owner); - - // TODO (#6312): This will break with key rotation. Fix this. Will not be able to pass this after rotating keys. - assert(note.npk_m_hash.eq(owner_npk_m_hash)); - +pub fn destroy_note(balance: PrivateSet, note: ValueNote) -> Field { balance.remove(note); note.value diff --git a/noir-projects/noir-contracts/contracts/card_game_contract/src/cards.nr b/noir-projects/noir-contracts/contracts/card_game_contract/src/cards.nr index a14d62cdf4ba..57dfa32f6cdf 100644 --- a/noir-projects/noir-contracts/contracts/card_game_contract/src/cards.nr +++ b/noir-projects/noir-contracts/contracts/card_game_contract/src/cards.nr @@ -118,19 +118,13 @@ impl Deck<&mut PrivateContext> { inserted_cards } - pub fn get_cards(&mut self, cards: [Card; N], owner: AztecAddress) -> [CardNote; N] { - let owner_npk_m_hash = get_npk_m_hash(self.set.context, owner); - + pub fn get_cards(&mut self, cards: [Card; N]) -> [CardNote; N] { let options = NoteGetterOptions::with_filter(filter_cards, cards); let maybe_notes = self.set.get_notes(options); let mut found_cards = [Option::none(); N]; for i in 0..maybe_notes.len() { if maybe_notes[i].is_some() { let card_note = CardNote::from_note(maybe_notes[i].unwrap_unchecked()); - // Ensure the notes are actually owned by the owner (to prevent user from generating a valid proof while - // spending someone else's notes). - // TODO (#6312): This will break with key rotation. Fix this. Will not be able to pass this assert after rotating keys. - assert(card_note.note.npk_m_hash.eq(owner_npk_m_hash)); for j in 0..cards.len() { if found_cards[j].is_none() @@ -150,8 +144,8 @@ impl Deck<&mut PrivateContext> { ) } - pub fn remove_cards(&mut self, cards: [Card; N], owner: AztecAddress) { - let card_notes = self.get_cards(cards, owner); + pub fn remove_cards(&mut self, cards: [Card; N]) { + let card_notes = self.get_cards(cards); for card_note in card_notes { self.set.remove(card_note.note); } diff --git a/noir-projects/noir-contracts/contracts/card_game_contract/src/main.nr b/noir-projects/noir-contracts/contracts/card_game_contract/src/main.nr index 53ab64848dfa..bfb982eef571 100644 --- a/noir-projects/noir-contracts/contracts/card_game_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/card_game_contract/src/main.nr @@ -36,7 +36,7 @@ contract CardGame { let player = context.msg_sender(); let mut collection = storage.collections.at(player); - collection.remove_cards(cards, player); + collection.remove_cards(cards); let mut game_deck = storage.game_decks.at(game as Field).at(player); let _added_to_game_deck = game_deck.add_cards(cards, player); let strength = compute_deck_strength(cards); @@ -68,7 +68,7 @@ contract CardGame { let player = context.msg_sender(); let mut game_deck = storage.game_decks.at(game as Field).at(player); - game_deck.remove_cards([card], player); + game_deck.remove_cards([card]); // docs:start:call_public_function CardGame::at(context.this_address()).on_card_played(game, player, card.to_field()).enqueue(&mut context); diff --git a/noir-projects/noir-contracts/contracts/child_contract/src/main.nr b/noir-projects/noir-contracts/contracts/child_contract/src/main.nr index 5c97d9a5c640..3000edb59ecb 100644 --- a/noir-projects/noir-contracts/contracts/child_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/child_contract/src/main.nr @@ -1,6 +1,6 @@ // A contract used along with `Parent` contract to test nested calls. contract Child { - use dep::aztec::prelude::{AztecAddress, FunctionSelector, PublicMutable, PrivateSet, PrivateContext, Deserialize}; + use dep::aztec::prelude::{AztecAddress, FunctionSelector, PublicMutable, PrivateSet, PrivateContext, Deserialize, Map}; use dep::aztec::{ context::gas::GasOpts, @@ -13,7 +13,7 @@ contract Child { #[aztec(storage)] struct Storage { current_value: PublicMutable, - a_private_value: PrivateSet, + a_map_with_private_values: Map>, } // Returns a sum of the input and the chain id and version of the contract in private circuit public input's return_values. @@ -56,22 +56,15 @@ contract Child { let owner_ivpk_m = get_ivpk_m(&mut context, owner); let mut note = ValueNote::new(new_value, owner_npk_m_hash); - storage.a_private_value.insert(&mut note, true, owner_ivpk_m); + storage.a_map_with_private_values.at(owner).insert(&mut note, true, owner_ivpk_m); new_value } #[aztec(private)] fn private_get_value(amount: Field, owner: AztecAddress) -> Field { - let owner_npk_m_hash = get_npk_m_hash(&mut context, owner); - - // TODO (#6312): This will break with key rotation. Fix this. Will not be able to find any notes after rotating key. let mut options = NoteGetterOptions::new(); - options = options.select(ValueNote::properties().value, amount, Option::none()).select( - ValueNote::properties().npk_m_hash, - owner_npk_m_hash, - Option::none() - ).set_limit(1); - let notes = storage.a_private_value.get_notes(options); + options = options.select(ValueNote::properties().value, amount, Option::none()).set_limit(1); + let notes = storage.a_map_with_private_values.at(owner).get_notes(options); notes[0].unwrap_unchecked().value } diff --git a/noir-projects/noir-contracts/contracts/claim_contract/src/main.nr b/noir-projects/noir-contracts/contracts/claim_contract/src/main.nr index 72f80b872a48..41dfa6e0899a 100644 --- a/noir-projects/noir-contracts/contracts/claim_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/claim_contract/src/main.nr @@ -30,17 +30,13 @@ contract Claim { target_address == proof_note.header.contract_address, "Note does not correspond to the target contract" ); - let msg_sender_npk_m_hash = get_npk_m_hash(&mut context, context.msg_sender()); - - // TODO (#6312): This will break with key rotation. Fix this. Will not be able to pass this assert after key rotation. - assert_eq(proof_note.npk_m_hash, msg_sender_npk_m_hash, "Note does not belong to the sender"); - // 2) Prove that the note hash exists in the note hash tree prove_note_inclusion(proof_note, context); // 3) Compute and emit a nullifier which is unique to the note and this contract to ensure the reward can be // claimed only once with the given note. - // Note: The nullifier is unique to the note and THIS contract because the protocol siloes all nullifiers with + // Note: Only the owner of the npk_m will be able to produce the nsk_app and compute this nullifier. + // The nullifier is unique to the note and THIS contract because the protocol siloes all nullifiers with // the address of a contract it was emitted from. context.push_new_nullifier(proof_note.compute_nullifier(&mut context), 0); diff --git a/noir-projects/noir-contracts/contracts/delegated_on_contract/src/main.nr b/noir-projects/noir-contracts/contracts/delegated_on_contract/src/main.nr index f38d4bff58c6..bcf3db8c7613 100644 --- a/noir-projects/noir-contracts/contracts/delegated_on_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/delegated_on_contract/src/main.nr @@ -2,7 +2,7 @@ contract DelegatedOn { use dep::aztec::prelude::{ AztecAddress, FunctionSelector, NoteHeader, NoteGetterOptions, NoteViewerOptions, PublicMutable, - PrivateSet, PrivateContext + PrivateSet, PrivateContext, Map }; use dep::aztec::{protocol_types::grumpkin_point::GrumpkinPoint, keys::getters::{get_npk_m_hash, get_ivpk_m}}; use dep::value_note::value_note::ValueNote; @@ -10,7 +10,7 @@ contract DelegatedOn { #[aztec(storage)] struct Storage { current_value: PublicMutable, - a_private_value: PrivateSet, + a_map_with_private_values: Map>, } #[aztec(private)] @@ -19,7 +19,8 @@ contract DelegatedOn { let owner_ivpk_m = get_ivpk_m(&mut context, owner); let mut note = ValueNote::new(new_value, owner_npk_m_hash); - storage.a_private_value.insert(&mut note, true, owner_ivpk_m); + storage.a_map_with_private_values.at(owner).insert(&mut note, true, owner_ivpk_m); + new_value } @@ -31,16 +32,9 @@ contract DelegatedOn { #[aztec(private)] fn get_private_value(amount: Field, owner: AztecAddress) -> pub Field { - let owner_npk_m_hash = get_npk_m_hash(&mut context, owner); - - // TODO (#6312): This will break with key rotation. Fix this. Will not be able to find any notes after rotating key. let mut options = NoteGetterOptions::new(); - options = options.select(ValueNote::properties().value, amount, Option::none()).select( - ValueNote::properties().npk_m_hash, - owner_npk_m_hash, - Option::none() - ).set_limit(1); - let notes = storage.a_private_value.get_notes(options); + options = options.select(ValueNote::properties().value, amount, Option::none()).set_limit(1); + let notes = storage.a_map_with_private_values.at(owner).get_notes(options); notes[0].unwrap_unchecked().value } diff --git a/noir-projects/noir-contracts/contracts/delegator_contract/src/main.nr b/noir-projects/noir-contracts/contracts/delegator_contract/src/main.nr index 023974cd81ff..2096c41487ea 100644 --- a/noir-projects/noir-contracts/contracts/delegator_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/delegator_contract/src/main.nr @@ -1,6 +1,6 @@ // A contract used along with `Parent` contract to test nested calls. contract Delegator { - use dep::aztec::prelude::{AztecAddress, FunctionSelector, NoteHeader, NoteGetterOptions, PublicMutable, PrivateSet, Deserialize}; + use dep::aztec::prelude::{AztecAddress, FunctionSelector, NoteHeader, NoteGetterOptions, PublicMutable, PrivateSet, Deserialize, Map}; use dep::aztec::keys::getters::get_npk_m_hash; use dep::value_note::value_note::ValueNote; use dep::delegated_on::DelegatedOn; @@ -8,7 +8,7 @@ contract Delegator { #[aztec(storage)] struct Storage { current_value: PublicMutable, - a_private_value: PrivateSet, + a_map_with_private_values: Map>, } #[aztec(private)] @@ -33,16 +33,9 @@ contract Delegator { #[aztec(private)] fn get_private_value(amount: Field, owner: AztecAddress) -> pub Field { - let owner_npk_m_hash = get_npk_m_hash(&mut context, owner); - - // TODO (#6312): This will break with key rotation. Fix this. Will not be able to find any notes after rotating key. let mut options = NoteGetterOptions::new(); - options = options.select(ValueNote::properties().value, amount, Option::none()).select( - ValueNote::properties().npk_m_hash, - owner_npk_m_hash, - Option::none() - ).set_limit(1); - let notes = storage.a_private_value.get_notes(options); + options = options.select(ValueNote::properties().value, amount, Option::none()).set_limit(1); + let notes = storage.a_map_with_private_values.at(owner).get_notes(options); notes[0].unwrap_unchecked().value } diff --git a/noir-projects/noir-contracts/contracts/docs_example_contract/src/main.nr b/noir-projects/noir-contracts/contracts/docs_example_contract/src/main.nr index 267c6affff95..caca3fed3d65 100644 --- a/noir-projects/noir-contracts/contracts/docs_example_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/docs_example_contract/src/main.nr @@ -21,7 +21,7 @@ contract DocsExample { use dep::aztec::{note::note_getter_options::Comparator, keys::getters::{get_npk_m_hash, get_ivpk_m}}; use dep::aztec::protocol_types::grumpkin_point::GrumpkinPoint; // how to import methods from other files/folders within your workspace - use crate::options::create_account_card_getter_options; + use crate::options::create_points_card_getter_options; use crate::types::{card_note::{CardNote, CARD_NOTE_LEN}, leader::Leader}; #[aztec(storage)] diff --git a/noir-projects/noir-contracts/contracts/docs_example_contract/src/options.nr b/noir-projects/noir-contracts/contracts/docs_example_contract/src/options.nr index 86cb169fe554..df478482f1f0 100644 --- a/noir-projects/noir-contracts/contracts/docs_example_contract/src/options.nr +++ b/noir-projects/noir-contracts/contracts/docs_example_contract/src/options.nr @@ -7,17 +7,12 @@ use dep::aztec::note::note_getter_options::{Sort, SortOrder}; // Shows how to use NoteGetterOptions and query for notes. // docs:start:state_vars-NoteGetterOptionsSelectSortOffset -pub fn create_account_card_getter_options( - account_npk_m_hash: Field, +pub fn create_points_card_getter_options( + points: Field, offset: u32 ) -> NoteGetterOptions { - // TODO (#6312): This will break with key rotation. Fix this. Will not be able to find any notes after rotating key. let mut options = NoteGetterOptions::new(); - options.select( - CardNote::properties().npk_m_hash, - account_npk_m_hash, - Option::none() - ).sort(CardNote::properties().points, SortOrder.DESC).set_offset(offset) + options.select(CardNote::properties().points, points, Option::none()).sort(CardNote::properties().points, SortOrder.DESC).set_offset(offset) } // docs:end:state_vars-NoteGetterOptionsSelectSortOffset @@ -27,7 +22,6 @@ pub fn create_exact_card_getter_options( secret: Field, account_npk_m_hash: Field ) -> NoteGetterOptions { - // TODO (#6312): This will break with key rotation. Fix this. Will not be able to find any notes after rotating key. let mut options = NoteGetterOptions::new(); options.select(CardNote::properties().points, points as Field, Option::none()).select(CardNote::properties().randomness, secret, Option::none()).select( CardNote::properties().npk_m_hash, @@ -55,27 +49,14 @@ pub fn filter_min_points( // docs:end:state_vars-OptionFilter // docs:start:state_vars-NoteGetterOptionsFilter -pub fn create_account_cards_with_min_points_getter_options( - account_npk_m_hash: Field, - min_points: u8 -) -> NoteGetterOptions { - // TODO (#6312): This will break with key rotation. Fix this. Will not be able to find any notes after rotating key. - NoteGetterOptions::with_filter(filter_min_points, min_points).select( - CardNote::properties().npk_m_hash, - account_npk_m_hash, - Option::none() - ).sort(CardNote::properties().points, SortOrder.ASC) +pub fn create_cards_with_min_points_getter_options(min_points: u8) -> NoteGetterOptions { + NoteGetterOptions::with_filter(filter_min_points, min_points).sort(CardNote::properties().points, SortOrder.ASC) } // docs:end:state_vars-NoteGetterOptionsFilter // docs:start:state_vars-NoteGetterOptionsPickOne -pub fn create_largest_account_card_getter_options(account_npk_m_hash: Field) -> NoteGetterOptions { - // TODO (#6312): This will break with key rotation. Fix this. Will not be able to find any notes after rotating key. +pub fn create_largest_card_getter_options() -> NoteGetterOptions { let mut options = NoteGetterOptions::new(); - options.select( - CardNote::properties().npk_m_hash, - account_npk_m_hash, - Option::none() - ).sort(CardNote::properties().points, SortOrder.DESC).set_limit(1) + options.sort(CardNote::properties().points, SortOrder.DESC).set_limit(1) } // docs:end:state_vars-NoteGetterOptionsPickOne diff --git a/noir-projects/noir-contracts/contracts/easy_private_voting_contract/src/main.nr b/noir-projects/noir-contracts/contracts/easy_private_voting_contract/src/main.nr index bb19fa9505e5..554e76b17bef 100644 --- a/noir-projects/noir-contracts/contracts/easy_private_voting_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/easy_private_voting_contract/src/main.nr @@ -2,9 +2,9 @@ contract EasyPrivateVoting { // docs:start:imports use dep::aztec::prelude::{ AztecAddress, FunctionSelector, NoteHeader, NoteInterface, NoteGetterOptions, PrivateContext, - Map, PublicMutable + Map, PublicMutable, SharedImmutable }; - use dep::aztec::keys::getters::get_npk_m_hash; + use dep::aztec::keys::getters::get_npk_m_hash_at; // docs:end:imports // docs:start:storage_struct #[aztec(storage)] @@ -12,6 +12,7 @@ contract EasyPrivateVoting { admin: PublicMutable, // admin can end vote tally: Map>, // we will store candidate as key and number of votes as value vote_ended: PublicMutable, // vote_ended is boolean + active_at_block: SharedImmutable, } // docs:end:storage_struct @@ -21,14 +22,26 @@ contract EasyPrivateVoting { fn constructor(admin: AztecAddress) { storage.admin.write(admin); storage.vote_ended.write(false); + storage.active_at_block.initialize(context.historical_header.global_variables.block_number as u32); } // docs:end:constructor // docs:start:cast_vote #[aztec(private)] // annotation to mark function as private and expose private context fn cast_vote(candidate: Field) { - let msg_sender_npk_m_hash = get_npk_m_hash(&mut context, context.msg_sender()); - // TODO (#6312): This will break with key rotation. Fix this. Can vote multiple times by rotating keys. + let active_at_block = storage.active_at_block.read_private(); + assert( + context.historical_header.global_variables.block_number as u32 >= active_at_block, "Voting isn't allowed yet" + ); + + let header_at_active_at_block = context.get_header_at(active_at_block); + let msg_sender_npk_m_hash = get_npk_m_hash_at( + &mut context, + context.msg_sender(), + active_at_block, + header_at_active_at_block + ); + let secret = context.request_nsk_app(msg_sender_npk_m_hash); // get secret key of caller of function let nullifier = dep::std::hash::pedersen_hash([context.msg_sender().to_field(), secret]); // derive nullifier from sender and secret context.push_new_nullifier(nullifier, 0); // push nullifier diff --git a/noir-projects/noir-contracts/contracts/inclusion_proofs_contract/src/main.nr b/noir-projects/noir-contracts/contracts/inclusion_proofs_contract/src/main.nr index d3fbbcf4b162..ebd2d64f9e3b 100644 --- a/noir-projects/noir-contracts/contracts/inclusion_proofs_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/inclusion_proofs_contract/src/main.nr @@ -59,18 +59,11 @@ contract InclusionProofs { block_number: u32, // The block at which we'll prove that the note exists nullified: bool ) { - let owner_npk_m_hash = get_npk_m_hash(&mut context, owner); - - // TODO (#6312): This will break with key rotation. Fix this. Will not be able to find any notes after rotating key. // docs:start:get_note_from_pxe // 1) Get the note from PXE. let private_values = storage.private_values.at(owner); let mut options = NoteGetterOptions::new(); - options = options.select( - ValueNote::properties().npk_m_hash, - owner_npk_m_hash, - Option::none() - ).set_limit(1); + options = options.set_limit(1); if (nullified) { options = options.set_status(NoteStatus.ACTIVE_OR_NULLIFIED); } @@ -115,17 +108,10 @@ contract InclusionProofs { // because PXE performs note commitment inclusion check when you add a new note). fail_case: bool ) { - let owner_npk_m_hash = get_npk_m_hash(&mut context, owner); - - // TODO (#6312): This will break with key rotation. Fix this. Will not be able to find any notes after rotating key. // 2) Get the note from PXE let private_values = storage.private_values.at(owner); let mut options = NoteGetterOptions::new(); - options = options.select( - ValueNote::properties().npk_m_hash, - owner_npk_m_hash, - Option::none() - ).set_limit(1); + options = options.set_limit(1); if (fail_case) { options = options.set_status(NoteStatus.ACTIVE_OR_NULLIFIED); } @@ -149,17 +135,10 @@ contract InclusionProofs { block_number: u32, // The block at which we'll prove that the note exists and is not nullified nullified: bool ) { - let owner_npk_m_hash = get_npk_m_hash(&mut context, owner); - - // TODO (#6312): This will break with key rotation. Fix this. Will not be able to find any notes after rotating key. // 1) Get the note from PXE. let private_values = storage.private_values.at(owner); let mut options = NoteGetterOptions::new(); - options = options.select( - ValueNote::properties().npk_m_hash, - owner_npk_m_hash, - Option::none() - ).set_limit(1); + options = options.set_limit(1); if (nullified) { options = options.set_status(NoteStatus.ACTIVE_OR_NULLIFIED); } @@ -179,16 +158,9 @@ contract InclusionProofs { // docs:start:nullify_note #[aztec(private)] fn nullify_note(owner: AztecAddress) { - let owner_npk_m_hash = get_npk_m_hash(&mut context, owner); - - // TODO (#6312): This will break with key rotation. Fix this. Will not be able to find any notes after rotating key. let private_values = storage.private_values.at(owner); let mut options = NoteGetterOptions::new(); - options = options.select( - ValueNote::properties().npk_m_hash, - owner_npk_m_hash, - Option::none() - ).set_limit(1); + options = options.set_limit(1); let notes = private_values.get_notes(options); let note = notes[0].unwrap(); diff --git a/yarn-project/end-to-end/src/e2e_crowdfunding_and_claim.test.ts b/yarn-project/end-to-end/src/e2e_crowdfunding_and_claim.test.ts index 2dedcbade813..cb54db932736 100644 --- a/yarn-project/end-to-end/src/e2e_crowdfunding_and_claim.test.ts +++ b/yarn-project/end-to-end/src/e2e_crowdfunding_and_claim.test.ts @@ -1,6 +1,8 @@ +import { createAccounts } from '@aztec/accounts/testing'; import { type AccountWallet, type AztecAddress, + type AztecNode, type CheatCodes, type DebugLogger, ExtendedNote, @@ -19,7 +21,7 @@ import { TokenContract } from '@aztec/noir-contracts.js/Token'; import { jest } from '@jest/globals'; -import { setup } from './fixtures/utils.js'; +import { setup, setupPXEService } from './fixtures/utils.js'; jest.setTimeout(200_000); @@ -37,7 +39,11 @@ describe('e2e_crowdfunding_and_claim', () => { decimals: 18n, }; - let teardown: () => Promise; + let teardownA: () => Promise; + let teardownB: () => Promise; + + let aztecNode: AztecNode; + let operatorWallet: AccountWallet; let donorWallets: AccountWallet[]; let wallets: AccountWallet[]; @@ -76,7 +82,7 @@ describe('e2e_crowdfunding_and_claim', () => { }; beforeAll(async () => { - ({ cheatCodes, teardown, logger, pxe, wallets } = await setup(3)); + ({ cheatCodes, teardown: teardownA, logger, pxe, wallets, aztecNode } = await setup(3)); operatorWallet = wallets[0]; donorWallets = wallets.slice(1); @@ -130,7 +136,10 @@ describe('e2e_crowdfunding_and_claim', () => { await mintDNTToDonors(); }); - afterAll(() => teardown()); + afterAll(async () => { + await teardownA(); + await teardownB(); + }); const mintDNTToDonors = async () => { const secret = Fr.random(); @@ -262,7 +271,7 @@ describe('e2e_crowdfunding_and_claim', () => { ).rejects.toThrow(); }); - it('cannot claim with a different address than the one that donated', async () => { + it('cannot claim without access to the nsk_app tied to the npk_m specified in the proof note', async () => { const donationAmount = 1000n; { const action = donationToken @@ -291,15 +300,27 @@ describe('e2e_crowdfunding_and_claim', () => { // Set the value note in a format which can be passed to claim function const anotherDonationNote = await processExtendedNote(notes![0]); - // 3) We claim the reward token via the Claim contract + // We create an unrelated pxe and wallet without access to the nsk_app that correlates to the npk_m specified in the proof note. + let unrelatedWallet: AccountWallet; + { + const { pxe: pxeB, teardown: _teardown } = await setupPXEService(aztecNode!, {}, undefined, true); + teardownB = _teardown; + [unrelatedWallet] = await createAccounts(pxeB, 1); + await pxeB.registerContract({ + artifact: ClaimContract.artifact, + instance: claimContract.instance, + }); + } + + // 3) We try to claim the reward token via the Claim contract with the unrelated wallet { await expect( claimContract - .withWallet(donorWallets[0]) - .methods.claim(anotherDonationNote, donorWallets[1].getAddress()) + .withWallet(unrelatedWallet) + .methods.claim(anotherDonationNote, unrelatedWallet.getAddress()) .send() .wait(), - ).rejects.toThrow('Note does not belong to the sender'); + ).rejects.toThrow('Could not find key prefix.'); } }); diff --git a/yarn-project/end-to-end/src/e2e_private_voting_contract.test.ts b/yarn-project/end-to-end/src/e2e_private_voting_contract.test.ts index aeda6f45ee83..72b655446c0a 100644 --- a/yarn-project/end-to-end/src/e2e_private_voting_contract.test.ts +++ b/yarn-project/end-to-end/src/e2e_private_voting_contract.test.ts @@ -1,14 +1,18 @@ import { type AccountWallet, type AztecAddress, type DebugLogger, Fr } from '@aztec/aztec.js'; +import { TestContract } from '@aztec/noir-contracts.js'; import { EasyPrivateVotingContract } from '@aztec/noir-contracts.js/EasyPrivateVoting'; import { setup } from './fixtures/utils.js'; +const SHARED_MUTABLE_DELAY = 5; + describe('e2e_voting_contract', () => { let wallet: AccountWallet; let logger: DebugLogger; let teardown: () => Promise; + let testContract: TestContract; let votingContract: EasyPrivateVotingContract; let owner: AztecAddress; @@ -17,6 +21,7 @@ describe('e2e_voting_contract', () => { ({ teardown, wallet, logger } = await setup(1)); owner = wallet.getAddress(); + testContract = await TestContract.deploy(wallet).send().deployed(); votingContract = await EasyPrivateVotingContract.deploy(wallet, owner).send().deployed(); logger.info(`Counter contract deployed at ${votingContract.address}`); @@ -24,11 +29,27 @@ describe('e2e_voting_contract', () => { afterAll(() => teardown()); + const crossDelay = async () => { + for (let i = 0; i < SHARED_MUTABLE_DELAY; i++) { + // We send arbitrary tx to mine a block + await testContract.methods.emit_unencrypted(0).send().wait(); + } + }; + describe('votes', () => { - it('votes', async () => { + it('votes, rotates nullifier keys, then tries to vote again', async () => { const candidate = new Fr(1); await votingContract.methods.cast_vote(candidate).send().wait(); expect(await votingContract.methods.get_vote(candidate).simulate()).toBe(1n); + + // We rotate our nullifier keys + await wallet.rotateNullifierKeys(); + await crossDelay(); + + // We try voting again, but our TX is dropped due to trying to emit duplicate nullifiers + await expect(votingContract.methods.cast_vote(candidate).send().wait()).rejects.toThrow( + 'Reason: Tx dropped by P2P node.', + ); }); }); });