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

perf: in-circuit ECDSA on secp256k1 #497

Merged
merged 10 commits into from
Mar 1, 2023
Merged

perf: in-circuit ECDSA on secp256k1 #497

merged 10 commits into from
Mar 1, 2023

Conversation

yelhousni
Copy link
Contributor

Groth16: 4.2M --> 3M
Plonk: 10M --> 7M
todo:

@yelhousni yelhousni added the perf label Feb 27, 2023
@yelhousni yelhousni added this to the v0.9.0 milestone Feb 27, 2023
@yelhousni yelhousni requested a review from ivokub February 27, 2023 15:30
@ivokub
Copy link
Collaborator

ivokub commented Feb 28, 2023

Suggested edit:

diff --git a/std/algebra/weierstrass/point.go b/std/algebra/weierstrass/point.go
index c32932d4..afa19788 100644
--- a/std/algebra/weierstrass/point.go
+++ b/std/algebra/weierstrass/point.go
@@ -912,11 +912,15 @@ func (c *Curve[B, S]) Double(p *AffinePoint[B]) *AffinePoint[B] {
 	}
 }
 
-// Triple triples p and return it.
-// It follows [ELM03]: https://arxiv.org/pdf/math/0208038.pdf, 3.1
+// Triple triples p and return it. It follows [ELM03] (Section 3.1).
 // Saves the computation of the y coordinate of 2p as it is used only in the computation of λ2,
-// which can be computed as λ2 = -λ1-2*p.y/(x2-p.x) instead.
+// which can be computed as
+//
+//	λ2 = -λ1-2*p.y/(x2-p.x) instead.
+//
 // It doesn't modify p.
+//
+// [ELM03]: https://arxiv.org/pdf/math/0208038.pdf
 func (c *Curve[B, S]) Triple(p *AffinePoint[B]) *AffinePoint[B] {
 
 	// compute λ1 = (3p.x²+a)/2p.y, here we assume a=0 (j invariant 0 curve)
@@ -955,10 +959,15 @@ func (c *Curve[B, S]) Triple(p *AffinePoint[B]) *AffinePoint[B] {
 	}
 }
 
-// DoubleAndAdd computes 2p+q as (p+q)+p. It follows [ELM03]: https://arxiv.org/pdf/math/0208038.pdf, 3.1
+// DoubleAndAdd computes 2p+q as (p+q)+p. It follows [ELM03] (Section 3.1)
 // Saves the computation of the y coordinate of p+q as it is used only in the computation of λ2,
-// which can be computed as λ2 = -λ1-2*p.y/(x2-p.x) instead.
-// It doesn't modify p nor q.
+// which can be computed as
+//
+//	λ2 = -λ1-2*p.y/(x2-p.x)
+//
+// instead. It doesn't modify p nor q.
+//
+// [ELM03]: https://arxiv.org/pdf/math/0208038.pdf
 func (c *Curve[B, S]) DoubleAndAdd(p, q *AffinePoint[B]) *AffinePoint[B] {
 
 	// compute λ1 = (q.y-p.y)/(q.x-p.x)
@@ -996,7 +1005,7 @@ func (c *Curve[B, S]) DoubleAndAdd(p, q *AffinePoint[B]) *AffinePoint[B] {
 
 }
 
-// Select selects between p and q given the selector b. If b == 0, then returns
+// Select selects between p and q given the selector b. If b == 1, then returns
 // p and q otherwise.
 func (c *Curve[B, S]) Select(b frontend.Variable, p, q *AffinePoint[B]) *AffinePoint[B] {
 	x := c.baseApi.Select(b, &p.X, &q.X)
@@ -1008,8 +1017,11 @@ func (c *Curve[B, S]) Select(b frontend.Variable, p, q *AffinePoint[B]) *AffineP
 }
 
 // Lookup2 performs a 2-bit lookup between i0, i1, i2, i3 based on bits b0
-// and b1. Returns i0 if b0=b1=0, i1 if b0=1 and b1=0, i2 if b0=0 and b1=1
-// and i3 if b0=b1=1.
+// and b1. Returns:
+//   - i0 if b0=0 and b1=0,
+//   - i1 if b0=1 and b1=0,
+//   - i2 if b0=0 and b1=1,
+//   - i3 if b0=1 and b1=1.
 func (c *Curve[B, S]) Lookup2(b0, b1 frontend.Variable, i0, i1, i2, i3 *AffinePoint[B]) *AffinePoint[B] {
 	x := c.baseApi.Lookup2(b0, b1, &i0.X, &i1.X, &i2.X, &i3.X)
 	y := c.baseApi.Lookup2(b0, b1, &i0.Y, &i1.Y, &i2.Y, &i3.Y)

Copy link
Collaborator

@ivokub ivokub left a comment

Choose a reason for hiding this comment

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

Looks good and checks out. I just have a question about precomputing the tables or lazy-computing them. If it is better to completely recompute, then can keep as is but otherwise I would compute lazily for better auditability.

I also proposed a few edits which makes Documentation nicer. It would be nice to update std/algebra/weierstrass/doc_test.go to include the new methods, but isn't that importan as the package doc is already very good!

@ivokub ivokub linked an issue Mar 1, 2023 that may be closed by this pull request
3 tasks
@ivokub
Copy link
Collaborator

ivokub commented Mar 1, 2023

@yelhousni - I implemented the table version. Can you have a look and merge if is OK.

@ivokub ivokub mentioned this pull request Mar 1, 2023
3 tasks
@yelhousni
Copy link
Contributor Author

@yelhousni - I implemented the table version. Can you have a look and merge if is OK.

That was quick ^^
It looks great! we can merge.

@yelhousni yelhousni mentioned this pull request Mar 1, 2023
7 tasks
@yelhousni yelhousni merged commit 9ada798 into develop Mar 1, 2023
@yelhousni yelhousni deleted the perf/ecdsa branch March 1, 2023 12:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

perf: optimize ecdsa circuit
2 participants