trezor: restore support for external inputs #590
Conversation
matejcik
force-pushed
the
restore-trezor-external
branch
from
790807c
to
6054be5
Compare
|
Fixed big_tx failure by extending the size of the keypool. I am still not entirely sure why it carries over between test runs but 🤷♀️ Also added support for forwarding signature data to Trezor if available. This enables Trezor T "verified external input" mode. Adding support for SLIP-19 ownership proofs would be simple, assuming they are either (a) added to BIP-174 or (b) SLIP-19 is changed to use a vendor prefix. |
|
Since this requires disabling safety checks, I'm not sure that this is something that we want to have, or would be useful to users. |
|
I thought lots of people use HWI to sign mixed environment transactions that bring external inputs into the mix, no? |
No idea. No one's complained about the latest release which includes #581. |
| * 1 - Since firmware 1.10.6, safety checks must be disabled. | ||
| * 2 - Since firmware 2.4.4, safety checks must be disabled. |
There was a problem hiding this comment.
These are the correct firmware versions:
| * 1 - Since firmware 1.10.6, safety checks must be disabled. | |
| * 2 - Since firmware 2.4.4, safety checks must be disabled. | |
| * 1 - Since firmware 1.11.1, safety checks must be disabled. | |
| * 2 - Since firmware 2.5.1, safety checks must be disabled. |
| @@ -275,6 +275,14 @@ def get_path_transport( | |||
| raise BadArgumentError(f"Could not find device by path: {path}") | |||
|
|
|||
|
|
|||
| def _use_external_script_type(client) -> bool: | |||
| if client.features.model == "1" and client.version > (1, 10, 5): | |||
There was a problem hiding this comment.
This is better and matches more how people think:
| if client.features.model == "1" and client.version > (1, 10, 5): | |
| if client.features.model == "1" and client.version >= (1, 11, 1): |
| def _use_external_script_type(client) -> bool: | ||
| if client.features.model == "1" and client.version > (1, 10, 5): | ||
| return True | ||
| if client.features.model == "T" and client.version > (2, 4, 3): |
There was a problem hiding this comment.
This is better and matches more how people think:
| if client.features.model == "T" and client.version > (2, 4, 3): | |
| if client.features.model == "T" and client.version > (2, 5, 1): |
|
@achow101 I think it's the other way around. No one uses external inputs because no one implemented it as an useful tool. Sparrow Wallet recently implemented privacy enhancing features such as Stonewall 2x (two party CoinJoin) and Stowaway (PayJoin) which people actually use on daily basis. With Trezor's external inputs it would be possible to do that from hardware wallets which in my opinion would close the gap where users needs to choose between security and privacy tradeoffs. |
|
Is this still relevant? |
|
I think so. Can you please confirm @matejcik? |
|
I will need to update the PR in light of #713 but otherwise yes, this is still desired. |
partially reverts #581
implements unverified external inputs for Trezor, per https://github.com/trezor/trezor-firmware/blob/master/docs/common/communication/bitcoin-signing.md#external-inputs
Unit tests are passing for me locally, except big_tx in the newly added test suite, but that's bitcoincore complaining that "This wallet has no available keys (-4)". I have no idea what to do about that -- maybe something went wrong because of the way I subclass?