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.

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

Documentation for multi-opening methods #129

Merged
merged 3 commits into from
Oct 26, 2023

Conversation

Antonio95
Copy link
Contributor

@Antonio95 Antonio95 commented Oct 24, 2023

Description

Added documentation for batch_open, batch_check, open_combinations and check_combinations, as well as an auxiliary method.

The batch_open method has been moved to a more logical place inside the lib.rs file (yielding the method order from the preceding sentence).


Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

  • Targeted PR against correct branch (master)
  • Linked to Github issue with discussion and accepted design OR have an explanation in the PR that describes this work.
  • Wrote unit tests
  • Updated relevant documentation in the code
  • Added a relevant changelog entry to the Pending section in CHANGELOG.md
  • Re-reviewed Files changed in the Github PR explorer

…tions; shifted batch_open into a more logical position
@Antonio95 Antonio95 requested a review from a team as a code owner October 24, 2023 08:59
@Antonio95 Antonio95 requested review from Pratyush, mmagician and weikengchen and removed request for a team October 24, 2023 08:59
src/lib.rs Outdated Show resolved Hide resolved
src/lib.rs Outdated
Comment on lines 250 to 258
/// Behaviour is undefined if `query_set` contains the entries with the
/// same point label but different actual points.
///
/// The opening challenges are independent for each batch of polynomials.
///
/// The default implementation achieves this by rearranging the queries in
/// order to gather (i.e. batch) all polynomials that should be queried at
/// the same point, then opening their commitments simultaneously with a
/// single call to `open` (per point)
Copy link
Member

Choose a reason for hiding this comment

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

@Antonio95 as you pointed out offline, I think this indeed belongs to the inline comment of the default implementation rather than on the trait.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed. Done! thanks

Antonio95 and others added 2 commits October 26, 2023 11:27
Co-authored-by: mmagician <marcin.gorny.94@protonmail.com>
Copy link
Member

@Pratyush Pratyush left a comment

Choose a reason for hiding this comment

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

This is great, thanks!

@Pratyush Pratyush added this pull request to the merge queue Oct 26, 2023
Merged via the queue into arkworks-rs:master with commit dac4f46 Oct 26, 2023
4 checks passed
mmagician added a commit to NP-Eng/poly-commit that referenced this pull request Nov 15, 2023
* added documentation for batch_{check, open} and {check, open} combinations; shifted batch_open into a more logical position

* Update src/lib.rs

Co-authored-by: mmagician <marcin.gorny.94@protonmail.com>

* moved description of default implementations into function body

---------

Co-authored-by: mmagician <marcin.gorny.94@protonmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants