lightningdevkit / rust-lightning

A highly modular Bitcoin Lightning library written in Rust. It's rust-lightning, not Rusty's Lightning!
Other
1.15k stars 363 forks source link

Improve docs for `Persist::archive_persisted_channel` #3122

Open tnull opened 3 months ago

tnull commented 3 months ago

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.