Skip to content
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

trezor: Remove support for external inputs #581

Merged
merged 3 commits into from
Mar 15, 2022

Conversation

achow101
Copy link
Member

Trezor will be closing the loophole that we use to be able to sign external inputs. So we will need to tell users that they will not be able to sign such transactions if they are using those firmware versions.

@achow101 achow101 force-pushed the disable-trezor-external branch 8 times, most recently from d76a452 to 43f9878 Compare February 23, 2022 23:09
@grdddj
Copy link

grdddj commented Feb 24, 2022

I can confirm that the updated trezor and test code works well with the ScriptPubKey-checking Trezors.

All the tests passed when running this branch locally with new (unreleased) Trezor version.

Thanks for the cooperation!

The simulator is having trouble with anything larger, so the device
probably will too. We change the signing to do 15 signing bundles at a
time rather than all them at the same time.
Use inputs belonging to the device instead of external inputs.

Apparently this fixes the coldcard problem with this test too.
Trezor is going to close the loophole that we use to get external
input support, so we need to remove trying to sign with that loophole
for future firmware versions.
@andrewkozlik
Copy link

Trezor will be closing the loophole that we use to be able to sign external inputs.

We decided to allow this loophole if the user disables "safety checks" on their Trezor. Then they will just need to confirm a warning that the scriptPubKey doesn't match the provided path.

@andrewkozlik
Copy link

For the record, as @matejcik already mentioned, Trezor T will accept all EXTERNAL inputs that already have a valid signature and EXTERNAL inputs that come with a SLIP-19 ownership proof. See also the documentation on signing external inputs. Trezor 1 doesn't support EXTERNAL inputs at all.

Note that I proposed a PSBT extension for SLIP-19 ownership proofs in https://github.com/satoshilabs/slips/blob/master/slip-0019.md#psbt-bip-174-extension, but we haven't implemented it anywhere yet.

@achow101
Copy link
Member Author

achow101 commented Mar 2, 2022

For the record, as @matejcik already mentioned, Trezor T will accept all EXTERNAL inputs that already have a valid signature

Signature or scriptSig? AFAICT, there's only a scriptSig field. As HWI does not contain a finalizer, this is not necessarily useful for us.

@andrewkozlik
Copy link

Signature or scriptSig? AFAICT, there's only a scriptSig field. As HWI does not contain a finalizer, this is not necessarily useful for us.

We mean script_sig (if the input is P2PKH or P2SH) and/or witness (if the input is any kind of SegWit) in the TxInputType message. Both fields need to be set in case the input is P2WPKH/P2WSH nested in BIP16 P2SH.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants