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

perf/fix: assume variable as zero constant when subtracting from itself #1089

Merged
merged 6 commits into from
Mar 22, 2024
Merged

perf/fix: assume variable as zero constant when subtracting from itself #1089

merged 6 commits into from
Mar 22, 2024

Conversation

ivokub
Copy link
Collaborator

@ivokub ivokub commented Mar 22, 2024 •
edited
Loading

Description

The issue has been reported by @wuestholz. When we perform x = api.Sub(A, A), then we do not take the fast path that x is a constant zero neither for scs or r1cs builder.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How has this been tested?

  • TestSubSameNoConstraint for scs
  • TestSubSameNoConstraint for r1cs

How has this been benchmarked?

Not benchmarked

Checklist:

  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • I did not modify files generated from templates
  • golangci-lint does not output errors locally
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

@ivokub ivokub added bug Something isn't working perf labels Mar 22, 2024
@ivokub ivokub requested a review from gbotrel March 22, 2024 13:21
@ivokub ivokub self-assigned this Mar 22, 2024
@ivokub ivokub merged commit 6fed1e2 into master Mar 22, 2024
7 checks passed
@ivokub ivokub deleted the fix/omit-same-subtract branch March 22, 2024 14:07
Tabaie added a commit that referenced this pull request Mar 27, 2024
@Tabaie
Squashed commit of the following:
33d71da 
commit 3abde11
Author: Ivo Kubjas <ivo.kubjas@consensys.net>
Date:   Fri Mar 22 16:35:33 2024 +0100

    feat: add range check selector retrieval (#1066)

    * feat: add range check selector retrieval

    * perf: on-the-fly wirestorer

    * docs: use constraint externally

    * feat: add sanity check that new gates are for witness

commit 6fed1e2
Author: Ivo Kubjas <ivo.kubjas@consensys.net>
Date:   Fri Mar 22 15:07:03 2024 +0100

    perf/fix: assume variable as zero constant when subtracting from itself (#1089)

    * test: add test case for no-op subtraction

    * fix: mark value as constant when coefficient zero

    * test: add expected constant equality assertion

    * perf: direct equality check for constants

    * fix: return constant zero if coeff zero

    * fix: test assert to zero

commit 9bb4153
Merge: ce0186e 2d17ac1
Author: Youssef El Housni <youssef.elhousni@consensys.net>
Date:   Wed Mar 20 10:23:57 2024 -0400

    Merge pull request #1085 from Consensys/perf/ec-arithmetic-2chain

    Perf: optimize scalar multiplication for 2-chains

commit 2d17ac1
Author: Youssef El Housni <youssef.housni21@gmail.com>
Date:   Tue Mar 19 22:13:06 2024 -0400

    chore: update stats

commit a7b94f7
Author: Youssef El Housni <youssef.housni21@gmail.com>
Date:   Tue Mar 19 19:09:18 2024 -0400

    perf(2-chain/pairing): few small optims

commit 95c2270
Author: Youssef El Housni <youssef.housni21@gmail.com>
Date:   Tue Mar 19 17:52:11 2024 -0400

    perf(2-chain/pairing): replace subs with adds

commit c15c7be
Author: Youssef El Housni <youssef.housni21@gmail.com>
Date:   Tue Mar 19 13:02:43 2024 -0400

    perf(kzg): use MSM instead of two SM in CheckOpeningProof

commit b97db99
Author: Youssef El Housni <youssef.housni21@gmail.com>
Date:   Mon Mar 18 16:37:35 2024 -0400

    perf(2-chain/bls24): optimize varScalarMul for G2

commit 14d4784
Author: Youssef El Housni <youssef.housni21@gmail.com>
Date:   Mon Mar 18 16:27:28 2024 -0400

    perf(2-chain/bls12): optimize varScalarMul for G2

commit 92a9d38
Author: Youssef El Housni <youssef.housni21@gmail.com>
Date:   Mon Mar 18 15:54:30 2024 -0400

    perf(bls24): optimize varScalarMul

commit 902fc1b
Author: Youssef El Housni <youssef.housni21@gmail.com>
Date:   Mon Mar 18 12:54:22 2024 -0400

    perf: replace dummy G by (0,1) in ScalarMul

commit beccb36
Author: Youssef El Housni <youssef.housni21@gmail.com>
Date:   Mon Mar 18 12:15:50 2024 -0400

    fix: folded MSM scalar decomposition

commit dafaacb
Author: Youssef El Housni <youssef.housni21@gmail.com>
Date:   Fri Mar 15 15:44:39 2024 -0400

    perf(2-chain): optimize folded MSM

commit 0457871
Author: Youssef El Housni <youssef.housni21@gmail.com>
Date:   Fri Mar 15 15:42:59 2024 -0400

    perf(2-chain): handle edge cases in varScalarMul

commit 9bc2788
Author: Youssef El Housni <youssef.housni21@gmail.com>
Date:   Fri Mar 15 14:41:55 2024 -0400

    perf(2-chain): optimize varScalarMul

commit ce0186e
Author: Ivo Kubjas <ivo.kubjas@consensys.net>
Date:   Tue Mar 12 18:56:55 2024 +0100

    feat: add MulNoReduce and Sum methods in field emulation (#1072)

    * feat: implement mulnoreduce

    * test: mulnoreduce test

    * docs: add method doc

    * feat: add AddMany

    * refactor: rename AddMany to Sum

    * feat: if only single input then return as is

    * test: non-native sum

commit 781de03
Merge: bb26665 9f72d90
Author: Youssef El Housni <youssef.elhousni@consensys.net>
Date:   Tue Mar 12 11:14:39 2024 -0400

    Merge pull request #1061 from Consensys/perf/ec-arithmetic

    Perf: optimize EC arithmetic

commit 9f72d90
Author: Youssef El Housni <youssef.housni21@gmail.com>
Date:   Tue Mar 12 11:03:43 2024 -0400

    docs: clean comments

commit ce6b81c
Merge: 3ce0ffa bb26665
Author: Youssef El Housni <youssef.housni21@gmail.com>
Date:   Tue Mar 12 11:03:12 2024 -0400

    Merge branch 'master' into perf/ec-arithmetic

commit bb26665
Author: Ivo Kubjas <ivo.kubjas@consensys.net>
Date:   Tue Mar 12 15:56:53 2024 +0100

    fix: emulated hint tests (#1083)

    * fix: include solver hints

    * fix: add dummy constraint to allow plonk SRS gen

commit 3ce0ffa
Merge: 6709f1b 4ae5707
Author: Youssef El Housni <youssef.housni21@gmail.com>
Date:   Tue Mar 12 10:22:17 2024 -0400

    Merge branch 'master' into perf/ec-arithmetic

commit 6709f1b
Author: Youssef El Housni <youssef.housni21@gmail.com>
Date:   Tue Mar 12 10:21:51 2024 -0400

    Revert "feat: add non-native hint with native output"

    This reverts commit cdedeca.

commit 4ae5707
Merge: 732620b 4ed9999
Author: Youssef El Housni <youssef.elhousni@consensys.net>
Date:   Tue Mar 12 10:21:05 2024 -0400

    Merge pull request #1080 from Consensys/feat/emulated-nativehint

    feat: add hint calling with either native inputs or outputs

commit 4ed9999
Author: Ivo Kubjas <ivo.kubjas@consensys.net>
Date:   Mon Mar 11 13:47:41 2024 +0000

    test: add tests for all types of hints

commit ebf9326
Author: Ivo Kubjas <ivo.kubjas@consensys.net>
Date:   Mon Mar 11 13:47:26 2024 +0000

    docs: add hint definition for native inputs

commit df7cc97
Author: Ivo Kubjas <ivo.kubjas@consensys.net>
Date:   Mon Mar 11 13:45:27 2024 +0000

    docs: method doc native output

commit c9cf735
Author: Ivo Kubjas <ivo.kubjas@consensys.net>
Date:   Mon Mar 11 13:44:37 2024 +0000

    feat: add non-native hint with native inputs

commit 2c49a0f
Author: Ivo Kubjas <ivo.kubjas@consensys.net>
Date:   Fri Mar 8 13:20:15 2024 +0000

    feat: add non-native hint with native output

commit 5cfccb7
Merge: 1b7c6d0 732620b
Author: Youssef El Housni <youssef.housni21@gmail.com>
Date:   Sun Mar 10 20:39:17 2024 -0400

    Merge branch 'master' into perf/ec-arithmetic

commit 732620b
Merge: 2e80e8a a485ada
Author: Youssef El Housni <youssef.elhousni@consensys.net>
Date:   Sun Mar 10 20:37:04 2024 -0400

    Merge pull request #1077 from shramee/faster-fq6-01

    Faster cubic 012 mul 01

commit a485ada
Merge: 09a3327 2e80e8a
Author: Youssef El Housni <youssef.housni21@gmail.com>
Date:   Sun Mar 10 20:28:10 2024 -0400

    Merge branch 'master' into faster-fq6-01

commit 2e80e8a
Merge: 1ed22f7 19c7716
Author: Youssef El Housni <youssef.elhousni@consensys.net>
Date:   Sun Mar 10 20:09:21 2024 -0400

    Merge pull request #1076 from shramee/faster-fq6-01-01

    Faster cubic 01 01 mul

commit 09a3327
Author: Shramee Srivastav <shramee.srivastav@gmail.com>
Date:   Sat Mar 9 23:41:44 2024 +0530

    chore update stats

commit 39cfb22
Author: Shramee Srivastav <shramee.srivastav@gmail.com>
Date:   Sat Mar 9 14:30:34 2024 +0530

    bls24315: faster e12 MulBy01

commit 1ba12f6
Author: Shramee Srivastav <shramee.srivastav@gmail.com>
Date:   Sat Mar 9 14:30:27 2024 +0530

    bls24315: test e12 MulBy01

commit 1818bce
Author: Shramee Srivastav <shramee.srivastav@gmail.com>
Date:   Sat Mar 9 14:21:10 2024 +0530

    bls12377: faster e6 MulBy01

commit 4eff191
Author: Shramee Srivastav <shramee.srivastav@gmail.com>
Date:   Sat Mar 9 14:20:19 2024 +0530

    bls12377: test e6 MulBy01

commit c2a37fd
Author: Shramee Srivastav <shramee.srivastav@gmail.com>
Date:   Sat Mar 9 14:02:57 2024 +0530

    bw6761: faster e3 MulBy01

commit 859ee92
Author: Shramee Srivastav <shramee.srivastav@gmail.com>
Date:   Sat Mar 9 14:00:20 2024 +0530

    bls12381: faster e6 MulBy01

commit 444f06f
Author: Shramee Srivastav <shramee.srivastav@gmail.com>
Date:   Sat Mar 9 13:54:22 2024 +0530

    bn254: faster e6 MulBy01

commit 19c7716
Author: Shramee Srivastav <shramee.srivastav@gmail.com>
Date:   Sat Mar 9 10:32:22 2024 +0530

    chore update stats

commit 1b7c6d0
Author: Youssef El Housni <youssef.housni21@gmail.com>
Date:   Fri Mar 8 23:16:55 2024 -0500

    perf(kzg): remove folding and shrinked scalars options in MSM

commit 9fc5c14
Author: Youssef El Housni <youssef.housni21@gmail.com>
Date:   Fri Mar 8 20:12:18 2024 -0500

    docs: add comments

commit c2031a6
Author: Shramee Srivastav <shramee.srivastav@gmail.com>
Date:   Sat Mar 9 02:13:21 2024 +0530

    chore lint

commit c7f1e51
Author: Shramee Srivastav <shramee.srivastav@gmail.com>
Date:   Sat Mar 9 02:10:17 2024 +0530

    comments for mul 01 by 01 tests

commit 426e330
Author: Shramee Srivastav <shramee.srivastav@gmail.com>
Date:   Sat Mar 9 01:49:52 2024 +0530

    bw6761: test mul 01 by 01

commit 10215c6
Author: Shramee Srivastav <shramee.srivastav@gmail.com>
Date:   Sat Mar 9 01:22:59 2024 +0530

    bls12377: test mul 01 by 01

commit 1c35291
Merge: a2f0bdc 1ed22f7
Author: Youssef El Housni <youssef.housni21@gmail.com>
Date:   Fri Mar 8 14:37:22 2024 -0500

    Merge branch 'master' into perf/ec-arithmetic

commit 718671d
Author: Shramee Srivastav <shramee.srivastav@gmail.com>
Date:   Sat Mar 9 01:04:49 2024 +0530

    bn254: test mul 01 by 01

commit 2845b9b
Author: Shramee Srivastav <shramee.srivastav@gmail.com>
Date:   Sat Mar 9 00:16:09 2024 +0530

    faster Mul01By01

commit a2f0bdc
Author: Youssef El Housni <youssef.housni21@gmail.com>
Date:   Fri Mar 8 14:23:03 2024 -0500

    fix: edge cases in SM and JSM were inverted + comments

commit 1ed22f7
Author: Gautam Botrel <gautam.botrel@gmail.com>
Date:   Fri Mar 8 10:39:40 2024 -0600

    refactor: kill backend.PLONK_FRI (#1075)

commit 94124b6
Author: Gautam Botrel <gautam.botrel@gmail.com>
Date:   Fri Mar 8 09:43:49 2024 -0600

    Revert "refactor: kill backend.PLONK_FRI"

    This reverts commit e7885c3.

commit e7885c3
Author: Gautam Botrel <gautam.botrel@gmail.com>
Date:   Fri Mar 8 09:42:44 2024 -0600

    refactor: kill backend.PLONK_FRI

commit 9fa2c4c
Author: Ivo Kubjas <ivo.kubjas@consensys.net>
Date:   Fri Mar 8 14:07:05 2024 +0000

    fix: incorrect parameter

commit a3f25f3
Author: Ivo Kubjas <ivo.kubjas@consensys.net>
Date:   Fri Mar 8 13:29:04 2024 +0000

    perf: use less outputs (joint)

commit aeb2509
Author: Ivo Kubjas <ivo.kubjas@consensys.net>
Date:   Fri Mar 8 13:21:11 2024 +0000

    perf: use less outputs from hints

commit 2238e16
Author: Ivo Kubjas <ivo.kubjas@consensys.net>
Date:   Fri Mar 8 13:20:52 2024 +0000

    perf: optimize hint computation with corresponding output field

commit cdedeca
Author: Ivo Kubjas <ivo.kubjas@consensys.net>
Date:   Fri Mar 8 13:20:15 2024 +0000

    feat: add non-native hint with native output

commit 3c6741c
Author: Ivo Kubjas <ivo.kubjas@consensys.net>
Date:   Fri Mar 8 12:37:59 2024 +0000

    perf: do not use multiplication for subscalar check

commit bc1c711
Author: Youssef El Housni <youssef.housni21@gmail.com>
Date:   Thu Mar 7 19:10:36 2024 -0500

    chore: update stats

commit e992856
Merge: 289413d 3dedc99
Author: Youssef El Housni <youssef.housni21@gmail.com>
Date:   Thu Mar 7 19:09:27 2024 -0500

    Merge branch 'master' into perf/ec-arithmetic

commit 289413d
Author: Youssef El Housni <youssef.housni21@gmail.com>
Date:   Thu Mar 7 18:58:46 2024 -0500

    fix(emulated/JointScalarMul): avoid malicious hint in decomposeScalar

commit c35311d
Author: Youssef El Housni <youssef.housni21@gmail.com>
Date:   Thu Mar 7 18:37:06 2024 -0500

    perf: simplify the glv decomposition hint

commit 2f2fadc
Author: Youssef El Housni <youssef.housni21@gmail.com>
Date:   Thu Mar 7 18:30:02 2024 -0500

    fix(emulated/JointScalarMul): edge case where P+Q is maliciously crafted

commit 3dedc99
Author: Ivo Kubjas <ivo.kubjas@consensys.net>
Date:   Thu Mar 7 15:19:41 2024 +0100

    perf: emulated equality assertion (#1064)

    * perf: sum over limbs for IsZero

    * perf: use mulmod for equality assertion

    * fix: handle edge case in mulcheck with zero limbs

    * refactor: do not use temp var

    * feat: remove AssertLimbsEquality

    * feat: implement shortOne() method

    * chore: remove unused private methods

    * docs: equality assertion

    * fix: deduce maximum degree from all mulcheck inputs

    * test: enable all mul tests

    * chore: stats

    * refactor: generic impl for assert/mul

    * fix: mul pre cond overflow computation

    * docs: comments

    * chore: stats

commit c7d831d
Author: Youssef El Housni <youssef.housni21@gmail.com>
Date:   Wed Mar 6 16:17:27 2024 -0500

    perf: big optim for JointScalarMul and MSM

commit 92b6a8d
Author: Youssef El Housni <youssef.housni21@gmail.com>
Date:   Wed Mar 6 13:43:52 2024 -0500

    perf: save some negs in ec arithmetic

commit c759df0
Author: Youssef El Housni <youssef.housni21@gmail.com>
Date:   Wed Mar 6 13:21:32 2024 -0500

    fix: JointScalarMulBase without GLV (for ecdsa package)

commit 64299a1
Author: Youssef El Housni <youssef.housni21@gmail.com>
Date:   Wed Mar 6 12:28:12 2024 -0500

    chore: update stats

commit 0fda05c
Author: Youssef El Housni <youssef.housni21@gmail.com>
Date:   Wed Mar 6 11:22:14 2024 -0500

    perf: big optim for JointScalarMulBase

commit d6b0320
Author: Youssef El Housni <youssef.housni21@gmail.com>
Date:   Tue Mar 5 13:41:22 2024 -0500

    perf(ecrecover): save 1 MulMod in ecrecover

commit c90e690
Author: Youssef El Housni <youssef.housni21@gmail.com>
Date:   Tue Mar 5 13:17:00 2024 -0500

    perf(2-chain): small scs optim to doubleAndAdd

commit a354496
Merge: a883c92 22d2c33
Author: Youssef El Housni <youssef.housni21@gmail.com>
Date:   Tue Mar 5 12:15:01 2024 -0500

    Merge branch 'master' into perf/ec-arithmetic

commit 22d2c33
Merge: 7cfcd5a 833fd73
Author: Youssef El Housni <youssef.elhousni@consensys.net>
Date:   Tue Mar 5 12:13:50 2024 -0500

    Merge pull request #1068 from Consensys/fix/recorded-scs

    fix: scs add/mul when recorded constraint is 0

commit 833fd73
Author: Ivo Kubjas <ivo.kubjas@consensys.net>
Date:   Tue Mar 5 16:57:46 2024 +0000

    perf: do not store zero mul constraint

commit 91cd05e
Author: Ivo Kubjas <ivo.kubjas@consensys.net>
Date:   Tue Mar 5 16:57:34 2024 +0000

    test: add test case for not recording zero mul constraint

commit 742120e
Author: Ivo Kubjas <ivo.kubjas@consensys.net>
Date:   Tue Mar 5 16:57:14 2024 +0000

    test: add regression test for zero mul duplicate

commit e2072ae
Author: Ivo Kubjas <ivo.kubjas@consensys.net>
Date:   Tue Mar 5 16:56:09 2024 +0000

    fix: remove duplicate error check

commit e223800
Author: Ivo Kubjas <ivo.kubjas@consensys.net>
Date:   Tue Mar 5 16:54:33 2024 +0000

    Revert "fix: scs add/mul when recorded constraint is 0"

    This reverts commit d94f455.

commit a883c92
Author: Youssef El Housni <youssef.housni21@gmail.com>
Date:   Sun Mar 3 15:11:30 2024 -0700

    perf: save 4 scs in lookup2 api

commit d94f455
Author: Youssef El Housni <youssef.housni21@gmail.com>
Date:   Fri Mar 1 17:00:51 2024 -0700

    fix: scs add/mul when recorded constraint is 0

commit 7cc8816
Author: Youssef El Housni <youssef.housni21@gmail.com>
Date:   Thu Feb 22 17:29:13 2024 -0500

    perf(emulated): ScalarMulBase with GLV is better

commit 18d4d10
Merge: 4c69e3e 7cfcd5a
Author: Youssef El Housni <youssef.housni21@gmail.com>
Date:   Thu Feb 22 16:54:54 2024 -0500

    Merge branch 'master' into perf/ec-arithmetic

commit 4c69e3e
Merge: 8bc71b4 45d201a
Author: Youssef El Housni <youssef.housni21@gmail.com>
Date:   Thu Feb 22 16:54:08 2024 -0500

    Merge branch 'master' into perf/ec-arithmetic

commit 8bc71b4
Author: Youssef El Housni <youssef.housni21@gmail.com>
Date:   Thu Feb 22 13:55:54 2024 -0500

    perf(2-chain): save 1 add in varScalarMul in G2

commit da9513e
Author: Youssef El Housni <youssef.housni21@gmail.com>
Date:   Wed Feb 21 20:36:03 2024 -0500

    perf(emulated): save 1 add in scalarMulGLV

commit 16990de
Author: Youssef El Housni <youssef.housni21@gmail.com>
Date:   Tue Feb 20 19:07:31 2024 -0500

    perf(emulated): huge optim scalarMulGLV

commit 33b31c5
Author: Youssef El Housni <youssef.housni21@gmail.com>
Date:   Sun Feb 18 14:50:41 2024 -0500

    perf: more small optim to jointScalarMulGLV

commit f7b7d9a
Author: Youssef El Housni <youssef.housni21@gmail.com>
Date:   Sun Feb 18 14:37:57 2024 -0500

    perf: more optim to jointScalarMulGLV

commit 8ee1fbc
Author: Youssef El Housni <youssef.housni21@gmail.com>
Date:   Fri Feb 16 17:00:21 2024 -0500

    perf(emulated): big optim jointScalarMulGLV

commit 56b7937
Author: Youssef El Housni <youssef.housni21@gmail.com>
Date:   Fri Feb 16 12:53:39 2024 -0500

    perf(emulated): big optim scalarMulGLV

commit 1f2d155
Author: Youssef El Housni <youssef.housni21@gmail.com>
Date:   Fri Feb 16 11:27:41 2024 -0500

    perf(2-chains): small optim in varScalarMul and JointScalarMul

commit 10c242a
Author: Youssef El Housni <youssef.housni21@gmail.com>
Date:   Fri Feb 16 11:04:34 2024 -0500

    perf: small optim in jointScalarMulGLV

commit 5cd0913
Author: Youssef El Housni <youssef.housni21@gmail.com>
Date:   Fri Feb 16 10:39:10 2024 -0500

    Revert "perf(2-chains): save an addition per iteration in ScalarMul"

    This reverts commit 4d71f79.

commit b2b96a6
Author: Youssef El Housni <youssef.housni21@gmail.com>
Date:   Fri Feb 16 10:35:34 2024 -0500

    perf(emulated): optimize GLV hint

commit 73a7cd6
Author: Youssef El Housni <youssef.housni21@gmail.com>
Date:   Thu Feb 15 17:37:10 2024 -0500

    perf: small optim replacing Sub by Add

commit 4d71f79
Author: Youssef El Housni <youssef.housni21@gmail.com>
Date:   Wed Feb 14 16:20:07 2024 -0500

    perf(2-chains): save an addition per iteration in ScalarMul
@ivokub ivokub added the fuzzing Issue found using a fuzzing tool label Jun 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gbotrel gbotrel gbotrel approved these changes

Assignees

@ivokub ivokub

Labels
bug Something isn't working fuzzing Issue found using a fuzzing tool perf
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ivokub @gbotrel