feat: add verifier role (#148)

* feat: add verifier collection role

* refactor: apply verifier role to verify ap functions

* test: hardhat tests for verifier role

* refactor: grant verifier role for deployer

* test: foundry tests for verifier role

* test: fix fixture after changing deployer to be verifier

* test: change token owner to match connected account on verifier role check

contracts/contracts/FleekAccessControl.sol

@@ -14,7 +14,8 @@ contract FleekAccessControl is Initializable {
      * @dev All available collection roles.
      */
     enum CollectionRoles {
-        Owner
+        Owner,
+        Verifier
     }
 
     /**
@@ -78,6 +79,7 @@ contract FleekAccessControl is Initializable {
      */
     function __FleekAccessControl_init() internal onlyInitializing {
         _grantCollectionRole(CollectionRoles.Owner, msg.sender);
+        _grantCollectionRole(CollectionRoles.Verifier, msg.sender);
     }
 
     /**

contracts/contracts/FleekERC721.sol

@@ -604,13 +604,13 @@ contract FleekERC721 is Initializable, ERC721Upgradeable, FleekAccessControl, Fl
      * Requirements:
      *
      * - the AP must exist.
-     * - the sender must have the token controller role.
+     * - the sender must have collection verifier role.
      *
      */
     function setAccessPointContentVerify(
         string memory apName,
         bool verified
-    ) public requireAP(apName) requireTokenRole(_accessPoints[apName].tokenId, TokenRoles.Controller) {
+    ) public requireAP(apName) requireCollectionRole(CollectionRoles.Verifier) {
         _accessPoints[apName].contentVerified = verified;
         emit ChangeAccessPointContentVerify(apName, _accessPoints[apName].tokenId, verified, msg.sender);
     }
@@ -623,13 +623,13 @@ contract FleekERC721 is Initializable, ERC721Upgradeable, FleekAccessControl, Fl
      * Requirements:
      *
      * - the AP must exist.
-     * - the sender must have the token controller role.
+     * - the sender must have collection verifier role.
      *
      */
     function setAccessPointNameVerify(
         string memory apName,
         bool verified
-    ) public requireAP(apName) requireTokenRole(_accessPoints[apName].tokenId, TokenRoles.Controller) {
+    ) public requireAP(apName) requireCollectionRole(CollectionRoles.Verifier) {
         _accessPoints[apName].nameVerified = verified;
         emit ChangeAccessPointNameVerify(apName, _accessPoints[apName].tokenId, verified, msg.sender);
     }

contracts/test/foundry/FleekERC721/AccessControl.t.sol

@@ -19,6 +19,7 @@ contract Test_FleekERC721_AccessControlAssertions is Test {
 contract Test_FleekERC721_AccessControl is Test_FleekERC721_Base, Test_FleekERC721_AccessControlAssertions {
     uint256 internal tokenId;
     address internal collectionOwner = address(100);
+    address internal collectionVerifier = address(101);
     address internal tokenOwner = address(200);
     address internal tokenController = address(300);
     address internal anyAddress = address(400);
@@ -28,6 +29,8 @@ contract Test_FleekERC721_AccessControl is Test_FleekERC721_Base, Test_FleekERC7
 
         // Set collectionOwner
         CuT.grantCollectionRole(FleekAccessControl.CollectionRoles.Owner, collectionOwner);
+        // Set collectionVerifier
+        CuT.grantCollectionRole(FleekAccessControl.CollectionRoles.Verifier, collectionVerifier);
         // Mint to tokenOwner to set tokenOwner
         mintDefault(tokenOwner);
         // Set tokenController to minted token
@@ -36,20 +39,33 @@ contract Test_FleekERC721_AccessControl is Test_FleekERC721_Base, Test_FleekERC7
     }
 
     function test_setUp() public {
+        // Check deployer
+        assertTrue(CuT.hasCollectionRole(FleekAccessControl.CollectionRoles.Owner, deployer));
+        assertTrue(CuT.hasCollectionRole(FleekAccessControl.CollectionRoles.Verifier, deployer));
+
         // Check collectionOwner
         assertTrue(CuT.hasCollectionRole(FleekAccessControl.CollectionRoles.Owner, collectionOwner));
+        assertFalse(CuT.hasCollectionRole(FleekAccessControl.CollectionRoles.Verifier, collectionOwner));
         assertFalse(CuT.ownerOf(tokenId) == collectionOwner);
         assertFalse(CuT.hasTokenRole(tokenId, FleekAccessControl.TokenRoles.Controller, collectionOwner));
+        // Check collectionVerifier
+        assertFalse(CuT.hasCollectionRole(FleekAccessControl.CollectionRoles.Owner, collectionVerifier));
+        assertTrue(CuT.hasCollectionRole(FleekAccessControl.CollectionRoles.Verifier, collectionVerifier));
+        assertFalse(CuT.ownerOf(tokenId) == collectionVerifier);
+        assertFalse(CuT.hasTokenRole(tokenId, FleekAccessControl.TokenRoles.Controller, collectionVerifier));
         // Check tokenOwner
         assertFalse(CuT.hasCollectionRole(FleekAccessControl.CollectionRoles.Owner, tokenOwner));
+        assertFalse(CuT.hasCollectionRole(FleekAccessControl.CollectionRoles.Verifier, tokenOwner));
         assertTrue(CuT.ownerOf(tokenId) == tokenOwner);
         assertFalse(CuT.hasTokenRole(tokenId, FleekAccessControl.TokenRoles.Controller, tokenOwner));
         // Check tokenController
         assertFalse(CuT.hasCollectionRole(FleekAccessControl.CollectionRoles.Owner, tokenController));
+        assertFalse(CuT.hasCollectionRole(FleekAccessControl.CollectionRoles.Verifier, tokenController));
         assertFalse(CuT.ownerOf(tokenId) == tokenController);
         assertTrue(CuT.hasTokenRole(tokenId, FleekAccessControl.TokenRoles.Controller, tokenController));
         // Check anyAddress
         assertFalse(CuT.hasCollectionRole(FleekAccessControl.CollectionRoles.Owner, anyAddress));
+        assertFalse(CuT.hasCollectionRole(FleekAccessControl.CollectionRoles.Verifier, anyAddress));
         assertFalse(CuT.ownerOf(tokenId) == anyAddress);
         assertFalse(CuT.hasTokenRole(tokenId, FleekAccessControl.TokenRoles.Controller, anyAddress));
     }
@@ -65,6 +81,14 @@ contract Test_FleekERC721_AccessControl is Test_FleekERC721_Base, Test_FleekERC7
         assertFalse(CuT.hasCollectionRole(FleekAccessControl.CollectionRoles.Owner, randomAddress));
         vm.stopPrank();
 
+        // CollectionVerifier
+        vm.startPrank(collectionVerifier);
+        expectRevertWithCollectionRole(FleekAccessControl.CollectionRoles.Owner);
+        CuT.grantCollectionRole(FleekAccessControl.CollectionRoles.Owner, randomAddress);
+        expectRevertWithCollectionRole(FleekAccessControl.CollectionRoles.Owner);
+        CuT.revokeCollectionRole(FleekAccessControl.CollectionRoles.Owner, randomAddress);
+        vm.stopPrank();
+
         // TokenOwner
         vm.startPrank(tokenOwner);
         expectRevertWithCollectionRole(FleekAccessControl.CollectionRoles.Owner);
@@ -101,6 +125,14 @@ contract Test_FleekERC721_AccessControl is Test_FleekERC721_Base, Test_FleekERC7
         CuT.revokeTokenRole(tokenId, FleekAccessControl.TokenRoles.Controller, randomAddress);
         vm.stopPrank();
 
+        // CollectionVerifier
+        vm.startPrank(collectionVerifier);
+        expectRevertWithMustBeTokenOwner(tokenId);
+        CuT.grantTokenRole(tokenId, FleekAccessControl.TokenRoles.Controller, randomAddress);
+        expectRevertWithMustBeTokenOwner(tokenId);
+        CuT.revokeTokenRole(tokenId, FleekAccessControl.TokenRoles.Controller, randomAddress);
+        vm.stopPrank();
+
         // TokenOwner
         vm.startPrank(tokenOwner);
         CuT.grantTokenRole(tokenId, FleekAccessControl.TokenRoles.Controller, randomAddress);
@@ -147,6 +179,11 @@ contract Test_FleekERC721_AccessControl is Test_FleekERC721_Base, Test_FleekERC7
         expectRevertWithTokenRole(tokenId, FleekAccessControl.TokenRoles.Controller);
         CuT.setTokenExternalURL(tokenId, externalURL);
 
+        // VerifierRole
+        vm.prank(collectionVerifier);
+        expectRevertWithTokenRole(tokenId, FleekAccessControl.TokenRoles.Controller);
+        CuT.setTokenExternalURL(tokenId, externalURL);
+
         // TokenOwner
         vm.prank(tokenOwner);
         CuT.setTokenExternalURL(tokenId, externalURL);
@@ -169,6 +206,11 @@ contract Test_FleekERC721_AccessControl is Test_FleekERC721_Base, Test_FleekERC7
         expectRevertWithTokenRole(tokenId, FleekAccessControl.TokenRoles.Controller);
         CuT.setTokenENS(tokenId, ens);
 
+        // VerifierRole
+        vm.prank(collectionVerifier);
+        expectRevertWithTokenRole(tokenId, FleekAccessControl.TokenRoles.Controller);
+        CuT.setTokenENS(tokenId, ens);
+
         // TokenOwner
         vm.prank(tokenOwner);
         CuT.setTokenENS(tokenId, ens);
@@ -191,6 +233,11 @@ contract Test_FleekERC721_AccessControl is Test_FleekERC721_Base, Test_FleekERC7
         expectRevertWithTokenRole(tokenId, FleekAccessControl.TokenRoles.Controller);
         CuT.setTokenName(tokenId, name);
 
+        // VerifierRole
+        vm.prank(collectionVerifier);
+        expectRevertWithTokenRole(tokenId, FleekAccessControl.TokenRoles.Controller);
+        CuT.setTokenName(tokenId, name);
+
         // TokenOwner
         vm.prank(tokenOwner);
         CuT.setTokenName(tokenId, name);
@@ -213,6 +260,11 @@ contract Test_FleekERC721_AccessControl is Test_FleekERC721_Base, Test_FleekERC7
         expectRevertWithTokenRole(tokenId, FleekAccessControl.TokenRoles.Controller);
         CuT.setTokenDescription(tokenId, description);
 
+        // VerifierRole
+        vm.prank(collectionVerifier);
+        expectRevertWithTokenRole(tokenId, FleekAccessControl.TokenRoles.Controller);
+        CuT.setTokenDescription(tokenId, description);
+
         // TokenOwner
         vm.prank(tokenOwner);
         CuT.setTokenDescription(tokenId, description);
@@ -235,6 +287,11 @@ contract Test_FleekERC721_AccessControl is Test_FleekERC721_Base, Test_FleekERC7
         expectRevertWithTokenRole(tokenId, FleekAccessControl.TokenRoles.Controller);
         CuT.setTokenLogo(tokenId, logo);
 
+        // CollectionVerifier
+        vm.prank(collectionVerifier);
+        expectRevertWithTokenRole(tokenId, FleekAccessControl.TokenRoles.Controller);
+        CuT.setTokenLogo(tokenId, logo);
+
         // TokenOwner
         vm.prank(tokenOwner);
         CuT.setTokenLogo(tokenId, logo);
@@ -257,6 +314,11 @@ contract Test_FleekERC721_AccessControl is Test_FleekERC721_Base, Test_FleekERC7
         expectRevertWithTokenRole(tokenId, FleekAccessControl.TokenRoles.Controller);
         CuT.setTokenColor(tokenId, color);
 
+        // CollectionVerifier
+        vm.prank(collectionVerifier);
+        expectRevertWithTokenRole(tokenId, FleekAccessControl.TokenRoles.Controller);
+        CuT.setTokenColor(tokenId, color);
+
         // TokenOwner
         vm.prank(tokenOwner);
         CuT.setTokenColor(tokenId, color);
@@ -280,6 +342,11 @@ contract Test_FleekERC721_AccessControl is Test_FleekERC721_Base, Test_FleekERC7
         expectRevertWithTokenRole(tokenId, FleekAccessControl.TokenRoles.Controller);
         CuT.setTokenLogoAndColor(tokenId, logo, color);
 
+        // CollectionVerifier
+        vm.prank(collectionVerifier);
+        expectRevertWithTokenRole(tokenId, FleekAccessControl.TokenRoles.Controller);
+        CuT.setTokenLogoAndColor(tokenId, logo, color);
+
         // TokenOwner
         vm.prank(tokenOwner);
         CuT.setTokenLogoAndColor(tokenId, logo, color);
@@ -303,6 +370,11 @@ contract Test_FleekERC721_AccessControl is Test_FleekERC721_Base, Test_FleekERC7
         expectRevertWithTokenRole(tokenId, FleekAccessControl.TokenRoles.Controller);
         CuT.setTokenBuild(tokenId, commitHash, gitRepository);
 
+        // CollectionVerifier
+        vm.prank(collectionVerifier);
+        expectRevertWithTokenRole(tokenId, FleekAccessControl.TokenRoles.Controller);
+        CuT.setTokenBuild(tokenId, commitHash, gitRepository);
+
         // TokenOwner
         vm.prank(tokenOwner);
         CuT.setTokenBuild(tokenId, commitHash, gitRepository);
@@ -323,6 +395,11 @@ contract Test_FleekERC721_AccessControl is Test_FleekERC721_Base, Test_FleekERC7
         expectRevertWithMustBeTokenOwner(tokenId);
         CuT.burn(tokenId);
 
+        // CollectionVerifier
+        vm.prank(collectionVerifier);
+        expectRevertWithMustBeTokenOwner(tokenId);
+        CuT.burn(tokenId);
+
         // TokenController
         vm.prank(tokenController);
         expectRevertWithMustBeTokenOwner(tokenId);
@@ -338,11 +415,74 @@ contract Test_FleekERC721_AccessControl is Test_FleekERC721_Base, Test_FleekERC7
         CuT.burn(tokenId);
     }
 
+    function test_setAccessPointContentVerify() public {
+        string memory apName = "random.com";
+        CuT.addAccessPoint(tokenId, apName);
+
+        // CollectionOwner
+        vm.prank(collectionOwner);
+        expectRevertWithCollectionRole(FleekAccessControl.CollectionRoles.Verifier);
+        CuT.setAccessPointContentVerify(apName, true);
+
+        // CollectionVerifier
+        vm.prank(collectionVerifier);
+        CuT.setAccessPointContentVerify(apName, true);
+
+        // TokenOwner
+        vm.prank(tokenOwner);
+        expectRevertWithCollectionRole(FleekAccessControl.CollectionRoles.Verifier);
+        CuT.setAccessPointContentVerify(apName, false);
+
+        // TokenController
+        vm.prank(tokenController);
+        expectRevertWithCollectionRole(FleekAccessControl.CollectionRoles.Verifier);
+        CuT.setAccessPointContentVerify(apName, false);
+
+        // AnyAddress
+        vm.prank(anyAddress);
+        expectRevertWithCollectionRole(FleekAccessControl.CollectionRoles.Verifier);
+        CuT.setAccessPointContentVerify(apName, false);
+    }
+
+    function test_setAccessPointNameVerify() public {
+        string memory apName = "random.com";
+        CuT.addAccessPoint(tokenId, apName);
+
+        // CollectionOwner
+        vm.prank(collectionOwner);
+        expectRevertWithCollectionRole(FleekAccessControl.CollectionRoles.Verifier);
+        CuT.setAccessPointNameVerify(apName, true);
+
+        // CollectionVerifier
+        vm.prank(collectionVerifier);
+        CuT.setAccessPointNameVerify(apName, true);
+
+        // TokenOwner
+        vm.prank(tokenOwner);
+        expectRevertWithCollectionRole(FleekAccessControl.CollectionRoles.Verifier);
+        CuT.setAccessPointNameVerify(apName, false);
+
+        // TokenController
+        vm.prank(tokenController);
+        expectRevertWithCollectionRole(FleekAccessControl.CollectionRoles.Verifier);
+        CuT.setAccessPointNameVerify(apName, false);
+
+        // AnyAddress
+        vm.prank(anyAddress);
+        expectRevertWithCollectionRole(FleekAccessControl.CollectionRoles.Verifier);
+        CuT.setAccessPointNameVerify(apName, false);
+    }
+
     function test_setBilling() public {
         // ColletionOwner
         vm.prank(collectionOwner);
         CuT.setBilling(FleekBilling.Billing.Mint, 1 ether);
 
+        // CollectionVerifier
+        vm.prank(collectionVerifier);
+        expectRevertWithCollectionRole(FleekAccessControl.CollectionRoles.Owner);
+        CuT.setBilling(FleekBilling.Billing.Mint, 2 ether);
+
         // TokenOwner
         vm.prank(tokenOwner);
         expectRevertWithCollectionRole(FleekAccessControl.CollectionRoles.Owner);
@@ -365,6 +505,11 @@ contract Test_FleekERC721_AccessControl is Test_FleekERC721_Base, Test_FleekERC7
         vm.prank(collectionOwner);
         CuT.withdraw();
 
+        // CollectionVerifier
+        vm.prank(collectionVerifier);
+        expectRevertWithCollectionRole(FleekAccessControl.CollectionRoles.Owner);
+        CuT.withdraw();
+
         // TokenOwner
         vm.prank(tokenOwner);
         expectRevertWithCollectionRole(FleekAccessControl.CollectionRoles.Owner);
@@ -381,13 +526,6 @@ contract Test_FleekERC721_AccessControl is Test_FleekERC721_Base, Test_FleekERC7
         CuT.withdraw();
     }
 
-    /**
-     * @dev `receive` and `fallback` are required for test contract receive ETH
-     */
-    receive() external payable {}
-
-    fallback() external payable {}
-
     function test_pauseAndUnpause() public {
         // ColletionOwner
         vm.startPrank(collectionOwner);
@@ -395,6 +533,14 @@ contract Test_FleekERC721_AccessControl is Test_FleekERC721_Base, Test_FleekERC7
         CuT.unpause();
         vm.stopPrank();
 
+        // CollectionVerifier
+        vm.startPrank(collectionVerifier);
+        expectRevertWithCollectionRole(FleekAccessControl.CollectionRoles.Owner);
+        CuT.pause();
+        expectRevertWithCollectionRole(FleekAccessControl.CollectionRoles.Owner);
+        CuT.unpause();
+        vm.stopPrank();
+
         // TokenOwner
         vm.startPrank(tokenOwner);
         expectRevertWithCollectionRole(FleekAccessControl.CollectionRoles.Owner);
@@ -425,6 +571,11 @@ contract Test_FleekERC721_AccessControl is Test_FleekERC721_Base, Test_FleekERC7
         vm.prank(collectionOwner);
         CuT.setPausable(false);
 
+        // CollectionVerifier
+        vm.prank(collectionVerifier);
+        expectRevertWithCollectionRole(FleekAccessControl.CollectionRoles.Owner);
+        CuT.setPausable(true);
+
         // TokenOwner
         vm.prank(tokenOwner);
         expectRevertWithCollectionRole(FleekAccessControl.CollectionRoles.Owner);
@@ -464,4 +615,11 @@ contract Test_FleekERC721_AccessControl is Test_FleekERC721_Base, Test_FleekERC7
         vm.prank(tokenOwner);
         CuT.revokeTokenRole(tokenId, FleekAccessControl.TokenRoles.Controller, anyAddress);
     }
+
+    /**
+     * @dev `receive` and `fallback` are required for test contract receive ETH
+     */
+    receive() external payable {}
+
+    fallback() external payable {}
 }

contracts/test/foundry/FleekERC721/AccessPoints/AccessPointsAutoApprovalOff.t.sol

@@ -169,13 +169,13 @@ contract Test_FleekERC721_AccessPoint is Test_FleekERC721_Base, APConstants {
         CuT.addAccessPoint(tokenId, accessPointName);
 
         vm.startPrank(randomAddress);
-        expectRevertWithTokenRole(tokenId, FleekAccessControl.TokenRoles.Controller);
+        expectRevertWithCollectionRole(FleekAccessControl.CollectionRoles.Verifier);
         CuT.setAccessPointNameVerify(accessPointName, true);
-        expectRevertWithTokenRole(tokenId, FleekAccessControl.TokenRoles.Controller);
+        expectRevertWithCollectionRole(FleekAccessControl.CollectionRoles.Verifier);
         CuT.setAccessPointContentVerify(accessPointName, true);
         vm.stopPrank();
 
-        CuT.grantTokenRole(tokenId, FleekAccessControl.TokenRoles.Controller, randomAddress);
+        CuT.grantCollectionRole(FleekAccessControl.CollectionRoles.Verifier, randomAddress);
 
         vm.startPrank(randomAddress);
         CuT.setAccessPointNameVerify(accessPointName, true);

contracts/test/foundry/FleekERC721/AccessPoints/AccessPointsAutoApprovalOn.t.sol

@@ -171,13 +171,13 @@ contract Test_FleekERC721_AccessPoint is Test_FleekERC721_Base, APConstants {
         CuT.addAccessPoint(tokenId, accessPointName);
 
         vm.startPrank(randomAddress);
-        expectRevertWithTokenRole(tokenId, FleekAccessControl.TokenRoles.Controller);
+        expectRevertWithCollectionRole(FleekAccessControl.CollectionRoles.Verifier);
         CuT.setAccessPointNameVerify(accessPointName, true);
-        expectRevertWithTokenRole(tokenId, FleekAccessControl.TokenRoles.Controller);
+        expectRevertWithCollectionRole(FleekAccessControl.CollectionRoles.Verifier);
         CuT.setAccessPointContentVerify(accessPointName, true);
         vm.stopPrank();
 
-        CuT.grantTokenRole(tokenId, FleekAccessControl.TokenRoles.Controller, randomAddress);
+        CuT.grantCollectionRole(FleekAccessControl.CollectionRoles.Verifier, randomAddress);
 
         vm.startPrank(randomAddress);
         CuT.setAccessPointNameVerify(accessPointName, true);

contracts/test/hardhat/contracts/FleekERC721/access-point/access-points-autoapproval-off.t.ts

@@ -1,5 +1,7 @@
 import { loadFixture } from '@nomicfoundation/hardhat-network-helpers';
+import { SignerWithAddress } from '@nomiclabs/hardhat-ethers/signers';
 import { expect } from 'chai';
+import { ethers } from 'hardhat';
 import { TestConstants, Fixtures, Errors } from '../helpers';
 const { AccessPointStatus } = TestConstants;
 

contracts/test/hardhat/contracts/FleekERC721/collection-roles.t.ts

@@ -175,4 +175,39 @@ describe('FleekERC721.CollectionRoles', () => {
       contract.revokeCollectionRole(CollectionRoles.Owner, owner.address)
     ).to.be.revertedWithCustomError(contract, Errors.MustHaveAtLeastOneOwner);
   });
+
+  it('should not be able to verify access point if not verifier', async () => {
+    const { contract, otherAccount } = fixture;
+
+    await contract.mint(
+      otherAccount.address,
+      TestConstants.MintParams.name,
+      TestConstants.MintParams.description,
+      TestConstants.MintParams.externalUrl,
+      TestConstants.MintParams.ens,
+      TestConstants.MintParams.commitHash,
+      TestConstants.MintParams.gitRepository,
+      TestConstants.MintParams.logo,
+      TestConstants.MintParams.color,
+      TestConstants.MintParams.accessPointAutoApprovalSettings
+    );
+
+    await contract.addAccessPoint(0, 'random.com');
+
+    await expect(
+      contract
+        .connect(otherAccount)
+        .setAccessPointContentVerify('random.com', true)
+    )
+      .to.be.revertedWithCustomError(contract, Errors.MustHaveCollectionRole)
+      .withArgs(CollectionRoles.Verifier);
+
+    await expect(
+      contract
+        .connect(otherAccount)
+        .setAccessPointNameVerify('random.com', true)
+    )
+      .to.be.revertedWithCustomError(contract, Errors.MustHaveCollectionRole)
+      .withArgs(CollectionRoles.Verifier);
+  });
 });

contracts/test/hardhat/contracts/FleekERC721/helpers/constants.ts

@@ -1,6 +1,7 @@
 export const TestConstants = Object.freeze({
   CollectionRoles: {
     Owner: 0,
+    Verifier: 1,
   },
   TokenRoles: {
     Controller: 0,