search

LedgerHQ / btchip-python

Ledger HW.1 Python API
http://www.ledger.co
Apache License 2.0
65 stars 51 forks source link

Move ecdsa from extras_require to install_requires #42

Closed achow101 closed 3 years ago

achow101 commented 5 years ago

ecdsa is imported by btchipKeyRecovery.py which is imported by btchip.py. Importing btchip.py results in an ImportError without ecdsa installed, so it is a required module.

SomberNight commented 3 years ago

First of all, yes, fully agree, ecdsa is a required dependency, so please merge this PR if you just want the easy fix.

However, btchipKeyRecovery.py, the only module that imports ecdsa is all just broken dead code (assuming python3). https://github.com/LedgerHQ/btchip-python/blob/17f27c1996c75145b8eb5d16583bddcb6e2bf691/btchip/btchipKeyRecovery.py#L10-L13 It is copied from old electrum code, from python2 times. import msqr is not valid in python3; it should be replaced with from . import msqr

In light of of no one complaining for years, maybe btchipKeyRecovery.py should just be removed (along with references), and then ecdsa would not be a dependency at all.

@btchip please either make ecdsa a required dependency in setup.py as in this PR, or change the code so that ecdsa is not required for using the library

Electrum no longer requires ecdsa since https://github.com/spesmilo/electrum/commit/5a2d588e8bfa36889117b205ae6b04ec7e0ddd3a, and this is now causing troubles here (see https://github.com/spesmilo/electrum/issues/6928)

SomberNight commented 3 years ago

Please merge this and do a release. Since Electrum no longer requires ecdsa, we keep getting support requests due to this from Ledger users.