monero-java icon indicating copy to clipboard operation
monero-java copied to clipboard

MoneroWalletLight implementation

Open everoddandeven opened this issue 1 year ago • 5 comments

This PR enables monero wallet light jni implementation

everoddandeven avatar Mar 29 '24 23:03 everoddandeven

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 avatar Jun 27 '24 14:06 woodser

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.

woodser avatar Aug 11 '24 13:08 woodser

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!

woodser avatar Sep 01 '24 13:09 woodser

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, https://github.com/monero-project/monero/pull/9269, I think you should include https://github.com/monero-project/monero/pull/9487/commits/886b47048cf428d30b4b93f846a0ab43b0b500ff, which is a fix for macOS, and then add the changes from https://github.com/monero-project/monero/pull/9487/commits/7b5213c0079a0866d1d20798c6703fc271f484b3 in your commit, so there would 2 commits total in your PR.

Then your PR to monero-cpp, https://github.com/woodser/monero-cpp/pull/58, needs updated with my changes from https://github.com/woodser/monero-cpp/pull/69.

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

woodser avatar Sep 21 '24 12:09 woodser

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: https://github.com/vtnerd/monero-lws/issues/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.

woodser avatar Oct 03 '24 14:10 woodser