sxt-proof-of-sql icon indicating copy to clipboard operation
sxt-proof-of-sql copied to clipboard

Make opaque `Scalar` conversions explicit rather than using `From`/`Into` trait.

Open JayWhite2357 opened this issue 4 months ago • 6 comments

Background and Motivation

Currently, the Scalar trait requires various conversions using the From and Into traits. Some of these are confusing, while others are clear. In general, the rule should be that for types with a natural embedding into the associated Scalar field, Scalar should implement From. In all other cases, the conversion should either be auto-implemented, or a trait method, where appropriate.

NOTE: The following issue also necessitates changes to Scalar, so be aware of potential merge conflicts: https://github.com/spaceandtimelabs/sxt-proof-of-sql/issues/234

Changes Required

Each of these should be a separate PR.

  • [ ] Remove Into<[u64; 4]>, From<[u64; 4]>, and RefInto<[u64; 4]> bounds.
    • Replace these with the trait methods fn from_limbs(val: [u64; 4]) -> Self and fn to_limbs(&self) -> [u64; 4]
    • These three conversions are the base conversions that are used for many other conversions. A Scalar is a field element (number mod some prime) with slightly less than 256 bits. Usually, Scalars are internally stored in Montgomery form, so these conversions are non-free. However, they are needed in some situations.
  • [ ] Remove From<&'a String>, From<String>, and From<&'a str> bounds. These three implementations ultimately rely on the conversions to and from limbs. As a result, these should have blanket/default implementations.
    • Replace these with fn from_str_via_hash(val: &str) -> Self and add a default implementation.
    • While it would be nice to impl<S: Scalar> From<String> for S, this is not allowed by the Rust type system. Solutions other than the default implementation of a trait method are welcome.
    • This conversion requires that the string is hashed in order to convert it to 256 bits which are ultimately converted into the Scalar itself. This is different from a parsing method.
  • [ ] Remove the VarInt bound and replace it with a blanket impl<S: Scalar> VarInt for S implementation. The existing (and requested) implementation relies on the limb conversions only.

JayWhite2357 avatar Oct 07 '24 09:10 JayWhite2357