perun-network / erdstall-ts-sdk

TypeScript client SDK to interact with Erdstall.
Apache License 2.0
5 stars 2 forks source link

Fix naive `NewUintXXX` implementation. #57

Closed ndzik closed 3 years ago

ndzik commented 3 years ago

Current implementation has a big problem :clown_face:

// NewBigInt generates a random `BigInt` in range 0 <= `CEIL`.
export function NewBigInt(rng: PRNG, CEIL: bigint): bigint {
    const base = CEIL / 2n;
    const offset = (BigInt(Math.floor(100 * rng.uFloat32())) * base) / 100n; // <-- Not good.
    const add = (v1: bigint, v2: bigint): bigint => {
        return v1 + v2;
    };
    const sub = (v1: bigint, v2: bigint): bigint => {
        return v1 - v2;
    };
    const op = rng.uFloat32() < 0.5 ? add : sub;
    return op(base, offset);
}

This results in a lot of repeated values and false negatives in some tests. Even better if we can find a battletested library which we could utilise for stuff like this.

sebastianst commented 3 years ago

Could we short-term just use our newRandomUint8Array and truncate the last byte, if CEIL is not a power of 256?

sebastianst commented 3 years ago

Besides that I'm also not too happy with the prng lib that we use. Martin Helmut's number-generator has only 9 ⭐ It still looks like a solid project though.