Make opaque `Scalar` conversions explicit rather than using `From`/`Into` trait.
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]>, andRefInto<[u64; 4]>bounds.- Replace these with the trait methods
fn from_limbs(val: [u64; 4]) -> Selfandfn to_limbs(&self) -> [u64; 4] - These three conversions are the base conversions that are used for many other conversions. A
Scalaris 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.
- Replace these with the trait methods
- [ ] Remove
From<&'a String>,From<String>, andFrom<&'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) -> Selfand 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
Scalaritself. This is different from a parsing method.
- Replace these with
- [ ] Remove the
VarIntbound and replace it with a blanketimpl<S: Scalar> VarInt for Simplementation. The existing (and requested) implementation relies on the limb conversions only.
/bounty $100
💎 $100 bounty • Space and Time
Steps to solve:
- Start working: (Optional) Comment
/attempt #228with your implementation plan. Note: we will only assign an issue if you include an implementation plan with a time estimate. Additionally, to be assigned an issue, you must have previously contributed to the project. You can still work on an issue and submit a PR without being assigned. - Submit work: Create a pull request including
/claim #228in the PR body to claim the bounty - Receive payment: 100% of the bounty is received 2-5 days post-reward. Make sure you are eligible for payouts
Thank you for contributing to spaceandtimelabs/sxt-proof-of-sql!
Add a bounty • Share on socials
| Attempt | Started (GMT+0) | Solution |
|---|---|---|
| 🟢 @b4s36t4 | Oct 9, 2024, 5:27:42 AM | WIP |
| 🟢 @varshith257 | Oct 13, 2024, 7:36:07 AM | WIP |
| 🟢 @maujim | Nov 4, 2024, 5:20:18 AM | #335 |
/attempt #228
| Algora profile | Completed bounties | Tech | Active attempts | Options |
|---|---|---|---|---|
| @varshith257 | 15 bounties from 7 projects | Go, TypeScript, Scala & more |
﹟232 |
Cancel attempt |
@JayWhite2357 can you please take a look at the liked PR?
@JayWhite2357 can you please take a look at the liked PR?
@maujim Sorry for the delay on this, the PR is still open and needs conflict resolution, I can review ASAP if you still want to try to get it in. If you still want to try this I think its possible to resolve the formatter error. This issue might need to be resolved in order to accompany #229
Let us know when you can
@Dustin-Ray I'm on macos / arm and I'm getting two compilation errors on main related to my machine.
Given this, I don't think I'll be diving back into this one.
error: failed to run custom build command for `blitzar-sys v1.99.0`
note: To improve backtraces for build dependencies, set the CARGO_PROFILE_DEV_BUILD_OVERRIDE_DEBUG=true environment variable to enable debug information generation.
Caused by:
process didn't exit successfully: `/Users/mukund/dev/sxt-proof-of-sql/target/debug/build/blitzar-sys-a863c08174abcbdf/build-script-build` (exit status: 101)
--- stderr
thread 'main' panicked at /Users/mukund/.config/cargo/registry/src/index.crates.io-6f17d22bba15001f/blitzar-sys-1.99.0/build.rs:14:9:
Unsupported OS. Only Linux is supported.
stack backtrace:
0: rust_begin_unwind
at /rustc/9fc6b43126469e3858e2fe86cafb4f0fd5068869/library/std/src/panicking.rs:665:5
1: core::panicking::panic_fmt
at /rustc/9fc6b43126469e3858e2fe86cafb4f0fd5068869/library/core/src/panicking.rs:76:14
2: build_script_build::main
3: core::ops::function::FnOnce::call_once
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
warning: build failed, waiting for other jobs to finish...
error: failed to run custom build command for `halo2curves v0.8.0`
note: To improve backtraces for build dependencies, set the CARGO_PROFILE_DEV_BUILD_OVERRIDE_DEBUG=true environment variable to enable debug information generation.
Caused by:
process didn't exit successfully: `/Users/mukund/dev/sxt-proof-of-sql/target/debug/build/halo2curves-0db72ce93f44cb27/build-script-build` (exit status: 1)
--- stderr
Currently feature `asm` can only be enabled on x86_64 arch.
@maujim I think You can fix this error by turning off blitzar using this env variable:
export BLITZAR_BACKEND=cpu
If you're ok passing on this issue I can go ahead and manually take over, thanks for the effort so far we really appreciate the help.
I tried a fresh git clone && cargo build and I get the halo2curves issue before the blitzar one.
Took a quick look and it seems that the arm feature is enabled by default via the nova-snark dependency.
https://github.com/microsoft/Nova/blob/main/Cargo.toml#L70
So I think its best for someone else to take over for now
I'd like to pick up this issue. I'm currently reading about Montgomery scalars. What exactly is the correct conversion to and from Scalar <--> [u64; 4] ?