From 81b52c733642be39ccfca1c267ad13862515dbc1 Mon Sep 17 00:00:00 2001 From: Niklas Adolfsson Date: Fri, 2 Mar 2018 18:30:25 +0100 Subject: [PATCH] [hardware wallet] sleeping -> pollling (#8018) * Use polling, enable missing doc warnings & docs * make try_connect_polling() a free function --- hw/src/ledger.rs | 34 ++++++++++++++++++++++------------ hw/src/lib.rs | 9 +++++++++ hw/src/trezor.rs | 35 ++++++++++++++++++++++------------- 3 files changed, 53 insertions(+), 25 deletions(-) diff --git a/hw/src/ledger.rs b/hw/src/ledger.rs index 130149c47..e7d616d4f 100644 --- a/hw/src/ledger.rs +++ b/hw/src/ledger.rs @@ -21,8 +21,7 @@ use std::cmp::min; use std::fmt; use std::str::FromStr; use std::sync::{Arc, Weak}; -use std::time::Duration; -use std::thread; +use std::time::{Duration, Instant}; use ethereum_types::{H256, Address}; use ethkey::Signature; @@ -353,17 +352,30 @@ impl Manager { Err(Error::InvalidDevice) } } +} +// Try to connect to the device using polling in at most the time specified by the `timeout` +fn try_connect_polling(ledger: Arc, timeout: Duration) -> bool { + let start_time = Instant::now(); + while start_time.elapsed() <= timeout { + if let Ok(_) = ledger.update_devices() { + return true + } + } + false } /// Ledger event handler -/// A seperate thread is handling incoming events +/// A seperate thread is hanedling incoming events +/// +/// Note, that this run to completion and race-conditions can't occur but this can +/// therefore starve other events for being process with a spinlock or similar pub struct EventHandler { ledger: Weak, } impl EventHandler { - /// Ledger event handler constructor + /// Ledger event handler constructor pub fn new(ledger: Weak) -> Self { Self { ledger: ledger } } @@ -371,21 +383,19 @@ impl EventHandler { impl libusb::Hotplug for EventHandler { fn device_arrived(&mut self, device: libusb::Device) { + debug!(target: "hw", "Ledger arrived"); if let (Some(ledger), Ok(_)) = (self.ledger.upgrade(), Manager::is_valid_ledger(&device)) { - debug!(target: "hw", "Ledger arrived"); - // Wait for the device to boot up - thread::sleep(Duration::from_millis(1000)); - if let Err(e) = ledger.update_devices() { - debug!(target: "hw", "Ledger connect error: {:?}", e); + if try_connect_polling(ledger, Duration::from_millis(500)) != true { + debug!(target: "hw", "Ledger connect timeout"); } } } fn device_left(&mut self, device: libusb::Device) { + debug!(target: "hw", "Ledger left"); if let (Some(ledger), Ok(_)) = (self.ledger.upgrade(), Manager::is_valid_ledger(&device)) { - debug!(target: "hw", "Ledger left"); - if let Err(e) = ledger.update_devices() { - debug!(target: "hw", "Ledger disconnect error: {:?}", e); + if try_connect_polling(ledger, Duration::from_millis(500)) != true { + debug!(target: "hw", "Ledger disconnect timeout"); } } } diff --git a/hw/src/lib.rs b/hw/src/lib.rs index 060aead3a..9bfec8341 100644 --- a/hw/src/lib.rs +++ b/hw/src/lib.rs @@ -16,6 +16,8 @@ //! Hardware wallet management. +#![warn(missing_docs)] + extern crate ethereum_types; extern crate ethkey; extern crate hidapi; @@ -61,12 +63,19 @@ pub enum Error { /// or less a duplicate of ethcore::transaction::Transaction, but we can't /// import ethcore here as that would be a circular dependency. pub struct TransactionInfo { + /// Nonce pub nonce: U256, + /// Gas price pub gas_price: U256, + /// Gas limit pub gas_limit: U256, + /// Receiver pub to: Option
, + /// Value pub value: U256, + /// Data pub data: Vec, + /// Chain ID pub chain_id: Option, } diff --git a/hw/src/trezor.rs b/hw/src/trezor.rs index 4105bb7b5..7db226718 100644 --- a/hw/src/trezor.rs +++ b/hw/src/trezor.rs @@ -24,11 +24,9 @@ use super::{WalletInfo, TransactionInfo, KeyPath}; use std::cmp::{min, max}; use std::fmt; use std::sync::{Arc, Weak}; -use std::time::Duration; -use std::thread; +use std::time::{Duration, Instant}; use ethereum_types::{U256, H256, Address}; - use ethkey::Signature; use hidapi; use libusb; @@ -40,8 +38,8 @@ use trezor_sys::messages::{EthereumAddress, PinMatrixAck, MessageType, EthereumT /// Trezor v1 vendor ID pub const TREZOR_VID: u16 = 0x534c; -/// Trezor product IDs -pub const TREZOR_PIDS: [u16; 1] = [0x0001]; +/// Trezor product IDs +pub const TREZOR_PIDS: [u16; 1] = [0x0001]; const ETH_DERIVATION_PATH: [u32; 5] = [0x8000002C, 0x8000003C, 0x80000000, 0, 0]; // m/44'/60'/0'/0/0 const ETC_DERIVATION_PATH: [u32; 5] = [0x8000002C, 0x8000003D, 0x80000000, 0, 0]; // m/44'/61'/0'/0/0 @@ -402,14 +400,28 @@ impl Manager { } } +// Try to connect to the device using polling in at most the time specified by the `timeout` +fn try_connect_polling(trezor: Arc, duration: Duration) -> bool { + let start_time = Instant::now(); + while start_time.elapsed() <= duration { + if let Ok(_) = trezor.update_devices() { + return true + } + } + false +} + /// Trezor event handler /// A separate thread is handeling incoming events +/// +/// Note, that this run to completion and race-conditions can't occur but this can +/// therefore starve other events for being process with a spinlock or similar pub struct EventHandler { trezor: Weak, } impl EventHandler { - // Trezor event handler constructor + /// Trezor event handler constructor pub fn new(trezor: Weak) -> Self { Self { trezor: trezor } } @@ -419,20 +431,17 @@ impl libusb::Hotplug for EventHandler { fn device_arrived(&mut self, _device: libusb::Device) { debug!(target: "hw", "Trezor V1 arrived"); if let Some(trezor) = self.trezor.upgrade() { - // Wait for the device to boot up - thread::sleep(Duration::from_millis(1000)); - if let Err(e) = trezor.update_devices() { - debug!(target: "hw", "Trezor V1 connect error: {:?}", e); + if try_connect_polling(trezor, Duration::from_millis(500)) != true { + debug!(target: "hw", "Ledger connect timeout"); } - } } fn device_left(&mut self, _device: libusb::Device) { debug!(target: "hw", "Trezor V1 left"); if let Some(trezor) = self.trezor.upgrade() { - if let Err(e) = trezor.update_devices() { - debug!(target: "hw", "Trezor V1 disconnect error: {:?}", e); + if try_connect_polling(trezor, Duration::from_millis(500)) != true { + debug!(target: "hw", "Ledger disconnect timeout"); } } }