Cleanup and refactor - version 3.0.0 #166
Conversation
|
Since this is a big update I'd like some eyes on it before I merge it in. Hopefully in a week or two it should be good to go. |
example/webwallet.html
Outdated
| @@ -103,7 +110,7 @@ | |||
| function showSeed() { | |||
| var password = prompt('Enter password to show your seed. Do not let anyone else see your seed.', 'Password'); | |||
|
|
|||
| lightwallet.keystore.deriveKeyFromPassword(password, function(err, pwDerivedKey) { | |||
| global_keystore.keyFromPassword(password, function(err, pwDerivedKey) { | |||
| var seed = global_keystore.getSeed(pwDerivedKey); | |||
| alert('Your seed is: "' + seed + '". Please write it down.') | |||
There was a problem hiding this comment.
This line and a few others below have missing ;
There was a problem hiding this comment.
I'll probably do like a linting with standard-js at some point, but for now I'll follow your recommendation here! :)
| throw new Error("Incorrect derived key!"); | ||
| } | ||
| function addressToPublicEncKey (keystore, pwDerivedKey, address) { | ||
| var privKey = keystore.exportPrivateKey(address, pwDerivedKey) |
| @@ -123,7 +107,7 @@ var multiEncryptString = function (keystore, pwDerivedKey, msg, myPubKey, theirP | |||
| encryptedSymKey = [] | |||
There was a problem hiding this comment.
This should be named encryptedSymKeys and encSymKey you can either name it key because of the context the reader can understand it's an encrypted key, or encryptedSymKey (singular).
How it is now it's a little confusing because encryptedSymKey and encSymKey means basically the same thing.
There was a problem hiding this comment.
This is a good point, naming things is hard! :)
There was a problem hiding this comment.
@dekuyper This is a bit confusing. The reason it's called encryptedSymKey is that every entry in the list is an encrypted version of the same symmetric key. So there is only one symmetric key involved and it's encrypted to different counterparties. So I haven't been able to come up with a better name since encryptedSymKeys makes it sound like there are several symmetric keys involved.
There was a problem hiding this comment.
Also I'm a bit reluctant to change the name since it affects the serialized output.
lib/txutils.js
Outdated
| @@ -15,11 +17,25 @@ function add0x (input) { | |||
| } | |||
| } | |||
|
|
|||
| function strip0x (input) { | |||
| if (typeof(input) !== 'string') { | |||
There was a problem hiding this comment.
You can just use the if statement here, it makes it easier to read.
And if someone calls this function passing something else than a string you should throw an Error.
For ex:
if (typeof(input) !== 'string') {
throw new Error('Parameter is not a string');
}
if (input.length >= 2 && input.slice(0,2) === '0x') {
return input.slice(2);
}
return input;
There was a problem hiding this comment.
Good point, I’ll add this update as well.
|
Thanks @dekuyper for the review! I’ll make updates with your suggestions and try to merge this in soon! |
This is a big release with the following highlights:
Major cleanup - you'll most likely need to update your code if you use lightwallet.
Remove legacy constructor - now only
createVaultis supported. AlsocreateVaultnow require seed and hd path as inputs.Remove
deriveKeyFromPasswordfunction in favor ofkeyFromPasswordfunction.Remove special handling of encryption keys in the keystore. You can still use keystore keys to encrypt, but they no longer use pubkeys to index. To get the pubkey corresponding to an address, please use the function
encryption.addressToPublicEncKey.Make the keystore and interfaces simpler by only allowing one
hdPathString. If you need to derive from more HD paths you need to create more keystores.Add
0xprefix for all addresses and transaction hex data.Remove unneeded
bitcore-libpackage dependency. Thanks to Srirangan.