Skip to content
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

Support multiple Web3 wallets #5978

Merged
merged 17 commits into from
Apr 15, 2020
Merged

Conversation

KiChjang
Copy link
Contributor

@KiChjang KiChjang commented Feb 12, 2020

  • Vendor Web3Connect, Fortmatic and Authereum into frontend code
  • Ensure Send Tip user flow works
    • Refactor web3.eth.coinbase and web3.eth.accounts to use their
      corresponding async API counterparts
Description

Allow user to choose the web3 wallet to connect to Gitcoin with.

Refers/Fixes

Fixes #5954, fixes #5909.

Testing

Manual smoke testing has been done on the following flows:

  1. Send Tip
  2. Receive Tip
  3. Create Bounty
  4. Payout Bounty
  5. Submit Bounty
  6. Create Grant
  7. Fund Grant
  8. Send Kudos
  9. Receive Kudos

@KiChjang
Copy link
Contributor Author

I've added the ability to select your wallets via the network button:

image

@codecov
Copy link

codecov bot commented Feb 16, 2020

Codecov Report

Merging #5978 into master will increase coverage by 8.83%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5978      +/-   ##
==========================================
+ Coverage   18.38%   27.22%   +8.83%     
==========================================
  Files         278      290      +12     
  Lines       26385    26542     +157     
  Branches     3918     3930      +12     
==========================================
+ Hits         4852     7227    +2375     
+ Misses      21521    19049    -2472     
- Partials       12      266     +254     
Impacted Files Coverage Δ
app/app/context.py 0.00% <ø> (ø)
app/app/settings.py 81.65% <100.00%> (+0.42%) ⬆️
app/dashboard/admin.py 60.77% <0.00%> (-0.77%) ⬇️
.../search/management/commands/update_search_index.py 0.00% <0.00%> (ø)
app/dashboard/templatetags/trim.py 85.71% <0.00%> (ø)
app/legacy/urls.py 100.00% <0.00%> (ø)
app/retail/templatetags/is_in_list.py 66.66% <0.00%> (ø)
app/dashboard/templatetags/add_url_schema.py 71.42% <0.00%> (ø)
app/kudos/templatetags/kudos_extras.py 70.00% <0.00%> (ø)
app/retail/templatetags/is_in_list_type_iexact.py 66.66% <0.00%> (ø)
... and 86 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 04057aa...f878e00. Read the comment docs.

@KiChjang KiChjang force-pushed the multiwallet branch 3 times, most recently from 91a3401 to a883fe4 Compare February 16, 2020 22:37
@KiChjang KiChjang marked this pull request as ready for review February 16, 2020 22:37
@KiChjang KiChjang force-pushed the multiwallet branch 2 times, most recently from 69e62c2 to 9da3aa2 Compare February 19, 2020 05:12
@danlipert
Copy link
Contributor

@KiChjang excited for this! Let us know when its ready for review

@faridrached
Copy link

@KiChjang btw, in the past we made a Gitcoin themed modal; we can deploy that if you send me the test and live api keys which you can find in the Fortmatic dashboard. Feel free to dm me on Telegram @faridrached or email me on [email protected]. Thanks!

@KiChjang
Copy link
Contributor Author

@danlipert Sorry I wasn't clear; this is ready for review.

@faridrached I am currently using my own personal test and live API keys on Fortmatic, and I'm suspecting that I should use the one for gitcoin instead? If so, then gitcoin would have to provide one.

@faridrached
Copy link

faridrached commented Feb 25, 2020

@faridrached I am currently using my own personal test and live API keys on Fortmatic, and I'm suspecting that I should use the one for gitcoin instead? If so, then gitcoin would have to provide one.

Yes, its wiser if someone from Gitcoin creates an account and shares the key indeed. Perhaps @danlipert or @thelostone-mc?

@owocki
Copy link
Contributor

owocki commented Mar 3, 2020

@KiChjang ill create my own key for the gitcoin production deployment. just make it configurable in the .env file (thats where we store secretse on the gitcoin app)

Manual smoke testing has been done on the following flows:

its such great to see that you've done this testing.. yay! did u test in chrome/metamask? brave? safari?

@KiChjang
Copy link
Contributor Author

KiChjang commented Mar 3, 2020

@owocki I actually did my testing on Firefox, because that's the only browser that didn't have any issues with the Authereum integration. The specific issued I encountered while testing Authereum on Chrome is that window.crypto.subtle is disabled on non-HTTPS pages except on localhost, and since I use WSL for local development, the docker container I connect to isn't localhost but a randomly assigned internal IP. Firefox is the only browser that still allowed window.crypto.subtle to be accessed even when I'm not connecting to localhost.

I'll push a commit up soon to make it configurable via .env.

@faridrached
Copy link

I actually did my testing on Firefox

Hi @KiChjang curious if you also tested Fortmatic on the mentioned browsers?

@octavioamu
Copy link
Contributor

Tested on my local but didn't worked.
I just tried to send a tip on townsquare and metamask never prompted
When reloading the home page is asking to connect a wallet even if I'm not logged, we want to support multiple chains so people need to be able to navigate the site even without wallet, and asked when just trying to interact with blockchain.
web3 browser support also is not working also.

@KiChjang
Copy link
Contributor Author

KiChjang commented Mar 5, 2020

@octavioamu Ah, sending tips through townsquare is a flow that I didn't test, but I tried to dig into it and find the root cause, and it looks like to me that the API call to /tip/send/3 is returning 500. Did you also encounter this error?

@KiChjang
Copy link
Contributor Author

KiChjang commented Mar 5, 2020

@octavioamu I did a rebase to master on this PR and the Send Tip flow should work as intended. Please re-review.

@KiChjang
Copy link
Contributor Author

KiChjang commented Mar 5, 2020

@faridrached I've tested Fortmatic on Firefox only, and everything worked fine.

@KiChjang
Copy link
Contributor Author

@danlipert it's rebased

@thelostone-mc thelostone-mc merged commit 113451d into gitcoinco:master Apr 15, 2020
@owocki
Copy link
Contributor

owocki commented Apr 15, 2020

@KiChjang @faridrached looks like this has been merged. we should get it testeed to make sure no regressions + looks good then maybe start doing some marketing :)

@faridrached
Copy link

Great job. As soon as its ready to go in production, then lets definitely market it!

@KiChjang KiChjang deleted the multiwallet branch May 27, 2022 22:31
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.

Add Authereum Support BOUNTY - Integrate Fortmatic into Gitcoin
6 participants