-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Convert Sage code to Python 3 (as used by Sage >= 9) #849
Convert Sage code to Python 3 (as used by Sage >= 9) #849
Conversation
fe90a98
to
dd72422
Compare
sage/group_prover.sage
Outdated
# Compatibility wrapper for Sage versions based on Python 2 | ||
def __div__(self,other): | ||
"""Divide two fractions.""" | ||
return __truediv__(self,other) |
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.
NameError: global name '__truediv__' is not defined
. This should be return self.__truediv__(other)
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.
Ok, I assumed I'm clever enough to write three lines of Python code without testing. But apparently I was wrong. (Sorry, it's a lot of work for me to install an older sage version...) Should work now.
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.
Fwiw, nix-shell -p sage
would work but the tests take forever. Therefore you can use nix-shell '-p sage.overrideAttrs (oldAttrs: rec { buildInputs = [ makeWrapper ]; })'
to disable the tests.
Co-authored-by: Tim Ruffing <crypto@timruffing.de>
dd72422
to
13c88ef
Compare
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.
ACK 13c88ef
Summary: Backport of [[bitcoin-core/secp256k1#849 | secp256k1#849]] Test Plan: Run from the sage dir sage group_prover.sage sage weierstrass_prover.sage Reviewers: #bitcoin_abc, PiRK Reviewed By: #bitcoin_abc, PiRK Differential Revision: https://reviews.bitcoinabc.org/D9374
Summary: Backport of [[bitcoin-core/secp256k1#849 | secp256k1#849]] Test Plan: Run from the sage dir sage group_prover.sage sage weierstrass_prover.sage Reviewers: #bitcoin_abc, PiRK Reviewed By: #bitcoin_abc, PiRK Differential Revision: https://reviews.bitcoinabc.org/D9374
Some of the C source files include Sage code in comments. This PR updates these to work with a current version based on Python3 (Sage 9.0+, see https://wiki.sagemath.org/Python3-Switch). This can be seen as a small follow-up to PR bitcoin-core#849 (commit 13c88ef).
… to Python3) Some of the C source files contain contain in-comment Sage code calculating secp256k1 parameters that are already defined in the file secp256k1_params.sage. Replace that by a corresponding load instruction and access the necessary variables. In ecdsa_impl.h, update the comment to use a one-line shell command calling sage to get the values. The remaining code (test `test_add_neg_y_diff_x` in tests.c) is updated to work with a current version based on Python3 (Sage 9.0+, see https://wiki.sagemath.org/Python3-Switch). The latter can be seen as a small follow-up to PR bitcoin-core#849 (commit 13c88ef).
… to Python3) Some of the C source files contain contain in-comment Sage code calculating secp256k1 parameters that are already defined in the file secp256k1_params.sage. Replace that by a corresponding load instruction and access the necessary variables. In ecdsa_impl.h, update the comment to use a one-line shell command calling sage to get the values. The remaining code (test `test_add_neg_y_diff_x` in tests.c) is updated to work with a current version based on Python3 (Sage 9.0+, see https://wiki.sagemath.org/Python3-Switch). The latter can be seen as a small follow-up to PR bitcoin-core#849 (commit 13c88ef).
… to Python3) Some of the C source files contain contain in-comment Sage code calculating secp256k1 parameters that are already defined in the file secp256k1_params.sage. Replace that by a corresponding load instruction and access the necessary variables. In ecdsa_impl.h, update the comment to use a one-line shell command calling sage to get the values. The remaining code (test `test_add_neg_y_diff_x` in tests.c) is updated to work with a current version based on Python3 (Sage 9.0+, see https://wiki.sagemath.org/Python3-Switch). The latter can be seen as a small follow-up to PR bitcoin-core#849 (commit 13c88ef).
….sage, update to Python3) 600c5ad clean up in-comment Sage code (refer to secp256k1_params.sage, update to Python3) (Sebastian Falbesoner) Pull request description: Some of the C source files contain contain in-comment Sage code calculating secp256k1 parameters that are already defined in the file secp256k1_params.sage. Replace that by a corresponding load instruction and access the necessary variables. In ecdsa_impl.h, update the comment to use a one-line shell command calling sage to get the values. The remaining code (test `test_add_neg_y_diff_x` in tests.c) is updated to work with a current version based on Python3 (Sage 9.0+, see https://wiki.sagemath.org/Python3-Switch). The latter can be seen as a small follow-up to PR #849 (commit 13c88ef). ACKs for top commit: sipa: ACK 600c5ad real-or-random: ACK 600c5ad Tree-SHA512: a9e52f6afbce65edd9ab14203612c3d423639f450fe8f0d269a3dda04bebefa95b607f7aa0faec864cb78b46d49f281632bb1277118749b7d8613e9f5dcc8f3d
This adds one more commit on top of #848 and squashes the commits.
Thanks to @fchapoton for spotting the issue and suggesting initial fixes.