refactor: eth registry now works with new roles arch

contracts/src/registry/ETHRegistry.sol

@@ -130,10 +130,8 @@ contract ETHRegistry is PermissionedRegistry {
         newTokenId = (tokenId & ~uint256(FLAGS_MASK)) | (newFlags & FLAGS_MASK);
         if (tokenId != newTokenId) {
             address owner = ownerOf(tokenId);
-            _burn(owner, tokenId, 1);
             _mint(owner, newTokenId, 1, "");
-            _copyRoles(tokenIdResource(tokenId), owner, tokenIdResource(newTokenId), owner);
-            _revokeAllRoles(tokenIdResource(tokenId), owner);
+            _burn(owner, tokenId, 1);
         }
     }
 

contracts/src/registry/EnhancedAccessControl.sol

@@ -206,7 +206,7 @@ abstract contract EnhancedAccessControl is Context, ERC165 {
      */
     function _copyRoles(bytes32 srcResource, address srcAccount, bytes32 dstResource, address dstAccount) internal virtual {
         uint256 srcRoles = _roles[srcResource][srcAccount];
-        _roles[dstResource][dstAccount] = srcRoles;
+        _roles[dstResource][dstAccount] |= srcRoles;
         emit EnhancedAccessControlRolesCopied(srcResource, dstResource, srcAccount, dstAccount, srcRoles);
     }
 
@@ -261,8 +261,7 @@ abstract contract EnhancedAccessControl is Context, ERC165 {
     /**
      * @dev Revoke all roles for account within resource.
      */
-    function _revokeAllRoles(bytes32 resource, address account) internal virtual {
-        _roles[resource][account] = 0;
-        emit EnhancedAccessControlAllRolesRevoked(resource, account, _msgSender());
+    function _revokeAllRoles(bytes32 resource, address account) internal virtual returns (bool) {
+        return _revokeRoles(resource, _roles[resource][account], account);
     }
 }

contracts/test/ETHRegistry.t.sol

@@ -87,11 +87,10 @@ contract TestETHRegistry is Test, ERC1155Holder, Roles {
     }
 
     function test_lock_name() public {
-        uint96 flags = registry.FLAG_FLAGS_LOCKED();
         uint256 oldTokenId = registry.register("test2", address(this), registry, 0, defaultRoleBitmap, uint64(block.timestamp) + 86400);
 
+        uint96 flags = registry.FLAG_FLAGS_LOCKED();
         uint256 expectedTokenId = (oldTokenId & ~uint256(registry.FLAGS_MASK())) | (flags & registry.FLAGS_MASK());
-
         uint256 newTokenId = registry.setFlags(oldTokenId, flags);
         vm.assertNotEq(newTokenId, oldTokenId);
         vm.assertEq(newTokenId, expectedTokenId);
@@ -103,10 +102,10 @@ contract TestETHRegistry is Test, ERC1155Holder, Roles {
 
     function test_cannot_unlock_name() public {
         uint96 flags = registry.FLAG_FLAGS_LOCKED();
-
         uint256 oldTokenId = registry.register("test2", address(this), registry, flags, defaultRoleBitmap, uint64(block.timestamp) + 86400);
-        uint256 newTokenId = registry.setFlags(oldTokenId, 0);
-        vm.assertEq(oldTokenId, newTokenId);
+
+        vm.expectRevert(abi.encodeWithSelector(BaseRegistry.InvalidSubregistryFlags.selector, oldTokenId, flags, 0));
+        registry.setFlags(oldTokenId, 0);
     }
 
     function test_Revert_cannot_mint_duplicates() public {
@@ -265,7 +264,7 @@ contract TestETHRegistry is Test, ERC1155Holder, Roles {
         assertEq(registry.getResolver("test2"), address(0));
     }
 
-    function test_setFlags_copies_roles_to_new_token_id() public {
+    function test_setFlags_moves_roles_to_new_token_id_context() public {
         // Register with default roles
         uint256 tokenId = registry.register("test2", user1, registry, 0, defaultRoleBitmap, uint64(block.timestamp) + 86400);
 
@@ -278,9 +277,11 @@ contract TestETHRegistry is Test, ERC1155Holder, Roles {
 
         // Set a flag that changes the token ID
         uint96 flags = 0x4; // Some arbitrary flag that's not FLAG_FLAGS_LOCKED
+        vm.prank(user1);
         uint256 newTokenId = registry.setFlags(tokenId, flags);
+        assertNotEq(newTokenId, tokenId);
 
-        // Verify roles are preserved after flag change
+        // Verify roles are copied after flag change
         assertEq(registry.hasRoles(registry.tokenIdResource(newTokenId), ROLE_SET_SUBREGISTRY, user1), true);
         assertEq(registry.hasRoles(registry.tokenIdResource(newTokenId), ROLE_SET_RESOLVER, user1), true);
         assertEq(registry.hasRoles(registry.tokenIdResource(newTokenId), 109, user1), true);

contracts/test/EnhancedAccessControl.t.sol

@@ -9,24 +9,24 @@ contract MockEnhancedAccessControl is EnhancedAccessControl {
     constructor() EnhancedAccessControl(msg.sender) {
     }
 
-    function setRoleAdmin(uint256 roleId, uint256 adminRoleId) external {
-        _setRoleAdmin(roleId, adminRoleId);
+    function setRoleAdmin(uint256 roles, uint256 adminRoleId) external {
+        _setRoleAdmin(roles, adminRoleId);
     }
 
-    function callOnlyRootRole(uint256 roleId) external onlyRootRole(roleId) {
+    function callOnlyRootRole(uint256 roles) external onlyRootRole(roles) {
         // Function that will revert if caller doesn't have the role in root resource
     }
 
     function copyRoles(bytes32 srcResource, address srcAccount, bytes32 dstResource, address dstAccount) external {
         _copyRoles(srcResource, srcAccount, dstResource, dstAccount);
     }
 
-    function revokeAllRoles(bytes32 resource, address account) external {
-        _revokeAllRoles(resource, account);
+    function revokeAllRoles(bytes32 resource, address account) external returns (bool) {
+        return _revokeAllRoles(resource, account);
     }
 
-    function grantRoles(bytes32 resource, uint256 roleBitmap, address account) external {
-        _grantRoles(resource, roleBitmap, account);
+    function grantRoles(bytes32 resource, uint256 roleBitmap, address account) external returns (bool) {
+        return _grantRoles(resource, roleBitmap, account);
     }
 }
 
@@ -166,9 +166,9 @@ contract EnhancedAccessControlTest is Test {
         Vm.Log[] memory entries = vm.getRecordedLogs();
         assertEq(entries.length, 1);
         assertEq(entries[0].topics[0], keccak256("EnhancedAccessControlRolesRevoked(bytes32,uint256,address,address)"));
-        (bytes32 resource, uint256 roleId, address account, address sender) = abi.decode(entries[0].data, (bytes32, uint256, address, address));
+        (bytes32 resource, uint256 roles, address account, address sender) = abi.decode(entries[0].data, (bytes32, uint256, address, address));
         assertEq(resource, RESOURCE_1);
-        assertEq(roleId, ROLE_A);
+        assertEq(roles, ROLE_A);
         assertEq(account, user1);
         assertEq(sender, address(this));
     }
@@ -189,8 +189,8 @@ contract EnhancedAccessControlTest is Test {
         Vm.Log[] memory entries = vm.getRecordedLogs();
         assertEq(entries.length, 1);
         assertEq(entries[0].topics[0], keccak256("EnhancedAccessControlRoleAdminChanged(uint256,uint256,uint256)"));
-        (uint256 roleId, uint256 previousAdmin, uint256 newAdmin) = abi.decode(entries[0].data, (uint256, uint256, uint256));
-        assertEq(roleId, ROLE_A);
+        (uint256 roles, uint256 previousAdmin, uint256 newAdmin) = abi.decode(entries[0].data, (uint256, uint256, uint256));
+        assertEq(roles, ROLE_A);
         assertEq(previousAdmin, 0);
         assertEq(newAdmin, ROLE_B);
 
@@ -366,7 +366,10 @@ contract EnhancedAccessControlTest is Test {
         vm.recordLogs();
 
         // Revoke all roles for RESOURCE_1
-        access.revokeAllRoles(RESOURCE_1, user1);
+        bool success = access.revokeAllRoles(RESOURCE_1, user1);
+
+        // Verify the operation was successful
+        assertTrue(success);
 
         // Verify all roles for RESOURCE_1 were revoked
         assertFalse(access.hasRoles(RESOURCE_1, ROLE_A, user1));
@@ -378,10 +381,65 @@ contract EnhancedAccessControlTest is Test {
         // Verify event was emitted correctly
         Vm.Log[] memory entries = vm.getRecordedLogs();
         assertEq(entries.length, 1);
-        assertEq(entries[0].topics[0], keccak256("EnhancedAccessControlAllRolesRevoked(bytes32,address,address)"));
-        (bytes32 resource, address account, address sender) = abi.decode(entries[0].data, (bytes32, address, address));
+        assertEq(entries[0].topics[0], keccak256("EnhancedAccessControlRolesRevoked(bytes32,uint256,address,address)"));
+        (bytes32 resource, uint256 roles, address account, address sender) = abi.decode(entries[0].data, (bytes32, uint256, address, address));
         assertEq(resource, RESOURCE_1);
+        assertEq(roles, ROLE_A | ROLE_B);
         assertEq(account, user1);
         assertEq(sender, address(this));
+
+        // Test revoking all roles when there are no roles to revoke
+        vm.recordLogs();
+        success = access.revokeAllRoles(RESOURCE_1, user1);
+
+        // Verify the operation was not successful (no roles to revoke)
+        assertFalse(success);
+
+        // Verify no event was emitted
+        entries = vm.getRecordedLogs();
+        assertEq(entries.length, 0);
+    }
+
+    function test_copy_roles_bitwise_or() public {
+        // Setup: Grant different roles to user1 and user2
+        access.grantRole(RESOURCE_1, ROLE_A, user1);
+        access.grantRole(RESOURCE_1, ROLE_B, user1);
+        access.grantRole(RESOURCE_1, ROLE_C, user2);
+        access.grantRole(RESOURCE_1, ROLE_D, user2);
+
+        // Verify initial state
+        assertTrue(access.hasRoles(RESOURCE_1, ROLE_A, user1));
+        assertTrue(access.hasRoles(RESOURCE_1, ROLE_B, user1));
+        assertFalse(access.hasRoles(RESOURCE_1, ROLE_C, user1));
+        assertFalse(access.hasRoles(RESOURCE_1, ROLE_D, user1));
+
+        assertTrue(access.hasRoles(RESOURCE_1, ROLE_C, user2));
+        assertTrue(access.hasRoles(RESOURCE_1, ROLE_D, user2));
+        assertFalse(access.hasRoles(RESOURCE_1, ROLE_A, user2));
+        assertFalse(access.hasRoles(RESOURCE_1, ROLE_B, user2));
+
+        // Record logs to verify event emission
+        vm.recordLogs();
+
+        // Copy roles from user1 to user2 for RESOURCE_1
+        // This should OR the roles, not overwrite them
+        access.copyRoles(RESOURCE_1, user1, RESOURCE_1, user2);
+
+        // Verify user2 now has all roles (original + copied)
+        assertTrue(access.hasRoles(RESOURCE_1, ROLE_A, user2));
+        assertTrue(access.hasRoles(RESOURCE_1, ROLE_B, user2));
+        assertTrue(access.hasRoles(RESOURCE_1, ROLE_C, user2));
+        assertTrue(access.hasRoles(RESOURCE_1, ROLE_D, user2));
+
+        // Verify event was emitted correctly with the correct bitmap
+        Vm.Log[] memory entries = vm.getRecordedLogs();
+        assertEq(entries.length, 1);
+        assertEq(entries[0].topics[0], keccak256("EnhancedAccessControlRolesCopied(bytes32,bytes32,address,address,uint256)"));
+        (bytes32 srcResource, bytes32 dstResource, address srcAccount, address dstAccount, uint256 roleBitmap) = abi.decode(entries[0].data, (bytes32, bytes32, address, address, uint256));
+        assertEq(srcResource, RESOURCE_1);
+        assertEq(dstResource, RESOURCE_1);
+        assertEq(srcAccount, user1);
+        assertEq(dstAccount, user2);
+        assertEq(roleBitmap, ROLE_A | ROLE_B);
     }
 } 

contracts/test/fixtures/deployEnsFixture.ts

@@ -20,9 +20,9 @@ export async function deployEnsFixture() {
   ]);
 
   const rootResource = await rootRegistry.read.ROOT_RESOURCE();
-
   const ROLE_TLD_ISSUER = await rootRegistry.read.ROLE_TLD_ISSUER();
   const ROLE_REGISTRAR_ROLE = await rootRegistry.read.ROLE_REGISTRAR_ROLE();
+  const ROLE_BITMAP_TOKEN_OWNER_DEFAULT = await rootRegistry.read.ROLE_BITMAP_TOKEN_OWNER_DEFAULT();
 
   await rootRegistry.write.grantRole([
     rootResource,
@@ -41,6 +41,7 @@ export async function deployEnsFixture() {
     accounts[0].address,
     ethRegistry.address,
     1n,
+    ROLE_BITMAP_TOKEN_OWNER_DEFAULT,
     "https://example.com/"
   ]);
 
@@ -93,8 +94,9 @@ export const registerName = async ({
   subregistryLocked?: boolean;
   resolverLocked?: boolean;
 }) => {
+  const DEFAULT_ROLE_BITMAP = await ethRegistry.read.ROLE_BITMAP_TOKEN_OWNER_DEFAULT();
   const owner =
     owner_ ?? (await hre.viem.getWalletClients())[0].account.address;
   const flags = (subregistryLocked ? 1n : 0n) | (resolverLocked ? 2n : 0n);
-  return ethRegistry.write.register([label, owner, subregistry, flags, expiry]);
+  return ethRegistry.write.register([label, owner, subregistry, flags, DEFAULT_ROLE_BITMAP, expiry]);
 };