summa-tx / coins

Rust implementations of BIP32/39 and Ledger device comms
Other
90 stars 31 forks source link

Feature request: make `coins-ledger` work on Android Termux #94

Closed xJonathanLEI closed 2 years ago

xJonathanLEI commented 2 years ago

Is your feature request related to a problem? Please describe. I'm using a downstream lib ethers-rs and want to run scripts on the go on Android Termux with my Ledger. Currently their Ledger signer (which uses coins-ledger) fails with a runtime error on Termux.

Describe the solution you'd like Make the signer work on Termux. If we manage to pull this off, we can even wrap it inside a Node module and use it for existing ethers.js scripts.

Describe alternatives you've considered Currently pretty much the only alternative that I'm aware of (tho outside the scope of this lib) would be to build the script as a WalletConnect-enabled webpage, open the said page in a browser, and connect to it from Ledger Live. This is far from ideal and limited by the feature set that WalletConnect supports (e.g. you can't sign transactions without broadcasting to RPC).

Additional context

My device:

$ uname --all
Linux localhost 5.4.86-qgki-********-*************** #1 SMP PREEMPT Mon Feb 28 16:19:22 KST 2022 aarch64 Android

Using master for ethers-rs with ledger feature enabled:

ethers = { git = "https://github.com/gakonst/ethers-rs", features = ["ledger"] }

Running this code:

use ethers::{prelude::*, utils::to_checksum};

#[tokio::main]
async fn main() {
    let ledger = Ledger::new(HDPath::Other(String::from("m/44'/60'/0'/0/0")), 1)
        .await
        .unwrap();

    println!("{}", to_checksum(&ledger.address(), None));
}

Yields this error:

thread 'main' panicked at 'Error getting api_mutex: Hid(InitializationError)', /data/data/com.termux/files/home/.cargo/registry/src/github.com-1ecc6299db9ec823/coins-ledger-0.6.0/src/transports/hid.rs:147:42
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
xJonathanLEI commented 2 years ago

Made some progress on my side. Am able to make rusb work on Termux:

$ uname -a
Linux localhost 5.4.86-qgki-********-*************** #1 SMP PREEMPT Mon Feb 28 16:19:22 KST 2022 aarch64 Android
$ termux-usb -e "cargo run" "/dev/bus/usb/001/007"
    Finished dev [unoptimized + debuginfo] target(s) in 0.09s
     Running `target/debug/rusb-test 7`
Vendor ID: 2c97
Product ID: 4015
Manufacturer: Ledger
Product: Nano X
Serial No.: 0001
prestwich commented 2 years ago

we intend to switch to an hidapi crate based on rusb as soon as one is reasonably mature and well maintained. I'm not convinced that that exists yet. any suggestions?

xJonathanLEI commented 2 years ago

Wait I thought this library is already using rusb:

https://github.com/summa-tx/bitcoins-rs/blob/b46b845e1f3c96e449a7f2c1e7238ba6798fbefb/ledger/Cargo.toml#L32

xJonathanLEI commented 2 years ago

Made it to work by patching coins-ledger as well as the upstream lib hidapi-rusb:

image

Other functions like getting addresses have also been tested. Works flawlessly. (They're the all the same from coins-ledger's perspective anyways)

I guess I'll have to submit an upstream PR first, and hopefully get it to merged before proceeding with this issue :)

xJonathanLEI commented 2 years ago

Made a PR to the upstream lib, hopefully getting merged soon:

https://github.com/joshieDo/hidapi-rs/pull/2

prestwich commented 2 years ago

I haven't had time to look into this yet, thanks for doing legwork on it :)

xJonathanLEI commented 2 years ago

Upstream PR merged and new version published with the patch. It's a patch version bump so no need for downstream crates like coins-ledger to do anything to incorporate.

Now we just need to build the Termux support into coins-ledger for good :)

prestwich commented 2 years ago

any action from me required besides bump dep version, test, and re-publish?

xJonathanLEI commented 2 years ago

I think that's all it will be needed from you side :)

I will submit a PR for anything else before you do.