-
Notifications
You must be signed in to change notification settings - Fork 15
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
Optimize ScalarMult using endomorphism #8
Conversation
c27a495
to
672ab72
Compare
I figured out why the splitK function needed the +3. It's because fieldVal doesn't take negative values. This optimization makes negative k values possible. However, adding enough to the c_n's at the right time seems to always produce positive numbers. |
No more fudging add 3. There was also another error besides the sign which wasn't passed through. That was another error related to left-to-right addition that I fixed. |
Removed the modulo logic (actually unnecessary) from the splitK function. Gave a 8-9% speed boost. |
Thanks for this Jimmy. I'll review it in detail over the weekend, but I briefly looked over it so far and confirmed the choices for λ and β are accurate. That is to say the following equations hold: β^3 (mod P) = 1 |
@davecgh, I finally understand how lambda and beta are derived using Fermat's Little Theorem: Might be of interest to you since you were wondering this yourself. |
// G^N = 1 and thus any other valid point on the elliptical curve has the | ||
// same order. | ||
func (curve *KoblitzCurve) moduloReduce(k []byte) []byte { | ||
var newK []byte |
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.
I think we can do without this additional local here. Inside the first if branch, just return tmpK.Bytes(). Then just eliminate the else branch and return k.
if len(k) > curve.BitSize/8 {
tmpK := new(big.Int).SetBytes(k)
tmpK.Mod(tmpK, curve.N)
return tmpK.Bytes()
}
return k
Thanks for updating @jimmysong. It appears Travis just updated their release version of Go, so I had to modify the Also, I'll just modify it after the PR goes in to avoid going back and forth, but I absolutely do not like the The constant needs to be defined on the curve, so other curves can work as well. |
@davecgh, rebased and took out the const stuff. Is there a way to make a const field in a struct in go or is the way I did it acceptable? |
@jimmysong The way you did it is great. Thanks! |
This implements a speedup to ScalarMult using the endomorphism available to secp256k1. Note the constants lambda, beta, a1, b1, a2 and b2 are from here: https://bitcointalk.org/index.php?topic=3238.0 Preliminary tests indicate a speedup of between 17%-20% (BenchScalarMult). More speedup can probably be achieved once splitK uses something more like what fieldVal uses. Unfortunately, the prime for this math is the order of G (N), not P. Note the NAF optimization was specifically not done as that's the purview of another issue. Changed both ScalarMult and ScalarBaseMult to take advantage of curve.N to reduce k. This results in a 80% speedup to large values of k for ScalarBaseMult. Note the new test BenchmarkScalarBaseMultLarge is how that speedup number can be checked. This closes btcsuite#1
@@ -18,8 +18,7 @@ var secp256k1BytePoints = []byte{} | |||
// 0..n-1 where n is the curve's bit size (256 in the case of secp256k1) | |||
// the coordinates are recorded as Jacobian coordinates. | |||
func (curve *KoblitzCurve) getDoublingPoints() [][3]fieldVal { | |||
bitSize := curve.Params().BitSize |
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 bitSize
is used below in the loop too. It should be updated to curve.BitSize
as well or the generation code will fail to compile.
You can run rm secp256k1.go; go generate
to test.
Alright, so I'm letting this run every signature on the block chain before merging just to be paranoid, but I've independently derived and double checked all of the math and everything looks accurate. In particular: The possible values for λ and ß are derived with:
The two possible λ and ß values are indeed squares of one another:
Benchmarking the available options for λ and ß empirically show that the values chosen in this PR provide the greatest speedup. In particular, the generator for λ chosen is 3 while the generator for ß chosen is 2. The values chosen for the linearly independent vectors used during computation of the endomorphism have been independently derived and verified to satisfy the equation
Finally, the following equations hold as required:
|
Optimize ScalarMult using endomorphism
This implements a speedup to ScalarMult using the endomorphism available to secp256k1.
Note the constants lambda, beta, a1, b1, a2 and b2 are from here:
https://bitcointalk.org/index.php?topic=3238.0
Preliminary tests indicate a speedup of between 23%-28% (BenchScalarMult).
More speedup can probably be achieved once splitK uses something more like what fieldVal uses. Unfortunately, the prime for this math is the order of G (N), not P.
Note the NAF optimization was specifically not done as that's the purview of another issue.
This closes #1