search

bitcoin-core / HWI

Bitcoin Hardware Wallet Interface
MIT License
494 stars 196 forks source link

trezor: signtx silently fails if we're signing with the wrong fingerprint #201

Closed jb55 closed 2 years ago

jb55 commented 5 years ago

I'm not sure who's responsible for this error, if trezor is throwing it but we're not returning it, or if trezor doesn't return an error, I haven't had time to investigate it yet.

I noticed this when I used signtx on some PSBT inputs that were in a BIP39 passworded BIP84 account, but I was using signtx without a passphrase. my trezor prompted me to sign like normal, but it didn't actually sign anything, and returned the unsigned PSBT without any errors.

I eventually got it to sign after working through other issues: #199 #200, but due to the lack of error messages it was really hard to figure out what went wrong.

instagibbs commented 5 years ago

Why should the signer complain about wrong fingerprint?

You could argue an error should return if the PSBT is unchanged after attempted signing(coldcard does this), but it won't know how to diagnose it. It could have just been an input from another device/seed.

jb55 commented 5 years ago

I think the thing that tripped be up is that it was prompting me to sign (I had to press the button), and it made it look like it was working properly, but it didn't do anything.

instagibbs commented 5 years ago

I see. Maybe the trezor driver could be a bit smarter.

On Fri, Sep 6, 2019, 12:48 PM William Casarin notifications@github.com wrote:

I think the thing that tripped be up is that it was prompting me to sign (I had to press the button), and it made it look like it was working properly, but it didn't do anything.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/bitcoin-core/HWI/issues/201?email_source=notifications&email_token=ABMAFUZZA7OG26A5HWALQPTQIKCU5A5CNFSM4H3XGTO2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD6DNBZY#issuecomment-528928999, or mute the thread https://github.com/notifications/unsubscribe-auth/ABMAFU6QOHSV73IL5B7OZXTQIKCU5ANCNFSM4H3XGTOQ .

achow101 commented 2 years ago

516 added a signed field in the response so it is now more obvious if any signing actually happened. The behavior of clicking through signing prompts on the Trezor is due to the way that HWI handles external inputs, which should be revisited at some point.