search

UnstoppableSwap / core

Bitcoin–Monero Cross-chain Atomic Swap (+ GUI)
GNU General Public License v3.0
19 stars 5 forks source link

Replace monero-wallet-rpc with monero bindings #114

Open binarybaron opened 2 weeks ago

binarybaron commented 2 weeks ago

Currently we are dependent on monero-wallet-rpc which causes a few issues:

  1. We need to spawn a new process for interacting with the wallets. This means we need to manage the process including killing it when we exit which is not always easy (see https://github.com/UnstoppableSwap/core/issues/21)
  2. We cannot spawn child processes on iOS (See here)
  3. Sometimes we get blocked by antivirus software because spawning another binary is seen as suspicious
  4. We need to download and verify the monero-wallet-rpc on startup which introduces complexity

We should replace this with native bindings. I see two ways to do this:

  1. Use the Rust FFI bindings to monero_c. @sneurlax is working on in this PR
    • A proof of concept already works but we need to work sure this works reliably.
    • We will be able to safely implement other wallet functionality as this is already implemented in wallet2.h which is the backbone for monero_c
  2. Use the re-implementation of the monero cryptography and wallet scanner developed by @serai. See here.
    • This would be cool as we would be able to use pure Rust but this is not a plug and play solution. We would have to implement mechanism for caching scanned blocks and for efficently downloading blocks.
    • This will also make it a lot harder to subsequently integrate wallet functionality into the GUI

I prefer option 1 for now. Once serai's implementation has been audited we can consider switching.

Einliterflasche commented 2 weeks ago

In any case we need to make sure our implementation is modular enough that we can still realistically switch later.

binarybaron commented 2 weeks ago

In any case we need to make sure our implementation is modular enough that we can still realistically switch later.

Yes definitely.

It should be reasonably simple to allow two exclusionary cargo features (monero-native and monero-rpc) to switch between the two at compile time.

I think especially for the asb we should allow users to keep using monero-wallet-rpc.

sneurlax commented 1 week ago

re:

  1. Use the re-implementation of the monero cryptography and wallet scanner developed by @serai. See here.

    • This would be cool as we would be able to use pure Rust but this is not a plug and play solution. We would have to implement mechanism for caching scanned blocks and for efficently downloading blocks.
    • This will also make it a lot harder to subsequently integrate wallet functionality into the GUI

after finishing monero_c/impls/monero.rs (just check_tx_key and get_version left now), I want to proceed to start working on this using my monero-rust crate (which will change soon: right now it's is FFI-focused, but all FFI functionality is going to be removed to another crate and monero-rust itself will just provide the same API you specified for monero_c/impls/monero.rs, but in pure rust using monero-wallet & co.

Is that API you specified "all" I need to do? I'll have to rope cuprate in to get it working completely but it's ready.

binarybaron commented 1 week ago

re:

  1. Use the re-implementation of the monero cryptography and wallet scanner developed by @serai. See here.

    • This would be cool as we would be able to use pure Rust but this is not a plug and play solution. We would have to implement mechanism for caching scanned blocks and for efficently downloading blocks.
    • This will also make it a lot harder to subsequently integrate wallet functionality into the GUI

after finishing monero_c/impls/monero.rs (just check_tx_key and get_version left now), I want to proceed to start working on this using my monero-rust crate (which will change soon: right now it's is FFI-focused, but all FFI functionality is going to be removed to another crate and monero-rust itself will just provide the same API you specified for monero_c/impls/monero.rs, but in pure rust using monero-wallet & co.

I'm glad to to hear that all the APIs needed have been implemented. We're currently very busy with https://github.com/UnstoppableSwap/core/pull/109 but afterwards, we'll tackle this issue.

As I see it: We chose to use FFI instead of monero-serai because of the stability it provides, it's security guarantees and it's similiar behaviour to monero-wallet-rpc. I think we are only really gonna consider switiching to serai's implementation once it gets audited and has wallet functionality built into the libary itself...

Is that API you specified "all" I need to do? I'll have to rope cuprate in to get it working completely but it's ready.

Yes, that's all we are gonna need for now. Thank you again for your effort. We really do appreciate it.

I'll have to rope cuprate in to get it working completely but it's ready.

What do you need Cuprate for?

sneurlax commented 1 week ago

kayaba mentioned that there's a scanner included in monero-wallet so I may not actually need cuprate for anything