Skip to content

Conversation

@pwltr
Copy link
Contributor

@pwltr pwltr commented Oct 28, 2025

Reverts #286

After more discussion I am reverting this, persisting locally first without ensuring remote backup succeeds has the same amount of risk or more than the previous logic, in the sense that remote and local state become out of sync. Restoring from backup subsequently can cause loss of funds.

@ovitrif
Copy link
Contributor

ovitrif commented Oct 28, 2025

Reverts #286

After more discussion I am reverting this, persisting locally first without ensuring remote backup succeeds has the same amount of risk or more than the previous logic, in the sense that remote and local state become out of sync. Restoring from backup subsequently can cause loss of funds.

Makes sense to some extent :)
I was thinking that it still unlocks a use-case, but it's all hypothetical:
what if restoring never happens from local but local backup could help confirm some info about the funds, for example to justify a refund… 🤷🏻

Still, it's all just a wild guess, and I don't really know if it's a real possiblility.

@pwltr
Copy link
Contributor Author

pwltr commented Oct 28, 2025

what if restoring never happens from local but local backup could help confirm some info about the funds, for example to justify a refund… 🤷🏻

IIUC you are suggesting we persist locally always but not in a directory that LDK would actually use, for debugging purposes? It's a nice idea. The issue we need to solve only really exists for new channels (isNew), because the push balance is not secured by LDK safety measures. So we could add extra logic just for new channels to always write the channel monitor locally, even if remote backup fails, even if that opens up that other edge case of having two states out of sync. But better then not having any backup at all.

@pwltr pwltr merged commit 3f0fec1 into master Oct 28, 2025
1 of 6 checks passed
@pwltr pwltr deleted the revert-286-fix/backup-client branch October 28, 2025 15:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants