From 22198e76e352ec2f30acbac7978456fe2b0f3010 Mon Sep 17 00:00:00 2001 From: Felipe Mendes Date: Mon, 19 Dec 2022 15:45:15 -0300 Subject: [PATCH] refactor: remove extension AccessControl from FleekAccessControl (#28) * refactor: remove extension AccessControl from FleekAccessControl * refactor: add version for roles * test: add collection roles tests * test: add more token role hardhat tests * refactor: remove lib/forge-std * feat: add role grant and revoke events * test: add access control role event emit tests * refactor: remove abstract keyword from FleekAccessControl * Merge conflicts for #28 (#49) * Add msg.sender to the triggered_by field in events * Document methods and make everything camelCase * Make event params all camelCase * fix: forge-std submodule * make vars camelCase, remove baseURI header, remove addTokenController and removeTokenController, update tests Co-authored-by: EmperorOrokuSaki Co-authored-by: Shredder <110225819+EmperorOrokuSaki@users.noreply.github.com> Co-authored-by: zoruka Co-authored-by: Janison Sivarajah Co-authored-by: EmperorOrokuSaki Co-authored-by: Shredder <110225819+EmperorOrokuSaki@users.noreply.github.com> --- contracts/FleekAccessControl.sol | 196 ++++++++++---- contracts/FleekERC721.sol | 66 ++--- test/FleekERC721.ts | 444 ++++++++++++++++++++++++++++++- 3 files changed, 603 insertions(+), 103 deletions(-) diff --git a/contracts/FleekAccessControl.sol b/contracts/FleekAccessControl.sol index 72a5e18..41f9035 100644 --- a/contracts/FleekAccessControl.sol +++ b/contracts/FleekAccessControl.sol @@ -1,45 +1,151 @@ -// SPDX-License-Identifier: MIT - -pragma solidity ^0.8.7; - -import "@openzeppelin/contracts/access/AccessControl.sol"; - -abstract contract FleekAccessControl is AccessControl { - bytes32 public constant COLLECTION_OWNER_ROLE = keccak256("COLLECTION_OWNER_ROLE"); - bytes32 public constant COLLECTION_CONTROLLER_ROLE = keccak256("COLLECTION_CONTROLLER_ROLE"); - - constructor() { - _setRoleAdmin(COLLECTION_OWNER_ROLE, DEFAULT_ADMIN_ROLE); - _grantRole(COLLECTION_OWNER_ROLE, msg.sender); - } - - modifier requireCollectionOwner() { - require(hasRole(COLLECTION_OWNER_ROLE, msg.sender), "FleekAccessControl: must have collection owner role"); - _; - } - - modifier requireCollectionController() { - require( - hasRole(COLLECTION_OWNER_ROLE, msg.sender) || hasRole(COLLECTION_CONTROLLER_ROLE, msg.sender), - "FleekAccessControl: must have collection controller role" - ); - _; - } - - modifier requireTokenController(uint256 tokenId) { - require(hasRole(_tokenRole(tokenId, "CONTROLLER"), msg.sender), "FleekAccessControl: must have token role"); - _; - } - - function isTokenController(uint256 tokenId, address account) public view returns (bool) { - return hasRole(_tokenRole(tokenId, "CONTROLLER"), account); - } - - function _tokenRole(uint256 tokenId, string memory role) internal pure returns (bytes32) { - return keccak256(abi.encodePacked("TOKEN_", role, tokenId)); - } - - function _clearTokenControllers(uint256 tokenId) internal { - // TODO: Remove token controllers from AccessControl - } -} +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.7; + +import "@openzeppelin/contracts/utils/Counters.sol"; + +contract FleekAccessControl { + using Counters for Counters.Counter; + + enum Roles { + Owner, + Controller + } + + event TokenRoleGranted(uint256 indexed tokenId, Roles indexed role, address indexed toAddress, address byAddress); + event TokenRoleRevoked(uint256 indexed tokenId, Roles indexed role, address indexed toAddress, address byAddress); + event CollectionRoleGranted(Roles indexed role, address indexed toAddress, address byAddress); + event CollectionRoleRevoked(Roles indexed role, address indexed toAddress, address byAddress); + + struct Role { + mapping(address => uint256) indexes; + address[] members; + } + + Counters.Counter private _collectionRolesVersion; + // _collectionRoles[version][role] + mapping(uint256 => mapping(Roles => Role)) private _collectionRoles; + + mapping(uint256 => Counters.Counter) private _tokenRolesVersion; + // _tokenRoles[tokenId][version][role] + mapping(uint256 => mapping(uint256 => mapping(Roles => Role))) private _tokenRoles; + + constructor() { + _grantCollectionRole(Roles.Owner, msg.sender); + } + + modifier requireCollectionRole(Roles role) { + require( + hasCollectionRole(role, msg.sender) || hasCollectionRole(Roles.Owner, msg.sender), + "FleekAccessControl: must have collection role" + ); + _; + } + + modifier requireTokenRole(uint256 tokenId, Roles role) { + require( + hasTokenRole(tokenId, role, msg.sender) || hasTokenRole(tokenId, Roles.Owner, msg.sender), + "FleekAccessControl: must have token role" + ); + _; + } + + function grantCollectionRole(Roles role, address account) public requireCollectionRole(Roles.Owner) { + _grantCollectionRole(role, account); + } + + function grantTokenRole( + uint256 tokenId, + Roles role, + address account + ) public requireTokenRole(tokenId, Roles.Owner) { + _grantTokenRole(tokenId, role, account); + } + + function revokeCollectionRole(Roles role, address account) public requireCollectionRole(Roles.Owner) { + _revokeCollectionRole(role, account); + } + + function revokeTokenRole( + uint256 tokenId, + Roles role, + address account + ) public requireTokenRole(tokenId, Roles.Owner) { + _revokeTokenRole(tokenId, role, account); + } + + function hasCollectionRole(Roles role, address account) public view returns (bool) { + uint256 currentVersion = _collectionRolesVersion.current(); + + return _collectionRoles[currentVersion][role].indexes[account] != 0; + } + + function hasTokenRole(uint256 tokenId, Roles role, address account) public view returns (bool) { + uint256 currentVersion = _tokenRolesVersion[tokenId].current(); + return _tokenRoles[tokenId][currentVersion][role].indexes[account] != 0; + } + + function getCollectionRoleMembers(Roles role) public view returns (address[] memory) { + uint256 currentVersion = _collectionRolesVersion.current(); + return _collectionRoles[currentVersion][role].members; + } + + function getTokenRoleMembers(uint256 tokenId, Roles role) public view returns (address[] memory) { + uint256 currentVersion = _tokenRolesVersion[tokenId].current(); + return _tokenRoles[tokenId][currentVersion][role].members; + } + + function _grantCollectionRole(Roles role, address account) internal { + uint256 currentVersion = _collectionRolesVersion.current(); + _grantRole(_collectionRoles[currentVersion][role], account); + emit CollectionRoleGranted(role, account, msg.sender); + } + + function _revokeCollectionRole(Roles role, address account) internal { + uint256 currentVersion = _collectionRolesVersion.current(); + _revokeRole(_collectionRoles[currentVersion][role], account); + emit CollectionRoleRevoked(role, account, msg.sender); + } + + function _grantTokenRole(uint256 tokenId, Roles role, address account) internal { + uint256 currentVersion = _tokenRolesVersion[tokenId].current(); + _grantRole(_tokenRoles[tokenId][currentVersion][role], account); + emit TokenRoleGranted(tokenId, role, account, msg.sender); + } + + function _revokeTokenRole(uint256 tokenId, Roles role, address account) internal { + uint256 currentVersion = _tokenRolesVersion[tokenId].current(); + _revokeRole(_tokenRoles[tokenId][currentVersion][role], account); + emit TokenRoleRevoked(tokenId, role, account, msg.sender); + } + + function _grantRole(Role storage role, address account) internal { + if (role.indexes[account] == 0) { + role.members.push(account); + role.indexes[account] = role.members.length; + } + } + + function _revokeRole(Role storage role, address account) internal { + if (role.indexes[account] != 0) { + uint256 index = role.indexes[account] - 1; + uint256 lastIndex = role.members.length - 1; + address lastAccount = role.members[lastIndex]; + + role.members[index] = lastAccount; + role.indexes[lastAccount] = index + 1; + + role.members.pop(); + delete role.indexes[account]; + } + } + + function _clearAllTokenRoles(uint256 tokenId) internal { + _tokenRolesVersion[tokenId].increment(); + } + + function _clearAllTokenRoles(uint256 tokenId, address newOwner) internal { + _clearAllTokenRoles(tokenId); + _grantTokenRole(tokenId, Roles.Owner, newOwner); + } +} \ No newline at end of file diff --git a/contracts/FleekERC721.sol b/contracts/FleekERC721.sol index 8a48a9f..9f52427 100644 --- a/contracts/FleekERC721.sol +++ b/contracts/FleekERC721.sol @@ -67,7 +67,7 @@ contract FleekERC721 is ERC721, FleekAccessControl { string memory ENS, string memory commitHash, string memory gitRepository - ) public payable requireCollectionOwner returns (uint256) { + ) public payable requireCollectionRole(Roles.Owner) returns (uint256) { uint256 tokenId = _tokenIds.current(); _mint(to, tokenId); _tokenIds.increment(); @@ -121,42 +121,10 @@ contract FleekERC721 is ERC721, FleekAccessControl { return string(abi.encodePacked(_baseURI(), Base64.encode((dataURI)))); } - /** - * @dev Adds a new address with the`tokenController` privileges to a previously minted `tokenId`. - * - * May emit a {RoleGranted} event. - * - * Requirements: - * - * - the tokenId must be minted and valid. - * - the sender must have the `tokenOwner` role. - * - */ - function addTokenController(uint256 tokenId, address controller) public requireTokenOwner(tokenId) { - _requireMinted(tokenId); - _grantRole(_tokenRole(tokenId, "CONTROLLER"), controller); - } - - /** - * @dev Strips an address from their `tokenController` privileges on a previously minted `tokenId`. - * - * May emit a {RoleRevoked} event. - * - * Requirements: - * - * - the tokenId must be minted and valid. - * - the sender must have the `tokenOwner` role. - * - */ - function removeTokenController(uint256 tokenId, address controller) public requireTokenOwner(tokenId) { - _requireMinted(tokenId); - _revokeRole(_tokenRole(tokenId, "CONTROLLER"), controller); - } - /** * @dev See {IERC165-supportsInterface}. */ - function supportsInterface(bytes4 interfaceId) public view virtual override(ERC721, AccessControl) returns (bool) { + function supportsInterface(bytes4 interfaceId) public view virtual override(ERC721) returns (bool) { return super.supportsInterface(interfaceId); } @@ -173,14 +141,13 @@ contract FleekERC721 is ERC721, FleekAccessControl { ) internal virtual override { if (from != address(0) && to != address(0)) { // Transfer - _clearTokenControllers(tokenId); - _grantRole(_tokenRole(tokenId, "CONTROLLER"), to); + _clearAllTokenRoles(tokenId, to); } else if (from == address(0)) { // Mint - _grantRole(_tokenRole(tokenId, "CONTROLLER"), to); + _grantTokenRole(tokenId, Roles.Owner, to); } else if (to == address(0)) { // Burn - _clearTokenControllers(tokenId); + _clearAllTokenRoles(tokenId); } super._beforeTokenTransfer(from, to, tokenId, batchSize); } @@ -206,7 +173,7 @@ contract FleekERC721 is ERC721, FleekAccessControl { function setTokenExternalURL( uint256 tokenId, string memory _tokenExternalURL - ) public virtual requireTokenController(tokenId) { + ) public virtual requireTokenRole(tokenId, Roles.Controller) { _requireMinted(tokenId); _apps[tokenId].externalURL = _tokenExternalURL; emit NewTokenExternalURL(tokenId, _tokenExternalURL, msg.sender); @@ -223,7 +190,10 @@ contract FleekERC721 is ERC721, FleekAccessControl { * - the sender must have the `tokenController` role. * */ - function setTokenENS(uint256 tokenId, string memory _tokenENS) public virtual requireTokenController(tokenId) { + function setTokenENS( + uint256 tokenId, + string memory _tokenENS + ) public virtual requireTokenRole(tokenId, Roles.Controller) { _requireMinted(tokenId); _apps[tokenId].ENS = _tokenENS; emit NewTokenENS(tokenId, _tokenENS, msg.sender); @@ -240,7 +210,10 @@ contract FleekERC721 is ERC721, FleekAccessControl { * - the sender must have the `tokenController` role. * */ - function setTokenName(uint256 tokenId, string memory _tokenName) public virtual requireTokenController(tokenId) { + function setTokenName( + uint256 tokenId, + string memory _tokenName + ) public virtual requireTokenRole(tokenId, Roles.Controller) { _requireMinted(tokenId); _apps[tokenId].name = _tokenName; emit NewTokenName(tokenId, _tokenName, msg.sender); @@ -260,7 +233,7 @@ contract FleekERC721 is ERC721, FleekAccessControl { function setTokenDescription( uint256 tokenId, string memory _tokenDescription - ) public virtual requireTokenController(tokenId) { + ) public virtual requireTokenRole(tokenId, Roles.Controller) { _requireMinted(tokenId); _apps[tokenId].description = _tokenDescription; emit NewTokenDescription(tokenId, _tokenDescription, msg.sender); @@ -277,7 +250,10 @@ contract FleekERC721 is ERC721, FleekAccessControl { * - the sender must have the `tokenController` role. * */ - function setTokenImage(uint256 tokenId, string memory _tokenImage) public virtual requireTokenController(tokenId) { + function setTokenImage( + uint256 tokenId, + string memory _tokenImage + ) public virtual requireTokenRole(tokenId, Roles.Controller) { _requireMinted(tokenId); _apps[tokenId].image = _tokenImage; emit NewTokenImage(tokenId, _tokenImage, msg.sender); @@ -298,7 +274,7 @@ contract FleekERC721 is ERC721, FleekAccessControl { uint256 tokenId, string memory _commitHash, string memory _gitRepository - ) public virtual requireTokenController(tokenId) { + ) public virtual requireTokenRole(tokenId, Roles.Controller) { _requireMinted(tokenId); _apps[tokenId].builds[++_apps[tokenId].currentBuild] = Build(_commitHash, _gitRepository); emit NewBuild(tokenId, _commitHash, msg.sender); @@ -315,7 +291,7 @@ contract FleekERC721 is ERC721, FleekAccessControl { * - the sender must have the `tokenOwner` role. * */ - function burn(uint256 tokenId) public virtual requireTokenOwner(tokenId) { + function burn(uint256 tokenId) public virtual requireTokenRole(tokenId, Roles.Owner) { super._burn(tokenId); if (bytes(_apps[tokenId].externalURL).length != 0) { diff --git a/test/FleekERC721.ts b/test/FleekERC721.ts index 50d1e4b..1934b97 100644 --- a/test/FleekERC721.ts +++ b/test/FleekERC721.ts @@ -1,10 +1,12 @@ import { loadFixture } from '@nomicfoundation/hardhat-network-helpers'; import { expect } from 'chai'; import { ethers } from 'hardhat'; -import web3 from 'web3'; describe('FleekERC721', () => { - const COLLECTION_OWNER_ROLE = web3.utils.keccak256('COLLECTION_OWNER_ROLE'); + const ROLES = Object.freeze({ + OWNER: 0, + CONTROLLER: 1, + }); const MINT_PARAMS = Object.freeze({ name: 'Fleek Test App', @@ -42,14 +44,6 @@ describe('FleekERC721', () => { expect(await contract.symbol()).to.equal(COLLECTION_PARAMS.symbol); }); - it('should assign the owner of the contract', async () => { - const { owner, contract } = await loadFixture(defaultFixture); - - expect( - await contract.hasRole(COLLECTION_OWNER_ROLE, owner.address) - ).to.equal(true); - }); - it('should support ERC721 interface', async () => { const { contract } = await loadFixture(defaultFixture); @@ -92,13 +86,42 @@ describe('FleekERC721', () => { MINT_PARAMS.commitHash, MINT_PARAMS.gitRepository ) - ).to.be.revertedWith( - 'FleekAccessControl: must have collection owner role' + ).to.be.revertedWith('FleekAccessControl: must have collection role'); + }); + + it('should have address to as owner', async () => { + const { owner, otherAccount, contract } = await loadFixture( + defaultFixture ); + + const response = await contract.mint( + owner.address, + MINT_PARAMS.name, + MINT_PARAMS.description, + MINT_PARAMS.image, + MINT_PARAMS.externalUrl, + MINT_PARAMS.ens, + MINT_PARAMS.commitHash, + MINT_PARAMS.gitRepository + ); + + const tokenId = response.value.toNumber(); + + expect(await contract.ownerOf(tokenId)).to.equal(owner.address); + expect(await contract.hasTokenRole(tokenId, ROLES.OWNER, owner.address)) + .to.be.true; + + expect(await contract.ownerOf(tokenId)).not.to.equal( + otherAccount.address + ); + expect( + await contract.hasTokenRole(tokenId, ROLES.OWNER, otherAccount.address) + ).to.be.false; }); }); - describe('Token', () => { + describe('Token URI', () => { + let tokenId: number; let fixture: Awaited>; @@ -157,11 +180,406 @@ describe('FleekERC721', () => { ], }); }); + }); + + describe('Token Roles', () => { + let tokenId: number; + let fixture: Awaited>; + + beforeEach(async () => { + fixture = await loadFixture(defaultFixture); + const { contract } = fixture; + + const response = await contract.mint( + fixture.owner.address, + MINT_PARAMS.name, + MINT_PARAMS.description, + MINT_PARAMS.image, + MINT_PARAMS.externalUrl, + MINT_PARAMS.ens, + MINT_PARAMS.commitHash, + MINT_PARAMS.gitRepository + ); + + tokenId = response.value.toNumber(); + }); it('should match the token owner', async () => { const { contract, owner } = fixture; const tokenOwner = await contract.ownerOf(tokenId); expect(tokenOwner).to.equal(owner.address); }); + + it('should match the owner role for minter', async () => { + const { contract, owner } = fixture; + const hasRole = await contract.hasTokenRole( + tokenId, + ROLES.OWNER, + owner.address + ); + + expect(hasRole).to.be.true; + }); + + it('should add a new controller', async () => { + const { contract, owner, otherAccount } = fixture; + await contract.grantTokenRole( + tokenId, + ROLES.CONTROLLER, + otherAccount.address + ); + + expect( + await contract.hasTokenRole( + tokenId, + ROLES.CONTROLLER, + otherAccount.address + ) + ).to.be.true; + }); + + it('should add a list of controllers', async () => { + const { contract } = fixture; + await contract.grantTokenRole( + tokenId, + ROLES.CONTROLLER, + '0x7ED735b7095C05d78dF169F991f2b7f1A1F1A049' + ); + await contract.grantTokenRole( + tokenId, + ROLES.CONTROLLER, + '0x2FEd6Ef3c495922263B403319FA6DDB323DD49E3' + ); + + expect( + await contract.getTokenRoleMembers(tokenId, ROLES.CONTROLLER) + ).to.eql([ + '0x7ED735b7095C05d78dF169F991f2b7f1A1F1A049', + '0x2FEd6Ef3c495922263B403319FA6DDB323DD49E3', + ]); + }); + + it('should add a list of owners', async () => { + const { contract, owner } = fixture; + await contract.grantTokenRole( + tokenId, + ROLES.OWNER, + '0x7ED735b7095C05d78dF169F991f2b7f1A1F1A049' + ); + await contract.grantTokenRole( + tokenId, + ROLES.OWNER, + '0x2FEd6Ef3c495922263B403319FA6DDB323DD49E3' + ); + + expect(await contract.getTokenRoleMembers(tokenId, ROLES.OWNER)).to.eql([ + owner.address, + '0x7ED735b7095C05d78dF169F991f2b7f1A1F1A049', + '0x2FEd6Ef3c495922263B403319FA6DDB323DD49E3', + ]); + }); + + it('should not match the owner role for other account', async () => { + const { contract, otherAccount } = fixture; + const hasRole = await contract.hasTokenRole( + tokenId, + ROLES.OWNER, + otherAccount.address + ); + + expect(hasRole).to.be.false; + }); + + it('should remove an added controller', async () => { + const { contract, owner, otherAccount } = fixture; + await contract.grantTokenRole( + tokenId, + ROLES.CONTROLLER, + otherAccount.address + ); + await contract.revokeTokenRole( + tokenId, + ROLES.CONTROLLER, + otherAccount.address + ); + + expect( + await contract.hasTokenRole( + tokenId, + ROLES.CONTROLLER, + otherAccount.address + ) + ).to.be.false; + }); + + it('should transfer the token owner role', async () => { + const { contract, owner, otherAccount } = fixture; + await contract.transferFrom(owner.address, otherAccount.address, tokenId); + + expect(await contract.ownerOf(tokenId)).to.equal(otherAccount.address); + expect( + await contract.hasTokenRole(tokenId, ROLES.OWNER, otherAccount.address) + ).to.be.true; + expect(await contract.hasTokenRole(tokenId, ROLES.OWNER, owner.address)) + .to.be.false; + }); + + it('should clean the token controller list after transfer', async () => { + const { contract, owner, otherAccount } = fixture; + await contract.grantTokenRole( + tokenId, + ROLES.CONTROLLER, + otherAccount.address + ); + await contract.transferFrom(owner.address, otherAccount.address, tokenId); + + expect(await contract.getTokenRoleMembers(tokenId, 1)).to.eql([]); + }); + + it('should not be able to add address role', async () => { + const { contract, owner, otherAccount } = fixture; + await expect( + contract + .connect(otherAccount) + .grantTokenRole(tokenId, ROLES.OWNER, otherAccount.address) + ).to.be.revertedWith('FleekAccessControl: must have token role'); + + await expect( + contract + .connect(otherAccount) + .grantTokenRole(tokenId, ROLES.CONTROLLER, otherAccount.address) + ).to.be.revertedWith('FleekAccessControl: must have token role'); + }); + + it('should not be able to remove address role', async () => { + const { contract, owner, otherAccount } = fixture; + await expect( + contract + .connect(otherAccount) + .revokeTokenRole(tokenId, ROLES.OWNER, otherAccount.address) + ).to.be.revertedWith('FleekAccessControl: must have token role'); + + await expect( + contract + .connect(otherAccount) + .revokeTokenRole(tokenId, ROLES.CONTROLLER, otherAccount.address) + ).to.be.revertedWith('FleekAccessControl: must have token role'); + }); + + it('should be able to add token role after owner role granted', async () => { + const { contract, owner, otherAccount } = fixture; + await contract.grantTokenRole(tokenId, ROLES.OWNER, otherAccount.address); + + expect( + await contract + .connect(otherAccount) + .grantTokenRole(tokenId, ROLES.CONTROLLER, otherAccount.address) + ).to.not.be.reverted; + }); + + it('should emit event when token role is granted', async () => { + const { contract, owner, otherAccount } = fixture; + await expect( + contract.grantTokenRole(tokenId, ROLES.CONTROLLER, otherAccount.address) + ) + .to.emit(contract, 'TokenRoleGranted') + .withArgs( + tokenId, + ROLES.CONTROLLER, + otherAccount.address, + owner.address + ); + }); + + it('should emit event when token role is revoked', async () => { + const { contract, owner, otherAccount } = fixture; + await contract.grantTokenRole( + tokenId, + ROLES.CONTROLLER, + otherAccount.address + ); + await expect( + contract.revokeTokenRole( + tokenId, + ROLES.CONTROLLER, + otherAccount.address + ) + ) + .to.emit(contract, 'TokenRoleRevoked') + .withArgs( + tokenId, + ROLES.CONTROLLER, + otherAccount.address, + owner.address + ); + }); + }); + + describe('Collection Roles', () => { + let fixture: Awaited>; + + beforeEach(async () => { + fixture = await loadFixture(defaultFixture); + }); + + it('should assign the owner of the contract on contract creation', async () => { + const { owner, contract } = fixture; + + expect(await contract.hasCollectionRole(ROLES.OWNER, owner.address)).to.be + .true; + }); + + it('should assign owner role to address', async () => { + const { otherAccount, contract } = fixture; + + await contract.grantCollectionRole(ROLES.OWNER, otherAccount.address); + + expect( + await contract.hasCollectionRole(ROLES.OWNER, otherAccount.address) + ).to.be.true; + }); + + it('should assign controller role to address', async () => { + const { owner, contract } = fixture; + + await contract.grantCollectionRole(ROLES.CONTROLLER, owner.address); + + expect(await contract.hasCollectionRole(ROLES.CONTROLLER, owner.address)) + .to.be.true; + }); + + it('should remove an assigned controller', async () => { + const { otherAccount, contract } = fixture; + + await contract.grantCollectionRole(ROLES.OWNER, otherAccount.address); + await contract.revokeCollectionRole(ROLES.OWNER, otherAccount.address); + + expect( + await contract.hasCollectionRole(ROLES.OWNER, otherAccount.address) + ).to.be.false; + }); + + it('should remove an assigned controller', async () => { + const { owner, contract } = fixture; + + await contract.grantCollectionRole(ROLES.CONTROLLER, owner.address); + await contract.revokeCollectionRole(ROLES.CONTROLLER, owner.address); + + expect(await contract.hasCollectionRole(ROLES.CONTROLLER, owner.address)) + .to.be.false; + }); + + it('should fetch the list of controllers', async () => { + const { owner, contract } = fixture; + + await contract.grantCollectionRole(ROLES.CONTROLLER, owner.address); + await contract.grantCollectionRole( + ROLES.CONTROLLER, + '0x7ED735b7095C05d78dF169F991f2b7f1A1F1A049' + ); + + expect(await contract.getCollectionRoleMembers(ROLES.CONTROLLER)).to.eql([ + owner.address, + '0x7ED735b7095C05d78dF169F991f2b7f1A1F1A049', + ]); + }); + + it('should fetch the list of owners', async () => { + const { owner, contract, otherAccount } = fixture; + + await contract.grantCollectionRole(ROLES.OWNER, otherAccount.address); + await contract.grantCollectionRole( + ROLES.OWNER, + '0x7ED735b7095C05d78dF169F991f2b7f1A1F1A049' + ); + + expect(await contract.getCollectionRoleMembers(ROLES.OWNER)).to.eql([ + owner.address, + otherAccount.address, + '0x7ED735b7095C05d78dF169F991f2b7f1A1F1A049', + ]); + }); + + it('should not be able to add new owner', async () => { + const { otherAccount, contract } = fixture; + + await expect( + contract + .connect(otherAccount) + .grantCollectionRole(ROLES.OWNER, otherAccount.address) + ).to.be.revertedWith('FleekAccessControl: must have collection role'); + }); + + it('should not be able to add new controller', async () => { + const { otherAccount, contract } = fixture; + + await expect( + contract + .connect(otherAccount) + .grantCollectionRole(ROLES.CONTROLLER, otherAccount.address) + ).to.be.revertedWith('FleekAccessControl: must have collection role'); + }); + + it('should be able to add roles after owner being granted', async () => { + const { otherAccount, contract } = fixture; + + await contract.grantCollectionRole(ROLES.OWNER, otherAccount.address); + + await expect( + contract + .connect(otherAccount) + .grantCollectionRole(ROLES.CONTROLLER, otherAccount.address) + ).to.not.be.reverted; + await expect( + contract + .connect(otherAccount) + .revokeCollectionRole(ROLES.CONTROLLER, otherAccount.address) + ).to.not.be.reverted; + }); + + it('should not be able to change roles for controllers', async () => { + const { owner, otherAccount, contract } = fixture; + + await contract.grantCollectionRole( + ROLES.CONTROLLER, + otherAccount.address + ); + + await expect( + contract + .connect(otherAccount) + .grantCollectionRole(ROLES.OWNER, owner.address) + ).to.be.revertedWith('FleekAccessControl: must have collection role'); + await expect( + contract + .connect(otherAccount) + .revokeCollectionRole(ROLES.OWNER, owner.address) + ).to.be.revertedWith('FleekAccessControl: must have collection role'); + }); + + it('should emit event when role is granted', async () => { + const { owner, contract, otherAccount } = fixture; + + await expect( + contract.grantCollectionRole(ROLES.CONTROLLER, otherAccount.address) + ) + .to.emit(contract, 'CollectionRoleGranted') + .withArgs(ROLES.CONTROLLER, otherAccount.address, owner.address); + }); + + it('should emit event when role is revoked', async () => { + const { owner, contract, otherAccount } = fixture; + + await contract.grantCollectionRole( + ROLES.CONTROLLER, + otherAccount.address + ); + + await expect( + contract.revokeCollectionRole(ROLES.CONTROLLER, otherAccount.address) + ) + .to.emit(contract, 'CollectionRoleRevoked') + .withArgs(ROLES.CONTROLLER, otherAccount.address, owner.address); + }); }); });