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.

Sign up for GitHub

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

Jump to bottom

Ensure default wallet option in brave://settings/wallet works correctly with MM and Crypto Wallets #17984

Closed
bbondy opened this issue Sep 10, 2021 · 3 comments · Fixed by brave/brave-core#10191
Closed

Ensure default wallet option in brave://settings/wallet works correctly with MM and Crypto Wallets #17984

bbondy opened this issue Sep 10, 2021 · 3 comments · Fixed by brave/brave-core#10191
Assignees
@bbondy
Labels
feature/web3/wallet Integrating Ethereum+ wallet support OS/Desktop QA Pass-Linux QA Pass-macOS QA Pass-Win64 QA/Test-All-Platforms QA/Yes release-notes/include
Milestone
1.31.x - Release

Comments

@bbondy
Copy link
Member

bbondy commented Sep 10, 2021 •
edited
Loading

This still needs to be checked, but I don't think it works correctly currently.

  • If a user installs MetaMask, that should become the Dapp provider and we shouldn't try to inject window.ethereum unless the user explicitly selects us later.
  • If a user selects Crypto Wallets explicitly window.ethereum should be provided by Crypto Wallets
  • If the user uninstalls MetaMask, we should set Brave Wallet as the Dapp provider
@bbondy bbondy added OS/Android Fixes related to Android browser functionality OS/Desktop labels Sep 10, 2021
@bbondy bbondy self-assigned this Sep 10, 2021
@bbondy
Copy link
Member Author

bbondy commented Sep 10, 2021

cc @jamesmudgett

@bbondy bbondy mentioned this issue Sep 22, 2021
Merged
24 tasks
@bbondy bbondy added this to the 1.32.x - Nightly milestone Sep 23, 2021
@brave-builds brave-builds mentioned this issue Sep 23, 2021
Merged
7 tasks
@bbondy bbondy added the QA/Yes label Sep 24, 2021
@bbondy bbondy changed the title Ensure Ethereum provider option in brave://wallet settings works correctly with MM and Crypto Wallets Ensure default wallet option in brave://settings/wallet works correctly with MM and Crypto Wallets Sep 24, 2021
@LaurenWags LaurenWags added feature/web3/wallet Integrating Ethereum+ wallet support QA/Test-All-Platforms labels Sep 28, 2021
@srirambv
Copy link
Contributor

srirambv commented Oct 1, 2021

Verification passed on

Brave 1.31.67 Chromium: 94.0.4606.61 (Official Build) beta (64-bit)
Revision 418b78f5838ed0b1c69bb4e51ea0252171854915-refs/branch-heads/4606@{#1204}
OS Windows 11 Version Dev (Build 22468.1000)
  • Verified when MM is set as default Dapp provider, window.ethereum is only provided by MM
  • Verified when CW is set as default Dapp provider, Window.ethereum is only provided by CW extension
  • Verified when MM is uninstalled, Brave Wallet is set as default Dapp provider when flag is enabled
  • Verified when MM is uninstalled, Crypto Wallet is set as default Dapp provider when flag is not enabled

Verification passed on

Brave 1.31.67 Chromium: 94.0.4606.61 (Official Build) beta (64-bit)
Revision 418b78f5838ed0b1c69bb4e51ea0252171854915-refs/branch-heads/4606@{#1204}
OS Linux
  • Verified when MM is set as default Dapp provider, window.ethereum is only provided by MM
  • Verified when CW is set as default Dapp provider, Window.ethereum is only provided by CW extension
  • Verified when MM is uninstalled, Brave Wallet is set as default Dapp provider when flag is enabled
  • Verified when MM is uninstalled, Crypto Wallet is set as default Dapp provider when flag is not enabled

Verification passed on

Brave 1.31.67 Chromium: 94.0.4606.61 (Official Build) beta (x86_64)
Revision 418b78f5838ed0b1c69bb4e51ea0252171854915-refs/branch-heads/4606@{#1204}
OS macOS Version 11.5.2 (Build 20G95)
  • Verified when MM is set as default Dapp provider, window.ethereum is only provided by MM
  • Verified when CW is set as default Dapp provider, Window.ethereum is only provided by CW extension
  • Verified when MM is uninstalled, Brave Wallet is set as default Dapp provider when flag is enabled
  • Verified when MM is uninstalled, Crypto Wallet is set as default Dapp provider when flag is not enabled

@srirambv srirambv added QA Pass-Linux QA Pass-macOS QA Pass-Win64 and removed OS/Android Fixes related to Android browser functionality labels Oct 1, 2021
@srirambv
Copy link
Contributor

srirambv commented Oct 5, 2021

Removed Android label for now

@kjozwiak kjozwiak mentioned this issue Oct 19, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@bbondy bbondy

Labels
feature/web3/wallet Integrating Ethereum+ wallet support OS/Desktop QA Pass-Linux QA Pass-macOS QA Pass-Win64 QA/Test-All-Platforms QA/Yes release-notes/include
Projects
None yet
Milestone
1.31.x - Release
4 participants
@bbondy @kjozwiak @srirambv @LaurenWags