Batch math & variable-time multiscalar multiplication for redpallas #2288
Conversation
dconnolly
added
A-consensus
@dconnolly can you open a ticket for this follow-up work? |
There was a problem hiding this comment.
Looks good overall, but I think there's a bug in the batch verification.
It might also help to add some docs to some private functions and types, but I'll leave the final call on that to @conradoplg and @upbqdn.
zebra-chain/benches/redpallas.rs
Outdated
| fn sigs_with_distinct_keys() -> impl Iterator<Item = Item> { | ||
| std::iter::repeat_with(|| { | ||
| let mut rng = thread_rng(); | ||
| let msg = b""; |
There was a problem hiding this comment.
As far as I can tell, we're currently benchmarking failing signatures, because this message does not match the verification message.
So let's:
- create a constant for the messages, so they all match
- check for this bug in our other benchmarks
- consider adding an assert to make sure the signatures are valid (and to avoid optimisations that remove unused code)
We should also re-do the benchmarks, because they're variable time.
There was a problem hiding this comment.
Done in 5cd8c6f, and retested:
There was a problem hiding this comment.
Also asserting that all these signature verifications succeed as part of these benchmarks: 83c4979
|
Couple of typos in the PR description: "muliscalar" -> "multiscalar", "Weierßrass" -> "Weierstraß" or "Weierstrass" (depending on whether you use the orthography from his lifetime or modern German orthography). |
Co-authored-by: teor <[email protected]>
Co-authored-by: teor <[email protected]>
Co-authored-by: teor <[email protected]>
dconnolly
force-pushed
the
redpallas-vartime-multiscalar
branch
from
2ef1525 to
ca4de96
Compare
Done: #2320 |
…re in batch verification Along with the explicit test for bad SpendAuth's in the batch.
…m from curve25519-dalek
Motivation
We need to validate RedPallas signatures for Orchard/NU5, which check SpendAuth signatures and Binding signatures. For RedJubjub, we utilized a variable-time multiscalar multiplication implementation to do batch validation of those signatures, managed by an async Tower service. This is a useful and performant pattern that we'd like to continue using for both performance reasons but also to maintain our current async work management model.
Solution
Port the RedJubjub multiscalar multiplication and lookup tables to work over
pallas::Point, a projective coordinate representation of the short Weierstrass form of the Pallas curve, instead of the Niels representation that was used in computation for the Edwards curve, Jubjub.I originally thought this multiscalar multiplication would require different math because of the different curve models, but the math is basically the same, it just can use different representations of the points being computed over internally, which are not required to be Niels, whichever works or is fastest for your curve model. Turns out, just using the default
pallas::Point, which is projective (vspallas::Affine), is plenty fast for us, so we can just use that.Finish the batch API, add correctness tests, and add benchmarks to compare the speed of batched vs unbatched verification (smaller times are better):
Resolves #2098
Review
Reviewer Checklist
@conradoplg and @upbqdn especially, would be great to get your eyes on this math, for both knowledge-sharing and in case you know any tips or tricks to make this better. :)
Follow Up Work
Use this to finish the
RedPallasVerifierservice: #2287We currently do not run our benchmarks as part of CI, the
zebra-chain::blockbenchmarks are currently broken by default because os a misconfig of what test resources are available under which feature flags. We may want to run benchmarks as part of CI in a manner similar to our coverage jobs (informational but not blocking) to track these over time and make sure they don't break silently: #2320