remove redundant comments + small fixes

contracts/access/ZNSAccessController.sol

@@ -16,11 +16,11 @@ contract ZNSAccessController is AccessControlUpgradeable, ZNSRoles, IZNSAccessCo
         _grantRoleToMany(GOVERNOR_ROLE, governorAddresses);
         _grantRoleToMany(ADMIN_ROLE, adminAddresses);
 
-        // all of the governors control admins TODO AC: ???
+        // all of the governors control admins
         _setRoleAdmin(ADMIN_ROLE, GOVERNOR_ROLE);
-        // all of the governors control governors TODO AC: ???
+        // all of the governors control governors
         _setRoleAdmin(GOVERNOR_ROLE, GOVERNOR_ROLE);
-        // all of the admins control registrar TODO AC: ???
+        // all of the admins control registrar
         _setRoleAdmin(REGISTRAR_ROLE, ADMIN_ROLE);
     }
 
@@ -50,14 +50,12 @@ contract ZNSAccessController is AccessControlUpgradeable, ZNSRoles, IZNSAccessCo
         return hasRole(REGISTRAR_ROLE, account);
     }
 
-    // TODO AC: is this function necessary? how often will it be used?
     function _grantRoleToMany(bytes32 role, address[] calldata addresses) internal {
         for (uint256 i = 0; i < addresses.length; i++) {
             _grantRole(role, addresses[i]);
         }
     }
 
-    // TODO AC: how safe is this?
     function setRoleAdmin(bytes32 role, bytes32 adminRole) external override onlyRole(GOVERNOR_ROLE) {
         _setRoleAdmin(role, adminRole);
     }

contracts/access/ZNSRoles.sol

@@ -2,18 +2,16 @@
 pragma solidity ^0.8.18;
 
 
+// TODO: can we declare these outside of contract, just in the ZNSAccessController file?
 abstract contract ZNSRoles {
-    // TODO AC: test getting this from AC contract vs inheriting these roles in every other contract
-    // the highest rank, only assigns Admins
+    // the highest rank, assigns Admins, new roles and Role Admins
     bytes32 public constant GOVERNOR_ROLE = keccak256("GOVERNOR_ROLE");
     // the main maintainer role, that gets access to all system functions
-    // TODO AC: should we split responsibilities in a better way?
     bytes32 public constant ADMIN_ROLE = keccak256("ADMIN_ROLE");
     // operator can be here to future proof, if we need a new role
     // so we don't have to upgrade all contracts
-    // TODO AC: decide what to do with this role
+    // TODO AC: possibly remove executor role?
     bytes32 public constant EXECUTOR_ROLE = keccak256("EXECUTOR_ROLE");
     // this role is here specifically for the ZNSEthRegistrar contract
     bytes32 public constant REGISTRAR_ROLE = keccak256("REGISTRAR_ROLE");
-    // TODO AC: what other roles do we need here?
 }

contracts/distribution/ZNSEthRegistrar.sol

@@ -45,8 +45,6 @@ contract ZNSEthRegistrar is AccessControlled, IZNSEthRegistrar {
         address znsAddressResolver_
     ) {
         _setAccessController(accessController_);
-        // TODO AC: should we call protected functions in the constructor/initialize?
-        //  test this!
         setRegistry(znsRegistry_);
         setTreasury(znsTreasury_);
         setDomainToken(znsDomainToken_);

contracts/distribution/ZNSTreasury.sol

@@ -95,7 +95,6 @@ contract ZNSTreasury is AccessControlled, IZNSTreasury {
         emit ZeroVaultAddressSet(zeroVaultAddress);
     }
 
-    // TODO AC: should we call a protected function in the constructor/initialize?
     function setPriceOracle(address znsPriceOracle_) public override onlyAdmin {
         require(
             znsPriceOracle_ != address(0),

test/ZNSEthRegistrar.test.ts

@@ -59,8 +59,7 @@ describe("ZNSEthRegistrar", () => {
     expect(allowance).to.eq(ethers.constants.MaxUint256);
   });
 
-  // TODO AC: is this a problem it is set up this way?
-  it("Should initialize correctly from an ADMIN account only", async () => {
+  it("Should revert when initialize() without ADMIN_ROLE", async () => {
     const userHasAdmin = await zns.accessController.hasRole(ADMIN_ROLE, user.address);
     expect(userHasAdmin).to.be.false;
 

test/helpers/deployZNS.ts

@@ -195,11 +195,6 @@ export const deployZNS = async ({
     registrar,
   };
 
-  // Final configuration steps
-  // TODO AC: remove all redundant calls here! and delete hashing of the root and the need
-  //  for Registrar to be owner/operator of the root
-  await registry.connect(deployer).setOwnerOperator(registrar.address, true);
-
   // Give 15 ZERO to the deployer and allowance to the treasury
   await zeroTokenMock.connect(deployer).approve(treasury.address, ethers.constants.MaxUint256);
   await zeroTokenMock.transfer(deployer.address, ethers.utils.parseEther("15"));