update scrypt shim using maxmem fix from node >= 12.8

[sidhujag]

referencing web3-js/scrypt-shim#3 can we update scrypt-shim so that it uses maxmem fallback (built-in scrypt) starting from node >= 12.8?

[michaelsbradleyjr]

The situation is a little weirder than that.

For Node <= 12.0.0 it could still be valid to install the old scrypt package. Note also that the maxmem change was backported into Node 10.x as of 10.17.0 (see the release notes).

It turns out that the maxmem thing won't prevent a blow-up with some scrypt parameters that are found e.g. in go-ethereum's (geth) and web3's test suites. That's because golang's scrypt isn't compliant with the RFC for scrypt whlie Node's is:

golang/go#33703
ethereum/wiki#674
ethereum/go-ethereum#19977
nodejs/node#28799 (comment)

So, if someone using Node wanted to be able to process the exact same set of scrypt parameters that's possible with golang (e.g. with geth) then they'd either need to use a non-compliant pure JS implementation or the old scrypt package (and in the latter case they'd need to be using Node <= 12.0.0).

One thing that web3 should probably do is remove its wallet test cases that have the non-compliant parameters.

[sidhujag]

@michaelsbradleyjr for path of least resistance there already is code that is around when maxmem is fixed (12.8), so I suggested to use that just uncomment and set the variable to 12.8. I didn't know tests still broke, so is 53 bits not enough for all cases or is it a bad test?

[michaelsbradleyjr]

You've misunderstood. Please follow the trail of links I included in my last comment, they're ordered with respect to the dates I posted those issues, newest at the top. While the maxmem bump is a nice feature, it's ultimately a red herring.

[michaelsbradleyjr]

For tests to pass on this project's 1.x branch with Node's built-in scrypt, the wallet test cases that replicate geth's non-RFC-compliant scrypt test cases will need to be removed.

[sidhujag]

For tests to pass on this project's 1.x branch [with Node's built-in scrypt], the wallet test cases that replicate geth's non-RFC-compliant scrypt test cases will need to be removed from this repo.

thx what a mess. Hopefully we can get it back on track and remove the dependency, ultimately that is my goal.

[cgewecke]

Believe this was resolved by #3536. Please ping if that's not the case.