ensdomains / ensjs

ENS javascript library for contract interaction
MIT License
113 stars 46 forks source link

`getOwner` crashes on Mainnet if the ENS domain name does not exist/has not been registtered #97

Closed Magofoco closed 1 year ago

Magofoco commented 1 year ago

On Mainnet, the function .getOwner crashes with an error if the ENS domain name input to the function has not been registered.

Example:

    const NOT_REGISTERED_ENS_DOMAIN = "thisisnotregisteredens.eth"
    const ownerObject = await ENSInstance.getOwner(NOT_REGISTERED_ENS_DOMAIN);
    console.log(ownerObject);

crashes with the following error instead of returning undefined

/home/x/node_modules/graphql-request/dist/index.js:67
            op = body.call(thisArg, _);
                      ^
ClientError: Type `Domain` has no field `registration`: {"response":{"errors":[{"locations":[{"line":3,"column":5}],"message":"Type `Domain` has no field `registration`"}],"status":200,"headers":{}},"request":{"query":"\n  query GetRegistrant($namehash: String!) {\n    domain(id: $namehash) {\n      registration {\n        registrant {\n          id\n        }\n      }\n    }\n  }\n","variables":{"namehash":"0xd4a5aec5954a3e37fc24f33c3bd145d0e258e521f74a15371e3d3079637047b3"}}}
    at /home/x/node_modules/graphql-request/src/index.ts:498:11
    at step (/home/x/node_modules/graphql-request/dist/index.js:67:23)
    at Object.next (/home/x/node_modules/graphql-request/dist/index.js:48:53)
    at fulfilled (/home/x/node_modules/graphql-request/dist/index.js:39:58)
    at processTicksAndRejections (node:internal/process/task_queues:96:5) {
  response: {
    errors: [ [Object] ],
    status: 200,
    headers: Headers { [Symbol(map)]: [Object: null prototype] }
  },
  request: {
    query: '\n' +
      '  query GetRegistrant($namehash: String!) {\n' +
      '    domain(id: $namehash) {\n' +
      '      registration {\n' +
      '        registrant {\n' +
      '          id\n' +
      '        }\n' +
      '      }\n' +
      '    }\n' +
      '  }\n',
    variables: {
      namehash: '0xd4a5aec5954a3e37fc24f33c3bd145d0e258e521f74a15371e3d3079637047b3'
    }
  }
}

However On Goerli Network, the same function does not crash and returns, correctly, undefined since the ens domain "thisisnotregisteredens.eth" is still available to be purchased

TateB commented 1 year ago

hey, this is because currently the mainnet and goerli ENS subgraphs differ slightly, meaning that sometimes things will break on mainnet for the moment. a fix for this issue at the moment could be to disable subgraph communication entirely by passing a null value for graphURI in the ENS class constructor.

e.g.

const ENSInstance = new ENS({ graphURI: null })
Magofoco commented 1 year ago

I tried this solution but the error persists

Magofoco commented 1 year ago

For now, I am solving it with a try and catch. I created a standalone function called getMaybeOwner that calls .getOwner(). If the ens name exists, the function returns either an object or null (as expected). Else, it enters the catch block and returns null

const getMaybeOwner = async (ensName: string, ENSInstance: ENS) => {
  try {
    const maybeEnsRegisteredObject = await ENSInstance.getOwner(
      ensName
    );
    return maybeEnsRegisteredObject;
  } catch (e) {
    return null;
  }
};

export default getMaybeOwner;

Now, instead of calling await ENSInstance.getOwner(ensName) I call await getMaybeOwner(ensName, ENSInstance);

@TateB Do you think it is worth it to add this function in the methods of the ENSInstance or adding it as a function into the library?

Magofoco commented 1 year ago

@TateB If you want, I can implement the getMaybeOwner function

TateB commented 1 year ago

a new subgraph is syncing now, will take a few days but once it's done this should work

TateB commented 1 year ago

@Magofoco hey, this should work now

Magofoco commented 1 year ago

@TateB Hello! I tried it with a ens domain that does not exist and returns correctly undefined. HOWEVER, it returns undefined also for a ens domain that exists (ex. vitalik.eth)

I am using: "@ensdomains/ensjs": "^3.0.0-alpha.42"

TateB commented 1 year ago

@Magofoco there were some issues with running the package through nodejs, should be fixed now (version 3.0.0-alpha.43)

Magofoco commented 1 year ago

@TateB I am experiencing the same error. It returns undefined also for a ens domain that exists (ex. vitalik.eth)

TateB commented 1 year ago

can you provide a gist of your code or something? i can't replicate the issue

Magofoco commented 1 year ago

@TateB Sure! Here it is

import { ethers } from "ethers";
import * as dotenv from "dotenv";
import { ENS } from "@ensdomains/ensjs";

dotenv.config();

const { INFURA_API_KEY } = process.env;
const INFURA_NODE_MAINNET = `https://mainnet.infura.io/v3/${INFURA_API_KEY}`;

const provider = new ethers.providers.JsonRpcProvider(INFURA_NODE_MAINNET);
const ENSInstance = new ENS();

const POLLING_INTERVAL_MILLISECONDS = 20000;

const main = async () => {
  await ENSInstance.setProvider(provider);

  const polledFunction = async () => {

    const existingEns = await ENSInstance.getOwner("vitalik.eth");
    const nonExistingEns = await ENSInstance.getOwner("thisisnotregisteredens.eth");
    console.log(existingEns); <--- RETURNS WRONGLY UNDEFINED
    console.log(nonExistingEns); <-- RETURNS UNDEFINED
  };

  setInterval(polledFunction, POLLING_INTERVAL_MILLISECONDS);
};

main()
TateB commented 1 year ago

still can't replicate the issue, do you think you could create a barebones repo for reproducing it?

Magofoco commented 1 year ago

@TateB Take a look here: https://github.com/Magofoco/ensIssueTest

TateB commented 1 year ago

a few minor things that got it working for me:

Magofoco commented 1 year ago

Thanks @TateB , with your changes the version 3.0.0-alpha.43 work as expected. Thank you!

(Just for your knowledge, 3.0.0-alpha.39 was working without needing to change the package.json and tsconfig.json)