Helped @deme investigate a bug on the Nimbus side, which he fixed, meaning we now receive the correct public key in messages, and we no longer have to hardcode it.
Cleaned up the code on the Go side.
The console app is now fully functional on Nimbus as well as Geth, based solely on a -nimbus command line argument. There are a few remaining improvements to be done on the Nimbus side, and now the remaining major issue is managing the libnimbus.so dependency. Right now it’s being copied manually, but ideally we’d have some form of dependency management for it. One way to do it is with Nix, by building our branch of the Nim compiler, and then using that to build the Nimbus repository. That’s what I’ll be working on next.
Nimbus node is fully integrated in console app. PRs are in review. Nimbus API library is built by console client project using Nix expression from Nimbus repo.
Some unused APIs are not yet implemented (e.g. NewMessageFilter, SubscribeEnvelopeEvents, GetFilter, SendMessagesRequest, RequestHistoricMessagesWithTimeout, SyncMessages), however this PoC should be enough of a foundation to clarify how those will need to be implemented.
I’m considering the console PoC closed for now, since it demonstrates that the idea works and can be extended.
Next steps would be to integrate Nimbus in status-react (without envelope monitor or mailserver functionality):
generalize StatusNode to use facades over the actual node implementation (tricky since there are many supporting types from geth):
plug in call to nimbus_start in StatusNode.StartWithOptions (in this case, we’d not register mailserversService)
add missing configuration options to nimbus_start (e.g. bootstrap and static nodes, etc.)
implement RPC functionality between Nimbus and status-go and abstract it behind a facade
implement facade over service management API (gethNode.Service(serviceInstance))
implement support for service management in Nimbus bridge
bridge with respective services in Nimbus (wallet, ShhExt, accounts manager)
Add an argument to params.NodeConfig to determine the type of node to use (this would be passed from the UI in status-react).
Add UI in status-react to determine the ETH node type.
Met today with @adam and @andre to discuss the course of action for integrating status-go/status-react with Nimbus and the outcome was:
We will define a minimal use case in status-react that we want to support and work towards. In this case it’ll be:
Creating an in-device account
Adding a public chat
Sending and receiving a message in the public chat
Logging out
In order to start this effort, we’ll determine the methods in status-go/mobile/status.go called by status-react during a test run by logging them out.
Implement adapters for this high-level interface for Nimbus and possibly Geth (to validate approach). This could potentially be done in a simplistic new repo (acting as a drop-in status-go replacement) to avoid the huge surface area that would need to be taken into account in status-go.
I don’t think we can get away from creating an isolated project that will provide the abstraction over node implementations (Geth or Nimbus), as well as supporting types (e.g. Hash, HexBytes, Address, etc). This project would be imported as a module by status-protocol-go and status-go (so they wouldn’t need to e.g. include libnimbus.h).
This would probably mean a separate repo, like we have already for e.g. status-im/whisper.
The dependency tree would look something like this:
status-react
status-go
status-eth-node (NEW)
status-protocol-go
whisper
status-eth-node (NEW)
go-ethereum
whisper
nimbus
Some of the benefits:
Currently we have a build-nimbus.sh script in status-console-client. We’ll need to replicate this script into each Nimbus-consuming app. If we create status-eth-node, the script will live there instead of being replicated.
Only status-eth-node will know about Nimbus’ C interface (libnimbus.h and libnimbus.so), and will expose a Go interface to the consuming projects, making it easier to consume Nimbus. This will also allow us to implement some additional logic on top of the node that is guaranteed to be shared.
Currently some supporting types (e.g. Hash, HexBytes) live in status-protocol-go, even though they are not strictly protocol-related. status-eth-node would provide a better home for these types.
NOTE: @adam is currently investigating the feasibility of a status-go mono repo.
I think that’s correct. These structs from go-ethereum and various other helpers are shared throughout Status Go codebase and probably shouldn’t be decoupled at all. And I definitely like the idea of putting the Nim-C-Go interface in one package.
TLDR; we should be safe with using monorepo and submodules. In Go 1.13, this combination works correctly and there are techniques which will nicely solve a few identified edge cases.
The main point of using monorepo is providing a developer-friendly environment. Currently, we suffer from a long chain of dependencies. For example, a change in whisper may require a change in status-protocol-go and then finally a change in status-go. All these changes needs to be committed in order and pushed to remote repositories in order to be functionally tested.
Submodules solves a problem of versioning and providing an interface for clients. For example, status-protocol-go provides a programatic interface to interact with the Status community through messages.
status-eth-node is a submodule without dependencies
status-protocol is a submodule depending on status-eth-node
As you can use, I vendored dependencies of status-protocol to see what is there and there are only relevant files.
However, to download a submodule, the whole repo is cloned and additionally a submodule tag with a version needs to be created like status-eth-node/v1.0.0. Then, such a submodule can be required like this go get github.com/status-im/status-go/[email protected] where the repo is github.com/status-im/status-go.
There are two problems with that:
The submodule path is long.
Development is still difficult.
These two problems are solvable, though.
We should be able to use vanity imports so go.status.im/geth-node or go.status.im/protocol should work fine regardless whether they are submodules or not. Relevant topic is canonical import paths.
It’s possible to use relative imports in go.mods using replace directive. Here is an example of a test which require a submodule with relative redirect: https://github.com/status-im/status-go/blob/experiment/submodules/t/test-status-protocol/messenger_test.go (it requires github.com/status-im/status-go/status-protocol which has go.mod entry replace github.com/status-im/status-go/status-eth-node => ../status-eth-node) and the test passes fine.
That doesn’t follow. How does using Git submodules improve this situation? The repos still remain separate repos, they still need to be committed to, and the changes still have to be orchestrated together to take effect, all you changed is how they are imported, not how their changes are synced. Unless I’m missing something.
Created and merged a PR in status-console-client to bring it up to date with status-go
Will be investigating how we can consume Nimbus in status-react/status-go. The PoCs done so far consume libnimbus.so (dynamic module) instead of libnimbus.a (static module), and status-react is based on consuming status-go statically (libstatus.a), so it would be very helpful to be able to build status-go agains libnimbus.a. Right now the issue is with linker symbol clashes in the secp256k1 library which is imported both by Nimbus and status-go.
Managed to consume libnimbus.a in status-console-client by passing -extldflags=-Wl,--allow-multiple-definition" to go build. This sidesteps the issue for the time being, but we should ensure that the Nimbus version of secp256k1 is the same as the Go version, since there are no guarantees on which one gets picked by the linker.
Will now start adapting the status-go Nix recipes in status-react to build a Nimbus version. Among other things, this requires cross-compiling Nimbus for Android.
Managed to cross-compile Nimbus for Android (32 and 64-bit), but still running into a linker error due to architecture mismatch when linking libnimbus.a with status-go through gomobile:
/nix/store/30ph8xgdbskprmhymyc7gfrw7hy4436n-androidsdk/libexec/android-sdk/ndk-bundle/toolchains/llvm/prebuilt/linux-x86_64/bin/../lib/gcc/aarch64-linux-android/4.9.x/../../../../aarch64-linux-android/bin/ld: skipping incompatible go/src/github.com/status-im/status-go/vendor/github.com/status-im/status-go/eth-node/bridge/nimbus/libnimbus.a when searching for -lnimbus
Linking with the armv7 (32-bit) version of libnimbus.a when building with gomobile bind -target=android/arm yields a slightly different message:
# github.com/status-im/status-go/vendor/github.com/status-im/status-go/eth-node/bridge/nimbus
/nix/store/30ph8xgdbskprmhymyc7gfrw7hy4436n-androidsdk/libexec/android-sdk/ndk-bundle/toolchains/llvm/prebuilt/linux-x86_64/bin/../lib/gcc/arm-linux-androideabi/4.9.x/../../../../arm-linux-androideabi/bin/ld: fatal error: go/src/github.com/status-im/status-go/vendor/github.com/status-im/status-go/eth-node/bridge/nimbus/libnimbus.a(secp256k1.c.o): unsupported ELF machine number 0
clang: error: linker command failed with exit code 1 (use -v to see invocation)
Indeed, if I run objdump -f on it, I can see that secp256k1.c.o is the only object file which has unknown architecture and wrong file format:
In archive /nix/store/pn94h52cl42cn54mi6jj67cmdrbsj0vw-nimbus-0.0.1-android-armv7/lib/libnimbus.a:
secp256k1.c.o: file format elf32-little
architecture: UNKNOWN!, flags 0x00000011:
HAS_RELOC, HAS_SYMS
start address 0x00000000
stdlib_assertions.nim.c.o: file format elf32-littlearm
architecture: arm, flags 0x00000011:
HAS_RELOC, HAS_SYMS
start address 0x00000000
Also, it looks like Nix is using the wrong strip binary (from host binutils, instead of the one from the cross-compiler toolchain), still need to adapt that:
post-installation fixup
shrinking RPATHs of ELF executables and libraries in /nix/store/pn94h52cl42cn54mi6jj67cmdrbsj0vw-nimbus-0.0.1-android-armv7
strip is /nix/store/a9ms1pkj00sgssxqr2cvryv3h5fki4an-binutils-2.31.1/bin/strip
stripping (with command strip and flags -S) in /nix/store/pn94h52cl42cn54mi6jj67cmdrbsj0vw-nimbus-0.0.1-android-armv7/lib