[Design Discussion] Zk Accel API via generics [skip ci - Don't merge]

[mratsim]

This draft PR should not be merged, it's for laying out design tradeoffs of ZAL in Halo2.

This uses:

• An extra generic to add any backend supporting MsmAccel<C: CurveAffine> to Halo2
• An extra lifetime parameter to represent the ZalEngine lifetime.

Zal: https://github.com/taikoxyz/halo2curves/blob/zal-on-0.3.2/src/zal.rs

// The ZK Accel Layer API
// ---------------------------------------------------

pub trait ZalEngine: Debug {}

pub trait MsmAccel<C: CurveAffine>: ZalEngine {
    fn msm(&self, coeffs: &[C::Scalar], base: &[C]) -> C::Curve;
}

// ZAL using Halo2curves as a backend
// ---------------------------------------------------

#[derive(Debug)]
pub struct H2cEngine;

impl H2cEngine {
    pub fn new() -> Self {
        Self {}
    }
}

impl ZalEngine for H2cEngine {}

impl<C: CurveAffine> MsmAccel<C> for H2cEngine {
    fn msm(&self, coeffs: &[C::Scalar], bases: &[C]) -> C::Curve {
        #[allow(deprecated)]
        best_multiexp(coeffs, bases)
    }
}

impl Default for H2cEngine {
    fn default() -> Self {
        Self::new()
    }
}

Issues:

• very noisy, polluting every callers and types with the new modification.
• having the ZalEngine has a generic parameter has no advantage compared to a trait object. Technically it avoids a fat pointer indirection, but that cache miss may cost at most 150 cycles or so (~30ns) while the ZK compute cost 10^9 times more.
• ZalEngine does not implement Send + Sync and some high-level code in shplonk requires it. (see the dyn trait object approach PR for more).

cc @einar-taiko

[einar-taiko]

Just my immediate comments to help me think.

• very noisy, polluting every callers and types with the new modification.

On the caller side pollution: Rust's associated types can be useful here:

trait Halo2 {
    // associated type
    type Engine: ZalEngine
    ..
}

// then user just need to supply it once:
let my_halo2_engine = Halo2<Engine = H2cEngine>::new();
// no pollution
let my_proof = my_halo2_engine.prove(p_eq_np);

but the internals still have to refer to the Self::Engine, so we will have some trait-internal pollution.

• having the ZalEngine has a generic parameter has no advantage compared to a trait object. Technically it avoids a fat pointer indirection, but that cache miss may cost at most 150 cycles or so (~30ns) while the ZK compute cost 10^9 times more.

True. We likely already know the engine at compile time, so we should still be able to make it work if we want to.

• ZalEngine does not implement Send + Sync and some high-level code in shplonk requires it. (see the dyn trait object approach PR for more).

AFAIU: ZalEngine is a trait and traits don't implement traits, but types such as H2cEngine do. But we can define it as a supertrait, enforcing any implementations are Sync and Send if we want to. It depends on the vision for the API though. Intuitively I don't see problems with the handle (singleton object) representing the engine to be Send around between threads. Not sure it is useful though. But let me try to understand the error messages better.

pub trait ZalEngine: Debug + Send + Sync {}

// Should be okay because an empty struct should be `Send` and `Sync`.
impl ZalEngine for H2cEngine {}

[mratsim]

AFAIU: ZalEngine is a trait and traits don't implement traits, but types such as H2cEngine do. But we can define it as a supertrait, enforcing any implementations are Sync and Send if we want to. It depends on the vision for the API though. Intuitively I don't see problems with the handle (singleton object) representing the engine to be Send around between threads. Not sure it is useful though. But let me try to understand the error messages better.

pub trait ZalEngine: Debug + Send + Sync {}

// Should be okay because an empty struct should be `Send` and `Sync`.
impl ZalEngine for H2cEngine {}

The issue is not H2cEngine, it's when the impl of ZalEngine uses thread-local storage. You cannot Send thread local storage.