Electron TREZOR Support #1946
Merged
Electron TREZOR Support #1946
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
wbobeirne
added
the
status: ready for review
wbobeirne
commented
Jun 13, 2018
| const { chainId, ...strTx } = getTransactionFields(tx); | ||
| const txFields = getTransactionFields(tx); | ||
|
|
||
| if (process.env.BUILD_ELECTRON) { |
There was a problem hiding this comment.
It would be great to make a class that extends HardwareWallet that does this for you. @HenryNguyen5 how would you suggest this best be done?
There was a problem hiding this comment.
I'd probably just make a utility HOF instead that does the branching for you
package.json
Outdated
| @@ -89,6 +91,7 @@ | |||
| "@types/webpack-env": "1.13.4", | |||
| "@types/zxcvbn": "4.4.0", | |||
| "autodll-webpack-plugin": "0.3.9", | |||
| "bchaddrjs": "0.2.1", | |||
There was a problem hiding this comment.
Note to self, this was fixed with trezor/trezor.js#58 so I can remove this.
dternyak
approved these changes
Jun 15, 2018
dternyak
pushed a commit
that referenced
this pull request
Jun 15, 2018
* Initial scaffold of enclave * Cleanup types * Add comments * Do not truncate errors, pretty output * Introduce helpers for sagas * Update yarn lock * Convert enclave into its own lib. Implement client and server. * Check in progress * Initial types * Remove unused lib * Finish types * cleanup * Switch over to using electron protocol, remove code thats no longer necessary * Refactor Ledger and Trezor wallets to provide all functionality via libs. Run chain code generation thru Enclave. * Check in trezor work * Transaction signing * Message signing * Display address * Fix deallocation of trezor * Adjust API * Remove unused getAddresses * Fix imports and filenames to cooperate with internal typings * Fix type uncertainty * Add persistent message to Ledger unlock. * Update ledger help link to kb * Convert ledger over to updated libs * Fix jest config * Enclave README * Unnecessary assertion * Adjust tip * Type ledger errors * Reduce enclave client code. * No default exports * l18n user facing enclave errors * Reduce repeated enclave code by splitting it into its own wallet lib. Fix some types * tslint * Reduce repeated enclave code by splitting it into its own wallet lib. Fix some types and error messages. * Electron TREZOR Support (#1946) * Type trezor connect. * Check in trezor code * Implement TREZOR wallet * Convert TREZOR to use enclave class like Ledger. * Switch to mycrypto fork of trezor lib. Remove unused dependencies. * remove unnecessary window attachment * tslint
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Builds off of #1836, finishes the other half of #1621.
Description
Implements the TREZOR side of hardware wallet integration in Electron using the enclave library introduced in #1621. Similar to that PR, there's an unfortunate amount of code duplication between the two wallet implementations. Any feedback on how to better unite those would be appreciated.
See "Steps to Test" for note about pending PR. I have personally tested that this all works.
Changes
TrezorConnect.)Steps to Test
NOTE: This is still pending on trezor/trezor-link#9. If you wish to test this PR locally, you will have to manually make the changes proposed their to
node_modules/trezor-link/lib/http.js.