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): first commit - waku relay (#1632) #1714

Merged
merged 11 commits into from
May 12, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -53,3 +53,6 @@ nimbus-build-system.paths
*.sqlite3
*.sqlite3-shm
*.sqlite3-wal

# Ignore autogenerated C-bingings compilation files
examples/v2/cbindings/cwakuv2.h
39 changes: 23 additions & 16 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -280,24 +280,31 @@ docker-push:
docker push $(DOCKER_IMAGE_NAME)


##############
## Wrappers ##
##############
# TODO: Remove unused target
libwaku.so: | build deps deps2
################
## C Bindings ##
################
.PHONY: cbindings cwakuv2example

libcwakuv2.a: | build deps
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should move away from always having to specify v2 for Waku. Waku v1 is deprecated, in a sense, and not used anywhere. We even want to remove Waku v1 code from this repo, so it becomes simpler if we start referring to Waku v2 simply as the Waku. In other words, I think this can be libwaku (just replacing the name occupied by previous unused target).

echo -e $(BUILD_MSG) "build/$@" && \
$(ENV_SCRIPT) nim libcwakuv2 $(NIM_PARAMS) $(EXPERIMENTAL_PARAMS) waku.nims && \
cp nimcache/release/cwakuv2/cwakuv2.h ./examples/v2/cbindings/

libcwakuv2.so: | build deps
# TODO: pending to enhance this part. Kindly use the static approach.
echo -e $(BUILD_MSG) "build/$@" && \
$(ENV_SCRIPT) nim c --app:lib --noMain --nimcache:nimcache/libwaku $(NIM_PARAMS) -o:build/$@.0 wrappers/libwaku.nim && \
rm -f build/$@ && \
ln -s $@.0 build/$@
$(ENV_SCRIPT) nim c --app:lib --opt:size --noMain --header -o:build/$@ library/cwakunode2.nim

# libraries for dynamic linking of non-Nim objects
EXTRA_LIBS_DYNAMIC := -L"$(CURDIR)/build" -lwaku -lm
cbindings: | build libcwakuv2.a

# TODO: Remove unused target
wrappers: | build deps librln libwaku.so
echo -e $(BUILD_MSG) "build/C_wrapper_example" && \
$(CC) wrappers/wrapper_example.c -Wl,-rpath,'$$ORIGIN' $(EXTRA_LIBS_DYNAMIC) -g -o build/C_wrapper_example
echo -e $(BUILD_MSG) "build/go_wrapper_example" && \
go build -ldflags "-linkmode external -extldflags '$(EXTRA_LIBS_DYNAMIC)'" -o build/go_wrapper_example wrappers/wrapper_example.go #wrappers/cfuncs.go
cwakuv2example: | build cbindings
echo -e $(BUILD_MSG) "build/$@" && \
cc -o build/cwakuv2example \
./examples/v2/cbindings/cwakuv2example.c \
-lcwakuv2 -Lbuild/ -pthread -ldl -lm \
-lminiupnpc -Lvendor/nim-nat-traversal/vendor/miniupnp/miniupnpc/build/ \
-lnatpmp -Lvendor/nim-nat-traversal/vendor/libnatpmp-upstream/ \
vendor/nim-libbacktrace/libbacktrace_wrapper.o \
vendor/nim-libbacktrace/install/usr/lib/libbacktrace.a

endif # "variables.mk" was not included
31 changes: 28 additions & 3 deletions apps/wakunode2/app.nim
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import
../../waku/v2/waku_enr,
../../waku/v2/waku_discv5,
../../waku/v2/waku_peer_exchange,
../../waku/v2/waku_relay/protocol,
../../waku/v2/waku_store,
../../waku/v2/waku_lightpush,
../../waku/v2/waku_filter,
Expand Down Expand Up @@ -66,7 +67,8 @@ logScope:
const git_version* {.strdefine.} = "n/a"

type
App* = object

App* = ref object
version: string
conf: WakuNodeConf

Expand Down Expand Up @@ -94,7 +96,16 @@ func version*(app: App): string =

## Initialisation

proc init*(T: type App, rng: ref HmacDrbgContext, conf: WakuNodeConf): T =
proc new*(T: type App,
rng: ref HmacDrbgContext = nil,
conf: WakuNodeConf = WakuNodeConf(
listenAddress: ValidIpAddress.init("127.0.0.1"),
rpcAddress: ValidIpAddress.init("127.0.0.1"),
restAddress: ValidIpAddress.init("127.0.0.1"),
metricsServerAddress: ValidIpAddress.init("127.0.0.1"),
nat: "any",
maxConnections: 50,
)): T =
App(version: git_version, conf: conf, rng: rng, node: nil)


Expand Down Expand Up @@ -125,7 +136,6 @@ proc setupDatabaseConnection(dbUrl: string): AppResult[Option[SqliteDatabase]] =

ok(some(connRes.value))


## Peer persistence

const PeerPersistenceDbUrl = "sqlite://peers.db"
Expand Down Expand Up @@ -547,6 +557,7 @@ proc setupProtocols(node: WakuNode, conf: WakuNodeConf,
peerExchangeHandler = some(handlePeerExchange)

if conf.relay:

let pubsubTopics = conf.topics.split(" ")
try:
await mountRelay(node, pubsubTopics, peerExchangeHandler = peerExchangeHandler)
Expand Down Expand Up @@ -726,6 +737,20 @@ proc startNode*(app: App): Future[AppResult[void]] {.async.} =
app.dynamicBootstrapNodes
)

proc subscribeCallbackToTopic*(app: App, pubSubTopic: cstring,
callback: PubsubRawHandler) {.gcsafe.} =
app.node.wakuRelay.subscribe(PubsubTopic($pubSubTopic), callback)

proc unsubscribeCallbackFromTopic*(app: App, pubSubTopic: cstring,
callback: PubsubRawHandler) {.gcsafe.} =
app.node.wakuRelay.unsubscribe(PubsubTopic($pubSubTopic), callback)

proc unsubscribeAllCallbackFromTopic*(app: App, pubSubTopic: cstring) {.gcsafe.} =
app.node.wakuRelay.unsubscribeAll(PubsubTopic($pubSubTopic))

proc publishMessage*(app: App, pubSubTopic: cstring, message: WakuMessage): Future[int] {.gcsafe, async.} =
# Returns the number of peers connected to the given pubSubTopic.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# Returns the number of peers connected to the given pubSubTopic.

return await app.node.wakuRelay.publish(PubsubTopic($pubSubTopic), message)

## Monitoring and external interfaces

Expand Down
10 changes: 6 additions & 4 deletions apps/wakunode2/config.nim
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ type
defaultValue: newSeq[ProtectedTopic](0)
name: "protected-topic" .}: seq[ProtectedTopic]


## Log configuration
logLevel* {.
desc: "Sets the log level for process. Supported levels: TRACE, DEBUG, INFO, NOTICE, WARN, ERROR or FATAL",
Expand All @@ -54,7 +53,6 @@ type
defaultValue: logging.LogFormat.TEXT,
name: "log-format" .}: logging.LogFormat


## General node config
agentString* {.
defaultValue: "nwaku",
Expand Down Expand Up @@ -570,14 +568,18 @@ proc readValue*(r: var EnvvarReader, value: var ProtectedTopic) {.raises: [Seria

{.push warning[ProveInit]: off.}

proc load*(T: type WakuNodeConf, version=""): ConfResult[T] =
proc load*(T: type WakuNodeConf,
version="",
configFile=""): ConfResult[T] =
try:
let conf = WakuNodeConf.load(
version=version,
secondarySources = proc (conf: WakuNodeConf, sources: auto) =
sources.addConfigFile(Envvar, InputFile("wakunode2"))

if conf.configFile.isSome():
if configFile != "":
sources.addConfigFile(Toml, InputFile(configFile))
elif conf.configFile.isSome():
sources.addConfigFile(Toml, conf.configFile.get())
)
ok(conf)
Expand Down
62 changes: 45 additions & 17 deletions apps/wakunode2/wakunode2.nim
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ else:

import
std/[options, strutils, os],
stew/results,
stew/shims/net as stewNet,
chronicles,
chronos,
Expand All @@ -13,30 +14,32 @@ import
system/ansi_c,
libp2p/crypto/crypto
import
../../waku/v2/waku_core/message/message,
../../waku/common/logging,
./config,
./app

logScope:
topics = "wakunode main"

const wakuNode2VersionString* = "version / git commit hash: " & git_version

{.pop.} # @TODO confutils.nim(775, 17) Error: can raise an unlisted exception: ref IOError
when isMainModule:
## Node setup happens in 6 phases:
## 1. Set up storage
## 2. Initialize node
## 3. Mount and initialize configured protocols
## 4. Start node and mounted protocols
## 5. Start monitoring tools and external interfaces
## 6. Setup graceful shutdown hooks
var wakunode2 {.threadvar.}: App
var confRes:ConfResult[WakuNodeConf]

const versionString = "version / git commit hash: " & app.git_version
{.pop.} # @TODO confutils.nim(775, 17) Error: can raise an unlisted exception: ref IOError
proc init*(configFilePath = "") =
Copy link
Member

Choose a reason for hiding this comment

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

I see init here uses a path vs the rfc which suggests a json string. We should decide which approach to use to unify behaviors between implementations. cc: @danielSanchezQ for his thoughts as he has worked with the rust+go bindings before

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we should init an app here, but operate on the WakuNode object as defined in waku_node.nim (see comments elsewhere). wakunode2 is simply an application written around the Nim API. As stated elsewhere, we want to provide C bindings wrapping the Nim API, not the wakunode2 application

Choose a reason for hiding this comment

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

I see init here uses a path vs the rfc which suggests a json string. We should decide which approach to use to unify behaviors between implementations. cc: @danielSanchezQ for his thoughts as he has worked with the rust+go bindings before

IMO using a configuration is better. A config file should be used for a binary, a configuration object for a library.

@jm-clius Where can I find the nim api documentation? Don't know if Im understanding correctly, but, does it mean that the wakunode2 is an external application running and the bindings just communicate with it. Or do this works in a similar way to go-waku in where we do embedded the node within the application?

Copy link
Contributor

Choose a reason for hiding this comment

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

The application simply provides a way to configure and instantiate the node, so I guess more similar to "embedded within the application". The Nim API documentation is sparse and outdated (since it's only used internally), but in theory lives here: https://github.com/waku-org/nwaku/blob/master/docs/api/v2/node.md

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, it's very outdated, in fact. 😟 @Ivansete-status perhaps a good idea that we clean the Nim API doc up at some point during the development of the C bindings, since we're likely to expand/change it in any case.

let rng = crypto.newRng()

let confRes = WakuNodeConf.load(version=versionString)
if confRes.isErr():
error "failure while loading the configuration", error=confRes.error
try:
confRes = WakuNodeConf.load(version=wakuNode2VersionString,
configFile=configFilePath)
if confRes.isErr():
error "failure while loading the configuration", error=confRes.error
quit(QuitFailure)

except ValueError:
error "Exception loading the configuration", error=getCurrentExceptionMsg()
quit(QuitFailure)

let conf = confRes.get()
Expand All @@ -50,8 +53,7 @@ when isMainModule:
logging.setupLogLevel(conf.logLevel)
logging.setupLogFormat(conf.logFormat, color)


var wakunode2 = App.init(rng, conf)
wakunode2 = App.new(rng, conf)

##############
# Node setup #
Expand Down Expand Up @@ -92,20 +94,19 @@ when isMainModule:
error "4/7 Mounting protocols failed", error=res5.error
quit(QuitFailure)

proc startNode*() =
Copy link
Contributor

Choose a reason for hiding this comment

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

Quite confused about this. Does this split the application's setup into an init and start phase?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Exaclty, that is a simple split. However that might not be needed, and I will likely revert it back, when I wrap the waku_node.nim library instead of the wakunode2 app.

debug "5/7 Starting node and mounted protocols"

let res6 = waitFor wakunode2.startNode()
if res6.isErr():
error "5/7 Starting node and protocols failed", error=res6.error
quit(QuitFailure)

debug "6/7 Starting monitoring and external interfaces"

let res7 = wakunode2.setupMonitoringAndExternalInterfaces()
if res7.isErr():
error "6/7 Starting monitoring and external interfaces failed", error=res7.error
quit(QuitFailure)

debug "7/7 Setting up shutdown hooks"
## Setup shutdown hooks for this process.
## Stop node gracefully on shutdown.
Expand Down Expand Up @@ -149,3 +150,30 @@ when isMainModule:
info "Node setup complete"

runForever()

proc subscribeCallbackToTopic*(pubSubTopic: cstring,
callback: proc(pubsubTopic: string, data: seq[byte]): Future[void] {.gcsafe, raises: [Defect].}) =
wakunode2.subscribeCallbackToTopic(pubSubTopic, callback)

proc unsubscribeCallbackFromTopic*(pubSubTopic: cstring,
callback: proc(pubsubTopic: string, data: seq[byte]): Future[void] {.gcsafe, raises: [Defect].}) =
wakunode2.unsubscribeCallbackFromTopic(pubSubTopic, callback)

proc unsubscribeAllCallbacksFromTopic*(pubSubTopic: cstring) =
wakunode2.unsubscribeAllCallbackFromTopic(pubSubTopic)

proc publishMessage*(pubSubTopic: cstring, message: WakuMessage): Future[int] {.gcsafe, async.} =
return await wakunode2.publishMessage(pubSubTopic, message)
Copy link
Contributor

Choose a reason for hiding this comment

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

The way I see it, these do not belong here. The C bindings should be a wrapper around the Nim API defined in waku_node.nim. As such the app modules should remain unchanged and preferably unimported elsewhere. Any additional API functionality should go to waku_node.nim. Of course, since we need a configured WakuNode object for those bindings, we may have to provide App-like functionality when initiating the node. For now, though I think the configuration will be much, much simpler than is needed for a wakunode2 application and providing the basic configuration in a JsonConfig via waku_new() will be enough.



when isMainModule:
## Node setup happens in 6 phases:
## 1. Set up storage
## 2. Initialize node
## 3. Mount and initialize configured protocols
## 4. Start node and mounted protocols
## 5. Start monitoring tools and external interfaces
## 6. Setup graceful shutdown hooks

init()
startNode()
Loading