Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added ZNS Ink Subgraph #1197

Closed
wants to merge 1 commit into from
Closed

Conversation

AayushCodes
Copy link
Contributor

@AayushCodes AayushCodes commented Jan 20, 2025

Summary by CodeRabbit

Based on the comprehensive changes, here are the release notes:

  • New Features

    • Added ZNS (Zero-knowledge Name Service) Registry subgraph for tracking domain events
    • Introduced support for multiple blockchain networks: Polygon, Blast, and Ink mainnet
    • Implemented domain registration, renewal, and transfer event tracking
  • Infrastructure

    • Created deployment and management scripts using Justfile
    • Added GraphQL schema for domain and resolver event tracking
    • Configured TypeScript and testing environment for the subgraph
  • Development Tools

    • Added utility functions for domain name and hash processing
    • Implemented comprehensive test utilities for event simulation
    • Configured project dependencies and build scripts

Copy link
Contributor

coderabbitai bot commented Jan 20, 2025

Walkthrough

The changes introduce a comprehensive configuration for a Zero-knowledge Name Service (ZNS) subgraph within the BlockScout ENS project. The addition includes multiple configuration files, a GraphQL schema, utility functions, and test utilities for managing domain registrations on the ink-mainnet blockchain. The project establishes a robust framework for tracking domain-related events such as registrations, renewals, and transfers, with support for multiple networks including polygon-mainnet, blast-mainnet, and ink-mainnet. The implementation provides detailed event handling, entity management, and a structured approach to subgraph development using the Graph Protocol.

Poem

🐰 Hop, hop, graph the domains bright,
Ink-mainnet's names now take their flight!
Registries dance, events unfurl,
A rabbit's subgraph starts to swirl 🌀
Blockchain naming, oh what delight! 🏷️

✨ Finishing Touches
  • 📝 Generate Docstrings (Beta)

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 11

🧹 Nitpick comments (9)
blockscout-ens/graph-node/subgraphs/zns-ink-subgraph/justfile (5)

1-4: Consider enhancing variable definitions with environment overrides and documentation.

The hardcoded localhost URLs might not be suitable for all environments. Consider:

  1. Adding comments to document the purpose of each variable
  2. Supporting environment variable overrides for flexibility
  3. Adding validation for required configurations
+# URL for the Graph Node API
-graph_node_url := "http://127.0.0.1:8020"
+graph_node_url := env_var_or_default("GRAPH_NODE_URL", "http://127.0.0.1:8020")
+
+# IPFS node URL for storing subgraph manifests
-ipfs_url := "http://127.0.0.1:5001"
+ipfs_url := env_var_or_default("IPFS_URL", "http://127.0.0.1:5001")
+
+# Target network for deployment
-network := "ink-mainnet"
+network := env_var_or_default("NETWORK", "ink-mainnet")
+
+# Subgraph name identifier
-name := "zns-ink-subgraph"
+name := env_var_or_default("SUBGRAPH_NAME", "zns-ink-subgraph")

6-14: Add prerequisites and status checks to build tasks.

The build-related tasks (init, codegen, build) should check for prerequisites and handle errors appropriately.

 init:
-    yarn install
+    #!/usr/bin/env bash
+    set -euo pipefail
+    yarn install
 
 codegen:
-    yarn codegen
+    #!/usr/bin/env bash
+    set -euo pipefail
+    [ -f "package.json" ] || { echo "Error: package.json not found"; exit 1; }
+    yarn codegen
 
 build:
-    yarn build
+    #!/usr/bin/env bash
+    set -euo pipefail
+    [ -f "schema.graphql" ] || { echo "Error: schema.graphql not found"; exit 1; }
+    yarn build

15-17: Add validation before creating subgraph.

The create task should validate the graph node URL before proceeding.

 create:
+    #!/usr/bin/env bash
+    set -euo pipefail
+    # Validate graph node URL
+    curl --fail --silent {{graph_node_url}}/graphql >/dev/null || { echo "Error: Graph node not accessible"; exit 1; }
     yarn graph create --node {{graph_node_url}} {{name}}

18-20: Add version validation to deploy task.

The deploy task should validate the version format before proceeding with deployment.

 deploy VERSION:
+    #!/usr/bin/env bash
+    set -euo pipefail
+    # Validate version format (e.g., v1.2.3)
+    [[ {{VERSION}} =~ ^v[0-9]+\.[0-9]+\.[0-9]+$ ]] || { echo "Error: Invalid version format. Use vX.Y.Z"; exit 1; }
     yarn graph deploy --node {{graph_node_url}} --ipfs {{ipfs_url}} {{name}} --network {{network}} --version-label {{VERSION}}

1-23: Consider adding common utility tasks.

The Justfile would benefit from additional utility tasks such as:

  • clean: Remove generated files
  • test: Run subgraph tests
  • lint: Check for code style issues
  • validate: Verify subgraph manifest
  • help: Display available commands and their usage

Would you like me to provide implementations for these additional utility tasks?

blockscout-ens/graph-node/subgraphs/zns-ink-subgraph/src/utils.ts (1)

38-48: Consider using TypedArray.set for better performance.

The current implementation uses manual loops, but TypedArray.set would be more efficient.

Consider this optimization:

 export function concat(a: ByteArray, b: ByteArray): ByteArray {
   let out = new Uint8Array(a.length + b.length);
-  for (let i = 0; i < a.length; i++) {
-    out[i] = a[i];
-  }
-  for (let j = 0; j < b.length; j++) {
-    out[a.length + j] = b[j];
-  }
+  out.set(a, 0);
+  out.set(b, a.length);
   return changetype<ByteArray>(out);
 }
blockscout-ens/graph-node/subgraphs/zns-ink-subgraph/src/zns-registry.ts (1)

19-21: Refactor domain initialization to reduce code duplication

The logic for initializing a Domain entity when it does not exist is duplicated in both handleMintedDomain (lines 19-21) and handleTransfer (lines 80-90). Refactoring this code into a shared function will improve maintainability and reduce potential inconsistencies.

Consider creating a helper function getOrCreateDomain(domainName: string, timestamp: BigInt): Domain to encapsulate this logic.

Also applies to: 80-90

blockscout-ens/graph-node/subgraphs/zns-ink-subgraph/networks.json (1)

1-20: Add network documentation.

Consider adding documentation about each network's purpose and configuration requirements.

Add a README.md file with:

  • Network descriptions
  • Contract deployment details
  • Start block selection rationale
  • Configuration update procedures
blockscout-ens/graph-node/subgraphs/zns-ink-subgraph/schema.graphql (1)

270-279: Consider adding indexes for Account entity.

The Account entity's relationships could benefit from indexing for better query performance.

 type Account @entity {
   "The unique identifier for the account"
   id: ID!
   "The domains owned by the account"
-  domains: [Domain!]! @derivedFrom(field: "owner")
+  domains: [Domain!]! @derivedFrom(field: "owner") @index
   "The WrappedDomains owned by the account"
-  wrappedDomains: [WrappedDomain!] @derivedFrom(field: "owner")
+  wrappedDomains: [WrappedDomain!] @derivedFrom(field: "owner") @index
   "The Registrations made by the account"
-  registrations: [Registration!] @derivedFrom(field: "registrant")
+  registrations: [Registration!] @derivedFrom(field: "registrant") @index
 }
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8f2caa9 and e49cdec.

⛔ Files ignored due to path filters (1)
  • blockscout-ens/graph-node/subgraphs/zns-ink-subgraph/yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (12)
  • blockscout-ens/graph-node/subgraphs/zns-ink-subgraph/.gitignore (1 hunks)
  • blockscout-ens/graph-node/subgraphs/zns-ink-subgraph/abis/ZNSRegistry.json (1 hunks)
  • blockscout-ens/graph-node/subgraphs/zns-ink-subgraph/justfile (1 hunks)
  • blockscout-ens/graph-node/subgraphs/zns-ink-subgraph/networks.json (1 hunks)
  • blockscout-ens/graph-node/subgraphs/zns-ink-subgraph/package.json (1 hunks)
  • blockscout-ens/graph-node/subgraphs/zns-ink-subgraph/schema.graphql (1 hunks)
  • blockscout-ens/graph-node/subgraphs/zns-ink-subgraph/src/utils.ts (1 hunks)
  • blockscout-ens/graph-node/subgraphs/zns-ink-subgraph/src/zns-registry.ts (1 hunks)
  • blockscout-ens/graph-node/subgraphs/zns-ink-subgraph/subgraph.yaml (1 hunks)
  • blockscout-ens/graph-node/subgraphs/zns-ink-subgraph/tests/zns-registry-utils.ts (1 hunks)
  • blockscout-ens/graph-node/subgraphs/zns-ink-subgraph/tests/zns-registry.test.ts (1 hunks)
  • blockscout-ens/graph-node/subgraphs/zns-ink-subgraph/tsconfig.json (1 hunks)
✅ Files skipped from review due to trivial changes (3)
  • blockscout-ens/graph-node/subgraphs/zns-ink-subgraph/tsconfig.json
  • blockscout-ens/graph-node/subgraphs/zns-ink-subgraph/.gitignore
  • blockscout-ens/graph-node/subgraphs/zns-ink-subgraph/package.json
🧰 Additional context used
🪛 Biome (1.9.4)
blockscout-ens/graph-node/subgraphs/zns-ink-subgraph/tests/zns-registry.test.ts

[error] 9-9: Do not shadow the global "BigInt" property.

Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.

(lint/suspicious/noShadowRestrictedNames)

blockscout-ens/graph-node/subgraphs/zns-ink-subgraph/tests/zns-registry-utils.ts

[error] 2-2: Do not shadow the global "BigInt" property.

Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.

(lint/suspicious/noShadowRestrictedNames)

⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: Unit, doc and integration tests
🔇 Additional comments (15)
blockscout-ens/graph-node/subgraphs/zns-ink-subgraph/src/utils.ts (2)

28-36: LGTM!

The implementation is clean and handles edge cases correctly.


50-52: LGTM!

The implementation is concise and uses the appropriate crypto primitives.

blockscout-ens/graph-node/subgraphs/zns-ink-subgraph/src/zns-registry.ts (1)

25-28: Ensure domain name parsing is robust

When extracting labelName by splitting event.params.domainName, ensure that the domain name contains a dot to prevent unexpected results. If domainName does not contain a dot, split(".")[0] will return the entire string, which might not be the intended label.

Consider adding a check or handling domains without dots appropriately.

blockscout-ens/graph-node/subgraphs/zns-ink-subgraph/networks.json (3)

14-19: Verify the ink-mainnet ZNSRegistry contract.

The contract address and start block look valid, but let's verify the contract deployment.


2-7: Verify the polygon-mainnet ZNSRegistry contract.

The contract address and start block look valid, but let's verify the contract deployment.


8-13: Verify the blast-mainnet ZNSRegistry contract.

The contract address and start block look valid, but let's verify the contract deployment.

blockscout-ens/graph-node/subgraphs/zns-ink-subgraph/subgraph.yaml (3)

1-3: Verify Graph Node compatibility.

The spec version (0.0.5) and schema file path look correct.


4-11: Verify contract configuration.

The data source configuration for ink-mainnet matches the networks.json entry.


12-31: Verify event handler implementation.

Ensure the event handlers in ./src/zns-registry.ts are implemented for all events.

✅ Verification successful

All event handlers are properly implemented

The source file contains complete implementations for all three event handlers defined in the subgraph manifest:

  • handleMintedDomain
  • handleRenewedDomain
  • handleTransfer
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check if all event handlers are implemented
# Expected: Each handler should exist in the source file

# Check for handler implementations
rg -l "export function handle(Minted|Renewed|Transferred)Domain" "src/zns-registry.ts" || echo "Handlers not found"

Length of output: 193


Script:

#!/bin/bash
# First, find the correct path to zns-registry.ts
fd -t f "zns-registry.ts$"

# Then use ast-grep to check for handler function implementations
ast-grep --pattern 'export function handleMintedDomain($_) { $$$ }'
ast-grep --pattern 'export function handleRenewedDomain($_) { $$$ }'
ast-grep --pattern 'export function handleTransfer($_) { $$$ }'

Length of output: 377


Script:

#!/bin/bash
# Check for handler implementations with context in the specific file
rg -A 5 "export function handle(Minted|Renewed|Transferred|Transfer)" "blockscout-ens/graph-node/subgraphs/zns-ink-subgraph/src/zns-registry.ts"

# Also check file content to ensure we're looking at the right file
cat "blockscout-ens/graph-node/subgraphs/zns-ink-subgraph/src/zns-registry.ts"

Length of output: 4502

blockscout-ens/graph-node/subgraphs/zns-ink-subgraph/schema.graphql (2)

1-46: Well-structured Domain entity definition.

The Domain entity captures all necessary fields and relationships for domain management.


48-57: Well-defined DomainEvent interface.

The interface provides a solid foundation for all domain-related events.

blockscout-ens/graph-node/subgraphs/zns-ink-subgraph/abis/ZNSRegistry.json (4)

119-149: Verify MintedDomain event structure.

The event structure matches the handler configuration in subgraph.yaml.


189-212: Verify RenewedDomain event structure.

The event structure matches the handler configuration in subgraph.yaml.


313-343: Verify TransferredDomain event structure.

The event structure matches the handler configuration in subgraph.yaml.


1-1006: Well-structured contract ABI.

The ABI is comprehensive and includes all necessary functions, events, and error definitions for the ZNS Registry.

Comment on lines +21 to +23
deploy-remote VERSION:
just graph_node_url={{graph_node_url}} ipfs_url=http://ipfs.node.blockscout.com network={{network}} name={{name}} \
deploy {{VERSION}}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Avoid hardcoding IPFS URL in deploy-remote task.

The deploy-remote task hardcodes an IPFS URL, which reduces flexibility and maintainability.

 deploy-remote VERSION:
-    just graph_node_url={{graph_node_url}} ipfs_url=http://ipfs.node.blockscout.com network={{network}} name={{name}} \
+    just graph_node_url={{graph_node_url}} ipfs_url={{env_var_or_default("REMOTE_IPFS_URL", "http://ipfs.node.blockscout.com")}} network={{network}} name={{name}} \
         deploy {{VERSION}}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
deploy-remote VERSION:
just graph_node_url={{graph_node_url}} ipfs_url=http://ipfs.node.blockscout.com network={{network}} name={{name}} \
deploy {{VERSION}}
deploy-remote VERSION:
just graph_node_url={{graph_node_url}} ipfs_url={{env_var_or_default("REMOTE_IPFS_URL", "http://ipfs.node.blockscout.com")}} network={{network}} name={{name}} \
deploy {{VERSION}}

Comment on lines +3 to +12
export function byteArrayFromHex(s: string): ByteArray {
if (s.length % 2 !== 0) {
throw new TypeError("Hex string must have an even number of characters");
}
let out = new Uint8Array(s.length / 2);
for (var i = 0; i < s.length; i += 2) {
out[i / 2] = parseInt(s.substring(i, i + 2), 16) as u32;
}
return changetype<ByteArray>(out);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add hex string validation and modernize syntax.

The function should validate hex string characters and use modern TypeScript practices.

Apply these improvements:

 export function byteArrayFromHex(s: string): ByteArray {
   if (s.length % 2 !== 0) {
     throw new TypeError("Hex string must have an even number of characters");
   }
+  if (!/^[0-9a-fA-F]+$/.test(s)) {
+    throw new TypeError("Invalid hex string");
+  }
   let out = new Uint8Array(s.length / 2);
-  for (var i = 0; i < s.length; i += 2) {
+  for (let i = 0; i < s.length; i += 2) {
     out[i / 2] = parseInt(s.substring(i, i + 2), 16) as u32;
   }
   return changetype<ByteArray>(out);
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export function byteArrayFromHex(s: string): ByteArray {
if (s.length % 2 !== 0) {
throw new TypeError("Hex string must have an even number of characters");
}
let out = new Uint8Array(s.length / 2);
for (var i = 0; i < s.length; i += 2) {
out[i / 2] = parseInt(s.substring(i, i + 2), 16) as u32;
}
return changetype<ByteArray>(out);
}
export function byteArrayFromHex(s: string): ByteArray {
if (s.length % 2 !== 0) {
throw new TypeError("Hex string must have an even number of characters");
}
if (!/^[0-9a-fA-F]+$/.test(s)) {
throw new TypeError("Invalid hex string");
}
let out = new Uint8Array(s.length / 2);
for (let i = 0; i < s.length; i += 2) {
out[i / 2] = parseInt(s.substring(i, i + 2), 16) as u32;
}
return changetype<ByteArray>(out);
}

Comment on lines +14 to +26
export function hashByName(name: string): ByteArray {
if (!name) {
return byteArrayFromHex("0".repeat(64));
} else {
const partition = splitStringOnce(name, ".");
const label = partition[0];
const remainder = partition[1];

return crypto.keccak256(
concat(hashByName(remainder), keccakFromStr(label))
);
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add domain name validation and prevent stack overflow.

The recursive implementation needs safeguards against malformed inputs and deep recursion.

Consider these improvements:

+const MAX_DOMAIN_PARTS = 127; // DNS technical limit
+const MAX_DOMAIN_LENGTH = 253; // DNS max length
+
 export function hashByName(name: string): ByteArray {
+  if (name.length > MAX_DOMAIN_LENGTH) {
+    throw new Error("Domain name exceeds maximum length");
+  }
+  if (name.split(".").length > MAX_DOMAIN_PARTS) {
+    throw new Error("Domain has too many parts");
+  }
   if (!name) {
     return byteArrayFromHex("0".repeat(64));
   } else {
     const partition = splitStringOnce(name, ".");
     const label = partition[0];
     const remainder = partition[1];
+    
+    if (label.length > 63) { // DNS label length limit
+      throw new Error("Domain label exceeds maximum length");
+    }

     return crypto.keccak256(
       concat(hashByName(remainder), keccakFromStr(label))
     );
   }
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export function hashByName(name: string): ByteArray {
if (!name) {
return byteArrayFromHex("0".repeat(64));
} else {
const partition = splitStringOnce(name, ".");
const label = partition[0];
const remainder = partition[1];
return crypto.keccak256(
concat(hashByName(remainder), keccakFromStr(label))
);
}
}
const MAX_DOMAIN_PARTS = 127; // DNS technical limit
const MAX_DOMAIN_LENGTH = 253; // DNS max length
export function hashByName(name: string): ByteArray {
if (name.length > MAX_DOMAIN_LENGTH) {
throw new Error("Domain name exceeds maximum length");
}
if (name.split(".").length > MAX_DOMAIN_PARTS) {
throw new Error("Domain has too many parts");
}
if (!name) {
return byteArrayFromHex("0".repeat(64));
} else {
const partition = splitStringOnce(name, ".");
const label = partition[0];
const remainder = partition[1];
if (label.length > 63) { // DNS label length limit
throw new Error("Domain label exceeds maximum length");
}
return crypto.keccak256(
concat(hashByName(remainder), keccakFromStr(label))
);
}
}

Comment on lines +54 to +57
export function domainNameIsCorrect(name: string): boolean {
const labelName = name.split(".")[0];
return labelName != null && labelName != ''
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add comprehensive domain name validation.

The current validation is too permissive and should follow DNS label requirements.

Consider this enhanced validation:

 export function domainNameIsCorrect(name: string): boolean {
   const labelName = name.split(".")[0];
-  return labelName != null && labelName != ''
+  if (!labelName || labelName.length === 0) {
+    return false;
+  }
+  // RFC 1035: Labels must be 1-63 characters and contain only a-z, A-Z, 0-9, and hyphen
+  // Hyphen cannot be first or last character
+  if (labelName.length > 63) {
+    return false;
+  }
+  return /^[a-zA-Z0-9]([a-zA-Z0-9-]*[a-zA-Z0-9])?$/.test(labelName);
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export function domainNameIsCorrect(name: string): boolean {
const labelName = name.split(".")[0];
return labelName != null && labelName != ''
}
export function domainNameIsCorrect(name: string): boolean {
const labelName = name.split(".")[0];
if (!labelName || labelName.length === 0) {
return false;
}
// RFC 1035: Labels must be 1-63 characters and contain only a-z, A-Z, 0-9, and hyphen
// Hyphen cannot be first or last character
if (labelName.length > 63) {
return false;
}
return /^[a-zA-Z0-9]([a-zA-Z0-9-]*[a-zA-Z0-9])?$/.test(labelName);
}

Comment on lines +91 to +93
let transfer = new Transfer(event.transaction.hash.toHex());
transfer.domain = domain.id;
transfer.owner = event.params.to.toHex();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Ensure unique IDs for Transfer entities to prevent collisions

Using event.transaction.hash.toHex() as the ID for the Transfer entity may cause collisions if multiple transfer events occur within the same transaction. To ensure uniqueness, include the log index in the ID.

Apply this diff to modify the Transfer entity ID:

-let transfer = new Transfer(event.transaction.hash.toHex());
+let transferId = event.transaction.hash
+  .toHexString()
+  .concat("-")
+  .concat(event.logIndex.toString());
+let transfer = new Transfer(transferId);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
let transfer = new Transfer(event.transaction.hash.toHex());
transfer.domain = domain.id;
transfer.owner = event.params.to.toHex();
let transferId = event.transaction.hash
.toHexString()
.concat("-")
.concat(event.logIndex.toString());
let transfer = new Transfer(transferId);
transfer.domain = domain.id;
transfer.owner = event.params.to.toHex();

Comment on lines +42 to +48
let mintedEvent = new Registration(event.params.tokenId.toString());
mintedEvent.registrant = event.params.owner.toHex();
mintedEvent.domain = domain.id;
mintedEvent.registrationDate = event.block.timestamp;
mintedEvent.expiryDate = event.params.expiry;
mintedEvent.cost = event.transaction.value;
mintedEvent.save();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Use unique IDs for Registration entities to prevent overwriting

Using event.params.tokenId.toString() as the ID for the Registration entity may lead to data overwrites if the same token ID is registered multiple times (e.g., after expiration and re-registration). To uniquely identify each registration event, include the transaction hash and log index in the ID.

Apply this diff to modify the Registration entity ID:

-let mintedEvent = new Registration(event.params.tokenId.toString());
+let registrationId = event.transaction.hash
+  .toHexString()
+  .concat("-")
+  .concat(event.logIndex.toString());
+let mintedEvent = new Registration(registrationId);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
let mintedEvent = new Registration(event.params.tokenId.toString());
mintedEvent.registrant = event.params.owner.toHex();
mintedEvent.domain = domain.id;
mintedEvent.registrationDate = event.block.timestamp;
mintedEvent.expiryDate = event.params.expiry;
mintedEvent.cost = event.transaction.value;
mintedEvent.save();
let registrationId = event.transaction.hash
.toHexString()
.concat("-")
.concat(event.logIndex.toString());
let mintedEvent = new Registration(registrationId);
mintedEvent.registrant = event.params.owner.toHex();
mintedEvent.domain = domain.id;
mintedEvent.registrationDate = event.block.timestamp;
mintedEvent.expiryDate = event.params.expiry;
mintedEvent.cost = event.transaction.value;
mintedEvent.save();

import { Address, BigInt, Bytes } from "@graphprotocol/graph-ts"
import { Approval } from "../generated/schema"
import { Approval as ApprovalEvent } from "../generated/ZNSRegistry/ZNSRegistry"
import { handleApproval } from "../src/zns-registry"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Implement 'handleApproval' handler function

The test invokes handleApproval, but there is no corresponding implementation in src/zns-registry.ts. This will cause the test to fail. Ensure that the handleApproval function is implemented in your handler code.

beforeAll,
afterAll
} from "matchstick-as/assembly/index"
import { Address, BigInt, Bytes } from "@graphprotocol/graph-ts"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Avoid shadowing the global 'BigInt' property

Importing BigInt shadows the global BigInt property, which can lead to confusion and potential issues. Consider renaming the imported BigInt to avoid conflicts.

Apply this diff to rename the imported BigInt:

-import { Address, BigInt, Bytes } from "@graphprotocol/graph-ts"
+import { Address, BigInt as GraphBigInt, Bytes } from "@graphprotocol/graph-ts"

Update all usages of BigInt in the file to GraphBigInt.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
import { Address, BigInt, Bytes } from "@graphprotocol/graph-ts"
import { Address, BigInt as GraphBigInt, Bytes } from "@graphprotocol/graph-ts"
🧰 Tools
🪛 Biome (1.9.4)

[error] 9-9: Do not shadow the global "BigInt" property.

Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.

(lint/suspicious/noShadowRestrictedNames)

Comment on lines +40 to +57
assert.fieldEquals(
"Approval",
"0xa16081f360e3847006db660bae1c6d1b2e17ec2a-1",
"owner",
"0x0000000000000000000000000000000000000001"
)
assert.fieldEquals(
"Approval",
"0xa16081f360e3847006db660bae1c6d1b2e17ec2a-1",
"approved",
"0x0000000000000000000000000000000000000001"
)
assert.fieldEquals(
"Approval",
"0xa16081f360e3847006db660bae1c6d1b2e17ec2a-1",
"tokenId",
"234"
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Use dynamic IDs in assertions to improve test reliability

The test uses hardcoded entity IDs, which can make tests brittle and less maintainable. It's better to generate IDs dynamically based on event data to ensure the test remains valid even if underlying behaviors change.

Modify the assertions to use the same ID generation logic as in the handler:

-    "0xa16081f360e3847006db660bae1c6d1b2e17ec2a-1",
+    newApprovalEvent.transaction.hash.toHexString().concat("-").concat(newApprovalEvent.logIndex.toString()),

Committable suggestion skipped: line range outside the PR's diff.

@@ -0,0 +1,295 @@
import { newMockEvent } from "matchstick-as"
import { ethereum, Address, BigInt, Bytes } from "@graphprotocol/graph-ts"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Avoid shadowing the global 'BigInt' property

Importing BigInt shadows the global BigInt property. To prevent confusion and potential errors, rename the imported BigInt.

Apply this diff to rename the imported BigInt:

-import { ethereum, Address, BigInt, Bytes } from "@graphprotocol/graph-ts"
+import { ethereum, Address, BigInt as GraphBigInt, Bytes } from "@graphprotocol/graph-ts"

Update all usages of BigInt in the file to GraphBigInt.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
import { ethereum, Address, BigInt, Bytes } from "@graphprotocol/graph-ts"
import { ethereum, Address, BigInt as GraphBigInt, Bytes } from "@graphprotocol/graph-ts"
🧰 Tools
🪛 Biome (1.9.4)

[error] 2-2: Do not shadow the global "BigInt" property.

Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.

(lint/suspicious/noShadowRestrictedNames)

@sevenzing
Copy link
Contributor

it's possible to reuse zns-subgraph code. I will deploy it to ink chain in bens production soon

@sevenzing sevenzing closed this Jan 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants