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

Allow rLogin to add supported providers without relying on web3modal #90

Merged
merged 5 commits into from
Jun 1, 2021

Conversation

jessgusclark
Copy link
Member

@jessgusclark jessgusclark commented May 24, 2021

summary

Adds a method for RSK to add providers to rLogin without having to wait for web3modal to approve and release commits.

initial approach

The initial method was to import and maintain the JSON file locally. However, this wasn't easy and required maintaining multiple files. Our rLogin module connects to web3Modal's ProviderController, which relies on two functions in the utils file that imports the JSON list.

this solution

This approach is a bit different. Since we are only concerned with renaming injected providers, we check them.

After receiving the array of providers, via the steps above, the checkRLoginInjectedProviders function checks the first provider if it is named "Web3" or "Metamask". The first provider is always "injected" (if available). If it is named 'Web3' it means that it is not one of web3modal's injected providers and is a default. We can use this to check against our local list.

Since it is only checking injected providers named 'web3' & 'Metamask', if web3modal approves a PR with one of our providers, we use web3modals. Then, rLogin needs to clean up the provider in the future. No conflict exists, just extra weight.

Why check for isMetamask?

Math wallet also exposes the isMetamask flag and therefore we should run through our local list. This isn't great, but they are exposing this flag for some reason.

how to test

I am not sure how to best test this without releasing it. There is currently an issue with MathWallet so it is not a good choice. Read the description on #92. I was unable to investigate further.

The best way to test is with ngrok with the RLogin Sample Apps using npm link. This is not the sample app in this repo since it has an absolute link to http:localhost:3005/main.js which will be broken. When I fired it up with the two dapp browsers, it looks like this:

image
image

@jessgusclark jessgusclark changed the title Check web3 [DRAFT] Allow rLogin to add supported providers without relying on web3modal May 24, 2021
@coveralls
Copy link

coveralls commented May 25, 2021

Pull Request Test Coverage Report for Build 892843797

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 0.0%

Totals Coverage Status
Change from base Build 836446889: 0.0%
Covered Lines: 0
Relevant Lines: 39

💛 - Coveralls

@jessgusclark jessgusclark changed the title [DRAFT] Allow rLogin to add supported providers without relying on web3modal Allow rLogin to add supported providers without relying on web3modal May 25, 2021
@jessgusclark jessgusclark marked this pull request as ready for review May 25, 2021 11:41
Copy link
Contributor

@ilanolkies ilanolkies left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LRGTM!

@ilanolkies ilanolkies merged commit 46337a8 into develop Jun 1, 2021
@delete-merged-branch delete-merged-branch bot deleted the check-web3 branch June 1, 2021 04:48
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.

None yet

3 participants