search

peterolson / BigInteger.js

An arbitrary length integer library for Javascript
The Unlicense
1.12k stars 187 forks source link

Add Cryptographically Secure RNG #108

Closed benjaminBrownlee closed 6 years ago

benjaminBrownlee commented 7 years ago

As a follow up to issue #97, I have modified this library to accommodate a cryptographically secure RNG.

benjaminBrownlee commented 7 years ago

It appears BigInteger.min.js failed some checks. Did I make a mistake while appending the minifed code?

peterolson commented 7 years ago

Hi, I'm looking at the commit and it looks like it only supports running in the browser, using window.crypto. BigInteger.js is used on both the browser and on Node, so we need to support both environments. Also, perhaps it should fallback to randBetween when there is no crypto library present.

The failing build says something about not being able to parse something, I'm not sure exactly what's going on.

Also, there are no unit tests for this function. These should be added so that we can be more confident that it works correctly.

We will also need to add TypeScript definitions for any new functions that are added. I can do this later if you have trouble with it.

benjaminBrownlee commented 7 years ago

I believe I have an edit so that this will work in a Node environment as well as a majority of browsers, and I also threw in your suggested fallback feature if users are using an environment without crypto tools. If you are curious, see my most recent gist edits.

I tried to append my minified scripts into the original minified file, which must have gone wrong. I will re-minify the forked regular file to avoid these errors next time.

However, as mentioned before, I have no idea where to turn to do "unit tests" on my function. Is there a preferred or formal process or host to do those?

Also, to be completely honest, I have not used TypeScript before. I actually have used a converter tool to convert your entire library to PHP, but TypeScript is a superset of JavaScript so tools can't convert in that direction.

peterolson commented 7 years ago

The unit tests are located in the spec/spec.js file. You can run them locally by opening the spec/SpecRunner.html file or doing npm test on node. You can look through the spec file and follow the example of the existing tests that specify what kinds of outputs we expect from the functions. You'll probably be most interested in the tests for the randBetween function.

The TypeScript definitions are in the BigInteger.d.ts file, which defines types and makes it more convenient for people using BigInteger.js in their TypeScript projects. The tsDefinitions.ts verifies that the library actually conforms to these type definitions.

As I said before, don't worry about the TypeScript definitions if you have trouble figuring it out, but it is important to have unit tests for every function in the library.