Refactor `UInt{8,16,64,128}` into one struct `UInt`

arkworks-rs / r1cs-std

R1CS constraints for bits, fields, and elliptic curves

https://www.arkworks.rs

Apache License 2.0

137 stars 59 forks source link

Refactor `UInt{8,16,64,128}` into one struct `UInt` #121

Closed Pratyush closed 10 months ago

Pratyush commented 1 year ago

Description

closes: #116

Before we can merge this PR, please make sure that all the following items have been checked off. If any of the checklist items are not applicable, please leave them but write a little note why.

[ ] Targeted PR against correct branch (master)
[ ] Linked to Github issue with discussion and accepted design OR have an explanation in the PR that describes this work.
[ ] Wrote unit tests
[ ] Updated relevant documentation in the code
[ ] Added a relevant changelog entry to the Pending section in CHANGELOG.md
[ ] Re-reviewed Files changed in the Github PR explorer

Pratyush commented 1 year ago

Unfortunately I think there's a special impl of ToConstraintField for [u8]s that's used in the new_input_vec impl. Generalizing it to support larger sizes shouldn't be difficult, but would require moving away from calling ToConstraintField. Maybe that's fine though.

mmagician commented 1 year ago

Rather than generalizing we could just implement ToConstraintField for &[u16] etc no?

autquis commented 10 months ago

@Pratyush After moving ToBytesGadget to convert, it is no longer part of prelude. Is it intended? I'm fixing crypto-primitive for this change. For example, in: https://github.com/arkworks-rs/crypto-primitives/blob/8bbb4824ede5f2cf115f1a37e08410b57944a4f6/src/signature/constraints.rs#L10

It was previously enough to use ark-r1cs-std::prelude::* but now I need either use it explicitly or update the prelude.