Skip to content
This repository has been archived by the owner on Nov 4, 2024. It is now read-only.

Remove the Halo2 frontend ConstraintSystem #282

Merged
merged 6 commits into from
Aug 6, 2024

Conversation

alxkzmn
Copy link
Collaborator

@alxkzmn alxkzmn commented Jul 25, 2024

This PR is a better approach to Halo2 middleware compilation than #277. It eliminates the need for TableColumns that are a purely frontend concept. If we choose to proceed with this path, we can completely remove the dependency on Halo2 frontend. I have prepared the corresponding changes here: 93f9553

@alxkzmn alxkzmn requested review from leolara and rutefig July 25, 2024 11:20
@leolara
Copy link
Collaborator

leolara commented Aug 5, 2024

@alxkzmn let's get this one that the imported columns and expressions are still from halo2 frontend and gives an error if there are. The rest is cool that it does not depend of the halo2 frontend at all

fn configure_columns_sub_circuit(&mut self, meta: &mut ConstraintSystem<F>) {
let mut advice_columns = HashMap::<UUID, Column<Advice>>::new();
let mut fixed_columns = HashMap::<UUID, Column<Fixed>>::new();
fn configure_halo2_columns(&mut self, meta: &mut ConstraintSystemBuilder<F>) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

@alxkzmn why is called meta here but cs_builder somewhere. meta was the name we were using in the times of zkevm

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll change it to constraint system too, meta is not a very meaningful name but it is used in Halo2 code.


fn convert_poly(&self, meta: &mut VirtualCells<'_, F>, src: &PolyExpr<F>) -> Expression<F> {
impl<F: PrimeField> ConstraintSystemBuilder<F> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

@alxkzmn I think all the impl of ConstraintSystemBuilder should be after the struct defintion

Copy link
Collaborator

@leolara leolara left a comment

Choose a reason for hiding this comment

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

A few style comments but approved

@alxkzmn alxkzmn merged commit 7086456 into chiquito-2024 Aug 6, 2024
4 checks passed
@alxkzmn alxkzmn deleted the halo2-middleware-compilation-refactoring branch August 6, 2024 04:43
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants