In lightningdevkit/ldk-node#307 (comment) @G8XSU observed that our current implementation of ChainMonitor::archive_fully_resolved_channel_monitors isn't necessarily suited for mobile use as we currently don't trigger persist after initially setting balances_empty_height in ChannelMonitor:
|
inner.balances_empty_height = Some(current_height); |
This means that it will be lost upon restart if we don't happen to re-persist it for some extraneous reasons, which will likely be the case on mobile where we may be killed as soon as users quit the app. As we furthermore currently try to reduce/remove the number of arbitrary/superfluous persists in general (and hence hopefully might end up in a place where a fully-resolved monitor is ~never re-persisted anymore), going forward this might even be the case for any application that restarts before archive_fully_resolved_channel_monitors is called again after the four-week BLOCKS_THRESHOLD is reached.
When balances_empty_height is lost, we'll restart the BLOCKS_THRESHOLD, which likely means we'd never actually come around to archiving channel monitors on short-lived (four weeks isn't that short actually) instances such as mobile.
I think to mitigate this, we should simply trigger ChannelMonitor persistence once after we set balances_empty_height.
In lightningdevkit/ldk-node#307 (comment) @G8XSU observed that our current implementation of
ChainMonitor::archive_fully_resolved_channel_monitorsisn't necessarily suited for mobile use as we currently don't trigger persist after initially settingbalances_empty_heightinChannelMonitor:rust-lightning/lightning/src/chain/channelmonitor.rs
Line 1906 in cc9fc65
This means that it will be lost upon restart if we don't happen to re-persist it for some extraneous reasons, which will likely be the case on mobile where we may be killed as soon as users quit the app. As we furthermore currently try to reduce/remove the number of arbitrary/superfluous persists in general (and hence hopefully might end up in a place where a fully-resolved monitor is ~never re-persisted anymore), going forward this might even be the case for any application that restarts before
archive_fully_resolved_channel_monitorsis called again after the four-weekBLOCKS_THRESHOLDis reached.When
balances_empty_heightis lost, we'll restart theBLOCKS_THRESHOLD, which likely means we'd never actually come around to archiving channel monitors on short-lived (four weeks isn't that short actually) instances such as mobile.I think to mitigate this, we should simply trigger
ChannelMonitorpersistence once after we setbalances_empty_height.