Signing session takes 20+ seconds on Node v21

[simonmcallister0210]

After updating the test matrix to include Node v21 I noticed signing takes ages, causing tests to fail. Maybe this is something to do with the fact I'm still using CryptoJS? I'm not sure...

I think for now we should look into moving over to the built-in crypto functions. Might improve performance and should have the added benefit of reducing the package size

[ryanwalters]

Also worth noting, crypto-js was discontinued as of October 2023. I'm not an expert on crypto stuff, but this issue might be useful for migrating over to the node:crypto equivalents of crypto-js functionality.

[simonmcallister0210]

Yes it's been at the back of my mind for a while and I've been thinking about how to approach it. I definitely want to move over to the standard crypto library very soon

I wasn't aware of this issue. Some useful info in there that'll come in handy. Thanks! If you (or anyone else) find any more useful guides or discussions feel free to drop them here. It'll make my life a lot easier when we eventually do make the transition 😁

[rhoberman]

I'm using React Native with Hermes and I was experiencing 5+ second waits on an iPhone 16.

I'm using the buffer polyfill. I did some profiling on both buffer and crypto-js and neither seemed to be the problem. Rather it seems it is jsbn. Replacing its BigInteger with the in-built BigInt gives me pretty good performance (around 6 times faster).

See my fork.

I know using BigInt may not work for everyone. If not, would it be possible to make the BigInteger library configurable?

[simonmcallister0210]

I'm using the buffer polyfill. I did some profiling on both buffer and crypto-js and neither seemed to be the problem. Rather it seems it is jsbn. Replacing its BigInteger with the in-built BigInt gives me pretty good performance (around 6 times faster).

Ah good spot! Thats really useful to know

I know using BigInt may not work for everyone. If not, would it be possible to make the BigInteger library configurable?

Maybe yeah.. I'm thinking we could create a layer of abstraction for large numbers, then swap out the underlying library based on config. Wouldn't need to install any new dependencies since BigInt is built-in. Shouldn't cause any interference for existing users

I'm a bit stuck on how we'd specify that config. Our library's all functions so we're limited in what we can do. We could use environment variable, a package level config object, more function params... I'll give it some thought

Should be do-able though. I'll put together a solution soon. Feel free to open a PR if you feel like it, always happy to collaborate

[rhoberman]

Hey, thanks for the thoughtful response and for considering configurability! Here’s my take: - My initial thought is to provide a higher-order function, something like createCognitoSrpHelper(config): CognitoSrpHelper. This would avoid configuring the library itself and stays aligned with the functional paradigm, which I think is important to preserve. - I’d be very happy to use an instance-based approach since I don’t have many integration points, so it’s easy for me to adopt. - To keep the current "import-and-use" style (and avoid breaking the existing API), you could internally create a default instance and have the existing functions delegate to that instance. This way, users who don’t need customization won’t be impacted, while others can benefit from the flexibility. Let me know your thoughts! I’d be happy to help out with a refactor if needed.