Fix: Avoid Hashed Labels in Registries

.vscode/settings.json

@@ -1,5 +1,5 @@
 {
   "solidity.packageDefaultDependenciesContractsDirectory": "./contracts/src",
   "solidity.packageDefaultDependenciesDirectory": "./contracts/lib",
-  "solidity.compileUsingRemoteVersion": "v0.8.24+commit.e11b9ed9",
+  "solidity.compileUsingRemoteVersion": "v0.8.25+commit.b61c2a91",
 }

contracts/src/L2/ETHRegistrar.sol

@@ -7,26 +7,43 @@ import {IERC1155Singleton} from "../common/IERC1155Singleton.sol";
 import {IPermissionedRegistry} from "../common/IPermissionedRegistry.sol";
 import {IPriceOracle} from "./IPriceOracle.sol";
 import {ERC165} from "@openzeppelin/contracts/utils/introspection/ERC165.sol";
-import {NameUtils} from "../common/NameUtils.sol";
 import {EnhancedAccessControl} from "../common/EnhancedAccessControl.sol";
 import {RegistryRolesMixin} from "../common/RegistryRolesMixin.sol";
 
-contract ETHRegistrar is IETHRegistrar, EnhancedAccessControl, RegistryRolesMixin {
-    uint256 private constant REGISTRATION_ROLE_BITMAP = ROLE_SET_SUBREGISTRY | ROLE_SET_SUBREGISTRY_ADMIN | ROLE_SET_RESOLVER | ROLE_SET_RESOLVER_ADMIN;
+contract ETHRegistrar is
+    IETHRegistrar,
+    EnhancedAccessControl,
+    RegistryRolesMixin
+{
+    uint256 private constant REGISTRATION_ROLE_BITMAP =
+        ROLE_SET_SUBREGISTRY |
+            ROLE_SET_SUBREGISTRY_ADMIN |
+            ROLE_SET_RESOLVER |
+            ROLE_SET_RESOLVER_ADMIN;
 
     uint256 private constant ROLE_SET_PRICE_ORACLE = 1 << 20;
-    uint256 private constant ROLE_SET_PRICE_ORACLE_ADMIN = ROLE_SET_PRICE_ORACLE << 128;
-
+    uint256 private constant ROLE_SET_PRICE_ORACLE_ADMIN =
+        ROLE_SET_PRICE_ORACLE << 128;
+
     uint256 private constant ROLE_SET_COMMITMENT_AGES = 1 << 24;
-    uint256 private constant ROLE_SET_COMMITMENT_AGES_ADMIN = ROLE_SET_COMMITMENT_AGES << 128;
+    uint256 private constant ROLE_SET_COMMITMENT_AGES_ADMIN =
+        ROLE_SET_COMMITMENT_AGES << 128;
 
     uint256 public constant MIN_REGISTRATION_DURATION = 28 days;
 
     error MaxCommitmentAgeTooLow();
     error UnexpiredCommitmentExists(bytes32 commitment);
     error DurationTooShort(uint64 duration, uint256 minDuration);
-    error CommitmentTooNew(bytes32 commitment, uint256 validFrom, uint256 blockTimestamp);
-    error CommitmentTooOld(bytes32 commitment, uint256 validTo, uint256 blockTimestamp);
+    error CommitmentTooNew(
+        bytes32 commitment,
+        uint256 validFrom,
+        uint256 blockTimestamp
+    );
+    error CommitmentTooOld(
+        bytes32 commitment,
+        uint256 validTo,
+        uint256 blockTimestamp
+    );
     error NameNotAvailable(string name);
     error InsufficientValue(uint256 required, uint256 provided);
 
@@ -35,9 +52,14 @@ contract ETHRegistrar is IETHRegistrar, EnhancedAccessControl, RegistryRolesMixi
     uint256 public minCommitmentAge;
     uint256 public maxCommitmentAge;
 
-    mapping(bytes32 => uint256) public commitments;    
+    mapping(bytes32 => uint256) public commitments;
 
-    constructor(address _registry, IPriceOracle _prices, uint256 _minCommitmentAge, uint256 _maxCommitmentAge) {
+    constructor(
+        address _registry,
+        IPriceOracle _prices,
+        uint256 _minCommitmentAge,
+        uint256 _maxCommitmentAge
+    ) {
         _grantRoles(ROOT_RESOURCE, ALL_ROLES, _msgSender(), true);
 
         registry = IPermissionedRegistry(_registry);
@@ -53,39 +75,40 @@ contract ETHRegistrar is IETHRegistrar, EnhancedAccessControl, RegistryRolesMixi
 
     /**
      * @dev Check if a name is valid.
-     * @param name The name to check.
+     * @param label The name to check.
      * @return True if the name is valid, false otherwise.
      */
-    function valid(string memory name) public pure returns (bool) {
-        return bytes(name).length >= 3;
+    function valid(string memory label) public pure returns (bool) {
+        return bytes(label).length >= 3 && bytes(label).length <= 255;
     }
 
     /**
      * @dev Check if a name is available.
-     * @param name The name to check.
+     * @param label The name to check.
      * @return True if the name is available, false otherwise.
      */
-    function available(string calldata name) external view returns (bool) {
-        (, uint64 expiry, ) = registry.getNameData(name);
+    function available(string calldata label) external view returns (bool) {
+        (, uint64 expiry, ) = registry.getNameData(label);
         return expiry < block.timestamp;
     }
 
-
     /**
      * @dev Get the price to register or renew a name.
-     * @param name The name to get the price for.
+     * @param label The name to get the price for.
      * @param duration The duration of the registration or renewal.
      * @return price The price to register or renew the name.
-     */ 
-    function rentPrice(string memory name, uint256 duration) public view override returns (IPriceOracle.Price memory price) {
-        (, uint64 expiry, ) = registry.getNameData(name);
-        price = prices.price(name, uint256(expiry), duration);
-    }    
-
+     */
+    function rentPrice(
+        string memory label,
+        uint256 duration
+    ) public view override returns (IPriceOracle.Price memory price) {
+        (, uint64 expiry, ) = registry.getNameData(label);
+        price = prices.price(label, uint256(expiry), duration);
+    }
 
     /**
      * @dev Make a commitment for a name.
-     * @param name The name to commit.
+     * @param label The name to commit.
      * @param owner The address of the owner of the name.
      * @param secret The secret of the name.
      * @param subregistry The registry to use for the commitment.
@@ -94,17 +117,17 @@ contract ETHRegistrar is IETHRegistrar, EnhancedAccessControl, RegistryRolesMixi
      * @return The commitment.
      */
     function makeCommitment(
-        string memory name,
+        string memory label,
         address owner,
         bytes32 secret,
         address subregistry,
         address resolver,
         uint64 duration
-    ) public pure override returns (bytes32) {        
+    ) public pure override returns (bytes32) {
         return
             keccak256(
                 abi.encode(
-                    name,
+                    label,
                     owner,
                     secret,
                     subregistry,
@@ -114,7 +137,6 @@ contract ETHRegistrar is IETHRegistrar, EnhancedAccessControl, RegistryRolesMixi
             );
     }
 
-
     /**
      * @dev Commit a commitment.
      * @param commitment The commitment to commit.
@@ -128,10 +150,9 @@ contract ETHRegistrar is IETHRegistrar, EnhancedAccessControl, RegistryRolesMixi
         emit CommitmentMade(commitment);
     }
 
-
     /**
      * @dev Register a name.
-     * @param name The name to register.
+     * @param label The name to register.
      * @param owner The owner of the name.
      * @param secret The secret of the name.
      * @param subregistry The subregistry to register the name in.
@@ -140,36 +161,61 @@ contract ETHRegistrar is IETHRegistrar, EnhancedAccessControl, RegistryRolesMixi
      * @return tokenId The token ID of the registered name.
      */
     function register(
-        string calldata name,
+        string calldata label,
         address owner,
         bytes32 secret,
         IRegistry subregistry,
         address resolver,
         uint64 duration
     ) external payable returns (uint256 tokenId) {
-        uint256 totalPrice = checkPrice(name, duration);
-
-        _consumeCommitment(name, duration, makeCommitment(name, owner, secret, address(subregistry), resolver, duration));
+        uint256 totalPrice = checkPrice(label, duration);
+
+        _consumeCommitment(
+            label,
+            duration,
+            makeCommitment(
+                label,
+                owner,
+                secret,
+                address(subregistry),
+                resolver,
+                duration
+            )
+        );
 
         uint64 expiry = uint64(block.timestamp) + duration;
-        tokenId = registry.register(name, owner, subregistry, resolver, REGISTRATION_ROLE_BITMAP, expiry);
+        tokenId = registry.register(
+            label,
+            owner,
+            subregistry,
+            resolver,
+            REGISTRATION_ROLE_BITMAP,
+            expiry
+        );
 
         if (msg.value > totalPrice) {
             payable(msg.sender).transfer(msg.value - totalPrice);
         }
 
-        emit NameRegistered(name, owner, subregistry, resolver, duration, tokenId);
+        emit NameRegistered(
+            label,
+            owner,
+            subregistry,
+            resolver,
+            duration,
+            tokenId
+        );
     }
 
     /**
      * @dev Renew a name.
-     * @param name The name to renew.
+     * @param label The name to renew.
      * @param duration The duration of the renewal.
      */
-    function renew(string calldata name, uint64 duration) external payable {
-        uint256 totalPrice = checkPrice(name, duration);
+    function renew(string calldata label, uint64 duration) external payable {
+        uint256 totalPrice = checkPrice(label, duration);
 
-        (uint256 tokenId, uint64 expiry, ) = registry.getNameData(name);
+        (uint256 tokenId, uint64 expiry, ) = registry.getNameData(label);
 
         registry.renew(tokenId, expiry + duration);
 
@@ -179,19 +225,28 @@ contract ETHRegistrar is IETHRegistrar, EnhancedAccessControl, RegistryRolesMixi
 
         uint64 newExpiry = registry.getExpiry(tokenId);
 
-        emit NameRenewed(name, duration, tokenId, newExpiry);
+        emit NameRenewed(label, duration, tokenId, newExpiry);
     }
 
-
-    function supportsInterface(bytes4 interfaceID) public view override(EnhancedAccessControl) returns (bool) {
-        return interfaceID == type(IETHRegistrar).interfaceId || super.supportsInterface(interfaceID);
+    /// @inheritdoc ERC165
+    function supportsInterface(
+        bytes4 interfaceId
+    ) public view override(EnhancedAccessControl) returns (bool) {
+        return
+            interfaceId == type(IETHRegistrar).interfaceId ||
+            super.supportsInterface(interfaceId);
     }
 
-    function setPriceOracle(IPriceOracle _prices) external onlyRoles(ROOT_RESOURCE, ROLE_SET_PRICE_ORACLE) {
+    function setPriceOracle(
+        IPriceOracle _prices
+    ) external onlyRoles(ROOT_RESOURCE, ROLE_SET_PRICE_ORACLE) {
         prices = _prices;
     }
 
-    function setCommitmentAges(uint256 _minCommitmentAge, uint256 _maxCommitmentAge) external onlyRoles(ROOT_RESOURCE, ROLE_SET_COMMITMENT_AGES) {
+    function setCommitmentAges(
+        uint256 _minCommitmentAge,
+        uint256 _maxCommitmentAge
+    ) external onlyRoles(ROOT_RESOURCE, ROLE_SET_COMMITMENT_AGES) {
         if (_maxCommitmentAge <= _minCommitmentAge) {
             revert MaxCommitmentAgeTooLow();
         }
@@ -202,25 +257,35 @@ contract ETHRegistrar is IETHRegistrar, EnhancedAccessControl, RegistryRolesMixi
     /* Internal functions */
 
     function _consumeCommitment(
-        string memory name,
+        string memory label,
         uint64 duration,
         bytes32 commitment
     ) internal {
         // Require an old enough commitment.
-        uint256 thisCommitmentValidFrom = commitments[commitment] + minCommitmentAge;
+        uint256 thisCommitmentValidFrom = commitments[commitment] +
+            minCommitmentAge;
         if (thisCommitmentValidFrom > block.timestamp) {
-            revert CommitmentTooNew(commitment, thisCommitmentValidFrom, block.timestamp);
+            revert CommitmentTooNew(
+                commitment,
+                thisCommitmentValidFrom,
+                block.timestamp
+            );
         }
 
         // Commit must not be too old
-        uint256 thisCommitmentValidTo = commitments[commitment] + maxCommitmentAge;
+        uint256 thisCommitmentValidTo = commitments[commitment] +
+            maxCommitmentAge;
         if (thisCommitmentValidTo <= block.timestamp) {
-            revert CommitmentTooOld(commitment, thisCommitmentValidTo, block.timestamp);
+            revert CommitmentTooOld(
+                commitment,
+                thisCommitmentValidTo,
+                block.timestamp
+            );
         }
 
         // Name must be available
-        if (!this.available(name)) {
-            revert NameNotAvailable(name);
+        if (!this.available(label)) {
+            revert NameNotAvailable(label);
         }
 
         if (duration < MIN_REGISTRATION_DURATION) {
@@ -232,16 +297,18 @@ contract ETHRegistrar is IETHRegistrar, EnhancedAccessControl, RegistryRolesMixi
 
     /**
      * @dev Check the price of a name and revert if insufficient value is provided.
-     * @param name The name to check the price for.
+     * @param label The name to check the price for.
      * @param duration The duration of the registration.
      * @return totalPrice The total price of the registration.
      */
-    function checkPrice(string memory name, uint64 duration) private view returns (uint256 totalPrice) {
-        IPriceOracle.Price memory price = rentPrice(name, duration);
+    function checkPrice(
+        string memory label,
+        uint64 duration
+    ) private view returns (uint256 totalPrice) {
+        IPriceOracle.Price memory price = rentPrice(label, duration);
         totalPrice = price.base + price.premium;
         if (msg.value < totalPrice) {
             revert InsufficientValue(totalPrice, msg.value);
         }
     }
-
-}
+}

contracts/src/common/IStandardRegistry.sol

@@ -8,6 +8,12 @@ import {IRegistry} from "./IRegistry.sol";
  * @dev Interface for the a standard registry.
  */
 interface IStandardRegistry is IRegistry {
+
+    /**
+     * @dev Error emitted when a name is too long.
+     */
+    error NameTooLong(string label);
+
     /**
      * @dev Error emitted when a name is already registered.
      */