From 70d3749d0876bb943c3bfa5780095d928ad5bece Mon Sep 17 00:00:00 2001 From: Cyrille Derche Date: Fri, 30 Aug 2024 18:47:22 +0200 Subject: [PATCH] Migrate to AccessManager constants --- contracts/OIDPermissionManager.sol | 7 ++- contracts/OIDResolver.sol | 8 +++- test/ApplicationManager.ts | 26 +++++++---- test/OIDPermissionManager.ts | 49 +++++--------------- test/OIDResolver.ts | 74 +++++++++++------------------- utils/deployEAS.ts | 39 ++++++++++++++++ utils/roles.ts | 14 ------ 7 files changed, 106 insertions(+), 111 deletions(-) create mode 100644 utils/deployEAS.ts delete mode 100644 utils/roles.ts diff --git a/contracts/OIDPermissionManager.sol b/contracts/OIDPermissionManager.sol index 0704a32..3c81b1f 100644 --- a/contracts/OIDPermissionManager.sol +++ b/contracts/OIDPermissionManager.sol @@ -6,6 +6,7 @@ import {AccessManaged} from "@openzeppelin/contracts/access/manager/AccessManage import {IEAS} from "@ethereum-attestation-service/eas-contracts/contracts/IEAS.sol"; import {Attestation} from "@ethereum-attestation-service/eas-contracts/contracts/Common.sol"; import {IAccessManager} from "@openzeppelin/contracts/access/manager/IAccessManager.sol"; +import {OIDAccessManager} from "./OIDAccessManager.sol"; contract OIDPermissionManager is IOIDPermissionManager, AccessManaged { error UnauthorizedAccess(address caller); @@ -64,7 +65,11 @@ contract OIDPermissionManager is IOIDPermissionManager, AccessManaged { } function _isPermissionManager() internal view returns (bool) { - (bool isMember, ) = IAccessManager(authority()).hasRole(3, msg.sender); + OIDAccessManager access = OIDAccessManager(authority()); + (bool isMember, ) = access.hasRole( + access.PERMISSION_MANAGER_ROLE(), + msg.sender + ); return isMember; } diff --git a/contracts/OIDResolver.sol b/contracts/OIDResolver.sol index 97be5a3..e484b7a 100644 --- a/contracts/OIDResolver.sol +++ b/contracts/OIDResolver.sol @@ -6,7 +6,7 @@ import {AccessManagedUpgradeable} from "@openzeppelin/contracts-upgradeable/acce import {IEAS} from "@ethereum-attestation-service/eas-contracts/contracts/IEAS.sol"; import {Attestation} from "@ethereum-attestation-service/eas-contracts/contracts/Common.sol"; import {SchemaResolver} from "@ethereum-attestation-service/eas-contracts/contracts/resolver/SchemaResolver.sol"; -import {IAccessManager} from "@openzeppelin/contracts/access/manager/IAccessManager.sol"; +import {OIDAccessManager} from "./OIDAccessManager.sol"; contract OIDResolver is SchemaResolver, AccessManagedUpgradeable { error UnauthorizedAttester(address attester); @@ -47,7 +47,11 @@ contract OIDResolver is SchemaResolver, AccessManagedUpgradeable { } function _checkAttester(address attester) internal virtual { - (bool isMember, ) = IAccessManager(authority()).hasRole(2, attester); + OIDAccessManager authority = OIDAccessManager(authority()); + (bool isMember, ) = authority.hasRole( + authority.ATTESTATION_MANAGER_ROLE(), + attester + ); if (!isMember) { revert UnauthorizedAttester(attester); } diff --git a/test/ApplicationManager.ts b/test/ApplicationManager.ts index 5ef27b3..3d753c2 100644 --- a/test/ApplicationManager.ts +++ b/test/ApplicationManager.ts @@ -9,6 +9,7 @@ import { toFunctionSelector, } from "viem"; import { generatePrivateKey, privateKeyToAccount } from "viem/accounts"; +import { deployAccessManager } from "../utils/deployAccessManager"; interface Application { id?: bigint; @@ -16,8 +17,6 @@ interface Application { account: Address; } -const MANAGER_ROLE = 1n; - const CREATE_APPLICATION_SELECTOR = toFunctionSelector( "createApplication((string, address))", ); @@ -42,13 +41,23 @@ describe("ApplicationManager", () => { // Contracts are deployed using the first signer/account by default const [deployer, manager, otherAccount] = await hre.viem.getWalletClients(); - const access = await hre.viem.deployContract("OIDAccessManager"); - await access.write.initialize(); + const access = await deployAccessManager(deployer); + + const APPLICATION_MANAGER_ROLE = + await access.read.APPLICATION_MANAGER_ROLE(); - await access.write.grantRole([MANAGER_ROLE, manager.account.address, 0]); + await access.write.grantRole([ + APPLICATION_MANAGER_ROLE, + manager.account.address, + 0, + ]); - // Assign deployer to MANAGER_ROLE for simplicity - await access.write.grantRole([MANAGER_ROLE, deployer.account.address, 0]); + // Assign deployer to APPLICATION_MANAGER_ROLE for simplicity + await access.write.grantRole([ + APPLICATION_MANAGER_ROLE, + deployer.account.address, + 0, + ]); const contract = await hre.viem.deployContract("ApplicationManager", [ access.address, @@ -61,7 +70,7 @@ describe("ApplicationManager", () => { UPDATE_APPLICATION_SELECTOR, DELETE_APPLICATION_SELECTOR, ], - MANAGER_ROLE, + APPLICATION_MANAGER_ROLE, ]); const publicClient = await hre.viem.getPublicClient(); @@ -73,6 +82,7 @@ describe("ApplicationManager", () => { manager, otherAccount, publicClient, + APPLICATION_MANAGER_ROLE, }; } diff --git a/test/OIDPermissionManager.ts b/test/OIDPermissionManager.ts index 010a31f..deb818c 100644 --- a/test/OIDPermissionManager.ts +++ b/test/OIDPermissionManager.ts @@ -20,10 +20,8 @@ import { } from "viem"; import { clientToSigner } from "../utils/clientToSigner"; import { SIMPLE_SCHEMA } from "../utils/constants"; -import { ROLES } from "../utils/roles"; - -const { ID: PERMISSION_MANAGER_ROLE_ID, LABEL: PERMISSION_MANAGER_ROLE_LABEL } = - ROLES.PERMISSION_MANAGER; +import { deployAccessManager } from "../utils/deployAccessManager"; +import { deployEAS, deploySchema } from "../utils/deployEAS"; describe("OIDPermissionManager", () => { async function attest( @@ -54,27 +52,6 @@ describe("OIDPermissionManager", () => { return { attestationUID }; } - async function deployEAS(deployer: Client) { - const registry = await hre.viem.deployContract("SchemaRegistry"); - const eas = await hre.viem.deployContract("EAS", [registry.address]); - - // Need to mix in ethers - const signer = clientToSigner(deployer); - const schemaRegistry = new SchemaRegistry(registry.address); - schemaRegistry.connect(signer); - const tx = await schemaRegistry.register({ schema: SIMPLE_SCHEMA }); - await tx.wait(); - - const events = await registry.getEvents.Registered(); - const schemaUID = events[0].args.uid as Address; - - return { - registry, - eas, - schemaUID, - }; - } - // We define a fixture to reuse the same setup in every test. // We use loadFixture to run this setup once, snapshot that state, // and reset Hardhat Network to that snapshot in every test. @@ -84,7 +61,12 @@ describe("OIDPermissionManager", () => { await hre.viem.getWalletClients(); // EAS Deployment - const { registry, eas, schemaUID } = await deployEAS(deployer); + const { registry, eas } = await deployEAS(deployer); + const schemaUID = await deploySchema( + deployer, + registry.address, + SIMPLE_SCHEMA, + ); const { attestationUID } = await attest( attester, recipient.account.address, @@ -94,21 +76,12 @@ describe("OIDPermissionManager", () => { [{ name: "id", value: 1, type: "uint256" }], ); - const access = await hre.viem.deployContract("OIDAccessManager"); - await access.write.initialize(); - await access.write.labelRole([ - PERMISSION_MANAGER_ROLE_ID, - PERMISSION_MANAGER_ROLE_LABEL, - ]); - await access.write.grantRole([ - PERMISSION_MANAGER_ROLE_ID, - manager.account.address, - 0, - ]); + const access = await deployAccessManager(deployer); + const PERMISSION_MANAGER_ROLE = await access.read.PERMISSION_MANAGER_ROLE(); // Assign manager to PERMISSION_MANAGER_ROLE await access.write.grantRole([ - PERMISSION_MANAGER_ROLE_ID, + PERMISSION_MANAGER_ROLE, manager.account.address, 0, ]); diff --git a/test/OIDResolver.ts b/test/OIDResolver.ts index b5dfa8c..cdac03e 100644 --- a/test/OIDResolver.ts +++ b/test/OIDResolver.ts @@ -24,29 +24,10 @@ import { parseSignature, zeroHash, } from "viem"; -import { generatePrivateKey, privateKeyToAccount } from "viem/accounts"; - -const ATTESTER_ROLE = 1n; - -const schema = "uint256 id"; - -function generateRandomAddress(): Address { - const randomKey = generatePrivateKey(); - const account = privateKeyToAccount(randomKey); - return account.address; -} - -function clientToSigner(client: Client) { - const { account, chain, transport } = client; - const network = { - chainId: chain.id, - name: chain.name, - ensAddress: chain.contracts?.ensRegistry?.address, - }; - const provider = new BrowserProvider(transport, network); - const signer = new JsonRpcSigner(provider, account.address); - return signer; -} +import { clientToSigner } from "../utils/clientToSigner"; +import { SIMPLE_SCHEMA } from "../utils/constants"; +import { deployAccessManager } from "../utils/deployAccessManager"; +import { deployEAS, deploySchema } from "../utils/deployEAS"; describe("OIDResolver", () => { // We define a fixture to reuse the same setup in every test. @@ -57,41 +38,36 @@ describe("OIDResolver", () => { const [deployer, attester, otherAccount] = await hre.viem.getWalletClients(); - const registry = await hre.viem.deployContract("SchemaRegistry"); + const { eas, registry } = await deployEAS(deployer); - const eas = await hre.viem.deployContract("EAS", [registry.address]); + const authority = await deployAccessManager(deployer); + const ATTESTATION_MANAGER_ROLE = + await authority.read.ATTESTATION_MANAGER_ROLE(); - const access = await hre.viem.deployContract("OIDAccessManager"); - await access.write.initialize(); - - await access.write.grantRole([ATTESTER_ROLE, attester.account.address, 0]); + await authority.write.grantRole([ + ATTESTATION_MANAGER_ROLE, + attester.account.address, + 0, + ]); const resolver = await hre.viem.deployContract("OIDResolver", [ eas.address, ]); - - await resolver.write.initialize([access.address]); + await resolver.write.initialize([authority.address]); const publicClient = await hre.viem.getPublicClient(); - // Need to mix in ethers - const signer = clientToSigner(deployer); - const schemaRegistry = new SchemaRegistry(registry.address); - schemaRegistry.connect(signer); - const tx = await schemaRegistry.register({ - schema, - resolverAddress: resolver.address, - revocable: true, - }); - await tx.wait(); - - const events = await registry.getEvents.Registered(); - const schemaUID = events[0].args.uid as Address; + const schemaUID = await deploySchema( + deployer, + registry.address, + SIMPLE_SCHEMA, + resolver.address, + ); return { registry, eas, - access, + authority, resolver, deployer, attester, @@ -109,9 +85,11 @@ describe("OIDResolver", () => { }); describe("Initialize", () => { - it("Should set AccessManager", async () => { - const { access, resolver } = await loadFixture(deploy); - expect(await resolver.read.authority()).to.eq(getAddress(access.address)); + it("Should set authority", async () => { + const { authority, resolver } = await loadFixture(deploy); + expect(await resolver.read.authority()).to.eq( + getAddress(authority.address), + ); }); }); diff --git a/utils/deployEAS.ts b/utils/deployEAS.ts new file mode 100644 index 0000000..8f46ba7 --- /dev/null +++ b/utils/deployEAS.ts @@ -0,0 +1,39 @@ +import { SchemaRegistry } from "@ethereum-attestation-service/eas-sdk"; +import hre from "hardhat"; +import type { + Account, + Address, + Chain, + Client, + Transport, + WalletClient, +} from "viem"; +import { clientToSigner } from "./clientToSigner"; + +export async function deployEAS(deployer: WalletClient) { + const registry = await hre.viem.deployContract("SchemaRegistry", [], { + account: deployer.account, + }); + const eas = await hre.viem.deployContract("EAS", [registry.address], { + account: deployer.account, + }); + return { + registry, + eas, + }; +} + +export async function deploySchema( + deployer: Client, + registryAddress: string, + schema: string, + resolverAddress = "0x0000000000000000000000000000000000000000", + revocable = true, +): Promise
{ + const registry = new SchemaRegistry(registryAddress); + + const signer = clientToSigner(deployer); + registry.connect(signer); + const tx = await registry.register({ schema, resolverAddress, revocable }); + return (await tx.wait()) as Address; +} diff --git a/utils/roles.ts b/utils/roles.ts deleted file mode 100644 index 5f17241..0000000 --- a/utils/roles.ts +++ /dev/null @@ -1,14 +0,0 @@ -export const ROLES = { - APPLICATION_MANAGER: { - ID: 1n, - LABEL: "APPLICATION_MANAGER_ROLE", - }, - ATTESTER: { - ID: 2n, - LABEL: "ATTESTER_ROLE", - }, - PERMISSION_MANAGER: { - ID: 3n, - LABEL: "PERMISSION_MANAGER_ROLE", - }, -};