almindor / etherwall

Ethereum QT5 Wallet
GNU General Public License v3.0
142 stars 59 forks source link

Support for Trezor Model T & on-device passphrase entry – signed #101

Closed gkaindl closed 6 years ago

gkaindl commented 6 years ago

As a follow-up to #100, this is the same PR, but with signed commits – Apparently, rebasing the existing PR wasn't working well.

The other pull request can be closed, I suppose.

almindor commented 6 years ago

Currently stuck on /usr/bin/ld: wire.o: undefined reference to symbol 'libusb_get_device' /usr/lib/libusb-1.0.so.0: error adding symbols: DSO missing from command line collect2: error: ld returned 1 exit status in linux(when added libusb as dependency).

Note sure if it has something to do with hidapi also linking in libusb but I'm currently stumped on why it's giving me that error given -lusb is there and libusb is installed on my system. Will try to solve ASAP and continue testing.

gkaindl commented 6 years ago

that's odd...

what's your libusb API version (it's in libusb.h)? Maybe there's a mismatch with mine?

 * Internally, LIBUSB_API_VERSION is defined as follows:
 * (libusb major << 24) | (libusb minor << 16) | (16 bit incremental)
 */
#define LIBUSB_API_VERSION 0x01000106
almindor commented 6 years ago

#define LIBUSB_API_VERSION 0x01000106

So same. I'm fairly sure this is some odd linking issue. The command-line looks sane too:

g++  -o Etherwall main.o accountmodel.o settings.o transactionmodel.o clipboard.o currencymodel.o accountproxymodel.o contractmodel.o contractinfo.o eventmodel.o filtermodel.o trezor.o messages.pb.o config.pb.o storage.pb.o types.pb.o wire.o hdpath.o devicemanager.o initializer.o tokenmodel.o etherlog.o gethlog.o helpers.o types.o tx.o bigint.o nodeipc.o nodews.o gethlogapp.o etherlogapp.o qrc_qml.o moc_accountmodel.o moc_settings.o moc_transactionmodel.o moc_clipboard.o moc_currencymodel.o moc_accountproxymodel.o moc_contractmodel.o moc_eventmodel.o moc_filtermodel.o moc_trezor.o moc_devicemanager.o moc_initializer.o moc_tokenmodel.o moc_etherlog.o moc_gethlog.o moc_helpers.o moc_nodeipc.o moc_nodews.o moc_gethlogapp.o moc_etherlogapp.o   -lhidapi-libusb -lusb -lprotobuf -pthread -ludev -lQt5Quick -lQt5Widgets -lQt5Gui -lQt5Qml -lQt5WebSockets -lQt5Network -lQt5Core -lGL -lpthread
/usr/bin/ld: wire.o: undefined reference to symbol 'libusb_get_device'
/usr/lib/libusb-1.0.so.0: error adding symbols: DSO missing from command line
collect2: error: ld returned 1 exit status
almindor commented 6 years ago

I also tried moving the libusb dependency after the hidapi one but no change either. pkg-config --libs libusbgives me just -lusb which we have here so that's correct as well. I had something similar happen before and it was usually linking order. I wonder if maybe -lusb needs to be at the utter end with -lpthread somewhere. Not sure how to coerce qmake to do that tho, all my pkg-config libs are done as before the qt5 and system stuff.

gkaindl commented 6 years ago

hm, yes, the error adding symbols: DSO missing from command line part actually hints at the linking order.

It could actually also be a problem to link both libusb and libhidapi – maybe it's not even needed on linux, since libhidapi already links against libusb anyway? On MacOS, it doesn't, though.

almindor commented 6 years ago

That's the most odd part. If I remove hidapi from the pkg list it still errors out with the same exact linking error about the DSO in libusb.

gkaindl commented 6 years ago

also odd that this would be the only function it complains about during linking...

anyway, libusb_get_device is only used to determine usb_max_size in wire.cpp, so you could hardcode this to something like 256 for a quick test if it links then.

this is just necessary because libusb will throw overflow errors if the receive buffer isn't a multiple of the max packet size.

almindor commented 6 years ago

I'm going to try and remove the hidapi parts completely and just use libusb. If that fails I'll make a simple project that uses the function from libusb to see if at least basic linking works. will have to resume tomorrow tho, have a busy day today :(

I'll try to test this on a mac in the meantime too just so I see if trezor one still works.

almindor commented 6 years ago

Figured it out. I need to use libusb-1.0 with pkg-config (to get the -lusb-1.0 instead of just lusb). It seems there's a big divide between pre 1.0 and 1.0+ versions on linux and some libs/apps still need the pre 1.0 version. They just used the -1.0 suffix to do that split. So I was linking the old libusb wrongly.

Can't test more today but will try to get things done tomorrow.