From 3bd08a83d11e76111b027b99d6022f986ec34d7e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Bene=C5=A1?= Date: Mon, 19 Aug 2024 20:23:28 +0200 Subject: [PATCH] chore: nuking old registry contract (#8057) Fixes #7955 --- .../src/core/libraries/ConstantsGen.sol | 2 +- noir-projects/noir-contracts/Nargo.toml | 1 - .../key_registry_contract/src/main.nr | 99 ++++++++----------- .../new_key_registry_contract/Nargo.toml | 9 -- .../new_key_registry_contract/src/main.nr | 63 ------------ .../crates/types/src/constants.nr | 2 +- yarn-project/circuits.js/src/constants.gen.ts | 2 +- .../circuits.js/src/contract/artifact_hash.ts | 2 +- .../cli/src/cmds/misc/deploy_contracts.ts | 2 +- .../end-to-end/src/e2e_key_registry.test.ts | 6 +- yarn-project/end-to-end/src/fixtures/utils.ts | 4 +- .../scripts/copy-contracts.sh | 2 +- .../src/key-registry/artifact.ts | 4 +- 13 files changed, 52 insertions(+), 146 deletions(-) delete mode 100644 noir-projects/noir-contracts/contracts/new_key_registry_contract/Nargo.toml delete mode 100644 noir-projects/noir-contracts/contracts/new_key_registry_contract/src/main.nr diff --git a/l1-contracts/src/core/libraries/ConstantsGen.sol b/l1-contracts/src/core/libraries/ConstantsGen.sol index 4ef594869b0..9e3bc20bb90 100644 --- a/l1-contracts/src/core/libraries/ConstantsGen.sol +++ b/l1-contracts/src/core/libraries/ConstantsGen.sol @@ -128,7 +128,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 = - 21209182303070804160941065409360795406831433542792830301721453026531461944353; + 9694109890306420370616891858093188542026876097103155811681068343994212062621; uint256 internal constant CANONICAL_AUTH_REGISTRY_ADDRESS = 16522644890256297179255458951626875692461008240031142745359776058397274208468; uint256 internal constant DEPLOYER_CONTRACT_ADDRESS = diff --git a/noir-projects/noir-contracts/Nargo.toml b/noir-projects/noir-contracts/Nargo.toml index c0d18d96b91..1bb0fafff27 100644 --- a/noir-projects/noir-contracts/Nargo.toml +++ b/noir-projects/noir-contracts/Nargo.toml @@ -26,7 +26,6 @@ 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/key_registry_contract/src/main.nr b/noir-projects/noir-contracts/contracts/key_registry_contract/src/main.nr index 49f0ef0c544..aee3fb26c4e 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 @@ -2,83 +2,62 @@ contract KeyRegistry { use dep::authwit::auth::assert_current_call_valid_authwit_public; use dep::aztec::{ - keys::PublicKeys, state_vars::{SharedMutable, Map}, + keys::{PublicKeys, stored_keys::StoredKeys}, state_vars::{PublicMutable, Map}, protocol_types::{point::Point, address::{AztecAddress, PartialAddress}} }; - global KEY_ROTATION_DELAY = 5; - #[aztec(storage)] - struct Storage { - // The following stores a hash of individual master public 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>, - - 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>, - } + struct Storage { + current_keys: Map>, + } - #[aztec(public)] - fn rotate_npk_m(address: AztecAddress, new_npk_m: Point, nonce: Field) { - // TODO: (#6137) - if (!address.eq(context.msg_sender())) { - assert_current_call_valid_authwit_public(&mut context, address); - } else { - assert(nonce == 0, "invalid nonce"); + 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) } + ) + } } - - 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_npk_m.x); - npk_m_y_registry.schedule_value_change(new_npk_m.y); } - // We need to have two separate register functions because a single one would produce too many storage writes, since - // each SharedMutable.schedule_value_change call results in 5 writes (pre, post, block_of_change, delay and hash), - // totaling 40 writes, while the kernels only accept up to 32 writes. - // Once SharedMutable accepts multi-field values, we can have a single state variable hold all keys, and that way - // also have a single block of change, hash, and delay. - // TODO (#5491): make this be a single function with a single schedule call. + 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_npk_and_ivpk(address: AztecAddress, partial_address: PartialAddress, keys: PublicKeys) { + 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"); - assert(computed_address.eq(address), "Computed address does not match supplied 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); - - npk_m_x_registry.schedule_value_change(keys.npk_m.x); - npk_m_y_registry.schedule_value_change(keys.npk_m.y); - ivpk_m_x_registry.schedule_value_change(keys.ivpk_m.x); - ivpk_m_y_registry.schedule_value_change(keys.ivpk_m.y); + storage.current_keys.at(account).write(StoredKeys::new(keys)); } #[aztec(public)] - fn register_ovpk_and_tpk(address: AztecAddress, partial_address: PartialAddress, keys: PublicKeys) { - let computed_address = AztecAddress::compute(keys.hash(), partial_address); + 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"); + } - assert(computed_address.eq(address), "Computed address does not match supplied address"); + let account_key_storage = storage.current_keys.at(account); - 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); + // 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; - ovpk_m_x_registry.schedule_value_change(keys.ovpk_m.x); - ovpk_m_y_registry.schedule_value_change(keys.ovpk_m.y); - tpk_m_x_registry.schedule_value_change(keys.tpk_m.x); - tpk_m_y_registry.schedule_value_change(keys.tpk_m.y); + account_key_storage.write(StoredKeys::new(current_keys)); } } 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 deleted file mode 100644 index 6486c235e1a..00000000000 --- a/noir-projects/noir-contracts/contracts/new_key_registry_contract/Nargo.toml +++ /dev/null @@ -1,9 +0,0 @@ -[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 deleted file mode 100644 index 9a4744d2c81..00000000000 --- a/noir-projects/noir-contracts/contracts/new_key_registry_contract/src/main.nr +++ /dev/null @@ -1,63 +0,0 @@ -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-protocol-circuits/crates/types/src/constants.nr b/noir-projects/noir-protocol-circuits/crates/types/src/constants.nr index 14dc49d4220..2a4467f8c37 100644 --- a/noir-projects/noir-protocol-circuits/crates/types/src/constants.nr +++ b/noir-projects/noir-protocol-circuits/crates/types/src/constants.nr @@ -183,7 +183,7 @@ global L2_GAS_PER_NOTE_HASH: u32 = 32; global L2_GAS_PER_NULLIFIER: u32 = 64; // CANONICAL CONTRACT ADDRESSES -global CANONICAL_KEY_REGISTRY_ADDRESS = AztecAddress::from_field(0x2ee3f8c67efa88f9e6fb44242f1e9dcc0f9a6752ded07af0d9fac3875a61d421); +global CANONICAL_KEY_REGISTRY_ADDRESS = AztecAddress::from_field(0x156eabf84e3ea50d40e3330224f2d2e81648fff8f1f7ec1bc6d2873cca6e959d); global CANONICAL_AUTH_REGISTRY_ADDRESS = AztecAddress::from_field(0x24877c50868f86712240eb535d90d1c97403d074805dd3758c3aecb02958f8d4); global DEPLOYER_CONTRACT_ADDRESS = AztecAddress::from_field(0x2ab1a2bd6d07d8d61ea56d85861446349e52c6b7c0612b702cb1e6db6ad0b089); global REGISTERER_CONTRACT_ADDRESS = AztecAddress::from_field(0x05d15342d76e46e5be07d3cda0d753158431cdc5e39d29ce4e8fe1f5c070564a); diff --git a/yarn-project/circuits.js/src/constants.gen.ts b/yarn-project/circuits.js/src/constants.gen.ts index 2348c3ec7ac..0952b7f1a84 100644 --- a/yarn-project/circuits.js/src/constants.gen.ts +++ b/yarn-project/circuits.js/src/constants.gen.ts @@ -114,7 +114,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 = - 21209182303070804160941065409360795406831433542792830301721453026531461944353n; + 9694109890306420370616891858093188542026876097103155811681068343994212062621n; 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 96bafbc6875..a5d6f7673af 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', - 'NewKeyRegistry', + 'KeyRegistry', '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 32196437bf8..8aeaed0089e 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 { NewKeyRegistryContract: KeyRegistryContract } = await import('@aztec/noir-contracts.js'); + const { 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 f98c3605b84..c97dadd7d8c 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 { NewKeyRegistryContract, TestContract } from '@aztec/noir-contracts.js'; +import { KeyRegistryContract, TestContract } from '@aztec/noir-contracts.js'; import { getCanonicalKeyRegistryAddress } from '@aztec/protocol-contracts/key-registry'; import { jest } from '@jest/globals'; @@ -10,7 +10,7 @@ import { publicDeployAccounts, setup } from './fixtures/utils.js'; const TIMEOUT = 120_000; describe('Key Registry', () => { - let keyRegistry: NewKeyRegistryContract; + let keyRegistry: KeyRegistryContract; let pxe: PXE; let testContract: TestContract; @@ -24,7 +24,7 @@ describe('Key Registry', () => { beforeAll(async () => { ({ teardown, pxe, wallets } = await setup(2)); - keyRegistry = await NewKeyRegistryContract.at(getCanonicalKeyRegistryAddress(), wallets[0]); + keyRegistry = await KeyRegistryContract.at(getCanonicalKeyRegistryAddress(), wallets[0]); testContract = await TestContract.deploy(wallets[0]).send().deployed(); diff --git a/yarn-project/end-to-end/src/fixtures/utils.ts b/yarn-project/end-to-end/src/fixtures/utils.ts index 160449b6df7..1d9e5a2ec66 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, NewKeyRegistryContract } from '@aztec/noir-contracts.js'; +import { AuthRegistryContract, KeyRegistryContract } 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'; @@ -691,7 +691,7 @@ export async function deployCanonicalKeyRegistry(deployer: Wallet) { return; } - const keyRegistry = await NewKeyRegistryContract.deploy(deployer) + const keyRegistry = await KeyRegistryContract.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 ca6cba229d5..d9f4a60a25c 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 - new_key_registry_contract-NewKeyRegistry + key_registry_contract-KeyRegistry 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 72bbe2ebe82..5feb280a624 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 NewKeyRegistryJson from '../../artifacts/NewKeyRegistry.json' assert { type: 'json' }; +import KeyRegistryJson from '../../artifacts/KeyRegistry.json' assert { type: 'json' }; -export const KeyRegistryArtifact = loadContractArtifact(NewKeyRegistryJson as NoirCompiledContract); +export const KeyRegistryArtifact = loadContractArtifact(KeyRegistryJson as NoirCompiledContract);