Allow getting multiple Receiver
#52
Conversation
rustaceanrob
force-pushed
the
client-09-21
branch
from
93859bf
to
42795e0
Compare
src/lib.rs
Outdated
| /// When generating a new channel [`Receiver`], a message must be consumed by _all_ [`Receiver`] before it is removed from memory. | ||
| /// For channels that process data slowly, this can cause messages to build up in the channel, and may even cause errors if too many messages | ||
| /// are held in the channel at one time. For the best results, applications may not want to call this method at all, or only a few times. | ||
| pub fn channel_receiver(&self) -> Receiver<NodeMessage> { | ||
| self.client.receiver() |
There was a problem hiding this comment.
This all looks good to me. My one formatting nit would be to wrap the comments at 100 columns.
I tested this briefly in tests/client.rs by getting a new Receiver and saw all the node messages come through. I imagine this is useful for somebody interested in getting node messages with a "no frills" interface (I guess that was already possible when it returned &mut Receiver?). You mentioned lightning as a motivating factor, is there a specific pain point you're referring to? because it seems like a subtle change to me but I'm still de-crusting the intricacies of channels.
There was a problem hiding this comment.
conceptually the message channel is just a block of memory in the stack that will hold messages that threads can share. for the broadcast channel, the messages will stick around on the stack (or their pointers will stick around) until all of the receivers have gotten the message.
i was thinking the LN context will use the Client by building a Node directly, but i am not so sure that is the case anymore. after looking through ldk-node i see they just use Wallet. with out current API, if a user chooses to use the builder, they won't be able to call update and get a &mut Receiver simultaneously, at least to my knowledge. in the ldk-node case, they might want to do this since they are working with Wallet, but will still need to look at blocks for revocation transactions
on style, i believe the default is 80, so i'll add a .rustfmt.toml file with that configured?
rustaceanrob
force-pushed
the
client-09-21
branch
from
42795e0
to
f97514e
Compare
|
i had struggles with the |
|
Appreciate you @rustaceanrob ! |
i realized that only allowing
&mut Receiver<NodeMessage>may be a bit too limiting in the LN context. there is no API limitation preventing multipleReceiver<NodeMessage>out at one time, so i go ahead with that herecc @ValuedMammal