From 74cb61fc002a1dd6996fe72208195b8180c4b9e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ernani=20S=C3=A3o=20Thiago?= Date: Wed, 20 Dec 2023 19:23:22 -0500 Subject: [PATCH 01/19] refactor view functions --- .../SftRolesRegistrySingleRole.sol | 12 +++++-- .../interfaces/ISftRolesRegistry.sol | 22 ++++++++---- .../SftRolesRegistrySingleRole.spec.ts | 35 +++++++++++++------ 3 files changed, 51 insertions(+), 18 deletions(-) diff --git a/contracts/RolesRegistry/SftRolesRegistrySingleRole.sol b/contracts/RolesRegistry/SftRolesRegistrySingleRole.sol index b64be53..e501bf2 100644 --- a/contracts/RolesRegistry/SftRolesRegistrySingleRole.sol +++ b/contracts/RolesRegistry/SftRolesRegistrySingleRole.sol @@ -116,8 +116,8 @@ contract SftRolesRegistrySingleRole is ISftRolesRegistry, ERC1155Holder { uint256 _recordId, bytes32 _role, address _grantee - ) external view sameGrantee(_recordId, _role, _grantee) returns (RoleAssignment memory) { - return roleAssignments[_recordId][_role]; + ) external view sameGrantee(_recordId, _role, _grantee) returns (bytes memory data_) { + return roleAssignments[_recordId][_role].data; } function roleExpirationDate( @@ -128,6 +128,14 @@ contract SftRolesRegistrySingleRole is ISftRolesRegistry, ERC1155Holder { return roleAssignments[_recordId][_role].expirationDate; } + function isRoleRevocable( + uint256 _recordId, + bytes32 _role, + address _grantee + ) external view sameGrantee(_recordId, _role, _grantee) returns (bool revocable_) { + return roleAssignments[_recordId][_role].revocable; + } + function isRoleApprovedForAll( address _tokenAddress, address _grantor, diff --git a/contracts/RolesRegistry/interfaces/ISftRolesRegistry.sol b/contracts/RolesRegistry/interfaces/ISftRolesRegistry.sol index 7c579b8..d416a71 100644 --- a/contracts/RolesRegistry/interfaces/ISftRolesRegistry.sol +++ b/contracts/RolesRegistry/interfaces/ISftRolesRegistry.sol @@ -6,7 +6,7 @@ import { IERC165 } from '@openzeppelin/contracts/utils/introspection/IERC165.sol /// @title ERC-XXXX Semi-Fungible Token Roles /// @dev See https://eips.ethereum.org/EIPS/eip-XXXX -/// Note: the ERC-165 identifier for this interface is 0xa4629326 +/// Note: the ERC-165 identifier for this interface is 0x42ba720c interface ISftRolesRegistry is IERC165 { struct RoleAssignment { address grantee; @@ -123,29 +123,39 @@ interface ISftRolesRegistry is IERC165 { /// @param _recordId The record identifier. /// @param _role The role identifier. /// @param _grantee The user that received the role. - function roleData( + /// @return data_ The custom data. + function roleData(uint256 _recordId, bytes32 _role, address _grantee) external view returns (bytes memory data_); + + /// @notice Returns the expiration date of a role assignment. + /// @param _recordId The record identifier. + /// @param _role The role identifier. + /// @param _grantee The user that received the role. + /// @return expirationDate_ The expiration date. + function roleExpirationDate( uint256 _recordId, bytes32 _role, address _grantee - ) external view returns (RoleAssignment memory data_); + ) external view returns (uint64 expirationDate_); /// @notice Returns the expiration date of a role assignment. /// @param _recordId The record identifier. /// @param _role The role identifier. /// @param _grantee The user that received the role. - function roleExpirationDate( + /// @return revocable_ Whether the role is revocable or not. + function isRoleRevocable( uint256 _recordId, bytes32 _role, address _grantee - ) external view returns (uint64 expirationDate_); + ) external view returns (bool revocable_); /// @notice Checks if the grantor approved the operator for all NFTs. /// @param _tokenAddress The token address. /// @param _grantor The user that approved the operator. /// @param _operator The user that can grant and revoke roles. + /// @return isApproved_ Whether the operator is approved or not. function isRoleApprovedForAll( address _tokenAddress, address _grantor, address _operator - ) external view returns (bool); + ) external view returns (bool isApproved_); } diff --git a/test/SftRolesRegistry/SftRolesRegistrySingleRole.spec.ts b/test/SftRolesRegistry/SftRolesRegistrySingleRole.spec.ts index b52412b..a090596 100644 --- a/test/SftRolesRegistry/SftRolesRegistrySingleRole.spec.ts +++ b/test/SftRolesRegistry/SftRolesRegistrySingleRole.spec.ts @@ -566,6 +566,7 @@ describe('SftRolesRegistrySingleRole', async () => { await expect( SftRolesRegistry.connect(grantor).roleData(GrantRoleData.recordId, GrantRoleData.role, anotherUser.address), ).to.be.revertedWith('SftRolesRegistry: grantee mismatch') + await expect( SftRolesRegistry.connect(grantor).roleExpirationDate( GrantRoleData.recordId, @@ -573,6 +574,14 @@ describe('SftRolesRegistrySingleRole', async () => { anotherUser.address, ), ).to.be.revertedWith('SftRolesRegistry: grantee mismatch') + + await expect( + SftRolesRegistry.connect(grantor).isRoleRevocable( + GrantRoleData.recordId, + GrantRoleData.role, + anotherUser.address, + ), + ).to.be.revertedWith('SftRolesRegistry: grantee mismatch') }) it('should return role data', async () => { @@ -584,15 +593,21 @@ describe('SftRolesRegistrySingleRole', async () => { ), ).to.be.equal(GrantRoleData.expirationDate) - const roleDate = await SftRolesRegistry.connect(grantor).roleData( - GrantRoleData.recordId, - GrantRoleData.role, - GrantRoleData.grantee, - ) - expect(roleDate.grantee).to.be.equal(GrantRoleData.grantee) - expect(roleDate.expirationDate).to.be.equal(GrantRoleData.expirationDate) - expect(roleDate.revocable).to.be.equal(GrantRoleData.revocable) - expect(roleDate.data).to.be.equal(GrantRoleData.data) + expect( + await SftRolesRegistry.connect(grantor).roleData( + GrantRoleData.recordId, + GrantRoleData.role, + GrantRoleData.grantee, + ), + ).to.be.equal(GrantRoleData.data) + + expect( + await SftRolesRegistry.connect(grantor).isRoleRevocable( + GrantRoleData.recordId, + GrantRoleData.role, + GrantRoleData.grantee, + ), + ).to.be.equal(GrantRoleData.revocable) }) }) @@ -602,7 +617,7 @@ describe('SftRolesRegistrySingleRole', async () => { }) it('should return true if IERCXXXX interface id', async () => { - expect(await SftRolesRegistry.supportsInterface('0xa4629326')).to.be.true + expect(await SftRolesRegistry.supportsInterface('0x42ba720c')).to.be.true }) }) }) From 0ef071e080100babacefcc8b4df9a7d7a9deb606 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ernani=20S=C3=A3o=20Thiago?= Date: Tue, 26 Dec 2023 15:21:29 -0500 Subject: [PATCH 02/19] fixed lint --- scripts/02-grant-role.ts | 2 -- test/RolesRegistry.spec.ts | 8 +------- test/SftRolesRegistry/types.ts | 2 +- test/helpers.ts | 2 +- 4 files changed, 3 insertions(+), 11 deletions(-) diff --git a/scripts/02-grant-role.ts b/scripts/02-grant-role.ts index 7635fce..fd6f0a5 100644 --- a/scripts/02-grant-role.ts +++ b/scripts/02-grant-role.ts @@ -30,8 +30,6 @@ async function main() { const tokenIds = [217] // get hardhat accounts - const accounts = await ethers.getSigners() - const granteeAddress = accounts[0].address const grantorAddress = await kmsSigner.getAddress() const blockTimestamp = (await provider.getBlock('latest')).timestamp diff --git a/test/RolesRegistry.spec.ts b/test/RolesRegistry.spec.ts index bed1966..e5e7744 100644 --- a/test/RolesRegistry.spec.ts +++ b/test/RolesRegistry.spec.ts @@ -4,7 +4,6 @@ import { SignerWithAddress } from '@nomiclabs/hardhat-ethers/signers' import { expect } from 'chai' import { ERC7432InterfaceId } from './contants' import nock from 'nock' -import axios from 'axios' import { defaultAbiCoder, solidityKeccak256 } from 'ethers/lib/utils' import { NftMetadata, RoleAssignment } from './types' @@ -88,7 +87,6 @@ describe('RolesRegistry', () => { describe('Main Functions', async () => { let expirationDate: number const data = HashZero - let nftMetadata: NftMetadata let roleAssignment: RoleAssignment beforeEach(async () => { @@ -96,9 +94,6 @@ describe('RolesRegistry', () => { const block = await hre.ethers.provider.getBlock(blockNumber) expirationDate = block.timestamp + ONE_DAY - const tokenURI = await mockERC721.tokenURI(tokenId) - const response = await axios.get(tokenURI) - nftMetadata = response.data roleAssignment = { role: PROPERTY_MANAGER, tokenAddress: mockERC721.address, @@ -134,8 +129,7 @@ describe('RolesRegistry', () => { it('should NOT grant role from if expiration date is in the past', async () => { const blockNumber = await hre.ethers.provider.getBlockNumber() const block = await hre.ethers.provider.getBlock(blockNumber) - const expirationDateInThePast = block.timestamp - ONE_DAY - roleAssignment.expirationDate = expirationDateInThePast + roleAssignment.expirationDate = block.timestamp - ONE_DAY await expect(RolesRegistry.connect(grantor).grantRoleFrom(roleAssignment)).to.be.revertedWith( 'RolesRegistry: expiration date must be in the future', diff --git a/test/SftRolesRegistry/types.ts b/test/SftRolesRegistry/types.ts index 05dd3b0..6acc1b7 100644 --- a/test/SftRolesRegistry/types.ts +++ b/test/SftRolesRegistry/types.ts @@ -9,7 +9,7 @@ export interface GrantRoleData { recordId: number role: string grantee: string - expirationDate: number | null + expirationDate: number revocable: boolean data: string } diff --git a/test/helpers.ts b/test/helpers.ts index f43dc40..5e5bcba 100644 --- a/test/helpers.ts +++ b/test/helpers.ts @@ -60,7 +60,7 @@ export async function assertListItem( itemExpirationDate: number, expectedPosition: number, ) { - const { expirationDate, previous, next } = await LinkedLists.getListItem(itemNonce) + const { expirationDate, previous } = await LinkedLists.getListItem(itemNonce) expect(expirationDate, `Item ${itemNonce} expiration date is not ${itemExpirationDate}`).to.be.equal( itemExpirationDate, ) From 51649ddd64d366c70e55fa727519a1878858dd11 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ernani=20S=C3=A3o=20Thiago?= Date: Tue, 26 Dec 2023 15:27:44 -0500 Subject: [PATCH 03/19] This commit includes the following changes: * added recordInfo function. * refactor both implementations. * fixed bug on _findCaller. --- contracts/RolesRegistry/SftRolesRegistry.sol | 371 ++++--- .../SftRolesRegistrySingleRole.sol | 24 +- .../interfaces/ISftRolesRegistry.sol | 12 +- .../RolesRegistry/libraries/LinkedLists.sol | 22 +- contracts/mocks/MockLinkedLists.sol | 3 +- .../SftRolesRegistry/SftRolesRegistry.spec.ts | 979 ++++++------------ .../SftRolesRegistrySingleRole.spec.ts | 43 +- test/SftRolesRegistry/helpers/assertEvents.ts | 94 ++ .../{helpers.ts => helpers/mockData.ts} | 4 +- 9 files changed, 624 insertions(+), 928 deletions(-) create mode 100644 test/SftRolesRegistry/helpers/assertEvents.ts rename test/SftRolesRegistry/{helpers.ts => helpers/mockData.ts} (97%) diff --git a/contracts/RolesRegistry/SftRolesRegistry.sol b/contracts/RolesRegistry/SftRolesRegistry.sol index 12d9b44..1e34041 100644 --- a/contracts/RolesRegistry/SftRolesRegistry.sol +++ b/contracts/RolesRegistry/SftRolesRegistry.sol @@ -2,36 +2,38 @@ pragma solidity 0.8.9; -import { IERCXXXX } from './interfaces/IERCXXXX.sol'; +import { ISftRolesRegistry } from './interfaces/ISftRolesRegistry.sol'; import { IERC165 } from '@openzeppelin/contracts/utils/introspection/IERC165.sol'; import { IERC1155 } from '@openzeppelin/contracts/token/ERC1155/IERC1155.sol'; import { IERC1155Receiver } from '@openzeppelin/contracts/token/ERC1155/IERC1155Receiver.sol'; import { ERC1155Holder, ERC1155Receiver } from '@openzeppelin/contracts/token/ERC1155/utils/ERC1155Holder.sol'; import { ERC165Checker } from '@openzeppelin/contracts/utils/introspection/ERC165Checker.sol'; import { LinkedLists } from './libraries/LinkedLists.sol'; +import { EnumerableSet } from '@openzeppelin/contracts/utils/structs/EnumerableSet.sol'; // Semi-fungible token (SFT) roles registry -contract SftRolesRegistry is IERCXXXX, ERC1155Holder { +contract SftRolesRegistry is ISftRolesRegistry, ERC1155Holder { using LinkedLists for LinkedLists.Lists; using LinkedLists for LinkedLists.ListItem; + using EnumerableSet for EnumerableSet.UintSet; + using EnumerableSet for EnumerableSet.Bytes32Set; + uint256 public recordCount; LinkedLists.Lists internal lists; - // grantor => tokenAddress => operator => isApproved - mapping(address => mapping(address => mapping(address => bool))) public tokenApprovals; + // recordId => Record + mapping(uint256 => Record) public records; - modifier validExpirationDate(uint64 _expirationDate) { - require(_expirationDate > block.timestamp, 'SftRolesRegistry: expiration date must be in the future'); - _; - } + // recordId => role => lastGrantee + mapping(uint256 => mapping(bytes32 => address)) internal lastGrantee; - modifier onlyOwnerOrApprovedWithBalance( - address _account, - address _tokenAddress, - uint256 _tokenId, - uint256 _tokenAmount - ) { - require(_tokenAmount > 0, 'SftRolesRegistry: tokenAmount must be greater than zero'); + // recordId => role[] + mapping(uint256 => EnumerableSet.Bytes32Set) internal recordIdToRoles; + + // grantor => tokenAddress => operator => isApproved + mapping(address => mapping(address => mapping(address => bool))) public roleApprovals; + + modifier onlyOwnerOrApproved(address _account, address _tokenAddress) { require( _account == msg.sender || isRoleApprovedForAll(_tokenAddress, _account, msg.sender), 'SftRolesRegistry: account not approved' @@ -41,177 +43,120 @@ contract SftRolesRegistry is IERCXXXX, ERC1155Holder { /** External Functions **/ - function grantRoleFrom( - RoleAssignment calldata _roleAssignment - ) - external - override - validExpirationDate(_roleAssignment.expirationDate) - onlyOwnerOrApprovedWithBalance( - _roleAssignment.grantor, - _roleAssignment.tokenAddress, - _roleAssignment.tokenId, - _roleAssignment.tokenAmount - ) - { - bytes32 hash = _hashRoleData( - _roleAssignment.nonce, - _roleAssignment.role, - _roleAssignment.tokenAddress, - _roleAssignment.tokenId, - _roleAssignment.grantor - ); - bytes32 rootKey = _getHeadKey( - _roleAssignment.grantee, - _roleAssignment.role, - _roleAssignment.tokenAddress, - _roleAssignment.tokenId - ); - LinkedLists.ListItem storage item = lists.items[_roleAssignment.nonce]; - if (item.data.expirationDate == 0) { - // nonce is not being used - - _transferFrom( - _roleAssignment.grantor, - address(this), - _roleAssignment.tokenAddress, - _roleAssignment.tokenId, - _roleAssignment.tokenAmount - ); - } else { - // nonce is being used - require(item.data.hash == hash, 'SftRolesRegistry: nonce exist, but data mismatch'); // validates nonce, role, tokenAddress, tokenId, grantor - require( - item.data.expirationDate < block.timestamp || item.data.revocable, - 'SftRolesRegistry: nonce is not expired or is not revocable' - ); - - // deposit or withdraw tokens - _depositOrWithdrawTokens( - _roleAssignment.tokenAddress, - _roleAssignment.tokenId, - _roleAssignment.grantor, - item.data.tokenAmount, - _roleAssignment.tokenAmount - ); - - // remove from the list - if (item.data.grantee != _roleAssignment.grantee) { - bytes32 oldRootKey = _getHeadKey( - item.data.grantee, - _roleAssignment.role, - _roleAssignment.tokenAddress, - _roleAssignment.tokenId - ); - lists.remove(oldRootKey, _roleAssignment.nonce); - } else { - lists.remove(rootKey, _roleAssignment.nonce); - } - } - - // insert on the list - _insert(hash, rootKey, _roleAssignment); - } - - function _depositOrWithdrawTokens( + function createRecordFrom( + address _grantor, address _tokenAddress, uint256 _tokenId, - address _account, - uint256 _depositedAmount, - uint256 _amountRequired - ) internal { - if (_depositedAmount > _amountRequired) { - // return leftover tokens - uint256 tokensToReturn = _depositedAmount - _amountRequired; - _transferFrom(address(this), _account, _tokenAddress, _tokenId, tokensToReturn); - } else if (_amountRequired > _depositedAmount) { - // deposit missing tokens - uint256 tokensToDeposit = _amountRequired - _depositedAmount; - _transferFrom(_account, address(this), _tokenAddress, _tokenId, tokensToDeposit); - } + uint256 _tokenAmount + ) external override onlyOwnerOrApproved(_grantor, _tokenAddress) returns (uint256 recordId_) { + require(_tokenAmount > 0, 'SftRolesRegistry: tokenAmount must be greater than zero'); + recordId_ = _createRecord(_grantor, _tokenAddress, _tokenId, _tokenAmount); } - function _insert(bytes32 _hash, bytes32 _rootKey, RoleAssignment calldata _roleAssignment) internal { - RoleData memory data = RoleData( - _hash, - _roleAssignment.grantee, - _roleAssignment.tokenAmount, - _roleAssignment.expirationDate, - _roleAssignment.revocable, - _roleAssignment.data - ); - - lists.insert(_rootKey, _roleAssignment.nonce, data); - - emit RoleGranted( - _roleAssignment.nonce, - _roleAssignment.role, - _roleAssignment.tokenAddress, - _roleAssignment.tokenId, - _roleAssignment.tokenAmount, - _roleAssignment.grantor, - _roleAssignment.grantee, - _roleAssignment.expirationDate, - _roleAssignment.revocable, - _roleAssignment.data - ); + function grantRole( + uint256 _recordId, + bytes32 _role, + address _grantee, + uint64 _expirationDate, + bool _revocable, + bytes calldata _data + ) external override onlyOwnerOrApproved(records[_recordId].grantor, records[_recordId].tokenAddress) { + require(_expirationDate > block.timestamp, 'SftRolesRegistry: expiration date must be in the future'); + _grantOrUpdateRole(_recordId, _role, _grantee, _expirationDate, _revocable, _data); } - function revokeRoleFrom(RevokeRoleData calldata _revokeRoleData) external override { - LinkedLists.ListItem storage item = lists.items[_revokeRoleData.nonce]; - address _grantee = item.data.grantee; - require(item.data.hash == _hashRoleData(_revokeRoleData), 'SftRolesRegistry: could not find role assignment'); + function revokeRoleFrom(uint256 _recordId, bytes32 _role, address _grantee) external override { + uint256 itemId = _getItemId(_recordId, _role, _grantee); + LinkedLists.RoleData storage data = lists.items[itemId].data; + require(data.expirationDate > 0, 'SftRolesRegistry: could not find role assignment'); - address caller = _findCaller(_revokeRoleData, _grantee); - if (item.data.expirationDate > block.timestamp && !item.data.revocable) { + Record storage record = records[_recordId]; + address caller = _findCaller(record.grantor, _grantee, record.tokenAddress); + if (data.expirationDate > block.timestamp && !data.revocable) { // if role is not expired and is not revocable, only the grantee can revoke it require(caller == _grantee, 'SftRolesRegistry: role is not revocable or caller is not the approved'); } - uint256 tokensToReturn = item.data.tokenAmount; + // remove from the list + bytes32 headKey = _getHeadKey(_grantee, _role, record.tokenAddress, record.tokenId); + lists.remove(headKey, itemId); - bytes32 rootKey = _getHeadKey( - _grantee, - _revokeRoleData.role, - _revokeRoleData.tokenAddress, - _revokeRoleData.tokenId - ); + // remove from recordIdToRoles + recordIdToRoles[_recordId].remove(_role); + delete lastGrantee[_recordId][_role]; - // remove from the list - lists.remove(rootKey, _revokeRoleData.nonce); - - emit RoleRevoked( - _revokeRoleData.nonce, - _revokeRoleData.role, - _revokeRoleData.tokenAddress, - _revokeRoleData.tokenId, - tokensToReturn, - _revokeRoleData.grantor, - _grantee - ); + emit RoleRevoked(_recordId, _role, _grantee); + } + + function withdrawFrom( + uint256 _recordId + ) external onlyOwnerOrApproved(records[_recordId].grantor, records[_recordId].tokenAddress) { + uint256 numberOfRoles = recordIdToRoles[_recordId].length(); + for (uint256 i = 0; i < numberOfRoles; i++) { + bytes32 role = recordIdToRoles[_recordId].at(i); + address grantee = lastGrantee[_recordId][role]; + uint256 itemId = _getItemId(_recordId, role, grantee); + + LinkedLists.RoleData storage data = lists.items[itemId].data; + require( + data.expirationDate < block.timestamp || data.revocable, + 'SftRolesRegistry: role is not expired and is not revocable' + ); + + // remove from list and storage + bytes32 headKey = _getHeadKey(grantee, role, records[_recordId].tokenAddress, records[_recordId].tokenId); + lists.remove(headKey, itemId); + recordIdToRoles[_recordId].remove(role); + delete lastGrantee[_recordId][role]; + } _transferFrom( address(this), - _revokeRoleData.grantor, - _revokeRoleData.tokenAddress, - _revokeRoleData.tokenId, - tokensToReturn + records[_recordId].grantor, + records[_recordId].tokenAddress, + records[_recordId].tokenId, + records[_recordId].tokenAmount ); + + delete records[_recordId]; + emit Withdrew(_recordId); } function setRoleApprovalForAll(address _tokenAddress, address _operator, bool _isApproved) external override { - tokenApprovals[msg.sender][_tokenAddress][_operator] = _isApproved; + roleApprovals[msg.sender][_tokenAddress][_operator] = _isApproved; emit RoleApprovalForAll(_tokenAddress, _operator, _isApproved); } /** View Functions **/ - function roleData(uint256 _nonce) external view returns (RoleData memory) { - return lists.items[_nonce].data; + function recordInfo( + uint256 _recordId + ) external view returns (address grantor_, address tokenAddress_, uint256 tokenId_, uint256 tokenAmount_) { + Record memory record = records[_recordId]; + grantor_ = record.grantor; + tokenAddress_ = record.tokenAddress; + tokenId_ = record.tokenId; + tokenAmount_ = record.tokenAmount; + } + + function roleData(uint256 _recordId, bytes32 _role, address _grantee) external view returns (bytes memory data_) { + return lists.items[_getItemId(_recordId, _role, _grantee)].data.data; } - function roleExpirationDate(uint256 _nonce) external view returns (uint64 expirationDate_) { - return lists.items[_nonce].data.expirationDate; + function roleExpirationDate( + uint256 _recordId, + bytes32 _role, + address _grantee + ) external view returns (uint64 expirationDate_) { + return lists.items[_getItemId(_recordId, _role, _grantee)].data.expirationDate; + } + + function isRoleRevocable( + uint256 _recordId, + bytes32 _role, + address _grantee + ) external view returns (bool revocable_) { + return lists.items[_getItemId(_recordId, _role, _grantee)].data.revocable; } function isRoleApprovedForAll( @@ -219,17 +164,17 @@ contract SftRolesRegistry is IERCXXXX, ERC1155Holder { address _grantor, address _operator ) public view override returns (bool) { - return tokenApprovals[_grantor][_tokenAddress][_operator]; + return roleApprovals[_grantor][_tokenAddress][_operator]; } - function roleBalanceOf( + /*function roleBalanceOf( bytes32 _role, address _tokenAddress, uint256 _tokenId, address _grantee ) external view returns (uint256 balance_) { - bytes32 rootKey = _getHeadKey(_grantee, _role, _tokenAddress, _tokenId); - uint256 currentNonce = lists.heads[rootKey]; + bytes32 headKey = _getHeadKey(_grantee, _role, _tokenAddress, _tokenId); + uint256 currentNonce = lists.heads[headKey]; if (currentNonce == 0) { return 0; } @@ -244,59 +189,91 @@ contract SftRolesRegistry is IERCXXXX, ERC1155Holder { balance_ += currentItem.data.tokenAmount; currentNonce = currentItem.next; } - } + }*/ function supportsInterface( bytes4 interfaceId ) public view virtual override(ERC1155Receiver, IERC165) returns (bool) { - return interfaceId == type(IERCXXXX).interfaceId || interfaceId == type(IERC1155Receiver).interfaceId; + return interfaceId == type(ISftRolesRegistry).interfaceId || interfaceId == type(IERC1155Receiver).interfaceId; } /** Helper Functions **/ - function _transferFrom( - address _from, - address _to, + function _createRecord( + address _grantor, address _tokenAddress, uint256 _tokenId, uint256 _tokenAmount - ) internal { - IERC1155(_tokenAddress).safeTransferFrom(_from, _to, _tokenId, _tokenAmount, ''); + ) internal returns (uint256 recordId_) { + recordId_ = ++recordCount; + records[recordId_] = Record(_grantor, _tokenAddress, _tokenId, _tokenAmount); + _transferFrom(_grantor, address(this), _tokenAddress, _tokenId, _tokenAmount); + emit RecordCreated(_grantor, recordId_, _tokenAddress, _tokenId, _tokenAmount); } - function _hashRoleData(RevokeRoleData calldata _revokeRoleData) internal pure returns (bytes32) { - return - _hashRoleData( - _revokeRoleData.nonce, - _revokeRoleData.role, - _revokeRoleData.tokenAddress, - _revokeRoleData.tokenId, - _revokeRoleData.grantor - ); + function _grantOrUpdateRole( + uint256 _recordId, + bytes32 _role, + address _grantee, + uint64 _expirationDate, + bool _revocable, + bytes calldata _data + ) internal { + // verify if a role exist + address latestGrantee = lastGrantee[_recordId][_role]; + // if exist, make sure that is expired or revocable + uint256 itemId = _getItemId(_recordId, _role, latestGrantee); + LinkedLists.RoleData storage lastRoleData = lists.items[itemId].data; + require( + lastRoleData.expirationDate < block.timestamp || lastRoleData.revocable, + 'SftRolesRegistry: role is not expired and is not revocable' + ); + + // insert in the list + _insert(_recordId, _role, _grantee, _expirationDate, _revocable, _data); + + // store last grantee and role + recordIdToRoles[_recordId].add(_role); + lastGrantee[_recordId][_role] = _grantee; + + emit RoleGranted(_recordId, _role, _grantee, _expirationDate, _revocable, _data); } - function _hashRoleData( - uint256 _nonce, + function _insert( + uint256 _recordId, bytes32 _role, + address _grantee, + uint64 _expirationDate, + bool _revocable, + bytes calldata _data + ) internal { + bytes32 headKey = _getHeadKey(_grantee, _role, records[_recordId].tokenAddress, records[_recordId].tokenId); + LinkedLists.RoleData memory data = LinkedLists.RoleData(_expirationDate, _revocable, _data); + uint256 itemId = _getItemId(_recordId, _role, _grantee); + lists.insert(headKey, itemId, data); + } + + function _transferFrom( + address _from, + address _to, address _tokenAddress, uint256 _tokenId, - address _grantor - ) internal pure returns (bytes32) { - return keccak256(abi.encode(_nonce, _role, _tokenAddress, _tokenId, _grantor)); + uint256 _tokenAmount + ) internal { + IERC1155(_tokenAddress).safeTransferFrom(_from, _to, _tokenId, _tokenAmount, ''); } - function _findCaller(RevokeRoleData calldata _revokeRoleData, address _grantee) internal view returns (address) { - if ( - _revokeRoleData.grantor == msg.sender || - isRoleApprovedForAll(_revokeRoleData.tokenAddress, _revokeRoleData.grantor, msg.sender) - ) { - return _revokeRoleData.grantor; - } - - if (_grantee == msg.sender || isRoleApprovedForAll(_revokeRoleData.tokenAddress, _grantee, msg.sender)) { + // careful with the following edge case: + // if grantee is approved by grantor, the first one checked is returned + // if grantor is returned instead of grantee, the grantee won't be able + // to revoke the role assignment before the expiration date + function _findCaller(address _grantor, address _grantee, address _tokenAddress) internal view returns (address) { + if (_grantee == msg.sender || isRoleApprovedForAll(_tokenAddress, _grantee, msg.sender)) { return _grantee; } - + if (_grantor == msg.sender || isRoleApprovedForAll(_tokenAddress, _grantor, msg.sender)) { + return _grantor; + } revert('SftRolesRegistry: sender must be approved'); } @@ -305,7 +282,11 @@ contract SftRolesRegistry is IERCXXXX, ERC1155Holder { bytes32 _role, address _tokenAddress, uint256 _tokenId - ) internal pure returns (bytes32 rootKey_) { + ) internal pure returns (bytes32) { return keccak256(abi.encodePacked(_grantee, _role, _tokenAddress, _tokenId)); } + + function _getItemId(uint256 _recordId, bytes32 _role, address _grantee) internal pure returns (uint256) { + return uint256(keccak256(abi.encodePacked(_recordId, _role, _grantee))); + } } diff --git a/contracts/RolesRegistry/SftRolesRegistrySingleRole.sol b/contracts/RolesRegistry/SftRolesRegistrySingleRole.sol index e501bf2..33d03b1 100644 --- a/contracts/RolesRegistry/SftRolesRegistrySingleRole.sol +++ b/contracts/RolesRegistry/SftRolesRegistrySingleRole.sol @@ -70,7 +70,9 @@ contract SftRolesRegistrySingleRole is ISftRolesRegistry, ERC1155Holder { } function revokeRoleFrom( - uint256 _recordId, bytes32 _role, address _grantee + uint256 _recordId, + bytes32 _role, + address _grantee ) external override sameGrantee(_recordId, _role, _grantee) { RoleAssignment storage roleAssignment = roleAssignments[_recordId][_role]; Record storage record = records[_recordId]; @@ -112,6 +114,16 @@ contract SftRolesRegistrySingleRole is ISftRolesRegistry, ERC1155Holder { /** View Functions **/ + function recordInfo( + uint256 _recordId + ) external view returns (address grantor_, address tokenAddress_, uint256 tokenId_, uint256 tokenAmount_) { + Record memory record = records[_recordId]; + grantor_ = record.grantor; + tokenAddress_ = record.tokenAddress; + tokenId_ = record.tokenId; + tokenAmount_ = record.tokenAmount; + } + function roleData( uint256 _recordId, bytes32 _role, @@ -186,13 +198,17 @@ contract SftRolesRegistrySingleRole is ISftRolesRegistry, ERC1155Holder { IERC1155(_tokenAddress).safeTransferFrom(_from, _to, _tokenId, _tokenAmount, ''); } + // careful with the following edge case: + // if grantee is approved by grantor, the first one checked is returned + // if grantor is returned instead of grantee, the grantee won't be able + // to revoke the role assignment before the expiration date function _findCaller(address _grantor, address _grantee, address _tokenAddress) internal view returns (address) { - if (_grantor == msg.sender || isRoleApprovedForAll(_tokenAddress, _grantor, msg.sender)) { - return _grantor; - } if (_grantee == msg.sender || isRoleApprovedForAll(_tokenAddress, _grantee, msg.sender)) { return _grantee; } + if (_grantor == msg.sender || isRoleApprovedForAll(_tokenAddress, _grantor, msg.sender)) { + return _grantor; + } revert('SftRolesRegistry: sender must be approved'); } } diff --git a/contracts/RolesRegistry/interfaces/ISftRolesRegistry.sol b/contracts/RolesRegistry/interfaces/ISftRolesRegistry.sol index d416a71..d373821 100644 --- a/contracts/RolesRegistry/interfaces/ISftRolesRegistry.sol +++ b/contracts/RolesRegistry/interfaces/ISftRolesRegistry.sol @@ -6,7 +6,7 @@ import { IERC165 } from '@openzeppelin/contracts/utils/introspection/IERC165.sol /// @title ERC-XXXX Semi-Fungible Token Roles /// @dev See https://eips.ethereum.org/EIPS/eip-XXXX -/// Note: the ERC-165 identifier for this interface is 0x42ba720c +/// Note: the ERC-165 identifier for this interface is 0xf254051c interface ISftRolesRegistry is IERC165 { struct RoleAssignment { address grantee; @@ -119,6 +119,16 @@ interface ISftRolesRegistry is IERC165 { /** View Functions **/ + /// @notice Returns all the information regarding a record. + /// @param _recordId The record identifier. + /// @return grantor_ The record owner. + /// @return tokenAddress_ The token address. + /// @return tokenId_ The token identifier. + /// @return tokenAmount_ The token amount. + function recordInfo( + uint256 _recordId + ) external view returns (address grantor_, address tokenAddress_, uint256 tokenId_, uint256 tokenAmount_); + /// @notice Returns the custom data of a role assignment. /// @param _recordId The record identifier. /// @param _role The role identifier. diff --git a/contracts/RolesRegistry/libraries/LinkedLists.sol b/contracts/RolesRegistry/libraries/LinkedLists.sol index 21a9ca3..5588884 100644 --- a/contracts/RolesRegistry/libraries/LinkedLists.sol +++ b/contracts/RolesRegistry/libraries/LinkedLists.sol @@ -2,7 +2,7 @@ pragma solidity 0.8.9; -import { IERCXXXX } from '../interfaces/IERCXXXX.sol'; +import { ISftRolesRegistry } from '../interfaces/ISftRolesRegistry.sol'; /// LinkedLists allow developers to manage multiple linked lists at once. /// all lists are identified by a head key (bytes32) @@ -11,18 +11,26 @@ import { IERCXXXX } from '../interfaces/IERCXXXX.sol'; library LinkedLists { uint256 public constant EMPTY = 0; - struct Lists { - mapping(bytes32 => uint256) heads; - mapping(uint256 => ListItem) items; + struct RoleData { + uint64 expirationDate; + bool revocable; + bytes data; } struct ListItem { - IERCXXXX.RoleData data; + RoleData data; uint256 previous; uint256 next; } - function insert(Lists storage _self, bytes32 _headKey, uint256 _nonce, IERCXXXX.RoleData memory _data) internal { + struct Lists { + // headKey => recordId + mapping(bytes32 => uint256) heads; + // hash(recordId, role, grantee) => item + mapping(uint256 => ListItem) items; + } + + function insert(Lists storage _self, bytes32 _headKey, uint256 _nonce, RoleData memory _data) internal { require(_nonce != EMPTY, 'LinkedLists: invalid nonce'); uint256 headNonce = _self.heads[_headKey]; @@ -61,7 +69,7 @@ library LinkedLists { Lists storage _self, uint256 _previousNonce, uint256 _dataNonce, - IERCXXXX.RoleData memory _data + RoleData memory _data ) internal { ListItem storage previousItem = _self.items[_previousNonce]; if (previousItem.next == EMPTY) { diff --git a/contracts/mocks/MockLinkedLists.sol b/contracts/mocks/MockLinkedLists.sol index 82fc1b9..7e1a301 100644 --- a/contracts/mocks/MockLinkedLists.sol +++ b/contracts/mocks/MockLinkedLists.sol @@ -2,7 +2,6 @@ pragma solidity 0.8.9; -import { IERCXXXX } from '../RolesRegistry/interfaces/IERCXXXX.sol'; import { LinkedLists } from '../RolesRegistry/libraries/LinkedLists.sol'; contract MockLinkedLists { @@ -19,7 +18,7 @@ contract MockLinkedLists { function insert(bytes32 _headKey, uint256 _nonce, uint64 _expirationDate) external { // the only attribute that affects the list sorting is the expiration date - IERCXXXX.RoleData memory data = IERCXXXX.RoleData('', address(0), 1, _expirationDate, true, ''); + LinkedLists.RoleData memory data = LinkedLists.RoleData(_expirationDate, true, ''); lists.insert(_headKey, _nonce, data); } diff --git a/test/SftRolesRegistry/SftRolesRegistry.spec.ts b/test/SftRolesRegistry/SftRolesRegistry.spec.ts index bc9bfbd..d786b7f 100644 --- a/test/SftRolesRegistry/SftRolesRegistry.spec.ts +++ b/test/SftRolesRegistry/SftRolesRegistry.spec.ts @@ -2,12 +2,12 @@ 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 { RoleAssignment, RevokeRoleData } from './types' +import { ONE_DAY, generateRoleId, buildRecord, buildGrantRole } from './helpers/mockData' +import { Record, GrantRoleData } from './types' import { generateRandomInt } from '../helpers' +import { assertCreateRecordEvent, assertGrantRoleEvent, assertRevokeRoleEvent } from './helpers/assertEvents' const { AddressZero } = ethers.constants @@ -34,666 +34,345 @@ describe('SftRolesRegistry', async () => { await loadFixture(deployContracts) }) - describe('grantRole', async () => { - it('should revert without a reason if tokenAddress is not an ERC-1155 contract', async () => { - const roleAssignment = await buildRoleAssignment() - await expect(SftRolesRegistry.connect(grantor).grantRoleFrom(roleAssignment)).to.be.reverted - }) - - it('should grant role for two different grantees with the same tokenId', async function () { - const roleAssignment = await buildRoleAssignment({ - tokenAddress: MockToken.address, + describe('createRecordFrom', async () => { + it('should revert when sender is not grantor or approved', async () => { + const record = buildRecord({ grantor: grantor.address, - grantee: grantee.address, + tokenAddress: MockToken.address, }) - await MockToken.mint(grantor.address, roleAssignment.tokenId, roleAssignment.tokenAmount * 2 + 3) - await MockToken.connect(grantor).setApprovalForAll(SftRolesRegistry.address, true) - - const sharedNonce = roleAssignment.nonce - const grantee1 = grantee.address - const grantee2 = '0x0000000000000000000000000000000000000001' - const originalTokenAmount = 7 - roleAssignment.tokenAmount = originalTokenAmount - - // 1. grant role to grantee1 with nonce 444 - roleAssignment.nonce = 444 - await expect(SftRolesRegistry.connect(grantor).grantRoleFrom(roleAssignment)).to.not.be.reverted - - // check if grantee1 balance is correct - expect( - await SftRolesRegistry.roleBalanceOf( - roleAssignment.role, - roleAssignment.tokenAddress, - roleAssignment.tokenId, - roleAssignment.grantee, - ), - ).to.be.equal(roleAssignment.tokenAmount) - - // 2. grant role to grantee1 with sharedNonce - roleAssignment.nonce = sharedNonce - await expect(SftRolesRegistry.connect(grantor).grantRoleFrom(roleAssignment)).to.not.be.reverted - - // now grantee1 should have double of the tokens - expect( - await SftRolesRegistry.roleBalanceOf( - roleAssignment.role, - roleAssignment.tokenAddress, - roleAssignment.tokenId, - roleAssignment.grantee, - ), - ).to.be.equal(roleAssignment.tokenAmount * 2) - - // 3. grant role to grantee2 with nonce 1 - roleAssignment.nonce = 1 - roleAssignment.grantee = grantee2 - roleAssignment.tokenAmount = 1 - await expect(SftRolesRegistry.connect(grantor).grantRoleFrom(roleAssignment)).to.not.be.reverted - - // grantee2 should have 1 token as balance - expect( - await SftRolesRegistry.roleBalanceOf( - roleAssignment.role, - roleAssignment.tokenAddress, - roleAssignment.tokenId, - roleAssignment.grantee, - ), - ).to.be.equal(1) - - // 4. grant role to grantee2 with sharedNonce - roleAssignment.nonce = sharedNonce - await expect(SftRolesRegistry.connect(grantor).grantRoleFrom(roleAssignment)) - .to.emit(MockToken, 'TransferSingle') - .withArgs( - SftRolesRegistry.address, //operator - SftRolesRegistry.address, //from - roleAssignment.grantor, //to - roleAssignment.tokenId, - originalTokenAmount - 1, - ) - - // now grantee2 should have 2 tokens as balance - expect( - await SftRolesRegistry.roleBalanceOf( - roleAssignment.role, - roleAssignment.tokenAddress, - roleAssignment.tokenId, - roleAssignment.grantee, - ), - ).to.be.equal(2) - - expect( - await SftRolesRegistry.roleBalanceOf( - roleAssignment.role, - roleAssignment.tokenAddress, - roleAssignment.tokenId, - grantee1, + await expect( + SftRolesRegistry.connect(anotherUser).createRecordFrom( + record.grantor, + record.tokenAddress, + record.tokenId, + record.tokenAmount, ), - ).to.be.equal(originalTokenAmount) + ).to.be.revertedWith('SftRolesRegistry: account not approved') }) - it('should revert if expirationDate is in the past', async () => { - const roleAssignment = await buildRoleAssignment({ - expirationDate: (await time.latest()) - ONE_DAY, - }) - await expect(SftRolesRegistry.grantRoleFrom(roleAssignment)).to.be.revertedWith( - 'SftRolesRegistry: expiration date must be in the future', - ) - }) - - it('should revert when sender is not grantor or approved', async () => { - const roleAssignment = await buildRoleAssignment({ + it('should revert when tokenAmount is zero', async () => { + const record = buildRecord({ + grantor: grantor.address, tokenAddress: MockToken.address, + tokenAmount: 0, }) - await expect(SftRolesRegistry.connect(grantor).grantRoleFrom(roleAssignment)).to.be.revertedWith( - 'SftRolesRegistry: account not approved', - ) + await expect( + SftRolesRegistry.connect(grantor).createRecordFrom( + record.grantor, + record.tokenAddress, + record.tokenId, + record.tokenAmount, + ), + ).to.be.revertedWith('SftRolesRegistry: tokenAmount must be greater than zero') }) - it('should revert if contract cannot transfer tokens', async () => { - const roleAssignment = await buildRoleAssignment({ - tokenAddress: MockToken.address, + it('should revert without a reason if tokenAddress is not an ERC-1155 contract', async () => { + const record = buildRecord({ grantor: grantor.address, + tokenAmount: generateRandomInt(), }) - await MockToken.mint(grantor.address, roleAssignment.tokenId, roleAssignment.tokenAmount) - await expect(SftRolesRegistry.connect(grantor).grantRoleFrom(roleAssignment)).to.be.revertedWith( - 'ERC1155: caller is not token owner or approved', - ) + await expect( + SftRolesRegistry.connect(grantor).createRecordFrom( + record.grantor, + record.tokenAddress, + record.tokenId, + record.tokenAmount, + ), + ).to.be.reverted }) - it('should revert if tokenAmount is zero', async () => { - const roleAssignment = await buildRoleAssignment({ - tokenAmount: 0, + it('should revert if contract is not approved to transfer tokens', async () => { + const record = buildRecord({ + grantor: grantor.address, + tokenAddress: MockToken.address, + tokenAmount: generateRandomInt(), }) - await expect(SftRolesRegistry.connect(grantor).grantRoleFrom(roleAssignment)).to.be.revertedWith( - 'SftRolesRegistry: tokenAmount must be greater than zero', - ) + await MockToken.mint(grantor.address, record.tokenId, record.tokenAmount) + await expect( + SftRolesRegistry.connect(grantor).createRecordFrom( + record.grantor, + record.tokenAddress, + record.tokenId, + record.tokenAmount, + ), + ).to.be.revertedWith('ERC1155: caller is not token owner or approved') }) it('should revert when grantor does not have enough tokens', async () => { - const roleAssignment = await buildRoleAssignment({ - tokenAddress: MockToken.address, + const record = buildRecord({ grantor: grantor.address, - tokenAmount: 100, + tokenAddress: MockToken.address, + tokenAmount: generateRandomInt() + 1, }) - await MockToken.mint(grantor.address, roleAssignment.tokenId, roleAssignment.tokenAmount - 10) + await MockToken.mint(grantor.address, record.tokenId, record.tokenAmount - 1) await MockToken.connect(grantor).setApprovalForAll(SftRolesRegistry.address, true) - await expect(SftRolesRegistry.connect(grantor).grantRoleFrom(roleAssignment)).to.be.revertedWith( - 'ERC1155: insufficient balance for transfer', - ) + await expect( + SftRolesRegistry.connect(grantor).createRecordFrom( + record.grantor, + record.tokenAddress, + record.tokenId, + record.tokenAmount, + ), + ).to.be.revertedWith('ERC1155: insufficient balance for transfer') }) - it('should revert if nonce is zero', async () => { - const roleAssignment = await buildRoleAssignment({ - nonce: 0, - tokenAddress: MockToken.address, - grantor: grantor.address, - }) - await MockToken.mint(grantor.address, roleAssignment.tokenId, roleAssignment.tokenAmount) - await MockToken.connect(grantor).setApprovalForAll(SftRolesRegistry.address, true) - await expect(SftRolesRegistry.connect(grantor).grantRoleFrom(roleAssignment)).to.be.revertedWith( - 'LinkedLists: invalid nonce', - ) - expect(await MockToken.balanceOf(grantor.address, roleAssignment.tokenId)).to.be.equal(roleAssignment.tokenAmount) - }) - - describe('when nonce does not exist', async () => { - it('should grant role when grantor is sender and has enough tokens', async () => { - 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 expect(SftRolesRegistry.connect(grantor).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, - ) - }) + it('should create record when sender is grantor', async () => { + await assertCreateRecordEvent(SftRolesRegistry, MockToken, grantor, 1) + }) - it('should grant role when sender is approved and grantor has enough tokens', async () => { - 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, - ) - }) + it('should create record when sender is approved', async () => { + await assertCreateRecordEvent(SftRolesRegistry, MockToken, grantor, 1, anotherUser) }) }) - describe('when nonce exists', async () => { - let RoleAssignment: RoleAssignment + describe('grantRole', async () => { + let GrantRoleData: GrantRoleData beforeEach(async () => { - RoleAssignment = await buildRoleAssignment({ - tokenAddress: MockToken.address, - grantor: grantor.address, + await assertCreateRecordEvent(SftRolesRegistry, MockToken, grantor, 1) + GrantRoleData = await buildGrantRole({ + recordId: 1, + grantee: grantee.address, }) - await MockToken.mint(grantor.address, RoleAssignment.tokenId, RoleAssignment.tokenAmount) - await MockToken.connect(grantor).setApprovalForAll(SftRolesRegistry.address, true) - await expect(SftRolesRegistry.connect(grantor).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, - ) - }) - - it('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, - ) + it('should revert when sender is not grantor or approved', async () => { await expect( - SftRolesRegistry.connect(grantor).grantRoleFrom({ - ...RoleAssignment, - grantor: anotherUser.address, - }), - ).to.be.revertedWith('SftRolesRegistry: nonce exist, but data mismatch') + SftRolesRegistry.connect(anotherUser).grantRole( + GrantRoleData.recordId, + GrantRoleData.role, + GrantRoleData.grantee, + GrantRoleData.expirationDate, + GrantRoleData.revocable, + GrantRoleData.data, + ), + ).to.be.revertedWith('SftRolesRegistry: account not approved') }) - it('should revert if nonce is not expired', async () => { - const revocableRoleAssignment = await buildRoleAssignment({ - tokenAddress: MockToken.address, - grantor: grantor.address, - revocable: false, - }) - - await MockToken.mint(grantor.address, revocableRoleAssignment.tokenId, revocableRoleAssignment.tokenAmount) - await expect(SftRolesRegistry.connect(grantor).grantRoleFrom(revocableRoleAssignment)) - .to.emit(SftRolesRegistry, 'RoleGranted') - .withArgs( - revocableRoleAssignment.nonce, - revocableRoleAssignment.role, - revocableRoleAssignment.tokenAddress, - revocableRoleAssignment.tokenId, - revocableRoleAssignment.tokenAmount, - revocableRoleAssignment.grantor, - revocableRoleAssignment.grantee, - revocableRoleAssignment.expirationDate, - revocableRoleAssignment.revocable, - revocableRoleAssignment.data, - ) - - await expect(SftRolesRegistry.connect(grantor).grantRoleFrom(revocableRoleAssignment)).to.be.revertedWith( - 'SftRolesRegistry: nonce is not expired or is not revocable', - ) + it('should revert when expirationDate is zero', async () => { + await expect( + SftRolesRegistry.connect(grantor).grantRole( + GrantRoleData.recordId, + GrantRoleData.role, + GrantRoleData.grantee, + await time.latest(), + GrantRoleData.revocable, + GrantRoleData.data, + ), + ).to.be.revertedWith('SftRolesRegistry: expiration date must be in the future') }) - it('should NOT revert if nonce is expired', async () => { - const roleAssignment = await buildRoleAssignment({ - tokenAddress: MockToken.address, - grantor: grantor.address, - revocable: false, - }) - - await MockToken.mint(grantor.address, roleAssignment.tokenId, roleAssignment.tokenAmount) - await expect(SftRolesRegistry.connect(grantor).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, - ) - - // increase time in 1 day - await time.increase(ONE_DAY + 1) - roleAssignment.expirationDate = (await time.latest()) + ONE_DAY - - await expect(SftRolesRegistry.connect(grantor).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, - ) - }) - - it("should revert if grantor's balance is insufficient", async () => { + it('should revert when role is not revocable and is not expired', async () => { + await assertGrantRoleEvent(SftRolesRegistry, grantor, GrantRoleData.recordId, GrantRoleData.grantee, false) await expect( - SftRolesRegistry.connect(grantor).grantRoleFrom({ - ...RoleAssignment, - tokenAmount: RoleAssignment.tokenAmount + 1, - }), - ).to.be.revertedWith('ERC1155: insufficient balance for transfer') + SftRolesRegistry.connect(grantor).grantRole( + GrantRoleData.recordId, + GrantRoleData.role, + GrantRoleData.grantee, + GrantRoleData.expirationDate, + GrantRoleData.revocable, + GrantRoleData.data, + ), + ).to.be.revertedWith('SftRolesRegistry: role is not expired and is not revocable') }) - it('should grant role if tokens deposited are greater than requested', async () => { - const newTokenAmount = RoleAssignment.tokenAmount - 1 - await expect( - SftRolesRegistry.connect(grantor).grantRoleFrom({ - ...RoleAssignment, - tokenAmount: newTokenAmount, - }), - ) - .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, //operator - SftRolesRegistry.address, //from - RoleAssignment.grantor, //to - RoleAssignment.tokenId, - RoleAssignment.tokenAmount - newTokenAmount, - ) - }) - - it('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 grant role when sender is grantor', async () => { + await assertGrantRoleEvent(SftRolesRegistry, grantor, GrantRoleData.recordId, GrantRoleData.grantee) + }) - await expect( - SftRolesRegistry.connect(grantor).grantRoleFrom({ - ...RoleAssignment, - tokenAmount: newTokenAmount, - }), + it('should grant role when sender is approved', async () => { + await assertGrantRoleEvent( + SftRolesRegistry, + grantor, + GrantRoleData.recordId, + GrantRoleData.grantee, + true, + anotherUser, ) - .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, - ) - }) - - it('should grant role if tokens deposited are equal to tokens requested', async () => { - await expect(SftRolesRegistry.connect(grantor).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, - ) - // should not transfer any tokens - .to.not.emit(MockToken, 'TransferSingle') }) }) describe('revokeRole', async () => { - let RoleAssignment: RoleAssignment - let RevokeRoleData: RevokeRoleData + let RecordCreated: Record + let GrantRoleData: GrantRoleData beforeEach(async () => { - RoleAssignment = await buildRoleAssignment({ - tokenAddress: MockToken.address, - grantor: grantor.address, - grantee: grantee.address, - }) - RevokeRoleData = buildRevokeRoleData(RoleAssignment) - await MockToken.connect(grantor).setApprovalForAll(SftRolesRegistry.address, true) - await MockToken.mint(grantor.address, RoleAssignment.tokenId, RoleAssignment.tokenAmount) - await expect(SftRolesRegistry.connect(grantor).grantRoleFrom(RoleAssignment)).to.not.be.reverted + RecordCreated = await assertCreateRecordEvent(SftRolesRegistry, MockToken, grantor, 1) + GrantRoleData = await assertGrantRoleEvent(SftRolesRegistry, grantor, 1, grantee.address) }) - it('should revert when hash is invalid', async () => { - // hash validates nonce, role, tokenAddress, tokenId and revoker - - // nonce + it('should revert when sender is not grantor or approved', async () => { await expect( - SftRolesRegistry.connect(grantor).revokeRoleFrom({ - ...RevokeRoleData, - nonce: generateRandomInt(), - }), - ).to.be.revertedWith('SftRolesRegistry: could not find role assignment') + SftRolesRegistry.connect(anotherUser).revokeRoleFrom( + GrantRoleData.recordId, + GrantRoleData.role, + GrantRoleData.grantee, + ), + ).to.be.revertedWith('SftRolesRegistry: sender must be approved') + }) - // role + it('should revert when role assignment does not exist', async () => { + // different recordId await expect( - SftRolesRegistry.connect(grantor).revokeRoleFrom({ - ...RevokeRoleData, - role: solidityKeccak256(['string'], ['Role(uint256 newArg)']), - }), + SftRolesRegistry.connect(grantor).revokeRoleFrom( + GrantRoleData.recordId + 1, + GrantRoleData.role, + GrantRoleData.grantee, + ), ).to.be.revertedWith('SftRolesRegistry: could not find role assignment') - // tokenAddress + // different role await expect( - SftRolesRegistry.connect(grantor).revokeRoleFrom({ - ...RevokeRoleData, - tokenAddress: AddressZero, - }), + SftRolesRegistry.connect(grantor).revokeRoleFrom( + GrantRoleData.recordId, + generateRoleId('ANOTHER_ROLE'), + GrantRoleData.grantee, + ), ).to.be.revertedWith('SftRolesRegistry: could not find role assignment') - // tokenId + // different grantee await expect( - SftRolesRegistry.connect(grantor).revokeRoleFrom({ - ...RevokeRoleData, - tokenId: generateRandomInt(), - }), + SftRolesRegistry.connect(grantor).revokeRoleFrom( + GrantRoleData.recordId + 1, + GrantRoleData.role, + anotherUser.address, + ), ).to.be.revertedWith('SftRolesRegistry: could not find role assignment') + }) - // revoker + it('should revert when role assignment is not expired nor revocable', async () => { + const roleAssignment = await assertGrantRoleEvent( + SftRolesRegistry, + grantor, + GrantRoleData.recordId, + grantee.address, + false, + ) await expect( - SftRolesRegistry.connect(grantor).revokeRoleFrom({ - ...RevokeRoleData, - grantor: anotherUser.address, - }), - ).to.be.revertedWith('SftRolesRegistry: could not find role assignment') + SftRolesRegistry.connect(grantor).revokeRoleFrom( + roleAssignment.recordId, + GrantRoleData.role, + GrantRoleData.grantee, + ), + ).to.be.revertedWith('SftRolesRegistry: role is not revocable or caller is not the approved') }) - it('should revert if nonce is not expired and is not revocable', async () => { - const newRoleAssignment = await buildRoleAssignment({ - tokenAddress: MockToken.address, - grantor: grantor.address, - revocable: false, - }) - - const newRevokeRoleData = buildRevokeRoleData(newRoleAssignment) - await MockToken.mint(newRoleAssignment.grantor, newRoleAssignment.tokenId, newRoleAssignment.tokenAmount) - await expect(SftRolesRegistry.connect(grantor).grantRoleFrom(newRoleAssignment)) - - await expect(SftRolesRegistry.connect(grantor).revokeRoleFrom(newRevokeRoleData)).to.be.revertedWith( - 'SftRolesRegistry: role is not revocable or caller is not the approved', + it('should revoke when role is expired', async () => { + const roleAssignment = await assertGrantRoleEvent( + SftRolesRegistry, + grantor, + GrantRoleData.recordId, + grantee.address, + false, ) + await time.increase(ONE_DAY) + await assertRevokeRoleEvent(SftRolesRegistry, grantor, roleAssignment.recordId, roleAssignment.role, grantee) }) - it('should NOT revert if nonce is not expired and is not revocable, but the caller is the grantee', async () => { - const newRoleAssignment = await buildRoleAssignment({ - tokenAddress: MockToken.address, - grantor: grantor.address, - grantee: grantee.address, - revocable: false, - }) - const newRevokeRoleData = buildRevokeRoleData(newRoleAssignment) - await MockToken.mint(newRoleAssignment.grantor, newRoleAssignment.tokenId, newRoleAssignment.tokenAmount) - await expect(SftRolesRegistry.connect(grantor).grantRoleFrom(newRoleAssignment)) + it('should revoke when role is revocable', async () => { + await assertRevokeRoleEvent(SftRolesRegistry, grantor, GrantRoleData.recordId, GrantRoleData.role, grantee) + }) - await expect(SftRolesRegistry.connect(grantee).revokeRoleFrom(newRevokeRoleData)) - .to.emit(SftRolesRegistry, 'RoleRevoked') - .withArgs( - newRevokeRoleData.nonce, - newRevokeRoleData.role, - newRevokeRoleData.tokenAddress, - newRevokeRoleData.tokenId, - newRoleAssignment.tokenAmount, - newRevokeRoleData.grantor, - newRoleAssignment.grantee, - ) - }) - - it('should revert if caller is not approved', async () => { - await expect(SftRolesRegistry.connect(anotherUser).revokeRoleFrom(RevokeRoleData)).to.be.revertedWith( - 'SftRolesRegistry: sender must be approved', + it('should revoke when sender is grantee', async () => { + const roleAssignment = await assertGrantRoleEvent( + SftRolesRegistry, + grantor, + GrantRoleData.recordId, + grantee.address, + false, + ) + await assertRevokeRoleEvent( + SftRolesRegistry, + grantor, + roleAssignment.recordId, + roleAssignment.role, + grantee, + grantee, ) }) - it('should revoke role if sender is revoker', async () => { - await expect(SftRolesRegistry.connect(grantor).revokeRoleFrom(RevokeRoleData)) - .to.emit(SftRolesRegistry, 'RoleRevoked') - .withArgs( - RevokeRoleData.nonce, - RevokeRoleData.role, - RevokeRoleData.tokenAddress, - RevokeRoleData.tokenId, - RoleAssignment.tokenAmount, - RevokeRoleData.grantor, - RevokeRoleData.grantee, - ) - // transfer tokens back to owner - .to.emit(MockToken, 'TransferSingle') - .withArgs( - SftRolesRegistry.address, - SftRolesRegistry.address, - RevokeRoleData.grantor, - RevokeRoleData.tokenId, - RoleAssignment.tokenAmount, - ) - }) - - it('should revoke role if sender is approved by grantor', async () => { - await SftRolesRegistry.connect(grantor).setRoleApprovalForAll( - RoleAssignment.tokenAddress, + it('should revoke when sender is approved by grantee', async () => { + await assertGrantRoleEvent(SftRolesRegistry, grantor, GrantRoleData.recordId, grantee.address, false) + await SftRolesRegistry.connect(grantee).setRoleApprovalForAll( + RecordCreated.tokenAddress, anotherUser.address, true, ) - await expect(SftRolesRegistry.connect(anotherUser).revokeRoleFrom(RevokeRoleData)) + + await expect( + SftRolesRegistry.connect(anotherUser).revokeRoleFrom( + GrantRoleData.recordId, + GrantRoleData.role, + grantee.address, + ), + ) .to.emit(SftRolesRegistry, 'RoleRevoked') - .withArgs( - RevokeRoleData.nonce, - RevokeRoleData.role, - RevokeRoleData.tokenAddress, - RevokeRoleData.tokenId, - RoleAssignment.tokenAmount, - RevokeRoleData.grantor, - RevokeRoleData.grantee, - ) - // transfer tokens back to owner - .to.emit(MockToken, 'TransferSingle') - .withArgs( - SftRolesRegistry.address, - SftRolesRegistry.address, - RevokeRoleData.grantor, - RevokeRoleData.tokenId, - RoleAssignment.tokenAmount, - ) - }) - it('should revoke role if sender is approved by grantee', async () => { - await SftRolesRegistry.connect(grantee).setRoleApprovalForAll( - RoleAssignment.tokenAddress, + .withArgs(GrantRoleData.recordId, GrantRoleData.role, grantee.address) + }) + + it('should revoke when sender is approved by grantor', async () => { + await SftRolesRegistry.connect(grantor).setRoleApprovalForAll( + RecordCreated.tokenAddress, anotherUser.address, true, ) - await expect(SftRolesRegistry.connect(anotherUser).revokeRoleFrom(RevokeRoleData)) - .to.emit(SftRolesRegistry, 'RoleRevoked') - .withArgs( - RevokeRoleData.nonce, - RevokeRoleData.role, - RevokeRoleData.tokenAddress, - RevokeRoleData.tokenId, - RoleAssignment.tokenAmount, - RevokeRoleData.grantor, - RevokeRoleData.grantee, - ) - // transfer tokens back to owner - .to.emit(MockToken, 'TransferSingle') - .withArgs( - SftRolesRegistry.address, - SftRolesRegistry.address, - RevokeRoleData.grantor, - RevokeRoleData.tokenId, - RoleAssignment.tokenAmount, - ) - }) - - it('should revoke role if sender is grantee', async () => { - await expect(SftRolesRegistry.connect(grantee).revokeRoleFrom(RevokeRoleData)) + await expect( + SftRolesRegistry.connect(anotherUser).revokeRoleFrom( + GrantRoleData.recordId, + GrantRoleData.role, + grantee.address, + ), + ) .to.emit(SftRolesRegistry, 'RoleRevoked') - .withArgs( - RevokeRoleData.nonce, - RevokeRoleData.role, - RevokeRoleData.tokenAddress, - RevokeRoleData.tokenId, - RoleAssignment.tokenAmount, - RevokeRoleData.grantor, - RoleAssignment.grantee, - ) - // transfer tokens back to owner - .to.emit(MockToken, 'TransferSingle') - .withArgs( - SftRolesRegistry.address, - SftRolesRegistry.address, - RevokeRoleData.grantor, - RevokeRoleData.tokenId, - RoleAssignment.tokenAmount, - ) + .withArgs(GrantRoleData.recordId, GrantRoleData.role, grantee.address) + }) + }) + + describe('withdrawFrom', async () => { + let GrantRoleData: GrantRoleData + + beforeEach(async () => { + await assertCreateRecordEvent(SftRolesRegistry, MockToken, grantor, 1) + GrantRoleData = await assertGrantRoleEvent(SftRolesRegistry, grantor, 1, grantee.address) + }) + + it('should revert when sender is not grantor or approved', async () => { + await expect(SftRolesRegistry.connect(anotherUser).withdrawFrom(GrantRoleData.recordId)).to.be.revertedWith( + 'SftRolesRegistry: account not approved', + ) + }) + + it('should revert when there is an active role', async () => { + await assertGrantRoleEvent(SftRolesRegistry, grantor, 1, grantee.address, false) + await expect(SftRolesRegistry.connect(grantor).withdrawFrom(GrantRoleData.recordId)).to.be.revertedWith( + 'SftRolesRegistry: role is not expired and is not revocable', + ) + }) + + it('should withdraw when there are revocable roles', async () => { + await expect(SftRolesRegistry.connect(grantor).withdrawFrom(GrantRoleData.recordId)) + .to.emit(SftRolesRegistry, 'Withdrew') + .withArgs(GrantRoleData.recordId) + }) + + it('should withdraw when there are no roles', async () => { + await assertRevokeRoleEvent(SftRolesRegistry, grantor, GrantRoleData.recordId, GrantRoleData.role, grantee) + await expect(SftRolesRegistry.connect(grantor).withdrawFrom(GrantRoleData.recordId)) + .to.emit(SftRolesRegistry, 'Withdrew') + .withArgs(GrantRoleData.recordId) + }) + + it('should withdraw when there are expired roles', async () => { + await assertGrantRoleEvent(SftRolesRegistry, grantor, 1, grantee.address, false) + await time.increase(ONE_DAY) + await expect(SftRolesRegistry.connect(grantor).withdrawFrom(GrantRoleData.recordId)) + .to.emit(SftRolesRegistry, 'Withdrew') + .withArgs(GrantRoleData.recordId) }) }) describe('setRoleApprovalForAll', async () => { - it('should approve and revoke approval', async () => { + it('should approve and revoke role approval for all', async () => { expect(await SftRolesRegistry.isRoleApprovedForAll(AddressZero, grantor.address, anotherUser.address)).to.be.false expect(await SftRolesRegistry.connect(grantor).setRoleApprovalForAll(AddressZero, anotherUser.address, true)) .to.emit(SftRolesRegistry, 'RoleApprovalForAll') @@ -703,121 +382,47 @@ describe('SftRolesRegistry', async () => { }) describe('View Functions', async () => { - let RoleAssignment: RoleAssignment + let RecordCreated: Record + let GrantRoleData: GrantRoleData beforeEach(async () => { - RoleAssignment = await buildRoleAssignment({ - tokenAddress: MockToken.address, - grantor: grantor.address, - grantee: grantee.address, - }) - await MockToken.connect(grantor).setApprovalForAll(SftRolesRegistry.address, true) - await MockToken.mint(grantor.address, RoleAssignment.tokenId, RoleAssignment.tokenAmount) - await expect(SftRolesRegistry.connect(grantor).grantRoleFrom(RoleAssignment)).to.not.be.reverted - }) - - it('should return the role data', async () => { - const roleData = await SftRolesRegistry.roleData(RoleAssignment.nonce) - 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) + RecordCreated = await assertCreateRecordEvent(SftRolesRegistry, MockToken, grantor, 1) + GrantRoleData = await assertGrantRoleEvent(SftRolesRegistry, grantor, 1, grantee.address) }) - it('should return the expiration date', async () => { - expect(await SftRolesRegistry.roleExpirationDate(RoleAssignment.nonce)).to.be.equal(RoleAssignment.expirationDate) + it('recordInfo', async () => { + const record = await SftRolesRegistry.recordInfo(GrantRoleData.recordId) + expect(record.grantor_).to.be.equal(RecordCreated.grantor) + expect(record.tokenAddress_).to.be.equal(RecordCreated.tokenAddress) + expect(record.tokenId_).to.be.equal(RecordCreated.tokenId) + expect(record.tokenAmount_).to.be.equal(RecordCreated.tokenAmount) }) - it('should return balance zero if grantee has no roles', async () => { - expect( - await SftRolesRegistry.roleBalanceOf( - RoleAssignment.role, - RoleAssignment.tokenAddress, - RoleAssignment.tokenId, - AddressZero, - ), - ).to.be.equal(0) + it('roleData', async () => { + const roleData = await SftRolesRegistry.roleData( + GrantRoleData.recordId, + GrantRoleData.role, + GrantRoleData.grantee, + ) + expect(roleData).to.be.equal(GrantRoleData.data) }) - it("should return the grantee's balance of tokens", async () => { - expect( - await SftRolesRegistry.roleBalanceOf( - RoleAssignment.role, - RoleAssignment.tokenAddress, - RoleAssignment.tokenId, - RoleAssignment.grantee, - ), - ).to.be.equal(RoleAssignment.tokenAmount) - }) - it("should return the grantee's balance zero of tokens if grants are expired", async () => { - await time.increase(ONE_DAY + 1) - expect( - await SftRolesRegistry.roleBalanceOf( - RoleAssignment.role, - RoleAssignment.tokenAddress, - RoleAssignment.tokenId, - RoleAssignment.grantee, - ), - ).to.be.equal(0) + it('roleExpirationDate', async () => { + const roleExpirationDate = await SftRolesRegistry.roleExpirationDate( + GrantRoleData.recordId, + GrantRoleData.role, + GrantRoleData.grantee, + ) + expect(roleExpirationDate).to.be.equal(GrantRoleData.expirationDate) }) - }) - - describe('RoleBalanceOf @skip-on-coverage', async () => { - it('should check at least 4300 grant roles without run out of gas', async function () { - const tokenId = generateRandomInt() - const role = generateRoleId('Role()') - const expirationDate = (await time.latest()) + ONE_DAY - await MockToken.connect(grantor).setApprovalForAll(SftRolesRegistry.address, true) - - let totalAmount = 0 - const times = new Array(4300).fill(0) - const roleAssignments = times.map((t, i) => { - const newRoleAssignment = { - nonce: i + 1, - role, - tokenAddress: MockToken.address, - tokenId, - tokenAmount: generateRandomInt(), - grantor: grantor.address, - grantee: grantee.address, - expirationDate, - revocable: false, - data: '0x', - } - totalAmount += newRoleAssignment.tokenAmount - - return newRoleAssignment - }) - - await MockToken.mint(grantor.address, tokenId, totalAmount * 2) - - const promises = roleAssignments.map(t => SftRolesRegistry.connect(grantor).grantRoleFrom(t)) - await Promise.all(promises) - - expect(await SftRolesRegistry.roleBalanceOf(role, MockToken.address, tokenId, grantee.address)).to.be.equal( - totalAmount, + it('isRoleRevocable', async () => { + const isRoleRevocable = await SftRolesRegistry.isRoleRevocable( + GrantRoleData.recordId, + GrantRoleData.role, + GrantRoleData.grantee, ) - - const revokePromises = roleAssignments.map(async t => { - const revokeRoleData = buildRevokeRoleData(t) - return SftRolesRegistry.connect(grantee).revokeRoleFrom(revokeRoleData) - }) - - await Promise.all(revokePromises) - - expect(await SftRolesRegistry.roleBalanceOf(role, MockToken.address, tokenId, grantee.address)).to.be.equal(0) + expect(isRoleRevocable).to.be.true }) }) @@ -827,7 +432,7 @@ describe('SftRolesRegistry', async () => { }) it('should return true if SftRolesRegistry interface id (0x89cb6ab6)', async () => { - expect(await SftRolesRegistry.supportsInterface('0x89cb6ab6')).to.be.true + expect(await SftRolesRegistry.supportsInterface('0xf254051c')).to.be.true }) }) }) diff --git a/test/SftRolesRegistry/SftRolesRegistrySingleRole.spec.ts b/test/SftRolesRegistry/SftRolesRegistrySingleRole.spec.ts index a090596..292647a 100644 --- a/test/SftRolesRegistry/SftRolesRegistrySingleRole.spec.ts +++ b/test/SftRolesRegistry/SftRolesRegistrySingleRole.spec.ts @@ -4,9 +4,10 @@ import { beforeEach } from 'mocha' import { expect } from 'chai' import { loadFixture, time } from '@nomicfoundation/hardhat-network-helpers' import { SignerWithAddress } from '@nomiclabs/hardhat-ethers/signers' -import { generateRoleId, buildRecord, buildGrantRole } from './helpers' +import { generateRoleId, buildRecord, buildGrantRole } from './helpers/mockData' import { GrantRoleData, Record } from './types' import { generateRandomInt } from '../helpers' +import { assertCreateRecordEvent, assertGrantRoleEvent } from './helpers/assertEvents' describe('SftRolesRegistrySingleRole', async () => { let SftRolesRegistry: Contract @@ -532,34 +533,16 @@ describe('SftRolesRegistrySingleRole', async () => { let GrantRoleData: GrantRoleData beforeEach(async () => { - RecordCreated = buildRecord({ - grantor: grantor.address, - tokenAddress: MockToken.address, - }) - GrantRoleData = await buildGrantRole({ - recordId: 1, - grantee: grantee.address, - }) - await MockToken.mint(grantor.address, RecordCreated.tokenId, RecordCreated.tokenAmount) - await MockToken.connect(grantor).setApprovalForAll(SftRolesRegistry.address, true) - await expect( - SftRolesRegistry.connect(grantor).createRecordFrom( - RecordCreated.grantor, - RecordCreated.tokenAddress, - RecordCreated.tokenId, - RecordCreated.tokenAmount, - ), - ).to.not.be.reverted - await expect( - SftRolesRegistry.connect(grantor).grantRole( - GrantRoleData.recordId, - GrantRoleData.role, - GrantRoleData.grantee, - GrantRoleData.expirationDate, - GrantRoleData.revocable, - GrantRoleData.data, - ), - ).to.not.be.reverted + RecordCreated = await assertCreateRecordEvent(SftRolesRegistry, MockToken, grantor, 1) + GrantRoleData = await assertGrantRoleEvent(SftRolesRegistry, grantor, 1, grantee.address) + }) + + it('recordInfo', async () => { + const record = await SftRolesRegistry.recordInfo(GrantRoleData.recordId) + expect(record.grantor_).to.be.equal(RecordCreated.grantor) + expect(record.tokenAddress_).to.be.equal(RecordCreated.tokenAddress) + expect(record.tokenId_).to.be.equal(RecordCreated.tokenId) + expect(record.tokenAmount_).to.be.equal(RecordCreated.tokenAmount) }) it('should revert when grantee is not the same', async () => { @@ -617,7 +600,7 @@ describe('SftRolesRegistrySingleRole', async () => { }) it('should return true if IERCXXXX interface id', async () => { - expect(await SftRolesRegistry.supportsInterface('0x42ba720c')).to.be.true + expect(await SftRolesRegistry.supportsInterface('0xf254051c')).to.be.true }) }) }) diff --git a/test/SftRolesRegistry/helpers/assertEvents.ts b/test/SftRolesRegistry/helpers/assertEvents.ts new file mode 100644 index 0000000..e494e38 --- /dev/null +++ b/test/SftRolesRegistry/helpers/assertEvents.ts @@ -0,0 +1,94 @@ +import { buildGrantRole, buildRecord } from './mockData' +import { expect } from 'chai' +import { Contract } from 'ethers' +import { SignerWithAddress } from '@nomiclabs/hardhat-ethers/signers' + +export async function assertCreateRecordEvent( + SftRolesRegistry: Contract, + MockToken: Contract, + grantor: SignerWithAddress, + expectedRecordId: number, + anotherUser?: SignerWithAddress, +) { + const record = buildRecord({ + grantor: grantor.address, + tokenAddress: MockToken.address, + }) + await MockToken.mint(grantor.address, record.tokenId, record.tokenAmount) + await MockToken.connect(grantor).setApprovalForAll(SftRolesRegistry.address, true) + + if (anotherUser) { + await SftRolesRegistry.connect(grantor).setRoleApprovalForAll(record.tokenAddress, anotherUser.address, true) + } + + await expect( + SftRolesRegistry.connect(anotherUser || grantor).createRecordFrom( + record.grantor, + record.tokenAddress, + record.tokenId, + record.tokenAmount, + ), + ) + .to.emit(SftRolesRegistry, 'RecordCreated') + .withArgs(record.grantor, expectedRecordId, record.tokenAddress, record.tokenId, record.tokenAmount) + .to.emit(MockToken, 'TransferSingle') + .withArgs(SftRolesRegistry.address, grantor.address, SftRolesRegistry.address, record.tokenId, record.tokenAmount) + return { ...record, recordId: expectedRecordId } +} + +export async function assertGrantRoleEvent( + SftRolesRegistry: Contract, + grantor: SignerWithAddress, + recordId: number, + grantee: string, + revocable = true, + anotherUser?: SignerWithAddress, +) { + const grantRoleData = await buildGrantRole({ + recordId, + grantee, + revocable, + }) + if (anotherUser) { + const record = await SftRolesRegistry.recordInfo(recordId) + await SftRolesRegistry.connect(grantor).setRoleApprovalForAll(record.tokenAddress_, anotherUser.address, true) + } + await expect( + SftRolesRegistry.connect(anotherUser || grantor).grantRole( + recordId, + grantRoleData.role, + grantee, + grantRoleData.expirationDate, + grantRoleData.revocable, + grantRoleData.data, + ), + ) + .to.emit(SftRolesRegistry, 'RoleGranted') + .withArgs( + recordId, + grantRoleData.role, + grantee, + grantRoleData.expirationDate, + grantRoleData.revocable, + grantRoleData.data, + ) + return grantRoleData +} + +export async function assertRevokeRoleEvent( + SftRolesRegistry: Contract, + grantor: SignerWithAddress, + recordId: number, + role: string, + grantee: SignerWithAddress, + revoker?: SignerWithAddress, +) { + if (revoker) { + const record = await SftRolesRegistry.recordInfo(recordId) + await SftRolesRegistry.connect(grantor).setRoleApprovalForAll(record.tokenAddress_, revoker.address, true) + } + await expect(SftRolesRegistry.connect(revoker || grantor).revokeRoleFrom(recordId, role, grantee.address)) + .to.emit(SftRolesRegistry, 'RoleRevoked') + .withArgs(recordId, role, grantee.address) + return { recordId, role, grantee: grantee.address } +} diff --git a/test/SftRolesRegistry/helpers.ts b/test/SftRolesRegistry/helpers/mockData.ts similarity index 97% rename from test/SftRolesRegistry/helpers.ts rename to test/SftRolesRegistry/helpers/mockData.ts index 04dd927..6330e23 100644 --- a/test/SftRolesRegistry/helpers.ts +++ b/test/SftRolesRegistry/helpers/mockData.ts @@ -1,6 +1,6 @@ import { solidityKeccak256 } from 'ethers/lib/utils' -import { GrantRoleData, Record, RevokeRoleData, RoleAssignment } from './types' -import { generateRandomInt } from '../helpers' +import { GrantRoleData, Record, RevokeRoleData, RoleAssignment } from '../types' +import { generateRandomInt } from '../../helpers' import { ethers } from 'hardhat' import { time } from '@nomicfoundation/hardhat-network-helpers' From a7f051f56c3cd7e83dee8c63ac378a5d027aa147 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ernani=20S=C3=A3o=20Thiago?= Date: Tue, 26 Dec 2023 16:53:20 -0500 Subject: [PATCH 04/19] renamed createRecordFrom and RecordCreated to commitTokens and CommittedTokens respectively --- contracts/RolesRegistry/SftRolesRegistry.sol | 150 +++++---- .../SftRolesRegistrySingleRole.sol | 110 ++++--- .../interfaces/ISftRolesRegistry.sol | 73 ++--- .../RolesRegistry/libraries/LinkedLists.sol | 4 +- .../SftRolesRegistry/SftRolesRegistry.spec.ts | 191 +++++------ .../SftRolesRegistrySingleRole.spec.ts | 301 +++++++++--------- test/SftRolesRegistry/helpers/assertEvents.ts | 66 ++-- test/SftRolesRegistry/helpers/mockData.ts | 33 +- test/SftRolesRegistry/types.ts | 4 +- 9 files changed, 490 insertions(+), 442 deletions(-) diff --git a/contracts/RolesRegistry/SftRolesRegistry.sol b/contracts/RolesRegistry/SftRolesRegistry.sol index 1e34041..14c35c0 100644 --- a/contracts/RolesRegistry/SftRolesRegistry.sol +++ b/contracts/RolesRegistry/SftRolesRegistry.sol @@ -18,17 +18,17 @@ contract SftRolesRegistry is ISftRolesRegistry, ERC1155Holder { using EnumerableSet for EnumerableSet.UintSet; using EnumerableSet for EnumerableSet.Bytes32Set; - uint256 public recordCount; + uint256 public commitmentCount; LinkedLists.Lists internal lists; - // recordId => Record - mapping(uint256 => Record) public records; + // commitmentId => Commitment + mapping(uint256 => Commitment) public commitments; - // recordId => role => lastGrantee + // commitmentId => role => lastGrantee mapping(uint256 => mapping(bytes32 => address)) internal lastGrantee; - // recordId => role[] - mapping(uint256 => EnumerableSet.Bytes32Set) internal recordIdToRoles; + // commitmentId => role[] + mapping(uint256 => EnumerableSet.Bytes32Set) internal commitmentIdToRoles; // grantor => tokenAddress => operator => isApproved mapping(address => mapping(address => mapping(address => bool))) public roleApprovals; @@ -43,59 +43,63 @@ contract SftRolesRegistry is ISftRolesRegistry, ERC1155Holder { /** External Functions **/ - function createRecordFrom( + function commitTokens( address _grantor, address _tokenAddress, uint256 _tokenId, uint256 _tokenAmount - ) external override onlyOwnerOrApproved(_grantor, _tokenAddress) returns (uint256 recordId_) { + ) external override onlyOwnerOrApproved(_grantor, _tokenAddress) returns (uint256 commitmentId_) { require(_tokenAmount > 0, 'SftRolesRegistry: tokenAmount must be greater than zero'); - recordId_ = _createRecord(_grantor, _tokenAddress, _tokenId, _tokenAmount); + commitmentId_ = _createCommitment(_grantor, _tokenAddress, _tokenId, _tokenAmount); } function grantRole( - uint256 _recordId, + uint256 _commitmentId, bytes32 _role, address _grantee, uint64 _expirationDate, bool _revocable, bytes calldata _data - ) external override onlyOwnerOrApproved(records[_recordId].grantor, records[_recordId].tokenAddress) { + ) + external + override + onlyOwnerOrApproved(commitments[_commitmentId].grantor, commitments[_commitmentId].tokenAddress) + { require(_expirationDate > block.timestamp, 'SftRolesRegistry: expiration date must be in the future'); - _grantOrUpdateRole(_recordId, _role, _grantee, _expirationDate, _revocable, _data); + _grantOrUpdateRole(_commitmentId, _role, _grantee, _expirationDate, _revocable, _data); } - function revokeRoleFrom(uint256 _recordId, bytes32 _role, address _grantee) external override { - uint256 itemId = _getItemId(_recordId, _role, _grantee); + function revokeRoleFrom(uint256 _commitmentId, bytes32 _role, address _grantee) external override { + uint256 itemId = _getItemId(_commitmentId, _role, _grantee); LinkedLists.RoleData storage data = lists.items[itemId].data; require(data.expirationDate > 0, 'SftRolesRegistry: could not find role assignment'); - Record storage record = records[_recordId]; - address caller = _findCaller(record.grantor, _grantee, record.tokenAddress); + Commitment storage commitment = commitments[_commitmentId]; + address caller = _findCaller(commitment.grantor, _grantee, commitment.tokenAddress); if (data.expirationDate > block.timestamp && !data.revocable) { // if role is not expired and is not revocable, only the grantee can revoke it require(caller == _grantee, 'SftRolesRegistry: role is not revocable or caller is not the approved'); } // remove from the list - bytes32 headKey = _getHeadKey(_grantee, _role, record.tokenAddress, record.tokenId); + bytes32 headKey = _getHeadKey(_grantee, _role, commitment.tokenAddress, commitment.tokenId); lists.remove(headKey, itemId); - // remove from recordIdToRoles - recordIdToRoles[_recordId].remove(_role); - delete lastGrantee[_recordId][_role]; + // remove from commitmentIdToRoles + commitmentIdToRoles[_commitmentId].remove(_role); + delete lastGrantee[_commitmentId][_role]; - emit RoleRevoked(_recordId, _role, _grantee); + emit RoleRevoked(_commitmentId, _role, _grantee); } function withdrawFrom( - uint256 _recordId - ) external onlyOwnerOrApproved(records[_recordId].grantor, records[_recordId].tokenAddress) { - uint256 numberOfRoles = recordIdToRoles[_recordId].length(); + uint256 _commitmentId + ) external onlyOwnerOrApproved(commitments[_commitmentId].grantor, commitments[_commitmentId].tokenAddress) { + uint256 numberOfRoles = commitmentIdToRoles[_commitmentId].length(); for (uint256 i = 0; i < numberOfRoles; i++) { - bytes32 role = recordIdToRoles[_recordId].at(i); - address grantee = lastGrantee[_recordId][role]; - uint256 itemId = _getItemId(_recordId, role, grantee); + bytes32 role = commitmentIdToRoles[_commitmentId].at(i); + address grantee = lastGrantee[_commitmentId][role]; + uint256 itemId = _getItemId(_commitmentId, role, grantee); LinkedLists.RoleData storage data = lists.items[itemId].data; require( @@ -104,22 +108,27 @@ contract SftRolesRegistry is ISftRolesRegistry, ERC1155Holder { ); // remove from list and storage - bytes32 headKey = _getHeadKey(grantee, role, records[_recordId].tokenAddress, records[_recordId].tokenId); + bytes32 headKey = _getHeadKey( + grantee, + role, + commitments[_commitmentId].tokenAddress, + commitments[_commitmentId].tokenId + ); lists.remove(headKey, itemId); - recordIdToRoles[_recordId].remove(role); - delete lastGrantee[_recordId][role]; + commitmentIdToRoles[_commitmentId].remove(role); + delete lastGrantee[_commitmentId][role]; } _transferFrom( address(this), - records[_recordId].grantor, - records[_recordId].tokenAddress, - records[_recordId].tokenId, - records[_recordId].tokenAmount + commitments[_commitmentId].grantor, + commitments[_commitmentId].tokenAddress, + commitments[_commitmentId].tokenId, + commitments[_commitmentId].tokenAmount ); - delete records[_recordId]; - emit Withdrew(_recordId); + delete commitments[_commitmentId]; + emit Withdrew(_commitmentId); } function setRoleApprovalForAll(address _tokenAddress, address _operator, bool _isApproved) external override { @@ -129,34 +138,38 @@ contract SftRolesRegistry is ISftRolesRegistry, ERC1155Holder { /** View Functions **/ - function recordInfo( - uint256 _recordId + function commitmentInfo( + uint256 _commitmentId ) external view returns (address grantor_, address tokenAddress_, uint256 tokenId_, uint256 tokenAmount_) { - Record memory record = records[_recordId]; - grantor_ = record.grantor; - tokenAddress_ = record.tokenAddress; - tokenId_ = record.tokenId; - tokenAmount_ = record.tokenAmount; + Commitment memory commitment = commitments[_commitmentId]; + grantor_ = commitment.grantor; + tokenAddress_ = commitment.tokenAddress; + tokenId_ = commitment.tokenId; + tokenAmount_ = commitment.tokenAmount; } - function roleData(uint256 _recordId, bytes32 _role, address _grantee) external view returns (bytes memory data_) { - return lists.items[_getItemId(_recordId, _role, _grantee)].data.data; + function roleData( + uint256 _commitmentId, + bytes32 _role, + address _grantee + ) external view returns (bytes memory data_) { + return lists.items[_getItemId(_commitmentId, _role, _grantee)].data.data; } function roleExpirationDate( - uint256 _recordId, + uint256 _commitmentId, bytes32 _role, address _grantee ) external view returns (uint64 expirationDate_) { - return lists.items[_getItemId(_recordId, _role, _grantee)].data.expirationDate; + return lists.items[_getItemId(_commitmentId, _role, _grantee)].data.expirationDate; } function isRoleRevocable( - uint256 _recordId, + uint256 _commitmentId, bytes32 _role, address _grantee ) external view returns (bool revocable_) { - return lists.items[_getItemId(_recordId, _role, _grantee)].data.revocable; + return lists.items[_getItemId(_commitmentId, _role, _grantee)].data.revocable; } function isRoleApprovedForAll( @@ -199,20 +212,20 @@ contract SftRolesRegistry is ISftRolesRegistry, ERC1155Holder { /** Helper Functions **/ - function _createRecord( + function _createCommitment( address _grantor, address _tokenAddress, uint256 _tokenId, uint256 _tokenAmount - ) internal returns (uint256 recordId_) { - recordId_ = ++recordCount; - records[recordId_] = Record(_grantor, _tokenAddress, _tokenId, _tokenAmount); + ) internal returns (uint256 commitmentId_) { + commitmentId_ = ++commitmentCount; + commitments[commitmentId_] = Commitment(_grantor, _tokenAddress, _tokenId, _tokenAmount); _transferFrom(_grantor, address(this), _tokenAddress, _tokenId, _tokenAmount); - emit RecordCreated(_grantor, recordId_, _tokenAddress, _tokenId, _tokenAmount); + emit TokensCommitted(_grantor, commitmentId_, _tokenAddress, _tokenId, _tokenAmount); } function _grantOrUpdateRole( - uint256 _recordId, + uint256 _commitmentId, bytes32 _role, address _grantee, uint64 _expirationDate, @@ -220,9 +233,9 @@ contract SftRolesRegistry is ISftRolesRegistry, ERC1155Holder { bytes calldata _data ) internal { // verify if a role exist - address latestGrantee = lastGrantee[_recordId][_role]; + address latestGrantee = lastGrantee[_commitmentId][_role]; // if exist, make sure that is expired or revocable - uint256 itemId = _getItemId(_recordId, _role, latestGrantee); + uint256 itemId = _getItemId(_commitmentId, _role, latestGrantee); LinkedLists.RoleData storage lastRoleData = lists.items[itemId].data; require( lastRoleData.expirationDate < block.timestamp || lastRoleData.revocable, @@ -230,26 +243,31 @@ contract SftRolesRegistry is ISftRolesRegistry, ERC1155Holder { ); // insert in the list - _insert(_recordId, _role, _grantee, _expirationDate, _revocable, _data); + _insert(_commitmentId, _role, _grantee, _expirationDate, _revocable, _data); // store last grantee and role - recordIdToRoles[_recordId].add(_role); - lastGrantee[_recordId][_role] = _grantee; + commitmentIdToRoles[_commitmentId].add(_role); + lastGrantee[_commitmentId][_role] = _grantee; - emit RoleGranted(_recordId, _role, _grantee, _expirationDate, _revocable, _data); + emit RoleGranted(_commitmentId, _role, _grantee, _expirationDate, _revocable, _data); } function _insert( - uint256 _recordId, + uint256 _commitmentId, bytes32 _role, address _grantee, uint64 _expirationDate, bool _revocable, bytes calldata _data ) internal { - bytes32 headKey = _getHeadKey(_grantee, _role, records[_recordId].tokenAddress, records[_recordId].tokenId); + bytes32 headKey = _getHeadKey( + _grantee, + _role, + commitments[_commitmentId].tokenAddress, + commitments[_commitmentId].tokenId + ); LinkedLists.RoleData memory data = LinkedLists.RoleData(_expirationDate, _revocable, _data); - uint256 itemId = _getItemId(_recordId, _role, _grantee); + uint256 itemId = _getItemId(_commitmentId, _role, _grantee); lists.insert(headKey, itemId, data); } @@ -286,7 +304,7 @@ contract SftRolesRegistry is ISftRolesRegistry, ERC1155Holder { return keccak256(abi.encodePacked(_grantee, _role, _tokenAddress, _tokenId)); } - function _getItemId(uint256 _recordId, bytes32 _role, address _grantee) internal pure returns (uint256) { - return uint256(keccak256(abi.encodePacked(_recordId, _role, _grantee))); + function _getItemId(uint256 _commitmentId, bytes32 _role, address _grantee) internal pure returns (uint256) { + return uint256(keccak256(abi.encodePacked(_commitmentId, _role, _grantee))); } } diff --git a/contracts/RolesRegistry/SftRolesRegistrySingleRole.sol b/contracts/RolesRegistry/SftRolesRegistrySingleRole.sol index 33d03b1..e5f57a8 100644 --- a/contracts/RolesRegistry/SftRolesRegistrySingleRole.sol +++ b/contracts/RolesRegistry/SftRolesRegistrySingleRole.sol @@ -13,15 +13,15 @@ import { ERC165Checker } from '@openzeppelin/contracts/utils/introspection/ERC16 contract SftRolesRegistrySingleRole is ISftRolesRegistry, ERC1155Holder { bytes32 public constant UNIQUE_ROLE = keccak256('UNIQUE_ROLE'); - uint256 public recordCount; + uint256 public commitmentCount; // grantor => tokenAddress => operator => isApproved mapping(address => mapping(address => mapping(address => bool))) public roleApprovals; - // recordId => Record - mapping(uint256 => Record) public records; + // commitmentId => Commitment + mapping(uint256 => Commitment) public commitments; - // recordId => role => RoleAssignment + // commitmentId => role => RoleAssignment mapping(uint256 => mapping(bytes32 => RoleAssignment)) internal roleAssignments; modifier onlyOwnerOrApproved(address _account, address _tokenAddress) { @@ -33,12 +33,12 @@ contract SftRolesRegistrySingleRole is ISftRolesRegistry, ERC1155Holder { } modifier sameGrantee( - uint256 _recordId, + uint256 _commitmentId, bytes32 _role, address _grantee ) { require( - _grantee != address(0) && _grantee == roleAssignments[_recordId][_role].grantee, + _grantee != address(0) && _grantee == roleAssignments[_commitmentId][_role].grantee, 'SftRolesRegistry: grantee mismatch' ); _; @@ -46,65 +46,69 @@ contract SftRolesRegistrySingleRole is ISftRolesRegistry, ERC1155Holder { /** External Functions **/ - function createRecordFrom( + function commitTokens( address _grantor, address _tokenAddress, uint256 _tokenId, uint256 _tokenAmount - ) external override onlyOwnerOrApproved(_grantor, _tokenAddress) returns (uint256 recordId_) { + ) external override onlyOwnerOrApproved(_grantor, _tokenAddress) returns (uint256 commitmentId_) { require(_tokenAmount > 0, 'SftRolesRegistry: tokenAmount must be greater than zero'); - recordId_ = _createRecord(_grantor, _tokenAddress, _tokenId, _tokenAmount); + commitmentId_ = _createCommitment(_grantor, _tokenAddress, _tokenId, _tokenAmount); } function grantRole( - uint256 _recordId, + uint256 _commitmentId, bytes32 _role, address _grantee, uint64 _expirationDate, bool _revocable, bytes calldata _data - ) external override onlyOwnerOrApproved(records[_recordId].grantor, records[_recordId].tokenAddress) { + ) + external + override + onlyOwnerOrApproved(commitments[_commitmentId].grantor, commitments[_commitmentId].tokenAddress) + { require(_role == UNIQUE_ROLE, 'SftRolesRegistry: role not supported'); require(_expirationDate > block.timestamp, 'SftRolesRegistry: expiration date must be in the future'); - _grantOrUpdateRole(_recordId, _role, _grantee, _expirationDate, _revocable, _data); + _grantOrUpdateRole(_commitmentId, _role, _grantee, _expirationDate, _revocable, _data); } function revokeRoleFrom( - uint256 _recordId, + uint256 _commitmentId, bytes32 _role, address _grantee - ) external override sameGrantee(_recordId, _role, _grantee) { - RoleAssignment storage roleAssignment = roleAssignments[_recordId][_role]; - Record storage record = records[_recordId]; - address caller = _findCaller(record.grantor, roleAssignment.grantee, record.tokenAddress); + ) external override sameGrantee(_commitmentId, _role, _grantee) { + RoleAssignment storage roleAssignment = roleAssignments[_commitmentId][_role]; + Commitment storage commitment = commitments[_commitmentId]; + address caller = _findCaller(commitment.grantor, roleAssignment.grantee, commitment.tokenAddress); if (roleAssignment.expirationDate > block.timestamp && !roleAssignment.revocable) { // if role is not expired and is not revocable, only the grantee can revoke it require(caller == roleAssignment.grantee, 'SftRolesRegistry: role is not expired and is not revocable'); } - emit RoleRevoked(_recordId, _role, roleAssignment.grantee); - delete roleAssignments[_recordId][_role]; + emit RoleRevoked(_commitmentId, _role, roleAssignment.grantee); + delete roleAssignments[_commitmentId][_role]; } function withdrawFrom( - uint256 _recordId - ) external onlyOwnerOrApproved(records[_recordId].grantor, records[_recordId].tokenAddress) { + uint256 _commitmentId + ) external onlyOwnerOrApproved(commitments[_commitmentId].grantor, commitments[_commitmentId].tokenAddress) { require( - roleAssignments[_recordId][UNIQUE_ROLE].expirationDate < block.timestamp, + roleAssignments[_commitmentId][UNIQUE_ROLE].expirationDate < block.timestamp, 'SftRolesRegistry: token has an active role' ); - delete roleAssignments[_recordId][UNIQUE_ROLE]; + delete roleAssignments[_commitmentId][UNIQUE_ROLE]; _transferFrom( address(this), - records[_recordId].grantor, - records[_recordId].tokenAddress, - records[_recordId].tokenId, - records[_recordId].tokenAmount + commitments[_commitmentId].grantor, + commitments[_commitmentId].tokenAddress, + commitments[_commitmentId].tokenId, + commitments[_commitmentId].tokenAmount ); - delete records[_recordId]; - emit Withdrew(_recordId); + delete commitments[_commitmentId]; + emit Withdrew(_commitmentId); } function setRoleApprovalForAll(address _tokenAddress, address _operator, bool _isApproved) external override { @@ -114,38 +118,38 @@ contract SftRolesRegistrySingleRole is ISftRolesRegistry, ERC1155Holder { /** View Functions **/ - function recordInfo( - uint256 _recordId + function commitmentInfo( + uint256 _commitmentId ) external view returns (address grantor_, address tokenAddress_, uint256 tokenId_, uint256 tokenAmount_) { - Record memory record = records[_recordId]; - grantor_ = record.grantor; - tokenAddress_ = record.tokenAddress; - tokenId_ = record.tokenId; - tokenAmount_ = record.tokenAmount; + Commitment memory commitment = commitments[_commitmentId]; + grantor_ = commitment.grantor; + tokenAddress_ = commitment.tokenAddress; + tokenId_ = commitment.tokenId; + tokenAmount_ = commitment.tokenAmount; } function roleData( - uint256 _recordId, + uint256 _commitmentId, bytes32 _role, address _grantee - ) external view sameGrantee(_recordId, _role, _grantee) returns (bytes memory data_) { - return roleAssignments[_recordId][_role].data; + ) external view sameGrantee(_commitmentId, _role, _grantee) returns (bytes memory data_) { + return roleAssignments[_commitmentId][_role].data; } function roleExpirationDate( - uint256 _recordId, + uint256 _commitmentId, bytes32 _role, address _grantee - ) external view sameGrantee(_recordId, _role, _grantee) returns (uint64 expirationDate_) { - return roleAssignments[_recordId][_role].expirationDate; + ) external view sameGrantee(_commitmentId, _role, _grantee) returns (uint64 expirationDate_) { + return roleAssignments[_commitmentId][_role].expirationDate; } function isRoleRevocable( - uint256 _recordId, + uint256 _commitmentId, bytes32 _role, address _grantee - ) external view sameGrantee(_recordId, _role, _grantee) returns (bool revocable_) { - return roleAssignments[_recordId][_role].revocable; + ) external view sameGrantee(_commitmentId, _role, _grantee) returns (bool revocable_) { + return roleAssignments[_commitmentId][_role].revocable; } function isRoleApprovedForAll( @@ -164,28 +168,28 @@ contract SftRolesRegistrySingleRole is ISftRolesRegistry, ERC1155Holder { /** Helper Functions **/ - function _createRecord( + function _createCommitment( address _grantor, address _tokenAddress, uint256 _tokenId, uint256 _tokenAmount - ) internal returns (uint256 recordId_) { - recordId_ = ++recordCount; - records[recordId_] = Record(_grantor, _tokenAddress, _tokenId, _tokenAmount); + ) internal returns (uint256 commitmentId_) { + commitmentId_ = ++commitmentCount; + commitments[commitmentId_] = Commitment(_grantor, _tokenAddress, _tokenId, _tokenAmount); _transferFrom(_grantor, address(this), _tokenAddress, _tokenId, _tokenAmount); - emit RecordCreated(_grantor, recordId_, _tokenAddress, _tokenId, _tokenAmount); + emit TokensCommitted(_grantor, commitmentId_, _tokenAddress, _tokenId, _tokenAmount); } function _grantOrUpdateRole( - uint256 _recordId, + uint256 _commitmentId, bytes32 _role, address _grantee, uint64 _expirationDate, bool _revocable, bytes calldata _data ) internal { - roleAssignments[_recordId][_role] = RoleAssignment(_grantee, _expirationDate, _revocable, _data); - emit RoleGranted(_recordId, _role, _grantee, _expirationDate, _revocable, _data); + roleAssignments[_commitmentId][_role] = RoleAssignment(_grantee, _expirationDate, _revocable, _data); + emit RoleGranted(_commitmentId, _role, _grantee, _expirationDate, _revocable, _data); } function _transferFrom( diff --git a/contracts/RolesRegistry/interfaces/ISftRolesRegistry.sol b/contracts/RolesRegistry/interfaces/ISftRolesRegistry.sol index d373821..347f539 100644 --- a/contracts/RolesRegistry/interfaces/ISftRolesRegistry.sol +++ b/contracts/RolesRegistry/interfaces/ISftRolesRegistry.sol @@ -4,9 +4,6 @@ pragma solidity 0.8.9; import { IERC165 } from '@openzeppelin/contracts/utils/introspection/IERC165.sol'; -/// @title ERC-XXXX Semi-Fungible Token Roles -/// @dev See https://eips.ethereum.org/EIPS/eip-XXXX -/// Note: the ERC-165 identifier for this interface is 0xf254051c interface ISftRolesRegistry is IERC165 { struct RoleAssignment { address grantee; @@ -15,7 +12,7 @@ interface ISftRolesRegistry is IERC165 { bytes data; } - struct Record { + struct Commitment { address grantor; address tokenAddress; uint256 tokenId; @@ -24,29 +21,29 @@ interface ISftRolesRegistry is IERC165 { /** Events **/ - /// @notice Emitted when a record is created. + /// @notice Emitted when tokens are committed (deposited or frozen). /// @param _grantor The owner of the NFTs. - /// @param _recordId The identifier of the record created. + /// @param _commitmentId The identifier of the commitment created. /// @param _tokenAddress The token address. /// @param _tokenId The token identifier. /// @param _tokenAmount The token amount. - event RecordCreated( + event TokensCommitted( address indexed _grantor, - uint256 indexed _recordId, + uint256 indexed _commitmentId, address indexed _tokenAddress, uint256 _tokenId, uint256 _tokenAmount ); /// @notice Emitted when a role is granted. - /// @param _recordId The record identifier. + /// @param _commitmentId The commitment identifier. /// @param _role The role identifier. /// @param _grantee The user receiving the role. /// @param _expirationDate The expiration date of the role. /// @param _revocable Whether the role is revocable or not. /// @param _data Any additional data about the role. event RoleGranted( - uint256 indexed _recordId, + uint256 indexed _commitmentId, bytes32 indexed _role, address indexed _grantee, uint64 _expirationDate, @@ -55,14 +52,14 @@ interface ISftRolesRegistry is IERC165 { ); /// @notice Emitted when a role is revoked. - /// @param _recordId The record identifier. + /// @param _commitmentId The commitment identifier. /// @param _role The role identifier. /// @param _grantee The user that receives the role revocation. - event RoleRevoked(uint256 indexed _recordId, bytes32 indexed _role, address indexed _grantee); + event RoleRevoked(uint256 indexed _commitmentId, bytes32 indexed _role, address indexed _grantee); - /// @notice Emitted when a user withdraws tokens from a record. - /// @param _recordId The record identifier. - event Withdrew(uint256 indexed _recordId); + /// @notice Emitted when a user withdraws tokens from a commitment. + /// @param _commitmentId The commitment identifier. + event Withdrew(uint256 indexed _commitmentId); /// @notice Emitted when a user is approved to manage roles on behalf of another user. /// @param _tokenAddress The token address. @@ -72,28 +69,28 @@ interface ISftRolesRegistry is IERC165 { /** External Functions **/ - /// @notice Creates a new record for on behalf of a user. + /// @notice Commits tokens (deposits on a contract or freezes balance). /// @param _grantor The owner of the NFTs. /// @param _tokenAddress The token address. /// @param _tokenId The token identifier. /// @param _tokenAmount The token amount. - /// @return recordId_ The unique identifier of the record created. - function createRecordFrom( + /// @return commitmentId_ The unique identifier of the commitment created. + function commitTokens( address _grantor, address _tokenAddress, uint256 _tokenId, uint256 _tokenAmount - ) external returns (uint256 recordId_); + ) external returns (uint256 commitmentId_); /// @notice Grants a role to `_grantee`. - /// @param _recordId The identifier of the record. + /// @param _commitmentId The identifier of the commitment. /// @param _role The role identifier. /// @param _grantee The user receiving the role. /// @param _expirationDate The expiration date of the role. /// @param _revocable Whether the role is revocable or not. /// @param _data Any additional data about the role. function grantRole( - uint256 _recordId, + uint256 _commitmentId, bytes32 _role, address _grantee, uint64 _expirationDate, @@ -102,14 +99,14 @@ interface ISftRolesRegistry is IERC165 { ) external; /// @notice Revokes a role on behalf of a user. - /// @param _recordId The record identifier. + /// @param _commitmentId The commitment identifier. /// @param _role The role identifier. /// @param _grantee The user that gets their role revoked. - function revokeRoleFrom(uint256 _recordId, bytes32 _role, address _grantee) external; + function revokeRoleFrom(uint256 _commitmentId, bytes32 _role, address _grantee) external; /// @notice Withdraws tokens back to grantor. - /// @param _recordId The record identifier. - function withdrawFrom(uint256 _recordId) external; + /// @param _commitmentId The commitment identifier. + function withdrawFrom(uint256 _commitmentId) external; /// @notice Approves operator to grant and revoke roles on behalf of another user. /// @param _tokenAddress The token address. @@ -119,41 +116,45 @@ interface ISftRolesRegistry is IERC165 { /** View Functions **/ - /// @notice Returns all the information regarding a record. - /// @param _recordId The record identifier. - /// @return grantor_ The record owner. + /// @notice Returns all information regarding a commitment. + /// @param _commitmentId The commitment identifier. + /// @return grantor_ The commitment owner. /// @return tokenAddress_ The token address. /// @return tokenId_ The token identifier. /// @return tokenAmount_ The token amount. - function recordInfo( - uint256 _recordId + function commitmentInfo( + uint256 _commitmentId ) external view returns (address grantor_, address tokenAddress_, uint256 tokenId_, uint256 tokenAmount_); /// @notice Returns the custom data of a role assignment. - /// @param _recordId The record identifier. + /// @param _commitmentId The commitment identifier. /// @param _role The role identifier. /// @param _grantee The user that received the role. /// @return data_ The custom data. - function roleData(uint256 _recordId, bytes32 _role, address _grantee) external view returns (bytes memory data_); + function roleData( + uint256 _commitmentId, + bytes32 _role, + address _grantee + ) external view returns (bytes memory data_); /// @notice Returns the expiration date of a role assignment. - /// @param _recordId The record identifier. + /// @param _commitmentId The commitment identifier. /// @param _role The role identifier. /// @param _grantee The user that received the role. /// @return expirationDate_ The expiration date. function roleExpirationDate( - uint256 _recordId, + uint256 _commitmentId, bytes32 _role, address _grantee ) external view returns (uint64 expirationDate_); /// @notice Returns the expiration date of a role assignment. - /// @param _recordId The record identifier. + /// @param _commitmentId The commitment identifier. /// @param _role The role identifier. /// @param _grantee The user that received the role. /// @return revocable_ Whether the role is revocable or not. function isRoleRevocable( - uint256 _recordId, + uint256 _commitmentId, bytes32 _role, address _grantee ) external view returns (bool revocable_); diff --git a/contracts/RolesRegistry/libraries/LinkedLists.sol b/contracts/RolesRegistry/libraries/LinkedLists.sol index 5588884..2e58c00 100644 --- a/contracts/RolesRegistry/libraries/LinkedLists.sol +++ b/contracts/RolesRegistry/libraries/LinkedLists.sol @@ -24,9 +24,9 @@ library LinkedLists { } struct Lists { - // headKey => recordId + // headKey => commitmentId mapping(bytes32 => uint256) heads; - // hash(recordId, role, grantee) => item + // hash(commitmentId, role, grantee) => item mapping(uint256 => ListItem) items; } diff --git a/test/SftRolesRegistry/SftRolesRegistry.spec.ts b/test/SftRolesRegistry/SftRolesRegistry.spec.ts index d786b7f..0d37b30 100644 --- a/test/SftRolesRegistry/SftRolesRegistry.spec.ts +++ b/test/SftRolesRegistry/SftRolesRegistry.spec.ts @@ -4,10 +4,16 @@ import { beforeEach } from 'mocha' import { expect } from 'chai' import { loadFixture, time } from '@nomicfoundation/hardhat-network-helpers' import { SignerWithAddress } from '@nomiclabs/hardhat-ethers/signers' -import { ONE_DAY, generateRoleId, buildRecord, buildGrantRole } from './helpers/mockData' -import { Record, GrantRoleData } from './types' +import { Commitment, GrantRoleData } from './types' import { generateRandomInt } from '../helpers' -import { assertCreateRecordEvent, assertGrantRoleEvent, assertRevokeRoleEvent } from './helpers/assertEvents' +import { assertCreateCommitmentEvent, assertGrantRoleEvent, assertRevokeRoleEvent } from './helpers/assertEvents' +import { + ONE_DAY, + generateRoleId, + buildCommitment, + buildGrantRole, + getSftRolesRegistryInterfaceId, +} from './helpers/mockData' const { AddressZero } = ethers.constants @@ -34,94 +40,94 @@ describe('SftRolesRegistry', async () => { await loadFixture(deployContracts) }) - describe('createRecordFrom', async () => { + describe('commitTokens', async () => { it('should revert when sender is not grantor or approved', async () => { - const record = buildRecord({ + const commitment = buildCommitment({ grantor: grantor.address, tokenAddress: MockToken.address, }) await expect( - SftRolesRegistry.connect(anotherUser).createRecordFrom( - record.grantor, - record.tokenAddress, - record.tokenId, - record.tokenAmount, + SftRolesRegistry.connect(anotherUser).commitTokens( + commitment.grantor, + commitment.tokenAddress, + commitment.tokenId, + commitment.tokenAmount, ), ).to.be.revertedWith('SftRolesRegistry: account not approved') }) it('should revert when tokenAmount is zero', async () => { - const record = buildRecord({ + const commitment = buildCommitment({ grantor: grantor.address, tokenAddress: MockToken.address, tokenAmount: 0, }) await expect( - SftRolesRegistry.connect(grantor).createRecordFrom( - record.grantor, - record.tokenAddress, - record.tokenId, - record.tokenAmount, + SftRolesRegistry.connect(grantor).commitTokens( + commitment.grantor, + commitment.tokenAddress, + commitment.tokenId, + commitment.tokenAmount, ), ).to.be.revertedWith('SftRolesRegistry: tokenAmount must be greater than zero') }) it('should revert without a reason if tokenAddress is not an ERC-1155 contract', async () => { - const record = buildRecord({ + const commitment = buildCommitment({ grantor: grantor.address, tokenAmount: generateRandomInt(), }) await expect( - SftRolesRegistry.connect(grantor).createRecordFrom( - record.grantor, - record.tokenAddress, - record.tokenId, - record.tokenAmount, + SftRolesRegistry.connect(grantor).commitTokens( + commitment.grantor, + commitment.tokenAddress, + commitment.tokenId, + commitment.tokenAmount, ), ).to.be.reverted }) it('should revert if contract is not approved to transfer tokens', async () => { - const record = buildRecord({ + const commitment = buildCommitment({ grantor: grantor.address, tokenAddress: MockToken.address, tokenAmount: generateRandomInt(), }) - await MockToken.mint(grantor.address, record.tokenId, record.tokenAmount) + await MockToken.mint(grantor.address, commitment.tokenId, commitment.tokenAmount) await expect( - SftRolesRegistry.connect(grantor).createRecordFrom( - record.grantor, - record.tokenAddress, - record.tokenId, - record.tokenAmount, + SftRolesRegistry.connect(grantor).commitTokens( + commitment.grantor, + commitment.tokenAddress, + commitment.tokenId, + commitment.tokenAmount, ), ).to.be.revertedWith('ERC1155: caller is not token owner or approved') }) it('should revert when grantor does not have enough tokens', async () => { - const record = buildRecord({ + const commitment = buildCommitment({ grantor: grantor.address, tokenAddress: MockToken.address, tokenAmount: generateRandomInt() + 1, }) - await MockToken.mint(grantor.address, record.tokenId, record.tokenAmount - 1) + await MockToken.mint(grantor.address, commitment.tokenId, commitment.tokenAmount - 1) await MockToken.connect(grantor).setApprovalForAll(SftRolesRegistry.address, true) await expect( - SftRolesRegistry.connect(grantor).createRecordFrom( - record.grantor, - record.tokenAddress, - record.tokenId, - record.tokenAmount, + SftRolesRegistry.connect(grantor).commitTokens( + commitment.grantor, + commitment.tokenAddress, + commitment.tokenId, + commitment.tokenAmount, ), ).to.be.revertedWith('ERC1155: insufficient balance for transfer') }) - it('should create record when sender is grantor', async () => { - await assertCreateRecordEvent(SftRolesRegistry, MockToken, grantor, 1) + it('should create commitment when sender is grantor', async () => { + await assertCreateCommitmentEvent(SftRolesRegistry, MockToken, grantor, 1) }) - it('should create record when sender is approved', async () => { - await assertCreateRecordEvent(SftRolesRegistry, MockToken, grantor, 1, anotherUser) + it('should create commitment when sender is approved', async () => { + await assertCreateCommitmentEvent(SftRolesRegistry, MockToken, grantor, 1, anotherUser) }) }) @@ -129,9 +135,9 @@ describe('SftRolesRegistry', async () => { let GrantRoleData: GrantRoleData beforeEach(async () => { - await assertCreateRecordEvent(SftRolesRegistry, MockToken, grantor, 1) + await assertCreateCommitmentEvent(SftRolesRegistry, MockToken, grantor, 1) GrantRoleData = await buildGrantRole({ - recordId: 1, + commitmentId: 1, grantee: grantee.address, }) }) @@ -139,7 +145,7 @@ describe('SftRolesRegistry', async () => { it('should revert when sender is not grantor or approved', async () => { await expect( SftRolesRegistry.connect(anotherUser).grantRole( - GrantRoleData.recordId, + GrantRoleData.commitmentId, GrantRoleData.role, GrantRoleData.grantee, GrantRoleData.expirationDate, @@ -152,7 +158,7 @@ describe('SftRolesRegistry', async () => { it('should revert when expirationDate is zero', async () => { await expect( SftRolesRegistry.connect(grantor).grantRole( - GrantRoleData.recordId, + GrantRoleData.commitmentId, GrantRoleData.role, GrantRoleData.grantee, await time.latest(), @@ -163,10 +169,10 @@ describe('SftRolesRegistry', async () => { }) it('should revert when role is not revocable and is not expired', async () => { - await assertGrantRoleEvent(SftRolesRegistry, grantor, GrantRoleData.recordId, GrantRoleData.grantee, false) + await assertGrantRoleEvent(SftRolesRegistry, grantor, GrantRoleData.commitmentId, GrantRoleData.grantee, false) await expect( SftRolesRegistry.connect(grantor).grantRole( - GrantRoleData.recordId, + GrantRoleData.commitmentId, GrantRoleData.role, GrantRoleData.grantee, GrantRoleData.expirationDate, @@ -177,14 +183,14 @@ describe('SftRolesRegistry', async () => { }) it('should grant role when sender is grantor', async () => { - await assertGrantRoleEvent(SftRolesRegistry, grantor, GrantRoleData.recordId, GrantRoleData.grantee) + await assertGrantRoleEvent(SftRolesRegistry, grantor, GrantRoleData.commitmentId, GrantRoleData.grantee) }) it('should grant role when sender is approved', async () => { await assertGrantRoleEvent( SftRolesRegistry, grantor, - GrantRoleData.recordId, + GrantRoleData.commitmentId, GrantRoleData.grantee, true, anotherUser, @@ -193,18 +199,18 @@ describe('SftRolesRegistry', async () => { }) describe('revokeRole', async () => { - let RecordCreated: Record + let CommitmentCreated: Commitment let GrantRoleData: GrantRoleData beforeEach(async () => { - RecordCreated = await assertCreateRecordEvent(SftRolesRegistry, MockToken, grantor, 1) + CommitmentCreated = await assertCreateCommitmentEvent(SftRolesRegistry, MockToken, grantor, 1) GrantRoleData = await assertGrantRoleEvent(SftRolesRegistry, grantor, 1, grantee.address) }) it('should revert when sender is not grantor or approved', async () => { await expect( SftRolesRegistry.connect(anotherUser).revokeRoleFrom( - GrantRoleData.recordId, + GrantRoleData.commitmentId, GrantRoleData.role, GrantRoleData.grantee, ), @@ -212,10 +218,10 @@ describe('SftRolesRegistry', async () => { }) it('should revert when role assignment does not exist', async () => { - // different recordId + // different commitmentId await expect( SftRolesRegistry.connect(grantor).revokeRoleFrom( - GrantRoleData.recordId + 1, + GrantRoleData.commitmentId + 1, GrantRoleData.role, GrantRoleData.grantee, ), @@ -224,7 +230,7 @@ describe('SftRolesRegistry', async () => { // different role await expect( SftRolesRegistry.connect(grantor).revokeRoleFrom( - GrantRoleData.recordId, + GrantRoleData.commitmentId, generateRoleId('ANOTHER_ROLE'), GrantRoleData.grantee, ), @@ -233,7 +239,7 @@ describe('SftRolesRegistry', async () => { // different grantee await expect( SftRolesRegistry.connect(grantor).revokeRoleFrom( - GrantRoleData.recordId + 1, + GrantRoleData.commitmentId + 1, GrantRoleData.role, anotherUser.address, ), @@ -244,13 +250,13 @@ describe('SftRolesRegistry', async () => { const roleAssignment = await assertGrantRoleEvent( SftRolesRegistry, grantor, - GrantRoleData.recordId, + GrantRoleData.commitmentId, grantee.address, false, ) await expect( SftRolesRegistry.connect(grantor).revokeRoleFrom( - roleAssignment.recordId, + roleAssignment.commitmentId, GrantRoleData.role, GrantRoleData.grantee, ), @@ -261,30 +267,30 @@ describe('SftRolesRegistry', async () => { const roleAssignment = await assertGrantRoleEvent( SftRolesRegistry, grantor, - GrantRoleData.recordId, + GrantRoleData.commitmentId, grantee.address, false, ) await time.increase(ONE_DAY) - await assertRevokeRoleEvent(SftRolesRegistry, grantor, roleAssignment.recordId, roleAssignment.role, grantee) + await assertRevokeRoleEvent(SftRolesRegistry, grantor, roleAssignment.commitmentId, roleAssignment.role, grantee) }) it('should revoke when role is revocable', async () => { - await assertRevokeRoleEvent(SftRolesRegistry, grantor, GrantRoleData.recordId, GrantRoleData.role, grantee) + await assertRevokeRoleEvent(SftRolesRegistry, grantor, GrantRoleData.commitmentId, GrantRoleData.role, grantee) }) it('should revoke when sender is grantee', async () => { const roleAssignment = await assertGrantRoleEvent( SftRolesRegistry, grantor, - GrantRoleData.recordId, + GrantRoleData.commitmentId, grantee.address, false, ) await assertRevokeRoleEvent( SftRolesRegistry, grantor, - roleAssignment.recordId, + roleAssignment.commitmentId, roleAssignment.role, grantee, grantee, @@ -292,39 +298,39 @@ describe('SftRolesRegistry', async () => { }) it('should revoke when sender is approved by grantee', async () => { - await assertGrantRoleEvent(SftRolesRegistry, grantor, GrantRoleData.recordId, grantee.address, false) + await assertGrantRoleEvent(SftRolesRegistry, grantor, GrantRoleData.commitmentId, grantee.address, false) await SftRolesRegistry.connect(grantee).setRoleApprovalForAll( - RecordCreated.tokenAddress, + CommitmentCreated.tokenAddress, anotherUser.address, true, ) await expect( SftRolesRegistry.connect(anotherUser).revokeRoleFrom( - GrantRoleData.recordId, + GrantRoleData.commitmentId, GrantRoleData.role, grantee.address, ), ) .to.emit(SftRolesRegistry, 'RoleRevoked') - .withArgs(GrantRoleData.recordId, GrantRoleData.role, grantee.address) + .withArgs(GrantRoleData.commitmentId, GrantRoleData.role, grantee.address) }) it('should revoke when sender is approved by grantor', async () => { await SftRolesRegistry.connect(grantor).setRoleApprovalForAll( - RecordCreated.tokenAddress, + CommitmentCreated.tokenAddress, anotherUser.address, true, ) await expect( SftRolesRegistry.connect(anotherUser).revokeRoleFrom( - GrantRoleData.recordId, + GrantRoleData.commitmentId, GrantRoleData.role, grantee.address, ), ) .to.emit(SftRolesRegistry, 'RoleRevoked') - .withArgs(GrantRoleData.recordId, GrantRoleData.role, grantee.address) + .withArgs(GrantRoleData.commitmentId, GrantRoleData.role, grantee.address) }) }) @@ -332,42 +338,42 @@ describe('SftRolesRegistry', async () => { let GrantRoleData: GrantRoleData beforeEach(async () => { - await assertCreateRecordEvent(SftRolesRegistry, MockToken, grantor, 1) + await assertCreateCommitmentEvent(SftRolesRegistry, MockToken, grantor, 1) GrantRoleData = await assertGrantRoleEvent(SftRolesRegistry, grantor, 1, grantee.address) }) it('should revert when sender is not grantor or approved', async () => { - await expect(SftRolesRegistry.connect(anotherUser).withdrawFrom(GrantRoleData.recordId)).to.be.revertedWith( + await expect(SftRolesRegistry.connect(anotherUser).withdrawFrom(GrantRoleData.commitmentId)).to.be.revertedWith( 'SftRolesRegistry: account not approved', ) }) it('should revert when there is an active role', async () => { await assertGrantRoleEvent(SftRolesRegistry, grantor, 1, grantee.address, false) - await expect(SftRolesRegistry.connect(grantor).withdrawFrom(GrantRoleData.recordId)).to.be.revertedWith( + await expect(SftRolesRegistry.connect(grantor).withdrawFrom(GrantRoleData.commitmentId)).to.be.revertedWith( 'SftRolesRegistry: role is not expired and is not revocable', ) }) it('should withdraw when there are revocable roles', async () => { - await expect(SftRolesRegistry.connect(grantor).withdrawFrom(GrantRoleData.recordId)) + await expect(SftRolesRegistry.connect(grantor).withdrawFrom(GrantRoleData.commitmentId)) .to.emit(SftRolesRegistry, 'Withdrew') - .withArgs(GrantRoleData.recordId) + .withArgs(GrantRoleData.commitmentId) }) it('should withdraw when there are no roles', async () => { - await assertRevokeRoleEvent(SftRolesRegistry, grantor, GrantRoleData.recordId, GrantRoleData.role, grantee) - await expect(SftRolesRegistry.connect(grantor).withdrawFrom(GrantRoleData.recordId)) + await assertRevokeRoleEvent(SftRolesRegistry, grantor, GrantRoleData.commitmentId, GrantRoleData.role, grantee) + await expect(SftRolesRegistry.connect(grantor).withdrawFrom(GrantRoleData.commitmentId)) .to.emit(SftRolesRegistry, 'Withdrew') - .withArgs(GrantRoleData.recordId) + .withArgs(GrantRoleData.commitmentId) }) it('should withdraw when there are expired roles', async () => { await assertGrantRoleEvent(SftRolesRegistry, grantor, 1, grantee.address, false) await time.increase(ONE_DAY) - await expect(SftRolesRegistry.connect(grantor).withdrawFrom(GrantRoleData.recordId)) + await expect(SftRolesRegistry.connect(grantor).withdrawFrom(GrantRoleData.commitmentId)) .to.emit(SftRolesRegistry, 'Withdrew') - .withArgs(GrantRoleData.recordId) + .withArgs(GrantRoleData.commitmentId) }) }) @@ -382,25 +388,25 @@ describe('SftRolesRegistry', async () => { }) describe('View Functions', async () => { - let RecordCreated: Record + let CommitmentCreated: Commitment let GrantRoleData: GrantRoleData beforeEach(async () => { - RecordCreated = await assertCreateRecordEvent(SftRolesRegistry, MockToken, grantor, 1) + CommitmentCreated = await assertCreateCommitmentEvent(SftRolesRegistry, MockToken, grantor, 1) GrantRoleData = await assertGrantRoleEvent(SftRolesRegistry, grantor, 1, grantee.address) }) - it('recordInfo', async () => { - const record = await SftRolesRegistry.recordInfo(GrantRoleData.recordId) - expect(record.grantor_).to.be.equal(RecordCreated.grantor) - expect(record.tokenAddress_).to.be.equal(RecordCreated.tokenAddress) - expect(record.tokenId_).to.be.equal(RecordCreated.tokenId) - expect(record.tokenAmount_).to.be.equal(RecordCreated.tokenAmount) + it('commitmentInfo', async () => { + const commitment = await SftRolesRegistry.commitmentInfo(GrantRoleData.commitmentId) + expect(commitment.grantor_).to.be.equal(CommitmentCreated.grantor) + expect(commitment.tokenAddress_).to.be.equal(CommitmentCreated.tokenAddress) + expect(commitment.tokenId_).to.be.equal(CommitmentCreated.tokenId) + expect(commitment.tokenAmount_).to.be.equal(CommitmentCreated.tokenAmount) }) it('roleData', async () => { const roleData = await SftRolesRegistry.roleData( - GrantRoleData.recordId, + GrantRoleData.commitmentId, GrantRoleData.role, GrantRoleData.grantee, ) @@ -409,7 +415,7 @@ describe('SftRolesRegistry', async () => { it('roleExpirationDate', async () => { const roleExpirationDate = await SftRolesRegistry.roleExpirationDate( - GrantRoleData.recordId, + GrantRoleData.commitmentId, GrantRoleData.role, GrantRoleData.grantee, ) @@ -418,7 +424,7 @@ describe('SftRolesRegistry', async () => { it('isRoleRevocable', async () => { const isRoleRevocable = await SftRolesRegistry.isRoleRevocable( - GrantRoleData.recordId, + GrantRoleData.commitmentId, GrantRoleData.role, GrantRoleData.grantee, ) @@ -431,8 +437,9 @@ describe('SftRolesRegistry', async () => { expect(await SftRolesRegistry.supportsInterface('0x4e2312e0')).to.be.true }) - it('should return true if SftRolesRegistry interface id (0x89cb6ab6)', async () => { - expect(await SftRolesRegistry.supportsInterface('0xf254051c')).to.be.true + it('should return true if SftRolesRegistry interface id', async () => { + const interfaceId = getSftRolesRegistryInterfaceId() + expect(await SftRolesRegistry.supportsInterface(interfaceId)).to.be.true }) }) }) diff --git a/test/SftRolesRegistry/SftRolesRegistrySingleRole.spec.ts b/test/SftRolesRegistry/SftRolesRegistrySingleRole.spec.ts index 292647a..c34a380 100644 --- a/test/SftRolesRegistry/SftRolesRegistrySingleRole.spec.ts +++ b/test/SftRolesRegistry/SftRolesRegistrySingleRole.spec.ts @@ -4,10 +4,10 @@ import { beforeEach } from 'mocha' import { expect } from 'chai' import { loadFixture, time } from '@nomicfoundation/hardhat-network-helpers' import { SignerWithAddress } from '@nomiclabs/hardhat-ethers/signers' -import { generateRoleId, buildRecord, buildGrantRole } from './helpers/mockData' -import { GrantRoleData, Record } from './types' +import { generateRoleId, buildCommitment, buildGrantRole, getSftRolesRegistryInterfaceId } from './helpers/mockData' +import { GrantRoleData, Commitment } from './types' import { generateRandomInt } from '../helpers' -import { assertCreateRecordEvent, assertGrantRoleEvent } from './helpers/assertEvents' +import { assertCreateCommitmentEvent, assertGrantRoleEvent } from './helpers/assertEvents' describe('SftRolesRegistrySingleRole', async () => { let SftRolesRegistry: Contract @@ -32,165 +32,165 @@ describe('SftRolesRegistrySingleRole', async () => { await loadFixture(deployContracts) }) - describe('createRecordFrom', async () => { + describe('commitTokens', async () => { it('should revert when sender is not grantor or approved', async () => { - const record = buildRecord({ + const commitment = buildCommitment({ grantor: grantor.address, tokenAddress: MockToken.address, }) await expect( - SftRolesRegistry.connect(anotherUser).createRecordFrom( - record.grantor, - record.tokenAddress, - record.tokenId, - record.tokenAmount, + SftRolesRegistry.connect(anotherUser).commitTokens( + commitment.grantor, + commitment.tokenAddress, + commitment.tokenId, + commitment.tokenAmount, ), ).to.be.revertedWith('SftRolesRegistry: account not approved') }) it('should revert when tokenAmount is zero', async () => { - const record = buildRecord({ + const commitment = buildCommitment({ grantor: grantor.address, tokenAddress: MockToken.address, tokenAmount: 0, }) await expect( - SftRolesRegistry.connect(grantor).createRecordFrom( - record.grantor, - record.tokenAddress, - record.tokenId, - record.tokenAmount, + SftRolesRegistry.connect(grantor).commitTokens( + commitment.grantor, + commitment.tokenAddress, + commitment.tokenId, + commitment.tokenAmount, ), ).to.be.revertedWith('SftRolesRegistry: tokenAmount must be greater than zero') }) it('should revert without a reason if tokenAddress is not an ERC-1155 contract', async () => { - const record = buildRecord({ + const commitment = buildCommitment({ grantor: grantor.address, tokenAmount: generateRandomInt(), }) await expect( - SftRolesRegistry.connect(grantor).createRecordFrom( - record.grantor, - record.tokenAddress, - record.tokenId, - record.tokenAmount, + SftRolesRegistry.connect(grantor).commitTokens( + commitment.grantor, + commitment.tokenAddress, + commitment.tokenId, + commitment.tokenAmount, ), ).to.be.reverted }) it('should revert if contract is not approved to transfer tokens', async () => { - const record = buildRecord({ + const commitment = buildCommitment({ grantor: grantor.address, tokenAddress: MockToken.address, tokenAmount: generateRandomInt(), }) - await MockToken.mint(grantor.address, record.tokenId, record.tokenAmount) + await MockToken.mint(grantor.address, commitment.tokenId, commitment.tokenAmount) await expect( - SftRolesRegistry.connect(grantor).createRecordFrom( - record.grantor, - record.tokenAddress, - record.tokenId, - record.tokenAmount, + SftRolesRegistry.connect(grantor).commitTokens( + commitment.grantor, + commitment.tokenAddress, + commitment.tokenId, + commitment.tokenAmount, ), ).to.be.revertedWith('ERC1155: caller is not token owner or approved') }) it('should revert when grantor does not have enough tokens', async () => { - const record = buildRecord({ + const commitment = buildCommitment({ grantor: grantor.address, tokenAddress: MockToken.address, tokenAmount: generateRandomInt() + 1, }) - await MockToken.mint(grantor.address, record.tokenId, record.tokenAmount - 1) + await MockToken.mint(grantor.address, commitment.tokenId, commitment.tokenAmount - 1) await MockToken.connect(grantor).setApprovalForAll(SftRolesRegistry.address, true) await expect( - SftRolesRegistry.connect(grantor).createRecordFrom( - record.grantor, - record.tokenAddress, - record.tokenId, - record.tokenAmount, + SftRolesRegistry.connect(grantor).commitTokens( + commitment.grantor, + commitment.tokenAddress, + commitment.tokenId, + commitment.tokenAmount, ), ).to.be.revertedWith('ERC1155: insufficient balance for transfer') }) - it('should create record when sender is grantor', async () => { - const record = buildRecord({ + it('should create commitment when sender is grantor', async () => { + const commitment = buildCommitment({ grantor: grantor.address, tokenAddress: MockToken.address, }) - await MockToken.mint(grantor.address, record.tokenId, record.tokenAmount) + await MockToken.mint(grantor.address, commitment.tokenId, commitment.tokenAmount) await MockToken.connect(grantor).setApprovalForAll(SftRolesRegistry.address, true) await expect( - SftRolesRegistry.connect(grantor).createRecordFrom( - record.grantor, - record.tokenAddress, - record.tokenId, - record.tokenAmount, + SftRolesRegistry.connect(grantor).commitTokens( + commitment.grantor, + commitment.tokenAddress, + commitment.tokenId, + commitment.tokenAmount, ), ) - .to.emit(SftRolesRegistry, 'RecordCreated') - .withArgs(record.grantor, 1, record.tokenAddress, record.tokenId, record.tokenAmount) + .to.emit(SftRolesRegistry, 'TokensCommitted') + .withArgs(commitment.grantor, 1, commitment.tokenAddress, commitment.tokenId, commitment.tokenAmount) .to.emit(MockToken, 'TransferSingle') .withArgs( SftRolesRegistry.address, grantor.address, SftRolesRegistry.address, - record.tokenId, - record.tokenAmount, + commitment.tokenId, + commitment.tokenAmount, ) }) - it('should create record when sender is approved', async () => { - const record = buildRecord({ + it('should create commitment when sender is approved', async () => { + const commitment = buildCommitment({ grantor: grantor.address, tokenAddress: MockToken.address, }) - await MockToken.mint(grantor.address, record.tokenId, record.tokenAmount) + await MockToken.mint(grantor.address, commitment.tokenId, commitment.tokenAmount) await MockToken.connect(grantor).setApprovalForAll(SftRolesRegistry.address, true) - await SftRolesRegistry.connect(grantor).setRoleApprovalForAll(record.tokenAddress, anotherUser.address, true) + await SftRolesRegistry.connect(grantor).setRoleApprovalForAll(commitment.tokenAddress, anotherUser.address, true) await expect( - SftRolesRegistry.connect(anotherUser).createRecordFrom( - record.grantor, - record.tokenAddress, - record.tokenId, - record.tokenAmount, + SftRolesRegistry.connect(anotherUser).commitTokens( + commitment.grantor, + commitment.tokenAddress, + commitment.tokenId, + commitment.tokenAmount, ), ) - .to.emit(SftRolesRegistry, 'RecordCreated') - .withArgs(record.grantor, 1, record.tokenAddress, record.tokenId, record.tokenAmount) + .to.emit(SftRolesRegistry, 'TokensCommitted') + .withArgs(commitment.grantor, 1, commitment.tokenAddress, commitment.tokenId, commitment.tokenAmount) .to.emit(MockToken, 'TransferSingle') .withArgs( SftRolesRegistry.address, grantor.address, SftRolesRegistry.address, - record.tokenId, - record.tokenAmount, + commitment.tokenId, + commitment.tokenAmount, ) }) }) describe('grantRole', async () => { - let RecordCreated: Record + let CommitmentCreated: Commitment let GrantRoleData: GrantRoleData beforeEach(async () => { - RecordCreated = buildRecord({ + CommitmentCreated = buildCommitment({ grantor: grantor.address, tokenAddress: MockToken.address, }) GrantRoleData = await buildGrantRole({ - recordId: 1, + commitmentId: 1, grantee: grantee.address, }) - await MockToken.mint(grantor.address, RecordCreated.tokenId, RecordCreated.tokenAmount) + await MockToken.mint(grantor.address, CommitmentCreated.tokenId, CommitmentCreated.tokenAmount) await MockToken.connect(grantor).setApprovalForAll(SftRolesRegistry.address, true) await expect( - SftRolesRegistry.connect(grantor).createRecordFrom( - RecordCreated.grantor, - RecordCreated.tokenAddress, - RecordCreated.tokenId, - RecordCreated.tokenAmount, + SftRolesRegistry.connect(grantor).commitTokens( + CommitmentCreated.grantor, + CommitmentCreated.tokenAddress, + CommitmentCreated.tokenId, + CommitmentCreated.tokenAmount, ), ).to.not.be.reverted }) @@ -198,7 +198,7 @@ describe('SftRolesRegistrySingleRole', async () => { it('should revert when sender is not grantor or approved', async () => { await expect( SftRolesRegistry.connect(anotherUser).grantRole( - GrantRoleData.recordId, + GrantRoleData.commitmentId, GrantRoleData.role, GrantRoleData.grantee, GrantRoleData.expirationDate, @@ -211,7 +211,7 @@ describe('SftRolesRegistrySingleRole', async () => { it('should revert when role is not supported', async () => { await expect( SftRolesRegistry.connect(grantor).grantRole( - GrantRoleData.recordId, + GrantRoleData.commitmentId, generateRoleId('ANOTHER_ROLE'), GrantRoleData.grantee, GrantRoleData.expirationDate, @@ -224,7 +224,7 @@ describe('SftRolesRegistrySingleRole', async () => { it('should revert when expirationDate is zero', async () => { await expect( SftRolesRegistry.connect(grantor).grantRole( - GrantRoleData.recordId, + GrantRoleData.commitmentId, GrantRoleData.role, GrantRoleData.grantee, await time.latest(), @@ -237,7 +237,7 @@ describe('SftRolesRegistrySingleRole', async () => { it('should grant role when sender is grantor', async () => { await expect( SftRolesRegistry.connect(grantor).grantRole( - GrantRoleData.recordId, + GrantRoleData.commitmentId, GrantRoleData.role, GrantRoleData.grantee, GrantRoleData.expirationDate, @@ -247,7 +247,7 @@ describe('SftRolesRegistrySingleRole', async () => { ) .to.emit(SftRolesRegistry, 'RoleGranted') .withArgs( - GrantRoleData.recordId, + GrantRoleData.commitmentId, GrantRoleData.role, GrantRoleData.grantee, GrantRoleData.expirationDate, @@ -258,13 +258,13 @@ describe('SftRolesRegistrySingleRole', async () => { it('should grant role when sender is approved', async () => { await SftRolesRegistry.connect(grantor).setRoleApprovalForAll( - RecordCreated.tokenAddress, + CommitmentCreated.tokenAddress, anotherUser.address, true, ) await expect( SftRolesRegistry.connect(anotherUser).grantRole( - GrantRoleData.recordId, + GrantRoleData.commitmentId, GrantRoleData.role, GrantRoleData.grantee, GrantRoleData.expirationDate, @@ -274,7 +274,7 @@ describe('SftRolesRegistrySingleRole', async () => { ) .to.emit(SftRolesRegistry, 'RoleGranted') .withArgs( - GrantRoleData.recordId, + GrantRoleData.commitmentId, GrantRoleData.role, GrantRoleData.grantee, GrantRoleData.expirationDate, @@ -285,31 +285,31 @@ describe('SftRolesRegistrySingleRole', async () => { }) describe('revokeRole', async () => { - let RecordCreated: Record + let CommitmentCreated: Commitment let GrantRoleData: GrantRoleData beforeEach(async () => { - RecordCreated = buildRecord({ + CommitmentCreated = buildCommitment({ grantor: grantor.address, tokenAddress: MockToken.address, }) GrantRoleData = await buildGrantRole({ - recordId: 1, + commitmentId: 1, grantee: grantee.address, }) - await MockToken.mint(grantor.address, RecordCreated.tokenId, RecordCreated.tokenAmount) + await MockToken.mint(grantor.address, CommitmentCreated.tokenId, CommitmentCreated.tokenAmount) await MockToken.connect(grantor).setApprovalForAll(SftRolesRegistry.address, true) await expect( - SftRolesRegistry.connect(grantor).createRecordFrom( - RecordCreated.grantor, - RecordCreated.tokenAddress, - RecordCreated.tokenId, - RecordCreated.tokenAmount, + SftRolesRegistry.connect(grantor).commitTokens( + CommitmentCreated.grantor, + CommitmentCreated.tokenAddress, + CommitmentCreated.tokenId, + CommitmentCreated.tokenAmount, ), ).to.not.be.reverted await expect( SftRolesRegistry.connect(grantor).grantRole( - GrantRoleData.recordId, + GrantRoleData.commitmentId, GrantRoleData.role, GrantRoleData.grantee, GrantRoleData.expirationDate, @@ -322,7 +322,7 @@ describe('SftRolesRegistrySingleRole', async () => { it('should revert when sender is not grantor or approved', async () => { await expect( SftRolesRegistry.connect(anotherUser).revokeRoleFrom( - GrantRoleData.recordId, + GrantRoleData.commitmentId, GrantRoleData.role, grantee.address, ), @@ -332,7 +332,7 @@ describe('SftRolesRegistrySingleRole', async () => { it('should revert when the grantee is not the same', async () => { await expect( SftRolesRegistry.connect(grantor).revokeRoleFrom( - GrantRoleData.recordId, + GrantRoleData.commitmentId, GrantRoleData.role, anotherUser.address, ), @@ -340,19 +340,19 @@ describe('SftRolesRegistrySingleRole', async () => { }) it('should revert when role is not expired and is not revocable', async () => { - const newRecordId = 2 - await MockToken.mint(grantor.address, RecordCreated.tokenId, RecordCreated.tokenAmount) + const newcommitmentId = 2 + await MockToken.mint(grantor.address, CommitmentCreated.tokenId, CommitmentCreated.tokenAmount) await expect( - SftRolesRegistry.connect(grantor).createRecordFrom( - RecordCreated.grantor, - RecordCreated.tokenAddress, - RecordCreated.tokenId, - RecordCreated.tokenAmount, + SftRolesRegistry.connect(grantor).commitTokens( + CommitmentCreated.grantor, + CommitmentCreated.tokenAddress, + CommitmentCreated.tokenId, + CommitmentCreated.tokenAmount, ), ).to.not.be.reverted await expect( SftRolesRegistry.connect(grantor).grantRole( - newRecordId, + newcommitmentId, GrantRoleData.role, GrantRoleData.grantee, GrantRoleData.expirationDate, @@ -361,14 +361,14 @@ describe('SftRolesRegistrySingleRole', async () => { ), ).to.not.be.reverted await expect( - SftRolesRegistry.connect(grantor).revokeRoleFrom(newRecordId, GrantRoleData.role, GrantRoleData.grantee), + SftRolesRegistry.connect(grantor).revokeRoleFrom(newcommitmentId, GrantRoleData.role, GrantRoleData.grantee), ).to.be.revertedWith('SftRolesRegistry: role is not expired and is not revocable') }) it('should revoke role when sender is grantee, and role is not expired nor revocable', async () => { await expect( SftRolesRegistry.connect(grantor).grantRole( - GrantRoleData.recordId, + GrantRoleData.commitmentId, GrantRoleData.role, GrantRoleData.grantee, GrantRoleData.expirationDate, @@ -379,109 +379,109 @@ describe('SftRolesRegistrySingleRole', async () => { await expect( SftRolesRegistry.connect(grantee).revokeRoleFrom( - GrantRoleData.recordId, + GrantRoleData.commitmentId, GrantRoleData.role, GrantRoleData.grantee, ), ) .to.emit(SftRolesRegistry, 'RoleRevoked') - .withArgs(GrantRoleData.recordId, GrantRoleData.role, GrantRoleData.grantee) + .withArgs(GrantRoleData.commitmentId, GrantRoleData.role, GrantRoleData.grantee) }) it('should revoke role when sender is grantor', async () => { await expect( SftRolesRegistry.connect(grantor).revokeRoleFrom( - GrantRoleData.recordId, + GrantRoleData.commitmentId, GrantRoleData.role, GrantRoleData.grantee, ), ) .to.emit(SftRolesRegistry, 'RoleRevoked') - .withArgs(GrantRoleData.recordId, GrantRoleData.role, GrantRoleData.grantee) + .withArgs(GrantRoleData.commitmentId, GrantRoleData.role, GrantRoleData.grantee) }) it('should revoke role when sender is grantee', async () => { await expect( SftRolesRegistry.connect(grantee).revokeRoleFrom( - GrantRoleData.recordId, + GrantRoleData.commitmentId, GrantRoleData.role, GrantRoleData.grantee, ), ) .to.emit(SftRolesRegistry, 'RoleRevoked') - .withArgs(GrantRoleData.recordId, GrantRoleData.role, GrantRoleData.grantee) + .withArgs(GrantRoleData.commitmentId, GrantRoleData.role, GrantRoleData.grantee) }) it('should revoke role when sender is approved by grantor', async () => { await SftRolesRegistry.connect(grantor).setRoleApprovalForAll( - RecordCreated.tokenAddress, + CommitmentCreated.tokenAddress, anotherUser.address, true, ) await expect( SftRolesRegistry.connect(anotherUser).revokeRoleFrom( - GrantRoleData.recordId, + GrantRoleData.commitmentId, GrantRoleData.role, GrantRoleData.grantee, ), ) .to.emit(SftRolesRegistry, 'RoleRevoked') - .withArgs(GrantRoleData.recordId, GrantRoleData.role, GrantRoleData.grantee) + .withArgs(GrantRoleData.commitmentId, GrantRoleData.role, GrantRoleData.grantee) }) it('should revoke role when sender is approved by grantee', async () => { await SftRolesRegistry.connect(grantee).setRoleApprovalForAll( - RecordCreated.tokenAddress, + CommitmentCreated.tokenAddress, anotherUser.address, true, ) await expect( SftRolesRegistry.connect(anotherUser).revokeRoleFrom( - GrantRoleData.recordId, + GrantRoleData.commitmentId, GrantRoleData.role, GrantRoleData.grantee, ), ) .to.emit(SftRolesRegistry, 'RoleRevoked') - .withArgs(GrantRoleData.recordId, GrantRoleData.role, GrantRoleData.grantee) + .withArgs(GrantRoleData.commitmentId, GrantRoleData.role, GrantRoleData.grantee) }) }) describe('withdrawFrom', async () => { - let RecordCreated: Record + let CommitmentCreated: Commitment let GrantRoleData: GrantRoleData beforeEach(async () => { - RecordCreated = buildRecord({ + CommitmentCreated = buildCommitment({ grantor: grantor.address, tokenAddress: MockToken.address, }) GrantRoleData = await buildGrantRole({ - recordId: 1, + commitmentId: 1, grantee: grantee.address, }) - await MockToken.mint(grantor.address, RecordCreated.tokenId, RecordCreated.tokenAmount) + await MockToken.mint(grantor.address, CommitmentCreated.tokenId, CommitmentCreated.tokenAmount) await MockToken.connect(grantor).setApprovalForAll(SftRolesRegistry.address, true) await expect( - SftRolesRegistry.connect(grantor).createRecordFrom( - RecordCreated.grantor, - RecordCreated.tokenAddress, - RecordCreated.tokenId, - RecordCreated.tokenAmount, + SftRolesRegistry.connect(grantor).commitTokens( + CommitmentCreated.grantor, + CommitmentCreated.tokenAddress, + CommitmentCreated.tokenId, + CommitmentCreated.tokenAmount, ), ).to.not.be.reverted }) it('should revert when sender is not grantor or approved', async () => { - await expect(SftRolesRegistry.connect(anotherUser).withdrawFrom(GrantRoleData.recordId)).to.be.revertedWith( + await expect(SftRolesRegistry.connect(anotherUser).withdrawFrom(GrantRoleData.commitmentId)).to.be.revertedWith( 'SftRolesRegistry: account not approved', ) }) - it('should revert when record has an active role', async () => { + it('should revert when commitment has an active role', async () => { await expect( SftRolesRegistry.connect(grantor).grantRole( - GrantRoleData.recordId, + GrantRoleData.commitmentId, GrantRoleData.role, GrantRoleData.grantee, GrantRoleData.expirationDate, @@ -489,70 +489,70 @@ describe('SftRolesRegistrySingleRole', async () => { GrantRoleData.data, ), ).to.not.be.reverted - await expect(SftRolesRegistry.connect(grantor).withdrawFrom(GrantRoleData.recordId)).to.be.revertedWith( + await expect(SftRolesRegistry.connect(grantor).withdrawFrom(GrantRoleData.commitmentId)).to.be.revertedWith( 'SftRolesRegistry: token has an active role', ) }) it('should withdraw tokens when sender is grantor', async () => { - await expect(SftRolesRegistry.connect(grantor).withdrawFrom(GrantRoleData.recordId)) + await expect(SftRolesRegistry.connect(grantor).withdrawFrom(GrantRoleData.commitmentId)) .to.emit(SftRolesRegistry, 'Withdrew') - .withArgs(GrantRoleData.recordId) + .withArgs(GrantRoleData.commitmentId) .to.emit(MockToken, 'TransferSingle') .withArgs( SftRolesRegistry.address, SftRolesRegistry.address, grantor.address, - RecordCreated.tokenId, - RecordCreated.tokenAmount, + CommitmentCreated.tokenId, + CommitmentCreated.tokenAmount, ) }) it('should withdraw tokens when sender is approved', async () => { await SftRolesRegistry.connect(grantor).setRoleApprovalForAll( - RecordCreated.tokenAddress, + CommitmentCreated.tokenAddress, anotherUser.address, true, ) - await expect(SftRolesRegistry.connect(anotherUser).withdrawFrom(GrantRoleData.recordId)) + await expect(SftRolesRegistry.connect(anotherUser).withdrawFrom(GrantRoleData.commitmentId)) .to.emit(SftRolesRegistry, 'Withdrew') - .withArgs(GrantRoleData.recordId) + .withArgs(GrantRoleData.commitmentId) .to.emit(MockToken, 'TransferSingle') .withArgs( SftRolesRegistry.address, SftRolesRegistry.address, grantor.address, - RecordCreated.tokenId, - RecordCreated.tokenAmount, + CommitmentCreated.tokenId, + CommitmentCreated.tokenAmount, ) }) }) describe('view functions', async () => { - let RecordCreated: Record + let CommitmentCreated: Commitment let GrantRoleData: GrantRoleData beforeEach(async () => { - RecordCreated = await assertCreateRecordEvent(SftRolesRegistry, MockToken, grantor, 1) + CommitmentCreated = await assertCreateCommitmentEvent(SftRolesRegistry, MockToken, grantor, 1) GrantRoleData = await assertGrantRoleEvent(SftRolesRegistry, grantor, 1, grantee.address) }) - it('recordInfo', async () => { - const record = await SftRolesRegistry.recordInfo(GrantRoleData.recordId) - expect(record.grantor_).to.be.equal(RecordCreated.grantor) - expect(record.tokenAddress_).to.be.equal(RecordCreated.tokenAddress) - expect(record.tokenId_).to.be.equal(RecordCreated.tokenId) - expect(record.tokenAmount_).to.be.equal(RecordCreated.tokenAmount) + it('commitmentInfo', async () => { + const commitment = await SftRolesRegistry.commitmentInfo(GrantRoleData.commitmentId) + expect(commitment.grantor_).to.be.equal(CommitmentCreated.grantor) + expect(commitment.tokenAddress_).to.be.equal(CommitmentCreated.tokenAddress) + expect(commitment.tokenId_).to.be.equal(CommitmentCreated.tokenId) + expect(commitment.tokenAmount_).to.be.equal(CommitmentCreated.tokenAmount) }) it('should revert when grantee is not the same', async () => { await expect( - SftRolesRegistry.connect(grantor).roleData(GrantRoleData.recordId, GrantRoleData.role, anotherUser.address), + SftRolesRegistry.connect(grantor).roleData(GrantRoleData.commitmentId, GrantRoleData.role, anotherUser.address), ).to.be.revertedWith('SftRolesRegistry: grantee mismatch') await expect( SftRolesRegistry.connect(grantor).roleExpirationDate( - GrantRoleData.recordId, + GrantRoleData.commitmentId, GrantRoleData.role, anotherUser.address, ), @@ -560,7 +560,7 @@ describe('SftRolesRegistrySingleRole', async () => { await expect( SftRolesRegistry.connect(grantor).isRoleRevocable( - GrantRoleData.recordId, + GrantRoleData.commitmentId, GrantRoleData.role, anotherUser.address, ), @@ -570,7 +570,7 @@ describe('SftRolesRegistrySingleRole', async () => { it('should return role data', async () => { expect( await SftRolesRegistry.connect(grantor).roleExpirationDate( - GrantRoleData.recordId, + GrantRoleData.commitmentId, GrantRoleData.role, GrantRoleData.grantee, ), @@ -578,7 +578,7 @@ describe('SftRolesRegistrySingleRole', async () => { expect( await SftRolesRegistry.connect(grantor).roleData( - GrantRoleData.recordId, + GrantRoleData.commitmentId, GrantRoleData.role, GrantRoleData.grantee, ), @@ -586,7 +586,7 @@ describe('SftRolesRegistrySingleRole', async () => { expect( await SftRolesRegistry.connect(grantor).isRoleRevocable( - GrantRoleData.recordId, + GrantRoleData.commitmentId, GrantRoleData.role, GrantRoleData.grantee, ), @@ -599,8 +599,9 @@ describe('SftRolesRegistrySingleRole', async () => { expect(await SftRolesRegistry.supportsInterface('0x4e2312e0')).to.be.true }) - it('should return true if IERCXXXX interface id', async () => { - expect(await SftRolesRegistry.supportsInterface('0xf254051c')).to.be.true + it('should return true if SftRolesRegistry interface id', async () => { + const interfaceId = getSftRolesRegistryInterfaceId() + expect(await SftRolesRegistry.supportsInterface(interfaceId)).to.be.true }) }) }) diff --git a/test/SftRolesRegistry/helpers/assertEvents.ts b/test/SftRolesRegistry/helpers/assertEvents.ts index e494e38..4449fb2 100644 --- a/test/SftRolesRegistry/helpers/assertEvents.ts +++ b/test/SftRolesRegistry/helpers/assertEvents.ts @@ -1,61 +1,73 @@ -import { buildGrantRole, buildRecord } from './mockData' +import { buildGrantRole, buildCommitment } from './mockData' import { expect } from 'chai' import { Contract } from 'ethers' import { SignerWithAddress } from '@nomiclabs/hardhat-ethers/signers' -export async function assertCreateRecordEvent( +export async function assertCreateCommitmentEvent( SftRolesRegistry: Contract, MockToken: Contract, grantor: SignerWithAddress, - expectedRecordId: number, + expectedcommitmentId: number, anotherUser?: SignerWithAddress, ) { - const record = buildRecord({ + const commitment = buildCommitment({ grantor: grantor.address, tokenAddress: MockToken.address, }) - await MockToken.mint(grantor.address, record.tokenId, record.tokenAmount) + await MockToken.mint(grantor.address, commitment.tokenId, commitment.tokenAmount) await MockToken.connect(grantor).setApprovalForAll(SftRolesRegistry.address, true) if (anotherUser) { - await SftRolesRegistry.connect(grantor).setRoleApprovalForAll(record.tokenAddress, anotherUser.address, true) + await SftRolesRegistry.connect(grantor).setRoleApprovalForAll(commitment.tokenAddress, anotherUser.address, true) } await expect( - SftRolesRegistry.connect(anotherUser || grantor).createRecordFrom( - record.grantor, - record.tokenAddress, - record.tokenId, - record.tokenAmount, + SftRolesRegistry.connect(anotherUser || grantor).commitTokens( + commitment.grantor, + commitment.tokenAddress, + commitment.tokenId, + commitment.tokenAmount, ), ) - .to.emit(SftRolesRegistry, 'RecordCreated') - .withArgs(record.grantor, expectedRecordId, record.tokenAddress, record.tokenId, record.tokenAmount) + .to.emit(SftRolesRegistry, 'TokensCommitted') + .withArgs( + commitment.grantor, + expectedcommitmentId, + commitment.tokenAddress, + commitment.tokenId, + commitment.tokenAmount, + ) .to.emit(MockToken, 'TransferSingle') - .withArgs(SftRolesRegistry.address, grantor.address, SftRolesRegistry.address, record.tokenId, record.tokenAmount) - return { ...record, recordId: expectedRecordId } + .withArgs( + SftRolesRegistry.address, + grantor.address, + SftRolesRegistry.address, + commitment.tokenId, + commitment.tokenAmount, + ) + return { ...commitment, commitmentId: expectedcommitmentId } } export async function assertGrantRoleEvent( SftRolesRegistry: Contract, grantor: SignerWithAddress, - recordId: number, + commitmentId: number, grantee: string, revocable = true, anotherUser?: SignerWithAddress, ) { const grantRoleData = await buildGrantRole({ - recordId, + commitmentId, grantee, revocable, }) if (anotherUser) { - const record = await SftRolesRegistry.recordInfo(recordId) - await SftRolesRegistry.connect(grantor).setRoleApprovalForAll(record.tokenAddress_, anotherUser.address, true) + const commitment = await SftRolesRegistry.commitmentInfo(commitmentId) + await SftRolesRegistry.connect(grantor).setRoleApprovalForAll(commitment.tokenAddress_, anotherUser.address, true) } await expect( SftRolesRegistry.connect(anotherUser || grantor).grantRole( - recordId, + commitmentId, grantRoleData.role, grantee, grantRoleData.expirationDate, @@ -65,7 +77,7 @@ export async function assertGrantRoleEvent( ) .to.emit(SftRolesRegistry, 'RoleGranted') .withArgs( - recordId, + commitmentId, grantRoleData.role, grantee, grantRoleData.expirationDate, @@ -78,17 +90,17 @@ export async function assertGrantRoleEvent( export async function assertRevokeRoleEvent( SftRolesRegistry: Contract, grantor: SignerWithAddress, - recordId: number, + commitmentId: number, role: string, grantee: SignerWithAddress, revoker?: SignerWithAddress, ) { if (revoker) { - const record = await SftRolesRegistry.recordInfo(recordId) - await SftRolesRegistry.connect(grantor).setRoleApprovalForAll(record.tokenAddress_, revoker.address, true) + const commitment = await SftRolesRegistry.commitmentInfo(commitmentId) + await SftRolesRegistry.connect(grantor).setRoleApprovalForAll(commitment.tokenAddress_, revoker.address, true) } - await expect(SftRolesRegistry.connect(revoker || grantor).revokeRoleFrom(recordId, role, grantee.address)) + await expect(SftRolesRegistry.connect(revoker || grantor).revokeRoleFrom(commitmentId, role, grantee.address)) .to.emit(SftRolesRegistry, 'RoleRevoked') - .withArgs(recordId, role, grantee.address) - return { recordId, role, grantee: grantee.address } + .withArgs(commitmentId, role, grantee.address) + return { commitmentId, role, grantee: grantee.address } } diff --git a/test/SftRolesRegistry/helpers/mockData.ts b/test/SftRolesRegistry/helpers/mockData.ts index 6330e23..c502d6b 100644 --- a/test/SftRolesRegistry/helpers/mockData.ts +++ b/test/SftRolesRegistry/helpers/mockData.ts @@ -1,23 +1,24 @@ import { solidityKeccak256 } from 'ethers/lib/utils' -import { GrantRoleData, Record, RevokeRoleData, RoleAssignment } from '../types' +import { GrantRoleData, Commitment, RoleAssignment } from '../types' import { generateRandomInt } from '../../helpers' -import { ethers } from 'hardhat' import { time } from '@nomicfoundation/hardhat-network-helpers' +import { ethers } from 'ethers' +import { ISftRolesRegistry__factory } from '../../../typechain-types' const { HashZero, AddressZero } = ethers.constants export const ONE_DAY = 60 * 60 * 24 -export function buildRecord({ +export function buildCommitment({ grantor = AddressZero, tokenAddress = AddressZero, tokenId = generateRandomInt(), tokenAmount = generateRandomInt(), -}): Record { +}): Commitment { return { grantor, tokenAddress, tokenId, tokenAmount } } export async function buildGrantRole({ - recordId = generateRandomInt(), + commitmentId = generateRandomInt(), role = 'UNIQUE_ROLE', grantee = AddressZero, expirationDate = null, @@ -25,7 +26,7 @@ export async function buildGrantRole({ data = HashZero, }): Promise { return { - recordId, + commitmentId, role: generateRoleId(role), grantee, expirationDate: expirationDate ? expirationDate : (await time.latest()) + ONE_DAY, @@ -76,13 +77,17 @@ export async function buildRoleAssignment({ data, } } -export function buildRevokeRoleData(roleAssignment: RoleAssignment): RevokeRoleData { - return { - nonce: roleAssignment.nonce, - role: roleAssignment.role, - tokenAddress: roleAssignment.tokenAddress, - tokenId: roleAssignment.tokenId, - grantor: roleAssignment.grantor, - grantee: roleAssignment.grantee, + +export function getSftRolesRegistryInterfaceId() { + const interfaceI = ISftRolesRegistry__factory.createInterface() + return generateErc165InterfaceId(interfaceI) +} + +function generateErc165InterfaceId(contractInterface: ethers.utils.Interface) { + let interfaceID = ethers.constants.Zero + const functions: string[] = Object.keys(contractInterface.functions).filter(f => f !== 'supportsInterface(bytes4)') + for (let i = 0; i < functions.length; i++) { + interfaceID = interfaceID.xor(contractInterface.getSighash(functions[i])) } + return interfaceID } diff --git a/test/SftRolesRegistry/types.ts b/test/SftRolesRegistry/types.ts index 6acc1b7..abf0f3c 100644 --- a/test/SftRolesRegistry/types.ts +++ b/test/SftRolesRegistry/types.ts @@ -1,4 +1,4 @@ -export interface Record { +export interface Commitment { grantor: string tokenAddress: string tokenId: number @@ -6,7 +6,7 @@ export interface Record { } export interface GrantRoleData { - recordId: number + commitmentId: number role: string grantee: string expirationDate: number From 3f789d63faa7200f69b1d05befa7234112f3b902 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ernani=20S=C3=A3o=20Thiago?= Date: Tue, 26 Dec 2023 17:25:52 -0500 Subject: [PATCH 05/19] renamed revokeRoleFrom to revokeRole --- contracts/RolesRegistry/SftRolesRegistry.sol | 2 +- .../RolesRegistry/SftRolesRegistrySingleRole.sol | 2 +- .../interfaces/ISftRolesRegistry.sol | 4 ++-- test/SftRolesRegistry/SftRolesRegistry.spec.ts | 14 +++++++------- .../SftRolesRegistrySingleRole.spec.ts | 16 ++++++++-------- test/SftRolesRegistry/helpers/assertEvents.ts | 2 +- 6 files changed, 20 insertions(+), 20 deletions(-) diff --git a/contracts/RolesRegistry/SftRolesRegistry.sol b/contracts/RolesRegistry/SftRolesRegistry.sol index 14c35c0..a042ce9 100644 --- a/contracts/RolesRegistry/SftRolesRegistry.sol +++ b/contracts/RolesRegistry/SftRolesRegistry.sol @@ -69,7 +69,7 @@ contract SftRolesRegistry is ISftRolesRegistry, ERC1155Holder { _grantOrUpdateRole(_commitmentId, _role, _grantee, _expirationDate, _revocable, _data); } - function revokeRoleFrom(uint256 _commitmentId, bytes32 _role, address _grantee) external override { + function revokeRole(uint256 _commitmentId, bytes32 _role, address _grantee) external override { uint256 itemId = _getItemId(_commitmentId, _role, _grantee); LinkedLists.RoleData storage data = lists.items[itemId].data; require(data.expirationDate > 0, 'SftRolesRegistry: could not find role assignment'); diff --git a/contracts/RolesRegistry/SftRolesRegistrySingleRole.sol b/contracts/RolesRegistry/SftRolesRegistrySingleRole.sol index e5f57a8..4104a27 100644 --- a/contracts/RolesRegistry/SftRolesRegistrySingleRole.sol +++ b/contracts/RolesRegistry/SftRolesRegistrySingleRole.sol @@ -73,7 +73,7 @@ contract SftRolesRegistrySingleRole is ISftRolesRegistry, ERC1155Holder { _grantOrUpdateRole(_commitmentId, _role, _grantee, _expirationDate, _revocable, _data); } - function revokeRoleFrom( + function revokeRole( uint256 _commitmentId, bytes32 _role, address _grantee diff --git a/contracts/RolesRegistry/interfaces/ISftRolesRegistry.sol b/contracts/RolesRegistry/interfaces/ISftRolesRegistry.sol index 347f539..43c5f2f 100644 --- a/contracts/RolesRegistry/interfaces/ISftRolesRegistry.sol +++ b/contracts/RolesRegistry/interfaces/ISftRolesRegistry.sol @@ -98,11 +98,11 @@ interface ISftRolesRegistry is IERC165 { bytes calldata _data ) external; - /// @notice Revokes a role on behalf of a user. + /// @notice Revokes a role. /// @param _commitmentId The commitment identifier. /// @param _role The role identifier. /// @param _grantee The user that gets their role revoked. - function revokeRoleFrom(uint256 _commitmentId, bytes32 _role, address _grantee) external; + function revokeRole(uint256 _commitmentId, bytes32 _role, address _grantee) external; /// @notice Withdraws tokens back to grantor. /// @param _commitmentId The commitment identifier. diff --git a/test/SftRolesRegistry/SftRolesRegistry.spec.ts b/test/SftRolesRegistry/SftRolesRegistry.spec.ts index 0d37b30..8dc7804 100644 --- a/test/SftRolesRegistry/SftRolesRegistry.spec.ts +++ b/test/SftRolesRegistry/SftRolesRegistry.spec.ts @@ -209,7 +209,7 @@ describe('SftRolesRegistry', async () => { it('should revert when sender is not grantor or approved', async () => { await expect( - SftRolesRegistry.connect(anotherUser).revokeRoleFrom( + SftRolesRegistry.connect(anotherUser).revokeRole( GrantRoleData.commitmentId, GrantRoleData.role, GrantRoleData.grantee, @@ -220,7 +220,7 @@ describe('SftRolesRegistry', async () => { it('should revert when role assignment does not exist', async () => { // different commitmentId await expect( - SftRolesRegistry.connect(grantor).revokeRoleFrom( + SftRolesRegistry.connect(grantor).revokeRole( GrantRoleData.commitmentId + 1, GrantRoleData.role, GrantRoleData.grantee, @@ -229,7 +229,7 @@ describe('SftRolesRegistry', async () => { // different role await expect( - SftRolesRegistry.connect(grantor).revokeRoleFrom( + SftRolesRegistry.connect(grantor).revokeRole( GrantRoleData.commitmentId, generateRoleId('ANOTHER_ROLE'), GrantRoleData.grantee, @@ -238,7 +238,7 @@ describe('SftRolesRegistry', async () => { // different grantee await expect( - SftRolesRegistry.connect(grantor).revokeRoleFrom( + SftRolesRegistry.connect(grantor).revokeRole( GrantRoleData.commitmentId + 1, GrantRoleData.role, anotherUser.address, @@ -255,7 +255,7 @@ describe('SftRolesRegistry', async () => { false, ) await expect( - SftRolesRegistry.connect(grantor).revokeRoleFrom( + SftRolesRegistry.connect(grantor).revokeRole( roleAssignment.commitmentId, GrantRoleData.role, GrantRoleData.grantee, @@ -306,7 +306,7 @@ describe('SftRolesRegistry', async () => { ) await expect( - SftRolesRegistry.connect(anotherUser).revokeRoleFrom( + SftRolesRegistry.connect(anotherUser).revokeRole( GrantRoleData.commitmentId, GrantRoleData.role, grantee.address, @@ -323,7 +323,7 @@ describe('SftRolesRegistry', async () => { true, ) await expect( - SftRolesRegistry.connect(anotherUser).revokeRoleFrom( + SftRolesRegistry.connect(anotherUser).revokeRole( GrantRoleData.commitmentId, GrantRoleData.role, grantee.address, diff --git a/test/SftRolesRegistry/SftRolesRegistrySingleRole.spec.ts b/test/SftRolesRegistry/SftRolesRegistrySingleRole.spec.ts index c34a380..eb1798d 100644 --- a/test/SftRolesRegistry/SftRolesRegistrySingleRole.spec.ts +++ b/test/SftRolesRegistry/SftRolesRegistrySingleRole.spec.ts @@ -321,7 +321,7 @@ describe('SftRolesRegistrySingleRole', async () => { it('should revert when sender is not grantor or approved', async () => { await expect( - SftRolesRegistry.connect(anotherUser).revokeRoleFrom( + SftRolesRegistry.connect(anotherUser).revokeRole( GrantRoleData.commitmentId, GrantRoleData.role, grantee.address, @@ -331,7 +331,7 @@ describe('SftRolesRegistrySingleRole', async () => { it('should revert when the grantee is not the same', async () => { await expect( - SftRolesRegistry.connect(grantor).revokeRoleFrom( + SftRolesRegistry.connect(grantor).revokeRole( GrantRoleData.commitmentId, GrantRoleData.role, anotherUser.address, @@ -361,7 +361,7 @@ describe('SftRolesRegistrySingleRole', async () => { ), ).to.not.be.reverted await expect( - SftRolesRegistry.connect(grantor).revokeRoleFrom(newcommitmentId, GrantRoleData.role, GrantRoleData.grantee), + SftRolesRegistry.connect(grantor).revokeRole(newcommitmentId, GrantRoleData.role, GrantRoleData.grantee), ).to.be.revertedWith('SftRolesRegistry: role is not expired and is not revocable') }) @@ -378,7 +378,7 @@ describe('SftRolesRegistrySingleRole', async () => { ).to.not.be.reverted await expect( - SftRolesRegistry.connect(grantee).revokeRoleFrom( + SftRolesRegistry.connect(grantee).revokeRole( GrantRoleData.commitmentId, GrantRoleData.role, GrantRoleData.grantee, @@ -390,7 +390,7 @@ describe('SftRolesRegistrySingleRole', async () => { it('should revoke role when sender is grantor', async () => { await expect( - SftRolesRegistry.connect(grantor).revokeRoleFrom( + SftRolesRegistry.connect(grantor).revokeRole( GrantRoleData.commitmentId, GrantRoleData.role, GrantRoleData.grantee, @@ -402,7 +402,7 @@ describe('SftRolesRegistrySingleRole', async () => { it('should revoke role when sender is grantee', async () => { await expect( - SftRolesRegistry.connect(grantee).revokeRoleFrom( + SftRolesRegistry.connect(grantee).revokeRole( GrantRoleData.commitmentId, GrantRoleData.role, GrantRoleData.grantee, @@ -419,7 +419,7 @@ describe('SftRolesRegistrySingleRole', async () => { true, ) await expect( - SftRolesRegistry.connect(anotherUser).revokeRoleFrom( + SftRolesRegistry.connect(anotherUser).revokeRole( GrantRoleData.commitmentId, GrantRoleData.role, GrantRoleData.grantee, @@ -436,7 +436,7 @@ describe('SftRolesRegistrySingleRole', async () => { true, ) await expect( - SftRolesRegistry.connect(anotherUser).revokeRoleFrom( + SftRolesRegistry.connect(anotherUser).revokeRole( GrantRoleData.commitmentId, GrantRoleData.role, GrantRoleData.grantee, diff --git a/test/SftRolesRegistry/helpers/assertEvents.ts b/test/SftRolesRegistry/helpers/assertEvents.ts index 4449fb2..557df04 100644 --- a/test/SftRolesRegistry/helpers/assertEvents.ts +++ b/test/SftRolesRegistry/helpers/assertEvents.ts @@ -99,7 +99,7 @@ export async function assertRevokeRoleEvent( const commitment = await SftRolesRegistry.commitmentInfo(commitmentId) await SftRolesRegistry.connect(grantor).setRoleApprovalForAll(commitment.tokenAddress_, revoker.address, true) } - await expect(SftRolesRegistry.connect(revoker || grantor).revokeRoleFrom(commitmentId, role, grantee.address)) + await expect(SftRolesRegistry.connect(revoker || grantor).revokeRole(commitmentId, role, grantee.address)) .to.emit(SftRolesRegistry, 'RoleRevoked') .withArgs(commitmentId, role, grantee.address) return { commitmentId, role, grantee: grantee.address } From 6b4b3869d1a1c8ce0221cb3ea045712fe0edc472 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ernani=20S=C3=A3o=20Thiago?= Date: Tue, 26 Dec 2023 17:58:36 -0500 Subject: [PATCH 06/19] renamed withdrawFrom and Withdrew to withdrawNfts and NftsWithdrawn respectively --- contracts/RolesRegistry/SftRolesRegistry.sol | 4 ++-- .../SftRolesRegistrySingleRole.sol | 4 ++-- .../interfaces/ISftRolesRegistry.sol | 6 ++--- .../SftRolesRegistry/SftRolesRegistry.spec.ts | 24 +++++++++---------- .../SftRolesRegistrySingleRole.spec.ts | 18 +++++++------- 5 files changed, 28 insertions(+), 28 deletions(-) diff --git a/contracts/RolesRegistry/SftRolesRegistry.sol b/contracts/RolesRegistry/SftRolesRegistry.sol index a042ce9..ab8018e 100644 --- a/contracts/RolesRegistry/SftRolesRegistry.sol +++ b/contracts/RolesRegistry/SftRolesRegistry.sol @@ -92,7 +92,7 @@ contract SftRolesRegistry is ISftRolesRegistry, ERC1155Holder { emit RoleRevoked(_commitmentId, _role, _grantee); } - function withdrawFrom( + function withdrawNfts( uint256 _commitmentId ) external onlyOwnerOrApproved(commitments[_commitmentId].grantor, commitments[_commitmentId].tokenAddress) { uint256 numberOfRoles = commitmentIdToRoles[_commitmentId].length(); @@ -128,7 +128,7 @@ contract SftRolesRegistry is ISftRolesRegistry, ERC1155Holder { ); delete commitments[_commitmentId]; - emit Withdrew(_commitmentId); + emit NftsWithdrawn(_commitmentId); } function setRoleApprovalForAll(address _tokenAddress, address _operator, bool _isApproved) external override { diff --git a/contracts/RolesRegistry/SftRolesRegistrySingleRole.sol b/contracts/RolesRegistry/SftRolesRegistrySingleRole.sol index 4104a27..7957590 100644 --- a/contracts/RolesRegistry/SftRolesRegistrySingleRole.sol +++ b/contracts/RolesRegistry/SftRolesRegistrySingleRole.sol @@ -89,7 +89,7 @@ contract SftRolesRegistrySingleRole is ISftRolesRegistry, ERC1155Holder { delete roleAssignments[_commitmentId][_role]; } - function withdrawFrom( + function withdrawNfts( uint256 _commitmentId ) external onlyOwnerOrApproved(commitments[_commitmentId].grantor, commitments[_commitmentId].tokenAddress) { require( @@ -108,7 +108,7 @@ contract SftRolesRegistrySingleRole is ISftRolesRegistry, ERC1155Holder { ); delete commitments[_commitmentId]; - emit Withdrew(_commitmentId); + emit NftsWithdrawn(_commitmentId); } function setRoleApprovalForAll(address _tokenAddress, address _operator, bool _isApproved) external override { diff --git a/contracts/RolesRegistry/interfaces/ISftRolesRegistry.sol b/contracts/RolesRegistry/interfaces/ISftRolesRegistry.sol index 43c5f2f..d405cfa 100644 --- a/contracts/RolesRegistry/interfaces/ISftRolesRegistry.sol +++ b/contracts/RolesRegistry/interfaces/ISftRolesRegistry.sol @@ -57,9 +57,9 @@ interface ISftRolesRegistry is IERC165 { /// @param _grantee The user that receives the role revocation. event RoleRevoked(uint256 indexed _commitmentId, bytes32 indexed _role, address indexed _grantee); - /// @notice Emitted when a user withdraws tokens from a commitment. + /// @notice Emitted when a user withdrawNfts tokens from a commitment. /// @param _commitmentId The commitment identifier. - event Withdrew(uint256 indexed _commitmentId); + event NftsWithdrawn(uint256 indexed _commitmentId); /// @notice Emitted when a user is approved to manage roles on behalf of another user. /// @param _tokenAddress The token address. @@ -106,7 +106,7 @@ interface ISftRolesRegistry is IERC165 { /// @notice Withdraws tokens back to grantor. /// @param _commitmentId The commitment identifier. - function withdrawFrom(uint256 _commitmentId) external; + function withdrawNfts(uint256 _commitmentId) external; /// @notice Approves operator to grant and revoke roles on behalf of another user. /// @param _tokenAddress The token address. diff --git a/test/SftRolesRegistry/SftRolesRegistry.spec.ts b/test/SftRolesRegistry/SftRolesRegistry.spec.ts index 8dc7804..c7486f9 100644 --- a/test/SftRolesRegistry/SftRolesRegistry.spec.ts +++ b/test/SftRolesRegistry/SftRolesRegistry.spec.ts @@ -334,7 +334,7 @@ describe('SftRolesRegistry', async () => { }) }) - describe('withdrawFrom', async () => { + describe('withdrawNfts', async () => { let GrantRoleData: GrantRoleData beforeEach(async () => { @@ -343,36 +343,36 @@ describe('SftRolesRegistry', async () => { }) it('should revert when sender is not grantor or approved', async () => { - await expect(SftRolesRegistry.connect(anotherUser).withdrawFrom(GrantRoleData.commitmentId)).to.be.revertedWith( + await expect(SftRolesRegistry.connect(anotherUser).withdrawNfts(GrantRoleData.commitmentId)).to.be.revertedWith( 'SftRolesRegistry: account not approved', ) }) it('should revert when there is an active role', async () => { await assertGrantRoleEvent(SftRolesRegistry, grantor, 1, grantee.address, false) - await expect(SftRolesRegistry.connect(grantor).withdrawFrom(GrantRoleData.commitmentId)).to.be.revertedWith( + await expect(SftRolesRegistry.connect(grantor).withdrawNfts(GrantRoleData.commitmentId)).to.be.revertedWith( 'SftRolesRegistry: role is not expired and is not revocable', ) }) - it('should withdraw when there are revocable roles', async () => { - await expect(SftRolesRegistry.connect(grantor).withdrawFrom(GrantRoleData.commitmentId)) - .to.emit(SftRolesRegistry, 'Withdrew') + it('should withdrawNfts when there are revocable roles', async () => { + await expect(SftRolesRegistry.connect(grantor).withdrawNfts(GrantRoleData.commitmentId)) + .to.emit(SftRolesRegistry, 'NftsWithdrawn') .withArgs(GrantRoleData.commitmentId) }) - it('should withdraw when there are no roles', async () => { + it('should withdrawNfts when there are no roles', async () => { await assertRevokeRoleEvent(SftRolesRegistry, grantor, GrantRoleData.commitmentId, GrantRoleData.role, grantee) - await expect(SftRolesRegistry.connect(grantor).withdrawFrom(GrantRoleData.commitmentId)) - .to.emit(SftRolesRegistry, 'Withdrew') + await expect(SftRolesRegistry.connect(grantor).withdrawNfts(GrantRoleData.commitmentId)) + .to.emit(SftRolesRegistry, 'NftsWithdrawn') .withArgs(GrantRoleData.commitmentId) }) - it('should withdraw when there are expired roles', async () => { + it('should withdrawNfts when there are expired roles', async () => { await assertGrantRoleEvent(SftRolesRegistry, grantor, 1, grantee.address, false) await time.increase(ONE_DAY) - await expect(SftRolesRegistry.connect(grantor).withdrawFrom(GrantRoleData.commitmentId)) - .to.emit(SftRolesRegistry, 'Withdrew') + await expect(SftRolesRegistry.connect(grantor).withdrawNfts(GrantRoleData.commitmentId)) + .to.emit(SftRolesRegistry, 'NftsWithdrawn') .withArgs(GrantRoleData.commitmentId) }) }) diff --git a/test/SftRolesRegistry/SftRolesRegistrySingleRole.spec.ts b/test/SftRolesRegistry/SftRolesRegistrySingleRole.spec.ts index eb1798d..c870555 100644 --- a/test/SftRolesRegistry/SftRolesRegistrySingleRole.spec.ts +++ b/test/SftRolesRegistry/SftRolesRegistrySingleRole.spec.ts @@ -447,7 +447,7 @@ describe('SftRolesRegistrySingleRole', async () => { }) }) - describe('withdrawFrom', async () => { + describe('withdrawNfts', async () => { let CommitmentCreated: Commitment let GrantRoleData: GrantRoleData @@ -473,7 +473,7 @@ describe('SftRolesRegistrySingleRole', async () => { }) it('should revert when sender is not grantor or approved', async () => { - await expect(SftRolesRegistry.connect(anotherUser).withdrawFrom(GrantRoleData.commitmentId)).to.be.revertedWith( + await expect(SftRolesRegistry.connect(anotherUser).withdrawNfts(GrantRoleData.commitmentId)).to.be.revertedWith( 'SftRolesRegistry: account not approved', ) }) @@ -489,14 +489,14 @@ describe('SftRolesRegistrySingleRole', async () => { GrantRoleData.data, ), ).to.not.be.reverted - await expect(SftRolesRegistry.connect(grantor).withdrawFrom(GrantRoleData.commitmentId)).to.be.revertedWith( + await expect(SftRolesRegistry.connect(grantor).withdrawNfts(GrantRoleData.commitmentId)).to.be.revertedWith( 'SftRolesRegistry: token has an active role', ) }) - it('should withdraw tokens when sender is grantor', async () => { - await expect(SftRolesRegistry.connect(grantor).withdrawFrom(GrantRoleData.commitmentId)) - .to.emit(SftRolesRegistry, 'Withdrew') + it('should withdrawNfts tokens when sender is grantor', async () => { + await expect(SftRolesRegistry.connect(grantor).withdrawNfts(GrantRoleData.commitmentId)) + .to.emit(SftRolesRegistry, 'NftsWithdrawn') .withArgs(GrantRoleData.commitmentId) .to.emit(MockToken, 'TransferSingle') .withArgs( @@ -508,14 +508,14 @@ describe('SftRolesRegistrySingleRole', async () => { ) }) - it('should withdraw tokens when sender is approved', async () => { + it('should withdrawNfts tokens when sender is approved', async () => { await SftRolesRegistry.connect(grantor).setRoleApprovalForAll( CommitmentCreated.tokenAddress, anotherUser.address, true, ) - await expect(SftRolesRegistry.connect(anotherUser).withdrawFrom(GrantRoleData.commitmentId)) - .to.emit(SftRolesRegistry, 'Withdrew') + await expect(SftRolesRegistry.connect(anotherUser).withdrawNfts(GrantRoleData.commitmentId)) + .to.emit(SftRolesRegistry, 'NftsWithdrawn') .withArgs(GrantRoleData.commitmentId) .to.emit(MockToken, 'TransferSingle') .withArgs( From 2fca9ce71661ec74d7c51572c8087153ef77cae5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ernani=20S=C3=A3o=20Thiago?= Date: Wed, 27 Dec 2023 09:01:57 -0500 Subject: [PATCH 07/19] split function commitmentInfo into four additional functions --- contracts/RolesRegistry/SftRolesRegistry.sol | 22 ++++++++++------- .../SftRolesRegistrySingleRole.sol | 22 ++++++++++------- .../interfaces/ISftRolesRegistry.sol | 18 ++++++++++---- .../SftRolesRegistry/SftRolesRegistry.spec.ts | 24 ++++++++++++++----- .../SftRolesRegistrySingleRole.spec.ts | 24 ++++++++++++++----- test/SftRolesRegistry/helpers/assertEvents.ts | 8 +++---- 6 files changed, 82 insertions(+), 36 deletions(-) diff --git a/contracts/RolesRegistry/SftRolesRegistry.sol b/contracts/RolesRegistry/SftRolesRegistry.sol index ab8018e..942ea96 100644 --- a/contracts/RolesRegistry/SftRolesRegistry.sol +++ b/contracts/RolesRegistry/SftRolesRegistry.sol @@ -138,14 +138,20 @@ contract SftRolesRegistry is ISftRolesRegistry, ERC1155Holder { /** View Functions **/ - function commitmentInfo( - uint256 _commitmentId - ) external view returns (address grantor_, address tokenAddress_, uint256 tokenId_, uint256 tokenAmount_) { - Commitment memory commitment = commitments[_commitmentId]; - grantor_ = commitment.grantor; - tokenAddress_ = commitment.tokenAddress; - tokenId_ = commitment.tokenId; - tokenAmount_ = commitment.tokenAmount; + function grantorOf(uint256 _commitmentId) external view returns (address grantor_) { + grantor_ = commitments[_commitmentId].grantor; + } + + function tokenAddressOf(uint256 _commitmentId) external view returns (address tokenAddress_) { + tokenAddress_ = commitments[_commitmentId].tokenAddress; + } + + function tokenIdOf(uint256 _commitmentId) external view returns (uint256 tokenId_) { + tokenId_ = commitments[_commitmentId].tokenId; + } + + function tokenAmountOf(uint256 _commitmentId) external view returns (uint256 tokenAmount_) { + tokenAmount_ = commitments[_commitmentId].tokenAmount; } function roleData( diff --git a/contracts/RolesRegistry/SftRolesRegistrySingleRole.sol b/contracts/RolesRegistry/SftRolesRegistrySingleRole.sol index 7957590..0189226 100644 --- a/contracts/RolesRegistry/SftRolesRegistrySingleRole.sol +++ b/contracts/RolesRegistry/SftRolesRegistrySingleRole.sol @@ -118,14 +118,20 @@ contract SftRolesRegistrySingleRole is ISftRolesRegistry, ERC1155Holder { /** View Functions **/ - function commitmentInfo( - uint256 _commitmentId - ) external view returns (address grantor_, address tokenAddress_, uint256 tokenId_, uint256 tokenAmount_) { - Commitment memory commitment = commitments[_commitmentId]; - grantor_ = commitment.grantor; - tokenAddress_ = commitment.tokenAddress; - tokenId_ = commitment.tokenId; - tokenAmount_ = commitment.tokenAmount; + function grantorOf(uint256 _commitmentId) external view returns (address grantor_) { + grantor_ = commitments[_commitmentId].grantor; + } + + function tokenAddressOf(uint256 _commitmentId) external view returns (address tokenAddress_) { + tokenAddress_ = commitments[_commitmentId].tokenAddress; + } + + function tokenIdOf(uint256 _commitmentId) external view returns (uint256 tokenId_) { + tokenId_ = commitments[_commitmentId].tokenId; + } + + function tokenAmountOf(uint256 _commitmentId) external view returns (uint256 tokenAmount_) { + tokenAmount_ = commitments[_commitmentId].tokenAmount; } function roleData( diff --git a/contracts/RolesRegistry/interfaces/ISftRolesRegistry.sol b/contracts/RolesRegistry/interfaces/ISftRolesRegistry.sol index d405cfa..79456c7 100644 --- a/contracts/RolesRegistry/interfaces/ISftRolesRegistry.sol +++ b/contracts/RolesRegistry/interfaces/ISftRolesRegistry.sol @@ -116,15 +116,25 @@ interface ISftRolesRegistry is IERC165 { /** View Functions **/ - /// @notice Returns all information regarding a commitment. + /// @notice Returns the owner of the commitment (grantor). /// @param _commitmentId The commitment identifier. /// @return grantor_ The commitment owner. + function grantorOf(uint256 _commitmentId) external view returns (address grantor_); + + /// @notice Returns the address of the token committed. + /// @param _commitmentId The commitment identifier. /// @return tokenAddress_ The token address. + function tokenAddressOf(uint256 _commitmentId) external view returns (address tokenAddress_); + + /// @notice Returns the identifier of the token committed. + /// @param _commitmentId The commitment identifier. /// @return tokenId_ The token identifier. + function tokenIdOf(uint256 _commitmentId) external view returns (uint256 tokenId_); + + /// @notice Returns the amount of tokens committed. + /// @param _commitmentId The commitment identifier. /// @return tokenAmount_ The token amount. - function commitmentInfo( - uint256 _commitmentId - ) external view returns (address grantor_, address tokenAddress_, uint256 tokenId_, uint256 tokenAmount_); + function tokenAmountOf(uint256 _commitmentId) external view returns (uint256 tokenAmount_); /// @notice Returns the custom data of a role assignment. /// @param _commitmentId The commitment identifier. diff --git a/test/SftRolesRegistry/SftRolesRegistry.spec.ts b/test/SftRolesRegistry/SftRolesRegistry.spec.ts index c7486f9..8cb46df 100644 --- a/test/SftRolesRegistry/SftRolesRegistry.spec.ts +++ b/test/SftRolesRegistry/SftRolesRegistry.spec.ts @@ -396,12 +396,24 @@ describe('SftRolesRegistry', async () => { GrantRoleData = await assertGrantRoleEvent(SftRolesRegistry, grantor, 1, grantee.address) }) - it('commitmentInfo', async () => { - const commitment = await SftRolesRegistry.commitmentInfo(GrantRoleData.commitmentId) - expect(commitment.grantor_).to.be.equal(CommitmentCreated.grantor) - expect(commitment.tokenAddress_).to.be.equal(CommitmentCreated.tokenAddress) - expect(commitment.tokenId_).to.be.equal(CommitmentCreated.tokenId) - expect(commitment.tokenAmount_).to.be.equal(CommitmentCreated.tokenAmount) + it('grantorOf', async () => { + expect(await SftRolesRegistry.grantorOf(GrantRoleData.commitmentId)).to.be.equal(CommitmentCreated.grantor) + }) + + it('tokenAddressOf', async () => { + expect(await SftRolesRegistry.tokenAddressOf(GrantRoleData.commitmentId)).to.be.equal( + CommitmentCreated.tokenAddress, + ) + }) + + it('tokenIdOf', async () => { + expect(await SftRolesRegistry.tokenIdOf(GrantRoleData.commitmentId)).to.be.equal(CommitmentCreated.tokenId) + }) + + it('tokenAmountOf', async () => { + expect(await SftRolesRegistry.tokenAmountOf(GrantRoleData.commitmentId)).to.be.equal( + CommitmentCreated.tokenAmount, + ) }) it('roleData', async () => { diff --git a/test/SftRolesRegistry/SftRolesRegistrySingleRole.spec.ts b/test/SftRolesRegistry/SftRolesRegistrySingleRole.spec.ts index c870555..2f1dc40 100644 --- a/test/SftRolesRegistry/SftRolesRegistrySingleRole.spec.ts +++ b/test/SftRolesRegistry/SftRolesRegistrySingleRole.spec.ts @@ -537,12 +537,24 @@ describe('SftRolesRegistrySingleRole', async () => { GrantRoleData = await assertGrantRoleEvent(SftRolesRegistry, grantor, 1, grantee.address) }) - it('commitmentInfo', async () => { - const commitment = await SftRolesRegistry.commitmentInfo(GrantRoleData.commitmentId) - expect(commitment.grantor_).to.be.equal(CommitmentCreated.grantor) - expect(commitment.tokenAddress_).to.be.equal(CommitmentCreated.tokenAddress) - expect(commitment.tokenId_).to.be.equal(CommitmentCreated.tokenId) - expect(commitment.tokenAmount_).to.be.equal(CommitmentCreated.tokenAmount) + it('grantorOf', async () => { + expect(await SftRolesRegistry.grantorOf(GrantRoleData.commitmentId)).to.be.equal(CommitmentCreated.grantor) + }) + + it('tokenAddressOf', async () => { + expect(await SftRolesRegistry.tokenAddressOf(GrantRoleData.commitmentId)).to.be.equal( + CommitmentCreated.tokenAddress, + ) + }) + + it('tokenIdOf', async () => { + expect(await SftRolesRegistry.tokenIdOf(GrantRoleData.commitmentId)).to.be.equal(CommitmentCreated.tokenId) + }) + + it('tokenAmountOf', async () => { + expect(await SftRolesRegistry.tokenAmountOf(GrantRoleData.commitmentId)).to.be.equal( + CommitmentCreated.tokenAmount, + ) }) it('should revert when grantee is not the same', async () => { diff --git a/test/SftRolesRegistry/helpers/assertEvents.ts b/test/SftRolesRegistry/helpers/assertEvents.ts index 557df04..08c56b1 100644 --- a/test/SftRolesRegistry/helpers/assertEvents.ts +++ b/test/SftRolesRegistry/helpers/assertEvents.ts @@ -62,8 +62,8 @@ export async function assertGrantRoleEvent( revocable, }) if (anotherUser) { - const commitment = await SftRolesRegistry.commitmentInfo(commitmentId) - await SftRolesRegistry.connect(grantor).setRoleApprovalForAll(commitment.tokenAddress_, anotherUser.address, true) + const tokenAddress = await SftRolesRegistry.tokenAddressOf(commitmentId) + await SftRolesRegistry.connect(grantor).setRoleApprovalForAll(tokenAddress, anotherUser.address, true) } await expect( SftRolesRegistry.connect(anotherUser || grantor).grantRole( @@ -96,8 +96,8 @@ export async function assertRevokeRoleEvent( revoker?: SignerWithAddress, ) { if (revoker) { - const commitment = await SftRolesRegistry.commitmentInfo(commitmentId) - await SftRolesRegistry.connect(grantor).setRoleApprovalForAll(commitment.tokenAddress_, revoker.address, true) + const tokenAddress = await SftRolesRegistry.tokenAddressOf(commitmentId) + await SftRolesRegistry.connect(grantor).setRoleApprovalForAll(tokenAddress, revoker.address, true) } await expect(SftRolesRegistry.connect(revoker || grantor).revokeRole(commitmentId, role, grantee.address)) .to.emit(SftRolesRegistry, 'RoleRevoked') From 50911c93a4cef1336de447e563d50788c5e24c37 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ernani=20S=C3=A3o=20Thiago?= Date: Wed, 27 Dec 2023 09:37:58 -0500 Subject: [PATCH 08/19] clean up tests, and ensuring tokens can only be withdrawn when there are no non-revocable roles --- contracts/RolesRegistry/SftRolesRegistry.sol | 2 +- .../SftRolesRegistrySingleRole.sol | 5 +- .../RolesRegistry/interfaces/IERCXXXX.sol | 159 ------------ .../SftRolesRegistry/SftRolesRegistry.spec.ts | 37 ++- .../SftRolesRegistrySingleRole.spec.ts | 236 +++--------------- test/SftRolesRegistry/helpers/assertEvents.ts | 6 +- 6 files changed, 75 insertions(+), 370 deletions(-) delete mode 100644 contracts/RolesRegistry/interfaces/IERCXXXX.sol diff --git a/contracts/RolesRegistry/SftRolesRegistry.sol b/contracts/RolesRegistry/SftRolesRegistry.sol index 942ea96..2f34316 100644 --- a/contracts/RolesRegistry/SftRolesRegistry.sol +++ b/contracts/RolesRegistry/SftRolesRegistry.sol @@ -104,7 +104,7 @@ contract SftRolesRegistry is ISftRolesRegistry, ERC1155Holder { LinkedLists.RoleData storage data = lists.items[itemId].data; require( data.expirationDate < block.timestamp || data.revocable, - 'SftRolesRegistry: role is not expired and is not revocable' + 'SftRolesRegistry: commitment has an active non-revocable role' ); // remove from list and storage diff --git a/contracts/RolesRegistry/SftRolesRegistrySingleRole.sol b/contracts/RolesRegistry/SftRolesRegistrySingleRole.sol index 0189226..5082574 100644 --- a/contracts/RolesRegistry/SftRolesRegistrySingleRole.sol +++ b/contracts/RolesRegistry/SftRolesRegistrySingleRole.sol @@ -93,8 +93,9 @@ contract SftRolesRegistrySingleRole is ISftRolesRegistry, ERC1155Holder { uint256 _commitmentId ) external onlyOwnerOrApproved(commitments[_commitmentId].grantor, commitments[_commitmentId].tokenAddress) { require( - roleAssignments[_commitmentId][UNIQUE_ROLE].expirationDate < block.timestamp, - 'SftRolesRegistry: token has an active role' + roleAssignments[_commitmentId][UNIQUE_ROLE].expirationDate < block.timestamp || + roleAssignments[_commitmentId][UNIQUE_ROLE].revocable, + 'SftRolesRegistry: commitment has an active non-revocable role' ); delete roleAssignments[_commitmentId][UNIQUE_ROLE]; diff --git a/contracts/RolesRegistry/interfaces/IERCXXXX.sol b/contracts/RolesRegistry/interfaces/IERCXXXX.sol deleted file mode 100644 index 5e5a23a..0000000 --- a/contracts/RolesRegistry/interfaces/IERCXXXX.sol +++ /dev/null @@ -1,159 +0,0 @@ -// SPDX-License-Identifier: CC0-1.0 - -pragma solidity 0.8.9; - -import { IERC165 } from '@openzeppelin/contracts/utils/introspection/IERC165.sol'; - -/// @title ERC-XXXX Semi-Fungible Token Roles -/// @dev See https://eips.ethereum.org/EIPS/eip-XXXX -/// Note: the ERC-165 identifier for this interface is 0xTBD -interface IERCXXXX is IERC165 { - struct RoleData { - bytes32 hash; - address grantee; - uint256 tokenAmount; - uint64 expirationDate; - bool revocable; - bytes data; - } - - struct DepositInfo { - address grantor; - address tokenAddress; - uint256 tokenId; - uint256 tokenAmount; - } - - struct RoleAssignment { - uint256 nonce; - bytes32 role; - address tokenAddress; - uint256 tokenId; - uint256 tokenAmount; - address grantor; - address grantee; - uint64 expirationDate; - bool revocable; - bytes data; - } - - struct RevokeRoleData { - uint256 nonce; - bytes32 role; - address tokenAddress; - uint256 tokenId; - address grantor; - } - - /** Events **/ - - /// @notice Emitted when a role is granted. - /// @param _nonce The identifier of the role assignment. - /// @param _role The role identifier. - /// @param _tokenAddress The token address. - /// @param _tokenId The token identifier. - /// @param _tokenAmount The token amount. - /// @param _grantor The user assigning the role. - /// @param _grantee The user receiving the role. - /// @param _expirationDate The expiration date of the role. - /// @param _revocable Whether the role is revocable or not. - /// @param _data Any additional data about the role. - event RoleGranted( - uint256 indexed _nonce, - bytes32 indexed _role, - address _tokenAddress, - uint256 _tokenId, - uint256 _tokenAmount, - address _grantor, - address _grantee, - uint64 _expirationDate, - bool _revocable, - bytes _data - ); - - /// @notice Emitted when a role is revoked. - /// @param _nonce The identifier of the role assignment. - /// @param _role The role identifier. - /// @param _tokenAddress The token address. - /// @param _tokenId The token identifier. - /// @param _tokenAmount The token amount. - /// @param _grantor The user revoking the role. - /// @param _grantee The user that receives the role revocation. - event RoleRevoked( - uint256 indexed _nonce, - bytes32 indexed _role, - address _tokenAddress, - uint256 _tokenId, - uint256 _tokenAmount, - address _grantor, - address _grantee - ); - - /// @notice Emitted when a user is approved to manage roles on behalf of another user. - /// @param _tokenAddress The token address. - /// @param _operator The user approved to grant and revoke roles. - /// @param _isApproved The approval status. - event RoleApprovalForAll(address indexed _tokenAddress, address indexed _operator, bool _isApproved); - - /// @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. - /// @param _tokenAddress The token address. - /// @param _tokenId The token identifier. - /// @param _tokenAmount The token amount withdrawn. - event Withdrew( - uint256 indexed _nonce, - address indexed _grantor, - address _tokenAddress, - uint256 _tokenId, - uint256 _tokenAmount - ); - - /** External Functions **/ - - /// @notice Grants a role on behalf of a user. - /// @param _roleAssignment The role assignment data. - function grantRoleFrom(RoleAssignment calldata _roleAssignment) external; - - /// @notice Revokes a role on behalf of a user. - /// @param _revokeRoleData The role revocation data. - function revokeRoleFrom(RevokeRoleData calldata _revokeRoleData) external; - - /// @notice Approves operator to grant and revoke any roles on behalf of another user. - /// @param _tokenAddress The token address. - /// @param _operator The user approved to grant and revoke roles. - /// @param _approved The approval status. - function setRoleApprovalForAll(address _tokenAddress, address _operator, bool _approved) external; - - /** View Functions **/ - - /// @notice Returns the custom data of a role assignment. - /// @param _nonce The identifier of the role assignment. - function roleData(uint256 _nonce) external view returns (RoleData memory data_); - - /// @notice Returns the expiration date of a role assignment. - /// @param _nonce The identifier of the role assignment. - function roleExpirationDate(uint256 _nonce) external view returns (uint64 expirationDate_); - - /// @notice Checks if the grantor approved the operator for all NFTs. - /// @param _tokenAddress The token address. - /// @param _grantor The user that approved the operator. - /// @param _operator The user that can grant and revoke roles. - function isRoleApprovedForAll( - address _tokenAddress, - address _grantor, - address _operator - ) external view returns (bool); - - /// @notice Calculates the amount of ERC-1155 tokens delegated to the specified _grantee. - /// @param _role The role identifier. - /// @param _tokenAddress The token address. - /// @param _tokenId The token identifier. - /// @param _grantee The user that received the role. - function roleBalanceOf( - bytes32 _role, - address _tokenAddress, - uint256 _tokenId, - address _grantee - ) external view returns (uint256 balance_); -} diff --git a/test/SftRolesRegistry/SftRolesRegistry.spec.ts b/test/SftRolesRegistry/SftRolesRegistry.spec.ts index 8cb46df..a450993 100644 --- a/test/SftRolesRegistry/SftRolesRegistry.spec.ts +++ b/test/SftRolesRegistry/SftRolesRegistry.spec.ts @@ -279,7 +279,7 @@ describe('SftRolesRegistry', async () => { await assertRevokeRoleEvent(SftRolesRegistry, grantor, GrantRoleData.commitmentId, GrantRoleData.role, grantee) }) - it('should revoke when sender is grantee', async () => { + it('should revoke when sender is grantee, and role is not expired nor revocable', async () => { const roleAssignment = await assertGrantRoleEvent( SftRolesRegistry, grantor, @@ -335,10 +335,11 @@ describe('SftRolesRegistry', async () => { }) describe('withdrawNfts', async () => { + let CommitmentCreated: Commitment let GrantRoleData: GrantRoleData beforeEach(async () => { - await assertCreateCommitmentEvent(SftRolesRegistry, MockToken, grantor, 1) + CommitmentCreated = await assertCreateCommitmentEvent(SftRolesRegistry, MockToken, grantor, 1) GrantRoleData = await assertGrantRoleEvent(SftRolesRegistry, grantor, 1, grantee.address) }) @@ -351,29 +352,53 @@ describe('SftRolesRegistry', async () => { it('should revert when there is an active role', async () => { await assertGrantRoleEvent(SftRolesRegistry, grantor, 1, grantee.address, false) await expect(SftRolesRegistry.connect(grantor).withdrawNfts(GrantRoleData.commitmentId)).to.be.revertedWith( - 'SftRolesRegistry: role is not expired and is not revocable', + 'SftRolesRegistry: commitment has an active non-revocable role', ) }) - it('should withdrawNfts when there are revocable roles', async () => { + it('should withdraw when there are revocable roles', async () => { await expect(SftRolesRegistry.connect(grantor).withdrawNfts(GrantRoleData.commitmentId)) .to.emit(SftRolesRegistry, 'NftsWithdrawn') .withArgs(GrantRoleData.commitmentId) + .to.emit(MockToken, 'TransferSingle') + .withArgs( + SftRolesRegistry.address, + SftRolesRegistry.address, + grantor.address, + CommitmentCreated.tokenId, + CommitmentCreated.tokenAmount, + ) }) - it('should withdrawNfts when there are no roles', async () => { + it('should withdraw when there are no roles', async () => { await assertRevokeRoleEvent(SftRolesRegistry, grantor, GrantRoleData.commitmentId, GrantRoleData.role, grantee) await expect(SftRolesRegistry.connect(grantor).withdrawNfts(GrantRoleData.commitmentId)) .to.emit(SftRolesRegistry, 'NftsWithdrawn') .withArgs(GrantRoleData.commitmentId) + .to.emit(MockToken, 'TransferSingle') + .withArgs( + SftRolesRegistry.address, + SftRolesRegistry.address, + grantor.address, + CommitmentCreated.tokenId, + CommitmentCreated.tokenAmount, + ) }) - it('should withdrawNfts when there are expired roles', async () => { + it('should withdraw when there are expired roles', async () => { await assertGrantRoleEvent(SftRolesRegistry, grantor, 1, grantee.address, false) await time.increase(ONE_DAY) await expect(SftRolesRegistry.connect(grantor).withdrawNfts(GrantRoleData.commitmentId)) .to.emit(SftRolesRegistry, 'NftsWithdrawn') .withArgs(GrantRoleData.commitmentId) + .to.emit(MockToken, 'TransferSingle') + .withArgs( + SftRolesRegistry.address, + SftRolesRegistry.address, + grantor.address, + CommitmentCreated.tokenId, + CommitmentCreated.tokenAmount, + ) }) }) diff --git a/test/SftRolesRegistry/SftRolesRegistrySingleRole.spec.ts b/test/SftRolesRegistry/SftRolesRegistrySingleRole.spec.ts index 2f1dc40..3793e33 100644 --- a/test/SftRolesRegistry/SftRolesRegistrySingleRole.spec.ts +++ b/test/SftRolesRegistry/SftRolesRegistrySingleRole.spec.ts @@ -7,7 +7,7 @@ import { SignerWithAddress } from '@nomiclabs/hardhat-ethers/signers' import { generateRoleId, buildCommitment, buildGrantRole, getSftRolesRegistryInterfaceId } from './helpers/mockData' import { GrantRoleData, Commitment } from './types' import { generateRandomInt } from '../helpers' -import { assertCreateCommitmentEvent, assertGrantRoleEvent } from './helpers/assertEvents' +import { assertCreateCommitmentEvent, assertGrantRoleEvent, assertRevokeRoleEvent } from './helpers/assertEvents' describe('SftRolesRegistrySingleRole', async () => { let SftRolesRegistry: Contract @@ -115,84 +115,23 @@ describe('SftRolesRegistrySingleRole', async () => { }) it('should create commitment when sender is grantor', async () => { - const commitment = buildCommitment({ - grantor: grantor.address, - tokenAddress: MockToken.address, - }) - await MockToken.mint(grantor.address, commitment.tokenId, commitment.tokenAmount) - await MockToken.connect(grantor).setApprovalForAll(SftRolesRegistry.address, true) - await expect( - SftRolesRegistry.connect(grantor).commitTokens( - commitment.grantor, - commitment.tokenAddress, - commitment.tokenId, - commitment.tokenAmount, - ), - ) - .to.emit(SftRolesRegistry, 'TokensCommitted') - .withArgs(commitment.grantor, 1, commitment.tokenAddress, commitment.tokenId, commitment.tokenAmount) - .to.emit(MockToken, 'TransferSingle') - .withArgs( - SftRolesRegistry.address, - grantor.address, - SftRolesRegistry.address, - commitment.tokenId, - commitment.tokenAmount, - ) + await assertCreateCommitmentEvent(SftRolesRegistry, MockToken, grantor, 1) }) it('should create commitment when sender is approved', async () => { - const commitment = buildCommitment({ - grantor: grantor.address, - tokenAddress: MockToken.address, - }) - await MockToken.mint(grantor.address, commitment.tokenId, commitment.tokenAmount) - await MockToken.connect(grantor).setApprovalForAll(SftRolesRegistry.address, true) - await SftRolesRegistry.connect(grantor).setRoleApprovalForAll(commitment.tokenAddress, anotherUser.address, true) - await expect( - SftRolesRegistry.connect(anotherUser).commitTokens( - commitment.grantor, - commitment.tokenAddress, - commitment.tokenId, - commitment.tokenAmount, - ), - ) - .to.emit(SftRolesRegistry, 'TokensCommitted') - .withArgs(commitment.grantor, 1, commitment.tokenAddress, commitment.tokenId, commitment.tokenAmount) - .to.emit(MockToken, 'TransferSingle') - .withArgs( - SftRolesRegistry.address, - grantor.address, - SftRolesRegistry.address, - commitment.tokenId, - commitment.tokenAmount, - ) + await assertCreateCommitmentEvent(SftRolesRegistry, MockToken, grantor, 1, anotherUser) }) }) describe('grantRole', async () => { - let CommitmentCreated: Commitment let GrantRoleData: GrantRoleData beforeEach(async () => { - CommitmentCreated = buildCommitment({ - grantor: grantor.address, - tokenAddress: MockToken.address, - }) + await assertCreateCommitmentEvent(SftRolesRegistry, MockToken, grantor, 1) GrantRoleData = await buildGrantRole({ commitmentId: 1, grantee: grantee.address, }) - await MockToken.mint(grantor.address, CommitmentCreated.tokenId, CommitmentCreated.tokenAmount) - await MockToken.connect(grantor).setApprovalForAll(SftRolesRegistry.address, true) - await expect( - SftRolesRegistry.connect(grantor).commitTokens( - CommitmentCreated.grantor, - CommitmentCreated.tokenAddress, - CommitmentCreated.tokenId, - CommitmentCreated.tokenAmount, - ), - ).to.not.be.reverted }) it('should revert when sender is not grantor or approved', async () => { @@ -235,52 +174,18 @@ describe('SftRolesRegistrySingleRole', async () => { }) it('should grant role when sender is grantor', async () => { - await expect( - SftRolesRegistry.connect(grantor).grantRole( - GrantRoleData.commitmentId, - GrantRoleData.role, - GrantRoleData.grantee, - GrantRoleData.expirationDate, - GrantRoleData.revocable, - GrantRoleData.data, - ), - ) - .to.emit(SftRolesRegistry, 'RoleGranted') - .withArgs( - GrantRoleData.commitmentId, - GrantRoleData.role, - GrantRoleData.grantee, - GrantRoleData.expirationDate, - GrantRoleData.revocable, - GrantRoleData.data, - ) + await assertGrantRoleEvent(SftRolesRegistry, grantor, GrantRoleData.commitmentId, GrantRoleData.grantee) }) it('should grant role when sender is approved', async () => { - await SftRolesRegistry.connect(grantor).setRoleApprovalForAll( - CommitmentCreated.tokenAddress, - anotherUser.address, + await assertGrantRoleEvent( + SftRolesRegistry, + grantor, + GrantRoleData.commitmentId, + GrantRoleData.grantee, true, + anotherUser, ) - await expect( - SftRolesRegistry.connect(anotherUser).grantRole( - GrantRoleData.commitmentId, - GrantRoleData.role, - GrantRoleData.grantee, - GrantRoleData.expirationDate, - GrantRoleData.revocable, - GrantRoleData.data, - ), - ) - .to.emit(SftRolesRegistry, 'RoleGranted') - .withArgs( - GrantRoleData.commitmentId, - GrantRoleData.role, - GrantRoleData.grantee, - GrantRoleData.expirationDate, - GrantRoleData.revocable, - GrantRoleData.data, - ) }) }) @@ -289,34 +194,8 @@ describe('SftRolesRegistrySingleRole', async () => { let GrantRoleData: GrantRoleData beforeEach(async () => { - CommitmentCreated = buildCommitment({ - grantor: grantor.address, - tokenAddress: MockToken.address, - }) - GrantRoleData = await buildGrantRole({ - commitmentId: 1, - grantee: grantee.address, - }) - await MockToken.mint(grantor.address, CommitmentCreated.tokenId, CommitmentCreated.tokenAmount) - await MockToken.connect(grantor).setApprovalForAll(SftRolesRegistry.address, true) - await expect( - SftRolesRegistry.connect(grantor).commitTokens( - CommitmentCreated.grantor, - CommitmentCreated.tokenAddress, - CommitmentCreated.tokenId, - CommitmentCreated.tokenAmount, - ), - ).to.not.be.reverted - await expect( - SftRolesRegistry.connect(grantor).grantRole( - GrantRoleData.commitmentId, - GrantRoleData.role, - GrantRoleData.grantee, - GrantRoleData.expirationDate, - GrantRoleData.revocable, - GrantRoleData.data, - ), - ).to.not.be.reverted + CommitmentCreated = await assertCreateCommitmentEvent(SftRolesRegistry, MockToken, grantor, 1) + GrantRoleData = await assertGrantRoleEvent(SftRolesRegistry, grantor, 1, grantee.address) }) it('should revert when sender is not grantor or approved', async () => { @@ -366,50 +245,25 @@ describe('SftRolesRegistrySingleRole', async () => { }) it('should revoke role when sender is grantee, and role is not expired nor revocable', async () => { - await expect( - SftRolesRegistry.connect(grantor).grantRole( - GrantRoleData.commitmentId, - GrantRoleData.role, - GrantRoleData.grantee, - GrantRoleData.expirationDate, - false, - GrantRoleData.data, - ), - ).to.not.be.reverted - - await expect( - SftRolesRegistry.connect(grantee).revokeRole( - GrantRoleData.commitmentId, - GrantRoleData.role, - GrantRoleData.grantee, - ), + const roleAssignment = await assertGrantRoleEvent( + SftRolesRegistry, + grantor, + GrantRoleData.commitmentId, + grantee.address, + false, ) - .to.emit(SftRolesRegistry, 'RoleRevoked') - .withArgs(GrantRoleData.commitmentId, GrantRoleData.role, GrantRoleData.grantee) - }) - - it('should revoke role when sender is grantor', async () => { - await expect( - SftRolesRegistry.connect(grantor).revokeRole( - GrantRoleData.commitmentId, - GrantRoleData.role, - GrantRoleData.grantee, - ), + await assertRevokeRoleEvent( + SftRolesRegistry, + grantor, + roleAssignment.commitmentId, + roleAssignment.role, + grantee, + grantee, ) - .to.emit(SftRolesRegistry, 'RoleRevoked') - .withArgs(GrantRoleData.commitmentId, GrantRoleData.role, GrantRoleData.grantee) }) - it('should revoke role when sender is grantee', async () => { - await expect( - SftRolesRegistry.connect(grantee).revokeRole( - GrantRoleData.commitmentId, - GrantRoleData.role, - GrantRoleData.grantee, - ), - ) - .to.emit(SftRolesRegistry, 'RoleRevoked') - .withArgs(GrantRoleData.commitmentId, GrantRoleData.role, GrantRoleData.grantee) + it('should revoke role when sender is grantor', async () => { + await assertRevokeRoleEvent(SftRolesRegistry, grantor, GrantRoleData.commitmentId, GrantRoleData.role, grantee) }) it('should revoke role when sender is approved by grantor', async () => { @@ -422,11 +276,11 @@ describe('SftRolesRegistrySingleRole', async () => { SftRolesRegistry.connect(anotherUser).revokeRole( GrantRoleData.commitmentId, GrantRoleData.role, - GrantRoleData.grantee, + grantee.address, ), ) .to.emit(SftRolesRegistry, 'RoleRevoked') - .withArgs(GrantRoleData.commitmentId, GrantRoleData.role, GrantRoleData.grantee) + .withArgs(GrantRoleData.commitmentId, GrantRoleData.role, grantee.address) }) it('should revoke role when sender is approved by grantee', async () => { @@ -452,24 +306,8 @@ describe('SftRolesRegistrySingleRole', async () => { let GrantRoleData: GrantRoleData beforeEach(async () => { - CommitmentCreated = buildCommitment({ - grantor: grantor.address, - tokenAddress: MockToken.address, - }) - GrantRoleData = await buildGrantRole({ - commitmentId: 1, - grantee: grantee.address, - }) - await MockToken.mint(grantor.address, CommitmentCreated.tokenId, CommitmentCreated.tokenAmount) - await MockToken.connect(grantor).setApprovalForAll(SftRolesRegistry.address, true) - await expect( - SftRolesRegistry.connect(grantor).commitTokens( - CommitmentCreated.grantor, - CommitmentCreated.tokenAddress, - CommitmentCreated.tokenId, - CommitmentCreated.tokenAmount, - ), - ).to.not.be.reverted + CommitmentCreated = await assertCreateCommitmentEvent(SftRolesRegistry, MockToken, grantor, 1) + GrantRoleData = await assertGrantRoleEvent(SftRolesRegistry, grantor, 1, grantee.address) }) it('should revert when sender is not grantor or approved', async () => { @@ -478,23 +316,23 @@ describe('SftRolesRegistrySingleRole', async () => { ) }) - it('should revert when commitment has an active role', async () => { + it('should revert when commitment has an active non-revocable role', async () => { await expect( SftRolesRegistry.connect(grantor).grantRole( GrantRoleData.commitmentId, GrantRoleData.role, GrantRoleData.grantee, GrantRoleData.expirationDate, - GrantRoleData.revocable, + false, GrantRoleData.data, ), ).to.not.be.reverted await expect(SftRolesRegistry.connect(grantor).withdrawNfts(GrantRoleData.commitmentId)).to.be.revertedWith( - 'SftRolesRegistry: token has an active role', + 'SftRolesRegistry: commitment has an active non-revocable role', ) }) - it('should withdrawNfts tokens when sender is grantor', async () => { + it('should withdraw tokens when sender is grantor', async () => { await expect(SftRolesRegistry.connect(grantor).withdrawNfts(GrantRoleData.commitmentId)) .to.emit(SftRolesRegistry, 'NftsWithdrawn') .withArgs(GrantRoleData.commitmentId) @@ -508,7 +346,7 @@ describe('SftRolesRegistrySingleRole', async () => { ) }) - it('should withdrawNfts tokens when sender is approved', async () => { + it('should withdraw tokens when sender is approved', async () => { await SftRolesRegistry.connect(grantor).setRoleApprovalForAll( CommitmentCreated.tokenAddress, anotherUser.address, diff --git a/test/SftRolesRegistry/helpers/assertEvents.ts b/test/SftRolesRegistry/helpers/assertEvents.ts index 08c56b1..0793071 100644 --- a/test/SftRolesRegistry/helpers/assertEvents.ts +++ b/test/SftRolesRegistry/helpers/assertEvents.ts @@ -7,7 +7,7 @@ export async function assertCreateCommitmentEvent( SftRolesRegistry: Contract, MockToken: Contract, grantor: SignerWithAddress, - expectedcommitmentId: number, + expectedCommitmentId: number, anotherUser?: SignerWithAddress, ) { const commitment = buildCommitment({ @@ -32,7 +32,7 @@ export async function assertCreateCommitmentEvent( .to.emit(SftRolesRegistry, 'TokensCommitted') .withArgs( commitment.grantor, - expectedcommitmentId, + expectedCommitmentId, commitment.tokenAddress, commitment.tokenId, commitment.tokenAmount, @@ -45,7 +45,7 @@ export async function assertCreateCommitmentEvent( commitment.tokenId, commitment.tokenAmount, ) - return { ...commitment, commitmentId: expectedcommitmentId } + return { ...commitment, commitmentId: expectedCommitmentId } } export async function assertGrantRoleEvent( From c165ef95ffbc54e1486dd79026b75330c40920f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ernani=20S=C3=A3o=20Thiago?= Date: Wed, 27 Dec 2023 09:57:36 -0500 Subject: [PATCH 09/19] improved readme --- README.md | 30 +++++++++++++++++++++++------- 1 file changed, 23 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index bccc51a..72f1db1 100644 --- a/README.md +++ b/README.md @@ -7,21 +7,37 @@ [![Discord](https://img.shields.io/discord/1009147970832322632?label=discord&logo=discord&logoColor=white)](https://discord.gg/NaNTgPK5rx) [![Twitter Follow](https://img.shields.io/twitter/follow/oriumnetwork?label=Follow&style=social)](https://twitter.com/OriumNetwork) -This repository contains a minimal implementation of ERC-7432 (Non-Fungible Token Roles). -ERC-7432 introduces role management for NFTs. Each role assignment is associated with a single NFT and expires automatically at a given timestamp. +[comment]: <> (TODO: Add link and EIP number for ERC-1155 Roles Registry when available) -ERC-7432 can be deeply integrated with dApps to create a utility-sharing mechanism. A good example is in digital real estate. A user can create a digital property NFT and grant a `keccak256("PROPERTY_MANAGER")` role to another user, allowing them to delegate specific utility without compromising ownership. The same user could also grant multiple `keccak256("PROPERTY_TENANT")` roles, allowing the grantees to access and interact with the digital property. +This repository contains multiple implementations of two EIPs (Ethereum Improvement Proposals): +* ERC-7432 (Non-Fungible Token Roles). +* ERC-TBD (Semi-Fungible Token Roles). -You can find the full specification [here](https://github.com/ethereum/EIPs/blob/master/EIPS/eip-7432.md). +The goal of these EIPs is to introduce role management for NFTs. Each role assignment is associated with one or more +NFTs and expire automatically at a given timestamp. Token Roles can be deeply integrated with dApps to create a +utility-sharing mechanism. A good example is in digital real estate. A user can create a digital property NFT and grant +a `keccak256("PROPERTY_MANAGER")` role to another user, allowing them to delegate specific utility without compromising +ownership. The same user could also grant multiple `keccak256("PROPERTY_TENANT")` roles, allowing additional users to +access the digital property. -# Build +You can find the full specification here: [ERC-721 Token Roles](https://eips.ethereum.org/EIPS/eip-7432) and +[ERC-1155 Token Roles](TBD). + +## Implementations + +* [ERC-7432 NFT Roles Registry](./contracts/RolesRegistry.sol): ERC-721 NFT Owners can grant roles without depositing NFTs. +* [ERC-TBD SFT Roles Registry](./contracts/RolesRegistry/SftRolesRegistry.sol): ERC-1155 NFT Owners can grant roles after depositing NFTs. +* [ERC-TBD Single-Role SFT Roles Registry](./contracts/RolesRegistry/SftRolesRegistrySingleRole.sol): ERC-1155 NFT Owners can grant a single pre-defined role after depositing + NFTs. + +## Build ```bash -npm install +npm ci npm run build ``` -# Test +## Test ```bash npm run test From 9d087c581b22e04a5fef5b16ac53ed466df4a655 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ernani=20S=C3=A3o=20Thiago?= Date: Wed, 27 Dec 2023 11:34:26 -0500 Subject: [PATCH 10/19] add optional interface ICommitTokensAndGrantRoleExtension --- contracts/RolesRegistry/SftRolesRegistry.sol | 27 ++- .../SftRolesRegistrySingleRole.sol | 28 ++- .../ICommitTokensAndGrantRoleExtension.sol | 27 +++ .../SftRolesRegistry/SftRolesRegistry.spec.ts | 112 ++++++++++++ .../SftRolesRegistrySingleRole.spec.ts | 161 +++++++++++++++++- test/SftRolesRegistry/helpers/mockData.ts | 10 +- 6 files changed, 358 insertions(+), 7 deletions(-) create mode 100644 contracts/RolesRegistry/interfaces/ICommitTokensAndGrantRoleExtension.sol diff --git a/contracts/RolesRegistry/SftRolesRegistry.sol b/contracts/RolesRegistry/SftRolesRegistry.sol index 2f34316..fe27d61 100644 --- a/contracts/RolesRegistry/SftRolesRegistry.sol +++ b/contracts/RolesRegistry/SftRolesRegistry.sol @@ -3,6 +3,7 @@ pragma solidity 0.8.9; import { ISftRolesRegistry } from './interfaces/ISftRolesRegistry.sol'; +import { ICommitTokensAndGrantRoleExtension } from './interfaces/ICommitTokensAndGrantRoleExtension.sol'; import { IERC165 } from '@openzeppelin/contracts/utils/introspection/IERC165.sol'; import { IERC1155 } from '@openzeppelin/contracts/token/ERC1155/IERC1155.sol'; import { IERC1155Receiver } from '@openzeppelin/contracts/token/ERC1155/IERC1155Receiver.sol'; @@ -12,7 +13,7 @@ import { LinkedLists } from './libraries/LinkedLists.sol'; import { EnumerableSet } from '@openzeppelin/contracts/utils/structs/EnumerableSet.sol'; // Semi-fungible token (SFT) roles registry -contract SftRolesRegistry is ISftRolesRegistry, ERC1155Holder { +contract SftRolesRegistry is ISftRolesRegistry, ERC1155Holder, ICommitTokensAndGrantRoleExtension { using LinkedLists for LinkedLists.Lists; using LinkedLists for LinkedLists.ListItem; using EnumerableSet for EnumerableSet.UintSet; @@ -136,6 +137,25 @@ contract SftRolesRegistry is ISftRolesRegistry, ERC1155Holder { emit RoleApprovalForAll(_tokenAddress, _operator, _isApproved); } + /** Optional External Functions **/ + + function commitTokensAndGrantRole( + address _grantor, + address _tokenAddress, + uint256 _tokenId, + uint256 _tokenAmount, + bytes32 _role, + address _grantee, + uint64 _expirationDate, + bool _revocable, + bytes calldata _data + ) external override onlyOwnerOrApproved(_grantor, _tokenAddress) returns (uint256 commitmentId_) { + require(_tokenAmount > 0, 'SftRolesRegistry: tokenAmount must be greater than zero'); + require(_expirationDate > block.timestamp, 'SftRolesRegistry: expiration date must be in the future'); + commitmentId_ = _createCommitment(_grantor, _tokenAddress, _tokenId, _tokenAmount); + _grantOrUpdateRole(commitmentId_, _role, _grantee, _expirationDate, _revocable, _data); + } + /** View Functions **/ function grantorOf(uint256 _commitmentId) external view returns (address grantor_) { @@ -213,7 +233,10 @@ contract SftRolesRegistry is ISftRolesRegistry, ERC1155Holder { function supportsInterface( bytes4 interfaceId ) public view virtual override(ERC1155Receiver, IERC165) returns (bool) { - return interfaceId == type(ISftRolesRegistry).interfaceId || interfaceId == type(IERC1155Receiver).interfaceId; + return + interfaceId == type(ISftRolesRegistry).interfaceId || + interfaceId == type(IERC1155Receiver).interfaceId || + interfaceId == type(ICommitTokensAndGrantRoleExtension).interfaceId; } /** Helper Functions **/ diff --git a/contracts/RolesRegistry/SftRolesRegistrySingleRole.sol b/contracts/RolesRegistry/SftRolesRegistrySingleRole.sol index 5082574..f90d894 100644 --- a/contracts/RolesRegistry/SftRolesRegistrySingleRole.sol +++ b/contracts/RolesRegistry/SftRolesRegistrySingleRole.sol @@ -3,6 +3,7 @@ pragma solidity 0.8.9; import { ISftRolesRegistry } from './interfaces/ISftRolesRegistry.sol'; +import { ICommitTokensAndGrantRoleExtension } from './interfaces/ICommitTokensAndGrantRoleExtension.sol'; import { IERC165 } from '@openzeppelin/contracts/utils/introspection/IERC165.sol'; import { IERC1155 } from '@openzeppelin/contracts/token/ERC1155/IERC1155.sol'; import { IERC1155Receiver } from '@openzeppelin/contracts/token/ERC1155/IERC1155Receiver.sol'; @@ -10,7 +11,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 SftRolesRegistrySingleRole is ISftRolesRegistry, ERC1155Holder { +contract SftRolesRegistrySingleRole is ISftRolesRegistry, ERC1155Holder, ICommitTokensAndGrantRoleExtension { bytes32 public constant UNIQUE_ROLE = keccak256('UNIQUE_ROLE'); uint256 public commitmentCount; @@ -117,6 +118,26 @@ contract SftRolesRegistrySingleRole is ISftRolesRegistry, ERC1155Holder { emit RoleApprovalForAll(_tokenAddress, _operator, _isApproved); } + /** Optional External Functions **/ + + function commitTokensAndGrantRole( + address _grantor, + address _tokenAddress, + uint256 _tokenId, + uint256 _tokenAmount, + bytes32 _role, + address _grantee, + uint64 _expirationDate, + bool _revocable, + bytes calldata _data + ) external override onlyOwnerOrApproved(_grantor, _tokenAddress) returns (uint256 commitmentId_) { + require(_tokenAmount > 0, 'SftRolesRegistry: tokenAmount must be greater than zero'); + require(_role == UNIQUE_ROLE, 'SftRolesRegistry: role not supported'); + require(_expirationDate > block.timestamp, 'SftRolesRegistry: expiration date must be in the future'); + commitmentId_ = _createCommitment(_grantor, _tokenAddress, _tokenId, _tokenAmount); + _grantOrUpdateRole(commitmentId_, _role, _grantee, _expirationDate, _revocable, _data); + } + /** View Functions **/ function grantorOf(uint256 _commitmentId) external view returns (address grantor_) { @@ -170,7 +191,10 @@ contract SftRolesRegistrySingleRole is ISftRolesRegistry, ERC1155Holder { function supportsInterface( bytes4 interfaceId ) public view virtual override(ERC1155Receiver, IERC165) returns (bool) { - return interfaceId == type(ISftRolesRegistry).interfaceId || interfaceId == type(IERC1155Receiver).interfaceId; + return + interfaceId == type(ISftRolesRegistry).interfaceId || + interfaceId == type(IERC1155Receiver).interfaceId || + interfaceId == type(ICommitTokensAndGrantRoleExtension).interfaceId; } /** Helper Functions **/ diff --git a/contracts/RolesRegistry/interfaces/ICommitTokensAndGrantRoleExtension.sol b/contracts/RolesRegistry/interfaces/ICommitTokensAndGrantRoleExtension.sol new file mode 100644 index 0000000..54fb598 --- /dev/null +++ b/contracts/RolesRegistry/interfaces/ICommitTokensAndGrantRoleExtension.sol @@ -0,0 +1,27 @@ +// SPDX-License-Identifier: CC0-1.0 + +pragma solidity 0.8.9; + +interface ICommitTokensAndGrantRoleExtension { + /// @notice Commits tokens and grant role in a single transaction. + /// @param _grantor The owner of the NFTs. + /// @param _tokenAddress The token address. + /// @param _tokenId The token identifier. + /// @param _tokenAmount The token amount. + /// @param _role The role identifier. + /// @param _grantee The user receiving the role. + /// @param _expirationDate The expiration date of the role. + /// @param _revocable Whether the role is revocable or not. + /// @param _data Any additional data about the role. + function commitTokensAndGrantRole( + address _grantor, + address _tokenAddress, + uint256 _tokenId, + uint256 _tokenAmount, + bytes32 _role, + address _grantee, + uint64 _expirationDate, + bool _revocable, + bytes calldata _data + ) external returns (uint256 commitmentId_); +} diff --git a/test/SftRolesRegistry/SftRolesRegistry.spec.ts b/test/SftRolesRegistry/SftRolesRegistry.spec.ts index a450993..ec90474 100644 --- a/test/SftRolesRegistry/SftRolesRegistry.spec.ts +++ b/test/SftRolesRegistry/SftRolesRegistry.spec.ts @@ -13,6 +13,7 @@ import { buildCommitment, buildGrantRole, getSftRolesRegistryInterfaceId, + getCommitTokensAndGrantRoleInterfaceId, } from './helpers/mockData' const { AddressZero } = ethers.constants @@ -412,6 +413,112 @@ describe('SftRolesRegistry', async () => { }) }) + describe('commitTokensAndGrantRole', async () => { + let CommitmentCreated: Commitment + let GrantRoleData: GrantRoleData + + beforeEach(async () => { + CommitmentCreated = buildCommitment({ + grantor: grantor.address, + tokenAddress: MockToken.address, + }) + GrantRoleData = await buildGrantRole({ + commitmentId: 1, + }) + }) + + it('should revert when sender is not grantor or approved', async () => { + await expect( + SftRolesRegistry.connect(anotherUser).commitTokensAndGrantRole( + CommitmentCreated.grantor, + CommitmentCreated.tokenAddress, + CommitmentCreated.tokenId, + CommitmentCreated.tokenAmount, + GrantRoleData.role, + GrantRoleData.grantee, + GrantRoleData.expirationDate, + GrantRoleData.revocable, + GrantRoleData.data, + ), + ).to.be.revertedWith('SftRolesRegistry: account not approved') + }) + + it('should revert when tokenAmount is zero', async () => { + await expect( + SftRolesRegistry.connect(grantor).commitTokensAndGrantRole( + CommitmentCreated.grantor, + CommitmentCreated.tokenAddress, + CommitmentCreated.tokenId, + 0, + GrantRoleData.role, + GrantRoleData.grantee, + GrantRoleData.expirationDate, + GrantRoleData.revocable, + GrantRoleData.data, + ), + ).to.be.revertedWith('SftRolesRegistry: tokenAmount must be greater than zero') + }) + + it('should revert when expirationDate is not in the future', async () => { + await expect( + SftRolesRegistry.connect(grantor).commitTokensAndGrantRole( + CommitmentCreated.grantor, + CommitmentCreated.tokenAddress, + CommitmentCreated.tokenId, + CommitmentCreated.tokenAmount, + GrantRoleData.role, + GrantRoleData.grantee, + await time.latest(), + GrantRoleData.revocable, + GrantRoleData.data, + ), + ).to.be.revertedWith('SftRolesRegistry: expiration date must be in the future') + }) + + it('should commit tokens and grant role', async () => { + await MockToken.mint(grantor.address, CommitmentCreated.tokenId, CommitmentCreated.tokenAmount) + await MockToken.connect(grantor).setApprovalForAll(SftRolesRegistry.address, true) + await expect( + SftRolesRegistry.connect(grantor).commitTokensAndGrantRole( + CommitmentCreated.grantor, + CommitmentCreated.tokenAddress, + CommitmentCreated.tokenId, + CommitmentCreated.tokenAmount, + GrantRoleData.role, + GrantRoleData.grantee, + GrantRoleData.expirationDate, + GrantRoleData.revocable, + GrantRoleData.data, + ), + ) + .to.emit(SftRolesRegistry, 'TokensCommitted') + .withArgs( + CommitmentCreated.grantor, + GrantRoleData.commitmentId, + CommitmentCreated.tokenAddress, + CommitmentCreated.tokenId, + CommitmentCreated.tokenAmount, + ) + .to.emit(SftRolesRegistry, 'RoleGranted') + .withArgs( + GrantRoleData.commitmentId, + GrantRoleData.role, + GrantRoleData.grantee, + GrantRoleData.expirationDate, + GrantRoleData.revocable, + GrantRoleData.data, + ) + .to.emit(MockToken, 'TransferSingle') + .withArgs( + SftRolesRegistry.address, + CommitmentCreated.grantor, + SftRolesRegistry.address, + CommitmentCreated.tokenId, + CommitmentCreated.tokenAmount, + ) + }) + }) + describe('View Functions', async () => { let CommitmentCreated: Commitment let GrantRoleData: GrantRoleData @@ -478,5 +585,10 @@ describe('SftRolesRegistry', async () => { const interfaceId = getSftRolesRegistryInterfaceId() expect(await SftRolesRegistry.supportsInterface(interfaceId)).to.be.true }) + + it('should return true if ICommitTokensAndGrantRoleExtension interface id', async () => { + const interfaceId = getCommitTokensAndGrantRoleInterfaceId() + expect(await SftRolesRegistry.supportsInterface(interfaceId)).to.be.true + }) }) }) diff --git a/test/SftRolesRegistry/SftRolesRegistrySingleRole.spec.ts b/test/SftRolesRegistry/SftRolesRegistrySingleRole.spec.ts index 3793e33..85fa0d2 100644 --- a/test/SftRolesRegistry/SftRolesRegistrySingleRole.spec.ts +++ b/test/SftRolesRegistry/SftRolesRegistrySingleRole.spec.ts @@ -4,7 +4,14 @@ import { beforeEach } from 'mocha' import { expect } from 'chai' import { loadFixture, time } from '@nomicfoundation/hardhat-network-helpers' import { SignerWithAddress } from '@nomiclabs/hardhat-ethers/signers' -import { generateRoleId, buildCommitment, buildGrantRole, getSftRolesRegistryInterfaceId } from './helpers/mockData' +import { + generateRoleId, + buildCommitment, + buildGrantRole, + getSftRolesRegistryInterfaceId, + getCommitTokensAndGrantRoleInterfaceId, + ONE_DAY, +} from './helpers/mockData' import { GrantRoleData, Commitment } from './types' import { generateRandomInt } from '../helpers' import { assertCreateCommitmentEvent, assertGrantRoleEvent, assertRevokeRoleEvent } from './helpers/assertEvents' @@ -332,6 +339,31 @@ describe('SftRolesRegistrySingleRole', async () => { ) }) + it('should withdraw when role has an expired non-revocable role', async () => { + await expect( + SftRolesRegistry.connect(grantor).grantRole( + GrantRoleData.commitmentId, + GrantRoleData.role, + GrantRoleData.grantee, + GrantRoleData.expirationDate, + false, + GrantRoleData.data, + ), + ).to.not.be.reverted + await time.increase(ONE_DAY) + await expect(SftRolesRegistry.connect(grantor).withdrawNfts(GrantRoleData.commitmentId)) + .to.emit(SftRolesRegistry, 'NftsWithdrawn') + .withArgs(GrantRoleData.commitmentId) + .to.emit(MockToken, 'TransferSingle') + .withArgs( + SftRolesRegistry.address, + SftRolesRegistry.address, + grantor.address, + CommitmentCreated.tokenId, + CommitmentCreated.tokenAmount, + ) + }) + it('should withdraw tokens when sender is grantor', async () => { await expect(SftRolesRegistry.connect(grantor).withdrawNfts(GrantRoleData.commitmentId)) .to.emit(SftRolesRegistry, 'NftsWithdrawn') @@ -366,6 +398,128 @@ describe('SftRolesRegistrySingleRole', async () => { }) }) + describe('commitTokensAndGrantRole', async () => { + let CommitmentCreated: Commitment + let GrantRoleData: GrantRoleData + + beforeEach(async () => { + CommitmentCreated = buildCommitment({ + grantor: grantor.address, + tokenAddress: MockToken.address, + }) + GrantRoleData = await buildGrantRole({ + commitmentId: 1, + }) + }) + + it('should revert when sender is not grantor or approved', async () => { + await expect( + SftRolesRegistry.connect(anotherUser).commitTokensAndGrantRole( + CommitmentCreated.grantor, + CommitmentCreated.tokenAddress, + CommitmentCreated.tokenId, + CommitmentCreated.tokenAmount, + GrantRoleData.role, + GrantRoleData.grantee, + GrantRoleData.expirationDate, + GrantRoleData.revocable, + GrantRoleData.data, + ), + ).to.be.revertedWith('SftRolesRegistry: account not approved') + }) + + it('should revert when tokenAmount is zero', async () => { + await expect( + SftRolesRegistry.connect(grantor).commitTokensAndGrantRole( + CommitmentCreated.grantor, + CommitmentCreated.tokenAddress, + CommitmentCreated.tokenId, + 0, + GrantRoleData.role, + GrantRoleData.grantee, + GrantRoleData.expirationDate, + GrantRoleData.revocable, + GrantRoleData.data, + ), + ).to.be.revertedWith('SftRolesRegistry: tokenAmount must be greater than zero') + }) + + it('should revert when role is not supported', async () => { + await expect( + SftRolesRegistry.connect(grantor).commitTokensAndGrantRole( + CommitmentCreated.grantor, + CommitmentCreated.tokenAddress, + CommitmentCreated.tokenId, + CommitmentCreated.tokenAmount, + generateRoleId('ANOTHER_ROLE'), + GrantRoleData.grantee, + GrantRoleData.expirationDate, + GrantRoleData.revocable, + GrantRoleData.data, + ), + ).to.be.revertedWith('SftRolesRegistry: role not supported') + }) + + it('should revert when expirationDate is not in the future', async () => { + await expect( + SftRolesRegistry.connect(grantor).commitTokensAndGrantRole( + CommitmentCreated.grantor, + CommitmentCreated.tokenAddress, + CommitmentCreated.tokenId, + CommitmentCreated.tokenAmount, + GrantRoleData.role, + GrantRoleData.grantee, + await time.latest(), + GrantRoleData.revocable, + GrantRoleData.data, + ), + ).to.be.revertedWith('SftRolesRegistry: expiration date must be in the future') + }) + + it('should commit tokens and grant role', async () => { + await MockToken.mint(grantor.address, CommitmentCreated.tokenId, CommitmentCreated.tokenAmount) + await MockToken.connect(grantor).setApprovalForAll(SftRolesRegistry.address, true) + await expect( + SftRolesRegistry.connect(grantor).commitTokensAndGrantRole( + CommitmentCreated.grantor, + CommitmentCreated.tokenAddress, + CommitmentCreated.tokenId, + CommitmentCreated.tokenAmount, + GrantRoleData.role, + GrantRoleData.grantee, + GrantRoleData.expirationDate, + GrantRoleData.revocable, + GrantRoleData.data, + ), + ) + .to.emit(SftRolesRegistry, 'TokensCommitted') + .withArgs( + CommitmentCreated.grantor, + GrantRoleData.commitmentId, + CommitmentCreated.tokenAddress, + CommitmentCreated.tokenId, + CommitmentCreated.tokenAmount, + ) + .to.emit(SftRolesRegistry, 'RoleGranted') + .withArgs( + GrantRoleData.commitmentId, + GrantRoleData.role, + GrantRoleData.grantee, + GrantRoleData.expirationDate, + GrantRoleData.revocable, + GrantRoleData.data, + ) + .to.emit(MockToken, 'TransferSingle') + .withArgs( + SftRolesRegistry.address, + CommitmentCreated.grantor, + SftRolesRegistry.address, + CommitmentCreated.tokenId, + CommitmentCreated.tokenAmount, + ) + }) + }) + describe('view functions', async () => { let CommitmentCreated: Commitment let GrantRoleData: GrantRoleData @@ -453,5 +607,10 @@ describe('SftRolesRegistrySingleRole', async () => { const interfaceId = getSftRolesRegistryInterfaceId() expect(await SftRolesRegistry.supportsInterface(interfaceId)).to.be.true }) + + it('should return true if ICommitTokensAndGrantRoleExtension interface id', async () => { + const interfaceId = getCommitTokensAndGrantRoleInterfaceId() + expect(await SftRolesRegistry.supportsInterface(interfaceId)).to.be.true + }) }) }) diff --git a/test/SftRolesRegistry/helpers/mockData.ts b/test/SftRolesRegistry/helpers/mockData.ts index c502d6b..b27b4aa 100644 --- a/test/SftRolesRegistry/helpers/mockData.ts +++ b/test/SftRolesRegistry/helpers/mockData.ts @@ -4,6 +4,7 @@ import { generateRandomInt } from '../../helpers' import { time } from '@nomicfoundation/hardhat-network-helpers' import { ethers } from 'ethers' import { ISftRolesRegistry__factory } from '../../../typechain-types' +import { ICommitTokensAndGrantRoleExtension__factory } from '../../../typechain-types' const { HashZero, AddressZero } = ethers.constants export const ONE_DAY = 60 * 60 * 24 @@ -79,8 +80,13 @@ export async function buildRoleAssignment({ } export function getSftRolesRegistryInterfaceId() { - const interfaceI = ISftRolesRegistry__factory.createInterface() - return generateErc165InterfaceId(interfaceI) + const iface = ISftRolesRegistry__factory.createInterface() + return generateErc165InterfaceId(iface) +} + +export function getCommitTokensAndGrantRoleInterfaceId() { + const iface = ICommitTokensAndGrantRoleExtension__factory.createInterface() + return generateErc165InterfaceId(iface) } function generateErc165InterfaceId(contractInterface: ethers.utils.Interface) { From f9ce5653125724aecdcc9d609bfbec20fe05936f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ernani=20S=C3=A3o=20Thiago?= Date: Wed, 27 Dec 2023 17:51:29 -0500 Subject: [PATCH 11/19] add optional interface IRoleBalanceOfExtension --- contracts/RolesRegistry/SftRolesRegistry.sol | 59 ++-- .../interfaces/IRoleBalanceOfExtension.sol | 18 ++ .../RolesRegistry/libraries/LinkedLists.sol | 3 +- contracts/mocks/MockLinkedLists.sol | 18 +- .../SftRolesRegistry/SftRolesRegistry.spec.ts | 253 ++++++++++++++++++ test/SftRolesRegistry/helpers/mockData.ts | 6 + 6 files changed, 320 insertions(+), 37 deletions(-) create mode 100644 contracts/RolesRegistry/interfaces/IRoleBalanceOfExtension.sol diff --git a/contracts/RolesRegistry/SftRolesRegistry.sol b/contracts/RolesRegistry/SftRolesRegistry.sol index fe27d61..69f9930 100644 --- a/contracts/RolesRegistry/SftRolesRegistry.sol +++ b/contracts/RolesRegistry/SftRolesRegistry.sol @@ -3,6 +3,7 @@ pragma solidity 0.8.9; import { ISftRolesRegistry } from './interfaces/ISftRolesRegistry.sol'; +import { IRoleBalanceOfExtension } from './interfaces/IRoleBalanceOfExtension.sol'; import { ICommitTokensAndGrantRoleExtension } from './interfaces/ICommitTokensAndGrantRoleExtension.sol'; import { IERC165 } from '@openzeppelin/contracts/utils/introspection/IERC165.sol'; import { IERC1155 } from '@openzeppelin/contracts/token/ERC1155/IERC1155.sol'; @@ -13,7 +14,12 @@ import { LinkedLists } from './libraries/LinkedLists.sol'; import { EnumerableSet } from '@openzeppelin/contracts/utils/structs/EnumerableSet.sol'; // Semi-fungible token (SFT) roles registry -contract SftRolesRegistry is ISftRolesRegistry, ERC1155Holder, ICommitTokensAndGrantRoleExtension { +contract SftRolesRegistry is + ISftRolesRegistry, + ERC1155Holder, + ICommitTokensAndGrantRoleExtension, + IRoleBalanceOfExtension +{ using LinkedLists for LinkedLists.Lists; using LinkedLists for LinkedLists.ListItem; using EnumerableSet for EnumerableSet.UintSet; @@ -156,6 +162,28 @@ contract SftRolesRegistry is ISftRolesRegistry, ERC1155Holder, ICommitTokensAndG _grantOrUpdateRole(commitmentId_, _role, _grantee, _expirationDate, _revocable, _data); } + function roleBalanceOf( + bytes32 _role, + address _tokenAddress, + uint256 _tokenId, + address _grantee + ) external view returns (uint256 balance_) { + bytes32 headKey = _getHeadKey(_grantee, _role, _tokenAddress, _tokenId); + uint256 currentItemId = lists.heads[headKey]; + + balance_ = 0; + LinkedLists.ListItem storage currentItem; + while (currentItemId != 0) { + currentItem = lists.items[currentItemId]; + if (currentItem.data.expirationDate < block.timestamp) { + return balance_; + } + uint256 commitmentId = currentItem.data.commitmentId; + balance_ += commitments[commitmentId].tokenAmount; + currentItemId = currentItem.next; + } + } + /** View Functions **/ function grantorOf(uint256 _commitmentId) external view returns (address grantor_) { @@ -206,37 +234,14 @@ contract SftRolesRegistry is ISftRolesRegistry, ERC1155Holder, ICommitTokensAndG return roleApprovals[_grantor][_tokenAddress][_operator]; } - /*function roleBalanceOf( - bytes32 _role, - address _tokenAddress, - uint256 _tokenId, - address _grantee - ) external view returns (uint256 balance_) { - bytes32 headKey = _getHeadKey(_grantee, _role, _tokenAddress, _tokenId); - uint256 currentNonce = lists.heads[headKey]; - if (currentNonce == 0) { - return 0; - } - - balance_ = 0; - LinkedLists.ListItem storage currentItem; - while (currentNonce != 0) { - currentItem = lists.items[currentNonce]; - if (currentItem.data.expirationDate < block.timestamp) { - return balance_; - } - balance_ += currentItem.data.tokenAmount; - currentNonce = currentItem.next; - } - }*/ - function supportsInterface( bytes4 interfaceId ) public view virtual override(ERC1155Receiver, IERC165) returns (bool) { return interfaceId == type(ISftRolesRegistry).interfaceId || interfaceId == type(IERC1155Receiver).interfaceId || - interfaceId == type(ICommitTokensAndGrantRoleExtension).interfaceId; + interfaceId == type(ICommitTokensAndGrantRoleExtension).interfaceId || + interfaceId == type(IRoleBalanceOfExtension).interfaceId; } /** Helper Functions **/ @@ -295,7 +300,7 @@ contract SftRolesRegistry is ISftRolesRegistry, ERC1155Holder, ICommitTokensAndG commitments[_commitmentId].tokenAddress, commitments[_commitmentId].tokenId ); - LinkedLists.RoleData memory data = LinkedLists.RoleData(_expirationDate, _revocable, _data); + LinkedLists.RoleData memory data = LinkedLists.RoleData(_commitmentId, _expirationDate, _revocable, _data); uint256 itemId = _getItemId(_commitmentId, _role, _grantee); lists.insert(headKey, itemId, data); } diff --git a/contracts/RolesRegistry/interfaces/IRoleBalanceOfExtension.sol b/contracts/RolesRegistry/interfaces/IRoleBalanceOfExtension.sol new file mode 100644 index 0000000..1a1f549 --- /dev/null +++ b/contracts/RolesRegistry/interfaces/IRoleBalanceOfExtension.sol @@ -0,0 +1,18 @@ +// SPDX-License-Identifier: CC0-1.0 + +pragma solidity 0.8.9; + +interface IRoleBalanceOfExtension { + /// @notice Returns the sum of all tokenAmounts granted to the grantee for the given role. + /// @param _grantee The user for which the balance is returned. + /// @param _tokenAddress The token address. + /// @param _tokenId The token identifier. + /// @param _role The role identifier. + /// @return balance_ The balance of the grantee for the given role. + function roleBalanceOf( + bytes32 _role, + address _tokenAddress, + uint256 _tokenId, + address _grantee + ) external returns (uint256 balance_); +} diff --git a/contracts/RolesRegistry/libraries/LinkedLists.sol b/contracts/RolesRegistry/libraries/LinkedLists.sol index 2e58c00..7708159 100644 --- a/contracts/RolesRegistry/libraries/LinkedLists.sol +++ b/contracts/RolesRegistry/libraries/LinkedLists.sol @@ -12,6 +12,7 @@ library LinkedLists { uint256 public constant EMPTY = 0; struct RoleData { + uint256 commitmentId; uint64 expirationDate; bool revocable; bytes data; @@ -24,7 +25,7 @@ library LinkedLists { } struct Lists { - // headKey => commitmentId + // headKey => itemId mapping(bytes32 => uint256) heads; // hash(commitmentId, role, grantee) => item mapping(uint256 => ListItem) items; diff --git a/contracts/mocks/MockLinkedLists.sol b/contracts/mocks/MockLinkedLists.sol index 7e1a301..4a3fe71 100644 --- a/contracts/mocks/MockLinkedLists.sol +++ b/contracts/mocks/MockLinkedLists.sol @@ -16,27 +16,27 @@ contract MockLinkedLists { LinkedLists.Lists internal lists; - function insert(bytes32 _headKey, uint256 _nonce, uint64 _expirationDate) external { + function insert(bytes32 _headKey, uint256 _commitmentId, uint64 _expirationDate) external { // the only attribute that affects the list sorting is the expiration date - LinkedLists.RoleData memory data = LinkedLists.RoleData(_expirationDate, true, ''); - lists.insert(_headKey, _nonce, data); + LinkedLists.RoleData memory data = LinkedLists.RoleData(_commitmentId, _expirationDate, true, ''); + lists.insert(_headKey, _commitmentId, data); } - function remove(bytes32 _headKey, uint256 _nonce) external { - lists.remove(_headKey, _nonce); + function remove(bytes32 _headKey, uint256 _commitmentId) external { + lists.remove(_headKey, _commitmentId); } function getHeadNonce(bytes32 _headKey) external view returns (uint256) { return lists.heads[_headKey]; } - function getListItem(uint256 _nonce) public view returns (ListItem memory) { - LinkedLists.ListItem memory item = lists.items[_nonce]; + function getListItem(uint256 _commitmentId) public view returns (ListItem memory) { + LinkedLists.ListItem memory item = lists.items[_commitmentId]; return ListItem(item.data.expirationDate, item.previous, item.next); } function getListHead(bytes32 _headKey) external view returns (ListItem memory) { - uint256 nonce = lists.heads[_headKey]; - return getListItem(nonce); + uint256 commitmentId = lists.heads[_headKey]; + return getListItem(commitmentId); } } diff --git a/test/SftRolesRegistry/SftRolesRegistry.spec.ts b/test/SftRolesRegistry/SftRolesRegistry.spec.ts index ec90474..328f9b5 100644 --- a/test/SftRolesRegistry/SftRolesRegistry.spec.ts +++ b/test/SftRolesRegistry/SftRolesRegistry.spec.ts @@ -14,6 +14,7 @@ import { buildGrantRole, getSftRolesRegistryInterfaceId, getCommitTokensAndGrantRoleInterfaceId, + getRoleBalanceOfInterfaceId, } from './helpers/mockData' const { AddressZero } = ethers.constants @@ -519,6 +520,253 @@ describe('SftRolesRegistry', async () => { }) }) + describe('roleBalanceOf', async () => { + let CommitmentCreated: Commitment + let GrantRoleData: GrantRoleData + + beforeEach(async () => { + CommitmentCreated = buildCommitment({ + grantor: grantor.address, + tokenAddress: MockToken.address, + }) + GrantRoleData = await buildGrantRole({ + commitmentId: 1, + }) + await MockToken.connect(grantor).setApprovalForAll(SftRolesRegistry.address, true) + }) + + it('should return balance zero if grantee has no roles', async () => { + expect( + await SftRolesRegistry.roleBalanceOf( + GrantRoleData.role, + CommitmentCreated.tokenAddress, + CommitmentCreated.tokenId, + GrantRoleData.grantee, + ), + ).to.be.equal(0) + }) + + it('should add balance to three grantees', async () => { + const roles = [grantee.address, anotherUser.address, grantor.address].map(g => ({ + grantee: g, + tokenAmount: generateRandomInt(), + })) + + const totalTokenAmount = roles.map(r => r.tokenAmount).reduce((a, b) => a + b, 0) + await MockToken.connect(grantor).mint(grantor.address, CommitmentCreated.tokenId, totalTokenAmount) + + for (const role of roles) { + // make sure initial balance of grantee is zero + expect( + await SftRolesRegistry.roleBalanceOf( + GrantRoleData.role, + CommitmentCreated.tokenAddress, + CommitmentCreated.tokenId, + role.grantee, + ), + ).to.be.equal(0) + + // grant role + await expect( + SftRolesRegistry.connect(grantor).commitTokensAndGrantRole( + CommitmentCreated.grantor, + CommitmentCreated.tokenAddress, + CommitmentCreated.tokenId, + role.tokenAmount, + GrantRoleData.role, + role.grantee, + GrantRoleData.expirationDate, + GrantRoleData.revocable, + GrantRoleData.data, + ), + ).to.not.be.reverted + + // confirm that the balance increased + expect( + await SftRolesRegistry.roleBalanceOf( + GrantRoleData.role, + CommitmentCreated.tokenAddress, + CommitmentCreated.tokenId, + role.grantee, + ), + ).to.be.equal(role.tokenAmount) + } + }) + + it('should grant 10 roles, then revoke them, withdraw and wait for them to expire', async () => { + const currentDate = await time.latest() + const roles = Array(10) + .fill(0) + .map((v, index) => ({ + tokenAmount: generateRandomInt(), + expirationDate: currentDate + (index + 1) * ONE_DAY, + })) + const totalTokenAmount = roles.map(r => r.tokenAmount).reduce((a, b) => a + b, 0) + await MockToken.connect(grantor).mint(grantor.address, CommitmentCreated.tokenId, totalTokenAmount) + + // make sure initial balance of grantee is zero + expect( + await SftRolesRegistry.roleBalanceOf( + GrantRoleData.role, + CommitmentCreated.tokenAddress, + CommitmentCreated.tokenId, + GrantRoleData.grantee, + ), + ).to.be.equal(0) + + for (const role of roles) { + await expect( + SftRolesRegistry.connect(grantor).commitTokensAndGrantRole( + CommitmentCreated.grantor, + CommitmentCreated.tokenAddress, + CommitmentCreated.tokenId, + role.tokenAmount, + GrantRoleData.role, + GrantRoleData.grantee, + role.expirationDate, + GrantRoleData.revocable, + GrantRoleData.data, + ), + ).to.not.be.reverted + } + + // check if correctly summed tokenAmount + expect( + await SftRolesRegistry.roleBalanceOf( + GrantRoleData.role, + CommitmentCreated.tokenAddress, + CommitmentCreated.tokenId, + GrantRoleData.grantee, + ), + ).to.be.equal(totalTokenAmount) + + // revoke the first role + await expect(SftRolesRegistry.connect(grantor).revokeRole(1, GrantRoleData.role, GrantRoleData.grantee)).to.not.be + .reverted + + // revoke the last role + await expect( + SftRolesRegistry.connect(grantor).revokeRole(roles.length, GrantRoleData.role, GrantRoleData.grantee), + ).to.not.be.reverted + + // check if correctly summed leftovers + let leftoverTokenAmount = roles + .slice(1, roles.length - 1) + .map(r => r.tokenAmount) + .reduce((a, b) => a + b, 0) + + expect( + await SftRolesRegistry.roleBalanceOf( + GrantRoleData.role, + CommitmentCreated.tokenAddress, + CommitmentCreated.tokenId, + GrantRoleData.grantee, + ), + ).to.be.equal(leftoverTokenAmount) + + // expire second and third role + leftoverTokenAmount = roles + .slice(3, roles.length - 1) + .map(r => r.tokenAmount) + .reduce((a, b) => a + b, 0) + + await time.increase(ONE_DAY * 3) + expect( + await SftRolesRegistry.roleBalanceOf( + GrantRoleData.role, + CommitmentCreated.tokenAddress, + CommitmentCreated.tokenId, + GrantRoleData.grantee, + ), + ).to.be.equal(leftoverTokenAmount) + + // withdraw commitment + for (let i = 1; i < roles.length + 1; i++) { + await expect(SftRolesRegistry.connect(grantor).withdrawNfts(i)).to.not.be.reverted + } + expect( + await SftRolesRegistry.roleBalanceOf( + GrantRoleData.role, + CommitmentCreated.tokenAddress, + CommitmentCreated.tokenId, + GrantRoleData.grantee, + ), + ).to.be.equal(0) + }) + + it('should grant 1,000 roles and sum them up @skip-on-coverage', async () => { + const currentDate = await time.latest() + const roles = Array(1000) + .fill(0) + .map((v, index) => ({ + tokenAmount: generateRandomInt(), + expirationDate: currentDate + (1000 - index) * ONE_DAY, + })) + const totalTokenAmount = roles.map(r => r.tokenAmount).reduce((a, b) => a + b, 0) + await MockToken.connect(grantor).mint(grantor.address, CommitmentCreated.tokenId, totalTokenAmount) + + for (const role of roles) { + await expect( + SftRolesRegistry.connect(grantor).commitTokensAndGrantRole( + CommitmentCreated.grantor, + CommitmentCreated.tokenAddress, + CommitmentCreated.tokenId, + role.tokenAmount, + GrantRoleData.role, + GrantRoleData.grantee, + role.expirationDate, + GrantRoleData.revocable, + GrantRoleData.data, + ), + ).to.not.be.reverted + } + + // check if correctly summed tokenAmount + expect( + await SftRolesRegistry.roleBalanceOf( + GrantRoleData.role, + CommitmentCreated.tokenAddress, + CommitmentCreated.tokenId, + GrantRoleData.grantee, + ), + ).to.be.equal(totalTokenAmount) + }) + + it('should grant three roles inserting the last one in the middle of the list', async () => { + const currentDate = await time.latest() + const roles = [currentDate + ONE_DAY, currentDate + 3 * ONE_DAY, currentDate + 2 * ONE_DAY].map( + expirationDate => ({ expirationDate, tokenAmount: generateRandomInt() }), + ) + const totalTokenAmount = roles.map(r => r.tokenAmount).reduce((a, b) => a + b, 0) + await MockToken.connect(grantor).mint(grantor.address, CommitmentCreated.tokenId, totalTokenAmount) + + for (const role of roles) { + await expect( + SftRolesRegistry.connect(grantor).commitTokensAndGrantRole( + CommitmentCreated.grantor, + CommitmentCreated.tokenAddress, + CommitmentCreated.tokenId, + role.tokenAmount, + GrantRoleData.role, + GrantRoleData.grantee, + role.expirationDate, + GrantRoleData.revocable, + GrantRoleData.data, + ), + ).to.not.be.reverted + } + + expect( + await SftRolesRegistry.roleBalanceOf( + GrantRoleData.role, + CommitmentCreated.tokenAddress, + CommitmentCreated.tokenId, + GrantRoleData.grantee, + ), + ).to.be.equal(totalTokenAmount) + }) + }) + describe('View Functions', async () => { let CommitmentCreated: Commitment let GrantRoleData: GrantRoleData @@ -590,5 +838,10 @@ describe('SftRolesRegistry', async () => { const interfaceId = getCommitTokensAndGrantRoleInterfaceId() expect(await SftRolesRegistry.supportsInterface(interfaceId)).to.be.true }) + + it('should return true if IRoleBalanceOfExtension interface id', async () => { + const interfaceId = getRoleBalanceOfInterfaceId() + expect(await SftRolesRegistry.supportsInterface(interfaceId)).to.be.true + }) }) }) diff --git a/test/SftRolesRegistry/helpers/mockData.ts b/test/SftRolesRegistry/helpers/mockData.ts index b27b4aa..7e0956f 100644 --- a/test/SftRolesRegistry/helpers/mockData.ts +++ b/test/SftRolesRegistry/helpers/mockData.ts @@ -5,6 +5,7 @@ import { time } from '@nomicfoundation/hardhat-network-helpers' import { ethers } from 'ethers' import { ISftRolesRegistry__factory } from '../../../typechain-types' import { ICommitTokensAndGrantRoleExtension__factory } from '../../../typechain-types' +import { IRoleBalanceOfExtension__factory } from '../../../typechain-types' const { HashZero, AddressZero } = ethers.constants export const ONE_DAY = 60 * 60 * 24 @@ -89,6 +90,11 @@ export function getCommitTokensAndGrantRoleInterfaceId() { return generateErc165InterfaceId(iface) } +export function getRoleBalanceOfInterfaceId() { + const iface = IRoleBalanceOfExtension__factory.createInterface() + return generateErc165InterfaceId(iface) +} + function generateErc165InterfaceId(contractInterface: ethers.utils.Interface) { let interfaceID = ethers.constants.Zero const functions: string[] = Object.keys(contractInterface.functions).filter(f => f !== 'supportsInterface(bytes4)') From 52be8ef62bfd94d9ff78660d9abbd6e246968ba8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ernani=20S=C3=A3o=20Thiago?= Date: Wed, 27 Dec 2023 18:19:31 -0500 Subject: [PATCH 12/19] removed fuzzing tests --- contracts/test/SetupTest.sol | 21 ----- contracts/test/StfRolesRegistryTest.sol | 120 ------------------------ hardhat.config.ts | 2 +- 3 files changed, 1 insertion(+), 142 deletions(-) delete mode 100644 contracts/test/SetupTest.sol delete mode 100644 contracts/test/StfRolesRegistryTest.sol diff --git a/contracts/test/SetupTest.sol b/contracts/test/SetupTest.sol deleted file mode 100644 index a03c160..0000000 --- a/contracts/test/SetupTest.sol +++ /dev/null @@ -1,21 +0,0 @@ -// SPDX-License-Identifier: CC0-1.0 - -pragma solidity 0.8.9; - -import { Test } from 'forge-std/Test.sol'; -import { SftRolesRegistry } from '../RolesRegistry/SftRolesRegistry.sol'; -import { MockERC1155 } from '../mocks/MockERC1155.sol'; - -contract SetupTest is Test { - SftRolesRegistry public sftRolesRegistry; - MockERC1155 public mockERC1155; - - function setUp() public virtual { - _deployContracts(); - } - - function _deployContracts() internal { - sftRolesRegistry = new SftRolesRegistry(); - mockERC1155 = new MockERC1155(); - } -} diff --git a/contracts/test/StfRolesRegistryTest.sol b/contracts/test/StfRolesRegistryTest.sol deleted file mode 100644 index 70e08bd..0000000 --- a/contracts/test/StfRolesRegistryTest.sol +++ /dev/null @@ -1,120 +0,0 @@ -// SPDX-License-Identifier: CC0-1.0 - -pragma solidity 0.8.9; - -import { SetupTest } from './SetupTest.sol'; -import { IERCXXXX } from '../RolesRegistry/interfaces/IERCXXXX.sol'; - -contract SftRolesRegistryTest is SetupTest { - function testFuzz_grantRoleFrom( - uint256 nonce, - bytes32 role, - uint256 tokenId, - uint256 tokenAmount, - address grantee, - uint64 expirationDate, - bool revocable, - bytes memory data - ) public { - vm.assume(tokenAmount > 0); - vm.assume(nonce > 0); - vm.assume(expirationDate > block.timestamp + 1 days); - - IERCXXXX.RoleAssignment memory _roleAssignment = IERCXXXX.RoleAssignment({ - nonce: nonce, - role: role, - tokenAddress: address(mockERC1155), - tokenId: tokenId, - tokenAmount: tokenAmount, - grantor: msg.sender, - grantee: grantee, - expirationDate: expirationDate, - revocable: revocable, - data: data - }); - - _roleAssignment.tokenAddress = address(mockERC1155); - vm.startPrank(msg.sender); - mockERC1155.mint(msg.sender, _roleAssignment.tokenId, _roleAssignment.tokenAmount); - mockERC1155.setApprovalForAll(address(sftRolesRegistry), true); - sftRolesRegistry.grantRoleFrom(_roleAssignment); - vm.stopPrank(); - - uint256 _roleBalance = sftRolesRegistry.roleBalanceOf(role, address(mockERC1155), tokenId, grantee); - assertEq(_roleBalance, tokenAmount); - } - - function testFuzz_revokeRoleFrom( - uint256 nonce, - bytes32 role, - uint256 tokenId, - uint256 tokenAmount, - address grantee, - uint64 expirationDate, - bool revocable - ) public { - testFuzz_grantRoleFrom(nonce, role, tokenId, tokenAmount, grantee, expirationDate, revocable, ''); - - IERCXXXX.RevokeRoleData memory _revokeRoleData = IERCXXXX.RevokeRoleData({ - nonce: nonce, - role: role, - tokenAddress: address(mockERC1155), - tokenId: tokenId, - grantor: msg.sender - }); - - vm.startPrank(grantee); - sftRolesRegistry.revokeRoleFrom(_revokeRoleData); - vm.stopPrank(); - - uint256 _roleBalance = sftRolesRegistry.roleBalanceOf(role, address(mockERC1155), tokenId, grantee); - assertEq(_roleBalance, 0); - } - - function testFuzz_expirationDate( - uint256 nonce, - bytes32 role, - uint256 tokenId, - uint256 tokenAmount, - address grantee, - uint64 expirationDate, - bool revocable - ) public { - testFuzz_grantRoleFrom(nonce, role, tokenId, tokenAmount, grantee, expirationDate, revocable, ''); - uint256 _duration = expirationDate - block.timestamp; - skip(_duration + 1); - uint256 _roleBalance = sftRolesRegistry.roleBalanceOf(role, address(mockERC1155), tokenId, grantee); - assertEq(_roleBalance, 0); - } - - struct NonceTest { - uint256 nonce; - uint256 tokenId; - uint256 tokenAmount; - address grantee; - uint64 expirationDate; - } - - mapping(uint256 => bool) alreadyTestedNonce; - - function testFuzz_batchGrantRoleFrom(NonceTest[] memory _nonceTest) public { - bytes32 _role = keccak256('testFuzz_nonce'); - - for (uint256 i = 0; i < _nonceTest.length; i++) { - if (alreadyTestedNonce[_nonceTest[i].nonce]) continue; - alreadyTestedNonce[_nonceTest[i].nonce] = true; - vm.assume(_nonceTest[i].tokenAmount < 1000); - - testFuzz_grantRoleFrom( - _nonceTest[i].nonce, - _role, - _nonceTest[i].tokenId, - _nonceTest[i].tokenAmount, - _nonceTest[i].grantee, - _nonceTest[i].expirationDate, - true, - '' - ); - } - } -} diff --git a/hardhat.config.ts b/hardhat.config.ts index dd7fef5..468731a 100644 --- a/hardhat.config.ts +++ b/hardhat.config.ts @@ -6,7 +6,7 @@ import '@openzeppelin/hardhat-upgrades' import 'hardhat-spdx-license-identifier' import '@nomicfoundation/hardhat-toolbox' import '@openzeppelin/hardhat-defender' -import '@nomicfoundation/hardhat-foundry' +// import '@nomicfoundation/hardhat-foundry' dotenv.config() From f2e5c04fce8f18d75da21657adb3abd15ecdbd24 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ernani=20S=C3=A3o=20Thiago?= Date: Wed, 27 Dec 2023 18:22:42 -0500 Subject: [PATCH 13/19] renamed nonce to itemId --- .../RolesRegistry/libraries/LinkedLists.sol | 75 +++++++++---------- contracts/mocks/MockLinkedLists.sol | 2 +- test/SftRolesRegistry/helpers/mockData.ts | 6 +- test/SftRolesRegistry/types.ts | 4 +- test/helpers.ts | 44 +++++------ 5 files changed, 65 insertions(+), 66 deletions(-) diff --git a/contracts/RolesRegistry/libraries/LinkedLists.sol b/contracts/RolesRegistry/libraries/LinkedLists.sol index 7708159..2777a56 100644 --- a/contracts/RolesRegistry/libraries/LinkedLists.sol +++ b/contracts/RolesRegistry/libraries/LinkedLists.sol @@ -4,10 +4,9 @@ pragma solidity 0.8.9; import { ISftRolesRegistry } from '../interfaces/ISftRolesRegistry.sol'; -/// LinkedLists allow developers to manage multiple linked lists at once. -/// all lists are identified by a head key (bytes32) -/// each list item is identified by a nonce -/// the list is sorted by the expiration date in decreasing order +/// LinkedLists allow developers to manage multiple linked lists simultaneously. +/// All lists are identified by a head key (bytes32), and items by an itemId (uint256). +/// Lists are ordered by descending expiration date. library LinkedLists { uint256 public constant EMPTY = 0; @@ -31,88 +30,88 @@ library LinkedLists { mapping(uint256 => ListItem) items; } - function insert(Lists storage _self, bytes32 _headKey, uint256 _nonce, RoleData memory _data) internal { - require(_nonce != EMPTY, 'LinkedLists: invalid nonce'); + function insert(Lists storage _self, bytes32 _headKey, uint256 _itemId, RoleData memory _data) internal { + require(_itemId != EMPTY, 'LinkedLists: invalid itemId'); - uint256 headNonce = _self.heads[_headKey]; - if (headNonce == EMPTY) { + uint256 headItemId = _self.heads[_headKey]; + if (headItemId == EMPTY) { // if list is empty // insert as head - _self.heads[_headKey] = _nonce; - _self.items[_nonce] = ListItem(_data, EMPTY, EMPTY); + _self.heads[_headKey] = _itemId; + _self.items[_itemId] = ListItem(_data, EMPTY, EMPTY); return; } - ListItem storage headItem = _self.items[headNonce]; + ListItem storage headItem = _self.items[headItemId]; if (_data.expirationDate > headItem.data.expirationDate) { // if expirationDate is greater than head's expirationDate // update current head - headItem.previous = _nonce; + headItem.previous = _itemId; // insert as head - _self.heads[_headKey] = _nonce; - _self.items[_nonce] = ListItem(_data, EMPTY, headNonce); + _self.heads[_headKey] = _itemId; + _self.items[_itemId] = ListItem(_data, EMPTY, headItemId); return; } // search where to insert - uint256 currentNonce = headNonce; + uint256 currentItemId = headItemId; while ( - _self.items[currentNonce].next != EMPTY && - _data.expirationDate < _self.items[_self.items[currentNonce].next].data.expirationDate + _self.items[currentItemId].next != EMPTY && + _data.expirationDate < _self.items[_self.items[currentItemId].next].data.expirationDate ) { - currentNonce = _self.items[currentNonce].next; + currentItemId = _self.items[currentItemId].next; } - _insertAt(_self, currentNonce, _nonce, _data); + _insertAt(_self, currentItemId, _itemId, _data); } function _insertAt( Lists storage _self, - uint256 _previousNonce, - uint256 _dataNonce, + uint256 _previousItemId, + uint256 _dataItemId, RoleData memory _data ) internal { - ListItem storage previousItem = _self.items[_previousNonce]; + ListItem storage previousItem = _self.items[_previousItemId]; if (previousItem.next == EMPTY) { // insert as last item - _self.items[_dataNonce] = ListItem(_data, _previousNonce, EMPTY); + _self.items[_dataItemId] = ListItem(_data, _previousItemId, EMPTY); } else { // insert in the middle - _self.items[_dataNonce] = ListItem(_data, _previousNonce, previousItem.next); + _self.items[_dataItemId] = ListItem(_data, _previousItemId, previousItem.next); // modify next item - _self.items[previousItem.next].previous = _dataNonce; + _self.items[previousItem.next].previous = _dataItemId; } // modify previous item - previousItem.next = _dataNonce; + previousItem.next = _dataItemId; } - function remove(Lists storage _self, bytes32 _headKey, uint256 _nonce) internal { - uint256 headNonce = _self.heads[_headKey]; + function remove(Lists storage _self, bytes32 _headKey, uint256 _itemId) internal { + uint256 headItemId = _self.heads[_headKey]; require( - headNonce != EMPTY && _self.items[_nonce].data.expirationDate != 0, - 'LinkedLists: empty list or invalid nonce' + headItemId != EMPTY && _self.items[_itemId].data.expirationDate != 0, + 'LinkedLists: empty list or invalid itemId' ); // only the head has previous as empty - if (_self.items[_nonce].previous == EMPTY) { + if (_self.items[_itemId].previous == EMPTY) { // if item is the head // check if correct headKey was provided - require(headNonce == _nonce, 'LinkedLists: invalid headKey provided'); + require(headItemId == _itemId, 'LinkedLists: invalid headKey provided'); // remove head - if (_self.items[_nonce].next == EMPTY) { + if (_self.items[_itemId].next == EMPTY) { // list contains only one item delete _self.heads[_headKey]; } else { // list contains more than one item // set new head - uint256 newHeadNonce = _self.items[_nonce].next; - _self.heads[_headKey] = newHeadNonce; + uint256 newHeadItemId = _self.items[_itemId].next; + _self.heads[_headKey] = newHeadItemId; // remove previous item of new head - _self.items[newHeadNonce].previous = EMPTY; + _self.items[newHeadItemId].previous = EMPTY; } } else { // remove non-head item - ListItem storage itemToRemove = _self.items[_nonce]; + ListItem storage itemToRemove = _self.items[_itemId]; // update previous item _self.items[itemToRemove.previous].next = itemToRemove.next; if (itemToRemove.next != EMPTY) { @@ -123,6 +122,6 @@ library LinkedLists { } // delete item from storage - delete _self.items[_nonce]; + delete _self.items[_itemId]; } } diff --git a/contracts/mocks/MockLinkedLists.sol b/contracts/mocks/MockLinkedLists.sol index 4a3fe71..1357489 100644 --- a/contracts/mocks/MockLinkedLists.sol +++ b/contracts/mocks/MockLinkedLists.sol @@ -26,7 +26,7 @@ contract MockLinkedLists { lists.remove(_headKey, _commitmentId); } - function getHeadNonce(bytes32 _headKey) external view returns (uint256) { + function getHeadItemId(bytes32 _headKey) external view returns (uint256) { return lists.heads[_headKey]; } diff --git a/test/SftRolesRegistry/helpers/mockData.ts b/test/SftRolesRegistry/helpers/mockData.ts index 7e0956f..bc844a5 100644 --- a/test/SftRolesRegistry/helpers/mockData.ts +++ b/test/SftRolesRegistry/helpers/mockData.ts @@ -43,7 +43,7 @@ export function generateRoleId(role: string) { export async function buildRoleAssignment({ // default values - nonce = generateRandomInt(), + itemId = generateRandomInt(), role = 'UNIQUE_ROLE', tokenAddress = AddressZero, tokenId = generateRandomInt(), @@ -55,7 +55,7 @@ export async function buildRoleAssignment({ data = HashZero, }: { // types - nonce?: number + itemId?: number role?: string tokenAddress?: string tokenId?: number @@ -67,7 +67,7 @@ export async function buildRoleAssignment({ data?: string } = {}): Promise { return { - nonce, + itemId, role: generateRoleId(role), tokenAddress, tokenId, diff --git a/test/SftRolesRegistry/types.ts b/test/SftRolesRegistry/types.ts index abf0f3c..7ad0e2c 100644 --- a/test/SftRolesRegistry/types.ts +++ b/test/SftRolesRegistry/types.ts @@ -15,7 +15,7 @@ export interface GrantRoleData { } export interface RoleAssignment { - nonce: number + itemId: number role: string tokenAddress: string tokenId: number @@ -28,7 +28,7 @@ export interface RoleAssignment { } export interface RevokeRoleData { - nonce: number + itemId: number role: string tokenAddress: string tokenId: number diff --git a/test/helpers.ts b/test/helpers.ts index 5e5bcba..f7f795a 100644 --- a/test/helpers.ts +++ b/test/helpers.ts @@ -8,16 +8,16 @@ import { expect } from 'chai' * @param expectedLength The number of items expected in the list */ export async function assertList(LinkedLists: Contract, listId: string, expectedLength: number) { - const headNonce = await LinkedLists.getHeadNonce(listId) - if (headNonce.toNumber() === 0) { + const headItemId = await LinkedLists.getHeadItemId(listId) + if (headItemId.toNumber() === 0) { return expect(expectedLength, 'List is empty, head should be zero').to.be.equal(0) } // assert head - let item = await LinkedLists.getListItem(headNonce) + let item = await LinkedLists.getListItem(headItemId) expect(item.previous, 'Head previous should be zero').to.be.equal(0) - let previous = headNonce.toNumber() + let previous = headItemId.toNumber() let previousExpirationDate = item.expirationDate.toNumber() let next = item.next.toNumber() let listLength = 1 @@ -46,28 +46,28 @@ export async function assertList(LinkedLists: Contract, listId: string, expected } /** - * Validates the item nonce, expiration date, and position in the list + * Validates the item itemId, expiration date, and position in the list * @param LinkedLists The MockLinkedLists contract * @param listId The bytes32 identifier of the list - * @param itemNonce The nonce of the item + * @param itemId The itemId of the item * @param itemExpirationDate The expiration date of the item * @param expectedPosition The expected position of the item in the list */ export async function assertListItem( LinkedLists: Contract, listId: string, - itemNonce: number, + itemId: number, itemExpirationDate: number, expectedPosition: number, ) { - const { expirationDate, previous } = await LinkedLists.getListItem(itemNonce) - expect(expirationDate, `Item ${itemNonce} expiration date is not ${itemExpirationDate}`).to.be.equal( + const { expirationDate, previous } = await LinkedLists.getListItem(itemId) + expect(expirationDate, `Item ${itemId} expiration date is not ${itemExpirationDate}`).to.be.equal( itemExpirationDate, ) if (expectedPosition === 0) { // if item is the head - expect(await LinkedLists.getHeadNonce(listId), `Item ${itemNonce} should be the head`).to.equal(itemNonce) + expect(await LinkedLists.getHeadItemId(listId), `Item ${itemId} should be the head`).to.equal(itemId) return expect(previous, 'Head previous should be zero').to.be.equal(0) } @@ -76,14 +76,14 @@ export async function assertListItem( // assert position let position = 0 - let currentNonce = (await LinkedLists.getHeadNonce(listId)).toNumber() + let currentItem = (await LinkedLists.getHeadItemId(listId)).toNumber() let item = await LinkedLists.getListHead(listId) - while (currentNonce !== 0) { - if (currentNonce === itemNonce) { + while (currentItem !== 0) { + if (currentItem === itemId) { return expect(position, 'Item is not on expected position').to.be.equal(expectedPosition) } - item = await LinkedLists.getListItem(currentNonce) - currentNonce = item.next.toNumber() + item = await LinkedLists.getListItem(currentItem) + currentItem = item.next.toNumber() position += 1 } expect.fail('Item not found in list') @@ -91,22 +91,22 @@ export async function assertListItem( export async function printList(LinkedLists: Contract, listId: string) { console.log('\n== List ==============================================') - const headNonce = (await LinkedLists.getHeadNonce(listId)).toNumber() - if (headNonce === 0) { + const headItemId = (await LinkedLists.getHeadItemId(listId)).toNumber() + if (headItemId === 0) { console.log('\tList is empty!') return console.log('== End of List =======================================\n') } let position = 0 - let currentNonce = headNonce - while (currentNonce !== 0) { - const currentItem = await LinkedLists.getListItem(currentNonce) + let currentItemId = headItemId + while (currentItemId !== 0) { + const currentItem = await LinkedLists.getListItem(currentItemId) console.log(`\n\tItem ${position}:`) - console.log(`\t\tNonce: ${currentNonce}`) + console.log(`\t\tItemId: ${currentItem}`) console.log(`\t\tExpiration Date: ${currentItem.expirationDate}`) console.log(`\t\tPrevious: ${currentItem.previous.toNumber()}`) console.log(`\t\tNext: ${currentItem.next.toNumber()}`) - currentNonce = currentItem.next.toNumber() + currentItemId = currentItem.next.toNumber() position += 1 } From a035e1cc679a31429ce1ca80b805ae2f33c1a3c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ernani=20S=C3=A3o=20Thiago?= Date: Thu, 28 Dec 2023 08:39:32 -0500 Subject: [PATCH 14/19] fixed lint & removed forge from CI --- .github/workflows/all.yml | 4 ++-- test/helpers.ts | 4 +--- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/.github/workflows/all.yml b/.github/workflows/all.yml index 37cdeeb..e09d64e 100644 --- a/.github/workflows/all.yml +++ b/.github/workflows/all.yml @@ -25,8 +25,8 @@ jobs: NODE_AUTH_TOKEN: ${{ secrets.GHB_TOKEN }} - name: Lint run: npm run lint - - name: Run Foundry tests - run: forge test --verbosity -vvv +# - name: Run Foundry tests +# run: forge test --verbosity -vvv - name: Compile Smart Contracts run: npm run compile env: diff --git a/test/helpers.ts b/test/helpers.ts index f7f795a..3aacbf7 100644 --- a/test/helpers.ts +++ b/test/helpers.ts @@ -61,9 +61,7 @@ export async function assertListItem( expectedPosition: number, ) { const { expirationDate, previous } = await LinkedLists.getListItem(itemId) - expect(expirationDate, `Item ${itemId} expiration date is not ${itemExpirationDate}`).to.be.equal( - itemExpirationDate, - ) + expect(expirationDate, `Item ${itemId} expiration date is not ${itemExpirationDate}`).to.be.equal(itemExpirationDate) if (expectedPosition === 0) { // if item is the head From e0c43dc10cdcec092cd9479b5ef251b0ea84d4f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ernani=20S=C3=A3o=20Thiago?= Date: Thu, 4 Jan 2024 17:47:13 -0500 Subject: [PATCH 15/19] renamed function withdrawNfts to releaseTokens, and event NftsWithdrawn to TokensReleased --- contracts/RolesRegistry/SftRolesRegistry.sol | 4 +-- .../SftRolesRegistrySingleRole.sol | 4 +-- .../ICommitTokensAndGrantRoleExtension.sol | 5 ++-- .../interfaces/IRoleBalanceOfExtension.sol | 4 +-- .../interfaces/ISftRolesRegistry.sol | 28 ++++++++--------- .../SftRolesRegistry/SftRolesRegistry.spec.ts | 30 +++++++++---------- .../SftRolesRegistrySingleRole.spec.ts | 24 +++++++-------- 7 files changed, 50 insertions(+), 49 deletions(-) diff --git a/contracts/RolesRegistry/SftRolesRegistry.sol b/contracts/RolesRegistry/SftRolesRegistry.sol index 69f9930..58abc5e 100644 --- a/contracts/RolesRegistry/SftRolesRegistry.sol +++ b/contracts/RolesRegistry/SftRolesRegistry.sol @@ -99,7 +99,7 @@ contract SftRolesRegistry is emit RoleRevoked(_commitmentId, _role, _grantee); } - function withdrawNfts( + function releaseTokens( uint256 _commitmentId ) external onlyOwnerOrApproved(commitments[_commitmentId].grantor, commitments[_commitmentId].tokenAddress) { uint256 numberOfRoles = commitmentIdToRoles[_commitmentId].length(); @@ -135,7 +135,7 @@ contract SftRolesRegistry is ); delete commitments[_commitmentId]; - emit NftsWithdrawn(_commitmentId); + emit TokensReleased(_commitmentId); } function setRoleApprovalForAll(address _tokenAddress, address _operator, bool _isApproved) external override { diff --git a/contracts/RolesRegistry/SftRolesRegistrySingleRole.sol b/contracts/RolesRegistry/SftRolesRegistrySingleRole.sol index f90d894..8ae97ee 100644 --- a/contracts/RolesRegistry/SftRolesRegistrySingleRole.sol +++ b/contracts/RolesRegistry/SftRolesRegistrySingleRole.sol @@ -90,7 +90,7 @@ contract SftRolesRegistrySingleRole is ISftRolesRegistry, ERC1155Holder, ICommit delete roleAssignments[_commitmentId][_role]; } - function withdrawNfts( + function releaseTokens( uint256 _commitmentId ) external onlyOwnerOrApproved(commitments[_commitmentId].grantor, commitments[_commitmentId].tokenAddress) { require( @@ -110,7 +110,7 @@ contract SftRolesRegistrySingleRole is ISftRolesRegistry, ERC1155Holder, ICommit ); delete commitments[_commitmentId]; - emit NftsWithdrawn(_commitmentId); + emit TokensReleased(_commitmentId); } function setRoleApprovalForAll(address _tokenAddress, address _operator, bool _isApproved) external override { diff --git a/contracts/RolesRegistry/interfaces/ICommitTokensAndGrantRoleExtension.sol b/contracts/RolesRegistry/interfaces/ICommitTokensAndGrantRoleExtension.sol index 54fb598..c45fcd3 100644 --- a/contracts/RolesRegistry/interfaces/ICommitTokensAndGrantRoleExtension.sol +++ b/contracts/RolesRegistry/interfaces/ICommitTokensAndGrantRoleExtension.sol @@ -4,15 +4,16 @@ pragma solidity 0.8.9; interface ICommitTokensAndGrantRoleExtension { /// @notice Commits tokens and grant role in a single transaction. - /// @param _grantor The owner of the NFTs. + /// @param _grantor The owner of the SFTs. /// @param _tokenAddress The token address. /// @param _tokenId The token identifier. /// @param _tokenAmount The token amount. /// @param _role The role identifier. - /// @param _grantee The user receiving the role. + /// @param _grantee The recipient the role. /// @param _expirationDate The expiration date of the role. /// @param _revocable Whether the role is revocable or not. /// @param _data Any additional data about the role. + /// @return commitmentId_ The identifier of the commitment created. function commitTokensAndGrantRole( address _grantor, address _tokenAddress, diff --git a/contracts/RolesRegistry/interfaces/IRoleBalanceOfExtension.sol b/contracts/RolesRegistry/interfaces/IRoleBalanceOfExtension.sol index 1a1f549..fb0887d 100644 --- a/contracts/RolesRegistry/interfaces/IRoleBalanceOfExtension.sol +++ b/contracts/RolesRegistry/interfaces/IRoleBalanceOfExtension.sol @@ -4,10 +4,10 @@ pragma solidity 0.8.9; interface IRoleBalanceOfExtension { /// @notice Returns the sum of all tokenAmounts granted to the grantee for the given role. - /// @param _grantee The user for which the balance is returned. + /// @param _role The role identifier. /// @param _tokenAddress The token address. /// @param _tokenId The token identifier. - /// @param _role The role identifier. + /// @param _grantee The user for which the balance is returned. /// @return balance_ The balance of the grantee for the given role. function roleBalanceOf( bytes32 _role, diff --git a/contracts/RolesRegistry/interfaces/ISftRolesRegistry.sol b/contracts/RolesRegistry/interfaces/ISftRolesRegistry.sol index 79456c7..f850574 100644 --- a/contracts/RolesRegistry/interfaces/ISftRolesRegistry.sol +++ b/contracts/RolesRegistry/interfaces/ISftRolesRegistry.sol @@ -22,7 +22,7 @@ interface ISftRolesRegistry is IERC165 { /** Events **/ /// @notice Emitted when tokens are committed (deposited or frozen). - /// @param _grantor The owner of the NFTs. + /// @param _grantor The owner of the SFTs. /// @param _commitmentId The identifier of the commitment created. /// @param _tokenAddress The token address. /// @param _tokenId The token identifier. @@ -38,7 +38,7 @@ interface ISftRolesRegistry is IERC165 { /// @notice Emitted when a role is granted. /// @param _commitmentId The commitment identifier. /// @param _role The role identifier. - /// @param _grantee The user receiving the role. + /// @param _grantee The recipient the role. /// @param _expirationDate The expiration date of the role. /// @param _revocable Whether the role is revocable or not. /// @param _data Any additional data about the role. @@ -54,12 +54,12 @@ interface ISftRolesRegistry is IERC165 { /// @notice Emitted when a role is revoked. /// @param _commitmentId The commitment identifier. /// @param _role The role identifier. - /// @param _grantee The user that receives the role revocation. + /// @param _grantee The recipient of the role revocation. event RoleRevoked(uint256 indexed _commitmentId, bytes32 indexed _role, address indexed _grantee); - /// @notice Emitted when a user withdrawNfts tokens from a commitment. + /// @notice Emitted when a user releases tokens from a commitment. /// @param _commitmentId The commitment identifier. - event NftsWithdrawn(uint256 indexed _commitmentId); + event TokensReleased(uint256 indexed _commitmentId); /// @notice Emitted when a user is approved to manage roles on behalf of another user. /// @param _tokenAddress The token address. @@ -70,7 +70,7 @@ interface ISftRolesRegistry is IERC165 { /** External Functions **/ /// @notice Commits tokens (deposits on a contract or freezes balance). - /// @param _grantor The owner of the NFTs. + /// @param _grantor The owner of the SFTs. /// @param _tokenAddress The token address. /// @param _tokenId The token identifier. /// @param _tokenAmount The token amount. @@ -85,7 +85,7 @@ interface ISftRolesRegistry is IERC165 { /// @notice Grants a role to `_grantee`. /// @param _commitmentId The identifier of the commitment. /// @param _role The role identifier. - /// @param _grantee The user receiving the role. + /// @param _grantee The recipient the role. /// @param _expirationDate The expiration date of the role. /// @param _revocable Whether the role is revocable or not. /// @param _data Any additional data about the role. @@ -101,12 +101,12 @@ interface ISftRolesRegistry is IERC165 { /// @notice Revokes a role. /// @param _commitmentId The commitment identifier. /// @param _role The role identifier. - /// @param _grantee The user that gets their role revoked. + /// @param _grantee The recipient of the role revocation. function revokeRole(uint256 _commitmentId, bytes32 _role, address _grantee) external; - /// @notice Withdraws tokens back to grantor. + /// @notice Releases tokens back to grantor. /// @param _commitmentId The commitment identifier. - function withdrawNfts(uint256 _commitmentId) external; + function releaseTokens(uint256 _commitmentId) external; /// @notice Approves operator to grant and revoke roles on behalf of another user. /// @param _tokenAddress The token address. @@ -139,7 +139,7 @@ interface ISftRolesRegistry is IERC165 { /// @notice Returns the custom data of a role assignment. /// @param _commitmentId The commitment identifier. /// @param _role The role identifier. - /// @param _grantee The user that received the role. + /// @param _grantee The recipient the role. /// @return data_ The custom data. function roleData( uint256 _commitmentId, @@ -150,7 +150,7 @@ interface ISftRolesRegistry is IERC165 { /// @notice Returns the expiration date of a role assignment. /// @param _commitmentId The commitment identifier. /// @param _role The role identifier. - /// @param _grantee The user that received the role. + /// @param _grantee The recipient the role. /// @return expirationDate_ The expiration date. function roleExpirationDate( uint256 _commitmentId, @@ -161,7 +161,7 @@ interface ISftRolesRegistry is IERC165 { /// @notice Returns the expiration date of a role assignment. /// @param _commitmentId The commitment identifier. /// @param _role The role identifier. - /// @param _grantee The user that received the role. + /// @param _grantee The recipient the role. /// @return revocable_ Whether the role is revocable or not. function isRoleRevocable( uint256 _commitmentId, @@ -169,7 +169,7 @@ interface ISftRolesRegistry is IERC165 { address _grantee ) external view returns (bool revocable_); - /// @notice Checks if the grantor approved the operator for all NFTs. + /// @notice Checks if the grantor approved the operator for all SFTs. /// @param _tokenAddress The token address. /// @param _grantor The user that approved the operator. /// @param _operator The user that can grant and revoke roles. diff --git a/test/SftRolesRegistry/SftRolesRegistry.spec.ts b/test/SftRolesRegistry/SftRolesRegistry.spec.ts index 328f9b5..2ae2e33 100644 --- a/test/SftRolesRegistry/SftRolesRegistry.spec.ts +++ b/test/SftRolesRegistry/SftRolesRegistry.spec.ts @@ -336,7 +336,7 @@ describe('SftRolesRegistry', async () => { }) }) - describe('withdrawNfts', async () => { + describe('releaseTokens', async () => { let CommitmentCreated: Commitment let GrantRoleData: GrantRoleData @@ -346,21 +346,21 @@ describe('SftRolesRegistry', async () => { }) it('should revert when sender is not grantor or approved', async () => { - await expect(SftRolesRegistry.connect(anotherUser).withdrawNfts(GrantRoleData.commitmentId)).to.be.revertedWith( + await expect(SftRolesRegistry.connect(anotherUser).releaseTokens(GrantRoleData.commitmentId)).to.be.revertedWith( 'SftRolesRegistry: account not approved', ) }) it('should revert when there is an active role', async () => { await assertGrantRoleEvent(SftRolesRegistry, grantor, 1, grantee.address, false) - await expect(SftRolesRegistry.connect(grantor).withdrawNfts(GrantRoleData.commitmentId)).to.be.revertedWith( + await expect(SftRolesRegistry.connect(grantor).releaseTokens(GrantRoleData.commitmentId)).to.be.revertedWith( 'SftRolesRegistry: commitment has an active non-revocable role', ) }) - it('should withdraw when there are revocable roles', async () => { - await expect(SftRolesRegistry.connect(grantor).withdrawNfts(GrantRoleData.commitmentId)) - .to.emit(SftRolesRegistry, 'NftsWithdrawn') + it('should release when there are revocable roles', async () => { + await expect(SftRolesRegistry.connect(grantor).releaseTokens(GrantRoleData.commitmentId)) + .to.emit(SftRolesRegistry, 'TokensReleased') .withArgs(GrantRoleData.commitmentId) .to.emit(MockToken, 'TransferSingle') .withArgs( @@ -372,10 +372,10 @@ describe('SftRolesRegistry', async () => { ) }) - it('should withdraw when there are no roles', async () => { + it('should release when there are no roles', async () => { await assertRevokeRoleEvent(SftRolesRegistry, grantor, GrantRoleData.commitmentId, GrantRoleData.role, grantee) - await expect(SftRolesRegistry.connect(grantor).withdrawNfts(GrantRoleData.commitmentId)) - .to.emit(SftRolesRegistry, 'NftsWithdrawn') + await expect(SftRolesRegistry.connect(grantor).releaseTokens(GrantRoleData.commitmentId)) + .to.emit(SftRolesRegistry, 'TokensReleased') .withArgs(GrantRoleData.commitmentId) .to.emit(MockToken, 'TransferSingle') .withArgs( @@ -387,11 +387,11 @@ describe('SftRolesRegistry', async () => { ) }) - it('should withdraw when there are expired roles', async () => { + it('should release when there are expired roles', async () => { await assertGrantRoleEvent(SftRolesRegistry, grantor, 1, grantee.address, false) await time.increase(ONE_DAY) - await expect(SftRolesRegistry.connect(grantor).withdrawNfts(GrantRoleData.commitmentId)) - .to.emit(SftRolesRegistry, 'NftsWithdrawn') + await expect(SftRolesRegistry.connect(grantor).releaseTokens(GrantRoleData.commitmentId)) + .to.emit(SftRolesRegistry, 'TokensReleased') .withArgs(GrantRoleData.commitmentId) .to.emit(MockToken, 'TransferSingle') .withArgs( @@ -593,7 +593,7 @@ describe('SftRolesRegistry', async () => { } }) - it('should grant 10 roles, then revoke them, withdraw and wait for them to expire', async () => { + it('should grant 10 roles, then revoke them, release and wait for them to expire', async () => { const currentDate = await time.latest() const roles = Array(10) .fill(0) @@ -680,9 +680,9 @@ describe('SftRolesRegistry', async () => { ), ).to.be.equal(leftoverTokenAmount) - // withdraw commitment + // release commitment for (let i = 1; i < roles.length + 1; i++) { - await expect(SftRolesRegistry.connect(grantor).withdrawNfts(i)).to.not.be.reverted + await expect(SftRolesRegistry.connect(grantor).releaseTokens(i)).to.not.be.reverted } expect( await SftRolesRegistry.roleBalanceOf( diff --git a/test/SftRolesRegistry/SftRolesRegistrySingleRole.spec.ts b/test/SftRolesRegistry/SftRolesRegistrySingleRole.spec.ts index 85fa0d2..dd27d7f 100644 --- a/test/SftRolesRegistry/SftRolesRegistrySingleRole.spec.ts +++ b/test/SftRolesRegistry/SftRolesRegistrySingleRole.spec.ts @@ -308,7 +308,7 @@ describe('SftRolesRegistrySingleRole', async () => { }) }) - describe('withdrawNfts', async () => { + describe('releaseTokens', async () => { let CommitmentCreated: Commitment let GrantRoleData: GrantRoleData @@ -318,7 +318,7 @@ describe('SftRolesRegistrySingleRole', async () => { }) it('should revert when sender is not grantor or approved', async () => { - await expect(SftRolesRegistry.connect(anotherUser).withdrawNfts(GrantRoleData.commitmentId)).to.be.revertedWith( + await expect(SftRolesRegistry.connect(anotherUser).releaseTokens(GrantRoleData.commitmentId)).to.be.revertedWith( 'SftRolesRegistry: account not approved', ) }) @@ -334,12 +334,12 @@ describe('SftRolesRegistrySingleRole', async () => { GrantRoleData.data, ), ).to.not.be.reverted - await expect(SftRolesRegistry.connect(grantor).withdrawNfts(GrantRoleData.commitmentId)).to.be.revertedWith( + await expect(SftRolesRegistry.connect(grantor).releaseTokens(GrantRoleData.commitmentId)).to.be.revertedWith( 'SftRolesRegistry: commitment has an active non-revocable role', ) }) - it('should withdraw when role has an expired non-revocable role', async () => { + it('should release when role has an expired non-revocable role', async () => { await expect( SftRolesRegistry.connect(grantor).grantRole( GrantRoleData.commitmentId, @@ -351,8 +351,8 @@ describe('SftRolesRegistrySingleRole', async () => { ), ).to.not.be.reverted await time.increase(ONE_DAY) - await expect(SftRolesRegistry.connect(grantor).withdrawNfts(GrantRoleData.commitmentId)) - .to.emit(SftRolesRegistry, 'NftsWithdrawn') + await expect(SftRolesRegistry.connect(grantor).releaseTokens(GrantRoleData.commitmentId)) + .to.emit(SftRolesRegistry, 'TokensReleased') .withArgs(GrantRoleData.commitmentId) .to.emit(MockToken, 'TransferSingle') .withArgs( @@ -364,9 +364,9 @@ describe('SftRolesRegistrySingleRole', async () => { ) }) - it('should withdraw tokens when sender is grantor', async () => { - await expect(SftRolesRegistry.connect(grantor).withdrawNfts(GrantRoleData.commitmentId)) - .to.emit(SftRolesRegistry, 'NftsWithdrawn') + it('should release tokens when sender is grantor', async () => { + await expect(SftRolesRegistry.connect(grantor).releaseTokens(GrantRoleData.commitmentId)) + .to.emit(SftRolesRegistry, 'TokensReleased') .withArgs(GrantRoleData.commitmentId) .to.emit(MockToken, 'TransferSingle') .withArgs( @@ -378,14 +378,14 @@ describe('SftRolesRegistrySingleRole', async () => { ) }) - it('should withdraw tokens when sender is approved', async () => { + it('should release tokens when sender is approved', async () => { await SftRolesRegistry.connect(grantor).setRoleApprovalForAll( CommitmentCreated.tokenAddress, anotherUser.address, true, ) - await expect(SftRolesRegistry.connect(anotherUser).withdrawNfts(GrantRoleData.commitmentId)) - .to.emit(SftRolesRegistry, 'NftsWithdrawn') + await expect(SftRolesRegistry.connect(anotherUser).releaseTokens(GrantRoleData.commitmentId)) + .to.emit(SftRolesRegistry, 'TokensReleased') .withArgs(GrantRoleData.commitmentId) .to.emit(MockToken, 'TransferSingle') .withArgs( From 2e2e94c8d511595bf2814b960f6c0602342d3bb2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ernani=20S=C3=A3o=20Thiago?= Date: Mon, 8 Jan 2024 16:22:28 -0500 Subject: [PATCH 16/19] included correct eip number --- README.md | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 72f1db1..b1761a8 100644 --- a/README.md +++ b/README.md @@ -7,11 +7,9 @@ [![Discord](https://img.shields.io/discord/1009147970832322632?label=discord&logo=discord&logoColor=white)](https://discord.gg/NaNTgPK5rx) [![Twitter Follow](https://img.shields.io/twitter/follow/oriumnetwork?label=Follow&style=social)](https://twitter.com/OriumNetwork) -[comment]: <> (TODO: Add link and EIP number for ERC-1155 Roles Registry when available) - This repository contains multiple implementations of two EIPs (Ethereum Improvement Proposals): * ERC-7432 (Non-Fungible Token Roles). -* ERC-TBD (Semi-Fungible Token Roles). +* ERC-7589 (Semi-Fungible Token Roles). The goal of these EIPs is to introduce role management for NFTs. Each role assignment is associated with one or more NFTs and expire automatically at a given timestamp. Token Roles can be deeply integrated with dApps to create a @@ -21,13 +19,13 @@ ownership. The same user could also grant multiple `keccak256("PROPERTY_TENANT") access the digital property. You can find the full specification here: [ERC-721 Token Roles](https://eips.ethereum.org/EIPS/eip-7432) and -[ERC-1155 Token Roles](TBD). +[ERC-1155 Token Roles](https://eips.ethereum.org/EIPS/eip-7589). ## Implementations * [ERC-7432 NFT Roles Registry](./contracts/RolesRegistry.sol): ERC-721 NFT Owners can grant roles without depositing NFTs. -* [ERC-TBD SFT Roles Registry](./contracts/RolesRegistry/SftRolesRegistry.sol): ERC-1155 NFT Owners can grant roles after depositing NFTs. -* [ERC-TBD Single-Role SFT Roles Registry](./contracts/RolesRegistry/SftRolesRegistrySingleRole.sol): ERC-1155 NFT Owners can grant a single pre-defined role after depositing +* [ERC-7589 SFT Roles Registry](./contracts/RolesRegistry/SftRolesRegistry.sol): ERC-1155 NFT Owners can grant roles after depositing NFTs. +* [ERC-7589 Single-Role SFT Roles Registry](./contracts/RolesRegistry/SftRolesRegistrySingleRole.sol): ERC-1155 NFT Owners can grant a single pre-defined role after depositing NFTs. ## Build From f0084e5740735d7319906aecec71ed2cac5b4bbc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ernani=20S=C3=A3o=20Thiago?= Date: Mon, 8 Jan 2024 16:49:59 -0500 Subject: [PATCH 17/19] fixed EnumerableSet clear bug --- contracts/RolesRegistry/SftRolesRegistry.sol | 4 ++-- .../SftRolesRegistry/SftRolesRegistry.spec.ts | 23 +++++++++++-------- test/SftRolesRegistry/helpers/assertEvents.ts | 2 ++ 3 files changed, 18 insertions(+), 11 deletions(-) diff --git a/contracts/RolesRegistry/SftRolesRegistry.sol b/contracts/RolesRegistry/SftRolesRegistry.sol index 58abc5e..3c4bc25 100644 --- a/contracts/RolesRegistry/SftRolesRegistry.sol +++ b/contracts/RolesRegistry/SftRolesRegistry.sol @@ -103,8 +103,8 @@ contract SftRolesRegistry is uint256 _commitmentId ) external onlyOwnerOrApproved(commitments[_commitmentId].grantor, commitments[_commitmentId].tokenAddress) { uint256 numberOfRoles = commitmentIdToRoles[_commitmentId].length(); - for (uint256 i = 0; i < numberOfRoles; i++) { - bytes32 role = commitmentIdToRoles[_commitmentId].at(i); + for (uint256 i = numberOfRoles; i > 0; i--) { + bytes32 role = commitmentIdToRoles[_commitmentId].at(i - 1); address grantee = lastGrantee[_commitmentId][role]; uint256 itemId = _getItemId(_commitmentId, role, grantee); diff --git a/test/SftRolesRegistry/SftRolesRegistry.spec.ts b/test/SftRolesRegistry/SftRolesRegistry.spec.ts index 2ae2e33..b5759b3 100644 --- a/test/SftRolesRegistry/SftRolesRegistry.spec.ts +++ b/test/SftRolesRegistry/SftRolesRegistry.spec.ts @@ -358,18 +358,23 @@ describe('SftRolesRegistry', async () => { ) }) - it('should release when there are revocable roles', async () => { + it('should release when there are two revocable roles', async () => { + + await assertGrantRoleEvent( + SftRolesRegistry, grantor, 1, grantee.address, true, grantor, 'ANOTHER_ROLE' + ) + await expect(SftRolesRegistry.connect(grantor).releaseTokens(GrantRoleData.commitmentId)) .to.emit(SftRolesRegistry, 'TokensReleased') - .withArgs(GrantRoleData.commitmentId) + .withArgs(GrantRoleData.commitmentId) .to.emit(MockToken, 'TransferSingle') - .withArgs( - SftRolesRegistry.address, - SftRolesRegistry.address, - grantor.address, - CommitmentCreated.tokenId, - CommitmentCreated.tokenAmount, - ) + .withArgs( + SftRolesRegistry.address, + SftRolesRegistry.address, + grantor.address, + CommitmentCreated.tokenId, + CommitmentCreated.tokenAmount, + ) }) it('should release when there are no roles', async () => { diff --git a/test/SftRolesRegistry/helpers/assertEvents.ts b/test/SftRolesRegistry/helpers/assertEvents.ts index 0793071..c72b98b 100644 --- a/test/SftRolesRegistry/helpers/assertEvents.ts +++ b/test/SftRolesRegistry/helpers/assertEvents.ts @@ -55,11 +55,13 @@ export async function assertGrantRoleEvent( grantee: string, revocable = true, anotherUser?: SignerWithAddress, + role?: string, ) { const grantRoleData = await buildGrantRole({ commitmentId, grantee, revocable, + role }) if (anotherUser) { const tokenAddress = await SftRolesRegistry.tokenAddressOf(commitmentId) From c6569d8f5360aa79b90d509f4a5f60e7c5a5b73a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ernani=20S=C3=A3o=20Thiago?= Date: Mon, 8 Jan 2024 16:52:59 -0500 Subject: [PATCH 18/19] fixed lint --- .../SftRolesRegistry/SftRolesRegistry.spec.ts | 21 ++++++++----------- test/SftRolesRegistry/helpers/assertEvents.ts | 2 +- 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/test/SftRolesRegistry/SftRolesRegistry.spec.ts b/test/SftRolesRegistry/SftRolesRegistry.spec.ts index b5759b3..1d5215a 100644 --- a/test/SftRolesRegistry/SftRolesRegistry.spec.ts +++ b/test/SftRolesRegistry/SftRolesRegistry.spec.ts @@ -359,22 +359,19 @@ describe('SftRolesRegistry', async () => { }) it('should release when there are two revocable roles', async () => { - - await assertGrantRoleEvent( - SftRolesRegistry, grantor, 1, grantee.address, true, grantor, 'ANOTHER_ROLE' - ) + await assertGrantRoleEvent(SftRolesRegistry, grantor, 1, grantee.address, true, grantor, 'ANOTHER_ROLE') await expect(SftRolesRegistry.connect(grantor).releaseTokens(GrantRoleData.commitmentId)) .to.emit(SftRolesRegistry, 'TokensReleased') - .withArgs(GrantRoleData.commitmentId) + .withArgs(GrantRoleData.commitmentId) .to.emit(MockToken, 'TransferSingle') - .withArgs( - SftRolesRegistry.address, - SftRolesRegistry.address, - grantor.address, - CommitmentCreated.tokenId, - CommitmentCreated.tokenAmount, - ) + .withArgs( + SftRolesRegistry.address, + SftRolesRegistry.address, + grantor.address, + CommitmentCreated.tokenId, + CommitmentCreated.tokenAmount, + ) }) it('should release when there are no roles', async () => { diff --git a/test/SftRolesRegistry/helpers/assertEvents.ts b/test/SftRolesRegistry/helpers/assertEvents.ts index c72b98b..8d877ad 100644 --- a/test/SftRolesRegistry/helpers/assertEvents.ts +++ b/test/SftRolesRegistry/helpers/assertEvents.ts @@ -61,7 +61,7 @@ export async function assertGrantRoleEvent( commitmentId, grantee, revocable, - role + role, }) if (anotherUser) { const tokenAddress = await SftRolesRegistry.tokenAddressOf(commitmentId) From 59e8bc25066567cf41fe5cfb97d822098c8307a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ernani=20S=C3=A3o=20Thiago?= Date: Mon, 8 Jan 2024 16:55:15 -0500 Subject: [PATCH 19/19] improved for loop --- contracts/RolesRegistry/SftRolesRegistry.sol | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/contracts/RolesRegistry/SftRolesRegistry.sol b/contracts/RolesRegistry/SftRolesRegistry.sol index 3c4bc25..a5a6e21 100644 --- a/contracts/RolesRegistry/SftRolesRegistry.sol +++ b/contracts/RolesRegistry/SftRolesRegistry.sol @@ -103,8 +103,9 @@ contract SftRolesRegistry is uint256 _commitmentId ) external onlyOwnerOrApproved(commitments[_commitmentId].grantor, commitments[_commitmentId].tokenAddress) { uint256 numberOfRoles = commitmentIdToRoles[_commitmentId].length(); - for (uint256 i = numberOfRoles; i > 0; i--) { - bytes32 role = commitmentIdToRoles[_commitmentId].at(i - 1); + for (uint256 i = numberOfRoles; i > 0; ) { + i--; + bytes32 role = commitmentIdToRoles[_commitmentId].at(i); address grantee = lastGrantee[_commitmentId][role]; uint256 itemId = _getItemId(_commitmentId, role, grantee);