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.

Sign up for GitHub

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

Rectify language fields #423

Merged
merged 4 commits into from
Jun 3, 2023
Merged

Rectify language fields #423

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
aecb979
Assign correct FIELD.
porcuquine Jun 2, 2023
c9d5472
Use obviously-identifying Scalar type aliases.
porcuquine Jun 2, 2023
14bdbf0
Correct fields and use obvious aliases.
porcuquine Jun 2, 2023
5b3cb67
Document field names.
porcuquine Jun 2, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 17 additions & 20 deletions benches/end2end.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ use lurk::{
public_parameters,
store::Store,
};
use pasta_curves::pallas;
use std::sync::Arc;
use std::time::Duration;

Expand Down Expand Up @@ -46,8 +47,7 @@ fn end2end_benchmark(c: &mut Criterion) {
.sample_size(10);

let limit = 1_000_000_000;
let lang_pallas =
Lang::<pasta_curves::pallas::Scalar, Coproc<pasta_curves::pallas::Scalar>>::new();
let lang_pallas = Lang::<pallas::Scalar, Coproc<pallas::Scalar>>::new();
let lang_pallas_rc = Arc::new(lang_pallas.clone());
let reduction_count = DEFAULT_REDUCTION_COUNT;

Expand All @@ -64,7 +64,7 @@ fn end2end_benchmark(c: &mut Criterion) {

group.bench_with_input(benchmark_id, &size, |b, &s| {
b.iter(|| {
let ptr = go_base::<pasta_curves::pallas::Scalar>(&mut store, s.0, s.1);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can chuck the following in a clippy.toml at the root of the repo to have the yelling done by tooling instead of you:

disallowed-methods = [
    # we use strict naming conventions for fields of our workhorse curve
    { path = "pasta_curves::Fp", reason = "use pasta_curves::pallas::Scalar instead" },
    ...
]

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. However, please note that the code you provided here reproduces the error I sought to correct here. In fact, pasta_curves::Fq is the one that corresponds to pallas::Scalar — as noted in the added documentation.

I realize this isn't what you were intending to communicate, and I mention it only because:

  • It's still wrong…
  • It's a perfect and timely example of how allowing these wrong expressions into our source of truth creates ongoing confusion.

As always, I recognize that I did approve the PR(s) introducing these errors.

let ptr = go_base::<pallas::Scalar>(&mut store, s.0, s.1);
let _result = prover
.evaluate_and_prove(&pp, ptr, env, &mut store, limit, lang_pallas_rc.clone())
.unwrap();
Expand All @@ -81,7 +81,7 @@ fn store_benchmark(c: &mut Criterion) {
.sample_size(60);

let mut bls12_store = Store::<Fr>::default();
let mut pallas_store = Store::<pasta_curves::Fp>::default();
let mut pallas_store = Store::<pallas::Scalar>::default();

// todo!() rfc out into more flexible test cases
let sizes = vec![(10, 16), (10, 160)];
Expand All @@ -99,7 +99,7 @@ fn store_benchmark(c: &mut Criterion) {
let pasta_id = BenchmarkId::new("store_go_base_pallas", &parameter_string);
group.bench_with_input(pasta_id, &size, |b, &s| {
b.iter(|| {
let result = go_base::<pasta_curves::Fp>(&mut pallas_store, s.0, s.1);
let result = go_base::<pallas::Scalar>(&mut pallas_store, s.0, s.1);
black_box(result)
})
});
Expand All @@ -115,7 +115,7 @@ fn hydration_benchmark(c: &mut Criterion) {
.sample_size(60);

let mut bls12_store = Store::<Fr>::default();
let mut pallas_store = Store::<pasta_curves::Fp>::default();
let mut pallas_store = Store::<pallas::Scalar>::default();

// todo!() rfc out into more flexible test cases
let sizes = vec![(10, 16), (10, 160)];
Expand All @@ -133,7 +133,7 @@ fn hydration_benchmark(c: &mut Criterion) {
{
let benchmark_id = BenchmarkId::new("hydration_go_base_pallas", &parameter_string);
group.bench_with_input(benchmark_id, &size, |b, &s| {
let _ptr = go_base::<pasta_curves::Fp>(&mut pallas_store, s.0, s.1);
let _ptr = go_base::<pallas::Scalar>(&mut pallas_store, s.0, s.1);
b.iter(|| pallas_store.hydrate_scalar_cache())
});
}
Expand All @@ -150,9 +150,9 @@ fn eval_benchmark(c: &mut Criterion) {

let limit = 1_000_000_000;
let lang_bls12 = Lang::<Fr, Coproc<Fr>>::new();
let lang_pallas = Lang::<pasta_curves::Fp, Coproc<pasta_curves::Fp>>::new();
let lang_pallas = Lang::<pallas::Scalar, Coproc<pallas::Scalar>>::new();
let mut bls12_store = Store::<Fr>::default();
let mut pallas_store = Store::<pasta_curves::Fp>::default();
let mut pallas_store = Store::<pallas::Scalar>::default();

// todo!() rfc out into more flexible test cases
let sizes = vec![(10, 16), (10, 160)];
Expand All @@ -179,7 +179,7 @@ fn eval_benchmark(c: &mut Criterion) {
{
let benchmark_id = BenchmarkId::new("eval_go_base_pallas", &parameter_string);
group.bench_with_input(benchmark_id, &size, |b, &s| {
let ptr = go_base::<pasta_curves::Fp>(&mut pallas_store, s.0, s.1);
let ptr = go_base::<pallas::Scalar>(&mut pallas_store, s.0, s.1);
b.iter(|| {
Evaluator::new(
ptr,
Expand All @@ -205,15 +205,15 @@ fn eval_benchmark(c: &mut Criterion) {

// let limit = 1_000_000_000;
// let _lang_bls = Lang::<Fr, Coproc<Fr>>::new();
// let _lang_pallas = Lang::<pasta_curves::Fp, Coproc<pasta_curves::Fp>>::new();
// let lang_pallas = Lang::<pasta_curves::pallas::Scalar, Coproc<pasta_curves::pallas::Scalar>>::new();
// let _lang_pallas = Lang::<pallas::Scalar, Coproc<pallas::Scalar>>::new();
// let lang_pallas = Lang::<pallas::Scalar, Coproc<pallas::Scalar>>::new();

// let reduction_count = DEFAULT_REDUCTION_COUNT;

// group.bench_function("circuit_generation_go_base_10_16_nova", |b| {
// let mut store = Store::default();
// let env = empty_sym_env(&store);
// let ptr = go_base::<pasta_curves::pallas::Scalar>(&mut store, black_box(10), black_box(16));
// let ptr = go_base::<pallas::Scalar>(&mut store, black_box(10), black_box(16));
// let prover = NovaProver::new(reduction_count, lang_pallas.clone());

// let pp = public_parameters::public_params(reduction_count).unwrap();
Expand All @@ -238,8 +238,7 @@ fn prove_benchmark(c: &mut Criterion) {
.sample_size(10);

let limit = 1_000_000_000;
let lang_pallas =
Lang::<pasta_curves::pallas::Scalar, Coproc<pasta_curves::pallas::Scalar>>::new();
let lang_pallas = Lang::<pallas::Scalar, Coproc<pallas::Scalar>>::new();
let lang_pallas_rc = Arc::new(lang_pallas.clone());
let mut store = Store::default();
let reduction_count = DEFAULT_REDUCTION_COUNT;
Expand All @@ -248,7 +247,7 @@ fn prove_benchmark(c: &mut Criterion) {
let benchmark_id = BenchmarkId::new("prove_go_base_nova", format!("_{}_{}", size.0, size.1));

group.bench_with_input(benchmark_id, &size, |b, &s| {
let ptr = go_base::<pasta_curves::pallas::Scalar>(&mut store, s.0, s.1);
let ptr = go_base::<pallas::Scalar>(&mut store, s.0, s.1);
let prover = NovaProver::new(reduction_count, lang_pallas.clone());
let pp = public_parameters::public_params(reduction_count, lang_pallas_rc.clone()).unwrap();
let frames = prover
Expand All @@ -271,8 +270,7 @@ fn verify_benchmark(c: &mut Criterion) {
.sample_size(10);

let limit = 1_000_000_000;
let lang_pallas =
Lang::<pasta_curves::pallas::Scalar, Coproc<pasta_curves::pallas::Scalar>>::new();
let lang_pallas = Lang::<pallas::Scalar, Coproc<pallas::Scalar>>::new();
let lang_pallas_rc = Arc::new(lang_pallas.clone());
let mut store = Store::default();
let reduction_count = DEFAULT_REDUCTION_COUNT;
Expand Down Expand Up @@ -314,8 +312,7 @@ fn verify_compressed_benchmark(c: &mut Criterion) {
.sample_size(10);

let limit = 1_000_000_000;
let lang_pallas =
Lang::<pasta_curves::pallas::Scalar, Coproc<pasta_curves::pallas::Scalar>>::new();
let lang_pallas = Lang::<pallas::Scalar, Coproc<pallas::Scalar>>::new();
let lang_pallas_rc = Arc::new(lang_pallas.clone());
let mut store = Store::default();
let reduction_count = DEFAULT_REDUCTION_COUNT;
Expand Down
16 changes: 8 additions & 8 deletions benches/fibonacci.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ use criterion::{
BenchmarkId, Criterion, SamplingMode,
};

use pasta_curves::pallas;

use lurk::{
eval::{
empty_sym_env,
Expand Down Expand Up @@ -41,8 +43,7 @@ fn fib<F: LurkField>(store: &mut Store<F>, a: u64) -> Ptr<F> {
#[allow(dead_code)]
fn fibo_total<M: measurement::Measurement>(name: &str, iterations: u64, c: &mut BenchmarkGroup<M>) {
let limit: usize = 10_000_000_000;
let lang_pallas =
Lang::<pasta_curves::pallas::Scalar, Coproc<pasta_curves::pallas::Scalar>>::new();
let lang_pallas = Lang::<pallas::Scalar, Coproc<pallas::Scalar>>::new();
let lang_rc = Arc::new(lang_pallas.clone());
let reduction_count = DEFAULT_REDUCTION_COUNT;

Expand All @@ -55,7 +56,7 @@ fn fibo_total<M: measurement::Measurement>(name: &str, iterations: u64, c: &mut
|b, iterations| {
let mut store = Store::default();
let env = empty_sym_env(&store);
let ptr = fib::<pasta_curves::pallas::Scalar>(&mut store, black_box(*iterations));
let ptr = fib::<pallas::Scalar>(&mut store, black_box(*iterations));
let prover = NovaProver::new(reduction_count, lang_pallas.clone());

b.iter_batched(
Expand All @@ -75,14 +76,14 @@ fn fibo_total<M: measurement::Measurement>(name: &str, iterations: u64, c: &mut
#[allow(dead_code)]
fn fibo_eval<M: measurement::Measurement>(name: &str, iterations: u64, c: &mut BenchmarkGroup<M>) {
let limit = 10_000_000_000;
let lang_pallas = Lang::<pasta_curves::Fp, Coproc<pasta_curves::Fp>>::new();
let lang_pallas = Lang::<pallas::Scalar, Coproc<pallas::Scalar>>::new();

c.bench_with_input(
BenchmarkId::new(name.to_string(), iterations),
&(iterations),
|b, iterations| {
let mut store = Store::default();
let ptr = fib::<pasta_curves::Fp>(&mut store, black_box(*iterations));
let ptr = fib::<pallas::Scalar>(&mut store, black_box(*iterations));
b.iter(|| {
let result =
Evaluator::new(ptr, empty_sym_env(&store), &mut store, limit, &lang_pallas)
Expand All @@ -95,8 +96,7 @@ fn fibo_eval<M: measurement::Measurement>(name: &str, iterations: u64, c: &mut B

fn fibo_prove<M: measurement::Measurement>(name: &str, iterations: u64, c: &mut BenchmarkGroup<M>) {
let limit = 10_000_000_000;
let lang_pallas =
Lang::<pasta_curves::pallas::Scalar, Coproc<pasta_curves::pallas::Scalar>>::new();
let lang_pallas = Lang::<pallas::Scalar, Coproc<pallas::Scalar>>::new();
let lang_rc = Arc::new(lang_pallas.clone());
let reduction_count = DEFAULT_REDUCTION_COUNT;
let pp = public_params(reduction_count, lang_rc.clone()).unwrap();
Expand All @@ -107,7 +107,7 @@ fn fibo_prove<M: measurement::Measurement>(name: &str, iterations: u64, c: &mut
|b, iterations| {
let mut store = Store::default();
let env = empty_sym_env(&store);
let ptr = fib::<pasta_curves::pallas::Scalar>(&mut store, black_box(*iterations));
let ptr = fib::<pallas::Scalar>(&mut store, black_box(*iterations));
let prover = NovaProver::new(reduction_count, lang_pallas.clone());

let frames = prover
Expand Down
21 changes: 16 additions & 5 deletions clutch/src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ use clutch::ClutchState;

use lurk::eval::lang::{Coproc, Lang};
use lurk::field::LanguageField;
use lurk::proof::nova;
use lurk::repl::repl_cli;
use pasta_curves::pallas;

fn main() -> Result<()> {
pretty_env_logger::init();
Expand All @@ -24,10 +24,21 @@ fn main() -> Result<()> {

match field {
LanguageField::Pallas => repl_cli::<
nova::S1,
ClutchState<nova::S1, Coproc<nova::S1>>,
Coproc<nova::S1>,
>(Lang::<nova::S1, Coproc<nova::S1>>::new()),
pallas::Scalar,
ClutchState<pallas::Scalar, Coproc<pallas::Scalar>>,
Coproc<pallas::Scalar>,
>(Lang::<pallas::Scalar, Coproc<pallas::Scalar>>::new()),
// TODO: Support all LanguageFields.
// LanguageField::BLS12_381 => repl_cli::<
// blstrs::Scalar,
// ClutchState<blstrs::Scalar, Coproc<blstrs::Scalar>>,
// Coproc<blstrs::Scalar>,
// >(Lang::<blstrs::Scalar, Coproc<blstrs::Scalar>>::new()),
// LanguageField::Vesta => repl_cli::<
// vesta::Scalar,
// ClutchState<vesta::Scalar, Coproc<vesta::Scalar>>,
// Coproc<vesta::Scalar>,
// >(Lang::<vesta::Scalar, Coproc<vesta::Scalar>>::new()),
_ => panic!("unsupported field"),
}
}
25 changes: 17 additions & 8 deletions src/field.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,14 @@ use crate::tag::{ContTag, ExprTag, Op1, Op2};

/// The type of finite fields used in the language
/// For Pallas/Vesta see https://electriccoin.co/blog/the-pasta-curves-for-halo-2-and-beyond/
///
/// Please note:
/// - pasta_curves::pallas::Scalar = pasta_curves::Fq
/// - pasta_curves::vesta::Scalar = pasta_curves::Fp
///
/// Because confusion on this point, perhaps combined with cargo-cult copying of incorrect previous usage has led to
/// inconsistencies and inaccuracies in the code base, please prefer the named Scalar forms when correspondence to a
/// named `LanguageField` is important.
pub enum LanguageField {
/// The Pallas field,
Pallas,
Expand Down Expand Up @@ -215,11 +223,11 @@ impl LurkField for blstrs::Scalar {
const FIELD: LanguageField = LanguageField::BLS12_381;
}

impl LurkField for pasta_curves::Fp {
impl LurkField for pasta_curves::pallas::Scalar {
const FIELD: LanguageField = LanguageField::Pallas;
}

impl LurkField for pasta_curves::Fq {
impl LurkField for pasta_curves::vesta::Scalar {
const FIELD: LanguageField = LanguageField::Vesta;
}

Expand Down Expand Up @@ -332,6 +340,7 @@ impl<'de, F: LurkField> Deserialize<'de> for FWrap<F> {
pub mod tests {
use crate::light_data::Encodable;
use blstrs::Scalar as Fr;
use pasta_curves::{pallas, vesta};

use super::*;

Expand All @@ -347,11 +356,11 @@ pub mod tests {
repr_bytes_consistency(f1)
}
#[test]
fn prop_pallas_repr_bytes_consistency(f1 in any::<FWrap<pasta_curves::Fp>>()) {
fn prop_pallas_repr_bytes_consistency(f1 in any::<FWrap<pallas::Scalar>>()) {
repr_bytes_consistency(f1)
}
#[test]
fn prop_vesta_repr_bytes_consistency(f1 in any::<FWrap<pasta_curves::Fq>>()) {
fn prop_vesta_repr_bytes_consistency(f1 in any::<FWrap<vesta::Scalar>>()) {
repr_bytes_consistency(f1)
}
}
Expand Down Expand Up @@ -406,11 +415,11 @@ pub mod tests {
repr_canonicity(f1)
}
#[test]
fn prop_pallas_repr_canonicity(f1 in any::<FWrap<pasta_curves::Fp>>()) {
fn prop_pallas_repr_canonicity(f1 in any::<FWrap<pallas::Scalar>>()) {
repr_canonicity(f1)
}
#[test]
fn prop_vesta_repr_canonicity(f1 in any::<FWrap<pasta_curves::Fq>>()) {
fn prop_vesta_repr_canonicity(f1 in any::<FWrap<vesta::Scalar>>()) {
repr_canonicity(f1)
}
#[test]
Expand All @@ -437,7 +446,7 @@ pub mod tests {
proptest! {
#[test]
fn prop_pallas_tag_roundtrip(x in any::<u64>()){
let f1 = pasta_curves::Fp::from(x);
let f1 = pallas::Scalar::from(x);
let bytes = f1.to_repr().as_ref().to_vec();
let mut bytes_from_u64 = [0u8; 32];
bytes_from_u64[..8].copy_from_slice(&x.to_le_bytes());
Expand All @@ -446,7 +455,7 @@ pub mod tests {

#[test]
fn prop_vesta_tag_roundtrip(x in any::<u64>()){
let f1 = pasta_curves::Fq::from(x);
let f1 = vesta::Scalar::from(x);
let bytes = f1.to_repr().as_ref().to_vec();
let mut bytes_from_u64 = [0u8; 32];
bytes_from_u64[..8].copy_from_slice(&x.to_le_bytes());
Expand Down
18 changes: 9 additions & 9 deletions src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ use anyhow::Result;

use lurk::eval::lang::{Coproc, Lang};
use lurk::field::LanguageField;
use lurk::proof::nova;
use lurk::repl::{repl_cli, ReplState};
use pasta_curves::{pallas, vesta};

fn main() -> Result<()> {
pretty_env_logger::init();
Expand All @@ -27,14 +27,14 @@ fn main() -> Result<()> {
Coproc<blstrs::Scalar>,
>(Lang::<blstrs::Scalar, Coproc<blstrs::Scalar>>::new()),
LanguageField::Pallas => repl_cli::<
nova::S1,
ReplState<nova::S1, Coproc<nova::S1>>,
Coproc<nova::S1>,
>(Lang::<nova::S1, Coproc<nova::S1>>::new()),
pallas::Scalar,
ReplState<pallas::Scalar, Coproc<pallas::Scalar>>,
Coproc<pallas::Scalar>,
>(Lang::<pallas::Scalar, Coproc<pallas::Scalar>>::new()),
LanguageField::Vesta => repl_cli::<
nova::S2,
ReplState<nova::S2, Coproc<nova::S2>>,
Coproc<nova::S2>,
>(Lang::<nova::S2, Coproc<nova::S2>>::new()),
vesta::Scalar,
ReplState<vesta::Scalar, Coproc<vesta::Scalar>>,
Coproc<vesta::Scalar>,
>(Lang::<vesta::Scalar, Coproc<vesta::Scalar>>::new()),
}
}