20 BTC bounty: Javascript implementation of BIP 38 [claimed]

[casascius]

Bounty has been paid!

[coblee]

I submitted a pull request at https://github.com/pointbiz/bitaddress.org/pull/8 . (Edit: demo page at http://scintill.github.com/bitaddress.org-bip38.html )

I'm open to suggestions on improvements, or requests on other ways to package it up.  The core code is pretty short and simple, but it depends on EC crypto, biginteger, AES, SHA256, scrypt, and Base58 coding, so could need some adjustment if anyone would like to use it outside of the bitaddress.org page.  I may also release my scrypt code separately as it looks like there are no other browser-ready implementations.

Please send the bounty to 1GSo3Z3fgsvUH6yKr6s8kJHMFDWvLEuXjs, and thank you!

Congrats, I was nearly done but had some annoying bug left. I'll check performance in firefox and see if my code happens to do better though. (Unlikely though if your scrypt is also based on this one: https://github.com/cheongwy/node-scrypt-js)

Yes, my scrypt is based on that, replacing the node module dependencies with Crypto-JS functions.  I also used Web Workers to do 2 scrypt threads in parallel.

Cool. How about also adding an option to bitaddress.org's paper wallet to allow generation of the encrypted private key and bitcoin address. I would even go so far as to not show the unencrypted version at all.

[Zeilap]

I submitted a pull request at https://github.com/pointbiz/bitaddress.org/pull/8 . (Edit: demo page at http://scintill.github.com/bitaddress.org-bip38.html )

I'm open to suggestions on improvements, or requests on other ways to package it up.  The core code is pretty short and simple, but it depends on EC crypto, biginteger, AES, SHA256, scrypt, and Base58 coding, so could need some adjustment if anyone would like to use it outside of the bitaddress.org page.  I may also release my scrypt code separately as it looks like there are no other browser-ready implementations.

Please send the bounty to 1GSo3Z3fgsvUH6yKr6s8kJHMFDWvLEuXjs, and thank you!

Congrats, I was nearly done but had some annoying bug left. I'll check performance in firefox and see if my code happens to do better though. (Unlikely though if your scrypt is also based on this one: https://github.com/cheongwy/node-scrypt-js)

Yes, my scrypt is based on that, replacing the node module dependencies with Crypto-JS functions.  I also used Web Workers to do 2 scrypt threads in parallel.

Cool. How about also adding an option to bitaddress.org's paper wallet to allow generation of the encrypted private key and bitcoin address. I would even go so far as to not show the unencrypted version at all.

That's exactly what I want. I will give 10 BTC bounty for someone to create a pull request to modify bitaddress.org to give me an option to create an encrypted paper wallet using BIP 38.
See: https://bitcointalk.org/index.php?topic=139969.0

Done, just cleaning up the code for the pull request. I'll have it ready in a while. The only problem (other than the speed) is that the BIP38 encrypted keys are longer than the height of the pretty notes and the font is already pretty small. The original image for the note needs changing to accommodate. Also needs to change 'Private key' to 'Private key (BIP38 encrypted)' or something. My graphics/css manipulation skills are awful, so I can't really help.

[casascius]

My version splits the key into two lines and says "Password Required".

Ideal to keep them consistent.

[Zeilap]

Please split it into 2 lines like casascius suggested. Are you creating compressed keys or not? Also, make sure you have tests that tests your code.

I had changed the font size slightly and got it to fit while still readable, most will use the QR code rather than trying to punch in 60 random characters anyway.
Anyway, yea I can split it to 2 lines, you want both encrypted and plain-text keys split on 2 lines?

I'm generating non-compressed keys. I guess I should add an option to choose.

Thanks for the reminder about tests!

In the mean time, you can find the code here to play with
https://github.com/Zeilap/bitaddress.org

[casascius]

I'm generating non-compressed keys. I guess I should add an option to choose.

I would suggest always generating non-compressed keys until community-wide support for importing compressed keys is ubiquitous.  The option to compress the keys takes more "bytes" of the user's brain than it saves bytes on the block chain, and isn't worth the user finding out that he "can't" import his key in the venue of his choice because they don't support compressed keys.

[Zeilap]

Done, just cleaning up the code for the pull request. I'll have it ready in a while. The only problem (other than the speed) is that the BIP38 encrypted keys are longer than the height of the pretty notes and the font is already pretty small. The original image for the note needs changing to accommodate. Also needs to change 'Private key' to 'Private key (BIP38 encrypted)' or something. My graphics/css manipulation skills are awful, so I can't really help.

Please split it into 2 lines like casascius suggested. Are you creating compressed keys or not? Also, make sure you have tests that tests your code.

Done. I modified the image to include the word 'Encrypted' below 'Private Key'. Also I changed the colour to blue for encrypted paper wallets to provide distinction between encrypted/unencrypted paper wallets - a version in the original yellow is included in case you really like yellow, just delete 'note_encrypted.png' and rename 'note_yellow.png' in its place.



In case you or anyone else doesn't know, to try it you can simply download the code by clicking the 'Zip' button github, then unzip anywhere you like and double-click 'bitcoinaddress.org.html'.

here's my address for the bounty
1X3XKkw7tnYSQknKPLqLrJkdMzyHoZ7Jf

[Zeilap]

I think blue looks good, but you should use the image data URI scheme to encode the image so that everything is in a self contained html page like the original page.
Also, the unit tests don't seem to run properly.

Since you've completed what I originally asked for, I will release the bounty. But please fix things up and create a pull request for pointbiz.

Unit tests are problematic, I'll explain:
The encryption/decryption is implemented asynchronously, so when the unit tests run, the browser will be trying to perform 6 decryptions and 4 encryptions from the official test vectors and 2 random encryption-decryption cycles, all at once! Also because they are run asynchronous the encryption/decryption tests are still running well after the alert that says all synchronous tests have completed.

There's a warning in the source of the unit tests to say not to run all the tests at once or your browser will probably crash (certainly did on chrome) or take forever on Firefox.

I'll try to implement something to run the call one after another. In the mean time you can comment out some of the tests so that only a few can run. Also if you open the javascript console (in Firefox 'Web developer' menu, Chrome in 'tools' menu) you can see the messages of individual tests passing (they take a while!) - they only alert when they fail.

Also, thanks for the bounty

[casascius]

Here are two things I would strongly push for before considering this worthy for real world use and awarding a bounty:

1. The methodology for creating the keys.  I notice the sample one starts with 6PR.  This indicates the non-EC-multiplication is being used.  This also indicates that the scrypt must be re-run for e-v-e-r-y s-i-n-g-l-e a-d-d-r-e-s-s.  It's going to be really slow to generate a batch.

If you're going to generate a batch of private keys all with the same passphrase, you should use the EC multiplied method, and simply increment the sequence number.  This way you don't have to repeat the scrypt.

2.  The layout of the private key is confusing.  It looks as though there are two fields: "Private key: 6PRblablhablah, Encrypted: blahblahblah".  The average user will think he has two pieces of information, one called "Private key" and one called "Encrypted", this is unduly confusing.  It should be clear to the user that he has a private key that requires a password.  (Encrypted != Requires Password, since password is not the only kind of key usable for encryption)

[scintill]

I think blue looks good, but you should use the image data URI scheme to encode the image so that everything is in a self contained html page like the original page.

This is somewhat off-topic, but has anyone considered refactoring the code to have the pieces split into separate files that are built into one big file for release?  I think it is a bit unmanageable with all the scripts, CSS, base64 images in one huge file, with deeply-nested indentation.  I might have done it earlier, but didn't want to waste the time if pointbiz rejected it for whatever reason.

The encryption/decryption is implemented asynchronously, so when the unit tests run, the browser will be trying to perform 6 decryptions and 4 encryptions from the official test vectors and 2 random encryption-decryption cycles, all at once!

Author of the decryption code for Casascius' bounty here.  I enjoyed having them all run concurrently on my 8-core machine, others may not like it so much.   It should be possible to chain the asynchronous tests so they run serially, and they probably should be chained into the standard tests so the alert at the end can mean the async tests passed too.  (There could also be a flag to force scrypt to run synchronously for the tests, but then I would worry about the web-worker asynchronous version not being tested.)

[Zeilap]

The encryption/decryption is implemented asynchronously, so when the unit tests run, the browser will be trying to perform 6 decryptions and 4 encryptions from the official test vectors and 2 random encryption-decryption cycles, all at once!

Author of the decryption code for Casascius' bounty here.  I enjoyed having them all run concurrently on my 8-core machine, others may not like it so much.   It should be possible to chain the asynchronous tests so they run serially, and they probably should be chained into the standard tests so the alert at the end can mean the async tests passed too.  (There could also be a flag to force scrypt to run synchronously for the tests, but then I would worry about the web-worker asynchronous version not being tested.)

Yea, I've since implemented some asynchronous serialization code for the testing and used your nice busy indicator so that it doesn't appear everything has finished when it has really only just begun.
Had to add a 5 second wait between each BIP test though because the VM is not fast enough at releasing memory from the previous test - repeatedly ate up all 8gig on my machine lol.

[pointbiz]

scintill, I'm in the process of reviewing the original pull request.

I looked at the Crypto-JS dependencies in your pull request and have decided to upgrade the entire Crypto-JS code used in bitaddress.org to v2.5.4
Each file from Crypto-JS now is in it's own html script tag. To make it easier to upgrade in the future and I commented with the version number and original file name.

I still need to merge the Scrypt code and the unit tests and UI etc. In v2.3 I modified the unit test code and how it outputs. Now it's a text area. I'll try and get the async unit tests incorporated with the textarea.

I haven't put much thought into splitting the file into pieces and having a build process. I've just put up the same file on github that I put on the website. In Visual Studio you can collapse script tag blocks so I have chosen to divide the code into blocks, in most cases, that correspond to the files in the external dependencies. That helps me navigate the various parts of the code and maintain them with upstream dependencies.

I'm not sure yet if I'll release your pull request alone and then Zeilaps or both at the same time.

[scintill]

I looked at the Crypto-JS dependencies in your pull request and have decided to upgrade the entire Crypto-JS code used in bitaddress.org to v2.5.4

OK.  Be aware I stripped out some stuff from BlockModes.js that wasn't needed for our purposes, which you may or may not want to do as well.

I haven't put much thought into splitting the file into pieces and having a build process. I've just put up the same file on github that I put on the website. In Visual Studio you can collapse script tag blocks so I have chosen to divide the code into blocks, in most cases, that correspond to the files in the external dependencies. That helps me navigate the various parts of the code and maintain them with upstream dependencies.

There's definitely a simplicity and transparency to having a single file as you have.  I see now why you split them into separate blocks, and that's fine.

[casascius]

I am not sure how well the scrypt will or will not run in various browsers, but I would like to throw out the idea of having it so that if you're using a browser that is unlikely to perform the encryption well or is likely to crash, simply disable the encryption option and say "To enable encryption, please use Firefox or Chrome" etc.

This may not be an issue if the scrypt code can be tuned to run well under other browsers.  If Chrome simply performs much faster, it may be worthwhile to point out that they'll be waiting extra long due to their browser choice, and that if they'll be doing a lot of these, to use Chrome instead.

[scintill]

I am not sure how well the scrypt will or will not run in various browsers, but I would like to throw out the idea of having it so that if you're using a browser that is unlikely to perform the encryption well or is likely to crash, simply disable the encryption option and say "To enable encryption, please use Firefox or Chrome" etc.

This may not be an issue if the scrypt code can be tuned to run well under other browsers.  If Chrome simply performs much faster, it may be worthwhile to point out that they'll be waiting extra long due to their browser choice, and that if they'll be doing a lot of these, to use Chrome instead.

Good idea.  I know IE9 takes two minutes and becomes unresponsive doing a non-EC-multiplied decryption.  It could potentially be more responsive if we chunked up the scrypt loops into separate setTimeout() calls with some delay every now and then, but I'm not sure it's worth the effort.  I haven't tested IE10, but it supports web workers, so I think it will be more responsive and probably faster since it can do 2 threads.

Speaking of crashing the browser, I just tried running my unit tests and Chrome (on Linux) is consistently crashing.  When I changed the BIP38 decryption tests to not run in parallel, it stopped crashing.  It also runs fine doing a manual decryption.  I notice there's been a Chrome update pushed out since I released the pull request, which is the only reason I can think for this change.

[Zeilap]

I added the intermediate point creation and use it now to generate the batch of keys. It runs much faster than before

[canton]

Relevant and important thread:

PSA: Do not use Safari 6 to make BIP38 encrypted paper wallets.

https://bitcointalk.org/index.php?topic=416324.0