zksecurity / noname

Noname: a programming language to write zkapps
https://zksecurity.github.io/noname/
161 stars 35 forks source link

update to arkworks 0.4.0 #138

Open mimoo opened 1 month ago

mimoo commented 1 month ago

https://github.com/zksecurity/noname/pull/137 had to downgrade to 0.3.0, let's try to move to arkworks 0.4.0!

Autoparallel commented 1 month ago

I could take a stab at this!

Autoparallel commented 1 month ago

Curious what your thoughts are:

The naive approach of "just increment the version" yields some errors that point to the fact that kimchi may need a bump as well, unless we want to create some wrappers (which could work, but seems a bit iffy).

Namely: https://github.com/zksecurity/noname/blob/cf9af0b07c248561985fa13107d267655e3619de/src/backends/kimchi/mod.rs#L35-L42

We'll get:

the trait bound `ark_ff::fields::models::Fp256<kimchi::mina_curves::pasta::fields::FpParameters>: ark_ff::PrimeField` is not satisfied

when we try impl BackendField for VestaField {}

Stepping into kimchi, it's using ark_ff::PrimeField from 0.3.0 and implements PrimeField on Fp256 via the macro:

impl_Fp!(
    Fp256,
    Fp256Parameters,
    BigInteger256,
    BigInteger256,
    4,
    "256"
);

Is it worth trying to push up to 0.4.0 inside of kimchi prior to making the change here? Do we instead want to do:

pub struct VestaField(kimchi::mina_curves::pasta::Fp);

and implement the necessary traits on this wrapper? This solution seems less useful as it will likely just be undone if kimchi bumps arkworks.

katat commented 1 month ago

Nice analysis @Autoparallel !

I think we don't know when kimchi will bump the arkworks version. The wrapper approach you described seems to be way to go. If this wrapper works well, it would become the "template" solution to similar issues in the future as we may add new backend dependencies which we don't have control of.

How many traits does it have to implement? (We may need to evaluate if it is too much work to do so also.)

Autoparallel commented 1 month ago

@katat, I don't have the number of traits to impl off the top of my head, but I don't think it is too many. Also, they will likely be pretty easy to implement. My worry would be a bump in kimchi leads to undoing all of this.

If possible, poking at kimchi itself to push for a bump may alleviate more pain for everyone down the road. I'm happy to go there and chat about it too!

mimoo commented 1 month ago

there was a PR for that but it was never merged: https://github.com/o1-labs/proof-systems/pull/998

Autoparallel commented 1 month ago

Oof, and it is quite old.

Maybe it's safe to say that there is little initiative to get that bumped upstream then?

mimoo commented 1 month ago

yeah. I'm not sure what's best to do here... We might want to either:

Autoparallel commented 4 weeks ago

@mimoo what would be the replacement for kimchi?

mimoo commented 3 weeks ago

There's no real replacement for kimchi, kimchi is one backend among other backends. I think we should punt on this decision and pause this issue at the moment