Encapsulate Client functionality with typestate#535
Encapsulate Client functionality with typestate#535rustaceanrob wants to merge 2 commits into2140-dev:masterfrom
Client functionality with typestate#535Conversation
First step in cleaning up the `Node`/`Client` relationship. This abstracts running a `task`, which will require some new methods to run with more flexibility (bring your own runtime or use an OS thread). The panic introduced will be impossible to hit in a later commit where I will introduce sealed traits.
rustaceanrob
force-pushed
the
26-2-24-typestate
branch
from
144d6ef to
ffafc83
Compare
Uses the sealed trait hack to only allow for compiler enforced state transitions. The flow is `Idle -> Subscribed -> Actice`, where a user decides what to do with the events, runs the node, then gets access to all the usual client features. I decided to scrap the `Requester` and instead encapsulate the event receivers with a newtype. The only downside I see with this is `&mut self` doesn't seem to be possible, as each impl block is defined for it's `State` type. I need to fiddle with that to see if we can't remove the need to return the client each state transition.
rustaceanrob
force-pushed
the
26-2-24-typestate
branch
from
ffafc83 to
652b2bc
Compare
There was a problem hiding this comment.
Took me a bit of time to wrap my head around the code because I haven't played with this library directly, but after some poking around I think I understand it well. It's surprising how little change is required here to make this work well for both use cases.
I think maybe a little more documentation on how the Subscribed/Active distinction is useful in practice would be good, in case there is something users might want to do in between transitioning between these two states. I read your line 66 in client.rs but I'm still not sure when I'd use the client in that state.
The exact shape this will take in the bindings I haven't really dug into. We know generics can't be used, so I suspect the pattern will turn into a slightly more "crude" version of itself here, but that's a problem for another repo! Thanks for thinking about #527 and working on a cool solution.
The test has an example where the logging is setup before we actually spin up the node. You can imagine this being potentially more complex (i.e. set up logging to a file on disk). We'd only want to run the node in the case something like that succeeds. I can add some documentation for rationale, probably after this PR, maybe with a code example.
I am imagining the builder makes a "Node" which you can call |
2 participants
Take two of #527, which is more flexible but leaves
Nodeexposed for those that need it.Similar to #534, however I think the
Nodemay ultimately remain in the public interface. This is due to the original reason for separation in the first place, as this crate does not make an assumption of how theNode::runfuture will be executed. For instance, LDK node has a unique way of spawning tasks.I do think the typestate can simplify the use cases for the majority of users though. In the usual case, the user can get a
Clientthat can be subscribed to, ran, and published events to.cc @thunderbiscuit @nyonson