SubRegistrar tests improvement (Domain helper class) #161

MichaelKorchagin · 2025-10-03T02:02:40Z

No description provided.

…ister them using Domain class.

… statement to signer var.

…n paths"

…oken.

…r optimization according to registration and revocation.

…WIP]

…s. Adjusted distrConfig setter for Domain class. WIP

…plicate

Copilot

Pull Request Overview

Improves test coverage and refactors the Domain test helper to use object parameter patterns while adding new helper methods; introduces a comprehensive ZNSSubRegistrar unit test suite and adjusts existing tests to the new Domain API. Key changes include: refactoring Domain helper method signatures, adding retrieval and state mutation helpers, and expanding sub-registrar test scenarios (bulk + nested registration, config mutation, and access control behaviors).

Refactored Domain helper methods to accept single object params and added config/query utilities.
Added extensive ZNSSubRegistrar.unit.test.ts covering bulk registration, nested domains, and state mutation paths.
Updated existing tests (RootRegistrar, AddressResolver) to new Domain helper API and added minor type simplifications.

Reviewed Changes

Copilot reviewed 9 out of 10 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
test/helpers/types.ts	Narrowed pricerContract type; potential API restriction.
test/helpers/register-setup.ts	Simplified approval flow; removed conditional allowance check.
test/helpers/domain/domain.ts	Major refactor: new helper methods, object param signatures, pricing logic changes; introduced several logic issues.
test/helpers/deploy/deploy-zns.ts	Removed unused import.
test/helpers/ac.ts	Added file containing only commented-out (inactive) test scaffold.
test/ZNSSubRegistrar.unit.test.ts	New comprehensive unit tests; contains a loop logic bug.
test/ZNSRootRegistrar.test.ts	Updated calls to new Domain helper signatures; switched to helper getters.
test/ZNSAddressResolver.test.ts	Updated operator-setting call to new object-style API.
test/DeployCampaignInt.test.ts	Removed unused git tagging utility invocation/import.

Comments suppressed due to low confidence (1)

test/helpers/ac.ts:1

New file consists only of commented-out code; keeping inactive scaffolding increases noise and can confuse contributors. Remove the file or add active tests; if kept for future work, add a brief comment explaining its intentional placeholder status.

// import { SignerWithAddress } from "@nomicfoundation/hardhat-ethers/signers";

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

test/helpers/types.ts

test/helpers/register-setup.ts

Copilot · 2025-10-06T21:23:38Z

test/helpers/domain/domain.ts

    if (priceConfig ||
      this.priceConfig !== undefined ||
      Object.keys(this.priceConfig).length === 0
    ) {
+      const args = [this.hash];


Logical condition is flawed: (a) Object.keys(this.priceConfig) is accessed when this.priceConfig may be undefined (runtime TypeError), (b) using ORs makes the branch almost always true, and (c) when priceConfig is undefined you still cast and encode it at line 301 (likely undefined -> error). Replace with a guarded condition such as: if (priceConfig) { ... } else if (this.priceConfig) { reuse existing } else { throw error } and only call Object.keys after confirming existence.

Copilot · 2025-10-06T21:23:38Z

test/helpers/domain/domain.ts

+        this.priceConfig = decodePriceConfig(priceConfig);
+
+      } else {
+        args.push(encodePriceConfig(priceConfig as ICurvePriceConfig | IFixedPriceConfig));


Logical condition is flawed: (a) Object.keys(this.priceConfig) is accessed when this.priceConfig may be undefined (runtime TypeError), (b) using ORs makes the branch almost always true, and (c) when priceConfig is undefined you still cast and encode it at line 301 (likely undefined -> error). Replace with a guarded condition such as: if (priceConfig) { ... } else if (this.priceConfig) { reuse existing } else { throw error } and only call Object.keys after confirming existence.

test/helpers/domain/domain.ts

Copilot · 2025-10-06T21:23:39Z

test/helpers/domain/domain.ts

+  async setPricerDataForDomain ({
+    priceConfig,
+    pricerContract,
+    executor,
+  } : {
+    priceConfig ?: ICurvePriceConfig | IFixedPriceConfig | string;
+    pricerContract ?: string;
+    executor ?: SignerWithAddress;
+  }) {
    if (priceConfig ||
      this.priceConfig !== undefined ||
      Object.keys(this.priceConfig).length === 0
    ) {
+      const args = [this.hash];
+
+      if (typeof priceConfig === "string") {
+        args.push(priceConfig);
+
+        this.priceConfig = decodePriceConfig(priceConfig);
+
+      } else {
+        args.push(encodePriceConfig(priceConfig as ICurvePriceConfig | IFixedPriceConfig));
+
+        this.priceConfig = priceConfig as ICurvePriceConfig | IFixedPriceConfig;
+      }
+
+      args.push(pricerContract ? pricerContract : this.distrConfig.pricerContract);
+
      await this.zns.subRegistrar.connect(executor ? executor : this.owner).setPricerDataForDomain(
-        this.hash,
-        priceConfig ? encodePriceConfig(priceConfig) : encodePriceConfig(this.priceConfig),
-        pricerContract ? pricerContract : this.distrConfig.pricerContract
+        ...args
      );
    } else {
      throw new Error("Domain Helper: priceConfig is not specified");
    }
-
-    // updating local var
-    if (priceConfig) {
-      this.priceConfig = priceConfig;
-    }
  }



[nitpick] Allowing priceConfig to be either a structured config object or a pre-encoded string mixes responsibilities and weakens type safety. Consider splitting into two distinct methods (setPricerDataWithConfig vs setPricerDataWithBytes) or using a discriminated union to avoid ambiguous runtime branching.

Copilot · 2025-10-06T21:23:39Z

test/ZNSSubRegistrar.unit.test.ts

+      });
+
+      // eslint-disable-next-line @typescript-eslint/prefer-for-of
+      for (let i = logs.length - 1; i === 0; i--) {


Loop condition uses i === 0, so it only executes when logs.length - 1 == 0 (single log); for multiple logs the body never runs. Replace condition with i >= 0 to iterate all logs in reverse.

Suggested change

for (let i = logs.length - 1; i === 0; i--) {

for (let i = logs.length - 1; i >= 0; i--) {

codecov · 2025-10-07T02:30:14Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (dbf2d7d) to head (47c8a50).

Additional details and impacted files

@@              Coverage Diff              @@
##           development      #161   +/-   ##
=============================================
  Coverage       100.00%   100.00%           
=============================================
  Files               13        13           
  Lines              596       596           
  Branches           137       137           
=============================================
  Hits               596       596

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

MichaelKorchagin added 26 commits July 3, 2025 19:02

Started updating SubReg test. Changed rootDomains registration to reg…

d9093d5

…ister them using Domain class.

Subregistrar tests optimization

1623b21

More test fixes according to domain registration. Changed executor if…

6b18561

… statement to signer var.

Test fixes. Corrected approval for a domain in the Domain class.

5f08a2b

More registration fixes.

233748d

Done with all bulk registration tests. Started "Operations with domai…

3ddc55d

…n paths"

Fails fixes for first tests in "Operations with domain paths"

540d2dc

2nd describe test fixes

fff89ac

Added a couple of getters from the class and one setter for payment t…

b728da0

…oken.

Redundant checks removal in "Single Subdomain Registration"

cd0398e

Added passing parameters by objects in the Domain class

3cbdd97

Getters have been moved to the appropriate location. More subregistra…

25bbfef

…r optimization according to registration and revocation.

Test failing fix

17954d6

RootReg test minor fixes

0eb739b

Merge branch 'development' into domain-helper-128-duplicate

f26aa62

Moved most part of unit tests to separate module. Updated proccess. […

a7918ab

…WIP]

Moved more unit tests to appropriated module. Corrected order of test…

985885a

…s. Adjusted distrConfig setter for Domain class. WIP

Minor test fix

1b138e8

Tests type fixes

c03901e

Merge branch 'test/domain-helper-class-128' into domain-helper-128-du…

c9c4b6d

…plicate

After merge fixes

05a0410

Updated Domain class functions to work with object params. Updated tests

07349e7

Test fixes in accordance with new domain methods.

5eb6ebc

More tests updated in accordance of Domain class changes.

4724d7e

Minor Domain class fixes

2511f14

Linter fixes.

e467e06

Whytecrowe requested a review from Copilot October 6, 2025 21:21

Copilot AI reviewed Oct 6, 2025

View reviewed changes

MichaelKorchagin added 2 commits October 6, 2025 15:12

Merge branch 'development' into domain-helper-128-duplicate

c0545bc

Fixes based on Copilot's review

fbc4468

CI check

47c8a50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

SubRegistrar tests improvement (Domain helper class) #161

SubRegistrar tests improvement (Domain helper class) #161

Uh oh!

MichaelKorchagin commented Oct 3, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI Oct 6, 2025

Uh oh!

Copilot AI Oct 6, 2025

Uh oh!

Uh oh!

Uh oh!

Copilot AI Oct 6, 2025

Uh oh!

Copilot AI Oct 6, 2025

Uh oh!

codecov bot commented Oct 7, 2025 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	for (let i = logs.length - 1; i === 0; i--) {
	for (let i = logs.length - 1; i >= 0; i--) {

SubRegistrar tests improvement (Domain helper class) #161

Are you sure you want to change the base?

SubRegistrar tests improvement (Domain helper class) #161

Uh oh!

Conversation

MichaelKorchagin commented Oct 3, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI Oct 6, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 6, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI Oct 6, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 6, 2025

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Oct 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov bot commented Oct 7, 2025 •

edited

Loading