diff --git a/ethcore/src/account_provider/mod.rs b/ethcore/src/account_provider/mod.rs index 249ca40af..890932dc1 100755 --- a/ethcore/src/account_provider/mod.rs +++ b/ethcore/src/account_provider/mod.rs @@ -168,9 +168,9 @@ impl AccountProvider { pub fn new(sstore: Box, settings: AccountProviderSettings) -> Self { let mut hardware_store = None; if settings.enable_hardware_wallets { - match HardwareWalletManager::new() { + let key_path = if settings.hardware_wallet_classic_key { KeyPath::EthereumClassic } else { KeyPath::Ethereum }; + match HardwareWalletManager::new(key_path) { Ok(manager) => { - manager.set_key_path(if settings.hardware_wallet_classic_key { KeyPath::EthereumClassic } else { KeyPath::Ethereum }); hardware_store = Some(manager) }, Err(e) => debug!("Error initializing hardware wallets: {}", e), diff --git a/hw/src/ledger.rs b/hw/src/ledger.rs index 0d787588d..e27c5783d 100644 --- a/hw/src/ledger.rs +++ b/hw/src/ledger.rs @@ -25,6 +25,7 @@ use std::time::Duration; use super::WalletInfo; use ethkey::{Address, Signature}; use bigint::hash::H256; +use parking_lot::{Mutex, RwLock}; const LEDGER_VID: u16 = 0x2c97; const LEDGER_PIDS: [u16; 2] = [0x0000, 0x0001]; // Nano S and Blue @@ -84,11 +85,14 @@ impl From for Error { /// Ledger device manager. pub struct Manager { - usb: hidapi::HidApi, - devices: Vec, + // always lock in that order + usb: Mutex, + devices: RwLock>, key_path: KeyPath, } + + #[derive(Debug)] struct Device { path: String, @@ -96,20 +100,21 @@ struct Device { } impl Manager { - /// Create a new instance. - pub fn new() -> Result { + /// Create a new instance with given + pub fn new(key_path: KeyPath) -> Result { let manager = Manager { - usb: hidapi::HidApi::new()?, - devices: Vec::new(), - key_path: KeyPath::Ethereum, + usb: Mutex::new(hidapi::HidApi::new()?), + devices: RwLock::new(Vec::new()), + key_path, }; Ok(manager) } /// Re-populate device list. Only those devices that have Ethereum app open will be added. - pub fn update_devices(&mut self) -> Result { - self.usb.refresh_devices(); - let devices = self.usb.devices(); + pub fn update_devices(&self) -> Result { + let mut usb = self.usb.lock(); + usb.refresh_devices(); + let devices = usb.devices(); let mut new_devices = Vec::new(); let mut num_new_devices = 0; for device in devices { @@ -117,10 +122,10 @@ impl Manager { if device.vendor_id != LEDGER_VID || !LEDGER_PIDS.contains(&device.product_id) { continue; } - match self.read_device_info(&device) { + match self.read_device_info(&device, &mut usb) { Ok(info) => { debug!("Found device: {:?}", info); - if !self.devices.iter().any(|d| d.path == info.path) { + if !self.devices.read().iter().any(|d| d.path == info.path) { num_new_devices += 1; } new_devices.push(info); @@ -129,17 +134,12 @@ impl Manager { Err(e) => debug!("Error reading device info: {}", e), }; } - self.devices = new_devices; + *self.devices.write() = new_devices; Ok(num_new_devices) } - /// Select key derivation path for a known chain. - pub fn set_key_path(&mut self, key_path: KeyPath) { - self.key_path = key_path; - } - - fn read_device_info(&self, dev_info: &hidapi::HidDeviceInfo) -> Result { - let mut handle = self.open_path(&dev_info.path)?; + fn read_device_info(&self, dev_info: &hidapi::HidDeviceInfo, usb: &mut hidapi::HidApi) -> Result { + let mut handle = self.open_path(&dev_info.path, usb)?; let address = Self::read_wallet_address(&mut handle, self.key_path)?; let manufacturer = dev_info.manufacturer_string.clone().unwrap_or("Unknown".to_owned()); let name = dev_info.product_string.clone().unwrap_or("Unknown".to_owned()); @@ -187,20 +187,23 @@ impl Manager { /// List connected wallets. This only returns wallets that are ready to be used. pub fn list_devices(&self) -> Vec { - self.devices.iter().map(|d| d.info.clone()).collect() + self.devices.read().iter().map(|d| d.info.clone()).collect() } /// Get wallet info. pub fn device_info(&self, address: &Address) -> Option { - self.devices.iter().find(|d| &d.info.address == address).map(|d| d.info.clone()) + self.devices.read().iter().find(|d| &d.info.address == address).map(|d| d.info.clone()) } /// Sign transaction data with wallet managing `address`. pub fn sign_transaction(&self, address: &Address, data: &[u8]) -> Result { - let device = self.devices.iter().find(|d| &d.info.address == address) + // mind the lock order + let mut usb = self.usb.lock(); + let devices = self.devices.read(); + let device = devices.iter().find(|d| &d.info.address == address) .ok_or(Error::KeyNotFound)?; - let handle = self.open_path(&device.path)?; + let handle = self.open_path(&device.path, &mut usb)?; let eth_path = Ð_DERIVATION_PATH_BE[..]; let etc_path = &ETC_DERIVATION_PATH_BE[..]; @@ -236,11 +239,11 @@ impl Manager { Ok(Signature::from_rsv(&r, &s, v)) } - fn open_path(&self, path: &str) -> Result { + fn open_path<'a>(&self, path: &str, usb: &'a mut hidapi::HidApi) -> Result, Error> { let mut err = Error::KeyNotFound; /// Try to open device a few times. for _ in 0..10 { - match self.usb.open_path(&path) { + match usb.open_path(&path) { Ok(handle) => return Ok(handle), Err(e) => err = From::from(e), } @@ -340,10 +343,12 @@ impl Manager { #[test] fn smoke() { + use super::KeyPath; use rustc_hex::FromHex; - let mut manager = Manager::new().unwrap(); + + let manager = Manager::new(KeyPath::Ethereum).unwrap(); manager.update_devices().unwrap(); - for d in &manager.devices { + for d in &*manager.devices.read() { println!("Device: {:?}", d); } diff --git a/hw/src/lib.rs b/hw/src/lib.rs index cda6c2241..b1819da59 100644 --- a/hw/src/lib.rs +++ b/hw/src/lib.rs @@ -32,7 +32,6 @@ use std::sync::atomic; use std::sync::{Arc, Weak}; use std::sync::atomic::AtomicBool; use std::time::Duration; -use parking_lot::Mutex; use ethkey::{Address, Signature}; pub use ledger::KeyPath; @@ -90,11 +89,11 @@ impl From for Error { pub struct HardwareWalletManager { update_thread: Option>, exiting: Arc, - ledger: Arc>, + ledger: Arc, } struct EventHandler { - ledger: Weak>, + ledger: Weak, } impl libusb::Hotplug for EventHandler { @@ -103,7 +102,7 @@ impl libusb::Hotplug for EventHandler { if let Some(l) = self.ledger.upgrade() { for _ in 0..10 { // The device might not be visible right away. Try a few times. - if l.lock().update_devices().unwrap_or_else(|e| { + if l.update_devices().unwrap_or_else(|e| { debug!("Error enumerating Ledger devices: {}", e); 0 }) > 0 { @@ -117,7 +116,7 @@ impl libusb::Hotplug for EventHandler { fn device_left(&mut self, _device: libusb::Device) { debug!("USB Device lost"); if let Some(l) = self.ledger.upgrade() { - if let Err(e) = l.lock().update_devices() { + if let Err(e) = l.update_devices() { debug!("Error enumerating Ledger devices: {}", e); } } @@ -125,15 +124,15 @@ impl libusb::Hotplug for EventHandler { } impl HardwareWalletManager { - pub fn new() -> Result { + pub fn new(key_path: KeyPath) -> Result { let usb_context = Arc::new(libusb::Context::new()?); - let ledger = Arc::new(Mutex::new(ledger::Manager::new()?)); + let ledger = Arc::new(ledger::Manager::new(key_path)?); usb_context.register_callback(None, None, None, Box::new(EventHandler { ledger: Arc::downgrade(&ledger) }))?; let exiting = Arc::new(AtomicBool::new(false)); let thread_exiting = exiting.clone(); let l = ledger.clone(); let thread = thread::Builder::new().name("hw_wallet".to_string()).spawn(move || { - if let Err(e) = l.lock().update_devices() { + if let Err(e) = l.update_devices() { debug!("Error updating ledger devices: {}", e); } loop { @@ -150,25 +149,19 @@ impl HardwareWalletManager { }) } - /// Select key derivation path for a chain. - pub fn set_key_path(&self, key_path: KeyPath) { - self.ledger.lock().set_key_path(key_path); - } - - /// List connected wallets. This only returns wallets that are ready to be used. pub fn list_wallets(&self) -> Vec { - self.ledger.lock().list_devices() + self.ledger.list_devices() } /// Get connected wallet info. pub fn wallet_info(&self, address: &Address) -> Option { - self.ledger.lock().device_info(address) + self.ledger.device_info(address) } /// Sign transaction data with wallet managing `address`. pub fn sign_transaction(&self, address: &Address, data: &[u8]) -> Result { - Ok(self.ledger.lock().sign_transaction(address, data)?) + Ok(self.ledger.sign_transaction(address, data)?) } }