Feature/trezor reconnect #1874
Feature/trezor reconnect #1874
Conversation
OBorce
requested review from
erubboli,
azarovh and
ImplOfAnImpl
as code owners
OBorce
force-pushed
the
feature/trezor-recconect
branch
2 times, most recently
from
4f6708d to
e0f00b5
Compare
OBorce
force-pushed
the
refactor/wallet-generic-signer
branch
from
69592f8 to
f16c3e3
Compare
OBorce
force-pushed
the
feature/trezor-recconect
branch
2 times, most recently
from
8a33777 to
f8578ef
Compare
| Err(trezor_client::Error::TransportSendMessage( | ||
| trezor_client::transport::error::Error::Usb(rusb::Error::Io), | ||
| )) => { | ||
| let (mut new_client, data) = find_trezor_device()?; |
There was a problem hiding this comment.
Calling find_trezor_device on each operation is not a great idea. What if another device is returned?
There was a problem hiding this comment.
we check the public keys again, and if it is a different device we error out.
There was a problem hiding this comment.
we check the public keys again, and if it is a different device we error out.
I understand this, the point was that it looks conceptually wrong.
Currently, the question is why do we need it if you already call init_device every time. Can't it recover the session after a usb error?
And even if it can't, calling find_trezor_device still seems wrong. Perhaps then you should split find_trezor_device into 2 functions - the "find" part and the "connect" part, so that the former is still called only once.
| Err(trezor_client::Error::TransportSendMessage( | ||
| trezor_client::transport::error::Error::Usb(rusb::Error::Io), | ||
| )) => { |
There was a problem hiding this comment.
IMO instead of calling the operation and then checking for error we should just unconditionally call Trezor::initialize, passing to it the previosly obtained session_id (it's inside features). At least this is recommended in the docs.
There was a problem hiding this comment.
added an init_device(session_id) call which internally calls initialize before each operation.
OBorce
force-pushed
the
refactor/wallet-generic-signer
branch
2 times, most recently
from
82b71a8 to
c9c1c91
Compare
OBorce
force-pushed
the
feature/trezor-recconect
branch
from
f8578ef to
c9c1c91
Compare
OBorce
force-pushed
the
feature/trezor-recconect
branch
2 times, most recently
from
2df0787 to
cf6ce8c
Compare
OBorce
force-pushed
the
refactor/wallet-generic-signer
branch
from
48716c3 to
67609d5
Compare
OBorce
force-pushed
the
feature/trezor-recconect
branch
from
cf6ce8c to
66a4f17
Compare
OBorce
force-pushed
the
refactor/wallet-generic-signer
branch
from
67609d5 to
220ace1
Compare
If the trezor device returns an USB IO error it might mean that the trezor device has been disconnected, in that case try to reconnect to the device and try the failed operation again.