[wallet]Store encrypted local signing key #1188
Conversation
ashishb
force-pushed
the
ashishb/web3_local_signing_encrypt_password
branch
from
8409590 to
989dd42
Compare
Codecov Report
@@ Coverage Diff @@
## master #1188 +/- ##
=========================================
- Coverage 66.16% 66.1% -0.07%
=========================================
Files 261 261
Lines 7599 7618 +19
Branches 508 440 -68
=========================================
+ Hits 5028 5036 +8
- Misses 2474 2487 +13
+ Partials 97 95 -2
Continue to review full report at Codecov.
|
| @@ -230,8 +235,34 @@ export async function readPrivateKeyFromLocalDisk( | |||
| ): Promise<string> { | |||
| const filePath = getPrivateKeyFilePath(account) | |||
| Logger.debug('readPrivateKeyFromLocalDisk', `Reading private key from ${filePath}`) | |||
| // TODO(ashishb): Read and decrypt private key instead | |||
| return RNFS.readFile(getPrivateKeyFilePath(account)) | |||
| const hexEncodedEncryptedData: string = await RNFS.readFile(filePath) | |||
There was a problem hiding this comment.
Generally, these explicit typings (varName: type) aren't necessary unless the function/object your calling is missing a type, which usually shouldnt be the case
There was a problem hiding this comment.
It is not necessary, but I find it helpful when there is a mix going on.
const hexEncodedEncryptedData: string = await RNFS.readFile(filePath)
const encryptedDataBuffer: Buffer = new Buffer(hexEncodedEncryptedData, 'hex')
const privateKey: string = getDecryptedData(encryptedDataBuffer, encryptionPassword)
| Logger.debug('savePrivateKeyToLocalDisk', `Writing private key to ${filePath}`) | ||
| // TODO(ashishb): Store encrypted private key instead | ||
| await RNFS.writeFile(getPrivateKeyFilePath(account), privateKey) | ||
| const plainTextData = privateKey |
There was a problem hiding this comment.
nit: why not skip the variable assignment and just pass privateKey into the function
There was a problem hiding this comment.
getEncryptedData(privateKey, encryptionPassword) felt a bit confusing to read.
| // TODO(ashishb): Read and decrypt private key instead | ||
| return RNFS.readFile(getPrivateKeyFilePath(account)) | ||
| const hexEncodedEncryptedData: string = await RNFS.readFile(filePath) | ||
| const encryptedDataBuffer: Buffer = new Buffer(hexEncodedEncryptedData, 'hex') |
There was a problem hiding this comment.
@jmrossy On the other hand, I find this explicit typing to help with readability in cases that switch between strings/buffers/etc
Some linting rules will prevent explicit typing for common types like numbers, strings (ex: https://palantir.github.io/tslint/rules/no-inferrable-types/) but in the case of less common types I find the explicit typing helpful
Store encrypted local signing key, the encryption password is coming from the Android keystore.
ashishb
force-pushed
the
ashishb/web3_local_signing_encrypt_password
branch
from
83f5cf3 to
6b587f4
Compare
* master: (35 commits) [Wallet] Fix top of emojis cut off in the activity feed (#1243) Adding a contract to store minimum required client version (#1081) Revert "Feature #909 proxy delegatecall (#1152)" (#1241) Use ContractKit to get addresses for Blockchain API (#1175) Feature #909 proxy delegatecall (#1152) Fix Faucet done message (#1217) Updated SETUP.md with new yarn process (#1224) Adding `increaseAllowance` and `decreaseAllowance` methods (#1196) extracting function signatures (#1061) Fix integration hardcode (#1208) Fixing flaky governance test (#1155) Restore CI branch (#1223) [wallet] e2e back to green (#1210) [Wallet] Implement new import wallet flow designs (#1209) [Wallet] Fix disable conditions for butons on Enter Invite screen (#1214) [protocol] Rename infrastructureFraction to proposerFraction (#1174) [ck] Proper promise treatment to avoid UnhandledPromises (#1219) [ck] Transform StableToken parameters from fixidity format (#1218) [wallet]Store encrypted local signing key (#1188) 2019-10-03 alfajores deployment (#1200) ...
Description
Store encrypted local signing key, the encryption password is coming
from the Android Keystore.
Tested
Unit test
$ node_modules/.bin/jest src/web3/saga.test.tsManual test
DEFAULT_SYNC_MODE=-1inpackages/mobile/.envpackages/mobile $ yarn run dev. Log in verify that you can send money./data/data/$APP_ID/files/private_key_for_<address>.txtare encrypted hex-encoded key8672a59c5d55302a5d64288084137303f5cd431abbff3d1edeca4b1c9f8266c41148d712c99d26b52165a6426b048ab1710c6311154c66e75e071386538c7a0091a920d1167d729a8916ad646099adfdRelated issues
Backwards compatibility
Not backward compatible but this code path was added yesterday (Oct 1) and is still behind a flag. So, it is not an issue.