refactor view functions

OriumNetwork · Dec 21, 2023 · 74cb61f · 74cb61f
1 parent 2aaff2c
commit 74cb61f
Show file tree

Hide file tree

Showing 3 changed files with 51 additions and 18 deletions.
diff --git 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
@@ -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
@@ -566,13 +566,22 @@ 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,
           GrantRoleData.role,
           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
     })
   })
 })