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

Use reverse resolution to fetch main ENS name from address #190

Merged
merged 3 commits into from
Feb 1, 2023

Conversation

santteegt
Copy link
Contributor

@santteegt santteegt commented Jan 31, 2023

GitHub Issue

Fixes #186

Changes

Apparently, it's no that easy to identify the main ENS name from an address using the subgraph. Going through the ENS docs, I find out how it uses Reverse Resolution using a special purpose domain to set the primary ENS name a.k.a main reverse record. The recommended approach is to use an etherjs provider and call lookupAddress, otherwise seems that we need to manually concatenate .${address}.addr.reverse, namehash it and look for the resolver in the subgraph.

I used the recommended approach to fetch the main ENS record. If not found, it fallbacks to query the subgraph for the first non-expired ens domain registered by an account address.

Packages Added

None

Checks

Before making your PR, please check the following:

  • Critical lint errors are resolved
  • App runs locally
  • App builds locally (run the build command for any impacted package and check for any errors before the PR)

@santteegt santteegt requested a review from skuhlmann January 31, 2023 03:41
@santteegt
Copy link
Contributor Author

Tasks done: explore subgraph queries that allow us to identify the main ENS record from an address. Ended up implementing reverse resolution using etherjs
Time spent: 3 hours

@santteegt santteegt force-pushed the fix/ens-reverse-solver branch from 322870a to 1a5ab31 Compare January 31, 2023 04:17
Copy link
Contributor

@skuhlmann skuhlmann left a comment

Choose a reason for hiding this comment

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

@santteegt Is there no way to get this from the subgraph?

I was really hoping to avoid dependency on the rpc provider. You're importing HAUS_RPC for that - which is fine for mos of our apps - but what if someone is using this package outside of daohaus connect they will need a way to pass the provider/rivet key.

@skuhlmann
Copy link
Contributor

skuhlmann commented Jan 31, 2023

If there is no way to use the subgraph, maybe this function should take a provider url as an argument. our apps can pass it from HAUS_RPC. But then others can use it and pass their own.

@santteegt
Copy link
Contributor Author

After digging a little bit more into how to get the same data from the subgraph, I found this in the docs:

...
Listing all primary names
...
...
TheGraph = subgraph currently does not index primary name info ([github issue](https://github.com/ensdomains/ens-subgraph/issues/25))
...
...

Going through the issues I found the following here:

The new reverse registrar, currently deployed on testnets,
will support emitting the appropriate events - so we will need to 
write support for that rather than doing transaction tracing like this.

It looks like this is still WIP.

On the other hand, I also found a PR that Bryan pushed for DAOHaus V2 however it also relies on an RPC endpoint.

Until there's primary ens domain data indexed by the subgraph, I think it's better to use etherjs library instead of our own implementation so I'm going to go with the second approach to update my PR: passing the RPC to the profile function. @skuhlmann now my question is, which one is better to pass over: the RPC URI or the etherjs provider instance?

@skuhlmann
Copy link
Contributor

skuhlmann commented Jan 31, 2023

@santteegt

Cool. I'd say pass the provider url and set up ethers in the function.

@santteegt santteegt requested a review from skuhlmann February 1, 2023 03:04
@skuhlmann skuhlmann merged commit 1913d95 into develop Feb 1, 2023
@scottrepreneur scottrepreneur deleted the fix/ens-reverse-solver branch February 9, 2023 13:50
@o-az
Copy link

o-az commented May 18, 2023

You can call the reverse resolution directly through a fetch call w/o needing ethers

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.

Get correct ENS name in profile fetch
3 participants