Bring core-js into erdjs. Browser tests improvements. #97
Conversation
andreibancioiu
commented
Dec 2, 2020
•
andreibancioiu
commented
- Bring core-js into erdjs (user wallets & signing, validator signing).
- Run all tests (unit and integration) in browser, as well.
- Separate builds: erdjs with / without wallet components.
andreibancioiu
changed the title
andreibancioiu
changed the title
andreibancioiu
changed the title
| */ | ||
| private static generateDerivedKey(password: Buffer, salt: Buffer, kdfparams: ScryptKeyDerivationParams): Buffer { | ||
| // Question for review: @ccorcoveanu, why not this implementation? | ||
| // https://nodejs.org/api/crypto.html#crypto_crypto_scrypt_password_salt_keylen_options_callback |
There was a problem hiding this comment.
If I remember correctly, because scryptsy works in the browser, does the scrypt in the crypto package work also? We can test/benchmark it and if it does, we can switch to it and remove some extra dependencies
There was a problem hiding this comment.
With the original one, on a i3-8100 CPU @ 3.60GHz:
- 80-90 ms in Node.js
- 350-360 ms in browser (Firefox)
With the one from the crypto package:
- 15-17 ms in Node.js
- ... (not available) ...
... not added by the browserify pipeline: https://www.npmjs.com/package/crypto-browserify.
Therefore, left as it was, until the package mentioned in https://github.com/crypto-browserify/crypto-browserify/issues/181 is included in browserify by default. Currently, it isn't:
https://github.com/browserify/browserify/blob/master/package.json#L36
|
erdjs/src/webSigner.ts can be removed as it is empty. |
| let pair = tweetnacl.sign.keyPair.fromSeed(this.buffer); | ||
| let signingKey = pair.secretKey; | ||
| let signature = tweetnacl.sign(new Uint8Array(message), signingKey); | ||
| signature = signature.slice(0, signature.length - message.length); |
There was a problem hiding this comment.
does it return the concatenated message with sig?
There was a problem hiding this comment.
Yes, tweetnacl.sign does that. I've added a comment.
Should not exist in this branch anymore: https://github.com/ElrondNetwork/elrond-sdk/tree/core-js/erdjs/src |
