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

Fixed CS for ECPAIRING #620

Merged
merged 7 commits into from
Mar 16, 2025
Merged

Fixed CS for ECPAIRING #620

merged 7 commits into from
Mar 16, 2025

Conversation

lorenzogentile404
Copy link
Contributor

No description provided.

@lorenzogentile404 lorenzogentile404 linked an issue Mar 13, 2025 that may be closed by this pull request
@lorenzogentile404 lorenzogentile404 self-assigned this Mar 13, 2025
(begin
(if-not-zero IS_ECPAIRING_DATA (eq! CS_ECPAIRING ACCPC))
(if-not-zero IS_ECPAIRING_RESULT (eq! CS_ECPAIRING (* SUCCESS_BIT (- 1 TRIVIAL_PAIRING))))
(if-zero (is_ecpairing) (vanishes! CS_ECPAIRING))
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't know if this is already the case in the definition but we should (force-bool (is_ecpairing)) at some point.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should we turn

(defun (is_ecpairing)
  (+ IS_ECPAIRING_DATA IS_ECPAIRING_RESULT))

into

(defun (is_ecpairing)
  (force-bool (+ IS_ECPAIRING_DATA IS_ECPAIRING_RESULT)))

?

@@ -735,7 +739,12 @@
(eq! CS_ECMUL (* ICP (is_ecmul))))

(defconstraint ecpairing-circuit-selector ()
(eq! CS_ECPAIRING ACCPC))
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm confused why this constraint didn't blow up in the past. Don't we constrain ACCPC to be zero when it's not pairing data ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

For me ACCPC was about acceptable pairs of points for the pairing circuit, at least that's what I remember the acronym to mean from yesterday's discussion @lorenzogentile404.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe it was not blowing up as it was underconstrained. That is, along pairing data it was correctly forced to be 1 for acceptable pairs. However, along pairing result rows, when SUCCESS_BIT was 1, ACCPC could have been anything.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Screenshot 2025-03-13 at 23 55 05
This is what we had, and actually still have.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't it be the case that ACCPC can only be nonzero along is_ecpairing_data ?

Copy link
Collaborator

@OlivierBBB OlivierBBB left a comment

Choose a reason for hiding this comment

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

LGTM

@lorenzogentile404 lorenzogentile404 merged commit 52757aa into master Mar 16, 2025
3 checks passed
@lorenzogentile404 lorenzogentile404 deleted the 619-fix-cs-for-ecpairing branch March 16, 2025 13:36
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.

Fix CS for ECPAIRING
2 participants