grassrootseconomics/openethereum
5
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Hardware-wallets Clean up things I missed in the latest PR (#8890)

Browse Source 
* Clean up things that I missed

* Address grumbles

* &Arc<Manager> -> &Manager
* Rustdoc -> Crate doc
This commit is contained in:
Niklas Adolfsson 2018-06-18 11:55:23 +02:00 committed by Afri Schoedon
parent c9cee8fd52
commit b47218521f
3 changed files with 72 additions and 75 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

47
hw/src/ledger.rs
View File

File diff suppressed because one or more lines are too long

34
hw/src/lib.rs
View File

@ -16,8 +16,8 @@
//! Hardware wallet management. //! Hardware wallet management.
#[warn(missing_docs)] #![warn(missing_docs)]
#[warn(warnings)] #![warn(warnings)]
extern crate ethereum_types; extern crate ethereum_types;
extern crate ethkey; extern crate ethkey;
@ -34,22 +34,24 @@ extern crate trezor_sys;
mod ledger; mod ledger;
mod trezor; mod trezor;
use ethkey::{Address, Signature};
use parking_lot::Mutex;
use std::{fmt, time::Duration};
use std::sync::{Arc, atomic, atomic::AtomicBool}; use std::sync::{Arc, atomic, atomic::AtomicBool};
use std::{fmt, time::Duration};
use ethereum_types::U256; use ethereum_types::U256;
use ethkey::{Address, Signature};
use parking_lot::Mutex;
const USB_DEVICE_CLASS_DEVICE: u8 = 0; const USB_DEVICE_CLASS_DEVICE: u8 = 0;
const POLLING_DURATION: Duration = Duration::from_millis(500); const POLLING_DURATION: Duration = Duration::from_millis(500);
/// `HardwareWallet` device
#[derive(Debug)] #[derive(Debug)]
pub struct Device { pub struct Device {
path: String, path: String,
info: WalletInfo, info: WalletInfo,
} }
/// `Wallet` trait
pub trait Wallet<'a> { pub trait Wallet<'a> {
/// Error /// Error
type Error; type Error;
@ -109,7 +111,7 @@ pub enum Error {
} }
/// This is the transaction info we need to supply to Trezor message. It's more /// This is the transaction info we need to supply to Trezor message. It's more
/// or less a duplicate of ethcore::transaction::Transaction, but we can't /// or less a duplicate of `ethcore::transaction::Transaction`, but we can't
/// import ethcore here as that would be a circular dependency. /// import ethcore here as that would be a circular dependency.
pub struct TransactionInfo { pub struct TransactionInfo {
/// Nonce /// Nonce
@ -163,7 +165,7 @@ impl fmt::Display for Error {
} }
impl From<ledger::Error> for Error { impl From<ledger::Error> for Error {
fn from(err: ledger::Error) -> Error { fn from(err: ledger::Error) -> Self {
match err { match err {
ledger::Error::KeyNotFound => Error::KeyNotFound, ledger::Error::KeyNotFound => Error::KeyNotFound,
_ => Error::LedgerDevice(err), _ => Error::LedgerDevice(err),
@ -172,7 +174,7 @@ impl From<ledger::Error> for Error {
} }
impl From<trezor::Error> for Error { impl From<trezor::Error> for Error {
fn from(err: trezor::Error) -> Error { fn from(err: trezor::Error) -> Self {
match err { match err {
trezor::Error::KeyNotFound => Error::KeyNotFound, trezor::Error::KeyNotFound => Error::KeyNotFound,
_ => Error::TrezorDevice(err), _ => Error::TrezorDevice(err),
@ -181,7 +183,7 @@ impl From<trezor::Error> for Error {
} }
impl From<libusb::Error> for Error { impl From<libusb::Error> for Error {
fn from(err: libusb::Error) -> Error { fn from(err: libusb::Error) -> Self {
Error::Usb(err) Error::Usb(err)
} }
} }
@ -189,7 +191,9 @@ impl From<libusb::Error> for Error {
/// Specifies the direction of the `HardwareWallet` i.e, whether it arrived or left /// Specifies the direction of the `HardwareWallet` i.e, whether it arrived or left
#[derive(Debug, Copy, Clone)] #[derive(Debug, Copy, Clone)]
pub enum DeviceDirection { pub enum DeviceDirection {
/// Device arrived
Arrived, Arrived,
/// Device left
Left, Left,
} }
@ -211,16 +215,16 @@ pub struct HardwareWalletManager {
impl HardwareWalletManager { impl HardwareWalletManager {
/// Hardware wallet constructor /// Hardware wallet constructor
pub fn new() -> Result<HardwareWalletManager, Error> { pub fn new() -> Result<Self, Error> {
let exiting = Arc::new(AtomicBool::new(false)); let exiting = Arc::new(AtomicBool::new(false));
let hidapi = Arc::new(Mutex::new(hidapi::HidApi::new().map_err(|e| Error::Hid(e.to_string().clone()))?)); let hidapi = Arc::new(Mutex::new(hidapi::HidApi::new().map_err(|e| Error::Hid(e.to_string().clone()))?));
let ledger = ledger::Manager::new(hidapi.clone(), exiting.clone())?; let ledger = ledger::Manager::new(hidapi.clone(), exiting.clone())?;
let trezor = trezor::Manager::new(hidapi.clone(), exiting.clone())?; let trezor = trezor::Manager::new(hidapi.clone(), exiting.clone())?;
Ok(HardwareWalletManager { Ok(Self {
exiting: exiting, exiting,
ledger: ledger, ledger,
trezor: trezor, trezor,
}) })
} }

60
hw/src/trezor.rs
View File

@ -15,12 +15,10 @@
// along with Parity. If not, see <http://www.gnu.org/licenses/>. // along with Parity. If not, see <http://www.gnu.org/licenses/>.
//! Trezor hardware wallet module. Supports Trezor v1. //! Trezor hardware wallet module. Supports Trezor v1.
//! See http://doc.satoshilabs.com/trezor-tech/api-protobuf.html //! See <http://doc.satoshilabs.com/trezor-tech/api-protobuf.html>
//! and https://github.com/trezor/trezor-common/blob/master/protob/protocol.md //! and <https://github.com/trezor/trezor-common/blob/master/protob/protocol.md>
//! for protocol details. //! for protocol details.
use super::{DeviceDirection, WalletInfo, TransactionInfo, KeyPath, Wallet, Device, USB_DEVICE_CLASS_DEVICE, POLLING_DURATION};
use std::cmp::{min, max}; use std::cmp::{min, max};
use std::sync::{atomic, atomic::AtomicBool, Arc, Weak}; use std::sync::{atomic, atomic::AtomicBool, Arc, Weak};
use std::time::{Duration, Instant}; use std::time::{Duration, Instant};
@ -31,9 +29,8 @@ use ethkey::Signature;
use hidapi; use hidapi;
use libusb; use libusb;
use parking_lot::{Mutex, RwLock}; use parking_lot::{Mutex, RwLock};
use protobuf::{Message, ProtobufEnum}; use protobuf::{self, Message, ProtobufEnum};
use protobuf; use super::{DeviceDirection, WalletInfo, TransactionInfo, KeyPath, Wallet, Device, USB_DEVICE_CLASS_DEVICE, POLLING_DURATION};
use trezor_sys::messages::{EthereumAddress, PinMatrixAck, MessageType, EthereumTxRequest, EthereumSignTx, EthereumGetAddress, EthereumTxAck, ButtonAck}; use trezor_sys::messages::{EthereumAddress, PinMatrixAck, MessageType, EthereumTxRequest, EthereumSignTx, EthereumGetAddress, EthereumTxAck, ButtonAck};
/// Trezor v1 vendor ID /// Trezor v1 vendor ID
@ -41,8 +38,8 @@ const TREZOR_VID: u16 = 0x534c;
/// Trezor product IDs /// Trezor product IDs
const TREZOR_PIDS: [u16; 1] = [0x0001]; const TREZOR_PIDS: [u16; 1] = [0x0001];
const ETH_DERIVATION_PATH: [u32; 5] = [0x8000002C, 0x8000003C, 0x80000000, 0, 0]; // m/44'/60'/0'/0/0 const ETH_DERIVATION_PATH: [u32; 5] = [0x8000_002C, 0x8000_003C, 0x8000_0000, 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 const ETC_DERIVATION_PATH: [u32; 5] = [0x8000_002C, 0x8000_003D, 0x8000_0000, 0, 0]; // m/44'/61'/0'/0/0
/// Hardware wallet error. /// Hardware wallet error.
#[derive(Debug)] #[derive(Debug)]
@ -84,13 +81,13 @@ impl fmt::Display for Error {
} }
impl From<hidapi::HidError> for Error { impl From<hidapi::HidError> for Error {
fn from(err: hidapi::HidError) -> Error { fn from(err: hidapi::HidError) -> Self {
Error::Usb(err) Error::Usb(err)
} }
} }
impl From<protobuf::ProtobufError> for Error { impl From<protobuf::ProtobufError> for Error {
fn from(_: protobuf::ProtobufError) -> Error { fn from(_: protobuf::ProtobufError) -> Self {
Error::Protocol(&"Could not read response from Trezor Device") Error::Protocol(&"Could not read response from Trezor Device")
} }
} }
@ -111,8 +108,8 @@ enum HidVersion {
impl Manager { impl Manager {
/// Create a new instance. /// Create a new instance.
pub fn new(hidapi: Arc<Mutex<hidapi::HidApi>>, exiting: Arc<AtomicBool>) -> Result<Arc<Manager>, libusb::Error> { pub fn new(hidapi: Arc<Mutex<hidapi::HidApi>>, exiting: Arc<AtomicBool>) -> Result<Arc<Self>, libusb::Error> {
let manager = Arc::new(Manager { let manager = Arc::new(Self {
usb: hidapi, usb: hidapi,
devices: RwLock::new(Vec::new()), devices: RwLock::new(Vec::new()),
locked_devices: RwLock::new(Vec::new()), locked_devices: RwLock::new(Vec::new()),
@ -171,7 +168,7 @@ impl Manager {
} }
fn u256_to_be_vec(&self, val: &U256) -> Vec<u8> { fn u256_to_be_vec(&self, val: &U256) -> Vec<u8> {
let mut buf = [0u8; 32]; let mut buf = [0_u8; 32];
val.to_big_endian(&mut buf); val.to_big_endian(&mut buf);
buf.iter().skip_while(|x| **x == 0).cloned().collect() buf.iter().skip_while(|x| **x == 0).cloned().collect()
} }
@ -226,8 +223,8 @@ impl Manager {
let mut data = Vec::new(); let mut data = Vec::new();
let hid_version = self.probe_hid_version(device)?; let hid_version = self.probe_hid_version(device)?;
// Magic constants // Magic constants
data.push('#' as u8); data.push(b'#');
data.push('#' as u8); data.push(b'#');
// Convert msg_id to BE and split into bytes // Convert msg_id to BE and split into bytes
data.push(((msg_id >> 8) & 0xFF) as u8); data.push(((msg_id >> 8) & 0xFF) as u8);
data.push((msg_id & 0xFF) as u8); data.push((msg_id & 0xFF) as u8);
@ -243,8 +240,8 @@ impl Manager {
let mut total_written = 0; let mut total_written = 0;
for chunk in data.chunks(63) { for chunk in data.chunks(63) {
let mut padded_chunk = match hid_version { let mut padded_chunk = match hid_version {
HidVersion::V1 => vec!['?' as u8], HidVersion::V1 => vec![b'?'],
HidVersion::V2 => vec![0, '?' as u8], HidVersion::V2 => vec![0, b'?'],
}; };
padded_chunk.extend_from_slice(&chunk); padded_chunk.extend_from_slice(&chunk);
total_written += device.write(&padded_chunk)?; total_written += device.write(&padded_chunk)?;
@ -253,10 +250,10 @@ impl Manager {
} }
fn probe_hid_version(&self, device: &hidapi::HidDevice) -> Result<HidVersion, Error> { fn probe_hid_version(&self, device: &hidapi::HidDevice) -> Result<HidVersion, Error> {
let mut buf2 = [0xFFu8; 65]; let mut buf2 = [0xFF_u8; 65];
buf2[0] = 0; buf2[0] = 0;
buf2[1] = 63; buf2[1] = 63;
let mut buf1 = [0xFFu8; 64]; let mut buf1 = [0xFF_u8; 64];
buf1[0] = 63; buf1[0] = 63;
if device.write(&buf2)? == 65 { if device.write(&buf2)? == 65 {
Ok(HidVersion::V2) Ok(HidVersion::V2)
@ -272,7 +269,7 @@ impl Manager {
let mut buf = vec![0; 64]; let mut buf = vec![0; 64];
let first_chunk = device.read_timeout(&mut buf, 300_000)?; let first_chunk = device.read_timeout(&mut buf, 300_000)?;
if first_chunk < 9 || buf[0] != '?' as u8 || buf[1] != '#' as u8 || buf[2] != '#' as u8 { if first_chunk < 9 || buf[0] != b'?' || buf[1] != b'#' || buf[2] != b'#' {
return Err(protocol_err); return Err(protocol_err);
} }
let msg_type = MessageType::from_i32(((buf[3] as i32 & 0xFF) << 8) + (buf[4] as i32 & 0xFF)).ok_or(protocol_err)?; let msg_type = MessageType::from_i32(((buf[3] as i32 & 0xFF) << 8) + (buf[4] as i32 & 0xFF)).ok_or(protocol_err)?;
@ -308,12 +305,9 @@ impl <'a>Wallet<'a> for Manager {
message.set_gas_price(self.u256_to_be_vec(&t_info.gas_price)); message.set_gas_price(self.u256_to_be_vec(&t_info.gas_price));
message.set_value(self.u256_to_be_vec(&t_info.value)); message.set_value(self.u256_to_be_vec(&t_info.value));
match t_info.to { if let Some(addr) = t_info.to {
Some(addr) => {
message.set_to(addr.to_vec()) message.set_to(addr.to_vec())
} }
None => (),
}
let first_chunk_length = min(t_info.data.len(), 1024); let first_chunk_length = min(t_info.data.len(), 1024);
let chunk = &t_info.data[0..first_chunk_length]; let chunk = &t_info.data[0..first_chunk_length];
message.set_data_initial_chunk(chunk.to_vec()); message.set_data_initial_chunk(chunk.to_vec());
@ -387,9 +381,9 @@ impl <'a>Wallet<'a> for Manager {
Ok(Device { Ok(Device {
path: dev_info.path.clone(), path: dev_info.path.clone(),
info: WalletInfo { info: WalletInfo {
name: name, name,
manufacturer: manufacturer, manufacturer,
serial: serial, serial,
address: addr, address: addr,
}, },
}) })
@ -439,7 +433,7 @@ impl <'a>Wallet<'a> for Manager {
} }
// Try to connect to the device using polling in at most the time specified by the `timeout` // Try to connect to the device using polling in at most the time specified by the `timeout`
fn try_connect_polling(trezor: Arc<Manager>, duration: &Duration, dir: DeviceDirection) -> bool { fn try_connect_polling(trezor: &Manager, duration: &Duration, dir: DeviceDirection) -> bool {
let start_time = Instant::now(); let start_time = Instant::now();
while start_time.elapsed() <= *duration { while start_time.elapsed() <= *duration {
if let Ok(num_devices) = trezor.update_devices(dir) { if let Ok(num_devices) = trezor.update_devices(dir) {
@ -462,7 +456,7 @@ struct EventHandler {
impl EventHandler { impl EventHandler {
/// Trezor event handler constructor /// Trezor event handler constructor
pub fn new(trezor: Weak<Manager>) -> Self { pub fn new(trezor: Weak<Manager>) -> Self {
Self { trezor: trezor } Self { trezor }
} }
} }
@ -470,7 +464,7 @@ impl libusb::Hotplug for EventHandler {
fn device_arrived(&mut self, _device: libusb::Device) { fn device_arrived(&mut self, _device: libusb::Device) {
debug!(target: "hw", "Trezor V1 arrived"); debug!(target: "hw", "Trezor V1 arrived");
if let Some(trezor) = self.trezor.upgrade() { if let Some(trezor) = self.trezor.upgrade() {
if try_connect_polling(trezor, &POLLING_DURATION, DeviceDirection::Arrived) != true { if try_connect_polling(&trezor, &POLLING_DURATION, DeviceDirection::Arrived) != true {
trace!(target: "hw", "No Trezor connected"); trace!(target: "hw", "No Trezor connected");
} }
} }
@ -479,7 +473,7 @@ impl libusb::Hotplug for EventHandler {
fn device_left(&mut self, _device: libusb::Device) { fn device_left(&mut self, _device: libusb::Device) {
debug!(target: "hw", "Trezor V1 left"); debug!(target: "hw", "Trezor V1 left");
if let Some(trezor) = self.trezor.upgrade() { if let Some(trezor) = self.trezor.upgrade() {
if try_connect_polling(trezor, &POLLING_DURATION, DeviceDirection::Left) != true { if try_connect_polling(&trezor, &POLLING_DURATION, DeviceDirection::Left) != true {
trace!(target: "hw", "No Trezor disconnected"); trace!(target: "hw", "No Trezor disconnected");
} }
} }
@ -500,7 +494,7 @@ fn test_signature() {
let addr: Address = H160::from("some_addr"); let addr: Address = H160::from("some_addr");
assert_eq!(try_connect_polling(manager.clone(), &POLLING_DURATION, DeviceDirection::Arrived), true); assert_eq!(try_connect_polling(&manager.clone(), &POLLING_DURATION, DeviceDirection::Arrived), true);
let t_info = TransactionInfo { let t_info = TransactionInfo {
nonce: U256::from(1), nonce: U256::from(1),