Consensys / corset

4 stars 10 forks source link

Lookup takes register instead of columns #128

Closed letypequividelespoubelles closed 1 month ago

letypequividelespoubelles commented 1 month ago

Source columns of lookup are the register instead of the column:

Here corset searches for

image

but the first (non-padding) row of the trace is not a prprc perspective where live the EUC_FLAG, EUC_QUOTIENT etc ... columns:

image

So the lookup fails.

On the other side, if the perspective columns is the target, we must be sure too that it looks only in the ciolumns, not in the register of shared columns.

delehef commented 1 month ago

Lookups act on registers because that's the only thing that the prover knows. For it to work however, a masking (multiplication) by the perspective trigger should be added in Corset.

DavePearce commented 1 month ago

@delehef One question. Is it possible to construct a JSON trace involving columns in a perspective? I did not figure this out yet, and the code suggests to me you cannot (i.e. because Handle::new() assigns perspective to None). If this were to be supported then I guess the question is how data from two overlapping columns would be combined ?

DavePearce commented 1 month ago

For it to work however, a masking (multiplication) by the perspective trigger should be added in Corset.

@delehef Am thinking about how this would work. The perspective trigger(s) need to come from the source columns. Since these can be arbitrary expressions, we need to: (1) traverse the source expressions; (2) identify any column accesses for perspectives; (3) collect up the perspectives and return them. I feel like this should happen, at least in part, in src/transformer/selectors. How does that sound?

Presumably, we are happy that 0 is always in the target column (i.e. because we always have at least one padding row). That way, when the perspective selector is 0 it reduces to a check that 0 is in the target column.

delehef commented 1 month ago

Is it possible to construct a JSON trace involving columns in a perspective?

It should be working IIRC.

i.e. because Handle::new() assigns perspective to None

JSON parsing should be working at the register level, so the perspective does not matter anymore.

how data from two overlapping columns would be combined ?

The tracer should handle that and generate traces with registers (not columns) that can directly be stored as such.

delehef commented 1 month ago

we need to: (1) traverse the source expressions; (2) identify any column accesses for perspectives; (3) collect up the perspectives and return them

The dependencies function will do the traversing and give you back a list of Handle, from which you may (i) extract the perspectives, (ii) ensure that everything is in the same one, (iii) feed into get_perspective to retrieve the trigger.

I feel like this should happen, at least in part, in src/transformer/selectors.

Yes, I think so.

letypequividelespoubelles commented 1 month ago

After discussion with @OlivierBBB, he is in favor of dealing with perspective manually in lookups, as he deals with multi-row / multi-perspective lookups

OlivierBBB commented 4 weeks ago

There are various points where we have columns of varying perspectives in a single lookup, e.g. the following for LOG's where we have both stack columns and context columns.

image
DavePearce commented 4 weeks ago

@delehef Thanks for the comments. Looks like I'm not going to get to do it after all!

DavePearce commented 4 weeks ago

JSON parsing should be working at the register level, so the perspective does not matter anymore.

Hmmm, I will investigate further then but I didn't manage to get it working yet. Let's see if I can figure it out.