Remove IPA / Part1

[adria0]

In this first part, IPA functionality is removed.
Some traits still exists ( like CommitmentScheme, but I'll do it in the second part )

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 91.66667% with 2 lines in your changes missing coverage. Please review.

Project coverage is 83.87%. Comparing base (433dfbb) to head (b6f4082).

Files with missing lines	Patch %	Lines
halo2_backend/src/poly/query.rs	33.33%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #377      +/-   ##
==========================================
- Coverage   85.14%   83.87%   -1.28%     
==========================================
  Files          85       76       -9     
  Lines       18872    17583    -1289     
==========================================
- Hits        16068    14747    -1321     
- Misses       2804     2836      +32     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[guorong009]

I see that blind feature is removed through whole codebase.
I think blind is needed for privacy feature of ZK.
Why do we remove that feature?

[davidnevadoc]

Why are the blinders removed? 🤔

[adria0]

ditto (Please, take note that I removed everything that the compiler said as "unused", that are just few things.)

[guorong009]

I did some research.
Found that blinders are really used in IPA commitment scheme.
But, when @kilic and @han0110 introduce the KZG commitment scheme, they didn't use this blinder/r in commitment scheme.
51d523d#diff-928bb816879ab4beb4d0ad6d02b06d158f760aa2e95f410b2694e0f803e57486R167-R171
I believe the reason is that it(ParamsKZG::commit_lagrange) should be compatible with existing IPA scheme, at that time.
In other words, the blinder/r is not needed for KZG scheme.

If so, we can safely remove all of these blinder/r features from codebase.
Plus, update ParamsKZG::commit_lagrange function definition.

Overall, I think it is better to get to know exact reason, before removing this feature from codebase.

[adria0]

Overall, I think it is better to get to know exact reason, before removing this feature from codebase.

Sure, more knowledge is always good.

@guorong009, are you talking about changing the function definition from

    /// This commits to a polynomial using its evaluations over the $2^k$ size
    /// evaluation domain. The commitment will be blinded by the blinding factor
    /// `r`.
    fn commit_lagrange(
        &self,
        engine: &impl MsmAccel<C>,
        poly: &Polynomial<C::ScalarExt, LagrangeCoeff>,
        r: Blind<C::ScalarExt>,
    ) -> C::CurveExt;

to ?

    /// This commits to a polynomial using its evaluations over the $2^k$ size
    /// evaluation domain.
    fn commit_lagrange(
        &self,
        engine: &impl MsmAccel<C>,
        poly: &Polynomial<C::ScalarExt, LagrangeCoeff>
    ) -> C::CurveExt;

if yes, this needs refactoring in other parts of the code that I think that is better to do it in another PR.

[guorong009]

Also, Params::commit function. @adria0

[davidnevadoc]

I'm looking a bit more into this. I hadn't realized that KZG was completely ignoring blinders.
I don't think there is any bugs in the proving system though, because it was always being called with Blind::default().

It seems wrong that there is no possibility to blind the witnesses... where is the zk then?
This has been a point of discussion in the past (#73 #76) but I ignore what came out of these.

@han0110 Could you comment on this? Are we blinding witnesses at some other level, or did we just remove zk?

[guorong009]

I think we still have zk, by blinding witnesses at other level.
The ProverMulti::commit_phase has the logic of adding random scalars into blinding_factors rows of witness/advice columns.
The columns to be blinded or not, are determined by unblinded_advice_columns.
I think nobody uses this feature in circuit development.

In above 2 PRs, they discuss whether/how to remove the blinding_factors row logic from the codebase.
As long as we "blind" some rows of witness columns, there would be no way for exposing the witness polys as is.
Hence, I think we can remove extra blinder/r feature from codebase.
I believe they come from the IPA scheme which needs explicit hiding value - r, for poly commitment.

Anyway, hope to get confirmation from @han0110 or @kilic .

[davidnevadoc]

I see. If that is the case I'm fine with removing the blinders 👍

[adria0]

Why do we remove that feature?

Because its no longer used anywhere after IPA removal.

[davidnevadoc]

After revisiting the blinders issue I think that it is fine to get them removed, since the ZK need is achieved with a different mechanism. So LGTM! 🚀
Looking forward to part 2 and the removal of the rest of IPA related code!