Skip to content

Commit

Permalink
Merge pull request #3442 from arik-so/archive-monitor-persistence-tri…
Browse files Browse the repository at this point in the history
…gger

Persist unresolved ChannelMonitors on empty height change
  • Loading branch information
TheBlueMatt authored Dec 6, 2024
2 parents b96b19a + 6f9300f commit 020be44
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 9 deletions.
11 changes: 8 additions & 3 deletions lightning/src/chain/chainmonitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -640,17 +640,22 @@ where C::Target: chain::Filter,
/// data could be moved to an archive location or removed entirely.
pub fn archive_fully_resolved_channel_monitors(&self) {
let mut have_monitors_to_prune = false;
for (_, monitor_holder) in self.monitors.read().unwrap().iter() {
for (funding_txo, monitor_holder) in self.monitors.read().unwrap().iter() {
let logger = WithChannelMonitor::from(&self.logger, &monitor_holder.monitor, None);
if monitor_holder.monitor.is_fully_resolved(&logger) {
let (is_fully_resolved, needs_persistence) = monitor_holder.monitor.check_and_update_full_resolution_status(&logger);
if is_fully_resolved {
have_monitors_to_prune = true;
}
if needs_persistence {
self.persister.update_persisted_channel(*funding_txo, None, &monitor_holder.monitor);
}
}
if have_monitors_to_prune {
let mut monitors = self.monitors.write().unwrap();
monitors.retain(|funding_txo, monitor_holder| {
let logger = WithChannelMonitor::from(&self.logger, &monitor_holder.monitor, None);
if monitor_holder.monitor.is_fully_resolved(&logger) {
let (is_fully_resolved, _) = monitor_holder.monitor.check_and_update_full_resolution_status(&logger);
if is_fully_resolved {
log_info!(logger,
"Archiving fully resolved ChannelMonitor for funding txo {}",
funding_txo
Expand Down
17 changes: 11 additions & 6 deletions lightning/src/chain/channelmonitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1994,10 +1994,15 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {

/// Checks if the monitor is fully resolved. Resolved monitor is one that has claimed all of
/// its outputs and balances (i.e. [`Self::get_claimable_balances`] returns an empty set).
/// Additionally may update state to track when the balances set became empty.
///
/// This function returns true only if [`Self::get_claimable_balances`] has been empty for at least
/// This function returns a tuple of two booleans, the first indicating whether the monitor is
/// fully resolved, and the second whether the monitor needs persistence to ensure it is
/// reliably marked as resolved within 4032 blocks.
///
/// The first boolean is true only if [`Self::get_claimable_balances`] has been empty for at least
/// 4032 blocks as an additional protection against any bugs resulting in spuriously empty balance sets.
pub fn is_fully_resolved<L: Logger>(&self, logger: &L) -> bool {
pub fn check_and_update_full_resolution_status<L: Logger>(&self, logger: &L) -> (bool, bool) {
let mut is_all_funds_claimed = self.get_claimable_balances().is_empty();
let current_height = self.current_best_block().height;
let mut inner = self.inner.lock().unwrap();
Expand All @@ -2011,7 +2016,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
match (inner.balances_empty_height, is_all_funds_claimed) {
(Some(balances_empty_height), true) => {
// Claimed all funds, check if reached the blocks threshold.
current_height >= balances_empty_height + BLOCKS_THRESHOLD
(current_height >= balances_empty_height + BLOCKS_THRESHOLD, false)
},
(Some(_), false) => {
// previously assumed we claimed all funds, but we have new funds to claim.
Expand All @@ -2021,7 +2026,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
"WARNING: LDK thought it was done claiming all the available funds in the ChannelMonitor for channel {}, but later decided it had more to claim. This is potentially an important bug in LDK, please report it at https://github.com/lightningdevkit/rust-lightning/issues/new",
inner.get_funding_txo().0);
inner.balances_empty_height = None;
false
(false, true)
},
(None, true) => {
// Claimed all funds but `balances_empty_height` is None. It is set to the
Expand All @@ -2030,11 +2035,11 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
"ChannelMonitor funded at {} is now fully resolved. It will become archivable in {} blocks",
inner.get_funding_txo().0, BLOCKS_THRESHOLD);
inner.balances_empty_height = Some(current_height);
false
(false, true)
},
(None, false) => {
// Have funds to claim.
false
(false, false)
},
}
}
Expand Down

0 comments on commit 020be44

Please sign in to comment.