Eth-wallet rebased #14734

Slava · 2018-07-13T18:43:17Z

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

diracdeltas · 2018-07-13T18:52:01Z

app/browser/tabs.js

@@ -721,6 +721,16 @@ const api = {
        }
      })

+      tab.on('did-detach', (e, oldTabId) => {


@Slava is this block supposed to be here?

I don't understand why this is needed. Maybe you can remember the context of this commit? c375ba3

@Slava lol wow i have no idea. sorry i thought it was left over from the rebase.

er yeah this needs to - looks like a rebase issue from before -> after the signle-webview project. Tab's now detach every time they are made inactive / active, so this will break something (minor).

@petemill sorry do you mean this chunk should be deleted or kept (or other)?

@petemill ping

looks like this block got added during rebase because of a fix within in the original ethwallet branch - 00cdfdd#diff-2be38b3d97f35cd818780bac4be979c4R629

I think we should delete this block

oops missed out a key word

this needs to go

Thanks for the spot

srirambv · 2018-07-20T12:00:12Z

Selecting Open/Transfer/Create from extension popup, opens up a new tab each time. Is this expected behaviour?

bsclifton · 2018-07-24T16:14:23Z

Steps from #13177

Steps:

git pull

npm install

open Brave, turn on "Ethereum Wallet" in about:preferences#extensions

create a wallet (or plug in your hardware wallet) and have fun

diracdeltas · 2018-07-25T21:51:46Z

app/extensions.js

+    'style-src': '\'self\' \'unsafe-inline\'',
+    'connect-src': 'blob: \'self\' ws://localhost:* http://localhost:* https://min-api.cryptocompare.com https://mini-api.cryptocompare.com',
+    'img-src': '\'self\' data:',
+    'script-src': '\'self\' \'unsafe-eval\' \'sha256-7B6rTuXUsu9shBeECmDFH4h7RDsfogQ3kIonJnIL40o\' \'sha256-zgjB35Pd2ax7Wwfk9iKnAH8r+gNrD2cHpxDkH81DHzw=\'',


@Slava do we need unsafe-eval here?

I think I added it because whatever the Meteor app compiles into uses new Function. I can look into how tweaking the minifier and try to get something that doesn't use eval.

Ok after spending some time on this I tracked it down to this
facebook/regenerator#336. It is an unsafe line in regenerator that used to compile web3js package for Meteor.

hmm maybe not exactly this, but something very similar that tries to get the value of global

NejcZdovc · 2018-07-31T15:34:54Z

package.json

@@ -95,7 +97,8 @@
    "bat-client": "^3.3.2",
    "bat-publisher": "^2.0.18",
    "bignumber.js": "^4.0.4",
-    "bloodhound-js": "brave/bloodhound",
+    "bloodhound-js": "github:brave/bloodhound",


is this change needed?

ryanml · 2018-07-31T19:37:11Z

@Slava could you rebase when you get a chance?

mrose17 · 2018-08-01T18:42:50Z

app/extensions.js

+
+      geth = spawn(gethProcessPath, gethArgs, spawnOptions)
+
+      geth.on('exit', function (code, signal) {


both the exit/close handlers should do the same thing: log the code & siginal, and restart geth.

there should be a handler for error that logs the error and does not restart geth.

mrose17 · 2018-08-01T18:43:37Z

app/extensions.js

+      if (fs.existsSync(pidPath)) {
+        try {
+          const pid = fs.readFileSync(pidPath)
+          cleanupGeth(pid)


since geth is probably undefined at this point, will cleanupGeth actually kill that pid?

Good catch, in the event that geth is undefined the kill will not go forward. Will fix.

ryanml

lgtm

ryanml

re-approved

Eth-wallet rebased

bsclifton · 2018-08-14T06:23:14Z

master 0845eaf
0.24.x 72820e8
0.23.x 1814e19

delete did-detach block accidentally brought back in #14734

Slava mentioned this pull request Jul 13, 2018

Eth-wallet #14727

Closed

10 tasks

diracdeltas changed the base branch from feature/ethwallet to master July 13, 2018 18:48

diracdeltas mentioned this pull request Jul 13, 2018

Add geth + Ethereum Wallet integration into Brave #13177

Closed

10 tasks

diracdeltas reviewed Jul 13, 2018

View reviewed changes

bsclifton assigned Slava Jul 24, 2018

bsclifton added this to the 0.23.x Release 4 w/ Chromium 68 (BETA Channel) milestone Jul 24, 2018

This was referenced Jul 24, 2018

Unable to create ETH Wallet on via extension menu #14822

Closed

Fund Brave wallet field should be read only #14825

Closed

kjozwiak mentioned this pull request Jul 24, 2018

"Unable to connect" modal for geth keeps being displayed #14826

Closed

This was referenced Jul 24, 2018

Buttons doesn't change to Primary button when clicked #14827

Closed

Ether Text moves to next line once prod network is connected #14832

Closed

bsclifton modified the milestones: 0.23.x Release 4 w/ Chromium 68 (BETA Channel), ETH Wallet Jul 25, 2018

diracdeltas reviewed Jul 25, 2018

View reviewed changes

kjozwiak mentioned this pull request Jul 27, 2018

progress bar not always updating when pulling latest blocks/state #14855

Closed

NejcZdovc reviewed Jul 31, 2018

View reviewed changes

LaurenWags mentioned this pull request Jul 31, 2018

Brave crashes after enabling ETH-Wallet #14877

Closed

ryanml added the PR/needs-rebase ۞ label Jul 31, 2018

mrose17 reviewed Aug 1, 2018

View reviewed changes

Slava and others added 8 commits August 13, 2018 14:31

instant wallet creation in ui

4779310

Using spawnSync

3fbdbd5

metamask button interactions

ac7b1bc

additional text on eth wallet feature desc

537b436

message about backing up keys

91b7680

remove add contract button

e4e1bac

fixing settings persistance

c76812a

write static-nodes.json to the right location

c623dd5

ryanml force-pushed the feature/ethwallet branch from 326048d to edb3ba8 Compare August 13, 2018 21:31

ryanml previously approved these changes Aug 13, 2018

View reviewed changes

ryanml dismissed their stale review via 26cd63f August 13, 2018 22:37

wallet creation tweaks

be48f37

ryanml force-pushed the feature/ethwallet branch from 5b1fd0d to be48f37 Compare August 13, 2018 23:21

Making ensureGethDataDir synchronous, removing unecessary call

b36ab69

ryanml force-pushed the feature/ethwallet branch from e4d805c to b36ab69 Compare August 14, 2018 02:12

ryanml added 2 commits August 13, 2018 19:20

Moving configurePeers

989a6c2

on uncaught removal

64f6897

ryanml approved these changes Aug 14, 2018

View reviewed changes

bsclifton merged commit 0845eaf into brave:master Aug 14, 2018

bsclifton added a commit that referenced this pull request Aug 14, 2018

Merge pull request #14734 from Slava/feature/ethwallet

72820e8

Eth-wallet rebased

bsclifton added a commit that referenced this pull request Aug 14, 2018

Merge pull request #14734 from Slava/feature/ethwallet

1814e19

Eth-wallet rebased

This was referenced Aug 14, 2018

Review Issue: tab.detatch #15019

Closed

Merged Review Issue: unsafe eval #15020

Closed

eth wallet sec review issues #15012

Closed

evq added a commit that referenced this pull request Aug 14, 2018

delete did-detach block accidentally brought back in #14734

af68abb

petemill added a commit that referenced this pull request Aug 15, 2018

Merge pull request #15022 from brave/deleteDidDetach

d8c52a4

delete did-detach block accidentally brought back in #14734

petemill added a commit that referenced this pull request Aug 15, 2018

Merge pull request #15022 from brave/deleteDidDetach

c209fbd

delete did-detach block accidentally brought back in #14734

petemill added a commit that referenced this pull request Aug 15, 2018

Merge pull request #15022 from brave/deleteDidDetach

b433bc2

delete did-detach block accidentally brought back in #14734

bsclifton mentioned this pull request Aug 16, 2018

Revert ETH Wallet functionality #15046

Merged

10 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Eth-wallet rebased #14734

Eth-wallet rebased #14734

Slava commented Jul 13, 2018

diracdeltas Jul 13, 2018

Slava Jul 13, 2018

diracdeltas Jul 13, 2018

petemill Jul 24, 2018

diracdeltas Aug 2, 2018

diracdeltas Aug 9, 2018

evq Aug 14, 2018

petemill Aug 15, 2018

srirambv commented Jul 20, 2018

bsclifton commented Jul 24, 2018

diracdeltas Jul 25, 2018

Slava Jul 25, 2018

Slava Jul 31, 2018

Slava Jul 31, 2018

NejcZdovc Jul 31, 2018 •

edited

Loading

ryanml commented Jul 31, 2018

mrose17 Aug 1, 2018

ryanml Aug 1, 2018

mrose17 Aug 1, 2018

ryanml Aug 1, 2018

ryanml Aug 1, 2018

ryanml left a comment •

edited

Loading

ryanml left a comment

bsclifton commented Aug 14, 2018


		geth = spawn(gethProcessPath, gethArgs, spawnOptions)

		geth.on('exit', function (code, signal) {

Eth-wallet rebased #14734

Eth-wallet rebased #14734

Conversation

Slava commented Jul 13, 2018

Submitter Checklist:

Test Plan:

Reviewer Checklist:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

srirambv commented Jul 20, 2018

bsclifton commented Jul 24, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NejcZdovc Jul 31, 2018 • edited Loading

Choose a reason for hiding this comment

ryanml commented Jul 31, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ryanml left a comment • edited Loading

Choose a reason for hiding this comment

ryanml left a comment

Choose a reason for hiding this comment

bsclifton commented Aug 14, 2018

NejcZdovc Jul 31, 2018 •

edited

Loading

ryanml left a comment •

edited

Loading