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

MoneroWalletLight implementation #84

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

everoddandeven
Copy link
Contributor

This PR enables monero wallet light jni implementation

@woodser
Copy link
Owner

woodser commented Jun 27, 2024

Hey there, how's it going? Was wondering if these PRs are in or near a stable state to be able to merge, with basic functionality?

@woodser
Copy link
Owner

woodser commented Aug 11, 2024

Just starting to test this with light wallet prs applied to monero-java, monero-cpp, and monero-project, but I'm getting a segfault in the full wallet running testCreateWalletRandomFull in TestMoneroWalletFull.java:

Stack: [0x000000016dbd8000,0x000000016dddb000],  sp=0x000000016ddd82d0,  free space=2048k
Native frames: (J=compiled Java code, j=interpreted, Vv=VM code, C=native code)
C  [libmonero-cpp.dylib+0x783110]  epee::wipeable_string::wipeable_string(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>> const&)+0x1c
C  [libmonero-cpp.dylib+0x25fffc]  monero::monero_wallet_full::move_to(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>> const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>> const&)+0x118
C  [libmonero-java.dylib+0x1c06c]  Java_monero_wallet_MoneroWalletFull_saveJni+0xdc
j  monero.wallet.MoneroWalletFull.saveJni()V+0
j  monero.wallet.MoneroWalletFull.save()V+5
j  utils.TestUtils.getWalletFull()Lmonero/wallet/MoneroWalletFull;+173
j  TestMoneroWalletFull.getTestWallet()Lmonero/wallet/MoneroWallet;+0
j  TestMoneroWalletCommon.beforeAll()V+2
j  TestMoneroWalletFull.beforeAll()V+1

The existing tests will need to continue working before I can start testing and reviewing.


// ------------------------ FULL WALLET INSTANCE METHODS --------------------------

JNIEXPORT jboolean JNICALL Java_monero_wallet_MoneroWalletLight_isViewOnlyJni(JNIEnv *env, jobject instance) {
Copy link
Owner

@woodser woodser Aug 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would really like to avoid duplicating the JNI bridge code for every light wallet function.

Is it possible to not override the native functions in MoneroWalletLight, and to call the existing JNI bridge functions without duplicating code?

@woodser
Copy link
Owner

woodser commented Sep 1, 2024

Hey, just letting you know that folks are becoming quite interested in your work. :)

They're discussing it in #monero-community-dev:monero.social. Hope to see you around soon!

@everoddandeven everoddandeven reopened this Sep 9, 2024
@woodser
Copy link
Owner

woodser commented Sep 21, 2024

I've tested your latest PRs and the segfault is no longer happening. I will test further.

Just wanted try to explain the build situation one more time.

For your PR to monero-project, monero-project/monero#9269, I think you should include monero-project/monero@886b470, which is a fix for macOS, and then add the changes from monero-project/monero@7b5213c in your commit, so there would 2 commits total in your PR.

Then your PR to monero-cpp, woodser/monero-cpp#58, needs updated with my changes from woodser/monero-cpp#69.

This will allow me to build the full stack on macOS with the latest API updates in monero-project.

@woodser
Copy link
Owner

woodser commented Oct 3, 2024

The pre-existing full wallet tests are passing with these PRs which is good progress.

I'm not able to build monero-lws at the moment due to a build issue: vtnerd/monero-lws#135

While we work to get that fixed in the meantime, one change I would really like to see to this PR is to not duplicate the implementations within monero_jni_bridge.cpp.

For example, Java_monero_wallet_MoneroWalletLight_getTxsJni and Java_monero_wallet_MoneroWalletFull_getTxsJni have duplicated implementations. Instead, they should call a common function for a single implementation.

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.

2 participants