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(cbindings): adding tiny Waku Relay example in Golang #1794

Closed
wants to merge 4 commits into from

Conversation

Ivansete-status
Copy link
Collaborator

Description

Nwaku integration library (libwaku.so) in Golang.

Changes

  • Adding new waku.go that shows an example on how to integrate nwaku (libwaku.so) in Golang.
  • Adding README.md file to explain how to use the example.

How to test

Golang should be installed beforehand. Tested with go version go1.20.5 linux/amd64.

  1. Open one terminal from the repo's root folder.
  •   make libwaku
    
  • export LD_LIBRARY_PATH=build
    
  • go run examples/golang/waku.go -key 364d111d729a6eb6d2e6113e163f017b5ef03a6f94c9b5b7bb1bb36fa5cb07a9
    
  1. Open a second terminal from the repo's root folder.
  • export LD_LIBRARY_PATH=build
    
  • ./build/wakunode2 --config-file=cfg_node_b.txt
    

cfg_node_b.txt

  1. Open a third terminal from the repo's root folder.
  • while [ true ]; do curl -d "{\"jsonrpc\":\"2.0\",\"id\":"$(date +%s)",\"method\":\"post_waku_v2_relay_v1_message\", \"params\":[\"/waku/2/default-waku/proto\", {\"timestamp\":"$(date +%s)", \"payload\":\"VGhpcyBpcyBhIG1lc3NhZ2UK\"}]}" --header "Content-Type: application/json" http://localhost:8546; done
    

From the first terminal (Golang), it should appear perioric logs like:

Event received: {"pubsubTopic":"/waku/2/default-waku/proto","messageId":"TODO","wakuMessage":{"payload":"This is a message\n","contentTopic":"/waku/2/default-content/proto","version":0,"timestamp":1686589489},"eventType":"message"}

Issue

#1632

@Ivansete-status Ivansete-status self-assigned this Jun 12, 2023
@Ivansete-status Ivansete-status marked this pull request as ready for review June 13, 2023 08:22

#include "libwaku.h"

static NimStringDesc wakuString;
Copy link
Contributor

Choose a reason for hiding this comment

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

the nim string layout is not stable between Nim versions and it is part of the garbage collected set of types - throughout, we should use cstring for FFI instead

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Morning and thanks for the comment @arnetheduck !

The thing is that, for example, the next Nim C-exposed proc:

proc waku_new(config: ConfigNode,
              jsonResp: var string): bool
              {.dynlib, exportc.} =
...

Is automatically translated to the next when creating the libwaku.so library (nim c --app:lib ...)

N_LIB_IMPORT N_CDECL(NIM_BOOL, waku_new)(ConfigNode* config, NimStringDesc** jsonResp);

And the jsonResp variable should be a var string. This approach is followed by all the functions that need to return a variable amount of data out of the library. And this is achieved by allocating the "string" variable outside the library and then offering it to the library to get populated from within it.

What would be the best approach to do the same but using cstring instead?
Also, I want to avoid allocating memory inside the library.

Copy link
Contributor

Choose a reason for hiding this comment

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

You would use the same approach but use jsonResp: cstring (or, more likely, jsonResp: ptr cchar) - there's also the question of how to communicate the length: this would again be a separate argument, ie jsonResp: ptr cchar, jsonRespLen: csize_t).

Is automatically translated to the next when creating the libwaku.so library (nim c --app:lib ...)

this may sound convenient initially, but it is a problem for the aforementioned reason: between Nim versions this declaration will not be stable. For example, depending on what ConfigNode is and its size, it may be trnslated to a pointer or a value and you don't control which.

For C FFI, ConfigNode should be an object (not a ref object) and the exported API declaration would be config: ptr ConfigNode to indicate that it must be passed by pointer.

typedef N_CDECL_PTR(void, tyProc__5cp59bim9aJ4WupX5aVaD1Sg) (NCSTRING signal_0);
N_LIB_PRIVATE N_NOCONV(void, signalHandler)(int sign);
N_LIB_PRIVATE N_NIMCALL(NI, getRefcount)(void* p);
N_LIB_IMPORT N_CDECL(NIM_BOOL, waku_new)(ConfigNode* config, NimStringDesc** jsonResp);
Copy link
Contributor

Choose a reason for hiding this comment

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

instead of defining the API in terms of nim types, it's generally better to declare it in terms of c types in the nim code (ie cint, cstring etc) then import these "standard" types into the other languages

Copy link
Contributor

Choose a reason for hiding this comment

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

this is for the same reason as below, ie the generated Nim code is not ABI-stable this way except certain parts

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for the comments @arnetheduck !

All of these variables get translated into C types thanks to the nimbase.h header (included in libwaku.h.)

nimbase.h brings the next definitions:

  • # define N_LIB_IMPORT extern
  • # define N_LIB_PRIVATE __attribute__((visibility("hidden")))
  • # define N_CDECL(rettype, name) rettype name
  • #define NIM_BOOL _Bool, where _Bool is the C99 standard representation of a boolean.
  • typedef NI64 NI; -> typedef int64_t NI64; -> ··· -> typedef signed long int __int64_t;

Personally, given that the libwaku.h file is auto-generated by the Nim compiler, I wouldn't touch it.

Of course, I'm open to any other option that suits better from your point of view.
Kindly let me know if the above definitions are good enough :)

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, but please understand: nimbase.h changes between Nim versions: if you compile this code with nim 1.6 and nim 2.0, you will not get the same result: you will get a different, incompatible, ABI: the result of this is that the consumer of the API will need to adjust as well: when they import the code in their code, their types will change depending on which version of Nim the code was compiled with. This is highly undesireable.

The above point is why all languages tend to use C as the "lingua franca": developers have informally agreed that plain C types are good enough for this purpose, so the "pipeline" to consume something in one language from another is that one language translates its own native types to C types and the other language does the same journey in reverse.

NimStringDesc is a good example: it's no longer part of nim 2.0: it has been replaced by a different type - NimStringDesc, NIM_BOOL and so on, as their names suggest, are internal implementation details of the nim compiler - they are not meant to be exposed to other languages or outside of Nim.

Copy link
Contributor

Choose a reason for hiding this comment

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

keep in mind that exporting code from Nim is only half of the story: the code also needs to be imported on the other side.

When importing code on the other side, this is done by translating C types seen in libwaku.h to "native" types: if, between nim versions, NIM_BOOL changes from int to char (this is a valid change for Nim to make), the import into the target language also must change - there's no reason this should be necessary: the aim of a "C bindings" module is to remove this variability by explicitly using "C FFI" types that exist for this purpose on the Nim side.

@Ivansete-status
Copy link
Collaborator Author

I close this PR as it doesn't make sense to maintain this example given that we have a much better and mature go-waku implementation ( @richard-ramos .)

I created it initially as an exercise to try the nim-libwaku integration in Golang, and check if we had similar issues that we were facing in NodeJs. This PR brought a very interesting discussion with @arnetheduck, which enhanced the nim-libwaku implementation.

If this is needed for any reason in the future, I'll re-open it again.
Thanks all.

@Ivansete-status Ivansete-status deleted the feat-1632-relay-golang branch September 21, 2023 10:22
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