wizardsardine / async-hwi Goto Github PK

Similar to #25.

i got this w/ specter with a loaded key:

$ hwi device list                                                                                   
Error: UnimplementedMethod

if no key loaded in specter, hwi don't return => maybe we should return an error after some timeout

works well w/ Nano s & Nano S+ btw

Currently, the examples/hwi.rs uses and tests only the functions related to enumeration of the devices (try_connect, device_kind, get_master_fingerprint, get_version).

It would be great if there was another example (maybe examples/sign.rs) which also tests functions related to signing (get_extended_pubkey , register_wallet , sign_tx ). This code should assume that the device is loaded with the standard SLIP-0014 mnemonic (12 * all). If this existed, it would be trivial to check whether implementation behaves correctly for new HW wallets (Trezor) being added to the project.

Guys, you started a great effort I have been looking for the last two years - writing pure rust HWI library. Thank you!

One thing though is that I avoid async rust (due to many reasons, mainly listed in ), and thus I am planning to work on a fork of this library which will remove tokio and all async things. I am willing to maintain and support it under auspices of https://github.com/BP-WG/ and use in the https://github.com/mycitadel. Will be glad to see if you - or others - will be also interested in such a thing.

I think we are sending a lot of needless data that creates tons of roundtrips on the Ledger.

Otherwise this logic would be wrong:

It would be nice if we could make this project more friendly to downstream users. At the moment it is tailored to the needs of the Liana GUI, but i think a HWI library in a compiled language is dearly needed in the Bitcoin space. And Rust seems like a good fit for this. Plus, we already have the implementation for the Ledger and Specter devices.

I tweeted about this and it looks like there is interest from other Bitcoin developers in contributing to a Rust HWI. Given the interest in using or contributing to this library, i'm happy for Wizardsardine to dedicate some resources to extend the scope of this library to a wider audience than the Liana project. Let's discuss on this issue the path forward.

Regarding the goal of the library i think there isn't much to discuss. It should aim to be a high-level interface to the largest reasonable number of Bitcoin signing devices. I would just add a requirement that output descriptors be first class citizens. I'm not sure yet how (maybe using @bigspider's wallet policies?).

Regarding the path toward this goal, i think we:

1. First need to cleanup the current code. It's been a one man (@edouardparis)'s project and it could use some review (probably mine but anyone's welcome) / bit more testing.
2. Make the asyncness optional.
3. Re-design the main interface, the HWI trait, with the new goals of this library in mind.
4. Add support for more signing devices.
5. Provide Python bindings.
6. Get to feature parity with the existing Python HWI.
7. ?Get the existing Python HWI to migrate to Rust HWI under the hood?
8. ?Honour an MSRV and move this repo to an org that isn't our company? Rust-Bitcoin maybe?

Thoughts? People interested in contributing to or using this Rust-HWI please chime in!

The regex currently only matches on extended keys:

Maybe also get rid of zpub ypub etc..?

Looking into examples/hwi.rs it seems that only one connected wallet of a certain type is supported.

What needs to be done so the lib supports multiple connected devices of the same type?

to check compatibility of device to a feature supported versions collection like HashMap<DeviceKind, Option>

I was hoping to create a test for register_wallet when I encountered this bug. The code is as follows:

let policy = "pk([5c9e228d/44'/0'/0']xpub6BiVtCpG9fQPxnPmHXG8PhtzQdWC2Su4qWu6XW9tpWFYhxydCLJGrWBJZ5H6qTAHdPQ7pQhtpjiYZVZARo14qHiay2fvrX996oEP42u8wZy/**)";
hw.register_wallet("Ledger", &policy).await.unwrap();

This results in a panic on the second line with message:

called `Result::unwrap()` on an `Err` value: Device("Device {\n    command: 2,\n    status: IncorrectData,\n}")

The ledger is initialized with seed all all all all all all all all all all all all. The fingerprint and xpub in the policy match with the ones reported by other async-hwi functions (and the SLIP-0014 specification)

https://github.com/revault/async-hwi/blob/master/src/ledger.rs#L107|
should return an error

This was reported to me by @bigspider, and i think it makes sense. Copy paste of the report follows.

async fn sign_tx(&self, tx: &mut Psbt) -> Result<(), Error>;

I think you shouldn't abstract the policy in the HWI trait; sign_psbt without specifying what policy you're signing with puts the burden of figuring it out to the library.
But the caller of sign_psbt always knows which policy you're signing with.
if you have multiple inputs with different policies, most likely it's going to be from different signers − so multiple calls to sign_psbt with different policies
sure, one might imagine signing with multiple policies at the same time (if they are all controlled by the same hardware wallet), but I wouldn't bother yet, can generalize in the future

The comment sounds like we are doing this ourselves (which'd be awkward), but it's actually the Specter that does it.

rust-bitcoin 0.30.0 introduced many changes, including but not limited to moving stuff out of util. The dependency for this crate needs to be updated to reflect the changes.

Resources:
https://github.com/rust-bitcoin/rust-bitcoin/blob/master/bitcoin/CHANGELOG.md
https://rust-bitcoin.org/blog/release-0.30.0/

See wizardsardine/liana#532