From 8f793afef44262e1a0f6fe2fbb828b125a0dcbea Mon Sep 17 00:00:00 2001 From: Daniel Lima Date: Tue, 26 Sep 2023 17:48:00 -0300 Subject: [PATCH 1/2] ON-483: Remove 1155 --- contracts/RolesRegistry/RolesRegistry.sol | 15 +++--------- contracts/mocks/MockERC1155.sol | 13 ---------- test/RolesRegistry.spec.ts | 30 ----------------------- 3 files changed, 3 insertions(+), 55 deletions(-) delete mode 100644 contracts/mocks/MockERC1155.sol diff --git a/contracts/RolesRegistry/RolesRegistry.sol b/contracts/RolesRegistry/RolesRegistry.sol index cae6f06..4c16d84 100644 --- a/contracts/RolesRegistry/RolesRegistry.sol +++ b/contracts/RolesRegistry/RolesRegistry.sol @@ -31,7 +31,7 @@ contract RolesRegistry is IERC7432 { address _account ) { require( - _isOwner(_tokenAddress, _tokenId, msg.sender) || + msg.sender == IERC721(_tokenAddress).ownerOf(_tokenId) || _isRoleApproved(_tokenAddress, _tokenId, _account, msg.sender), "RolesRegistry: sender must be token owner or approved" ); @@ -43,7 +43,7 @@ contract RolesRegistry is IERC7432 { uint256 _tokenId, address _account ) { - require(_isOwner(_tokenAddress, _tokenId, _account), "RolesRegistry: account must be token owner"); + require(_account == IERC721(_tokenAddress).ownerOf(_tokenId), "RolesRegistry: account must be token owner"); _; } @@ -118,7 +118,7 @@ contract RolesRegistry is IERC7432 { address _revoker, address _grantee ) external override isTokenOwner(_tokenAddress, _tokenId, _revoker) { - address _caller = _isOwner(_tokenAddress, _tokenId, msg.sender) ? _revoker : _getApprovedCaller(_tokenAddress, _tokenId, _revoker, _grantee); + address _caller = msg.sender == IERC721(_tokenAddress).ownerOf(_tokenId) ? _revoker : _getApprovedCaller(_tokenAddress, _tokenId, _revoker, _grantee); _revokeRole(_role, _tokenAddress, _tokenId, _revoker, _grantee, _caller); } @@ -239,13 +239,4 @@ contract RolesRegistry is IERC7432 { isRoleApprovedForAll(_tokenAddress, _grantor, _operator) || getApprovedRole(_tokenAddress, _tokenId, _grantor, _operator); } - - function _isERC1155(address _tokenAddress) internal view returns (bool) { - return ERC165Checker.supportsInterface(_tokenAddress, type(IERC1155).interfaceId); - } - - function _isOwner(address _tokenAddress, uint256 _tokenId, address _account) internal view returns (bool) { - if (_isERC1155(_tokenAddress)) return IERC1155(_tokenAddress).balanceOf(_account, _tokenId) > 0; - else return _account == IERC721(_tokenAddress).ownerOf(_tokenId); // Assuming that the token implements ERC721 - } } diff --git a/contracts/mocks/MockERC1155.sol b/contracts/mocks/MockERC1155.sol deleted file mode 100644 index 05759cf..0000000 --- a/contracts/mocks/MockERC1155.sol +++ /dev/null @@ -1,13 +0,0 @@ -// SPDX-License-Identifier: CC0-1.0 - -pragma solidity 0.8.9; - -import { ERC1155 } from '@openzeppelin/contracts/token/ERC1155/ERC1155.sol'; - -contract MockERC1155 is ERC1155 { - constructor() ERC1155('https://example.com/{id}') {} - - function mint(address to, uint256 tokenId, uint256 amount, bytes memory data) external { - _mint(to, tokenId, amount, data); - } -} \ No newline at end of file diff --git a/test/RolesRegistry.spec.ts b/test/RolesRegistry.spec.ts index 912298b..1bf0ed0 100644 --- a/test/RolesRegistry.spec.ts +++ b/test/RolesRegistry.spec.ts @@ -14,7 +14,6 @@ const ONE_DAY = 60 * 60 * 24 describe('RolesRegistry', () => { let RolesRegistry: Contract let mockERC721: Contract - let mockERC1155: Contract // eslint-disable-next-line @typescript-eslint/no-unused-vars let deployer: SignerWithAddress @@ -83,12 +82,7 @@ describe('RolesRegistry', () => { mockERC721 = await MockERC721Factory.deploy() await mockERC721.deployed() - const MockERC1155Factory = await ethers.getContractFactory('MockERC1155') - mockERC1155 = await MockERC1155Factory.deploy() - await mockERC1155.deployed() - await mockERC721.mint(grantor.address, tokenId) - await mockERC1155.mint(grantor.address, tokenId, 1, HashZero) }) describe('Main Functions', async () => { @@ -131,30 +125,6 @@ describe('RolesRegistry', () => { data, ) }) - it('should grant role for ERC1155', async () => { - await expect( - RolesRegistry.connect(grantor).grantRole( - PROPERTY_MANAGER, - mockERC1155.address, - tokenId, - userOne.address, - expirationDate, - revocable, - data, - ), - ) - .to.emit(RolesRegistry, 'RoleGranted') - .withArgs( - PROPERTY_MANAGER, - mockERC1155.address, - tokenId, - grantor.address, - userOne.address, - expirationDate, - revocable, - data, - ) - }) it('should NOT grant role if expiration date is in the past', async () => { const blockNumber = await hre.ethers.provider.getBlockNumber() const block = await hre.ethers.provider.getBlock(blockNumber) From d506dfed5ac3136b1b0ce60c68943071848e7673 Mon Sep 17 00:00:00 2001 From: Daniel Lima Date: Tue, 26 Sep 2023 17:48:39 -0300 Subject: [PATCH 2/2] ON-483: Remove unused --- contracts/RolesRegistry/RolesRegistry.sol | 1 - test/RolesRegistry.spec.ts | 13 ------------- 2 files changed, 14 deletions(-) diff --git a/contracts/RolesRegistry/RolesRegistry.sol b/contracts/RolesRegistry/RolesRegistry.sol index 4c16d84..7f9b484 100644 --- a/contracts/RolesRegistry/RolesRegistry.sol +++ b/contracts/RolesRegistry/RolesRegistry.sol @@ -3,7 +3,6 @@ pragma solidity 0.8.9; import { IERC7432 } from "./interfaces/IERC7432.sol"; -import { IERC1155 } from "@openzeppelin/contracts/token/ERC1155/IERC1155.sol"; import { IERC721 } from "@openzeppelin/contracts/token/ERC721/IERC721.sol"; import { ERC165Checker } from "@openzeppelin/contracts/utils/introspection/ERC165Checker.sol"; diff --git a/test/RolesRegistry.spec.ts b/test/RolesRegistry.spec.ts index 1bf0ed0..e8fa59a 100644 --- a/test/RolesRegistry.spec.ts +++ b/test/RolesRegistry.spec.ts @@ -155,19 +155,6 @@ describe('RolesRegistry', () => { ), ).to.be.revertedWith(`RolesRegistry: account must be token owner`) }) - it.skip('should NOT grant role if token is neither ERC721 nor ERC1155', async () => { - await expect( - RolesRegistry.connect(grantor).grantRole( - PROPERTY_MANAGER, - deployer.address, - tokenId, - userOne.address, - expirationDate, - revocable, - HashZero, - ), - ).to.be.revertedWith(`RolesRegistry: token address is not ERC1155 or ERC721`) - }) }) describe('Revoke role', async () => {