chore: android support #2554

richard-ramos · 2024-03-25T22:54:14Z

Description

Adds android support

To test this:

export ANDROID_NDK_HOME=/path/to/Android/ndk/home
make libwaku-android

Architecures

amd64 (used in simulaor)
armeabi-v7a
arm64-v8a
x86

Changes

Update Makefile and waku.nimble
Crosscompile zerokit (TODO: copy scripts from go-zerokit-rln-*)
Fix unhandled exception: value out of range in getNanosecondTime for 32b architecures(arm and x86)
libnatpmp only compatible witth x86_64
libminiupnpc only compatible witth x86_64
libbacktrace only compatible with x86_64
Test makefile on macosx
Test against HelloWorld app in https://github.com/richard-ramos/gonimffi to make sure we're linking against required android libs
Setup CI (?)
~~Use static linking for zerokit RLN (might not be a good idea since the resulting lib might be too large?)~~. Decided against it. Resulting libraries are smalller this way and in the case of Android it's suggested that the app size should be < 100MB

github-actions · 2024-03-25T22:58:43Z

You can find the image built from this PR at

quay.io/wakuorg/nwaku-pr:2554-rln-v2-false

Built from f46c028

richard-ramos · 2024-03-26T20:31:36Z

I ended up disabling linking against libbacktrace when building for android.
@Ivansete-status, @arnetheduck:
How important is it to be able to crosscompile this library?

richard-ramos · 2024-03-27T01:43:01Z

While attempting to compile for i386, I ran into this error:

ld: error: relocation R_386_PC32 cannot be used against symbol 'sendnatpmprequest'; recompile with -fPIC
>>> defined in /home/richard/waku-org/nwaku/vendor/nim-nat-traversal/vendor/libnatpmp-upstream/libnatpmp.a(natpmp.o)
>>> referenced by natpmp.c
>>>               natpmp.o:(sendpublicaddressrequest) in archive /home/richard/waku-org/nwaku/vendor/nim-nat-traversal/vendor/libnatpmp-upstream/libnatpmp.a

Probably because we're linking against a .a, and building a .so? Will investigate further tomorrow

arnetheduck · 2024-03-27T05:09:14Z

While attempting to compile for i386, I ran into this error:

I've run into this before - the solution is to add -fPIC to the CFLAGS of both libnatpmp and libminiupnpc - this should be done for all platforms, not just i386 (it can happen on some linux distros that disable -fPIC for x86_64 platforms as well). There is no (sigificant) downside to compiling with -fPIC everywhere - ie on i386 there is a tiny performance overhead if you use -fPIC on object files that are used to produce an executable (as opposed to a shared library) but it doesn't greatly matter.

A static library (.a) is simply a collection of object files - if those object files are going to be used to create a shared library (.so), they need to be compiled with -fPIC.

Currently, both natpmp and miniupnp are built using their native build systems (cmake/make) - we've considered switching both so that nim builds them instead which "automatically" would take care of this problem and get rid of the need for cmake / make / etc in the build process significantly simplifying the whole setup - both for android and everyone else.

This is something we do for practically all other libraries, ie secp, bearssl and so on.

Ivansete-status · 2024-03-27T10:26:22Z

How important is it to be able to crosscompile this library?

I think this is interesting for debugging purposes. Maybe we could check what happens (which error appears) by forcing an unmanaged exception somewhere in the code

richard-ramos · 2024-03-27T20:25:01Z

Just tested this in the example mobile app i created the other day, and was able to link succesfully against libwaku + librln in android! :) , I just had to do some minimal changes in the example to remove calls to libhello, and use instead

            System.loadLibrary("rln")
            System.loadLibrary("waku")

Originally It failed to load the libraries because i forgot to link against -llog and -lrln so it was complaining about missing functions, but once that was fixed, the app opened, indicating that the shared libs were loaded succesfully.

Now the next steps would be to figure out how to call functions from these libraries, probably by using the work @Ivansete-status has done for the go bindings, by adding a gomobile step, since it would be similar to what we would do in status-go... or perhaps we should use jni? 🤔.

richard-ramos · 2024-04-28T16:30:19Z

library/libwaku.nim

@@ -2,7 +2,10 @@
 {.pragma: callback, cdecl, raises: [], gcsafe.}
 {.passc: "-fPIC".}

-import std/[json, sequtils, times, strformat, options, atomics, strutils]
+if defined(linux):
+  {.passl: "-Wl,-soname,libwaku.so"}


Required so the dynamic linker/loader can resolve symbols at runtime.

richard-ramos · 2024-04-28T16:33:11Z

library/libwaku.nim

@@ -39,6 +42,17 @@ const RET_MISSING_CALLBACK: cint = 2
 ################################################################################
 ### Not-exported components

+
+template foreignThreadGc(body: untyped) =


I use this template when executing the callbacks instead of having a single call. We need to ensure that Nim's GC can properly manage memory allocated by Nim code that might be running in other language threads.

richard-ramos · 2024-04-28T16:34:01Z

library/libwaku.nim

+if defined(android):
+  # Redirect chronicles to Android System logs
+  when compiles(defaultChroniclesStream.outputs[0].writer):
+    defaultChroniclesStream.outputs[0].writer =
+      proc (logLevel: LogLevel, msg: LogOutputStr) {.raises: [].} =
+        echo logLevel, msg


Without this, the logs for nwaku can't be seen unless we redirect somehow stdio to adb logcat

richard-ramos · 2024-04-28T16:36:44Z

waku/waku_core/time.nim

+proc getNanosecondTime*(timeInSeconds: int64): Timestamp =
+  let ns = Timestamp(timeInSeconds * int64(1_000_000_000))
+  return ns
+
+proc getNanosecondTime*(timeInSeconds: float64): Timestamp =
+  let ns = Timestamp(timeInSeconds * float64(1_000_000_000))
  return ns


For some reason proc getNanosecondTime*(timeInSeconds: int64 | float64): Timestamp didn't work in Android x86. I think in practice these two functions I defined here have the same behavior 🤷

richard-ramos · 2024-04-28T16:41:49Z

library/libwaku.nim

+proc waku_setup() {.dynlib, exportc.} =
+  NimMain()
+  if not initialized.load:
+    initialized.store(true)
+
+    # TODO: ask Ivan what is nimGC_setStackBottom for
+    when declared(nimGC_setStackBottom):
+      var locals {.volatile, noinit.}: pointer
+      locals = addr(locals)
+      nimGC_setStackBottom(locals)


I had to introduce this function waku_setup in the bindings.

When I tried to use the original waku_init function, Android crashed. Looks like NimMain() (at least on android) needs to be called before any nim logic is executed, because the code if not initialized.exchange(true): that existed before did fail. I imagine it might be related to the initialized atomic being setup by NimMain()?

Anyway, I think I heard/read somewhere that NimMain() function is not required in Nim 2.0? so perhaps on that version we can get rid of the existence of waku_setup then.

Also, I couldn't find docs for nimGC_setStackBottom. Is it really necessary?

I imagine it might be related to the initialized atomic being setup by NimMain()?

I think you are right

Also, I couldn't find docs for nimGC_setStackBottom. Is it really necessary?

I could get some interesting explanations from ChatGPT about nimGC_setStackBottom.
It should ideally be called at the beginning and is interesting to prevent stack issues, in async or multithreaded apps. By calling this function, we inform the GC about the stack boundaries.

richard-ramos · 2024-04-28T16:44:13Z

I think this is ready for review. (The PR is big enough as it is right now).
In separate PRs I'll do the following:

Add a default react native app (unmodified. So should be a simple PR)
Modify the default react native app to use libwaku
Compile libbacktrace for all the android archs
Setup CI?

Makefile

scripts/build_rln_android.sh

arnetheduck · 2024-05-09T17:07:56Z

this is really nice - does it make sense for any of it be ported back to nimbus-build-system or included in https://status-im.github.io/nim-style-guide/tooling.html ?

gabrielmer

Went over it again and looks amazing :))

Thanks so much!

richard-ramos · 2024-05-10T14:28:41Z

this is really nice - does it make sense for any of it be ported back to nimbus-build-system or included in https://status-im.github.io/nim-style-guide/tooling.html ?

In tooling.html might be a good idea to document the process followed to cross compile android using the ndk toolchain.

For porting back to nimbus-build-system, this part I'm not sure and would likely need your review to understand what can be extracted from waku repo to nimbus-build-system. The changes required for getting nwaku to run in android were few and limited to the Makefile and a config.nims file, as well as having a separate script for cross compiling RLN

I think a good candidate could be to improve the build process for the nat-libs, to not have to require this target:

rebuild-nat-libs: | clean-cross nat-libs

Which i had to add to delete the nat libs before building nwaku for android in the diff architectures, I'm thinking it should be possible to be able to specify the output dir of the libs somehow?

Ivansete-status

This is amazing ❤️ !

I just added some comments that I hope you find useful.

Not approving yet because few points to clarify:

is it needed the change in nim-chronos and nimbus-build-system?
We need to adapt the other examples so that the waku_setup is called at first. Or instead, try to avoid waku_setup. I will double-check if it is feasible to do so.

Congrats again for such wonderful PR 🥳

examples/mobile/android/app/src/main/jni/waku_ffi.c

Makefile

examples/mobile/android/app/src/debug/AndroidManifest.xml

examples/mobile/ios/mobile/AppDelegate.h

Ivansete-status · 2024-05-10T15:40:03Z

library/libwaku.nim

+proc waku_setup() {.dynlib, exportc.} =
+  NimMain()
+  if not initialized.load:
+    initialized.store(true)
+
+    # TODO: ask Ivan what is nimGC_setStackBottom for
+    when declared(nimGC_setStackBottom):
+      var locals {.volatile, noinit.}: pointer
+      locals = addr(locals)
+      nimGC_setStackBottom(locals)


I imagine it might be related to the initialized atomic being setup by NimMain()?

I think you are right

Also, I couldn't find docs for nimGC_setStackBottom. Is it really necessary?

I could get some interesting explanations from ChatGPT about nimGC_setStackBottom.
It should ideally be called at the beginning and is interesting to prevent stack issues, in async or multithreaded apps. By calling this function, we inform the GC about the stack boundaries.

richard-ramos · 2024-05-10T17:49:06Z

@Ivansete-status: is it needed the change in nim-chronos and nimbus-build-system?

Yes, it's because of some issues I ran into that were fixed in specific PRs (merged already)

nim-chronos
- Fix RangeError defect being happened using android toolchain. status-im/nim-chronos#538
nimbus-build-system
- Use -fPIC when building nat traversal status-im/nimbus-build-system#77
- allow overriding CC for libimiupnpc.a and libnatpmp.a targets status-im/nimbus-build-system#81

github-actions · 2024-05-14T13:40:43Z

You can find the image built from this PR at

quay.io/wakuorg/nwaku-pr:2554-rln-v2

Built from 79d6465

github-actions · 2024-05-14T13:40:47Z

You can find the image built from this PR at

quay.io/wakuorg/nwaku-pr:2554-rln-v1

Built from 79d6465

richard-ramos · 2024-05-14T19:16:07Z

@Ivansete-status I implemented all the suggested changes except by calling the waku_setup function in the examples. Let me know if you find a way to avoid having to call that function, otherwise, i'll do the changes on all the examples.

Ivansete-status · 2024-05-16T12:33:08Z

@Ivansete-status I implemented all the suggested changes except by calling the waku_setup function in the examples. Let me know if you find a way to avoid having to call that function, otherwise, i'll do the changes on all the examples.

Hey @richard-ramos ! I've been playing and learning from what you've done :)

I haven't found a way to avoid having the waku_setup proc. As you already mentioned, it always crashes with messages like the following: 05-16 11:46:09.675 24060 24697 F libc : Fatal signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 0x80 in tid 24697 (mqt_native_modu), pid 24060 (com.mobile)

Nevertheless, I think is fine to have the waku_setup proc and make the library user responsible for invoking it only once (as you implemented in the ReactNative example.)

Let me know if I can help to adapt the examples and include that new waku_setup proc.

Besides, it would be useful to define the following macros at the top of waku_ffi.c so that anyone could add some logs:

#define APPNAME "waku-jni"
#define LOGD(TAG) __android_log_print(ANDROID_LOG_DEBUG , APPNAME,TAG);

Then, the following can be added somewhere: LOGD("log example for debugging purposes...")

Ivansete-status

LGTM! Thanks for it! 💯 🥳
Congrats for such a great milestone that you've achieved!

Missing points:

We need to adapt the examples to use waku_setup
Make sure the CI pass correctly. Your changes shouldn't impact the CI but we are having some flaky tests on mac-os

chore: disable backtrace for android feat: crosscompile rln for android refactor: properly build amd64 and arm64 targets refactor: makefile and build for all android architectures in a single step

fix: link against llog, lm, and lrln fix: remove additional instances of waitFor and attempt to get signal to fire refactor: bindings

richard-ramos · 2024-05-21T18:35:44Z

@Ivansete-status I updated all the examples except the nodejs and python. Maybe later during the week we can pair program to add the waku_setup call? in the meantime, as soon as i get green lights i'll merge this PR since it's been open for a while and it's mostly complete :)

Ivansete-status · 2024-05-21T21:16:29Z

@Ivansete-status I updated all the examples except the nodejs and python. Maybe later during the week we can pair program to add the waku_setup call? in the meantime, as soon as i get green lights i'll merge this PR since it's been open for a while and it's mostly complete :)

Yes go ahead thanks!
We will update the examples next week ;) Feel free to send me an invite for next Thursday, the 30th, for example

Cheers

richard-ramos self-assigned this Mar 25, 2024

richard-ramos force-pushed the android branch 2 times, most recently from 95700de to fd380ad Compare March 27, 2024 19:35

richard-ramos force-pushed the android branch from c3456f9 to 496c639 Compare March 28, 2024 00:13

richard-ramos force-pushed the android branch 3 times, most recently from 8d32e3e to 786b4e3 Compare April 28, 2024 16:26

richard-ramos commented Apr 28, 2024

View reviewed changes

richard-ramos marked this pull request as ready for review April 28, 2024 16:44

richard-ramos requested review from Ivansete-status, gabrielmer and NagyZoltanPeter April 28, 2024 16:44

gabrielmer reviewed May 1, 2024

View reviewed changes

Makefile Outdated Show resolved Hide resolved

Makefile Outdated Show resolved Hide resolved

gabrielmer self-requested a review May 1, 2024 09:51

gabrielmer reviewed May 1, 2024

View reviewed changes

scripts/build_rln_android.sh Show resolved Hide resolved

gabrielmer self-requested a review May 3, 2024 08:11

richard-ramos force-pushed the android branch from 76583bb to 55855b8 Compare May 3, 2024 21:30

gabrielmer approved these changes May 10, 2024

View reviewed changes

Ivansete-status reviewed May 10, 2024

View reviewed changes

richard-ramos force-pushed the android branch 3 times, most recently from 33f2688 to d4777b8 Compare May 14, 2024 13:32

Ivansete-status approved these changes May 16, 2024

View reviewed changes

richard-ramos added 7 commits May 21, 2024 13:50

chore: android support

fcd212d

chore: disable backtrace for android feat: crosscompile rln for android refactor: properly build amd64 and arm64 targets refactor: makefile and build for all android architectures in a single step

fix: value out of range for 32b systems

6557ce8

fix: linking, refactor bindings, and usage of waitFor

0037963

fix: link against llog, lm, and lrln fix: remove additional instances of waitFor and attempt to get signal to fire refactor: bindings

chore: add default react native app to examples (#2641)

5a713f2

feat(mobile-android): waku example app (#2642)

c9827fc

fix: code review

405f87d

fix: examples

19af7c5

richard-ramos force-pushed the android branch from fc2e1da to 19af7c5 Compare May 21, 2024 18:35

richard-ramos merged commit 1e2aa57 into master May 22, 2024
6 of 15 checks passed

richard-ramos deleted the android branch May 22, 2024 01:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: android support #2554

chore: android support #2554

richard-ramos commented Mar 25, 2024 •

edited

Loading

github-actions bot commented Mar 25, 2024 •

edited

Loading

richard-ramos commented Mar 26, 2024

richard-ramos commented Mar 27, 2024 •

edited

Loading

arnetheduck commented Mar 27, 2024 •

edited

Loading

Ivansete-status commented Mar 27, 2024

richard-ramos commented Mar 27, 2024

richard-ramos Apr 28, 2024

richard-ramos Apr 28, 2024

richard-ramos Apr 28, 2024

richard-ramos Apr 28, 2024

richard-ramos Apr 28, 2024

Ivansete-status May 10, 2024

richard-ramos commented Apr 28, 2024

arnetheduck commented May 9, 2024

gabrielmer left a comment

richard-ramos commented May 10, 2024

Ivansete-status left a comment

Ivansete-status May 10, 2024

richard-ramos commented May 10, 2024

github-actions bot commented May 14, 2024 •

edited

Loading

github-actions bot commented May 14, 2024 •

edited

Loading

richard-ramos commented May 14, 2024

Ivansete-status commented May 16, 2024

Ivansete-status left a comment

richard-ramos commented May 21, 2024

Ivansete-status commented May 21, 2024

chore: android support #2554

chore: android support #2554

Conversation

richard-ramos commented Mar 25, 2024 • edited Loading

Description

Changes

github-actions bot commented Mar 25, 2024 • edited Loading

richard-ramos commented Mar 26, 2024

richard-ramos commented Mar 27, 2024 • edited Loading

arnetheduck commented Mar 27, 2024 • edited Loading

Ivansete-status commented Mar 27, 2024

richard-ramos commented Mar 27, 2024

richard-ramos Apr 28, 2024

Choose a reason for hiding this comment

richard-ramos Apr 28, 2024

Choose a reason for hiding this comment

richard-ramos Apr 28, 2024

Choose a reason for hiding this comment

richard-ramos Apr 28, 2024

Choose a reason for hiding this comment

richard-ramos Apr 28, 2024

Choose a reason for hiding this comment

Ivansete-status May 10, 2024

Choose a reason for hiding this comment

richard-ramos commented Apr 28, 2024

arnetheduck commented May 9, 2024

gabrielmer left a comment

Choose a reason for hiding this comment

richard-ramos commented May 10, 2024

Ivansete-status left a comment

Choose a reason for hiding this comment

Ivansete-status May 10, 2024

Choose a reason for hiding this comment

richard-ramos commented May 10, 2024

github-actions bot commented May 14, 2024 • edited Loading

github-actions bot commented May 14, 2024 • edited Loading

richard-ramos commented May 14, 2024

Ivansete-status commented May 16, 2024

Ivansete-status left a comment

Choose a reason for hiding this comment

richard-ramos commented May 21, 2024

Ivansete-status commented May 21, 2024

richard-ramos commented Mar 25, 2024 •

edited

Loading

github-actions bot commented Mar 25, 2024 •

edited

Loading

richard-ramos commented Mar 27, 2024 •

edited

Loading

arnetheduck commented Mar 27, 2024 •

edited

Loading

github-actions bot commented May 14, 2024 •

edited

Loading

github-actions bot commented May 14, 2024 •

edited

Loading