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

[MyMeta] ENS usernames #1176

Closed
peth-yursick opened this issue Mar 1, 2022 · 16 comments
Closed

[MyMeta] ENS usernames #1176

peth-yursick opened this issue Mar 1, 2022 · 16 comments
Labels
ceramic grant Issues tracking MG's 2023 Ceramic Grant dework feature request Feature Requests

Comments

@peth-yursick
Copy link
Member

peth-yursick commented Mar 1, 2022

What would you like to be added?

The ability for people to set their ENS address as their username.

Why is this needed?

Given its popularity in the space, its kind of embarrassing not to allow people to set their username as so.
The problem seems to be at the ceramic level? You're simply not allows to have . in your username? @dys

image

@alalonde
Copy link
Contributor

What if we just added a "Use ENS" checkbox as an alternative to username? Since we already have their ETH address at that point. I'm not sure that we want to store ENS names in our DB (I believe they can expire, be re-assigned, etc).

We could support ENS out of the box already by just looking up ENS names from players that have no "username" (name in the profile table I believe)

@dysbulic
Copy link
Member

I agree with not storing the names. Not only might they be transfered, but we'd also have to worry about people registering someone else's name with ill intent.

I like the idea of disallowing .eth as the terminal part of a username, resolving the ENS name of any profiles requested with a .eth extension, then serving up the profile of the associated address.

It's very similar to the situation with addresses. Usernames of the same format as an address (/^0x[a-f\d]{40}$/i) aren't allowed, and if a profile is requested using one, it is looked up using a method specifically for addresses.

I don't think it's necessary to ask them to do it.

@alalonde alalonde added the ceramic grant Issues tracking MG's 2023 Ceramic Grant label Jan 8, 2023
@Seroxdesign
Copy link
Contributor

Hey, I found this task on Dework, I've worked on adding ENS support for multiple DAOs and I can see this task through if it's still available. @alalonde

@alalonde
Copy link
Contributor

@dysbulic I'd like to make this task's requirements much more explicit so that new devs can start working on it. How does this sound?

  • Update the backend regex filter to allow .eth as a suffix, but otherwise still disallow . in a username
  • When a user types in a .eth username, query ENS from the frontend to see if the username is valid?

Outstanding questions:

  • I assume we want the user to go through some validation process as part of the ENS query to ensure that the username is in fact theirs.
  • We would still need to do some verification on the backend, since the hasura graphQL endpoint allows a user to update their own username. So do we only allow a user to use the ENS name associated with the eth address linked to their profile? And if that's the case, why not just pull in a linked ENS name automatically when they set their ETH address?

@Seroxdesign
Copy link
Contributor

Milestones:

  • Allow user to update their username with an ENS by updating RegEx
  • Resolve ENS from connected user wallet and validate input ENS is owned by user
  • If user's connected wallet does not resolve an ENS disallow them from saving/updating username's with dedected . chars
  • Either trigger a cron job in Hasura to ensure ENS's are in sync, or preferably create a webhook to handle this.

Implementation plan:

  • I will begin by adding the ENS queries and data to the Web3Context file in the src/packages/web/contexts/web3context to make available for later validation
  • I will update the Regex in the input field for updating usernames to allow for .ens/.eth patterns if user's address resolves an ENS
  • I will block user's from submit a username change request with any . chars if their address does not resolve an ENS
  • I will research whether to use a webhook or Cron Job to keep ENS addresses in SYNC with usernames
  • I will choose an implementation, discuss with the Meta team and then continue from there.

@alalonde This is my plan, I will do remaining research of implementation tomorrow then begin the set up on Monday.

@dysbulic
Copy link
Member

@alalonde, @Seroxdesign, rather than having people set their .eth. address as their username, I like the idea of disallowing .s is usernames, and if there is one then we attempt to resolve that name as an ENS name.

The username configured for an account would be something different, or nothing at all if they only want to to use ENS.

Simply resolving ENS names adds a small amount of time for the resolution step, but does away with the need for any verification of ownership. If we find resolution time to be an issue, we could keep a cache of ENS name resolutions.

Currently, it is possible to set any domain name (that has DNSSEC) to be a valid ENS name, not just .eth addresses.

Also, rather than assume the user will have their wallet connected or that it will be on mainnet, I think we should simply use a JSONRPCProvider connected to a mainnet RPC endpoint to do the resolution and not rely on the user's wallet.

@alalonde
Copy link
Contributor

but does away with the need for any verification of ownership

We need to verify that they actually have the ENS name, no? So they can't just impersonate e.g. vitalik.eth

@dysbulic
Copy link
Member

dysbulic commented Jan 23, 2023

but does away with the need for any verification of ownership

We need to verify that they actually have the ENS name, no? So they can't just impersonate e.g. vitalik.eth

If a user visits /player/vitalik.eth, it would load the profile for the associated address, i.e. Vitalik's.

@alalonde
Copy link
Contributor

alalonde commented Jan 23, 2023

OK, so you're suggesting just looking up the ENS name associated with the player's eth address. Do we make username nullable then? And if set it just takes precendence over any resolved ENS name?

This seems preferable as that takes out all the effort of syncing our usernames with ENS

@dysbulic
Copy link
Member

dysbulic commented Jan 23, 2023

OK, so you're suggesting just looking up the ENS name associated with the player's eth address.

Yeah, if there's a . in it, try to resolve it and load the associated profile.

Do we make username nullable then?

Yeah, I think it should be possible to remove it.

And if set it just takes precedence over any resolved ENS name?

It would work in addition to any ENS names. There would be three ways to access a user's profile: address, username, & ENS name. Since the username can't contain .s, there's no possibility of a collision.

When dereferencing the /player/<name> route, if name matches /^0x[0-9a-f]{40}$/i then load the profile for the address. If name matches /\./ then look name up in ENS. Otherwise, search for name as a username.

@alalonde
Copy link
Contributor

alalonde commented Jan 23, 2023

Yup, got it. Thanks for clarifying. @Seroxdesign Does this make sense? It will be a vastly simpler implementation this way.

There will be minimal backend work then, perhaps just making username nullable in Hasura? Ah, and also restricting . (frontend and backend validation). In Hasura you should be able to do a postgres constraint check

@Seroxdesign
Copy link
Contributor

@alalonde all clear, this makes things a lot easier. Will edit the tasks to follow these recommendations

@peth-yursick
Copy link
Member Author

Don't want to add any extra work but wanted to throw in the idea that it would be cool if ENS was auto-detected & filled in like guild memberships, on the "choose your name" step of the profile creation flow:
image
Would also be fitting since our "name" only allows lowercase characters anyway 😂

@dysbulic
Copy link
Member

Don't want to add any extra work but wanted to throw in the idea that it would be cool if ENS was auto-detected & filled in like guild memberships, on the "choose your name" step of the profile creation flow

@peth-yursick, they're not specifying their ENS name as their username. It's simply that if an ENS name is specified in /player/<name>, it will be resolved and the appropriate profile displayed.

@Seroxdesign
Copy link
Contributor

Ready for review

@alalonde
Copy link
Contributor

alalonde commented Feb 7, 2023

Ah, and also restricting . (frontend and backend validation). In Hasura you should be able to do a postgres constraint check

I don't see this in the PR

Also, we should include a data migration in this PR that removes any current usernames with a dot. In fact, you'll have to do this before adding the backend constraint on that column

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ceramic grant Issues tracking MG's 2023 Ceramic Grant dework feature request Feature Requests
Projects
None yet
Development

No branches or pull requests

4 participants