Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Key derivation in Worker #4071

Merged
merged 6 commits into from Jan 9, 2017
Merged

Key derivation in Worker #4071

merged 6 commits into from Jan 9, 2017

Conversation

ngotchac
Copy link
Contributor

@ngotchac ngotchac commented Jan 6, 2017

Closes #3233

Also display the password hint if found in the key.

To test :

  1. Create a new account
  2. Send some eth to the new account
  3. Create a transaction from the new account sending the whole balance (or whatever), but don't validate it
  4. Move the key from ~/.parity/keys/test/.... to somewhere else
  5. Reload the UI
  6. The transaction is still there, waiting for the right key

@ngotchac ngotchac added A0-pleasereview 🤓 Pull request needs code review. M7-ui labels Jan 6, 2017
// make sure to display some kind of "in-progress" state.
return signerPromise
.then((signer) => {
return noncePromise
Copy link
Contributor

Choose a reason for hiding this comment

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

You may want to use Promise.all here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍


// Setup the Service Worker
if ('serviceWorker' in navigator) {
workerRegistration = runtime
Copy link
Contributor

Choose a reason for hiding this comment

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

You can return here directly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep that's right (wasn't in a function previously...)

const state = getState();
const stateWorker = state.worker.worker;

if (stateWorker !== undefined && !(stateWorker && stateWorker._worker.state === 'redundant')) {
Copy link
Contributor

@derhuerst derhuerst Jan 6, 2017

Choose a reason for hiding this comment

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

state.worker.worker might also be null, judging from the reducer.

if (stateWorker && stateWorker._worker.state !== 'redundant')?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is just because worker can be undefined if it hasn't loaded yet, or null if not supported

Copy link
Contributor

Choose a reason for hiding this comment

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

So my proposal would work?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well it wouldn't take into account when stateWorker is null.


const { worker } = store.getState().worker;

const signerPromise = worker && worker._worker.state === 'activated'
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not store the worker object as state.worker itself?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because it also have an error field that gets displayed in Contract Dev. Could get rid of it though

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(We might want to add other fields, like stats, ...)

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.002%) to 86.548% when pulling e547636 on ng-signer-worker into 9c00eb4 on master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 86.55% when pulling e547636 on ng-signer-worker into 9c00eb4 on master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.04%) to 86.514% when pulling e547636 on ng-signer-worker into 9c00eb4 on master.

@jacogr jacogr added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Jan 9, 2017
@gavofyork gavofyork merged commit 40f0ee0 into master Jan 9, 2017
@gavofyork gavofyork deleted the ng-signer-worker branch January 9, 2017 10:14
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants