Skip to content

Use closures in enforce_constraints and new_lc#176

Merged
Pratyush merged 7 commits into
masterfrom
update-to-use-closures
Jun 13, 2025
Merged

Use closures in enforce_constraints and new_lc#176
Pratyush merged 7 commits into
masterfrom
update-to-use-closures

Conversation

@Pratyush

Copy link
Copy Markdown
Member

Description

Corresponds to arkworks-rs/snark#405

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

@Pratyush Pratyush requested a review from a team as a code owner June 10, 2025 20:36
@Pratyush Pratyush requested review from mmagician, weikengchen and z-tech and removed request for a team June 10, 2025 20:36
@Pratyush Pratyush requested a review from Copilot June 12, 2025 16:12

This comment was marked as outdated.

@Pratyush Pratyush requested a review from Copilot June 13, 2025 16:42

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors linear combination construction to defer computation via closures and updates related APIs accordingly.

  • Replaced eager cs.new_lc(...) calls with closure-based invocations (|| …) throughout the code.
  • Refactored add_many, linear_combination, and inner-product methods to build LinearCombination lazily.
  • Adjusted dependency sourcing in Cargo.toml to use a git reference for ark-relations.

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/fields/fp/mod.rs Swapped cs.new_lc to use closures; updated signatures for batching and linear-comb methods.
src/eq.rs Converted enforce_r1cs_constraint calls to closure form.
src/boolean/select.rs Updated constraint invocations to use closures and lc_diff!.
src/boolean/mod.rs Enhanced lc for Constant(true) and added a variable() helper.
src/boolean/eq.rs Refactored conditional equality methods to closures and lc_diff!.
src/boolean/allocated.rs Switched R1CS constraints to closure syntax and lc_diff!.
Cargo.toml Pointed ark-relations to a git repo and commented out patch entries.
Comments suppressed due to low confidence (2)

src/fields/fp/mod.rs:159

  • [nitpick] The signature change from a generic IntoIterator to a slice &[B] is a breaking API change. To maintain flexibility, consider re-introducing an overload or supporting IntoIterator directly.
pub fn add_many<B: Borrow<Self>>(iter: &[B]) -> Option<Self> {

src/fields/fp/mod.rs:207

  • The generic bound I1: IntoIterator<Item = B1, IntoIter: Clone> is invalid Rust syntax. Split it into separate bounds: I1: IntoIterator<Item = B1>, I1::IntoIter: Clone to compile successfully.
pub fn linear_combination<B1, B2, I1>(this: I1, other: &[B2]) -> Option<Self>

Comment thread src/fields/fp/mod.rs
@Pratyush Pratyush enabled auto-merge June 13, 2025 16:45
@Pratyush Pratyush added this pull request to the merge queue Jun 13, 2025
Merged via the queue into master with commit a9a01a7 Jun 13, 2025
11 checks passed
@Pratyush Pratyush deleted the update-to-use-closures branch June 13, 2025 17:01
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