Skip to content
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

Improve docs for Persist::archive_persisted_channel #3122

Open
tnull opened this issue Jun 13, 2024 · 0 comments
Open

Improve docs for Persist::archive_persisted_channel #3122

tnull opened this issue Jun 13, 2024 · 0 comments
Labels
Take a Friday Leave a Friday Stomp the Bugs, Without Much Commitment

Comments

@tnull
Copy link
Contributor

tnull commented Jun 13, 2024

In our current implementation of Persist::archive_persisted_channel we first read the monitor, write it at the archive location and finally lazily remove the monitor to 'move' it to the archive. This is fine, except that a lazy remove isn't guaranteed to finish if we encounter a persistence failure or happen to restart. This means that we could end up in a situation with the channel monitor at both, the original and the archive location until the call to archive_fully_resolved_channel_monitors succeeds once.

All of this is fine, but I think we should clarify the docs:

  1. Currently the Persist::archive_persisted_channel docs state "Prevents the channel monitor from being loaded on startup.". We might want to clarify that this is not true if above mentioned scenario occurs.
  2. We should give users guidance how to handle this case, i.e., which monitor a user might want to restore from if they need it.
  3. We should mention that any custom implementations of Persist::archive_persisted_channel need to be idempotent, i.e., need to be able to be run multiple times without erroring if the monitor is already present at the archive location.
@tnull tnull changed the title Improve docs for ChainMonitor::archive_fully_resolved_channel_monitors Improve docs for Persist::archive_persisted_channel Jun 13, 2024
@TheBlueMatt TheBlueMatt added the Take a Friday Leave a Friday Stomp the Bugs, Without Much Commitment label Jun 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Take a Friday Leave a Friday Stomp the Bugs, Without Much Commitment
Projects
None yet
Development

No branches or pull requests

2 participants