Add synchronous way to use scrypt.

[dchest]

Now there are three general ways to call scrypt:

scrypt(password, salt, logN, r, dkLen, interruptStep, callback, [encoding])

Derives a key from password and salt and calls callback with derived key as the only argument. The calculations are interrupted with zero setTimeout at the given interruptSteps to avoid freezing the browser. Encoding is optional.

scrypt(password, salt, logN, r, dkLen, callback, [encoding])

Same as first, but uses default interruptStep (1000). Encoding is optional.

scrypt(password, salt, logN, r, dkLen, [encoding]) -> returns result

Synchronous: doesn't interrupt calculations and returns the result instead of passing it to callback. Encoding is optional. Perfect for use in web workers.

[coveralls]

Coverage increased (+0.92%) to 98.81% when pulling 4499f69ba61fac1be0777c1d622904d6932e521c on sync into 5c066a52352cd5929c116130b62ed3fd3f1de95d on master.

[dchest]

@evilaliv3 what do you think?

[evilaliv3]

let me say that i've a lot of criticism (constructive). i do not like the approach you had in creating the function in order to permit it to be called with some arguments or others. no javascript function is generally builded this way so that they accept some parameters and the mix them like you do internally with callback = interruptstep. this make code less clear (think that you code could be reused or grow up with more functionality.

this is what i suggest to do in obtain the same goal: i would refactor the code packaging all inside an anonymous function and exporting using the notation this.name1 = , this.name2 =. i would then export two function: scrypt, script_sync defined using the schema shown below:

    (function() {
        'use strict';
        function SHA256(m) {
        }
        .....
        this.scrypt = function () {
            // Note: step argument for interruptedFor must be divisible by
              // two, since smixStepX work in increments of 2.
              if (!interruptStep) interruptStep = 1000;

              smixStart();
              interruptedFor(0, N, interruptStep*2, smixStep1, function() {
                interruptedFor(0, N, interruptStep*2, smixStep2, function () {
                  smixFinish();
                  var result = PBKDF2_HMAC_SHA256_OneIter(password, B, dkLen);
                  if (encoding == "base64")
                    callback(bytesToBase64(result));
                  else if (encoding == "hex")
                    callback(bytesToHex(result));
                  else
                    callback(result);
                });
              });
        }
        this.scrypt_sync = function() {
            // the new simplified code with respect to this.scrypt_above
        }
    })();

[evilaliv3]

you can see here a packaging identical to the one i'm proposing: https://github.com/EvanHahn/HumanizeDuration.js/blob/master/humanize-duration.js

so that we can package the library with the name scrypt-async-js and have:

• scrypt-async-js.scrypt()
• scrypt-async-js.scrypt_sync()

[dchest]

Well, handling optional arguments like this is pretty common, however I agree that redefining how the function returns result based on this is uncommon. Another option that I like more than having two functions is to namespace the sync version under scrypt: that is, we'll have scrypt() and scrypt.sync():

scrypt(password, salt, logN, r, dkLen, [interruptStep], callback, [encoding]) scrypt.sync(password, salt, logN, r, dkLen, [encoding]) -> returns result

[evilaliv3]

i do not like so much the word sync to mean a module but also a function :)

[evilaliv3]

why you do not like calling them scrypt and scrypt_sync ?

[dchest]

It should be scrypt and scryptSync then :-) Having them in a separate namespace would break backwards compatibility, having two exported functions is excessive IMHO :-) I think it's a common pattern, I certainly saw a few of such (something and something.sync) on npm.

[evilaliv3]

i'm sure some fairies will die if you do that. do you mind?

[dchest]

It's JavaScript, all fairies are already dead :-D

[evilaliv3]

damn. look a unicorn!

[evilaliv3]

anyway ok i see your point, this is why i liked more the idea of packaging all inside a module called "scrypt-async-js" with two public functions:

• scrypt-async-js.scrypt()
• scrypt-async-js.scryptSync()

i do not see any issue in the retrocompatibility, as it's a one line fix and frankly i do not think a lot of project are using the library right now.

[dchest]

All right, let's drop this sync thing for now and just merge setTimeout avoidance: #15.