-
Notifications
You must be signed in to change notification settings - Fork 78
feat: waku api send #3669
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: waku api send #3669
Conversation
Ivansete-status
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome PR! Thanks so much for it! 🙌
Just adding some nitpicks that I hope you find useful.
| let healthStatus = RequestNodeHealth.request(w.brokerCtx) | ||
|
|
||
| if healthStatus.isErr(): | ||
| warn "Failed to get Waku node health status: ", error = healthStatus.error | ||
| # Let's suppose the node is hesalthy enough, go ahead | ||
| else: | ||
| if healthStatus.get().healthStatus != NodeHealth.Unhealthy: | ||
| return err("Waku node is not healthy, has got no connections.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think is better to make sure the health API is working properly. If not, we are blind.
| let healthStatus = RequestNodeHealth.request(w.brokerCtx) | |
| if healthStatus.isErr(): | |
| warn "Failed to get Waku node health status: ", error = healthStatus.error | |
| # Let's suppose the node is hesalthy enough, go ahead | |
| else: | |
| if healthStatus.get().healthStatus != NodeHealth.Unhealthy: | |
| return err("Waku node is not healthy, has got no connections.") | |
| let healthStatus = RequestNodeHealth.request(w.brokerCtx).valueOr: | |
| return err("could not retrieve node's health: " & $error) | |
| if healthStatus.healthStatus != NodeHealth.Unhealthy: | |
| return err("Waku node is not healthy, has got no connections.") |
btw, this healthStatus.healthStatus seems not super correct :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll address the issue here in #3689
Ivansete-status
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome PR! Thanks so much for it! 🙌
Just adding some nitpicks that I hope you find useful.
|
Basic tests are now working, I consider feature ready as an initial state (more PRs to follow). |
fcecin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Added events and requests for support. Reworked delivery_monitor into a featured devlivery_service, that - supports relay publish and lightpush depending on configuration but with fallback options - if available and configured it utilizes store api to confirm message delivery - emits message delivery events accordingly Notice: There are parts still in WIP and needs review and follow ups. prepare for use in api_example
… but return with result properly
… where appropriate, removed leftover
…auto-subscribe for send api
b98b8dd to
3642646
Compare
|
You can find the image built from this PR at Built from e258e11 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR implements a comprehensive Waku Send API that provides an opinionated, high-level interface for reliably sending messages over the Waku network. The implementation introduces a delivery service architecture with event-driven feedback and multi-protocol fallback mechanisms.
Changes:
- Introduces a new Send API with
MessageEnvelopeabstraction andRequestIdtracking for non-blocking message delivery - Implements DeliveryService with SendService, RecvService, and SubscriptionService for managing message lifecycle
- Replaces observer pattern with EventBroker and RequestBroker systems for decoupled, context-aware event handling
- Adds delivery tracking with three event types: MessagePropagatedEvent, MessageSentEvent, and MessageErrorEvent
- Refactors protocol signatures to remove unused peer parameters from PushMessageHandler
Reviewed changes
Copilot reviewed 58 out of 58 changed files in this pull request and generated 14 comments.
Show a summary per file
| File | Description |
|---|---|
| waku/api/api.nim | New high-level Send API implementation with health checks and delivery task creation |
| waku/api/types.nim | Core types including MessageEnvelope, RequestId, and NodeHealth enumerations |
| waku/api/api_conf.nim | Configuration support for Edge mode and p2pReliability settings |
| waku/events/message_events.nim | Event definitions for message delivery lifecycle tracking |
| waku/events/delivery_events.nim | Filter subscription and delivery feedback event definitions |
| waku/node/delivery_service/send_service/send_service.nim | Main delivery orchestration with retry logic, store validation, and event emission |
| waku/node/delivery_service/send_service/delivery_task.nim | State machine for tracking individual message delivery attempts |
| waku/node/delivery_service/send_service/relay_processor.nim | Relay protocol delivery processor with fallback logic |
| waku/node/delivery_service/send_service/lightpush_processor.nim | Lightpush protocol delivery processor implementation |
| waku/node/delivery_service/recv_service/recv_service.nim | Refactored receive service with event-based subscription handling |
| waku/node/delivery_service/subscription_service.nim | Manages relay/filter subscriptions for content topics |
| waku/node/delivery_service/delivery_service.nim | Top-level delivery service coordinating send/receive operations |
| waku/node/waku_node.nim | Integration of broker context and request providers for shard deduction |
| waku/node/kernel_api/relay.nim | Updated relay API to return peer count and support isSubscribed query |
| waku/factory/waku.nim | Replaced DeliveryMonitor with DeliveryService, added health provider setup |
| waku/common/broker/broker_context.nim | Thread-safe broker context implementation for multi-instance support |
| waku/common/broker/event_broker.nim | Generic event broker macro for type-safe, decoupled event handling |
| waku/common/broker/request_broker.nim | Request/response broker for synchronous cross-module communication |
| waku/requests/rln_requests.nim | RLN proof generation request abstraction |
| waku/requests/health_request.nim | Health status request definitions for node and relay topics |
| waku/requests/node_requests.nim | Node-level requests for relay shard deduction |
| waku/waku_rln_relay/rln_relay.nim | RLN proof generation exposed via request broker |
| waku/waku_relay/protocol.nim | Removed PublishObserver pattern, added health provider, subscribe triggers health update |
| waku/waku_lightpush/client.nim | Refactored sendPushRequest with unified connection handling |
| waku/waku_lightpush/common.nim | Updated PushMessageHandler signature (removed peer parameter) |
| waku/waku_lightpush/protocol.nim | Updated to use new PushMessageHandler signature |
| waku/waku_lightpush/callbacks.nim | Updated handler implementations for signature change |
| waku/waku_lightpush_legacy/* | Legacy lightpush updated for signature compatibility |
| waku/waku_filter_v2/client.nim | Replaced subscription observers with event broker emissions |
| waku/waku_core/message/digest.nim | Added to0xHex helper for message hash logging |
| waku/node/health_monitor/topic_health.nim | Added NOT_SUBSCRIBED state for topic health |
| waku/node/delivery_monitor/* | Removed old delivery monitor implementation |
| examples/api_example/api_example.nim | New example demonstrating send API usage with event listeners |
| tests/api/test_api_send.nim | Comprehensive test suite for send API with multiple scenarios |
| tests/wakunode2/test_app.nim | Updated for new stop() signature returning Result |
| apps/wakunode2/wakunode2.nim | Updated shutdown handlers for Result-based stop |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Ivansete-status
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is looking really great! 🙌
Adding some more nitpick comments.
|
@NagyZoltanPeter - something I forgot to mention is that we need to expose the Send API in |
@Ivansete-status CI: I fixed some more as I found rln and rest relay tests was failing due to my changes. |
That's a great idea indeed! Let's have a new Re
Thanks! 🙌 Ping m if there's anything I can check. |
What about "waku" being retconned as the name of the internals of "logos-messaging" / "lm"? So the current "libwaku" in an internal library by definition. Everything "lm" or "logos-messaging" is the product interface, and everything "waku" is internals (all previous product interfaces named waku are either de-productized or renamed to "lm"). |
Yes, agreed, that's the way. As remember of a discussion with Franck before, keep lmapi simple as is for the first time and if any verifiable demand arise we can add it instead of exposing kernel-api. So that can be good guideline for lmapi future. |
|
nwaku-nwaku-interop-tests. fails due to the Relay publish breaking change as returning error in case no peers found for relaying. |
Notice
This PR is still under work, do expect some additions and fixes.
For completion:
Currently the brokers are thread wise global, but this leads to test failures where we instantiate many Waku node instances.
Description
Waku Send API implementation.
Introduce new, simple
sendinterface atwakulogos-messaging apilevel. That takes a new abstraction callMessageEnvelopwhich embodies content_topic and payload and ephemeral information in client side.The interface returns with a RequestId - unique to each call and enables tracking of message events - on success, or error description.
The operation is non blocking, which means at the moment of call there are very limited what information can be reported to the caller.
Therefore user can expect three events to be emitted upon the operation result.
MessagePropagatedEvent- Message is delivered to the network with less confidence (is relayed or lightpush initiated with success)MessageSentEvent- Message is validated via a store node, ensures that message is on the gossip network.MessageErrorEvent- Message could not be sent, reason is described as string.Former - unused - DeliverManager is reworked completely to fulfill the role of
DeliveryService. Send is supported bySendServicethat managesDeliveryTasks in a state machine manner and takes care of Message event reporting.Breaking change:
Relay publish will return error
NoPeersToPublishinstead of returning OK with 0 peer count!This derives to REST Api use of relay and lightpush also.
Issue