Use blockstack.js W3C crypto #1966
Conversation
….js: update to latest bip39 (had breaking async changes) for baseline before migrating to blockstack.js
|
@zone117x Does this work require that we update all the |
…t are no longer needed
There was a problem hiding this comment.
Overall these are fantastic changes. I was a bit intimidated by the number of files changed, but the vast majority are simple changes due to removing Flow and @utils/@common resolvers.
The actual crypto code looks sound to me, and is familiar since I had to do some similar migration away from the HDNode interface for @blockstack/keychain.
I am curious about where exactly we're getting the W3C crypto polyfill. Is it because of the Babel changes, which include core-js? I didn't see anything in core-js about W3C crypto specifically. Or, are they just coming from blockstack.js?
I am impressed and grateful for this PR. I haven't finished a full functional QA yet, but there are tons of QOL improvements in this code.
|
|
||
| import { HDNode } from 'bitcoinjs-lib' | ||
| import { bip32 } from 'bitcoinjs-lib' | ||
| import * as bip39 from 'bip39' |
There was a problem hiding this comment.
I had to do a bunch of these changes as well in @blockstack/keychain. I'm not sure if it's worth more refactoring to bring that package in here (maybe that would help QA that package) but it's worth keeping in mind.
| } from '@utils' | ||
| import { isCoreEndpointDisabled } from '@utils/window-utils' | ||
| } from '../../../utils' | ||
| import { isCoreEndpointDisabled } from '../../../utils/window-utils' |
There was a problem hiding this comment.
Another point for the future (if ever) - since packaging a core node is gone, we could probably remove this and other code references.
| const dataBuffer = Buffer.from(encryptedBackupPhrase, 'hex') | ||
| const { password } = this.state | ||
|
|
||
| const updateProfileUrls = localIdentities.map((identity, index) => |
There was a problem hiding this comment.
Good refactor - this file could certainly use more of it
|
|
||
| // Stub out workers | ||
| const workers = ['encrypt', 'decrypt', 'crypto-check'] | ||
| const workers = [] |
There was a problem hiding this comment.
Were these not necessary because of the webpack Workbox plugin?
| ] | ||
| }, | ||
| { | ||
| test: /\.worker\.js$/, |
There was a problem hiding this comment.
Ah, so we just aren't using Web Workers anymore, since these are now async?
There was a problem hiding this comment.
Correct. Should I get rid of the rest of the workerize related dependencies and setup?
There was a problem hiding this comment.
It wouldn't hurt to remove the dependency - is there other setup I'm missing?
|
Functional QA I'm doing:
Signing in with my magic recovery code still seems kind of slow and UI blocking - though not completely. Might have just been a one-off. Signing up was quick and definitely not blocking. Github social proofs didn't work, but they're probably already broken? I know we changed the |
|
I'm seeing a weird issue with sending a BTC payment. I have $30 USD in my wallet, about 0.004 BTC, and no matter how much I try and send, I get "Not enough coin to fund fees transaction fees. Fees would be 5824, specified spend is 10". I see this comes from |
The W3C crypto usage comes from blockstack.js, in this PR hirosystems/stacks.js#737 It's not really a |
This PR drastically speeds up the user registration and restoration process, by using the native Web Crypto operations in
blockstack.js v21-alphahirosystems/stacks.js#691.Some sample benchmarks using a Pixel 3 XL, on both Chrome and Firefox:
browser.blockstack.org.Additional performance testing is needed with lower-end Android phones, and regression testing is needed on all platforms.
This PR has an absurd changeset from the process of trying to debug errors while getting this working. Notably, all the synchronous code that needed to be refactored into asynchronous.
Debugging capabilities essentially were non-existent -- like readable stack traces, breakpoints in mocha tests, breakpoints within Chrome, etc.
Some of these notable tangental changes are:
Now that the material code that actually needs changed is known, it is possible to cherrypick out only those changes. Let me know if we want to try that.