Remove the dependency on VirtualCells to query cells from columns

[Brechtpd]

Some discussion here: privacy-scaling-explorations/zkevm-circuits#1115 (comment)

It's pretty inconvenient to have to go through a mutable VirtualCells object to be able to query Expressions from columns. When building a circuits we only care about the column, the relative offset and the phase. This is just simple data that does not depend on any internal circuit data in VirtualCells. The requirement to query from VirtualCells is only there because it's used to set the query index, but that is something that can be done just as well when these expressions are submitted to halo2.

[CPerezz]

I like this TBH.

Indeed I just feel that we should have some sepparated entity that makes is easier to simply obtain expressions. Have you considered any solution design specifically?

[naure]

(reposting this comment).

TL;DR:

• The ConstraintSystem could expose the query_* methods directly.
• The constraints could be given by a method ConstraintSystem::add_gate.

[naure]

About the functionality of queried_cells and queried_selectors. Some options are:

1. Drop it. This will break MockProver and require changes there. This should not break anything else because it is private to the crate.
2. Or discover queried_cells in add_gate by walking down the Expression trees.
3. Or with a function ConstraintSystem::start_gate(…) that returns a &mut VirtualCells (no closures).

[Brechtpd]

We have some WIP code here: taikoxyz#1.

This currently exposes some extra functions on the Column type to query Expressions using just the column and some rotation, and then we set the query index when the expression gets submitted to halo2 in create_gate. For backward compatibility the old API will still work.

I'm not sure if this will be sufficient to be as flexible as possible with Expressions, but it's a start and should be easy to add additional API calls if necessary.

[naure]

Yes that makes a lot of sense, better than query_* methods.

What do you think about the ability to submit gates without a closure? ConstraintSystem::add_gate

[Brechtpd]

Would maybe be cleaner, but because some other abstraction (like the ConstraintBuilder) should already be used to collect all constraints I'm not sure if it's worth changing. That helper object will simply submits all constraints in a single create_gate call or whatever the API is so doesn't really impact how circuits are or can be written.

[naure]

Agree, moving the queries out is the important bit.

Do you intend to return a Query, or Expression(Query)?

The difference is that the query is what the synthesis needs in order to assign a cell (with a rotation abstraction instead of hard-coded offsets). So we would want it available. Then Expression can be obtained by an expr method. So:

column.cur() -> Query
    Or
column.query(offset) -> Query

query.expr() -> Expression

[Brechtpd]

Just the Expression, which basically is the same data as Query I think with the query index still set to None. I think ideally expressions on the app side would actually be Querys but but that would need too many changes. So I guess you should have all the data you need to be able to so this on the app side, unless you really need it on the halo2 side?

[naure]

If ConstraintBuilder returns Query, this is easily wrapped into Expression::Advice(query), depending which one the app wants at this place. But the reverse is not true, an Expression cannot always be unwrapped into query.

[han0110]

Resolved by #154