Skip to content

Chore add api interfaces, phase3#3975

Closed
NagyZoltanPeter wants to merge 17 commits into
masterfrom
chore-add-api-interfaces
Closed

Chore add api interfaces, phase3#3975
NagyZoltanPeter wants to merge 17 commits into
masterfrom
chore-add-api-interfaces

Conversation

@NagyZoltanPeter

Copy link
Copy Markdown
Contributor

Description

Based on the proposed shape of APIs from #3974
This PR suggest to introduce interface definitions for all three api layers.

The benefit that it adds, easier read what is intended to be public really and user/dev facing of any layers.
This also allows us to extract types, structures, and definitions into the highest level that represent the data layer of our APIs.

Changes

IKernel, IMessagingClient and IRealiableChannelManager abstract classes added at logose_delivery/api/

Issue

closes #

@NagyZoltanPeter NagyZoltanPeter self-assigned this Jun 23, 2026
@NagyZoltanPeter
NagyZoltanPeter changed the base branch from master to chore-api-shape-phase2 June 23, 2026 22:11
@github-actions

github-actions Bot commented Jun 23, 2026

Copy link
Copy Markdown

You can find the image built from this PR at

quay.io/wakuorg/nwaku-pr:3975

Built from 1c3e5c1

@NagyZoltanPeter
NagyZoltanPeter marked this pull request as ready for review June 24, 2026 13:22
@Ivansete-status
Ivansete-status requested a review from fcecin June 24, 2026 13:22
Base automatically changed from chore-api-shape-phase2 to master June 25, 2026 07:27
@Ivansete-status

Copy link
Copy Markdown
Collaborator

Thanks for the PR!
I really think we can simplify the approach. Defining the interfaces this way adds a layer of indirection that the solution doesn't really need. We can instead have each layer define its own concrete API modules directly. That keeps the design simpler.

@NagyZoltanPeter
NagyZoltanPeter force-pushed the chore-add-api-interfaces branch from e6c6149 to 8f9ddc8 Compare June 25, 2026 08:04
@NagyZoltanPeter

Copy link
Copy Markdown
Contributor Author

Thanks for the PR! I really think we can simplify the approach. Defining the interfaces this way adds a layer of indirection that the solution doesn't really need. We can instead have each layer define its own concrete API modules directly. That keeps the design simpler.

I think having such indirection can help keep API clean. Detaching from the implementation files helps easier understanding the flow because the reader should not go through implementation details unless he/she wants to.
I'm just advocating the having interface class articulates our intent as lib developers.
I know it looks just semantic, but helps us in long term with its constraint what you can do and what should not.
Also by leveling surfacing types out of implementation also gives us cleaner dependencies in the lower layers.

@fcecin

fcecin commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

We need some sort of facade solution to major library boundaries (layers are equivalent to independent libraries -- that's why we call them layers). We should not expose function someApiName { lotsOfLinesOfLogicHere } in a single file.

We already suffered this before: directories, files, procs, structures named "API" that are not "I" at all, they carry implementation logic, that is not split between the "API" (not) folder and the thing it is dispatching to. And then it starts growing the "local" dispatch logic.

The nice thing about proper interfaces (however they are implemented) is that they read as lists of things you can do, without interspersing the names of the things you can call with a bunch of irrelevant stuff that doesn't even solve the whole problem (all they do is force your logic to be split in at least two places for no reason). In C++ that would be an "interface" header file where all the "dispatch" methods are inlined; we don't do that.

If we don't go with the tried-and-true ISomething / Something Java/OOP classic here:

  • The bare minimum is that we need some strict discipline on top of "list of flat procs." E.g. each proc is only allowed to have one line in the implementation, which is to delegate to some static (never nil) internal component. No if pointer not nil, if lifetime state, if config equals. Just call something else.
  • I would vote against "manual" function tables, since that just restates what is proposed here in this PR, and by nim-brokers (POC: structured api interface  #3946 / Separate core (Waku) and MessagingClient using nim-brokers #3904) for that matter.
  • Nim concepts could do it, but to speedrun this now doesn't fit the risk tolerance profile/pattern we are already displaying against the other stuff we tried so my guess would be that we don't want this now.

NagyZoltanPeter added a commit that referenced this pull request Jun 30, 2026
* Reshape per-layer API into api/ folders and thin the FFI over them

Each layer now separates its constructible core from its public surface:

  - core module (waku.nim / messaging_client.nim /
    reliable_channel_manager.nim): the type plus new/start/stop and the
    private construction helpers.
  - api/ folder: one module per differentiated set of operations
    (waku: topics/relay/filter/lightpush/store/peer_manager/discovery/
    debug/health) plus an events surface.

The waku api is reshaped to be the complete operation surface the C
bindings need, so the library no longer reaches into node internals:
relayPublish returns the message hash, relaySubscribe takes an optional
handler, filter/lightpush auto-select the service peer, connectedPeersInfo
returns structured data, pingPeer honours the timeout, plus
relayNumPeersInMesh / relayNumConnectedPeers / isOnline. library/ is now a
thin C-ABI shim: each {.ffi.} proc only marshals cstring/JSON/callbacks and
delegates to ctx.myLib[].waku.<op> (or messagingClient.<op>).
app_callbacks re-exports the modules defining its handler types, which the
included FFI files previously relied on by leakage.

Events move next to the surface that owns them, with each dependency kept
pointing the right way:

  - waku/events/ relocated under waku/api/events/.
  - channel events live in channels/api/events.nim.
  - the four messaging-level message events move to messaging/api/events;
    MessageSeenEvent stays in waku because it is emitted by waku core, so
    moving it would make waku depend on the messaging layer.
  - delivery_events renamed to filter_subscribe_events to match the
    OnFilterSubscribe/Unsubscribe events it actually declares.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* Add reliable-channel FFI ops + events (nim-ffi v0.1.3)

Expose the reliable-channel layer through the v0.1.3 FFI:
- channel_create / channel_send / channel_close call the
  ReliableChannelManager api (createReliableChannel / send / closeChannel),
  marshalling channel id + base64 payload + ephemeral by hand
- channel message received / sent / errored are surfaced by listening to the
  channel-layer broker events in start_node and forwarding them through
  callEventCallback (received payload base64-encoded), dropped in stop_node

Stays on nim-ffi v0.1.3 (no typed/CBOR rewrite).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* Expose reliable-channel ops in the stable C header (#3851)

The library already ships as a single .so with a tiered header surface
(liblogosdelivery.h = stable Messaging/Reliable-Channels, liblogosdelivery_kernel.h
= advanced Kernel). Per that tiering, the reliable-channel ops belong on the
stable surface, so declare channel_create / channel_send / channel_close in
liblogosdelivery.h and document the channel lifecycle events delivered through
the event callback.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* Graft PR#3975 interface layer onto decomposed foundation (events deduped)

Add IKernel/IMessagingClient/IReliableChannelManager/ILogosDelivery interface
classes under logos_delivery/api/. The EventBroker types PR#3975 hoisted into
these files already exist in PR#3989's decomposed */api/events/ modules, so the
interface files re-export those modules instead of redefining the types
(avoids 8 duplicate EventBroker definitions). api/types.nim kept at the
foundation version (ChannelId stays in channels/types.nim, which the decomposed
modules import).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* Wire impl classes to interfaces (inherit; relocate SendHandler)

- Waku : IKernel, MessagingClient : IMessagingClient,
  ReliableChannelManager : IReliableChannelManager.
- The operation procs already live in PR#3989's decomposed */api/ modules and
  stay as plain procs (nothing dispatches through the interface types, so no
  method-ization is needed).
- SendHandler now lives in reliable_channel_manager_api.nim (its PR#3975 home);
  removed the duplicate from reliable_channel.nim, which re-exports the
  interface module so channels/api/{channel_lifecycle,send} still see it.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* Wire LogosDelivery to ILogosDelivery orchestrator interface

LogosDelivery : ILogosDelivery; start/stop/isOnline become method overrides.
Peripheral PR#3975 edits (lightpush/store clients, self_req_handlers,
statistics) are import-reorg artifacts of deleting waku/utils/requests.nim,
which the decomposed structure keeps -- so they are intentionally not ported.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* Dedup EventConnectionStatusChange (re-export from health_events)

9th duplicate EventBroker type: defined in both logos_delivery_api.nim and the
decomposed waku/api/events/health_events.nim. The interface file now re-exports
it. liblogosdelivery builds clean.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* Move events back into interface-class source files (restore #3975 placement)

Reverses the earlier dedup-by-re-export: event TYPE definitions now live in the
interface classes, and the emptied decomposed event files are removed.

- MessageSeenEvent            -> logos_delivery/api/kernel_api.nim
- Message{Sent,Error,Propagated,Received}Event -> api/messaging_client_api.nim
- ChannelMessage{Received,Sent,Error}Event     -> api/reliable_channel_manager_api.nim
- EventConnectionStatusChange -> api/logos_delivery_api.nim

Deleted (became empty after the move):
- logos_delivery/waku/api/events/message_events.nim
- logos_delivery/messaging/api/events.nim
- logos_delivery/channels/api/events.nim
health_events.nim keeps its two remaining events (content/shard topic health).

Rewiring: each layer re-exports its interface module (waku->kernel_api,
messaging_client->messaging_client_api, reliable_channel->reliable_channel_manager_api,
which also re-exports messaging_client_api). Deep emitters/listeners
(subscription_manager, waku_node, waku_node/relay, node_health_monitor,
recv_service, send_service) import the owning interface module directly.
kernel_api stays below node level (types/topics/message/store-common) so the
node->kernel_api imports are acyclic. liblogosdelivery builds.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* nph formatting

---------

Co-authored-by: Ivan FB <ivansete@status.im>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Ivansete-status pushed a commit that referenced this pull request Jun 30, 2026
…cement)

Reverses the earlier dedup-by-re-export: event TYPE definitions now live in the
interface classes, and the emptied decomposed event files are removed.

- MessageSeenEvent            -> logos_delivery/api/kernel_api.nim
- Message{Sent,Error,Propagated,Received}Event -> api/messaging_client_api.nim
- ChannelMessage{Received,Sent,Error}Event     -> api/reliable_channel_manager_api.nim
- EventConnectionStatusChange -> api/logos_delivery_api.nim

Deleted (became empty after the move):
- logos_delivery/waku/api/events/message_events.nim
- logos_delivery/messaging/api/events.nim
- logos_delivery/channels/api/events.nim
health_events.nim keeps its two remaining events (content/shard topic health).

Rewiring: each layer re-exports its interface module (waku->kernel_api,
messaging_client->messaging_client_api, reliable_channel->reliable_channel_manager_api,
which also re-exports messaging_client_api). Deep emitters/listeners
(subscription_manager, waku_node, waku_node/relay, node_health_monitor,
recv_service, send_service) import the owning interface module directly.
kernel_api stays below node level (types/topics/message/store-common) so the
node->kernel_api imports are acyclic. liblogosdelivery builds.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@NagyZoltanPeter

Copy link
Copy Markdown
Contributor Author

Closing as of #4001 defines the API layer with concepts.

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.

3 participants