Implement to/fromBits in TS, and set max length to 254 bits

[jackryanservia]

Closes: #1023

• Implements toBits and fromBits methods in TS.
• Sets length limit 254bits for both to prevent issue outlined in Field.toBits() and fromBits() are unsound #1023

[jackryanservia]

I will think more about the to/fromBits() implementation and check the constraint count before I merge, but for now I just have a few questions about other things to make sure I'm on the right track! :)

[jackryanservia]

My understanding is that there isn't a clever way to make this secure (other than just limiting it to 254 bits) because any other method would necessarily involve checking if the value overflowed or not. Does that reasoning check out?

[mitschabaude]

we could provide a secure, 255-bits version by treating the bits as a ForeignField element and asserting that it's less than the field size. That would add some overhead so it's better to not do it by default. It could be a separate method toFullBits() or similar. But not urgent IMO

[jackryanservia]

This makes sense. I will implement tomorrow if there is time.

[jackryanservia]

Ok, it looks like the number of rows for both methods is exactly the same as before (and the digest is identical for fromBits()).

Let me know if I should add vk regression test for these methods; aside from that I think this is ready to merge if looks ok to reviewer :)

[mitschabaude]

Looks great, just a few comments that should be addressed

Also @jackryanservia please add a changelog! Breaking change

[mitschabaude]

1-row reduction makes sense because the hello-world example does privateKey.toPublicKey() which as the final step converts the Group (uncompressed EC point (x,y)) into a PublicKey (compressed EC point (x, isOdd)) and to do that it needs to get all of y's bits with toBits(). toBits() needs 1 row less now because it adds up 254 bits, not 255.

It does mean that certain public keys are no longer supported by this method. I think we should address that separately by introducing a dedicated parity() gadget using range checks, which makes it more efficient as well

[jackryanservia]

I had wondered about this. Would it make sense to have this use toFullBits() here in the meantime?

[mitschabaude]

Yes that would make sense

[ymekuria]

Nice work @jackryanservia !