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

Jump to bottom

Optional halo2 proofs in CLI #301

Merged
merged 1 commit into from
Jun 2, 2023
Merged

Optional halo2 proofs in CLI #301

merged 1 commit into from
Jun 2, 2023

Conversation

Schaeff
Copy link
Collaborator

@Schaeff Schaeff commented May 24, 2023 •
edited
Loading

Pass --prove_with halo2 to create a KZG-GWC-Keccak halo2 proof.
Fail in the backend if we're not using bn254.
Puts halo2 behind a feature, off by default, enabled with e.g cargo run --features halo2

@Schaeff Schaeff mentioned this pull request May 24, 2023
Merged
chriseth
chriseth reviewed May 24, 2023
View reviewed changes
powdr_cli/src/main.rs Outdated
@@ -54,6 +56,11 @@ enum Commands {
#[arg(short, long)]
#[arg(default_value_t = false)]
force: bool,

/// Generate a halo2 proof
Copy link
Member

Choose a reason for hiding this comment

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

I think it would be nice to already start with an enum instead of just a bool

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

With the different backends?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Changed to Option<Backend>

@Schaeff Schaeff marked this pull request as ready for review May 24, 2023 12:41
@leonardoalt
Copy link
Member

ohh so this is the PR with the actual proofs

@Schaeff Schaeff changed the base branch from halo_poc to main May 25, 2023 13:23
leonardoalt
leonardoalt reviewed May 27, 2023
View reviewed changes
Copy link
Member

@leonardoalt leonardoalt left a comment

Choose a reason for hiding this comment

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

lgtm, tiny comments

common/src/lib.rs Outdated Show resolved Hide resolved
compiler/src/lib.rs Outdated Show resolved Hide resolved
halo2/src/prover.rs Show resolved Hide resolved
powdr_cli/Cargo.toml Outdated Show resolved Hide resolved
@Schaeff Schaeff force-pushed the optional-halo2-proofs branch 2 times, most recently from c9db0da to 3bf21d1 Compare May 30, 2023 10:18
@leonardoalt
Copy link
Member

This doesn't fully work yet because Halo2 doesn't have direct support to lookups. PSE is currently implementing it in their fork: privacy-scaling-explorations/halo2#185

@leonardoalt
Copy link
Member

Lookups are fixed now.
Permutations are ignored.

@leonardoalt
Copy link
Member

The test asm/full_pil_constant.asm currently fails in halo2 because of

impl<'a, T: FieldElement> CircuitData<'a, T> {
    pub fn from(fixed: Vec<(&'a str, Vec<T>)>, witness: Vec<(&'a str, Vec<T>)>) -> Self {
        assert_eq!(
            fixed.get(0).unwrap().1.len(),
            witness.get(0).unwrap().1.len()
        );

@leonardoalt
Copy link
Member

If there are no constants or no commits this will fail. We need a way to fake one if it doesn't exist.

Schaeff
Schaeff commented Jun 2, 2023
View reviewed changes
compiler/tests/asm.rs Outdated
);
let f = "simple_sum.asm";
let i = [16, 4, 1, 2, 8, 5];
verify_asm::<GoldilocksField>(f, i.iter().cloned().map(|x| x.into()).collect());
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Maybe we should have a util function for this [i32; N] -> Vec conversion

Copy link
Member

Choose a reason for hiding this comment

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

added in the file itself for now

@leonardoalt leonardoalt merged commit aea0b87 into main Jun 2, 2023
@leonardoalt leonardoalt deleted the optional-halo2-proofs branch June 19, 2023 08:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chriseth chriseth chriseth left review comments

@leonardoalt leonardoalt leonardoalt approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Schaeff @leonardoalt @chriseth