fix: Restrict registration for owners of parent domains

contracts/registrar/ZNSSubRegistrar.sol

@@ -37,6 +37,12 @@ contract ZNSSubRegistrar is AAccessControlled, ARegistryWired, UUPSUpgradeable,
         uint256 ownerIndex;
     }
 
+    struct Ownership {
+        address owner;
+        bool ownsBoth;
+        bool isOperatorForOwner;
+    }
+
     /**
      * @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
@@ -99,14 +105,30 @@ contract ZNSSubRegistrar is AAccessControlled, ARegistryWired, UUPSUpgradeable,
             !registry.exists(domainHash),
             "ZNSSubRegistrar: Subdomain already exists"
         );
+        require(
+            registry.exists(parentHash),
+            "ZNSSubRegistrar: Parent domain does not exist"
+        );
 
         DistributionConfig memory parentConfig = distrConfigs[parentHash];
 
-        bool isOwnerOrOperator = registry.isOwnerOrOperator(parentHash, msg.sender);
-        require(
-            parentConfig.accessType != AccessType.LOCKED || isOwnerOrOperator,
-            "ZNSSubRegistrar: Parent domain's distribution is locked or parent does not exist"
-        );
+        Ownership memory parent = Ownership({
+            owner: registry.getDomainOwner(parentHash),
+            ownsBoth: false,
+            isOperatorForOwner: false
+        });
+
+        parent.ownsBoth = rootRegistrar.isOwnerOf(parentHash, parent.owner, IZNSRootRegistrar.OwnerOf.BOTH);
+        parent.isOperatorForOwner = registry.isOperatorFor(msg.sender, parent.owner);
+
+        if (parentConfig.accessType == AccessType.LOCKED) {
+            require(
+                // Require that the parent owns both the token and the domain name
+                // as well as that the caller either is the parent owner, or an allowed operator
+                parent.ownsBoth && (parent.isOperatorForOwner || address(msg.sender) == parent.owner),
+                "ZNSSubRegistrar: Parent domain's distribution is locked"
+            );
+        }
 
         if (parentConfig.accessType == AccessType.MINTLIST) {
             require(
@@ -131,7 +153,9 @@ contract ZNSSubRegistrar is AAccessControlled, ARegistryWired, UUPSUpgradeable,
             paymentConfig: paymentConfig
         });
 
-        if (!isOwnerOrOperator) {
+        // If parent owns both and caller is either parent or an operator, mint for free
+        // If parent does not own both or the call is not an operator or the owner, pay to mint
+        if (!parent.ownsBoth || !(parent.isOperatorForOwner || address(msg.sender) == parent.owner)) {
             if (coreRegisterArgs.isStakePayment) {
                 (coreRegisterArgs.price, coreRegisterArgs.stakeFee) = IZNSPricer(address(parentConfig.pricerContract))
                     .getPriceAndFee(

test/ZNSSubRegistrar.test.ts

@@ -6,7 +6,6 @@ import {
   DEFAULT_TOKEN_URI,
   deployZNS,
   distrConfigEmpty,
-  DISTRIBUTION_LOCKED_NOT_EXIST_ERR,
   fullDistrConfigEmpty,
   getAccessRevertMsg,
   getPriceObject,
@@ -20,6 +19,8 @@ import {
   DECAULT_PRECISION,
   DEFAULT_PRICE_CONFIG,
   validateUpgrade,
+  PARENT_NOT_EXIST_ERR,
+  DISTRIBUTION_LOCKED_ERR,
 } from "./helpers";
 import * as hre from "hardhat";
 import * as ethers from "ethers";
@@ -39,7 +40,6 @@ import {
 import { deployCustomDecToken } from "./helpers/deploy/mocks";
 import { getProxyImplAddress } from "./helpers/utils";
 
-
 describe("ZNSSubRegistrar", () => {
   let deployer : SignerWithAddress;
   let rootOwner : SignerWithAddress;
@@ -139,6 +139,123 @@ describe("ZNSSubRegistrar", () => {
       expect(config.beneficiary).to.eq(lvl2SubOwner.address);
     });
 
+    it("Allows the owner of parent domain and token to register for free", async () => {
+      const subdomain = "another-subs";
+
+      const balanceBefore = await zns.meowToken.balanceOf(rootOwner.address);
+
+      await zns.subRegistrar.connect(rootOwner).registerSubdomain(
+        rootHash,
+        subdomain,
+        rootOwner.address,
+        subTokenURI,
+        distrConfigEmpty,
+        {
+          token: await zns.meowToken.getAddress(),
+          beneficiary: rootOwner.address,
+        },
+      );
+
+      const balanceAfter = await zns.meowToken.balanceOf(rootOwner.address);
+
+      expect(balanceAfter).to.eq(balanceBefore);
+    });
+
+    // cant any user now register for free if owner owns both?
+    it("Allows an operator to register for free if the owner owns both", async () => {
+      const subdomain = "another-sub";
+
+      const balanceBefore = await zns.meowToken.balanceOf(lvl2SubOwner.address);
+
+      await zns.meowToken.connect(lvl2SubOwner).approve(await zns.treasury.getAddress(), ethers.MaxUint256);
+
+      await zns.registry.connect(rootOwner).setOwnersOperator(lvl2SubOwner.address, true);
+
+      await zns.subRegistrar.connect(lvl2SubOwner).registerSubdomain(
+        rootHash,
+        subdomain,
+        rootOwner.address,
+        subTokenURI,
+        distrConfigEmpty,
+        {
+          token: await zns.meowToken.getAddress(),
+          beneficiary: rootOwner.address,
+        },
+      );
+
+      const balanceAfter = await zns.meowToken.balanceOf(lvl2SubOwner.address);
+
+      // Disable operator after check
+      await zns.registry.connect(rootOwner).setOwnersOperator(lvl2SubOwner.address, false);
+
+      expect(balanceAfter).to.eq(balanceBefore);
+    });
+
+    it("Charges a price for an owner that does not own both the token and name", async () => {
+      const subdomain = "no-diggity";
+
+      const balanceBefore = await zns.meowToken.balanceOf(rootOwner.address);
+
+      // Give token to lvl2SubOwner
+      await zns.domainToken.connect(rootOwner).transferFrom(rootOwner.address, lvl2SubOwner.address, rootHash);
+
+      await zns.subRegistrar.connect(rootOwner).registerSubdomain(
+        rootHash,
+        subdomain,
+        rootOwner.address,
+        subTokenURI,
+        distrConfigEmpty,
+        {
+          token: await zns.meowToken.getAddress(),
+          beneficiary: rootOwner.address,
+        },
+      );
+
+      // Restore token owner
+      await zns.domainToken.connect(lvl2SubOwner).transferFrom(lvl2SubOwner.address, rootOwner.address, rootHash);
+
+      const balanceAfter = await zns.meowToken.balanceOf(rootOwner.address);
+
+      expect(balanceAfter).to.not.eq(balanceBefore);
+    });
+
+    it("Charges a price for an operator of an owner that does not own both the token and name", async () => {
+      const subdomain = "no-diggities";
+
+      const balanceBefore = await zns.meowToken.balanceOf(lvl2SubOwner.address);
+
+      // Give token to lvl2SubOwner
+      await zns.domainToken.connect(rootOwner).transferFrom(rootOwner.address, lvl2SubOwner.address, rootHash);
+
+      await zns.registry.connect(rootOwner).setOwnersOperator(lvl2SubOwner.address, true);
+
+      await zns.subRegistrar.connect(lvl2SubOwner).registerSubdomain(
+        rootHash,
+        subdomain,
+        lvl2SubOwner.address,
+        subTokenURI,
+        distrConfigEmpty,
+        {
+          token: await zns.meowToken.getAddress(),
+          beneficiary: rootOwner.address,
+        },
+      );
+
+      // Restore token owner
+      await zns.domainToken.connect(lvl2SubOwner).transferFrom(lvl2SubOwner.address, rootOwner.address, rootHash);
+
+      await zns.registry.connect(rootOwner).setOwnersOperator(lvl2SubOwner.address, false);
+
+      const balanceAfter = await zns.meowToken.balanceOf(lvl2SubOwner.address);
+
+      expect(balanceAfter).to.not.eq(balanceBefore);
+    });
+
+    // disallows an owner from registering for free if they don't own both
+    // disallows an operator from registering for free if the owner does not own both
+    // owner surpasses locked domains
+    // operator surpasses locked domains, maybe already tested these two
+
     it("Does not set the payment config when the beneficiary is the zero address", async () => {
       const subdomain = "not-world-subdomain";
       await expect(
@@ -354,7 +471,7 @@ describe("ZNSSubRegistrar", () => {
           paymentConfigEmpty,
         )
       ).to.be.revertedWith(
-        DISTRIBUTION_LOCKED_NOT_EXIST_ERR
+        INVALID_TOKENID_ERC_ERR
       );
 
       // check that a random non-existent hash can NOT be passed as parentHash
@@ -369,7 +486,8 @@ describe("ZNSSubRegistrar", () => {
           paymentConfigEmpty,
         )
       ).to.be.revertedWith(
-        DISTRIBUTION_LOCKED_NOT_EXIST_ERR
+        // ERC721's `_requireMinted()` will fail when parent hash doesn't exist
+        PARENT_NOT_EXIST_ERR
       );
     });
 
@@ -940,7 +1058,7 @@ describe("ZNSSubRegistrar", () => {
           paymentConfigEmpty,
         )
       ).to.be.revertedWith(
-        DISTRIBUTION_LOCKED_NOT_EXIST_ERR
+        PARENT_NOT_EXIST_ERR
       );
 
       const dataFromReg = await zns.registry.getDomainRecord(domainHash);
@@ -1005,7 +1123,7 @@ describe("ZNSSubRegistrar", () => {
           paymentConfigEmpty,
         )
       ).to.be.revertedWith(
-        DISTRIBUTION_LOCKED_NOT_EXIST_ERR
+        PARENT_NOT_EXIST_ERR
       );
 
       const dataFromReg = await zns.registry.getDomainRecord(domainHash);
@@ -1232,7 +1350,7 @@ describe("ZNSSubRegistrar", () => {
           distrConfigEmpty,
           paymentConfigEmpty,
         )
-      ).to.be.revertedWith(DISTRIBUTION_LOCKED_NOT_EXIST_ERR);
+      ).to.be.revertedWith(PARENT_NOT_EXIST_ERR);
 
       // register root back for other tests
       await registrationWithSetup({
@@ -1264,7 +1382,7 @@ describe("ZNSSubRegistrar", () => {
           distrConfigEmpty,
           paymentConfigEmpty,
         )
-      ).to.be.revertedWith(DISTRIBUTION_LOCKED_NOT_EXIST_ERR);
+      ).to.be.revertedWith(PARENT_NOT_EXIST_ERR);
     });
 
     // eslint-disable-next-line max-len
@@ -2544,7 +2662,7 @@ describe("ZNSSubRegistrar", () => {
           paymentConfigEmpty,
         )
       ).to.be.revertedWith(
-        DISTRIBUTION_LOCKED_NOT_EXIST_ERR
+        DISTRIBUTION_LOCKED_ERR
       );
     });
 
@@ -2768,7 +2886,7 @@ describe("ZNSSubRegistrar", () => {
           paymentConfigEmpty,
         )
       ).to.be.revertedWith(
-        DISTRIBUTION_LOCKED_NOT_EXIST_ERR
+        DISTRIBUTION_LOCKED_ERR
       );
 
       // switch to mintlist
@@ -2850,7 +2968,7 @@ describe("ZNSSubRegistrar", () => {
           paymentConfigEmpty,
         )
       ).to.be.revertedWith(
-        DISTRIBUTION_LOCKED_NOT_EXIST_ERR
+        DISTRIBUTION_LOCKED_ERR
       );
     });
   });

test/gas/gas-costs.json

@@ -1,4 +1,4 @@
 {
 	"Root Domain Price": "475352",
-	"Subdomain Price": "469054"
+	"Subdomain Price": "478990"
 }

test/helpers/deploy-helpers.ts

@@ -117,7 +117,6 @@ export const registerRootDomainBulk = async (
     const domainHash = hashDomainLabel(domain);
     expect(await zns.registry.exists(domainHash)).to.be.true;
 
-    // TODO figure out if we want to do this on prod?
     // To mint subdomains from this domain we must first set the price config and the payment config
     await zns.curvePricer.connect(signers[index]).setPriceConfig(domainHash, priceConfig);
 

test/helpers/errors.ts

@@ -26,7 +26,8 @@ export const NOT_BOTH_OWNER_RAR_ERR = "ZNSRootRegistrar: Not the owner of both N
 
 // Subdomain Registrar
 // eslint-disable-next-line max-len
-export const DISTRIBUTION_LOCKED_NOT_EXIST_ERR = "ZNSSubRegistrar: Parent domain's distribution is locked or parent does not exist";
+export const DISTRIBUTION_LOCKED_ERR = "ZNSSubRegistrar: Parent domain's distribution is locked";
+export const PARENT_NOT_EXIST_ERR = "ZNSSubRegistrar: Parent domain does not exist";
 
 // StringUtils
 export const INVALID_NAME_ERR = "StringUtils: Invalid domain label";