multi: allow remote deletion of local payment attempt results #9289
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
Allow node configuration to specify whether to treat HTLC attempts in the normal way or as being tracked by a remote entity. This has the practical effect of disabling the automatic cleanup of network result state within the Switch. Instead, state deletion must be completed via RPC. This is an all or nothing designation until we can determine whether it can be set on a per-HTLC attempt basis.
The SwitchRPC server will be hidden behind a build tag.
This will faciliate the coordinated deletion of results from local payment attempts in scenarios in which the ChannelRouter runs remotely from the Switch.
Add FetchAttemptResults and DeleteAttemptResult to harness.
|
Important Review skippedAuto reviews are limited to specific labels. 🏷️ Labels to auto review (1)
Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Change Description
The (lnd) ChannelRouter cleans up the results store maintained by the HTLCSwitch when it restarts via the CleanStore method. Any network results stored for attempts that are no longer
IN-FLIGHTare removed from disk. This presents some difficulty when attempting to deploy the ChannelRouter in a separate process from the HTLCSwitch and allowing communication via RPC. Specifically, we may encounter the following scenario:The issue is that we're relying on state kept by an external entity (lnd) to drive state transitions about payment status within the ChannelRouter and that external entity has automated clean up of that state (at least with the way SendOnion RPC works now). There is a slippery scenario in which the lnd instance hits the logic to automate the cleanup of state needed by ChannelRouter to track the payment to completion before the ChannelRouter has actually read it.
In the same way that lnd does not run CleanStore while payments are being processed, a remote ChannelRouter cannot allow lnd to run CleanStore while payments are being processed. There must be synchronization between ChannelRouter and lnd with respect to the maintenance of state in this store.
Solution
For this we can allow lnd to disable automatic cleanup of the Switch result store and add a switchrpc sub-server which supports remote deletion from this store. Then the remote ChannelRouter can clean the result stores of its switches when it restarts by providing an implementation of CleanStore which leverages these RPCs.
NOTE: This has an advantage over adding a "tracked remotely" flag to network results and skipping the deletion of any result with such a flag in that I think that requires a DB migration, where-as suspending automated local deletion in favor of remote deletion via RPC does not.
Will probably relocate SendOnion/TrackOnion rpcs to switchrpc sub-server.
Steps to Test
make itest icase=switch_store_rpcQuestions
toKeepstyle map which specifies the list of results to keep since the list of in-flight payments is likely to be shorter than the list of completed payments. This would mean that, instead of multiple calls to DeleteAttemptResult(s), a single call would be delete multiple results and could probably be named CleanResultStore.