feat: add name service #1
Conversation
src/actions/nameService.ts
Outdated
| export async function getHashedName(name: string): Promise<Buffer> { | ||
| const input = HASH_PREFIX + name; | ||
| return Buffer.from(Hex.stringify(sha256(input)), 'hex'); | ||
| } |
There was a problem hiding this comment.
i wasn't able to get the createHash method using crypto to work. Got a Module "crypto" has been externalized for browser compatibility. Cannot access "crypto.createHash" in client code
arein
force-pushed
the
feat/name-service
branch
from
a470b12
to
1a01311
Compare
There was a problem hiding this comment.
Using our own fetch method to target clusters gives us more fine-grain control over error handling, message emitting, etc and makes it more future proof.
For a library that's on lower level than typical endd-user lib. I know this is controversial, but I believe the pros outweigh the cons.
It's awesome you got this working, I'm going to approve for now, but I might go in and update the implementation to use the cluster fetch method
package.json
Outdated
| "@types/bn.js": "^5.1.1", | ||
| "@types/crypto-js": "^4.1.1", |
There was a problem hiding this comment.
these need to be devDependencies
| return otherDomains.length > 0 ? otherDomains[0] : null; | ||
| } | ||
|
|
||
| async function getSolDomainsFromPublicKey(connection: Connection, wallet: PublicKey):Promise<string[]>{ |
There was a problem hiding this comment.
We don't to trouble users with instantiating a PublicKey. Function APIs should be similar to Wagmi, wehre you just pass base-58 encoded pubkey.
There was a problem hiding this comment.
this method is not exposed to users.
| static async retrieve(connection: Connection, key: PublicKey) { | ||
| const accountInfo = await connection.getAccountInfo(key); | ||
| if (!accountInfo || !accountInfo.data) { | ||
| throw new Error("Favourite domain not found"); | ||
| } | ||
| return this.deserialize(accountInfo.data); | ||
| } |
There was a problem hiding this comment.
most these methods can be replaced with just calls to the base connector's requestCluster
There was a problem hiding this comment.
There's quite a bit of serde logic going on in Connection. The base connector doesn't do that and it's quite a bit of copy/pasta to get that working. Happy for you to prove me wrong. But we already have the Connection in the footprint so might as well.
There was a problem hiding this comment.
I actually spent quite a bit of time trying to get this to work. I highly suggest you timebox that effort if you go down that rabbithole :)
Adding Solana Name Service (SNS) support.
Code is copied from
@bonfida/splwith two adjustments:Notes
For @devceline - I'm a bit confused why we have our own cluster concept which seems like a reimplementation of a lot of things that are already implemented in
Connection. I am not using it here - I am using aConnectionobject.Following your recommendation and our tenet to keep the footprint low I copied a lot of the logic from SPL over here in accordance with their license.
Testing
Tested locally using various Bonfida domains.