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

Add built-in Ethereum remote client / wallet / Dapp browser #4494

Closed
bbondy opened this issue May 18, 2019 · 7 comments

Comments

@bbondy
Copy link
Member

commented May 18, 2019

We're in the process of adding a first class Ethereum remote client / wallet feature to Brave by integrating a fork of Metamask that we'll modify, enhance and maintain. Sometimes native APIs that only Brave supports will be used, in those cases it probably doesn't make sense to upstream those parts.

We'll be upstreaming as much as we can to MetaMask. We have been, and will continue to collaborate with the MetaMask team as this work progresses.

Some bigger main differences will be:

  • Initial UI will not include a browser action button, but be a menu item in the hamburger menu that opens up the full wallet view.
  • Dapp detection UI and popup UI will be used. The user will need to give permission to a page to see any available API (e.g. EIP 1102 window.ethereum and web3).
  • It will use Brave branding.
  • The UI will be modified in some ways.
  • The seed key will be derived from the Brave key (currently only used for Sync) via a 1 way transformation. Which means you only need to backup 1 set of BIP-39 style codewords and then you can use either or both of client side encrypted sync and the wallet. Sync uses curve25519 whereas Ethereum uses secp256k1.

Rollout:

  • Desktop will be rolled out first.
  • Android is in the process of being re-written on top of Brave Core and it will support this feature after we add extension support here #4493. We are planning to also tie into Samsung and HTC private enclave and / or wallet.

Future Brave rewards features will integrate and use this wallet support for P2P transactions.


Some stats from MetaMask which are relevant :

Here's some stats for the month of April for MetaMask:

264,000 MAU
30% of active users confirm on chain transactions
1.5152M transactions 

Of 100% of network type activity:
- 63% of activity happens on mainnet
- 14.5% Ropsten
- 8% Rinkeby
- 6% Ganache
- 2% Kovan
- 6.5% Other

Of 100% of initiation type for transactions:
- 76% via popup from apps
- 24% via the user
Privacy Preserving Product Analytics (P3A)

Of 100% transaction types:
- 18% token transfers
- 12% ETH sends
- 3% approve
- 2.5% transfer from
- 0.2% contract deployment
- 64% Other contract interaction

Top mainnet sites by volume:
- 0x universe
- Cryptokitties
- Dice 2 Win
- Axie Infinity
- Fork Delta

Diversity of transactions to apps:
- Over 25 apps have over 5k transactions

Source:
https://medium.com/metamask/metamask-metrics-fbec0e2ceaa7


Here are the issues closed as part of this feature:
https://github.com/brave/brave-browser/issues?q=label%3Afeature%2Fcrypto-wallets+is%3Aclosed

Test Plan

I'm going to need help with this but:

  • New profile and navigating to brave://wallet or chrome://wallet should take a bit longer the first time and actually install the component and work seamlessly.

  • New profile and instead going to the hamburger menu for crypto wallet should do the same and install and open the wallet.

  • Chromium Process Task Manager in the hamburger menu tools should not show that it is running before installing it via one of the 2 above methods.

  • Test basic wallet functionality for ETH send/receive

  • Test basic wallet functionality for ERC20 send/receive

  • Test basic Dapp functionality, buy a cryptokitty and Sire it. Use a Dex.

  • Try changing from mainnet to another network, make sure it works.

  • Check closed issues that are labeled feature/crypto-wallets inside of 0.70.x milestone for more ideas.

@bbondy

This comment has been minimized.

Copy link
Member Author

commented Aug 14, 2019

This is on Nightly, there will be follow up work but I'm closing!

@bbondy

This comment has been minimized.

Copy link
Member Author

commented Aug 14, 2019

Added test plan above.

@kjozwiak

This comment has been minimized.

Copy link
Member

commented Oct 1, 2019

@bbondy @tomlowenthal would running through https://github.com/brave/ethereum-remote-client/wiki/Testing be sufficient enough for QA verification? It's basically similar to what's outlined under #4494 (comment) but more detailed.

@bbondy

This comment has been minimized.

Copy link
Member Author

commented Oct 2, 2019

sgtm

@kjozwiak

This comment has been minimized.

Copy link
Member

commented Oct 2, 2019

Verification PASSED on macOS 10.14.6 x64 using the following build:

Brave 0.69.131 Chromium: 77.0.3865.90 (Official Build) (64-bit)
Revision 58c425ba843df2918d9d4b409331972646c393dd-refs/branch-heads/3865@{#830}
OS macOS Version 10.14.6 (Build 18G95)

Account Creation

  • There should be no prompt to opt-in to MetaMetrics telemetry during the onboarding process.
  • After password creation, a 24 word seed phrase should be generated for you, and it should process during the verification step.
  • The backup file should contain the intact 24 word seed phrase.

Import & Recovery

  • Ensure that text fields for seed phrases only accept 24 word phrases
  • Upon successful recovery, the exact wallet should be loaded as the main account. Balances for each network should be retained.

Settings page

  • There should be no About tab present --> Created #6271
  • Under the advanced tab, there should be no prompt for Mobile Sync --> Created #6272
  • The Info & Help tab should not be present in the main dropdown menu --> Created #6273
  • There should be no link to the MetaMask TOS
  • There should be no option to opt-in to MetaMetrics telemetry from the Security tab

Branding

  • There are no appearances of the word MetaMask.
  • Links to MetaMask resources, such as websites and emails, should point to Brave resources.
  • In downloadable resources, such as the recovery key, the name MetaMask should not be in the filename. Created --> #6275
  • The MetaFox logo should not be present anywhere. (Check the welcome page, header, and login page)
  • Muli and Poppins fonts should be in use throughout the extension

BAT

  • The BAT Token should be added by default to each eth network.

Dapps

Use a dapp such as https://www.cryptokitties.co/ for testing

  • The Dapp can successfully connect to your crypto wallets account, signed via a pop-up notification
  • Transactions relating to the dapp's functionality can be signed and completed via a pop-up notification
  • All dapp transactions where assets are sent/received show up in the account activity view within the extension

Transactions

  • A QR code is visible for the generated address on Account Details
  • Eth/ERC-20 tokens can be received at the addresses generated for your account
  • Eth/ERC-20 tokens can be sent to other addresses from your account
  • For send transactions, Gas is calculated appropriately and can be adjusted for different speeds

Accounts/Hardware

  • Multiple accounts be created via the dropdown menu, whether new or imported
  • An account can be created by connecting a Trezor hardware device
  • An account can be created by connecting a Ledger hardware device
    ERC-20 assets can be added to the token list

Failed Transactions

Screen Shot 2019-10-02 at 3 47 41 AM

Screen Shot 2019-10-02 at 3 33 57 AM

Successful Transactions

Screen Shot 2019-10-02 at 3 40 41 AM

Screen Shot 2019-10-02 at 3 40 23 AM

Transaction appearing under Uphold

Screen Shot 2019-10-02 at 3 44 15 AM

Connected to Nano Ledger

Screen Shot 2019-10-02 at 3 59 41 AM

Screen Shot 2019-10-02 at 3 59 21 AM

Screen Shot 2019-10-02 at 4 00 44 AM

Connected to Trezor

Screen Shot 2019-10-02 at 4 13 07 AM

@srirambv

This comment has been minimized.

Copy link
Collaborator

commented Oct 2, 2019

Most verification passed on

Brave 0.69.131 Chromium: 77.0.3865.90 (Official Build) (64-bit)
Revision 58c425ba843df2918d9d4b409331972646c393dd-refs/branch-heads/3865@{#830}
OS Linux

Account Creation

  • There should be no prompt to opt-in to MetaMetrics telemetry during the onboarding process.
  • After password creation, a 24 word seed phrase should be generated for you, and it should process during the verification step.
  • The backup file should contain the intact 24 word seed phrase.

Import & Recovery

  • Ensure that text fields for seed phrases only accept 24 word phrases
  • Upon successful recovery, the exact wallet should be loaded as the main account. Balances for each network should be retained.

Settings page

  • There should be no About tab present --> Blocked on #6271
  • Under the advanced tab, there should be no prompt for Mobile Sync --> Blocked on #6272
  • The Info & Help tab should not be present in the main dropdown menu --> Blocked #6273
  • There should be no link to the MetaMask TOS
  • There should be no option to opt-in to MetaMetrics telemetry from the Security tab

Branding

  • There are no appearances of the word MetaMask. Blocked on #6103, #6102, #6100, #6038
  • Links to MetaMask resources, such as websites and emails, should point to Brave resources. Blocked on #6039, #6101, #6102, #6103, #6104
  • In downloadable resources, such as the recovery key, the name MetaMask should not be in the filename. Blocked on #6275, #6281
  • The MetaFox logo should not be present anywhere. (Check the welcome page, header, and login page). This is blocked on #6038
  • Muli and Poppins fonts should be in use throughout the extension

BAT

  • The BAT Token should be added by default to each eth network.

Dapps

Use a dapp such as https://www.cryptokitties.co/ for testing

  • The Dapp can successfully connect to your crypto wallets account, signed via a pop-up notification
  • Transactions relating to the dapp's functionality can be signed and completed via a pop-up notification
  • All dapp transactions where assets are sent/received show up in the account activity view within the extension

Transactions

  • A QR code is visible for the generated address on Account Details
  • Eth/ERC-20 tokens can be received at the addresses generated for your account
  • Eth/ERC-20 tokens can be sent to other addresses from your account
  • For send transactions, Gas is calculated appropriately and can be adjusted for different speeds

Accounts/Hardware

  • Multiple accounts be created via the dropdown menu, whether new or imported
  • An account can be created by connecting a Trezor hardware device
  • An account can be created by connecting a Ledger hardware device
  • ERC-20 assets can be added to the token list

Wallet creation success

image

Connect to Daap site

image

Signin to Daap site

image

Hardware Wallet Detect

image

Connect new hardware and unlock wallet

image

Trezor Wallet connected

image

Ledger wallet connected

image

Confirmed transaction

image

Transaction notification

image

Insufficient funds message

image

@srirambv

This comment has been minimized.

Copy link
Collaborator

commented Oct 2, 2019

Most verification passed on

Brave 0.69.131 Chromium: 77.0.3865.90 (Official Build) (64-bit)
Revision 58c425ba843df2918d9d4b409331972646c393dd-refs/branch-heads/3865@{#830}
OS Windows 10 OS Version 1803 (Build 17134.523)

Account Creation

  • There should be no prompt to opt-in to MetaMetrics telemetry during the onboarding process.
  • After password creation, a 24 word seed phrase should be generated for you, and it should process during the verification step.
  • The backup file should contain the intact 24 word seed phrase.

Import & Recovery

  • Ensure that text fields for seed phrases only accept 24 word phrases
  • Upon successful recovery, the exact wallet should be loaded as the main account. Balances for each network should be retained.

Settings page

  • There should be no About tab present --> Blocked on #6271
  • Under the advanced tab, there should be no prompt for Mobile Sync --> Blocked on #6272
  • The Info & Help tab should not be present in the main dropdown menu --> Blocked #6273
  • There should be no link to the MetaMask TOS
  • There should be no option to opt-in to MetaMetrics telemetry from the Security tab

Branding

  • There are no appearances of the word MetaMask. Blocked on #6103, #6102, #6100, #6038
  • Links to MetaMask resources, such as websites and emails, should point to Brave resources. Blocked on #6039, #6101, #6102, #6103, #6104
  • In downloadable resources, such as the recovery key, the name MetaMask should not be in the filename. Blocked on #6275, #6281
  • The MetaFox logo should not be present anywhere. (Check the welcome page, header, and login page). This is blocked on #6038
  • Muli and Poppins fonts should be in use throughout the extension

BAT

  • The BAT Token should be added by default to each eth network.

Dapps

Use a dapp such as https://www.cryptokitties.co/ for testing

  • The Dapp can successfully connect to your crypto wallets account, signed via a pop-up notification
  • Transactions relating to the dapp's functionality can be signed and completed via a pop-up notification
  • All dapp transactions where assets are sent/received show up in the account activity view within the extension

Transactions

  • A QR code is visible for the generated address on Account Details
  • Eth/ERC-20 tokens can be received at the addresses generated for your account
  • Eth/ERC-20 tokens can be sent to other addresses from your account
  • For send transactions, Gas is calculated appropriately and can be adjusted for different speeds

Accounts/Hardware

  • Multiple accounts be created via the dropdown menu, whether new or imported
  • An account can be created by connecting a Trezor hardware device
  • An account can be created by connecting a Ledger hardware device
  • ERC-20 assets can be added to the token list

Successful wallet creation

image

Ledger Nano connected

image

Trezor Connected

image

Successful transaction

image

Transaction notification

image

Dapp connect

image

Dapp sign

image

Insufficient funds

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.