Integrate Parity Signer into Fether

[axelchalon]

Integrate Parity Signer (closes #146)

• Add option to import a Parity Signer account. It is then stored in local storage.
• Send transactions with Parity Signer accounts
• Refactor sendStore so that "token" (and chainId) is inside this.tx; avoids having to pass token around every function, and it makes sense to put it in the object since it is part of the tx info.
• Add account type (either node or signer) to accountsInfo
• Add account type to withAccount; output props are now {account: {address, type}} instead of {accountAddress}

For some reason some of the changes I introduced makes TxForm (and seemingly AccountsList as well) reset/remount on every block; not sure why that is, need to track this down. Makes it difficult to send transactions.

[axelchalon]

            light({
              transactionCount: () =>
                transactionCountOf$(accountAddress).pipe(withoutLoading())
            }),

This makes the component remount on every new block (blank screen for a short period then TxForm appears again, reset). Not sure how to rewrite this?
If I remove those lines the TxForm fields "just" reset on every block (even though there is no remounting), even after removing all RequireHealth; because of the RPC calls in withBalance. Wasn't the case before; not sure how to fix.

[amaury1093]

I believ HOCs in the render method is an anti-pattern.

Would something like this work?

const WithAccount = compose(
  withRouter,
  withAccountsInfo,
  light({
    transactionCount: props =>
      transactionCountOf$(props.router.accountAddress).pipe(withoutLoading())
  }),
  mapProps(...)
)(props => props.children(props.account));

export default Component => initialProps => (
  <WithAccount>
    {account => <Component {...initialProps} account={account} />}
  </WithAccount>
);

[axelchalon]

it does work! much cleaner 👍 still doesn't resolve the reset issue though, I'll continue investigating

[axelchalon]

actually my bad, it does seem to have resolved the issue!! 🎉

[Tbaut]

Awesome!! I gave it a go and here is an early feedback:

• I opened the scanner view to import an account, it took me a minute or so to grab my phone and bring up the Parity Signer QR code, in the mean time, Fether switched to the "new account" view without me doing anything (note that I could reproduce it another time, but it doesn't happen every time), that's not critical.

• The left part of the webcam scanner is grey (here is my webcam with a postit on it) the gray column on the left is not expected I guess.
  

edit: problems on my machine

• The confirm button is not the default one when entering name (similar to bug: Enter keypress maps to 'back' #280)

• We don't need to have a password for such an account as the security is handled by Parity Signer itself.

• When clicking on any account balance to send some ETH I get a
  TypeError: this.project is not a function in switchMap.js:55

[axelchalon]

The left part of the webcam scanner is grey (here is my webcam with a postit on it) the gray column on the left is not expected I guess.

Okay, I have the same thing. Good to know it's not just me :P Might be an issue in @parity/qr-signer

The confirm button is not the default one when entering name (similar to #280)

Not sure I get it. When I'm importing a Parity Signer account, after scanning I land on the AccountName screen with the account name field automatically focused ; once I've entered something, if I press enter it submits the form as expected

We don't need to have a password for such an account as the security is handled by Parity Signer itself.

Cannot reproduce; after entering the account name for a Parity Signer account I'm redirected to the accounts list.

When clicking on any account balance to send some ETH I get a TypeError: this.project is not a function in switchMap.js:55

Cannot reproduce. Maybe reinstall the dependencies?

[Tbaut]

Yup, most of the problems above where because of me, I updated the post above. Here is what I found (beside the refresh problem):

• The QR code to scan the transaction isn't in a card (with white backgrounds) and also not centerd
  
• Same for the Webcam scanner (this time the column on the left is black)
  

Other than that it's working great!!

[Tbaut]

I found little things apart from the styling nits mentioned before.

[ltfschoen]

I tried using the new features using a Samsung Galaxy S8 Note mobile using Parity Signer 2.0 Beta and on macOS Mojave 10.14.

1) Creating an account just using Fether without Parity Signer

If we create an account just using Fether (without Parity Signer), then once its created nothing is in localStorage until you associate a token (other than ETH) like THIBCoin with the account it's stored in localStorage as:

• Key: localforage/__paritylight::tokens::0x005072Fb985cc64d8A1Ce00ECe26279f73c12F8a::kovan
• Value:

{"ETH":{"address":"ETH","decimals":18,"logo":"data:image/png;base64,iVB...II=","name":"Ether","symbol":"ETH"},"0x4A6e6C3868A279e1D9047B42C3fB356FF4680003":{"address":"0x4A6e6C3868A279e1D9047B42C3fB356FF4680003","decimals":18,"name":"THIBCoin","symbol":"TIB","logo":"http://pix.toile-libre.org/upload/original/1525765045.jpg"}}

Note: If we import an account that was created with Parity Signer that is from a different network other than Kovan, it just shows the Parity Signer logo inside their identicon, could we also show what network it is associated with
just like we do in Parity Signer and colour-code it?

2) Local Storage values after importing an Account from Parity Signer

After importing from Parity Signer it's stored in localStorage as the following. Notice how I've had to name the accounts with the network name they were created on?:

• Key: localforage/__paritylight::paritySignerAccounts
• Value:

[
    {"address":"0x006e27b6a72e1f34c626762f3c4761547aff1421","name":"test-signer-kovan"},
    {"address":"0x000ec8c79dac74d13b7e4743ae52f78d7186ba6d","name":"test-signer-kovan"},
    {"address":"0x00d7bb9dbd761fdf00d78f74710ad6bdcaed8d48","name":"test-signer-mainnet"}
]

After importing an account from Parity Signer it:

• Resets the value stored under the following key (which may have contained information about a different account, see Step 3, why do we need to store these values in local storage?) to:
  • Key: _localforage_sys/_localforage_observable_sys
  • Value:

{"name":"localforage","storeName":"keyvaluepairs","key":"__paritylight::paritySignerAccounts","methodName":"setItem","valueChange":true,"success":true,"ticks":1547094958181}

• Note: It allows us to import an account that we previous imported again, but it appears to do so correctly. Should we even allow the user to do this anyway? Or on the "Import Account" page should we show an error that the "Account is already in the account list"?

• Note: It scans the "account" QR code really quickly and even when I aim it at 45 degree angle. Whereas later on when I scan the "signature" QR code I had to aim it properly

• Note: I never saw the gray column on the "Import account" page where I scanned my QR code, which @Tbaut experienced. This is what I see

3) After associating a token like THIBCoin with the account that we imported from Parity Signer:

The following was observed:

• Overwrites the values stored under the following key in localStorage to the following:
  • Key: _localforage_sys/_localforage_observable_sys
  • Value:

{"name":"localforage","storeName":"keyvaluepairs","key":"__paritylight::tokens::0x00ca490242abb36cfad5697bafa69fbd1824389d::kovan","methodName":"setItem","valueChange":true,"success":true,"ticks":1547095163503}

* Note: **If I "Add token" (other than ETH) like THIBCoin to an account that I created using Parity Signer on a network other than Kovan (i.e. Ethereum Mainnet), then it still adds it as shown above, but appears to associate the tokens with Kovan Network since it has `::kovan`, is this correct?**

• Adds the following key/value to localStorage. If we import an account that we previously imported it retains the tokens associated correctly:

  • Key: localforage/__paritylight::tokens::0x005072Fb985cc64d8A1Ce00ECe26279f73c12F8a::kovan
  • Value:

{"ETH":{"address":"ETH","decimals":18,"logo":"data:image/png;base64,iV...II=","name":"Ether","symbol":"ETH"},"0x4A6e6C3868A279e1D9047B42C3fB356FF4680003":{"address":"0x4A6e6C3868A279e1D9047B42C3fB356FF4680003","decimals":18,"name":"THIBCoin","symbol":"TIB","logo":"http://pix.toile-libre.org/upload/original/1525765045.jpg"}}

* Note: The address shown as `0x4A6e6C3868A279e1D9047B42C3fB356FF4680003` is the Contract Address of the THIBCoin token

4) Removing the token THIBCoin from an account that we imported from Parity Signer:

The following was observed:

• Overwrites
  • Key: _localforage_sys/_localforage_observable_sys
  • Value:

{"name":"localforage","storeName":"keyvaluepairs","key":"__paritylight::tokens::0x00d7bb9dbd761fdf00d78f74710ad6bdcaed8d48::kovan","methodName":"setItem","valueChange":true,"success":true,"ticks":1547096914039}

• Updates the following in localStorage:
  • Key: localforage/__paritylight::tokens::0x00d7bb9dbd761fdf00d78f74710ad6bdcaed8d48::kovan
  • Value:

{"ETH":{"address":"ETH","decimals":18,"logo":"data:image/png;base64,iVB...II=","name":"Ether","symbol":"ETH"}}

5) Importing Account in Fether using the passphrase that was generated when I created an in Parity Signer

After importing an an account from Parity Signer that was created on a network (tested so far using Ethereum mainnet, and Kovan). If within Fether I try to "Import Account" and enter the recovery words that were shown in Parity Signer, it returns "The passphrase was not recognised... ". Shouldn't we be able to recover an account using the keyphrase both from Parity Signer and Fether?

6) Error with an unknown cause

At one stage after I was testing to see if I could recover a Parity Signer keyphrase using Fether, when I then went to scan a QR code to import an account it wasn't working, and when I checked the console it displayed error:

6b468d40-e396-45c6-aede-2c878f3e578a:1 Uncaught RangeError: Invalid typed array length: Infinity
    at typedArrayConstructByLength (<anonymous>)
    at new Uint8ClampedArray (native)
    at Function.o.createEmpty (blob:http://localhost:3000/6b468d40-e396-45c6-aede-2c878f3e578a:1:917)
    at Object.a [as extract] (blob:http://localhost:3000/6b468d40-e396-45c6-aede-2c878f3e578a:4:26907)
    at t (blob:http://localhost:3000/6b468d40-e396-45c6-aede-2c878f3e578a:1:4753)
    at blob:http://localhost:3000/6b468d40-e396-45c6-aede-2c878f3e578a:5:744
o.createEmpty @ 6b468d40-e396-45c6-aede-2c878f3e578a:1
a @ 6b468d40-e396-45c6-aede-2c878f3e578a:4
t @ 6b468d40-e396-45c6-aede-2c878f3e578a:1
(anonymous) @ 6b468d40-e396-45c6-aede-2c878f3e578a:5

Screenshot:

7) Unable to Send Transaction using Fether from an account imported from Parity Signer

After transferring some ETH to the Parity Signer account that I imported into Fether. I then tried to see what would happen if I transferred some of that ETH from an account that was created using Parity Signer on the Kovan network to another account (I tried sending it to another account that was created using Parity Signer on the Ropsten network, and then tried sending it to an account that was created using Fether on the Kovan network, but both exhibited the same behaviour as follows).

It showed "Scan" instead of "Send" correctly, and instead of prompting me for a password (as it does for accounts created in Fether), it correctly asks me to "Please scan the QR code of the transaction on Parity Signer". So I scan the QR code that's shown with Parity Signer, and clicked the "Sign" button within Parity Signer, and then Parity Signer displays another QR code and prompts me to "SCAN SIGNATURE", so in Fether I click the "Next" button and on the next screen I use my webcam to scan the QR code that's shown in Parity Signer.

But then it just goes back to the "Send Ether" page with the amount and recipient values that I previously entered still in the input fields but the fields aren't editable, and nothing was sent when I check the account on BlockScout, and nothing is shown in the console to indicate any error.

Note: I think the QR code that's shown needs some padding and should be centred on the screen with CSS (as mentioned by @Tbaut

8) Gap between Identicon and Account name

I think this is unrelated to this PR, but on the account page, the gap between the identicon and the account name is too small in my opinion

Very impressive work! So far I've only performed "user testing" and haven't yet reviewed the code, apart from having a quick peek :). I'll look at it tonight

[Tbaut]

1. could we also show what network it is associated with just like we do in Parity Signer and colour-code it?

This will be needed soon (if we have some network change ability) but I think it's outside of the scope of this PR. (I want to release 0.2 as soon as this PR is merged, let's keep it simple).

2. It allows us to import an account that we previous imported again, but it appears to do so correctly. Should we even allow the user to do this anyway?

Good catch. Probably not if we want to be consistent.

2. This is what I see

Too much sun/light visible on the picture, my (Berlin calibrated) eyes couldn't process it 🌧️

2. It scans the "account" QR code really quickly and even when I aim it at 45 degree angle. Whereas later on when I scan the "signature" QR code I had to aim it properly

The signature contains much more info, it is a much more complex QR code that requires better light and overall scanning conditions.

5. Shouldn't we be able to recover an account using the keyphrase both from Parity Signer and Fether?

Damn, that's clearly a bug in Parity Signer, for some reason it generates a 11 words recovery. This must be fixed on Signer. For those who created an account with 11 words though it's be nice to support it in Fether.
edit: done in #350

6. transferred some of that ETH from an account that was created using Parity Signer on the Kovan network to another account (I tried sending it to another account that was created using Parity Signer on the Ropsten network

This is an edge case that I propose to handle separately. For the beta let's assume ppl work on the same network.

[amaury1093]

👍 . The resetting of TxForm needs to be fixed though, I'm not 100% how either, I proposed something in the comments

[amaury1093]

I believ HOCs in the render method is an anti-pattern.

Would something like this work?

const WithAccount = compose(
  withRouter,
  withAccountsInfo,
  light({
    transactionCount: props =>
      transactionCountOf$(props.router.accountAddress).pipe(withoutLoading())
  }),
  mapProps(...)
)(props => props.children(props.account));

export default Component => initialProps => (
  <WithAccount>
    {account => <Component {...initialProps} account={account} />}
  </WithAccount>
);

[amaury1093]

It would make sense to factorize code between this and TxForm, if it's easily doable

[ltfschoen]

5) Importing Account in Fether using the passphrase that was generated when I created an in Parity Signer (Continued)

Since my previous comment #336 (comment), @Tbaut created this PR #351 so that if we created an account on Parity Signer we could import it into Fether using its 11 Recovery Words, and his fix works!

But I encountered a problem, which I'll now explain...

So I'd previously created a Parity Signer account and imported it into Fether using the QR scanner, and it displays successfully in my account list (with the Parity Signer logo embedded in the identicon). I had topped it up with some ETH and THIBCoin.

In my previous comment #336 (comment) I tried importing the same Parity Signer account into Fether that I had already previously imported into Fether from Parity Signer and was already shown in my account list, and found that it didn't prevent me from doing so (even though I expected it to say Account 0x... already listed), so I think we need to apply to "Recover from Parity Signer" the changes that were made in this PR for the other ways to import #327, but it also didn't cause any issues.

But now I wanted to see what would happen if I tried to import a Parity Signer account using "Recover from Seed Phrase" in Fether, even though I'd already previously imported it using "Recover from Parity Signer" and the account was already in my account list in Fether under the name "test-signer2", as shown below:

So I entered the recovery words that were generated when I created my Parity Signer account, but instead of it displaying Account 0x... already listed it took me to the "Please give this account a name:" page as shown below. Is this a bug?

And on that page it showed an identicon with the Parity Signer logo embedded inside it. Is this a bug too?

i.e. how does it know that it was generated with Parity Signer when I'm importing it using the recovery words approach instead of the QR scanner approach, or is it showing the Parity Signer logo embedded in the identicon because it's already in my Fether account list since it was previously imported using the QR code?

So then I gave it a different name "test-signer2-again", and clicked "Next" where it prompted me for a password, which I entered as shown below:

It then returned me to the Accounts page, but now I have both the "test-signer2" account (WITH the Parity Signer logo embedded), and the "test-signer2-again" account (WITHOUT the Parity Signer logo embedded), and they both have the same Ethereum address

When I went to the "test-signer2" account page it still showed both ETH and THIBCoin tabs,
but when I went to the "test-signer2-again" account page it only showed the ETH tab, and I had to add the THIBCoin tab again.

If I check Dev Tools > Application > Local Storage, it hasn't updated the following list of Parity Signer accounts:

• Key: localforage/__paritylight::paritySignerAccounts
• Value:

[{"address":"0x006e27b6a72e1f34c626762f3c4761547aff1421","name":"test-signer"},{"address":"0x000ec8c79dac74d13b7e4743ae52f78d7186ba6d","name":"test-signer2"},{"address":"0x00d7bb9dbd761fdf00d78f74710ad6bdcaed8d48","name":"test-signer-mainnet"},{"address":"0x00ca490242abb36cfad5697bafa69fbd1824389d","name":"test4-ropsten"}]

And the following entry for the account hasn't changed:

• Key: localforage/__paritylight::tokens::0x000ec8c79dac74d13b7e4743ae52f78d7186ba6d::kovan
• Value:

{"ETH":{"address":"ETH","decimals":18,"logo":"data:image/png;base64,iVB...YII=","name":"Ether","symbol":"ETH"},"0x4A6e6C3868A279e1D9047B42C3fB356FF4680003":{"address":"0x4A6e6C3868A279e1D9047B42C3fB356FF4680003","decimals":18,"name":"THIBCoin","symbol":"TIB","logo":"http://pix.toile-libre.org/upload/original/1525765045.jpg"}}

So... my thoughts are:

i) If the user tries to import the same Parity Signer account into Fether using "Recover from Recovery Words" when they had already previously imported that account into Fether from Parity Signer and that account is already shown in their account list, then it should display Account 0x... already listed), instead of creating a duplicate account. This will require making updates to apply to "Recover from Parity Signer" similar changes to those that were made in this PR #327.

ii) If the user does not already have a specific Parity Signer account in their Fether account list and they choose to import it using "Recover from Recovery Words" instead of "Recover from Parity Signer", do we still consider it as being "Parity Signer"-originated and show the Parity Signer logo embedded in the account identicon? I don't think we should.

I think we should somehow communicate to the user that if they don't import an account that they created with Parity Signer using "Recover from Parity Signer" that it won't have the Parity Signer logo embedded in the account identicon...

I also think that if they forget to import an account that they created with Parity Signer using "Recover from Parity Signer", they may find themselves upset that they don't have the Parity Signer logo embedded in the account identicon. But there's currently no way to delete their account from Fether and then re-import it again but using "Recover from Parity Signer" so that they get the Parity Signer logo embedded in the account identicon. So I think we need a Fether feature to Delete Account just like we have in Parity Signer (once we've got a Fether menu #264 (comment))

[axelchalon]

Thanks for the write-up Luke,

could we also show what network it is associated with just like we do in Parity Signer and colour-code it?

@parity/qr-signer returns us the address and chainId when scanning the account, so this can be done. However I think it might be easier to simply show the Signer account in the accounts list if we're connected to the right chain (we might want to show a warning at one point if we import a Signer account for a chainId other than the chain we're currently connected to -- otherwise the user might be surprised not to see their imported signer account show up in the accounts list after completing the import). I completely forgot to scope the Signer accounts to their chainId; fixed it in the latest commit (also fixed being able to import the same (address, chainId) twice). delete localStorage['localforage/__paritylight::paritySignerAccounts'] to reset and test since the new commit!

6. Error with an unknown cause

also happened to me. I haven't checked if it prevents us form scanning?

7. Unable to Send Transaction using Fether from an account imported from Parity Signer

I don't think I can reproduce; or the issue specifically linked to transferring across chains? I'd be surprised because the account addresses all use the same format afaik?

8. Gap between Identicon and Account name

it is actually related to this PR; added some quickfix for the margin.

[axelchalon]

@ltfschoen I fixed being able to import an existing account with Signer; now if I import a Parity Signer account with QRCode in Fether then try to reimport it using the passphrase it tells me "account already exists". Same if I import with recovery phrase on fether then try to reimport again with qrcode 👍

ii) If the user does not already have a specific Parity Signer account in their Fether account list and they choose to import it using "Recover from Recovery Words" instead of "Recover from Parity Signer", do we still consider it as being "Parity Signer"-originated and show the Parity Signer logo embedded in the account identicon? I don't think we should.

no, this is unintended behaviour, linked to the lack of verification that the account already exists. fixed now

[axelchalon]

TODO:

• Fix refeshing
• Fix webcam bars (tracking Remove margin around video parity-js/qr-signer#39)
• Fix occasional bug when submitting the TxForm "Cannot read toNumber of null" because somehow gasPrice is sometimes undefined when submitting the TxForm, not sure why (dedicated issue: Unhandled Rejection (Error): Unable to estimate gas, when trying to send transaction with bad connectivity #354)

[Tbaut]

some nits with the buttons that.

[ltfschoen]

9. Handling a webcam hardware error

I just pulled the latest changes from this branch git pull --rebase origin ac-signer.
I then started it up yarn; yarn start.
I then went to the recover from QR code page and it displayed the following:

I killed it and restarted again with yarn; yarn start, but it still occurred.
So I added console.log('Unknown error: ', e); in the switch statement where it's printing out "Unknown error." and found that the error name is NotReadableError

I think it's because the FaceTime HD Camera (built-in) webcam on my MacBook Pro encountered an error, because when I try to create a New Movie Recording in QuickTime it says it cannot use the webcam. Hopefully I just have to restart the computer.
UPDATE: Yes, after restarting my computer the webcam started working again and the error no longer appeared

So I think we need to update the default case in the switch statement to perform a console.log for unknown error reporting. And to add an additional case with a message similar to the following:

case 'NotReadableError':
  errorMessage = 'Webcam hardware error. Try restarting your computer';
  break;

[ltfschoen]

10. Backup to JSON file of an account that was created with Parity Signer but imported into Fether using "Recover from Seed Phrase"

I just created an account using Parity Signer.

But I wanted to have the option of either:

• Recovering the account using the recovery words (seed phrase), OR
• Recovering the account from a JSON file backup.

Now I know that if I imported the account into Fether using "Recover from Parity Signer" then if I went to that account's page in Fether it wouldn't show the option to "Backup Account" so I could save it to a JSON file as an additional backup option

So instead of importing the account into Fether using "Recover from Parity Signer", I chose to "Recover from Seed Phrase", and it successfully imported the account (but it does not show the Parity Signer logo embedded into the identicon).

In the Fether, on the Account List I clicked the account to take me to the Account page, where it displayed the "Backup Account" page, and I got all excited! So I downloaded the JSON file backup for the account.

So to I assumed that I could import the JSON file since it's worked in the past, but I wanted to double check. So I navigated to cd ~/Library/Application Support/io.parity.ethereum/keys/kovan, and viewed the accounts by creation date ls -al, and temporarily moved the most recently created one into a ./temp subdirectory. i.e. mkdir -p ./temp && UTC--2019-01-12T00-18-13Z--6fa...5fc ./temp. Then it was no longer listed in the Fether account list.

Then in Fether I went to "+ > Import account > Recover from JSON Keyfile" and chose the JSON file that was downloaded. It successfully imported the account.

Why don't users who import an account they created with Parity Signer using "Recover from Parity Signer" get the same privilege as users who import it with "Recover from Seed Phrase" instead, such that they are able to backup the account to a JSON file if they want to?
Note: I think we should allow users to backup their account to a JSON file even if they have been imported from Parity Signer

[ltfschoen]

11. TypeError: Invalid hex string, when accidently scanned the sending accounts QR code instead of the signed transaction's QR code

I just tried sending some Ether from one imported Parity Signer account to another imported Parity Signer account.

I entered 0.01 ETH and the recipient, and clicked "Send".

It prompted me to scan the QR Code of the transaction on Parity Signer, so in Parity Signer I clicked "Scanner", and aimed the mobile phone's camera at the transaction QR code shown in the Fether window. Parity Signer then showed me the "Sign Transaction" screen with a "Sign Transaction" button.

I then clicked the "Next Step" button in Fether.

And was prompted to show the QR code of the signed transaction on the webcam.

But I somehow accidently clicked the wrong button in Parity Signer and lost the screen that had the QR code for the transaction.

So as a new user who doesn't read instructions carefully (and also trying to deliberately do the wrong/clumsy/confused/panicy thing that new users may do) I went back to the account that I was sending it from and clicked the "Show Account QR Code" button (when I should have just clicked the "Scanner" button an it would have taken me back to the screen with the "signed transaction" QR code to sign the transaction).

I then tried aimed the "account" QR code at the webcam (instead of the "signed transaction" QR code), and it displayed the following error in Fether window:

×
TypeError: Invalid hex string
hexWrite
src/paritytech/fether/node_modules/buffer/index.js:852
  849 | 
  850 | 
  851 | var strLen = string.length;
> 852 | if (strLen % 2 !== 0) throw new TypeError('Invalid hex string');
      | ^  853 | 
  854 | if (length > strLen / 2) {
  855 |   length = strLen / 2;
View compiled
Uint8Array.write
src/paritytech/fether/node_modules/buffer/index.js:925
  922 | for (;;) {
  923 |   switch (encoding) {
  924 |     case 'hex':
> 925 |       return hexWrite(this, string, offset, length);
      | ^  926 | 
  927 |     case 'utf8':
  928 |     case 'utf-8':
View compiled
fromString
src/paritytech/fether/node_modules/buffer/index.js:245
  242 | 
  243 | var length = byteLength(string, encoding) | 0;
  244 | that = createBuffer(that, length);
> 245 | var actual = that.write(string, encoding);
      | ^  246 | 
  247 | if (actual !== length) {
  248 |   // Writing a hex string, for example, that contains invalid characters will
View compiled
from
src/paritytech/fether/node_modules/buffer/index.js:139
  136 | }
  137 | 
  138 | if (typeof value === 'string') {
> 139 |   return fromString(that, value, encodingOrOffset);
      | ^  140 | }
  141 | 
  142 | return fromObject(that, value);
View compiled
Function.push.../../node_modules/buffer/index.js.Buffer.from
src/paritytech/fether/node_modules/buffer/index.js:155
  152 | 
  153 | 
  154 | Buffer.from = function (value, encodingOrOffset, length) {
> 155 |   return from(null, value, encodingOrOffset, length);
  156 | };
  157 | 
  158 | if (Buffer.TYPED_ARRAY_SUPPORT) {
View compiled
signTransactionWithSignature
src/paritytech/fether/packages/fether-react/src/utils/transaction.js:170
  167 | export const signTransactionWithSignature = (thisTx, signature) => {
  168 |   const tx = getEthereumTx(thisTx);
  169 | 
> 170 |   const sigBuf = Buffer.from(signature.substr(2), 'hex');
  171 | 
  172 |   // Mimicking the way tx.sign() works
  173 |   let v = sigBuf[64] + 27;
View compiled
(anonymous function)
src/paritytech/fether/packages/fether-react/src/stores/sendStore.js:120
  117 |  */
  118 | @action
  119 | signRaw = signature => {
> 120 |   const signed = signTransactionWithSignature(this.tx, signature);
      | ^  121 | 
  122 |   this.tx.rawSigned = '0x' + signed.toString('hex');
  123 | };
View compiled
▶ 2 stack frames were collapsed.
ScanSignedTx._this.onScanSignedTx [as onScan]
src/paritytech/fether/packages/fether-react/src/Send/ScanSignedTx/ScanSignedTx.js:33
  30 |   token
  31 | } = this.props;
  32 | 
> 33 | signRaw('0x' + signature);
     | ^  34 | 
  35 | history.push(
  36 |   `/send/${token.address}/from/${accountAddress}/signedtxsummary`
View compiled
▶ 2 stack frames were collapsed.
This screen is visible only in development. It will not appear if the app crashes in production.
Open your browser’s developer console to further inspect this error.

And in the Dev Tools Console it showed:

I think we should show just show an error on the page if the QR code that is read is incorrect, and remind them that it should be the signed transaction QR code, not an account QR code or some other random one.

[ltfschoen]

12. Broken 'Details' button

See this issue that relates to this PR #354

[Tbaut]

10. Why don't users who import an account they created with Parity Signer using "Recover from Parity Signer"...

This is the whole point of Parity Signer, nobody else than the phone has the private key. The JSON is the (encrypted) private key. Fether doesn't have it for such accounts.

[axelchalon]

it looks like the reset/refreshing bug is fixed thanks to Amaury's refactoring suggestion 🎉

[axelchalon]

I'll just update the qr-signer dependency to get rid of the bar around the webcam and I think this is good to merge

[Tbaut]

Yes yes yes, tested and approved 🎉

[ltfschoen]

I've gone through all my previous comments and checked to see if they've been fixed (able to close my dot points 1, 2, 4, 5, 8, 10). Here are the results using Fether generated Parity Ethereum node using latest from this branch:

• Tested that can send ETH from an account created without Parity Signer on Kovan network to another account account that was created using Parity Signer on Mainnet or Ropsten network

• Tried importing a Parity Signer account that had already been imported both using Recover from Parity Signer and using Recover from Seed Phrase and the functionality works for both

• If I move all the keys out of this folder ~/Library/Application Support/io.parity.ethereum/keys/kovan, all the Parity Signer accounts that I imported are the only ones shown in the Fether account list (since they are only stored in Local Storage)

• Local storage values of an account imported from Parity Signer has the correct network name since they are being stored using a chainId associated with the account. And if I add tokens using a Kovan network account it shows ::kovan in the Local Storage entry key

i.e.

[{"address":"0x00d7bb9dbd761fdf00d78f74710ad6bdcaed8d48","name":"test3-mainnet","chainId":1},{"address":"0x00ca490242abb36cfad5697bafa69fbd1824389d","name":"test4-ropsten","chainId":3},{"address":"0x000ec8c79dac74d13b7e4743ae52f78d7186ba6d","name":"test2-kovan","chainId":42}]

• I was able to successully send a transaction from an Kovan network account imported from Parity Signer using QR codes to sign the tx to a non-Parity Signer generated account

• Tested that if the user does not already have a specific Parity Signer account in their Fether account list and they choose to import it using "Recover from Recovery Words" instead of "Recover from Parity Signer", it no longer shows the Parity Signer logo embedded in the account identicon since it's not considered "Parity Signer"-originated. They are able to Backup Account to JSON file

• "9) Handling a webcam hardware error" appears to have been addressed

• "11) TypeError: Invalid hex string, when accidently scanned the sending accounts QR code instead of the signed transaction's QR code" is resolved, as it now displays "The QR code doesn't appear to be a valid transaction"

• "12) Broken 'Details' button" is fixed

• Since I am no longer able to list non-"Kovan" network accounts in the Fether account list, I am no longer able to encounter the issue that occurred when trying to transfer a value across chains that I mentioned in "7) Unable to Send Transaction using Fether from an account imported from Parity Signer"

• Tested that can send ETH from account created with Parity Signer on Ropsten network to another account created with Parity Signer on Mainnet. After entering amount and recipient address, it showed QR code and when scanned with Parity Signer it showed error in Parity Signer (where account 00ca49...69f is the Parity Signer on Ropsten network account):

• After trying to import an account that already exists the notification "Account xxx already listed" should be centred

• When I try to create any new account in Parity Signer it doesn't show any Recovery Words anymore, the area is blank. I tried restarting Parity Signer but the problem still persists. I haven't tried reinstalling Parity Signer yet (as I'd probably have to add all the accounts again)

Update: This is unrelated to this PR. I've restored an old issue in Parity Signer here novasamatech/parity-signer#127 (comment)

• If I remove a Parity Signer "Mainnet" account from the account list in Fether by updating Local Storage by removing the key/value localforage/__paritylight::tokens::<ACCOUNT_ID> for the account address entry and removing the account from the array under the key localforage/__paritylight::paritySignerAccounts. And then in Fether Recover from Parity Signer with QR code, it adds it again to the array in Local Storage [...., {"address":"0x00d7bb9dbd761fdf00d78f74710ad6bdcaed8d48","name":"test3-signer-mainnet","chainId":1}], but it doesn't display in the Fether account list again, even after refreshing.

So I cleared all the key/value pairs from Local Storage and refreshed Fether and it displayed the T&Cs page, then when I pressed Accept, it showed the following error:

When I refreshed again it went to T&Cs page again and clicked Accept it and it went to the account list page ok. So now I didn't have any accounts listed in the account list page.

I tried to import the Parity Signer "Mainnet" account again using Recover from Parity Signer, but although it did add it to Local Storage as shown below, it didn't appear in the account list page again, perhaps because it's now stored with chainID: 1 (mainnet)

So I decided to start a Parity Ethereum node on "Mainnet" separately ./target/release/parity --chain ethereum --light and then restart Fether to see if only that Parity Signer "Mainnet" account that I had imported would be shown in the account list, but when Fether loaded it showed the T&Cs page, then when I clicked "Accept" it showed error below.

This is the same as the error that occurs if you try to run a 2nd Parity Ethereum instance. See #333

If I clicked the "X" button it went back to T&Cs page, and this time when I clicked "Accept" the UI went blank, even after refreshing, and in the Dev Tools console it displayed:

So I went back to starting a Parity Ethereum node on "Kovan" separately ./target/release/parity --chain kovan --light and then restart Fether to try and get it working again, but when Fether loaded it showed a blank screen, even after refreshing, and it showed the same chainId error in Dev Tools console.

So as a last resort I just started Fether by itself so it would generate the Parity Ethereum node, and that worked as before.

So it looks like Fether is crashing now when you run a separate Parity Ethereum Light node even with the Kovan network before starting Fether. Perhaps it's trying to create a 2nd node?. Note that I tried starting it with Parity Ethereum version Parity-Ethereum/v2.1.10-stable-b47e064f8-20181213/x86_64-macos/rustc1.31.0, which worked previously in 'master' branch.

But it is correctly not listing accounts that were created for non-Kovan networks with Parity Signer that the user has imported into Fether (since Fether generates only a Kovan network node at the moment)

• regarding "5) Importing Account in Fether using the passphrase that was generated when I created an in Parity Signer (Continued)".
  How are we communicating/warning the user that if they don't import an account that they created with Parity Signer using "Recover from Parity Signer" that it won't have the Parity Signer logo embedded in the account identicon?
  And how are we communicating/warning the user that if they don't import an account that they created with Parity Signer using "Recover using Seed Phrase" that it won't give them the option to Backup Account to a JSON file (so they know they have that option available too)?

• If the user forgets to import an account that they created with Parity Signer using "Recover from Parity Signer", they may find themselves upset that they don't have the Parity Signer logo embedded in the account identicon and want to re-import it again using "Recover from Seed Phrase" instead, but we don't have a Delete Account option in Fether yet.
  Or if they import an account that they created with Parity Signer using "Recover from Parity Signer", but they wanted the ability to Backup Account, there isn't a Delete Account option in Fether to allow them to re-import it using "Recover from Seed Phrase" instead so they have that option available.
  So I think we need a Fether feature to Delete Account just like we have in Parity Signer (once we've got a Fether menu Design brainstorming - account screen #264 (comment)). Should we create an New Issue for this?

• If I try to send 0.0005 ETH from a Parity Signer imported account in Fether to another account, it prompts me to scan the QR code of the transaction on Parity Signer, which I did, and in Parity Signer it shows "Transaction Details" of 0.0005 ETH from 0x000ec8... to 0.005072... (both on Kovan). If I then click "Next Step" in Fether, it asks me to show the QR code of the signed transaction on the webcam, which I would get by clicking "Sign Transaction" in Parity Signer.
  But instead I decide to scan a QR code signature from a "previous" transaction of only 0.001 ETH.
  In Fether it then takes me to the following screen where it still shows I want to send 0.0005 ETH, and a "Send" button.

If I click "Send" it signs it and shows "Sending your transaction...". It's taking a long time and I still haven't received any block confirmations...
But anyway, the purpose was just to check if 0.001 ETH or 0.0005 ETH gets transferred at https://blockscout.com/eth/kovan/address/0x005072fb985cc64d8a1ce00ece26279f73c12f8a/transactions. What amount do you think should be transferred and if the signature QR that I used was associated with a previous transaction to a different recipient would it send it to them, or the recipient that was entered in the input field in Fether?

If I wanted to repeat the same transaction amount multiple times to the same recipient could I just ignore scanning the QR code shown by Fether and skip to the next step and then just show the same screenshot of the original signature QR code that I saved to my device?

• regarding "6) Error with an unknown cause", should we create a New Issue? I'm not sure how to reproduce this again though...

[Tbaut]

I see no issue left in any of the points raised @ltfschoen (except the last one, "6) Error with an unknown cause", if you can reproduce.).

The centring of the error message was known to me but doesn't apply to this PR.

If the code is ok, I'm good to merge.

[ltfschoen]

I see no issue left in any of the points raised @ltfschoen (except the last one, "6) Error with an unknown cause", if you can reproduce.).

The centring of the error message was known to me but doesn't apply to this PR.

If the code is ok, I'm good to merge.

Yes I don't see any issues merging it. It'd be good to get some quick feedback on the "unchecked" checkboxes though (i.e. whether a separate issue should be raised after this PR is merged, or whether I've potentially misunderstood something, or ..). Sorry it's a bit long-winded but wanted to try and cover all bases

[Tbaut]

Sure, here's my feedback:

And how are we communicating/warning the user that if they don't import an account that they created with Parity Signer using "Recover using Seed Phrase" that it won't give them the option to Backup Account to a JSON file (so they know they have that option available too)?

I think this should be understood separately from Fether and as part of the signer workflow. A user installing and using Signer will know why it does this. Recovery is not something users do every day.

Importing Account in Fether using the passphrase that was generated when I created an in Parity Signer

if you import the account with the recovery phrase, then it'll be in Fether, and since the recovery passphrase = private key, Signer has no role to play anymore. It's normal not to have the Signer logo.

If I wanted to repeat the same transaction amount multiple times to the same recipient could I just ignore scanning the QR code shown by Fether and skip to the next step and then just show the same screenshot of the original signature QR code that I saved to my device?

This wouldn't work and this is what transaction nonces are made for, to avoid replay transactions.

[amaury1093]

Code lgtm! Feel free to merge.