Consensys / starknet-snap

The MetaMask Snap for Starknet
https://snaps.consensys.net/starknet
Apache License 2.0
69 stars 24 forks source link

feat: allow users to transfer money to a .stark name #185

Open irisdv opened 7 months ago

irisdv commented 7 months ago

This feature will add support to sending money to a .stark domain within the app. Here is what it could look like:

Capture d’écran 2023-11-30 à 15 25 16

Implementation proposal

To retrieve the address from the user stark name, we can call the function domain_to_address from the starknet ID naming contract. This function takes in argument an encoded domain as a felt array and returns the a Starknet address. You can find more information on our encoding and decoding algorithm here.

This feature will follow a similar implementation than #184.

In starknet-snap :

In wallet-ui :

If a significant number of contributors agree with our approach I'll start working on Pull Request.

Please feel free to answer on this issue and tell me what you think !

cpp-phoenix commented 6 months ago

Hey @irisdv, I can work on it

stanleyyconsensys commented 4 months ago

if the name not found, we should block it from the UI (the snap should already implemented a logic to block the txn)

irisdv commented 3 months ago

if the name not found, we should block it from the UI (the snap should already implemented a logic to block the txn)

Ok will do ! I'd like to finish this issue, but I would need version of starknet.js above 6.3.0 so the getAddressFromStarkname function works with the latest version of our contracts. I saw you have a draft PR ongoing to update starknet.js, can I base mine on this PR, or is it subject to change ?

stanleyyconsensys commented 3 months ago

The PR that upgrading the starknet.js to latest version, which is because of supporting TXN v3, however, we discover some bugs in starknet.js, and we resolved by local patching

the PR is there https://github.com/Consensys/starknet-snap/issues/185

Yes, you can base on this PR, but we may not merge that soon, we have to merge the cairo contract upgrade first, i would suggest you to wait until the cairo contract upgrade merged, as it has some breaking change on UI too

irisdv commented 1 month ago

I saw that #219 upgrading starknet.js was merged. Was the cairo contract upgrade PR also merged ?

stanleyyconsensys commented 1 month ago

No, the cairo contract PR has to re do some work due to library update

khanti42 commented 1 week ago

Hi @irisdv , the Cairo contract PR has been merged to main. What is the status with this ?

irisdv commented 1 week ago

Ah great, then I'll update the PR I prepared and I'll make a PR this week on the repo if that's ok with you ?

khanti42 commented 1 week ago

Sounds great!