Skip to content

Commit

Permalink
fix: greatly simplify rusb usage (#129)
Browse files Browse the repository at this point in the history
* fix: greatly simplify rusb usage

* lints: clippies and more

* fix: restore getrandom dep

* fix: fmt

* fix: avoid locking mutex twice

* fix: android compilation error (#130)

* fix: correct buffer in write

* clippy

---------

Co-authored-by: Jonathan LEI <[email protected]>
  • Loading branch information
prestwich and xJonathanLEI authored Dec 2, 2023
1 parent de4de26 commit 87027bd
Show file tree
Hide file tree
Showing 10 changed files with 312 additions and 253 deletions.
7 changes: 2 additions & 5 deletions ledger/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,12 @@ crate-type = ["cdylib", "rlib"]
async-trait = "0.1"
cfg-if = "1.0"
hex = "0.4"
serde = { version = "1.0", features = ["derive"] }
tap = "1.0"
thiserror = "1.0"

# native
[target.'cfg(not(target_arch = "wasm32"))'.dependencies]
lazy_static = "1.4"
once_cell = "1.18.0"
byteorder = "1.5"
libc = "0.2"
matches = "0.1"
tracing = "0.1"
hidapi-rusb = "1.3"
tokio = { version = "1.34", features = ["sync", "rt"] }
Expand All @@ -53,3 +49,4 @@ tokio = { version = "1.34", features = ["rt-multi-thread", "macros"] }
[features]
browser = []
node = []

9 changes: 5 additions & 4 deletions ledger/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ impl APDUData {
}

/// Consume the struct and get the underlying data
#[allow(clippy::missing_const_for_fn)] // false positive
pub fn data(self) -> Vec<u8> {
self.0
}
Expand Down Expand Up @@ -234,12 +235,12 @@ impl std::fmt::Display for APDUResponseCodes {

impl APDUResponseCodes {
/// True if the response is a success, else false.
pub fn is_success(self) -> bool {
self == APDUResponseCodes::NoError
pub const fn is_success(self) -> bool {
matches!(self, APDUResponseCodes::NoError)
}

/// Return a description of the response code.
pub fn description(self) -> &'static str {
pub const fn description(self) -> &'static str {
match self {
APDUResponseCodes::NoError => "[APDU_CODE_NOERROR]",
APDUResponseCodes::ExecutionError => {
Expand All @@ -265,7 +266,7 @@ impl APDUResponseCodes {
}
APDUResponseCodes::InvalidP1P2 => "[APDU_CODE_INVALIDP1P2] Wrong parameter(s) P1-P2",
APDUResponseCodes::InsNotSupported => {
"[APDU_CODE_INS_NOT_SUPPORTED] Instruction code not supported or invalid"
"[APDU_CODE_INS_NOT_SUPPORTED] Instruction code not supported or invalid. Hint: Is the correct application open on the device?"
}
APDUResponseCodes::ClaNotSupported => {
"[APDU_CODE_CLA_NOT_SUPPORTED] Class not supported"
Expand Down
14 changes: 12 additions & 2 deletions ledger/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,16 @@
//! Ledger utilites and transports
#![warn(missing_docs)]
#![warn(unused_extern_crates)]
#![warn(
missing_copy_implementations,
missing_debug_implementations,
missing_docs,
unreachable_pub,
clippy::missing_const_for_fn,
rustdoc::all
)]
#![deny(unused_must_use, rust_2018_idioms)]
#![cfg_attr(not(test), warn(unused_crate_dependencies))]
#![cfg_attr(docsrs, feature(doc_cfg, doc_auto_cfg))]

/// APDU utilities.
pub mod common;
Expand All @@ -19,3 +28,4 @@ pub use {
};

mod protocol;
pub use protocol::LedgerProtocol;
3 changes: 3 additions & 0 deletions ledger/src/protocol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use crate::{Ledger, LedgerError};
/// be invoked. The protocol may also fail to recover, in which case future
/// uses of the device may fail.
pub trait LedgerProtocol {
/// The output of the protocol.
type Output;

/// Run the protocol. This sends commands to the device, and receives
Expand All @@ -29,6 +30,8 @@ pub trait LedgerProtocol {
Ok(())
}

/// Run the protocol. This sends commands to the device, and receives
/// responses. The transport is locked while this function is running.
fn run(&mut self, transport: &mut Ledger) -> Result<Self::Output, LedgerError> {
match self.execute(transport) {
Ok(output) => Ok(output),
Expand Down
3 changes: 2 additions & 1 deletion ledger/src/transports/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ cfg_if::cfg_if! {
/// A Ledger device connection. This wraps the default transport type. In
/// native code, this is the `hidapi` library. When the `node` or `browser`
/// feature is selected, it is a Ledger JS transport library.
#[derive(Debug)]
pub struct Ledger(DefaultTransport);

#[cfg_attr(not(target_arch = "wasm32"), async_trait)]
Expand Down Expand Up @@ -68,7 +69,7 @@ impl LedgerAsync for Ledger {
"Received response from device"
)
}
Err(e) => error!(err = format!("{}", &e), "Received error from device"),
Err(err) => error!(%err, "Error during communication"),
}
resp
}
Expand Down
50 changes: 50 additions & 0 deletions ledger/src/transports/native/error.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
use thiserror::Error;

// Mock the type in other target_os
#[cfg(not(target_os = "linux"))]
mod nix {
#[derive(thiserror::Error, Debug, Copy, Clone)]
pub enum Error {
#[error("")]
Unimplemented,
}
}

/// Ledger transport errors
#[derive(Error, Debug)]
pub enum NativeTransportError {
/// Device not found error
#[error("Ledger device not found")]
DeviceNotFound,
/// Device open error.
#[error("Error opening device. {0}. Hint: This usually means that the device is already in use by another transport instance.")]
CantOpen(hidapi_rusb::HidError),
/// SequenceMismatch
#[error("Sequence mismatch. Got {got} from device. Expected {expected}")]
SequenceMismatch {
/// The sequence returned by the device
got: u16,
/// The expected sequence
expected: u16,
},
/// Communication error
#[error("Ledger device: communication error `{0}`")]
Comm(&'static str),
/// Ioctl error
#[error(transparent)]
Ioctl(#[from] nix::Error),
/// i/o error
#[error(transparent)]
Io(#[from] std::io::Error),
/// HID error
#[error(transparent)]
Hid(#[from] hidapi_rusb::HidError),
/// UT8F error
#[error(transparent)]
UTF8(#[from] std::str::Utf8Error),
/// Termux USB FD env var does not exist or fails to parse. This error is
/// only returned by android-specific code paths, and may be safely ignored
/// by non-android users
#[error("Invalid TERMUX_USB_FD variable. Are you using termux-usb?")]
InvalidTermuxUsbFd,
}
Loading

0 comments on commit 87027bd

Please sign in to comment.