-
Notifications
You must be signed in to change notification settings - Fork 2
fix: delete vss network graph using new ffi client #775
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
ovitrif
wants to merge
9
commits into
fix/node-stopping-bg-payments
Choose a base branch
from
fix/stale-graph-reset
base: fix/node-stopping-bg-payments
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+390
−217
Conversation
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
2 tasks
82a5527 to
ba44df3
Compare
ba44df3 to
f120581
Compare
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
3dfa34b to
e556b04
Compare
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
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.
Closes #765
This PR:
Description
Builds on #740. The stale graph auto-recovery added in #765 attempted to deleted the graph from VSS using the app-level backup client but due to different key obfuscation namespacing in ldk-node, the graph backup was never deleted.
This PR switches to a dedicated LDK-specific FFI client implementation that points operations to the correct obfuscated file names backed up by ldk-node on VSS. This new VSS client implementation adjusted for supporting OPs on ldk-node backups is found in PR:
In practice this translate to successful LN payments to nodes that would otherwise fail, one example being payments to Blink wallet if having a stale graph in your wallet's VSS backups.
LDK-node already has the required mechanism to trigger redownload of the network graph if attempts to retrieve it from its backup on VSS fails. The cleanup mechanism is triggered when the graph is found to miss any of the expected trusted peers, which in turn triggers ldk-node to kick-off its re-download of the graph from the latest RGS server snapshot.
Preview
Test 1️⃣ - Payment to Blink
pay2blink.mp4
Test 2️⃣ - VSS app data preserved across
vss-rust-clientupdate0.4.0)0.5.5)QA Notes
Tip
No need to go all the way to replicate a stale graph on your VSS. To validate, simply test on mainnet some random LN payments to a couple wallets you have on other apps like Blink, Phoenix, WoS. If payments succeed, PR is good to be merged.
1. Verify stale graph reset clears VSS via FFI client
Network graph is stale, resetting and restarting...Cleared stale network graph from VSS (first delete)2. Verify VSS app data preserved across versions
master)