chore: some suggestions for waku api #66

Ivansete-status · 2025-06-19T09:08:31Z

This just serves as a simple way to share ideas about waku-api

fryorcraken · 2025-06-24T02:25:19Z

standards/application/waku-api.md


 - No `default` means that the value is mandatory
- Primitive types are `string`, `int`, `bool` and `uint`
+- Primitive types are `string`, `int`, `bool`, `uint`, and `pointer`


I would disagree with pointer because this is now very implementation dependent and idiomatic to the language.

Also let's review if we really need it.

The pointer type is needed because it is the type returned by waku_new, which return a raw memory reference to the created Waku context.

EDITED: Double-thinking about that, if we consider a higher API level, then the pointer type is not needed, indeed

standards/application/waku-api.md

fryorcraken · 2025-06-24T02:37:03Z

standards/application/waku-api.md

+          static_servers:
            type: array<string>
            default: []
-            description: "Only the passed nodes are used for store queries, discovered store nodes are discarded."
+            description: "Array of server nodes' multiaddresses. Discovered nodes may also be considered."
+            examples:
+              ex1: ["/dns4/node-01.do-ams3.waku.sandbox.status.im/tcp/30303/p2p/16Uiu2HAmNaeL4p3WEYzC9mgXBmBWSgWjPHRvatZTXnp8Jgv3iKsb"]
+              ex2: ["/dns4/node-01.do-ams3.waku.sandbox.status.im/tcp/8000/wss/p2p/16Uiu2HAmNaeL4p3WEYzC9mgXBmBWSgWjPHRvatZTXnp8Jgv3iKsb"]


What is the difference with bootstrap nodes?

The bootstrap node represents a super thin node which doesn't perform any Waku protocol action. It is just a discovery facilitator.

In turn, a static_server (or static_node), is a fixed well-known node that can participate on any Waku protocol (relay, store, etc.)

Oh! No servers in Waku, please. 😝 Where we treat nodes as server-like, it's a temporary error.

Update: I notice this has been discussed in another comment. :)

I would argue that we do not want static servers.
The only "static" service node we need is store, so we can temporarily have that in there, to be removed once we are able to trust that store node can be discovered and used in a more decentralised manner.

standards/application/waku-api.md

fryorcraken · 2025-06-24T02:38:24Z

standards/application/waku-api.md

-            description: "The auto-sharding config, if sharding mode is `auto`"
-          active_relay_shards:
+            description: "The cluster subscribed to and participating in. A cluster is composed by multiple shards."
+          shards:


You seem to have ignored the original PR. The concept of shards in a configuration is very ambiguous. Are we talkin about shards available in the cluster? are we talking about shards for relay to subscribe to?

You seem to have ignored the original PR

Sorry if I gave that impression. I might have overlooked something but not ignore ofc :)

Are we talkin about shards available in the cluster? are we talking about shards for relay to subscribe to?

I was thinking about the shards available in the cluster, which is a superset of the shards for relay to subscribe to

The shards in the clusters are defined with numShardsInCluster when auto-sharding is used.
Not needed for static sharding.

In terms of shard subscription, I have reviewed after more code analysis and I would suggest to remove from init config: 25984c0

fryorcraken · 2025-06-24T02:38:56Z

standards/application/waku-api.md

+      - 3: When the waku node is blocked in a certain callback, or due to other reason, for too long.
+
+  WakuCallback:
+    type: pointer


This is a function before being a pointer.

Every function is a pointer, isn't it :) ?

fryorcraken · 2025-06-24T02:44:03Z

standards/application/waku-api.md

        - name: config
          type: Config
-          description: "The Waku node configuration."
+          description: "The Waku configuration."


I am not sure to understand the subtle changes you are proposing.

I am not sure to understand the subtle changes you are proposing.

Yes, from nwaku PoV, we are talking about Waku instances, not waku nodes.

Waku is composed by:

waku node

health monitor

discovery service

In turn, waku node is just a libp2p switch and a set of libp2p protocols.

In the waku api we are exposing Waku instances and not just waku nodes. Hence, I think is more accurate to only use the waku term.

From a user point of view, they indeed want an instance of a waku node.

How is the discovery service not part of a waku node?

How is the discovery service not part of a waku node?

The above is the design decision that is being followed in nwaku.
For example, discv5 represents a different network than GossipSub and hence it is handled at higher layer, i.e. Waku instance.

I think @jm-clius can shed more light as to why we consider that: waku node is just a libp2p switch and a set of libp2p protocols

I think @jm-clius can shed more light as to why we consider that: waku node is just a libp2p switch and a set of libp2p protocols

This is just the internal view to properly separate concerns and initialisation. The WakuNode object (in Nim) wraps a libp2p switch and mounts various libp2p protocols onto this switch. Composing the various discovery models, startup/config logic, etc. into this object caused many circular references and complex code. In other words, it's an implementation detail. However, externally the compiled library and API is still seen as a single, cohesive thing - a "Waku Node" in other words.

After spending more time in the code, I would suggest that from an API PoV, A "Waku Node" is:

libp2p switch and related protocols implementations

discv5 service (other discovery services are libp2p protocols)

health monitor

metrics

Then, as per your previous comment around the REST API @Ivansete-status, I would say that the REST API service is not part of a Waku node, and instead should be seen as a external library that one may want to use with a Waku node implementation.

Which makes it very interesting because in terms of clean up for the REST API (which we have not scheduled yet), it means that will be force to only expose on the REST API, what is exposed by nwaku, and the REST API will not be able to access "internal", allowing for a better design. cc @NagyZoltanPeter

fryorcraken · 2025-06-24T02:46:52Z

standards/application/waku-api.md

+          description: "The Waku configuration."
+        - name: callback
+          type: WakuCallback
+          description: Gives feedback in case of failure when invoking the function.


If there is a failure when evoking the function then an error should be returned. Why using a callback?

The callback is needed to give richer feedback to the API consumer

Can you please expand? a result pattern is simpler and preferred when doing a call that can return ONE error.

The object you can return in a callback or in a result is the same. how is it richer?

The main reason why we use callback to return richer feedback is to avoid having memory leaks, i.e., every memory allocated within the libwaku should be deallocated within libwaku itself. We cannot have library functions returning complex structures. It just return int.

// The possible returned values for the functions that return int #define RET_OK 0 #define RET_ERR 1 #define RET_MISSING_CALLBACK 2

Notice though, that this suggestion considers a very low C level.

Therefore, I think you are absolutely right about the Result pattern and I believe we can apply it at higher API levels.

In fact, we are applying it in waku-rust-bindings. See for example: https://github.com/waku-org/waku-rust-bindings/blob/master/waku-bindings/src/node/management.rs

This is definitely the challenge here, is to find a common way of phrasing the API and leaving space for language specific/idiomatic solutions.

Which is also why I am spearheading this effort, as I am familiar with js-waku, getting familiar with nim/nwaku, and also familiar with Rust and C(++) :)

standards/application/waku-api.md

fryorcraken · 2025-06-30T04:38:30Z

@Ivansete-status feel free to update this PR as per comment and keep the suggestion you'd like to see upstream.

Please keep the scope narrow focusing on the init function and the related configuration. Let's have other functions out of scope for now,

some suggestions

d82bd5c

Ivansete-status marked this pull request as draft June 19, 2025 09:08

Ivansete-status requested a review from fryorcraken June 19, 2025 09:43

fryorcraken reviewed Jun 24, 2025

View reviewed changes

standards/application/waku-api.md Show resolved Hide resolved

fryorcraken reviewed Jun 24, 2025

View reviewed changes

standards/application/waku-api.md Show resolved Hide resolved

Base automatically changed from f/waku-api-custom-idl to master October 1, 2025 06:32

chore: some suggestions for waku api #66

Are you sure you want to change the base?

chore: some suggestions for waku api #66

Uh oh!

Conversation

Ivansete-status commented Jun 19, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Ivansete-status Jun 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Ivansete-status Jun 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jm-clius Jun 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

fryorcraken commented Jun 30, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Ivansete-status Jun 27, 2025 •

edited

Loading

Ivansete-status Jun 27, 2025 •

edited

Loading

jm-clius Jun 27, 2025 •

edited

Loading