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

Faster computation of quotient polynomial for large rotation sets #107

Closed
jonathanpwang opened this issue Dec 5, 2022 · 4 comments
Closed
Labels
enhancement New feature or request wontfix This will not be worked on

Comments

@jonathanpwang
Copy link

fn div_by_vanishing<F: FieldExt>(poly: Polynomial<F, Coeff>, roots: &[F]) -> Vec<F> {

The current division algorithm is O(N * R) where N is degree of polynomial and R is size of rotation set. For small R this is optimal, but as an improvement for larger R we may consider adding a separate FFT-based algorithm to do it in O(N log N). Just flagging this as a possibility, maybe in practice no one uses close to log N number of rotations.

Here's a random reference implementation with further references: https://stackoverflow.com/a/44854935

@CPerezz CPerezz added the enhancement New feature or request label Dec 5, 2022
@CPerezz
Copy link
Member

CPerezz commented Dec 5, 2022

IIRC the max Rotation we have is 20something. Maybe this was reduced in the last months. @han0110 will be able to clarify.

From there we can judge whether to implement this or not.

@jonathanpwang
Copy link
Author

In that case since N is around 2^20 anyways probably there's no immediate need.
It'd only be potentially interesting if we have some gates that all access the same set of >20 rotation offsets (so the shplonk verification is cheap, and this would make prover computation cheap).

@CPerezz
Copy link
Member

CPerezz commented Dec 8, 2022

I completelly agree! And wouldn't be harmfull at all. So definitely worth exploring at some point. I'm not sure if we will prioritize this item. But definitely will be happy to recieve PRs implementing it!

@CPerezz CPerezz moved this to 🆕 Product Backlog Items in zkEVM Community Edition Jan 17, 2023
@CPerezz CPerezz added the wontfix This will not be worked on label Jan 17, 2023
@CPerezz CPerezz moved this from 🆕 Product Backlog Items to Icebox in zkEVM Community Edition Jan 17, 2023
iquerejeta pushed a commit to input-output-hk/halo2 that referenced this issue May 8, 2024
…-hk/dev-feature/gl-112-implement-generic-range-check

Implement generic range check
iquerejeta pushed a commit to input-output-hk/halo2 that referenced this issue May 8, 2024
…-hk/dev-feature/53-gen-eccc

Implement Ecc Chip for Pluto ... and Vesta and Eris

# Project Context

This issue corresponds to the Milestone 3 updated SOW task "New: Extend EccChip to support Pluto". There is no corresponding GitHub issue, since this is the Pluto/Eris analog of supporting Pallas, which was the starting state. However, the work for this issue overlaps significantly with SOW task "zcash#578: Extend EccChip to support ~~Vesta~~ Eris" ([zcash#578](zcash#578)), since it seems easiest to just generalize to all curves at the same time. So, while we only need to ensure Pluto support for Milestone 3, the strategy we're using also gives Eris and Vesta support "for free".

This builds on top of privacy-scaling-explorations#145 directly, depends directly on privacy-scaling-explorations#143 and privacy-scaling-explorations#144 as well, and is the culmination of generalization work that started at a lower level with privacy-scaling-explorations#101, privacy-scaling-explorations#107, and privacy-scaling-explorations#114. 

The corresponding Galois internal issue is [Galois#53](https://gitlab-ext.galois.com/iog-midnight/halo2/-/issues/53).

# Issue Description

Generalize the Ecc Chip to arbitrary curves, allowing any assumptions that apply to all of Pallas, Vesta, Pluto, and Eris. This work is broken up into several subtasks, concerned with updating the major components of the Ecc Chip ([vbsm](privacy-scaling-explorations#143), [fbsm](privacy-scaling-explorations#145), and [point witnessing](privacy-scaling-explorations#144)), along with the final work here of updating the full Ecc Chip to use all of the updated components, have tests for all curves, and provide instantiations for all curves.

Behind the scenes there are various traits associated with the vbsm and fbsm generalizations, but the final Ecc Chip interface exposes `EccCurve` and `EccField` as the traits required for curves and fields used with Ecc Chip (and of course these traits are implemented for Pallas, Vesta, Pluto, Eris, and their fields).
@davidnevadoc
Copy link

It seems that this optimization wouldn't make an improvement in most common cases (where R< logN).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

3 participants