Refactor to use new dependencies

[asoltys]

I've refactored the code to switch to using @paulmillr's noble and scure libraries for most of the crypto operations. The old dependencies were causing me grief when bundling for the browser with vite and when running in newer node versions as Ivan mentioned in #87.

This PR is more ambitious than Ivan's. Besides refactoring to use new dependencies it also gets rid of all Buffer references in favour of Uint8Array, changes to ES6, and replaces standard with biome, a faster JS linter.

The tests pass nearly 10x faster on my laptop now.

I removed the promiseInterval parameter from the async methods since the noble scrypt library doesn't seem to support it, so I've gone ahead and bumped the major version number to 4.

[paulmillr]

Looks great!

Jonathan made bs58check module, which is used in other repos of the organization - I think it can be used for consistency if he asks for it. It also uses these sub-deps.

[asoltys]

Ok, switched to using bs58check and the latest version seems to work well. I had swapped it out before because the 2.1.2 version that bip38 was on had a create-hash dependency, I didn't realize there was a new one.

[junderw]

A few quick comments after a quick look over:

1. This is way too big. Why not just create your own library and just create an issue asking for the bip38 package on npm? If you replace every board on the ship is it even the same ship anymore? lol
2. ESM-only will cause headaches in maintaining. A few years back I released an ESM-only library and got tons of issues about it breaking their setup. (Mostly their fault, aka I had to play the role of ESM-teacher to them and look up their bundler docs for them...) This has probably gotten better since then... but at least it should be hybrid (allow CJS or ESM for consumers)
3. Completely removing asyncInterval will cause problems. In noble I think it's called asyncTick and you should expose it here too. (People complained about their UI freezing for 10 seconds every time they decrypt on low power devices etc.)

Some big-picture comments:

BIP38 has been discouraged for implementation since the days of paper wallets. The only reason we created this library was to offer a path for people to create features in their wallet software that will allow people who made the mistake of using BIP38 in the past a way to recover funds... I agree with this sentiment that BIP38 should not be encouraged as a feature, and looking into your other projects (which I assume are the impetus for creating this essentially new library), it gives me pause to encourage other such projects. (What I mean by this: seeing BIP38 last release being a long time ago and the commits are kind of old is in itself sort of a hint to devs that "this shouldn't be used" and having a shiny new modern BIP38 library might encourage more people to use it in their projects.)

I'm leaning toward NACK, but I'm not yet decided.

But either way I would definitely encourage you to fork it and create a new package. (npm name spaces are great for this, @yourname/bip38 is a fine package name and would show up well in searches for bip38 on npmjs.)

We can always hand over the package name or something after the fact (with a promise of a major semver bump).

[paulmillr]

I think that anyone who’ve spent many hours, refreshing old package, replacing dangerous cryptography should be encouraged. This is pretty cool. Don’t like the concept of the feature? People are going to do it anyway. If it shouldn’t be used, you should put a big, fat depreciation notice in readme and to npm using “npm deprecate”.

In my experience, it’s pretty rare for maintainers to give out package name. Also, this would come with responsibility — what if the person would start uploading malware?

As for esm — yeah, it’s still not ideal today, hybrid packages are cooler, but they are only relevant if there’s typescript source and 2 outputs.

[junderw]

dangerous

If there is a clear danger, it would make more sense to create a more urgent PR with a smaller scope pinpointed on removing that specific danger.

Unless this is some attempt to obfuscate some vulnerability patch? In which case I would expect an out of band communication beforehand... So I'm guessing that's not it.

[paulmillr]

To clarify, no specific vulnerabilities. Just risky exposure to supply chain attacks, and using libraries which haven’t had much fuzzing. Not a great fit for money relayed stuff.

We’ve did the same thing with ethereumjs/wallet and keythereum packages, which are discouraged and barely used, but still received a proper update just in case.

[junderw]

Just to clarify, I have no problem with switching deps to noble.

The other PR was mostly overlooked for other reasons, not because of some sort of strong objection to noble.

If this PR reduced its scope to just replacing the deps and minimizing changes to the rest of the code / API surface, I would have been more open to just merging it as is.

[asoltys]

Hi @junderw thanks for your consideration.

1. This is way too big. Why not just create your own library and just create an issue asking for the bip38 package on npm?

I initially reached out to Paul to let him know that I'd rewritten it to use his libraries and see if he might want to fork the package under one of his namespaces but he suggested I open a PR here instead. I've gone ahead and published @asoltys/bip38 for the time being.

2. ESM-only will cause headaches in maintaining.

I made it a hybrid now by using Typescript and following this article: https://www.sensedeep.com/blog/posts/2021/how-to-create-single-source-npm-module.html

It's my first time doing that so let me know if I missed anything.

3. Completely removing asyncInterval will cause problems.

I didn't notice asyncTick was an option. I've mentioned it in the README now. The API is slightly different than before because it's part of the scryptParams rather than a separate param. I'm merging the provided params with the defaults now so users can provide partial params.

BIP38 has been discouraged for implementation since the days of paper wallets.

I think the main criticism was around using it in EC Multiply mode -- something to do with the Confirmation Code being broken. I suspect most people just use the basic encrypt and decrypt methods.

As for paper wallets being discouraged in general, I've always felt they got a bad shake because of all the phishing sites and incidents with broken crypto in the browser etc. but I still like them for their simplicity, even just as a learning tool to demonstrate the concept of an address and private key to noobs.

Unlike hardware wallets they're free and fast to setup and there's no supply chain worry. I suspect a lot of people do still have old ones lying around that their Bitcoiner uncles and the like setup for them years ago so I don't see any harm in building better tools to access them.

[junderw]

I've gone ahead and published @asoltys/bip38 for the time being.

Sounds good. If I ever get the urge to include some of your fork's code I'll ping you on the PR.

I'll close this PR though. As I've stated before, I'm open to PRs with smaller scope.

Good work!