Replace device libraries with stripped down versions#120
Conversation
|
Concept ACK. I am a bit concerned about long term maintainability. I'd have to review the code get a better sense of how many moving parts we're dealing with. Ideally though we'd upstream these lighter libraries back to the manufactures (and then pin them by hash, like Bitcoin Core I'll review Ledger and Trezor stuff some time after Bitcoin Core 0.18 feature freeze. |
I don't really think that will be a problem. The changes that we are concerned about are limited to a specific set of files. After looking through the commit history of python-trezor and python-keepkey, it seems like many of the changes happening to these libraries have been for supporting altcoins or changes to their tests, neither of which are things we need here. And
I don't think that is possible. The manufacturers have obligations to support older hardware, altcoins, and other functionality that we don't use or care about. These lighter libraries are just versions of the original ones with all of that stuff deleted. |
|
concept ACK. Updating these stripped down libraries should be pretty easy even in the case of them being updated upstream. Some tests are failing. |
|
Test failure caused by bad hww branch. Should be fixed now. |
|
With #122 Ledger tests still pass for me, without the ledger python library installed. |
|
light tACK 681a52d for ledger |
7c0fd1e Update dependencies in the readme (Andrew Chow) 681a52d Replace btchip-python with our own stripped down version of it (Andrew Chow) f7d04ff Replace ckcc dependency with our own version of the library itself (Andrew Chow) f0c5aa7 Add Keepkey to Trezorlib and have KeepkeyClient use TrezorClient (Andrew Chow) 2693ae3 Change TrezorClient to do what TrezorNoInit did (Andrew Chow) aa3665c Replace trezorlib with our own stripped down version (Andrew Chow) Pull request description: This PR replaces all of the device libraries (`trezor`, `keepkey`, `ckcc-protocol,` and `btchip-python`) with stripped down versions of them directly in the source tree. These stripped down versions have several unused functions removed, including support for outdated hardware (e.g. Ledger HW.1) and altcoins. Some additional modifications have been made to them in order to make our implementations cleaner and easier to understand. One major part of this change is replacing the Keepkey implementation with the Trezor implementation entirely. I found during testing that the Keepkey and Trezor use largely the same protocol. Some parts were different but those did not affect our usage of the Keepkey and the Trezor, so those differences were removed and reconciled. This allows us to have only one library which serves both the Trezor and Keepkey and have just one implementation that works for both Trezor and Keepkey. Overall, by replacing the vendor provided libraries with our own versions of them, we are able to reduce the total number of dependencies required, and reduce some possible implementation errors (e.g. it would be a simple mistake to let Trezor or Keepkey do their default behavior of phoning home to fetch previous transactions). We can also make changes to the lower level implementations as needed instead of waiting on changes and fixes to be merged into upstream libraries. This PR is based on #118 so that everything that was changed could be tested. Tree-SHA512: 2e8e581ebf533a9306d76edfff607d6c5fe85f3764e80015b03638abb15edc76f174b013a64604d3b3e70dacf8b70d00c5a1484702d4fbb7c26625dd18da54dc
… distribution archives 8931ae0 Run tests using the binary distribution (Andrew Chow) 39a6fc9 Fixes for Windows (Andrew Chow) a229de0 Update .travis.yml to use poetry for build (Andrew Chow) 77257a1 Add build scripts and documentation for building releases (Andrew Chow) 9e04d1a Add a hwi.spec file for pyinstaller to build standalone binaries (Andrew Chow) d6b24b8 Add pyproject.toml and poetry.lock for poetry dependency manager (Andrew Chow) Pull request description: This PR adds several scripts and tools for making standalone binaries of the `hwi.py` script and for creating distribution archives that will go on pypi.org. To achieve deterministic builds, the dependencies used must be locked to specific versions and hashes. To do this, I have added configuration files for using the [Poetry dependency manager](https://github.com/sdispater/poetry). Because Poetry uses a `pyproject.toml` file instead of `setup.py`, I have created a helper script which will automatically generate the proper `setup.py` file from `pyproject.toml`. The reason I chose Poetry instead of Pipenv for this task is because it has the ability to do deterministic builds of the distribution archives (python wheel and source tar for pypi.org) which Pipenv does not have. Additionally scripts have been added to the newly created `contrib/` folder which will perform the deterministic builds of the binaries and distribution archives. The builds of the binaries are done using [pyinstaller](http://www.pyinstaller.org/). In order to build for different platforms, the `contrib/build_bin.sh` script needs to be run on each of the platforms we wish to release for. It can also be run in wine to do windows builds (see `contrib/build_wine.sh`). The configuration file that pyinstaller needs has also been added. Lastly the pyenv version for this project has been bumped to 3.6.8 since using python 3.5.x produced standalone binaries that did not work. This PR is built on #120 as reducing the number of dependencies fixed several issues with the standalone binary builds. Tree-SHA512: abc1a6ac06d663b1316cde254980b0b1e8c392a6ffe478710df7c8e48a344cd57105e83555ec8fdcdc30e2b7d6d9cd6464367afda80653cb3cbc3acaf6119f48
3 participants
This PR replaces all of the device libraries (
trezor,keepkey,ckcc-protocol,andbtchip-python) with stripped down versions of them directly in the source tree. These stripped down versions have several unused functions removed, including support for outdated hardware (e.g. Ledger HW.1) and altcoins. Some additional modifications have been made to them in order to make our implementations cleaner and easier to understand.One major part of this change is replacing the Keepkey implementation with the Trezor implementation entirely. I found during testing that the Keepkey and Trezor use largely the same protocol. Some parts were different but those did not affect our usage of the Keepkey and the Trezor, so those differences were removed and reconciled. This allows us to have only one library which serves both the Trezor and Keepkey and have just one implementation that works for both Trezor and Keepkey.
Overall, by replacing the vendor provided libraries with our own versions of them, we are able to reduce the total number of dependencies required, and reduce some possible implementation errors (e.g. it would be a simple mistake to let Trezor or Keepkey do their default behavior of phoning home to fetch previous transactions). We can also make changes to the lower level implementations as needed instead of waiting on changes and fixes to be merged into upstream libraries.
This PR is based on #118 so that everything that was changed could be tested.