Add Trezor support

[wszdexdrf]

This PR aims to add Trezor functionality to this library using trezor-client crate. The following commits add an implementation of trait HWI using the above mentioned Trezor API in rust.

[prusnak]

@wszdexdrf it seems the CI is failing: https://github.com/wizardsardine/async-hwi/actions/runs/5775689020/job/15668289653?pr=41

[wszdexdrf]

I fixed the CI tests. The problem was the the CI didn't have a protobuf compiler, which is needed to build the trezor-client crate (dependency).

[edouardparis]

Thank you for the PR !

[prusnak]

https://crates.io/crates/trezor-client 0.1.2 with added Sync and Send to Transport has been released

[edouardparis]

Seems good, targeting next week to release it after testing with simulator 👍

[edouardparis]

Can you add an implementation of From<TrezorClient> for Box<dyn HWI + Send> please
like https://github.com/wizardsardine/async-hwi/blob/master/src/specter.rs#L145

[wszdexdrf]

Can you add an implementation of From<TrezorClient> for Box<dyn HWI + Send> please like https://github.com/wizardsardine/async-hwi/blob/master/src/specter.rs#L145

Something like this?

[edouardparis]

Yes perfect ! , my bad for commenting too quickly.

[matejcik]

sorry @wszdexdrf i'm having trouble following the logic but, can this actually sign transactions?

ISTM from sign_tx you get back a SignTxProgress object ... but that would require you to repeatedly call ack_psbt(tx) until you get an error or until progress.finished() is true ... or is there some magic loop that I am missing?

[wszdexdrf]

sorry @wszdexdrf i'm having trouble following the logic but, can this actually sign transactions?

Yes, about that. I didn't want to make any assumptions, hence I created the sign_tx to be pretty much one-to-one implementation of the sign_tx in the trezor_client library. I was expecting the user of the library to create the loop or whatever they wish, and I returned a hopefully helpful error value of DeviceDidNotSign if the signing process was still incomplete.

I honestly don't feel strongly about either way. We could also create a while loop (with some sleeps in between, hopefully using the async nature somehow), or simply leaving it up to the user. What do you think about this @matejcik?

[matejcik]

well, sign_tx from HWI trait doesn't return anything, it expects you to modify the passed in psbt object (as far as I can tell anyway). there is no way for the caller to get ahold of the SignTxProgress object so you'll need to do the loop internally somehow.

given that the whole thing is async, it would be nice to do it in an async manner -- but trezor-client library is currently synchronous, so I'm not sure how to accomplish that

[wszdexdrf]

but that would require you to repeatedly call ack_psbt(tx) until you get an error or until progress.finished() is true

@matejcik please take a look at https://github.com/wszdexdrf/async-hwi/pull/3 where I used some tests to test the functionality.

[wszdexdrf]

I did some testing and now the code is able to sign transactions. If anyone needs to look at the tests, they are on branch https://github.com/wszdexdrf/async-hwi/tree/tests

[wszdexdrf]

Pinging reviewers for review and merge. @matejcik @edouardparis @prusnak

[edouardparis]

It is not clear here why it is serialize and deserialize, can you not just clone the key ?

[wszdexdrf]

The pk from the bip32_derivation is of type bitcoin::secp256k1::PublicKey and the type needed in partial_sigs is bitcoin::PublicKey. The serializing and deserializing is used as a rudimentary type-casting.

[edouardparis]

Sorry I did not forget you, I am just very busy. But I am going to a conference next week and I hope to test with a real device here.