summa-tx / coins

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

can Ledger be Send ? #124

Closed naps62 closed 10 months ago

naps62 commented 11 months ago

I was running into trouble using ethers::signers::Ledger in an async environment (#[tauri::command], which requires Send)

Was a painful couple of hours, since this touches on some Rust parts I'm not that confident in, but this patch seemed to solve my problems.

Without this patch, I was getting the following error, which can be traced down to this line of my project

 1  error: future cannot be sent between threads safely
   --> crates/wallets/src/ledger.rs:21:66
    |
 21 |       async fn create(params: serde_json::Value) -> Result<Wallet> {
    |  __________________________________________________________________^
 22 | |         let params = serde_json::from_value(params)?;
 23 | |         Ok(Wallet::Ledger(Self::from_params(params).await?))
 24 | |     }
    | |_____^ future created by async block is not `Send`
    |
    = help: the trait `std::marker::Send` is not implemented for `dyn Future<Output = std::result::Result<coins_ledger::transp
 orts::Ledger, coins_ledger::errors::LedgerError>>`
 note: future is not `Send` as this value is used across an await
   --> /home/naps62/.cargo/git/checkouts/ethers-rs-c3a7c0a0ae0fe6be/841ff8c/ethers-signers/src/ledger/app.rs:57:40
    |
 57 |         let transport = Ledger::init().await?;
    |                         -------------- ^^^^^ - `Ledger::init()` is later dropped here
    |                         |              |
    |                         |              await occurs here, with `Ledger::init()` maybe used later
    |                         has type `Pin<Box<dyn Future<Output = std::result::Result<coins_ledger::transports::Ledger, coin
 s_ledger::errors::LedgerError>>>>` which is not `Send`
    = note: required for the cast from `Pin<Box<[async block@crates/wallets/src/ledger.rs:21:66: 24:6]>>` to `Pin<Box<(dyn Fut
 ure<Output = std::result::Result<wallet::Wallet, error::Error>> + std::marker::Send + 'async_trait)>>`

 2  error: future cannot be sent between threads safely
   --> crates/wallets/src/ledger.rs:21:66
    |
 21 |       async fn create(params: serde_json::Value) -> Result<Wallet> {
    |  __________________________________________________________________^
 22 | |         let params = serde_json::from_value(params)?;
 23 | |         Ok(Wallet::Ledger(Self::from_params(params).await?))
 24 | |     }
    | |_____^ future created by async block is not `Send`
    |
    = help: within `coins_ledger::transports::Ledger`, the trait `Sync` is not implemented for `*mut c_void`
 note: future is not `Send` as this value is used across an await
   --> /home/naps62/.cargo/git/checkouts/ethers-rs-c3a7c0a0ae0fe6be/841ff8c/ethers-signers/src/ledger/app.rs:58:86
    |
 58 |         let address = Self::get_address_with_path_transport(&transport, &derivation).await?;
    |                                                             ----------               ^^^^^ - `&transport` is later dropp
 ed here
    |                                                             |                        |
    |                                                             |                        await occurs here, with `&transport
 ` maybe used later
    |                                                             has type `&coins_ledger::transports::Ledger` which is not `S
 end`
    = note: required for the cast from `Pin<Box<[async block@crates/wallets/src/ledger.rs:21:66: 24:6]>>` to `Pin<Box<(dyn Fut
 ure<Output = std::result::Result<wallet::Wallet, error::Error>> + std::marker::Send + 'async_trait)>>`

 error: could not compile `iron-wallets` (lib) due to 2 previous errors
prestwich commented 11 months ago

I will look into this, but i can't promise it will happen on any given timeline. If you need a Send instance, consider wrapping it in an Arc?

naps62 commented 11 months ago

@prestwich my understanding is that Arc is only Send if T itself is Send.

But regardless, the problem happens within ethers-rs. more specifically here (which also shows up in the compilation log above)

So the problem needs to be solved either in ethers-rs or here.

Not sure if it was clear on the initial issue, but I do have a branch that seems to fix it (already got a full ledger integration working, been testing for the past few hours with 0 issues). I'll open a PR so you can review directly.