Ledger implementation #3
Draft
Conversation
This file contains 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
There are a number of trezor and keepkey tests which rely on enumerate and then looking at the enumerated results. However some are looking for the wrong thing and inadverdently doing nothing. So extra checks are added to make sure they are actually doing something.
0735b09 tests: Make sure device is found when enumerating (Andrew Chow) 6b043d3 test: Fix label tests (Andrew Chow) Pull request description: These tests weren't working. Also fixed another keepkey test bug. Top commit has no ACKs. Tree-SHA512: 2baab00110428672f5b726707a7c996363d2cd884dcb6c9e94c9d099094330021ee6fb1a386e4415c340306a55c08e26575964d5af73dbc0897c4ac012f15267
We bumped deps, but didn't need to restrict to all of the new versions. Some older versions can still be used, so allow them.
…setup.py a8f955a tests: Remove extraneous debug line (Andrew Chow) 8f6b7e9 deps: regenerate setup.py (Andrew Chow) 8719bfe deps: regenerate poetry.lock (Andrew Chow) 3fde13e deps: Allow older versions (Andrew Chow) Pull request description: After bitcoin-core#531, we updated dependencies but accidentally removed support for older versions that aren't actually incompatible with our usage. So re-allow those. Also regenerates the setup.py which I forgot to do. Closes bitcoin-core#537 Top commit has no ACKs. Tree-SHA512: 86e520c8444a1cd9985beb0a4a0081d07d6e4d4fce958fd133743c20cafb07bf9fa31927308e7519a4fdf3b287d4c2d8de02a8160cc9d24d9ce8f535b07f017f
Keepkey is now displaying the passphrase to users after it receives it and responding with a ButtonRequest message. However this was not being properly handled by sendpin which would cause the device to hang as it waited for a ButtonAck. To resolve this, use self.call but avoid the firmware check which will fail due to the lack of self.features.
Keepkey and Trezor had firmware changes that made sendpin not work when passphrase protection was enabled and a passphrase was provided. Modify the test to check for this case.
…ent during sendpin 6fbb2a2 tests: Test sendpin with passphrase (Andrew Chow) 2fa0c94 keepkey, trezor: Handle if ButtonRequest is sent during sendpin (Andrew Chow) Pull request description: Keepkey and Trezor may ask the user for confirmation of their passphrase after the PIN is sent. This could cause some issues as `ButtonRequests` were not being handled by `sendpin`. Additionally, `TrezorClient.call` would not work because it needs access to `TrezorClient.features`, which is not set for the `sendpin` command. In order to resolve this, `TrezorClient.call` is modified to optionally skip the firmware check that requires `TrezorClient.features`, and `sendpin` uses `TrezorClient.call` instead of `TrezorClient.call_raw`. Fixes bitcoin-core#526 Fixes $539 Top commit has no ACKs. Tree-SHA512: 87b0cc2d21f8e4647e17540aa16291aade5cc514efcc2b592484f227a96c2e5be029bd1953e3df00ad1036aa8084fdd5869e9bde990cd5f81968e1e5755f964e
HWI is compatible with Python 3.9. Although 3.9.8 has been released, the most recent version of Python shipped with PyEnv on homebrew (macOS) is 3.9.7 In order to test backwareds compatibility, the version is not changed elsewhere.
d1055df Bump PyEnv version to 3.9.7 (Sjors Provoost) Pull request description: Since the release notes mention support for Python 3.9. Or do we also want to guarantee backwards compatibility? Using 3.9.1 for PyEnv (latest available on macOS) and 3.9.2 for the Windows installer. ACKs for top commit: achow101: ACK d1055df Tree-SHA512: 8913672fe2c5aefcd76558c5d574984612606f94c883078c0f19df4d952fd55f42d79ac6d4dbc6738d441fd9c2f93105ebbe89e76b079255eb99b9cee7c7946c
db17fed Add tests for Jade hww using qemu emulator (Jamie C. Driver) 44d556e Add support for the Blockstream Jade hww (Jamie C. Driver) Pull request description: ACKs for top commit: achow101: ACK db17fed Tree-SHA512: 879f510fb171a6570bab415d880bd86de9e02ef481d901802515e07ac4c7ba93cdfddf62a5663cf85538f3b78224f8699b7d5aacc8120395d959b5f95510b794
Track the set of keys that have been read in order to better catch duplicate keys. Matches Core's handling of duplicate keys.
Instead of using 0 as a magic number, make self.sighash an Optional.
Instead of including field numbers explicitly, use their names, with the numbers defined as static members of their respective classes.
Instead of serializing the field number explicitly as bytes, use the named members for the field numbers. Also serializes them with ser_compact_size in accordance with BIP 174
c5fba03 tests: Add Taproot field test vectors (Andrew Chow) 19d2e0a psbt: Implement Taproot fields (Andrew Chow) fe85a31 psbt: Use static member field numbers for serialization (Andrew Chow) 6b59c75 psbt: Use static members for field numbers (Andrew Chow) bc7efb9 psbt: Make sighash Optional (Andrew Chow) 92d64e7 psbt: Remove extra newlines (Andrew Chow) 76ad4dd psbt: Check for duplicate keys by tracking set of read keys (Andrew Chow) Pull request description: Adds the Taproot PSBT fields as described in BIP 371 ACKs for top commit: Sjors: re-utACK c5fba03 Tree-SHA512: 93505dbad541324c9f419888134fe9543bccba603a863eed8561a0b514f0b77a6e27160222f90b8011e9e46f34d9e3897dce78739268a339204873939a48a6db
…lators 7e283f6 ci: run install_script before installing sims (Andrew Chow) Pull request description: This fixes the CI failures of Python 3.8 Ledger jobs. Apparently in 3.8, `poetry install` removes some of the dependencies we installed for speculos during the `sim_install` part of the job. By doing `poetry install` first, and then installing the simulator dependencies afterwards, we can avoid this problem. Top commit has no ACKs. Tree-SHA512: c9abf8d9e42afda147b8da623aec2f847023bc853079caec255e5c26ff2ff14a93796f380bcde9370a1d76e22b2d78cc398d15c3dbb95d019416f8b34e9ddce9
7d811cb ci: do git clean before setting up keepkey (Andrew Chow) Pull request description: Need to make sure that nanopb does not exist before cloning. `git clean` takes care of this, as well as removing anything else that could potentially be problematic. Top commit has no ACKs. Tree-SHA512: 0b6eb432bcce24a3df4c856f0edc37cd778d082bc8052f18baa5d771ca3c60084e665c591eda189844f69a436899cf757c838400a1cd7ad08d10c5eab82730bf
Instead of checking that a function is not in a disallowed context, check that it is in an allowed context. This makes adding new functions easier.
Future descriptors may have multiple subdescriptors
Add the class for tr() descriptors, parsing such descriptors, and basic tests
fbb34a3 Improve gui (Giacomo Caironi) Pull request description: This PR tries to improve the GUI look and usability: * Improve SignMessageDialog, GetXpubDialog, SignPsbtDialog and MainWindow widget size and spacing * Add buttons to import a psbt from a file and to export it after signing ACKs for top commit: achow101: ACK fbb34a3 Tree-SHA512: 613f2d993dec96c7a1fb90e5234414aa9afe1bce93c3769460692e8659d2442928961e0baf5876eb3e0d68d9d56eb11e4d401acb18a08ce1c4f86adc66a72841
If the passphrase is not provided and passphrase protection is enabled, default to the empty string.
9ed9dbe trezor: Default to empty string passphrase (Andrew Chow) 0c4df0c trezor, test: Test enumerate with empty passphrase (Andrew Chow) d74fcbc test: Handle Optional emulator password (Andrew Chow) d4cf550 Change passphrase (password) to Optional (Andrew Chow) Pull request description: We should be distinguishing from the empty passphrase and no passphrase provided, so instead of defaulting to the empty passphrase, change passphrase (aka password) to an `Optional`. This way, if it is not provided, it is `None` and empty passphrases can be handled correctly. Fixes bitcoin-core#639 Top commit has no ACKs. Tree-SHA512: aac6763934dd40fd55c7ad0ed9bb57e387ce9eefeb4c198521fb09faaeed538966bc50f598c024dfffdc8e6a5a646f87ff64cb77103c296ec2c30ba992763274
5191d21 jade: clean all jade build artifacts when building jade simulator (Jamie C. Driver) Pull request description: It appears that the Jade CI is being polluted by the output of prior runs. This needs removing, otherwise we're not getting a true clean build for this commit using the current tools, but rather some cached values (now out of date and not applicable) can interfere with the current build. Top commit has no ACKs. Tree-SHA512: 3395d3103b2593231220e30bd448a136d52c9ba225815369aadc627945ff114002b9635af8377bdfe9464680f5840d21dd0ab8382a33230f758a8150f62ea340
…ions These arguments are currently discarded/ignored for the enumerate command. Pass them through to the per-hw enumerate() calls. In the Jade client, use the 'chain' parameter to enable testnet-configured Jade units to enumerate successfully. Also update error message to reflect latest wording on Jade hw.
…out-gui` as their first parameters to build without the GUI support.
…nts to enumerate() functions f7e6126 Pass '--expert' and '--chain' cmd-line arguments to enumerate() functions (Jamie C. Driver) Pull request description: These arguments are currently discarded/ignored for the enumerate command. Pass them through to the per-hw enumerate() calls. In the Jade client, use the 'chain' parameter to enable testnet-configured Jade units to enumerate successfully. ACKs for top commit: achow101: ACK f7e6126 Tree-SHA512: 5a0132acbb186e17d027d7f8ea5a7bcd455aaff21b6fb757a21c78b4646245b68ca04ccfa71d95e833a10132f9382f0374728360d8f279155835c1f2b18f59b9
f6d246a Typo (Kiminuo) 56e541f `contrib/build_bin.sh` and `contrib/build_bin.sh` now accepts `--without-gui` as their first parameters to build without the GUI support. (Kiminuo) Pull request description: Alternative to bitcoin-core#561 Addresses bitcoin-core#561 (comment): > Instead of a new script, I would prefer if an option were just added to the existing script(s) that would disable the qt build. ACKs for top commit: achow101: ACK f6d246a Tree-SHA512: fbce40b6d21c3523b747439e7018153b18589e736aae717bced28e42af4c12f0104afead537683e5d9d027e64023f728d3e6803fc95cfd21b6871c4cb1984d6a
Since bitcoin-core#644, not providing a passphrase to a trezor that has passphrases enabled will only result in a warning rather than setting `needs_passphrase_set` to false.
…es following passphrase changes b8a379c test, trezor: Properly pass empty string passphrase (Andrew Chow) c3f490f test, trezor: Check for warnings rather than needs_passphrase_sent (Andrew Chow) 203c844 trezor: Add missing NoPasswordError import (Andrew Chow) Pull request description: Fixes a couple things were missed following bitcoin-core#644 which are causing CI failures in Trezor tests. Top commit has no ACKs. Tree-SHA512: 508c6c9cdbc18833360a146296293500480938723843927e980e124ab24cb17f035418623e7fcb3e1a196e3e6bb7754bdd3bd2903d5d9064918ad875124efd1a
…bytes for ranged desc a1045bc descriptor.py: fix PubkeyProvider.get_pubkey_bytes for ranged desc (SomberNight) Pull request description: To trigger bug: ```py >>> from hwilib.descriptor import parse_descriptor >>> b = parse_descriptor("wsh(multi(1,xpub661MyMwAqRbcFW31YEwpkMuc5THy2PSt5bDMsktWQcFF8syAmRUapSCGu8ED9W6oDMSgv6Zz8idoc4a6mr8BDzTJY47LJhkJ8UB7WEGuduB/1/0/*,xpub69H7F5d8KSRgmmdJg2KhpAK8SR3DjMwAdkxj3ZuxV27CprR9LgpeyGmXUbC6wb7ERfvrnKZjXoUmmDznezpbZb7ap6r1D3tgFxHmwMkQTPH/0/0/*))") >>> b.expand(3).output_script.hex() Traceback (most recent call last): File "/home/user/wspace/HWI/hwilib/key.py", line 361, in parse_path return [str_to_harden(x) for x in n] File "/home/user/wspace/HWI/hwilib/key.py", line 361, in <listcomp> return [str_to_harden(x) for x in n] File "/home/user/wspace/HWI/hwilib/key.py", line 358, in str_to_harden return int(x) ValueError: invalid literal for int() with base 10: '*3' During handling of the above exception, another exception occurred: Traceback (most recent call last): File "<stdin>", line 1, in <module> File "/home/user/wspace/HWI/hwilib/descriptor.py", line 385, in expand witness_script, _, _ = self.subdescriptors[0].expand(pos) File "/home/user/wspace/HWI/hwilib/descriptor.py", line 340, in expand der_pks = [p.get_pubkey_bytes(pos) for p in self.pubkeys] File "/home/user/wspace/HWI/hwilib/descriptor.py", line 340, in <listcomp> der_pks = [p.get_pubkey_bytes(pos) for p in self.pubkeys] File "/home/user/wspace/HWI/hwilib/descriptor.py", line 170, in get_pubkey_bytes path = parse_path(path_str) File "/home/user/wspace/HWI/hwilib/key.py", line 363, in parse_path raise ValueError("Invalid BIP32 path", nstr) ValueError: ('Invalid BIP32 path', '*3') ``` ACKs for top commit: achow101: ACK a1045bc Tree-SHA512: 6d0aaeb8ce2bc220de3b47ca31994ead2caea7867298a8008c06a6d8ffa20b4190cc1bf2174ffabcc70dab6f0191d09c445e429bf6988b37039f40e4bdc9e45c
…for non-segwit only 7a6fd0e trezor: Set multisig change as PAYTOMULTISIG for non-segwit only (Andrew Chow) Pull request description: `PAYTOMULTISIG` is for P2SH only. For p2sh-segwit and native segwit, the `PAYTOWITNESS` and `PAYTOP2SHWITNESS` that are set earlier are correct. Fixes bitcoin-core#661 Top commit has no ACKs. Tree-SHA512: 1ed6cb8c23ab2c225dc033f9b7499687606c498a8684d26c5c63d39db78cb7df22f0ac68063c88697e9a714b296ec82e33d7b48ae2650543fd215a44672d5bab
It's a function.
ad29646 ci: Bump protoc version (Andrew Chow) 08b5d93 trezor: Fix is_p2sh call (Andrew Chow) d3cee84 poetry lock (Kim Neunert) bda4e57 bump typing-extensions dependency (Kim Neunert) Pull request description: Upgrade typing-extensions dependency ( bitcoin-core#572) I chose not the most recent one but try to also be uptodate.  Top commit has no ACKs. Tree-SHA512: 91642a5401333bcf974366cd67acc161e4fae9509846fe9d89fbda6057e15c14d0536d4eaaf4d2ca97183a16947ec43fc3c4aec8f7006748a3b7b8cfe8649b57
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
No description provided.