fix: [audit] ZNS-8 Mintlist (#60)

zer0-os · Oct 31, 2023 · 8f8bdff · 8f8bdff
2 parents 90c745b + a07d0f0
commit 8f8bdff
Show file tree

Hide file tree

Showing 5 changed files with 85 additions and 22 deletions.
diff --git a/contracts/registrar/IZNSSubRegistrar.sol b/contracts/registrar/IZNSSubRegistrar.sol
@@ -46,6 +46,11 @@ interface IZNSSubRegistrar is IDistributionConfig {
         bool[] allowed
     );
 
+    /*
+     * @notice Emitted when a `mintlist` is removed for a domain by the owner or through `ZNSRootRegistrar.revokeDomain()`.
+     */
+    event MintlistCleared(bytes32 indexed domainHash);
+
     /**
      * @notice Emitted when the ZNSRootRegistrar address is set in state.
     */
@@ -59,7 +64,7 @@ interface IZNSSubRegistrar is IDistributionConfig {
         AccessType accessType
     );
 
-    function mintlist(
+    function isMintlistedForDomain(
         bytes32 domainHash,
         address candidate
     ) external view returns (bool);
@@ -109,6 +114,10 @@ interface IZNSSubRegistrar is IDistributionConfig {
         bool[] calldata allowed
     ) external;
 
+    function clearMintlistForDomain(bytes32 domainHash) external;
+
+    function clearMintlistAndLock(bytes32 domainHash) external;
+
     function setRegistry(address registry_) external;
 
     function setRootRegistrar(address registrar_) external;

diff --git a/contracts/registrar/ZNSRootRegistrar.sol b/contracts/registrar/ZNSRootRegistrar.sol
@@ -250,7 +250,7 @@ contract ZNSRootRegistrar is
             "ZNSRootRegistrar: Not the owner of both Name and Token"
         );
 
-        subRegistrar.setAccessTypeForDomain(domainHash, AccessType.LOCKED);
+        subRegistrar.clearMintlistAndLock(domainHash);
         _coreRevoke(domainHash, msg.sender);
     }
 

diff --git a/contracts/registrar/ZNSSubRegistrar.sol b/contracts/registrar/ZNSSubRegistrar.sol
@@ -29,12 +29,17 @@ contract ZNSSubRegistrar is AAccessControlled, ARegistryWired, UUPSUpgradeable,
     */
     mapping(bytes32 domainHash => DistributionConfig config) public override distrConfigs;
 
+    struct Mintlist {
+        mapping(uint256 idx => mapping(address candidate => bool allowed)) list;
+        uint256 ownerIndex;
+    }
+
     /**
      * @notice Mapping of domainHash to mintlist set by the domain owner/operator.
      * These configs are used to determine who can register subdomains for every parent
      * in the case where parent's DistributionConfig.AccessType is set to AccessType.MINTLIST.
     */
-    mapping(bytes32 domainHash => mapping(address candidate => bool allowed)) public override mintlist;
+    mapping(bytes32 domainHash => Mintlist mintStruct) public mintlist;
 
     modifier onlyOwnerOperatorOrRegistrar(bytes32 domainHash) {
         require(
@@ -95,7 +100,10 @@ contract ZNSSubRegistrar is AAccessControlled, ARegistryWired, UUPSUpgradeable,
 
         if (parentConfig.accessType == AccessType.MINTLIST) {
             require(
-                mintlist[parentHash][msg.sender],
+                mintlist[parentHash]
+                    .list
+                    [mintlist[parentHash].ownerIndex]
+                    [msg.sender],
                 "ZNSSubRegistrar: Sender is not approved for purchase"
             );
         }
@@ -245,8 +253,9 @@ contract ZNSSubRegistrar is AAccessControlled, ARegistryWired, UUPSUpgradeable,
     function setAccessTypeForDomain(
         bytes32 domainHash,
         AccessType accessType
-    ) external override onlyOwnerOperatorOrRegistrar(domainHash) {
-        _setAccessTypeForDomain(domainHash, accessType);
+    ) public override onlyOwnerOperatorOrRegistrar(domainHash) {
+        distrConfigs[domainHash].accessType = accessType;
+        emit AccessTypeSet(domainHash, accessType);
     }
 
     /**
@@ -269,13 +278,48 @@ contract ZNSSubRegistrar is AAccessControlled, ARegistryWired, UUPSUpgradeable,
             "ZNSSubRegistrar: Not authorized"
         );
 
+        Mintlist storage mintlistForDomain = mintlist[domainHash];
+        uint256 ownerIndex = mintlistForDomain.ownerIndex;
+
         for (uint256 i; i < candidates.length; i++) {
-            mintlist[domainHash][candidates[i]] = allowed[i];
+            mintlistForDomain.list[ownerIndex][candidates[i]] = allowed[i];
         }
 
         emit MintlistUpdated(domainHash, candidates, allowed);
     }
 
+    function isMintlistedForDomain(
+        bytes32 domainHash,
+        address candidate
+    ) external view override returns (bool) {
+        uint256 ownerIndex = mintlist[domainHash].ownerIndex;
+        return mintlist[domainHash].list[ownerIndex][candidate];
+    }
+
+    /*
+     * @notice Function to completely clear/remove the whole mintlist set for a given domain.
+     * Can only be called by the owner/operator of the domain or by `ZNSRootRegistrar` as a part of the
+     * `revokeDomain()` flow.
+     * Emits `MintlistCleared` event.
+     * @param domainHash The domain hash to clear the mintlist for
+     */
+    function clearMintlistForDomain(bytes32 domainHash)
+    public
+    override
+    onlyOwnerOperatorOrRegistrar(domainHash) {
+        mintlist[domainHash].ownerIndex = mintlist[domainHash].ownerIndex + 1;
+
+        emit MintlistCleared(domainHash);
+    }
+
+    function clearMintlistAndLock(bytes32 domainHash)
+    external
+    override
+    onlyOwnerOperatorOrRegistrar(domainHash) {
+        setAccessTypeForDomain(domainHash, AccessType.LOCKED);
+        clearMintlistForDomain(domainHash);
+    }
+
     /**
      * @notice Sets the registry address in state.
      * @dev This function is required for all contracts inheriting `ARegistryWired`.
@@ -296,18 +340,6 @@ contract ZNSSubRegistrar is AAccessControlled, ARegistryWired, UUPSUpgradeable,
         emit RootRegistrarSet(registrar_);
     }
 
-    /**
-     * @dev Internal function used by this contract to set the access type for a subdomain
-     * during revocation process.
-    */
-    function _setAccessTypeForDomain(
-        bytes32 domainHash,
-        AccessType accessType
-    ) internal {
-        distrConfigs[domainHash].accessType = accessType;
-        emit AccessTypeSet(domainHash, accessType);
-    }
-
     /**
      * @notice To use UUPS proxy we override this function and revert if `msg.sender` isn't authorized
      * @param newImplementation The implementation contract to upgrade to

diff --git a/test/ZNSRootRegistrar.test.ts b/test/ZNSRootRegistrar.test.ts
@@ -764,7 +764,7 @@ describe("ZNSRootRegistrar", () => {
   });
 
   describe("Revoking Domains", () => {
-    it("Revokes a Top level Domain - Happy Path", async () => {
+    it("Revokes a Top level Domain, locks distribution and removes mintlist", async () => {
       // Register Top level
       const topLevelTx = await defaultRootRegistration({
         user,
@@ -779,6 +779,13 @@ describe("ZNSRootRegistrar", () => {
 
       const domainHash = await getDomainHashFromReceipt(topLevelTx);
 
+      // add mintlist to check revocation
+      await zns.subRegistrar.connect(user).updateMintlistForDomain(
+        domainHash,
+        [user.address, zeroVault.address],
+        [true, true]
+      );
+
       const ogPrice = BigNumber.from(135);
       await zns.fixedPricer.connect(user).setPriceConfig(
         domainHash,
@@ -808,6 +815,10 @@ describe("ZNSRootRegistrar", () => {
       // validate access type has been set to LOCKED
       const { accessType } = await zns.subRegistrar.distrConfigs(domainHash);
       expect(accessType).to.eq(AccessType.LOCKED);
+
+      // validate mintlist has been removed
+      expect(await zns.subRegistrar.isMintlistedForDomain(domainHash, user.address)).to.be.false;
+      expect(await zns.subRegistrar.isMintlistedForDomain(domainHash, zeroVault.address)).to.be.false;
     });
 
     it("Cannot revoke a domain that doesnt exist", async () => {

diff --git a/test/ZNSSubRegistrar.test.ts b/test/ZNSSubRegistrar.test.ts
@@ -771,9 +771,16 @@ describe("ZNSSubRegistrar", () => {
       );
     });
 
-    it("should revoke lvl 6 domain without refund and lock registration", async () => {
+    it("should revoke lvl 6 domain without refund, lock registration and remove mintlist", async () => {
       const domainHash = regResults[5].domainHash;
 
+      // add to mintlist
+      await zns.subRegistrar.connect(lvl6SubOwner).updateMintlistForDomain(
+        domainHash,
+        [lvl6SubOwner.address, lvl2SubOwner.address],
+        [true, true]
+      );
+
       const userBalBefore = await zns.zeroToken.balanceOf(lvl6SubOwner.address);
 
       await zns.rootRegistrar.connect(lvl6SubOwner).revokeDomain(
@@ -789,6 +796,10 @@ describe("ZNSSubRegistrar", () => {
       const { accessType: accessTypeFromSC } = await zns.subRegistrar.distrConfigs(domainHash);
       expect(accessTypeFromSC).to.eq(AccessType.LOCKED);
 
+      // make sure that mintlist has been removed
+      expect(await zns.subRegistrar.isMintlistedForDomain(domainHash, lvl6SubOwner.address)).to.eq(false);
+      expect(await zns.subRegistrar.isMintlistedForDomain(domainHash, lvl2SubOwner.address)).to.eq(false);
+
       await expect(
         zns.subRegistrar.connect(lvl6SubOwner).registerSubdomain(
           domainHash,
@@ -2529,7 +2540,7 @@ describe("ZNSSubRegistrar", () => {
         [true],
       );
 
-      const mintlisted = await zns.subRegistrar.mintlist(
+      const mintlisted = await zns.subRegistrar.isMintlistedForDomain(
         domainHash,
         lvl5SubOwner.address
       );