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

Plonkup - Clarify the arity structure for the tables #589

Closed
vlopes11 opened this issue Sep 27, 2021 · 1 comment
Closed

Plonkup - Clarify the arity structure for the tables #589

vlopes11 opened this issue Sep 27, 2021 · 1 comment
Assignees
@ureeves
Labels
module:plonkup Issue related to plonkup implementation team:Core Low Level Core Development Team (Rust) type:docs deliverable should be documentation
Milestone
Implementing PlonkUp

Comments

@vlopes11
Copy link
Contributor

Describe what kind of specification you want to have created
Currently plonkup added PlonkupTable3Arity and PlonkupTable4Arity. However, the turbo composer is implemented to support only PlonkupTable4Arity as in https://github.com/dusk-network/plonk/blob/add_lookups/src/constraint_system/composer.rs#L102

We need:

  1. Understand if we have a clear benefit or use case to have multiple arity definitions
  2. Generalize the test cases in https://github.com/dusk-network/plonk/blob/add_lookups/src/plonkup/table/lookup_table.rs#L585 to cover every table implementation (currently its covering only PlonkupTable3Arity
@vlopes11 vlopes11 added type:docs deliverable should be documentation team:Core Low Level Core Development Team (Rust) module:plonkup Issue related to plonkup implementation labels Sep 27, 2021
@marta-belles
Copy link
Contributor

Current documentation of plonkup contains a section explaining how to handle multiple tables which does not match the current implementation. The document should also be changed according to the code.

@marta-belles marta-belles mentioned this issue Sep 30, 2021
Closed
@ZER0 ZER0 added this to the Implementing PlonkUp milestone Oct 6, 2021
@ureeves ureeves self-assigned this Oct 14, 2021
ureeves pushed a commit that referenced this issue Oct 14, 2021
Remove 3arity table and refactor accordingly
4e92ea4 
* Remove `PlonkupTable3Arity`
* Change to use `PlonkupTable4Arity`
* Rename `PlonkupTable4Arity` to `IndexTable`
* Fix tests to work with `IndexTable`

See also: #589
@ureeves ureeves mentioned this issue Oct 14, 2021
Merged
ureeves added a commit that referenced this issue Oct 15, 2021
@ureeves @ZER0
Remove Plonkup3Arity (#619)
15476be 
* Remove `PlonkupTable3Arity`
* Change to use `PlonkupTable4Arity`
* Rename `PlonkupTable4Arity` to `IndexTable`
* Fix tests to work with `IndexTable`
* Remove bigint crate and the usage of `u256` from plonkup

See also: #589 

Co-authored-by: zer0 <matteo@dusk.network>
vlopes11 pushed a commit that referenced this issue Oct 18, 2021
@ureeves @ZER0 @vlopes11
Remove Plonkup3Arity (#619)
c299350 
* Remove `PlonkupTable3Arity`
* Change to use `PlonkupTable4Arity`
* Rename `PlonkupTable4Arity` to `IndexTable`
* Fix tests to work with `IndexTable`
* Remove bigint crate and the usage of `u256` from plonkup

See also: #589 

Co-authored-by: zer0 <matteo@dusk.network>
@ZER0 ZER0 closed this as completed Nov 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ureeves ureeves

Labels
module:plonkup Issue related to plonkup implementation team:Core Low Level Core Development Team (Rust) type:docs deliverable should be documentation
Projects
None yet
Milestone
Implementing PlonkUp
Development

No branches or pull requests
4 participants
@ZER0 @vlopes11 @ureeves @marta-belles