Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make FieldElement generic to simplify alternate backend creation. #5055

Open
Philogy opened this issue May 19, 2024 · 0 comments
Open

Make FieldElement generic to simplify alternate backend creation. #5055

Philogy opened this issue May 19, 2024 · 0 comments
Labels
backend Proving backends enhancement New feature or request

Comments

@Philogy
Copy link

Philogy commented May 19, 2024

Problem

Personally, I'm very bullish on the vision for Noir to become "the LLVM of ZK" which is why we recently attempted to (and somewhat succeeded) at developing an MVP alternate backend for Noir that targets a different proving system (AIR+FRI STARK) that also uses a different prime field (BabyBear).

This was made unnecessarily difficult by the fact that the commonly referred to FieldElement type is dynamically defined at compile time via feature flags propagated throughout the codebase (relevant snippet):

cfg_if::cfg_if! {
    if #[cfg(feature = "bn254")] {
        mod generic_ark;
        pub type FieldElement = generic_ark::FieldElement<ark_bn254::Fr>;
        pub const CHOSEN_FIELD : FieldOptions = FieldOptions::BN254;

    } else if #[cfg(feature = "bls12_381")] {
        mod generic_ark;
        pub type FieldElement = generic_ark::FieldElement<ark_bls12_381::Fr>;
        pub const CHOSEN_FIELD : FieldOptions = FieldOptions::BLS12_381;
    } else {
        compile_error!("please specify a field to compile with");
    }
}

This was further complicated by the fact that bn254 is the default feature, meaning that despite adding a 3rd curve here and defining the feature for it the flags had to be changed in all relevant sub-crates of our fork as well as leading to hard to track down assert_unique_feature errors arising from crates seemingly still referencing the bn254 feature (The difficulty of tracking down the issue was mainly because Rust wouldn't tell us which crate is implicitly using the conflicting bn254 feature flag but instead just point to the defined compile-time check).

In the hackathon we fixed this by just deleting crates we didn't immediately need and updating the features wherever we could.

Happy Case

Ideally FieldElement would be defined and propagated through the crates as a generic type, only being fixed at top-level interaction points such as nargo_cli so that forks can select/switch the field.

Even better would be if custom prime fields were definable or pre-existing fields selectable via options in your Nargo.toml file.

Project Impact

Blocker

Impact Context

When it comes to building alternative backends this is a fundamental blocker as creating and integrating a new one would require making the aforementioned non-trivial changes to the codebase, requiring devs to install, compile and run forks of noir which is sub-optimal.

Workaround

None

Workaround Description

No response

Additional Context

No response

Would you like to submit a PR for this Issue?

None

Support Needs

No response

@Philogy Philogy added the enhancement New feature or request label May 19, 2024
@Savio-Sou Savio-Sou added the backend Proving backends label May 22, 2024
github-merge-queue bot pushed a commit that referenced this issue May 28, 2024
# Description

Step towards #5055 

## Summary\*

This PR starts us down the road towards removing the compile-time flags
for specifying which field is being used by replacing the usage of the
`FieldElement` type with an `AcirField` trait. This trait is mostly all
of the external methods from `FieldElement` dumped into it for now but
we can break it down in future PRs (XOR and AND are looking ready for
culling)

I've taken this route rather than just using `generic_ark::FieldElement`
as we'd need to have trait bounds for `PrimeField` anyway so we might as
well have our own trait.

I've had to delete a couple of trait implementations which fall afoul of
the orphan rule now it's being implemented for a generic type (e.g. we
need to use `MemoryValue::new_field` explicitly now) but nothing too
bad.

## Additional Context


## Documentation\*

Check one:
- [x] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[For Experimental Features]** Documentation to be submitted in a
separate PR.

# PR Checklist\*

- [x] I have tested the changes locally.
- [x] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.
github-merge-queue bot pushed a commit that referenced this issue Jun 10, 2024
# Description

## Problem\*

Related to #5055 

## Summary\*

This PR throws generics into more of the codebase.

## Additional Context



## Documentation\*

Check one:
- [x] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[For Experimental Features]** Documentation to be submitted in a
separate PR.

# PR Checklist\*

- [x] I have tested the changes locally.
- [x] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.
github-merge-queue bot pushed a commit that referenced this issue Jun 10, 2024
…r` (#5209)

# Description

## Problem\*

Related to #5055 
## Summary\*

We don't need the field element to know about AND and XOR as we just
need to be able to convert into byte arrays and back. I've then moved
this into the blackbox solver crate (the only place we were actually
using this) to simplify the `AcirField` trait.

## Additional Context



## Documentation\*

Check one:
- [x] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[For Experimental Features]** Documentation to be submitted in a
separate PR.

# PR Checklist\*

- [x] I have tested the changes locally.
- [x] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend Proving backends enhancement New feature or request
Projects
Status: 📋 Backlog
Development

No branches or pull requests

2 participants