blockful-io / external-resolver-dapp

https://external-resolver-dapp.vercel.app
2 stars 2 forks source link

fix: show domain without valid resolver #147

Closed pikonha closed 2 months ago

pikonha commented 3 months ago

Bug Report

Current behavior

Domains that don't have a valid resolver cannot be seen because an error is displayed in the Ui.

Expected behavior

Domains that don't have a valid resolver should display at least the ownership data of the domain.

Steps to reproduce

Look how the same domain (which does not have a valid resolver) is displayed differently in ENS dApp and in our ENS dApp: https://app.ens.domains/alek.eth - showing normally https://dev-nameful.vercel.app/domains/alek.eth - Returning error

Image

  1. Change the resolver address to an invalid resolver
  2. Check the domain page

Related code

ensService.ts

Definition of Done

Display domain ownership data when a domain has an invalid ENS Resolver address.

FrancoAguzzi commented 3 months ago

Suggested skeleton for a validation of an address as an ENS Resolver:

export const isAddressAnENSResolver = async (address: `0x${string}`): Promise<boolean> => {
  let resolverContract;
  try {
     resolverContract = await getContract(address, DomainResolverABI);
  } catch {
    throw new Error("Could not instantiate Resolver Contract, thus, the address is not a resolver")
  }

  try {
    resolverContract.readContract({
      functionName: "addr",
      args: [...]
    });
  } catch (error) {
    if (error === "functionNotFound") {
      return false
    } else {
      return null
    }
  }

  return true;
}
FrancoAguzzi commented 3 months ago

@eduramme suggested we look for ENS libraries ways of validating this, which is prioritary when compared to creating our custom validation.

FrancoAguzzi commented 3 months ago

Default ENS resolver specs can be found in here: https://docs.ens.domains/resolvers/writing

The EIP that defined the Default ENS Resolver was this one, any functions present in here should be implemented in every ENS Resolver, meaning that these are reliable validators of an ENS Resolver

eduramme commented 2 months ago

@FrancoAguzzi We only use the Resolver address if it is metadata compatible. That being said, I don't think it makes sense to validate whether the resolver is valid or not.

Our current flow is Load domain page -> check if the resolver is metadata compatible -> if it isn't, get information through a subgraph batch request

Now we're adding a third layer where, if the subgraph batch request fails, we get the basic information one by one. And only if that fails we set the Error page.

pikonha commented 2 months ago
  1. When the resolver is invalid the edit record isn't capable of changing the records which is expected. However, the error is shown below the modal Image

  2. The loading screen of a domain with an invalid resolver takes about 5s to load, couldn't that be reduced somehow?

  3. When changing the resolver to an invalid one on the ENS dapp it alerts you that the resolver isn't compatible with the Resolver interface. might not be related to this task, but it is surely a good experience for the user that we should consider adding in the future. this could be done with the supportsInterface (EIP-165). Image

eduramme commented 2 months ago

@pikonha

  1. This is happening with all modals, so I don't believe it's specific to this issue.
  2. It will require some investigation time to figure out.
  3. That's a great idea!
pikonha commented 2 months ago

LGTM then