sidecar channels 2/3: non-functional preparations for sidecar channels #242
Conversation
This file contains 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
With this commit we add a first description and HOWTO about what sidecar channels are and how they can be created. We add this commit first to give any reviewer the full context about what we're trying to achieve first.
For proper and safe channel acceptance the recipient node also needs to know which key index it used for a ticket's multisig key so it can sign the authentication flow with the auctioneer correctly (which takes a key locator and not key descriptor). To make the offer and sidecar contract more explicit, we also include the lease duration in blocks in the offer part. This field will overwrite any value set when using the CLI to submit the bid order.
Both the trader offering to buy a sidecar channel for another node as well as the recipient node will need to store the involved sidecar tickets in their local databases. We add a simple store interface and its implementation that will be used in future commits.
For maximum re-usability of the signing/verifying code, we add them as helper methods to the sidecar package itself.
With this commit we add the optional sidecar ticket to bid orders. Because the node submitting the bid order doesn't necessarily need to be the node creating the offer, we just store the ticket as is and don't require it to be in the database already.
The name SubscribeAccountUpdates could indicate that this is a long-running operation. The method returns after successful authentication of the stream though. That's why we rename it to StartAccountSubscription and update the comment to make it more clear that this should return after a few seconds at the most.
The channel funding process for a sidecar channel will look different from the normal process. To avoid too much code duplication, we first extract the parts that remain the same into its own method. We no longer need to hold a mutex since when we get to the waitForChannelOpen method all previously spawned goroutines must have completed.
To prepare for accepting sidecar channels where we don't have a real order item in our local database, we use the order fetcher function in the funding manager instead of the DB directly. That way we can pass in fake bid orders during the sidecar acceptance.
As a preparation to reuse that code in the sidecar acceptor, we extract it into its own function.
As a preparation to add non-exported fields to the funding manager, we want to create a NewManager function that takes a config instead of instantiating the manager directly.
As a preparation to be able to serve the pending channel updates from lnd to multiple internal subscribers, we move the handling of those events to the funding manager itself.
As a preparation for allowing multiple managers to consume the pending channel updates, we move those events from a single channel to a subscribe.Server construct. We also pass in the subscription client to use into waitForChannelOpen so we can use a different one in different flows.
This was referenced Apr 2, 2021
Roasbeef
approved these changes
Apr 6, 2021
| // for every account as soon as it's confirmed open. This method will | ||
| // return as soon as the authentication was successful. Messages sent | ||
| // from the server can then be received on the FromServerChan channel. | ||
| StartAccountSubscription(context.Context, *keychain.KeyDescriptor) error |
positiveblue
pushed a commit
to positiveblue/pool
that referenced
this pull request
Oct 11, 2022
Hotfix reserved value
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
I realized that having a large PR that cannot be merged after review because of dependencies against the server is not ideal.
That's why I extracted all refactoring and other non-functional commits from #222 and #235 into this PR.
Because this adds no new user-facing functionality, the PR can be merged after successful review. Most of the commits have already been at least partially reviewed in #222.
I hope this makes the review flow a bit easier, even if it's done at a very late stage. If this makes it even more complicated, I apologize to the reviewers!