Remove Node from public interface#534
Draft
rustaceanrob wants to merge 4 commits into2140-dev:masterfrom
Draft
Conversation
rustaceanrob
force-pushed
the
26-2-18-encapsulate-node
branch
2 times, most recently
from
24fe41e to
606976a
Compare
Collaborator
Is this the typestate pattern? |
Collaborator
Author
|
Ah yup that's the one |
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.
This is particularly useful in the native language bindings case. ref: https://github.com/bitcoindevkit/bdk-ffi/blob/master/bdk-ffi/src/kyoto.rs#L68
rustaceanrob
force-pushed
the
26-2-18-encapsulate-node
branch
from
606976a to
dfe64ef
Compare
Uses the sealed trait hack to only allow for compiler enforced state transactions. There is first a `run` step, followed by the typical methods a client would expect. I decided to scrap the `Requester` and instead encapsulate the event receivers with a newtype. Running a client gives access to the event receivers as well as the handle to the task. 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-18-encapsulate-node
branch
from
dfe64ef to
373ecc5
Compare
Collaborator
Author
|
After poking around potential dependent crates I think this needs a bit more work, particularly to work with LDK node |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #527
This addresses the long standing architecture question of why there was a
NodeandClientseparation. Before I learned about using this sealed trait technique (not sure if it has a name), I didn't know how to limit the user to callruna single time. This PR introduces guided state-transitions for the client, with eachimplbody having different methods available. WhenIdle, the user can call one ofrun,run_detached,run_with_runtime, which covers all the ways I know of in terms of how to spawn a node process withtokio. The client then has all of the available methods the user might expect, i.e broadcast a transaction or shutdown the node. I decided to also remove theRequestertype, since this design allowsClientto implement that functionality. The only downside I am seeing is theClientmust be returned for each state transition. Using&mut selfand changing the state is invalid AFICT.Tried to make the commits somewhat atomic here, but just a high level review is cool, a.k.a just looking at the API updates in the examples and getting a gut-feel. @thunderbiscuit
cc @nyonson if you wanna poke around the changes