Allow hardware device reads without lock. (#6517)

This commit is contained in:
Tomasz Drwięga 2017-09-15 14:47:24 +02:00 committed by Arkadiy Paronyan
parent 980418898a
commit 3e60b221c8
3 changed files with 45 additions and 47 deletions

View File

@ -168,9 +168,9 @@ impl AccountProvider {
pub fn new(sstore: Box<SecretStore>, settings: AccountProviderSettings) -> Self { pub fn new(sstore: Box<SecretStore>, settings: AccountProviderSettings) -> Self {
let mut hardware_store = None; let mut hardware_store = None;
if settings.enable_hardware_wallets { 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) => { Ok(manager) => {
manager.set_key_path(if settings.hardware_wallet_classic_key { KeyPath::EthereumClassic } else { KeyPath::Ethereum });
hardware_store = Some(manager) hardware_store = Some(manager)
}, },
Err(e) => debug!("Error initializing hardware wallets: {}", e), Err(e) => debug!("Error initializing hardware wallets: {}", e),

View File

@ -25,6 +25,7 @@ use std::time::Duration;
use super::WalletInfo; use super::WalletInfo;
use ethkey::{Address, Signature}; use ethkey::{Address, Signature};
use bigint::hash::H256; use bigint::hash::H256;
use parking_lot::{Mutex, RwLock};
const LEDGER_VID: u16 = 0x2c97; const LEDGER_VID: u16 = 0x2c97;
const LEDGER_PIDS: [u16; 2] = [0x0000, 0x0001]; // Nano S and Blue const LEDGER_PIDS: [u16; 2] = [0x0000, 0x0001]; // Nano S and Blue
@ -84,11 +85,14 @@ impl From<hidapi::HidError> for Error {
/// Ledger device manager. /// Ledger device manager.
pub struct Manager { pub struct Manager {
usb: hidapi::HidApi, // always lock in that order
devices: Vec<Device>, usb: Mutex<hidapi::HidApi>,
devices: RwLock<Vec<Device>>,
key_path: KeyPath, key_path: KeyPath,
} }
#[derive(Debug)] #[derive(Debug)]
struct Device { struct Device {
path: String, path: String,
@ -96,20 +100,21 @@ struct Device {
} }
impl Manager { impl Manager {
/// Create a new instance. /// Create a new instance with given
pub fn new() -> Result<Manager, Error> { pub fn new(key_path: KeyPath) -> Result<Manager, Error> {
let manager = Manager { let manager = Manager {
usb: hidapi::HidApi::new()?, usb: Mutex::new(hidapi::HidApi::new()?),
devices: Vec::new(), devices: RwLock::new(Vec::new()),
key_path: KeyPath::Ethereum, key_path,
}; };
Ok(manager) Ok(manager)
} }
/// Re-populate device list. Only those devices that have Ethereum app open will be added. /// Re-populate device list. Only those devices that have Ethereum app open will be added.
pub fn update_devices(&mut self) -> Result<usize, Error> { pub fn update_devices(&self) -> Result<usize, Error> {
self.usb.refresh_devices(); let mut usb = self.usb.lock();
let devices = self.usb.devices(); usb.refresh_devices();
let devices = usb.devices();
let mut new_devices = Vec::new(); let mut new_devices = Vec::new();
let mut num_new_devices = 0; let mut num_new_devices = 0;
for device in devices { for device in devices {
@ -117,10 +122,10 @@ impl Manager {
if device.vendor_id != LEDGER_VID || !LEDGER_PIDS.contains(&device.product_id) { if device.vendor_id != LEDGER_VID || !LEDGER_PIDS.contains(&device.product_id) {
continue; continue;
} }
match self.read_device_info(&device) { match self.read_device_info(&device, &mut usb) {
Ok(info) => { Ok(info) => {
debug!("Found device: {:?}", 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; num_new_devices += 1;
} }
new_devices.push(info); new_devices.push(info);
@ -129,17 +134,12 @@ impl Manager {
Err(e) => debug!("Error reading device info: {}", e), Err(e) => debug!("Error reading device info: {}", e),
}; };
} }
self.devices = new_devices; *self.devices.write() = new_devices;
Ok(num_new_devices) Ok(num_new_devices)
} }
/// Select key derivation path for a known chain. fn read_device_info(&self, dev_info: &hidapi::HidDeviceInfo, usb: &mut hidapi::HidApi) -> Result<Device, Error> {
pub fn set_key_path(&mut self, key_path: KeyPath) { let mut handle = self.open_path(&dev_info.path, usb)?;
self.key_path = key_path;
}
fn read_device_info(&self, dev_info: &hidapi::HidDeviceInfo) -> Result<Device, Error> {
let mut handle = self.open_path(&dev_info.path)?;
let address = Self::read_wallet_address(&mut handle, self.key_path)?; let address = Self::read_wallet_address(&mut handle, self.key_path)?;
let manufacturer = dev_info.manufacturer_string.clone().unwrap_or("Unknown".to_owned()); let manufacturer = dev_info.manufacturer_string.clone().unwrap_or("Unknown".to_owned());
let name = dev_info.product_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. /// List connected wallets. This only returns wallets that are ready to be used.
pub fn list_devices(&self) -> Vec<WalletInfo> { pub fn list_devices(&self) -> Vec<WalletInfo> {
self.devices.iter().map(|d| d.info.clone()).collect() self.devices.read().iter().map(|d| d.info.clone()).collect()
} }
/// Get wallet info. /// Get wallet info.
pub fn device_info(&self, address: &Address) -> Option<WalletInfo> { pub fn device_info(&self, address: &Address) -> Option<WalletInfo> {
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`. /// Sign transaction data with wallet managing `address`.
pub fn sign_transaction(&self, address: &Address, data: &[u8]) -> Result<Signature, Error> { pub fn sign_transaction(&self, address: &Address, data: &[u8]) -> Result<Signature, Error> {
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)?; .ok_or(Error::KeyNotFound)?;
let handle = self.open_path(&device.path)?; let handle = self.open_path(&device.path, &mut usb)?;
let eth_path = &ETH_DERIVATION_PATH_BE[..]; let eth_path = &ETH_DERIVATION_PATH_BE[..];
let etc_path = &ETC_DERIVATION_PATH_BE[..]; let etc_path = &ETC_DERIVATION_PATH_BE[..];
@ -236,11 +239,11 @@ impl Manager {
Ok(Signature::from_rsv(&r, &s, v)) Ok(Signature::from_rsv(&r, &s, v))
} }
fn open_path(&self, path: &str) -> Result<hidapi::HidDevice, Error> { fn open_path<'a>(&self, path: &str, usb: &'a mut hidapi::HidApi) -> Result<hidapi::HidDevice<'a>, Error> {
let mut err = Error::KeyNotFound; let mut err = Error::KeyNotFound;
/// Try to open device a few times. /// Try to open device a few times.
for _ in 0..10 { for _ in 0..10 {
match self.usb.open_path(&path) { match usb.open_path(&path) {
Ok(handle) => return Ok(handle), Ok(handle) => return Ok(handle),
Err(e) => err = From::from(e), Err(e) => err = From::from(e),
} }
@ -340,10 +343,12 @@ impl Manager {
#[test] #[test]
fn smoke() { fn smoke() {
use super::KeyPath;
use rustc_hex::FromHex; use rustc_hex::FromHex;
let mut manager = Manager::new().unwrap();
let manager = Manager::new(KeyPath::Ethereum).unwrap();
manager.update_devices().unwrap(); manager.update_devices().unwrap();
for d in &manager.devices { for d in &*manager.devices.read() {
println!("Device: {:?}", d); println!("Device: {:?}", d);
} }

View File

@ -32,7 +32,6 @@ use std::sync::atomic;
use std::sync::{Arc, Weak}; use std::sync::{Arc, Weak};
use std::sync::atomic::AtomicBool; use std::sync::atomic::AtomicBool;
use std::time::Duration; use std::time::Duration;
use parking_lot::Mutex;
use ethkey::{Address, Signature}; use ethkey::{Address, Signature};
pub use ledger::KeyPath; pub use ledger::KeyPath;
@ -90,11 +89,11 @@ impl From<libusb::Error> for Error {
pub struct HardwareWalletManager { pub struct HardwareWalletManager {
update_thread: Option<thread::JoinHandle<()>>, update_thread: Option<thread::JoinHandle<()>>,
exiting: Arc<AtomicBool>, exiting: Arc<AtomicBool>,
ledger: Arc<Mutex<ledger::Manager>>, ledger: Arc<ledger::Manager>,
} }
struct EventHandler { struct EventHandler {
ledger: Weak<Mutex<ledger::Manager>>, ledger: Weak<ledger::Manager>,
} }
impl libusb::Hotplug for EventHandler { impl libusb::Hotplug for EventHandler {
@ -103,7 +102,7 @@ impl libusb::Hotplug for EventHandler {
if let Some(l) = self.ledger.upgrade() { if let Some(l) = self.ledger.upgrade() {
for _ in 0..10 { for _ in 0..10 {
// The device might not be visible right away. Try a few times. // 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); debug!("Error enumerating Ledger devices: {}", e);
0 0
}) > 0 { }) > 0 {
@ -117,7 +116,7 @@ impl libusb::Hotplug for EventHandler {
fn device_left(&mut self, _device: libusb::Device) { fn device_left(&mut self, _device: libusb::Device) {
debug!("USB Device lost"); debug!("USB Device lost");
if let Some(l) = self.ledger.upgrade() { 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); debug!("Error enumerating Ledger devices: {}", e);
} }
} }
@ -125,15 +124,15 @@ impl libusb::Hotplug for EventHandler {
} }
impl HardwareWalletManager { impl HardwareWalletManager {
pub fn new() -> Result<HardwareWalletManager, Error> { pub fn new(key_path: KeyPath) -> Result<HardwareWalletManager, Error> {
let usb_context = Arc::new(libusb::Context::new()?); 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) }))?; usb_context.register_callback(None, None, None, Box::new(EventHandler { ledger: Arc::downgrade(&ledger) }))?;
let exiting = Arc::new(AtomicBool::new(false)); let exiting = Arc::new(AtomicBool::new(false));
let thread_exiting = exiting.clone(); let thread_exiting = exiting.clone();
let l = ledger.clone(); let l = ledger.clone();
let thread = thread::Builder::new().name("hw_wallet".to_string()).spawn(move || { 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); debug!("Error updating ledger devices: {}", e);
} }
loop { 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. /// List connected wallets. This only returns wallets that are ready to be used.
pub fn list_wallets(&self) -> Vec<WalletInfo> { pub fn list_wallets(&self) -> Vec<WalletInfo> {
self.ledger.lock().list_devices() self.ledger.list_devices()
} }
/// Get connected wallet info. /// Get connected wallet info.
pub fn wallet_info(&self, address: &Address) -> Option<WalletInfo> { pub fn wallet_info(&self, address: &Address) -> Option<WalletInfo> {
self.ledger.lock().device_info(address) self.ledger.device_info(address)
} }
/// Sign transaction data with wallet managing `address`. /// Sign transaction data with wallet managing `address`.
pub fn sign_transaction(&self, address: &Address, data: &[u8]) -> Result<Signature, Error> { pub fn sign_transaction(&self, address: &Address, data: &[u8]) -> Result<Signature, Error> {
Ok(self.ledger.lock().sign_transaction(address, data)?) Ok(self.ledger.sign_transaction(address, data)?)
} }
} }