-
Notifications
You must be signed in to change notification settings - Fork 89
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
implement WalletConnect (v1) as a dapp option into hdwallet #499
Comments
NOTE: Web integration is out of scope for this bounty and will come after this is completed :)
. |
Issue Status: 1. Open 2. Started 3. Submitted 4. Done This issue now has a funding of 5500.0 FOX (1717.99 USD @ $0.31/FOX) attached to it as part of the ShapeShift fund.
|
Issue Status: 1. Open 2. Started 3. Submitted 4. Done Work has been started. These users each claimed they can complete the work by 264 years, 7 months from now. 1) bielwenass has applied to start work (Funders only: approve worker | reject worker). Excited about the possibility to work on another Shapeshift bounty! Have already implemented walletConnect in another project. Will research the requirements here I have worked with WalletConnect before, and would follow the specification exactly. I work in a testing oriented fashion and in a timely manner. As senior iOS developer, I already implemented this function for my project. Learn more on the Gitcoin Issue Details page. |
I think we need to work more on the specification of this issue before starting work.
Overall, I'm not happy about doing more development on hdwallet's existing interface. We need to talk about hdwallet++ before creating yet another mostly cut-n-paste boilerplate wallet package. Each one of these we do makes doing maintenance harder. I don't think we can afford to start work on new features in this package until addressing things like #454, #456, #457, #458, #459, #462, and #466. |
To clarify this bounty is only for HDwallet. And for v1 version of wallet-connect. Ive added v1 to title, and specified the exact module to add. I confirmed All the code examples posted are for v1
This was my concern when we added MetaMask. However it does seem like HDwallet has matured into more of a wallet-wrapper more than a wallet library. I do feel that this pattern is already well established, and the MetaMask module is very close in architecture to this bounty and should give clarity to the path forward.
I think that should be its own issue, not sure its relevance in this bounty. |
In WC, the "dapp" has to start the connection procedure by opening the connection to the WC bridge server and delivering a WC url to the client. This is much more complex than in MM, where hdwallet-metamask makes a single call to the provider and can just throws if the user rejects. We don't have a good way to represent a wallet in an intermediate state of connection like this in our current interfaces, hence my concern about needing to work on the adapter/initialization paradigms upfront. Code we accept to hdwallet must be of high quality. Allowing submissions for this bounty to break eslint rules would mean shifting the burden of fixing it to meet those standards later to NeOMakinG or myself. It's gonna be a PITA to get #485 in as-is; please don't make my life harder. |
WC accepts supports multiple wallets ( a huge list actually) and each wallet supports multiple/different assets. WC api's do not provide any API for supported assets by each wallet. How do envision managing support for assets? |
Per @0xdef1cafe - we are moving forward with this bounty. Will select a bounty hunter today. |
👋 I'm starting this and have a couple of questions/notes to make, some of these things are related to my lack of knowledge of this repo (first time working with it):
I'm writing here just to keep things in a place, I'll ping you in #engineering-public if there's something more urgent. |
|
Brilliant, I see the eslint branch got merged so I just rebased master in mine. Thank you! |
@EdgarBarrantes - can you please provide an update on your progress and expected completion of this bounty? Thank you! |
@0xean of course, I was set to finish this in the next couple of days (15 days), I haven't really been able to commit as much time to this ticket as I'd like, I have a shell for this package and it's tests from about a week ago. I'd like to pospone my expected delivery time for about 10 days. I'll give more details on chat. |
@michals92 - I just approved you on gitcoin, please let me know a rough timeline for completion on this bounty. TY! |
Hi, thanks, I have to go through all the info provided, but I would like to finish it in 14 days. |
@michals92 - great, please let us know if you have questions. Discord is always a great place (#engineer-public). |
@michals92 - can we please get an update of progress and an ETA for this bounty? |
Hi, I created package for wallet connect, studied documentation and also went through metamask and xdefi implementation. I want to get it done this weekend. |
I added UI for WalletConnect, dialog pops up in my branch, now I need to finish implementation and write test for it. I want to finish it ASAP. |
@michals92 can you please open a draft PR for the current WIP, even if it's unstable? we need this as a top priority and would like to see the progress so far. |
@0xdef1cafe posting to dework, how many hours should the estimate be? |
@michals92 you initially advised this would be done over the weekend. we need to see tangible progress here, can you please post a draft PR within 24 hours, as we need to consider bringing this bounty in house. |
Ok, sorry that I didnt deliver it on time |
Heya @GMSteuart - are you taking on this bounty? if so, i'll assign this to you on Dework if possible. coz I can see contributors applying to this |
@GMSteuart can you please advise if you are assigned this bounty? Thank you |
@MBMaria I am! Thanks for checking. |
Issue Status: 1. Open 2. Started 3. Submitted 4. Done The funding of 5500.0 FOX (776.85 USD @ $0.14/FOX) attached to this issue has been cancelled by the bounty submitter
|
reopening - mistakenly closed as walletconnect as a dapp, vs walletconnect as a wallet |
as discussed with @pastaghost, southbears is working on a different walletconnect feature, where shapeshift is the wallet, rather than the dapp. this is implemented and done. |
Overview
Overview
https://walletconnect.com/
Wallet connect is a protocol allowing wallets to connect with Dapps. ShapeShift as a Dapp would like to allow any v1 integrated wallet in the wallet connect ecosystem to connect to app.shapeshift.com as a wallet, and utilize all supported features. These features are specific to ETH, and ETH compatible EVM’s.
HDwallet is a collection of supported wallet modules that implement a common base class. This bounty is to create a module that supports wallet-connect as a wallet option and conform to this base class of HDwallet.
Wallet connect provides the following example dapp
Example Dapp: https://github.com/WalletConnect/walletconnect-example-dapp
Example MetaMask addition PR
Example: https://github.com/shapeshift/hdwallet/pull/359/files
WalletConnect documentation:
Docs: https://docs.walletconnect.com/
References and additional details
References
I am referencing the MetaMask integration and code snippets here to help explain the process of adding a new wallet to this HDwallet repo. As it is similar in scope/function.
See it complete:
HDwallet sandbox: https://hdwallet-shapeshift.vercel.app/
Notice “Pair Wallet”
Wallet Connect will have a more interactive pairing process.
Summary of pairing:
For testing manually I would recommend metamask mobile (supports wallet-connect) or trustwallet (mobile).
There will be a walletconnect modal displayed to the user to require the user to scan a QR code in their mobile wallet.
(there are also desktop wallet intents that can launch desktop apps automatically.) as well as intents to launch mobile wallets if the user is viewing the dapp on their mobile wallet.
Example dapp:https://example.walletconnect.org/
After pairing, the sandbox should implement all supported functions, and display errors on those without support.
Note: asset support is flagged in the wallet class
https://github.com/shapeshift/hdwallet/blob/master/packages/hdwallet-metamask/src/metamask.ts#L23
Metamask package dir
https://github.com/shapeshift/hdwallet/tree/master/packages/hdwallet-metamask
Metamask Tests
https://github.com/shapeshift/hdwallet/blob/master/integration/src/wallets/metamask.ts
Acceptance Criteria
A.C.
Need By Date
no
Screenshots/Mockups
N/A
Ownership
Estimated effort
2-3 days
Sponsor / Stakeholder
@BitHighlander
Bounty Hunters
The text was updated successfully, but these errors were encountered: