diff --git a/contracts/RolesRegistry/SftRolesRegistry.sol b/contracts/RolesRegistry/SftRolesRegistry.sol index 0104a2f..ab7186c 100644 --- a/contracts/RolesRegistry/SftRolesRegistry.sol +++ b/contracts/RolesRegistry/SftRolesRegistry.sol @@ -10,7 +10,7 @@ import { ERC1155Holder, ERC1155Receiver } from '@openzeppelin/contracts/token/ER import { ERC165Checker } from '@openzeppelin/contracts/utils/introspection/ERC165Checker.sol'; // Semi-fungible token (SFT) registry with only one role (UNIQUE_ROLE) -contract SftRolesRegistry is IERCXXXX, ERC1155Holder { +contract SftRolesRegistrySingleRole is IERCXXXX, ERC1155Holder { bytes32 public constant UNIQUE_ROLE = keccak256('UNIQUE_ROLE'); // grantor => tokenAddress => operator => isApproved @@ -19,8 +19,8 @@ contract SftRolesRegistry is IERCXXXX, ERC1155Holder { // nonce => DepositInfo mapping(uint256 => DepositInfo) public deposits; - // nonce => role => RoleAssignment - mapping(uint256 => mapping(bytes32 => RoleData)) internal roleAssignments; + // nonce => RoleAssignment + mapping(uint256 => RoleData) internal roleAssignments; modifier validExpirationDate(uint64 _expirationDate) { require(_expirationDate > block.timestamp, 'SftRolesRegistry: expiration date must be in the future'); @@ -54,7 +54,6 @@ contract SftRolesRegistry is IERCXXXX, ERC1155Holder { _grantRoleData.tokenAmount ) { - require(_grantRoleData.role == UNIQUE_ROLE, 'SftRolesRegistry: role not supported'); require(_grantRoleData.nonce > 0, 'SftRolesRegistry: nonce must be greater than zero'); if (deposits[_grantRoleData.nonce].grantor == address(0)) { @@ -71,18 +70,19 @@ contract SftRolesRegistry is IERCXXXX, ERC1155Holder { } else { // nonce exists + require(deposits[_grantRoleData.nonce].grantor == _grantRoleData.grantor, "SftRolesRegistry: grantor mismatch"); require(deposits[_grantRoleData.nonce].tokenAddress == _grantRoleData.tokenAddress, "SftRolesRegistry: tokenAddress mismatch"); require(deposits[_grantRoleData.nonce].tokenId == _grantRoleData.tokenId, "SftRolesRegistry: tokenId mismatch"); require(deposits[_grantRoleData.nonce].tokenAmount == _grantRoleData.tokenAmount, "SftRolesRegistry: tokenAmount mismatch"); - RoleData storage _roleData = roleAssignments[_grantRoleData.nonce][_grantRoleData.role]; + RoleData storage _roleData = roleAssignments[_grantRoleData.nonce]; require(_roleData.expirationDate < block.timestamp || _roleData.revocable, "SftRolesRegistry: nonce is not expired or is not revocable"); } _grantOrUpdateRole(_grantRoleData); } function _grantOrUpdateRole(RoleAssignment calldata _grantRoleData) internal { - roleAssignments[_grantRoleData.nonce][_grantRoleData.role] = RoleData( + roleAssignments[_grantRoleData.nonce] = RoleData( _grantRoleData.grantee, _grantRoleData.expirationDate, _grantRoleData.revocable, @@ -91,7 +91,7 @@ contract SftRolesRegistry is IERCXXXX, ERC1155Holder { emit RoleGranted( _grantRoleData.nonce, - _grantRoleData.role, + UNIQUE_ROLE, _grantRoleData.tokenAddress, _grantRoleData.tokenId, _grantRoleData.tokenAmount, @@ -106,14 +106,6 @@ contract SftRolesRegistry is IERCXXXX, ERC1155Holder { function _deposit(uint256 _nonce, DepositInfo memory _depositInfo) internal { deposits[_nonce] = _depositInfo; - emit Deposited( - _nonce, - _depositInfo.tokenAddress, - _depositInfo.tokenId, - _depositInfo.tokenAmount, - _depositInfo.grantor - ); - _transferFrom( _depositInfo.grantor, address(this), @@ -124,7 +116,7 @@ contract SftRolesRegistry is IERCXXXX, ERC1155Holder { } function revokeRoleFrom(uint256 _nonce, bytes32 _role, address _grantee) external override { - RoleData memory _roleData = roleAssignments[_nonce][_role]; + RoleData memory _roleData = roleAssignments[_nonce]; require(_roleData.grantee != address(0), 'SftRolesRegistry: invalid grantee'); DepositInfo memory _depositInfo = deposits[_nonce]; @@ -134,7 +126,7 @@ contract SftRolesRegistry is IERCXXXX, ERC1155Holder { require(caller == _roleData.grantee, 'SftRolesRegistry: nonce is not expired or is not revocable'); } - delete roleAssignments[_nonce][_role]; + delete roleAssignments[_nonce]; emit RoleRevoked( _nonce, @@ -158,14 +150,14 @@ contract SftRolesRegistry is IERCXXXX, ERC1155Holder { { DepositInfo memory _depositInfo = deposits[_nonce]; require( - roleAssignments[_nonce][UNIQUE_ROLE].grantee == address(0) || - roleAssignments[_nonce][UNIQUE_ROLE].expirationDate < block.timestamp || - roleAssignments[_nonce][UNIQUE_ROLE].revocable, + roleAssignments[_nonce].grantee == address(0) || + roleAssignments[_nonce].expirationDate < block.timestamp || + roleAssignments[_nonce].revocable, 'SftRolesRegistry: token has an active role' ); delete deposits[_nonce]; - delete roleAssignments[_nonce][UNIQUE_ROLE]; + delete roleAssignments[_nonce]; _transferFrom( address(this), @@ -192,11 +184,11 @@ contract SftRolesRegistry is IERCXXXX, ERC1155Holder { /** View Functions **/ function roleData(uint256 _nonce, bytes32 _role, address _grantee) external view returns (RoleData memory) { - return roleAssignments[_nonce][_role]; + return roleAssignments[_nonce]; } function roleExpirationDate(uint256 _nonce, bytes32 _role, address _grantee) external view returns (uint64 expirationDate_) { - return roleAssignments[_nonce][_role].expirationDate; + return roleAssignments[_nonce].expirationDate; } function isRoleApprovedForAll( diff --git a/contracts/RolesRegistry/interfaces/IERCXXXX.sol b/contracts/RolesRegistry/interfaces/IERCXXXX.sol index 8403f08..bbe183a 100644 --- a/contracts/RolesRegistry/interfaces/IERCXXXX.sol +++ b/contracts/RolesRegistry/interfaces/IERCXXXX.sol @@ -85,14 +85,6 @@ interface IERCXXXX is IERC165 { /// @param _isApproved The approval status. event RoleApprovalForAll(address indexed _tokenAddress, address indexed _operator, bool _isApproved); - /// @notice Emitted when a user deposits tokens to grant roles. - /// @param _nonce The identifier of the role assignment. - /// @param _tokenAddress The token address. - /// @param _tokenId The token identifier. - /// @param _tokenAmount The token amount deposited. - /// @param _grantor The user depositing the tokens. - event Deposited(uint256 indexed _nonce, address indexed _tokenAddress, uint256 indexed _tokenId, uint256 _tokenAmount, address _grantor); - /// @notice Emitted when a user withdraws tokens from a role assignment. /// @param _nonce The identifier of the role assignment. /// @param _grantor The user withdrawing the tokens. diff --git a/test/SftRegistry/SftRolesRegistry.spec.ts b/test/SftRegistry/SftRolesRegistrySingleRole.spec.ts similarity index 76% rename from test/SftRegistry/SftRolesRegistry.spec.ts rename to test/SftRegistry/SftRolesRegistrySingleRole.spec.ts index 8ee1cec..78fe6b6 100644 --- a/test/SftRegistry/SftRolesRegistry.spec.ts +++ b/test/SftRegistry/SftRolesRegistrySingleRole.spec.ts @@ -2,16 +2,15 @@ import { ethers } from 'hardhat' import { Contract } from 'ethers' import { beforeEach } from 'mocha' import { expect } from 'chai' -import { solidityKeccak256 } from 'ethers/lib/utils' import { loadFixture, time } from '@nomicfoundation/hardhat-network-helpers' import { SignerWithAddress } from '@nomiclabs/hardhat-ethers/signers' -import { buildRoleAssignment, ONE_DAY, generateRoleId, buildRevokeRoleData } from './helpers' +import { buildRoleAssignment, ONE_DAY, buildRevokeRoleData } from './helpers' import { RoleAssignment, RevokeRoleData } from './types' import { generateRandomInt } from '../helpers' const { AddressZero } = ethers.constants -describe('SftRolesRegistry', async () => { +describe('SftRolesRegistrySingleRole', async () => { let SftRolesRegistry: Contract let MockToken: Contract let grantor: SignerWithAddress @@ -19,7 +18,7 @@ describe('SftRolesRegistry', async () => { let anotherUser: SignerWithAddress async function deployContracts() { - const SftRegistryFactory = await ethers.getContractFactory('SftRolesRegistry') + const SftRegistryFactory = await ethers.getContractFactory('SftRolesRegistrySingleRole') SftRolesRegistry = await SftRegistryFactory.deploy() const MockTokenFactory = await ethers.getContractFactory('MockERC1155') MockToken = await MockTokenFactory.deploy() @@ -156,6 +155,38 @@ describe('SftRolesRegistry', async () => { roleAssignment.data, ) }) + it('should revert if grantor tries to update a grant with a nonce that its not theirs', async function () { + const roleAssignment = await buildRoleAssignment({ + tokenAddress: MockToken.address, + grantor: grantor.address, + }) + await MockToken.mint(grantor.address, roleAssignment.tokenId, roleAssignment.tokenAmount) + await MockToken.connect(grantor).setApprovalForAll(SftRolesRegistry.address, true) + await SftRolesRegistry.connect(grantor).setRoleApprovalForAll( + roleAssignment.tokenAddress, + anotherUser.address, + true, + ) + await expect(SftRolesRegistry.connect(anotherUser).grantRoleFrom(roleAssignment)) + .to.emit(SftRolesRegistry, 'RoleGranted') + .withArgs( + roleAssignment.nonce, + roleAssignment.role, + roleAssignment.tokenAddress, + roleAssignment.tokenId, + roleAssignment.tokenAmount, + roleAssignment.grantor, + roleAssignment.grantee, + roleAssignment.expirationDate, + roleAssignment.revocable, + roleAssignment.data, + ) + + roleAssignment.grantor = anotherUser.address + await expect(SftRolesRegistry.connect(anotherUser).grantRoleFrom(roleAssignment)).to.be.revertedWith( + 'SftRolesRegistry: grantor mismatch', + ) + }) }) }) @@ -185,47 +216,6 @@ describe('SftRolesRegistry', async () => { ) }) - it.skip('should revert if hash is different', async () => { - // hash validates role, tokenAddress, tokenId, grantor - - // different role - await expect( - SftRolesRegistry.connect(grantor).grantRoleFrom({ - ...RoleAssignment, - role: generateRoleId('Role(uint256 newArg)'), - }), - ).to.be.revertedWith('SftRolesRegistry: nonce exist, but data mismatch') - - // tokenAddress - await expect( - SftRolesRegistry.connect(grantor).grantRoleFrom({ - ...RoleAssignment, - tokenAddress: AddressZero, - }), - ).to.be.revertedWith('SftRolesRegistry: nonce exist, but data mismatch') - - // tokenId - await expect( - SftRolesRegistry.connect(grantor).grantRoleFrom({ - ...RoleAssignment, - tokenId: generateRandomInt(), - }), - ).to.be.revertedWith('SftRolesRegistry: nonce exist, but data mismatch') - - // grantor - await SftRolesRegistry.connect(anotherUser).setRoleApprovalForAll( - RoleAssignment.tokenAddress, - grantor.address, - true, - ) - await expect( - SftRolesRegistry.connect(grantor).grantRoleFrom({ - ...RoleAssignment, - grantor: anotherUser.address, - }), - ).to.be.revertedWith('SftRolesRegistry: nonce exist, but data mismatch') - }) - it('should revert if nonce is not expired', async () => { const revocableRoleAssignment = await buildRoleAssignment({ tokenAddress: MockToken.address, @@ -250,71 +240,29 @@ describe('SftRolesRegistry', async () => { ).to.be.revertedWith('ERC1155: insufficient balance for transfer') }) - it.skip('should grant role if tokens deposited are greater than requested', async () => { - const newTokenAmount = RoleAssignment.tokenAmount - 1 + it('should revert if tokenAddress mismatch', async () => { await expect( SftRolesRegistry.connect(grantor).grantRoleFrom({ ...RoleAssignment, - tokenAmount: newTokenAmount, + tokenAddress: anotherUser.address, }), - ) - .to.emit(SftRolesRegistry, 'RoleGranted') - .withArgs( - RoleAssignment.nonce, - RoleAssignment.role, - RoleAssignment.tokenAddress, - RoleAssignment.tokenId, - newTokenAmount, - RoleAssignment.grantor, - RoleAssignment.grantee, - RoleAssignment.expirationDate, - RoleAssignment.revocable, - RoleAssignment.data, - ) - // transfer leftover tokens to grantor - .to.emit(MockToken, 'TransferSingle') - .withArgs( - SftRolesRegistry.address, - SftRolesRegistry.address, - RoleAssignment.grantor, - RoleAssignment.tokenId, - RoleAssignment.tokenAmount - newTokenAmount, - ) + ).to.be.revertedWith('SftRolesRegistry: tokenAddress mismatch') }) - - it.skip('should grant role if tokens deposited are lower than deposited (but grantor deposits more)', async () => { - const additionalAmount = 1 - const newTokenAmount = RoleAssignment.tokenAmount + additionalAmount - await MockToken.mint(grantor.address, RoleAssignment.tokenId, additionalAmount) - + it('should revert if tokenId mismatch', async () => { await expect( SftRolesRegistry.connect(grantor).grantRoleFrom({ ...RoleAssignment, - tokenAmount: newTokenAmount, + tokenId: generateRandomInt(), }), - ) - .to.emit(SftRolesRegistry, 'RoleGranted') - .withArgs( - RoleAssignment.nonce, - RoleAssignment.role, - RoleAssignment.tokenAddress, - RoleAssignment.tokenId, - newTokenAmount, - RoleAssignment.grantor, - RoleAssignment.grantee, - RoleAssignment.expirationDate, - RoleAssignment.revocable, - RoleAssignment.data, - ) - // transfer additional tokens to contract - .to.emit(MockToken, 'TransferSingle') - .withArgs( - SftRolesRegistry.address, - RoleAssignment.grantor, - SftRolesRegistry.address, - RoleAssignment.tokenId, - additionalAmount, - ) + ).to.be.revertedWith('SftRolesRegistry: tokenId mismatch') + }) + it('should revert if tokenAmount mismatch', async () => { + await expect( + SftRolesRegistry.connect(grantor).grantRoleFrom({ + ...RoleAssignment, + tokenAmount: generateRandomInt(), + }), + ).to.be.revertedWith('SftRolesRegistry: tokenAmount mismatch') }) it('should grant role if tokens deposited are equal to tokens requested', async () => { @@ -353,51 +301,6 @@ describe('SftRolesRegistry', async () => { await expect(SftRolesRegistry.connect(grantor).grantRoleFrom(RoleAssignment)).to.not.be.reverted }) - it.skip('should revert when hash is invalid', async () => { - // hash validates nonce, role, tokenAddress, tokenId and revoker - - // nonce - await expect( - SftRolesRegistry.connect(grantor).revokeRoleFrom( - generateRandomInt(), - RevokeRoleData.role, - RevokeRoleData.grantee, - ), - ).to.be.revertedWith('SftRolesRegistry: could not find role assignment') - - // role - await expect( - SftRolesRegistry.connect(grantor).revokeRoleFrom( - RevokeRoleData.nonce, - solidityKeccak256(['string'], ['Role(uint256 newArg)']), - RevokeRoleData.grantee, - ), - ).to.be.revertedWith('SftRolesRegistry: could not find role assignment') - - // tokenAddress - await expect( - SftRolesRegistry.connect(grantor).revokeRoleFrom({ - ...RevokeRoleData, - }), - ).to.be.revertedWith('SftRolesRegistry: could not find role assignment') - - /* // tokenId - await expect( - SftRolesRegistry.sol.connect(grantor).revokeRoleFrom({ - ...RevokeRoleData, - tokenId: generateRandomInt(), - }), - ).to.be.revertedWith('SftRolesRegistry.sol: could not find role assignment') - */ - // revoker - /* await expect( - SftRolesRegistry.sol.connect(grantor).revokeRoleFrom({ - ...RevokeRoleData, - revoker: anotherUser.address, - }), - ).to.be.revertedWith('SftRolesRegistry.sol: could not find role assignment') */ - }) - it('should revert if grantee is invalid', async () => { const newRoleAssignment = await buildRoleAssignment({ tokenAddress: MockToken.address, @@ -468,15 +371,6 @@ describe('SftRolesRegistry', async () => { RevokeRoleData.revoker, RevokeRoleData.grantee, ) - // transfer tokens back to owner - /* .to.emit(MockToken, 'TransferSingle') - .withArgs( - SftRolesRegistry.sol.address, - SftRolesRegistry.sol.address, - RevokeRoleData.revoker, - RevokeRoleData.tokenId, - RoleAssignment.tokenAmount, - ) */ }) it('should revoke role if sender is approved by grantor', async () => { @@ -502,15 +396,6 @@ describe('SftRolesRegistry', async () => { RevokeRoleData.revoker, RevokeRoleData.grantee, ) - // transfer tokens back to owner - /* .to.emit(MockToken, 'TransferSingle') - .withArgs( - SftRolesRegistry.sol.address, - SftRolesRegistry.sol.address, - RevokeRoleData.revoker, - RevokeRoleData.tokenId, - RoleAssignment.tokenAmount, - ) */ }) it('should revoke role if sender is approved by grantee', async () => { await SftRolesRegistry.connect(grantee).setRoleApprovalForAll( @@ -535,15 +420,6 @@ describe('SftRolesRegistry', async () => { RevokeRoleData.revoker, RevokeRoleData.grantee, ) - // transfer tokens back to owner - /* .to.emit(MockToken, 'TransferSingle') - .withArgs( - SftRolesRegistry.sol.address, - SftRolesRegistry.sol.address, - RevokeRoleData.revoker, - RevokeRoleData.tokenId, - RoleAssignment.tokenAmount, - ) */ }) it('should revoke role if sender is grantee', async () => { @@ -577,15 +453,6 @@ describe('SftRolesRegistry', async () => { newRevokeRoleData.revoker, newRevokeRoleData.grantee, ) - // transfer tokens back to owner - /* .to.emit(MockToken, 'TransferSingle') - .withArgs( - SftRolesRegistry.sol.address, - SftRolesRegistry.sol.address, - RevokeRoleData.revoker, - RevokeRoleData.tokenId, - RoleAssignment.tokenAmount, - ) */ }) }) @@ -696,18 +563,7 @@ describe('SftRolesRegistry', async () => { RoleAssignment.role, RoleAssignment.grantee, ) - /* const hash = ethers.utils.defaultAbiCoder.encode( - ['uint256', 'bytes32', 'address', 'uint256', 'address'], - [ - RoleAssignment.nonce, - RoleAssignment.role, - RoleAssignment.tokenAddress, - RoleAssignment.tokenId, - RoleAssignment.grantor, - ], - ) - expect(roleData.hash).to.be.equal(ethers.utils.keccak256(hash)) */ - /* expect(roleData.tokenAmount).to.be.equal(RoleAssignment.tokenAmount) */ + expect(roleData.expirationDate).to.be.equal(RoleAssignment.expirationDate) expect(roleData.revocable).to.be.equal(RoleAssignment.revocable) expect(roleData.data).to.be.equal(RoleAssignment.data)