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

Improve lookup support #1966

Closed
wants to merge 32 commits into from
Closed

Improve lookup support #1966

wants to merge 32 commits into from

Conversation

dannywillems
Copy link
Member

@dannywillems dannywillems commented Mar 12, 2024 •
edited
Loading

TODO:

  • integrate with serialization
  • implement the lookup env for the quotient polynomial
  • fix domain for evaluation in mvlookup.rs

@dannywillems dannywillems self-assigned this Mar 12, 2024
@dannywillems dannywillems marked this pull request as draft March 12, 2024 16:55
@dannywillems dannywillems added the enhancement New feature or request label Mar 12, 2024
volhovm
volhovm reviewed Mar 13, 2024
View reviewed changes
msm/src/ffa/constraint.rs
@@ -1,6 +1,6 @@
use crate::{
columns::{Column, ColumnIndexer},
expr::MSMExpr,
expr::E,
Copy link
Member

Choose a reason for hiding this comment

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

I'm happy with removing MSM from MSMExpr, but can we at least have Expr instead of just E? One can always do expr::Expr as E to save up three letters (although I think it decreases readability significantly, similarly to D sometimes used alongside Domain).

@dannywillems
Expr: remove name prefix related to MSM
ab0d900 
We want to have it generic
@dannywillems
Generic prover: add routine to combine the lookup
c32810d 
See https://eprint.iacr.org/2022/1530.pdf, eq 17.
@dannywillems
ColumnEnv: remove refernece to MSM in name
6e4d754 
We want to be generic
@dannywillems
LookupTableID: add into_constraint and add table id in combine_lookup
143c3af
@dannywillems
TMP: lookup column computation
4dc09e4
@dannywillems
Make clippy happy regarding into_* name and references
d739529 
```
methods called `into_*` usually take `self` by value
```

In our case, we want to avoid cloning values, therefore using references makes
more sense.
Maybe later we should refactorize the code using `into` trait implementation.
@dannywillems
MSM: add rangecheck4 and rangecheck15 lookups
c409abf
@dannywillems
MISC: use E<Fp> instead of full Expr parameters
7f026c0
@dannywillems
Move combine_lookups into lookup.rs
10cb5b6
@dannywillems
Move combine_lookups into serializatio/constraints
dad217d
@dannywillems
MISC: remove useless _
be67bcb
@dannywillems
Add specialized columns for lookup protocol
f036565
@dannywillems
Reorg main mod serialization
fc3ca46
@dannywillems
WIP
6c5a0be
@dannywillems
Change panic message when writing into lookup columns
d419623
@dannywillems
Use to_u32 instead of to_field, and implement to_field with
e8d4204
@dannywillems
Use curr_cell instead of full call
2b58c9a
@dannywillems
Column: comment partial sums
92b17d8
@dannywillems
Column: add LookupMultiplicity
ea64196
@dannywillems
Column: add LookupFixedTable(u32)
885679d
@dannywillems
Column: comment lookup aggregation
c944c34
@dannywillems
Cargo fmt
91e14dd
@dannywillems
Continue constraint_lookups
512a20a
This was referenced Mar 13, 2024
Merged
Merged
Merged
Merged
@dannywillems
Copy link
Member Author

Closing as it is done in smaller PRs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@volhovm volhovm volhovm left review comments

Assignees

@dannywillems dannywillems

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@dannywillems @volhovm