-
Notifications
You must be signed in to change notification settings - Fork 5.8k
Update EIP-2537: Rephrased subgroup check part #8456
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
Conversation
✅ All reviewers have approved. |
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.
All Reviewers Have Approved; Performing Automatic Merge...
EIPS/eip-2537.md
Outdated
A subgroup check **is mandatory** during the pairing call. Implementations *should* use fast subgroup checks: at the time of writing, multiplication gas cost is based on the `double-and-add` multiplication method that has a clear "worst case" (all bits are equal to one). For pairing operations, it is expected that implementations use faster subgroup checks, e.g. by using the wNAF multiplication method for elliptic curves that is ~ `40%` cheaper with windows size equal to 4. (Tested empirically. Savings are due to lower hamming weight of the group order and even lower hamming weight for wNAF. Concretely, subgroup check for both G1 and G2 points in a pair are around `35000` combined). | ||
|
||
The check **is mandatory** during **ALL** the calls. Implementations *should* use fast subgroup checks. It can be stated that it **must** be performed by fast subgroup checks to achieve a speedup, resulting in a discount over naive implementations based on the `double-and-add` multiplication method, which has a clear "worst-case" scenario where all bits are equal to one. | ||
Before accepting an input, it is recommended to subject the input to the appropriate endomorphism test as described in https://eprint.iacr.org/2021/1130.pdf. |
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.
@asanso this link won't work in the EIP. I suggest either removing or uploading the full PDF as an asset in the repo.
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.
All Reviewers Have Approved; Performing Automatic Merge...
for rephrasing according to #8322 (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.
All Reviewers Have Approved; Performing Automatic Merge...
Co-authored-by: lightclient <14004106+lightclient@users.noreply.github.com>
Head branch was pushed to by a user without write access
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.
All Reviewers Have Approved; Performing Automatic Merge...
The commit 5832c53 (as a parent of 490ef62) contains errors. |
eip2537: clarify subgroup errors
Head branch was pushed to by a user without write access
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.
All Reviewers Have Approved; Performing Automatic Merge...
See ethereum/EIPs#8456 & ethereum/EIPs#8497. I updated the tests from https://github.com/ethereum/EIPs/tree/master/assets/eip-2537 and split them into separate files from the previously existing tests. --------- Co-authored-by: Somnath Banerjee <snb895@outlook.com>
See ethereum/EIPs#8456 & ethereum/EIPs#8497. I updated the tests from https://github.com/ethereum/EIPs/tree/master/assets/eip-2537 and split them into separate files from the previously existing tests. --------- Co-authored-by: Somnath Banerjee <snb895@outlook.com>
Reference: - PR: ethereum/go-ethereum#29637 - spec: ethereum/EIPs#8456
Reference: - PR: ethereum/go-ethereum#29637 - spec: ethereum/EIPs#8456
Reference: - PR: ethereum/go-ethereum#29637 - spec: ethereum/EIPs#8456
* core/vm: add precompiled contracts, addresses Prague reference: ethereum/EIPs#8945 * core/vm: remove redundant MUL precompiles in tests reference: ethereum/EIPs#8945 * params: adjust gas of BLS precompiled address References: - ethereum/EIPs#9097 - ethereum/EIPs#9098 * core/vm,params: update DiscountTable for Bls12381G1 and Bls12381G2 reference: ethereum/EIPs#9116 * core/vm: add subgroup checks for mulexp for G1/G2 Reference: - PR: ethereum/go-ethereum#29637 - spec: ethereum/EIPs#8456 * core/vm/testdata/precompiles: update precompile tests EIP-2537 * core/vm: fix wrong addresses of precompiled contracts Prague * core/vm: use gnark BLS12-381 instead of the older PR: ethereum/go-ethereum#29441 * core/vm,tests/fuzzers/bls12381: Remove unused tests
This is a continuation of #8322 (closed by mistake)
ATTENTION: ERC-RELATED PULL REQUESTS NOW OCCUR IN ETHEREUM/ERCS
--
When opening a pull request to submit a new EIP, please use the suggested template: https://github.com/ethereum/EIPs/blob/master/eip-template.md
We have a GitHub bot that automatically merges some PRs. It will merge yours immediately if certain criteria are met: