feat: allow users to transfer money to a .stark name #437
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
irisdv
requested review from
Akaryatrh and
stanleyyconsensys
and removed request for
a team
stanleyyconsensys
requested changes
Nov 27, 2024
There was a problem hiding this comment.
Thank you very much for the work @irisdv
We did some revamp on the SNAP, and some of the RPC is consist by new structure
for this API, you can refer to the example of ./rpcs/watch-asset.ts
the major changes are :
- we use OOP and inherit from a RpcController
- we use superstruct to validate the input
- we have NetworkStateManager to get network by chain id
- we dont do logging of the response, the base RpcController will do it
- we use jest, but not mocha
Thanks @stanleyyconsensys , I updated my PR |
There was a problem hiding this comment.
Hi @irisdv , overall look good on SNAP side, but just some questions that i wonder on the UI side
packages/wallet-ui/src/components/ui/molecule/AddressInput/AddressInput.view.tsx
Outdated
Show resolved
Hide resolved
packages/wallet-ui/src/components/ui/organism/Header/SendModal/SendModal.view.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Hi @irisdv , the SNAP side look good to me, thanks for the change
but the UI side may have some issue
packages/wallet-ui/src/components/ui/organism/Header/SendModal/SendModal.view.tsx
Outdated
Show resolved
Hide resolved
packages/wallet-ui/src/components/ui/organism/Header/SendModal/SendModal.view.tsx
Outdated
Show resolved
Hide resolved
packages/wallet-ui/src/components/ui/organism/Header/SendModal/SendModal.view.tsx
Outdated
Show resolved
Hide resolved
packages/wallet-ui/src/components/ui/organism/Header/SendModal/SendModal.view.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Hi @irisdv thank you very much for your help
there is not bug at all, just some nit comment to simplify the code and enhance the performance
packages/wallet-ui/src/components/ui/organism/Header/SendModal/SendModal.view.tsx
Outdated
Show resolved
Hide resolved
packages/wallet-ui/src/components/ui/organism/Header/SendModal/SendModal.view.tsx
Show resolved
Hide resolved
packages/wallet-ui/src/components/ui/organism/Header/SendModal/SendModal.view.tsx
Outdated
Show resolved
Hide resolved
packages/wallet-ui/src/components/ui/molecule/AddressInput/AddressInput.view.tsx
Outdated
Show resolved
Hide resolved
packages/wallet-ui/src/components/ui/molecule/AddressInput/AddressInput.view.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
hi @irisdv , would you mind to do a rebase?
no worry let me do it for you
stanleyyconsensys
approved these changes
Dec 4, 2024
stanleyyconsensys
changed the base branch from
main
to
feat/transfer_to_stark_name
stanleyyconsensys
merged commit b3c39d0
into
Consensys:feat/transfer_to_stark_name
9 of 12 checks passed
github-merge-queue bot
pushed a commit
that referenced
this pull request
Dec 4, 2024
* feat: transfer to a stark name * fix: lint errors * test: update getAddrFromStarkName network * fix: update to new RPC structure * chore: lint * chore: lint * fix: racing condition in SendModal * fix: move validate logic out of addressInput & update requests made to snap * fix: simplify name check in frontend * fix: resolved addr is validated by superstruct * chore: add snap permission boundary and update UI text * chore: update RpcController import location --------- Co-authored-by: Iris <[email protected]>
Merged
4 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds support to sending money to a .stark domain.
Close #185