Fix __pow__ recursion for s390x compatibility#160
Open
ottok wants to merge 1 commit intoethereum:mainfrom
Open
Fix __pow__ recursion for s390x compatibility#160ottok wants to merge 1 commit intoethereum:mainfrom
ottok wants to merge 1 commit intoethereum:mainfrom
Conversation
Author
|
CI failed on these: I will fix them tomorrow. |
In Debian, where the tests of this Python package across multiple
architectures, it was discovered that the pairing tests fail on s390x.
Fix the recursion issue by converting FQP.__pow__ from a recursive to an
iterative square-and-multiply implementation. The new implementation is
mathematically equivalent, handles negative exponents (which the old
version didn't), and won't hit maximum recursion depth on large
exponents. With this change tests are fully passing on architecture
s390x in Debian.
Additionally, also convert the FQ classes to use the same iterative
square-and-multiply algorithm. Thehey were not seen failing on s390, but
probably because tests only run with smaller exponents on those
functions.
Also add the `inv` method to both `FQ` classes so that mypy can properly
type check the negative exponent handling in `__pow__` by calling was
calling `self.inv()`.
This is needed to avoid `mypy` failing on:
py_ecc/fields/optimized_field_elements.py:162: error: Returning Any from function declared to return "T_FQ" [no-any-return]
py_ecc/fields/optimized_field_elements.py:162: error: "T_FQ" has no attribute "inv" [attr-defined]
py_ecc/fields/field_elements.py:150: error: Returning Any from function declared to return "T_FQ" [no-any-return]
py_ecc/fields/field_elements.py:150: error: "T_FQ" has no attribute "inv" [attr-defined]
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What was wrong?
In Debian, where the tests of this Python package across multiple architectures, it was discovered that the pairing tests fail on s390x.
Supercedes PR #159.
How was it fixed?
Fix the recursion issue by converting FQP.pow from a recursive to an iterative square-and-multiply implementation. The new implementation is mathematically equivalent, handles negative exponents (which the old version didn't), and won't hit maximum recursion depth on large exponents. With this change tests are fully passing on architecture s390x in Debian.
Additionally, also convert the FQ classes to use the same iterative square-and-multiply algorithm. Thehey were not seen failing on s390, but probably because tests only run with smaller exponents on those functions.
Proof that latest version also has all tests passing on s390x: https://autopkgtest.ubuntu.com/results/autopkgtest-resolute-otto-ppa/resolute/s390x/p/python-py-ecc/20260328_153048_a9f0e@/log.gz
Todo:
Cute Animal Picture
🐱