diff --git a/contracts/UPGRADE_GUIDE.md b/contracts/UPGRADE_GUIDE.md index 58d8fa307..08e5c96b8 100644 --- a/contracts/UPGRADE_GUIDE.md +++ b/contracts/UPGRADE_GUIDE.md @@ -6,7 +6,7 @@ ```bash CELO_RPC_URL=https://forno.celo.org CELO_PRIVATE_KEY= -CRITICAL_GOVERNANCE_ADDRESS= +SECURITY_GOVERNANCE_ADDRESS= STANDARD_GOVERNANCE_ADDRESS= ``` @@ -70,8 +70,8 @@ forge script script/MigratePCR0Manager.s.sol:MigratePCR0Manager \ # Check governance roles cast call 0xe57F4773bd9c9d8b6Cd70431117d353298B9f5BF \ "hasRole(bytes32,address)" \ - $(cast keccak "CRITICAL_ROLE") \ - $CRITICAL_GOVERNANCE_ADDRESS \ + $(cast keccak "SECURITY_ROLE") \ + $SECURITY_GOVERNANCE_ADDRESS \ --rpc-url $CELO_RPC_URL # Check PCR0 values migrated (example) @@ -96,4 +96,4 @@ cast call \ If issues occur, the multisigs can: 1. Deploy previous implementation versions -2. Use `upgradeTo()` to revert (requires `CRITICAL_ROLE`) +2. Use `upgradeTo()` to revert (requires `SECURITY_ROLE`) diff --git a/contracts/contracts/IdentityVerificationHubImplV1.sol b/contracts/contracts/IdentityVerificationHubImplV1.sol index 798f6e140..3a4478f72 100644 --- a/contracts/contracts/IdentityVerificationHubImplV1.sol +++ b/contracts/contracts/IdentityVerificationHubImplV1.sol @@ -406,7 +406,7 @@ contract IdentityVerificationHubImplV1 is IdentityVerificationHubStorageV1, IIde * @notice Updates the registry address. * @param registryAddress The new registry address. */ - function updateRegistry(address registryAddress) external virtual onlyProxy onlyRole(CRITICAL_ROLE) { + function updateRegistry(address registryAddress) external virtual onlyProxy onlyRole(SECURITY_ROLE) { _registry = registryAddress; emit RegistryUpdated(registryAddress); } @@ -417,7 +417,7 @@ contract IdentityVerificationHubImplV1 is IdentityVerificationHubStorageV1, IIde */ function updateVcAndDiscloseCircuit( address vcAndDiscloseCircuitVerifierAddress - ) external virtual onlyProxy onlyRole(CRITICAL_ROLE) { + ) external virtual onlyProxy onlyRole(SECURITY_ROLE) { _vcAndDiscloseCircuitVerifier = vcAndDiscloseCircuitVerifierAddress; emit VcAndDiscloseCircuitUpdated(vcAndDiscloseCircuitVerifierAddress); } @@ -430,7 +430,7 @@ contract IdentityVerificationHubImplV1 is IdentityVerificationHubStorageV1, IIde function updateRegisterCircuitVerifier( uint256 typeId, address verifierAddress - ) external virtual onlyProxy onlyRole(CRITICAL_ROLE) { + ) external virtual onlyProxy onlyRole(SECURITY_ROLE) { _sigTypeToRegisterCircuitVerifiers[typeId] = verifierAddress; emit RegisterCircuitVerifierUpdated(typeId, verifierAddress); } @@ -440,7 +440,7 @@ contract IdentityVerificationHubImplV1 is IdentityVerificationHubStorageV1, IIde * @param typeId The signature type identifier. * @param verifierAddress The new DSC circuit verifier address. */ - function updateDscVerifier(uint256 typeId, address verifierAddress) external virtual onlyProxy onlyRole(CRITICAL_ROLE) { + function updateDscVerifier(uint256 typeId, address verifierAddress) external virtual onlyProxy onlyRole(SECURITY_ROLE) { _sigTypeToDscCircuitVerifiers[typeId] = verifierAddress; emit DscCircuitVerifierUpdated(typeId, verifierAddress); } @@ -453,7 +453,7 @@ contract IdentityVerificationHubImplV1 is IdentityVerificationHubStorageV1, IIde function batchUpdateRegisterCircuitVerifiers( uint256[] calldata typeIds, address[] calldata verifierAddresses - ) external virtual onlyProxy onlyRole(CRITICAL_ROLE) { + ) external virtual onlyProxy onlyRole(SECURITY_ROLE) { if (typeIds.length != verifierAddresses.length) { revert LENGTH_MISMATCH(); } @@ -471,7 +471,7 @@ contract IdentityVerificationHubImplV1 is IdentityVerificationHubStorageV1, IIde function batchUpdateDscCircuitVerifiers( uint256[] calldata typeIds, address[] calldata verifierAddresses - ) external virtual onlyProxy onlyRole(CRITICAL_ROLE) { + ) external virtual onlyProxy onlyRole(SECURITY_ROLE) { if (typeIds.length != verifierAddresses.length) { revert LENGTH_MISMATCH(); } diff --git a/contracts/contracts/IdentityVerificationHubImplV2.sol b/contracts/contracts/IdentityVerificationHubImplV2.sol index ad073e644..8a9f50ef2 100644 --- a/contracts/contracts/IdentityVerificationHubImplV2.sol +++ b/contracts/contracts/IdentityVerificationHubImplV2.sol @@ -28,7 +28,7 @@ import {Formatter} from "./libraries/Formatter.sol"; * @custom:version 2.12.0 * @custom:version-history * - v2.11.0 (Initializer v11): V2 hub deployment with Ownable2StepUpgradeable governance - * - v2.12.0 (Initializer v12): Governance upgrade - migrated to AccessControlUpgradeable with multi-tier governance (CRITICAL_ROLE, STANDARD_ROLE) + * - v2.12.0 (Initializer v12): Governance upgrade - migrated to AccessControlUpgradeable with multi-tier governance (SECURITY_ROLE, OPERATIONS_ROLE) */ contract IdentityVerificationHubImplV2 is ImplRoot { /// @custom:storage-location erc7201:self.storage.IdentityVerificationHub @@ -357,7 +357,7 @@ contract IdentityVerificationHubImplV2 is ImplRoot { * @notice Updates the AADHAAR registration window. * @param window The new AADHAAR registration window. */ - function setAadhaarRegistrationWindow(uint256 window) external virtual onlyProxy onlyRole(CRITICAL_ROLE) { + function setAadhaarRegistrationWindow(uint256 window) external virtual onlyProxy onlyRole(SECURITY_ROLE) { AADHAAR_REGISTRATION_WINDOW = window; } @@ -400,7 +400,7 @@ contract IdentityVerificationHubImplV2 is ImplRoot { * @notice Updates the registry address. * @param registryAddress The new registry address. */ - function updateRegistry(bytes32 attestationId, address registryAddress) external virtual onlyProxy onlyRole(CRITICAL_ROLE) { + function updateRegistry(bytes32 attestationId, address registryAddress) external virtual onlyProxy onlyRole(SECURITY_ROLE) { IdentityVerificationHubStorage storage $ = _getIdentityVerificationHubStorage(); $._registries[attestationId] = registryAddress; emit RegistryUpdated(attestationId, registryAddress); @@ -413,7 +413,7 @@ contract IdentityVerificationHubImplV2 is ImplRoot { function updateVcAndDiscloseCircuit( bytes32 attestationId, address vcAndDiscloseCircuitVerifierAddress - ) external virtual onlyProxy onlyRole(CRITICAL_ROLE) { + ) external virtual onlyProxy onlyRole(SECURITY_ROLE) { IdentityVerificationHubStorage storage $ = _getIdentityVerificationHubStorage(); $._discloseVerifiers[attestationId] = vcAndDiscloseCircuitVerifierAddress; emit VcAndDiscloseCircuitUpdated(attestationId, vcAndDiscloseCircuitVerifierAddress); @@ -429,7 +429,7 @@ contract IdentityVerificationHubImplV2 is ImplRoot { bytes32 attestationId, uint256 typeId, address verifierAddress - ) external virtual onlyProxy onlyRole(CRITICAL_ROLE) { + ) external virtual onlyProxy onlyRole(SECURITY_ROLE) { IdentityVerificationHubStorage storage $ = _getIdentityVerificationHubStorage(); $._registerCircuitVerifiers[attestationId][typeId] = verifierAddress; emit RegisterCircuitVerifierUpdated(typeId, verifierAddress); @@ -445,7 +445,7 @@ contract IdentityVerificationHubImplV2 is ImplRoot { bytes32 attestationId, uint256 typeId, address verifierAddress - ) external virtual onlyProxy onlyRole(CRITICAL_ROLE) { + ) external virtual onlyProxy onlyRole(SECURITY_ROLE) { IdentityVerificationHubStorage storage $ = _getIdentityVerificationHubStorage(); $._dscCircuitVerifiers[attestationId][typeId] = verifierAddress; emit DscCircuitVerifierUpdated(typeId, verifierAddress); @@ -461,7 +461,7 @@ contract IdentityVerificationHubImplV2 is ImplRoot { bytes32[] calldata attestationIds, uint256[] calldata typeIds, address[] calldata verifierAddresses - ) external virtual onlyProxy onlyRole(CRITICAL_ROLE) { + ) external virtual onlyProxy onlyRole(SECURITY_ROLE) { if (attestationIds.length != typeIds.length || attestationIds.length != verifierAddresses.length) { revert LengthMismatch(); } @@ -482,7 +482,7 @@ contract IdentityVerificationHubImplV2 is ImplRoot { bytes32[] calldata attestationIds, uint256[] calldata typeIds, address[] calldata verifierAddresses - ) external virtual onlyProxy onlyRole(CRITICAL_ROLE) { + ) external virtual onlyProxy onlyRole(SECURITY_ROLE) { if (attestationIds.length != typeIds.length || attestationIds.length != verifierAddresses.length) { revert LengthMismatch(); } diff --git a/contracts/contracts/registry/IdentityRegistryAadhaarImplV1.sol b/contracts/contracts/registry/IdentityRegistryAadhaarImplV1.sol index a28031fc9..27e99701e 100644 --- a/contracts/contracts/registry/IdentityRegistryAadhaarImplV1.sol +++ b/contracts/contracts/registry/IdentityRegistryAadhaarImplV1.sol @@ -74,7 +74,7 @@ abstract contract IdentityRegistryAadhaarStorageV1 is ImplRoot { * @custom:version 1.2.0 * @custom:version-history * - v1.1.0 (Initializer v1): Initial deployment with Ownable2StepUpgradeable governance - * - v1.2.0 (Initializer v2): Migrated to AccessControlUpgradeable with multi-tier governance (CRITICAL_ROLE, STANDARD_ROLE) + * - v1.2.0 (Initializer v2): Migrated to AccessControlUpgradeable with multi-tier governance (SECURITY_ROLE, OPERATIONS_ROLE) */ contract IdentityRegistryAadhaarImplV1 is IdentityRegistryAadhaarStorageV1, IIdentityRegistryAadhaarV1 { using InternalLeanIMT for LeanIMTData; @@ -299,7 +299,7 @@ contract IdentityRegistryAadhaarImplV1 is IdentityRegistryAadhaarStorageV1, IIde /// @notice Updates the hub address. /// @dev Callable only via a proxy and restricted to the contract owner. /// @param newHubAddress The new address of the hub. - function updateHub(address newHubAddress) external onlyProxy onlyRole(CRITICAL_ROLE) { + function updateHub(address newHubAddress) external onlyProxy onlyRole(SECURITY_ROLE) { if (newHubAddress == address(0)) revert HUB_ADDRESS_ZERO(); _hub = newHubAddress; emit HubUpdated(newHubAddress); @@ -308,7 +308,7 @@ contract IdentityRegistryAadhaarImplV1 is IdentityRegistryAadhaarStorageV1, IIde /// @notice Updates the name and date of birth OFAC root. /// @dev Callable only via a proxy and restricted to the contract owner. /// @param newNameAndDobOfacRoot The new name and date of birth OFAC root value. - function updateNameAndDobOfacRoot(uint256 newNameAndDobOfacRoot) external onlyProxy onlyRole(CRITICAL_ROLE) { + function updateNameAndDobOfacRoot(uint256 newNameAndDobOfacRoot) external onlyProxy onlyRole(SECURITY_ROLE) { _nameAndDobOfacRoot = newNameAndDobOfacRoot; emit NameAndDobOfacRootUpdated(newNameAndDobOfacRoot); } @@ -316,7 +316,7 @@ contract IdentityRegistryAadhaarImplV1 is IdentityRegistryAadhaarStorageV1, IIde /// @notice Updates the name and year of birth OFAC root. /// @dev Callable only via a proxy and restricted to the contract owner. /// @param newNameAndYobOfacRoot The new name and year of birth OFAC root value. - function updateNameAndYobOfacRoot(uint256 newNameAndYobOfacRoot) external onlyProxy onlyRole(CRITICAL_ROLE) { + function updateNameAndYobOfacRoot(uint256 newNameAndYobOfacRoot) external onlyProxy onlyRole(SECURITY_ROLE) { _nameAndYobOfacRoot = newNameAndYobOfacRoot; emit NameAndYobOfacRootUpdated(newNameAndYobOfacRoot); } @@ -324,7 +324,7 @@ contract IdentityRegistryAadhaarImplV1 is IdentityRegistryAadhaarStorageV1, IIde /// @notice Registers a new UIDAI pubkey commitment. /// @dev Callable only via a proxy and restricted to the contract owner. /// @param commitment The UIDAI pubkey commitment to register. - function registerUidaiPubkeyCommitment(uint256 commitment) external onlyProxy onlyRole(CRITICAL_ROLE) { + function registerUidaiPubkeyCommitment(uint256 commitment) external onlyProxy onlyRole(SECURITY_ROLE) { _uidaiPubkeyCommitments[commitment] = true; emit UidaiPubkeyCommitmentRegistered(commitment, block.timestamp); } @@ -332,7 +332,7 @@ contract IdentityRegistryAadhaarImplV1 is IdentityRegistryAadhaarStorageV1, IIde /// @notice Removes a UIDAI pubkey commitment. /// @dev Callable only via a proxy and restricted to the contract owner. /// @param commitment The UIDAI pubkey commitment to remove. - function removeUidaiPubkeyCommitment(uint256 commitment) external onlyProxy onlyRole(CRITICAL_ROLE) { + function removeUidaiPubkeyCommitment(uint256 commitment) external onlyProxy onlyRole(SECURITY_ROLE) { delete _uidaiPubkeyCommitments[commitment]; emit UidaiPubkeyCommitmentRemoved(commitment, block.timestamp); } @@ -340,7 +340,7 @@ contract IdentityRegistryAadhaarImplV1 is IdentityRegistryAadhaarStorageV1, IIde /// @notice Updates a UIDAI pubkey commitment. /// @dev Callable only via a proxy and restricted to the contract owner. /// @param commitment The UIDAI pubkey commitment to update. - function updateUidaiPubkeyCommitment(uint256 commitment) external onlyProxy onlyRole(CRITICAL_ROLE) { + function updateUidaiPubkeyCommitment(uint256 commitment) external onlyProxy onlyRole(SECURITY_ROLE) { _uidaiPubkeyCommitments[commitment] = true; emit UidaiPubkeyCommitmentUpdated(commitment, block.timestamp); } @@ -354,7 +354,7 @@ contract IdentityRegistryAadhaarImplV1 is IdentityRegistryAadhaarStorageV1, IIde bytes32 attestationId, uint256 nullifier, uint256 commitment - ) external onlyProxy onlyRole(CRITICAL_ROLE) { + ) external onlyProxy onlyRole(SECURITY_ROLE) { _nullifiers[nullifier] = true; uint256 imt_root = _identityCommitmentIMT._insert(commitment); _rootTimestamps[imt_root] = block.timestamp; @@ -371,7 +371,7 @@ contract IdentityRegistryAadhaarImplV1 is IdentityRegistryAadhaarStorageV1, IIde uint256 oldLeaf, uint256 newLeaf, uint256[] calldata siblingNodes - ) external onlyProxy onlyRole(CRITICAL_ROLE) { + ) external onlyProxy onlyRole(SECURITY_ROLE) { uint256 imt_root = _identityCommitmentIMT._update(oldLeaf, newLeaf, siblingNodes); _rootTimestamps[imt_root] = block.timestamp; emit DevCommitmentUpdated(oldLeaf, newLeaf, imt_root, block.timestamp); @@ -381,7 +381,7 @@ contract IdentityRegistryAadhaarImplV1 is IdentityRegistryAadhaarStorageV1, IIde /// @dev Caller must be the owner. Provides sibling nodes for proof of position. /// @param oldLeaf The identity commitment to remove. /// @param siblingNodes An array of sibling nodes for Merkle proof generation. - function devRemoveCommitment(uint256 oldLeaf, uint256[] calldata siblingNodes) external onlyProxy onlyRole(CRITICAL_ROLE) { + function devRemoveCommitment(uint256 oldLeaf, uint256[] calldata siblingNodes) external onlyProxy onlyRole(SECURITY_ROLE) { uint256 imt_root = _identityCommitmentIMT._remove(oldLeaf, siblingNodes); _rootTimestamps[imt_root] = block.timestamp; emit DevCommitmentRemoved(oldLeaf, imt_root, block.timestamp); diff --git a/contracts/contracts/registry/IdentityRegistryIdCardImplV1.sol b/contracts/contracts/registry/IdentityRegistryIdCardImplV1.sol index 78b2a3626..a7132c526 100644 --- a/contracts/contracts/registry/IdentityRegistryIdCardImplV1.sol +++ b/contracts/contracts/registry/IdentityRegistryIdCardImplV1.sol @@ -81,7 +81,7 @@ abstract contract IdentityRegistryIdCardStorageV1 is ImplRoot { * @custom:version 1.2.0 * @custom:version-history * - v1.1.0 (Initializer v1): Initial deployment with Ownable2StepUpgradeable governance - * - v1.2.0 (Initializer v2): Migrated to AccessControlUpgradeable with multi-tier governance (CRITICAL_ROLE, STANDARD_ROLE) + * - v1.2.0 (Initializer v2): Migrated to AccessControlUpgradeable with multi-tier governance (SECURITY_ROLE, OPERATIONS_ROLE) */ contract IdentityRegistryIdCardImplV1 is IdentityRegistryIdCardStorageV1, IIdentityRegistryIdCardV1 { using InternalLeanIMT for LeanIMTData; @@ -399,7 +399,7 @@ contract IdentityRegistryIdCardImplV1 is IdentityRegistryIdCardStorageV1, IIdent * @dev Callable only via a proxy and restricted to the contract owner. * @param newHubAddress The new address of the hub. */ - function updateHub(address newHubAddress) external onlyProxy onlyRole(CRITICAL_ROLE) { + function updateHub(address newHubAddress) external onlyProxy onlyRole(SECURITY_ROLE) { _hub = newHubAddress; emit HubUpdated(newHubAddress); } @@ -409,7 +409,7 @@ contract IdentityRegistryIdCardImplV1 is IdentityRegistryIdCardStorageV1, IIdent * @dev Callable only via a proxy and restricted to the contract owner. * @param newNameAndDobOfacRoot The new name and date of birth OFAC root value. */ - function updateNameAndDobOfacRoot(uint256 newNameAndDobOfacRoot) external onlyProxy onlyRole(CRITICAL_ROLE) { + function updateNameAndDobOfacRoot(uint256 newNameAndDobOfacRoot) external onlyProxy onlyRole(SECURITY_ROLE) { _nameAndDobOfacRoot = newNameAndDobOfacRoot; emit NameAndDobOfacRootUpdated(newNameAndDobOfacRoot); } @@ -419,7 +419,7 @@ contract IdentityRegistryIdCardImplV1 is IdentityRegistryIdCardStorageV1, IIdent * @dev Callable only via a proxy and restricted to the contract owner. * @param newNameAndYobOfacRoot The new name and year of birth OFAC root value. */ - function updateNameAndYobOfacRoot(uint256 newNameAndYobOfacRoot) external onlyProxy onlyRole(CRITICAL_ROLE) { + function updateNameAndYobOfacRoot(uint256 newNameAndYobOfacRoot) external onlyProxy onlyRole(SECURITY_ROLE) { _nameAndYobOfacRoot = newNameAndYobOfacRoot; emit NameAndYobOfacRootUpdated(newNameAndYobOfacRoot); } @@ -429,7 +429,7 @@ contract IdentityRegistryIdCardImplV1 is IdentityRegistryIdCardStorageV1, IIdent * @dev Callable only via a proxy and restricted to the contract owner. * @param newCscaRoot The new CSCA root value. */ - function updateCscaRoot(uint256 newCscaRoot) external onlyProxy onlyRole(CRITICAL_ROLE) { + function updateCscaRoot(uint256 newCscaRoot) external onlyProxy onlyRole(SECURITY_ROLE) { _cscaRoot = newCscaRoot; emit CscaRootUpdated(newCscaRoot); } @@ -445,7 +445,7 @@ contract IdentityRegistryIdCardImplV1 is IdentityRegistryIdCardStorageV1, IIdent bytes32 attestationId, uint256 nullifier, uint256 commitment - ) external onlyProxy onlyRole(CRITICAL_ROLE) { + ) external onlyProxy onlyRole(SECURITY_ROLE) { _nullifiers[attestationId][nullifier] = true; uint256 imt_root = _addCommitment(_identityCommitmentIMT, commitment); _rootTimestamps[imt_root] = block.timestamp; @@ -464,7 +464,7 @@ contract IdentityRegistryIdCardImplV1 is IdentityRegistryIdCardStorageV1, IIdent uint256 oldLeaf, uint256 newLeaf, uint256[] calldata siblingNodes - ) external onlyProxy onlyRole(CRITICAL_ROLE) { + ) external onlyProxy onlyRole(SECURITY_ROLE) { uint256 imt_root = _updateCommitment(_identityCommitmentIMT, oldLeaf, newLeaf, siblingNodes); _rootTimestamps[imt_root] = block.timestamp; emit DevCommitmentUpdated(oldLeaf, newLeaf, imt_root, block.timestamp); @@ -476,7 +476,7 @@ contract IdentityRegistryIdCardImplV1 is IdentityRegistryIdCardStorageV1, IIdent * @param oldLeaf The identity commitment to remove. * @param siblingNodes An array of sibling nodes for Merkle proof generation. */ - function devRemoveCommitment(uint256 oldLeaf, uint256[] calldata siblingNodes) external onlyProxy onlyRole(CRITICAL_ROLE) { + function devRemoveCommitment(uint256 oldLeaf, uint256[] calldata siblingNodes) external onlyProxy onlyRole(SECURITY_ROLE) { uint256 imt_root = _removeCommitment(_identityCommitmentIMT, oldLeaf, siblingNodes); _rootTimestamps[imt_root] = block.timestamp; emit DevCommitmentRemoved(oldLeaf, imt_root, block.timestamp); @@ -487,7 +487,7 @@ contract IdentityRegistryIdCardImplV1 is IdentityRegistryIdCardStorageV1, IIdent * @dev Callable only by the owner for testing or administration. * @param dscCommitment The DSC key commitment to add. */ - function devAddDscKeyCommitment(uint256 dscCommitment) external onlyProxy onlyRole(CRITICAL_ROLE) { + function devAddDscKeyCommitment(uint256 dscCommitment) external onlyProxy onlyRole(SECURITY_ROLE) { _isRegisteredDscKeyCommitment[dscCommitment] = true; uint256 imt_root = _addCommitment(_dscKeyCommitmentIMT, dscCommitment); uint256 index = _dscKeyCommitmentIMT._indexOf(dscCommitment); @@ -505,7 +505,7 @@ contract IdentityRegistryIdCardImplV1 is IdentityRegistryIdCardStorageV1, IIdent uint256 oldLeaf, uint256 newLeaf, uint256[] calldata siblingNodes - ) external onlyProxy onlyRole(CRITICAL_ROLE) { + ) external onlyProxy onlyRole(SECURITY_ROLE) { uint256 imt_root = _updateCommitment(_dscKeyCommitmentIMT, oldLeaf, newLeaf, siblingNodes); emit DevDscKeyCommitmentUpdated(oldLeaf, newLeaf, imt_root); } @@ -516,7 +516,7 @@ contract IdentityRegistryIdCardImplV1 is IdentityRegistryIdCardStorageV1, IIdent * @param oldLeaf The DSC key commitment to remove. * @param siblingNodes An array of sibling nodes for Merkle proof generation. */ - function devRemoveDscKeyCommitment(uint256 oldLeaf, uint256[] calldata siblingNodes) external onlyProxy onlyRole(CRITICAL_ROLE) { + function devRemoveDscKeyCommitment(uint256 oldLeaf, uint256[] calldata siblingNodes) external onlyProxy onlyRole(SECURITY_ROLE) { uint256 imt_root = _removeCommitment(_dscKeyCommitmentIMT, oldLeaf, siblingNodes); emit DevDscKeyCommitmentRemoved(oldLeaf, imt_root); } @@ -532,7 +532,7 @@ contract IdentityRegistryIdCardImplV1 is IdentityRegistryIdCardStorageV1, IIdent bytes32 attestationId, uint256 nullifier, bool state - ) external onlyProxy onlyRole(CRITICAL_ROLE) { + ) external onlyProxy onlyRole(SECURITY_ROLE) { _nullifiers[attestationId][nullifier] = state; emit DevNullifierStateChanged(attestationId, nullifier, state); } @@ -543,7 +543,7 @@ contract IdentityRegistryIdCardImplV1 is IdentityRegistryIdCardStorageV1, IIdent * @param dscCommitment The DSC key commitment. * @param state The new state of the DSC key commitment (true for registered, false for not registered). */ - function devChangeDscKeyCommitmentState(uint256 dscCommitment, bool state) external onlyProxy onlyRole(CRITICAL_ROLE) { + function devChangeDscKeyCommitmentState(uint256 dscCommitment, bool state) external onlyProxy onlyRole(SECURITY_ROLE) { _isRegisteredDscKeyCommitment[dscCommitment] = state; emit DevDscKeyCommitmentStateChanged(dscCommitment, state); } diff --git a/contracts/contracts/registry/IdentityRegistryImplV1.sol b/contracts/contracts/registry/IdentityRegistryImplV1.sol index 7a98c370a..c0a81d666 100644 --- a/contracts/contracts/registry/IdentityRegistryImplV1.sol +++ b/contracts/contracts/registry/IdentityRegistryImplV1.sol @@ -86,7 +86,7 @@ abstract contract IdentityRegistryStorageV1 is ImplRoot { * @custom:version 1.2.0 * @custom:version-history * - v1.1.0 (Initializer v1): Initial deployment with Ownable2StepUpgradeable governance - * - v1.2.0 (Initializer v2): Migrated to AccessControlUpgradeable with multi-tier governance (CRITICAL_ROLE, STANDARD_ROLE) + * - v1.2.0 (Initializer v2): Migrated to AccessControlUpgradeable with multi-tier governance (SECURITY_ROLE, OPERATIONS_ROLE) */ contract IdentityRegistryImplV1 is IdentityRegistryStorageV1, IIdentityRegistryV1 { using InternalLeanIMT for LeanIMTData; @@ -422,7 +422,7 @@ contract IdentityRegistryImplV1 is IdentityRegistryStorageV1, IIdentityRegistryV * @dev Callable only via a proxy and restricted to the contract owner. * @param newHubAddress The new address of the hub. */ - function updateHub(address newHubAddress) external onlyProxy onlyRole(CRITICAL_ROLE) { + function updateHub(address newHubAddress) external onlyProxy onlyRole(SECURITY_ROLE) { _hub = newHubAddress; emit HubUpdated(newHubAddress); } @@ -432,7 +432,7 @@ contract IdentityRegistryImplV1 is IdentityRegistryStorageV1, IIdentityRegistryV * @dev Callable only via a proxy and restricted to the contract owner. * @param newPassportNoOfacRoot The new passport number OFAC root value. */ - function updatePassportNoOfacRoot(uint256 newPassportNoOfacRoot) external onlyProxy onlyRole(CRITICAL_ROLE) { + function updatePassportNoOfacRoot(uint256 newPassportNoOfacRoot) external onlyProxy onlyRole(SECURITY_ROLE) { _passportNoOfacRoot = newPassportNoOfacRoot; emit PassportNoOfacRootUpdated(newPassportNoOfacRoot); } @@ -442,7 +442,7 @@ contract IdentityRegistryImplV1 is IdentityRegistryStorageV1, IIdentityRegistryV * @dev Callable only via a proxy and restricted to the contract owner. * @param newNameAndDobOfacRoot The new name and date of birth OFAC root value. */ - function updateNameAndDobOfacRoot(uint256 newNameAndDobOfacRoot) external onlyProxy onlyRole(CRITICAL_ROLE) { + function updateNameAndDobOfacRoot(uint256 newNameAndDobOfacRoot) external onlyProxy onlyRole(SECURITY_ROLE) { _nameAndDobOfacRoot = newNameAndDobOfacRoot; emit NameAndDobOfacRootUpdated(newNameAndDobOfacRoot); } @@ -452,7 +452,7 @@ contract IdentityRegistryImplV1 is IdentityRegistryStorageV1, IIdentityRegistryV * @dev Callable only via a proxy and restricted to the contract owner. * @param newNameAndYobOfacRoot The new name and year of birth OFAC root value. */ - function updateNameAndYobOfacRoot(uint256 newNameAndYobOfacRoot) external onlyProxy onlyRole(CRITICAL_ROLE) { + function updateNameAndYobOfacRoot(uint256 newNameAndYobOfacRoot) external onlyProxy onlyRole(SECURITY_ROLE) { _nameAndYobOfacRoot = newNameAndYobOfacRoot; emit NameAndYobOfacRootUpdated(newNameAndYobOfacRoot); } @@ -462,7 +462,7 @@ contract IdentityRegistryImplV1 is IdentityRegistryStorageV1, IIdentityRegistryV * @dev Callable only via a proxy and restricted to the contract owner. * @param newCscaRoot The new CSCA root value. */ - function updateCscaRoot(uint256 newCscaRoot) external onlyProxy onlyRole(CRITICAL_ROLE) { + function updateCscaRoot(uint256 newCscaRoot) external onlyProxy onlyRole(SECURITY_ROLE) { _cscaRoot = newCscaRoot; emit CscaRootUpdated(newCscaRoot); } @@ -478,7 +478,7 @@ contract IdentityRegistryImplV1 is IdentityRegistryStorageV1, IIdentityRegistryV bytes32 attestationId, uint256 nullifier, uint256 commitment - ) external onlyProxy onlyRole(CRITICAL_ROLE) { + ) external onlyProxy onlyRole(SECURITY_ROLE) { _nullifiers[attestationId][nullifier] = true; uint256 imt_root = _addCommitment(_identityCommitmentIMT, commitment); _rootTimestamps[imt_root] = block.timestamp; @@ -497,7 +497,7 @@ contract IdentityRegistryImplV1 is IdentityRegistryStorageV1, IIdentityRegistryV uint256 oldLeaf, uint256 newLeaf, uint256[] calldata siblingNodes - ) external onlyProxy onlyRole(CRITICAL_ROLE) { + ) external onlyProxy onlyRole(SECURITY_ROLE) { uint256 imt_root = _updateCommitment(_identityCommitmentIMT, oldLeaf, newLeaf, siblingNodes); _rootTimestamps[imt_root] = block.timestamp; emit DevCommitmentUpdated(oldLeaf, newLeaf, imt_root, block.timestamp); @@ -509,7 +509,7 @@ contract IdentityRegistryImplV1 is IdentityRegistryStorageV1, IIdentityRegistryV * @param oldLeaf The identity commitment to remove. * @param siblingNodes An array of sibling nodes for Merkle proof generation. */ - function devRemoveCommitment(uint256 oldLeaf, uint256[] calldata siblingNodes) external onlyProxy onlyRole(CRITICAL_ROLE) { + function devRemoveCommitment(uint256 oldLeaf, uint256[] calldata siblingNodes) external onlyProxy onlyRole(SECURITY_ROLE) { uint256 imt_root = _removeCommitment(_identityCommitmentIMT, oldLeaf, siblingNodes); _rootTimestamps[imt_root] = block.timestamp; emit DevCommitmentRemoved(oldLeaf, imt_root, block.timestamp); @@ -520,7 +520,7 @@ contract IdentityRegistryImplV1 is IdentityRegistryStorageV1, IIdentityRegistryV * @dev Callable only by the owner for testing or administration. * @param dscCommitment The DSC key commitment to add. */ - function devAddDscKeyCommitment(uint256 dscCommitment) external onlyProxy onlyRole(CRITICAL_ROLE) { + function devAddDscKeyCommitment(uint256 dscCommitment) external onlyProxy onlyRole(SECURITY_ROLE) { _isRegisteredDscKeyCommitment[dscCommitment] = true; uint256 imt_root = _addCommitment(_dscKeyCommitmentIMT, dscCommitment); uint256 index = _dscKeyCommitmentIMT._indexOf(dscCommitment); @@ -538,7 +538,7 @@ contract IdentityRegistryImplV1 is IdentityRegistryStorageV1, IIdentityRegistryV uint256 oldLeaf, uint256 newLeaf, uint256[] calldata siblingNodes - ) external onlyProxy onlyRole(CRITICAL_ROLE) { + ) external onlyProxy onlyRole(SECURITY_ROLE) { uint256 imt_root = _updateCommitment(_dscKeyCommitmentIMT, oldLeaf, newLeaf, siblingNodes); emit DevDscKeyCommitmentUpdated(oldLeaf, newLeaf, imt_root); } @@ -549,7 +549,7 @@ contract IdentityRegistryImplV1 is IdentityRegistryStorageV1, IIdentityRegistryV * @param oldLeaf The DSC key commitment to remove. * @param siblingNodes An array of sibling nodes for Merkle proof generation. */ - function devRemoveDscKeyCommitment(uint256 oldLeaf, uint256[] calldata siblingNodes) external onlyProxy onlyRole(CRITICAL_ROLE) { + function devRemoveDscKeyCommitment(uint256 oldLeaf, uint256[] calldata siblingNodes) external onlyProxy onlyRole(SECURITY_ROLE) { uint256 imt_root = _removeCommitment(_dscKeyCommitmentIMT, oldLeaf, siblingNodes); emit DevDscKeyCommitmentRemoved(oldLeaf, imt_root); } @@ -565,7 +565,7 @@ contract IdentityRegistryImplV1 is IdentityRegistryStorageV1, IIdentityRegistryV bytes32 attestationId, uint256 nullifier, bool state - ) external onlyProxy onlyRole(CRITICAL_ROLE) { + ) external onlyProxy onlyRole(SECURITY_ROLE) { _nullifiers[attestationId][nullifier] = state; emit DevNullifierStateChanged(attestationId, nullifier, state); } @@ -576,7 +576,7 @@ contract IdentityRegistryImplV1 is IdentityRegistryStorageV1, IIdentityRegistryV * @param dscCommitment The DSC key commitment. * @param state The new state of the DSC key commitment (true for registered, false for not registered). */ - function devChangeDscKeyCommitmentState(uint256 dscCommitment, bool state) external onlyProxy onlyRole(CRITICAL_ROLE) { + function devChangeDscKeyCommitmentState(uint256 dscCommitment, bool state) external onlyProxy onlyRole(SECURITY_ROLE) { _isRegisteredDscKeyCommitment[dscCommitment] = state; emit DevDscKeyCommitmentStateChanged(dscCommitment, state); } diff --git a/contracts/contracts/sdk/VerifyAll.sol b/contracts/contracts/sdk/VerifyAll.sol index aa5091825..03f92348d 100644 --- a/contracts/contracts/sdk/VerifyAll.sol +++ b/contracts/contracts/sdk/VerifyAll.sol @@ -11,10 +11,10 @@ import {CircuitConstants} from "../constants/CircuitConstants.sol"; /// @dev This contract interacts with IdentityVerificationHub and IdentityRegistry contract VerifyAll is AccessControl { /// @notice Critical operations and role management requiring 3/5 multisig consensus - bytes32 public constant CRITICAL_ROLE = keccak256("CRITICAL_ROLE"); + bytes32 public constant SECURITY_ROLE = keccak256("SECURITY_ROLE"); /// @notice Standard operations requiring 2/5 multisig consensus - bytes32 public constant STANDARD_ROLE = keccak256("STANDARD_ROLE"); + bytes32 public constant OPERATIONS_ROLE = keccak256("OPERATIONS_ROLE"); IIdentityVerificationHubV1 public hub; IIdentityRegistryV1 public registry; @@ -30,12 +30,12 @@ contract VerifyAll is AccessControl { registry = IIdentityRegistryV1(registryAddress); // Grant all roles to deployer initially - _grantRole(CRITICAL_ROLE, msg.sender); - _grantRole(STANDARD_ROLE, msg.sender); + _grantRole(SECURITY_ROLE, msg.sender); + _grantRole(OPERATIONS_ROLE, msg.sender); - // Set role admins - CRITICAL_ROLE manages all roles - _setRoleAdmin(CRITICAL_ROLE, CRITICAL_ROLE); - _setRoleAdmin(STANDARD_ROLE, CRITICAL_ROLE); + // Set role admins - SECURITY_ROLE manages all roles + _setRoleAdmin(SECURITY_ROLE, SECURITY_ROLE); + _setRoleAdmin(OPERATIONS_ROLE, SECURITY_ROLE); } /// @notice Verifies identity proof and reveals selected data @@ -124,15 +124,15 @@ contract VerifyAll is AccessControl { /// @notice Updates the hub contract address /// @param hubAddress The new hub contract address - /// @dev Only callable by accounts with CRITICAL_ROLE - function setHub(address hubAddress) external onlyRole(CRITICAL_ROLE) { + /// @dev Only callable by accounts with SECURITY_ROLE + function setHub(address hubAddress) external onlyRole(SECURITY_ROLE) { hub = IIdentityVerificationHubV1(hubAddress); } /// @notice Updates the registry contract address /// @param registryAddress The new registry contract address - /// @dev Only callable by accounts with CRITICAL_ROLE - function setRegistry(address registryAddress) external onlyRole(CRITICAL_ROLE) { + /// @dev Only callable by accounts with SECURITY_ROLE + function setRegistry(address registryAddress) external onlyRole(SECURITY_ROLE) { registry = IIdentityRegistryV1(registryAddress); } } diff --git a/contracts/contracts/tests/MockUpgradedHub.sol b/contracts/contracts/tests/MockUpgradedHub.sol index 4f25cffad..b91ad97e6 100644 --- a/contracts/contracts/tests/MockUpgradedHub.sol +++ b/contracts/contracts/tests/MockUpgradedHub.sol @@ -40,19 +40,19 @@ contract MockUpgradedHub is ImplRoot { } /** - * @notice Updates the registry address (now requires CRITICAL_ROLE) + * @notice Updates the registry address (now requires SECURITY_ROLE) * @param registryAddress The new registry address */ - function updateRegistry(address registryAddress) external onlyRole(CRITICAL_ROLE) { + function updateRegistry(address registryAddress) external onlyRole(SECURITY_ROLE) { _registry = registryAddress; emit RegistryUpdated(registryAddress); } /** - * @notice Updates circuit version (requires CRITICAL_ROLE) + * @notice Updates circuit version (requires SECURITY_ROLE) * @param version The new circuit version */ - function updateCircuitVersion(uint256 version) external onlyRole(CRITICAL_ROLE) { + function updateCircuitVersion(uint256 version) external onlyRole(SECURITY_ROLE) { _circuitVersion = version; } @@ -77,6 +77,6 @@ contract MockUpgradedHub is ImplRoot { function verifyStorageMigration() external view returns (bool) { // The important thing is that the contract is functional and governance works // Registry and circuit version should be preserved, deprecated owner may be zero - return hasRole(CRITICAL_ROLE, msg.sender) || hasRole(STANDARD_ROLE, msg.sender); + return hasRole(SECURITY_ROLE, msg.sender) || hasRole(OPERATIONS_ROLE, msg.sender); } } diff --git a/contracts/contracts/tests/MockUpgradedRegistry.sol b/contracts/contracts/tests/MockUpgradedRegistry.sol index 2fb6cc309..2365284b7 100644 --- a/contracts/contracts/tests/MockUpgradedRegistry.sol +++ b/contracts/contracts/tests/MockUpgradedRegistry.sol @@ -46,37 +46,37 @@ contract MockUpgradedRegistry is ImplRoot { } /** - * @notice Sets the hub address (now requires CRITICAL_ROLE) + * @notice Sets the hub address (now requires SECURITY_ROLE) * @param hubAddress The new hub address */ - function setHub(address hubAddress) external onlyRole(CRITICAL_ROLE) { + function setHub(address hubAddress) external onlyRole(SECURITY_ROLE) { _hub = hubAddress; emit HubUpdated(hubAddress); } /** - * @notice Updates the hub address (now requires CRITICAL_ROLE) + * @notice Updates the hub address (now requires SECURITY_ROLE) * @param hubAddress The new hub address */ - function updateHub(address hubAddress) external onlyRole(CRITICAL_ROLE) { + function updateHub(address hubAddress) external onlyRole(SECURITY_ROLE) { _hub = hubAddress; emit HubUpdated(hubAddress); } /** - * @notice Updates the CSCA root (now requires CRITICAL_ROLE) + * @notice Updates the CSCA root (now requires SECURITY_ROLE) * @param cscaRoot The new CSCA root */ - function updateCscaRoot(bytes32 cscaRoot) external onlyRole(CRITICAL_ROLE) { + function updateCscaRoot(bytes32 cscaRoot) external onlyRole(SECURITY_ROLE) { _cscaRoot = cscaRoot; emit CscaRootUpdated(cscaRoot); } /** - * @notice Adds a commitment (now requires CRITICAL_ROLE) + * @notice Adds a commitment (now requires SECURITY_ROLE) * @param commitment The commitment to add */ - function addCommitment(bytes32 commitment) external onlyRole(CRITICAL_ROLE) { + function addCommitment(bytes32 commitment) external onlyRole(SECURITY_ROLE) { _commitments[commitment] = true; } @@ -101,4 +101,3 @@ contract MockUpgradedRegistry is ImplRoot { return _commitments[commitment]; } } - diff --git a/contracts/contracts/upgradeable/ImplRoot.sol b/contracts/contracts/upgradeable/ImplRoot.sol index b429b339a..c5304fc22 100644 --- a/contracts/contracts/upgradeable/ImplRoot.sol +++ b/contracts/contracts/upgradeable/ImplRoot.sol @@ -11,15 +11,15 @@ import {AccessControlUpgradeable} from "@openzeppelin/contracts-upgradeable/acce * Serves as a base for upgradeable implementations. * * Governance Roles: - * - CRITICAL_ROLE: Critical operations and role management (3/5 multisig consensus) - * - STANDARD_ROLE: Standard operations (2/5 multisig consensus) + * - SECURITY_ROLE: Security-sensitive operations and role management (3/5 multisig consensus) + * - OPERATIONS_ROLE: Routine operational tasks (2/5 multisig consensus) */ abstract contract ImplRoot is UUPSUpgradeable, AccessControlUpgradeable { - /// @notice Critical operations requiring 3/5 multisig consensus - bytes32 public constant CRITICAL_ROLE = keccak256("CRITICAL_ROLE"); + /// @notice Security-sensitive operations requiring 3/5 multisig consensus + bytes32 public constant SECURITY_ROLE = keccak256("SECURITY_ROLE"); - /// @notice Standard operations requiring 2/5 multisig consensus - bytes32 public constant STANDARD_ROLE = keccak256("STANDARD_ROLE"); + /// @notice Routine operations requiring 2/5 multisig consensus + bytes32 public constant OPERATIONS_ROLE = keccak256("OPERATIONS_ROLE"); // Reserved storage space to allow for layout changes in the future. uint256[50] private __gap; @@ -34,21 +34,21 @@ abstract contract ImplRoot is UUPSUpgradeable, AccessControlUpgradeable { __AccessControl_init(); __UUPSUpgradeable_init(); - _grantRole(CRITICAL_ROLE, msg.sender); - _grantRole(STANDARD_ROLE, msg.sender); + _grantRole(SECURITY_ROLE, msg.sender); + _grantRole(OPERATIONS_ROLE, msg.sender); - // Set role admins - CRITICAL_ROLE manages all roles - _setRoleAdmin(CRITICAL_ROLE, CRITICAL_ROLE); - _setRoleAdmin(STANDARD_ROLE, CRITICAL_ROLE); + // Set role admins - SECURITY_ROLE manages all roles + _setRoleAdmin(SECURITY_ROLE, SECURITY_ROLE); + _setRoleAdmin(OPERATIONS_ROLE, SECURITY_ROLE); } /** * @dev Authorizes an upgrade to a new implementation. * Requirements: * - Must be called through a proxy. - * - Caller must have CRITICAL_ROLE. + * - Caller must have SECURITY_ROLE. * * @param newImplementation The address of the new implementation contract. */ - function _authorizeUpgrade(address newImplementation) internal virtual override onlyProxy onlyRole(CRITICAL_ROLE) {} + function _authorizeUpgrade(address newImplementation) internal virtual override onlyProxy onlyRole(SECURITY_ROLE) {} } diff --git a/contracts/contracts/utils/PCR0Manager.sol b/contracts/contracts/utils/PCR0Manager.sol index 3e0baa361..739e7ca4c 100644 --- a/contracts/contracts/utils/PCR0Manager.sol +++ b/contracts/contracts/utils/PCR0Manager.sol @@ -8,23 +8,23 @@ import {AccessControl} from "@openzeppelin/contracts/access/AccessControl.sol"; * @notice This contract manages a mapping of PCR0 values (provided as a 48-byte value) * to booleans. The PCR0 value (the 48-byte SHA384 output) is hashed * using keccak256 and then stored in the mapping. - * Only accounts with CRITICAL_ROLE can add or remove entries. + * Only accounts with SECURITY_ROLE can add or remove entries. */ contract PCR0Manager is AccessControl { /// @notice Critical operations and role management requiring 3/5 multisig consensus - bytes32 public constant CRITICAL_ROLE = keccak256("CRITICAL_ROLE"); + bytes32 public constant SECURITY_ROLE = keccak256("SECURITY_ROLE"); /// @notice Standard operations requiring 2/5 multisig consensus - bytes32 public constant STANDARD_ROLE = keccak256("STANDARD_ROLE"); + bytes32 public constant OPERATIONS_ROLE = keccak256("OPERATIONS_ROLE"); constructor() { // Grant all roles to deployer initially - _grantRole(CRITICAL_ROLE, msg.sender); - _grantRole(STANDARD_ROLE, msg.sender); + _grantRole(SECURITY_ROLE, msg.sender); + _grantRole(OPERATIONS_ROLE, msg.sender); - // Set role admins - CRITICAL_ROLE is admin of both roles - _setRoleAdmin(CRITICAL_ROLE, CRITICAL_ROLE); - _setRoleAdmin(STANDARD_ROLE, CRITICAL_ROLE); + // Set role admins - SECURITY_ROLE is admin of both roles + _setRoleAdmin(SECURITY_ROLE, SECURITY_ROLE); + _setRoleAdmin(OPERATIONS_ROLE, SECURITY_ROLE); } // Mapping from keccak256(pcr0) to its boolean state. @@ -44,7 +44,7 @@ contract PCR0Manager is AccessControl { * @dev Reverts if the PCR0 value is not 32 bytes or if it is already set. * @dev Pads the PCR0 value to 48 bytes by prefixing 16 zero bytes to maintain mobile app compatibility. */ - function addPCR0(bytes calldata pcr0) external onlyRole(CRITICAL_ROLE) { + function addPCR0(bytes calldata pcr0) external onlyRole(SECURITY_ROLE) { require(pcr0.length == 32, "PCR0 must be 32 bytes"); bytes memory paddedPcr0 = abi.encodePacked(new bytes(16), pcr0); bytes32 key = keccak256(paddedPcr0); @@ -59,7 +59,7 @@ contract PCR0Manager is AccessControl { * @dev Reverts if the PCR0 value is not 32 bytes or if it is not currently set. * @dev Pads the PCR0 value to 48 bytes by prefixing 16 zero bytes to maintain mobile app compatibility. */ - function removePCR0(bytes calldata pcr0) external onlyRole(CRITICAL_ROLE) { + function removePCR0(bytes calldata pcr0) external onlyRole(SECURITY_ROLE) { require(pcr0.length == 32, "PCR0 must be 32 bytes"); bytes memory paddedPcr0 = abi.encodePacked(new bytes(16), pcr0); bytes32 key = keccak256(paddedPcr0); diff --git a/contracts/script/MigratePCR0Manager.s.sol b/contracts/script/MigratePCR0Manager.s.sol index fc4968e0b..5628d0173 100644 --- a/contracts/script/MigratePCR0Manager.s.sol +++ b/contracts/script/MigratePCR0Manager.s.sol @@ -18,19 +18,19 @@ import {PCR0Manager} from "../contracts/utils/PCR0Manager.sol"; * * Usage: * - Set in .env file: - * CRITICAL_GOVERNANCE_ADDRESS=0x... - * STANDARD_GOVERNANCE_ADDRESS=0x... + * SECURITY_GOVERNANCE_ADDRESS=0x... + * OPERATIONS_GOVERNANCE_ADDRESS=0x... * - Dry run: forge script script/MigratePCR0Manager.s.sol --fork-url $CELO_RPC_URL -vvv * - Execute: forge script script/MigratePCR0Manager.s.sol --rpc-url https://forno.celo.org --broadcast --verify -vvv */ contract MigratePCR0Manager is Script { // Governance roles - bytes32 public constant CRITICAL_ROLE = keccak256("CRITICAL_ROLE"); - bytes32 public constant STANDARD_ROLE = keccak256("STANDARD_ROLE"); + bytes32 public constant SECURITY_ROLE = keccak256("SECURITY_ROLE"); + bytes32 public constant OPERATIONS_ROLE = keccak256("OPERATIONS_ROLE"); // Multisig addresses (from environment) - address criticalMultisig; - address standardMultisig; + address securityMultisig; + address operationsMultisig; function run() external returns (address newPCR0Manager) { console2.log("================================================================================"); @@ -41,15 +41,15 @@ contract MigratePCR0Manager is Script { console2.log("Chain ID:", block.chainid); // Get multisig addresses from .env - criticalMultisig = vm.envAddress("CRITICAL_GOVERNANCE_ADDRESS"); - standardMultisig = vm.envAddress("STANDARD_GOVERNANCE_ADDRESS"); + securityMultisig = vm.envAddress("SECURITY_GOVERNANCE_ADDRESS"); + operationsMultisig = vm.envAddress("OPERATIONS_GOVERNANCE_ADDRESS"); - require(criticalMultisig != address(0), "CRITICAL_GOVERNANCE_ADDRESS not set in .env"); - require(standardMultisig != address(0), "STANDARD_GOVERNANCE_ADDRESS not set in .env"); + require(securityMultisig != address(0), "SECURITY_GOVERNANCE_ADDRESS not set in .env"); + require(operationsMultisig != address(0), "OPERATIONS_GOVERNANCE_ADDRESS not set in .env"); console2.log("\nGovernance addresses:"); - console2.log(" Critical Multisig:", criticalMultisig); - console2.log(" Standard Multisig:", standardMultisig); + console2.log(" Critical Multisig:", securityMultisig); + console2.log(" Standard Multisig:", operationsMultisig); // Get finalized PCR0 values bytes[] memory pcr0Values = getFinalizedPCR0Values(); @@ -73,17 +73,17 @@ contract MigratePCR0Manager is Script { // Step 3: Transfer roles to multisigs console2.log("\n=== Step 3: Transfer Roles to Multisigs ==="); - pcr0Manager.grantRole(CRITICAL_ROLE, criticalMultisig); - pcr0Manager.grantRole(STANDARD_ROLE, standardMultisig); - console2.log(" Granted CRITICAL_ROLE to:", criticalMultisig); - console2.log(" Granted STANDARD_ROLE to:", standardMultisig); + pcr0Manager.grantRole(SECURITY_ROLE, securityMultisig); + pcr0Manager.grantRole(OPERATIONS_ROLE, operationsMultisig); + console2.log(" Granted SECURITY_ROLE to:", securityMultisig); + console2.log(" Granted OPERATIONS_ROLE to:", operationsMultisig); // Step 4: Deployer renounces roles console2.log("\n=== Step 4: Deployer Renounces All Roles ==="); - pcr0Manager.renounceRole(CRITICAL_ROLE, msg.sender); - pcr0Manager.renounceRole(STANDARD_ROLE, msg.sender); - console2.log(" Deployer renounced CRITICAL_ROLE"); - console2.log(" Deployer renounced STANDARD_ROLE"); + pcr0Manager.renounceRole(SECURITY_ROLE, msg.sender); + pcr0Manager.renounceRole(OPERATIONS_ROLE, msg.sender); + console2.log(" Deployer renounced SECURITY_ROLE"); + console2.log(" Deployer renounced OPERATIONS_ROLE"); vm.stopBroadcast(); @@ -97,8 +97,8 @@ contract MigratePCR0Manager is Script { console2.log("\nNew PCR0Manager:", newPCR0Manager); console2.log("Total PCR0 values:", pcr0Values.length); console2.log("Governance:"); - console2.log(" Critical Multisig:", criticalMultisig); - console2.log(" Standard Multisig:", standardMultisig); + console2.log(" Critical Multisig:", securityMultisig); + console2.log(" Standard Multisig:", operationsMultisig); console2.log("\nNext steps:"); console2.log("1. Update Hub to point to new PCR0Manager"); console2.log("2. Update documentation with new address"); @@ -139,17 +139,17 @@ contract MigratePCR0Manager is Script { console2.log(" [PASS] All", pcr0Values.length, "PCR0 values verified"); // Verify deployer has no roles - bool deployerHasCritical = pcr0Manager.hasRole(CRITICAL_ROLE, msg.sender); - bool deployerHasStandard = pcr0Manager.hasRole(STANDARD_ROLE, msg.sender); - require(!deployerHasCritical, "Deployer still has CRITICAL_ROLE"); - require(!deployerHasStandard, "Deployer still has STANDARD_ROLE"); + bool deployerHasCritical = pcr0Manager.hasRole(SECURITY_ROLE, msg.sender); + bool deployerHasStandard = pcr0Manager.hasRole(OPERATIONS_ROLE, msg.sender); + require(!deployerHasCritical, "Deployer still has SECURITY_ROLE"); + require(!deployerHasStandard, "Deployer still has OPERATIONS_ROLE"); console2.log(" [PASS] Deployer has no roles"); // Verify multisigs have roles - bool criticalHasRole = pcr0Manager.hasRole(CRITICAL_ROLE, criticalMultisig); - bool standardHasRole = pcr0Manager.hasRole(STANDARD_ROLE, standardMultisig); - require(criticalHasRole, "Critical multisig missing CRITICAL_ROLE"); - require(standardHasRole, "Standard multisig missing STANDARD_ROLE"); + bool criticalHasRole = pcr0Manager.hasRole(SECURITY_ROLE, securityMultisig); + bool standardHasRole = pcr0Manager.hasRole(OPERATIONS_ROLE, operationsMultisig); + require(criticalHasRole, "Critical multisig missing SECURITY_ROLE"); + require(standardHasRole, "Standard multisig missing OPERATIONS_ROLE"); console2.log(" [PASS] Multisigs have correct roles"); console2.log("\n [SUCCESS] All verifications passed!"); diff --git a/contracts/script/UpgradeToAccessControl.s.sol b/contracts/script/UpgradeToAccessControl.s.sol index ebcc020ca..7011874b1 100644 --- a/contracts/script/UpgradeToAccessControl.s.sol +++ b/contracts/script/UpgradeToAccessControl.s.sol @@ -23,8 +23,8 @@ import {IdentityRegistryAadhaarImplV1} from "../contracts/registry/IdentityRegis * * Usage: * - Set in .env file: - * CRITICAL_GOVERNANCE_ADDRESS=0x... - * STANDARD_GOVERNANCE_ADDRESS=0x... + * SECURITY_GOVERNANCE_ADDRESS=0x... + * OPERATIONS_GOVERNANCE_ADDRESS=0x... * * - Dry run: * forge script script/UpgradeToAccessControl.s.sol \ @@ -55,12 +55,12 @@ contract UpgradeToAccessControl is Script { address constant REGISTRY_AADHAAR_PROXY = 0xd603Fa8C8f4694E8DD1DcE1f27C0C3fc91e32Ac4; // IdentityRegistry (Aadhaar) proxy // Governance roles - bytes32 public constant CRITICAL_ROLE = keccak256("CRITICAL_ROLE"); - bytes32 public constant STANDARD_ROLE = keccak256("STANDARD_ROLE"); + bytes32 public constant SECURITY_ROLE = keccak256("SECURITY_ROLE"); + bytes32 public constant OPERATIONS_ROLE = keccak256("OPERATIONS_ROLE"); // Multisig addresses (from environment) - address criticalMultisig; - address standardMultisig; + address securityMultisig; + address operationsMultisig; function run() external { console2.log("================================================================================"); @@ -71,15 +71,15 @@ contract UpgradeToAccessControl is Script { console2.log("Chain ID:", block.chainid); // Get multisig addresses from .env - criticalMultisig = vm.envAddress("CRITICAL_GOVERNANCE_ADDRESS"); - standardMultisig = vm.envAddress("STANDARD_GOVERNANCE_ADDRESS"); + securityMultisig = vm.envAddress("SECURITY_GOVERNANCE_ADDRESS"); + operationsMultisig = vm.envAddress("OPERATIONS_GOVERNANCE_ADDRESS"); - require(criticalMultisig != address(0), "CRITICAL_GOVERNANCE_ADDRESS not set in .env"); - require(standardMultisig != address(0), "STANDARD_GOVERNANCE_ADDRESS not set in .env"); + require(securityMultisig != address(0), "SECURITY_GOVERNANCE_ADDRESS not set in .env"); + require(operationsMultisig != address(0), "OPERATIONS_GOVERNANCE_ADDRESS not set in .env"); console2.log("\nGovernance addresses (roles will be transferred):"); - console2.log(" Critical Multisig:", criticalMultisig); - console2.log(" Standard Multisig:", standardMultisig); + console2.log(" Critical Multisig:", securityMultisig); + console2.log(" Standard Multisig:", operationsMultisig); // Start broadcasting transactions vm.startBroadcast(); @@ -131,8 +131,8 @@ contract UpgradeToAccessControl is Script { // Verify governance roles IdentityVerificationHubImplV2 hub = IdentityVerificationHubImplV2(HUB_PROXY); - require(hub.hasRole(CRITICAL_ROLE, msg.sender), "Deployer missing CRITICAL_ROLE"); - require(hub.hasRole(STANDARD_ROLE, msg.sender), "Deployer missing STANDARD_ROLE"); + require(hub.hasRole(SECURITY_ROLE, msg.sender), "Deployer missing SECURITY_ROLE"); + require(hub.hasRole(OPERATIONS_ROLE, msg.sender), "Deployer missing OPERATIONS_ROLE"); console2.log(" Governance: Deployer has both roles"); } @@ -154,8 +154,8 @@ contract UpgradeToAccessControl is Script { // Verify governance roles IdentityRegistryImplV1 registry = IdentityRegistryImplV1(REGISTRY_PASSPORT_PROXY); - require(registry.hasRole(CRITICAL_ROLE, msg.sender), "Deployer missing CRITICAL_ROLE"); - require(registry.hasRole(STANDARD_ROLE, msg.sender), "Deployer missing STANDARD_ROLE"); + require(registry.hasRole(SECURITY_ROLE, msg.sender), "Deployer missing SECURITY_ROLE"); + require(registry.hasRole(OPERATIONS_ROLE, msg.sender), "Deployer missing OPERATIONS_ROLE"); console2.log(" Governance: Deployer has both roles"); } @@ -179,8 +179,8 @@ contract UpgradeToAccessControl is Script { // Verify governance roles IdentityRegistryIdCardImplV1 registry = IdentityRegistryIdCardImplV1(REGISTRY_ID_CARD_PROXY); - require(registry.hasRole(CRITICAL_ROLE, msg.sender), "Deployer missing CRITICAL_ROLE"); - require(registry.hasRole(STANDARD_ROLE, msg.sender), "Deployer missing STANDARD_ROLE"); + require(registry.hasRole(SECURITY_ROLE, msg.sender), "Deployer missing SECURITY_ROLE"); + require(registry.hasRole(OPERATIONS_ROLE, msg.sender), "Deployer missing OPERATIONS_ROLE"); console2.log(" Governance: Deployer has both roles"); } @@ -202,8 +202,8 @@ contract UpgradeToAccessControl is Script { // Verify governance roles IdentityRegistryAadhaarImplV1 registry = IdentityRegistryAadhaarImplV1(REGISTRY_AADHAAR_PROXY); - require(registry.hasRole(CRITICAL_ROLE, msg.sender), "Deployer missing CRITICAL_ROLE"); - require(registry.hasRole(STANDARD_ROLE, msg.sender), "Deployer missing STANDARD_ROLE"); + require(registry.hasRole(SECURITY_ROLE, msg.sender), "Deployer missing SECURITY_ROLE"); + require(registry.hasRole(OPERATIONS_ROLE, msg.sender), "Deployer missing OPERATIONS_ROLE"); console2.log(" Governance: Deployer has both roles"); } @@ -218,44 +218,44 @@ contract UpgradeToAccessControl is Script { // Grant roles to multisigs console2.log(" Granting roles to multisigs..."); - hub.grantRole(CRITICAL_ROLE, criticalMultisig); - hub.grantRole(STANDARD_ROLE, standardMultisig); + hub.grantRole(SECURITY_ROLE, securityMultisig); + hub.grantRole(OPERATIONS_ROLE, operationsMultisig); - passportRegistry.grantRole(CRITICAL_ROLE, criticalMultisig); - passportRegistry.grantRole(STANDARD_ROLE, standardMultisig); + passportRegistry.grantRole(SECURITY_ROLE, securityMultisig); + passportRegistry.grantRole(OPERATIONS_ROLE, operationsMultisig); - idCardRegistry.grantRole(CRITICAL_ROLE, criticalMultisig); - idCardRegistry.grantRole(STANDARD_ROLE, standardMultisig); + idCardRegistry.grantRole(SECURITY_ROLE, securityMultisig); + idCardRegistry.grantRole(OPERATIONS_ROLE, operationsMultisig); - aadhaarRegistry.grantRole(CRITICAL_ROLE, criticalMultisig); - aadhaarRegistry.grantRole(STANDARD_ROLE, standardMultisig); + aadhaarRegistry.grantRole(SECURITY_ROLE, securityMultisig); + aadhaarRegistry.grantRole(OPERATIONS_ROLE, operationsMultisig); // Deployer renounces roles console2.log(" Deployer renouncing all roles..."); - hub.renounceRole(CRITICAL_ROLE, msg.sender); - hub.renounceRole(STANDARD_ROLE, msg.sender); + hub.renounceRole(SECURITY_ROLE, msg.sender); + hub.renounceRole(OPERATIONS_ROLE, msg.sender); - passportRegistry.renounceRole(CRITICAL_ROLE, msg.sender); - passportRegistry.renounceRole(STANDARD_ROLE, msg.sender); + passportRegistry.renounceRole(SECURITY_ROLE, msg.sender); + passportRegistry.renounceRole(OPERATIONS_ROLE, msg.sender); - idCardRegistry.renounceRole(CRITICAL_ROLE, msg.sender); - idCardRegistry.renounceRole(STANDARD_ROLE, msg.sender); + idCardRegistry.renounceRole(SECURITY_ROLE, msg.sender); + idCardRegistry.renounceRole(OPERATIONS_ROLE, msg.sender); - aadhaarRegistry.renounceRole(CRITICAL_ROLE, msg.sender); - aadhaarRegistry.renounceRole(STANDARD_ROLE, msg.sender); + aadhaarRegistry.renounceRole(SECURITY_ROLE, msg.sender); + aadhaarRegistry.renounceRole(OPERATIONS_ROLE, msg.sender); console2.log(" Status: Roles transferred successfully"); // Verify deployer has no roles - require(!hub.hasRole(CRITICAL_ROLE, msg.sender), "Hub: Deployer still has CRITICAL_ROLE"); - require(!hub.hasRole(STANDARD_ROLE, msg.sender), "Hub: Deployer still has STANDARD_ROLE"); - require(!passportRegistry.hasRole(CRITICAL_ROLE, msg.sender), "Passport: Deployer still has CRITICAL_ROLE"); - require(!idCardRegistry.hasRole(CRITICAL_ROLE, msg.sender), "IDCard: Deployer still has CRITICAL_ROLE"); - require(!aadhaarRegistry.hasRole(CRITICAL_ROLE, msg.sender), "Aadhaar: Deployer still has CRITICAL_ROLE"); + require(!hub.hasRole(SECURITY_ROLE, msg.sender), "Hub: Deployer still has SECURITY_ROLE"); + require(!hub.hasRole(OPERATIONS_ROLE, msg.sender), "Hub: Deployer still has OPERATIONS_ROLE"); + require(!passportRegistry.hasRole(SECURITY_ROLE, msg.sender), "Passport: Deployer still has SECURITY_ROLE"); + require(!idCardRegistry.hasRole(SECURITY_ROLE, msg.sender), "IDCard: Deployer still has SECURITY_ROLE"); + require(!aadhaarRegistry.hasRole(SECURITY_ROLE, msg.sender), "Aadhaar: Deployer still has SECURITY_ROLE"); // Verify multisigs have roles - require(hub.hasRole(CRITICAL_ROLE, criticalMultisig), "Hub: Critical multisig missing CRITICAL_ROLE"); - require(hub.hasRole(STANDARD_ROLE, standardMultisig), "Hub: Standard multisig missing STANDARD_ROLE"); + require(hub.hasRole(SECURITY_ROLE, securityMultisig), "Hub: Critical multisig missing SECURITY_ROLE"); + require(hub.hasRole(OPERATIONS_ROLE, operationsMultisig), "Hub: Standard multisig missing OPERATIONS_ROLE"); console2.log(" Verification: Deployer has ZERO control"); console2.log(" Verification: Multisigs have full control"); diff --git a/contracts/scripts/testUpgradeLogic.ts b/contracts/scripts/testUpgradeLogic.ts index f3a0a25d8..b10dd5cac 100644 --- a/contracts/scripts/testUpgradeLogic.ts +++ b/contracts/scripts/testUpgradeLogic.ts @@ -94,12 +94,12 @@ async function testHubUpgradeLogic(customVerifierAddress: string) { // Verify governance roles const [deployer] = await ethers.getSigners(); - const hasRole = await hubContract.hasRole(await hubContract.CRITICAL_ROLE(), deployer.address); + const hasRole = await hubContract.hasRole(await hubContract.SECURITY_ROLE(), deployer.address); if (hasRole) { - log.success("✅ Governance verification: deployer has CRITICAL_ROLE"); + log.success("✅ Governance verification: deployer has SECURITY_ROLE"); } else { - log.error("❌ Governance verification failed: deployer doesn't have CRITICAL_ROLE"); + log.error("❌ Governance verification failed: deployer doesn't have SECURITY_ROLE"); } } @@ -152,12 +152,12 @@ async function testRegistryUpgradeLogic() { // Verify governance roles const [deployer] = await ethers.getSigners(); - const hasRole = await registryContract.hasRole(await registryContract.CRITICAL_ROLE(), deployer.address); + const hasRole = await registryContract.hasRole(await registryContract.SECURITY_ROLE(), deployer.address); if (hasRole) { - log.success("✅ Governance verification: deployer has CRITICAL_ROLE"); + log.success("✅ Governance verification: deployer has SECURITY_ROLE"); } else { - log.error("❌ Governance verification failed: deployer doesn't have CRITICAL_ROLE"); + log.error("❌ Governance verification failed: deployer doesn't have SECURITY_ROLE"); } } diff --git a/contracts/scripts/transferRolesToMultisigs.ts b/contracts/scripts/transferRolesToMultisigs.ts index c9e055748..57caa350f 100644 --- a/contracts/scripts/transferRolesToMultisigs.ts +++ b/contracts/scripts/transferRolesToMultisigs.ts @@ -172,12 +172,12 @@ async function transferUtilityContractRoles() { async function transferRolesForContract(contract: any, contractName: string) { const [deployer] = await ethers.getSigners(); - const CRITICAL_ROLE = await contract.CRITICAL_ROLE(); - const STANDARD_ROLE = await contract.STANDARD_ROLE(); + const SECURITY_ROLE = await contract.SECURITY_ROLE(); + const OPERATIONS_ROLE = await contract.OPERATIONS_ROLE(); // Check current roles - const deployerHasCritical = await contract.hasRole(CRITICAL_ROLE, deployer.address); - const deployerHasStandard = await contract.hasRole(STANDARD_ROLE, deployer.address); + const deployerHasCritical = await contract.hasRole(SECURITY_ROLE, deployer.address); + const deployerHasStandard = await contract.hasRole(OPERATIONS_ROLE, deployer.address); if (!deployerHasCritical || !deployerHasStandard) { log.warning(`⚠️ Deployer doesn't have all roles for ${contractName}, skipping...`); @@ -185,22 +185,22 @@ async function transferRolesForContract(contract: any, contractName: string) { } if (DRY_RUN) { - log.info(`[DRY RUN] Would grant CRITICAL_ROLE to ${CRITICAL_MULTISIG}`); - log.info(`[DRY RUN] Would grant STANDARD_ROLE to ${STANDARD_MULTISIG}`); + log.info(`[DRY RUN] Would grant SECURITY_ROLE to ${CRITICAL_MULTISIG}`); + log.info(`[DRY RUN] Would grant OPERATIONS_ROLE to ${STANDARD_MULTISIG}`); log.info(`[DRY RUN] Would renounce deployer roles`); return; } // Grant roles to multisigs - log.info(`Granting CRITICAL_ROLE to ${CRITICAL_MULTISIG}...`); - await contract.connect(deployer).grantRole(CRITICAL_ROLE, CRITICAL_MULTISIG); + log.info(`Granting SECURITY_ROLE to ${CRITICAL_MULTISIG}...`); + await contract.connect(deployer).grantRole(SECURITY_ROLE, CRITICAL_MULTISIG); - log.info(`Granting STANDARD_ROLE to ${STANDARD_MULTISIG}...`); - await contract.connect(deployer).grantRole(STANDARD_ROLE, STANDARD_MULTISIG); + log.info(`Granting OPERATIONS_ROLE to ${STANDARD_MULTISIG}...`); + await contract.connect(deployer).grantRole(OPERATIONS_ROLE, STANDARD_MULTISIG); // Verify roles were granted - const criticalGranted = await contract.hasRole(CRITICAL_ROLE, CRITICAL_MULTISIG); - const standardGranted = await contract.hasRole(STANDARD_ROLE, STANDARD_MULTISIG); + const criticalGranted = await contract.hasRole(SECURITY_ROLE, CRITICAL_MULTISIG); + const standardGranted = await contract.hasRole(OPERATIONS_ROLE, STANDARD_MULTISIG); if (!criticalGranted || !standardGranted) { throw new Error(`Failed to grant roles to multisigs for ${contractName}`); @@ -208,12 +208,12 @@ async function transferRolesForContract(contract: any, contractName: string) { // Renounce deployer roles log.info(`Renouncing deployer roles...`); - await contract.connect(deployer).renounceRole(CRITICAL_ROLE, deployer.address); - await contract.connect(deployer).renounceRole(STANDARD_ROLE, deployer.address); + await contract.connect(deployer).renounceRole(SECURITY_ROLE, deployer.address); + await contract.connect(deployer).renounceRole(OPERATIONS_ROLE, deployer.address); // Verify roles were renounced - const deployerStillHasCritical = await contract.hasRole(CRITICAL_ROLE, deployer.address); - const deployerStillHasStandard = await contract.hasRole(STANDARD_ROLE, deployer.address); + const deployerStillHasCritical = await contract.hasRole(SECURITY_ROLE, deployer.address); + const deployerStillHasStandard = await contract.hasRole(OPERATIONS_ROLE, deployer.address); if (deployerStillHasCritical || deployerStillHasStandard) { throw new Error(`Failed to renounce deployer roles for ${contractName}`); diff --git a/contracts/scripts/upgradeToMultisig.ts b/contracts/scripts/upgradeToMultisig.ts index a6d4f0797..6b7931d13 100644 --- a/contracts/scripts/upgradeToMultisig.ts +++ b/contracts/scripts/upgradeToMultisig.ts @@ -156,11 +156,11 @@ async function upgradeIdentityVerificationHub() { // Verify the upgrade was successful by checking the implementation const [deployer] = await ethers.getSigners(); try { - const hasRole = await hubContract.hasRole(await hubContract.CRITICAL_ROLE(), deployer.address); + const hasRole = await hubContract.hasRole(await hubContract.SECURITY_ROLE(), deployer.address); if (hasRole) { - log.success("✅ Governance verification: deployer has CRITICAL_ROLE"); + log.success("✅ Governance verification: deployer has SECURITY_ROLE"); } else { - log.warning("⚠️ Governance verification: deployer doesn't have CRITICAL_ROLE (may be expected)"); + log.warning("⚠️ Governance verification: deployer doesn't have SECURITY_ROLE (may be expected)"); } } catch (roleError) { log.warning(`⚠️ Could not verify governance roles: ${roleError}`); diff --git a/contracts/test/foundry/MigratePCR0Manager.t.sol b/contracts/test/foundry/MigratePCR0Manager.t.sol index 4845a13ff..d1c01b84e 100644 --- a/contracts/test/foundry/MigratePCR0Manager.t.sol +++ b/contracts/test/foundry/MigratePCR0Manager.t.sol @@ -21,16 +21,16 @@ import {PCR0Manager} from "../../contracts/utils/PCR0Manager.sol"; contract MigratePCR0ManagerTest is Test { // Test accounts address deployer; - address criticalMultisig; - address standardMultisig; + address securityMultisig; + address operationsMultisig; address unauthorized; // Contracts PCR0Manager pcr0Manager; // Governance roles - bytes32 public constant CRITICAL_ROLE = keccak256("CRITICAL_ROLE"); - bytes32 public constant STANDARD_ROLE = keccak256("STANDARD_ROLE"); + bytes32 public constant SECURITY_ROLE = keccak256("SECURITY_ROLE"); + bytes32 public constant OPERATIONS_ROLE = keccak256("OPERATIONS_ROLE"); // Finalized PCR0 values (32-byte format) bytes[] pcr0Values; @@ -42,15 +42,15 @@ contract MigratePCR0ManagerTest is Test { // Set up test accounts deployer = makeAddr("deployer"); - criticalMultisig = makeAddr("criticalMultisig"); - standardMultisig = makeAddr("standardMultisig"); + securityMultisig = makeAddr("securityMultisig"); + operationsMultisig = makeAddr("operationsMultisig"); unauthorized = makeAddr("unauthorized"); vm.deal(deployer, 100 ether); console2.log("Deployer:", deployer); - console2.log("Critical Multisig:", criticalMultisig); - console2.log("Standard Multisig:", standardMultisig); + console2.log("Critical Multisig:", securityMultisig); + console2.log("Standard Multisig:", operationsMultisig); // Populate finalized PCR0 values (32-byte format) pcr0Values.push(hex"eb71776987d5f057030823f591d160c9d5d5e0a96c9a2a826778be1da2b8302a"); @@ -70,8 +70,8 @@ contract MigratePCR0ManagerTest is Test { vm.stopPrank(); console2.log("Deployed at:", address(pcr0Manager)); - assertTrue(pcr0Manager.hasRole(CRITICAL_ROLE, deployer), "Deployer missing CRITICAL_ROLE"); - assertTrue(pcr0Manager.hasRole(STANDARD_ROLE, deployer), "Deployer missing STANDARD_ROLE"); + assertTrue(pcr0Manager.hasRole(SECURITY_ROLE, deployer), "Deployer missing SECURITY_ROLE"); + assertTrue(pcr0Manager.hasRole(OPERATIONS_ROLE, deployer), "Deployer missing OPERATIONS_ROLE"); console2.log("\n=== Step 2: Add PCR0 Values ==="); @@ -113,10 +113,10 @@ contract MigratePCR0ManagerTest is Test { console2.log("\n=== Step 4: Transfer Roles to Multisigs ==="); vm.startPrank(deployer); - pcr0Manager.grantRole(CRITICAL_ROLE, criticalMultisig); - pcr0Manager.grantRole(STANDARD_ROLE, standardMultisig); - pcr0Manager.renounceRole(CRITICAL_ROLE, deployer); - pcr0Manager.renounceRole(STANDARD_ROLE, deployer); + pcr0Manager.grantRole(SECURITY_ROLE, securityMultisig); + pcr0Manager.grantRole(OPERATIONS_ROLE, operationsMultisig); + pcr0Manager.renounceRole(SECURITY_ROLE, deployer); + pcr0Manager.renounceRole(OPERATIONS_ROLE, deployer); vm.stopPrank(); console2.log("Roles transferred to multisigs"); @@ -124,15 +124,15 @@ contract MigratePCR0ManagerTest is Test { console2.log("\n=== Step 5: Verify Final State ==="); // Deployer has no roles - assertFalse(pcr0Manager.hasRole(CRITICAL_ROLE, deployer), "Deployer still has CRITICAL_ROLE"); - assertFalse(pcr0Manager.hasRole(STANDARD_ROLE, deployer), "Deployer still has STANDARD_ROLE"); + assertFalse(pcr0Manager.hasRole(SECURITY_ROLE, deployer), "Deployer still has SECURITY_ROLE"); + assertFalse(pcr0Manager.hasRole(OPERATIONS_ROLE, deployer), "Deployer still has OPERATIONS_ROLE"); // Multisigs have roles - assertTrue(pcr0Manager.hasRole(CRITICAL_ROLE, criticalMultisig), "Critical multisig missing CRITICAL_ROLE"); - assertTrue(pcr0Manager.hasRole(STANDARD_ROLE, standardMultisig), "Standard multisig missing STANDARD_ROLE"); + assertTrue(pcr0Manager.hasRole(SECURITY_ROLE, securityMultisig), "Critical multisig missing SECURITY_ROLE"); + assertTrue(pcr0Manager.hasRole(OPERATIONS_ROLE, operationsMultisig), "Standard multisig missing OPERATIONS_ROLE"); // Multisig can manage, deployer cannot - vm.startPrank(criticalMultisig); + vm.startPrank(securityMultisig); bytes memory testPCR0_32_v2 = hex"2222222222222222222222222222222222222222222222222222222222222222"; bytes memory testPCR0_48_v2 = abi.encodePacked(new bytes(16), testPCR0_32_v2); pcr0Manager.addPCR0(testPCR0_32_v2); diff --git a/contracts/test/foundry/UpgradeToAccessControl.t.sol b/contracts/test/foundry/UpgradeToAccessControl.t.sol index 32ed75f8e..0afcdb33a 100644 --- a/contracts/test/foundry/UpgradeToAccessControl.t.sol +++ b/contracts/test/foundry/UpgradeToAccessControl.t.sol @@ -40,8 +40,8 @@ contract UpgradeToAccessControlTest is Test { // Test accounts address deployer; - address criticalMultisig; - address standardMultisig; + address securityMultisig; + address operationsMultisig; // Contracts IdentityVerificationHubImplV2 hub; @@ -50,8 +50,8 @@ contract UpgradeToAccessControlTest is Test { IdentityRegistryAadhaarImplV1 aadhaarRegistry; // Governance roles - bytes32 public constant CRITICAL_ROLE = keccak256("CRITICAL_ROLE"); - bytes32 public constant STANDARD_ROLE = keccak256("STANDARD_ROLE"); + bytes32 public constant SECURITY_ROLE = keccak256("SECURITY_ROLE"); + bytes32 public constant OPERATIONS_ROLE = keccak256("OPERATIONS_ROLE"); // Pre-upgrade state - captures ALL publicly accessible critical state variables struct PreUpgradeState { @@ -96,14 +96,14 @@ contract UpgradeToAccessControlTest is Test { deployer = Ownable2StepUpgradeable(address(hub)).owner(); // Set up multisig accounts for testing role transfer - criticalMultisig = makeAddr("criticalMultisig"); - standardMultisig = makeAddr("standardMultisig"); + securityMultisig = makeAddr("securityMultisig"); + operationsMultisig = makeAddr("operationsMultisig"); vm.deal(deployer, 100 ether); console2.log("Current Owner (will execute upgrade):", deployer); - console2.log("Critical Multisig (will receive roles):", criticalMultisig); - console2.log("Standard Multisig (will receive roles):", standardMultisig); + console2.log("Critical Multisig (will receive roles):", securityMultisig); + console2.log("Standard Multisig (will receive roles):", operationsMultisig); } function testFullUpgradeWorkflow() public { @@ -235,12 +235,12 @@ contract UpgradeToAccessControlTest is Test { console2.log("\n=== Phase 4: Verify Governance Roles ==="); // Deployer should have both roles initially - assertTrue(hub.hasRole(CRITICAL_ROLE, deployer), "Deployer missing CRITICAL_ROLE on Hub"); - assertTrue(hub.hasRole(STANDARD_ROLE, deployer), "Deployer missing STANDARD_ROLE on Hub"); - assertTrue(passportRegistry.hasRole(CRITICAL_ROLE, deployer), "Deployer missing CRITICAL_ROLE on Passport"); - assertTrue(passportRegistry.hasRole(STANDARD_ROLE, deployer), "Deployer missing STANDARD_ROLE on Passport"); - assertTrue(idCardRegistry.hasRole(CRITICAL_ROLE, deployer), "Deployer missing CRITICAL_ROLE on ID Card"); - assertTrue(idCardRegistry.hasRole(STANDARD_ROLE, deployer), "Deployer missing STANDARD_ROLE on ID Card"); + assertTrue(hub.hasRole(SECURITY_ROLE, deployer), "Deployer missing SECURITY_ROLE on Hub"); + assertTrue(hub.hasRole(OPERATIONS_ROLE, deployer), "Deployer missing OPERATIONS_ROLE on Hub"); + assertTrue(passportRegistry.hasRole(SECURITY_ROLE, deployer), "Deployer missing SECURITY_ROLE on Passport"); + assertTrue(passportRegistry.hasRole(OPERATIONS_ROLE, deployer), "Deployer missing OPERATIONS_ROLE on Passport"); + assertTrue(idCardRegistry.hasRole(SECURITY_ROLE, deployer), "Deployer missing SECURITY_ROLE on ID Card"); + assertTrue(idCardRegistry.hasRole(OPERATIONS_ROLE, deployer), "Deployer missing OPERATIONS_ROLE on ID Card"); console2.log("Deployer has all required roles"); @@ -249,20 +249,20 @@ contract UpgradeToAccessControlTest is Test { vm.startPrank(deployer); // Grant roles to multisigs - hub.grantRole(CRITICAL_ROLE, criticalMultisig); - hub.grantRole(STANDARD_ROLE, standardMultisig); - passportRegistry.grantRole(CRITICAL_ROLE, criticalMultisig); - passportRegistry.grantRole(STANDARD_ROLE, standardMultisig); - idCardRegistry.grantRole(CRITICAL_ROLE, criticalMultisig); - idCardRegistry.grantRole(STANDARD_ROLE, standardMultisig); + hub.grantRole(SECURITY_ROLE, securityMultisig); + hub.grantRole(OPERATIONS_ROLE, operationsMultisig); + passportRegistry.grantRole(SECURITY_ROLE, securityMultisig); + passportRegistry.grantRole(OPERATIONS_ROLE, operationsMultisig); + idCardRegistry.grantRole(SECURITY_ROLE, securityMultisig); + idCardRegistry.grantRole(OPERATIONS_ROLE, operationsMultisig); // Deployer renounces roles - hub.renounceRole(CRITICAL_ROLE, deployer); - hub.renounceRole(STANDARD_ROLE, deployer); - passportRegistry.renounceRole(CRITICAL_ROLE, deployer); - passportRegistry.renounceRole(STANDARD_ROLE, deployer); - idCardRegistry.renounceRole(CRITICAL_ROLE, deployer); - idCardRegistry.renounceRole(STANDARD_ROLE, deployer); + hub.renounceRole(SECURITY_ROLE, deployer); + hub.renounceRole(OPERATIONS_ROLE, deployer); + passportRegistry.renounceRole(SECURITY_ROLE, deployer); + passportRegistry.renounceRole(OPERATIONS_ROLE, deployer); + idCardRegistry.renounceRole(SECURITY_ROLE, deployer); + idCardRegistry.renounceRole(OPERATIONS_ROLE, deployer); vm.stopPrank(); @@ -271,16 +271,16 @@ contract UpgradeToAccessControlTest is Test { console2.log("\n=== Phase 6: Verify Final State ==="); // Deployer should have NO roles - assertFalse(hub.hasRole(CRITICAL_ROLE, deployer), "Deployer still has CRITICAL_ROLE on Hub"); - assertFalse(hub.hasRole(STANDARD_ROLE, deployer), "Deployer still has STANDARD_ROLE on Hub"); + assertFalse(hub.hasRole(SECURITY_ROLE, deployer), "Deployer still has SECURITY_ROLE on Hub"); + assertFalse(hub.hasRole(OPERATIONS_ROLE, deployer), "Deployer still has OPERATIONS_ROLE on Hub"); // Multisigs should have roles - assertTrue(hub.hasRole(CRITICAL_ROLE, criticalMultisig), "Critical multisig missing CRITICAL_ROLE on Hub"); - assertTrue(hub.hasRole(STANDARD_ROLE, standardMultisig), "Standard multisig missing STANDARD_ROLE on Hub"); - assertTrue(passportRegistry.hasRole(CRITICAL_ROLE, criticalMultisig), "Critical multisig missing CRITICAL_ROLE on Passport"); - assertTrue(passportRegistry.hasRole(STANDARD_ROLE, standardMultisig), "Standard multisig missing STANDARD_ROLE on Passport"); - assertTrue(idCardRegistry.hasRole(CRITICAL_ROLE, criticalMultisig), "Critical multisig missing CRITICAL_ROLE on ID Card"); - assertTrue(idCardRegistry.hasRole(STANDARD_ROLE, standardMultisig), "Standard multisig missing STANDARD_ROLE on ID Card"); + assertTrue(hub.hasRole(SECURITY_ROLE, securityMultisig), "Critical multisig missing SECURITY_ROLE on Hub"); + assertTrue(hub.hasRole(OPERATIONS_ROLE, operationsMultisig), "Standard multisig missing OPERATIONS_ROLE on Hub"); + assertTrue(passportRegistry.hasRole(SECURITY_ROLE, securityMultisig), "Critical multisig missing SECURITY_ROLE on Passport"); + assertTrue(passportRegistry.hasRole(OPERATIONS_ROLE, operationsMultisig), "Standard multisig missing OPERATIONS_ROLE on Passport"); + assertTrue(idCardRegistry.hasRole(SECURITY_ROLE, securityMultisig), "Critical multisig missing SECURITY_ROLE on ID Card"); + assertTrue(idCardRegistry.hasRole(OPERATIONS_ROLE, operationsMultisig), "Standard multisig missing OPERATIONS_ROLE on ID Card"); console2.log("Multisigs have full control"); console2.log("Deployer has ZERO control"); diff --git a/contracts/test/governance/FullUpgradeIntegration.test.ts b/contracts/test/governance/FullUpgradeIntegration.test.ts index 6a6780d61..e1408d511 100644 --- a/contracts/test/governance/FullUpgradeIntegration.test.ts +++ b/contracts/test/governance/FullUpgradeIntegration.test.ts @@ -46,8 +46,8 @@ describe("🚀 PRODUCTION UPGRADE: Ownable → AccessControl Governance", functi this.timeout(120000); let deployer: SignerWithAddress; - let criticalMultisig: SignerWithAddress; - let standardMultisig: SignerWithAddress; + let securityMultisig: SignerWithAddress; + let operationsMultisig: SignerWithAddress; let user1: SignerWithAddress; let user2: SignerWithAddress; @@ -64,20 +64,20 @@ describe("🚀 PRODUCTION UPGRADE: Ownable → AccessControl Governance", functi let poseidonT3: PoseidonT3; // Test constants - const CRITICAL_ROLE = ethers.keccak256(ethers.toUtf8Bytes("CRITICAL_ROLE")); - const STANDARD_ROLE = ethers.keccak256(ethers.toUtf8Bytes("STANDARD_ROLE")); + const SECURITY_ROLE = ethers.keccak256(ethers.toUtf8Bytes("SECURITY_ROLE")); + const OPERATIONS_ROLE = ethers.keccak256(ethers.toUtf8Bytes("OPERATIONS_ROLE")); // Sample production data const SAMPLE_CSCA_ROOT = "0x1111111111111111111111111111111111111111111111111111111111111111"; const SAMPLE_PCR0 = "0x" + "22".repeat(48); before(async function () { - [deployer, criticalMultisig, standardMultisig, user1, user2] = await ethers.getSigners(); + [deployer, securityMultisig, operationsMultisig, user1, user2] = await ethers.getSigners(); console.log("\n🎯 Production Upgrade Simulation"); console.log(` Deployer: ${deployer.address}`); - console.log(` Critical Multisig: ${criticalMultisig.address}`); - console.log(` Standard Multisig: ${standardMultisig.address}`); + console.log(` Critical Multisig: ${securityMultisig.address}`); + console.log(` Standard Multisig: ${operationsMultisig.address}`); console.log("\n📝 Scenario: Upgrade IdentityVerificationHubImplV2 & Registries"); console.log(" From: Ownable2StepUpgradeable (old ImplRoot)"); console.log(" To: AccessControlUpgradeable (new ImplRoot)"); @@ -207,8 +207,8 @@ describe("🚀 PRODUCTION UPGRADE: Ownable → AccessControl Governance", functi console.log(" ✅ Same proxy address (in-place upgrade)"); // Verify governance roles initialized - expect(await upgradedHub.hasRole(CRITICAL_ROLE, deployer.address)).to.be.true; - expect(await upgradedHub.hasRole(STANDARD_ROLE, deployer.address)).to.be.true; + expect(await upgradedHub.hasRole(SECURITY_ROLE, deployer.address)).to.be.true; + expect(await upgradedHub.hasRole(OPERATIONS_ROLE, deployer.address)).to.be.true; console.log(" ✅ Governance roles initialized (deployer has both roles)"); // Verify ALL state preserved @@ -244,8 +244,8 @@ describe("🚀 PRODUCTION UPGRADE: Ownable → AccessControl Governance", functi console.log(" ✅ Same proxy address (in-place upgrade)"); // Verify governance roles initialized - expect(await upgradedRegistry.hasRole(CRITICAL_ROLE, deployer.address)).to.be.true; - expect(await upgradedRegistry.hasRole(STANDARD_ROLE, deployer.address)).to.be.true; + expect(await upgradedRegistry.hasRole(SECURITY_ROLE, deployer.address)).to.be.true; + expect(await upgradedRegistry.hasRole(OPERATIONS_ROLE, deployer.address)).to.be.true; console.log(" ✅ Governance roles initialized (deployer has both roles)"); // Verify ALL state preserved @@ -275,7 +275,7 @@ describe("🚀 PRODUCTION UPGRADE: Ownable → AccessControl Governance", functi await idCardRegistryProxy.waitForDeployment(); console.log(` ✅ ID Card Registry: ${await idCardRegistryProxy.getAddress()}`); - expect(await idCardRegistryProxy.hasRole(CRITICAL_ROLE, deployer.address)).to.be.true; + expect(await idCardRegistryProxy.hasRole(SECURITY_ROLE, deployer.address)).to.be.true; console.log(" ✅ Deployer has governance roles"); }); @@ -286,7 +286,7 @@ describe("🚀 PRODUCTION UPGRADE: Ownable → AccessControl Governance", functi pcr0Manager = await PCR0ManagerFactory.deploy(); await pcr0Manager.waitForDeployment(); console.log(` ✅ PCR0Manager: ${await pcr0Manager.getAddress()}`); - expect(await pcr0Manager.hasRole(CRITICAL_ROLE, deployer.address)).to.be.true; + expect(await pcr0Manager.hasRole(SECURITY_ROLE, deployer.address)).to.be.true; console.log(" ✅ Deployer has governance roles"); }); }); @@ -295,58 +295,58 @@ describe("🚀 PRODUCTION UPGRADE: Ownable → AccessControl Governance", functi it("should transfer HubV2 roles to multisigs and remove deployer", async function () { console.log("\n🔑 Transferring HubV2 roles to multisigs..."); - await upgradedHub.grantRole(CRITICAL_ROLE, criticalMultisig.address); - await upgradedHub.grantRole(STANDARD_ROLE, standardMultisig.address); + await upgradedHub.grantRole(SECURITY_ROLE, securityMultisig.address); + await upgradedHub.grantRole(OPERATIONS_ROLE, operationsMultisig.address); console.log(" ✅ Granted roles to multisigs"); - await upgradedHub.renounceRole(CRITICAL_ROLE, deployer.address); - await upgradedHub.renounceRole(STANDARD_ROLE, deployer.address); + await upgradedHub.renounceRole(SECURITY_ROLE, deployer.address); + await upgradedHub.renounceRole(OPERATIONS_ROLE, deployer.address); console.log(" ✅ Deployer renounced roles"); - expect(await upgradedHub.hasRole(CRITICAL_ROLE, criticalMultisig.address)).to.be.true; - expect(await upgradedHub.hasRole(STANDARD_ROLE, standardMultisig.address)).to.be.true; - expect(await upgradedHub.hasRole(CRITICAL_ROLE, deployer.address)).to.be.false; - expect(await upgradedHub.hasRole(STANDARD_ROLE, deployer.address)).to.be.false; + expect(await upgradedHub.hasRole(SECURITY_ROLE, securityMultisig.address)).to.be.true; + expect(await upgradedHub.hasRole(OPERATIONS_ROLE, operationsMultisig.address)).to.be.true; + expect(await upgradedHub.hasRole(SECURITY_ROLE, deployer.address)).to.be.false; + expect(await upgradedHub.hasRole(OPERATIONS_ROLE, deployer.address)).to.be.false; console.log(" ✅ HubV2 now controlled by multisigs only"); }); it("should transfer Registry roles to multisigs and remove deployer", async function () { console.log("\n🔑 Transferring Registry roles to multisigs..."); - await upgradedRegistry.grantRole(CRITICAL_ROLE, criticalMultisig.address); - await upgradedRegistry.grantRole(STANDARD_ROLE, standardMultisig.address); - await upgradedRegistry.renounceRole(CRITICAL_ROLE, deployer.address); - await upgradedRegistry.renounceRole(STANDARD_ROLE, deployer.address); + await upgradedRegistry.grantRole(SECURITY_ROLE, securityMultisig.address); + await upgradedRegistry.grantRole(OPERATIONS_ROLE, operationsMultisig.address); + await upgradedRegistry.renounceRole(SECURITY_ROLE, deployer.address); + await upgradedRegistry.renounceRole(OPERATIONS_ROLE, deployer.address); - expect(await upgradedRegistry.hasRole(CRITICAL_ROLE, criticalMultisig.address)).to.be.true; - expect(await upgradedRegistry.hasRole(STANDARD_ROLE, standardMultisig.address)).to.be.true; - expect(await upgradedRegistry.hasRole(CRITICAL_ROLE, deployer.address)).to.be.false; + expect(await upgradedRegistry.hasRole(SECURITY_ROLE, securityMultisig.address)).to.be.true; + expect(await upgradedRegistry.hasRole(OPERATIONS_ROLE, operationsMultisig.address)).to.be.true; + expect(await upgradedRegistry.hasRole(SECURITY_ROLE, deployer.address)).to.be.false; console.log(" ✅ Registry now controlled by multisigs only"); }); it("should transfer ID Card Registry roles to multisigs", async function () { console.log("\n🔑 Transferring ID Card Registry roles to multisigs..."); - await idCardRegistryProxy.grantRole(CRITICAL_ROLE, criticalMultisig.address); - await idCardRegistryProxy.grantRole(STANDARD_ROLE, standardMultisig.address); - await idCardRegistryProxy.renounceRole(CRITICAL_ROLE, deployer.address); - await idCardRegistryProxy.renounceRole(STANDARD_ROLE, deployer.address); + await idCardRegistryProxy.grantRole(SECURITY_ROLE, securityMultisig.address); + await idCardRegistryProxy.grantRole(OPERATIONS_ROLE, operationsMultisig.address); + await idCardRegistryProxy.renounceRole(SECURITY_ROLE, deployer.address); + await idCardRegistryProxy.renounceRole(OPERATIONS_ROLE, deployer.address); - expect(await idCardRegistryProxy.hasRole(CRITICAL_ROLE, criticalMultisig.address)).to.be.true; - expect(await idCardRegistryProxy.hasRole(CRITICAL_ROLE, deployer.address)).to.be.false; + expect(await idCardRegistryProxy.hasRole(SECURITY_ROLE, securityMultisig.address)).to.be.true; + expect(await idCardRegistryProxy.hasRole(SECURITY_ROLE, deployer.address)).to.be.false; console.log(" ✅ ID Card Registry now controlled by multisigs only"); }); it("should transfer PCR0Manager roles to multisigs", async function () { console.log("\n🔑 Transferring PCR0Manager roles to multisigs..."); - await pcr0Manager.grantRole(CRITICAL_ROLE, criticalMultisig.address); - await pcr0Manager.grantRole(STANDARD_ROLE, standardMultisig.address); - await pcr0Manager.renounceRole(CRITICAL_ROLE, deployer.address); - await pcr0Manager.renounceRole(STANDARD_ROLE, deployer.address); + await pcr0Manager.grantRole(SECURITY_ROLE, securityMultisig.address); + await pcr0Manager.grantRole(OPERATIONS_ROLE, operationsMultisig.address); + await pcr0Manager.renounceRole(SECURITY_ROLE, deployer.address); + await pcr0Manager.renounceRole(OPERATIONS_ROLE, deployer.address); - expect(await pcr0Manager.hasRole(CRITICAL_ROLE, criticalMultisig.address)).to.be.true; - expect(await pcr0Manager.hasRole(CRITICAL_ROLE, deployer.address)).to.be.false; + expect(await pcr0Manager.hasRole(SECURITY_ROLE, securityMultisig.address)).to.be.true; + expect(await pcr0Manager.hasRole(SECURITY_ROLE, deployer.address)).to.be.false; console.log(" ✅ PCR0Manager now controlled by multisigs only"); }); }); @@ -355,7 +355,7 @@ describe("🚀 PRODUCTION UPGRADE: Ownable → AccessControl Governance", functi it("should allow critical multisig to update HubV2", async function () { console.log("\n✅ Testing HubV2 multisig control..."); const newRegistry = user1.address; - await upgradedHub.connect(criticalMultisig).updateRegistry(newRegistry); + await upgradedHub.connect(securityMultisig).updateRegistry(newRegistry); expect(await upgradedHub.getRegistry()).to.equal(newRegistry); console.log(" ✅ Critical multisig can update HubV2"); }); @@ -363,14 +363,14 @@ describe("🚀 PRODUCTION UPGRADE: Ownable → AccessControl Governance", functi it("should allow critical multisig to update Registry", async function () { console.log("\n✅ Testing Registry multisig control..."); const newRoot = "0x" + "33".repeat(32); - await upgradedRegistry.connect(criticalMultisig).updateCscaRoot(newRoot); + await upgradedRegistry.connect(securityMultisig).updateCscaRoot(newRoot); expect(await upgradedRegistry.getCscaRoot()).to.equal(newRoot); console.log(" ✅ Critical multisig can update Registry"); }); it("should allow critical multisig to manage PCR0", async function () { console.log("\n✅ Testing PCR0Manager multisig control..."); - await pcr0Manager.connect(criticalMultisig).addPCR0(SAMPLE_PCR0); + await pcr0Manager.connect(securityMultisig).addPCR0(SAMPLE_PCR0); expect(await pcr0Manager.isPCR0Set(SAMPLE_PCR0)).to.be.true; console.log(" ✅ Critical multisig can manage PCR0"); }); @@ -423,10 +423,10 @@ describe("🚀 PRODUCTION UPGRADE: Ownable → AccessControl Governance", functi console.log("\n🎯 Final HubV2 verification..."); const newRegistry = user2.address; - await upgradedHub.connect(criticalMultisig).updateRegistry(newRegistry); + await upgradedHub.connect(securityMultisig).updateRegistry(newRegistry); expect(await upgradedHub.getRegistry()).to.equal(newRegistry); - await upgradedHub.connect(criticalMultisig).updateCircuitVersion(3); + await upgradedHub.connect(securityMultisig).updateCircuitVersion(3); expect(await upgradedHub.getCircuitVersion()).to.equal(3); console.log(" ✅ HubV2 fully functional with multisig control"); }); @@ -435,7 +435,7 @@ describe("🚀 PRODUCTION UPGRADE: Ownable → AccessControl Governance", functi console.log("\n🎯 Final Registry verification..."); const finalRoot = "0x" + "99".repeat(32); - await upgradedRegistry.connect(criticalMultisig).updateCscaRoot(finalRoot); + await upgradedRegistry.connect(securityMultisig).updateCscaRoot(finalRoot); expect(await upgradedRegistry.getCscaRoot()).to.equal(finalRoot); console.log(" ✅ Registry fully functional with multisig control"); }); @@ -444,7 +444,7 @@ describe("🚀 PRODUCTION UPGRADE: Ownable → AccessControl Governance", functi console.log("\n🎯 Final PCR0Manager verification..."); const newPCR0 = "0x" + "88".repeat(48); - await pcr0Manager.connect(criticalMultisig).addPCR0(newPCR0); + await pcr0Manager.connect(securityMultisig).addPCR0(newPCR0); expect(await pcr0Manager.isPCR0Set(newPCR0)).to.be.true; console.log(" ✅ PCR0Manager fully functional with multisig control"); }); @@ -476,8 +476,8 @@ describe("🚀 PRODUCTION UPGRADE: Ownable → AccessControl Governance", functi console.log(" ✓ NO storage corruption"); console.log("\n🔑 Governance Configuration:"); - console.log(` Critical Multisig (3/5): ${criticalMultisig.address}`); - console.log(` Standard Multisig (2/5): ${standardMultisig.address}`); + console.log(` Critical Multisig (3/5): ${securityMultisig.address}`); + console.log(` Standard Multisig (2/5): ${operationsMultisig.address}`); console.log(" Critical Role: Upgrades, critical parameters, role management"); console.log(" Standard Role: Standard operational parameters"); diff --git a/contracts/test/governance/GovernanceUpgrade.test.ts b/contracts/test/governance/GovernanceUpgrade.test.ts index c00149ff9..278824882 100644 --- a/contracts/test/governance/GovernanceUpgrade.test.ts +++ b/contracts/test/governance/GovernanceUpgrade.test.ts @@ -13,8 +13,8 @@ import { describe("Governance Upgrade Tests", function () { let deployer: SignerWithAddress; - let criticalMultisig: SignerWithAddress; - let standardMultisig: SignerWithAddress; + let securityMultisig: SignerWithAddress; + let operationsMultisig: SignerWithAddress; let user: SignerWithAddress; // Contract instances for testing @@ -29,13 +29,13 @@ describe("Governance Upgrade Tests", function () { let poseidonT3: PoseidonT3; // Test constants - const CRITICAL_ROLE = ethers.keccak256(ethers.toUtf8Bytes("CRITICAL_ROLE")); - const STANDARD_ROLE = ethers.keccak256(ethers.toUtf8Bytes("STANDARD_ROLE")); + const SECURITY_ROLE = ethers.keccak256(ethers.toUtf8Bytes("SECURITY_ROLE")); + const OPERATIONS_ROLE = ethers.keccak256(ethers.toUtf8Bytes("OPERATIONS_ROLE")); const DEFAULT_ADMIN_ROLE = ethers.ZeroHash; beforeEach(async function () { // Set up test signers representing different roles in the governance system - [deployer, criticalMultisig, standardMultisig, user] = await ethers.getSigners(); + [deployer, securityMultisig, operationsMultisig, user] = await ethers.getSigners(); // Deploy CustomVerifier library once for reuse across tests const CustomVerifierFactory = await ethers.getContractFactory("CustomVerifier"); @@ -87,7 +87,7 @@ describe("Governance Upgrade Tests", function () { // This simulates upgrading a production contract to the new governance system // Verify initial state - deployer should have roles after initialization - expect(await hubProxy.hasRole(CRITICAL_ROLE, deployer.address)).to.be.true; + expect(await hubProxy.hasRole(SECURITY_ROLE, deployer.address)).to.be.true; // Deploy new implementation with governance using the same library instance const IdentityVerificationHubV3 = await ethers.getContractFactory("IdentityVerificationHubImplV2", { @@ -114,16 +114,16 @@ describe("Governance Upgrade Tests", function () { // For this test, we'll simulate that the migration script has already run // In production, this would be done by a separate migration transaction try { - await hubWithGovernance.grantRole(CRITICAL_ROLE, deployer.address); - await hubWithGovernance.grantRole(STANDARD_ROLE, deployer.address); + await hubWithGovernance.grantRole(SECURITY_ROLE, deployer.address); + await hubWithGovernance.grantRole(OPERATIONS_ROLE, deployer.address); // Verify governance roles are set correctly - expect(await hubWithGovernance.hasRole(CRITICAL_ROLE, deployer.address)).to.be.true; - expect(await hubWithGovernance.hasRole(STANDARD_ROLE, deployer.address)).to.be.true; + expect(await hubWithGovernance.hasRole(SECURITY_ROLE, deployer.address)).to.be.true; + expect(await hubWithGovernance.hasRole(OPERATIONS_ROLE, deployer.address)).to.be.true; // Verify role hierarchy (set up during __ImplRoot_init) - expect(await hubWithGovernance.getRoleAdmin(CRITICAL_ROLE)).to.equal(CRITICAL_ROLE); - expect(await hubWithGovernance.getRoleAdmin(STANDARD_ROLE)).to.equal(CRITICAL_ROLE); + expect(await hubWithGovernance.getRoleAdmin(SECURITY_ROLE)).to.equal(SECURITY_ROLE); + expect(await hubWithGovernance.getRoleAdmin(OPERATIONS_ROLE)).to.equal(SECURITY_ROLE); } catch (error) { // If role setup fails, it might mean the roles are already set up or the contract doesn't support it yet console.log("Role setup skipped:", (error as Error).message); @@ -163,7 +163,7 @@ describe("Governance Upgrade Tests", function () { // This is critical for production upgrades to maintain existing permissions and data // Verify initial state - check that roles are preserved - const initialHasCriticalRole = await hubProxy.hasRole(CRITICAL_ROLE, deployer.address); + const initialHasCriticalRole = await hubProxy.hasRole(SECURITY_ROLE, deployer.address); // Upgrade using the same library instance to avoid redeployment const IdentityVerificationHubV3 = await ethers.getContractFactory("IdentityVerificationHubImplV2", { @@ -184,7 +184,7 @@ describe("Governance Upgrade Tests", function () { ); // Verify state is preserved - roles should still exist - const finalHasCriticalRole = await hubProxy.hasRole(CRITICAL_ROLE, deployer.address); + const finalHasCriticalRole = await hubProxy.hasRole(SECURITY_ROLE, deployer.address); expect(finalHasCriticalRole).to.equal(initialHasCriticalRole); }); }); @@ -225,7 +225,7 @@ describe("Governance Upgrade Tests", function () { // This ensures the registry upgrade process works similarly to the hub upgrade // Verify initial state - deployer should have roles after initialization - expect(await registryProxy.hasRole(CRITICAL_ROLE, deployer.address)).to.be.true; + expect(await registryProxy.hasRole(SECURITY_ROLE, deployer.address)).to.be.true; // Upgrade to governance using the shared library instance const IdentityRegistryV2 = await ethers.getContractFactory("IdentityRegistryImplV1", { @@ -248,12 +248,12 @@ describe("Governance Upgrade Tests", function () { // After upgrade, the contract now has governance capabilities // For this test, we'll simulate that the migration script has already run try { - await registryProxy.grantRole(CRITICAL_ROLE, deployer.address); - await registryProxy.grantRole(STANDARD_ROLE, deployer.address); + await registryProxy.grantRole(SECURITY_ROLE, deployer.address); + await registryProxy.grantRole(OPERATIONS_ROLE, deployer.address); // Verify governance roles are set correctly - expect(await registryProxy.hasRole(CRITICAL_ROLE, deployer.address)).to.be.true; - expect(await registryProxy.hasRole(STANDARD_ROLE, deployer.address)).to.be.true; + expect(await registryProxy.hasRole(SECURITY_ROLE, deployer.address)).to.be.true; + expect(await registryProxy.hasRole(OPERATIONS_ROLE, deployer.address)).to.be.true; } catch (error) { // If role setup fails, it might mean the roles are already set up or the contract doesn't support it yet console.log("Role setup skipped:", (error as Error).message); @@ -288,15 +288,15 @@ describe("Governance Upgrade Tests", function () { it("should deploy PCR0Manager with deployer having initial roles", async function () { // Test: Verify that PCR0Manager is deployed with the deployer having both governance roles // This follows the pattern where deployer gets initial control before transferring to multisigs - expect(await pcr0Manager.hasRole(CRITICAL_ROLE, deployer.address)).to.be.true; - expect(await pcr0Manager.hasRole(STANDARD_ROLE, deployer.address)).to.be.true; + expect(await pcr0Manager.hasRole(SECURITY_ROLE, deployer.address)).to.be.true; + expect(await pcr0Manager.hasRole(OPERATIONS_ROLE, deployer.address)).to.be.true; }); it("should deploy VerifyAll with deployer having initial roles", async function () { // Test: Verify that VerifyAll is deployed with the deployer having both governance roles // This ensures consistent role initialization across all governance contracts - expect(await verifyAll.hasRole(CRITICAL_ROLE, deployer.address)).to.be.true; - expect(await verifyAll.hasRole(STANDARD_ROLE, deployer.address)).to.be.true; + expect(await verifyAll.hasRole(SECURITY_ROLE, deployer.address)).to.be.true; + expect(await verifyAll.hasRole(OPERATIONS_ROLE, deployer.address)).to.be.true; }); it("should allow role transfer and then critical multisig to manage PCR0", async function () { @@ -304,14 +304,14 @@ describe("Governance Upgrade Tests", function () { // This simulates the production process of deploying, transferring roles, and operating the contract // First transfer roles to multisigs (simulating production deployment workflow) - await pcr0Manager.connect(deployer).grantRole(CRITICAL_ROLE, criticalMultisig.address); - await pcr0Manager.connect(deployer).grantRole(STANDARD_ROLE, standardMultisig.address); + await pcr0Manager.connect(deployer).grantRole(SECURITY_ROLE, securityMultisig.address); + await pcr0Manager.connect(deployer).grantRole(OPERATIONS_ROLE, operationsMultisig.address); const testPCR0 = "0x" + "00".repeat(48); // 48 zero bytes (valid PCR0 format) // Critical multisig should be able to add PCR0 (testing governance functionality) await expect( - pcr0Manager.connect(criticalMultisig).addPCR0(testPCR0) + pcr0Manager.connect(securityMultisig).addPCR0(testPCR0) ).to.emit(pcr0Manager, "PCR0Added"); // Verify PCR0 was added successfully @@ -319,7 +319,7 @@ describe("Governance Upgrade Tests", function () { // Critical multisig should be able to remove PCR0 (testing full CRUD operations) await expect( - pcr0Manager.connect(criticalMultisig).removePCR0(testPCR0) + pcr0Manager.connect(securityMultisig).removePCR0(testPCR0) ).to.emit(pcr0Manager, "PCR0Removed"); // Verify PCR0 was removed successfully @@ -343,8 +343,8 @@ describe("Governance Upgrade Tests", function () { // This tests the governance of contract dependencies and configuration updates // First transfer roles to multisigs (following production deployment pattern) - await verifyAll.connect(deployer).grantRole(CRITICAL_ROLE, criticalMultisig.address); - await verifyAll.connect(deployer).grantRole(STANDARD_ROLE, standardMultisig.address); + await verifyAll.connect(deployer).grantRole(SECURITY_ROLE, securityMultisig.address); + await verifyAll.connect(deployer).grantRole(OPERATIONS_ROLE, operationsMultisig.address); // Generate new addresses for testing (simulating contract upgrades or migrations) const newHubAddress = ethers.Wallet.createRandom().address; @@ -352,12 +352,12 @@ describe("Governance Upgrade Tests", function () { // Critical multisig should be able to update hub address await expect( - verifyAll.connect(criticalMultisig).setHub(newHubAddress) + verifyAll.connect(securityMultisig).setHub(newHubAddress) ).to.not.be.reverted; // Critical multisig should be able to update registry address await expect( - verifyAll.connect(criticalMultisig).setRegistry(newRegistryAddress) + verifyAll.connect(securityMultisig).setRegistry(newRegistryAddress) ).to.not.be.reverted; // Verify addresses were updated correctly @@ -387,45 +387,45 @@ describe("Governance Upgrade Tests", function () { await pcr0Manager.waitForDeployment(); // Grant roles to multisigs (deployer has initial roles from constructor) - await pcr0Manager.connect(deployer).grantRole(CRITICAL_ROLE, criticalMultisig.address); - await pcr0Manager.connect(deployer).grantRole(STANDARD_ROLE, standardMultisig.address); + await pcr0Manager.connect(deployer).grantRole(SECURITY_ROLE, securityMultisig.address); + await pcr0Manager.connect(deployer).grantRole(OPERATIONS_ROLE, operationsMultisig.address); }); it("should allow critical multisig to manage roles", async function () { - // Test: Verify that CRITICAL_ROLE can manage other roles (role hierarchy) + // Test: Verify that SECURITY_ROLE can manage other roles (role hierarchy) // This tests the admin functionality where critical multisig manages all roles const newStandardUser = user.address; // Critical multisig (admin) should be able to grant standard role await expect( - pcr0Manager.connect(criticalMultisig).grantRole(STANDARD_ROLE, newStandardUser) + pcr0Manager.connect(securityMultisig).grantRole(OPERATIONS_ROLE, newStandardUser) ).to.not.be.reverted; // Verify role was granted successfully - expect(await pcr0Manager.hasRole(STANDARD_ROLE, newStandardUser)).to.be.true; + expect(await pcr0Manager.hasRole(OPERATIONS_ROLE, newStandardUser)).to.be.true; // Critical multisig should be able to revoke role (testing full role management) await expect( - pcr0Manager.connect(criticalMultisig).revokeRole(STANDARD_ROLE, newStandardUser) + pcr0Manager.connect(securityMultisig).revokeRole(OPERATIONS_ROLE, newStandardUser) ).to.not.be.reverted; // Verify role was revoked successfully - expect(await pcr0Manager.hasRole(STANDARD_ROLE, newStandardUser)).to.be.false; + expect(await pcr0Manager.hasRole(OPERATIONS_ROLE, newStandardUser)).to.be.false; }); it("should prevent non-admin from managing roles", async function () { - // Test: Verify that only CRITICAL_ROLE can manage roles (enforce role hierarchy) + // Test: Verify that only SECURITY_ROLE can manage roles (enforce role hierarchy) // This ensures that standard multisig and regular users cannot escalate privileges // Standard multisig should not be able to grant roles (lacks admin privileges) await expect( - pcr0Manager.connect(standardMultisig).grantRole(STANDARD_ROLE, user.address) + pcr0Manager.connect(operationsMultisig).grantRole(OPERATIONS_ROLE, user.address) ).to.be.revertedWithCustomError(pcr0Manager, "AccessControlUnauthorizedAccount"); // Random user should not be able to grant roles (no privileges at all) await expect( - pcr0Manager.connect(user).grantRole(STANDARD_ROLE, user.address) + pcr0Manager.connect(user).grantRole(OPERATIONS_ROLE, user.address) ).to.be.revertedWithCustomError(pcr0Manager, "AccessControlUnauthorizedAccount"); }); }); @@ -449,11 +449,11 @@ describe("Governance Upgrade Tests", function () { }); it("should allow critical multisig to authorize upgrades", async function () { - // Test: Verify that CRITICAL_ROLE can authorize contract upgrades + // Test: Verify that SECURITY_ROLE can authorize contract upgrades // This is essential for secure upgrade governance in production - // Grant CRITICAL_ROLE to criticalMultisig for this test - await testProxy.connect(deployer).grantRole(CRITICAL_ROLE, criticalMultisig.address); + // Grant SECURITY_ROLE to securityMultisig for this test + await testProxy.connect(deployer).grantRole(SECURITY_ROLE, securityMultisig.address); // Deploy new implementation for upgrade testing const NewImplementation = await ethers.getContractFactory("MockImplRoot"); @@ -472,16 +472,16 @@ describe("Governance Upgrade Tests", function () { }); it("should prevent non-critical roles from authorizing upgrades", async function () { - // Test: Verify that only CRITICAL_ROLE can authorize upgrades + // Test: Verify that only SECURITY_ROLE can authorize upgrades // This prevents unauthorized upgrades by standard multisig or regular users - // Grant STANDARD_ROLE to standardMultisig (but not CRITICAL_ROLE) - await testProxy.connect(deployer).grantRole(STANDARD_ROLE, standardMultisig.address); + // Grant OPERATIONS_ROLE to operationsMultisig (but not SECURITY_ROLE) + await testProxy.connect(deployer).grantRole(OPERATIONS_ROLE, operationsMultisig.address); - // The upgrade should fail when attempted without CRITICAL_ROLE + // The upgrade should fail when attempted without SECURITY_ROLE // This tests the _authorizeUpgrade function's access control directly await expect( - testProxy.connect(standardMultisig).exposed_authorizeUpgrade(ethers.ZeroAddress) + testProxy.connect(operationsMultisig).exposed_authorizeUpgrade(ethers.ZeroAddress) ).to.be.revertedWithCustomError(testProxy, "AccessControlUnauthorizedAccount"); }); }); diff --git a/contracts/test/governance/StorageLayoutUpgrade.test.ts b/contracts/test/governance/StorageLayoutUpgrade.test.ts index 091105a7e..ce3896b87 100644 --- a/contracts/test/governance/StorageLayoutUpgrade.test.ts +++ b/contracts/test/governance/StorageLayoutUpgrade.test.ts @@ -27,16 +27,16 @@ import { describe("ERC-7201 Namespaced Storage Upgrade Tests", function () { let deployer: SignerWithAddress; - let criticalMultisig: SignerWithAddress; - let standardMultisig: SignerWithAddress; + let securityMultisig: SignerWithAddress; + let operationsMultisig: SignerWithAddress; let user: SignerWithAddress; // Test constants - const CRITICAL_ROLE = ethers.keccak256(ethers.toUtf8Bytes("CRITICAL_ROLE")); - const STANDARD_ROLE = ethers.keccak256(ethers.toUtf8Bytes("STANDARD_ROLE")); + const SECURITY_ROLE = ethers.keccak256(ethers.toUtf8Bytes("SECURITY_ROLE")); + const OPERATIONS_ROLE = ethers.keccak256(ethers.toUtf8Bytes("OPERATIONS_ROLE")); beforeEach(async function () { - [deployer, criticalMultisig, standardMultisig, user] = await ethers.getSigners(); + [deployer, securityMultisig, operationsMultisig, user] = await ethers.getSigners(); }); describe("Ownable to AccessControl Upgrade (Storage Validation Demo)", function () { @@ -110,8 +110,8 @@ describe("ERC-7201 Namespaced Storage Upgrade Tests", function () { console.log(` ✅ Registry preserved: ${preservedRegistry}`); // Check that new governance is working - expect(await upgradedHub.hasRole(CRITICAL_ROLE, deployer.address)).to.be.true; - expect(await upgradedHub.hasRole(STANDARD_ROLE, deployer.address)).to.be.true; + expect(await upgradedHub.hasRole(SECURITY_ROLE, deployer.address)).to.be.true; + expect(await upgradedHub.hasRole(OPERATIONS_ROLE, deployer.address)).to.be.true; console.log(" ✅ New governance roles active"); // Verify the upgrade worked (unsafeSkipStorageCheck allows this test scenario) @@ -135,7 +135,7 @@ describe("ERC-7201 Namespaced Storage Upgrade Tests", function () { await upgradedHub.initialize(); // Test that governance functions work - const newRegistry = criticalMultisig.address; + const newRegistry = securityMultisig.address; await upgradedHub.updateRegistry(newRegistry); expect(await upgradedHub.getRegistry()).to.equal(newRegistry); @@ -185,24 +185,24 @@ describe("ERC-7201 Namespaced Storage Upgrade Tests", function () { await upgradedHub.initialize(); // Transfer roles to multisigs - await upgradedHub.grantRole(CRITICAL_ROLE, criticalMultisig.address); - await upgradedHub.grantRole(STANDARD_ROLE, standardMultisig.address); + await upgradedHub.grantRole(SECURITY_ROLE, securityMultisig.address); + await upgradedHub.grantRole(OPERATIONS_ROLE, operationsMultisig.address); // Verify multisigs have roles - expect(await upgradedHub.hasRole(CRITICAL_ROLE, criticalMultisig.address)).to.be.true; - expect(await upgradedHub.hasRole(STANDARD_ROLE, standardMultisig.address)).to.be.true; + expect(await upgradedHub.hasRole(SECURITY_ROLE, securityMultisig.address)).to.be.true; + expect(await upgradedHub.hasRole(OPERATIONS_ROLE, operationsMultisig.address)).to.be.true; // Test that multisig can perform governance functions - await upgradedHub.connect(criticalMultisig).updateRegistry(criticalMultisig.address); - expect(await upgradedHub.getRegistry()).to.equal(criticalMultisig.address); + await upgradedHub.connect(securityMultisig).updateRegistry(securityMultisig.address); + expect(await upgradedHub.getRegistry()).to.equal(securityMultisig.address); // Renounce deployer roles - await upgradedHub.renounceRole(CRITICAL_ROLE, deployer.address); - await upgradedHub.renounceRole(STANDARD_ROLE, deployer.address); + await upgradedHub.renounceRole(SECURITY_ROLE, deployer.address); + await upgradedHub.renounceRole(OPERATIONS_ROLE, deployer.address); // Verify deployer no longer has roles - expect(await upgradedHub.hasRole(CRITICAL_ROLE, deployer.address)).to.be.false; - expect(await upgradedHub.hasRole(STANDARD_ROLE, deployer.address)).to.be.false; + expect(await upgradedHub.hasRole(SECURITY_ROLE, deployer.address)).to.be.false; + expect(await upgradedHub.hasRole(OPERATIONS_ROLE, deployer.address)).to.be.false; }); }); @@ -250,8 +250,8 @@ describe("ERC-7201 Namespaced Storage Upgrade Tests", function () { console.log("\n📋 AFTER UPGRADE:"); console.log(` Registry (preserved): ${await upgradedHub.getRegistry()}`); console.log(` Circuit Version (preserved): ${await upgradedHub.getCircuitVersion()}`); - console.log(` Has CRITICAL_ROLE: ${await upgradedHub.hasRole(CRITICAL_ROLE, deployer.address)}`); - console.log(` Has STANDARD_ROLE: ${await upgradedHub.hasRole(STANDARD_ROLE, deployer.address)}`); + console.log(` Has SECURITY_ROLE: ${await upgradedHub.hasRole(SECURITY_ROLE, deployer.address)}`); + console.log(` Has OPERATIONS_ROLE: ${await upgradedHub.hasRole(OPERATIONS_ROLE, deployer.address)}`); // Verify storage preservation - application state is preserved expect(await upgradedHub.getRegistry()).to.equal(user.address); diff --git a/contracts/test/governance/UpgradeSafety.test.ts b/contracts/test/governance/UpgradeSafety.test.ts index 772c1e11f..a7cfd6515 100644 --- a/contracts/test/governance/UpgradeSafety.test.ts +++ b/contracts/test/governance/UpgradeSafety.test.ts @@ -4,11 +4,11 @@ import { SignerWithAddress } from "@nomicfoundation/hardhat-ethers/signers"; describe("Upgrade Safety Validation Tests", function () { let deployer: SignerWithAddress; - let criticalMultisig: SignerWithAddress; - let standardMultisig: SignerWithAddress; + let securityMultisig: SignerWithAddress; + let operationsMultisig: SignerWithAddress; beforeEach(async function () { - [deployer, criticalMultisig, standardMultisig] = await ethers.getSigners(); + [deployer, securityMultisig, operationsMultisig] = await ethers.getSigners(); }); describe("Storage Layout Validation", function () { @@ -129,12 +129,12 @@ describe("Upgrade Safety Validation Tests", function () { // Verify initialization worked correctly const DEFAULT_ADMIN_ROLE = ethers.ZeroHash; - const STANDARD_ROLE = ethers.keccak256(ethers.toUtf8Bytes("STANDARD_ROLE")); + const OPERATIONS_ROLE = ethers.keccak256(ethers.toUtf8Bytes("OPERATIONS_ROLE")); // PCR0Manager now grants initial roles to deployer - const CRITICAL_ROLE = ethers.keccak256(ethers.toUtf8Bytes("CRITICAL_ROLE")); - expect(await pcr0Manager.hasRole(CRITICAL_ROLE, deployer.address)).to.be.true; - expect(await pcr0Manager.hasRole(STANDARD_ROLE, deployer.address)).to.be.true; + const SECURITY_ROLE = ethers.keccak256(ethers.toUtf8Bytes("SECURITY_ROLE")); + expect(await pcr0Manager.hasRole(SECURITY_ROLE, deployer.address)).to.be.true; + expect(await pcr0Manager.hasRole(OPERATIONS_ROLE, deployer.address)).to.be.true; }); it("should prevent initialization with zero addresses", async function () { @@ -146,8 +146,8 @@ describe("Upgrade Safety Validation Tests", function () { await pcr0Manager.waitForDeployment(); // Verify deployer has initial roles - const CRITICAL_ROLE = ethers.keccak256(ethers.toUtf8Bytes("CRITICAL_ROLE")); - expect(await pcr0Manager.hasRole(CRITICAL_ROLE, deployer.address)).to.be.true; + const SECURITY_ROLE = ethers.keccak256(ethers.toUtf8Bytes("SECURITY_ROLE")); + expect(await pcr0Manager.hasRole(SECURITY_ROLE, deployer.address)).to.be.true; }); }); diff --git a/contracts/test/unit/ImplRoot.test.ts b/contracts/test/unit/ImplRoot.test.ts index 0fa0838b6..59369fc96 100644 --- a/contracts/test/unit/ImplRoot.test.ts +++ b/contracts/test/unit/ImplRoot.test.ts @@ -6,15 +6,15 @@ import { SignerWithAddress } from "@nomicfoundation/hardhat-ethers/signers"; describe("ImplRoot", () => { let mockImplRoot: MockImplRoot; let deployer: SignerWithAddress; - let criticalMultisig: SignerWithAddress; - let standardMultisig: SignerWithAddress; + let securityMultisig: SignerWithAddress; + let operationsMultisig: SignerWithAddress; let user1: SignerWithAddress; - const CRITICAL_ROLE = ethers.keccak256(ethers.toUtf8Bytes("CRITICAL_ROLE")); - const STANDARD_ROLE = ethers.keccak256(ethers.toUtf8Bytes("STANDARD_ROLE")); + const SECURITY_ROLE = ethers.keccak256(ethers.toUtf8Bytes("SECURITY_ROLE")); + const OPERATIONS_ROLE = ethers.keccak256(ethers.toUtf8Bytes("OPERATIONS_ROLE")); beforeEach(async () => { - [deployer, criticalMultisig, standardMultisig, user1] = await ethers.getSigners(); + [deployer, securityMultisig, operationsMultisig, user1] = await ethers.getSigners(); const MockImplRootFactory = await ethers.getContractFactory("MockImplRoot", deployer); mockImplRoot = await MockImplRootFactory.deploy(); @@ -23,8 +23,8 @@ describe("ImplRoot", () => { describe("Role Constants", () => { it("should have correct role constants", async () => { - expect(await mockImplRoot.CRITICAL_ROLE()).to.equal(CRITICAL_ROLE); - expect(await mockImplRoot.STANDARD_ROLE()).to.equal(STANDARD_ROLE); + expect(await mockImplRoot.SECURITY_ROLE()).to.equal(SECURITY_ROLE); + expect(await mockImplRoot.OPERATIONS_ROLE()).to.equal(OPERATIONS_ROLE); }); }); @@ -47,12 +47,12 @@ describe("ImplRoot", () => { await freshContract.exposed__ImplRoot_init(); // Check role assignments - deployer should have both roles - expect(await freshContract.hasRole(CRITICAL_ROLE, deployer.address)).to.be.true; - expect(await freshContract.hasRole(STANDARD_ROLE, deployer.address)).to.be.true; + expect(await freshContract.hasRole(SECURITY_ROLE, deployer.address)).to.be.true; + expect(await freshContract.hasRole(OPERATIONS_ROLE, deployer.address)).to.be.true; - // Check role admins - CRITICAL_ROLE manages all roles - expect(await freshContract.getRoleAdmin(CRITICAL_ROLE)).to.equal(CRITICAL_ROLE); - expect(await freshContract.getRoleAdmin(STANDARD_ROLE)).to.equal(CRITICAL_ROLE); + // Check role admins - SECURITY_ROLE manages all roles + expect(await freshContract.getRoleAdmin(SECURITY_ROLE)).to.equal(SECURITY_ROLE); + expect(await freshContract.getRoleAdmin(OPERATIONS_ROLE)).to.equal(SECURITY_ROLE); }); it("should allow role transfer after initialization", async () => { @@ -64,20 +64,20 @@ describe("ImplRoot", () => { await freshContract.exposed__ImplRoot_init(); // Transfer roles to multisigs - await freshContract.connect(deployer).grantRole(CRITICAL_ROLE, criticalMultisig.address); - await freshContract.connect(deployer).grantRole(STANDARD_ROLE, standardMultisig.address); + await freshContract.connect(deployer).grantRole(SECURITY_ROLE, securityMultisig.address); + await freshContract.connect(deployer).grantRole(OPERATIONS_ROLE, operationsMultisig.address); // Verify multisigs have roles - expect(await freshContract.hasRole(CRITICAL_ROLE, criticalMultisig.address)).to.be.true; - expect(await freshContract.hasRole(STANDARD_ROLE, standardMultisig.address)).to.be.true; + expect(await freshContract.hasRole(SECURITY_ROLE, securityMultisig.address)).to.be.true; + expect(await freshContract.hasRole(OPERATIONS_ROLE, operationsMultisig.address)).to.be.true; // Deployer can renounce roles - await freshContract.connect(deployer).renounceRole(CRITICAL_ROLE, deployer.address); - await freshContract.connect(deployer).renounceRole(STANDARD_ROLE, deployer.address); + await freshContract.connect(deployer).renounceRole(SECURITY_ROLE, deployer.address); + await freshContract.connect(deployer).renounceRole(OPERATIONS_ROLE, deployer.address); // Verify deployer no longer has roles - expect(await freshContract.hasRole(CRITICAL_ROLE, deployer.address)).to.be.false; - expect(await freshContract.hasRole(STANDARD_ROLE, deployer.address)).to.be.false; + expect(await freshContract.hasRole(SECURITY_ROLE, deployer.address)).to.be.false; + expect(await freshContract.hasRole(OPERATIONS_ROLE, deployer.address)).to.be.false; }); }); @@ -91,59 +91,59 @@ describe("ImplRoot", () => { // Initialize with deployer having roles, then transfer to multisigs await initializedContract.exposed__ImplRoot_init(); - await initializedContract.connect(deployer).grantRole(CRITICAL_ROLE, criticalMultisig.address); - await initializedContract.connect(deployer).grantRole(STANDARD_ROLE, standardMultisig.address); + await initializedContract.connect(deployer).grantRole(SECURITY_ROLE, securityMultisig.address); + await initializedContract.connect(deployer).grantRole(OPERATIONS_ROLE, operationsMultisig.address); }); it("should allow critical multisig to grant roles", async () => { await expect( - initializedContract.connect(criticalMultisig).grantRole(STANDARD_ROLE, user1.address) + initializedContract.connect(securityMultisig).grantRole(OPERATIONS_ROLE, user1.address) ).to.not.be.reverted; - expect(await initializedContract.hasRole(STANDARD_ROLE, user1.address)).to.be.true; + expect(await initializedContract.hasRole(OPERATIONS_ROLE, user1.address)).to.be.true; }); it("should allow critical multisig to revoke roles", async () => { // First grant a role to user1 - await initializedContract.connect(criticalMultisig).grantRole(STANDARD_ROLE, user1.address); - expect(await initializedContract.hasRole(STANDARD_ROLE, user1.address)).to.be.true; + await initializedContract.connect(securityMultisig).grantRole(OPERATIONS_ROLE, user1.address); + expect(await initializedContract.hasRole(OPERATIONS_ROLE, user1.address)).to.be.true; // Then revoke it await expect( - initializedContract.connect(criticalMultisig).revokeRole(STANDARD_ROLE, user1.address) + initializedContract.connect(securityMultisig).revokeRole(OPERATIONS_ROLE, user1.address) ).to.not.be.reverted; - expect(await initializedContract.hasRole(STANDARD_ROLE, user1.address)).to.be.false; + expect(await initializedContract.hasRole(OPERATIONS_ROLE, user1.address)).to.be.false; }); it("should prevent standard multisig from granting critical role", async () => { await expect( - initializedContract.connect(standardMultisig).grantRole(CRITICAL_ROLE, user1.address) + initializedContract.connect(operationsMultisig).grantRole(SECURITY_ROLE, user1.address) ).to.be.revertedWithCustomError(initializedContract, "AccessControlUnauthorizedAccount"); }); it("should prevent unauthorized users from granting roles", async () => { await expect( - initializedContract.connect(user1).grantRole(STANDARD_ROLE, user1.address) + initializedContract.connect(user1).grantRole(OPERATIONS_ROLE, user1.address) ).to.be.revertedWithCustomError(initializedContract, "AccessControlUnauthorizedAccount"); }); it("should allow role holders to renounce their own roles", async () => { // Grant role to user1 - await initializedContract.connect(criticalMultisig).grantRole(STANDARD_ROLE, user1.address); - expect(await initializedContract.hasRole(STANDARD_ROLE, user1.address)).to.be.true; + await initializedContract.connect(securityMultisig).grantRole(OPERATIONS_ROLE, user1.address); + expect(await initializedContract.hasRole(OPERATIONS_ROLE, user1.address)).to.be.true; // User1 can renounce their own role await expect( - initializedContract.connect(user1).renounceRole(STANDARD_ROLE, user1.address) + initializedContract.connect(user1).renounceRole(OPERATIONS_ROLE, user1.address) ).to.not.be.reverted; - expect(await initializedContract.hasRole(STANDARD_ROLE, user1.address)).to.be.false; + expect(await initializedContract.hasRole(OPERATIONS_ROLE, user1.address)).to.be.false; }); it("should prevent users from renouncing others' roles", async () => { await expect( - initializedContract.connect(user1).renounceRole(CRITICAL_ROLE, criticalMultisig.address) + initializedContract.connect(user1).renounceRole(SECURITY_ROLE, securityMultisig.address) ).to.be.revertedWithCustomError(initializedContract, "AccessControlBadConfirmation"); }); }); @@ -158,8 +158,8 @@ describe("ImplRoot", () => { // Initialize and transfer roles await initializedContract.exposed__ImplRoot_init(); - await initializedContract.connect(deployer).grantRole(CRITICAL_ROLE, criticalMultisig.address); - await initializedContract.connect(deployer).grantRole(STANDARD_ROLE, standardMultisig.address); + await initializedContract.connect(deployer).grantRole(SECURITY_ROLE, securityMultisig.address); + await initializedContract.connect(deployer).grantRole(OPERATIONS_ROLE, operationsMultisig.address); }); it("should allow critical multisig to authorize upgrades", async () => { @@ -167,21 +167,21 @@ describe("ImplRoot", () => { // Note: _authorizeUpgrade is internal and can only be called through proxy upgrade mechanism // We test this by verifying the critical multisig has the required role - expect(await initializedContract.hasRole(CRITICAL_ROLE, criticalMultisig.address)).to.be.true; + expect(await initializedContract.hasRole(SECURITY_ROLE, securityMultisig.address)).to.be.true; }); it("should prevent standard multisig from authorizing upgrades", async () => { const newImplementation = ethers.Wallet.createRandom().address; - // Standard multisig should not have CRITICAL_ROLE - expect(await initializedContract.hasRole(CRITICAL_ROLE, standardMultisig.address)).to.be.false; + // Standard multisig should not have SECURITY_ROLE + expect(await initializedContract.hasRole(SECURITY_ROLE, operationsMultisig.address)).to.be.false; }); it("should prevent unauthorized users from authorizing upgrades", async () => { const newImplementation = ethers.Wallet.createRandom().address; - // Unauthorized users should not have CRITICAL_ROLE - expect(await initializedContract.hasRole(CRITICAL_ROLE, user1.address)).to.be.false; + // Unauthorized users should not have SECURITY_ROLE + expect(await initializedContract.hasRole(SECURITY_ROLE, user1.address)).to.be.false; }); }); @@ -196,28 +196,28 @@ describe("ImplRoot", () => { await initializedContract.exposed__ImplRoot_init(); }); - it("should have CRITICAL_ROLE as admin of both roles", async () => { - expect(await initializedContract.getRoleAdmin(CRITICAL_ROLE)).to.equal(CRITICAL_ROLE); - expect(await initializedContract.getRoleAdmin(STANDARD_ROLE)).to.equal(CRITICAL_ROLE); + it("should have SECURITY_ROLE as admin of both roles", async () => { + expect(await initializedContract.getRoleAdmin(SECURITY_ROLE)).to.equal(SECURITY_ROLE); + expect(await initializedContract.getRoleAdmin(OPERATIONS_ROLE)).to.equal(SECURITY_ROLE); }); - it("should allow CRITICAL_ROLE holders to manage STANDARD_ROLE", async () => { - // Grant CRITICAL_ROLE to criticalMultisig - await initializedContract.connect(deployer).grantRole(CRITICAL_ROLE, criticalMultisig.address); + it("should allow SECURITY_ROLE holders to manage OPERATIONS_ROLE", async () => { + // Grant SECURITY_ROLE to securityMultisig + await initializedContract.connect(deployer).grantRole(SECURITY_ROLE, securityMultisig.address); - // Critical multisig should be able to grant STANDARD_ROLE + // Critical multisig should be able to grant OPERATIONS_ROLE await expect( - initializedContract.connect(criticalMultisig).grantRole(STANDARD_ROLE, user1.address) + initializedContract.connect(securityMultisig).grantRole(OPERATIONS_ROLE, user1.address) ).to.not.be.reverted; - expect(await initializedContract.hasRole(STANDARD_ROLE, user1.address)).to.be.true; + expect(await initializedContract.hasRole(OPERATIONS_ROLE, user1.address)).to.be.true; - // Critical multisig should be able to revoke STANDARD_ROLE + // Critical multisig should be able to revoke OPERATIONS_ROLE await expect( - initializedContract.connect(criticalMultisig).revokeRole(STANDARD_ROLE, user1.address) + initializedContract.connect(securityMultisig).revokeRole(OPERATIONS_ROLE, user1.address) ).to.not.be.reverted; - expect(await initializedContract.hasRole(STANDARD_ROLE, user1.address)).to.be.false; + expect(await initializedContract.hasRole(OPERATIONS_ROLE, user1.address)).to.be.false; }); }); @@ -236,35 +236,35 @@ describe("ImplRoot", () => { await contract.exposed__ImplRoot_init(); console.log("✅ Step 2: Contract initialized"); - console.log(` - Deployer has CRITICAL_ROLE: ${await contract.hasRole(CRITICAL_ROLE, deployer.address)}`); - console.log(` - Deployer has STANDARD_ROLE: ${await contract.hasRole(STANDARD_ROLE, deployer.address)}`); + console.log(` - Deployer has SECURITY_ROLE: ${await contract.hasRole(SECURITY_ROLE, deployer.address)}`); + console.log(` - Deployer has OPERATIONS_ROLE: ${await contract.hasRole(OPERATIONS_ROLE, deployer.address)}`); // 3. Grant roles to multisigs - await contract.connect(deployer).grantRole(CRITICAL_ROLE, criticalMultisig.address); - await contract.connect(deployer).grantRole(STANDARD_ROLE, standardMultisig.address); + await contract.connect(deployer).grantRole(SECURITY_ROLE, securityMultisig.address); + await contract.connect(deployer).grantRole(OPERATIONS_ROLE, operationsMultisig.address); console.log("✅ Step 3: Roles granted to multisigs"); - console.log(` - Critical multisig has CRITICAL_ROLE: ${await contract.hasRole(CRITICAL_ROLE, criticalMultisig.address)}`); - console.log(` - Standard multisig has STANDARD_ROLE: ${await contract.hasRole(STANDARD_ROLE, standardMultisig.address)}`); + console.log(` - Critical multisig has SECURITY_ROLE: ${await contract.hasRole(SECURITY_ROLE, securityMultisig.address)}`); + console.log(` - Standard multisig has OPERATIONS_ROLE: ${await contract.hasRole(OPERATIONS_ROLE, operationsMultisig.address)}`); // 4. Verify multisigs can operate (check role permissions) - expect(await contract.hasRole(CRITICAL_ROLE, criticalMultisig.address)).to.be.true; + expect(await contract.hasRole(SECURITY_ROLE, securityMultisig.address)).to.be.true; console.log("✅ Step 4: Multisigs verified functional"); // 5. Renounce deployer roles - await contract.connect(deployer).renounceRole(CRITICAL_ROLE, deployer.address); - await contract.connect(deployer).renounceRole(STANDARD_ROLE, deployer.address); + await contract.connect(deployer).renounceRole(SECURITY_ROLE, deployer.address); + await contract.connect(deployer).renounceRole(OPERATIONS_ROLE, deployer.address); console.log("✅ Step 5: Deployer roles renounced"); - console.log(` - Deployer has CRITICAL_ROLE: ${await contract.hasRole(CRITICAL_ROLE, deployer.address)}`); - console.log(` - Deployer has STANDARD_ROLE: ${await contract.hasRole(STANDARD_ROLE, deployer.address)}`); + console.log(` - Deployer has SECURITY_ROLE: ${await contract.hasRole(SECURITY_ROLE, deployer.address)}`); + console.log(` - Deployer has OPERATIONS_ROLE: ${await contract.hasRole(OPERATIONS_ROLE, deployer.address)}`); // 6. Final verification - expect(await contract.hasRole(CRITICAL_ROLE, criticalMultisig.address)).to.be.true; - expect(await contract.hasRole(STANDARD_ROLE, standardMultisig.address)).to.be.true; - expect(await contract.hasRole(CRITICAL_ROLE, deployer.address)).to.be.false; - expect(await contract.hasRole(STANDARD_ROLE, deployer.address)).to.be.false; + expect(await contract.hasRole(SECURITY_ROLE, securityMultisig.address)).to.be.true; + expect(await contract.hasRole(OPERATIONS_ROLE, operationsMultisig.address)).to.be.true; + expect(await contract.hasRole(SECURITY_ROLE, deployer.address)).to.be.false; + expect(await contract.hasRole(OPERATIONS_ROLE, deployer.address)).to.be.false; console.log("🎉 Complete ImplRoot workflow successful!"); }); diff --git a/contracts/test/unit/PCR0Manager.test.ts b/contracts/test/unit/PCR0Manager.test.ts index 3b8ae7254..a34cb0bfa 100644 --- a/contracts/test/unit/PCR0Manager.test.ts +++ b/contracts/test/unit/PCR0Manager.test.ts @@ -35,7 +35,7 @@ describe("PCR0Manager", function () { it("should not allow non-owner to add PCR0 value", async function () { await expect(pcr0Manager.connect(other).addPCR0(samplePCR0)) .to.be.revertedWithCustomError(pcr0Manager, "AccessControlUnauthorizedAccount") - .withArgs(other.address, await pcr0Manager.CRITICAL_ROLE()); + .withArgs(other.address, await pcr0Manager.SECURITY_ROLE()); }); it("should not allow adding PCR0 with invalid size", async function () { @@ -67,7 +67,7 @@ describe("PCR0Manager", function () { it("should not allow non-owner to remove PCR0 value", async function () { await expect(pcr0Manager.connect(other).removePCR0(samplePCR0)) .to.be.revertedWithCustomError(pcr0Manager, "AccessControlUnauthorizedAccount") - .withArgs(other.address, await pcr0Manager.CRITICAL_ROLE()); + .withArgs(other.address, await pcr0Manager.SECURITY_ROLE()); }); it("should not allow removing non-existent PCR0", async function () {