Add event kind filtering to event subscriptions - #300
benthecarman wants to merge 3 commits into
Conversation
|
I've assigned @TheBlueMatt as a reviewer! |
2d4520e to
b3cebfa
Compare
b3cebfa to
f8f53f5
Compare
Move the SubscribeEvents streaming loop out of the request dispatch match into its own function so it can be reused by additional event streaming RPCs. No behavior change. Co-Authored-By: Claude Opus 5.5 <[email protected]>
f8f53f5 to
ac49ed5
Compare
f3r10
left a comment
f3r10
left a comment
There was a problem hiding this comment.
LGTM. Just one comment, about a new event variant only showing up on SubscribeEvents.
SubscribeEvents delivers every event, so clients that only care about one kind of event have to receive and discard everything else. Add SubscribeChannelEvents, SubscribePaymentEvents, and SubscribeForwardingEvents RPCs, which stream only the matching subset of events. SubscribeEvents is unchanged. Channel events are ChannelStateChanged, SpliceNegotiated, and SpliceNegotiationFailed. Payment events are PaymentReceived, PaymentSuccessful, PaymentFailed, and PaymentClaimable. Forwarding events are PaymentForwarded; they get their own stream because they are the highest-volume event on a routing node and are not this node's own payments. Based on an earlier contribution that added an event kind filter to SubscribeEventsRequest; reworked into separate RPCs per review. Co-authored-by: Ekong Jemimah <[email protected]> Co-Authored-By: Claude Opus 5.5 <[email protected]>
pay --wait only looks at PaymentSuccessful and PaymentFailed events, so subscribe to payment events rather than every server event. Co-Authored-By: Claude Opus 5.5 <[email protected]>
ac49ed5 to
a145008
Compare
| continue; | ||
| } | ||
| let frame = encode_grpc_frame(&event.encode_to_vec()); | ||
| if tx.send(Ok(frame)).await.is_err() { |
There was a problem hiding this comment.
Just for my understanding, is the reason for using tx.send().await here over try_send() because we want to ensure reliability for clients? I understand this can be blocking as it waits until there's capacity before sending the event, and keeps waiting if the capacity is full. I'm wondering if this waiting could pause the loop from checking other branch of the code, like shutdown for example?
There was a problem hiding this comment.
Yeah it's intentional. With send().await a slow client gets the 64 event buffer plus the 1024 broadcast buffer before we start dropping events, try_send() would start dropping at 64.
It does block the other branches while waiting, but if the client disconnects the pending send() errors so the task still exits. Shutdown doesn't wait on these tasks either so a stuck client can't hang it. This is also how SubscribeEvents already worked before this PR
5 participants
Recreated #232
Instead of needing a global subscription for all events and requiring the user to filter themselves, we can instead have separate subs for different kinds. For now we add ones for channel events, payments, and forwards.
This allowed for one small clean up in the
pay --waitcli