Merge pull request #11 from TogetherCrew/roles

Roles

contracts/OIDAccessManager.sol

@@ -5,6 +5,10 @@ pragma solidity 0.8.26;
 import {AccessManagerUpgradeable} from "@openzeppelin/contracts-upgradeable/access/manager/AccessManagerUpgradeable.sol";
 
 contract OIDAccessManager is AccessManagerUpgradeable {
+    uint64 public constant APPLICATION_MANAGER_ROLE = 1;
+    uint64 public constant ATTESTATION_MANAGER_ROLE = 2;
+    uint64 public constant PERMISSION_MANAGER_ROLE = 3;
+
     function initialize() public initializer {
         __AccessManager_init(msg.sender);
     }

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;
     }
 

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);
         }

package.json

@@ -22,6 +22,7 @@
 		"ethers": "^6.13.1",
 		"hardhat": "^2.14.0",
 		"hardhat-gas-reporter": "^1.0.8",
+		"husky": "^9.1.5",
 		"solidity-coverage": "^0.8.0",
 		"ts-node": ">=8.0.0",
 		"typescript": "~5.0.4",

test/ApplicationManager.ts

@@ -9,15 +9,14 @@ import {
 	toFunctionSelector,
 } from "viem";
 import { generatePrivateKey, privateKeyToAccount } from "viem/accounts";
+import { deployAccessManager } from "../utils/deployAccessManager";
 
 interface Application {
 	id?: bigint;
 	name: string;
 	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,
 		};
 	}
 

test/OIDAccessManager.ts

@@ -30,5 +30,17 @@ describe("OIDAccessManager", () => {
 				await contract.read.hasRole([ADMIN_ROLE, deployer.account.address]),
 			).to.deep.eq([true, 0]);
 		});
+		it("Should have APPLICATION_MANAGER_ROLE", async () => {
+			const { contract, deployer } = await loadFixture(deploy);
+			expect(await contract.read.APPLICATION_MANAGER_ROLE()).to.eq(1n);
+		});
+		it("Should have ATTESTATION_MANAGER_ROLE", async () => {
+			const { contract, deployer } = await loadFixture(deploy);
+			expect(await contract.read.ATTESTATION_MANAGER_ROLE()).to.eq(2n);
+		});
+		it("Should have PERMISSION_MANAGER_ROLE", async () => {
+			const { contract, deployer } = await loadFixture(deploy);
+			expect(await contract.read.PERMISSION_MANAGER_ROLE()).to.eq(3n);
+		});
 	});
 });

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<Transport, Chain, Account>) {
-		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,
 		]);

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<Transport, Chain, Account>) {
-	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),
+			);
 		});
 	});
 

utils/deployAccessManager.ts

@@ -0,0 +1,10 @@
+import hre from "hardhat";
+import type { WalletClient } from "viem";
+
+export async function deployAccessManager(deployer: WalletClient) {
+	const contract = await hre.viem.deployContract("OIDAccessManager", [], {
+		account: deployer.account,
+	});
+	await contract.write.initialize();
+	return contract;
+}

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<Transport, Chain, Account>,
+	registryAddress: string,
+	schema: string,
+	resolverAddress = "0x0000000000000000000000000000000000000000",
+	revocable = true,
+): Promise<Address> {
+	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;
+}