diff --git a/docs/docs/aztec/concepts/accounts/keys.md b/docs/docs/aztec/concepts/accounts/keys.md index bb2c7340225..09a64e7e872 100644 --- a/docs/docs/aztec/concepts/accounts/keys.md +++ b/docs/docs/aztec/concepts/accounts/keys.md @@ -1,7 +1,5 @@ ---- title: Keys tags: [accounts, keys] ---- The goal of this section is to give app developer a good idea what keys there are used in the system. For a detailed description head over to the [protocol specification](../../../protocol-specs/addresses-and-keys/index.md). @@ -24,6 +22,7 @@ Instead it's up to the account contract developer to implement it. ::: ## Public keys retrieval + The keys can either be retrieved from a key registry contract or from the [Private eXecution Environment (PXE)](../pxe/index.md). :::note @@ -34,7 +33,7 @@ There is 1 key registry and its address is hardcoded in the protocol code. To retrieve them a developer can use one of the getters in Aztec.nr: -#include_code key-getters /noir-projects/aztec-nr/aztec/src/keys/getters.nr rust +#include_code key-getters /noir-projects/aztec-nr/aztec/src/keys/getters/mod.nr rust If the keys are registered in the key registry these methods can be called without any setup. If they are not there, it is necessary to first register the user as a recipient in our PXE. @@ -51,6 +50,7 @@ Then to register the recipient's [complete address](#complete-address) in PXE we During private function execution these keys are obtained via an oracle call from PXE. ## Key rotation + To prevent users from needing to migrate all their positions if some of their keys are leaked we allow for key rotation. Key rotation can be performed by calling the corresponding function on key registry. E.g. for nullifier key: @@ -62,6 +62,7 @@ This means that it will be possible to nullify the notes with the same old key a These guardrails are typically in place so a user should not lose her notes even if this unfortunate accident happens. ## Scoped keys + To minimize damage of potential key leaks the keys are scoped (also called app-siloed) to the contract that requests them. This means that the keys used for the same user in two different application contracts will be different and potential leak of the scoped keys would only affect 1 application. @@ -78,9 +79,11 @@ This is intentional and instead of directly trying to derive `Npk_m` from `nsk_a If you are curious how the derivation scheme works head over to [protocol specification](../../../protocol-specs/addresses-and-keys/example-usage/nullifier#diagram). ## Protocol key types + All the keys below are Grumpkin keys (public keys derived on the Grumpkin curve). ## Nullifier keys + Whenever a note is consumed, a nullifier deterministically derived from it is emitted. This mechanisms prevents double-spends, since nullifiers are checked by the protocol to be unique. Now, in order to preserve privacy, a third party should not be able to link a note hash to its nullifier - this link is enforced by the note implementation. @@ -95,15 +98,18 @@ Typically, `Npk_m` is stored in a note and later on, the note is nullified using Validity of `nsk_app` is verified by our [protocol kernel circuits](../../../protocol-specs/circuits/private-kernel-tail#verifying-and-splitting-ordered-data). ## Incoming viewing keys + The public key (denoted `Ivpk`) is used to encrypt a note for a recipient and the corresponding secret key (`ivsk`) is used by the recipient during decryption. ## Outgoing viewing keys + App-siloed versions of outgoing viewing keys are denoted `ovsk_app` and `Ovpk_app`. These keys are used to encrypt a note for a note sender which is necessary for reconstructing transaction history from on-chain data. For example, during a token transfer, the token contract may dictate that the sender encrypts the note with value with the recipient's `Ivpk`, but also records the transfer with its own `Ovpk_app` for bookkeeping purposes. If these keys were not used and a new device would be synched there would be no "direct" information available about notes that a user created for other people. ## Tagging keys + Used to compute tags in a [tagging note discovery scheme](../../../protocol-specs/private-message-delivery/private-msg-delivery#note-tagging). :::note @@ -152,6 +158,7 @@ Since there are no restrictions on the actions that an account contract may exec ### Complete address When deploying a contract, the contract address is deterministically derived using the following scheme: + ``` diff --git a/l1-contracts/src/core/libraries/ConstantsGen.sol b/l1-contracts/src/core/libraries/ConstantsGen.sol index a0863663b58..1983d5e2d22 100644 --- a/l1-contracts/src/core/libraries/ConstantsGen.sol +++ b/l1-contracts/src/core/libraries/ConstantsGen.sol @@ -127,7 +127,7 @@ library Constants { uint256 internal constant L2_GAS_PER_NOTE_HASH = 32; uint256 internal constant L2_GAS_PER_NULLIFIER = 64; uint256 internal constant CANONICAL_KEY_REGISTRY_ADDRESS = - 13457222047904330765774796260088567201269649167356521005501223652902339211182; + 21209182303070804160941065409360795406831433542792830301721453026531461944353; uint256 internal constant CANONICAL_AUTH_REGISTRY_ADDRESS = 16522644890256297179255458951626875692461008240031142745359776058397274208468; uint256 internal constant DEPLOYER_CONTRACT_ADDRESS = diff --git a/noir-projects/aztec-nr/aztec/src/context/unconstrained_context.nr b/noir-projects/aztec-nr/aztec/src/context/unconstrained_context.nr index 6df0103556a..c67cd74a3c3 100644 --- a/noir-projects/aztec-nr/aztec/src/context/unconstrained_context.nr +++ b/noir-projects/aztec-nr/aztec/src/context/unconstrained_context.nr @@ -31,6 +31,12 @@ impl UnconstrainedContext { Self { block_number, contract_address, version, chain_id } } + unconstrained fn at_historical(contract_address: AztecAddress, block_number: u32) -> Self { + let chain_id = get_chain_id(); + let version = get_version(); + Self { block_number, contract_address, version, chain_id } + } + fn block_number(self) -> u32 { self.block_number } 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 3a37b160a57..07356cbd252 100644 --- a/noir-projects/aztec-nr/aztec/src/history/public_storage.nr +++ b/noir-projects/aztec-nr/aztec/src/history/public_storage.nr @@ -10,7 +10,7 @@ trait PublicStorageHistoricalRead { fn public_storage_historical_read(header: Header, storage_slot: Field, contract_address: AztecAddress) -> Field; } -impl PublicStorageHistoricalRead for Header { +impl PublicStorageHistoricalRead for Header { fn public_storage_historical_read(self, storage_slot: Field, contract_address: AztecAddress) -> Field { // 1) Compute the leaf slot by siloing the storage slot with the contract address let public_data_tree_index = poseidon2_hash_with_separator( diff --git a/noir-projects/aztec-nr/aztec/src/keys/getters.nr b/noir-projects/aztec-nr/aztec/src/keys/getters.nr deleted file mode 100644 index 372e3a433a5..00000000000 --- a/noir-projects/aztec-nr/aztec/src/keys/getters.nr +++ /dev/null @@ -1,107 +0,0 @@ -use dep::protocol_types::{ - header::Header, abis::validation_requests::KeyValidationRequest, address::AztecAddress, - constants::CANONICAL_KEY_REGISTRY_ADDRESS, point::Point, storage::map::derive_storage_slot_in_map, - traits::is_empty -}; -use crate::{ - context::PrivateContext, - oracle::{keys::get_public_keys_and_partial_address, key_validation_request::get_key_validation_request}, - keys::{public_keys::PublicKeys, constants::{NULLIFIER_INDEX, INCOMING_INDEX, OUTGOING_INDEX, TAGGING_INDEX}}, - state_vars::{shared_mutable::shared_mutable_private_getter::SharedMutablePrivateGetter} -}; - -global DELAY = 5; - -// docs:start:key-getters -trait KeyGetters { - fn get_npk_m(header: Header, context: &mut PrivateContext, address: AztecAddress) -> Point; - fn get_ivpk_m(header: Header, context: &mut PrivateContext, address: AztecAddress) -> Point; - fn get_ovpk_m(header: Header, context: &mut PrivateContext, address: AztecAddress) -> Point; - fn get_tpk_m(header: Header, context: &mut PrivateContext, address: AztecAddress) -> Point; - fn get_npk_m_hash(header: Header, context: &mut PrivateContext, address: AztecAddress) -> Field; -} - -impl KeyGetters for Header { - fn get_npk_m(self, context: &mut PrivateContext, address: AztecAddress) -> Point { - get_master_key(context, address, NULLIFIER_INDEX, self) - } - - fn get_ivpk_m(self, context: &mut PrivateContext, address: AztecAddress) -> Point { - get_master_key(context, address, INCOMING_INDEX, self) - } - - fn get_ovpk_m(self, context: &mut PrivateContext, address: AztecAddress) -> Point { - get_master_key(context, address, OUTGOING_INDEX, self) - } - - fn get_tpk_m(self, context: &mut PrivateContext, address: AztecAddress) -> Point { - get_master_key(context, address, TAGGING_INDEX, self) - } - - fn get_npk_m_hash(self, context: &mut PrivateContext, address: AztecAddress) -> Field { - get_master_key(context, address, NULLIFIER_INDEX, self).hash() - } -} -// docs:end:key-getters - -fn get_master_key( - context: &mut PrivateContext, - address: AztecAddress, - key_index: Field, - header: Header -) -> Point { - let key = fetch_key_from_registry(context, key_index, address, header); - if is_empty(key) { - // Keys were not registered in registry yet --> fetch key from PXE - let keys = fetch_and_constrain_keys(address); - // Return the corresponding to index - keys.get_key_by_index(key_index) - } else { - // Keys were registered --> return the key - key - } -} - -fn fetch_key_from_registry( - context: &mut PrivateContext, - key_index: Field, - address: AztecAddress, - header: Header -) -> Point { - 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); - let y_coordinate_derived_slot = derive_storage_slot_in_map(y_coordinate_map_slot, address); - - let x_coordinate_registry: SharedMutablePrivateGetter = SharedMutablePrivateGetter::new( - context, - AztecAddress::from_field(CANONICAL_KEY_REGISTRY_ADDRESS), - x_coordinate_derived_slot - ); - let y_coordinate_registry: SharedMutablePrivateGetter = SharedMutablePrivateGetter::new( - context, - AztecAddress::from_field(CANONICAL_KEY_REGISTRY_ADDRESS), - y_coordinate_derived_slot - ); - let x_coordinate = x_coordinate_registry.get_value_in_private(header); - let y_coordinate = y_coordinate_registry.get_value_in_private(header); - - Point { x: x_coordinate, y: y_coordinate, is_infinite: false } -} - -// Passes only when keys were not rotated - is expected to be called only when keys were not registered yet -fn fetch_and_constrain_keys(address: AztecAddress) -> PublicKeys { - let (public_keys, partial_address) = get_public_keys_and_partial_address(address); - - let computed_address = AztecAddress::compute(public_keys.hash(), partial_address); - - assert(computed_address.eq(address)); - - public_keys -} - -// A helper function since requesting nsk_app is very common -// TODO(#6543) -pub fn get_nsk_app(npk_m_hash: Field) -> Field { - get_key_validation_request(npk_m_hash, NULLIFIER_INDEX).sk_app -} diff --git a/noir-projects/aztec-nr/aztec/src/keys/getters/mod.nr b/noir-projects/aztec-nr/aztec/src/keys/getters/mod.nr new file mode 100644 index 00000000000..3c53579109a --- /dev/null +++ b/noir-projects/aztec-nr/aztec/src/keys/getters/mod.nr @@ -0,0 +1,154 @@ +use dep::protocol_types::{ + header::Header, abis::validation_requests::KeyValidationRequest, address::AztecAddress, + constants::CANONICAL_KEY_REGISTRY_ADDRESS, point::Point, storage::map::derive_storage_slot_in_map, + traits::is_empty +}; +use crate::{ + context::{PrivateContext, UnconstrainedContext}, + oracle::{keys::get_public_keys_and_partial_address, key_validation_request::get_key_validation_request}, + keys::{ + public_keys::{PublicKeys, PUBLIC_KEYS_LENGTH}, stored_keys::StoredKeys, + constants::{NULLIFIER_INDEX, INCOMING_INDEX, OUTGOING_INDEX, TAGGING_INDEX} +}, + state_vars::{ + shared_mutable::shared_mutable_private_getter::SharedMutablePrivateGetter, + public_mutable::PublicMutable, map::Map +} +}; + +global DELAY = 5; + +mod test; + +// docs:start:key-getters +trait KeyGetters { + fn get_npk_m(header: Header, context: &mut PrivateContext, address: AztecAddress) -> Point; + fn get_ivpk_m(header: Header, context: &mut PrivateContext, address: AztecAddress) -> Point; + fn get_ovpk_m(header: Header, context: &mut PrivateContext, address: AztecAddress) -> Point; + fn get_tpk_m(header: Header, context: &mut PrivateContext, address: AztecAddress) -> Point; + fn get_npk_m_hash(header: Header, context: &mut PrivateContext, address: AztecAddress) -> Field; +} + +impl KeyGetters for Header { + fn get_npk_m(self, _context: &mut PrivateContext, address: AztecAddress) -> Point { + get_historical_public_keys(self, address).npk_m + } + + fn get_ivpk_m(self, _context: &mut PrivateContext, address: AztecAddress) -> Point { + get_historical_public_keys(self, address).ivpk_m + } + + fn get_ovpk_m(self, _context: &mut PrivateContext, address: AztecAddress) -> Point { + get_historical_public_keys(self, address).ovpk_m + } + + fn get_tpk_m(self, _context: &mut PrivateContext, address: AztecAddress) -> Point { + get_historical_public_keys(self, address).tpk_m + } + + fn get_npk_m_hash(self, context: &mut PrivateContext, address: AztecAddress) -> Field { + self.get_npk_m(context, address).hash() + } +} +// docs:end:key-getters + +// A helper function since requesting nsk_app is very common +// TODO(#6543) +pub fn get_nsk_app(npk_m_hash: Field) -> Field { + get_key_validation_request(npk_m_hash, NULLIFIER_INDEX).sk_app +} + +// This is the number of blocks that must pass after a key rotation event until the old keys are fully phased out and +// become invalid. +global KEY_REGISTRY_UPDATE_BLOCKS = 5; + +global KEY_REGISTRY_STORAGE_SLOT = 1; + +// Returns all current public keys for a given account, applying proper constraints to the context. We read all +// keys at once since the constraints for reading them all are actually fewer than if we read them one at a time - any +// read keys that are not required by the caller can simply be discarded. +pub fn get_current_public_keys(context: &mut PrivateContext, account: AztecAddress) -> PublicKeys { + // We're going to perform historical reads from public storage, and so need to constrain the caller so that they + // cannot use very old blocks when constructing proofs, and hence e.g. read very old keys. We are lax and allow + // _any_ recent block number to be used, regardless of whether there may have been a recent key rotation. This means + // that multiple sets of keys are valid for a while immediately after rotation, until the old keys become phased + // out. We *must* be lax to prevent denial of service and transaction fingerprinting attacks by accounts that rotate + // their keys frequently. + // Note that we constrain the max block number even if the registry ends up being empty: this ensures that proof of + // an empty registry is also fresh. + let current_header = context.get_header(); + context.set_tx_max_block_number(current_header.global_variables.block_number as u32 + KEY_REGISTRY_UPDATE_BLOCKS); + + get_historical_public_keys(current_header, account) +} + +// Returns historical public keys for a given account at some block determined by a block header. We read all keys at +// once since the constraints for reading them all are actually fewer than if we read them one at a time - any read keys +// that are not required by the caller can simply be discarded. +// WARNING: if called with a historical header created from a fixed block this function will explicitly ignore key +// rotation! This means that callers of this may force a user to use old keys, potentially leaking privacy (e.g. if the +// old keys were leaked). Only call this function with a header from a fixed block if you understand the implications of +// breaking key rotation very well. +pub fn get_historical_public_keys(historical_header: Header, account: AztecAddress) -> PublicKeys { + // TODO: improve this so that we always hint the correct set of keys (either registry or canonical) and hash them + // once instead of having two different hints and twice as many constraints due to the double hashing. + + // The key registry is the primary source of information for keys, as that's where accounts store their new keys + // when they perform rotation. The key registry conveniently stores a hash of each user's keys, so we can read that + // single field and then prove that we know its preimage (i.e. the current set of keys). + let key_registry_hash = key_registry_hash_public_historical_read(historical_header, account); + if key_registry_hash != 0 { + let hinted_registry_public_keys = key_registry_get_stored_keys_hint( + account, + historical_header.global_variables.block_number as u32 + ); + assert_eq(hinted_registry_public_keys.hash().to_field(), key_registry_hash); + + hinted_registry_public_keys + } else { + // If nothing was written to the registry, we may still be able to produce the correct keys if we happen to know + // the canonical set (i.e. the ones that are part of the account's preimage). + let (hinted_canonical_public_keys, partial_address) = get_public_keys_and_partial_address(account); + assert_eq( + account, AztecAddress::compute(hinted_canonical_public_keys.hash(), partial_address), "Invalid public keys hint for address" + ); + + hinted_canonical_public_keys + } +} + +fn key_registry_hash_public_historical_read(historical_header: Header, account: AztecAddress) -> Field { + // The keys are stored in a Map that is keyed with the address of each account, so we first derive the corresponding + // slot for this account. + let keys_storage_slot = derive_storage_slot_in_map(KEY_REGISTRY_STORAGE_SLOT, account); + + // The keys are stored as [ ...serialized_keys, hash ], and since arrays get allocated sequential storage slots + // (prior to siloing!), we simply add the length to the base slot to get the last element. + let hash_storage_slot = keys_storage_slot + PUBLIC_KEYS_LENGTH as Field; + + historical_header.public_storage_historical_read( + hash_storage_slot, + AztecAddress::from_field(CANONICAL_KEY_REGISTRY_ADDRESS) + ) +} + +unconstrained fn key_registry_get_stored_keys_hint(account: AztecAddress, block_number: u32) -> PublicKeys { + // This is equivalent to the key registry contract having an unconstrained getter that we call from an oracle, but + // PXE does not yet support that functionality so we do this manually instad. Note that this would be a *historical* + // call! + + // TODO (#7524): call the unconstrained KeyRegistry.get_current_keys() function instead + + let context = UnconstrainedContext::at_historical( + AztecAddress::from_field(CANONICAL_KEY_REGISTRY_ADDRESS), + block_number + ); + let keys_storage = Map::new( + context, + KEY_REGISTRY_STORAGE_SLOT, + |context, slot| { PublicMutable::new(context, slot) } + ); + + let stored_keys: StoredKeys = keys_storage.at(account).read(); + stored_keys.public_keys +} diff --git a/noir-projects/aztec-nr/aztec/src/keys/getters/test.nr b/noir-projects/aztec-nr/aztec/src/keys/getters/test.nr new file mode 100644 index 00000000000..41534e0de72 --- /dev/null +++ b/noir-projects/aztec-nr/aztec/src/keys/getters/test.nr @@ -0,0 +1,58 @@ +use crate::keys::getters::{get_current_public_keys, get_historical_public_keys, KEY_REGISTRY_UPDATE_BLOCKS}; +use crate::context::PrivateContext; +use dep::protocol_types::address::AztecAddress; + +use crate::test::helpers::{cheatcodes, test_environment::TestEnvironment, utils::TestAccount}; +use dep::std::test::OracleMock; + +global KEY_ORACLE_RESPONSE_LENGTH = 13; // 12 fields for the keys, one field for the partial address + +fn setup() -> (TestEnvironment, PrivateContext, TestAccount) { + let mut env = TestEnvironment::new(); + let account = cheatcodes::create_account(); + + let historical_block_number = env.block_number(); + let context = env.private_at(historical_block_number); + + (env, context, account) +} + +#[test(should_fail_with="Invalid public keys hint for address")] +fn test_get_current_keys_unknown_unregistered() { + let (_, context, account) = setup(); + + let _ = OracleMock::mock("getPublicKeysAndPartialAddress").returns([0; KEY_ORACLE_RESPONSE_LENGTH]).times(1); + let _ = get_current_public_keys(&mut context, account.address); +} + +#[test(should_fail_with="Invalid public keys hint for address")] +fn test_get_historical_keys_unknown_unregistered() { + let (_, context, account) = setup(); + let historical_header = context.get_header(); + + let _ = OracleMock::mock("getPublicKeysAndPartialAddress").returns([0; KEY_ORACLE_RESPONSE_LENGTH]).times(1); + let _ = get_historical_public_keys(historical_header, account.address); +} + +#[test] +fn test_get_current_keys_known_unregistered() { + let (_, mut context, account) = setup(); + + let current_keys = get_current_public_keys(&mut context, account.address); + + assert_eq(current_keys, account.keys); + assert_eq( + context.max_block_number.unwrap(), context.historical_header.global_variables.block_number as u32 + KEY_REGISTRY_UPDATE_BLOCKS + ); +} + +#[test] +fn test_get_historical_keys_known_unregistered() { + let (_, context, account) = setup(); + + let historical_header = context.get_header(); + + let historical_keys = get_historical_public_keys(historical_header, account.address); + assert_eq(historical_keys, account.keys); + assert(context.max_block_number.is_none()); +} diff --git a/noir-projects/aztec-nr/aztec/src/keys/mod.nr b/noir-projects/aztec-nr/aztec/src/keys/mod.nr index f55eaaa23ea..485ab4a5b2c 100644 --- a/noir-projects/aztec-nr/aztec/src/keys/mod.nr +++ b/noir-projects/aztec-nr/aztec/src/keys/mod.nr @@ -2,5 +2,6 @@ mod constants; mod getters; mod point_to_symmetric_key; mod public_keys; +mod stored_keys; use crate::keys::public_keys::{PublicKeys, PUBLIC_KEYS_LENGTH}; diff --git a/noir-projects/aztec-nr/aztec/src/keys/public_keys.nr b/noir-projects/aztec-nr/aztec/src/keys/public_keys.nr index 826029605a5..4dff5713fca 100644 --- a/noir-projects/aztec-nr/aztec/src/keys/public_keys.nr +++ b/noir-projects/aztec-nr/aztec/src/keys/public_keys.nr @@ -4,7 +4,7 @@ use dep::protocol_types::{ }; use crate::keys::constants::{NUM_KEY_TYPES, NULLIFIER_INDEX, INCOMING_INDEX, OUTGOING_INDEX}; -global PUBLIC_KEYS_LENGTH = 12; +global PUBLIC_KEYS_LENGTH: u32 = 12; struct PublicKeys { npk_m: Point, diff --git a/noir-projects/aztec-nr/aztec/src/keys/stored_keys.nr b/noir-projects/aztec-nr/aztec/src/keys/stored_keys.nr new file mode 100644 index 00000000000..20111f6e795 --- /dev/null +++ b/noir-projects/aztec-nr/aztec/src/keys/stored_keys.nr @@ -0,0 +1,59 @@ +use crate::keys::public_keys::{PublicKeys, PUBLIC_KEYS_LENGTH}; +use dep::protocol_types::traits::{Serialize, Deserialize}; + +// This struct represents how public keys are stored in the key registry. We store not just the keys themselves but also +// their hash, so that when reading in private we can perform a historical read for the hash and then show that it +// corresponds to a preimage obtained from an unconstrained hint. We do store the keys keys regardless as they might be +// needed during public execution, and since we need to broadcast and produce hints in some standardized way. +// While it might seem odd to create a struct for what is effectively some data and a pure function called on it, state +// variables rely on serializable structs in order to persist data to storage, so we must use this abstraction. +struct StoredKeys { + public_keys: PublicKeys, + hash: Field, +} + +impl StoredKeys { + // Instances of StoredKeys are expected to only be created by calling this function so that we guarantee that the + // hash field does indeed correspond to the hash of the keys. Ideally we'd forbid direct access to the struct, but + // Noir doesn't yet support private members. + fn new(public_keys: PublicKeys) -> Self { + Self { public_keys, hash: public_keys.hash().inner } + } +} + +// Our serialization is the concatenation of the public keys serialization plush the hash, so we need one extra field. +global STORED_KEYS_LENGTH: u32 = PUBLIC_KEYS_LENGTH + 1; + +impl Serialize for StoredKeys { + fn serialize(self) -> [Field; STORED_KEYS_LENGTH] { + // The code below is equivalent to: + // [ ...self.public_keys.serialize(), self.hash ] + + let mut array = [0; STORED_KEYS_LENGTH]; + + let serialized_keys = self.public_keys.serialize(); + for i in 0..serialized_keys.len() { + array[i] = serialized_keys[i]; + } + + array[PUBLIC_KEYS_LENGTH] = self.hash; + + array + } +} + +impl Deserialize for StoredKeys { + fn deserialize(array: [Field; STORED_KEYS_LENGTH]) -> Self { + // The code below is equivalent to: + // Self { public_keys: PublicKeys::deserialize(array[0 : PUBLIC_KEYS_LENGTH]), hash: array[PUBLIC_KEYS_LENGTH] } + + let mut serialized_keys = [0; PUBLIC_KEYS_LENGTH]; + for i in 0..serialized_keys.len() { + serialized_keys[i] = array[i]; + } + + let hash = array[PUBLIC_KEYS_LENGTH]; + + Self { public_keys: PublicKeys::deserialize(serialized_keys), hash } + } +} diff --git a/noir-projects/noir-contracts/Nargo.toml b/noir-projects/noir-contracts/Nargo.toml index e94fde5b221..6bbe50afe66 100644 --- a/noir-projects/noir-contracts/Nargo.toml +++ b/noir-projects/noir-contracts/Nargo.toml @@ -26,6 +26,7 @@ members = [ "contracts/fee_juice_contract", "contracts/import_test_contract", "contracts/key_registry_contract", + "contracts/new_key_registry_contract", "contracts/inclusion_proofs_contract", "contracts/lending_contract", "contracts/parent_contract", diff --git a/noir-projects/noir-contracts/contracts/new_key_registry_contract/Nargo.toml b/noir-projects/noir-contracts/contracts/new_key_registry_contract/Nargo.toml new file mode 100644 index 00000000000..6486c235e1a --- /dev/null +++ b/noir-projects/noir-contracts/contracts/new_key_registry_contract/Nargo.toml @@ -0,0 +1,9 @@ +[package] +name = "new_key_registry_contract" +authors = [""] +compiler_version = ">=0.25.0" +type = "contract" + +[dependencies] +aztec = { path = "../../../aztec-nr/aztec" } +authwit = { path = "../../../aztec-nr/authwit" } diff --git a/noir-projects/noir-contracts/contracts/new_key_registry_contract/src/main.nr b/noir-projects/noir-contracts/contracts/new_key_registry_contract/src/main.nr new file mode 100644 index 00000000000..9a4744d2c81 --- /dev/null +++ b/noir-projects/noir-contracts/contracts/new_key_registry_contract/src/main.nr @@ -0,0 +1,63 @@ +contract NewKeyRegistry { + use dep::authwit::auth::assert_current_call_valid_authwit_public; + + use dep::aztec::{ + keys::{PublicKeys, stored_keys::StoredKeys}, state_vars::{PublicMutable, Map}, + protocol_types::{point::Point, address::{AztecAddress, PartialAddress}} + }; + + #[aztec(storage)] + struct Storage { + current_keys: Map>, + } + + impl Storage { + // The init function is typically automatically generated by the macros - here we implement it manually in order + // to have control over which storage slot is assigned to the current_keys state variable. + fn init(context: Context) -> Self { + Storage { + // Ideally we'd do KEY_REGISTRY_STORAGE_SLOT instead of hardcoding the 1 here, but that is currently + // causing compilation errors. + // TODO(#7829): fix this + current_keys: Map::new( + context, + 1, + |context, slot| { PublicMutable::new(context, slot) } + ) + } + } + } + + unconstrained fn get_current_keys(account: AztecAddress) -> pub PublicKeys { + // If #7524 were to be implemented, this function could be called by an oracle from an unconstrained function + // in order to produce the preimage of the stored hash, and hence prove the correctness of the keys. + storage.current_keys.at(account).read().public_keys + } + + #[aztec(public)] + fn register_initial_keys(account: AztecAddress, partial_address: PartialAddress, keys: PublicKeys) { + let computed_address = AztecAddress::compute(keys.hash(), partial_address); + assert(computed_address.eq(account), "Computed address does not match supplied address"); + + storage.current_keys.at(account).write(StoredKeys::new(keys)); + } + + #[aztec(public)] + fn rotate_npk_m(account: AztecAddress, new_npk_m: Point, nonce: Field) { + if (!account.eq(context.msg_sender())) { + assert_current_call_valid_authwit_public(&mut context, account); + } else { + assert(nonce == 0, "invalid nonce"); + } + + let account_key_storage = storage.current_keys.at(account); + + // We read all other current keys so that we can compute the new hash - we can't update just the npk. This means + // updating all keys at once costs the same as updating just one (unless setting public storage to its current + // value is cheaper than changing it, e.g. EIP-2200). + let mut current_keys = account_key_storage.read().public_keys; + current_keys.npk_m = new_npk_m; + + account_key_storage.write(StoredKeys::new(current_keys)); + } +} diff --git a/noir-projects/noir-contracts/contracts/test_contract/src/main.nr b/noir-projects/noir-contracts/contracts/test_contract/src/main.nr index 27a2f6cd6b8..e021ea991e3 100644 --- a/noir-projects/noir-contracts/contracts/test_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/test_contract/src/main.nr @@ -501,25 +501,6 @@ contract Test { ret.to_field() } - // This function is used for testing the registry contract and fresh public key getters. If nothing exists in the registry, but we have added public - // keys to the pxe, this function will return nothing, but the public key getters will return the correct value - #[aztec(private)] - fn test_shared_mutable_private_getter_for_registry_contract( - storage_slot_of_shared_mutable: Field, - address_to_get_in_registry: AztecAddress - ) -> Field { - // We have to derive this slot to get the location of the shared mutable inside the Map - let derived_slot = derive_storage_slot_in_map(storage_slot_of_shared_mutable, address_to_get_in_registry); - - // It's a bit wonky because we need to know the delay for get_current_value_in_private to work correctly - let registry_private_getter: SharedMutablePrivateGetter = SharedMutablePrivateGetter::new( - &mut context, - AztecAddress::from_field(CANONICAL_KEY_REGISTRY_ADDRESS), - derived_slot - ); - registry_private_getter.get_value_in_private(context.get_header()) - } - #[aztec(private)] fn test_nullifier_key_freshness(address: AztecAddress, public_nullifying_key: Point) { assert_eq(context.get_header().get_npk_m(&mut context, address), public_nullifying_key); @@ -558,7 +539,7 @@ contract Test { many_notes: [DummyNote; 3], } - // Serializing using "canonical" form. + // Serializing using "canonical" form. // 1. Everything that fits in a field, *becomes* a Field // 2. Strings become arrays of bytes (no strings here) // 4. Arrays become arrays of Fields following rules 2 and 3 (no arrays here) 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 e0c3aa6de23..9c2aaa1c480 100644 --- a/noir-projects/noir-protocol-circuits/crates/types/src/constants.nr +++ b/noir-projects/noir-protocol-circuits/crates/types/src/constants.nr @@ -178,7 +178,7 @@ global L2_GAS_PER_NOTE_HASH: u32 = 32; global L2_GAS_PER_NULLIFIER: u32 = 64; // CANONICAL CONTRACT ADDRESSES -global CANONICAL_KEY_REGISTRY_ADDRESS = 0x1dc0848be99ba522c157b46ab5ed64d86703a74e7df8fe1b3a82f45e2dbdefae; +global CANONICAL_KEY_REGISTRY_ADDRESS = 0x2ee3f8c67efa88f9e6fb44242f1e9dcc0f9a6752ded07af0d9fac3875a61d421; global CANONICAL_AUTH_REGISTRY_ADDRESS = 0x24877c50868f86712240eb535d90d1c97403d074805dd3758c3aecb02958f8d4; global DEPLOYER_CONTRACT_ADDRESS = 0x2ab1a2bd6d07d8d61ea56d85861446349e52c6b7c0612b702cb1e6db6ad0b089; global REGISTERER_CONTRACT_ADDRESS = 0x05d15342d76e46e5be07d3cda0d753158431cdc5e39d29ce4e8fe1f5c070564a; diff --git a/yarn-project/circuits.js/src/constants.gen.ts b/yarn-project/circuits.js/src/constants.gen.ts index 693fbb373b0..e28d7e01cae 100644 --- a/yarn-project/circuits.js/src/constants.gen.ts +++ b/yarn-project/circuits.js/src/constants.gen.ts @@ -113,7 +113,7 @@ export const L2_GAS_PER_LOG_BYTE = 4; export const L2_GAS_PER_NOTE_HASH = 32; export const L2_GAS_PER_NULLIFIER = 64; export const CANONICAL_KEY_REGISTRY_ADDRESS = - 13457222047904330765774796260088567201269649167356521005501223652902339211182n; + 21209182303070804160941065409360795406831433542792830301721453026531461944353n; export const CANONICAL_AUTH_REGISTRY_ADDRESS = 16522644890256297179255458951626875692461008240031142745359776058397274208468n; export const DEPLOYER_CONTRACT_ADDRESS = 19310994760783330368337163480198602393920956587162708699802190083077641908361n; diff --git a/yarn-project/circuits.js/src/contract/artifact_hash.ts b/yarn-project/circuits.js/src/contract/artifact_hash.ts index a5d6f7673af..96bafbc6875 100644 --- a/yarn-project/circuits.js/src/contract/artifact_hash.ts +++ b/yarn-project/circuits.js/src/contract/artifact_hash.ts @@ -68,7 +68,7 @@ export function computeArtifactMetadataHash(artifact: ContractArtifact) { const exceptions: string[] = [ 'AuthRegistry', - 'KeyRegistry', + 'NewKeyRegistry', 'FeeJuice', 'ContractInstanceDeployer', 'ContractClassRegisterer', diff --git a/yarn-project/cli/src/cmds/misc/deploy_contracts.ts b/yarn-project/cli/src/cmds/misc/deploy_contracts.ts index 8aeaed0089e..32196437bf8 100644 --- a/yarn-project/cli/src/cmds/misc/deploy_contracts.ts +++ b/yarn-project/cli/src/cmds/misc/deploy_contracts.ts @@ -62,7 +62,7 @@ export async function deployCanonicalL2FeeJuice( export async function deployCanonicalKeyRegistry(deployer: Wallet, waitOpts = DefaultWaitOpts): Promise { // eslint-disable-next-line @typescript-eslint/ban-ts-comment // @ts-ignore - Importing noir-contracts.js even in devDeps results in a circular dependency error. Need to ignore because this line doesn't cause an error in a dev environment - const { KeyRegistryContract } = await import('@aztec/noir-contracts.js'); + const { NewKeyRegistryContract: KeyRegistryContract } = await import('@aztec/noir-contracts.js'); const canonicalKeyRegistry = getCanonicalKeyRegistry(); diff --git a/yarn-project/end-to-end/src/e2e_key_registry.test.ts b/yarn-project/end-to-end/src/e2e_key_registry.test.ts index 251ba141aee..f98c3605b84 100644 --- a/yarn-project/end-to-end/src/e2e_key_registry.test.ts +++ b/yarn-project/end-to-end/src/e2e_key_registry.test.ts @@ -1,6 +1,6 @@ import { type AccountWallet, AztecAddress, Fr, type PXE } from '@aztec/aztec.js'; import { CompleteAddress, Point, PublicKeys } from '@aztec/circuits.js'; -import { KeyRegistryContract, TestContract } from '@aztec/noir-contracts.js'; +import { NewKeyRegistryContract, TestContract } from '@aztec/noir-contracts.js'; import { getCanonicalKeyRegistryAddress } from '@aztec/protocol-contracts/key-registry'; import { jest } from '@jest/globals'; @@ -9,10 +9,8 @@ import { publicDeployAccounts, setup } from './fixtures/utils.js'; const TIMEOUT = 120_000; -const SHARED_MUTABLE_DELAY = 5; - describe('Key Registry', () => { - let keyRegistry: KeyRegistryContract; + let keyRegistry: NewKeyRegistryContract; let pxe: PXE; let testContract: TestContract; @@ -26,20 +24,13 @@ describe('Key Registry', () => { beforeAll(async () => { ({ teardown, pxe, wallets } = await setup(2)); - keyRegistry = await KeyRegistryContract.at(getCanonicalKeyRegistryAddress(), wallets[0]); + keyRegistry = await NewKeyRegistryContract.at(getCanonicalKeyRegistryAddress(), wallets[0]); testContract = await TestContract.deploy(wallets[0]).send().deployed(); await publicDeployAccounts(wallets[0], wallets.slice(0, 2)); }); - 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(); - } - }; - afterAll(() => teardown()); describe('failure cases', () => { @@ -60,7 +51,7 @@ describe('Key Registry', () => { await expect( keyRegistry .withWallet(wallets[0]) - .methods.register_npk_and_ivpk( + .methods.register_initial_keys( account, account.partialAddress, // TODO(#6337): Make calling `toNoirStruct()` unnecessary @@ -113,18 +104,7 @@ describe('Key Registry', () => { it('registers', async () => { await keyRegistry .withWallet(wallets[0]) - .methods.register_npk_and_ivpk( - account, - account.partialAddress, - // TODO(#6337): Make calling `toNoirStruct()` unnecessary - account.publicKeys.toNoirStruct(), - ) - .send() - .wait(); - - await keyRegistry - .withWallet(wallets[0]) - .methods.register_ovpk_and_tpk( + .methods.register_initial_keys( account, account.partialAddress, // TODO(#6337): Make calling `toNoirStruct()` unnecessary @@ -133,26 +113,6 @@ describe('Key Registry', () => { .send() .wait(); - // We check if our registered nullifier key is equal to the key obtained from the getter by - // reading our registry contract from the test contract. We expect this to fail because the change has not been applied yet - const emptyNullifierPublicKeyX = await testContract.methods - .test_shared_mutable_private_getter_for_registry_contract(1, account) - .simulate(); - - expect(new Fr(emptyNullifierPublicKeyX)).toEqual(Fr.ZERO); - - // We check it again after a delay and expect that the change has been applied and consequently the assert is true - await crossDelay(); - - const nullifierPublicKeyX = await testContract.methods - .test_shared_mutable_private_getter_for_registry_contract(1, account) - .simulate(); - - expect(new Fr(nullifierPublicKeyX)).toEqual(account.publicKeys.masterNullifierPublicKey.x); - }); - - // Note: This test case is dependent on state from the previous one - it('key lib succeeds for registered account', async () => { // Should succeed as the account is registered in key registry from tests before await testContract.methods .test_nullifier_key_freshness(account, account.publicKeys.masterNullifierPublicKey.toNoirStruct()) @@ -174,22 +134,10 @@ describe('Key Registry', () => { .wait(); // docs:end:key-rotation - // We check if our rotated nullifier key is equal to the key obtained from the getter by reading our registry - // contract from the test contract. We expect this to fail because the change has not been applied yet - const emptyNullifierPublicKeyX = await testContract.methods - .test_shared_mutable_private_getter_for_registry_contract(1, wallets[0].getAddress()) - .simulate(); - - expect(new Fr(emptyNullifierPublicKeyX)).toEqual(Fr.ZERO); - - // We check it again after a delay and expect that the change has been applied and consequently the assert is true - await crossDelay(); - - const nullifierPublicKeyX = await testContract.methods - .test_shared_mutable_private_getter_for_registry_contract(1, wallets[0].getAddress()) - .simulate(); - - expect(new Fr(nullifierPublicKeyX)).toEqual(firstNewMasterNullifierPublicKey.x); + await testContract.methods + .test_nullifier_key_freshness(wallets[0].getAddress(), firstNewMasterNullifierPublicKey.toNoirStruct()) + .send() + .wait(); }); it(`rotates npk_m with authwit`, async () => { @@ -204,25 +152,6 @@ describe('Key Registry', () => { await action.send().wait(); - // We get the old nullifier key as the change has not been applied yet - const oldNullifierPublicKeyX = await testContract.methods - .test_shared_mutable_private_getter_for_registry_contract(1, wallets[0].getAddress()) - .simulate(); - - expect(new Fr(oldNullifierPublicKeyX)).toEqual(firstNewMasterNullifierPublicKey.x); - - await crossDelay(); - - // We get the new nullifier key as the change has been applied - const newNullifierPublicKeyX = await testContract.methods - .test_shared_mutable_private_getter_for_registry_contract(1, wallets[0].getAddress()) - .simulate(); - - expect(new Fr(newNullifierPublicKeyX)).toEqual(secondNewMasterNullifierPublicKey.x); - }); - - it('fresh key lib gets new key after rotation', async () => { - // Change has been applied hence should succeed now await testContract.methods .test_nullifier_key_freshness(wallets[0].getAddress(), secondNewMasterNullifierPublicKey.toNoirStruct()) .send() 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 72b655446c0..9345caef70f 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 @@ -42,11 +42,13 @@ describe('e2e_voting_contract', () => { await votingContract.methods.cast_vote(candidate).send().wait(); expect(await votingContract.methods.get_vote(candidate).simulate()).toBe(1n); - // We rotate our nullifier keys + // We rotate our nullifier keys - this should be ignored by the voting contract, since it should always use the + // same set of keys to prevent double spends. await wallet.rotateNullifierKeys(); await crossDelay(); - // We try voting again, but our TX is dropped due to trying to emit duplicate nullifiers + // We try voting again, but our TX is dropped due to trying to emit duplicate nullifiers as the voting contract + // ignored our previous key rotation. await expect(votingContract.methods.cast_vote(candidate).send().wait()).rejects.toThrow( 'Reason: Tx dropped by P2P node.', ); diff --git a/yarn-project/end-to-end/src/fixtures/utils.ts b/yarn-project/end-to-end/src/fixtures/utils.ts index 502085a3138..015222b0d3d 100644 --- a/yarn-project/end-to-end/src/fixtures/utils.ts +++ b/yarn-project/end-to-end/src/fixtures/utils.ts @@ -55,7 +55,7 @@ import { RollupAbi, RollupBytecode, } from '@aztec/l1-artifacts'; -import { AuthRegistryContract, KeyRegistryContract } from '@aztec/noir-contracts.js'; +import { AuthRegistryContract, NewKeyRegistryContract } from '@aztec/noir-contracts.js'; import { FeeJuiceContract } from '@aztec/noir-contracts.js/FeeJuice'; import { getVKTreeRoot } from '@aztec/noir-protocol-circuits-types'; import { getCanonicalAuthRegistry } from '@aztec/protocol-contracts/auth-registry'; @@ -675,7 +675,7 @@ export async function deployCanonicalKeyRegistry(deployer: Wallet) { return; } - const keyRegistry = await KeyRegistryContract.deploy(deployer) + const keyRegistry = await NewKeyRegistryContract.deploy(deployer) .send({ contractAddressSalt: canonicalKeyRegistry.instance.salt, universalDeploy: true }) .deployed(); diff --git a/yarn-project/protocol-contracts/scripts/copy-contracts.sh b/yarn-project/protocol-contracts/scripts/copy-contracts.sh index d9f4a60a25c..ca6cba229d5 100755 --- a/yarn-project/protocol-contracts/scripts/copy-contracts.sh +++ b/yarn-project/protocol-contracts/scripts/copy-contracts.sh @@ -6,7 +6,7 @@ contracts=( contract_class_registerer_contract-ContractClassRegisterer contract_instance_deployer_contract-ContractInstanceDeployer fee_juice_contract-FeeJuice - key_registry_contract-KeyRegistry + new_key_registry_contract-NewKeyRegistry auth_registry_contract-AuthRegistry multi_call_entrypoint_contract-MultiCallEntrypoint ) diff --git a/yarn-project/protocol-contracts/src/key-registry/artifact.ts b/yarn-project/protocol-contracts/src/key-registry/artifact.ts index 5feb280a624..72bbe2ebe82 100644 --- a/yarn-project/protocol-contracts/src/key-registry/artifact.ts +++ b/yarn-project/protocol-contracts/src/key-registry/artifact.ts @@ -1,6 +1,6 @@ import { loadContractArtifact } from '@aztec/types/abi'; import { type NoirCompiledContract } from '@aztec/types/noir'; -import KeyRegistryJson from '../../artifacts/KeyRegistry.json' assert { type: 'json' }; +import NewKeyRegistryJson from '../../artifacts/NewKeyRegistry.json' assert { type: 'json' }; -export const KeyRegistryArtifact = loadContractArtifact(KeyRegistryJson as NoirCompiledContract); +export const KeyRegistryArtifact = loadContractArtifact(NewKeyRegistryJson as NoirCompiledContract);