Repository navigation
Trezor displays change address for confirmation #170
Description
One of our users reported that the way Trezor displays tx confirmation change address, too. What can be done about it? Wait for Trezor to fix it in the driver or you're planning to fix it here?
Sending coins- the Trezor asks me to confirm the address as usual however the first address presented on it is NOT the destination address. I assumed, correctly, that this must be the address for the change, and as it was for a tiny amount I took the chance. The second address presented on the Trezor was the destination address as expected. There could do with being an option to see the “change address” prior to it appearing on the Trezor, it’s really unnerving to be authorising a transaction to an address I have not seen anywhere! And it makes the whole confirmation on the Trezor pointless as I’m not actually checking it against any known (to me) address.
Activity
achow101 commentedon May 13, 2019
Yes, change detection is kinda finicky and prone to errors, so I decided to disable change detection for all of the devices for now. I consider change detection to be a security risk since it relies on the incoming PSBT to be correct and not lie about the master key fingerprint and derivation path for outputs as there is no way to verify whether the stated paths are actually for that output. Otherwise users could be tricked into sending more than they intended to a "change address" that isn't actually theirs. So for now, all outputs should be displayed on all devices.
The fix for this is to implement the extension proposed here: https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2019-April/016894.html.
instagibbs commentedon May 13, 2019
You mean multi-sig change detection is disabled? Ledger still supports the change detection for single-key: https://github.com/bitcoin-core/HWI/blob/master/hwilib/devices/ledger.py#L151
achow101 commentedon May 13, 2019
No. At one point in time, all change detection was disabled by explicitly stripping derivation info from the psbt before it is passed to each device driver. It looks like that changed so change detection is inconsistent between all devices.
Obviously change detection for Trezor and Keepkey is disabled somehow.
instagibbs commentedon Jul 10, 2019
I don't get this. At least for Ledger the output's scriptpubkey is re-derived on the device using the derivation path. It either matches or it doesn't.
edit: I do understand for the multisig case, sure.
instagibbs commentedon Jul 10, 2019
FWIW I don't think this is an appropriate solution, versus have the signer "register" the multisig chain ala ColdCard. Simple because the inputs match a template with outputs doesn't mean we should elide outputs in general.
achow101 commentedon Jul 12, 2019
If it is an input that is currently being signed, why not?
instagibbs commentedon Jul 12, 2019
Because it's an unstated assumption that I and others simply do not share. It's a bad idea. Going to not take this thread any further off-topic, we can discuss offline.
Merge bitcoin-core#211: Enable trezor and keepkey change detection fo…