-
Notifications
You must be signed in to change notification settings - Fork 839
Some BLS Optimizations #4201
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
base: master
Are you sure you want to change the base?
Some BLS Optimizations #4201
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
📦 Bundle Size Analysis
Generated by bundle-size workflow |
jochem-brouwer
left a comment
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.
Looks good!
mulG1 and mulG2 were part of BLS spec in the past but have since been moved in the actual spec (so the forked-in version) to the multi-scalar-multiplication (MSM) precompiles for G1 and G2. Reasoning was that extra precompiles are not necessary.
The modulo check you removed is I think a forgotten one from the past cleanups, so looks good.
Let's lower the bundle size 🎉 👍 😄
Some optimizations / code deletions I stumbled upon BLS coverage analysis using the official test vectors.
The modulo check was simply redundant.
The mul* methods are not used anymore since it was decided along EIP process to not do explicit mul* precompiles (since implicitly included in msm* if I recall correctly).
Will wait for a review from @jochem-brouwer before merge. Should be save though (all consensus tests passing still).