Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Add geth + Ethereum Wallet integration into Brave #13177

Closed
wants to merge 47 commits into from

Conversation

diracdeltas
Copy link
Member

@diracdeltas diracdeltas commented Feb 18, 2018

Steps:

  1. git pull
  2. npm install
  3. open Brave, turn on "Ethereum Wallet" in about:preferences#extensions
  4. create a wallet (or plug in your hardware wallet) and have fun

screen shot 2018-02-18 at 2 00 24 am

Video demo: https://youtu.be/gIS2RVq_5R4

Submitter Checklist:

  • Submitted a ticket for my issue if one did not already exist.
  • Used Github auto-closing keywords in the commit message.
  • Added/updated tests for this change (for new code or code which already has tests).
  • Ran git rebase -i to squash commits (if needed).
  • Tagged reviewers and labelled the pull request as needed.
  • Request a security/privacy review as needed. (Ask a Brave employee to help if you cannot access this document.)

Test Plan:

Reviewer Checklist:

  • Request a security/privacy review as needed if one was not already requested.

Tests

  • Adequate test coverage exists to prevent regressions
  • Tests should be independent and work correctly when run individually or as a suite ref
  • New files have MPL2 license header

@codecov-io
Copy link

codecov-io commented Feb 18, 2018

Codecov Report

Merging #13177 into master will decrease coverage by 0.22%.
The diff coverage is 37.5%.

@@            Coverage Diff             @@
##           master   #13177      +/-   ##
==========================================
- Coverage   56.18%   55.96%   -0.23%     
==========================================
  Files         279      281       +2     
  Lines       27873    27835      -38     
  Branches     4560     4566       +6     
==========================================
- Hits        15660    15577      -83     
- Misses      12213    12258      +45
Flag Coverage Δ
#unittest 55.96% <37.5%> (-0.23%) ⬇️
Impacted Files Coverage Δ
js/constants/settings.js 100% <ø> (ø) ⬆️
js/constants/appConfig.js 100% <ø> (ø) ⬆️
app/locale.js 35.77% <ø> (ø) ⬆️
js/constants/config.js 55.55% <ø> (ø) ⬆️
js/about/preferences.js 59.83% <0%> (+13.23%) ⬆️
app/browser/tabs.js 23.87% <0%> (-0.36%) ⬇️
app/renderer/lib/extensionsUtil.js 58.94% <75%> (+0.05%) ⬆️
app/browser/api/ledgerNotifications.js 70.95% <0%> (-4.38%) ⬇️
app/renderer/components/preferences/paymentsTab.js 71.22% <0%> (-4.22%) ⬇️
app/browser/reducers/ledgerReducer.js 44.16% <0%> (-2.77%) ⬇️
... and 20 more

sriharikapu
sriharikapu previously approved these changes Feb 19, 2018
@bsclifton bsclifton added this to the Completed work milestone Feb 28, 2018
let batAddress = null
const indexUrl = `${window.location.origin}/index.html`

ipc.send('get-popup-bat-balance')
Copy link
Collaborator

Choose a reason for hiding this comment

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

we don't have a good way to do state updates in extensions with redux so an ipc message is ok, but for sending messages to the browser process we should be using actions which do work in extensions

Copy link
Member Author

Choose a reason for hiding this comment

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

the problem is sending the message back to the popup window, which is a browserAction popup and not in the redux appState. is there a way to get the correct webcontents in reducers using event.sender.getId() if the sender isn't a redux tab?

@Slava Slava mentioned this pull request Jul 13, 2018
10 tasks
@diracdeltas
Copy link
Member Author

closing in favor of the updated branch #14734

@bsclifton bsclifton removed this from the Completed work milestone Jul 22, 2018
@bsclifton bsclifton mentioned this pull request Jul 24, 2018
10 tasks
@bsclifton bsclifton deleted the feature/ethwallet branch August 18, 2018 05:19
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants