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

feat: monero-native integration (FFI C wrapper) #244

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

binarybaron
Copy link

@binarybaron binarybaron commented Jan 7, 2025

See #114

Motivation

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 want to replace this with native bindings. We 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

TODO, in no specific order

  • Get this to build on all platforms
  • Get this to build in the CI pipeline
  • Get this to build in the CI pipeline for release builds
  • Get this to build on mobile (Android and iOS)
  • Replace all references to monero-wallet-rpc in the code with references to the monero_c rust bindings
  • Fix issue with MONERO_Wallet_checkTxKey (See: Rust develop MrCyjaneK/monero_c#103 (comment))
  • Decide if we want to keep support for monero-wallet-rpc with a feature flag. This could be useful for the asb or for our integration tests. However I'm leaning strongly towards removing support entirely.
  • Use monero_c for our monero-harness crate
  • Implement integration tests for the rust bindings
  • Remove monero-rpc crate
  • Thoroughly test manually and potentially get feedback from people who know the quirks of wallet2 in-and-out
  • Remove all .unwrap() calls
  • Conver the calls into async calls and spawn the wallet on a different thread

@binarybaron binarybaron changed the title swap/monero c wrapper feat: monero-native integration (FFI C wrapper) Jan 7, 2025
@binarybaron
Copy link
Author

@delta1 Hey Byron, I'm currently working on migrating from monero-wallet-rpc to native monero bindings. We use a wrapper called monero_c which is wrapped with FFI bindings (PR but the most up-to-date version is in our PR here. I'm not super familiar with C++ (especially the build systems and all that) and I'm struggling with getting it all running in a nice and reproducible way. If you have any pointers for me, or would like to contribute to getting that working on a reliable way, I'd be super thankful 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant