Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace monero-wallet-rpc with monero bindings #114

Open
binarybaron opened this issue Oct 13, 2024 · 5 comments
Open

Replace monero-wallet-rpc with monero bindings #114

binarybaron opened this issue Oct 13, 2024 · 5 comments
Labels
crate: swap Related to the "swap" crate enhancement New feature or request refactoring Related to refactorings. No new features are added.

Comments

@binarybaron
Copy link

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 Kill monero-wallet-rpc process on exit #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.

@binarybaron binarybaron added enhancement New feature or request refactoring Related to refactorings. No new features are added. crate: swap Related to the "swap" crate labels Oct 13, 2024
@binarybaron binarybaron changed the title Replace monro-wallet-rpc with monero bindings Replace monero-wallet-rpc with monero bindings Oct 13, 2024
@Einliterflasche
Copy link

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

@binarybaron
Copy link
Author

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
Copy link

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
Copy link
Author

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 #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
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crate: swap Related to the "swap" crate enhancement New feature or request refactoring Related to refactorings. No new features are added.
Projects
None yet
Development

No branches or pull requests

3 participants