trezor / trezord-go

:smiling_imp: Trezor Communication Daemon (written in Go)
GNU Lesser General Public License v3.0
241 stars 146 forks source link

Update libusb to v1.0.26, including our patches #270

Closed igor-hnizdo closed 1 year ago

igor-hnizdo commented 1 year ago

Also add documentation on how to update

prusnak commented 1 year ago

We don't need the following files, so please remove then from the PR:

usb/lowlevel/libusb/c/Makefile*
usb/lowlevel/libusb/c/libusb-1.0.def
usb/lowlevel/libusb/c/libusb-1.0.rc
usb/lowlevel/libusb/c/os/haiku*
usb/lowlevel/libusb/c/os/linux_udev.c
usb/lowlevel/libusb/c/os/netbsd_usb.c
usb/lowlevel/libusb/c/os/null_usb.c
usb/lowlevel/libusb/c/os/openbsd_usb.c
usb/lowlevel/libusb/c/os/sunos_usb.c
usb/lowlevel/libusb/c/os/sunos_usb.h
igor-hnizdo commented 1 year ago

OK. Also the instructions are a bit wrong, will update them

igor-hnizdo commented 1 year ago

Fixed, please review again and possibly merge

igor-hnizdo commented 1 year ago

@tsusanka

  1. we detect missing WinUSB driver in the C code, which we cannot do in the go code. If my memory serves well, it detects missing auto-installed WinUSB driver. I forgot what we do with that information later.
  2. we skip non-trezor faster, I remember that used to cause some issues
  3. we have custom code for cancelling sync transfer, to support cancelling. If we wanted to use libusb, we would need to rewrite the go USB code to async transfers
  4. we have some dumb function name clash with HIDApi and LibUSB windows_open

Those are the 4 things we need custom code for.

igor-hnizdo commented 1 year ago

Ah, no, 1. and 2. are the same thing.

We detect missing WinUSB driver and then skip this device.

If we don't have this code, Trezor with missing WinUSB driver shows in listing, but then it errors on read/write with some mysterious error.

It's issue with Windows and WinUSB. It was mostly an issue on older Windows (7, Vista), but I remember it can potentially be an issue in Win10.... maybe? I don't remember.

igor-hnizdo commented 1 year ago

Yeah it was only issue with 7 and Vista.

If we don't care about Windows 7 and Vista, we can remove the custom code with has_winusb_driver, and also remove a lot of other cruft.

(The wdi-simple.exe can definitely can be removed, as that is run only when user has Windows 7 or Vista.)

igor-hnizdo commented 1 year ago

Windows 7 is officially not supported by even Microsoft anymore, but, it still has more marketshare than all MacOS versions together. (Let alone all Linux versions together.)

It's your call if you want to cut the support :)

igor-hnizdo commented 1 year ago

Let's move it here

273

tsusanka commented 1 year ago

Great info, thx 👍 .