Fix error when hexes of length 64 are used as signer secrets

[jacque006]

Change behavior of signer secret parsing to do a direct Fr string set, with a fallback of hashing the secret input on error.
Add validateHex function.
Add wider range of secret values to signer tests.
Use mcl-wasm Typescript types instead of any.

Resolves #27

[jacque006]

After the mcl-wasm@1.0.0 update, parseFr no longer works with hexes of length 64 (string length 66). It does work on hex values of length 1 (i.e. 0x1) up to length 63, one less than what keccak256 produces.

@ChihChengLiang Do you know why this might have changed between mcl-wasm versions, and if there are any implications to switching to just use setHashFr?

[ChihChengLiang]

I don't know the changes in mcl-wasm. Maybe #22 is related?

[jacque006]

I was unable to determine a way to validate that the hex is in range. So I ended up setting up the secret parsing for the signer as such:

• If the secret is missing/empty, generate a random one.
• Attempt to directly set the hex value. If this fails, fall back to hashing the secret.

I think this should cover the range of use cases and expected outputs.

[ChihChengLiang]

Looks like a good solution. @philsippl Would you like to provide a test case example value to make sure this change doesn't break your use case?

[jacque006]

@ChihChengLiang Ready for your final review.

[ChihChengLiang]

LGTM on my side.

[ChihChengLiang]

Looks like a good solution. @philsippl Would you like to provide a test case example value to make sure this change doesn't break your use case?

[jacque006]

Going to merge and release this as is.

@philsippl if this breaks your use case we can fix in follow up work.

[philsippl]

Going to merge and release this as is.

@philsippl if this breaks your use case we can fix in follow up work.

@jacque006 @ChihChengLiang not sure if you saw my comment from a few weeks back, so posting here again in the thread 👇

@ChihChengLiang @jacque006 Sorry for my late reponse, I've been off for a bit and missed the notification. The proposed changes are fine for my use-case (because the default case is to call parseFr). However, I think the approach with try/catch leads to very unintentional behavior (my originally proposed implementation also suffers from this issue).
The error that @jacque006 filed actually only appears on some hashes, specifically those that converted back to bigint don't fit in the prime field (parseFr assumes a field element).
So if we go with the try/catch, we will call parseFr on some hashes and setHashFr on the rest. I don't think this is desired and might create weird compatibility issues with other libraries.
IMO instead of treating this implicitly like @jacque006 and I tried, we should just make it explicit what happens and introduce two distinct getSigner methods, one for passing a Fr directly, and another one that just accepts an arbitrary seed that then calls setHashFr. What do you guys think?

[jacque006]

@philsippl Agreed, I like the split approach. Created #30 to address.