From a95a10f32f5ffa347dd570d44ee8bea3fd5c5e93 Mon Sep 17 00:00:00 2001 From: benesjan Date: Fri, 3 May 2024 12:08:32 +0000 Subject: [PATCH 01/13] WIP --- noir-projects/aztec-nr/aztec/src/keys.nr | 2 +- .../aztec-nr/aztec/src/keys/getters.nr | 135 +++++++++------- .../key_registry_contract/src/main.nr | 151 ++++++++---------- .../contracts/test_contract/src/main.nr | 4 +- 4 files changed, 140 insertions(+), 152 deletions(-) diff --git a/noir-projects/aztec-nr/aztec/src/keys.nr b/noir-projects/aztec-nr/aztec/src/keys.nr index 0a79c4a024a5..999bd984de00 100644 --- a/noir-projects/aztec-nr/aztec/src/keys.nr +++ b/noir-projects/aztec-nr/aztec/src/keys.nr @@ -1,4 +1,4 @@ mod getters; mod point_to_symmetric_key; -use crate::keys::getters::get_fresh_nullifier_public_key_hash; +use crate::keys::getters::{get_npk_m, get_ivpk_m, get_ovpk_m, get_tpk_m}; diff --git a/noir-projects/aztec-nr/aztec/src/keys/getters.nr b/noir-projects/aztec-nr/aztec/src/keys/getters.nr index bbc0eb2dda85..171f2d61579f 100644 --- a/noir-projects/aztec-nr/aztec/src/keys/getters.nr +++ b/noir-projects/aztec-nr/aztec/src/keys/getters.nr @@ -1,79 +1,90 @@ -use dep::protocol_types::{ - address::{ - AztecAddress, - PartialAddress - }, - constants::{ - GENERATOR_INDEX__PUBLIC_KEYS_HASH, - GENERATOR_INDEX__CONTRACT_ADDRESS_V1, - CANONICAL_KEY_REGISTRY_ADDRESS - }, - grumpkin_point::GrumpkinPoint, +use dep::protocol_types::{address::AztecAddress, constants::CANONICAL_KEY_REGISTRY_ADDRESS, grumpkin_point::GrumpkinPoint}; +use crate::{ + context::PrivateContext, hash::poseidon2_hash, oracle::keys::get_public_keys_and_partial_address, + state_vars::{ + map::derive_storage_slot_in_map, + shared_mutable::shared_mutable_private_getter::SharedMutablePrivateGetter +} }; -use crate::context::PrivateContext; -use crate::hash::{ - pedersen_hash, - poseidon2_hash, -}; -use crate::oracle::keys::get_public_keys_and_partial_address; -use crate::state_vars::{ - map::derive_storage_slot_in_map, - shared_mutable::shared_mutable_private_getter::SharedMutablePrivateGetter, -}; +// Note: In fetch_hash_from_registry we expect that shared mutable slow is index + 1 (e.g. NULLIFIER_INDEX + 1), +// this changes the function will need to be refactored +global NULLIFIER_INDEX = 0; +global INCOMING_INDEX = 1; +global OUTGOING_INDEX = 2; +global TAGGING_INDEX = 3; -struct PublicKeyTypeEnum { - NULLIFIER: u8, +global DELAY = 5; + +pub fn get_npk_m(context: &mut PrivateContext, address: AztecAddress) -> GrumpkinPoint { + get_master_key(context, address, NULLIFIER_INDEX) } -global PublicKeyType = PublicKeyTypeEnum { - NULLIFIER: 0, -}; +pub fn get_ivpk_m(context: &mut PrivateContext, address: AztecAddress) -> GrumpkinPoint { + get_master_key(context, address, INCOMING_INDEX) +} -pub fn get_fresh_nullifier_public_key_hash( - context: &mut PrivateContext, - address: AztecAddress, -) -> Field { - // This is the storage slot of the nullifier_public_key inside the key registry contract - // TODO: (#6133) We should have this be directly imported from the other contract if possible, or at least this should not be this brittle - let storage_slot_of_nullifier_public_key = 1; +pub fn get_ovpk_m(context: &mut PrivateContext, address: AztecAddress) -> GrumpkinPoint { + get_master_key(context, address, OUTGOING_INDEX) +} - let derived_slot = derive_storage_slot_in_map(storage_slot_of_nullifier_public_key, address); +pub fn get_tpk_m(context: &mut PrivateContext, address: AztecAddress) -> GrumpkinPoint { + get_master_key(context, address, TAGGING_INDEX) +} - // We read from the canonical Key Registry - // TODO: (#6134) It's a bit wonky because we need to know the delay for get_current_value_in_private to work correctly. - // We should allow for this usecase without needing to hard code it here. - let registry_private_getter: SharedMutablePrivateGetter = SharedMutablePrivateGetter::new(*context, AztecAddress::from_field(CANONICAL_KEY_REGISTRY_ADDRESS), derived_slot); - let nullifier_public_key_hash_in_registry = registry_private_getter.get_current_value_in_private(); +fn get_master_key( + context: &mut PrivateContext, + address: AztecAddress, + key_index: Field +) -> GrumpkinPoint { + // 1. Get key hash from registry + let key_hash = fetch_hash_from_registry(context, key_index, address); + if key_hash == 0 { + // i. Keys were not registered yet --> fetch address preimage and check that the hash matches + fetch_and_constrain_keys(address)[key_index] + } else { + // ii. Keys were registered --> we fetch the key from oracle, hash it and check that it matches the hash in the registry + let (keys, _) = get_public_keys_and_partial_address(address); + let key = keys[key_index]; + let key_hash_from_oracle = poseidon2_hash(key.serialize()); + assert(key_hash_from_oracle == key_hash); + key + } +} - let nullifier_public_key_hash = if nullifier_public_key_hash_in_registry == 0 { - let keys = get_original_public_keys_internal(address); - poseidon2_hash(keys[PublicKeyType.NULLIFIER].serialize()) - } else { - nullifier_public_key_hash_in_registry - }; +fn fetch_hash_from_registry( + context: &mut PrivateContext, + key_index: Field, + address: AztecAddress +) -> Field { + let derived_slot = derive_storage_slot_in_map(key_index + 1, address); - nullifier_public_key_hash + let registry_private_getter: SharedMutablePrivateGetter = SharedMutablePrivateGetter::new( + *context, + AztecAddress::from_field(CANONICAL_KEY_REGISTRY_ADDRESS), + derived_slot + ); + registry_private_getter.get_current_value_in_private() } -// This constraint only works on keys that have not been rotated, otherwise this call will fail as the public keys are not constrained -fn get_original_public_keys_internal(address: AztecAddress) -> [GrumpkinPoint; 4] { - let (public_keys, partial_address) = get_public_keys_and_partial_address(address); +// 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) -> [GrumpkinPoint; 4] { + let (public_keys, partial_address) = get_public_keys_and_partial_address(address); - let nullifier_pub_key = public_keys[0]; - let incoming_pub_key = public_keys[1]; - let outgoing_pub_key = public_keys[2]; - let tagging_pub_key = public_keys[3]; + let nullifier_pub_key = public_keys[0]; + let incoming_pub_key = public_keys[1]; + let outgoing_pub_key = public_keys[2]; + let tagging_pub_key = public_keys[3]; - let computed_address = AztecAddress::compute_from_public_keys_and_partial_address( - nullifier_pub_key, - incoming_pub_key, - outgoing_pub_key, - tagging_pub_key, - partial_address, - ); + let computed_address = AztecAddress::compute_from_public_keys_and_partial_address( + nullifier_pub_key, + incoming_pub_key, + outgoing_pub_key, + tagging_pub_key, + partial_address + ); - assert(computed_address.eq(address)); + assert(computed_address.eq(address)); - [nullifier_pub_key, incoming_pub_key, outgoing_pub_key, tagging_pub_key] + [nullifier_pub_key, incoming_pub_key, outgoing_pub_key, tagging_pub_key] } diff --git a/noir-projects/noir-contracts/contracts/key_registry_contract/src/main.nr b/noir-projects/noir-contracts/contracts/key_registry_contract/src/main.nr index 2d2111d07ddc..1b60d2d2bd79 100644 --- a/noir-projects/noir-contracts/contracts/key_registry_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/key_registry_contract/src/main.nr @@ -1,104 +1,81 @@ contract KeyRegistry { - use dep::authwit::auth::assert_current_call_valid_authwit_public; + use dep::authwit::auth::assert_current_call_valid_authwit_public; - use dep::aztec::{ - state_vars::{ - SharedMutable, - Map - }, - protocol_types::{ - grumpkin_point::GrumpkinPoint, - address::{ - AztecAddress, - PublicKeysHash, - PartialAddress, - }, - constants::{ - GENERATOR_INDEX__CONTRACT_ADDRESS_V1, - GENERATOR_INDEX__PUBLIC_KEYS_HASH - }, - hash::poseidon2_hash, - }, - }; + use dep::aztec::{ + state_vars::{SharedMutable, Map}, + protocol_types::{ + grumpkin_point::GrumpkinPoint, address::{AztecAddress, PartialAddress}, + hash::poseidon2_hash + } + }; - global KEY_ROTATION_DELAY = 5; + global KEY_ROTATION_DELAY = 5; - #[aztec(storage)] + #[aztec(storage)] struct Storage { - //! This should stay at storage slot 1. If you change this, make sure you change the hardcoded value in keys/assert_public_key_freshness. - //! We use this hardcoded storage slot with derive_storage_slot_in_map and the SharedMutablePrivateGetter to directly read the value at an address in this contract. - nullifier_public_key_hash_registry: Map>, - - // We are not supporting rotating / changing keys other than the nullifier public in the registry at the moment, but will in the future. - // Uncomment lines below to enable that functionality - // incoming_public_key_registry: Map>, - // outgoing_public_key_registry: Map>, - // tagging_public_key_registry: Map>, + // The following stores a hash of individual master public keys + // If you change slots of vars bellow, you must update the slot in `SharedMutablePrivateGetter` in aztec-nr keys. + nullifier_registry: Map>, + incoming_registry: Map>, + outgoing_registry: Map>, + tagging_registry: Map>, } - #[aztec(public)] + #[aztec(public)] fn rotate_nullifier_public_key( - address: AztecAddress, - new_nullifier_public_key: GrumpkinPoint, - nonce: Field, - ) { - assert( - !new_nullifier_public_key.is_zero(), - "New nullifier public key must be non-zero" - ); - - // TODO: (#6137) - if (!address.eq(context.msg_sender())) { - assert_current_call_valid_authwit_public(&mut context, address); - } else { - assert(nonce == 0, "invalid nonce"); - } + address: AztecAddress, + new_nullifier_public_key: GrumpkinPoint, + nonce: Field + ) { + assert(!new_nullifier_public_key.is_zero(), "New nullifier public key must be non-zero"); - let nullifier_key_registry = storage.nullifier_public_key_hash_registry.at(address); + // TODO: (#6137) + if (!address.eq(context.msg_sender())) { + assert_current_call_valid_authwit_public(&mut context, address); + } else { + assert(nonce == 0, "invalid nonce"); + } - nullifier_key_registry.schedule_value_change(poseidon2_hash(new_nullifier_public_key.serialize())); - } + let nullifier_registry = storage.nullifier_registry.at(address); + nullifier_registry.schedule_value_change(poseidon2_hash(new_nullifier_public_key.serialize())); + } - #[aztec(public)] + #[aztec(public)] fn register( - address: AztecAddress, - partial_address: PartialAddress, - nullifier_public_key: GrumpkinPoint, - incoming_public_key: GrumpkinPoint, - outgoing_public_key: GrumpkinPoint, - tagging_public_key: GrumpkinPoint, - ) { - assert( - !partial_address.is_zero() & - !nullifier_public_key.is_zero() & - !incoming_public_key.is_zero() & - !outgoing_public_key.is_zero() & - !tagging_public_key.is_zero(), - "All public keys must be non-zero" - ); + address: AztecAddress, + partial_address: PartialAddress, + nullifier_public_key: GrumpkinPoint, + incoming_public_key: GrumpkinPoint, + outgoing_public_key: GrumpkinPoint, + tagging_public_key: GrumpkinPoint + ) { + assert( + !partial_address.is_zero() + & !nullifier_public_key.is_zero() + & !incoming_public_key.is_zero() + & !outgoing_public_key.is_zero() + & !tagging_public_key.is_zero(), "All public keys must be non-zero" + ); - // We could also pass in original_public_keys_hash instead of computing it here, if all we need the original one is for being able to prove ownership of address - let computed_address = AztecAddress::compute_from_public_keys_and_partial_address( - nullifier_public_key, - incoming_public_key, - outgoing_public_key, - tagging_public_key, - partial_address, - ); + // We could also pass in original_public_keys_hash instead of computing it here, if all we need the original one is for being able to prove ownership of address + let computed_address = AztecAddress::compute_from_public_keys_and_partial_address( + nullifier_public_key, + incoming_public_key, + outgoing_public_key, + tagging_public_key, + partial_address + ); - assert(computed_address.eq(address), "Computed address does not match supplied address"); + assert(computed_address.eq(address), "Computed address does not match supplied address"); - let nullifier_key_hash_registry = storage.nullifier_public_key_hash_registry.at(address); - // We are not supporting rotating / changing keys other than the nullifier public in the registry at the moment, but will in the future. - // Uncomment lines below to enable that functionality - // let incoming_key_registry = storage.incoming_public_key_registry.at(address); - // let outgoing_key_registry = storage.outgoing_public_key_registry.at(address); - // let tagging_key_registry = storage.taggin_public_key_registry.at(address); + let nullifier_registry = storage.nullifier_registry.at(address); + let incoming_registry = storage.incoming_registry.at(address); + let outgoing_registry = storage.outgoing_registry.at(address); + let tagging_registry = storage.tagging_registry.at(address); - nullifier_key_hash_registry.schedule_value_change(poseidon2_hash(nullifier_public_key.serialize())); - // We are not supporting rotating / changing keys other than the nullifier public in the registry at the moment, but will in the future. - // Uncomment lines below to enable that functionality // incoming_key_registry.schedule_value_change(new_incoming_public_key); - // outgoing_key_registry.schedule_value_change(new_outgoing_public_key); - // tagging_key_registry.schedule_value_change(new_tagging_public_key); - } + nullifier_registry.schedule_value_change(poseidon2_hash(nullifier_public_key.serialize())); + incoming_registry.schedule_value_change(poseidon2_hash(incoming_public_key.serialize())); + outgoing_registry.schedule_value_change(poseidon2_hash(outgoing_public_key.serialize())); + tagging_registry.schedule_value_change(poseidon2_hash(tagging_public_key.serialize())); + } } 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 b73da67a097f..ac43a92203ad 100644 --- a/noir-projects/noir-contracts/contracts/test_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/test_contract/src/main.nr @@ -19,7 +19,7 @@ contract Test { use dep::aztec::state_vars::{shared_mutable::SharedMutablePrivateGetter, map::derive_storage_slot_in_map}; use dep::aztec::{ - keys::getters::get_fresh_nullifier_public_key_hash, + keys::getters::get_npk_m, context::{Context, inputs::private_context_inputs::PrivateContextInputs}, hash::{pedersen_hash, poseidon2_hash, compute_secret_hash, ArgsHasher}, note::{ @@ -435,7 +435,7 @@ contract Test { address: AztecAddress, public_nullifying_key: GrumpkinPoint, ) { - assert_eq(get_fresh_nullifier_public_key_hash(&mut context, address), poseidon2_hash(public_nullifying_key.serialize())); + assert_eq(get_npk_m(&mut context, address), poseidon2_hash(public_nullifying_key.serialize())); } #[aztec(public)] From 9670af3effcfaceca6438b762407ed4cbfc54cb4 Mon Sep 17 00:00:00 2001 From: benesjan Date: Fri, 3 May 2024 14:37:08 +0000 Subject: [PATCH 02/13] WIP --- .../contracts/test_contract/src/main.nr | 2 +- yarn-project/circuits.js/src/keys/index.ts | 21 +- .../end-to-end/src/e2e_key_registry.test.ts | 386 +++++------------- 3 files changed, 117 insertions(+), 292 deletions(-) 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 ac43a92203ad..9f092b54381e 100644 --- a/noir-projects/noir-contracts/contracts/test_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/test_contract/src/main.nr @@ -435,7 +435,7 @@ contract Test { address: AztecAddress, public_nullifying_key: GrumpkinPoint, ) { - assert_eq(get_npk_m(&mut context, address), poseidon2_hash(public_nullifying_key.serialize())); + assert_eq(get_npk_m(&mut context, address), public_nullifying_key); } #[aztec(public)] diff --git a/yarn-project/circuits.js/src/keys/index.ts b/yarn-project/circuits.js/src/keys/index.ts index 21ec5c6b460b..f8da77fcba5a 100644 --- a/yarn-project/circuits.js/src/keys/index.ts +++ b/yarn-project/circuits.js/src/keys/index.ts @@ -5,6 +5,7 @@ import { type Fr, type GrumpkinScalar } from '@aztec/foundation/fields'; import { Grumpkin } from '../barretenberg/crypto/grumpkin/index.js'; import { GeneratorIndex } from '../constants.gen.js'; import { type GrumpkinPrivateKey } from '../types/grumpkin_private_key.js'; +import { type PublicKey } from '../types/public_key.js'; export function computeAppNullifierSecretKey(masterNullifierSecretKey: GrumpkinPrivateKey, app: AztecAddress): Fr { return poseidon2Hash([masterNullifierSecretKey.high, masterNullifierSecretKey.low, app, GeneratorIndex.NSK_M]); @@ -23,6 +24,21 @@ export function deriveSigningKey(secretKey: Fr): GrumpkinScalar { return sha512ToGrumpkinScalar([secretKey, GeneratorIndex.IVSK_M]); } +export function computePublicKeysHash( + masterNullifierPublicKey: PublicKey, + masterIncomingViewingPublicKey: PublicKey, + masterOutgoingViewingPublicKey: PublicKey, + masterTaggingPublicKey: PublicKey, +): Fr { + return poseidon2Hash([ + masterNullifierPublicKey, + masterIncomingViewingPublicKey, + masterOutgoingViewingPublicKey, + masterTaggingPublicKey, + GeneratorIndex.PUBLIC_KEYS_HASH, + ]); +} + /** * Computes secret and public keys and public keys hash from a secret key. * @param secretKey - The secret key to derive keys from. @@ -44,13 +60,12 @@ export function deriveKeys(secretKey: Fr) { const masterTaggingPublicKey = curve.mul(curve.generator(), masterTaggingSecretKey); // We hash the public keys to get the public keys hash - const publicKeysHash = poseidon2Hash([ + const publicKeysHash = computePublicKeysHash( masterNullifierPublicKey, masterIncomingViewingPublicKey, masterOutgoingViewingPublicKey, masterTaggingPublicKey, - GeneratorIndex.PUBLIC_KEYS_HASH, - ]); + ); return { masterNullifierSecretKey, 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 b801beed9e41..31d44cfbf4e7 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,5 +1,5 @@ import { type AccountWallet, AztecAddress, Fr, type PXE } from '@aztec/aztec.js'; -import { CompleteAddress, GeneratorIndex, type PartialAddress, Point } from '@aztec/circuits.js'; +import { CompleteAddress, GeneratorIndex, type PartialAddress, Point, deriveKeys } from '@aztec/circuits.js'; import { poseidon2Hash } from '@aztec/foundation/crypto'; import { KeyRegistryContract, TestContract } from '@aztec/noir-contracts.js'; import { getCanonicalKeyRegistryAddress } from '@aztec/protocol-contracts/key-registry'; @@ -21,6 +21,17 @@ describe('Key Registry', () => { let teardown: () => Promise; + // TODO(#5834): use AztecAddress.compute or smt + const { + masterNullifierPublicKey, + masterIncomingViewingPublicKey, + masterOutgoingViewingPublicKey, + masterTaggingPublicKey, + publicKeysHash, + } = deriveKeys(Fr.random()); + const partialAddress: PartialAddress = Fr.random(); + let account: AztecAddress; + beforeAll(async () => { ({ teardown, pxe, wallets } = await setup(3)); keyRegistry = await KeyRegistryContract.at(getCanonicalKeyRegistryAddress(), wallets[0]); @@ -28,6 +39,11 @@ describe('Key Registry', () => { testContract = await TestContract.deploy(wallets[0]).send().deployed(); await publicDeployAccounts(wallets[0], wallets.slice(0, 2)); + + // TODO(#5834): use AztecAddress.compute or smt + account = AztecAddress.fromField( + poseidon2Hash([publicKeysHash, partialAddress, GeneratorIndex.CONTRACT_ADDRESS_V1]), + ); }); const delay = async (blocks: number) => { @@ -39,66 +55,24 @@ describe('Key Registry', () => { afterAll(() => teardown()); describe('failure cases', () => { - let accountAddedToRegistry: AztecAddress; - - describe('should fail when registering with different types of invalid input', () => { - const masterNullifierPublicKey = Point.random(); - const masterIncomingViewingPublicKey = Point.random(); - const masterOutgoingViewingPublicKey = Point.random(); - const masterTaggingPublicKey = Point.random(); - const partialAddress: PartialAddress = Fr.random(); - - // TODO(#5726): use computePublicKeysHash function - const publicKeysHash = poseidon2Hash([ + it('throws when address preimage check fails', async () => { + const keys = [ masterNullifierPublicKey, masterIncomingViewingPublicKey, masterOutgoingViewingPublicKey, masterTaggingPublicKey, - GeneratorIndex.PUBLIC_KEYS_HASH, - ]); - - // TODO(#5726): Move the following line to AztecAddress class? - accountAddedToRegistry = AztecAddress.fromField( - poseidon2Hash([publicKeysHash, partialAddress, GeneratorIndex.CONTRACT_ADDRESS_V1]), - ); - - it('should fail when we register with a mismatched address', async () => { - const mismatchedAddress = AztecAddress.random(); + ]; - await expect( - keyRegistry - .withWallet(wallets[0]) - .methods.register( - mismatchedAddress, - partialAddress, - masterNullifierPublicKey, - masterIncomingViewingPublicKey, - masterOutgoingViewingPublicKey, - masterTaggingPublicKey, - ) - .send() - .wait(), - ).rejects.toThrow('Computed address does not match supplied address'); - }); - - it('should fail when we register with mismatched nullifier public key', async () => { - const mismatchedMasterNullifierPublicKey = Point.random(); + // we randomly invalidate some of the keys + keys[Math.floor(Math.random() * keys.length)] = Point.random(); - await expect( - keyRegistry - .withWallet(wallets[0]) - .methods.register( - AztecAddress.fromField(accountAddedToRegistry), - partialAddress, - mismatchedMasterNullifierPublicKey, - masterIncomingViewingPublicKey, - masterOutgoingViewingPublicKey, - masterTaggingPublicKey, - ) - .send() - .wait(), - ).rejects.toThrow('Computed address does not match supplied address'); - }); + await expect( + keyRegistry + .withWallet(wallets[0]) + .methods.register(AztecAddress.fromField(account), partialAddress, keys[0], keys[1], keys[2], keys[3]) + .send() + .wait(), + ).rejects.toThrow('Computed address does not match supplied address'); }); describe('should fail when rotating keys with different types of bad input', () => { @@ -125,32 +99,11 @@ describe('Key Registry', () => { }); describe('key registration flow', () => { - let accountAddedToRegistry: AztecAddress; - const masterNullifierPublicKey = Point.random(); - - it('should generate master public keys, a partial address, and register with the key registry', async () => { - const masterIncomingViewingPublicKey = Point.random(); - const masterOutgoingViewingPublicKey = Point.random(); - const masterTaggingPublicKey = Point.random(); - const partialAddress: PartialAddress = new Fr(420); - - const publicKeysHash = poseidon2Hash([ - masterNullifierPublicKey, - masterIncomingViewingPublicKey, - masterOutgoingViewingPublicKey, - masterTaggingPublicKey, - GeneratorIndex.PUBLIC_KEYS_HASH, - ]); - - // TODO(#5726): Move the following line to AztecAddress class? - accountAddedToRegistry = AztecAddress.fromField( - poseidon2Hash([publicKeysHash, partialAddress, GeneratorIndex.CONTRACT_ADDRESS_V1]), - ); - + it('registers', async () => { await keyRegistry .withWallet(wallets[0]) .methods.register( - AztecAddress.fromField(accountAddedToRegistry), + AztecAddress.fromField(account), partialAddress, masterNullifierPublicKey, masterIncomingViewingPublicKey, @@ -163,7 +116,7 @@ describe('Key Registry', () => { // 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 emptyNullifierPublicKey = await testContract.methods - .test_shared_mutable_private_getter_for_registry_contract(1, accountAddedToRegistry) + .test_shared_mutable_private_getter_for_registry_contract(1, account) .simulate(); expect(new Fr(emptyNullifierPublicKey)).toEqual(Fr.ZERO); @@ -172,7 +125,7 @@ describe('Key Registry', () => { await delay(5); const nullifierPublicKey = await testContract.methods - .test_shared_mutable_private_getter_for_registry_contract(1, accountAddedToRegistry) + .test_shared_mutable_private_getter_for_registry_contract(1, account) .simulate(); expect(new Fr(nullifierPublicKey)).toEqual(poseidon2Hash(masterNullifierPublicKey.toFields())); @@ -181,253 +134,110 @@ describe('Key Registry', () => { describe('key rotation flows', () => { const firstNewMasterNullifierPublicKey = Point.random(); + it('rotates npk_m', async () => { + await keyRegistry + .withWallet(wallets[0]) + .methods.rotate_nullifier_public_key(wallets[0].getAddress(), firstNewMasterNullifierPublicKey, Fr.ZERO) + .send() + .wait(); - describe('key rotation flow without authwit', () => { - it('we call the key registry to rotate our nullifier key', async () => { - await keyRegistry - .withWallet(wallets[0]) - .methods.rotate_nullifier_public_key(wallets[0].getAddress(), firstNewMasterNullifierPublicKey, Fr.ZERO) - .send() - .wait(); - - // 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 emptyNullifierPublicKey = await testContract.methods - .test_shared_mutable_private_getter_for_registry_contract(1, wallets[0].getAddress()) - .simulate(); + // 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 emptyNullifierPublicKey = await testContract.methods + .test_shared_mutable_private_getter_for_registry_contract(1, wallets[0].getAddress()) + .simulate(); - expect(new Fr(emptyNullifierPublicKey)).toEqual(Fr.ZERO); + expect(new Fr(emptyNullifierPublicKey)).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 delay(5); + // We check it again after a delay and expect that the change has been applied and consequently the assert is true + await delay(5); - const nullifierPublicKey = await testContract.methods - .test_shared_mutable_private_getter_for_registry_contract(1, wallets[0].getAddress()) - .simulate(); + const nullifierPublicKey = await testContract.methods + .test_shared_mutable_private_getter_for_registry_contract(1, wallets[0].getAddress()) + .simulate(); - expect(new Fr(nullifierPublicKey)).toEqual(poseidon2Hash(firstNewMasterNullifierPublicKey.toFields())); - }); + expect(new Fr(nullifierPublicKey)).toEqual(poseidon2Hash(firstNewMasterNullifierPublicKey.toFields())); }); - describe('key rotation flow with authwit', () => { + it(`rotates npk_m with authwit`, async () => { const secondNewMasterNullifierPublicKey = Point.random(); + const action = keyRegistry + .withWallet(wallets[1]) + .methods.rotate_nullifier_public_key(wallets[0].getAddress(), secondNewMasterNullifierPublicKey, Fr.ZERO); - it(`wallet 1 rotates wallet 0's nullifying public key with an authwit`, async () => { - const action = keyRegistry - .withWallet(wallets[1]) - .methods.rotate_nullifier_public_key(wallets[0].getAddress(), secondNewMasterNullifierPublicKey, Fr.ZERO); - - await wallets[0] - .setPublicAuthWit({ caller: wallets[1].getCompleteAddress().address, action }, true) - .send() - .wait(); - - await action.send().wait(); - - // 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 value to be the old one, because the new one hasn't been applied - const oldNullifierPublicKey = await testContract.methods - .test_shared_mutable_private_getter_for_registry_contract(1, wallets[0].getAddress()) - .simulate(); - - expect(new Fr(oldNullifierPublicKey)).toEqual(poseidon2Hash(firstNewMasterNullifierPublicKey.toFields())); - - // We check it again after a delay and expect that the change has been applied and consequently the assert is true - await delay(5); - - const newNullifierPublicKey = await testContract.methods - .test_shared_mutable_private_getter_for_registry_contract(1, wallets[0].getAddress()) - .simulate(); - - expect(new Fr(newNullifierPublicKey)).toEqual(poseidon2Hash(secondNewMasterNullifierPublicKey.toFields())); - }); - }); - }); - - describe('testing get_fresh_nullifier_public_key_hash: key registration flow, no PXE', () => { - const masterNullifierPublicKey = Point.random(); - const masterIncomingViewingPublicKey = Point.random(); - const masterOutgoingViewingPublicKey = Point.random(); - const masterTaggingPublicKey = Point.random(); - const partialAddress: PartialAddress = new Fr(420); - - const publicKeysHash = poseidon2Hash([ - masterNullifierPublicKey, - masterIncomingViewingPublicKey, - masterOutgoingViewingPublicKey, - masterTaggingPublicKey, - GeneratorIndex.PUBLIC_KEYS_HASH, - ]); - - // TODO(#5726): Move the following line to AztecAddress class? - const accountAddedToRegistry = AztecAddress.fromField( - poseidon2Hash([publicKeysHash, partialAddress, GeneratorIndex.CONTRACT_ADDRESS_V1]), - ); - - it('should fail as we have not registered anything to the registry nor have we registered a recipient', async () => { - await expect( - testContract.methods - .test_nullifier_key_freshness(accountAddedToRegistry, masterNullifierPublicKey) - .send() - .wait(), - ).rejects.toThrow(`Cannot satisfy constraint 'computed_address.eq(address)'`); - }); - - it('adds an entry to the key registry, and checks the key freshness without and with conflicting information from our pxe', async () => { - await keyRegistry - .withWallet(wallets[0]) - .methods.register( - AztecAddress.fromField(accountAddedToRegistry), - partialAddress, - masterNullifierPublicKey, - masterIncomingViewingPublicKey, - masterOutgoingViewingPublicKey, - masterTaggingPublicKey, - ) + await wallets[0] + .setPublicAuthWit({ caller: wallets[1].getCompleteAddress().address, action }, true) .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 - await expect( - testContract.methods - .test_nullifier_key_freshness(accountAddedToRegistry, masterNullifierPublicKey) - .send() - .wait(), - ).rejects.toThrow(`Cannot satisfy constraint 'computed_address.eq(address)'`); + await action.send().wait(); + + // 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 value to be the old one, because the new one hasn't been applied + const oldNullifierPublicKey = await testContract.methods + .test_shared_mutable_private_getter_for_registry_contract(1, wallets[0].getAddress()) + .simulate(); + + expect(new Fr(oldNullifierPublicKey)).toEqual(poseidon2Hash(firstNewMasterNullifierPublicKey.toFields())); // We check it again after a delay and expect that the change has been applied and consequently the assert is true await delay(5); - await testContract.methods - .test_nullifier_key_freshness(accountAddedToRegistry, masterNullifierPublicKey) - .send() - .wait(); - - // TODO: (#5834) Refactor complete address to move the public keys - await pxe.registerRecipient(CompleteAddress.create(accountAddedToRegistry, Point.ZERO, partialAddress), [ - new Point(Fr.random(), Fr.random()), - masterIncomingViewingPublicKey, - masterOutgoingViewingPublicKey, - masterTaggingPublicKey, - ]); + const newNullifierPublicKey = await testContract.methods + .test_shared_mutable_private_getter_for_registry_contract(1, wallets[0].getAddress()) + .simulate(); - // Our check should still succeed even if our pxe gives conflicting information, taking the registry as the source of truth. - await testContract.methods - .test_nullifier_key_freshness(accountAddedToRegistry, masterNullifierPublicKey) - .send() - .wait(); + expect(new Fr(newNullifierPublicKey)).toEqual(poseidon2Hash(secondNewMasterNullifierPublicKey.toFields())); }); }); - describe('testing assert_nullifier_key_is_fresh: key registration flow, with PXE', () => { - const masterNullifierPublicKey = Point.random(); - const masterIncomingViewingPublicKey = Point.random(); - const masterOutgoingViewingPublicKey = Point.random(); - const masterTaggingPublicKey = Point.random(); - const partialAddress: PartialAddress = new Fr(69420); - - const publicKeysHash = poseidon2Hash([ - masterNullifierPublicKey, - masterIncomingViewingPublicKey, - masterOutgoingViewingPublicKey, - masterTaggingPublicKey, - GeneratorIndex.PUBLIC_KEYS_HASH, - ]); - - // TODO(#5726): Move the following line to AztecAddress class? - const accountAddedToRegistry = AztecAddress.fromField( - poseidon2Hash([publicKeysHash, partialAddress, GeneratorIndex.CONTRACT_ADDRESS_V1]), - ); + describe('key freshness lib', () => { + it('succeeds for non-registered account available in PXE', async () => { + // TODO(#5834): Make this not disgusting + const newAccountKeys = deriveKeys(Fr.random()); + const newAccountPartialAddress = Fr.random(); + const newAccount = AztecAddress.fromField( + poseidon2Hash([newAccountKeys.publicKeysHash, newAccountPartialAddress, GeneratorIndex.CONTRACT_ADDRESS_V1]), + ); + const newAccountCompleteAddress = CompleteAddress.create( + newAccount, + newAccountKeys.masterIncomingViewingPublicKey, + newAccountPartialAddress, + ); - it('should fail as we have not registered anything to the registry nor have we registered a recipient', async () => { + // Should fail as the contract is not registered in key registry nor registered in PXE as a recipient await expect( testContract.methods - .test_nullifier_key_freshness(accountAddedToRegistry, masterNullifierPublicKey) + .test_nullifier_key_freshness(newAccount, newAccountKeys.masterNullifierPublicKey) .send() .wait(), ).rejects.toThrow(`Cannot satisfy constraint 'computed_address.eq(address)'`); - }); - it('should fail when we try to check the public keys for a invalid address', async () => { - const randAddress = AztecAddress.random(); - // TODO: (#5834) Refactor complete address to move the public keys - await pxe.registerRecipient(CompleteAddress.create(randAddress, Point.ZERO, partialAddress), [ - masterNullifierPublicKey, - masterIncomingViewingPublicKey, - masterOutgoingViewingPublicKey, - masterTaggingPublicKey, + await pxe.registerRecipient(newAccountCompleteAddress, [ + newAccountKeys.masterNullifierPublicKey, + newAccountKeys.masterIncomingViewingPublicKey, + newAccountKeys.masterOutgoingViewingPublicKey, + newAccountKeys.masterTaggingPublicKey, ]); - await expect( - testContract.methods.test_nullifier_key_freshness(randAddress, masterNullifierPublicKey).send().wait(), - ).rejects.toThrow(`Cannot satisfy constraint 'computed_address.eq(address)'`); - }); - - it('adds a recipient to our pxe, and checks the key freshness with and without adding an entry to our key registry', async () => { - // TODO: (#5834) Refactor complete address to move the public keys - await pxe.registerRecipient(CompleteAddress.create(accountAddedToRegistry, Point.ZERO, partialAddress), [ - masterNullifierPublicKey, - masterIncomingViewingPublicKey, - masterOutgoingViewingPublicKey, - masterTaggingPublicKey, - ]); - - // The check should succeed because we register our recipient manually and the lib checks our pxe + // Should succeed as the account is now registered as a recipient in PXE await testContract.methods - .test_nullifier_key_freshness(accountAddedToRegistry, masterNullifierPublicKey) - .send() - .wait(); - - // Now we add the keys to registry - await keyRegistry - .withWallet(wallets[0]) - .methods.register( - AztecAddress.fromField(accountAddedToRegistry), - partialAddress, - masterNullifierPublicKey, - masterIncomingViewingPublicKey, - masterOutgoingViewingPublicKey, - masterTaggingPublicKey, - ) - .send() - .wait(); - - // 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 be 0 because the change has not been applied yet - const emptyNullifierPublicKey = await testContract.methods - .test_shared_mutable_private_getter_for_registry_contract(1, accountAddedToRegistry) - .simulate(); - - expect(new Fr(emptyNullifierPublicKey)).toEqual(Fr.ZERO); - - // We check if our rotated nullifier key is equal to the key obtained from the getter. We expect this to succeed because even though the change - // has not been applied yet to the registry, we have manually the keys to our pxe - await testContract.methods - .test_nullifier_key_freshness(accountAddedToRegistry, masterNullifierPublicKey) - .send() - .wait(); - - // In the case where the key exists both in the pxe and our registry, we know that our assert will still remain true - await testContract.methods - .test_nullifier_key_freshness(accountAddedToRegistry, masterNullifierPublicKey) + .test_nullifier_key_freshness(newAccount, newAccountKeys.masterNullifierPublicKey) .send() .wait(); }); - }); - describe('testing assert_nullifier_key_is_fresh: key rotation flow', () => { - const newMasterNullifierPublicKey = Point.random(); - - it('we rotate the nullifier key and check that the key is fresh', async () => { + // TODO(benesjan): re-enable this test + it.skip('gets new key after rotation', async () => { + const newMasterNullifierPublicKey = Point.random(); + // Rotate the key await keyRegistry .withWallet(wallets[0]) .methods.rotate_nullifier_public_key(wallets[0].getAddress(), newMasterNullifierPublicKey, Fr.ZERO) .send() .wait(); - // 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 + // Fails as the rotation has not yet been applied await expect( testContract.methods .test_nullifier_key_freshness(wallets[0].getAddress(), newMasterNullifierPublicKey) @@ -437,9 +247,9 @@ describe('Key Registry', () => { `Cannot satisfy constraint 'assert_eq(get_fresh_nullifier_public_key_hash(&mut context, address), poseidon2_hash(public_nullifying_key.serialize()))'`, ); - // We check it again after a delay and expect that the change has been applied and consequently the assert is true await delay(5); + // Change has been applied hence should succeed now await testContract.methods .test_nullifier_key_freshness(wallets[0].getAddress(), newMasterNullifierPublicKey) .send() From fbccdd6c0a2992cc20cf3e8d7f58753eda202125 Mon Sep 17 00:00:00 2001 From: benesjan Date: Fri, 3 May 2024 14:46:34 +0000 Subject: [PATCH 03/13] nuking unnecessary zero key checks --- .../key_registry_contract/src/main.nr | 10 ------- .../end-to-end/src/e2e_key_registry.test.ts | 28 ++++++------------- 2 files changed, 8 insertions(+), 30 deletions(-) diff --git a/noir-projects/noir-contracts/contracts/key_registry_contract/src/main.nr b/noir-projects/noir-contracts/contracts/key_registry_contract/src/main.nr index 1b60d2d2bd79..7885401455f9 100644 --- a/noir-projects/noir-contracts/contracts/key_registry_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/key_registry_contract/src/main.nr @@ -27,8 +27,6 @@ contract KeyRegistry { new_nullifier_public_key: GrumpkinPoint, nonce: Field ) { - assert(!new_nullifier_public_key.is_zero(), "New nullifier public key must be non-zero"); - // TODO: (#6137) if (!address.eq(context.msg_sender())) { assert_current_call_valid_authwit_public(&mut context, address); @@ -49,14 +47,6 @@ contract KeyRegistry { outgoing_public_key: GrumpkinPoint, tagging_public_key: GrumpkinPoint ) { - assert( - !partial_address.is_zero() - & !nullifier_public_key.is_zero() - & !incoming_public_key.is_zero() - & !outgoing_public_key.is_zero() - & !tagging_public_key.is_zero(), "All public keys must be non-zero" - ); - // We could also pass in original_public_keys_hash instead of computing it here, if all we need the original one is for being able to prove ownership of address let computed_address = AztecAddress::compute_from_public_keys_and_partial_address( nullifier_public_key, 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 31d44cfbf4e7..21f165f3ce4d 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 @@ -75,26 +75,14 @@ describe('Key Registry', () => { ).rejects.toThrow('Computed address does not match supplied address'); }); - describe('should fail when rotating keys with different types of bad input', () => { - it('should fail when we try to rotate keys, while setting a 0 key', async () => { - await expect( - keyRegistry - .withWallet(wallets[0]) - .methods.rotate_nullifier_public_key(wallets[0].getAddress(), Point.ZERO, Fr.ZERO) - .send() - .wait(), - ).rejects.toThrow('New nullifier public key must be non-zero'); - }); - - it('should fail when we try to rotate keys for another address without authwit', async () => { - await expect( - keyRegistry - .withWallet(wallets[0]) - .methods.rotate_nullifier_public_key(wallets[1].getAddress(), Point.random(), Fr.ZERO) - .send() - .wait(), - ).rejects.toThrow('Assertion failed: Message not authorized by account'); - }); + it('should fail when we try to rotate keys for another address without authwit', async () => { + await expect( + keyRegistry + .withWallet(wallets[0]) + .methods.rotate_nullifier_public_key(wallets[1].getAddress(), Point.random(), Fr.ZERO) + .send() + .wait(), + ).rejects.toThrow('Assertion failed: Message not authorized by account'); }); }); From cc67ae9bd22514854c39e5c4ded9c492e6d7fa44 Mon Sep 17 00:00:00 2001 From: benesjan Date: Fri, 3 May 2024 16:08:01 +0000 Subject: [PATCH 04/13] WIP --- .../aztec-nr/aztec/src/keys/getters.nr | 54 +++++++------ .../key_registry_contract/src/main.nr | 51 ++++++++----- .../contracts/test_contract/src/main.nr | 10 +-- .../end-to-end/src/e2e_key_registry.test.ts | 75 +++++++++++-------- 4 files changed, 110 insertions(+), 80 deletions(-) diff --git a/noir-projects/aztec-nr/aztec/src/keys/getters.nr b/noir-projects/aztec-nr/aztec/src/keys/getters.nr index 171f2d61579f..f4c9c667e3f6 100644 --- a/noir-projects/aztec-nr/aztec/src/keys/getters.nr +++ b/noir-projects/aztec-nr/aztec/src/keys/getters.nr @@ -7,8 +7,8 @@ use crate::{ } }; -// Note: In fetch_hash_from_registry we expect that shared mutable slow is index + 1 (e.g. NULLIFIER_INDEX + 1), -// this changes the function will need to be refactored +// Note: In fetch_hash_from_registry we expect that shared mutable slow is index * 2 + 1 for x coordinate +// and index * 2 + 2 for y coordinate, if this changes the function will need to be refactored. global NULLIFIER_INDEX = 0; global INCOMING_INDEX = 1; global OUTGOING_INDEX = 2; @@ -24,13 +24,14 @@ pub fn get_ivpk_m(context: &mut PrivateContext, address: AztecAddress) -> Grumpk get_master_key(context, address, INCOMING_INDEX) } -pub fn get_ovpk_m(context: &mut PrivateContext, address: AztecAddress) -> GrumpkinPoint { - get_master_key(context, address, OUTGOING_INDEX) -} - -pub fn get_tpk_m(context: &mut PrivateContext, address: AztecAddress) -> GrumpkinPoint { - get_master_key(context, address, TAGGING_INDEX) -} +// Commented out as it's currently not enabled in key registry +// pub fn get_ovpk_m(context: &mut PrivateContext, address: AztecAddress) -> GrumpkinPoint { +// get_master_key(context, address, OUTGOING_INDEX) +// } +// +// pub fn get_tpk_m(context: &mut PrivateContext, address: AztecAddress) -> GrumpkinPoint { +// get_master_key(context, address, TAGGING_INDEX) +// } fn get_master_key( context: &mut PrivateContext, @@ -38,33 +39,40 @@ fn get_master_key( key_index: Field ) -> GrumpkinPoint { // 1. Get key hash from registry - let key_hash = fetch_hash_from_registry(context, key_index, address); - if key_hash == 0 { - // i. Keys were not registered yet --> fetch address preimage and check that the hash matches + let key = fetch_key_from_registry(context, key_index, address); + if key.is_zero() { + // i. Keys were not registered yet --> fetch key from PXE fetch_and_constrain_keys(address)[key_index] } else { - // ii. Keys were registered --> we fetch the key from oracle, hash it and check that it matches the hash in the registry - let (keys, _) = get_public_keys_and_partial_address(address); - let key = keys[key_index]; - let key_hash_from_oracle = poseidon2_hash(key.serialize()); - assert(key_hash_from_oracle == key_hash); + // ii. Keys were registered --> return the key key } } -fn fetch_hash_from_registry( +fn fetch_key_from_registry( context: &mut PrivateContext, key_index: Field, address: AztecAddress -) -> Field { - let derived_slot = derive_storage_slot_in_map(key_index + 1, address); +) -> GrumpkinPoint { + 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 registry_private_getter: SharedMutablePrivateGetter = SharedMutablePrivateGetter::new( + let x_coordinate_registry: SharedMutablePrivateGetter = SharedMutablePrivateGetter::new( *context, AztecAddress::from_field(CANONICAL_KEY_REGISTRY_ADDRESS), - derived_slot + x_coordinate_derived_slot ); - registry_private_getter.get_current_value_in_private() + 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_current_value_in_private(); + let y_coordinate = y_coordinate_registry.get_current_value_in_private(); + + GrumpkinPoint::new(x_coordinate, 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/noir-contracts/contracts/key_registry_contract/src/main.nr b/noir-projects/noir-contracts/contracts/key_registry_contract/src/main.nr index 7885401455f9..a2b983828f2f 100644 --- a/noir-projects/noir-contracts/contracts/key_registry_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/key_registry_contract/src/main.nr @@ -5,7 +5,6 @@ contract KeyRegistry { state_vars::{SharedMutable, Map}, protocol_types::{ grumpkin_point::GrumpkinPoint, address::{AztecAddress, PartialAddress}, - hash::poseidon2_hash } }; @@ -14,11 +13,19 @@ contract KeyRegistry { #[aztec(storage)] struct Storage { // The following stores a hash of individual master public keys - // If you change slots of vars bellow, you must update the slot in `SharedMutablePrivateGetter` in aztec-nr keys. - nullifier_registry: Map>, - incoming_registry: Map>, - outgoing_registry: Map>, - tagging_registry: Map>, + // If you change slots of vars bellow, you must update the slots in `SharedMutablePrivateGetter` in aztec-nr keys. + // We store x and y coordinates in individual shared mutables as shared mutable currently supports only 1 field + npk_m_x_registry: Map>, + npk_m_y_registry: Map>, + + ivpk_m_x_registry: Map>, + ivpk_m_y_registry: Map>, + + ovpk_m_x_registry: Map>, + ovpk_m_y_registry: Map>, + + tpk_m_x_registry: Map>, + tpk_m_y_registry: Map>, } #[aztec(public)] @@ -34,8 +41,10 @@ contract KeyRegistry { assert(nonce == 0, "invalid nonce"); } - let nullifier_registry = storage.nullifier_registry.at(address); - nullifier_registry.schedule_value_change(poseidon2_hash(new_nullifier_public_key.serialize())); + let npk_m_x_registry = storage.npk_m_x_registry.at(address); + let npk_m_y_registry = storage.npk_m_y_registry.at(address); + npk_m_x_registry.schedule_value_change(new_nullifier_public_key.x); + npk_m_y_registry.schedule_value_change(new_nullifier_public_key.y); } #[aztec(public)] @@ -47,7 +56,6 @@ contract KeyRegistry { outgoing_public_key: GrumpkinPoint, tagging_public_key: GrumpkinPoint ) { - // We could also pass in original_public_keys_hash instead of computing it here, if all we need the original one is for being able to prove ownership of address let computed_address = AztecAddress::compute_from_public_keys_and_partial_address( nullifier_public_key, incoming_public_key, @@ -58,14 +66,23 @@ contract KeyRegistry { assert(computed_address.eq(address), "Computed address does not match supplied address"); - let nullifier_registry = storage.nullifier_registry.at(address); - let incoming_registry = storage.incoming_registry.at(address); - let outgoing_registry = storage.outgoing_registry.at(address); - let tagging_registry = storage.tagging_registry.at(address); + let npk_m_x_registry = storage.npk_m_x_registry.at(address); + let npk_m_y_registry = storage.npk_m_y_registry.at(address); + let ivpk_m_x_registry = storage.ivpk_m_x_registry.at(address); + let ivpk_m_y_registry = storage.ivpk_m_y_registry.at(address); + // let ovpk_m_x_registry = storage.ovpk_m_x_registry.at(address); + // let ovpk_m_y_registry = storage.ovpk_m_y_registry.at(address); + // let tpk_m_x_registry = storage.tpk_m_x_registry.at(address); + // let tpk_m_y_registry = storage.tpk_m_y_registry.at(address); - nullifier_registry.schedule_value_change(poseidon2_hash(nullifier_public_key.serialize())); - incoming_registry.schedule_value_change(poseidon2_hash(incoming_public_key.serialize())); - outgoing_registry.schedule_value_change(poseidon2_hash(outgoing_public_key.serialize())); - tagging_registry.schedule_value_change(poseidon2_hash(tagging_public_key.serialize())); + npk_m_x_registry.schedule_value_change(nullifier_public_key.x); + npk_m_y_registry.schedule_value_change(nullifier_public_key.y); + ivpk_m_x_registry.schedule_value_change(incoming_public_key.x); + ivpk_m_y_registry.schedule_value_change(incoming_public_key.y); + // Commented out as we hit the max enqueued public calls limit when not done so + // ovpk_m_x_registry.schedule_value_change(outgoing_public_key.x); + // ovpk_m_y_registry.schedule_value_change(outgoing_public_key.y); + // tpk_m_x_registry.schedule_value_change(tagging_public_key.x); + // tpk_m_y_registry.schedule_value_change(tagging_public_key.y); } } 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 9f092b54381e..1a92cce5d600 100644 --- a/noir-projects/noir-contracts/contracts/test_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/test_contract/src/main.nr @@ -425,9 +425,7 @@ contract Test { // 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(context, AztecAddress::from_field(CANONICAL_KEY_REGISTRY_ADDRESS), derived_slot); - let nullifier_public_key = registry_private_getter.get_current_value_in_private(); - - nullifier_public_key + registry_private_getter.get_current_value_in_private() } #[aztec(private)] @@ -438,12 +436,6 @@ contract Test { assert_eq(get_npk_m(&mut context, address), public_nullifying_key); } - #[aztec(public)] - fn delay() { - // We use this as a util function to "mine a block" - context.emit_unencrypted_log("dummy"); - } - // Purely exists for testing unconstrained fn get_random(kinda_seed: Field) -> pub Field { kinda_seed * unsafe_rand() 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 21f165f3ce4d..4c850cf4528a 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 @@ -10,6 +10,8 @@ import { publicDeployAccounts, setup } from './fixtures/utils.js'; const TIMEOUT = 100_000; +const SHARED_MUTABLE_DELAY = 5; + describe('Key Registry', () => { let keyRegistry: KeyRegistryContract; @@ -46,9 +48,10 @@ describe('Key Registry', () => { ); }); - const delay = async (blocks: number) => { - for (let i = 0; i < blocks; i++) { - await testContract.methods.delay().send().wait(); + 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(); } }; @@ -91,7 +94,7 @@ describe('Key Registry', () => { await keyRegistry .withWallet(wallets[0]) .methods.register( - AztecAddress.fromField(account), + account, partialAddress, masterNullifierPublicKey, masterIncomingViewingPublicKey, @@ -103,20 +106,20 @@ describe('Key Registry', () => { // 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 emptyNullifierPublicKey = await testContract.methods + const emptyNullifierPublicKeyX = await testContract.methods .test_shared_mutable_private_getter_for_registry_contract(1, account) .simulate(); - expect(new Fr(emptyNullifierPublicKey)).toEqual(Fr.ZERO); + 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 delay(5); + await crossDelay(); - const nullifierPublicKey = await testContract.methods + const nullifierPublicKeyX = await testContract.methods .test_shared_mutable_private_getter_for_registry_contract(1, account) .simulate(); - expect(new Fr(nullifierPublicKey)).toEqual(poseidon2Hash(masterNullifierPublicKey.toFields())); + expect(new Fr(nullifierPublicKeyX)).toEqual(masterNullifierPublicKey.x); }); }); @@ -131,20 +134,20 @@ describe('Key Registry', () => { // 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 emptyNullifierPublicKey = await testContract.methods + const emptyNullifierPublicKeyX = await testContract.methods .test_shared_mutable_private_getter_for_registry_contract(1, wallets[0].getAddress()) .simulate(); - expect(new Fr(emptyNullifierPublicKey)).toEqual(Fr.ZERO); + 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 delay(5); + await crossDelay(); - const nullifierPublicKey = await testContract.methods + const nullifierPublicKeyX = await testContract.methods .test_shared_mutable_private_getter_for_registry_contract(1, wallets[0].getAddress()) .simulate(); - expect(new Fr(nullifierPublicKey)).toEqual(poseidon2Hash(firstNewMasterNullifierPublicKey.toFields())); + expect(new Fr(nullifierPublicKeyX)).toEqual(firstNewMasterNullifierPublicKey.x); }); it(`rotates npk_m with authwit`, async () => { @@ -162,24 +165,43 @@ describe('Key Registry', () => { // 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 value to be the old one, because the new one hasn't been applied - const oldNullifierPublicKey = await testContract.methods + const oldNullifierPublicKeyX = await testContract.methods .test_shared_mutable_private_getter_for_registry_contract(1, wallets[0].getAddress()) .simulate(); - expect(new Fr(oldNullifierPublicKey)).toEqual(poseidon2Hash(firstNewMasterNullifierPublicKey.toFields())); + expect(new Fr(oldNullifierPublicKeyX)).toEqual(firstNewMasterNullifierPublicKey.x); // We check it again after a delay and expect that the change has been applied and consequently the assert is true - await delay(5); + await crossDelay(); - const newNullifierPublicKey = await testContract.methods + const newNullifierPublicKeyX = await testContract.methods .test_shared_mutable_private_getter_for_registry_contract(1, wallets[0].getAddress()) .simulate(); - expect(new Fr(newNullifierPublicKey)).toEqual(poseidon2Hash(secondNewMasterNullifierPublicKey.toFields())); + expect(new Fr(newNullifierPublicKeyX)).toEqual(secondNewMasterNullifierPublicKey.x); }); }); describe('key freshness lib', () => { + it('fails for non-existent account', async () => { + // Should fail as the contract is not registered in key registry + + const randomAddress = AztecAddress.random(); + const randomMasterNullifierPublicKey = Point.random(); + + await expect( + testContract.methods.test_nullifier_key_freshness(randomAddress, randomMasterNullifierPublicKey).send().wait(), + ).rejects.toThrow(`Cannot satisfy constraint 'computed_address.eq(address)'`); + }); + + it.only('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, masterNullifierPublicKey) + .send() + .wait(); + }); + it('succeeds for non-registered account available in PXE', async () => { // TODO(#5834): Make this not disgusting const newAccountKeys = deriveKeys(Fr.random()); @@ -193,14 +215,6 @@ describe('Key Registry', () => { newAccountPartialAddress, ); - // Should fail as the contract is not registered in key registry nor registered in PXE as a recipient - await expect( - testContract.methods - .test_nullifier_key_freshness(newAccount, newAccountKeys.masterNullifierPublicKey) - .send() - .wait(), - ).rejects.toThrow(`Cannot satisfy constraint 'computed_address.eq(address)'`); - await pxe.registerRecipient(newAccountCompleteAddress, [ newAccountKeys.masterNullifierPublicKey, newAccountKeys.masterIncomingViewingPublicKey, @@ -215,8 +229,7 @@ describe('Key Registry', () => { .wait(); }); - // TODO(benesjan): re-enable this test - it.skip('gets new key after rotation', async () => { + it('gets new key after rotation', async () => { const newMasterNullifierPublicKey = Point.random(); // Rotate the key await keyRegistry @@ -232,10 +245,10 @@ describe('Key Registry', () => { .send() .wait(), ).rejects.toThrow( - `Cannot satisfy constraint 'assert_eq(get_fresh_nullifier_public_key_hash(&mut context, address), poseidon2_hash(public_nullifying_key.serialize()))'`, + `Cannot satisfy constraint 'assert_eq(get_npk_m(&mut context, address), public_nullifying_key)'`, ); - await delay(5); + await crossDelay(); // Change has been applied hence should succeed now await testContract.methods From efef327d12cafc69c6858d937fe1b93d7a48a9f4 Mon Sep 17 00:00:00 2001 From: benesjan Date: Fri, 3 May 2024 16:16:35 +0000 Subject: [PATCH 05/13] final touches --- .../end-to-end/src/e2e_key_registry.test.ts | 123 +++++++----------- 1 file changed, 49 insertions(+), 74 deletions(-) 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 4c850cf4528a..5df18fa9b087 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 @@ -87,6 +87,44 @@ describe('Key Registry', () => { .wait(), ).rejects.toThrow('Assertion failed: Message not authorized by account'); }); + + it('fresh key lib fails for non-existent account', async () => { + // Should fail as the contract is not registered in key registry + + const randomAddress = AztecAddress.random(); + const randomMasterNullifierPublicKey = Point.random(); + + await expect( + testContract.methods.test_nullifier_key_freshness(randomAddress, randomMasterNullifierPublicKey).send().wait(), + ).rejects.toThrow(`Cannot satisfy constraint 'computed_address.eq(address)'`); + }); + }); + + it('fresh key lib succeeds for non-registered account available in PXE', async () => { + // TODO(#5834): Make this not disgusting + const newAccountKeys = deriveKeys(Fr.random()); + const newAccountPartialAddress = Fr.random(); + const newAccount = AztecAddress.fromField( + poseidon2Hash([newAccountKeys.publicKeysHash, newAccountPartialAddress, GeneratorIndex.CONTRACT_ADDRESS_V1]), + ); + const newAccountCompleteAddress = CompleteAddress.create( + newAccount, + newAccountKeys.masterIncomingViewingPublicKey, + newAccountPartialAddress, + ); + + await pxe.registerRecipient(newAccountCompleteAddress, [ + newAccountKeys.masterNullifierPublicKey, + newAccountKeys.masterIncomingViewingPublicKey, + newAccountKeys.masterOutgoingViewingPublicKey, + newAccountKeys.masterTaggingPublicKey, + ]); + + // Should succeed as the account is now registered as a recipient in PXE + await testContract.methods + .test_nullifier_key_freshness(newAccount, newAccountKeys.masterNullifierPublicKey) + .send() + .wait(); }); describe('key registration flow', () => { @@ -121,10 +159,17 @@ describe('Key Registry', () => { expect(new Fr(nullifierPublicKeyX)).toEqual(masterNullifierPublicKey.x); }); + + 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, masterNullifierPublicKey).send().wait(); + }); }); describe('key rotation flows', () => { const firstNewMasterNullifierPublicKey = Point.random(); + const secondNewMasterNullifierPublicKey = Point.random(); + it('rotates npk_m', async () => { await keyRegistry .withWallet(wallets[0]) @@ -151,7 +196,6 @@ describe('Key Registry', () => { }); it(`rotates npk_m with authwit`, async () => { - const secondNewMasterNullifierPublicKey = Point.random(); const action = keyRegistry .withWallet(wallets[1]) .methods.rotate_nullifier_public_key(wallets[0].getAddress(), secondNewMasterNullifierPublicKey, Fr.ZERO); @@ -163,96 +207,27 @@ describe('Key Registry', () => { await action.send().wait(); - // 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 value to be the old one, because the new one hasn't been applied + // 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); - // We check it again after a delay and expect that the change has been applied and consequently the assert is true 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); }); - }); - - describe('key freshness lib', () => { - it('fails for non-existent account', async () => { - // Should fail as the contract is not registered in key registry - - const randomAddress = AztecAddress.random(); - const randomMasterNullifierPublicKey = Point.random(); - - await expect( - testContract.methods.test_nullifier_key_freshness(randomAddress, randomMasterNullifierPublicKey).send().wait(), - ).rejects.toThrow(`Cannot satisfy constraint 'computed_address.eq(address)'`); - }); - - it.only('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, masterNullifierPublicKey) - .send() - .wait(); - }); - - it('succeeds for non-registered account available in PXE', async () => { - // TODO(#5834): Make this not disgusting - const newAccountKeys = deriveKeys(Fr.random()); - const newAccountPartialAddress = Fr.random(); - const newAccount = AztecAddress.fromField( - poseidon2Hash([newAccountKeys.publicKeysHash, newAccountPartialAddress, GeneratorIndex.CONTRACT_ADDRESS_V1]), - ); - const newAccountCompleteAddress = CompleteAddress.create( - newAccount, - newAccountKeys.masterIncomingViewingPublicKey, - newAccountPartialAddress, - ); - - await pxe.registerRecipient(newAccountCompleteAddress, [ - newAccountKeys.masterNullifierPublicKey, - newAccountKeys.masterIncomingViewingPublicKey, - newAccountKeys.masterOutgoingViewingPublicKey, - newAccountKeys.masterTaggingPublicKey, - ]); - - // Should succeed as the account is now registered as a recipient in PXE - await testContract.methods - .test_nullifier_key_freshness(newAccount, newAccountKeys.masterNullifierPublicKey) - .send() - .wait(); - }); - - it('gets new key after rotation', async () => { - const newMasterNullifierPublicKey = Point.random(); - // Rotate the key - await keyRegistry - .withWallet(wallets[0]) - .methods.rotate_nullifier_public_key(wallets[0].getAddress(), newMasterNullifierPublicKey, Fr.ZERO) - .send() - .wait(); - - // Fails as the rotation has not yet been applied - await expect( - testContract.methods - .test_nullifier_key_freshness(wallets[0].getAddress(), newMasterNullifierPublicKey) - .send() - .wait(), - ).rejects.toThrow( - `Cannot satisfy constraint 'assert_eq(get_npk_m(&mut context, address), public_nullifying_key)'`, - ); - - await crossDelay(); + 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(), newMasterNullifierPublicKey) + .test_nullifier_key_freshness(wallets[0].getAddress(), secondNewMasterNullifierPublicKey) .send() .wait(); }); From 7f636500511a737178dbe1dd602ab207b24d2ff1 Mon Sep 17 00:00:00 2001 From: benesjan Date: Fri, 3 May 2024 16:18:19 +0000 Subject: [PATCH 06/13] fix --- noir-projects/aztec-nr/aztec/src/keys/getters.nr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/noir-projects/aztec-nr/aztec/src/keys/getters.nr b/noir-projects/aztec-nr/aztec/src/keys/getters.nr index f4c9c667e3f6..2e3682195e6f 100644 --- a/noir-projects/aztec-nr/aztec/src/keys/getters.nr +++ b/noir-projects/aztec-nr/aztec/src/keys/getters.nr @@ -7,7 +7,7 @@ use crate::{ } }; -// Note: In fetch_hash_from_registry we expect that shared mutable slow is index * 2 + 1 for x coordinate +// Note: In fetch_key_from_registry we expect that shared mutable slow is index * 2 + 1 for x coordinate // and index * 2 + 2 for y coordinate, if this changes the function will need to be refactored. global NULLIFIER_INDEX = 0; global INCOMING_INDEX = 1; From beb761b88f67e7cce1e5c592a643f3a1e3a6d197 Mon Sep 17 00:00:00 2001 From: benesjan Date: Fri, 3 May 2024 18:43:10 +0000 Subject: [PATCH 07/13] fix --- noir-projects/aztec-nr/aztec/src/keys.nr | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/noir-projects/aztec-nr/aztec/src/keys.nr b/noir-projects/aztec-nr/aztec/src/keys.nr index 999bd984de00..3bef24ec2bff 100644 --- a/noir-projects/aztec-nr/aztec/src/keys.nr +++ b/noir-projects/aztec-nr/aztec/src/keys.nr @@ -1,4 +1,8 @@ mod getters; mod point_to_symmetric_key; -use crate::keys::getters::{get_npk_m, get_ivpk_m, get_ovpk_m, get_tpk_m}; +use crate::keys::getters::{get_npk_m, get_ivpk_m, +// Commented out as it's currently not enabled in key registry +// get_ovpk_m, +// get_tpk_m +}; From f2bcfae0e62aa4107e688e623ae564dfbbfdf38a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Bene=C5=A1?= Date: Mon, 6 May 2024 08:55:40 +0200 Subject: [PATCH 08/13] Update noir-projects/aztec-nr/aztec/src/keys/getters.nr Co-authored-by: esau <152162806+sklppy88@users.noreply.github.com> --- noir-projects/aztec-nr/aztec/src/keys/getters.nr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/noir-projects/aztec-nr/aztec/src/keys/getters.nr b/noir-projects/aztec-nr/aztec/src/keys/getters.nr index 2e3682195e6f..ed0fb23d58b9 100644 --- a/noir-projects/aztec-nr/aztec/src/keys/getters.nr +++ b/noir-projects/aztec-nr/aztec/src/keys/getters.nr @@ -8,7 +8,7 @@ use crate::{ }; // Note: In fetch_key_from_registry we expect that shared mutable slow is index * 2 + 1 for x coordinate -// and index * 2 + 2 for y coordinate, if this changes the function will need to be refactored. +// and index * 2 + 2 for the y coordinate. If this changes the function will need to be refactored. global NULLIFIER_INDEX = 0; global INCOMING_INDEX = 1; global OUTGOING_INDEX = 2; From 56f19188f7eac04a72785a2fd3b9c868dd399816 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Bene=C5=A1?= Date: Mon, 6 May 2024 08:55:46 +0200 Subject: [PATCH 09/13] Update noir-projects/noir-contracts/contracts/key_registry_contract/src/main.nr Co-authored-by: esau <152162806+sklppy88@users.noreply.github.com> --- .../noir-contracts/contracts/key_registry_contract/src/main.nr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/noir-projects/noir-contracts/contracts/key_registry_contract/src/main.nr b/noir-projects/noir-contracts/contracts/key_registry_contract/src/main.nr index a2b983828f2f..f8e62b1e4d71 100644 --- a/noir-projects/noir-contracts/contracts/key_registry_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/key_registry_contract/src/main.nr @@ -13,7 +13,7 @@ contract KeyRegistry { #[aztec(storage)] struct Storage { // The following stores a hash of individual master public keys - // If you change slots of vars bellow, you must update the slots in `SharedMutablePrivateGetter` in aztec-nr keys. + // If you change slots of vars below, you must update the slots in `SharedMutablePrivateGetter` in aztec-nr/keys. // We store x and y coordinates in individual shared mutables as shared mutable currently supports only 1 field npk_m_x_registry: Map>, npk_m_y_registry: Map>, From 866c3cd46fbb68f5698800aa4c31459487e820ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Bene=C5=A1?= Date: Mon, 6 May 2024 08:56:11 +0200 Subject: [PATCH 10/13] Update noir-projects/aztec-nr/aztec/src/keys/getters.nr Co-authored-by: esau <152162806+sklppy88@users.noreply.github.com> --- noir-projects/aztec-nr/aztec/src/keys/getters.nr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/noir-projects/aztec-nr/aztec/src/keys/getters.nr b/noir-projects/aztec-nr/aztec/src/keys/getters.nr index ed0fb23d58b9..bfa42907a3d6 100644 --- a/noir-projects/aztec-nr/aztec/src/keys/getters.nr +++ b/noir-projects/aztec-nr/aztec/src/keys/getters.nr @@ -7,7 +7,7 @@ use crate::{ } }; -// Note: In fetch_key_from_registry we expect that shared mutable slow is index * 2 + 1 for x coordinate +// Note: In fetch_key_from_registry we expect that the shared mutable slot is index * 2 + 1 for the x coordinate. For example, the npk_m x coordinates will be stored in a map at storage slot 1. (0) * 2 + 1 = 1, and the novk_m y coordinates will be stored in a map at storage slot 6. (2) * 2 + 2 = 6. // and index * 2 + 2 for the y coordinate. If this changes the function will need to be refactored. global NULLIFIER_INDEX = 0; global INCOMING_INDEX = 1; From d338aad76074ff2f5c54a1520bf5c57b49e8fe86 Mon Sep 17 00:00:00 2001 From: benesjan Date: Mon, 6 May 2024 07:08:40 +0000 Subject: [PATCH 11/13] clarifications --- noir-projects/aztec-nr/aztec/src/keys/getters.nr | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/noir-projects/aztec-nr/aztec/src/keys/getters.nr b/noir-projects/aztec-nr/aztec/src/keys/getters.nr index bfa42907a3d6..8e811deae707 100644 --- a/noir-projects/aztec-nr/aztec/src/keys/getters.nr +++ b/noir-projects/aztec-nr/aztec/src/keys/getters.nr @@ -7,8 +7,10 @@ use crate::{ } }; -// Note: In fetch_key_from_registry we expect that the shared mutable slot is index * 2 + 1 for the x coordinate. For example, the npk_m x coordinates will be stored in a map at storage slot 1. (0) * 2 + 1 = 1, and the novk_m y coordinates will be stored in a map at storage slot 6. (2) * 2 + 2 = 6. -// and index * 2 + 2 for the y coordinate. If this changes the function will need to be refactored. +// Note: In fetch_key_from_registry we expect that the shared mutable slot is index * 2 + 1 for the x coordinate and +// index * 2 + 2 for the y coordinate. For example, the npk_m x coordinates will be stored in a map at storage slot +// 0 * 2 + 1 = 1, and the npk_m y coordinates at 2 * 2 + 2 = 6. If this changes the function will need to be +// refactored. global NULLIFIER_INDEX = 0; global INCOMING_INDEX = 1; global OUTGOING_INDEX = 2; @@ -38,13 +40,12 @@ fn get_master_key( address: AztecAddress, key_index: Field ) -> GrumpkinPoint { - // 1. Get key hash from registry let key = fetch_key_from_registry(context, key_index, address); if key.is_zero() { - // i. Keys were not registered yet --> fetch key from PXE + // Keys were not registered in registry yet --> fetch key from PXE fetch_and_constrain_keys(address)[key_index] } else { - // ii. Keys were registered --> return the key + // Keys were registered --> return the key key } } From 5a3cef2b74701cf6c6da0816329be3a0d8178c3d Mon Sep 17 00:00:00 2001 From: benesjan Date: Mon, 6 May 2024 07:31:38 +0000 Subject: [PATCH 12/13] cleanup --- noir-projects/aztec-nr/aztec/src/keys/getters.nr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/noir-projects/aztec-nr/aztec/src/keys/getters.nr b/noir-projects/aztec-nr/aztec/src/keys/getters.nr index 8e811deae707..b6fc2759fb7b 100644 --- a/noir-projects/aztec-nr/aztec/src/keys/getters.nr +++ b/noir-projects/aztec-nr/aztec/src/keys/getters.nr @@ -1,6 +1,6 @@ use dep::protocol_types::{address::AztecAddress, constants::CANONICAL_KEY_REGISTRY_ADDRESS, grumpkin_point::GrumpkinPoint}; use crate::{ - context::PrivateContext, hash::poseidon2_hash, oracle::keys::get_public_keys_and_partial_address, + context::PrivateContext, oracle::keys::get_public_keys_and_partial_address, state_vars::{ map::derive_storage_slot_in_map, shared_mutable::shared_mutable_private_getter::SharedMutablePrivateGetter From 87247d60f8df027e16b1872787f8c3f180676588 Mon Sep 17 00:00:00 2001 From: benesjan Date: Mon, 6 May 2024 07:43:11 +0000 Subject: [PATCH 13/13] improved comments --- yarn-project/end-to-end/src/e2e_key_registry.test.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) 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 5df18fa9b087..48fb1fa90abe 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 @@ -66,7 +66,7 @@ describe('Key Registry', () => { masterTaggingPublicKey, ]; - // we randomly invalidate some of the keys + // We randomly invalidate some of the keys keys[Math.floor(Math.random() * keys.length)] = Point.random(); await expect( @@ -160,6 +160,7 @@ describe('Key Registry', () => { expect(new Fr(nullifierPublicKeyX)).toEqual(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, masterNullifierPublicKey).send().wait(); @@ -177,8 +178,8 @@ describe('Key Registry', () => { .send() .wait(); - // 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 + // 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();