Skip to content

Conversation

@ghost
Copy link

@ghost ghost commented Dec 20, 2022

Overview

Basic attempt at rendering a .stars domain in place of a users "wallet username" if they have one.

Notes

  • IMO it's pretty uncommon that wallets actually expose a "username" to dapps. MetaMask, WalletConnect, etc all do not support this, seems to be a Keplr implementation detail
  • This approach is nice because consumers don't have to change anything to start having this data come through. Another way would be introducing another field on the cosmos-kit API like stargazeName so consumers can do stargazeName ?? username
  • Next step would be to bubble up the image associated with the name NFT, if the user has set one

Copy link

@shanev shanev left a comment

Choose a reason for hiding this comment

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

LGTM LFG

@pyramation
Copy link
Collaborator

pyramation commented Dec 20, 2022

hey @0xarbi appreciate this!

I notice this is in the core? I think it's a better practice to make this an option and put the code in a separate package similar to how we structure all the wallets:

$ tree -L 1 packages
packages
├── core
├── cosmostation
├── docs
├── example
├── keplr
├── leap
├── omni
├── react
├── trust
└── vectis

I think the proper way is similar to how we managed wallets, and create an adapter. Also some folks may not use name services in their apps and this type of feature makes sense to make as a plugin.

Perhaps we can call it dns or name-service or names?

If you put the name service code in a a package, our team can help make an adapter for it.

@pyramation pyramation requested a review from liujun93 December 20, 2022 16:27
@pyramation
Copy link
Collaborator

pyramation commented Dec 20, 2022

upon second review, maybe we allow developers to pass in nameResolvers: () -> Promise<string> as an async function — so the adapter would be overkill. This can be quite simple and we have an optional parameter for the provider.

then feel free to add the package in packages/example for testing, if you need me to publish the names package first, I can help!

@ghost
Copy link
Author

ghost commented Dec 21, 2022

@pyramation I've pushed up some changes

  • adding a new @cosmos-kit/name-resolvers package
  • letting people choose a name resolver to inject into the provider (most people will use the defaultNameResolver)
  • updating the example with the defaultNameResolver

Let me know what you think!

@pyramation
Copy link
Collaborator

awesome! thanks @0xarbi :)

This looks solid — in general the idea is to keep core pure and everything is extension/plugin with packages. I think what you did looks great, but will need June to do a review. We're just on the brink of finish our sprint for wallet connect v2 — will have team review and merge this ASAP!

@pyramation pyramation changed the base branch from main to feat/name-resolvers January 3, 2023 04:24
@pyramation pyramation merged commit ff0ceef into hyperweb-io:feat/name-resolvers Jan 3, 2023
@ghost ghost deleted the arb/stargaze-names branch January 3, 2023 04:42
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