-
Notifications
You must be signed in to change notification settings - Fork 270
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
fix: ECCVM correctly handles points at infinity and group operation edge cases #6388
Conversation
…inity, and have +/- methods that correctly handle points at infinity
… doubling edge cases upgraded eccvm transcript to correctly handle an MSM composed entirely of points at infinity and/or zero scalars
moved eccvm boolean relations into low degree relation class, added missing boolean checks removed unused transcript columns
9f6b4ef
to
175a9b2
Compare
6245aa9
to
08edd99
Compare
…s at infinity, and have +/- methods that correctly handle points at infinity" This reverts commit 9f6b4ef.
5762321
to
db9e070
Compare
`Relation ECCVMTranscriptRelation, subrelation index 24 failed at row 138`
Benchmark resultsMetrics with a significant change:
Detailed resultsAll benchmarks are run on txs on the This benchmark source data is available in JSON format on S3 here. Proof generationEach column represents the number of threads used in proof generation.
L2 block published to L1Each column represents the number of txs on an L2 block published to L1.
L2 chain processingEach column represents the number of blocks on the L2 chain where each block has 16 txs.
Circuits statsStats on running time and I/O sizes collected for every kernel circuit run across all benchmarks.
Stats on running time collected for app circuits
Tree insertion statsThe duration to insert a fixed batch of leaves into each tree type.
MiscellaneousTransaction sizes based on how many contract classes are registered in the tx.
Transaction size based on fee payment method | Metric | | |
@@ -119,7 +119,7 @@ TEST_F(ClientIVCTests, BasicFailure) | |||
* @brief Prove and verify accumulation of an arbitrary set of circuits | |||
* | |||
*/ | |||
TEST_F(ClientIVCTests, DISABLED_BasicLarge) | |||
TEST_F(ClientIVCTests, BasicLarge) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes in this PR allow us to reinstate these core tests.
@@ -37,17 +38,17 @@ class ECCVMFlavor { | |||
using RelationSeparator = FF; | |||
using MSM = bb::eccvm::MSM<CycleGroup>; | |||
|
|||
static constexpr size_t NUM_WIRES = 74; | |||
static constexpr size_t NUM_WIRES = 85; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note 15% additional time spent on commitments.
This PR adds handling for edge cases in the ECCVM to allow handling the point at infinity. In the ECCVM the point at infinity is encoded as
(0,0)
, which does not lie on either of BN254 or Grumpkin.From benchmarks below, additional ECCVM prover overhead: 1764$\leadsto$ 1895 is 7.4%.
Before
After